-
Notifications
You must be signed in to change notification settings - Fork 14.3k
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
Remove user sessions when resetting password #33347
Remove user sessions when resetting password #33347
Conversation
a089e98
to
aaa8c26
Compare
2708578
to
0e2ca67
Compare
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.
Nit, IMHO I think we should not cleanup the session table on each reset password, but if the others are ok with that, I would be ok too
When user's password is reset, we also remove all DB sessions for that user - for database session backend. In case we are using securecookie mechanism, resetting password does not invalidate old sessions, so instead we are displaying warning to the user performing the reset that in order to clear existing sessions of the user, the secure_key needs to be changed and it will invalidate all sessions for all users. Protection has been added in case the number of sessions in the DB is too big to effectively scan and remove sessions for the user. In such case we print warning for the user that sessions have not been reset, and we suggest to improve the way their deployment mechanisms create too many sessions - by either changing the way how automation of the API calls is done and/or by purging the sessions regularly by "airflow db clean".
0e2ca67
to
10c5e4e
Compare
Co-authored-by: Hussein Awala <hussein@awala.fr>
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.
LGTM
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.
LGTM
* Remove user sessions when resetting password When user's password is reset, we also remove all DB sessions for that user - for database session backend. In case we are using securecookie mechanism, resetting password does not invalidate old sessions, so instead we are displaying warning to the user performing the reset that in order to clear existing sessions of the user, the secure_key needs to be changed and it will invalidate all sessions for all users. Protection has been added in case the number of sessions in the DB is too big to effectively scan and remove sessions for the user. In such case we print warning for the user that sessions have not been reset, and we suggest to improve the way their deployment mechanisms create too many sessions - by either changing the way how automation of the API calls is done and/or by purging the sessions regularly by "airflow db clean". * Update airflow/auth/managers/fab/security_manager/override.py Co-authored-by: Hussein Awala <hussein@awala.fr> --------- Co-authored-by: Hussein Awala <hussein@awala.fr> (cherry picked from commit 2caa186)
* Remove user sessions when resetting password When user's password is reset, we also remove all DB sessions for that user - for database session backend. In case we are using securecookie mechanism, resetting password does not invalidate old sessions, so instead we are displaying warning to the user performing the reset that in order to clear existing sessions of the user, the secure_key needs to be changed and it will invalidate all sessions for all users. Protection has been added in case the number of sessions in the DB is too big to effectively scan and remove sessions for the user. In such case we print warning for the user that sessions have not been reset, and we suggest to improve the way their deployment mechanisms create too many sessions - by either changing the way how automation of the API calls is done and/or by purging the sessions regularly by "airflow db clean". * Update airflow/auth/managers/fab/security_manager/override.py Co-authored-by: Hussein Awala <hussein@awala.fr> --------- Co-authored-by: Hussein Awala <hussein@awala.fr>
When user's password is reset, we also remove all DB sessions for that user.
^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named
{pr_number}.significant.rst
or{issue_number}.significant.rst
, in newsfragments.