Refactor Oauth::User class to use instance methods

3 files changed, 80 insertions, 72 deletions

diff --git a/app/models/user.rb b/app/models/user.rb
index f1ff76edd15..15e56a62a68 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -474,10 +474,6 @@ class User < ActiveRecord::Base
     email =~ /\Atemp-email-for-oauth/
   end
 
-  def generate_tmp_oauth_email
-    self.email = "temp-email-for-oauth-#{username}@gitlab.localhost"
-  end
-
   def public_profile?
     authorized_projects.public_only.any?
   end
diff --git a/lib/gitlab/ldap/user.rb b/lib/gitlab/ldap/user.rb
index e6aa3890992..f15d723a063 100644
--- a/lib/gitlab/ldap/user.rb
+++ b/lib/gitlab/ldap/user.rb
@@ -68,6 +68,10 @@ module Gitlab
 
         private
 
+        def needs_blocking?
+          false
+        end
+
         def find_by_uid_and_provider
           find_by_uid(uid)
         end
diff --git a/lib/gitlab/oauth/user.rb b/lib/gitlab/oauth/user.rb
index 9670aad2c5d..f30aec36445 100644
--- a/lib/gitlab/oauth/user.rb
+++ b/lib/gitlab/oauth/user.rb
@@ -16,97 +16,105 @@ module Gitlab
 
         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
+          user = new(auth).user
 
+          user.save!
           log.info "(OAuth) Creating user #{email} from login with extern_uid => #{uid}"
-
-          if Gitlab.config.omniauth['block_auto_created_users'] && !ldap?
-            user.block
-          end
+          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
 
         private
 
         def find_by_uid_and_provider
-          model.where(provider: provider, extern_uid: uid).last
+          ::User.where(provider: provider, extern_uid: uid).last
+        end
+
+        def provider
+          auth.provider
         end
 
         def uid
           auth.uid.to_s
         end
 
-        def email
-          return unless auth.info.respond_to?(:email)
-          auth.info.email.downcase unless auth.info.email.nil?
+        def needs_blocking?
+          Gitlab.config.omniauth['block_auto_created_users']
         end
+      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
+      attr_accessor :auth, :user
 
-        def username
-          return unless auth.info.respond_to?(:nickname)
-          auth.info.nickname.to_s.force_encoding("utf-8")
-        end
+      def initialize(auth)
+        self.auth = auth
+        self.user = ::User.new(user_attributes)
+        user.skip_confirmation!
+      end
 
-        def provider
-          auth.provider
-        end
+      def user_attributes
+        {
+          extern_uid: uid,
+          provider: provider,
+          name: name,
+          username: username,
+          email: email,
+          password: password,
+          password_confirmation: password,
+        }
+      end
 
-        def log
-          Gitlab::AppLogger
-        end
+      def uid
+        auth.uid.to_s
+      end
 
-        def model
-          ::User
-        end
+      def provider
+        auth.provider
+      end
 
-        def raise_error(message)
-          raise OmniAuth::Error, "(OAuth) " + message
-        end
+      def info
+        auth.info
+      end
 
-        def ldap?
-          provider == 'ldap'
-        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
+
+      def log
+        Gitlab::AppLogger
+      end
+
+      def raise_error(message)
+        raise OmniAuth::Error, "(OAuth) " + message
+      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