Move auth hash to a seperate class

lib/gitlab/ldap/user.rb

@@ -10,23 +10,27 @@ module Gitlab
   module LDAP
     class User < Gitlab::OAuth::User
       class << self
-        def find_or_create(auth)
-          self.auth = auth
-          find(auth) || create(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
 
-        # 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
+        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: ldap_user.uid, provider: ldap_user.provider)
-            Gitlab::AppLogger.info("(LDAP) Updating legacy LDAP user #{ldap_user.email} with extern_uid => #{ldap_user.uid}")
+            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
+        end
+
+        # overloaded from Gitlab::Oauth::User
+        # TODO: it's messy, needs cleanup, less complexity
+        def create(auth_hash)
+          ldap_user = new(auth_hash)
+          # first try to find the user based on the returned email address
 
           # if the user isn't found by an exact email match, use oauth methods
           ldap_user.save_and_trigger_callbacks
@@ -58,7 +62,7 @@ module Gitlab
         protected
 
         def find_by_uid_and_provider
-          find_by_uid(uid)
+          find_by_uid(auth_hash.uid)
         end
 
         def find_by_uid(uid)
@@ -79,10 +83,6 @@ module Gitlab
         end
       end
 
-      def find_gitlab_user_by_email
-        self.class.model.find_by(email: email)
-      end
-
       def needs_blocking?
         false
       end

lib/gitlab/oauth/user.rb

@@ -7,15 +7,15 @@ module Gitlab
   module OAuth
     class User
       class << self
-        attr_accessor :auth
+        attr_reader :auth_hash
 
-        def find(auth)
-          self.auth = auth
+        def find(auth_hash)
+          self.auth_hash = auth_hash
           find_by_uid_and_provider
         end
 
-        def create(auth)
-          user = new(auth)
+        def create(auth_hash)
+          user = new(auth_hash)
           user.save_and_trigger_callbacks
         end
 
@@ -23,31 +23,32 @@ module Gitlab
           ::User
         end
 
-        protected
-        def find_by_uid_and_provider
-          model.where(provider: provider, extern_uid: uid).last
-        end
-
-        def provider
-          auth.provider
+        def auth_hash=(auth_hash)
+          @auth_hash = AuthHash.new(auth_hash)
         end
 
-        def uid
-          auth.uid.to_s
+        protected
+        def find_by_uid_and_provider
+          model.where(provider: auth_hash.provider, extern_uid: auth_hash.uid).last
         end
       end
 
-      attr_accessor :auth, :user
+      # Instance methods
+      attr_accessor :auth_hash, :user
 
-      def initialize(auth)
-        self.auth = auth
+      def initialize(auth_hash)
+        self.auth_hash = auth_hash
         self.user = self.class.model.new(user_attributes)
         user.skip_confirmation!
       end
 
+      def auth_hash=(auth_hash)
+        @auth_hash = AuthHash.new(auth_hash)
+      end
+
       def save_and_trigger_callbacks
         user.save!
-        log.info "(OAuth) Creating user #{email} from login with extern_uid => #{uid}"
+        log.info "(OAuth) Creating user #{auth_hash.email} from login with extern_uid => #{auth_hash.uid}"
         user.block if needs_blocking?
 
         user
@@ -58,48 +59,16 @@ module Gitlab
 
       def user_attributes
         {
-          extern_uid: uid,
-          provider: provider,
-          name: name,
-          username: username,
-          email: email,
-          password: password,
-          password_confirmation: password,
+          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 uid
-        auth.uid.to_s
-      end
-
-      def provider
-        auth.provider
-      end
-
-      def info
-        auth.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
-
       def log
         Gitlab::AppLogger
       end
@@ -108,16 +77,6 @@ module Gitlab
         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
-
       def needs_blocking?
         Gitlab.config.omniauth['block_auto_created_users']
       end

spec/lib/gitlab/ldap/user_spec.rb

 require 'spec_helper'
 
 describe Gitlab::LDAP::User do
-  let(:gl_auth) { Gitlab::LDAP::User }
+  let(:gl_user) { Gitlab::LDAP::User }
   let(:info) do
     double(
       name: 'John',
@@ -19,12 +19,12 @@ describe Gitlab::LDAP::User do
     it "finds the user if already existing" do
       existing_user = create(:user, extern_uid: 'my-uid', provider: 'ldap')
 
-      expect{ gl_auth.find_or_create(auth) }.to_not change{ User.count }
+      expect{ gl_user.find_or_create(auth) }.to_not change{ User.count }
     end
 
     it "connects to existing non-ldap user if the email matches" do
       existing_user = create(:user, email: 'john@example.com')
-      expect{ gl_auth.find_or_create(auth) }.to_not change{ User.count }
+      expect{ gl_user.find_or_create(auth) }.to_not change{ User.count }
 
       existing_user.reload
       expect(existing_user.extern_uid).to eql 'my-uid'
@@ -32,7 +32,7 @@ describe Gitlab::LDAP::User do
     end
 
     it "creates a new user if not found" do
-      expect{ gl_auth.find_or_create(auth) }.to change{ User.count }.by(1)
+      expect{ gl_user.find_or_create(auth) }.to change{ User.count }.by(1)
     end
   end
 end