Commit a7e2550f authored by Sebastián Arcila Valenzuela's avatar Sebastián Arcila Valenzuela Committed by Dmytro Zaporozhets

Add tab panels for authorizing the transfer

This change moves the current sign up tab panel to a partial and adds
new partials for the tabs links and the authorize/transfer ownership
parent 7948c4dd
...@@ -73,9 +73,13 @@ Without group-managed accounts, users can link their SAML identity with any exis ...@@ -73,9 +73,13 @@ Without group-managed accounts, users can link their SAML identity with any exis
When this option is enabled: When this option is enabled:
- All existing and new users in the group will be required to log in via the SSO URL associated with the group. - All existing and new users in the group will be required to log in via the SSO URL associated with the group.
- On successfully authenticating, GitLab will prompt the user to create a new, dedicated account using the email address received from the configured identity provider.
- After the group-managed account has been created, group activity will require the use of this user account. - After the group-managed account has been created, group activity will require the use of this user account.
Upon successful authentication, GitLab prompts the user with options, based on the email address received from the configured identity provider:
- To create a unique account with the newly received email address.
- If the received email address matches one of the user's verified GitLab email addresses, the option to convert the existing account to a group-managed account. ([Introduced in GitLab 12.9](https://gitlab.com/gitlab-org/gitlab/issues/13481).)
Since use of the group-managed account requires the use of SSO, users of group-managed accounts will lose access to these accounts when they are no longer able to authenticate with the connected identity provider. In the case of an offboarded employee who has been removed from your identity provider: Since use of the group-managed account requires the use of SSO, users of group-managed accounts will lose access to these accounts when they are no longer able to authenticate with the connected identity provider. In the case of an offboarded employee who has been removed from your identity provider:
- The user will be unable to access the group (their credentials will no longer work on the identity provider when prompted to SSO). - The user will be unable to access the group (their credentials will no longer work on the identity provider when prompted to SSO).
......
import UsernameValidator from '~/pages/sessions/new/username_validator'; import UsernameValidator from '~/pages/sessions/new/username_validator';
import initConfirmDangerModal from '~/confirm_danger_modal';
document.addEventListener('DOMContentLoaded', () => { document.addEventListener('DOMContentLoaded', () => {
new UsernameValidator(); // eslint-disable-line no-new new UsernameValidator(); // eslint-disable-line no-new
initConfirmDangerModal();
}); });
...@@ -9,7 +9,7 @@ class Groups::SsoController < Groups::ApplicationController ...@@ -9,7 +9,7 @@ class Groups::SsoController < Groups::ApplicationController
before_action :require_enabled_provider!, except: [:unlink] before_action :require_enabled_provider!, except: [:unlink]
before_action :check_user_can_sign_in_with_provider, only: [:saml] before_action :check_user_can_sign_in_with_provider, only: [:saml]
before_action :redirect_if_group_moved before_action :redirect_if_group_moved
before_action :check_oauth_data, only: [:sign_up_form, :sign_up] before_action :check_oauth_data, only: [:sign_up_form, :sign_up, :authorize_managed_account]
layout 'devise' layout 'devise'
...@@ -48,6 +48,20 @@ class Groups::SsoController < Groups::ApplicationController ...@@ -48,6 +48,20 @@ class Groups::SsoController < Groups::ApplicationController
end end
end end
def authorize_managed_account
transfer_membership_service = GroupSaml::GroupManagedAccounts::TransferMembershipService.new(current_user, unauthenticated_group, session)
if transfer_membership_service.execute
session['oauth_data'] = nil
flash[:notice] = nil
store_active_saml_session
redirect_to group_url(unauthenticated_group)
else
render_sign_up_form
end
end
private private
def new_user def new_user
...@@ -94,6 +108,10 @@ class Groups::SsoController < Groups::ApplicationController ...@@ -94,6 +108,10 @@ class Groups::SsoController < Groups::ApplicationController
@unauthenticated_group ||= Group.find_by_full_path(params[:group_id], follow_redirects: true) @unauthenticated_group ||= Group.find_by_full_path(params[:group_id], follow_redirects: true)
end end
def store_active_saml_session
Gitlab::Auth::GroupSaml::SsoEnforcer.new(unauthenticated_group.saml_provider).update_session
end
def require_configured_provider! def require_configured_provider!
unless unauthenticated_group&.feature_available?(:group_saml) && Gitlab::Auth::GroupSaml::Config.enabled? unless unauthenticated_group&.feature_available?(:group_saml) && Gitlab::Auth::GroupSaml::Config.enabled?
return route_not_found return route_not_found
......
# frozen_string_literal: true
module Groups::SsoHelper
def transfer_ownership_message(group_name)
_("You are about to transfer the control of your account to %{group_name} group. This action is NOT reversible, you won't be able to access any of your groups and projects outside of %{group_name} once this transfer is complete.") %
{ group_name: sanitize(group_name) }
end
end
# frozen_string_literal: true
module GroupSaml
module GroupManagedAccounts
class TransferMembershipService
attr_reader :group, :current_user, :oauth_data, :session, :saml_email
def initialize(current_user, group, session)
@current_user = current_user
@group = group
@oauth_data = session['oauth_data']
@saml_email = oauth_data&.info&.email
@session = session
end
def execute
return unless ::Feature.enabled?(:convert_user_to_group_managed_accounts)
return false unless current_user.verified_email?(saml_email)
ActiveRecord::Base.transaction do
if destroy_non_gma_identities && leave_non_gma_memberships && transfer_user
identity_linker = Gitlab::Auth::GroupSaml::IdentityLinker.new(current_user, oauth_data, session, group.saml_provider)
identity_linker.link
raise ActiveRecord::Rollback if identity_linker.failed?
true
else
raise ActiveRecord::Rollback
end
rescue Gitlab::Auth::Saml::IdentityLinker::UnverifiedRequest
raise ActiveRecord::Rollback
end
end
private
def transfer_user
current_user.managing_group = group
current_user.email = saml_email
current_user.encrypted_password = ''
current_user.save
end
def destroy_non_gma_identities
current_user.identities.all? do |identity|
identity.destroy
identity.destroyed?
end
end
def leave_non_gma_memberships
current_user.members.all? do |member|
next true if member.source == group
next true if member.source.owner == current_user
Members::DestroyService.new(current_user).execute(member)
member.destroyed?
end
end
end
end
end
#authorize-pane.login-box.tab-pane.rounded{ role: 'tabpanel' }
.login-body
= form_tag(group_authorize_managed_account_path(@unauthenticated_group), html: { class: "user gl-show-field-errors", "aria-live" => "assertive" }) do
.d-flex.flex-column.mt-3.mx-3
%p
= _("Alternatively, you can convert your account to a managed account by the %{group_name} group.") % { group_name: sanitize(@group_name) }
%ul
%li
= _('You will be removed from existing projects/groups')
%li
= _('Existing sign in methods may be removed')
.card.card-body.bs-callout-warning
= _("Only proceed if you trust %{idp_url} to control your GitLab account sign in.") % { idp_url: @unauthenticated_group.saml_provider.sso_url }
.submit-container
= button_to _("Transfer ownership"), '#', class: 'btn btn-remove js-confirm-danger', data: { 'confirm-danger-message' => transfer_ownership_message(@group_name), qa_selector: 'transfer_ownership_button' }
= render 'shared/confirm_modal', phrase: current_user.username
- if Gitlab::CurrentSettings.current_application_settings.enforce_terms?
.form-group
= check_box_tag :terms_opt_in, '1', false, required: true, class: 'qa-new-user-accept-terms'
= label_tag :terms_opt_in do
- terms_link = link_to s_("I accept the|Terms of Service and Privacy Policy"), terms_path, target: "_blank"
- accept_terms_label = _("I accept the %{terms_link}") % { terms_link: terms_link }
= accept_terms_label.html_safe
#register-pane.login-box.tab-pane.rounded.active{ role: 'tabpanel' }
.login-body
= form_for(resource, as: "new_user", url: group_sign_up_path(@unauthenticated_group), html: { class: "new_new_user gl-show-field-errors", "aria-live" => "assertive" }) do |f|
.devise-errors
= devise_error_messages!
.d-flex.flex-column.align-items-center.mt-3.mx-3
.avatar-container.rect-avatar.s64.home-panel-avatar.mb-3
= group_icon(@unauthenticated_group, class: 'avatar avatar-tile s64', width: 64, height: 64)
%p.text-center
= (_("Finish setting up your dedicated account for <strong>%{group_name}</strong>.") % { group_name: sanitize(@group_name) }).html_safe
.form-group
= f.label :email, class: 'label-bold'
= f.email_field :email, class: "form-control", required: true, disabled: true, data: { qa_selector: 'new_user_email_field' }
.name.form-group
= f.label :name, _('Full name'), class: 'label-bold'
= f.text_field :name, class: "form-control top js-block-emoji", required: true, data: { qa_selector: 'new_user_username_field' }, disabled: resource.name.present?
.username.form-group
= f.label :username, class: 'label-bold'
= f.text_field :username, class: "form-control qa-new-user-username js-block-emoji", pattern: Gitlab::PathRegex::NAMESPACE_FORMAT_REGEX_JS, required: true, title: _("Please create a username with only alphanumeric characters.")
%p.validation-error.hide= _('Username is already taken.')
%p.validation-success.hide= _('Username is available.')
%p.validation-pending.hide= _('Checking username availability...')
= render 'enforce_terms'
= render_if_exists 'devise/shared/email_opted_in', f: f
- if current_user
%p.text-center= _("You'll be signed out from your current account automatically.")
.d-flex.justify-content-center
= render 'user_info'
.submit-container
= f.submit _("Sign out & Register"), class: "btn-register btn qa-new-user-register-button", data: { qa_selector: 'sign_out_and_register_button' }
- else
.submit-container
= f.submit _("Register"), class: "btn-register btn qa-new-user-register-button"
%ul.nav-links.new-session-tabs.nav-tabs.nav{ role: 'tablist' }
%li.nav-item{ role: 'presentation' }
%a.nav-link.active{ href: '#register-pane', role: 'tab', data: { toggle: 'tab', qa_selector: 'register_pane_link' } }
= _('Register')
- if ::Feature.enabled?(:convert_user_to_group_managed_accounts) && current_user && current_user.verified_email?(resource.email)
%li.nav-item{ role: 'presentation' }
%a.nav-link{ href: '#authorize-pane', role: 'tab', data: { toggle: 'tab', qa_selector: 'authorize_pane_link' } }
= _('Authorize')
- page_title _('SAML SSO for %{group_name}') % { group_name: @group_name } - page_title _('SAML SSO for %{group_name}') % { group_name: sanitize(@group_name) }
.login-box.rounded %div
.login-body = render 'tabs', resource: resource, current_user: current_user
= form_for(resource, as: "new_user", url: group_sign_up_path(@unauthenticated_group), html: { class: "new_new_user gl-show-field-errors", "aria-live" => "assertive" }) do |f| .tab-content
.devise-errors = render 'register_pane', resource: resource
= devise_error_messages! - if ::Feature.enabled?(:convert_user_to_group_managed_accounts) && current_user && current_user.verified_email?(resource.email)
.d-flex.flex-column.align-items-center.mt3.mx3 = render 'authorize_pane'
.avatar-container.rect-avatar.s64.home-panel-avatar.mb-3
= group_icon(@unauthenticated_group, class: 'avatar avatar-tile s64', width: 64, height: 64)
%p.text-center
= (_("Finish setting up your dedicated account for <strong>%{group_name}</strong>.") % { group_name: @group_name }).html_safe
.form-group
= f.label :email, class: 'label-bold'
= f.email_field :email, class: "form-control", required: true, disabled: true, data: { qa_selector: 'new_user_email_field' }
.name.form-group
= f.label :name, _('Full name'), class: 'label-bold'
= f.text_field :name, class: "form-control top js-block-emoji", required: true, data: { qa_selector: 'new_user_username_field' }, disabled: resource.name.present?
.username.form-group
= f.label :username, class: 'label-bold'
= f.text_field :username, class: "form-control qa-new-user-username js-block-emoji", pattern: Gitlab::PathRegex::NAMESPACE_FORMAT_REGEX_JS, required: true, title: _("Please create a username with only alphanumeric characters.")
%p.validation-error.hide= _('Username is already taken.')
%p.validation-success.hide= _('Username is available.')
%p.validation-pending.hide= _('Checking username availability...')
- if Gitlab::CurrentSettings.current_application_settings.enforce_terms?
.form-group
= check_box_tag :terms_opt_in, '1', false, required: true, class: 'qa-new-user-accept-terms'
= label_tag :terms_opt_in do
- terms_link = link_to s_("I accept the|Terms of Service and Privacy Policy"), terms_path, target: "_blank"
- accept_terms_label = _("I accept the %{terms_link}") % { terms_link: terms_link }
= accept_terms_label.html_safe
= render_if_exists 'devise/shared/email_opted_in', f: f
- if current_user
%p.text-center= _("You'll be signed out from your current account automatically.")
.d-flex.justify-content-center
= render 'user_info'
.submit-container
= f.submit _("Sign out & Register"), class: "btn-register btn qa-new-user-register-button", data: { qa_selector: 'sign_out_and_register_button' }
- else
.submit-container
= f.submit _("Register"), class: "btn-register btn qa-new-user-register-button"
---
title: Allow existing users to transfer their account to SAML Group managed accounts
merge_request: 24329
author:
type: added
...@@ -140,6 +140,7 @@ constraints(::Constraints::GroupUrlConstrainer.new) do ...@@ -140,6 +140,7 @@ constraints(::Constraints::GroupUrlConstrainer.new) do
get :sign_up, to: 'sso#sign_up_form' get :sign_up, to: 'sso#sign_up_form'
post :sign_up, to: 'sso#sign_up' post :sign_up, to: 'sso#sign_up'
post :authorize_managed_account, to: 'sso#authorize_managed_account'
resource :roadmap, only: [:show], controller: 'roadmap' resource :roadmap, only: [:show], controller: 'roadmap'
......
...@@ -246,4 +246,46 @@ describe Groups::SsoController do ...@@ -246,4 +246,46 @@ describe Groups::SsoController do
end end
end end
end end
describe 'POST authorize_managed_account' do
subject do
post :authorize_managed_account,
params: { group_id: group },
session: session
end
let(:session) { { "oauth_data" => oauth_data, "oauth_group_id" => group.id } }
let(:oauth_data) { { "info" => { name: 'Test', email: 'testuser@email.com' } } }
let!(:saml_provider) { create(:saml_provider, :enforced_group_managed_accounts, group: group) }
let(:transfer_membership_service_spy) { spy('GroupSaml::GroupManagedAccounts::TransferMembershipService') }
before do
allow(GroupSaml::GroupManagedAccounts::TransferMembershipService)
.to receive(:new).with(user, group, hash_including(session)).and_return(transfer_membership_service_spy)
end
context 'when user is already signed in' do
context 'when service succeeds' do
before do
allow(transfer_membership_service_spy).to receive(:execute).and_return(true)
end
it 'redirects to group' do
subject
expect(response).to redirect_to group_url(group)
end
end
context 'when service fails' do
before do
allow(transfer_membership_service_spy).to receive(:execute).and_return(nil)
end
it 'renders the form' do
expect(subject).to render_template :sign_up_form
end
end
end
end
end end
# frozen_string_literal: true
require 'spec_helper'
describe 'Group SAML SSO', :js do
include CookieHelper
let(:user) { create(:user) }
let(:group) { create(:group) }
let!(:saml_provider) { create(:saml_provider, group: group, enforced_sso: true, enforced_group_managed_accounts: true) }
let(:saml_response) do
fixture = File.read('ee/spec/fixtures/saml/response.xml')
OneLogin::RubySaml::Response.new(Base64.encode64(fixture))
end
let(:saml_oauth_info) do
Hash[*saml_response.attributes.to_h.flatten(2)]
end
around(:all) do |example|
with_omniauth_full_host { example.run }
end
before do
stub_feature_flags(enforced_sso: true, group_managed_accounts: true, sign_up_on_sso: true, convert_user_to_group_managed_accounts: true)
stub_licensed_features(group_saml: true)
allow(Devise).to receive(:omniauth_providers).and_return(%i(group_saml))
Warden.on_next_request do |proxy|
proxy.raw_session['oauth_data'] = OmniAuth::AuthHash.new(info: saml_oauth_info, response_object: saml_response, uid: saml_response.name_id)
proxy.raw_session['oauth_group_id'] = group.id
end
end
context 'sign_up' do
context 'when signed in' do
before do
sign_in(user)
mock_group_saml(uid: '1')
visit group_sign_up_path(group)
end
context 'SAML response includes a verified email from the logged in user' do
let(:user) { create(:user, email: saml_oauth_info['email']) }
it 'allows to complete the transfer and sign in to the group' do
expect(page).to have_link('Authorize')
click_link 'Authorize'
expect(page).to have_button('Transfer ownership')
click_button('Transfer ownership')
expect(page).to have_content('This action can lead to data loss. To prevent accidental actions we ask you to confirm your intention.')
expect(page).to have_content("Please type #{user.username} to proceed or close this modal to cancel.")
fill_in 'confirm_name_input', with: user.username
click_button 'Confirm'
expect(page).to have_current_path(group_path(group))
end
end
it "doesn't display the authorize tab" do
expect(page).not_to have_link('Authorize')
end
end
end
context 'convert_user_to_group_managed_accounts flag is disable' do
before do
stub_feature_flags(convert_user_to_group_managed_accounts: false)
sign_in(user)
mock_group_saml(uid: '1')
visit group_sign_up_path(group)
end
it "doesn't display the authorize tab" do
expect(page).not_to have_link('Authorize')
end
end
end
# frozen_string_literal: true
require 'spec_helper'
describe GroupSaml::GroupManagedAccounts::TransferMembershipService do
subject(:service) { described_class.new(current_user, group, session) }
let(:group) { create(:group) }
let(:current_user) { create(:user) }
let(:oauth_data) { OmniAuth::AuthHash.new(info: { email: current_user.email }) }
let(:session) { { 'oauth_data' => oauth_data } }
before do
stub_feature_flags(convert_user_to_group_managed_accounts: true)
allow(Gitlab::Auth::GroupSaml::IdentityLinker)
.to receive(:new).with(current_user, oauth_data, session, group.saml_provider)
.and_return(instance_double('GitLab::Auth::GroupSaml::IdentityLinker', link: '', failed?: false))
end
it 'removes the current password' do
expect { service.execute }.to change { current_user.encrypted_password }.to('')
end
it 'returns true' do
expect(service.execute).to eq true
end
it 'reduces the amount of memberships' do
create(:project_member, :developer, user: current_user)
create(:group_member, :developer, user: current_user)
expect { service.execute }.to change { current_user.members.count }.by(-2)
end
context 'when at least one non-owner member was not removed' do
before do
member = create(:group_member, :developer, user: current_user)
allow_next_instance_of(Members::DestroyService) do |service|
allow(service).to receive(:execute).with(member).and_return(member)
end
end
it 'returns a falsy value' do
expect(service.execute).to be_falsy
end
end
context 'when the user changes are not saved' do
before do
allow(current_user).to receive(:save).and_return(false)
end
it "doesn't remove account's members" do
create(:group_member, :developer, user: current_user)
expect { service.execute }.not_to change { current_user.members.count }
end
it "doesn't remove account's identities" do
create(:identity, user: current_user)
expect { service.execute }.not_to change { current_user.identities.count }
end
end
it 'removes previous known identities of the account' do
create(:identity, user: current_user)
expect { service.execute }.to change { current_user.identities.count }.from(1).to(0)
end
context "when the email doesn't match" do
let(:oauth_data) { OmniAuth::AuthHash.new(info: { email: 'new@email.com' }) }
it 'returns a falsy value' do
expect(service.execute).to be_falsy
end
end
context 'convert_user_to_group_managed_accounts flag is disable' do
before do
stub_feature_flags(convert_user_to_group_managed_accounts: false)
end
it 'returns a falsy value' do
expect(service.execute).to be_falsy
end
it "doesn't remove non-owner members without dedicated accounts from the group" do
expect { service.execute }.not_to change { current_user.members.count }
end
end
context 'transferred account' do
before do
service.execute
end
it "doesn't allow blank passwords" do
expect(current_user.valid_password?('')).to be false
expect(current_user.valid_password?(nil)).to be false
expect(current_user.valid_password?(' ')).to be false
end
end
end
...@@ -1682,6 +1682,9 @@ msgstr "" ...@@ -1682,6 +1682,9 @@ msgstr ""
msgid "Alternate support URL for help page and help dropdown" msgid "Alternate support URL for help page and help dropdown"
msgstr "" msgstr ""
msgid "Alternatively, you can convert your account to a managed account by the %{group_name} group."
msgstr ""
msgid "Amazon EKS" msgid "Amazon EKS"
msgstr "" msgstr ""
...@@ -8024,6 +8027,9 @@ msgstr "" ...@@ -8024,6 +8027,9 @@ msgstr ""
msgid "Existing shares" msgid "Existing shares"
msgstr "" msgstr ""
msgid "Existing sign in methods may be removed"
msgstr ""
msgid "Expand" msgid "Expand"
msgstr "" msgstr ""
...@@ -20616,6 +20622,9 @@ msgstr "" ...@@ -20616,6 +20622,9 @@ msgstr ""
msgid "Track your project with Audit Events." msgid "Track your project with Audit Events."
msgstr "" msgstr ""
msgid "Transfer ownership"
msgstr ""
msgid "Transfer project" msgid "Transfer project"
msgstr "" msgstr ""
...@@ -22285,6 +22294,9 @@ msgstr "" ...@@ -22285,6 +22294,9 @@ msgstr ""
msgid "You" msgid "You"
msgstr "" msgstr ""
msgid "You are about to transfer the control of your account to %{group_name} group. This action is NOT reversible, you won't be able to access any of your groups and projects outside of %{group_name} once this transfer is complete."
msgstr ""
msgid "You are an admin, which means granting access to <strong>%{client_name}</strong> will allow them to interact with GitLab as an admin as well. Proceed with caution." msgid "You are an admin, which means granting access to <strong>%{client_name}</strong> will allow them to interact with GitLab as an admin as well. Proceed with caution."
msgstr "" msgstr ""
...@@ -22594,6 +22606,9 @@ msgstr "" ...@@ -22594,6 +22606,9 @@ msgstr ""
msgid "You tried to fork %{link_to_the_project} but it failed for the following reason:" msgid "You tried to fork %{link_to_the_project} but it failed for the following reason:"
msgstr "" msgstr ""
msgid "You will be removed from existing projects/groups"
msgstr ""
msgid "You will lose all changes you've made to this file. This action cannot be undone." msgid "You will lose all changes you've made to this file. This action cannot be undone."
msgstr "" msgstr ""
......
...@@ -5,7 +5,7 @@ module QA ...@@ -5,7 +5,7 @@ module QA
module Page module Page
module Group module Group
class SamlSSOSignUp < QA::Page::Base class SamlSSOSignUp < QA::Page::Base
view 'ee/app/views/groups/sso/sign_up_form.html.haml' do view 'ee/app/views/groups/sso/_register_pane.html.haml' do
element :sign_out_and_register_button element :sign_out_and_register_button
element :new_user_email_field element :new_user_email_field
element :new_user_username_field element :new_user_username_field
......
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