From 5dae40f579f66fdc060de633b183ede7bd8b2ce4 Mon Sep 17 00:00:00 2001 From: Izaak Alpert 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