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

Existing accounts cannot be merged when the email address obtained from LDAP has uppercase characters. #81

Open njpaul opened 4 years ago

njpaul commented 4 years ago

I've been asked to integrate Wekan with our company's Active Directory. I'm unable to merge existing users.

The error I am getting is

wekan_1    | [DEBUG] Identifying user with: objectGUID
wekan_1    | [INFO] Querying user
wekan_1    | [DEBUG] userQuery {
wekan_1    |   "services.ldap.id": "da4a9f103466e34baa9e40bd34280c29"
wekan_1    | }
wekan_1    | [DEBUG] userQuery {
wekan_1    |   "_id": "last.f",
wekan_1    |   "emails.0.address": "F.Last@xxxxxxx.com"
wekan_1    | }
wekan_1    | [INFO] No user exists with username "last.f"
wekan_1    | [DEBUG] userQuery {
wekan_1    |   "emails.0.address": "F.Last@xxxxxxx.com"
wekan_1    | }
wekan_1    | [INFO] User does not exist, creating "last.f"
wekan_1    | [DEBUG] Identifying user with: objectGUID
wekan_1    | [DEBUG] Identifying user with: objectGUID
wekan_1    | [DEBUG] New user data {
wekan_1    |   "username": "last.f",
wekan_1    |   "email": "F.Last@xxxxxxx.com"
wekan_1    | }
wekan_1    | [ERROR] Error creating user {
wekan_1    |   "isClientSafe": true,
wekan_1    |   "error": 403,
wekan_1    |   "reason": "Email already exists.",
wekan_1    |   "message": "Email already exists. [403]",
wekan_1    |   "errorType": "Meteor.Error"
wekan_1    | }

The user names are in the format <last>.<first initial>, and the email is <first initial>.<last>. The email addresses stored in Active Directory use uppercase letters for the first initial and the first letter of the last name. It turned out that this is an issue.

When non-LDAP users are created in Wekan, it appears the email address is converted to lowercase. When merging an LDAP account with an existing account, the email address is used to match the accounts. However, the email address retrieved from Active Directory is not converted to lowercase before calling Meteor.users.findOne. Since the match failed, Wekan attempts to create a new user, using the lowercase version of the email address, which fails because the email already exists.

I think all that's needed to fix this issue is to convert the email address to lowercase when creating the query that gets fed into Meteor.users.findOne.

To confirm this was the issue, I manually modified the database entry for an existing user's email address to make it match the case of the Active Directory entry. After doing that I was able to successfully login using the LDAP credentials, and the logs indicated the accounts were merged. This will be my workaround until the actual issue is fixed.

While tracing this issue I came across a very minor secondary issue. The log message log_info('No user exists with username', username, '- attempting to find by e-mail address instead'); is not fully written. The third argument, starting with - attempting..., is not written. That argument to log_info is lost since the underlying log function only accepts two arguments in addition to level.

My day is done, but I'll try to set up a development environment and fix the issues tomorrow, unless someone wants to beat me to it.

xet7 commented 4 years ago

@njpaul

You are welcome to send pull request. Some development info is here https://github.com/wekan/wekan/issues/3071#issuecomment-622300835

xet7 commented 3 weeks ago

Some problems: https://github.com/wekan/ldap/pull/82#issuecomment-2192080410