Merge branch 'fix_saml_signin' into 'master'

Fix subsequent SAML sign ins

Fixes a bug when `auto_link_ldap_user` is `true` that causes SAML users to be unable to sign in a second time.

Fix the problem for https://gitlab.zendesk.com/agent/tickets/22546

See merge request !4718

3 files changed, 19 insertions, 2 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 6c0e5c46afe..f73d5e82f16 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -15,6 +15,7 @@ v 8.9.1 (unreleased)
   - Add documentation for award emoji now that comments can be awarded with emojis. !4839
   - Fix typo in export failure email. !4847
   - Fix header vertical centering. !4170
+  - Fix subsequent SAML sign ins. !4718
 
 v 8.9.0
   - Fix builds API response not including commit data
diff --git a/lib/gitlab/o_auth/user.rb b/lib/gitlab/o_auth/user.rb
index 78f3ecb4cb4..7af75a9cc4c 100644
--- a/lib/gitlab/o_auth/user.rb
+++ b/lib/gitlab/o_auth/user.rb
@@ -74,7 +74,7 @@ module Gitlab
         if user
           # Case when a LDAP user already exists in Gitlab. Add the OAuth identity to existing account.
           log.info "LDAP account found for user #{user.username}. Building new #{auth_hash.provider} identity."
-          user.identities.build(extern_uid: auth_hash.uid, provider: auth_hash.provider)
+          user.identities.find_or_initialize_by(extern_uid: auth_hash.uid, provider: auth_hash.provider)
         else
           log.info "No existing LDAP account was found in GitLab. Checking for #{auth_hash.provider} account."
           user = find_by_uid_and_provider
diff --git a/spec/lib/gitlab/saml/user_spec.rb b/spec/lib/gitlab/saml/user_spec.rb
index 84c21ceefd9..2753aecc1f4 100644
--- a/spec/lib/gitlab/saml/user_spec.rb
+++ b/spec/lib/gitlab/saml/user_spec.rb
@@ -164,7 +164,14 @@ describe Gitlab::Saml::User, lib: true do
             end
 
             context 'and LDAP user has an account already' do
-              let!(:existing_user) { create(:omniauth_user, email: 'john@mail.com', extern_uid: 'uid=user1,ou=People,dc=example', provider: 'ldapmain', username: 'john') }
+              before do
+                create(:omniauth_user,
+                       email: 'john@mail.com',
+                       extern_uid: 'uid=user1,ou=People,dc=example',
+                       provider: 'ldapmain',
+                       username: 'john')
+              end
+
               it 'adds the omniauth identity to the LDAP account' do
                 saml_user.save
 
@@ -177,6 +184,15 @@ describe Gitlab::Saml::User, lib: true do
                                                             { provider: 'saml', extern_uid: uid }
                                                           ])
               end
+
+              it 'saves successfully on subsequent tries, when both identities are present' do
+                saml_user.save
+                local_saml_user = described_class.new(auth_hash)
+                local_saml_user.save
+
+                expect(local_saml_user.gl_user).to be_valid
+                expect(local_saml_user.gl_user).to be_persisted
+              end
             end
 
             context 'user has SAML user, and wants to add their LDAP identity' do