Commit 488a7f59 authored by Robert Speicher's avatar Robert Speicher

Merge branch 'add-2fa-check-to-oauth' into 'master'

Add 2FA check to the OAuth authentication mechanism

Needed for https://gitlab.com/gitlab-org/gitlab-ce/issues/19312

2FA checks were not being performed when logging in via any of the OAuth providers. Just LDAP had the check. This MR fixes that.

See merge request !1976
parents bd7d6124 24cf6b9f
...@@ -107,7 +107,11 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController ...@@ -107,7 +107,11 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController
# Only allow properly saved users to login. # Only allow properly saved users to login.
if @user.persisted? && @user.valid? if @user.persisted? && @user.valid?
log_audit_event(@user, with: oauth['provider']) log_audit_event(@user, with: oauth['provider'])
sign_in_and_redirect(@user) if @user.two_factor_enabled?
prompt_for_two_factor(@user)
else
sign_in_and_redirect(@user)
end
else else
error_message = @user.errors.full_messages.to_sentence error_message = @user.errors.full_messages.to_sentence
......
...@@ -28,6 +28,11 @@ feature 'Login', feature: true do ...@@ -28,6 +28,11 @@ feature 'Login', feature: true do
end end
describe 'with two-factor authentication' do 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 context 'with valid username/password' do
let(:user) { create(:user, :two_factor) } let(:user) { create(:user, :two_factor) }
...@@ -36,11 +41,6 @@ feature 'Login', feature: true do ...@@ -36,11 +41,6 @@ feature 'Login', feature: true do
expect(page).to have_content('Two-Factor Authentication') expect(page).to have_content('Two-Factor Authentication')
end 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 it 'does not show a "You are already signed in." error message' do
enter_code(user.current_otp) enter_code(user.current_otp)
expect(page).not_to have_content('You are already signed in.') expect(page).not_to have_content('You are already signed in.')
...@@ -108,6 +108,39 @@ feature 'Login', feature: true do ...@@ -108,6 +108,39 @@ feature 'Login', feature: true do
end end
end end
end end
context 'logging in via OAuth' do
def saml_config
OpenStruct.new(name: 'saml', label: 'saml', args: {
assertion_consumer_service_url: 'https://localhost:3443/users/auth/saml/callback',
idp_cert_fingerprint: '26:43:2C:47:AF:F0:6B:D0:07:9C:AD:A3:74:FE:5D:94:5F:4E:9E:52',
idp_sso_target_url: 'https://idp.example.com/sso/saml',
issuer: 'https://localhost:3443/',
name_identifier_format: 'urn:oasis:names:tc:SAML:2.0:nameid-format:transient'
})
end
def stub_omniauth_config(messages)
Rails.application.env_config['devise.mapping'] = Devise.mappings[:user]
Rails.application.routes.disable_clear_and_finalize = true
Rails.application.routes.draw do
post '/users/auth/saml' => 'omniauth_callbacks#saml'
end
allow(Gitlab::OAuth::Provider).to receive_messages(providers: [:saml], config_for: saml_config)
allow(Gitlab.config.omniauth).to receive_messages(messages)
allow_any_instance_of(Object).to receive(:user_omniauth_authorize_path).with('saml').and_return('/users/auth/saml')
end
it 'should show 2FA prompt after OAuth login' do
stub_omniauth_config(enabled: true, auto_link_saml_user: true, allow_single_sign_on: ['saml'], providers: [saml_config])
user = create(:omniauth_user, :two_factor, extern_uid: 'my-uid', provider: 'saml')
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 end
describe 'without two-factor authentication' do describe 'without two-factor authentication' do
......
...@@ -37,6 +37,40 @@ module LoginHelpers ...@@ -37,6 +37,40 @@ module LoginHelpers
Thread.current[:current_user] = user Thread.current[:current_user] = user
end end
def login_via(provider, user, uid)
mock_auth_hash(provider, uid, user.email)
visit new_user_session_path
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'
},
extra: {
raw_info: {
info: {
name: 'mockuser',
email: email,
image: 'mock_user_thumbnail_url'
}
}
}
})
Rails.application.env_config['omniauth.auth'] = OmniAuth.config.mock_auth[:saml]
end
# Requires Javascript driver. # Requires Javascript driver.
def logout def logout
find(".header-user-dropdown-toggle").click find(".header-user-dropdown-toggle").click
......
OmniAuth.config.test_mode = true
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