-
-
Notifications
You must be signed in to change notification settings - Fork 4.8k
Regenerate Email Verification Token on Email Request #4439
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Regenerate Email Verification Token on Email Request #4439
Conversation
@montymxb there seems to be an issue with postgres, I have trouble understanding why :) |
Oops, I've totally left this here. I'll see if i can correct it sometime
this weekend.
…On Jan 24, 2018 06:59, "Florent Vilmart" ***@***.***> wrote:
@montymxb <https://github.com/montymxb> there seems to be an issue with
postgres, I have trouble understanding why :)
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#4439 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AE1d4GL9ZIoT3BZ3K1o-E-IxRQN6O-soks5tN0VmgaJpZM4RHrPq>
.
|
@flovilmart Last weekend was packed, but I have some time today. I'm rerunning initially, but if it persists I'll test locally to see what changes may be causing this. |
Testing locally I have isolated it down to a postgres only issue, which produces the following error.
This is a bit troubling, as other tests in the same file (EmailVerificationToken.spec) perform a similar action but without the error. Whatever it is, it's definitely just in the test I modified, but I can't quite isolate it. Should have something on this soon. |
Codecov Report
@@ Coverage Diff @@
## master #4439 +/- ##
==========================================
- Coverage 92.89% 92.86% -0.04%
==========================================
Files 118 118
Lines 8445 8448 +3
==========================================
Hits 7845 7845
- Misses 600 603 +3
Continue to review full report at Codecov.
|
The issue has been resolved, codecov is being a bit mad with the coverage for some reason, although everything looks ok now. |
It’s all good. Merging? |
…#4439) * regenerate email verification token & expiration in /verificationEmailRequest * Remove password field when saving on postgres
This is a fix for an issue observed in #4369. The issue being that requesting additional verification emails from /verificationEmailRequest does not regenerate the verification token or expiration that is currently set. The problem here is that a verification request can simply time out and subsequent requests from this endpoint will produce emails that will not work. Note this is not an issue if no expiration is set.
Making a similar request to /apps/:appId/resend_verification_email is effectively the same as this endpoint, but it regenerates the verification token & expiration per request.
Making this change would prevent future users/devs from simply 'getting stuck' when trying to retrieve a new verification request when their existing one has expired. The key thing to note about this change is that there can only be one valid verification token at at time. Only the last requested email will have a valid token, all others (even if they were valid and unexpired) will be rendered invalid. I believe this is acceptable considering that one can simply request a new verification email rather than fuss over existing ones.
::edit::
This modifies an existing test to further validate that post-request the underlying verification token & expiration have been modified from their original values.