Commit b0e640ce authored by James Edwards-Jones's avatar James Edwards-Jones

Group SAML SSO page warns when linking account

Linking an account allows the external service to sign that user into
GitLab. We need to make users aware of this to reduce the risk of
phishing attacks where users are maliciously tricked into linking
accounts.

We do this by showing a warning and including the group path and the
URL that the user will be redirected to. We also limit the URL to
ASCII characters to avoid homoglyph attacks.
parent 0c4a47b0
......@@ -15,6 +15,8 @@ class Groups::SsoController < Groups::ApplicationController
def saml
@group_path = params[:group_id]
@group_name = @unauthenticated_group.full_name
@group_saml_identity = linked_identity
@idp_url = @unauthenticated_group.saml_provider.sso_url
end
def unlink
......
......@@ -5,7 +5,7 @@ class SamlProvider < ActiveRecord::Base
has_many :identities
validates :group, presence: true, top_level_group: true
validates :sso_url, presence: true, url: { protocols: %w(https) }
validates :sso_url, presence: true, url: { protocols: %w(https), ascii_only: true }
validates :certificate_fingerprint, presence: true, certificate_fingerprint: true
after_initialize :set_defaults, if: :new_record?
......
......@@ -3,8 +3,19 @@
= render 'devise/shared/tab_single', tab_title: _('SAML SSO')
.login-box
.login-body
%h4= _("Sign in to %{group_name}") % { group_name: @group_name }
- if @group_saml_identity
%h4= _('Sign in to "%{group_name}"') % { group_name: @group_name }
- else
%h4= _('Allow "%{group_name}" to sign you in') % { group_name: @group_name }
%p= _("This group allows you to sign in with your %{group_name} Single Sign-On account. This will redirect you to an external sign in page.") % { group_name: @group_name }
%p= _('The "%{group_path}" group allows you to sign in with your Single Sign-On Account') % { group_path: @group_path }
= saml_link _('Sign in with Single Sign-On'), @group_path, html_class: 'btn btn-success btn-block qa-saml-sso-signin-button'
- if @group_saml_identity
%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'
- 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'
---
title: Group SAML SSO page warns when linking account
merge_request: 8295
author:
type: changed
......@@ -154,15 +154,37 @@ describe 'SAML provider settings' do
context 'when signed in' do
before do
sign_in(user)
end
it 'shows warning that linking accounts authorizes control over sign in' do
visit sso_group_saml_providers_path(group)
expect(page).to have_content(/Allow .* to sign you in/)
expect(page).to have_content(saml_provider.sso_url)
expect(page).to have_content('Authorize')
end
it 'Sign in button redirects to auth flow and back to group' do
click_link 'Sign in with Single Sign-On'
it 'Authorize/link button redirects to auth flow' do
visit sso_group_saml_providers_path(group)
click_link 'Authorize'
expect(current_path).to eq callback_path
end
context 'with linked account' do
before do
create(:group_saml_identity, saml_provider: saml_provider, user: user)
end
it 'Sign in button redirects to auth flow' do
visit sso_group_saml_providers_path(group)
click_link 'Sign in with Single Sign-On'
expect(current_path).to eq callback_path
end
end
end
context 'for a private group' do
......@@ -187,7 +209,7 @@ describe 'SAML provider settings' do
expect(current_path).to eq sso_group_saml_providers_path(group)
within '.login-box' do
expect(page).to have_link 'Sign in with Single Sign-On'
expect(page).to have_link 'Authorize'
end
end
......
......@@ -16,6 +16,15 @@ describe SamlProvider do
expect(subject).not_to allow_value('http://example.com').for(:sso_url)
end
it 'prevents homoglyph phishing attacks by only allowing ascii URLs' do
expect(subject).to allow_value('https://gitlab.com/adfs/ls').for(:sso_url)
expect(subject).not_to allow_value('https://𝕘itⅼaƄ.ᴄοm/adfs/ls').for(:sso_url)
end
it 'allows unicode domain names when encoded as ascii punycode' do
expect(subject).to allow_value('https://xn--gitl-ocb944a.xn--m-rmb025q/adfs/ls').for(:sso_url)
end
it 'expects certificate_fingerprint to be in an accepted format' do
expect(subject).to allow_value('000030EDC285E01D6B5EA33010A79ADD142F5004').for(:certificate_fingerprint)
expect(subject).to allow_value('00:00:30:ED:C2:85:E0:1D:6B:5E:A3:30:10:A7:9A:DD:14:2F:50:04').for(:certificate_fingerprint)
......
......@@ -557,6 +557,9 @@ msgstr ""
msgid "All users"
msgstr ""
msgid "Allow \"%{group_name}\" to sign you in"
msgstr ""
msgid "Allow commits from members who can merge to the target branch."
msgstr ""
......@@ -5929,6 +5932,9 @@ msgstr ""
msgid "Only mirror protected branches"
msgstr ""
msgid "Only proceed if you trust %{idp_url} to control your GitLab account sign in."
msgstr ""
msgid "Only project members can comment."
msgstr ""
......@@ -7756,7 +7762,7 @@ msgstr ""
msgid "Sign in / Register"
msgstr ""
msgid "Sign in to %{group_name}"
msgid "Sign in to \"%{group_name}\""
msgstr ""
msgid "Sign in via 2FA code"
......@@ -8259,6 +8265,9 @@ msgstr ""
msgid "Thanks! Don't show me this again"
msgstr ""
msgid "The \"%{group_path}\" group allows you to sign in with your Single Sign-On Account"
msgstr ""
msgid "The Advanced Global Search in GitLab is a powerful search service that saves you time. Instead of creating duplicate code and wasting time, you can now search for code within other teams that can help your own project."
msgstr ""
......@@ -8475,9 +8484,6 @@ msgstr ""
msgid "This group"
msgstr ""
msgid "This group allows you to sign in with your %{group_name} Single Sign-On account. This will redirect you to an external sign in page."
msgstr ""
msgid "This group does not provide any group Runners yet."
msgstr ""
......@@ -8610,6 +8616,9 @@ msgstr ""
msgid "This user will be the author of all events in the activity feed that are the result of an update, like new branches being created or new commits being pushed to existing branches. Upon creation or when reassigning you can only assign yourself to be the mirror user."
msgstr ""
msgid "This will redirect you to an external sign in page."
msgstr ""
msgid "Those emails automatically become issues (with the comments becoming the email conversation) listed here."
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