[pve-devel] [PATCH access-control 6/9] Auth/LDAP: add necessary options for syncing

[Dominik Csapak]

On 3/9/20 11:43 AM, Fabian Grünbichler wrote:
> On March 6, 2020 11:05 am, Dominik Csapak wrote:
>> for syncing users/groups from ldap, we need some more options
>> so that the users can adapt it to their LDAP setup, which are very
>> different accross systems.
>>
>> sensible defaults are documented
>>
>> Signed-off-by: Dominik Csapak <d.csapak at proxmox.com>
>> ---
>>   PVE/Auth/LDAP.pm | 58 ++++++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 58 insertions(+)
>>
>> diff --git a/PVE/Auth/LDAP.pm b/PVE/Auth/LDAP.pm
>> index 5eef12c..6047dfb 100755
>> --- a/PVE/Auth/LDAP.pm
>> +++ b/PVE/Auth/LDAP.pm
>> @@ -57,6 +57,57 @@ sub properties {
>>   	    type => 'string',
>>   	    optional => 1,
>>   	},
>> +	filter => {
>> +	    description => "LDAP filter for user sync.",
>> +	    type => 'string',
>> +	    optional => 1,
>> +	    maxLength => 256,
> 
> this might benefit from a longer maxLength (filters could be complicated
> expressions)

ok

> 
>> +	},
>> +	sync_attributes => {
>> +	    description => "Comma seperated list of key=value pairs for ".
> 
> s/seperated/separated/
> 
>> +			   "selecting which ldap fields sync which user fields.".
> 
> this is a bit hard to read. maybe:
> 
> specifying which LDAP attributes map to which PVE user field.

yes, sounds better...


> 
>> +			   " By default, the ldap attribute name is the field name.".
> 
> By default, each PVE user field is represented by an LDAP attribute of
> the same name.
> 
>> +			   " If an attribute is not found, a sensible default is used.",
> 
> s/default/default value/
> 
> s/ldap/LDAP/ in general for all documentation ;)
> 
>> +	    optional => 1,
>> +	    type => 'string',
>> +	    pattern => '\w+=[^,]+(,\s*\w+=[^,]+)*',
>> +	},
>> +	user_classes => {
>> +	    description => "The objectclasses for users.",
>> +	    type => 'string',
>> +	    default => 'inetorgperson, posixaccount, person, user',
>> +	    format => 'ldap-simple-attr-list',
>> +	    optional => 1,
>> +	},
>> +	group_dn => {
>> +	    description => "LDAP base domain name for group sync. ".
>> +			   "If not given, the base_dn will be used.",
>> +	    type => 'string',
>> +	    pattern => '\w+=[^,]+(,\s*\w+=[^,]+)*',
>> +	    optional => 1,
>> +	    maxLength => 256,
>> +	},
>> +	group_attr => {
>> +	    description => "LDAP group attribute for its name. If not given or ".
>> +			   "found, the first value of the DN will be used as name.",
> 
> LDAP attribute representing a group's name.

OK, will also rename it to group_name_attr as Thomas suggested

> 
>> +	    type => 'string',
>> +	    format => 'ldap-simple-attr',
>> +	    optional => 1,
>> +	    maxLength => 256,
>> +	},
>> +	group_filter => {
>> +	    description => "LDAP filter for group sync.",
>> +	    type => 'string',
>> +	    optional => 1,
>> +	    maxLength => 256,
> 
> same as above

OK

> 
>> +	},
>> +	group_classes => {
>> +	    description => "The objectclasses for groups.",
>> +	    type => 'string',
>> +	    default => 'groupOfNames, group, univentionGroup, ipausergroup',
>> +	    format => 'ldap-simple-attr-list',
>> +	    optional => 1,
>> +	},
>>       };
>>   }
>>   
>> @@ -77,6 +128,13 @@ sub options {
>>   	capath => { optional => 1 },
>>   	cert => { optional => 1 },
>>   	certkey => { optional => 1 },
>> +	filter => { optional => 1 },
>> +	sync_attributes => { optional => 1 },
>> +	user_classes => { optional => 1 },
>> +	group_dn => { optional => 1 },
>> +	group_attr => { optional => 1 },
>> +	group_filter => { optional => 1 },
>> +	group_classes => { optional => 1 },
>>       };
>>   }
>>   
>> -- 
>> 2.20.1
>>
>>
>> _______________________________________________
>> pve-devel mailing list
>> pve-devel at pve.proxmox.com
>> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>>
>>
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel at pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>