Changing password in SessionStore when password change forced has no effect
|Reported by:||James Teh||Owned by:||Steffen Hoffmann|
|Severity:||major||Keywords:||password db overwrite|
When AccountManagerPlugin is configured to use SessionStore and a password change is forced due to a password reset, the first password change has no effect; i.e. the password remains unchanged.
Steps to reproduce:
- Ensure that AccountManagerPlugin is configured to use SessionStore and that there is an account with which to test.
- Request a password reset for an account using the Forgot Password link.
- Log in with the temporary password.
- You will be prompted to change your password, so change it. You will be informed that the change was successful.
- Log out.
- Try to log back in with the new password.
- Expected: You should be logged in.
- Actual: You can't log in.
- When this happens, you can still log back in with the temporary password (step 2).
- If you log in and change the password again, this change (not forced) works correctly.
I investigated the code a bit. Here's what I *think* is happening:
- When saving the password, SessionStore writes the password directly to the database.
- If a password change is being forced, the code deletes force_change_passwd from the request's session object (req.session) and then saves the session. This happens at source:/accountmanagerplugin/trunk/acct_mgr/web_ui.py@9584#L270:[[br]]
if force_change_password: del(req.session['force_change_passwd']) req.session.save()
- Unfortunately, the session object didn't know about the changed session attribute (the password) because it was written directly to the database (1), so when it is saved (2), it overwrites the password change (1).
Assuming I'm right, either:
- SessionStore needs to write to the request's session object. However, I don't think there's any way for SessionStore to access that; or
- The session object somehow needs to be notified that its data has changed. The only way I can see to do this is to call
req.session.get_session(req.authname, authenticated=True), but this is pretty ugly and I'm not sure if it has any nasty side effects.
Change History (8)
Changed 6 years ago by