After
https://github.com/bigbluebutton/greenlight/commit/28302107bd10525860e2124c0b567b59cb8b2ecd has been applied, if a password reset link has been generated before the patch has been applied, and a user follows that link, this leads to a 500;
The log shows:
INFO: [REDACTED] method=GET path=/b/password_resets/REDACTED/edit format=html controller=PasswordResetsController action=edit status=500 error='NoMethodError: undefined methodpassword_reset_expired?' for nil:NilClass' duration=10.65 view=0.00 db=1.93 host=greenlight
FATAL: [REDACTED]
FATAL: [REDACTED] NoMethodError (undefined method password_reset_expired?' for nil:NilClass):
FATAL: [REDACTED]
FATAL: [REDACTED] app/controllers/password_resets_controller.rb:78:incheck_expiration'`
Steps to reproduce:
Update the password reset token to an old style (base64) password reset token, and visit a password reset link.
Expected behavior:
The user is informed that the password reset fails and can request a new password reset (with a new token)
While debugging this, I noticed that _technically_ a new password reset request should rewrite the token to a new version one. However, i still got a user stuck in this. I do not know why the token was not updated when the user requested a new password reset; So, in practice this _should_ not happen; However, I just saw it in production, so it might be nice to add a check here.
Yeah this was kind of expected (and not really avoidable).
While debugging this, I noticed that technically a new password reset request should rewrite the token to a new version one.
That's what it should be doing, so I'll take a look to see if I can replicate it locally
Couldn't you just add a db migration script that clears all old password reset tokens?
We could, but I think we're better off fixing the underlying issue which is invalid tokens throw a 500. We should probably redirect them back to root with an error message that their token is invalid and to try resetting it again
Fixed in v2.6
Most helpful comment
We could, but I think we're better off fixing the underlying issue which is invalid tokens throw a 500. We should probably redirect them back to root with an error message that their token is invalid and to try resetting it again