Merge branch 'security-bypass-email-verification-using-salesforce' into '12-3-stable'

Prevent Bypassing Email Verification using Salesforce

See merge request gitlab/gitlabhq!3395

app/controllers/omniauth_callbacks_controller.rb

@@ -75,6 +75,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)
@@ -174,7 +182,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

changelogs/unreleased/security-bypass-email-verification-using-salesforce.yml

+---
+title: Prevent bypassing email verification using Salesforce
+merge_request:
+author:
+type: security

locale/gitlab.pot

@@ -5449,6 +5449,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 ""

spec/controllers/omniauth_callbacks_controller_spec.rb

@@ -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

spec/features/oauth_login_spec.rb

@@ -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

spec/support/helpers/login_helpers.rb

@@ -79,8 +79,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')
@@ -99,9 +99,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,
@@ -124,11 +125,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]