Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,18 @@ within one access-token lifetime rather than one refresh-token lifetime. Keep
`access_token_ttl_seconds` short for that reason: with refresh in place, a short
access token costs nothing and is what bounds revocation lag.

**A credential change ends connector sessions too.** Deleting a user, an admin
password set, a self-service password change, a reset-link redemption and the
`set-password` CLI all revoke the account's browser sessions *and* its refresh
families (plus any unredeemed authorization codes); the connector must be
re-authorized. Already-issued access tokens live out their TTL.

**Rotating a shared password ends its cookies.** A `shared_auth_<app>` cookie
carries a keyed fingerprint of the app's shared-password hash at the time it was
minted, so after you change the hash (and restart), every cookie from the old
password is refused. Upgrading to this version signs everyone out of
shared-password apps once.

Plus environment variables:

- `ENLACE_SIGNING_KEY` — signing key (32+ chars). Generate with `python -c
Expand Down
45 changes: 44 additions & 1 deletion enlace_auth/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,34 @@ def _load_session_store(toml_path: Path = Path("platform.toml")):
return SessionStore(factory("sessions"))


def _connector_tombstone_ttl(toml_path: Path = Path("platform.toml")) -> int:
"""The refresh-family tombstone lifetime the configured OAuth server uses."""
from enlace_auth.auth.revocation import refresh_tombstone_ttl

osc = coerce_auth_config(PlatformConfig.from_toml(toml_path).auth).oauth_server
return refresh_tombstone_ttl(
refresh_token_ttl=osc.refresh_token_ttl_seconds,
refresh_reuse_detection=osc.refresh_reuse_detection_seconds,
)


def _revoke_connector_subject(email: str, toml_path: Path) -> int:
"""Revoke *email*'s connector refresh families (tombstoned); return count."""
from enlace_auth.auth.revocation import revoke_refresh_subject
from enlace_auth.stores import make_file_store_factory

auth = coerce_auth_config(PlatformConfig.from_toml(toml_path).auth)
factory = make_file_store_factory(auth.stores.path)
return revoke_refresh_subject(
factory("oauth_refresh_tokens"),
email,
reason="revoked by the enlace-auth CLI",
tombstone_ttl=_connector_tombstone_ttl(toml_path),
marker_ttl=auth.oauth_server.refresh_family_max_lifetime_seconds,
code_store=factory("oauth_codes"),
)


def _load_user_store(toml_path: Path = Path("platform.toml")):
"""Open the platform's user store (email -> {password_hash, ...})."""
from enlace_auth.stores import make_file_store_factory
Expand Down Expand Up @@ -228,7 +256,20 @@ def set_password(email: str, *, toml: str = "platform.toml"):
store[key] = updated
# Same rule as the HTTP reset paths: a new password ends the old sessions.
revoked = _load_session_store(Path(toml)).revoke_user(key)
print(f"Password updated for {key}; {revoked} existing session(s) revoked.")
try:
families = _revoke_connector_subject(key, Path(toml))
except Exception as e: # noqa: BLE001 - the password IS changed; say what isn't
print(
f"Password updated for {key}; {revoked} existing session(s) revoked, "
f"but connector sessions were NOT revoked ({e}). Run "
f"`enlace-auth revoke-connector-session --email {key}`.",
file=sys.stderr,
)
sys.exit(1)
print(
f"Password updated for {key}; {revoked} existing session(s) and "
f"{families} connector session(s) revoked."
)


def reset_link(
Expand Down Expand Up @@ -413,6 +454,8 @@ def list_connector_sessions(*, json: bool = False, toml: str = "platform.toml"):
continue
if not record or record.get("consumed_at") is not None:
continue # spent tokens are tombstones, not sessions
if not record.get("family") or not record.get("email"):
continue # revocation markers, not sessions
families[record.get("family", key)] = {
"family": record.get("family"),
"email": record.get("email"),
Expand Down
15 changes: 13 additions & 2 deletions enlace_auth/admin/routes.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ def make_admin_router(
reset_link_ttl: int = DEFAULT_HANDOFF_TTL,
resource_allowlist: Optional[Mapping[str, list[str]]] = None,
public_base_url: Optional[str] = None,
on_credentials_changed=None, # CredentialsChanged; default: sessions only
) -> APIRouter:
"""Build a FastAPI router exposing ``/_admin/api/*`` endpoints.

Expand Down Expand Up @@ -115,7 +116,17 @@ def make_admin_router(
(it isn't) or, worse, to make some *other* app public by analogy (which
would be). Passing the allow-list lets the dashboard show who can actually
reach each one.

``on_credentials_changed`` (``hook(email, *, keep=None)``) runs after a
user is deleted or has their password set. The default revokes the
account's browser sessions; the plugin injects one that also revokes its
OAuth connector refresh families (``enlace_auth.auth.revocation``).
"""
from enlace_auth.auth.revocation import make_on_credentials_changed

credentials_changed = on_credentials_changed or make_on_credentials_changed(
session_store
)
admin_set = frozenset(e.lower() for e in admin_emails)
apps_snapshot = list(apps)
app_by_name = {a.name: a for a in apps_snapshot}
Expand Down Expand Up @@ -214,7 +225,7 @@ async def delete_user(email: str, request: Request) -> dict[str, Any]:
# a deleted account keeps working until its cookie expires unless its
# sessions go too. (An actor deleting themselves is logged out.)
_ = actor
session_store.revoke_user(target)
credentials_changed(target)
return {"ok": True, "email": target}

@router.post("/users/{email}/password")
Expand All @@ -234,7 +245,7 @@ async def admin_reset_password(
user_store[target] = record
# An admin reset is how a compromised account is recovered: whoever
# holds a session opened with the old password must lose it.
session_store.revoke_user(target)
credentials_changed(target)
return {"ok": True, "email": target}

@router.post("/users/{email}/reset-link")
Expand Down
16 changes: 11 additions & 5 deletions enlace_auth/auth/middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
from urllib.parse import unquote

from enlace_auth.auth.cookies import verify_cookie
from enlace_auth.auth.revocation import shared_cookie_valid
from enlace_auth.auth.sessions import SessionStore

_logger = logging.getLogger("enlace_auth.middleware")
Expand Down Expand Up @@ -291,16 +292,21 @@ async def __call__(self, scope, receive, send):
app_id = rule.app_id if rule is not None else ""
name = f"shared_auth_{app_id}"
token = cookies.get(name)
if (
not token
or verify_cookie(
# The cookie must carry the fingerprint of the app's CURRENT shared
# password hash: rotating the password ends every older cookie, and
# an app with no configured password admits no cookie at all.
value = (
verify_cookie(
token,
self._signing_key,
max_age=self._max_age,
salt=f"shared:{app_id}",
)
is None
):
if token
else None
)
expected_hash = rule.shared_password_hash if rule is not None else None
if not shared_cookie_valid(value, expected_hash, self._signing_key):
return await self._deny(scope, send, "shared")
state["user_id"] = "shared"
await self.app(scope, receive, send)
Expand Down
90 changes: 53 additions & 37 deletions enlace_auth/auth/oauth_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,12 @@

from enlace_auth.auth import pages
from enlace_auth.auth.cookies import sign_cookie, verify_cookie
from enlace_auth.auth.revocation import (
refresh_tombstone_ttl,
revoke_refresh_family,
revoked_family_key,
subject_revoked_before,
)
from enlace_auth.auth.sessions import SessionStore
from enlace_auth.stores.validation import sanitize_key

Expand Down Expand Up @@ -301,6 +307,10 @@ def _norm_resource(resource: str) -> str:
# One flag, consulted everywhere: a store with a zero TTL is NOT refresh
# support, and metadata that says otherwise mints tokens dead on arrival.
_refresh_enabled = refresh_store is not None and refresh_token_ttl > 0
_tombstone_ttl = refresh_tombstone_ttl(
refresh_token_ttl=refresh_token_ttl,
refresh_reuse_detection=refresh_reuse_detection,
)
_local_claims: set[str] = set()
_local_claim_lock = threading.Lock()
_client_sweep_cursor: dict = {"pos": 0}
Expand Down Expand Up @@ -364,40 +374,9 @@ def _revoke_family(family: str, *, reason: str) -> int:
"""
if refresh_store is None:
return 0
# Write the marker FIRST. Revocation used to be expressed as the ABSENCE
# of records, which cannot work while another worker is concurrently
# creating them: a successor written after the scan snapshot survived,
# and a tombstone write resurrected a parent the scan had just deleted,
# so a chain that happened to be rotating defeated detection outright
# (measured at ~22% of attempts). A single positive fact cannot be raced.
now = _now()
refresh_store[_revoked_key(family)] = {
"revoked_at": now,
"reason": reason,
# Must outlive every token that could still belong to this family.
"exp": now + max(refresh_token_ttl, refresh_reuse_detection),
}
revoked = 0
for key in list(refresh_store):
try:
record = refresh_store[key]
except KeyError:
continue
if (record or {}).get("family") != family:
continue # (the marker itself carries no "family" key)
try:
del refresh_store[key]
revoked += 1
except KeyError:
pass
_logger.warning(
"oauth: revoked refresh family %s (%d token(s)) — %s. The connector "
"using it is now dead until a human re-authorizes it.",
family,
revoked,
reason,
return revoke_refresh_family(
refresh_store, family, reason=reason, tombstone_ttl=_tombstone_ttl
)
return revoked

def _issue_refresh(
*,
Expand All @@ -424,6 +403,8 @@ def _issue_refresh(
"scope": scope,
"email": email,
"iat": now,
# When the family was authorized; carried through every rotation.
"auth_at": now,
"exp": now + refresh_token_ttl,
"family_exp": family_exp,
"consumed_at": None,
Expand All @@ -439,11 +420,23 @@ def _touch_client(client_id: str, now: int) -> None:

def _revoked_key(family: str) -> str:
"""Store key for the tombstone that marks a whole family revoked."""
return f"family:{family}"
return revoked_family_key(family)

def _family_revoked(family: Optional[str]) -> bool:
return bool(family) and refresh_store.get(_revoked_key(family)) is not None

def _authorized_before_revocation(record: dict) -> bool:
"""True if the subject's credentials changed after this family began."""
revoked_before = subject_revoked_before(refresh_store, record.get("email"))
if revoked_before is None:
return False
auth_at = record.get("auth_at")
if not isinstance(auth_at, (int, float)):
# Records minted before ``auth_at`` existed: derive it from the
# absolute ceiling, which is fixed at authorization.
auth_at = (record.get("family_exp") or 0) - refresh_family_max_lifetime
return auth_at <= revoked_before

def _grace_key(key: str) -> str:
"""Store key for the short-lived retry copy of a successor plaintext."""
return f"grace:{key}"
Expand Down Expand Up @@ -662,9 +655,15 @@ def _validate_authorize(
)
return auth, None

def _issue_code(auth: _Authorized, email: str) -> str:
def _issue_code(auth: _Authorized, email: str, *, issued_at: int) -> str:
"""Mint a code; *issued_at* is when the session was read, not written.

A credential change between reading the session and writing the code
would otherwise miss this code (see ``subject_revoked_before``).
"""
code = secrets.token_urlsafe(32)
code_store[code] = {
"iat": issued_at,
"client_id": auth.client_id,
"email": email,
"redirect_uri": auth.redirect_uri,
Expand All @@ -690,6 +689,7 @@ async def authorize(request: Request):
auth.redirect_uri, "invalid_request", auth.state, "PKCE S256 required"
)

session_read_at = _now()
email = _current_email(request)
if not email:
# Reuse the platform login, returning here once authenticated.
Expand All @@ -702,7 +702,7 @@ async def authorize(request: Request):
return HTMLResponse(_denied_page(email), status_code=403)

if not require_consent:
code = _issue_code(auth, email)
code = _issue_code(auth, email, issued_at=session_read_at)
return RedirectResponse(
_with_query(auth.redirect_uri, {"code": code, "state": auth.state}),
status_code=302,
Expand All @@ -725,6 +725,7 @@ async def authorize_consent(
csrf: str = Form(...),
decision: str = Form(...),
):
session_read_at = _now()
email = _current_email(request)
if not email:
return JSONResponse({"error": "login_required"}, status_code=401)
Expand All @@ -748,7 +749,7 @@ async def authorize_consent(
return _redirect_error(redirect_uri, "access_denied", state)
if decision != "approve":
return _redirect_error(redirect_uri, "access_denied", state)
code = _issue_code(auth, email)
code = _issue_code(auth, email, issued_at=session_read_at)
return RedirectResponse(
_with_query(redirect_uri, {"code": code, "state": state}),
status_code=302,
Expand Down Expand Up @@ -869,6 +870,12 @@ def _grant_authorization_code(
or not _verify_pkce_s256(code_verifier, data["code_challenge"])
):
return JSONResponse({"error": "invalid_grant"}, status_code=400)
# Issued before the account's credentials changed: dead, even if the
# revocation's scan could not see it (i2mint/enlace_auth#26).
revoked_before = subject_revoked_before(refresh_store, data["email"])
code_iat = data.get("iat", data["exp"] - code_ttl)
if revoked_before is not None and code_iat <= revoked_before:
return JSONResponse({"error": "invalid_grant"}, status_code=400)

body, _ = _token_payload(
iss=_issuer(request),
Expand Down Expand Up @@ -909,6 +916,11 @@ def _grant_refresh_token(
# that was meant to delete it.
if _family_revoked(record.get("family")):
return JSONResponse({"error": "invalid_grant"}, status_code=400)
if _authorized_before_revocation(record):
_revoke_family(
record.get("family"), reason="the account's credentials changed"
)
return JSONResponse({"error": "invalid_grant"}, status_code=400)

if record.get("consumed_at") is not None:
return _replayed(
Expand Down Expand Up @@ -1037,6 +1049,7 @@ def _rotate(
"scope": scope,
"email": email,
"iat": now,
"auth_at": record.get("auth_at"),
"exp": now + refresh_token_ttl,
"family_exp": family_exp,
"consumed_at": None,
Expand Down Expand Up @@ -1106,6 +1119,9 @@ def _replayed(
family = record.get("family")
if _family_revoked(family):
return JSONResponse({"error": "invalid_grant"}, status_code=400)
if _authorized_before_revocation(record):
_revoke_family(family, reason="the account's credentials changed")
return JSONResponse({"error": "invalid_grant"}, status_code=400)
within_grace = now - (record.get("consumed_at") or 0) < refresh_reuse_grace
same_client = record.get("client_id") == client_id

Expand Down
Loading
Loading