Introduce new sign up page for group SSO

Users should be able tu sign up automatically
if their current account is not eligible
to join the group or if they have no gitlab account

app/assets/stylesheets/pages/login.scss

@@ -42,8 +42,8 @@
   .login-box,
   .omniauth-container {
     box-shadow: 0 0 0 1px $border-color;
-    border-bottom-right-radius: $border-radius-small;
-    border-bottom-left-radius: $border-radius-small;
+    border-bottom-right-radius: $border-radius;
+    border-bottom-left-radius: $border-radius;
     padding: 15px;
 
     .login-heading h3 {
@@ -88,7 +88,7 @@
   }
 
   .omniauth-container {
-    border-radius: $border-radius-small;
+    border-radius: $border-radius;
     font-size: 13px;
 
     p {

app/views/devise/shared/_signup_box.html.haml

@@ -4,24 +4,24 @@
       .devise-errors
         = render "devise/shared/error_messages", resource: resource
       .name.form-group
-        = f.label :name, 'Full name', class: 'label-bold'
+        = f.label :name, _('Full name'), class: 'label-bold'
         = f.text_field :name, class: "form-control top qa-new-user-name js-block-emoji", required: true, title: _("This field is required.")
       .username.form-group
         = f.label :username, class: 'label-bold'
         = f.text_field :username, class: "form-control middle 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...
+        %p.validation-error.hide= _('Username is already taken.')
+        %p.validation-success.hide= _('Username is available.')
+        %p.validation-pending.hide= _('Checking username availability...')
       .form-group
         = f.label :email, class: 'label-bold'
-        = f.email_field :email, class: "form-control middle qa-new-user-email", required: true, title: "Please provide a valid email address."
+        = f.email_field :email, class: "form-control middle qa-new-user-email", required: true, title: _("Please provide a valid email address.")
       .form-group
         = f.label :email_confirmation, class: 'label-bold'
-        = f.email_field :email_confirmation, class: "form-control middle qa-new-user-email-confirmation", required: true, title: "Please retype the email address."
+        = f.email_field :email_confirmation, class: "form-control middle qa-new-user-email-confirmation", required: true, title: _("Please retype the email address.")
       .form-group.append-bottom-20#password-strength
         = f.label :password, class: 'label-bold'
-        = f.password_field :password, class: "form-control bottom qa-new-user-password", required: true, pattern: ".{#{@minimum_password_length},}", title: "Minimum length is #{@minimum_password_length} characters."
-        %p.gl-field-hint.text-secondary Minimum length is #{@minimum_password_length} characters
+        = f.password_field :password, class: "form-control bottom qa-new-user-password", required: true, pattern: ".{#{@minimum_password_length},}", title: _("Minimum length is %{minimum_password_length} characters.") % { minimum_password_length: @minimum_password_length }
+        %p.gl-field-hint.text-secondary= _('Minimum length is %{minimum_password_length} characters') % { minimum_password_length: @minimum_password_length }
       - 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'
@@ -34,4 +34,4 @@
       - if Gitlab::Recaptcha.enabled?
         = recaptcha_tags
       .submit-container
-        = f.submit "Register", class: "btn-register btn qa-new-user-register-button"
+        = f.submit _("Register"), class: "btn-register btn qa-new-user-register-button"

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

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

ee/app/controllers/groups/omniauth_callbacks_controller.rb

@@ -43,14 +43,16 @@ class Groups::OmniauthCallbacksController < OmniauthCallbacksController
   def redirect_identity_link_failed(error_message)
     flash[:notice] = "SAML authentication failed: #{error_message}"
 
-    redirect_to after_sign_in_path_for(current_user)
+    if ::Feature.enabled?(:sign_up_on_sso, @unauthenticated_group) && @saml_provider.enforced_group_managed_accounts?
+      redirect_to_group_sign_up
+    else
+      redirect_to after_sign_in_path_for(current_user)
+    end
   end
 
   override :sign_in_and_redirect
   def sign_in_and_redirect(user, *args)
-    flash[:notice] = "Signed in with SAML for #{@unauthenticated_group.name}"
-
-    super
+    super.tap { flash[:notice] = "Signed in with SAML for #{@unauthenticated_group.name}" }
   end
 
   override :sign_in
@@ -103,7 +105,11 @@ class Groups::OmniauthCallbacksController < OmniauthCallbacksController
     if user
       super
     else
-      redirect_to_login_or_register
+      if ::Feature.enabled?(:sign_up_on_sso, @unauthenticated_group) && @saml_provider.enforced_group_managed_accounts?
+        redirect_to_group_sign_up
+      else
+        redirect_to_login_or_register
+      end
     end
   end
 
@@ -113,9 +119,17 @@ class Groups::OmniauthCallbacksController < OmniauthCallbacksController
     after_gitlab_sign_in = sso_group_saml_providers_path(@unauthenticated_group)
 
     store_location_for(:redirect, after_gitlab_sign_in)
+
     redirect_to new_user_session_path, notice: notice
   end
 
+  def redirect_to_group_sign_up
+    session['oauth_data'] = oauth
+    session['oauth_group_id'] = @unauthenticated_group.id
+
+    redirect_to group_sign_up_path(@unauthenticated_group)
+  end
+
   def saml_redirect_path
     params['RelayState'].presence if current_user
   end

ee/app/controllers/groups/sso_controller.rb

@@ -2,22 +2,21 @@
 
 class Groups::SsoController < Groups::ApplicationController
   skip_before_action :group
-  before_action :unauthenticated_group
-  before_action :check_group_saml_configured
-  before_action :check_group_saml_available!
-  before_action :require_configured_provider
-  before_action :require_enabled_provider, except: [:unlink]
+
   before_action :authenticate_user!, only: [:unlink]
+  before_action :require_configured_provider!
+  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]
 
   layout 'devise'
 
   def saml
     @group_path = params[:group_id]
-    @group_name = @unauthenticated_group.full_name
+    @group_name = unauthenticated_group.full_name
     @group_saml_identity = linked_identity
-    @idp_url = @unauthenticated_group.saml_provider.sso_url
+    @idp_url = unauthenticated_group.saml_provider.sso_url
   end
 
   def unlink
@@ -28,42 +27,83 @@ class Groups::SsoController < Groups::ApplicationController
     redirect_to profile_account_path
   end
 
+  def sign_up_form
+    @group_name = unauthenticated_group.full_name
+
+    render_sign_up_form
+  end
+
+  def sign_up
+    sign_up_service = GroupSaml::SignUpService.new(new_user, unauthenticated_group, session)
+
+    if sign_up_service.execute
+      session['oauth_data'] = nil
+      sign_out
+      flash[:notice] = _('Sign up was successful! Please confirm your email to sign in.')
+      redirect_to_sign_in
+    else
+      render_sign_up_form
+    end
+  end
+
   private
 
-  def linked_identity
-    @linked_identity ||= GroupSamlIdentityFinder.new(user: current_user).find_linked(group: @unauthenticated_group)
+  def new_user
+    @new_user ||= User.new(new_user_params.merge(idp_user_data))
   end
+  # Devise compatible name
+  alias_method :resource, :new_user
+  helper_method :resource
 
-  def check_group_saml_available!
-    route_not_found unless @unauthenticated_group.feature_available?(:group_saml)
+  def new_user_params
+    params.fetch(:new_user, {}).permit(:username, :name)
   end
 
-  def check_group_saml_configured
-    route_not_found unless Gitlab::Auth::GroupSaml::Config.enabled?
+  def idp_user_data
+    return {} unless session['oauth_data'] && session['oauth_group_id'] == unauthenticated_group.id
+
+    data = Gitlab::Auth::OAuth::AuthHash.new(session['oauth_data'])
+
+    { email: data.email, name: data.name }
   end
 
-  def unauthenticated_group
-    @unauthenticated_group = Group.find_by_full_path(params[:group_id], follow_redirects: true)
+  def check_oauth_data
+    route_not_found unless unauthenticated_group.saml_provider.enforced_group_managed_accounts? && idp_user_data.present?
+  end
+
+  def render_sign_up_form
+    flash[:notice] = _('%{group_name} uses group managed accounts. You need to create a new GitLab account which will be managed by %{group_name}.') % { group_name: unauthenticated_group.full_name }
 
-    route_not_found unless @unauthenticated_group
+    render :sign_up_form
   end
 
-  def require_configured_provider
-    return if @unauthenticated_group.saml_provider
+  def linked_identity
+    @linked_identity ||= GroupSamlIdentityFinder.new(user: current_user).find_linked(group: unauthenticated_group)
+  end
+
+  def unauthenticated_group
+    @unauthenticated_group ||= Group.find_by_full_path(params[:group_id], follow_redirects: true)
+  end
+
+  def require_configured_provider!
+    unless unauthenticated_group&.feature_available?(:group_saml) && Gitlab::Auth::GroupSaml::Config.enabled?
+      return route_not_found
+    end
+
+    return if unauthenticated_group.saml_provider
 
     redirect_settings_or_not_found
   end
 
-  def require_enabled_provider
-    return if @unauthenticated_group.saml_provider&.enabled?
+  def require_enabled_provider!
+    return if unauthenticated_group.saml_provider&.enabled?
 
     redirect_settings_or_not_found
   end
 
   def redirect_settings_or_not_found
-    if can?(current_user, :admin_group_saml, @unauthenticated_group)
+    if can?(current_user, :admin_group_saml, unauthenticated_group)
       flash[:notice] = 'SAML sign on has not been configured for this group'
-
       redirect_to [@unauthenticated_group, :saml_providers]
     else
       route_not_found
@@ -72,7 +112,7 @@ class Groups::SsoController < Groups::ApplicationController
 
   def check_user_can_sign_in_with_provider
     actor = saml_discovery_token_actor || current_user
-    route_not_found unless can?(actor, :sign_in_with_saml_provider, @unauthenticated_group.saml_provider)
+    route_not_found unless can?(actor, :sign_in_with_saml_provider, unauthenticated_group.saml_provider)
   end
 
   def saml_discovery_token_actor
@@ -80,6 +120,10 @@ class Groups::SsoController < Groups::ApplicationController
   end
 
   def redirect_if_group_moved
-    ensure_canonical_path(@unauthenticated_group, params[:group_id])
+    ensure_canonical_path(unauthenticated_group, params[:group_id])
+  end
+
+  def redirect_to_sign_in
+    redirect_to sso_group_saml_providers_url(unauthenticated_group, token: unauthenticated_group.saml_discovery_token)
   end
 end

ee/app/finders/ee/group_members_finder.rb

@@ -9,8 +9,7 @@ module EE::GroupMembersFinder
 
   # rubocop: disable CodeReuse/ActiveRecord
   def not_managed
-    group.group_members.non_owners.joins(:user)
-      .where("users.managing_group_id IS NULL OR users.managing_group_id != ?", group.id)
+    group.group_members.non_owners.joins(:user).merge(User.not_managed(group: group))
   end
   # rubocop: enable CodeReuse/ActiveRecord
 end

ee/app/finders/group_saml_identity_finder.rb

@@ -11,6 +11,12 @@ class GroupSamlIdentityFinder
   end
   # rubocop: enable CodeReuse/ActiveRecord
 
+  # rubocop: disable CodeReuse/ActiveRecord
+  def self.not_managed_identities(group:)
+    group.saml_provider.identities.joins(:user).merge(User.not_managed(group: group))
+  end
+  # rubocop: enable CodeReuse/ActiveRecord
+
   def initialize(user:)
     @user = user
   end

ee/app/models/ee/identity.rb

@@ -16,6 +16,8 @@ module EE
           case_sensitive: false
         }
 
+      validate :validate_managing_group
+
       scope :with_secondary_extern_uid, ->(provider, secondary_extern_uid) do
         iwhere(secondary_extern_uid: normalize_uid(provider, secondary_extern_uid)).with_provider(provider)
       end
@@ -51,5 +53,13 @@ module EE
         preload(saml_provider: { group: :route })
       end
     end
+
+    private
+
+    def validate_managing_group
+      return unless saml_provider&.enforced_group_managed_accounts?
+
+      errors.add(:base, 'Group requires separate account') if saml_provider.group != user.managing_group
+    end
   end
 end

ee/app/models/ee/user.rb

@@ -57,6 +57,12 @@ module EE
 
       belongs_to :managing_group, class_name: 'Group', optional: true, inverse_of: :managed_users
 
+      scope :not_managed, ->(group: nil) {
+        scope = where(managing_group_id: nil)
+        scope = scope.or(where.not(managing_group_id: group.id)) if group
+        scope
+      }
+
       scope :excluding_guests, -> { joins(:members).where('members.access_level > ?', ::Gitlab::Access::GUEST).distinct }
 
       scope :subscribed_for_admin_email, -> { where(admin_email_unsubscribed_at: nil) }

ee/app/models/saml_provider.rb

@@ -34,7 +34,7 @@ class SamlProvider < ApplicationRecord
   end
 
   def enforced_group_managed_accounts?
-    enforced_sso? && super
+    super && enforced_sso? && Feature.enabled?(:group_managed_accounts, group)
   end
 
   class DefaultOptions

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

@@ -10,17 +10,34 @@ module GroupSaml
       end
 
       def execute
+        destroy_non_gma_members && destroy_non_gma_identities
+      end
+
+      private
+
+      def destroy_non_gma_members
         non_group_managed_accounts.all? do |group_membership|
-          membership = Members::DestroyService.new(current_user).execute(group_membership)
-          membership.destroyed?
+          Members::DestroyService.new(current_user).execute(group_membership)
+          group_membership.destroyed?
         end
       end
 
-      private
+      def destroy_non_gma_identities
+        non_group_managed_identities.all? do |identity|
+          next true if group.has_owner?(identity.user)
+
+          Identity::DestroyService.new(identity).execute
+          identity.destroyed?
+        end
+      end
 
       def non_group_managed_accounts
         @non_group_managed_accounts ||= GroupMembersFinder.new(group).not_managed
       end
+
+      def non_group_managed_identities
+        @non_group_managed_identities ||= GroupSamlIdentityFinder.not_managed_identities(group: group)
+      end
     end
   end
 end

ee/app/services/group_saml/sign_up_service.rb

+# frozen_string_literal: true
+module GroupSaml
+  class SignUpService
+    attr_reader :group, :new_user, :oauth_data, :session
+
+    def initialize(new_user, group, session)
+      @new_user = new_user
+      @group = group
+      @oauth_data = session['oauth_data']
+      @session = session
+    end
+
+    def execute
+      ActiveRecord::Base.transaction do
+        new_user.managing_group = group if group.saml_provider&.enforced_group_managed_accounts?
+
+        if new_user.save
+          identity_linker = Gitlab::Auth::GroupSaml::IdentityLinker.new(new_user, oauth_data, group.saml_provider, session)
+          identity_linker.link
+        end
+
+        new_user.persisted? && !identity_linker.failed?
+      end
+    end
+  end
+end

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

+.avatar-container.s32.mr-2
+  = user_avatar_without_link(user: current_user, size: 32)
+%div
+  %div
+    %strong= current_user.name
+    %span= current_user.to_reference
+  %span.text-secondary= current_user.email

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

+- page_title _('SAML SSO for %{group_name}') % { group_name: @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 qa-new-user-email", required: true, disabled: true
+      .name.form-group
+        = f.label :name, _('Full name'), class: 'label-bold'
+        = f.text_field :name, class: "form-control top qa-new-user-name js-block-emoji", required: true, 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"
+      - else
+        .submit-container
+          = f.submit _("Register"), class: "btn-register btn qa-new-user-register-button"

ee/changelogs/unreleased/5292-new-user-flow-for-group-managed-accounts.yml

+---
+title: New user flow for SSOing into a GitLab.com group
+merge_request: 10338
+author:
+type: other

ee/config/routes/group.rb

@@ -97,6 +97,9 @@ constraints(::Constraints::GroupUrlConstrainer.new) do
 
     resource :scim_oauth, only: [:show, :create], controller: :scim_oauth
 
+    get :sign_up, to: 'sso#sign_up_form'
+    post :sign_up, to: 'sso#sign_up'
+
     resource :roadmap, only: [:show], controller: 'roadmap'
 
     legacy_ee_group_boards_redirect = redirect do |params, request|

ee/spec/controllers/groups/omniauth_callbacks_controller_spec.rb

@@ -151,21 +151,40 @@ describe Groups::OmniauthCallbacksController do
             expect(flash[:notice]).to eq 'Request to link SAML account must be authorized'
           end
         end
+
+        context 'with enforced_group_managed_accounts enabled' do
+          let!(:saml_provider) { create(:saml_provider, :enforced_group_managed_accounts, group: group) }
+
+          it 'redirects to group sign up' do
+            post provider, params: { group_id: group }
+
+            expect(response).to redirect_to(group_sign_up_path(group))
+          end
+        end
       end
     end
 
     context "when not signed in" do
       context "and identity hasn't been linked" do
-        it "redirects to sign in page" do
-          post provider, params: { group_id: group }
+        let!(:saml_provider) { create(:saml_provider, :enforced_group_managed_accounts, group: group) }
 
-          expect(response).to redirect_to(new_user_session_path)
+        context 'when sign_up_on_sso feature flag is disabled' do
+          before do
+            stub_feature_flags(sign_up_on_sso: false)
+          end
+
+          it "redirects to sign in page with flash notice" do
+            post provider, params: { group_id: group }
+
+            expect(response).to redirect_to(new_user_session_path)
+            expect(flash[:notice]).to start_with("Login to a GitLab account to link with your SAML identity")
+          end
         end
 
-        it "informs users that they need to sign in to the GitLab instance first" do
+        it 'redirects to group sign up page' do
           post provider, params: { group_id: group }
 
-          expect(flash[:notice]).to start_with("Login to a GitLab account to link with your SAML identity")
+          expect(response).to redirect_to(group_sign_up_path(group))
         end
       end
 
@@ -244,6 +263,8 @@ describe Groups::OmniauthCallbacksController do
     end
 
     context "with access to SAML settings for the group" do
+      let(:user) { create_linked_user }
+
       before do
         group.add_owner(user)
         sign_in(user)

ee/spec/controllers/groups/sso_controller_spec.rb

@@ -2,7 +2,7 @@ require 'spec_helper'
 
 describe Groups::SsoController do
   let(:user) { create(:user) }
-  let(:group) { create(:group, :private, name: 'our-group') }
+  let(:group) { create(:group, :private, name: 'our-group', saml_discovery_token: 'test-token') }
 
   before do
     stub_licensed_features(group_saml: true)
@@ -119,4 +119,114 @@ describe Groups::SsoController do
       end
     end
   end
+
+  describe 'GET sign_up_form' do
+    subject do
+      get :sign_up_form,
+          params: { group_id: group },
+          session: { "oauth_data" => oauth_data, "oauth_group_id" => oauth_group_id }
+    end
+
+    let(:oauth_data) { nil }
+    let(:oauth_group_id) { group.id }
+
+    context 'with SAML configured' do
+      let!(:saml_provider) { create(:saml_provider, :enforced_group_managed_accounts, group: group) }
+
+      context 'and group managed accounts enforced' do
+        context 'and oauth data available' do
+          let(:oauth_data) { { "info" => { "name" => 'Test', "email" => 'email@email.com' } } }
+
+          it 'has status 200' do
+            expect(subject).to have_gitlab_http_status(200)
+          end
+
+          context 'and belongs to different group' do
+            let(:oauth_group_id) { group.id + 1 }
+
+            it 'renders 404' do
+              expect(subject).to have_gitlab_http_status(404)
+            end
+          end
+        end
+
+        it 'renders 404' do
+          expect(subject).to have_gitlab_http_status(404)
+        end
+      end
+
+      context 'and group managed accounts enforcing is disabled' do
+        before do
+          saml_provider.update(enforced_group_managed_accounts: false)
+        end
+
+        it 'renders 404' do
+          expect(subject).to have_gitlab_http_status(404)
+        end
+      end
+    end
+  end
+
+  describe 'POST sign_up' do
+    subject do
+      post :sign_up,
+           params: { group_id: group, new_user: new_user_data },
+           session: { "oauth_data" => oauth_data, "oauth_group_id" => group.id }
+    end
+
+    let(:new_user_data) { { username: "myusername" } }
+    let(:oauth_data) { { "info" => { "name" => 'Test', "email" => 'email@email.com' } } }
+
+    let!(:saml_provider) { create(:saml_provider, :enforced_group_managed_accounts, group: group) }
+
+    let(:sign_up_service_spy) { spy('GroupSaml::SignUpService') }
+
+    before do
+      allow(GroupSaml::SignUpService)
+        .to receive(:new).with(kind_of(User), group, anything).and_return(sign_up_service_spy)
+    end
+
+    it 'calls for GroupSaml::SignUpService' do
+      subject
+
+      expect(sign_up_service_spy).to have_received(:execute)
+    end
+
+    context 'when service fails' do
+      before do
+        allow(sign_up_service_spy).to receive(:execute).and_return(false)
+      end
+
+      it 'renders the form' do
+        expect(subject).to render_template :sign_up_form
+      end
+    end
+
+    context 'when service succeeds' do
+      before do
+        allow(sign_up_service_spy).to receive(:execute).and_return(true)
+      end
+
+      it 'redirects to sign in' do
+        subject
+
+        expect(flash[:notice]).to eq "Sign up was successful! Please confirm your email to sign in."
+        expect(response).to redirect_to sso_group_saml_providers_url(group, token: group.saml_discovery_token)
+      end
+
+      context 'when user is already signed in' do
+        let(:user) { create :user }
+
+        before do
+          sign_in user
+        end
+
+        it 'signs user out' do
+          subject
+
+          expect(request.env['warden']).not_to be_authenticated
+        end
+      end
+    end
+  end
 end

ee/spec/factories/saml_providers.rb

@@ -3,5 +3,11 @@ FactoryBot.define do
     group
     certificate_fingerprint '55:44:33:22:11:aa:bb:cc:dd:ee:ff:11:22:33:44:55:66:77:88:99'
     sso_url 'https://saml.example.com/adfs/ls'
+
+    trait :enforced_group_managed_accounts do
+      enabled true
+      enforced_sso true
+      enforced_group_managed_accounts true
+    end
   end
 end

ee/spec/finders/group_saml_identity_finder_spec.rb

@@ -7,7 +7,8 @@ describe GroupSamlIdentityFinder do
 
   let(:user) { create(:user) }
   let!(:identity) { create(:group_saml_identity, user: user) }
-  let(:group) { identity.saml_provider.group }
+  let(:saml_provider) { identity.saml_provider }
+  let(:group) { saml_provider.group }
 
   subject { described_class.new(user: user) }
 
@@ -23,6 +24,21 @@ describe GroupSamlIdentityFinder do
     end
   end
 
+  describe '.not_managed_identities' do
+    subject { described_class.not_managed_identities(group: group) }
+
+    let!(:group_managed_identity) do
+      create(:group_saml_identity, saml_provider: saml_provider, user: create(:user, managing_group: group))
+    end
+    let!(:different_group_managed_identity) do
+      create(:group_saml_identity, saml_provider: saml_provider, user: create(:user, :group_managed))
+    end
+
+    it 'returns all identities of users not managed by given group' do
+      expect(subject).to match_array([identity, different_group_managed_identity])
+    end
+  end
+
   describe "#find_linked" do
     it "finds identity matching user and group" do
       expect(subject.find_linked(group: group)).to eq(identity)

ee/spec/models/identity_spec.rb

@@ -19,5 +19,27 @@ describe Identity do
       expect(identity).not_to be_valid
       expect(identity.errors.full_messages.join).to include("NameID can't be blank")
     end
+
+    context 'with enforced_group_managed_accounts' do
+      subject { build_stubbed(:identity, provider: 'group_saml', saml_provider: saml_provider, user: user) }
+
+      let(:saml_provider) { build_stubbed(:saml_provider, :enforced_group_managed_accounts) }
+
+      context 'when managing_group matches saml_provider group' do
+        let(:user) { build_stubbed(:user, managing_group: saml_provider.group) }
+
+        it { is_expected.to be_valid }
+      end
+
+      context 'when managing_group does not match provider group' do
+        let(:user) { build_stubbed(:user, managing_group: Group.new) }
+
+        it 'is not valid' do
+          expect do
+            subject.valid?
+          end.to change { subject.errors[:base] }.to(['Group requires separate account'])
+        end
+      end
+    end
   end
 end

ee/spec/models/saml_provider_spec.rb

@@ -61,7 +61,7 @@ describe SamlProvider do
   end
 
   describe '#settings' do
-    let(:group) { create(:group, path: 'foo-group')}
+    let(:group) { create(:group, path: 'foo-group') }
     let(:settings) { subject.settings }
 
     subject(:saml_provider) { create(:saml_provider, group: group) }
@@ -103,6 +103,18 @@ describe SamlProvider do
         subject.enforced_sso = false
         expect(subject).not_to be_enforced_sso
       end
+
+      context 'and feature flag is disabled' do
+        before do
+          stub_feature_flags(enforced_sso: false)
+        end
+
+        it 'is false' do
+          subject.enforced_sso = true
+
+          expect(subject).not_to be_enforced_sso
+        end
+      end
     end
 
     context 'when provider is disabled' do
@@ -120,6 +132,10 @@ describe SamlProvider do
   end
 
   describe '#enforced_group_managed_accounts?' do
+    before do
+      stub_feature_flags(group_managed_accounts: true)
+    end
+
     context 'when enforced_sso is enabled' do
       before do
         subject.enabled = true
@@ -132,6 +148,18 @@ describe SamlProvider do
         subject.enforced_group_managed_accounts = false
         expect(subject).not_to be_enforced_group_managed_accounts
       end
+
+      context 'and feature flag is disabled' do
+        before do
+          stub_feature_flags(group_managed_accounts: false)
+        end
+
+        it 'is false' do
+          subject.enforced_group_managed_accounts = true
+
+          expect(subject).not_to be_enforced_group_managed_accounts
+        end
+      end
     end
 
     context 'when enforced_sso is disabled' do

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

@@ -6,19 +6,26 @@ describe GroupSaml::GroupManagedAccounts::CleanUpMembersService do
   subject(:service) { described_class.new(current_user, group) }
   let(:group) { Group.new }
   let(:current_user) { instance_double('User') }
-  let(:destroy_member_service_spy) { spy('GroupSaml::GroupManagedAccounts::CleanUpMembersService') }
+  let(:destroy_member_service_spy) { spy('Members::DestroyService') }
+  let(:destroy_identity_service_spy) { spy('GroupSaml::Identity::DestroyService') }
+
+  let(:group_member_membership) { instance_double('Member', destroyed?: true) }
+  let(:not_managed_identity) { instance_double('Identity', destroyed?: true, user: nil) }
 
-  let(:group_member_membership) { instance_double('Member')}
   before do
-    allow(Members::DestroyService)
-      .to receive(:new).with(current_user).and_return(destroy_member_service_spy)
+    allow(Members::DestroyService).to receive(:new).with(current_user).and_return(destroy_member_service_spy)
+    allow(GroupSaml::Identity::DestroyService)
+      .to receive(:new).with(not_managed_identity).and_return(destroy_identity_service_spy)
     allow(GroupMembersFinder)
       .to receive(:new).with(group)
             .and_return(instance_double('GroupMembersFinder', not_managed: [group_member_membership]))
+    allow(GroupSamlIdentityFinder)
+      .to receive(:not_managed_identities).with(group: group).and_return([not_managed_identity])
   end
 
   it 'removes non-owner members without dedicated accounts from the group' do
     service.execute
+
     expect(destroy_member_service_spy).to have_received(:execute).with(group_member_membership)
   end
 
@@ -28,11 +35,27 @@ describe GroupSaml::GroupManagedAccounts::CleanUpMembersService do
 
   context 'when at least one non-owner member was not removed' do
     before do
-      allow(destroy_member_service_spy).to receive(:destroyed?).and_return(false)
+      allow(group_member_membership).to receive(:destroyed?).and_return(false)
     end
 
     it 'returns false' do
       expect(service.execute).to eq false
     end
   end
+
+  it 'unlinks identities for accounts not managed by given group' do
+    service.execute
+
+    expect(destroy_identity_service_spy).to have_received(:execute)
+  end
+
+  context 'for last group owner identity' do
+    it 'does not remove the identity' do
+      allow(group).to receive(:has_owner?).with(not_managed_identity.user).and_return true
+
+      service.execute
+
+      expect(destroy_identity_service_spy).not_to have_received(:execute)
+    end
+  end
 end

ee/spec/services/group_saml/sign_up_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe GroupSaml::SignUpService do
+  subject(:service) { described_class.new(new_user, group, session) }
+
+  let(:new_user) { build(:user) }
+  let!(:group) { create(:saml_provider).group }
+  let(:session) { { 'oauth_data' => {} }}
+
+  describe '#execute' do
+    let(:linker_spy) { spy('Gitlab::Auth::GroupSaml::IdentityLinker') }
+
+    before do
+      allow(Gitlab::Auth::GroupSaml::IdentityLinker)
+        .to receive(:new).with(new_user, session['oauth_data'], group.saml_provider, session)
+              .and_return(linker_spy)
+    end
+
+    it 'creates new user' do
+      expect { service.execute }.to change { User.count }.by(1)
+    end
+
+    it 'links new user to oauth identity' do
+      service.execute
+
+      expect(linker_spy).to have_received(:link)
+    end
+
+    context 'when group has enforced_group_managed_accounts enabled' do
+      before do
+        allow(group.saml_provider).to receive(:enforced_group_managed_accounts?).and_return(true)
+      end
+
+      it 'creates new user managed by given group' do
+        expect { service.execute }.to change { group.managed_users.count }.by(1)
+      end
+    end
+  end
+end

lib/gitlab/auth/o_auth/auth_hash.rb

@@ -57,7 +57,7 @@ module Gitlab
         private
 
         def info
-          auth_hash.info
+          auth_hash['info']
         end
 
         def get_info(key)

locale/gitlab.pot

@@ -193,6 +193,9 @@ msgstr ""
 msgid "%{group_docs_link_start}Groups%{group_docs_link_end} allow you to manage and collaborate across multiple projects. Members of a group have access to all of its projects."
 msgstr ""
 
+msgid "%{group_name} uses group managed accounts. You need to create a new GitLab account which will be managed by %{group_name}."
+msgstr ""
+
 msgid "%{issuableType} will be removed! Are you sure?"
 msgstr ""
 
@@ -2322,6 +2325,9 @@ msgstr ""
 msgid "Checking branch availability..."
 msgstr ""
 
+msgid "Checking username availability..."
+msgstr ""
+
 msgid "Cherry-pick this commit"
 msgstr ""
 
@@ -5472,6 +5478,9 @@ msgstr ""
 msgid "Finish review"
 msgstr ""
 
+msgid "Finish setting up your dedicated account for <strong>%{group_name}</strong>."
+msgstr ""
+
 msgid "Finished"
 msgstr ""
 
@@ -5619,6 +5628,9 @@ msgstr ""
 msgid "From the Kubernetes cluster details view, install Runner from the applications list"
 msgstr ""
 
+msgid "Full name"
+msgstr ""
+
 msgid "GPG Key ID:"
 msgstr ""
 
@@ -8099,6 +8111,12 @@ msgstr ""
 msgid "Minimum capacity to be available before we schedule more mirrors preemptively."
 msgstr ""
 
+msgid "Minimum length is %{minimum_password_length} characters"
+msgstr ""
+
+msgid "Minimum length is %{minimum_password_length} characters."
+msgstr ""
+
 msgid "Minutes"
 msgstr ""
 
@@ -9264,6 +9282,12 @@ msgstr ""
 msgid "Please provide a name"
 msgstr ""
 
+msgid "Please provide a valid email address."
+msgstr ""
+
+msgid "Please retype the email address."
+msgstr ""
+
 msgid "Please select a file"
 msgstr ""
 
@@ -10379,6 +10403,9 @@ msgstr ""
 msgid "Regex pattern"
 msgstr ""
 
+msgid "Register"
+msgstr ""
+
 msgid "Register / Sign In"
 msgstr ""
 
@@ -11557,6 +11584,12 @@ msgstr ""
 msgid "Sign out"
 msgstr ""
 
+msgid "Sign out & Register"
+msgstr ""
+
+msgid "Sign up was successful! Please confirm your email to sign in."
+msgstr ""
+
 msgid "Sign-in restrictions"
 msgstr ""
 
@@ -13924,6 +13957,12 @@ msgstr ""
 msgid "UserProfile|Your projects can be available publicly, internally, or privately, at your choice."
 msgstr ""
 
+msgid "Username is already taken."
+msgstr ""
+
+msgid "Username is available."
+msgstr ""
+
 msgid "Users"
 msgstr ""
 
@@ -14682,6 +14721,9 @@ msgstr ""
 msgid "You won't be able to pull or push project code via SSH until you add an SSH key to your profile"
 msgstr ""
 
+msgid "You'll be signed out from your current account automatically."
+msgstr ""
+
 msgid "You'll need to use different branch names to get a valid comparison."
 msgstr ""