Merge branch 'fix-oauth-block' into 'master'

Fix user being blocked after oauth sign-in

See merge request !1190

2 files changed, 89 insertions, 19 deletions

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'
+        end
+      end
 
-        expect(gl_user).to be_valid
-        expect(gl_user.extern_uid).to eql uid
-        expect(gl_user.provider).to eql 'twitter'
+      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