From 5b86dab03bbd19d9a3e6090d4672d8f1493e6fe5 Mon Sep 17 00:00:00 2001
From: Jan-Willem van der Meer <mail@jewilmeer.nl>
Date: Thu, 4 Sep 2014 12:55:10 +0200
Subject: [PATCH] Move auth hash to a seperate class
---
lib/gitlab/ldap/user.rb | 32 +++++------
lib/gitlab/oauth/user.rb | 93 +++++++++----------------------
spec/lib/gitlab/ldap/user_spec.rb | 8 +--
3 files changed, 46 insertions(+), 87 deletions(-)
diff --git a/lib/gitlab/ldap/user.rb b/lib/gitlab/ldap/user.rb
index 99f23080a84..6d1bec5f54a 100644
--- a/lib/gitlab/ldap/user.rb
+++ b/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
diff --git a/lib/gitlab/oauth/user.rb b/lib/gitlab/oauth/user.rb
index 8ac040e3365..b768eda185f 100644
--- a/lib/gitlab/oauth/user.rb
+++ b/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
diff --git a/spec/lib/gitlab/ldap/user_spec.rb b/spec/lib/gitlab/ldap/user_spec.rb
index 725338965be..4ddf6b3039f 100644
--- a/spec/lib/gitlab/ldap/user_spec.rb
+++ b/spec/lib/gitlab/ldap/user_spec.rb
@@ -1,7 +1,7 @@
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
--
GitLab