Feature: Decouple LDAP username from Rocket Chat username

robertfromont commented 4 years ago

Description:

Users should be able to be authenticated via LDAP without having their LDAP username the same as their Rocket Chat username.

Steps to reproduce:

Enable LDAP authentication
Set Administration|Accounts|Registration|Manually Approve New Users to on
An existing user with RC username robertfromont logs in using their LDAP username, which is raf123 - (in both RC and LDAP they have the same email address)

Expected behavior:

They can log on, and have their username appear as the RC username.

Actual behavior:

Login fails.

Server Setup Information:

Version of Rocket.Chat Server: 3.7.1
Operating System: linux 5.4.0-1026-azure
Deployment Method: tar
Number of Running Instances: 1
DB Replicaset Oplog: Enabled
NodeJS Version: v12.18.4
MongoDB Version: 4.0.20

Client Setup Information

Desktop App or Browser Version: All
Operating System: All

Additional context

This issue expands on RocketChat/Rocket.Chat#18881.

I have migrated a small number of users of a very large organization from a Slack workspace to an in-house Rocket Chat instance. So all the current users previously had descriptive usernames in Slack. In order to make migration as painless as possible, but also to tie authorization to the large organization's user database, I have enabled LDAP auth.

This means that everyone can just log in with a username/password they already know, which is great!

Unfortunately the LDAP usernames imposed by the larger organization have a formal structure and are quite opaque. In short, nobody knows anybody else's LDAP usernames, so it's difficult to understand who is talking in chats, who users want to @mention, etc.

I have enabled the Administration|Accounts|Default User Preferences|Hide Usernames setting to try to address this issue, but unfortunately the coverage of this setting isn't exhaustive; it doesn't apply in certain notifications, in Diego Sampaio's Poll app, etc. Perhaps more importantly, users like to be able to choose the name they known by in this community (The Nickname profile setting should probably allow this, but it doesn't appear to do anything other than appear in parentheses).

Ideally, what I'd like is:

To be able to create a new user, specifying their RC username, email address and maybe their LDAP username in a custom field, but to not specify a password
When such a user logs in with their LDAP username or their email address, they are authenticated with their LDAP password
They can, if they want, change their RC username (currently I have this disabled so that can't accidentally lock themselves out of RC by changing the name)
When they interact with other users, the appear with the RC username

I'm able and motivated to make the code change for this myself, but I'd like to implement this in a way that's likely to be accepted as a pull-request, as I'd prefer to be using the 'canonical' version of Rocket.Chat in production.

Any feedback or comments from current maintainers, who seem to include @sampaiodiego and @gabriellsh would be gratefully received.

Relevant logs:

I20201020-04:48:33.864(13) API ➔ debug POST: /api/v1/method.callAnon/login I20201020-04:48:33.868(13) LDAPHandler ➔ info Init LDAP login raf123
I20201020-04:48:33.870(13) LDAP ➔ Connection.info Init setup 
I20201020-04:48:33.873(13) LDAP ➔ Connection.info Connecting ldap://canterbury.ac.nz:3268 
I20201020-04:48:33.878(13) LDAP ➔ Connection.debug connectionOptions {   url: 'ldap://<redacted>:3268',   timeout: 60000,   connectTimeout: 1000,   idleTimeout: 1000,   reconnect: true,   log: Logger {     _events: [Object: null prototype] {},     _eventsCount: 0,     _maxListeners: undefined,     _level: 30,     streams: [ [Object] ],     serializers: null,     src: false,     fields: {       name: 'ldapjs',       component: 'client',       hostname: 'ilbbchat1t.linux.<redacted>',       pid: 129148     }   } } 
I20201020-04:48:33.985(13) LDAP ➔ Connection.info LDAP connected 
I20201020-04:48:33.988(13) LDAP ➔ Bind.info Binding UserDN CN=redacted,CN=Users,DC=redacted 
I20201020-04:48:33.993(13) LDAP ➔ Search.info Searching user raf123 
I20201020-04:48:33.999(13) LDAP ➔ Search.debug searchOptions {   filter: '(&(objectclass=*)(|(mail=raf123)(cn=raf123)))',   scope: 'sub',   sizeLimit: 1000,   paged: { pageSize: 250, pagePause: false } } 
I20201020-04:48:34.000(13) LDAP ➔ Search.debug BaseDN dc=canterbury,dc=ac,dc=nz 
I20201020-04:48:34.008(13) LDAP ➔ Search.info Search result count 1 
I20201020-04:48:34.010(13) LDAP ➔ Auth.info Authenticating CN=raf123,OU=Staff,DC=redacted 
I20201020-04:48:34.024(13) LDAP ➔ Search.info Search result count 4 
I20201020-04:48:34.026(13) LDAP ➔ Auth.info Authenticated CN=raf123,OU=Staff,DC=redacted 
I20201020-04:48:34.028(13) LDAPHandler ➔ info Querying user 
I20201020-04:48:34.033(13) LDAPHandler ➔ debug userQuery { 'services.ldap.id': 'redacted' } 
I20201020-04:48:34.037(13) LDAPHandler ➔ debug userQuery { username: 'raf123' } 
I20201020-04:48:34.040(13) LDAPHandler ➔ info User does not exist, creating raf123 
I20201020-04:48:34.043(13) LDAPSync ➔ debug New user data { username: 'raf123', email: 'redacted' } 
I20201020-04:48:34.144(13) server.js:204 LDAPSync ➔ error Error creating user errorClass [Error]: Email already exists. [403]
     at handleError (packages/accounts-password/password_server.js:110:17)
     at checkForCaseInsensitiveDuplicates (packages/accounts-password/password_server.js:257:7)
     at createUser (packages/accounts-password/password_server.js:1126:3)
     at AccountsServer.Accounts.createUser (packages/accounts-password/password_server.js:1197:10)
     at addLdapUser (app/ldap/server/sync.js:472:29)
     at MethodInvocation.<anonymous> (app/ldap/server/loginHandler.js:147:17)
     at packages/accounts-base/accounts_server.js:475:31
     at tryLoginMethod (packages/accounts-base/accounts_server.js:1309:14)
     at AccountsServer._runLoginHandlers (packages/accounts-base/accounts_server.js:473:22)
     at AccountsServer.Accounts._runLoginHandlers (app/lib/server/lib/loginErrorMessageOverride.js:7:35)
     at MethodInvocation.methods.login (packages/accounts-base/accounts_server.js:533:31)
     at maybeAuditArgumentChecks (packages/ddp-server/livedata_server.js:1771:12)
     at packages/ddp-server/livedata_server.js:1689:15
     at Meteor.EnvironmentVariable.EVp.withValue (packages/meteor.js:1234:12)
     at packages/ddp-server/livedata_server.js:1687:36
     at new Promise (<anonymous>)
     at Server.applyAsync (packages/ddp-server/livedata_server.js:1686:12)
     at Server.apply (packages/ddp-server/livedata_server.js:1625:26)
     at Server.call (packages/ddp-server/livedata_server.js:1607:17)
     at Object.post (app/api/server/v1/misc.js:262:26)
     at app/api/server/api.js:394:82
     at Meteor.EnvironmentVariable.EVp.withValue (packages/meteor.js:1234:12)
     at Object._internalRouteActionHandler [as action] (app/api/server/api.js:394:39)
     at Route.share.Route.Route._callEndpoint (packages/nimble_restivus/lib/route.coffee:150:32)
     at packages/nimble_restivus/lib/route.coffee:59:33
     at packages/simple_json-routes.js:98:9 {   isClientSafe: true,   error: 403,   reason: 'Email already exists.',   details: undefined,   message: 'Email already exists. [403]',   errorType: 'Meteor.Error' } 
I20201020-04:48:34.146(13) Exception while invoking method login Error: Email already exists. [403]
     at handleError (packages/accounts-password/password_server.js:110:17)
     at checkForCaseInsensitiveDuplicates (packages/accounts-password/password_server.js:257:7)
     at createUser (packages/accounts-password/password_server.js:1126:3)
     at AccountsServer.Accounts.createUser (packages/accounts-password/password_server.js:1197:10)
     at addLdapUser (app/ldap/server/sync.js:472:29)
     at MethodInvocation.<anonymous> (app/ldap/server/loginHandler.js:147:17)
     at packages/accounts-base/accounts_server.js:475:31
     at tryLoginMethod (packages/accounts-base/accounts_server.js:1309:14)
     at AccountsServer._runLoginHandlers (packages/accounts-base/accounts_server.js:473:22)
     at AccountsServer.Accounts._runLoginHandlers (app/lib/server/lib/loginErrorMessageOverride.js:7:35)
     at MethodInvocation.methods.login (packages/accounts-base/accounts_server.js:533:31)
     at maybeAuditArgumentChecks (packages/ddp-server/livedata_server.js:1771:12)
     at packages/ddp-server/livedata_server.js:1689:15
     at Meteor.EnvironmentVariable.EVp.withValue (packages/meteor.js:1234:12)
     at packages/ddp-server/livedata_server.js:1687:36
     at new Promise (<anonymous>)
     at Server.applyAsync (packages/ddp-server/livedata_server.js:1686:12)
     at Server.apply (packages/ddp-server/livedata_server.js:1625:26)
     at Server.call (packages/ddp-server/livedata_server.js:1607:17)
     at Object.post (app/api/server/v1/misc.js:262:26)
     at app/api/server/api.js:394:82
     at Meteor.EnvironmentVariable.EVp.withValue (packages/meteor.js:1234:12)
     at Object._internalRouteActionHandler [as action] (app/api/server/api.js:394:39)
     at Route.share.Route.Route._callEndpoint (packages/nimble_restivus/lib/route.coffee:150:32)
     at packages/nimble_restivus/lib/route.coffee:59:33
     at packages/simple_json-routes.js:98:9  => awaited here:
     at Promise.await (/opt/Rocket.Chat/programs/server/npm/node_modules/meteor/promise/node_modules/meteor-promise/promise_server.js:60:12)
     at Server.apply (packages/ddp-server/livedata_server.js:1638:22)
     at Server.call (packages/ddp-server/livedata_server.js:1607:17)
     at Object.post (app/api/server/v1/misc.js:262:26)
     at app/api/server/api.js:394:82
     at Meteor.EnvironmentVariable.EVp.withValue (packages/meteor.js:1234:12)
     at Object._internalRouteActionHandler [as action] (app/api/server/api.js:394:39)
     at Route.share.Route.Route._callEndpoint (packages/nimble_restivus/lib/route.coffee:150:32)
     at packages/nimble_restivus/lib/route.coffee:59:33
     at packages/simple_json-routes.js:98:9 
I20201020-04:48:34.148(13) API ➔ debug Success {   statusCode: 200,   body: {
     message: '{"msg":"result","id":"15","error":{"isClientSafe":true,"error":403,"reason":"Email already exists.","message":"Email already exists. [403]","errorType":"Meteor.Error"}}',
     success: true   } } 
I20201020-04:48:35.024(13) LDAP ➔ Search.info Idle 
I20201020-04:48:35.026(13) LDAP ➔ Connection.info Disconecting 
I20201020-04:48:35.032(13) LDAP ➔ Search.info Closed

sampaiodiego commented 4 years ago

just to make sure, do you know about the admin setting called Use Real Name under Admin > Layout > User Interface? with this setting turned on you shouldn't see usernames in the chat UI anymore, but you'd still need to know them to mention someone.

cc @pierre-lehnen-rc

bbrauns commented 4 years ago

There is also the setting "User Data Field Map" which lets you specify how the RC "name" is filled

robertfromont commented 4 years ago

just to make sure, do you know about the admin setting called Use Real Name under Admin > Layout > User Interface? with this setting turned on you shouldn't see usernames in the chat UI anymore, but you'd still need to know them to mention someone.

@sampaiodiego yes, sorry I should have mentioned that we're already using that setting, and it works fairly well.

I think it appearing when you @mention someone is the primary source of annoyance, but there are still a few places where the username sneaks through, including:

Quoted messages:

quoted-messages

Polls:

polls

@mentions in channel previews on iOS:

iOS-@mentions

robertfromont commented 4 years ago

There is also the setting "User Data Field Map" which lets you specify how the RC "name" is filled

@bbrauns Actually, that looks pretty interesting for auto-population of the LDAP username in custom fields, thanks!

robertfromont commented 4 years ago

I've just been exploring the way OAuth works, and it doesn't impose usernames on users.

So what I'm proposing here is to allow the possibility of configuring LDAP to work in a similar way to OAuth: provide a mechanism to authenticate users, without necessarily determining their username or other profile details.

Does that make sense?

robertfromont commented 4 years ago

I've been investigating the code, and have discovered that I can get almost all the way to what I need without changing anything.

It turns out that the LDAP_Username_Field setting (LDAP|Sync/Import|"Username Field") is what links the RC username to the LDAP username. By default this is set to sAMAccountName, but if it's blanked out, then the RC username is not synced to the LDAP one (the first time an LDAP user logs in, they're asked what their RC should be).

This is great, but there's one wrinkle: if LDAP is enabled (LDAP_Enable is on) then users are not allowed to change their own usernames. This is determined here:

https://github.com/RocketChat/Rocket.Chat/blob/7d97938a8b3d6086aad8ffc1f871d22859dfecae/client/account/AccountProfilePage.js#L68

I guess this override of the Accounts_AllowUsernameChange setting makes sense when LDAP_Username_Field is not blank, as users might change their username and then find it's changed back again next time they log in.

However, in my case, I'd like users to be able to change their username, partly because they should be able to control their identity as they like, and partly because the default suggestion for a username when they first log is something nondescript like user-0 and if they click through that unintentionally, they should be able to fix it later.

Does this make sense?

RocketChat / feature-requests