Merge branch 'jej/group-saml-test-button' into 'master'

Group SAML test button

Closes #5901

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

ee/app/assets/javascripts/pages/groups/saml_providers/show/index.js

+import SamlSettingsForm from 'ee/saml_providers/saml_settings_form';
+
+document.addEventListener('DOMContentLoaded', () => {
+  new SamlSettingsForm('#js-saml-settings-form').init();
+});

ee/app/assets/javascripts/saml_providers/dirty_form_checker.js

+export default class DirtyFormChecker {
+  constructor(formSelector, onChange) {
+    this.form = document.querySelector(formSelector);
+    this.onChange = onChange;
+    this.isDirty = false;
+
+    this.editableInputs = Array.from(this.form.querySelectorAll('input[name]')).filter(
+      el => el.type !== 'submit' && el.type !== 'hidden',
+    );
+
+    this.startingStates = {};
+    this.editableInputs.forEach(input => {
+      this.startingStates[input.name] = input.value;
+    });
+  }
+
+  init() {
+    this.form.addEventListener('input', event => {
+      if (event.target.matches('input[name]')) {
+        this.recalculate();
+      }
+    });
+  }
+
+  recalculate() {
+    const wasDirty = this.isDirty;
+    this.isDirty = this.editableInputs.some(input => {
+      const currentValue = input.value;
+      const startValue = this.startingStates[input.name];
+
+      return currentValue !== startValue;
+    });
+
+    if (this.isDirty !== wasDirty) {
+      this.onChange();
+    }
+  }
+}

ee/app/assets/javascripts/saml_providers/saml_settings_form.js

+import $ from 'jquery';
+import { __ } from '~/locale';
+import DirtyFormChecker from './dirty_form_checker';
+
+export default class SamlSettingsForm {
+  constructor(formSelector) {
+    this.form = document.querySelector(formSelector);
+    this.enabledToggle = this.form.querySelector('#saml_provider_enabled');
+    this.testButtonTooltipWrapper = this.form.querySelector('#js-saml-test-button');
+    this.testButton = this.testButtonTooltipWrapper.querySelector('a');
+    this.dirtyFormChecker = new DirtyFormChecker(formSelector, () => this.updateView());
+  }
+
+  init() {
+    this.dirtyFormChecker.init();
+    this.updateEnabled();
+    this.updateView();
+
+    this.enabledToggle.addEventListener('change', () => this.onEnableToggle());
+  }
+
+  onEnableToggle() {
+    this.updateEnabled();
+    this.updateView();
+  }
+
+  updateEnabled() {
+    this.enabled = this.enabledToggle.checked;
+  }
+
+  testButtonTooltip() {
+    if (!this.enabled) {
+      return __('Group SAML must be enabled to test');
+    }
+
+    if (this.dirtyFormChecker.isDirty) {
+      return __('Save changes before testing');
+    }
+
+    return __('Redirect to SAML provider to test configuration');
+  }
+
+  updateView() {
+    if (this.enabled && !this.dirtyFormChecker.isDirty) {
+      this.testButton.removeAttribute('disabled');
+    } else {
+      this.testButton.setAttribute('disabled', true);
+    }
+
+    // Update tooltip using wrapper so it works when input disabled
+    this.testButtonTooltipWrapper.setAttribute('title', this.testButtonTooltip());
+    $(this.testButtonTooltipWrapper).tooltip('_fixTitle');
+  }
+}

ee/app/controllers/groups/omniauth_callbacks_controller.rb

@@ -28,6 +28,13 @@ class Groups::OmniauthCallbacksController < OmniauthCallbacksController
     redirect_to after_sign_in_path_for(current_user)
   end
 
+  override :redirect_identity_link_failed
+  def redirect_identity_link_failed(error_message)
+    flash[:notice] = "SAML authentication failed: #{error_message}"
+
+    redirect_to after_sign_in_path_for(current_user)
+  end
+
   override :after_sign_in_path_for
   def after_sign_in_path_for(resource)
     saml_redirect_path || super

ee/app/views/groups/saml_providers/_form.html.haml

-%section.saml_provider
+%section.saml_provider#js-saml-settings-form
   = form_for [group, saml_provider], url: group_saml_providers_path do |f|
     .form-group.row
       = form_errors(saml_provider)
@@ -26,3 +26,5 @@
 
     .form-actions
       = f.submit _("Save changes"), class: 'btn btn-success qa-save-changes-button'
+      #js-saml-test-button.has-tooltip.pull-right
+        = render 'test_button', saml_provider: @saml_provider

ee/app/views/groups/saml_providers/_test_button.html.haml

+- if saml_provider.persisted?
+  = saml_link_for_provider _('Test SAML SSO'), saml_provider, redirect: request.url, html_class: 'btn qa-saml-settings-test-button'

ee/changelogs/unreleased/jej-group-saml-test-button.yml

+---
+title: Add test button to Group SAML settings
+merge_request: 5622
+author:
+type: changed

ee/spec/features/groups/saml_providers_spec.rb

@@ -17,6 +17,10 @@ describe 'SAML provider settings' do
     click_button('Save changes')
   end
 
+  def test_sso
+    click_link('Test SAML SSO')
+  end
+
   def stub_saml_config
     stub_licensed_features(group_saml: true)
     allow(Devise).to receive(:omniauth_providers).and_return(%i(group_saml))
@@ -72,6 +76,23 @@ describe 'SAML provider settings' do
         expect(login_url).to end_with "/groups/#{group.full_path}/-/saml/sso"
       end
     end
+
+    describe 'test button' 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 }
+      end
+
+      it 'POSTs to the SSO path for the group' do
+        visit group_saml_providers_path(group)
+
+        test_sso
+
+        expect(current_path).to eq callback_path
+      end
+    end
   end
 
   describe '#sso' do

ee/spec/javascripts/fixtures/saml_providers.rb

+# frozen_string_literal: true
+require 'spec_helper'
+
+describe Groups::SamlProvidersController, '(JavaScript fixtures)', type: :controller do
+  include JavaScriptFixturesHelpers
+
+  let(:group) { create(:group, :private) }
+  let(:user) { create(:user) }
+
+  render_views
+
+  before(:all) do
+    clean_frontend_fixtures('groups/saml_providers/')
+  end
+
+  before do
+    sign_in(user)
+    group.add_owner(user)
+    allow(Devise).to receive(:omniauth_providers).and_return(%i(group_saml))
+    stub_licensed_features(group_saml: true)
+  end
+
+  it 'groups/saml_providers/show.html.raw' do |example|
+    create(:saml_provider, group: group)
+
+    get :show, group_id: group
+
+    expect(response).to be_success
+    expect(response).to render_template 'groups/saml_providers/show'
+    store_frontend_fixture(response, example.description)
+  end
+end

ee/spec/javascripts/saml_providers/dirty_form_checker_spec.js

+import DirtyFormChecker from 'ee/saml_providers/dirty_form_checker';
+
+describe('DirtyFormChecker', () => {
+  const FIXTURE = 'groups/saml_providers/show.html.raw';
+  preloadFixtures(FIXTURE);
+
+  beforeEach(() => {
+    loadFixtures(FIXTURE);
+  });
+
+  describe('constructor', () => {
+    let dirtyFormChecker;
+
+    beforeEach(() => {
+      dirtyFormChecker = new DirtyFormChecker('#js-saml-settings-form');
+    });
+
+    it('finds editable inputs', () => {
+      const editableInputs = dirtyFormChecker.editableInputs.map(input => input.name);
+      expect(editableInputs).toContain('saml_provider[sso_url]');
+      expect(editableInputs).not.toContain('authenticity_token');
+    });
+
+    it('tracks starting states for editable inputs', () => {
+      const enabledStartState = dirtyFormChecker.startingStates['saml_provider[enabled]'];
+      expect(enabledStartState).toEqual('1');
+    });
+  });
+
+  describe('recalculate', () => {
+    let dirtyFormChecker;
+    let onChangeCallback;
+
+    beforeEach(() => {
+      onChangeCallback = jasmine.createSpy('onChangeCallback');
+      dirtyFormChecker = new DirtyFormChecker('#js-saml-settings-form', onChangeCallback);
+    });
+
+    it('does not trigger callback when nothing changes', () => {
+      dirtyFormChecker.recalculate();
+
+      expect(onChangeCallback).not.toHaveBeenCalled();
+    });
+
+    it('triggers callback when form becomes dirty', () => {
+      dirtyFormChecker.startingStates['saml_provider[sso_url]'] = 'https://old.value';
+      dirtyFormChecker.recalculate();
+
+      expect(dirtyFormChecker.isDirty).toEqual(true);
+      expect(onChangeCallback).toHaveBeenCalled();
+    });
+
+    it('triggers callback when form returns to original state', () => {
+      dirtyFormChecker.isDirty = true;
+      dirtyFormChecker.recalculate();
+
+      expect(onChangeCallback).toHaveBeenCalled();
+    });
+  });
+});

ee/spec/javascripts/saml_providers/saml_settings_form_spec.js

+import SamlSettingsForm from 'ee/saml_providers/saml_settings_form';
+
+describe('SamlSettingsForm', () => {
+  const FIXTURE = 'groups/saml_providers/show.html.raw';
+  preloadFixtures(FIXTURE);
+
+  beforeEach(() => {
+    loadFixtures(FIXTURE);
+  });
+
+  describe('updateView', () => {
+    let samlSettingsForm;
+
+    beforeEach(() => {
+      samlSettingsForm = new SamlSettingsForm('#js-saml-settings-form');
+      samlSettingsForm.init();
+    });
+
+    it('disables Test button when form has changes', () => {
+      samlSettingsForm.dirtyFormChecker.isDirty = true;
+      expect(samlSettingsForm.testButton.hasAttribute('disabled')).toBe(false);
+
+      samlSettingsForm.updateView();
+
+      expect(samlSettingsForm.testButton.hasAttribute('disabled')).toBe(true);
+    });
+
+    it('re-enables Test button when form is returned to starting state', () => {
+      samlSettingsForm.testButton.setAttribute('disabled', true);
+
+      samlSettingsForm.updateView();
+
+      expect(samlSettingsForm.testButton.hasAttribute('disabled')).toBe(false);
+    });
+
+    it('keeps Test button disabled when SAML disabled for the group', () => {
+      samlSettingsForm.enabled = false;
+      samlSettingsForm.testButton.setAttribute('disabled', true);
+
+      samlSettingsForm.updateView();
+
+      expect(samlSettingsForm.testButton.hasAttribute('disabled')).toBe(true);
+    });
+  });
+});

lib/tasks/karma.rake

@@ -2,7 +2,7 @@ unless Rails.env.production?
   namespace :karma do
     desc 'GitLab | Karma | Generate fixtures for JavaScript tests'
     RSpec::Core::RakeTask.new(:fixtures, [:pattern]) do |t, args|
-      args.with_defaults(pattern: 'spec/javascripts/fixtures/*.rb')
+      args.with_defaults(pattern: '{spec,ee/spec}/javascripts/fixtures/*.rb')
       ENV['NO_KNAPSACK'] = 'true'
       t.pattern = args[:pattern]
       t.rspec_opts = '--format documentation'

locale/gitlab.pot

@@ -3888,6 +3888,9 @@ msgstr ""
 msgid "Group Runners"
 msgstr ""
 
+msgid "Group SAML must be enabled to test"
+msgstr ""
+
 msgid "Group avatar"
 msgstr ""
 
@@ -6459,6 +6462,9 @@ msgstr ""
 msgid "Recent searches"
 msgstr ""
 
+msgid "Redirect to SAML provider to test configuration"
+msgstr ""
+
 msgid "Reference:"
 msgstr ""
 
@@ -6805,6 +6811,9 @@ msgstr ""
 msgid "Save changes"
 msgstr ""
 
+msgid "Save changes before testing"
+msgstr ""
+
 msgid "Save pipeline schedule"
 msgstr ""
 
@@ -7571,6 +7580,9 @@ msgstr ""
 msgid "Terms of Service and Privacy Policy"
 msgstr ""
 
+msgid "Test SAML SSO"
+msgstr ""
+
 msgid "Test coverage parsing"
 msgstr ""
 

qa/qa/ee.rb

@@ -6,6 +6,7 @@ module QA
     module Runtime
       autoload :Env, 'qa/ee/runtime/env'
       autoload :Geo, 'qa/ee/runtime/geo'
+      autoload :Saml, 'qa/ee/runtime/saml'
     end
 
     module Page

qa/qa/ee/page/group/settings/saml_sso.rb

@@ -12,6 +12,10 @@ module QA
               element :save_changes_button
             end
 
+            view 'ee/app/views/groups/saml_providers/_test_button.html.haml' do
+              element :saml_settings_test_button
+            end
+
             view 'ee/app/views/groups/saml_providers/_info.html.haml' do
               element :user_login_url_link
             end
@@ -28,6 +32,10 @@ module QA
               click_element :save_changes_button
             end
 
+            def click_test_button
+              click_element :saml_settings_test_button
+            end
+
             def click_user_login_url_link
               click_element :user_login_url_link
             end

qa/qa/ee/runtime/saml.rb

+# frozen_string_literal: true
+module QA
+  module EE
+    module Runtime
+      module Saml
+        def self.idp_sso_url
+          "https://#{QA::Runtime::Env.simple_saml_hostname || 'localhost'}:8443/simplesaml/saml2/idp/SSOService.php"
+        end
+
+        def self.idp_certificate_fingerprint
+          '119b9e027959cdb7c662cfd075d9e2ef384e445f'
+        end
+      end
+    end
+  end
+end

qa/qa/specs/features/browser_ui/1_manage/ee_group/group_saml_sso_spec.rb

@@ -3,18 +3,20 @@
 module QA
   context :manage, :orchestrated, :group_saml do
     describe 'Group SAML SSO' do
-      it 'User logs in to group with SAML SSO' do
+      before do
         Runtime::Browser.visit(:gitlab, Page::Main::Login)
 
         Page::Main::Login.act { sign_in_using_credentials }
 
         Factory::Resource::Sandbox.fabricate!
+      end
 
+      it 'User logs in to group with SAML SSO' do
         EE::Page::Group::Menu.act { go_to_saml_sso_group_settings }
 
         EE::Page::Group::Settings::SamlSSO.act do
-          set_id_provider_sso_url("https://#{QA::Runtime::Env.simple_saml_hostname || 'localhost'}:8443/simplesaml/saml2/idp/SSOService.php")
-          set_cert_fingerprint('119b9e027959cdb7c662cfd075d9e2ef384e445f')
+          set_id_provider_sso_url(QA::EE::Runtime::Saml.idp_sso_url)
+          set_cert_fingerprint(QA::EE::Runtime::Saml.idp_certificate_fingerprint)
           click_save_changes
           click_user_login_url_link
         end
@@ -33,6 +35,22 @@ module QA
 
         expect(page).to have_content("Signed in with SAML for #{Runtime::Env.sandbox_name}")
       end
+
+      it 'Lets group admin test settings' do
+        EE::Page::Group::Menu.act { go_to_saml_sso_group_settings }
+
+        EE::Page::Group::Settings::SamlSSO.act do
+          set_id_provider_sso_url(QA::EE::Runtime::Saml.idp_sso_url)
+          set_cert_fingerprint(QA::EE::Runtime::Saml.idp_certificate_fingerprint)
+          click_save_changes
+
+          click_test_button
+        end
+
+        Vendor::SAMLIdp::Page::Login.act { login }
+
+        expect(page).to have_content("Test SAML SSO")
+      end
     end
   end
 end