diff --git a/lib/auth/managed_authenticator.rb b/lib/auth/managed_authenticator.rb index f42fe720128..5530f5bc6ba 100644 --- a/lib/auth/managed_authenticator.rb +++ b/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? diff --git a/spec/lib/auth/managed_authenticator_spec.rb b/spec/lib/auth/managed_authenticator_spec.rb index e279ebf0486..02faefcd736 100644 --- a/spec/lib/auth/managed_authenticator_spec.rb +++ b/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