diff options
author | Jacob Vosmaer <contact@jacobvosmaer.nl> | 2014-08-12 17:51:56 +0200 |
---|---|---|
committer | Jacob Vosmaer <contact@jacobvosmaer.nl> | 2014-08-29 15:38:05 +0200 |
commit | 614ca3ec6568f67883c914d43fd37a5758a8ed5b (patch) | |
tree | ff8d2eed428624fe5aa024600ba8bfbaddd341e3 | |
parent | 47ac48c03127b62212108442a8e61f41a7cee6ec (diff) | |
download | gitlab-ce-614ca3ec6568f67883c914d43fd37a5758a8ed5b.tar.gz |
Remove LDAP::Access#find_user
This method existed to allow LDAP users to take over existing GitLab
accounts if the part before the '@' of their LDAP email attribute
matched the username of an existing GitLab user. I propose to disable
this behavior in order to prevent unintended GitLab account takeovers.
After this change it is still possible to take over an existing GitLab
account with your LDAP credentials, as long as the GitLab account email
address matches the LDAP user email address.
-rw-r--r-- | lib/gitlab/ldap/user.rb | 17 | ||||
-rw-r--r-- | spec/lib/gitlab/ldap/ldap_user_auth_spec.rb | 12 |
2 files changed, 1 insertions, 28 deletions
diff --git a/lib/gitlab/ldap/user.rb b/lib/gitlab/ldap/user.rb index be3fcc4f035..79aa145d871 100644 --- a/lib/gitlab/ldap/user.rb +++ b/lib/gitlab/ldap/user.rb @@ -26,7 +26,7 @@ module Gitlab # * When user already has account and need to link their LDAP account. # * LDAP uid changed for user with same email and we need to update their uid # - user = find_user(email) + user = model.find_by(email: email) if user user.update_attributes(extern_uid: uid, provider: provider) @@ -43,21 +43,6 @@ module Gitlab user end - def find_user(email) - user = model.find_by(email: email) - - # If no user found and allow_username_or_email_login is true - # we look for user by extracting part of their email - if !user && email && ldap_conf['allow_username_or_email_login'] - uname = email.partition('@').first - # Strip apostrophes since they are disallowed as part of username - username = uname.gsub("'", "") - user = model.find_by(username: username) - end - - user - end - def authenticate(login, password) # Check user against LDAP backend if user is not authenticated # Only check with valid login and password to prevent anonymous bind results diff --git a/spec/lib/gitlab/ldap/ldap_user_auth_spec.rb b/spec/lib/gitlab/ldap/ldap_user_auth_spec.rb index 501642dca79..1d3df52f0c2 100644 --- a/spec/lib/gitlab/ldap/ldap_user_auth_spec.rb +++ b/spec/lib/gitlab/ldap/ldap_user_auth_spec.rb @@ -31,18 +31,6 @@ describe Gitlab::LDAP do gl_auth.find_or_create(@auth) end - it "should update credentials by username if missing uid and Gitlab.config.ldap.allow_username_or_email_login is true" do - user = double('User') - value = Gitlab.config.ldap.allow_username_or_email_login - Gitlab.config.ldap['allow_username_or_email_login'] = true - User.stub find_by_extern_uid_and_provider: nil - User.stub(:find_by).with(hash_including(email: anything())) { nil } - User.stub(:find_by).with(hash_including(username: anything())) { user } - user.should_receive :update_attributes - gl_auth.find_or_create(@auth) - Gitlab.config.ldap['allow_username_or_email_login'] = value - end - it "should not update credentials by username if missing uid and Gitlab.config.ldap.allow_username_or_email_login is false" do user = double('User') value = Gitlab.config.ldap.allow_username_or_email_login |