Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow LDAP user to update local user #2088

Closed
wants to merge 3 commits into from
Closed

Allow LDAP user to update local user #2088

wants to merge 3 commits into from

Conversation

colinho
Copy link

@colinho colinho commented Jan 30, 2016

Previously, if a user successfully logged in with a LDAP username that matched an existing local username, the login would fail. Now the login will succeed (with the rationale that the user provided legit LDAP credentials) and the existing user object is updated with ldap = true.

Signed-off-by: Colin Ho colin@colinho.com

Previously, if a user successfully logged in with a LDAP username that matched an existing local username, the login would fail. Now the login will succeed (with the rationale that the user provided legit LDAP credentials) and the existing user object is updated with ldap = true.

Signed-off-by: Colin Ho <colin@colinho.com>
@colinho
Copy link
Author

colinho commented Jan 30, 2016

I believe this will fix #1912.

@colinho
Copy link
Author

colinho commented Jan 30, 2016

It may also fix #1823, because I speculate that what happens is the LDAP auth succeeds with the username, which fires the exception that this commit removes.

@colinho
Copy link
Author

colinho commented Jan 30, 2016

Also, this commit addresses the "troubleshooting" issue mentioned on the wiki here: https://github.com/RocketChat/Rocket.Chat/wiki/LDAP-Authentication#i-cannot-login-even-everything-looks-good.

@rodrigok
Copy link
Member

rodrigok commented Feb 3, 2016

@colinho thank you, your solution is great, but I think that this should be optional and can be enabled by default.

Can you add an option to disable this in settings?

@engelgabriel
Copy link
Member

Thanks for the PR @colinho, we have added it into #2167

Can you help test it?

@colinho colinho deleted the feature/allow_ldap_user_to_update_local_user branch June 12, 2016 03:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants