Merge branch '33669-link-current-users-to-new-smartcards' into 'master'

Link existing user accounts to new Smartcards Identities on login

Closes #33669

See merge request gitlab-org/gitlab!20059

doc/administration/auth/smartcard.md

@@ -4,6 +4,10 @@ type: reference
 
 # Smartcard authentication **(PREMIUM ONLY)**
 
+> [Introduced](https://gitlab.com/gitlab-org/gitlab/issues/33669) in GitLab 12.6,
+if a user has a pre-existing username and password, they can still use that to log
+in by default. However, this can be disabled.
+
 GitLab supports authentication using smartcards.
 
 ## Authentication methods

ee/changelogs/unreleased/33669-link-current-users-to-new-smartcards.yml

+---
+title: Link user accounts to new Smartcards identities on login
+merge_request: 20059
+author:
+type: fixed

ee/lib/gitlab/auth/smartcard/base.rb

@@ -42,12 +42,28 @@ module Gitlab
           return unless valid?
 
           user = find_user
+          user ||= create_identity_for_existing_user
           user ||= create_user if allow_signup?
           user
         end
 
         private
 
+        def find_user
+          raise NotImplementedError,
+                "#{self.class.name} does not implement #{__method__}"
+        end
+
+        def create_identity_for_existing_user
+          raise NotImplementedError,
+                "#{self.class.name} does not implement #{__method__}"
+        end
+
+        def create_user
+          raise NotImplementedError,
+                "#{self.class.name} does not implement #{__method__}"
+        end
+
         def valid?
           self.class.store.verify(@certificate) if @certificate
         end

ee/lib/gitlab/auth/smartcard/certificate.rb

@@ -16,13 +16,13 @@ module Gitlab
           User.find_by_smartcard_identity(subject, issuer)
         end
 
-        def create_user
-          user = User.find_by_email(email)
-          if user
-            create_smartcard_identity_for(user)
-            return user
+        def create_identity_for_existing_user
+          User.find_by_email(email).tap do |user|
+            create_smartcard_identity_for(user) if user
+          end
         end
 
+        def create_user
           user_params = {
             name:                       common_name,
             username:                   username,

ee/lib/gitlab/auth/smartcard/ldap_certificate.rb

@@ -21,6 +21,10 @@ module Gitlab
           identity&.user
         end
 
+        def create_identity_for_existing_user
+          # TODO: create new identity for existing users as part of https://gitlab.com/gitlab-org/gitlab/issues/36808
+        end
+
         def create_user
           user_params = {
             name:                       ldap_user.name,

ee/spec/lib/gitlab/auth/smartcard/certificate_spec.rb

@@ -18,11 +18,10 @@ describe Gitlab::Auth::Smartcard::Certificate do
 
   shared_examples 'a new smartcard identity' do
     it 'creates smartcard identity' do
-      expect { subject }.to change { SmartcardIdentity.count }.from(0).to(1)
+      expect { subject }.to change { SmartcardIdentity.count }.by(1)
 
-      identity = SmartcardIdentity.first
-      expect(identity.subject).to eql(subject_dn)
-      expect(identity.issuer).to eql(issuer_dn)
+      identity = SmartcardIdentity.find_by(subject: subject_dn, issuer: issuer_dn)
+      expect(identity).not_to be_nil
     end
   end
 
@@ -63,6 +62,28 @@ describe Gitlab::Auth::Smartcard::Certificate do
       end
     end
 
+    context 'user exists but it is using a new smartcard' do
+      let_it_be(:user) { create(:user, email: 'gitlab-user@random-corp.org') }
+      let_it_be(:old_identity) do
+        create(:smartcard_identity,
+               subject: 'old_subject',
+               issuer: 'old_issuer_dn',
+               user: user)
+      end
+
+      it_behaves_like 'an existing user'
+
+      it_behaves_like 'a new smartcard identity'
+
+      it 'keeps both identities for the user' do
+        subject
+
+        new_identity = SmartcardIdentity.find_by(subject: subject_dn, issuer: issuer_dn)
+
+        expect(user.smartcard_identities).to contain_exactly(new_identity, old_identity)
+      end
+    end
+
     context 'user and smartcard identity do not exist' do
       let(:user) { create(:user) }