Use instance methods of LDAP::User as well

Still in need of some proper cleanup

2 files changed, 50 insertions, 52 deletions

diff --git a/lib/gitlab/ldap/user.rb b/lib/gitlab/ldap/user.rb
index f15d723a063..99f23080a84 100644
--- a/lib/gitlab/ldap/user.rb
+++ b/lib/gitlab/ldap/user.rb
@@ -11,36 +11,25 @@ module Gitlab
     class User < Gitlab::OAuth::User
       class << self
         def find_or_create(auth)
-          @auth = auth
-
-          if uid.blank? || email.blank? || username.blank?
-            raise_error("Account must provide a dn, uid and email address")
-          end
+          self.auth = auth
+          find(auth) || create(auth)
+        end
 
-          user = find(auth)
-
-          unless user
-            # Look for user with same emails
-            #
-            # Possible cases:
-            # * 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 = model.find_by(email: email)
-
-            if user
-              user.update_attributes(extern_uid: uid, provider: provider)
-              log.info("(LDAP) Updating legacy LDAP user #{email} with extern_uid => #{uid}")
-            else
-              # Create a new user inside GitLab database
-              # based on LDAP credentials
-              #
-              #
-              user = create(auth)
-            end
+        # overloaded from Gitlab::Oauth::User
+        # TODO: it's messy, needs cleanup, less complexity
+        def create(auth)
+          ldap_user = new(auth)
+          # first try to find the user based on the returned email address
+          user = ldap_user.find_gitlab_user_by_email
+
+          if user
+            user.update_attributes(extern_uid: ldap_user.uid, provider: ldap_user.provider)
+            Gitlab::AppLogger.info("(LDAP) Updating legacy LDAP user #{ldap_user.email} with extern_uid => #{ldap_user.uid}")
+            return user
           end
 
-          user
+          # if the user isn't found by an exact email match, use oauth methods
+          ldap_user.save_and_trigger_callbacks
         end
 
         def authenticate(login, password)
@@ -66,11 +55,7 @@ module Gitlab
           find_by_uid(ldap_user.dn) if ldap_user
         end
 
-        private
-
-        def needs_blocking?
-          false
-        end
+        protected
 
         def find_by_uid_and_provider
           find_by_uid(uid)
@@ -93,6 +78,14 @@ module Gitlab
           Gitlab.config.ldap
         end
       end
+
+      def find_gitlab_user_by_email
+        self.class.model.find_by(email: email)
+      end
+
+      def needs_blocking?
+        false
+      end
     end
   end
 end
diff --git a/lib/gitlab/oauth/user.rb b/lib/gitlab/oauth/user.rb
index f30aec36445..8ac040e3365 100644
--- a/lib/gitlab/oauth/user.rb
+++ b/lib/gitlab/oauth/user.rb
@@ -7,31 +7,25 @@ module Gitlab
   module OAuth
     class User
       class << self
-        attr_reader :auth
+        attr_accessor :auth
 
         def find(auth)
-          @auth = auth
+          self.auth = auth
           find_by_uid_and_provider
         end
 
         def create(auth)
-          @auth = auth
-          user = new(auth).user
-
-          user.save!
-          log.info "(OAuth) Creating user #{email} from login with extern_uid => #{uid}"
-          user.block if needs_blocking?
-
-          user
-        rescue ActiveRecord::RecordInvalid => e
-          log.info "(OAuth) Email #{e.record.errors[:email]}. Username #{e.record.errors[:username]}"
-          return nil, e.record.errors
+          user = new(auth)
+          user.save_and_trigger_callbacks
         end
 
-        private
+        def model
+          ::User
+        end
 
+        protected
         def find_by_uid_and_provider
-          ::User.where(provider: provider, extern_uid: uid).last
+          model.where(provider: provider, extern_uid: uid).last
         end
 
         def provider
@@ -41,20 +35,27 @@ module Gitlab
         def uid
           auth.uid.to_s
         end
-
-        def needs_blocking?
-          Gitlab.config.omniauth['block_auto_created_users']
-        end
       end
 
       attr_accessor :auth, :user
 
       def initialize(auth)
         self.auth = auth
-        self.user = ::User.new(user_attributes)
+        self.user = self.class.model.new(user_attributes)
         user.skip_confirmation!
       end
 
+      def save_and_trigger_callbacks
+        user.save!
+        log.info "(OAuth) Creating user #{email} from login with extern_uid => #{uid}"
+        user.block if needs_blocking?
+
+        user
+      rescue ActiveRecord::RecordInvalid => e
+        log.info "(OAuth) Email #{e.record.errors[:email]}. Username #{e.record.errors[:username]}"
+        return nil, e.record.errors
+      end
+
       def user_attributes
         {
           extern_uid: uid,
@@ -116,6 +117,10 @@ module Gitlab
       def generate_temporarily_email
         "temp-email-for-oauth-#{username}@gitlab.localhost"
       end
+
+      def needs_blocking?
+        Gitlab.config.omniauth['block_auto_created_users']
+      end
     end
   end
 end