Merge branch 'dblessing_group_saml_jit' into 'master'

Allow Group SAML to auto-created new users

See merge request gitlab-org/gitlab!48953

doc/user/group/saml_sso/index.md

@@ -194,11 +194,13 @@ If the information you need isn't listed above you may wish to check our [troubl
 
 Once Group SSO is configured and enabled, users can access the GitLab.com group through the identity provider's dashboard. If [SCIM](scim_setup.md) is configured, please see the [user access and linking setup section on the SCIM page](scim_setup.md#user-access-and-linking-setup).
 
-When a user tries to sign in with Group SSO, they need an account that's configured with one of the following:
+When a user tries to sign in with Group SSO, GitLab attempts to find or create a user based on the following:
 
-- [SCIM](scim_setup.md).
-- [Group-managed accounts](group_managed_accounts.md).
-- A GitLab.com account.
+- Find an existing user with a matching SAML identity. This would mean the user either had their account created by [SCIM](scim_setup.md) or they have previously signed in with the group's SAML IdP.
+- If there is no conflicting user with the same email address, create a new account automatically.
+- If there is a conflicting user with the same email address, redirect the user to the sign-in page to:
+  - Create a new account with another email address.
+  - Sign-in to their existing account to link the SAML identity.
 
 ### Linking SAML to your existing GitLab.com account
 

ee/app/controllers/groups/omniauth_callbacks_controller.rb

@@ -95,7 +95,9 @@ class Groups::OmniauthCallbacksController < OmniauthCallbacksController
 
   override :build_auth_user
   def build_auth_user(auth_user_class)
-    Gitlab::Auth::GroupSaml::User.new(oauth, @saml_provider)
+    super.tap do |auth_user|
+      auth_user.saml_provider = @saml_provider
+    end
   end
 
   override :fail_login

ee/changelogs/unreleased/dblessing_group_saml_jit.yml

+---
+title: Allow Group SAML to auto-created new users
+merge_request: 48953
+author:
+type: added

ee/lib/gitlab/auth/group_saml/identity_linker.rb

@@ -31,7 +31,8 @@ module Gitlab
         # rubocop: enable CodeReuse/ActiveRecord
 
         def update_group_membership
-          MembershipUpdater.new(current_user, saml_provider, oauth).execute
+          auth_hash = AuthHash.new(oauth)
+          MembershipUpdater.new(current_user, saml_provider, auth_hash).execute
         end
       end
     end

ee/lib/gitlab/auth/group_saml/membership_updater.rb

@@ -13,7 +13,7 @@ module Gitlab
         def initialize(user, saml_provider, auth_hash)
           @user = user
           @saml_provider = saml_provider
-          @auth_hash = AuthHash.new(auth_hash)
+          @auth_hash = auth_hash
         end
 
         def execute
@@ -54,7 +54,7 @@ module Gitlab
 
         def group_names_from_saml
           strong_memoize(:group_names_from_saml) do
-            auth_hash.groups
+            auth_hash.groups || []
           end
         end
 

ee/lib/gitlab/auth/group_saml/user.rb

@@ -3,42 +3,73 @@
 module Gitlab
   module Auth
     module GroupSaml
-      class User
-        attr_reader :auth_hash, :saml_provider
+      class User < Gitlab::Auth::OAuth::User
+        include ::Gitlab::Utils::StrongMemoize
+        extend ::Gitlab::Utils::Override
 
-        def initialize(auth_hash, saml_provider)
-          @auth_hash = auth_hash
-          @saml_provider = saml_provider
+        attr_accessor :saml_provider
+        attr_reader :auth_hash
+
+        override :initialize
+        def initialize(auth_hash)
+          @auth_hash = AuthHash.new(auth_hash)
         end
 
+        override :find_and_update!
         def find_and_update!
-          update_group_membership
+          save("GroupSaml Provider ##{saml_provider.id}")
 
-          user_from_identity
-        end
+          # Do not return un-persisted user so user is prompted
+          # to sign-in to existing account.
+          return unless valid_sign_in?
 
-        def valid_sign_in?
-          user_from_identity.present?
+          update_group_membership
+          gl_user
         end
 
+        override :bypass_two_factor?
         def bypass_two_factor?
           false
         end
 
         private
 
+        override :gl_user
+        def gl_user
+          strong_memoize(:gl_user) do
+            identity&.user || build_new_user
+          end
+        end
+
         def identity
-          @identity ||= ::Auth::GroupSamlIdentityFinder.new(saml_provider, auth_hash).first
+          strong_memoize(:identity) do
+            ::Auth::GroupSamlIdentityFinder.new(saml_provider, auth_hash).first
+          end
+        end
+
+        override :build_new_user
+        def build_new_user(skip_confirmation: false)
+          super.tap do |user|
+            user.provisioned_by_group_id = saml_provider.group_id
+          end
         end
 
-        def user_from_identity
-          @user_from_identity ||= identity&.user
+        override :user_attributes
+        def user_attributes
+          super.tap do |hash|
+            hash[:extern_uid] = auth_hash.uid
+            hash[:saml_provider_id] = @saml_provider.id
+            hash[:provider] = ::Users::BuildService::GROUP_SAML_PROVIDER
+          end
         end
 
         def update_group_membership
-          return unless user_from_identity
+          MembershipUpdater.new(gl_user, saml_provider, auth_hash).execute
+        end
 
-          MembershipUpdater.new(user_from_identity, saml_provider, auth_hash).execute
+        override :block_after_signup?
+        def block_after_signup?
+          false
         end
       end
     end

ee/spec/lib/gitlab/auth/group_saml/membership_updater_spec.rb

@@ -6,13 +6,15 @@ RSpec.describe Gitlab::Auth::GroupSaml::MembershipUpdater do
   let(:user) { create(:user) }
   let(:saml_provider) { create(:saml_provider, default_membership_role: Gitlab::Access::DEVELOPER) }
   let(:group) { saml_provider.group }
-  let(:omniauth_auth_hash) do
-    OmniAuth::AuthHash.new(extra: {
-      raw_info: OneLogin::RubySaml::Attributes.new('groups' => %w(Developers Owners))
-    })
+  let(:auth_hash) do
+    Gitlab::Auth::GroupSaml::AuthHash.new(
+      OmniAuth::AuthHash.new(extra: {
+        raw_info: OneLogin::RubySaml::Attributes.new('groups' => %w(Developers Owners))
+      })
+    )
   end
 
-  subject(:update_membership) { described_class.new(user, saml_provider, omniauth_auth_hash).execute }
+  subject(:update_membership) { described_class.new(user, saml_provider, auth_hash).execute }
 
   it 'adds the user to the group' do
     subject

ee/spec/lib/gitlab/auth/group_saml/user_spec.rb

@@ -5,11 +5,22 @@ require 'spec_helper'
 
 RSpec.describe Gitlab::Auth::GroupSaml::User do
   let(:uid) { 1234 }
-  let(:auth_hash) { OmniAuth::AuthHash.new(uid: uid) }
   let(:saml_provider) { create(:saml_provider) }
   let(:group) { saml_provider.group }
+  let(:auth_hash) { OmniAuth::AuthHash.new(uid: uid, provider: 'group_saml', info: info_hash) }
+  let(:info_hash) do
+    {
+      name: generate(:name),
+      email: generate(:email)
+    }
+  end
+
+  subject(:oauth_user) do
+    oauth_user = described_class.new(auth_hash)
+    oauth_user.saml_provider = saml_provider
 
-  subject { described_class.new(auth_hash, saml_provider) }
+    oauth_user
+  end
 
   def create_existing_identity
     create(:group_saml_identity, extern_uid: uid, saml_provider: saml_provider)
@@ -32,24 +43,59 @@ RSpec.describe Gitlab::Auth::GroupSaml::User do
   end
 
   describe '#find_and_update!' do
+    subject(:find_and_update) { oauth_user.find_and_update! }
+
     context 'with matching user for that group and uid' do
       let!(:identity) { create_existing_identity }
 
       it 'updates group membership' do
-        expect do
-          subject.find_and_update!
-        end.to change { group.members.count }.by(1)
+        expect { find_and_update }.to change { group.members.count }.by(1)
       end
 
       it 'returns the user' do
-        expect(subject.find_and_update!).to eq identity.user
+        expect(find_and_update).to eq identity.user
+      end
+
+      it 'does not mark the user as provisioned' do
+        expect(find_and_update.provisioned_by_group).to be_nil
       end
     end
 
     context 'with no matching user identity' do
-      it 'does nothing' do
-        expect(subject.find_and_update!).to eq nil
-        expect(group.members.count).to eq 0
+      context 'when a user does not exist' do
+        it 'creates the user' do
+          expect { find_and_update }.to change { User.count }.by(1)
+        end
+
+        it 'does not confirm the user' do
+          is_expected.not_to be_confirmed
+        end
+
+        it 'returns the correct user' do
+          expect(find_and_update.email).to eq info_hash[:email]
+        end
+
+        it 'marks the user as provisioned by the group' do
+          expect(find_and_update.provisioned_by_group).to eq group
+        end
+
+        it 'creates the user SAML identity' do
+          expect { find_and_update }.to change { Identity.count }.by(1)
+        end
+      end
+
+      context 'when a conflicting user already exists' do
+        before do
+          create(:user, email: info_hash[:email])
+        end
+
+        it 'does not update membership' do
+          expect { find_and_update }.not_to change { group.members.count }
+        end
+
+        it 'does not return a user' do
+          expect(find_and_update).to eq nil
+        end
       end
     end
   end

lib/gitlab/auth/o_auth/user.rb

@@ -204,8 +204,8 @@ module Gitlab
           self.class.find_by_uid_and_provider(auth_hash.uid, auth_hash.provider)
         end
 
-        def build_new_user
-          user_params = user_attributes.merge(skip_confirmation: true)
+        def build_new_user(skip_confirmation: true)
+          user_params = user_attributes.merge(skip_confirmation: skip_confirmation)
           Users::BuildService.new(nil, user_params).execute(skip_authorization: true)
         end