Allow OAuth to auto link LDAP users via email address
What does this MR do?
This merge requests changes that after a succesful oauth login a search for a user by their email-address will be performed against the mail of the user.
Lookups for the mail-address have previously only been made against the uid
which only works if this is the e-mail-address as well.
This MR takes the email-address attribute into consideration.
It implements a fix suggest by @juwo here.
Background
In an instance SAML is used for authentication. The idP returns a persistent ID after successful auth which is not in the format of an e-mail-address, nor ID, nor DN. LDAP is used in addition to sync user name and e-mail as well as blocking users. LDAP is not available for logging in.
Behaviour without this MR
User logs in through SAML. No user attributes are received through LDAP.
An error is displayed in production.log
.
User is able to change name and e-mail in his profile.
Excerpt from production.log
See the LDAP search error below.
Started POST "/users/auth/saml" for 10.10.10.202 at 2020-06-03 13:44:23 +0200
Processing by Gitlab::RequestForgeryProtection::Controller#index as HTML
Parameters: {"authenticity_token"=>"[FILTERED]"}
Completed 200 OK in 0ms (ActiveRecord: 0.0ms | Elasticsearch: 0.0ms | Allocations: 162)
Started POST "/users/auth/saml/callback" for 10.10.10.202 at 2020-06-03 13:44:24 +0200
Processing by OmniauthCallbacksController#saml as HTML
Parameters: {"SAMLResponse"=>"PHN...2U+"}
LDAP search error: Invalid DN Syntax
Redirected to https://gitlab.domain.tld/
Completed 302 Found in 237ms (ActiveRecord: 54.0ms | Elasticsearch: 0.0ms | Allocations: 92967)
Behaviour with this MR
Users log in through SAML and afterwards user attributes are received through LDAP. Users are not able to change their name or e-mail-address. The logs contain no error.
Screenshots
Does this MR meet the acceptance criteria?
Conformity
-
Changelog entry -
Documentation (if required) -
Code review guidelines -
Merge request performance guidelines -
Style guides -
Database guides -
Separation of EE specific content
Availability and Testing
The test only affects backend components without altering any APIs. Testing with browsers can be skipped with this.
I tested the change on the affected instance and it worked as desired. I'm not confident enough in Ruby to provide tests for this myself.
-
Review and add/update tests for this feature/bug. Consider all test levels. See the Test Planning Process. - [-] Tested in all supported browsers
- [-] Informed Infrastructure department of a default or new setting change, if applicable per definition of done
Configuration
To make testing easier these are settings used in the affected instance.
Configuration used in /etc/gitlab/gitlab.rb
Relevant settings from /etc/gitlab/gitlab.rb
.
gitlab_rails['ldap_servers'] = {
'main' => {
...
'attributes' => {
'username' => ['userPrincipalName'],
'email' => ['email'],
'name' => 'cn',
'first_name' => 'givenName',
'last_name' => 'sn'
},
}
}
gitlab_rails['sync_profile_attributes'] = ['name', 'email']
gitlab_rails['prevent_ldap_sign_in'] = true
gitlab_rails['omniauth_allow_single_sign_on'] = ['saml']
gitlab_rails['omniauth_block_auto_created_users'] = false
gitlab_rails['omniauth_auto_link_saml_user'] = true
gitlab_rails['omniauth_auto_link_ldap_user'] = true
gitlab_rails['omniauth_providers'] = [
{
name: 'saml',
args: {
...
name_identifier_format: 'urn:oasis:names:tc:SAML:2.0:nameid-format:persistent'
},
label: 'SAML'
}
]
Security
This touches retrieval of data from LDAP after the authentication. The way I can think this can be abused is if an user is able to change their e-mail-address in idP and then impersonate another user in LDAP through this.
-
Label as security and @ mention @gitlab-com/gl-security/appsec
- [-] The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
-
Security reports checked/validated by a reviewer from the AppSec team