diff options
author | Dmitriy Zaporozhets <dzaporozhets@gitlab.com> | 2014-10-17 13:57:28 +0000 |
---|---|---|
committer | Dmitriy Zaporozhets <dzaporozhets@gitlab.com> | 2014-10-17 13:57:28 +0000 |
commit | 564c4138e914abab570ad92c4850dd2b35d37198 (patch) | |
tree | e1cdcf8940605593b68922c5824652e07e39dcc6 | |
parent | 4da88dbb2d8d8c1201a18829e22e71837e39736e (diff) | |
parent | 5a4a1a2fa44630d00f7029ae0bb11d614147af23 (diff) | |
download | gitlab-ce-564c4138e914abab570ad92c4850dd2b35d37198.tar.gz |
Merge branch 'master' into '7-4-stable'
Merge master into 7-4-stable
See merge request !1189
-rw-r--r-- | app/controllers/omniauth_callbacks_controller.rb | 23 | ||||
-rw-r--r-- | lib/gitlab/oauth/user.rb | 47 | ||||
-rw-r--r-- | spec/lib/gitlab/oauth/user_spec.rb | 81 |
3 files changed, 117 insertions, 34 deletions
diff --git a/app/controllers/omniauth_callbacks_controller.rb b/app/controllers/omniauth_callbacks_controller.rb index 589f8387b03..bd4b310fcbf 100644 --- a/app/controllers/omniauth_callbacks_controller.rb +++ b/app/controllers/omniauth_callbacks_controller.rb @@ -49,22 +49,27 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController redirect_to profile_path else @user = Gitlab::OAuth::User.new(oauth) - - if Gitlab.config.omniauth['allow_single_sign_on'] && @user.new? - @user.save - end + @user.save # Only allow properly saved users to login. if @user.persisted? && @user.valid? sign_in_and_redirect(@user.gl_user) - elsif @user.gl_user.errors.any? - error_message = @user.gl_user.errors.map{ |attribute, message| "#{attribute} #{message}" }.join(", ") - redirect_to omniauth_error_path(oauth['provider'], error: error_message) and return else - flash[:notice] = "There's no such user!" - redirect_to new_user_session_path + error_message = + if @user.gl_user.errors.any? + @user.gl_user.errors.map do |attribute, message| + "#{attribute} #{message}" + end.join(", ") + else + '' + end + + redirect_to omniauth_error_path(oauth['provider'], error: error_message) and return end end + rescue StandardError + flash[:notice] = "There's no such user!" + redirect_to new_user_session_path end def oauth diff --git a/lib/gitlab/oauth/user.rb b/lib/gitlab/oauth/user.rb index 133445d3d05..47f62153a50 100644 --- a/lib/gitlab/oauth/user.rb +++ b/lib/gitlab/oauth/user.rb @@ -13,22 +13,28 @@ module Gitlab end def persisted? - gl_user.persisted? + gl_user.try(:persisted?) end def new? - !gl_user.persisted? + !persisted? end def valid? - gl_user.valid? + gl_user.try(:valid?) end def save - gl_user.save! - log.info "(OAuth) saving user #{auth_hash.email} from login with extern_uid => #{auth_hash.uid}" - gl_user.block if needs_blocking? + unauthorized_to_create unless gl_user + + if needs_blocking? + gl_user.save! + gl_user.block + else + gl_user.save! + end + log.info "(OAuth) saving user #{auth_hash.email} from login with extern_uid => #{auth_hash.uid}" gl_user rescue ActiveRecord::RecordInvalid => e log.info "(OAuth) Error saving user: #{gl_user.errors.full_messages}" @@ -36,10 +42,29 @@ module Gitlab end def gl_user - @user ||= find_by_uid_and_provider || build_new_user + @user ||= find_by_uid_and_provider + + if signup_enabled? + @user ||= build_new_user + end + + @user end protected + + def needs_blocking? + new? && block_after_signup? + end + + def signup_enabled? + Gitlab.config.omniauth.allow_single_sign_on + end + + def block_after_signup? + Gitlab.config.omniauth.block_auto_created_users + end + def auth_hash=(auth_hash) @auth_hash = AuthHash.new(auth_hash) end @@ -70,13 +95,13 @@ module Gitlab Gitlab::AppLogger end - def needs_blocking? - Gitlab.config.omniauth['block_auto_created_users'] - end - def model ::User end + + def raise_unauthorized_to_create + raise StandardError.new("Unauthorized to create user, signup disabled for #{auth_hash.provider}") + end end end end diff --git a/spec/lib/gitlab/oauth/user_spec.rb b/spec/lib/gitlab/oauth/user_spec.rb index e4e96fd9f49..8a83a1b2588 100644 --- a/spec/lib/gitlab/oauth/user_spec.rb +++ b/spec/lib/gitlab/oauth/user_spec.rb @@ -29,26 +29,79 @@ describe Gitlab::OAuth::User do end describe :save do - context "LDAP" do - let(:provider) { 'ldap' } - it "creates a user from LDAP" do - oauth_user.save - - expect(gl_user).to be_valid - expect(gl_user.extern_uid).to eql uid - expect(gl_user.provider).to eql 'ldap' + let(:provider) { 'twitter' } + + describe 'signup' do + context "with allow_single_sign_on enabled" do + before { Gitlab.config.omniauth.stub allow_single_sign_on: true } + + it "creates a user from Omniauth" do + oauth_user.save + + expect(gl_user).to be_valid + expect(gl_user.extern_uid).to eql uid + expect(gl_user.provider).to eql 'twitter' + end + end + + context "with allow_single_sign_on disabled (Default)" do + it "throws an error" do + expect{ oauth_user.save }.to raise_error StandardError + end end end - context "twitter" do + describe 'blocking' do let(:provider) { 'twitter' } + before { Gitlab.config.omniauth.stub allow_single_sign_on: true } + + context 'signup' do + context 'dont block on create' do + before { Gitlab.config.omniauth.stub block_auto_created_users: false } + + it do + oauth_user.save + gl_user.should be_valid + gl_user.should_not be_blocked + end + end + + context 'block on create' do + before { Gitlab.config.omniauth.stub block_auto_created_users: true } + + it do + oauth_user.save + gl_user.should be_valid + gl_user.should be_blocked + end + end + end + + context 'sign-in' do + before do + oauth_user.save + oauth_user.gl_user.activate + end + + context 'dont block on create' do + before { Gitlab.config.omniauth.stub block_auto_created_users: false } + + it do + oauth_user.save + gl_user.should be_valid + gl_user.should_not be_blocked + end + end - it "creates a user from Omniauth" do - oauth_user.save + context 'block on create' do + before { Gitlab.config.omniauth.stub block_auto_created_users: true } - expect(gl_user).to be_valid - expect(gl_user.extern_uid).to eql uid - expect(gl_user.provider).to eql 'twitter' + it do + oauth_user.save + gl_user.should be_valid + gl_user.should_not be_blocked + end + end end end end |