Merge branch '13481-map-users-to-group-managed-accounts' into 'master'

Allow to convert users to Group Managed Accounts

See merge request gitlab-org/gitlab!24329

doc/user/group/saml_sso/index.md

@@ -73,9 +73,13 @@ Without group-managed accounts, users can link their SAML identity with any exis
 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.
-- 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.
 
+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:
 
 - The user will be unable to access the group (their credentials will no longer work on the identity provider when prompted to SSO).

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

 import UsernameValidator from '~/pages/sessions/new/username_validator';
+import initConfirmDangerModal from '~/confirm_danger_modal';
 
 document.addEventListener('DOMContentLoaded', () => {
   new UsernameValidator(); // eslint-disable-line no-new
+  initConfirmDangerModal();
 });

ee/app/controllers/groups/sso_controller.rb

@@ -9,7 +9,7 @@ class Groups::SsoController < Groups::ApplicationController
   before_action :require_enabled_provider!, except: [:unlink]
   before_action :check_user_can_sign_in_with_provider, only: [:saml]
   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'
 
@@ -48,6 +48,20 @@ class Groups::SsoController < Groups::ApplicationController
     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
 
   def new_user
@@ -94,6 +108,10 @@ class Groups::SsoController < Groups::ApplicationController
     @unauthenticated_group ||= Group.find_by_full_path(params[:group_id], follow_redirects: true)
   end
 
+  def store_active_saml_session
+    Gitlab::Auth::GroupSaml::SsoEnforcer.new(unauthenticated_group.saml_provider).update_session
+  end
+
   def require_configured_provider!
     unless unauthenticated_group&.feature_available?(:group_saml) && Gitlab::Auth::GroupSaml::Config.enabled?
       return route_not_found

ee/app/helpers/groups/sso_helper.rb

+# 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

ee/app/services/group_saml/group_managed_accounts/transfer_membership_service.rb

+# 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

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

+#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

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

+- 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

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

+#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"

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

+%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')

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

-- 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
-  .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.mt3.mx3
-        .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"
+%div
+  = render 'tabs', resource: resource, current_user: current_user
+  .tab-content
+    = render 'register_pane', resource: resource
+    - if ::Feature.enabled?(:convert_user_to_group_managed_accounts) && current_user && current_user.verified_email?(resource.email)
+      = render 'authorize_pane'

ee/changelogs/unreleased/13481-map-users-to-group-managed-accounts.yml

+---
+title: Allow existing users to transfer their account to SAML Group managed accounts
+merge_request: 24329
+author:
+type: added

ee/config/routes/group.rb

@@ -141,6 +141,7 @@ constraints(::Constraints::GroupUrlConstrainer.new) do
 
     get :sign_up, to: 'sso#sign_up_form'
     post :sign_up, to: 'sso#sign_up'
+    post :authorize_managed_account, to: 'sso#authorize_managed_account'
 
     resource :roadmap, only: [:show], controller: 'roadmap'
 

ee/spec/controllers/groups/sso_controller_spec.rb

@@ -246,4 +246,46 @@ describe Groups::SsoController do
       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

ee/spec/features/groups/sso_spec.rb

+# 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

ee/spec/services/group_saml/group_managed_accounts/transfer_membership_service_spec.rb

+# 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

locale/gitlab.pot

@@ -1682,6 +1682,9 @@ msgstr ""
 msgid "Alternate support URL for help page and help dropdown"
 msgstr ""
 
+msgid "Alternatively, you can convert your account to a managed account by the %{group_name} group."
+msgstr ""
+
 msgid "Amazon EKS"
 msgstr ""
 
@@ -8036,6 +8039,9 @@ msgstr ""
 msgid "Existing shares"
 msgstr ""
 
+msgid "Existing sign in methods may be removed"
+msgstr ""
+
 msgid "Expand"
 msgstr ""
 
@@ -20625,6 +20631,9 @@ msgstr ""
 msgid "Track your project with Audit Events."
 msgstr ""
 
+msgid "Transfer ownership"
+msgstr ""
+
 msgid "Transfer project"
 msgstr ""
 
@@ -22294,6 +22303,9 @@ msgstr ""
 msgid "You"
 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."
 msgstr ""
 
@@ -22600,6 +22612,9 @@ msgstr ""
 msgid "You tried to fork %{link_to_the_project} but it failed for the following reason:"
 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."
 msgstr ""
 

qa/qa/ee/page/group/saml_sso_sign_up.rb

@@ -5,7 +5,7 @@ module QA
     module Page
       module Group
         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 :new_user_email_field
             element :new_user_username_field