DEV: Allow ManagedAuthenticator classes to match by username (#18517)

lib/auth/managed_authenticator.rb

@@ -24,6 +24,11 @@ class Auth::ManagedAuthenticator < Auth::Authenticator
     true
   end
 
+  # Depending on the authenticator, this could be insecure, so it's disabled by default
+  def match_by_username
+    false
+  end
+
   def primary_email_verified?(auth_token)
     # Omniauth providers should only provide verified emails in the :info hash.
     # This method allows additional checks to be added
@@ -67,6 +72,16 @@ class Auth::ManagedAuthenticator < Auth::Authenticator
       association.user = user
     end
 
+    # Matching an account by username
+    if match_by_username &&
+        association.user.nil? &&
+        SiteSetting.username_change_period.zero? &&
+        (user = find_user_by_username(auth_token))
+
+      UserAssociatedAccount.where(user: user, provider_name: auth_token[:provider]).destroy_all # Destroy existing associations for the new user
+      association.user = user
+    end
+
     # Update all the metadata in the association:
     association.info = auth_token[:info] || {}
     association.credentials = auth_token[:credentials] || {}
@@ -122,6 +137,13 @@ class Auth::ManagedAuthenticator < Auth::Authenticator
     end
   end
 
+  def find_user_by_username(auth_token)
+    username = auth_token.dig(:info, :nickname)
+    if username
+      User.find_by_username(username)
+    end
+  end
+
   def retrieve_avatar(user, url)
     return unless user && url
     return if user.user_avatar.try(:custom_upload_id).present?

spec/lib/auth/managed_authenticator_spec.rb

@@ -254,6 +254,78 @@ RSpec.describe Auth::ManagedAuthenticator do
         expect(user.user_profile.location).to eq("DiscourseVille")
       end
     end
+
+    describe 'match by username' do
+      let(:user_match_authenticator) {
+        Class.new(described_class) do
+          def name
+            "myauth"
+          end
+          def match_by_email
+            false
+          end
+          def match_by_username
+            true
+          end
+        end.new
+      }
+
+      it 'works normally' do
+        SiteSetting.username_change_period = 0
+        user = Fabricate(:user)
+        result = user_match_authenticator.after_authenticate(hash.deep_merge(info: { nickname: user.username }))
+        expect(result.user.id).to eq(user.id)
+        expect(UserAssociatedAccount.find_by(provider_name: 'myauth', provider_uid: "1234").user_id).to eq(user.id)
+      end
+
+      it 'works if there is already an association with the target account' do
+        SiteSetting.username_change_period = 0
+        user = Fabricate(:user, username: "IAmGroot")
+        result = user_match_authenticator.after_authenticate(hash)
+        expect(result.user.id).to eq(user.id)
+      end
+
+      it 'works if the username is different case' do
+        SiteSetting.username_change_period = 0
+        user = Fabricate(:user, username: "IAMGROOT")
+        result = user_match_authenticator.after_authenticate(hash)
+        expect(result.user.id).to eq(user.id)
+      end
+
+      it 'does not match if username_change_period isn\'t 0' do
+        SiteSetting.username_change_period = 3
+        user = Fabricate(:user, username: "IAmGroot")
+        result = user_match_authenticator.after_authenticate(hash)
+        expect(result.user).to eq(nil)
+      end
+
+      it 'does not match if default match_by_username not overriden' do
+        SiteSetting.username_change_period = 0
+        authenticator = Class.new(described_class) do
+          def name
+            "myauth"
+          end
+        end.new
+        user = Fabricate(:user, username: "IAmGroot")
+        result = authenticator.after_authenticate(hash)
+        expect(result.user).to eq(nil)
+      end
+
+      it 'does not match if match_by_username is false' do
+        SiteSetting.username_change_period = 0
+        authenticator = Class.new(described_class) do
+          def name
+            "myauth"
+          end
+          def match_by_username
+            false
+          end
+        end.new
+        user = Fabricate(:user, username: "IAmGroot")
+        result = authenticator.after_authenticate(hash)
+        expect(result.user).to eq(nil)
+      end
+    end
   end
 
   describe 'description_for_user' do