Added tests for 2FA check on OAuth request

app/controllers/omniauth_callbacks_controller.rb

@@ -107,8 +107,11 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController
     # Only allow properly saved users to login.
     if @user.persisted? && @user.valid?
       log_audit_event(@user, with: oauth['provider'])
-      prompt_for_two_factor(@user) and return if @user.two_factor_enabled?
+      if @user.two_factor_enabled?
+        prompt_for_two_factor(@user)
+      else
         sign_in_and_redirect(@user)
+      end
     else
       error_message = @user.errors.full_messages.to_sentence

spec/features/login_spec.rb

@@ -28,6 +28,11 @@ feature 'Login', feature: true do
   end
   describe 'with two-factor authentication' do
+    def enter_code(code)
+      fill_in 'Two-Factor Authentication code', with: code
+      click_button 'Verify code'
+    end
+
     context 'with valid username/password' do
       let(:user) { create(:user, :two_factor) }
@@ -36,11 +41,6 @@ feature 'Login', feature: true do
         expect(page).to have_content('Two-Factor Authentication')
       end
-      def enter_code(code)
-        fill_in 'Two-Factor Authentication code', with: code
-        click_button 'Verify code'
-      end
-
       it 'does not show a "You are already signed in." error message' do
         enter_code(user.current_otp)
         expect(page).not_to have_content('You are already signed in.')
@@ -108,6 +108,22 @@ feature 'Login', feature: true do
         end
       end
     end
+
+    context 'logging in via OAuth' do
+      def stub_omniauth_config(messages)
+        allow(Gitlab.config.omniauth).to receive_messages(messages)
+      end
+
+      it 'should show 2FA prompt after OAuth login' do
+        user = create(:omniauth_user, :two_factor, extern_uid: 'my-uid', provider: 'saml')
+        stub_omniauth_config(enabled: true, auto_link_saml_user: true, allow_single_sign_on: ['saml'], providers: [OpenStruct.new(name: 'saml', label: 'saml', args: {})])
+        login_via('saml', user, 'my-uid')
+
+        expect(page).to have_content('Two-Factor Authentication')
+        enter_code(user.current_otp)
+        expect(current_path).to eq root_path
+      end
+    end
   end
   describe 'without two-factor authentication' do

spec/spec_helper.rb

@@ -57,3 +57,5 @@ FactoryGirl::SyntaxRunner.class_eval do
 end
 ActiveRecord::Migration.maintain_test_schema!
+
+OmniAuth.config.test_mode = true

spec/support/login_helpers.rb

@@ -37,6 +37,31 @@ module LoginHelpers
     Thread.current[:current_user] = user
   end
+  def login_via(provider, user, uid)
+    mock_auth_hash(provider, uid, user.email)
+    visit new_user_session_path
+    #page.find('.oauth-image-link').click
+    click_link provider
+  end
+
+  def mock_auth_hash(provider, uid, email)
+    # The mock_auth configuration allows you to set per-provider (or default)
+    # authentication hashes to return during integration testing.
+    OmniAuth.config.mock_auth[provider.to_sym] = OmniAuth::AuthHash.new({
+      provider: provider,
+      uid: uid,
+      info: {
+        name: 'mockuser',
+        email: email,
+        image: 'mock_user_thumbnail_url'
+      },
+      credentials: {
+        token: 'mock_token',
+        secret: 'mock_secret'
+      }
+    })
+  end
+
   # Requires Javascript driver.
   def logout
     find(:css, ".fa.fa-sign-out").click