Commit 5dae40f579f66fdc060de633b183ede7bd8b2ce4
1 parent
d4d4a78f
Exists in
master
and in
4 other branches
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
Showing
6 changed files
with
143 additions
and
35 deletions
Show diff stats
app/controllers/admin/users_controller.rb
| @@ -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 |
app/models/user.rb
| @@ -199,10 +199,16 @@ class User < ActiveRecord::Base | @@ -199,10 +199,16 @@ class User < ActiveRecord::Base | ||
| 199 | end | 199 | end |
| 200 | end | 200 | end |
| 201 | 201 | ||
| 202 | - def defaults | ||
| 203 | - { 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 } | 202 | + def build_user(attrs = {}, options= {}) |
| 203 | + user = User.new(defaults.merge(attrs), options) | ||
| 204 | + # if not as: :admin force default settings | ||
| 205 | + user.with_defaults unless options[:as] == :admin | ||
| 206 | + user | ||
| 204 | end | 207 | end |
| 205 | 208 | ||
| 209 | + def defaults | ||
| 210 | + { projects_limit: Gitlab.config.gitlab.default_projects_limit, can_create_group: Gitlab.config.gitlab.default_can_create_group, theme_id: Gitlab::Theme::BASIC } | ||
| 211 | + end | ||
| 206 | end | 212 | end |
| 207 | 213 | ||
| 208 | # | 214 | # |
| @@ -213,14 +219,6 @@ class User < ActiveRecord::Base | @@ -213,14 +219,6 @@ class User < ActiveRecord::Base | ||
| 213 | username | 219 | username |
| 214 | end | 220 | end |
| 215 | 221 | ||
| 216 | - def with_defaults | ||
| 217 | - tap do |u| | ||
| 218 | - u.projects_limit = Gitlab.config.gitlab.default_projects_limit | ||
| 219 | - u.can_create_group = Gitlab.config.gitlab.default_can_create_group | ||
| 220 | - u.theme_id = Gitlab::Theme::MARS | ||
| 221 | - end | ||
| 222 | - end | ||
| 223 | - | ||
| 224 | def notification | 222 | def notification |
| 225 | @notification ||= Notification.new(self) | 223 | @notification ||= Notification.new(self) |
| 226 | end | 224 | end |
| @@ -380,4 +378,13 @@ class User < ActiveRecord::Base | @@ -380,4 +378,13 @@ class User < ActiveRecord::Base | ||
| 380 | group.owners == [self] | 378 | group.owners == [self] |
| 381 | end | 379 | end |
| 382 | end | 380 | end |
| 381 | + | ||
| 382 | + :private | ||
| 383 | + | ||
| 384 | + def with_defaults | ||
| 385 | + User.defaults.each do |k,v| | ||
| 386 | + self.send("#{k}=",v) | ||
| 387 | + end | ||
| 388 | + end | ||
| 389 | + | ||
| 383 | end | 390 | end |
lib/api/users.rb
| @@ -45,9 +45,8 @@ module API | @@ -45,9 +45,8 @@ module API | ||
| 45 | post do | 45 | post do |
| 46 | authenticated_as_admin! | 46 | authenticated_as_admin! |
| 47 | required_attributes! [:email, :password, :name, :username] | 47 | required_attributes! [:email, :password, :name, :username] |
| 48 | - | ||
| 49 | - attrs = User.defaults.merge(attributes_for_keys [:email, :name, :password, :skype, :linkedin, :twitter, :projects_limit, :username, :extern_uid, :provider, :bio]) | ||
| 50 | - user = User.new attrs, as: :admin | 48 | + attrs = attributes_for_keys [:email, :name, :password, :skype, :linkedin, :twitter, :projects_limit, :username, :extern_uid, :provider, :bio] |
| 49 | + user = User.build_user(attrs, as: :admin) | ||
| 51 | if user.save | 50 | if user.save |
| 52 | present user, with: Entities::User | 51 | present user, with: Entities::User |
| 53 | else | 52 | else |
lib/gitlab/auth.rb
| @@ -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 |
spec/models/user_spec.rb
| @@ -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 |
spec/requests/api/users_spec.rb
| @@ -64,7 +64,9 @@ describe API::API do | @@ -64,7 +64,9 @@ describe API::API do | ||
| 64 | expect { | 64 | expect { |
| 65 | post api("/users", admin), attr | 65 | post api("/users", admin), attr |
| 66 | }.to change { User.count }.by(1) | 66 | }.to change { User.count }.by(1) |
| 67 | - User.find_by_username(attr[:username]).projects_limit.should == limit | 67 | + user = User.find_by_username(attr[:username]) |
| 68 | + user.projects_limit.should == limit | ||
| 69 | + user.theme_id.should == Gitlab::Theme::BASIC | ||
| 68 | Gitlab.config.gitlab.unstub(:default_projects_limit) | 70 | Gitlab.config.gitlab.unstub(:default_projects_limit) |
| 69 | end | 71 | end |
| 70 | 72 |