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

[Dominik Csapak]

On 4/18/20 7:03 PM, Thomas Lamprecht wrote:
> On 4/8/20 9:00 AM, 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:
>> * use 'realm' dir instead of 'ldap' (with fallback and comment to removal)
>>
> 
> applied, but I split it as adding the on_{add,update,delete}_hook infrastructure
> has to be it's own commit, it has per se nothing to do with the ldap password
> stuff.
> 
> Also, while copy+adapting this from pve-storage is really nice IMO, I do not
> understand at all why you left out or changed things then unnecessarily - I mean
> missing documentation about how the hooks behave, some really strange $check
> parameter (arbitrary non-saying name) which is passed by all callers with a
> "FIXME remove in 7.0" instead of the single place it should have be addressed.
> 
> That's taking code, boiling and whirling it to get Spaghetti outta it, and
> that should be kept to kitchens :)
> 
> Anyway, this was long overdue, so thanks for providing the inertia and patch
> to address it.

hi,

yeah. the check parameter was this way because i first ported over
without the change of the directory, and then fixed it differently
than in pve-storage, ofc the docs were missing either way

anyway thanks for fixing it :)