mirror of
https://github.com/onyx-dot-app/onyx.git
synced 2026-10-23 11:53:08 -05:00
Justin Tahara
2fb071f63f
## Description
**Problem**
- A personal access token (PAT) created before a password reset still
returned 200 after the reset.
- Applies to the self-service password change and to the admin reset.
- The admin reset dialog did not tell the admin that the user is signed
out.
- The admin reset result dialog showed both Cancel and Done.
**Security rationale**
- A password reset is the recovery step after a credential leak.
- A PAT is a long-lived bearer credential. If it survives the reset, an
attacker keeps access.
- #16049 signs out sessions on reset. This change closes the same gap
for PATs.
**Backend**
- New `revoke_all_pats_for_user__no_commit` in `db/pat.py`: sets
`is_revoked` and `expires_at = now` on every active PAT of the user, all
types. Returns the count.
- `UserManager._update` override: every password write revokes the
user's PATs in the same commit as the new hash. Sessions are still
revoked after the commit (`revoke_sessions`).
- Paths that write a password (the same as #16049):
- Admin reset: `POST /password/reset_password` →
`reset_password_as_admin`
- Self-service change: `POST /password/change-password` →
`change_password_if_old_matches`
- Forgot-password token flow: `POST /auth/reset-password` →
fastapi-users `reset_password`
- A wrong current password revokes nothing.
- API keys (service accounts) are not affected.
**Behavior change for users**
- After any password reset or change, all of the user's PATs stop
working (401/403).
- Scripts, CLIs and integrations that use a PAT break after a reset. The
user must create a new PAT.
- Craft sandbox PATs are revoked too. A RUNNING sandbox gets a new PAT
at its next reservation (`reserve_sandbox__no_commit`); the proxy reads
the stored PAT on each request.
**UI**
- Admin reset dialog: the description now says the user is signed out of
all sessions and their PATs are revoked.
- Admin reset result: Cancel is hidden. Only Done shows.
- Change-password dialog: new description says other sessions are signed
out and PATs are revoked.
- Settings page clears the cached token list and refetches it after a
password change.
- New copy is translated in every locale.
Fixes ENG-4424
## Screenshots
Admin reset confirm
| Before | After |
| --- | --- |
| <img width="700" alt="Before: reset confirm"
src="https://github.com/user-attachments/assets/7973f213-2787-40a3-8ce2-e134de34c35c"
/> | <img width="700" alt="After: reset confirm"
src="https://github.com/user-attachments/assets/1ede0f6d-33ad-449b-947f-057bd31aa459"
/> |
Admin reset success
| Before | After |
| --- | --- |
| <img width="700" alt="Before: reset success with Cancel and Done"
src="https://github.com/user-attachments/assets/3e9cae99-d8ea-44b6-b615-5d1bd0537b1b"
/> | <img width="700" alt="After: reset success with Done only"
src="https://github.com/user-attachments/assets/cad3582c-6648-402b-be6f-8b50cbb683bc"
/> |
Self-service change password
| Before | After |
| --- | --- |
| <img width="700" alt="Before: change password"
src="https://github.com/user-attachments/assets/0a45df61-6fe9-4325-931b-6ea08ab5ba04"
/> | <img width="700" alt="After: change password"
src="https://github.com/user-attachments/assets/687a4007-f777-47a2-be5c-ad68ec6789eb"
/> |
## How Has This Been Tested?
- External dependency unit
(`tests/external_dependency_unit/auth/test_pat_revocation.py`, real
Postgres + Redis):
- Admin reset revokes USER and CRAFT PATs, another user's PAT stays
valid.
- Self-service change revokes PATs.
- Forgot-password `reset_password` (real token) revokes PATs.
- Wrong current password keeps PATs.
- Already-revoked PATs are not counted again.
- A failed PAT revocation keeps the old password (one commit). Fails if
the revocation commits separately.
- External dependency unit
(`tests/external_dependency_unit/craft/test_sandbox_pat.py`): password
change with a RUNNING sandbox, then `ensure_sandbox_ready` stores a new
valid PAT. Fails without the reservation hook.
- `test_session_revocation.py`, `test_auth_audit_events.py`, and the
Craft sandbox lifecycle, provisioning, and PAT resolver suites still
pass (62 passed, 1 known xfail).
- Integration
(`tests/integration/tests/security/test_session_revocation_on_reset.py`):
new cases for admin reset and self-service change (PAT 200 before,
401/403 after; admin's own PAT stays valid). Not run locally; CI runs
them.
- Jest (`UserActionModals.test.tsx`): confirm state shows the new copy
and Cancel; success state shows Done and no Cancel. The test fails
without `hideCancel`.
- `tsc` clean. `pre-commit` clean, including the i18n copy-change check.
## Additional Options
- [x] [Optional] Please cherry-pick this PR to the latest release
version.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
<!-- This is an auto-generated description by cubic. -->
---
## Summary by cubic
Revokes all of a user's personal access tokens whenever their password
is reset or changed, at the same three hook points that already sign out
sessions: admin reset, self-service change, and the forgot-password
flow. Previously a PAT minted before a reset kept working, leaving a
long-lived credential active after a suspected leak.
A wrong current password revokes nothing. API keys are unaffected. Craft
sandbox PATs are revoked too; a new one is minted on the next sandbox
run.
**UI**
- Admin and change-password dialogs now state that sessions end and PATs
are revoked, in every locale.
- Admin reset result shows only Done; Cancel is hidden.
- The settings page refreshes the token list after a password change.
**Migration**
- After any reset, scripts, CLIs, and integrations using PATs return
401/403 and require minting a new token.
<sup>Written for commit 734205b56a.
Summary will update on new commits.</sup>
<a
href="https://cubic.dev/pr/onyx-dot-app/onyx/pull/16449?utm_source=github"
target="_blank" rel="noopener noreferrer"
data-no-image-dialog="true"><picture><source
media="(prefers-color-scheme: dark)"
srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source
media="(prefers-color-scheme: light)"
srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img
alt="View guided diff"
src="https://www.cubic.dev/buttons/review-in-cubic-light.svg"></picture></a>
<a
href="https://www.cubic.dev/action/auto-fix/pr/onyx-dot-app/onyx/16449?returnTo=https%3A%2F%2Fgithub.com%2Fonyx-dot-app%2Fonyx%2Fpull%2F16449&source=description"
target="_blank" rel="noopener noreferrer"
data-no-image-dialog="true"><picture><source
media="(prefers-color-scheme: dark)"
srcset="https://www.cubic.dev/buttons/turn-on-auto-fix-dark.svg"><source
media="(prefers-color-scheme: light)"
srcset="https://www.cubic.dev/buttons/turn-on-auto-fix-light.svg"><img
alt="Turn on auto-fix"
src="https://www.cubic.dev/buttons/turn-on-auto-fix-light.svg"></picture></a>
<!-- End of auto-generated description by cubic. -->
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
<!-- coderabbit-ai-generated:v1 -->
## Summary by CodeRabbit
- **Security**
- Password resets and password changes now revoke active personal access
tokens and sign users out of other sessions.
- Service-account API keys and the current session remain unaffected.
- **Craft Sandboxes**
- Running sandboxes receive a replacement personal access token at their
next reservation if their previous token was revoked.
- **User Experience**
- Password-management warnings explain session sign-out and token
revocation across supported languages.
- Token lists refresh after password changes, and reset dialogs provide
clearer completion behavior.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
