DEV: Flip primary_email_verified? default to false (#19703)

This commit changes the default return value of `Auth::ManagedAuthenticator#primary_email_verified?` to false. We're changing the default to force developers to think about email verification when building a new authentication method. All existing authenticators (in core and official plugins) have been updated to explicitly define the `primary_email_verified?` method in their subclass of `Auth::ManagedAuthenticator` (example commit 65f57a4d05).

Internal topic: t/82084.
This commit is contained in:
Osama Sayegh 2023-01-04 10:51:10 +03:00 committed by GitHub
parent 42cf32169d
commit bbcdf74c58
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 15 additions and 5 deletions

View File

@ -32,7 +32,7 @@ class Auth::ManagedAuthenticator < Auth::Authenticator
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
true
false
end
def can_revoke?

View File

@ -6,6 +6,10 @@ RSpec.describe Auth::ManagedAuthenticator do
def name
"myauth"
end
def primary_email_verified?(auth_token)
auth_token[:info][:email_verified]
end
end.new
}
@ -16,7 +20,8 @@ RSpec.describe Auth::ManagedAuthenticator do
info: {
name: "Best Display Name",
email: "awesome@example.com",
nickname: "IAmGroot"
nickname: "IAmGroot",
email_verified: true
},
credentials: {
token: "supersecrettoken"
@ -59,16 +64,21 @@ RSpec.describe Auth::ManagedAuthenticator do
it 'only sets email valid for present strings' do
# (Twitter sometimes sends empty email strings)
result = authenticator.after_authenticate(create_hash.merge(info: { email: "email@example.com" }))
result = authenticator.after_authenticate(create_hash.merge(info: { email: "email@example.com", email_verified: true }))
expect(result.email_valid).to eq(true)
result = authenticator.after_authenticate(create_hash.merge(info: { email: "" }))
result = authenticator.after_authenticate(create_hash.merge(info: { email: "", email_verified: true }))
expect(result.email_valid).to be_falsey
result = authenticator.after_authenticate(create_hash.merge(info: { email: nil }))
result = authenticator.after_authenticate(create_hash.merge(info: { email: nil, email_verified: true }))
expect(result.email_valid).to be_falsey
end
it 'does not set email valid if email_verified is false' do
result = authenticator.after_authenticate(create_hash.merge(info: { email: "email@example.com", email_verified: false }))
expect(result.email_valid).to eq(false)
end
describe 'connecting to another user account' do
fab!(:user1) { Fabricate(:user) }
fab!(:user2) { Fabricate(:user) }