Merge branch 'jej/fix-group-saml-u2f' into 'master'

Ensure U2F javascript runs on GroupSAML callback

Closes #11796

See merge request gitlab-org/gitlab-ee!14262

ee/app/assets/javascripts/pages/groups/omniauth_callbacks/index.js

+import initU2F from '~/shared/sessions/u2f';
+
+document.addEventListener('DOMContentLoaded', initU2F);

ee/changelogs/unreleased/jej-fix-group-saml-u2f.yml

+---
+title: Ensure U2F javascript runs on GroupSAML callback
+merge_request: 14262
+author:
+type: fixed

ee/config/routes/group.rb

@@ -90,7 +90,8 @@ constraints(::Constraints::GroupUrlConstrainer.new) do
     end
 
     resource :saml_providers, path: 'saml', only: [:show, :create, :update] do
-      post :callback, to: 'omniauth_callbacks#group_saml'
+      callback_methods = Rails.env.test? ? [:get, :post] : [:post]
+      match :callback, to: 'omniauth_callbacks#group_saml', via: callback_methods
       get :sso, to: 'sso#saml'
       delete :unlink, to: 'sso#unlink'
     end

ee/lib/omni_auth/strategies/group_saml.rb

@@ -22,6 +22,10 @@ module OmniAuth
         settings = Gitlab::Auth::GroupSaml::DynamicSettings.new(group_lookup.group).to_h
         env['omniauth.strategy'].options.merge!(settings)
 
+        if OmniAuth.config.test_mode && on_request_path?
+          emulate_relay_state
+        end
+
         super
       end
 
@@ -50,6 +54,10 @@ module OmniAuth
         end
       end
 
+      def emulate_relay_state
+        request.query_string.sub!('redirect_to', 'RelayState')
+      end
+
       def self.invalid_group!(path)
         raise ActionController::RoutingError, path
       end
@@ -58,6 +66,19 @@ module OmniAuth
         env['PATH_INFO'] =~ Gitlab::PathRegex.saml_callback_regex
       end
 
+      override :callback_path
+      def callback_path
+        @callback_path ||= begin
+          if options[:callback_path].call(env)
+            current_path
+          elsif group_lookup.path
+            "/groups/#{group_lookup.path}/-/saml/callback"
+          else
+            super
+          end
+        end
+      end
+
       private
 
       def metadata_phase?

ee/spec/features/groups/saml_providers_spec.rb

@@ -13,6 +13,10 @@ describe 'SAML provider settings' do
     group.add_owner(user)
   end
 
+  around(:all) do |example|
+    with_omniauth_full_host { example.run }
+  end
+
   def submit
     click_button('Save changes')
   end
@@ -138,8 +142,7 @@ describe 'SAML provider settings' do
       let!(:saml_provider) { create(:saml_provider, group: group) }
 
       before do
-        sign_in(user)
-        allow_any_instance_of(OmniAuth::Strategies::GroupSaml).to receive(:callback_url) { callback_path }
+        mock_group_saml(uid: '123')
       end
 
       it 'POSTs to the SSO path for the group' do
@@ -147,7 +150,8 @@ describe 'SAML provider settings' do
 
         test_sso
 
-        expect(current_path).to eq callback_path
+        expect(current_path).to eq group_saml_providers_path(group)
+        expect(page).to have_content("SAML for #{group.name} was added to your connected accounts")
       end
     end
   end
@@ -177,10 +181,6 @@ describe 'SAML provider settings' do
     context 'with existing SAML provider' do
       let!(:saml_provider) { create(:saml_provider, group: group) }
 
-      before do
-        allow_any_instance_of(OmniAuth::Strategies::GroupSaml).to receive(:callback_url) { callback_path }
-      end
-
       context 'when not signed in' do
         it "shows the sso page so user can sign in" do
           visit sso_group_saml_providers_path(group)
@@ -205,16 +205,20 @@ describe 'SAML provider settings' do
         end
 
         it 'Authorize/link button redirects to auth flow' do
+          external_uid = '98765'
+          mock_group_saml(uid: external_uid)
           visit sso_group_saml_providers_path(group)
 
           click_link 'Authorize'
 
-          expect(current_path).to eq callback_path
+          expect(page).to have_content(/SAML for .* was added to your connected accounts/)
+          expect(user.identities.last.extern_uid).to eq external_uid
         end
 
         context 'with linked account' do
           before do
-            create(:group_saml_identity, saml_provider: saml_provider, user: user)
+            identity = create(:group_saml_identity, saml_provider: saml_provider, user: user)
+            mock_group_saml(uid: identity.extern_uid)
           end
 
           it 'Sign in button redirects to auth flow' do
@@ -222,7 +226,8 @@ describe 'SAML provider settings' do
 
             click_link 'Sign in with Single Sign-On'
 
-            expect(current_path).to eq callback_path
+            expect(current_path).to eq group_path(group)
+            expect(page).to have_content('Already signed in')
           end
         end
       end

ee/spec/features/users/login_spec.rb

@@ -3,6 +3,7 @@ require 'spec_helper'
 describe 'Login' do
   include LdapHelpers
   include UserLoginHelper
+  include DeviseHelpers
 
   before do
     stub_licensed_features(extended_audit_events: true)
@@ -148,4 +149,39 @@ describe 'Login' do
       end
     end
   end
+
+  describe 'via Group SAML' do
+    let(:saml_provider) { create(:saml_provider) }
+    let(:group) { saml_provider.group }
+    let(:identity) { create(:group_saml_identity, user: user, saml_provider: saml_provider) }
+
+    before do
+      stub_licensed_features(group_saml: true)
+    end
+
+    around(:all) do |example|
+      with_omniauth_full_host { example.run }
+    end
+
+    context 'with U2F two factor', :js do
+      let(:user) { create(:user, :two_factor_via_u2f) }
+
+      before do
+        mock_group_saml(uid: identity.extern_uid)
+      end
+
+      it 'shows U2F prompt after SAML' do
+        visit sso_group_saml_providers_path(group, token: group.saml_discovery_token)
+
+        click_link 'Sign in with Single Sign-On'
+
+        expect(page).to have_content('Trying to communicate with your device')
+        expect(page).to have_link('Sign in via 2FA code')
+
+        fake_successful_u2f_authentication
+
+        expect(current_path).to eq root_path
+      end
+    end
+  end
 end

ee/spec/support/helpers/ee/login_helpers.rb

+# frozen_string_literal: true
+
+module EE
+  module LoginHelpers
+    def configure_group_saml_mock_auth(uid:)
+      name = 'My name'
+      email = 'name@example.com'
+      response_object = { document: saml_xml(File.read('spec/fixtures/authentication/saml_response.xml')) }
+
+      OmniAuth.config.mock_auth[:group_saml] = OmniAuth::AuthHash.new({
+        provider: :group_saml,
+        uid: uid,
+        info: { name: name, email: email },
+        extra: {
+          raw_info: { info: { name: name, email: email } },
+          response_object: response_object
+        }
+      })
+    end
+
+    def mock_group_saml(uid:)
+      allow(Devise).to receive(:omniauth_providers).and_return(%i(group_saml))
+      allow_any_instance_of(::Gitlab::Auth::SamlOriginValidator).to receive(:gitlab_initiated?).and_return(true)
+      configure_group_saml_mock_auth(uid: uid)
+      stub_omniauth_provider(:group_saml)
+    end
+  end
+end

spec/features/oauth_login_spec.rb

@@ -16,16 +16,8 @@ describe 'OAuth Login', :js, :allow_forgery_protection do
   providers = [:github, :twitter, :bitbucket, :gitlab, :google_oauth2,
                :facebook, :cas3, :auth0, :authentiq, :salesforce]
 
-  before(:all) do
-    # The OmniAuth `full_host` parameter doesn't get set correctly (it gets set to something like `http://localhost`
-    # here), and causes integration tests to fail with 404s. We set the `full_host` by removing the request path (and
-    # anything after it) from the request URI.
-    @omniauth_config_full_host = OmniAuth.config.full_host
-    OmniAuth.config.full_host = ->(request) { request['REQUEST_URI'].sub(/#{request['REQUEST_PATH']}.*/, '') }
-  end
-
-  after(:all) do
-    OmniAuth.config.full_host = @omniauth_config_full_host
+  around(:all) do |example|
+    with_omniauth_full_host { example.run }
   end
 
   def login_with_provider(provider, enter_two_factor: false)

spec/support/helpers/devise_helpers.rb

@@ -21,4 +21,16 @@ module DeviseHelpers
       context.env
     end
   end
+
+  def with_omniauth_full_host(&block)
+    # The OmniAuth `full_host` parameter doesn't get set correctly (it gets set to something like `http://localhost`
+    # here), and causes integration tests to fail with 404s. We set the `full_host` by removing the request path (and
+    # anything after it) from the request URI.
+    omniauth_config_full_host = OmniAuth.config.full_host
+    OmniAuth.config.full_host = ->(request) { ActionDispatch::Request.new(request).base_url }
+
+    yield
+
+    OmniAuth.config.full_host = omniauth_config_full_host
+  end
 end

spec/support/helpers/fake_u2f_device.rb

@@ -32,6 +32,10 @@ class FakeU2fDevice
     ")
   end
 
+  def fake_u2f_authentication
+    @page.execute_script("window.gl.u2fAuthenticate.renderAuthenticated('abc');")
+  end
+
   private
 
   def u2f_device(app_id)

spec/support/helpers/login_helpers.rb

@@ -87,12 +87,17 @@ module LoginHelpers
     click_link "oauth-login-#{provider}"
   end
 
+  def fake_successful_u2f_authentication
+    allow(U2fRegistration).to receive(:authenticate).and_return(true)
+    FakeU2fDevice.new(page, nil).fake_u2f_authentication
+  end
+
   def mock_auth_hash_with_saml_xml(provider, uid, email, saml_response)
     response_object = { document: saml_xml(saml_response) }
     mock_auth_hash(provider, uid, email, response_object: response_object)
   end
 
-  def mock_auth_hash(provider, uid, email, response_object: nil)
+  def configure_mock_auth(provider, uid, email, response_object: nil)
     # 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({
@@ -118,6 +123,11 @@ module LoginHelpers
         response_object: response_object
       }
     })
+  end
+
+  def mock_auth_hash(provider, uid, email, response_object: nil)
+    configure_mock_auth(provider, uid, email, 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]
 
@@ -199,3 +209,5 @@ module LoginHelpers
     allow(Gitlab::Auth::Saml::Config).to receive_messages({ options: { name: 'saml', groups_attribute: 'groups', external_groups: groups, args: {} } })
   end
 end
+
+LoginHelpers.prepend(EE::LoginHelpers)