Conversation
Bind each token to a random nonce stored on the user. Requesting a reset rotates the nonce, revoking older links; any password change clears it, so a link works once and dies on a logged-in password change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Coverage Report for CI Build 36561611579Warning No base build found for commit Coverage: 92.694%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsRequires a base build to compare against. How to fix this → Coverage Stats
💛 - Coveralls |
MarcelGeo
left a comment
MarcelGeo
left a comment
There was a problem hiding this comment.
As I understand old tokens will be revoked, no?
| confirm_url = f"{url}/{token}" | ||
|
|
||
|
|
||
| def _send_token_email(app, user, confirm_url, template, header, **kwargs): |
There was a problem hiding this comment.
If we have such function, I am expecting here this token value as argument. Not hidden in some tricky string.
| ) | ||
| abort(404) | ||
| # token was already used, superseded by a newer one or revoked by a password change | ||
| if user.password_reset_nonce != payload["nonce"]: |
There was a problem hiding this comment.
can be payload["nonce"] None? If yes, reject everything too.
There was a problem hiding this comment.
that can be only for old tokens, but any new request should have some
|
|
||
| form = UserPasswordForm.from_json(request.json) | ||
| if form.validate(): | ||
| # also clears the reset nonce, making the token single-use |
There was a problem hiding this comment.
I think we do not need such comment here.
| "BEARER_TOKEN_EXPIRATION", default=3600 * 12, cast=int | ||
| ) # in seconds | ||
| PASSWORD_RESET_TOKEN_EXPIRATION = config( | ||
| "PASSWORD_RESET_TOKEN_EXPIRATION", default=600, cast=int |
There was a problem hiding this comment.
I think this is short window.
There was a problem hiding this comment.
I can make it 1800 or 3600 if you like -> but if you hit reset button, then I assume you want to do it straightaway
| send_email_async.delay(**email_data) | ||
|
|
||
|
|
||
| def generate_password_reset_token(app: Flask, user: User) -> str: |
There was a problem hiding this comment.
It's not better to add it to user model? Just wondering ;)
| return serializer.dumps(payload, salt=app.config["SECURITY_PASSWORD_SALT"]) | ||
|
|
||
|
|
||
| def confirm_password_reset_token(token: str) -> Optional[dict]: |
There was a problem hiding this comment.
It's not enough tom just use confirm_token function with some is instance check?
This branch has not been deployed
3 participants
Bind each token to a random nonce stored on the user. Requesting a reset rotates the nonce, revoking older links; any password change clears it, so a link works once and dies on a logged-in password change.
There is also a new config variable
PASSWORD_RESET_TOKEN_EXPIRATION.