From 5dae40f579f66fdc060de633b183ede7bd8b2ce4 Mon Sep 17 00:00:00 2001
From: Izaak Alpert <ialpert@blackberry.com>
Date: Thu, 15 Aug 2013 17:43:46 -0400
Subject: [PATCH] Update to only provide one way to get a default user
-calling build_user will now apply defaults and only override them if as: :admin is set
Change-Id: Id1d938c0967752ecc14370af54f2d88128d18c44
---
app/controllers/admin/users_controller.rb | 4 +-
app/models/user.rb | 27 +++++----
lib/api/users.rb | 5 +-
lib/gitlab/auth.rb | 66 +++++++++++++++++++++
spec/models/user_spec.rb | 72 +++++++++++++++++------
spec/requests/api/users_spec.rb | 4 +-
6 files changed, 143 insertions(+), 35 deletions(-)
diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb
index 7809a157dbc..70bbe306562 100644
--- a/app/controllers/admin/users_controller.rb
+++ b/app/controllers/admin/users_controller.rb
@@ -13,7 +13,7 @@ class Admin::UsersController < Admin::ApplicationController
end
def new
- @user = User.new.with_defaults
+ @user = User.build_user
end
def edit
@@ -44,7 +44,7 @@ class Admin::UsersController < Admin::ApplicationController
password_expires_at: Time.now
}
- @user = User.new(params[:user].merge(opts), as: :admin)
+ @user = User.build_user(params[:user].merge(opts), as: :admin)
@user.admin = (admin && admin.to_i > 0)
@user.created_by_id = current_user.id
diff --git a/app/models/user.rb b/app/models/user.rb
index 526edb9839e..fbcc004f9b0 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -199,10 +199,16 @@ class User < ActiveRecord::Base
end
end
- def defaults
- { projects_limit: Gitlab.config.gitlab.default_projects_limit, can_create_group: Gitlab.config.gitlab.default_can_create_group, can_create_team: Gitlab.config.gitlab.default_can_create_team }
+ def build_user(attrs = {}, options= {})
+ user = User.new(defaults.merge(attrs), options)
+ # if not as: :admin force default settings
+ user.with_defaults unless options[:as] == :admin
+ user
end
+ def defaults
+ { projects_limit: Gitlab.config.gitlab.default_projects_limit, can_create_group: Gitlab.config.gitlab.default_can_create_group, theme_id: Gitlab::Theme::BASIC }
+ end
end
#
@@ -213,14 +219,6 @@ class User < ActiveRecord::Base
username
end
- def with_defaults
- tap do |u|
- u.projects_limit = Gitlab.config.gitlab.default_projects_limit
- u.can_create_group = Gitlab.config.gitlab.default_can_create_group
- u.theme_id = Gitlab::Theme::MARS
- end
- end
-
def notification
@notification ||= Notification.new(self)
end
@@ -380,4 +378,13 @@ class User < ActiveRecord::Base
group.owners == [self]
end
end
+
+ :private
+
+ def with_defaults
+ User.defaults.each do |k,v|
+ self.send("#{k}=",v)
+ end
+ end
+
end
diff --git a/lib/api/users.rb b/lib/api/users.rb
index 3b7ae9f01a1..00dc2311ffd 100644
--- a/lib/api/users.rb
+++ b/lib/api/users.rb
@@ -45,9 +45,8 @@ module API
post do
authenticated_as_admin!
required_attributes! [:email, :password, :name, :username]
-
- attrs = User.defaults.merge(attributes_for_keys [:email, :name, :password, :skype, :linkedin, :twitter, :projects_limit, :username, :extern_uid, :provider, :bio])
- user = User.new attrs, as: :admin
+ attrs = attributes_for_keys [:email, :name, :password, :skype, :linkedin, :twitter, :projects_limit, :username, :extern_uid, :provider, :bio]
+ user = User.build_user(attrs, as: :admin)
if user.save
present user, with: Entities::User
else
diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb
index 0f196297477..b1e40defc7f 100644
--- a/lib/gitlab/auth.rb
+++ b/lib/gitlab/auth.rb
@@ -13,6 +13,72 @@ module Gitlab
end
end
+ def find_for_ldap_auth(auth, signed_in_resource = nil)
+ uid = auth.info.uid
+ provider = auth.provider
+ email = auth.info.email.downcase unless auth.info.email.nil?
+ raise OmniAuth::Error, "LDAP accounts must provide an uid and email address" if uid.nil? or email.nil?
+
+ if @user = User.find_by_extern_uid_and_provider(uid, provider)
+ @user
+ elsif @user = User.find_by_email(email)
+ log.info "Updating legacy LDAP user #{email} with extern_uid => #{uid}"
+ @user.update_attributes(extern_uid: uid, provider: provider)
+ @user
+ else
+ create_from_omniauth(auth, true)
+ end
+ end
+
+ def create_from_omniauth(auth, ldap = false)
+ provider = auth.provider
+ uid = auth.info.uid || auth.uid
+ uid = uid.to_s.force_encoding("utf-8")
+ name = auth.info.name.to_s.force_encoding("utf-8")
+ email = auth.info.email.to_s.downcase unless auth.info.email.nil?
+
+ ldap_prefix = ldap ? '(LDAP) ' : ''
+ raise OmniAuth::Error, "#{ldap_prefix}#{provider} does not provide an email"\
+ " address" if auth.info.email.blank?
+
+ log.info "#{ldap_prefix}Creating user from #{provider} login"\
+ " {uid => #{uid}, name => #{name}, email => #{email}}"
+ password = Devise.friendly_token[0, 8].downcase
+ @user = User.build_user({
+ extern_uid: uid,
+ provider: provider,
+ name: name,
+ username: email.match(/^[^@]*/)[0],
+ email: email,
+ password: password,
+ password_confirmation: password,
+ }, as: :admin)
+ @user.save!
+
+ if Gitlab.config.omniauth['block_auto_created_users'] && !ldap
+ @user.block
+ end
+
+ @user
+ end
+
+ def find_or_new_for_omniauth(auth)
+ provider, uid = auth.provider, auth.uid
+ email = auth.info.email.downcase unless auth.info.email.nil?
+
+ if @user = User.find_by_provider_and_extern_uid(provider, uid)
+ @user
+ elsif @user = User.find_by_email(email)
+ @user.update_attributes(extern_uid: uid, provider: provider)
+ @user
+ else
+ if Gitlab.config.omniauth['allow_single_sign_on']
+ @user = create_from_omniauth(auth)
+ @user
+ end
+ end
+ end
+
def log
Gitlab::AppLogger
end
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index d79d2b82b6a..330d22cf962 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -196,29 +196,63 @@ describe User do
it { User.not_in_project(@project).should include(@user, @project.owner) }
end
- describe 'normal user' do
- let(:user) { create(:user, name: 'John Smith') }
+ describe 'user creation' do
+ describe 'normal user' do
+ let(:user) { create(:user, name: 'John Smith') }
- it { user.is_admin?.should be_false }
- it { user.require_ssh_key?.should be_true }
- it { user.can_create_group?.should be_true }
- it { user.can_create_project?.should be_true }
- it { user.first_name.should == 'John' }
- end
+ it { user.is_admin?.should be_false }
+ it { user.require_ssh_key?.should be_true }
+ it { user.can_create_group?.should be_true }
+ it { user.can_create_project?.should be_true }
+ it { user.first_name.should == 'John' }
+ end
- describe 'without defaults' do
- let(:user) { User.new }
- it "should not apply defaults to user" do
- user.projects_limit.should == 10
- user.can_create_group.should == true
+ describe 'without defaults' do
+ let(:user) { User.new }
+ it "should not apply defaults to user" do
+ user.projects_limit.should == 10
+ user.can_create_group.should be_true
+ user.theme_id.should == Gitlab::Theme::BASIC
+ end
end
- end
+ context 'as admin' do
+ describe 'with defaults' do
+ let(:user) { User.build_user({}, as: :admin) }
+ it "should apply defaults to user" do
+ user.projects_limit.should == 42
+ user.can_create_group.should be_false
+ user.theme_id.should == Gitlab::Theme::BASIC
+ end
+ end
+
+ describe 'with default overrides' do
+ let(:user) { User.build_user({projects_limit: 123, can_create_group: true, can_create_team: true, theme_id: Gitlab::Theme::MARS}, as: :admin) }
+ it "should apply defaults to user" do
+ user.projects_limit.should == 123
+ user.can_create_group.should be_true
+ user.theme_id.should == Gitlab::Theme::MARS
+ end
+ end
+ end
+
+ context 'as user' do
+ describe 'with defaults' do
+ let(:user) { User.build_user }
+ it "should apply defaults to user" do
+ user.projects_limit.should == 42
+ user.can_create_group.should be_false
+ user.theme_id.should == Gitlab::Theme::BASIC
+ end
+ end
- describe 'with defaults' do
- let(:user) { User.new.with_defaults }
- it "should apply defaults to user" do
- user.projects_limit.should == 42
- user.can_create_group.should == false
+ describe 'with default overrides' do
+ let(:user) { User.build_user(projects_limit: 123, can_create_group: true, theme_id: Gitlab::Theme::MARS) }
+ it "should apply defaults to user" do
+ user.projects_limit.should == 42
+ user.can_create_group.should be_false
+ user.theme_id.should == Gitlab::Theme::BASIC
+ end
+ end
end
end
diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb
index c09d78993e1..e42c0567ef6 100644
--- a/spec/requests/api/users_spec.rb
+++ b/spec/requests/api/users_spec.rb
@@ -64,7 +64,9 @@ describe API::API do
expect {
post api("/users", admin), attr
}.to change { User.count }.by(1)
- User.find_by_username(attr[:username]).projects_limit.should == limit
+ user = User.find_by_username(attr[:username])
+ user.projects_limit.should == limit
+ user.theme_id.should == Gitlab::Theme::BASIC
Gitlab.config.gitlab.unstub(:default_projects_limit)
end
--
GitLab