Skip to content

Revoke connector refresh families and shared-password cookies on credential change - #29

Merged
thorwhalen merged 2 commits into
mainfrom
security/26-revoke-connector-and-shared
Sep 22, 2026
Merged

thorwhalen merged 2 commits into
mainfrom
security/26-revoke-connector-and-shared

Conversation

@thorwhalen

@thorwhalen thorwhalen commented Sep 22, 2026 •

Copy link
Copy Markdown
Member

Closes #26

What

  • One hook for every credential change. New enlace_auth.auth.revocation module. make_on_credentials_changed(session_store, *, refresh_store=None, code_store=None, tombstone_ttl=0) returns hook(email, *, keep=None). The plugin builds it and injects it into make_auth_router and make_admin_router through a new keyword-only on_credentials_changed argument. If the argument is not passed, the hook revokes sessions only, as before. It runs on admin delete, admin password set, self-service change and reset-link redemption. The set-password CLI also revokes the account's connector families.
  • Connector refresh families. revoke_refresh_subject finds every family whose records belong to the email (case-insensitive). For each one, revoke_refresh_family writes the family:<id> tombstone first and then deletes the records. This is the same logic the router's _revoke_family now delegates to, so the refresh grant refuses any family that is rotating concurrently. It also drops the subject's unredeemed authorization codes. Access JWTs that were already issued still live out their TTL.
  • Shared-password cookies. shared_auth_<app> now signs an HMAC (keyed with the signing key) of the app's current shared-password hash, where it used to sign the constant "1". The middleware and /auth/shared-login compare it in constant time. Rotating the hash therefore ends older cookies. An app with no configured hash now admits no cookie; before, it would accept a signed constant.
  • Session sweep. SessionStore(store, *, max_age=None, sweep_batch=100) gets a bounded sweep with a cursor, run on create(). The plugin passes session_max_age.

Compatibility

  • Upgrade effect: existing shared-password cookies (value "1") are refused after deploy, so shared-app users re-enter the password once.
  • tests/test_auth_middleware.py::test_protected_shared_with_valid_cookie asserted the old constant-cookie behaviour that this issue removes. It now mints a fingerprint cookie. Two added tests cover the legacy/rotated cookie and the no-hash case.
  • Not changed: enlace-auth revoke-connector-session still deletes records without writing tombstones. Its tests assume the store holds only token records.
  • All new parameters are keyword-only with the old behaviour as default. Dependents enlace, enlace_docker and tw_platform do not call the changed signatures; their suites were run against this branch (see comment).

Tests

tests/test_credential_revocation.py: helper unit tests; end-to-end through the plugin with the OAuth server enabled (admin set password, admin delete, self-service change → the victim's family is tombstoned, the other user's family survives, codes are dropped); shared-password rotation across a restart refuses the old cookie; sweep boundedness. Full suite: 399 passed.

Independent refute-review (security) — findings and fixes

  • Fixed: a family created during a password change survived. The window is between the code grant consuming the code and writing the family. A related window: a code written after the code-store scan. revoke_refresh_subject now writes a per-subject marker first (revoked_before, which lives for the family max lifetime). The code grant refuses codes issued at or before it; codes now record when the session was read. The refresh grant and _replayed refuse and revoke families authorized at or before it; records carry auth_at, and legacy records derive it from family_exp. Tests cover both cases and fail when the check is removed.
  • Fixed: the session sweep made every login O(n) on the file store. Measured at 0.6 s with 20k sessions. It now runs at most once per sweep_interval (1 h) per process.
  • Fixed: connector revocation is now wired even while the OAuth server is disabled. Connector revocation still runs if session revocation raises. set-password reports a partial failure clearly. list-connector-sessions skips markers.
  • Left as is: revoke-connector-session still deletes without tombstones (existing tests). Revocation cost is O(families × store), but it only runs on admin and credential-change requests.
  • Dependents re-run after the fixes: enlace 308 passed; enlace_docker 58 passed; tw_platform 576 passed with the same 4 pre-existing root-only failures. Suite: 403 passed.

🤖 Generated with Claude Code

…ential change

One on_credentials_changed(email, *, keep=None) hook, built by the plugin and
injected into the auth and admin routers, now runs on every path that changes an
account's credentials (admin delete, admin password set, self-service change,
reset-link redemption; the set-password CLI does the same). It revokes the
account's browser sessions as before and, when the OAuth server keeps refresh
tokens, tombstones and deletes every refresh family issued to that account and
drops its unredeemed authorization codes.

shared_auth_<app> cookies now sign a keyed fingerprint of the app's current
shared-password hash instead of a constant; the middleware and the shared-login
page compare it, so rotating a shared password ends older cookies, and an app
with no configured password admits no cookie.

SessionStore takes an optional max_age and sweeps a bounded batch of expired
records whenever a session is created.

Closes #26

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@thorwhalen

Copy link
Copy Markdown
Member Author

Dependents run against this branch (fresh venvs, local editable enlace + this enlace_auth):

  • i2mint/enlace: 308 passed, 1 skipped (with the venv's python on PATH; two test_build cases spawn a bare python).
  • i2mint/enlace_docker: 58 passed, 2 skipped.
  • tw_platform tests: 576 passed, 8 skipped, 4 failed. All 4 are test_grant_wrapper_refuses_before_doing_anything, which expects a non-root runner and is pre-existing (it fails identically on main when run as root).

- revoke_refresh_subject first writes a per-subject marker (revoked_before=T,
  lives for the family max lifetime). The code grant refuses codes issued at or
  before T (codes now record when the session was read), and the refresh grant
  and replay path refuse families authorized at or before T (records carry
  auth_at). This covers a family whose code was consumed before the scan but
  written after it, and a code written after the code-store scan.
- The plugin wires connector revocation even while the OAuth server is off.
- The hook runs connector revocation even if session revocation raises.
- set-password reports clearly if connector revocation fails.
- list-connector-sessions skips revocation markers.
- SessionStore sweeps at most once per sweep_interval (default 1h) per
  process, so a login never pays a directory walk.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@thorwhalen
thorwhalen merged commit b1bf0bc into main Sep 22, 2026
12 checks passed
@thorwhalen
thorwhalen deleted the security/26-revoke-connector-and-shared branch September 22, 2026 16:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Revoke connector tokens and shared-password cookies when credentials change

1 participant