-
Notifications
You must be signed in to change notification settings - Fork 938
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Invalidate existing sessions for a given User when User.password is changed #10859
Conversation
The trigger implementation is _best_ since it ensures that the column will stay up to date whenever it is changed, even if not via this method... Problematically the resulting value of the trigger isn't accessible inside the transaction (at least not that I was able to find). I tried flushes and savepoints but no matter what I did nothing worked :(
@@ -80,14 +82,19 @@ def __init__(self, session, *, ratelimiters=None, remote_addr, metrics): | |||
) | |||
self.remote_addr = remote_addr | |||
self._metrics = metrics | |||
self.cached_get_user = functools.lru_cache()(self._get_user) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Side effect of scrambling with figuring out why I couldn't get the latest password_date
value... but interestingly this may resolve a memory leak? https://rednafi.github.io/reflections/dont-wrap-instance-methods-with-functoolslru_cache-decorator-in-python.html
b71b9f3
to
0bd0f62
Compare
0bd0f62
to
2c1aad8
Compare
2c1aad8
to
57458ac
Compare
57458ac
to
61ce40b
Compare
Thanks @dstufft! db.refresh was the magic ticket to getting the resulting value from the trigger
2835ba2
to
8309b00
Compare
Sanity check done locally:
|
# we cannot say for sure, let it live its life. | ||
return False | ||
|
||
return current_password_timestamp != stored_password_timestamp |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like that the !=
comparison also covers our bases against time-travelers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Co-authored-by: Dustin Ingram <di@users.noreply.github.com>
…hanged (pypi#10859) * invalidate existing sessions for a given User when User.password is changed * set User.password_date whenever User.password is updated The trigger implementation is _best_ since it ensures that the column will stay up to date whenever it is changed, even if not via this method... Problematically the resulting value of the trigger isn't accessible inside the transaction (at least not that I was able to find). I tried flushes and savepoints but no matter what I did nothing worked :( * lint/translations * remove cache bypass for get_user, it is not necessary * translate string * cleanup 6942da2 * fix mass logout bug * revert to DB Trigger for password_date Thanks @dstufft! db.refresh was the magic ticket to getting the resulting value from the trigger * translations * explicitly flush before refresh to ensure modified state is maintained * Update warehouse/sessions.py Co-authored-by: Dustin Ingram <di@users.noreply.github.com> Co-authored-by: Dustin Ingram <di@users.noreply.github.com>
Closes #10849.