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

Fix user being blocked after oauth sign-in

See merge request !1190

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
 
+        if needs_blocking?
           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?
+          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

spec/lib/gitlab/oauth/user_spec.rb

@@ -31,6 +31,7 @@ describe Gitlab::OAuth::User do
   describe :save do
     let(:provider) { 'twitter' }
 
+    describe 'signup' do
       context "with allow_single_sign_on enabled" do
         before { Gitlab.config.omniauth.stub allow_single_sign_on: true }
 
@@ -49,4 +50,59 @@ describe Gitlab::OAuth::User do
         end
       end
     end
+
+    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
 end