[pve-devel] [PATCH access-control v2] auth ldap/ad: make password a parameter for the api

[Dominik Csapak]

On 4/7/20 5:02 PM, Thomas Lamprecht wrote:
> On 4/7/20 1:11 PM, Dominik Csapak wrote:
>> Instead of simply requiring it to exist in /etc/pve.
>>
>> Takes after the password handling of CIFS in pve-storage.
>>
>> Signed-off-by: Dominik Csapak <d.csapak at proxmox.com>
>> ---
>> changes from v1:
>> * delete pw when given via 'delete' parameter
>> * do not delete pw when updating without giving 'password' parameter
> 
> 
> 
>> diff --git a/PVE/Auth/LDAP.pm b/PVE/Auth/LDAP.pm
>> index 905cc47..1b2c606 100755
>> --- a/PVE/Auth/LDAP.pm
>> +++ b/PVE/Auth/LDAP.pm
>> @@ -37,6 +37,11 @@ sub properties {
>>   	    optional => 1,
>>   	    maxLength => 256,
>>   	},
>> +	password => {
>> +	    description => "LDAP bind password. Will be stored in '/etc/pve/priv/ldap/<REALM>.pw'.",
>> +	    type => 'string',
>> +	    optional => 1,
>> +	},
>>   	verify => {
>>   	    description => "Verify the server's SSL certificate",
>>   	    type => 'boolean',
> 
>> @@ -185,7 +191,7 @@ sub connect_and_bind {
>>   
>>       if ($config->{bind_dn}) {
>>   	$bind_dn = $config->{bind_dn};
>> -	$bind_pass = PVE::Tools::file_read_firstline("/etc/pve/priv/ldap/${realm}.pw");
>> +	$bind_pass = ldap_get_credentials($realm);
>>   	die "missing password for realm $realm\n" if !defined($bind_pass);
>>       }
>>   
>> @@ -343,4 +349,69 @@ sub authenticate_user {
>>       return 1;
>>   }
>>   
>> +my $ldap_pw_dir = "/etc/pve/priv/ldap";
>> +
>> +sub ldap_cred_filename {
>> +    my ($realm) = @_;
>> +    return "${ldap_pw_dir}/${realm}.pw";
>> +}
>> +
> 
> 
> looks mostly ok from a quick whiff, albeit I'd like to have the .pw
> file in a priv/realm/ directory, ldap is "wrong" here, we also use
> priv/storage/ as base directory for CIFS and PBS, not priv/cifs and
> priv/pbs .. >
yeah you're right, just wanted to maintain backwards compatibility,
but we can just do it like in storage, where we look in the new place
fall back to the old place (which we can/should remove in 7.0)

i'll send a v2