diff options
author | Dmitriy Zaporozhets <dzaporozhets@gitlab.com> | 2014-10-17 10:10:01 +0000 |
---|---|---|
committer | Dmitriy Zaporozhets <dzaporozhets@gitlab.com> | 2014-10-17 10:10:01 +0000 |
commit | fd22d32bcd177fba4413c2f4c6eb4940c49a0aac (patch) | |
tree | c5d37dcb26e2c4599c8cfbf43e26481bdd451ea4 | |
parent | 32d0bf3afdd25eaff1f3bd2e80696e39b5b69c35 (diff) | |
parent | d9bfebc0e87ef426aea7eb4fdd1338f04b106354 (diff) | |
download | gitlab-ce-fd22d32bcd177fba4413c2f4c6eb4940c49a0aac.tar.gz |
Merge branch 'issue-respect-single-sign-on' into 'master'
Issue respect single sign on
Adds regression test for #1677
See merge request !1188
-rw-r--r-- | app/controllers/omniauth_callbacks_controller.rb | 13 | ||||
-rw-r--r-- | lib/gitlab/oauth/user.rb | 17 | ||||
-rw-r--r-- | spec/lib/gitlab/oauth/user_spec.rb | 21 |
3 files changed, 28 insertions, 23 deletions
diff --git a/app/controllers/omniauth_callbacks_controller.rb b/app/controllers/omniauth_callbacks_controller.rb index 589f8387b03..58d1e37f655 100644 --- a/app/controllers/omniauth_callbacks_controller.rb +++ b/app/controllers/omniauth_callbacks_controller.rb @@ -49,22 +49,19 @@ 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? + else @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 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..18ec63a62a2 100644 --- a/lib/gitlab/oauth/user.rb +++ b/lib/gitlab/oauth/user.rb @@ -13,7 +13,7 @@ module Gitlab end def persisted? - gl_user.persisted? + gl_user.try(:persisted?) end def new? @@ -21,10 +21,12 @@ module Gitlab end def valid? - gl_user.valid? + gl_user.try(:valid?) end def save + unauthorized_to_create unless gl_user + 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? @@ -36,7 +38,12 @@ module Gitlab end def gl_user - @user ||= find_by_uid_and_provider || build_new_user + @user ||= find_by_uid_and_provider + + if Gitlab.config.omniauth.allow_single_sign_on + @user ||= build_new_user + end + @user end protected @@ -77,6 +84,10 @@ module Gitlab 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..e004d6edfab 100644 --- a/spec/lib/gitlab/oauth/user_spec.rb +++ b/spec/lib/gitlab/oauth/user_spec.rb @@ -29,19 +29,10 @@ 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 + let(:provider) { 'twitter' } - expect(gl_user).to be_valid - expect(gl_user.extern_uid).to eql uid - expect(gl_user.provider).to eql 'ldap' - end - end - - context "twitter" do - let(:provider) { 'twitter' } + 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 @@ -51,5 +42,11 @@ describe Gitlab::OAuth::User do 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 end |