[GitHub] [airflow] potiuk commented on a diff in pull request #33347: Remove user sessions when resetting password

["potiuk (via GitHub)" <gi...@apache.org>]

potiuk commented on code in PR #33347:
URL: https://github.com/apache/airflow/pull/33347#discussion_r1292834537


##########
airflow/auth/managers/fab/security_manager/override.py:
##########
@@ -230,8 +243,51 @@ def reset_password(self, userid, password):
         """
         user = self.get_user_by_id(userid)
         user.password = generate_password_hash(password)
+        self.reset_user_sessions(user)
         self.update_user(user)
 
+    def reset_user_sessions(self, user: User) -> None:
+        if isinstance(self.appbuilder.get_app.session_interface, AirflowDatabaseSessionInterface):
+            interface = self.appbuilder.get_app.session_interface
+            session = interface.db.session
+            user_session_model = interface.sql_session_model
+            num_sessions = session.query(user_session_model).count()
+            if num_sessions > MAX_NUM_DATABASE_USER_SESSIONS:
+                flash(
+                    Markup(
+                        f"The old sessions for user {user.username} have <b>NOT</b> been deleted!<br>"
+                        f"You have a lot ({num_sessions}) of user sessions in the 'SESSIONS' table in "
+                        f"your database.<br> "
+                        "This indicates that this deployment might have an automated API calls that create "
+                        "and not reuse sessions.<br>You should consider reusing sessions or cleaning them "
+                        "periodically using db clean.<br>"
+                        "Make sure to reset password for the user again after cleaning the session table "
+                        "to remove old sessions of the user."
+                    ),
+                    "warning",
+                )
+            else:
+                for s in session.query(user_session_model).all():
+                    if s.expiry < datetime.utcnow():
+                        # housekeeping: delete old sessions that have expired
+                        session.delete(s)

Review Comment:
   Agree. I think I added it yet before I've added "clean" command and  the warnigns / documentation updates. I think the documentation is more than enough for the user to run the cleanup and automated housekeeping here is quite a bit different feature, that also has a potential for some problematic side-effects. Let me remove it.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org