Email verification should work across devices #117
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Synchro
reviewed
Apr 18, 2025
|
|
||
| $user = User::findOrFail($id); | ||
|
|
||
| if (! hash_equals($hash, sha1($user->getEmailForVerification()))) { |
There was a problem hiding this comment.
Should this be calling sha1 directly? Isn't there a Laravel abstraction for this? Also bearing this in mind.
Synchro
reviewed
Apr 18, 2025
| event(new Verified($user)); | ||
| } | ||
|
|
||
| // Always log the user in, regardless of verification status | ||
| Auth::login($user); |
There was a problem hiding this comment.
This makes verification links a little dangerous – anyone who gets hold of one is instantly logged in with no other auth required. Usual practice after verification is to require a login, which solves that problem, though at a mild UX expense.
/cc @valorin
Note that this additional login doesn't really apply for OAuth, since verification and login are essentially the same thing there.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The current Verify Email functionality utilizes the EmailVerificationRequest here: https://github.com/laravel/framework/blob/12.x/src/Illuminate/Foundation/Auth/EmailVerificationRequest.php, which fails when a user clicks the verification email from a different device or browser.
This update removes the need for the user to be logged in when verifying their email. Instead, we check the id and hash directly from the signed URL. We use hasValidSignature() to make sure the link hasn’t been tampered with or expired, and then compare the hash to a SHA-1 version of the user’s email to confirm it’s legit.
With this update, the email verification flow is still secure and has a better user experience.
@taylorotwell, let me know if this looks good. I have PR's for this in the other starter kits but wanted to get your thoughts. Should I instead update the EmailVerificationRequest from inside the Laravel framework to work this way (https://github.com/laravel/framework/blob/12.x/src/Illuminate/Foundation/Auth/EmailVerificationRequest.php), or is it fine to add it in each starter kit?
Let me know 👍 Thanks!