Merge branch '292811-improve-saml-timeout-experience' into 'master'

Auto refresh SAML connection after 7 day timeout

See merge request gitlab-org/gitlab!50914

app/views/devise/shared/_footer.html.haml

+%hr.footer-fixed
+.container.footer-container
+  .footer-links
+    - unless public_visibility_restricted?
+      = link_to _("Explore"), explore_root_path
+      = link_to _("Help"), help_path
+    = link_to _("About GitLab"), "https://about.gitlab.com/"
+= footer_message

app/views/layouts/devise.html.haml

@@ -38,12 +38,4 @@
             .col-sm-5.order-1.order-sm-12.new-session-forms-container
               = yield
 
-      %hr.footer-fixed
-      .container.footer-container
-        .footer-links
-          - if !public_visibility_restricted?
-            = link_to _("Explore"), explore_root_path
-            = link_to _("Help"), help_path
-          = link_to _("About GitLab"), "https://about.gitlab.com/"
-
-      = footer_message
+      = render 'devise/shared/footer', footer_message: footer_message

app/views/layouts/devise_empty.html.haml

 !!! 5
-%html{ lang: "en", class: system_message_class }
+%html.devise-layout-html{ lang: "en", class: system_message_class }
   = render "layouts/head"
   %body.ui-indigo.login-page.application.navless{ class: "#{client_class_list}" }
     = header_message
@@ -11,11 +11,4 @@
         = render "layouts/flash"
         = yield
 
-    %hr
-    .container
-      .footer-links
-        - if !public_visibility_restricted?
-          = link_to _("Explore"), explore_root_path
-          = link_to _("Help"), help_path
-        = link_to _("About GitLab"), "https://about.gitlab.com/"
-    = footer_message
+    = render 'devise/shared/footer', footer_message: footer_message

doc/user/group/saml_sso/index.md

@@ -80,10 +80,13 @@ Please note that the certificate [fingerprint algorithm](#additional-providers-a
 
 - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/5291) in GitLab 11.8.
 - [Improved](https://gitlab.com/gitlab-org/gitlab/-/issues/9255) in GitLab 11.11 with ongoing enforcement in the GitLab UI.
+- [Improved](https://gitlab.com/gitlab-org/gitlab/-/issues/292811) in GitLab 13.8, with an updated timeout experience.
 
 With this option enabled, users must go through your group's GitLab single sign-on URL. They may also be added via SCIM, if configured. Users can't be added manually, and may only access project/group resources via the UI by signing in through the SSO URL.
 
-However, users are not prompted to sign in through SSO on each visit. GitLab checks whether a user has authenticated through SSO, and only prompts the user to sign in via SSO if the session has expired.
+However, users are not prompted to sign in through SSO on each visit. GitLab checks whether a user
+has authenticated through SSO. If it's been more than 7 days since the last sign-in, GitLab
+prompts the user to sign in again through SSO.
 You can see more information about how long a session is valid in our [user profile documentation](../../profile/#why-do-i-keep-getting-signed-out).
 
 We intend to add a similar SSO requirement for [Git and API activity](https://gitlab.com/gitlab-org/gitlab/-/issues/9152).

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

+import { redirectUserWithSSOIdentity } from 'ee/saml_sso';
 import UsernameValidator from '~/pages/sessions/new/username_validator';
 import initConfirmDangerModal from '~/confirm_danger_modal';
 
 new UsernameValidator(); // eslint-disable-line no-new
 initConfirmDangerModal();
+redirectUserWithSSOIdentity();

ee/app/assets/javascripts/saml_sso/constants.js

+export const AUTO_REDIRECT_TO_PROVIDER_BUTTON_SELECTOR = '#js-auto-redirect-to-provider';

ee/app/assets/javascripts/saml_sso/index.js

+import { AUTO_REDIRECT_TO_PROVIDER_BUTTON_SELECTOR } from './constants';
+
+export const redirectUserWithSSOIdentity = () => {
+  const signInButton = document.querySelector(AUTO_REDIRECT_TO_PROVIDER_BUTTON_SELECTOR);
+
+  if (!signInButton) {
+    return;
+  }
+
+  signInButton.click();
+};

ee/app/controllers/groups/sso_controller.rb

@@ -23,6 +23,9 @@ class Groups::SsoController < Groups::ApplicationController
     @group_name = unauthenticated_group.full_name
     @group_saml_identity = linked_identity
     @idp_url = unauthenticated_group.saml_provider.sso_url
+    @auto_redirect_to_provider = current_user&.group_sso?(unauthenticated_group)
+
+    render layout: 'devise_empty' if @auto_redirect_to_provider
   end
 
   def unlink

ee/app/helpers/ee/saml_providers_helper.rb

@@ -14,11 +14,11 @@ module EE
       saml_link(text, provider.group.full_path, **args)
     end
 
-    def saml_link(text, group_path, redirect: nil, html_class: 'btn')
+    def saml_link(text, group_path, redirect: nil, html_class: 'btn', id: nil)
       redirect ||= group_path(group_path)
       url = omniauth_authorize_path(:user, :group_saml, group_path: group_path, redirect_to: redirect)
 
-      link_to(text, url, method: :post, class: html_class)
+      link_to(text, url, method: :post, class: html_class, id: id)
     end
   end
 end

ee/app/views/groups/sso/saml.html.haml

 - page_title _('SAML SSO for %{group_name}') % { group_name: @group_name }
 
-= render 'devise/shared/tab_single', tab_title: _('SAML SSO')
-.login-box
+- if @auto_redirect_to_provider
+  .gl-spinner-container
+    .gl-spinner.gl-spinner-dark.gl-spinner-lg{ aria: { label: 'Loading' } }
+  .gl-text-center.gl-mt-7
+    %h4= _('Reauthenticating with SAML provider.')
+  = saml_link _('Sign in with Single Sign-On'), @group_path, id: 'js-auto-redirect-to-provider', html_class: 'gl-display-none', redirect: @redirect_path
+- else
+  = render 'devise/shared/tab_single', tab_title: _('SAML SSO')
+  .login-box
     .login-body
       - if @group_saml_identity || !user_signed_in?
         %h4= _('Sign in to "%{group_name}"') % { group_name: @group_name }
@@ -13,9 +20,9 @@
       - if @group_saml_identity || !user_signed_in?
         %p= _("This will redirect you to an external sign in page.")
 
-      = saml_link _('Sign in with Single Sign-On'), @group_path, html_class: 'btn btn-success btn-block qa-saml-sso-signin-button', redirect: @redirect_path
+        = saml_link _('Sign in with Single Sign-On'), @group_path, html_class: 'btn btn-success btn-md gl-button btn-block qa-saml-sso-signin-button', redirect: @redirect_path
       - else
         .card.card-body.bs-callout-warning
           = _("Only proceed if you trust %{idp_url} to control your GitLab account sign in.") % { idp_url: @idp_url }
 
-      = saml_link _('Authorize'), @group_path, html_class: 'btn btn-success btn-block qa-saml-sso-signin-button'
+        = saml_link _('Authorize'), @group_path, html_class: 'btn btn-success btn-md gl-button btn-block qa-saml-sso-signin-button'

ee/changelogs/unreleased/292811-improve-saml-timeout-experience.yml

+---
+title: Automatically redirect users to SAML provider after 7 day expiration
+merge_request: 50914
+author:
+type: changed

ee/spec/controllers/groups/sso_controller_spec.rb

@@ -100,6 +100,30 @@ RSpec.describe Groups::SsoController do
         expect(response).to redirect_to(sso_group_saml_providers_path(group))
       end
     end
+
+    context 'when current user has a SAML provider configured' do
+      let(:saml_provider) { create(:saml_provider, group: group, enforced_sso: true) }
+      let(:identity) { create(:group_saml_identity, saml_provider: saml_provider) }
+
+      before do
+        sign_out(user)
+        sign_in(identity.user)
+      end
+
+      it 'renders `devise_empty` template' do
+        get :saml, params: { group_id: group }
+
+        expect(response).to render_template('devise_empty')
+      end
+    end
+
+    context 'when current user does not have a SAML provider configured' do
+      it 'renders `devise` template' do
+        get :saml, params: { group_id: group }
+
+        expect(response).to render_template('devise')
+      end
+    end
   end
 
   context 'saml_provider is unconfigured for the group' do

ee/spec/features/groups/saml_enforcement_spec.rb

@@ -100,4 +100,24 @@ RSpec.describe 'SAML access enforcement' do
       end
     end
   end
+
+  context 'when SAML session expires' do
+    before do
+      mock_group_saml(uid: identity.extern_uid)
+    end
+
+    it 'shows loading screen and link used for auto-redirect' do
+      visit group_path(group)
+
+      click_link 'Sign in with Single Sign-On'
+
+      days_after_timeout = Gitlab::Auth::GroupSaml::SsoEnforcer::DEFAULT_SESSION_TIMEOUT + 2.days
+      travel_to(days_after_timeout.from_now) do
+        visit group_path(group)
+
+        expect(page).to have_content('Reauthenticating with SAML provider.')
+        expect(page).to have_selector('#js-auto-redirect-to-provider', visible: false)
+      end
+    end
+  end
 end

ee/spec/frontend/saml_sso/index_spec.js

+import { redirectUserWithSSOIdentity } from 'ee/saml_sso';
+
+describe('redirectUserWithSSOIdentity', () => {
+  describe('when auto redirect link exists', () => {
+    let link;
+
+    beforeEach(() => {
+      link = document.createElement('a');
+      link.setAttribute('id', 'js-auto-redirect-to-provider');
+      link.setAttribute('href', 'https://foobar.com/sso-service');
+      link.click = jest.fn();
+
+      document.body.appendChild(link);
+    });
+
+    afterEach(() => {
+      document.body.innerHTML = '';
+    });
+
+    it('clicks the link', () => {
+      redirectUserWithSSOIdentity();
+
+      expect(link.click).toHaveBeenCalled();
+    });
+  });
+
+  describe('when auto redirect link does not exist', () => {
+    it('does nothing', () => {
+      expect(redirectUserWithSSOIdentity()).toBeUndefined();
+    });
+  });
+});

locale/gitlab.pot

@@ -23136,6 +23136,9 @@ msgstr ""
 msgid "Real-time features"
 msgstr ""
 
+msgid "Reauthenticating with SAML provider."
+msgstr ""
+
 msgid "Rebase"
 msgstr ""
 

qa/qa/specs/features/ee/browser_ui/1_manage/group/group_saml_non_enforced_sso_spec.rb

@@ -61,8 +61,6 @@ module QA
 
           page.visit managed_group_url
 
-          EE::Page::Group::SamlSSOSignIn.perform(&:click_sign_in)
-
           expect(page).to have_content("Already signed in with SAML for #{@group.path}")
         end
       end