Merge branch 'ldap_allow_email' into 'master'

Avoid ldap.allow_username_or_email_login issues

See merge request !1016

4 files changed, 3 insertions, 29 deletions

diff --git a/CHANGELOG b/CHANGELOG
index bff2bf993f9..f26570965e6 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -9,6 +9,7 @@ v 7.3.0
   - Prevent project stars duplication when fork project
   - Support Unix domain sockets for Redis
   - Store session Redis keys in 'session:gitlab:' namespace
+  - Deprecate LDAP account takeover based on partial LDAP email / GitLab username match
 
 v 7.2.0
   - Explore page
diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example
index 47865ff4b44..0a0d9241e27 100644
--- a/config/gitlab.yml.example
+++ b/config/gitlab.yml.example
@@ -143,7 +143,7 @@ production: &base
     #
     # If you are using "uid: 'userPrincipalName'" on ActiveDirectory you need to
     # disable this setting, because the userPrincipalName contains an '@'.
-    allow_username_or_email_login: true
+    allow_username_or_email_login: false
 
     # Base where we can search for users
     #
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