Merge pull request #4743 from karlhungus/feature-users-api-respect-defaults

Update User api to respect default settings

Merge pull request #4743 from karlhungus/feature-users-api-respect-defaults
Update User api to respect default settings
Dmitriy Zaporozhets
2 parents fc6ed495 88d4559e
Showing 7 changed files with 157 additions and 32 deletions Show diff stats
app/controllers/admin/users_controller.rb
app/models/user.rb
lib/api/users.rb
lib/gitlab/auth.rb
lib/gitlab/oauth/user.rb
spec/models/user_spec.rb
spec/requests/api/users_spec.rb
@@ -13,7 +13,7 @@ class Admin::UsersController &lt; Admin::ApplicationController
   end
   def new
-    @user = User.new.with_defaults
+    @user = User.build_user
   end
   def edit
@@ -44,7 +44,7 @@ class Admin::UsersController &lt; 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
@@ -198,6 +198,21 @@ class User &lt; ActiveRecord::Base
         User.find_by_username(name_or_id)
       end
     end
+
+    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
   #
@@ -208,14 +223,6 @@ class User &lt; 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
@@ -375,4 +382,10 @@ class User &lt; ActiveRecord::Base
       group.owners == [self]
     end
   end
+
+  def with_defaults
+    User.defaults.each do |k,v|
+      self.send("#{k}=",v)
+    end
+  end
 end
@@ -45,9 +45,8 @@ module API
       post do
         authenticated_as_admin!
         required_attributes! [:email, :password, :name, :username]
-
         attrs = attributes_for_keys [:email, :name, :password, :skype, :linkedin, :twitter, :projects_limit, :username, :extern_uid, :provider, :bio]
-        user = User.new attrs, as: :admin
+        user = User.build_user(attrs, as: :admin)
         if user.save
           present user, with: Entities::User
         else
@@ -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
@@ -27,7 +27,7 @@ module Gitlab
             password_confirmation: password,
           }
-          user = model.new(opts, as: :admin).with_defaults
+          user = model.build_user(opts, as: :admin)
           user.save!
           log.info "(OAuth) Creating user #{email} from login with extern_uid => #{uid}"
@@ -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
@@ -57,6 +57,19 @@ describe API::API do
       response.status.should == 201
     end
+    it "creating a user should respect default project limit" do
+      limit = 123456
+      Gitlab.config.gitlab.stub(:default_projects_limit).and_return(limit)
+      attr = attributes_for(:user )
+      expect {
+        post api("/users", admin), attr
+      }.to change { User.count }.by(1)
+      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
+
     it "should not create user with invalid email" do
       post api("/users", admin), { email: "invalid email", password: 'password' }
       response.status.should == 400
	@@ -13,7 +13,7 @@ class Admin::UsersController < Admin::ApplicationController		@@ -13,7 +13,7 @@ class Admin::UsersController < Admin::ApplicationController
13	end	13	end
14		14
15	def new	15	def new
16	- @user = User.new.with_defaults	16	+ @user = User.build_user
17	end	17	end
18		18
19	def edit	19	def edit
	@@ -44,7 +44,7 @@ class Admin::UsersController < Admin::ApplicationController		@@ -44,7 +44,7 @@ class Admin::UsersController < Admin::ApplicationController
44	password_expires_at: Time.now	44	password_expires_at: Time.now
45	}	45	}
46		46
47	- @user = User.new(params[:user].merge(opts), as: :admin)	47	+ @user = User.build_user(params[:user].merge(opts), as: :admin)
48	@user.admin = (admin && admin.to_i > 0)	48	@user.admin = (admin && admin.to_i > 0)
49	@user.created_by_id = current_user.id	49	@user.created_by_id = current_user.id
50		50
	@@ -13,6 +13,72 @@ module Gitlab		@@ -13,6 +13,72 @@ module Gitlab
13	end	13	end
14	end	14	end
15		15
		16	+ def find_for_ldap_auth(auth, signed_in_resource = nil)
		17	+ uid = auth.info.uid
		18	+ provider = auth.provider
		19	+ email = auth.info.email.downcase unless auth.info.email.nil?
		20	+ raise OmniAuth::Error, "LDAP accounts must provide an uid and email address" if uid.nil? or email.nil?
		21	+
		22	+ if @user = User.find_by_extern_uid_and_provider(uid, provider)
		23	+ @user
		24	+ elsif @user = User.find_by_email(email)
		25	+ log.info "Updating legacy LDAP user #{email} with extern_uid => #{uid}"
		26	+ @user.update_attributes(extern_uid: uid, provider: provider)
		27	+ @user
		28	+ else
		29	+ create_from_omniauth(auth, true)
		30	+ end
		31	+ end
		32	+
		33	+ def create_from_omniauth(auth, ldap = false)
		34	+ provider = auth.provider
		35	+ uid = auth.info.uid \|\| auth.uid
		36	+ uid = uid.to_s.force_encoding("utf-8")
		37	+ name = auth.info.name.to_s.force_encoding("utf-8")
		38	+ email = auth.info.email.to_s.downcase unless auth.info.email.nil?
		39	+
		40	+ ldap_prefix = ldap ? '(LDAP) ' : ''
		41	+ raise OmniAuth::Error, "#{ldap_prefix}#{provider} does not provide an email"\
		42	+ " address" if auth.info.email.blank?
		43	+
		44	+ log.info "#{ldap_prefix}Creating user from #{provider} login"\
		45	+ " {uid => #{uid}, name => #{name}, email => #{email}}"
		46	+ password = Devise.friendly_token[0, 8].downcase
		47	+ @user = User.build_user({
		48	+ extern_uid: uid,
		49	+ provider: provider,
		50	+ name: name,
		51	+ username: email.match(/^[^@]*/)[0],
		52	+ email: email,
		53	+ password: password,
		54	+ password_confirmation: password,
		55	+ }, as: :admin)
		56	+ @user.save!
		57	+
		58	+ if Gitlab.config.omniauth['block_auto_created_users'] && !ldap
		59	+ @user.block
		60	+ end
		61	+
		62	+ @user
		63	+ end
		64	+
		65	+ def find_or_new_for_omniauth(auth)
		66	+ provider, uid = auth.provider, auth.uid
		67	+ email = auth.info.email.downcase unless auth.info.email.nil?
		68	+
		69	+ if @user = User.find_by_provider_and_extern_uid(provider, uid)
		70	+ @user
		71	+ elsif @user = User.find_by_email(email)
		72	+ @user.update_attributes(extern_uid: uid, provider: provider)
		73	+ @user
		74	+ else
		75	+ if Gitlab.config.omniauth['allow_single_sign_on']
		76	+ @user = create_from_omniauth(auth)
		77	+ @user
		78	+ end
		79	+ end
		80	+ end
		81	+
16	def log	82	def log
17	Gitlab::AppLogger	83	Gitlab::AppLogger
18	end	84	end
	@@ -27,7 +27,7 @@ module Gitlab		@@ -27,7 +27,7 @@ module Gitlab
27	password_confirmation: password,	27	password_confirmation: password,
28	}	28	}
29		29
30	- user = model.new(opts, as: :admin).with_defaults	30	+ user = model.build_user(opts, as: :admin)
31	user.save!	31	user.save!
32	log.info "(OAuth) Creating user #{email} from login with extern_uid => #{uid}"	32	log.info "(OAuth) Creating user #{email} from login with extern_uid => #{uid}"
33		33
	@@ -196,29 +196,63 @@ describe User do		@@ -196,29 +196,63 @@ describe User do
196	it { User.not_in_project(@project).should include(@user, @project.owner) }	196	it { User.not_in_project(@project).should include(@user, @project.owner) }
197	end	197	end
198		198
199	- describe 'normal user' do
200	- let(:user) { create(:user, name: 'John Smith') }	199	+ describe 'user creation' do
		200	+ describe 'normal user' do
		201	+ let(:user) { create(:user, name: 'John Smith') }
201		202
202	- it { user.is_admin?.should be_false }
203	- it { user.require_ssh_key?.should be_true }
204	- it { user.can_create_group?.should be_true }
205	- it { user.can_create_project?.should be_true }
206	- it { user.first_name.should == 'John' }
207	- end	203	+ it { user.is_admin?.should be_false }
		204	+ it { user.require_ssh_key?.should be_true }
		205	+ it { user.can_create_group?.should be_true }
		206	+ it { user.can_create_project?.should be_true }
		207	+ it { user.first_name.should == 'John' }
		208	+ end
208		209
209	- describe 'without defaults' do
210	- let(:user) { User.new }
211	- it "should not apply defaults to user" do
212	- user.projects_limit.should == 10
213	- user.can_create_group.should == true	210	+ describe 'without defaults' do
		211	+ let(:user) { User.new }
		212	+ it "should not apply defaults to user" do
		213	+ user.projects_limit.should == 10
		214	+ user.can_create_group.should be_true
		215	+ user.theme_id.should == Gitlab::Theme::BASIC
		216	+ end
214	end	217	end
215	- end	218	+ context 'as admin' do
		219	+ describe 'with defaults' do
		220	+ let(:user) { User.build_user({}, as: :admin) }
		221	+ it "should apply defaults to user" do
		222	+ user.projects_limit.should == 42
		223	+ user.can_create_group.should be_false
		224	+ user.theme_id.should == Gitlab::Theme::BASIC
		225	+ end
		226	+ end
		227	+
		228	+ describe 'with default overrides' do
		229	+ let(:user) { User.build_user({projects_limit: 123, can_create_group: true, can_create_team: true, theme_id: Gitlab::Theme::MARS}, as: :admin) }
		230	+ it "should apply defaults to user" do
		231	+ user.projects_limit.should == 123
		232	+ user.can_create_group.should be_true
		233	+ user.theme_id.should == Gitlab::Theme::MARS
		234	+ end
		235	+ end
		236	+ end
		237	+
		238	+ context 'as user' do
		239	+ describe 'with defaults' do
		240	+ let(:user) { User.build_user }
		241	+ it "should apply defaults to user" do
		242	+ user.projects_limit.should == 42
		243	+ user.can_create_group.should be_false
		244	+ user.theme_id.should == Gitlab::Theme::BASIC
		245	+ end
		246	+ end
216		247
217	- describe 'with defaults' do
218	- let(:user) { User.new.with_defaults }
219	- it "should apply defaults to user" do
220	- user.projects_limit.should == 42
221	- user.can_create_group.should == false	248	+ describe 'with default overrides' do
		249	+ let(:user) { User.build_user(projects_limit: 123, can_create_group: true, theme_id: Gitlab::Theme::MARS) }
		250	+ it "should apply defaults to user" do
		251	+ user.projects_limit.should == 42
		252	+ user.can_create_group.should be_false
		253	+ user.theme_id.should == Gitlab::Theme::BASIC
		254	+ end
		255	+ end
222	end	256	end
223	end	257	end
224		258
	@@ -57,6 +57,19 @@ describe API::API do		@@ -57,6 +57,19 @@ describe API::API do
57	response.status.should == 201	57	response.status.should == 201
58	end	58	end
59		59
		60	+ it "creating a user should respect default project limit" do
		61	+ limit = 123456
		62	+ Gitlab.config.gitlab.stub(:default_projects_limit).and_return(limit)
		63	+ attr = attributes_for(:user )
		64	+ expect {
		65	+ post api("/users", admin), attr
		66	+ }.to change { User.count }.by(1)
		67	+ user = User.find_by_username(attr[:username])
		68	+ user.projects_limit.should == limit
		69	+ user.theme_id.should == Gitlab::Theme::BASIC
		70	+ Gitlab.config.gitlab.unstub(:default_projects_limit)
		71	+ end
		72	+
60	it "should not create user with invalid email" do	73	it "should not create user with invalid email" do
61	post api("/users", admin), { email: "invalid email", password: 'password' }	74	post api("/users", admin), { email: "invalid email", password: 'password' }
62	response.status.should == 400	75	response.status.should == 400