Clear the session data on login by default. Added support for an allow-list of session keys to preserve.

[lavish]

Added a new configuration option PRESERVED_SESSION_KEYS to preserve specific values in the session between unauthenticated and authenticated states. By default, unrelated session data is cleared on login.

As privately reported to @maxcountryman and @davidism, this proposed change improves the security of Flask-Login when used in combination with CSRF protection libraries such as Flask-WTF. The patch ensures that CSRF tokens issued before login are invalidated after the user performs a successful authentication, eliminating the risk of CSRF token fixation attacks.

This PR was submitted publicly as requested by the project maintainer.

[coveralls]

Coverage decreased (-0.2%) to 98.521% when pulling e81f312 on lavish:main into bb25407 on maxcountryman:main.

[davidism]

I'd rather just have session.clear(). If users want to preserve something, they can store the value before calling login_user and restore it afterwards.

[lavish]

Hi @davidism, thanks for processing the PR. From a security standpoint doing a session.clear() would certainly get the job done.

However, this change would also delete all session keys populated by flask-login (including _user_id, _remember, _remember_seconds, _id, _fresh, next, and _permanent). As a consequence, two of the existing tests will fail: permanent_strong_session_protection_marks_session_unfresh and session_protection_strong_deletes_session. While the latter can be fixed by manually retrieving and setting the additional attribute foo, it's unclear how to deal with the former.

Furthermore, from a usability perspective, this would probably hamper the session management abstractions provided by flask-login.

[davidism]

I want to do session.clear(), so you're going to have to figure out how to address that. I don't know what that test is testing, but it sounds like it should in fact fail if logging in is supposed to clear the previous state. I'm not aware of the "session management abstractions" that we provide, so I don't think that matters.

[maxcountryman]

I agree with @davidism: clearing the session is the right path. We don't want to overfit a solution.

[lavish]

Alright then, totally fine with that! Shall I change the current PR to perform a simple session.clear()?