Commit 92bf9d78 authored by Małgorzata Ksionek's avatar Małgorzata Ksionek Committed by Yorick Peterse

Add checking for email_verified key

Fix rubocop offences and add changelog

Add email_verified key for feature specs

Add code review remarks

Add code review remarks

Fix specs
parent 5304da4e
......@@ -73,6 +73,14 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController
end
end
def salesforce
if oauth.dig('extra', 'email_verified')
handle_omniauth
else
fail_salesforce_login
end
end
private
def omniauth_flow(auth_module, identity_linker: nil)
......@@ -173,7 +181,15 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController
end
def fail_auth0_login
flash[:alert] = _('Wrong extern UID provided. Make sure Auth0 is configured correctly.')
fail_login_with_message(_('Wrong extern UID provided. Make sure Auth0 is configured correctly.'))
end
def fail_salesforce_login
fail_login_with_message(_('Email not verified. Please verify your email in Salesforce.'))
end
def fail_login_with_message(message)
flash[:alert] = message
redirect_to new_user_session_path
end
......
---
title: Prevent bypassing email verification using Salesforce
merge_request:
author:
type: security
......@@ -5569,6 +5569,9 @@ msgstr ""
msgid "Email domain is not editable in subgroups. Value inherited from top-level parent group."
msgstr ""
msgid "Email not verified. Please verify your email in Salesforce."
msgstr ""
msgid "Email patch"
msgstr ""
......
......@@ -7,9 +7,10 @@ describe OmniauthCallbacksController, type: :controller do
describe 'omniauth' do
let(:user) { create(:omniauth_user, extern_uid: extern_uid, provider: provider) }
let(:additional_info) { {} }
before do
@original_env_config_omniauth_auth = mock_auth_hash(provider.to_s, +extern_uid, user.email)
@original_env_config_omniauth_auth = mock_auth_hash(provider.to_s, extern_uid, user.email, additional_info: additional_info )
stub_omniauth_provider(provider, context: request)
end
......@@ -109,6 +110,14 @@ describe OmniauthCallbacksController, type: :controller do
end
context 'strategies' do
shared_context 'sign_up' do
let(:user) { double(email: 'new@example.com') }
before do
stub_omniauth_setting(block_auto_created_users: false)
end
end
context 'github' do
let(:extern_uid) { 'my-uid' }
let(:provider) { :github }
......@@ -146,14 +155,6 @@ describe OmniauthCallbacksController, type: :controller do
end
end
shared_context 'sign_up' do
let(:user) { double(email: +'new@example.com') }
before do
stub_omniauth_setting(block_auto_created_users: false)
end
end
context 'sign up' do
include_context 'sign_up'
......@@ -215,6 +216,33 @@ describe OmniauthCallbacksController, type: :controller do
expect(controller).to set_flash[:alert].to('Wrong extern UID provided. Make sure Auth0 is configured correctly.')
end
end
context 'salesforce' do
let(:extern_uid) { 'my-uid' }
let(:provider) { :salesforce }
let(:additional_info) { { extra: { email_verified: false } } }
context 'without verified email' do
it 'does not allow sign in' do
post 'salesforce'
expect(request.env['warden']).not_to be_authenticated
expect(response.status).to eq(302)
expect(controller).to set_flash[:alert].to('Email not verified. Please verify your email in Salesforce.')
end
end
context 'with verified email' do
include_context 'sign_up'
let(:additional_info) { { extra: { email_verified: true } } }
it 'allows sign in' do
post 'salesforce'
expect(request.env['warden']).to be_authenticated
end
end
end
end
end
......
......@@ -22,8 +22,8 @@ describe 'OAuth Login', :js, :allow_forgery_protection do
with_omniauth_full_host { example.run }
end
def login_with_provider(provider, enter_two_factor: false)
login_via(provider.to_s, user, uid, remember_me: remember_me)
def login_with_provider(provider, enter_two_factor: false, additional_info: {})
login_via(provider.to_s, user, uid, remember_me: remember_me, additional_info: additional_info)
enter_code(user.current_otp) if enter_two_factor
end
......@@ -33,6 +33,7 @@ describe 'OAuth Login', :js, :allow_forgery_protection do
let(:remember_me) { false }
let(:user) { create(:omniauth_user, extern_uid: uid, provider: provider.to_s) }
let(:two_factor_user) { create(:omniauth_user, :two_factor, extern_uid: uid, provider: provider.to_s) }
provider == :salesforce ? let(:additional_info) { { extra: { email_verified: true } } } : let(:additional_info) { {} }
before do
stub_omniauth_config(provider)
......@@ -41,7 +42,7 @@ describe 'OAuth Login', :js, :allow_forgery_protection do
context 'when two-factor authentication is disabled' do
it 'logs the user in' do
login_with_provider(provider)
login_with_provider(provider, additional_info: additional_info)
expect(current_path).to eq root_path
end
......@@ -51,20 +52,20 @@ describe 'OAuth Login', :js, :allow_forgery_protection do
let(:user) { two_factor_user }
it 'logs the user in' do
login_with_provider(provider, enter_two_factor: true)
login_with_provider(provider, additional_info: additional_info, enter_two_factor: true)
expect(current_path).to eq root_path
end
it 'when bypass-two-factor is enabled' do
allow(Gitlab.config.omniauth).to receive_messages(allow_bypass_two_factor: true)
login_via(provider.to_s, user, uid, remember_me: false)
login_via(provider.to_s, user, uid, remember_me: false, additional_info: additional_info)
expect(current_path).to eq root_path
end
it 'when bypass-two-factor is disabled' do
allow(Gitlab.config.omniauth).to receive_messages(allow_bypass_two_factor: false)
login_with_provider(provider, enter_two_factor: true)
login_with_provider(provider, enter_two_factor: true, additional_info: additional_info)
expect(current_path).to eq root_path
end
end
......@@ -74,7 +75,7 @@ describe 'OAuth Login', :js, :allow_forgery_protection do
context 'when two-factor authentication is disabled' do
it 'remembers the user after a browser restart' do
login_with_provider(provider)
login_with_provider(provider, additional_info: additional_info)
clear_browser_session
......@@ -87,7 +88,7 @@ describe 'OAuth Login', :js, :allow_forgery_protection do
let(:user) { two_factor_user }
it 'remembers the user after a browser restart' do
login_with_provider(provider, enter_two_factor: true)
login_with_provider(provider, enter_two_factor: true, additional_info: additional_info)
clear_browser_session
......@@ -100,7 +101,7 @@ describe 'OAuth Login', :js, :allow_forgery_protection do
context 'when "remember me" is not checked' do
context 'when two-factor authentication is disabled' do
it 'does not remember the user after a browser restart' do
login_with_provider(provider)
login_with_provider(provider, additional_info: additional_info)
clear_browser_session
......@@ -113,7 +114,7 @@ describe 'OAuth Login', :js, :allow_forgery_protection do
let(:user) { two_factor_user }
it 'does not remember the user after a browser restart' do
login_with_provider(provider, enter_two_factor: true)
login_with_provider(provider, enter_two_factor: true, additional_info: additional_info)
clear_browser_session
......
......@@ -87,8 +87,8 @@ module LoginHelpers
click_button "Sign in"
end
def login_via(provider, user, uid, remember_me: false)
mock_auth_hash(provider, uid, user.email)
def login_via(provider, user, uid, remember_me: false, additional_info: {})
mock_auth_hash(provider, uid, user.email, additional_info: additional_info)
visit new_user_session_path
expect(page).to have_content('Sign in with')
......@@ -107,9 +107,10 @@ module LoginHelpers
mock_auth_hash(provider, uid, email, response_object: response_object)
end
def configure_mock_auth(provider, uid, email, response_object: nil)
def configure_mock_auth(provider, uid, email, response_object: nil, additional_info: {})
# 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,
......@@ -132,11 +133,11 @@ module LoginHelpers
},
response_object: response_object
}
})
}).merge(additional_info) { |_, old_hash, new_hash| old_hash.merge(new_hash) }
end
def mock_auth_hash(provider, uid, email, response_object: nil)
configure_mock_auth(provider, uid, email, response_object: response_object)
def mock_auth_hash(provider, uid, email, additional_info: {}, response_object: nil)
configure_mock_auth(provider, uid, email, additional_info: additional_info, response_object: response_object)
original_env_config_omniauth_auth = Rails.application.env_config['omniauth.auth']
Rails.application.env_config['omniauth.auth'] = OmniAuth.config.mock_auth[provider.to_sym]
......
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