Allow LDAP users to authenticate by using their GitLab username
What does this MR do and why?
This MR allows LDAP users to authenticate by using their GitLab username. To understand this change, why we need it and how it will improve usability for LDAP users, let's review this change in context of 3 types of authentication flows GitLab provides.
Sign-in from web interface
Sign-in page provides separate interfaces/tabs for Standard and LDAP authentication methods. To sign in, user should provide their credentials as per selected tab. This MR is not related the sign-in from web interface.
Authentication for Git over HTTP(S)
Git over HTTP(S) provides single interface for Standard and LDAP authentication methods. Meaning that GitLab has to decide what authentication method to use based on credentials provided by a user. This MR will allow LDAP users to authenticate for Git over HTTP(S) by using their GitLab username and LDAP password - this should improve UX. Moreover, since Git over HTTP(S) provides single interface for authentication, there is one, rare, use case in which LDAP users can't authenticate for Git over HTTP(S). If a user's LDAP username matches another user's GitLab username, currently such user won't be able to authenticate by using their LDAP username because GitLab will try to authenticate as the another user as per username provided. As per this MR, the user will be able to use GitLab username to authenticate for Git over HTTP(S). This MR also added a spec to cover this use case.
Require re-authentication to approve MR
From UX perspective, we only ask current_user to provide their password for re-authentication to confirm some action, like MR approval.
#215357 (closed) reported that, currently, LDAP users can't confirm MR approval with password when their GitLab username does not match their LDAP username.
We use Gitlab::Auth.find_with_user_password to confirm current_user password. That method requires username. To not require from current_user manually provide their username, GitLab uses current_user's GitLab username when calling this method. That causes #215357 (closed). This MR resolves this issue.
Implementation details
As per this MR, we identify LDAP user's UID as per config.uid setting by querying LDAP server. We considered extracting UID from user.ldap_identity.extern_uid/DN, but there is no guarantee that UID is present in DN for all LDAP configurations.
We also considered in !186848 (comment 2435221203) to use user.ldap_identity.extern_uid/DN and LDAP password as credentials, but OmniAuth::LDAP::Adaptor#bind_as method we use for LDAP authentication does not support that. That method is a single source of truth for LDAP autentication[1, 2].
We could use Net::LDAP#bind method directly, but that will lead to two LDAP authentication implementations, which might lead to issues in the future.
The decision was in favor of keeping using OmniAuth::LDAP::Adaptor#bind_as method only for LDAP authentication. In the future, after that method provide ability to use DN and password for authentication, we could refactor the implementation to prevent the extra call to LDAP server that this MR introduces.
References
Screenshots or screen recordings
| Before | After |
|---|---|
How to set up and validate locally
MR acceptance checklist
Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.