Commit fe792b89 authored by Peter Hegman's avatar Peter Hegman Committed by Kushal Pandya

Add warning when SAML SSO checkbox is unchecked

parent 32474423
......@@ -5,13 +5,18 @@ import { fixTitle } from '~/tooltips';
const CALLOUT_SELECTOR = '.js-callout';
const HELPER_SELECTOR = '.js-helper-text';
const WARNING_SELECTOR = '.js-warning';
function getHelperText(el) {
return el.parentNode.querySelector(HELPER_SELECTOR);
return el?.parentNode?.querySelector(HELPER_SELECTOR) || null;
}
function getWarning(el) {
return el?.parentNode?.querySelector(WARNING_SELECTOR) || null;
}
function getCallout(el) {
return el.closest('.form-group').querySelector(CALLOUT_SELECTOR);
return el?.closest('.form-group')?.querySelector(CALLOUT_SELECTOR) || null;
}
function toggleElementVisibility(el, show) {
......@@ -55,6 +60,7 @@ export default class SamlSettingsForm {
.map((setting) => ({
...setting,
helperText: getHelperText(setting.el),
warning: getWarning(setting.el),
callout: getCallout(setting.el),
}));
......@@ -127,7 +133,7 @@ export default class SamlSettingsForm {
this.settings
.filter((setting) => setting.dependsOn)
.forEach((setting) => {
const { helperText, callout, el } = setting;
const { helperText, warning, callout, el } = setting;
const isRelatedToggleOn = this.getValueWithDeps(setting.dependsOn);
if (helperText) {
......@@ -136,6 +142,10 @@ export default class SamlSettingsForm {
el.disabled = !isRelatedToggleOn;
if (warning) {
toggleElementVisibility(warning, !setting.value);
}
if (callout) {
toggleElementVisibility(callout, setting.value && isRelatedToggleOn);
}
......
......@@ -8,11 +8,14 @@
= s_('GroupSAML|Enable SAML authentication for this group')
.form-group
.gl-form-checkbox.custom-control.custom-checkbox
= f.check_box :enforced_sso, { class: 'custom-control-input js-group-saml-enforced-sso-input', data: { qa_selector: 'enforced_sso_checkbox' } }
= f.check_box :enforced_sso, { checked: saml_provider.new_record? || saml_provider.enforced_sso?, class: 'custom-control-input js-group-saml-enforced-sso-input', data: { qa_selector: 'enforced_sso_checkbox' } }
= f.label :enforced_sso, { class: 'custom-control-label' } do
= s_('GroupSAML|Enforce SSO-only authentication for web activity for this group')
%p.help-text.js-helper-text{ class: saml_provider.enabled? && 'gl-display-none' }
= s_('GroupSAML|Before enforcing SSO, enable SAML authentication.')
%p.help-text.js-warning{ class: saml_provider.enforced_sso? && 'gl-display-none' }
- link_start = '<a href="%{url}" target="_blank" rel="noopener noreferrer">'.html_safe % { url: help_page_path('user/group/saml_sso/index', anchor: 'sso-enforcement') }
= html_escape(s_('GroupSAML|%{strongOpen}Warning%{strongClose} - Enabling %{linkStart}SSO enforcement%{linkEnd} can reduce security risks.')) % { strongOpen: '<strong>'.html_safe, strongClose: '</strong>'.html_safe, linkStart: link_start, linkEnd: '</a>'.html_safe }
.form-group
.gl-form-checkbox.custom-control.custom-checkbox
= f.check_box :git_check_enforced, { class: 'custom-control-input js-group-saml-enforced-git-check-input' }
......
---
title: Default enable SSO-only authentication for web activity and add warning if
it is disabled
merge_request: 60868
author:
type: changed
......@@ -57,6 +57,22 @@ RSpec.describe 'SAML provider settings' do
expect(response_headers['Content-Type']).to have_content("application/xml")
end
context '"Enforce SSO-only authentication for web activity for this group" checkbox' do
it 'is checked by default' do
visit group_saml_providers_path(group)
expect(find_field('Enforce SSO-only authentication for web activity for this group')).to be_checked
end
it 'displays warning if unchecked', :js do
visit group_saml_providers_path(group)
uncheck 'Enforce SSO-only authentication for web activity for this group'
expect(page).to have_content 'Warning - Enabling SSO enforcement can reduce security risks.'
end
end
it 'allows creation of new provider' do
visit group_saml_providers_path(group)
......@@ -75,7 +91,7 @@ RSpec.describe 'SAML provider settings' do
end
context 'with existing SAML provider' do
let!(:saml_provider) { create(:saml_provider, group: group, prohibited_outer_forks: false) }
let!(:saml_provider) { create(:saml_provider, group: group, prohibited_outer_forks: false, enforced_sso: true) }
it 'allows provider to be disabled', :js do
visit group_saml_providers_path(group)
......@@ -97,9 +113,10 @@ RSpec.describe 'SAML provider settings' do
it 'updates the enforced sso setting', :js do
visit group_saml_providers_path(group)
check 'Enforce SSO-only authentication for web activity for this group'
uncheck 'Enforce SSO-only authentication for web activity for this group'
expect { submit }.to change { saml_provider.reload.enforced_sso }.to(true)
expect { submit }.to change { saml_provider.reload.enforced_sso }.to(false)
expect(page).to have_content 'Warning - Enabling SSO enforcement can reduce security risks.'
end
context 'enforced_group_managed_accounts enabled', :js do
......@@ -111,7 +128,6 @@ RSpec.describe 'SAML provider settings' do
it 'updates the enforced_group_managed_accounts flag' do
visit group_saml_providers_path(group)
check 'Enforce SSO-only authentication for web activity for this group'
check 'Enforce users to have dedicated group-managed accounts for this group'
expect { submit }.to change { saml_provider.reload.enforced_group_managed_accounts }.to(true)
......@@ -120,7 +136,6 @@ RSpec.describe 'SAML provider settings' do
it 'updates the prohibited_outer_forks flag' do
visit group_saml_providers_path(group)
check 'Enforce SSO-only authentication for web activity for this group'
check 'Enforce users to have dedicated group-managed accounts for this group'
check 'Prohibit outer forks for this group'
......
......@@ -21,7 +21,7 @@ RSpec.describe Groups::SamlProvidersController, '(JavaScript fixtures)', type: :
end
it 'groups/saml_providers/show.html' do
create(:saml_provider, group: group)
create(:saml_provider, group: group, enforced_sso: true)
get :show, params: { group_id: group }
......
......@@ -13,6 +13,8 @@ describe('SamlSettingsForm', () => {
const findEnforcedGroupManagedAccountSetting = () =>
samlSettingsForm.settings.find((s) => s.name === 'enforced-group-managed-accounts');
const findEnforcedSsoSetting = () =>
samlSettingsForm.settings.find((s) => s.name === 'enforced-sso');
const findProhibitForksSetting = () =>
samlSettingsForm.settings.find((s) => s.name === 'prohibited-outer-forks');
......@@ -65,6 +67,16 @@ describe('SamlSettingsForm', () => {
expect(findProhibitForksSetting().value).toBe(true);
});
it('correctly shows warning text when checkbox is unchecked', () => {
expect(findEnforcedSsoSetting().warning.classList.contains('gl-display-none')).toBe(true);
findEnforcedSsoSetting().el.checked = false;
samlSettingsForm.updateSAMLSettings();
samlSettingsForm.updateView();
expect(findEnforcedSsoSetting().warning.classList.contains('gl-display-none')).toBe(false);
});
it('correctly disables multiple dependent toggles', () => {
samlSettingsForm.settings.forEach((s) => {
const { el } = s;
......
......@@ -15690,6 +15690,9 @@ msgstr ""
msgid "GroupRoadmap|To widen your search, change or remove filters; from %{startDate} to %{endDate}."
msgstr ""
msgid "GroupSAML|%{strongOpen}Warning%{strongClose} - Enabling %{linkStart}SSO enforcement%{linkEnd} can reduce security risks."
msgstr ""
msgid "GroupSAML|Active SAML Group Links (%{count})"
msgstr ""
......
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