From bbcdf74c5883bb48ce5a5a97e7f29a8334d6779c Mon Sep 17 00:00:00 2001 From: Osama Sayegh Date: Wed, 4 Jan 2023 10:51:10 +0300 Subject: [PATCH] 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 https://github.com/discourse/discourse/commit/65f57a4d05496d05a4de9a6a641ec760c7004c7a). Internal topic: t/82084. --- lib/auth/managed_authenticator.rb | 2 +- spec/lib/auth/managed_authenticator_spec.rb | 18 ++++++++++++++---- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/lib/auth/managed_authenticator.rb b/lib/auth/managed_authenticator.rb index 5530f5bc6b..41f36a611f 100644 --- a/lib/auth/managed_authenticator.rb +++ b/lib/auth/managed_authenticator.rb @@ -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? diff --git a/spec/lib/auth/managed_authenticator_spec.rb b/spec/lib/auth/managed_authenticator_spec.rb index 02faefcd73..a91cae9933 100644 --- a/spec/lib/auth/managed_authenticator_spec.rb +++ b/spec/lib/auth/managed_authenticator_spec.rb @@ -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) }