Fix handling of multiple GroupSAML identities

A user could have multiple identities with `provider: 'group_saml'`, one
for each GitLab.com group to which they belong. The change in
https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/14045 would only
update one of these identities via `find_or_create_by`. To fix this,
also account for `saml_provider_id` if it's available.

Closes https://gitlab.com/gitlab-org/gitlab-ee/issues/12303

app/services/users/update_service.rb

@@ -63,13 +63,21 @@ module Users
     def assign_identity
       return unless identity_params.present?
 
-      identity = user.identities.find_or_create_by(provider: identity_params[:provider]) # rubocop: disable CodeReuse/ActiveRecord
+      identity = user.identities.find_or_create_by(provider_params) # rubocop: disable CodeReuse/ActiveRecord
       identity.update(identity_params)
     end
 
     def identity_attributes
       [:provider, :extern_uid]
     end
+
+    def provider_attributes
+      [:provider]
+    end
+
+    def provider_params
+      identity_params.slice(*provider_attributes)
+    end
   end
 end
 

ee/app/services/ee/users/update_service.rb

@@ -40,6 +40,11 @@ module EE
         end
       end
 
+      override :provider_attributes
+      def provider_attributes
+        super.push(:saml_provider_id)
+      end
+
       override :identity_attributes
       def identity_attributes
         super.push(:saml_provider_id)

ee/spec/services/users/update_service_spec.rb

@@ -47,20 +47,32 @@ describe Users::UpdateService do
       context 'allowed params' do
         context 'with identity' do
           let(:provider) { create(:saml_provider) }
-          let(:identity_params) { { extern_uid: 'uid', provider: 'group_saml', saml_group_id: provider.group.id } }
+          let(:identity_params) { { extern_uid: 'uid', provider: 'group_saml', group_id_for_saml: provider.group.id } }
 
           before do
             params.merge!(identity_params)
           end
 
-          it 'successfully adds identity to user' do
-            result = update_user(user, { extern_uid: 'uid', provider: 'group_saml', saml_provider_id: provider.id })
+          it 'adds identity to user' do
+            result = update_user(user, params)
 
             expect(result).to be true
             expect(user.identities.last.saml_provider_id).to eq(provider.id)
             expect(user.identities.last.extern_uid).to eq('uid')
             expect(user.identities.last.provider).to eq('group_saml')
           end
+
+          it 'adds two different identities to user' do
+            second_provider = create(:saml_provider)
+            result_one = update_user(user, { extern_uid: 'uid', provider: 'group_saml', saml_provider_id: provider.id })
+            result_two = update_user(user, { extern_uid: 'uid2', provider: 'group_saml', group_id_for_saml: second_provider.group.id } )
+
+            expect(result_one).to be true
+            expect(result_two).to be true
+            expect(user.identities.count).to eq(2)
+            expect(user.identities.map(&:extern_uid)).to match_array(%w(uid uid2))
+            expect(user.identities.map(&:saml_provider_id)).to match_array([provider.id, second_provider.id])
+          end
         end
       end
     end