potiuk opened a new pull request, #72657:
URL: https://github.com/apache/airflow/pull/72657
Changing a password through `PATCH /auth/fab/v1/users/{username}` set the new
hash and saved the user, without ending the sessions the old password had
established.
A session captured before the change kept authenticating as that user, so an
administrator changing a password during recovery or incident response did
not
evict whoever held it. The old password *was* correctly rejected — which is
exactly what makes the surviving session easy to miss.
FAB already has a lifecycle-aware path: `reset_password` sets the hash, calls
`reset_user_sessions`, then saves. The user-management API bypassed that and
performed only the hash update.
### The change
`reset_user_sessions` is called when, and only when, the password actually
changed. A password supplied in the body but excluded by `update_mask` is not
applied and invalidates nothing; changes to other fields leave sessions
alone.
**Ordered after persistence, unlike `reset_password`.** `reset_user_sessions`
commits its deletions immediately, and `security_manager.update_user` rolls
back
and returns `False` on failure — a return value this service previously
discarded. Invalidating first would have logged the user out even when the
password update then failed, leaving the old password working, the user
evicted,
and the API reporting success. That return value is now checked and surfaced
as a
500.
### Scope, stated plainly
This ends FAB **server-side sessions on the database session backend**,
which is
where the issue was reported and reproduced.
It does **not** cover two adjacent cases, and neither is closed by this PR:
* `reset_user_sessions` is a no-op on other session backends — with
`securecookie` it only logs a warning.
* No session mechanism here revokes JWTs, so a `_token` cookie or bearer
token
issued before the password change stays valid until it expires.
Making a password change invalidate outstanding tokens needs a per-user token
version, a password-changed timestamp checked during token validation, or an
equivalent. That is a broader change than this one and is worth doing
separately
rather than folded in here.
### Tests
Four cases: the password change invalidates and does so *after* persistence;
a
non-password update does not; a password excluded by `update_mask` does not;
and
a failed persistence raises rather than evicting the user.
Against unpatched sources two fail — `Expected 'reset_user_sessions' to be
called
once. Called 0 times.` and `DID NOT RAISE HTTPException`. 153 tests pass
across
`providers/fab/tests/unit/fab/auth_manager/api_fastapi/`.
---
##### Was generative AI tooling used to co-author this PR?
- [X] Yes (please specify the tool below)
Generated-by: Claude Code following [the
guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions)
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
