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.
This commit is contained in:
parent
47ac48c031
commit
614ca3ec65
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
|
Loading…
Reference in New Issue