Add regressiontest to verify allow_single_sign_on setting

verification for #1677

Since testing omniauth_callback_controller.rb is very difficult, the logic
is moved to the models
parent fad588f2
...@@ -49,22 +49,19 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController ...@@ -49,22 +49,19 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController
redirect_to profile_path redirect_to profile_path
else else
@user = Gitlab::OAuth::User.new(oauth) @user = Gitlab::OAuth::User.new(oauth)
@user.save
if Gitlab.config.omniauth['allow_single_sign_on'] && @user.new?
@user.save
end
# Only allow properly saved users to login. # Only allow properly saved users to login.
if @user.persisted? && @user.valid? if @user.persisted? && @user.valid?
sign_in_and_redirect(@user.gl_user) 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(", ") error_message = @user.gl_user.errors.map{ |attribute, message| "#{attribute} #{message}" }.join(", ")
redirect_to omniauth_error_path(oauth['provider'], error: error_message) and return 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
end end
rescue StandardError
flash[:notice] = "There's no such user!"
redirect_to new_user_session_path
end end
def oauth def oauth
......
...@@ -13,7 +13,7 @@ module Gitlab ...@@ -13,7 +13,7 @@ module Gitlab
end end
def persisted? def persisted?
gl_user.persisted? gl_user.try(:persisted?)
end end
def new? def new?
...@@ -21,10 +21,12 @@ module Gitlab ...@@ -21,10 +21,12 @@ module Gitlab
end end
def valid? def valid?
gl_user.valid? gl_user.try(:valid?)
end end
def save def save
unauthorized_to_create unless gl_user
gl_user.save! gl_user.save!
log.info "(OAuth) saving user #{auth_hash.email} from login with extern_uid => #{auth_hash.uid}" log.info "(OAuth) saving user #{auth_hash.email} from login with extern_uid => #{auth_hash.uid}"
gl_user.block if needs_blocking? gl_user.block if needs_blocking?
...@@ -36,7 +38,12 @@ module Gitlab ...@@ -36,7 +38,12 @@ module Gitlab
end end
def gl_user 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 end
protected protected
...@@ -77,6 +84,10 @@ module Gitlab ...@@ -77,6 +84,10 @@ module Gitlab
def model def model
::User ::User
end end
def raise_unauthorized_to_create
raise StandardError.new("Unauthorized to create user, signup disabled for #{auth_hash.provider}")
end
end end
end end
end end
...@@ -31,17 +31,8 @@ describe Gitlab::OAuth::User do ...@@ -31,17 +31,8 @@ describe Gitlab::OAuth::User do
describe :save do describe :save do
let(:provider) { 'twitter' } let(:provider) { 'twitter' }
it "creates a user from Omniauth" do context "with allow_single_sign_on enabled" do
oauth_user.save before { Gitlab.config.omniauth.stub allow_single_sign_on: true }
expect(gl_user).to be_valid
expect(gl_user.extern_uid).to eql uid
expect(gl_user.provider).to eql 'twitter'
end
end
context "twitter" do
let(:provider) { 'twitter' }
it "creates a user from Omniauth" do it "creates a user from Omniauth" do
oauth_user.save oauth_user.save
...@@ -51,5 +42,11 @@ describe Gitlab::OAuth::User do ...@@ -51,5 +42,11 @@ describe Gitlab::OAuth::User do
expect(gl_user.provider).to eql 'twitter' expect(gl_user.provider).to eql 'twitter'
end end
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
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment