vaultwarden_ldap icon indicating copy to clipboard operation
vaultwarden_ldap copied to clipboard

Disable users that vanished from LDAP

Open mvalois opened this issue 2 years ago • 7 comments

Vaultwarden_ldap should be able to detect users that have been removed or disabled in LDAP and then disable the account on vaultwarden. There should be a settings to enable this or not.

mvalois avatar Mar 19 '22 13:03 mvalois

This should be possible with the admin API. I think that if it were to be implemented, it would need to be defaulted off because otherwise people who also add users via other means would find their accounts disabled.

I’d accept a PR for this if someone wants to implement it, but I’ve got a lot on my plate and won’t be writing it myself any time soon.

ViViDboarder avatar Mar 19 '22 17:03 ViViDboarder

I have thought about the same thing yesterday. Here are 2 things i came up with. A) Have a flag in config if a hard-delete is allowed. This way we can directly delete the users from vw if they are missing in ldap. This flag should obviously default to false, because many people will add vw accounts by other means (manually). By doing it this way it will be less work for smaller companies who have their source of truth in ldap only anyway.

B) Have "obsolete" or "overflow" users (those on vw but not on ldap), be listed in the admin portal as you mentioned.

I think A) is implemented way easier and faster, i can try looking at it but since i am new to rust, it may take a while. But since it is what i need, i could try. Are you interested in such a feature?

therealchfkch avatar Mar 24 '22 06:03 therealchfkch

Maybe there should be a status indicating if an user has been manually created or using vaultwarden_ldap. Then, an user is removed/disabled only if it is missing from LDAP and has been automatically created.

The A) feature seems to be interesting.

mvalois avatar Mar 24 '22 08:03 mvalois

I am not sure how this indicator work, since i think all we have in vw is email address and no information about ldap, correct? Because it would technically be possible to have a user manually added with the ldap email address. Then the sync wouldn't be able to distinguish. At least that is what i could read out of the code.

therealchfkch avatar Mar 24 '22 10:03 therealchfkch

Yes. Something like A is what I was expecting that allows you to opt into disabling users.

Any other implementation where a value gets stored with a user to indicate their source would not be possible without an upstream change to Vaultwarden. I'm not sure that that it would be accepted there either since they generally try to adhere to the Bitwarden APIs as much as possible.

ViViDboarder avatar Mar 24 '22 17:03 ViViDboarder

I think the only way to do it without messing up vaultwarden is to keep track of the users in a persistent storage (sqlite/postgres or something). That would add a whole new layer to this project and i am not sure if can be handles without too much hassle.

Plus i don't have experience in Rust modules for that, but the storage/sql part is not that hard.

therealchfkch avatar Mar 24 '22 18:03 therealchfkch

Just for the sake of sanity, the LDAP created accounts should be marked "VaultwardenLDAP" via the already existing external identifier when created by vaultwarden_ldap.

That way, when disabling an AD account, it would only disable the corresponding vaulwarden_ldap account if it has the "VaultwardenLDAP" identifier set.

deajan avatar May 19 '23 12:05 deajan