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.
This commit is contained in:
parent
ef19431e44
commit
a040f72f96
|
@ -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
|
||||
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
|
Loading…
Reference in New Issue