search

wekan / ldap

LDAP support for Wekan code has been moved to https://github.com/wekan/wekan/tree/master/packages/wekan-ldap , issues to https://github.com/wekan/wekan/issues , and if PRs are needed please add them instead to https://github.com/wekan/wekan/pulls
https://github.com/wekan/wekan/tree/master/packages/wekan-ldap
MIT License
12 stars 10 forks source link

No verification when merging accounts #36

Closed stevenpwaters closed 5 years ago

stevenpwaters commented 5 years ago

Hi,

Problem

We're attempting to implement LDAP for our existing standalone Wekan server. Existing users have been able to create their own usernames, which almost invariably don't match their LDAP usernames.

When Wekan detects a match between LDAP username and an existing account's username, and merge existing accounts is set to true, Wekan will login the LDAP user to the matched account. There's no verification step to check that the Wekan account truly belongs to the LDAP user (e.g. a request for Wekan user's password) before committing the merge.

This is problematic as a user may have already created an account with a username that matches another user's LDAP username (improbable for most but nonetheless possible). When the LDAP user goes to login, Wekan will detect an existing account which does not belong to the LDAP user and grant access, essentially commandeering the account.

While it doesn't apply to us, other organisations may allow users to change their LDAP usernames. This would allow exploitation in a more targeted manner to take control of accounts.

Solutions

Probably the most intuitive method would be to prompt the Wekan user for their existing password before committing the account merge.

A cheaper method could be to verify the match by LDAP e-mail address, i.e. confirm the matched Wekan user has a verified e-mail address that matches the LDAP user.

I'll put together a PR for the second option when I get a chance.

stevenpwaters commented 5 years ago

Second option introduced with #39