Merge branch 'feature-oauth-refactoring' into 'master'

Feature oauth refactoring

Only use class methods when needed. In addition, make a proper extension of Gitlab::Oauth::User.

See merge request !1062

3 files changed, 140 insertions, 122 deletions

diff --git a/lib/gitlab/ldap/user.rb b/lib/gitlab/ldap/user.rb
index e6aa3890992..25b5a702f9a 100644
--- a/lib/gitlab/ldap/user.rb
+++ b/lib/gitlab/ldap/user.rb
@@ -10,37 +10,20 @@ module Gitlab
   module LDAP
     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
-
-          user = find(auth)
+        def find_or_create(auth_hash)
+          self.auth_hash = auth_hash
+          find(auth_hash) || find_and_connect_by_email(auth_hash) || create(auth_hash)
+        end
 
-          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)
+        def find_and_connect_by_email(auth_hash)
+          self.auth_hash = auth_hash
+          user = model.find_by(email: self.auth_hash.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
+          if user
+            user.update_attributes(extern_uid: auth_hash.uid, provider: auth_hash.provider)
+            Gitlab::AppLogger.info("(LDAP) Updating legacy LDAP user #{self.auth_hash.email} with extern_uid => #{auth_hash.uid}")
+            return user
           end
-
-          user
         end
 
         def authenticate(login, password)
@@ -48,17 +31,8 @@ module Gitlab
           # Only check with valid login and password to prevent anonymous bind results
           return nil unless ldap_conf.enabled && login.present? && password.present?
 
-          ldap = OmniAuth::LDAP::Adaptor.new(ldap_conf)
-          filter = Net::LDAP::Filter.eq(ldap.uid, login)
-
-          # Apply LDAP user filter if present
-          if ldap_conf['user_filter'].present?
-            user_filter = Net::LDAP::Filter.construct(ldap_conf['user_filter'])
-            filter = Net::LDAP::Filter.join(filter, user_filter)
-          end
-
-          ldap_user = ldap.bind_as(
-            filter: filter,
+          ldap_user = adapter.bind_as(
+            filter: user_filter(login),
             size: 1,
             password: password
           )
@@ -66,10 +40,14 @@ module Gitlab
           find_by_uid(ldap_user.dn) if ldap_user
         end
 
-        private
+        def adapter
+          @adapter ||= OmniAuth::LDAP::Adaptor.new(ldap_conf)
+        end
+
+        protected
 
         def find_by_uid_and_provider
-          find_by_uid(uid)
+          find_by_uid(auth_hash.uid)
         end
 
         def find_by_uid(uid)
@@ -88,6 +66,20 @@ module Gitlab
         def ldap_conf
           Gitlab.config.ldap
         end
+
+        def user_filter(login)
+          filter = Net::LDAP::Filter.eq(adapter.uid, login)
+          # Apply LDAP user filter if present
+          if ldap_conf['user_filter'].present?
+            user_filter = Net::LDAP::Filter.construct(ldap_conf['user_filter'])
+            filter = Net::LDAP::Filter.join(filter, user_filter)
+          end
+          filter
+        end
+      end
+
+      def needs_blocking?
+        false
       end
     end
   end
diff --git a/lib/gitlab/oauth/auth_hash.rb b/lib/gitlab/oauth/auth_hash.rb
new file mode 100644
index 00000000000..0198f61f427
--- /dev/null
+++ b/lib/gitlab/oauth/auth_hash.rb
@@ -0,0 +1,54 @@
+# Class to parse and transform the info provided by omniauth
+#
+module Gitlab
+  module OAuth
+    class AuthHash
+      attr_reader :auth_hash
+      def initialize(auth_hash)
+        @auth_hash = auth_hash
+      end
+
+      def uid
+        auth_hash.uid.to_s
+      end
+
+      def provider
+        auth_hash.provider
+      end
+
+      def info
+        auth_hash.info
+      end
+
+      def name
+        (info.name || full_name).to_s.force_encoding('utf-8')
+      end
+
+      def full_name
+        "#{info.first_name} #{info.last_name}"
+      end
+
+      def username
+        (info.try(:nickname) || generate_username).to_s.force_encoding('utf-8')
+      end
+
+      def email
+        (info.try(:email) || generate_temporarily_email).downcase
+      end
+
+      def password
+        @password ||= Devise.friendly_token[0, 8].downcase
+      end
+
+      # Get the first part of the email address (before @)
+      # In addtion in removes illegal characters
+      def generate_username
+        email.match(/^[^@]*/)[0].parameterize
+      end
+
+      def generate_temporarily_email
+        "temp-email-for-oauth-#{username}@gitlab.localhost"
+      end
+    end
+  end
+end
diff --git a/lib/gitlab/oauth/user.rb b/lib/gitlab/oauth/user.rb
index 9670aad2c5d..b768eda185f 100644
--- a/lib/gitlab/oauth/user.rb
+++ b/lib/gitlab/oauth/user.rb
@@ -7,106 +7,78 @@ module Gitlab
   module OAuth
     class User
       class << self
-        attr_reader :auth
+        attr_reader :auth_hash
 
-        def find(auth)
-          @auth = auth
+        def find(auth_hash)
+          self.auth_hash = auth_hash
           find_by_uid_and_provider
         end
 
-        def create(auth)
-          @auth = auth
-          password = Devise.friendly_token[0, 8].downcase
-          opts = {
-            extern_uid: uid,
-            provider: provider,
-            name: name,
-            username: username,
-            email: email,
-            password: password,
-            password_confirmation: password,
-          }
-
-          user = model.build_user(opts)
-          user.skip_confirmation!
-
-          # Services like twitter and github does not return email via oauth
-          # In this case we generate temporary email and force user to fill it later
-          if user.email.blank?
-            user.generate_tmp_oauth_email
-          elsif provider != "ldap"
-            # Google oauth returns email but dont return nickname
-            # So we use part of email as username for new user
-            # For LDAP, username is already set to the user's
-            # uid/userid/sAMAccountName.
-            email_username = email.match(/^[^@]*/)[0]
-            # Strip apostrophes since they are disallowed as part of username
-            user.username = email_username.gsub("'", "")
-          end
-
-          begin
-            user.save!
-          rescue ActiveRecord::RecordInvalid => e
-            log.info "(OAuth) Email #{e.record.errors[:email]}. Username #{e.record.errors[:username]}"
-            return nil, e.record.errors
-          end
-
-          log.info "(OAuth) Creating user #{email} from login with extern_uid => #{uid}"
-
-          if Gitlab.config.omniauth['block_auto_created_users'] && !ldap?
-            user.block
-          end
+        def create(auth_hash)
+          user = new(auth_hash)
+          user.save_and_trigger_callbacks
+        end
 
-          user
+        def model
+          ::User
         end
 
-        private
+        def auth_hash=(auth_hash)
+          @auth_hash = AuthHash.new(auth_hash)
+        end
 
+        protected
         def find_by_uid_and_provider
-          model.where(provider: provider, extern_uid: uid).last
+          model.where(provider: auth_hash.provider, extern_uid: auth_hash.uid).last
         end
+      end
 
-        def uid
-          auth.uid.to_s
-        end
+      # Instance methods
+      attr_accessor :auth_hash, :user
 
-        def email
-          return unless auth.info.respond_to?(:email)
-          auth.info.email.downcase unless auth.info.email.nil?
-        end
+      def initialize(auth_hash)
+        self.auth_hash = auth_hash
+        self.user = self.class.model.new(user_attributes)
+        user.skip_confirmation!
+      end
 
-        def name
-          if auth.info.name.nil?
-            "#{auth.info.first_name} #{auth.info.last_name}".force_encoding('utf-8')
-          else
-            auth.info.name.to_s.force_encoding('utf-8')
-          end
-        end
+      def auth_hash=(auth_hash)
+        @auth_hash = AuthHash.new(auth_hash)
+      end
 
-        def username
-          return unless auth.info.respond_to?(:nickname)
-          auth.info.nickname.to_s.force_encoding("utf-8")
-        end
+      def save_and_trigger_callbacks
+        user.save!
+        log.info "(OAuth) Creating user #{auth_hash.email} from login with extern_uid => #{auth_hash.uid}"
+        user.block if needs_blocking?
 
-        def provider
-          auth.provider
-        end
+        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 log
-          Gitlab::AppLogger
-        end
+      def user_attributes
+        {
+          extern_uid: auth_hash.uid,
+          provider: auth_hash.provider,
+          name: auth_hash.name,
+          username: auth_hash.username,
+          email: auth_hash.email,
+          password: auth_hash.password,
+          password_confirmation: auth_hash.password,
+        }
+      end
 
-        def model
-          ::User
-        end
+      def log
+        Gitlab::AppLogger
+      end
 
-        def raise_error(message)
-          raise OmniAuth::Error, "(OAuth) " + message
-        end
+      def raise_error(message)
+        raise OmniAuth::Error, "(OAuth) " + message
+      end
 
-        def ldap?
-          provider == 'ldap'
-        end
+      def needs_blocking?
+        Gitlab.config.omniauth['block_auto_created_users']
       end
     end
   end