diff --git a/lib/gitlab/oauth/user.rb b/lib/gitlab/oauth/user.rb index 18ec63a62a2..47f62153a50 100644 --- a/lib/gitlab/oauth/user.rb +++ b/lib/gitlab/oauth/user.rb @@ -17,7 +17,7 @@ module Gitlab end def new? - !gl_user.persisted? + !persisted? end def valid? @@ -27,10 +27,14 @@ module Gitlab 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? + 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}" @@ -40,13 +44,27 @@ module Gitlab def gl_user @user ||= find_by_uid_and_provider - if Gitlab.config.omniauth.allow_single_sign_on + 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 @@ -77,10 +95,6 @@ module Gitlab Gitlab::AppLogger end - def needs_blocking? - Gitlab.config.omniauth['block_auto_created_users'] - end - def model ::User end diff --git a/spec/lib/gitlab/oauth/user_spec.rb b/spec/lib/gitlab/oauth/user_spec.rb index e004d6edfab..8a83a1b2588 100644 --- a/spec/lib/gitlab/oauth/user_spec.rb +++ b/spec/lib/gitlab/oauth/user_spec.rb @@ -31,21 +31,77 @@ describe Gitlab::OAuth::User do describe :save do let(:provider) { 'twitter' } - context "with allow_single_sign_on enabled" do - before { Gitlab.config.omniauth.stub allow_single_sign_on: true } + 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 + 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' + 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 "with allow_single_sign_on disabled (Default)" do - it "throws an error" do - expect{ oauth_user.save }.to raise_error StandardError + 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 + + 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_not be_blocked + end + end end end end