andrewmusselman opened a new pull request, #646:
URL: https://github.com/apache/tooling-trusted-releases/pull/646
# Invalidate PATs when user account is disabled
Fixes #598
## Problem
When an LDAP account is banned or deleted, the user's Personal Access Tokens
remain in the database. While the existing LDAP check in `issue_jwt()` prevents
banned users from exchanging PATs for JWTs, the stale tokens persist for up to
180 days, violating ASVS 7.4.2.
## Solution
Three layers of defense, two of which are new:
1. **LDAP check at JWT exchange** (already existed) — `issue_jwt()` rejects
banned users immediately
2. **Automated background cleanup** (new) — polls LDAP ~hourly and revokes
PATs for banned/deleted accounts
3. **Manual admin page** (new) — allows admins to revoke all PATs for a user
instantly
## Changes
### New files
- `atr/token_cleanup.py` — background cleanup loop with `cleanup_loop()` and
`revoke_pats_for_banned_users()`
- `atr/admin/templates/revoke-user-tokens.html` — admin page template
- `tests/unit/test_token_cleanup.py` — 7 unit tests covering active
accounts, banned accounts, deleted accounts, LDAP failures, race conditions,
and multi-user cleanup
- `tests/e2e/admin/` — e2e tests for the admin revoke tokens page (page
rendering, form validation, revocation flow, nav link)
### Modified files
- `atr/storage/writers/tokens.py` — adds `FoundationAdmin` class with
`revoke_all_user_tokens()`
- `atr/storage/__init__.py` — wires `tokens` into `WriteAsFoundationAdmin`
- `atr/server.py` — starts/stops the cleanup loop task
- `atr/admin/__init__.py` — adds `RevokeUserTokensForm`, GET/POST route
handlers
- `atr/templates/includes/topnav.html` — adds "Revoke user tokens" nav link
under Admin
- `atr/docs/authentication-security.md` — documents the three-layer defense
and automated cleanup
- `atr/docs/authorization-security.md` — documents admin bulk revocation and
automated cleanup access control
## Design decisions
- **Separate module** rather than adding to `cache.py` — `cache.py` is
solely about LDAP admin caching; token cleanup is active security enforcement.
A separate module also avoids circular imports (`cache` → `storage` → `user` →
`cache`).
- **Per-UID error handling** in the cleanup loop — one LDAP failure doesn't
block revocation of other users' tokens.
- **Poll interval offset** (3617s vs 3631s for admin cache) — avoids
simultaneous LDAP request spikes.
- **`revoke_pats_for_banned_users()` is public** — directly callable from
tests and potentially a future "run now" button.
- **REVOKE confirmation** on the admin form — prevents accidental bulk
revocation.
- **All revocations are audit logged** — both automated (via
`storage.audit()`) and manual (via `append_to_audit_log()`).
---
## Required acknowledgements
* [x] I have read and followed **CONTRIBUTING.md**
* [x] I have read **DEVELOPMENT.md**
* [x] I have run the required tests and checks locally
* [x] All required checks are currently passing
* [x] This branch is **rebased on the current `main` branch**
---
## Rebase confirmation details (optional but encouraged)
```
$ git fetch upstream main
From github.com:apache/tooling-trusted-releases
* branch main -> FETCH_HEAD
$ git rebase upstream/main
Current branch invalidate-pats-598 is up to date.
```
---
## Additional notes
* Tested locally, requires REVOKE, lists all users' PATs below
<img width="1260" height="783" alt="image"
src="https://github.com/user-attachments/assets/240b2513-8f8b-47fd-ae68-91f4b1bcf48b";
/>
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
