From a040f72f961d579d91768c69899d0c69d5f846dd Mon Sep 17 00:00:00 2001
From: David Taylor <david@taylorhq.com>
Date: Mon, 22 Feb 2021 12:05:36 +0000
Subject: [PATCH] FIX: Make email_valid handling consistent (#11556)

Previously we were checking truthiness in some places, and `== true` in
others. That can lead to some inconsistent UX where the interface says
the email is valid, but account creation fails.

This commit ensures values are boolean when set, and raises an error for
other value types.

If this safety check is triggered, it means the specific auth provider
needs to be updated to pass booleans.
---
 app/services/user_authenticator.rb       | 2 +-
 lib/auth/result.rb                       | 7 +++++++
 spec/services/user_authenticator_spec.rb | 9 +++++++++
 3 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/app/services/user_authenticator.rb b/app/services/user_authenticator.rb
index 5db04fa1bc2..822c9b5d1f2 100644
--- a/app/services/user_authenticator.rb
+++ b/app/services/user_authenticator.rb
@@ -41,7 +41,7 @@ class UserAuthenticator
   def authenticated?
     return false if !@auth_result
     return false if @auth_result&.email&.downcase != @user.email.downcase
-    return false if @auth_result.email_valid != true # strong check for truth, in case we have another object type
+    return false if !@auth_result.email_valid
     true
   end
 
diff --git a/lib/auth/result.rb b/lib/auth/result.rb
index 5cad43107b0..6abbdebcae1 100644
--- a/lib/auth/result.rb
+++ b/lib/auth/result.rb
@@ -52,6 +52,13 @@ class Auth::Result
     @email&.downcase
   end
 
+  def email_valid=(val)
+    if !val.in? [true, false, nil]
+      raise ArgumentError, "email_valid should be boolean or nil"
+    end
+    @email_valid = !!val
+  end
+
   def failed?
     !!@failed
   end
diff --git a/spec/services/user_authenticator_spec.rb b/spec/services/user_authenticator_spec.rb
index 5db539d6618..07cce902857 100644
--- a/spec/services/user_authenticator_spec.rb
+++ b/spec/services/user_authenticator_spec.rb
@@ -66,5 +66,14 @@ describe UserAuthenticator do
 
       expect(session[:authentication]).to eq(nil)
     end
+
+    it "raises an error for non-boolean values" do
+      user = Fabricate(:user, email: "user53@discourse.org")
+      session = { authentication: github_auth('string') }
+
+      expect do
+        UserAuthenticator.new(user, session).finish
+      end.to raise_error ArgumentError
+    end
   end
 end