FEATURE: Fetch email from auth provider if current user email is invalid (#7163)

If the existing email address for a user ends in `.invalid`, we should take the email address from an authentication payload, and replace the invalid address. This typically happens when we import users from a system without email addresses.

This commit also adds some extensibility so that plugin authenticators can define `always_update_user_email?`
This commit is contained in:
David Taylor 2019-03-14 11:33:30 +00:00 committed by GitHub
parent b9ab393d70
commit fc0cf3ecd1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 44 additions and 0 deletions

View File

@ -24,6 +24,10 @@ class Auth::ManagedAuthenticator < Auth::Authenticator
true
end
def always_update_user_email?
false
end
def revoke(user, skip_remote: false)
association = UserAssociatedAccount.find_by(provider_name: name, user_id: user.id)
raise Discourse::NotFound if association.nil?
@ -60,6 +64,17 @@ class Auth::ManagedAuthenticator < Auth::Authenticator
# Save to the DB. Do this even if we don't have a user - it might be linked up later in after_create_account
association.save!
# Update the user's email address from the auth payload
if association.user &&
(always_update_user_email? || association.user.email.end_with?(".invalid")) &&
primary_email_verified?(auth_token) &&
(email = auth_token.dig(:info, :email)) &&
(email != association.user.email) &&
!User.find_by_email(email)
association.user.update!(email: email)
end
# Update avatar/profile
retrieve_avatar(association.user, association.info["image"])
retrieve_profile(association.user, association.info)

View File

@ -180,6 +180,35 @@ describe Auth::ManagedAuthenticator do
end
end
describe "email update" do
let(:user) { Fabricate(:user) }
let!(:associated) { UserAssociatedAccount.create!(user: user, provider_name: 'myauth', provider_uid: "1234") }
it "updates the user's email if currently invalid" do
user.update!(email: "someemail@discourse.org")
# Existing email is valid, do not change
expect { result = authenticator.after_authenticate(hash) }
.not_to change { user.reload.email }
user.update!(email: "someemail@discourse.invalid")
# Existing email is invalid, expect change
expect { result = authenticator.after_authenticate(hash) }
.to change { user.reload.email }
expect(user.email).to eq("awesome@example.com")
end
it "doesn't raise error if email is taken" do
other_user = Fabricate(:user, email: "awesome@example.com")
user.update!(email: "someemail@discourse.invalid")
expect { result = authenticator.after_authenticate(hash) }
.not_to change { user.reload.email }
expect(user.email).to eq("someemail@discourse.invalid")
end
end
describe "avatar on create" do
let(:user) { Fabricate(:user) }
let!(:association) { UserAssociatedAccount.create!(provider_name: 'myauth', provider_uid: "1234") }