From 2ab4430f5733cf2cd2157d3b8bbb2b0f193586cc Mon Sep 17 00:00:00 2001 From: Brandon Corbett Date: Sat, 24 Oct 2026 21:56:12 -0400 Subject: [PATCH 1/5] fix(console): keep the API's framing and sniffing headers on the console proxy The console proxy forwarded only Content-Type, Cache-Control, ETag and Last-Modified, so the API's X-Frame-Options, Content-Security-Policy and X-Content-Type-Options were dropped and the console served on the application's origin could be framed. Forward those three as well. Closes #12 --- README.md | 3 ++- src/seamless_auth/_console.py | 12 +++++++++++- tests/test_console.py | 18 ++++++++++++++++++ 3 files changed, 31 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 0c9e39d..8a0deae 100644 --- a/README.md +++ b/README.md @@ -201,7 +201,8 @@ urlpatterns += [path("console/", include(console_urlpatterns))] It serves `GET` and `HEAD` only and forwards nothing but the method and path, so the browser's cookies never reach the upstream. It refuses any path that could leave the console (dot segments, encoded separators), and follows a redirect only while it stays inside the console on the auth API. -When you serve it, add your API's origin to the auth server's `ORIGINS` so passkey ceremonies +It keeps the auth API's `X-Frame-Options`, `Content-Security-Policy` and `X-Content-Type-Options`, +so the console cannot be framed on your origin. When you serve it, add your API's origin to the auth server's `ORIGINS` so passkey ceremonies started in the console verify. ## How it runs diff --git a/src/seamless_auth/_console.py b/src/seamless_auth/_console.py index 7d05f4a..f5c88f0 100644 --- a/src/seamless_auth/_console.py +++ b/src/seamless_auth/_console.py @@ -20,7 +20,17 @@ CONSOLE_BASE_PATH = "/console" UPSTREAM_TIMEOUT = 10.0 MAX_REDIRECTS = 5 -FORWARDED_HEADERS = ("content-type", "cache-control", "etag", "last-modified") +FORWARDED_HEADERS = ( + "content-type", + "cache-control", + "etag", + "last-modified", + # The console runs on the application's origin, so without the API's framing and + # sniffing policy it could be framed there. + "x-frame-options", + "content-security-policy", + "x-content-type-options", +) def _error(status: int, message: str) -> AuthResponse: diff --git a/tests/test_console.py b/tests/test_console.py index ebcc545..2269b6f 100644 --- a/tests/test_console.py +++ b/tests/test_console.py @@ -40,6 +40,24 @@ def test_the_console_is_proxied_from_the_auth_api(api: FakeApi) -> None: assert "cookie" not in recorded.headers and "authorization" not in recorded.headers +def test_the_console_keeps_the_apis_framing_and_sniffing_policy(api: FakeApi) -> None: + policy = { + "content-type": "text/html", + "x-frame-options": "DENY", + "content-security-policy": "frame-ancestors 'none'", + "x-content-type-options": "nosniff", + } + api.on("GET", "/console", lambda _r: httpx2.Response(200, text="", headers=policy)) + api.on("HEAD", "/console", lambda _r: httpx2.Response(200, headers=policy)) + adapter = api.adapter() + + for method in ("GET", "HEAD"): + r = read(adapter.console(method, "/")) + assert r.status == 200 + for name, value in policy.items(): + assert r.headers[name] == value, (method, name) + + def test_the_console_root_and_head(api: FakeApi) -> None: api.on("GET", "/console", html("")) # type: ignore[arg-type] api.on("HEAD", "/console", html("")) # type: ignore[arg-type] From 731a719a814d4e6abb6d9b41c78e789961905e2b Mon Sep 17 00:00:00 2001 From: Brandon Corbett Date: Sat, 24 Oct 2026 21:56:43 -0400 Subject: [PATCH 2/5] fix: refuse known placeholder secrets at startup A placeholder such as the starters' dev-only-cookie-signing-key-replace-me is long enough to pass the length check, and it is public. Refuse it, the GRAB_FROM_SEAMLESS_AUTH_PORTAL_DEV example, and anything containing replace-me, for both cookie_secret and service_secret, unless insecure_cookies is set for local development. Refs fells-code/seamless-auth-server#231 --- README.md | 10 +++++++--- src/seamless_auth/_adapter.py | 21 ++++++++++++++++++++ tests/test_secrets.py | 37 +++++++++++++++++++++++++++++++++++ 3 files changed, 65 insertions(+), 3 deletions(-) create mode 100644 tests/test_secrets.py diff --git a/README.md b/README.md index 8a0deae..229791e 100644 --- a/README.md +++ b/README.md @@ -119,11 +119,11 @@ JSON it would be a sign-in the user never made. The client SDKs already send JSO | `auth_server_url` | required | Where the adapter reaches the auth API | | `auth_server_issuer` | the URL | Expected `iss` of the API's tokens, when it differs from the URL you reach it at | | `audience` | the issuer | Expected `aud` of the API's tokens | -| `cookie_secret` | required | Signs the session cookies (32 bytes or more) | -| `service_secret` | required | The service token, read from `SEAMLESS_SERVICE_TOKEN` in the examples; the same value as the API's `API_SERVICE_TOKEN` (32 bytes or more) | +| `cookie_secret` | required | Signs the session cookies (32 bytes or more, and not a placeholder) | +| `service_secret` | required | The service token, read from `SEAMLESS_SERVICE_TOKEN` in the examples; the same value as the API's `API_SERVICE_TOKEN` (32 bytes or more, and not a placeholder) | | `jwks_kid` | `dev-main` | `kid` header on service tokens | | `cookie_domain` | none | Cookie `Domain` | -| `insecure_cookies` | `False` | Drops `Secure`, for local development over HTTP | +| `insecure_cookies` | `False` | Drops `Secure`, and allows placeholder secrets, for local development over HTTP | | `same_site` | `None`, or `Lax` with `insecure_cookies` | Cookie `SameSite` | | `allowed_origins` | none | The only cross-origin callers allowed to change state while cookies are `SameSite=None` | | `access_cookie_name`, `refresh_cookie_name` | `seamless-access`, `seamless-refresh` | Session cookie names | @@ -133,6 +133,10 @@ JSON it would be a sign-in the user never made. The client SDKs already send JSO | `disable_manifest_fetch` | `False` | Use only the manifest bundled with this version | | `http_client` | 15 second timeout, no redirects | Outbound `httpx2.Client` | +The adapter refuses to start with a known placeholder for either secret, such as the starters' +`dev-only-cookie-signing-key-replace-me`, `GRAB_FROM_SEAMLESS_AUTH_PORTAL_DEV`, or anything +containing `replace-me`, unless `insecure_cookies` is set. + ### Client IP The adapter forwards the end user's address and user agent so the auth API rate limits and diff --git a/src/seamless_auth/_adapter.py b/src/seamless_auth/_adapter.py index 1c3a9f4..36d4183 100644 --- a/src/seamless_auth/_adapter.py +++ b/src/seamless_auth/_adapter.py @@ -47,6 +47,13 @@ TRANSPORT_HEADER = "x-seamless-auth-transport" DELIVERY_MODE_HEADER = "x-seamless-auth-delivery-mode" +# Example values from the starters and docs. Each is long enough to pass the length +# check, and each is public. +PLACEHOLDER_SECRETS = frozenset( + {"dev-only-cookie-signing-key-replace-me", "GRAB_FROM_SEAMLESS_AUTH_PORTAL_DEV"} +) +PLACEHOLDER_MARKER = "replace-me" + # The auth API validates service tokens against a fixed issuer and audience, # whatever the adopter's own audience is. SERVICE_ISSUER = "seamless-portal-api" @@ -178,6 +185,16 @@ def __init__( raise ValueError( f"seamless-auth: service_secret must be at least {MIN_SECRET_LENGTH} bytes" ) + if not insecure_cookies: + for option, secret in ( + ("cookie_secret", cookie_secret), + ("service_secret", service_secret), + ): + if _is_placeholder(secret): + raise ValueError( + f"seamless-auth: {option} is a placeholder value. Generate a real " + "secret (it is allowed only with insecure_cookies, for local development)" + ) for name in ( access_cookie_name, refresh_cookie_name, @@ -839,6 +856,10 @@ def _is_access_token(token: str) -> bool: return bool(token) and str_claim(unverified_claims(token), "typ") == "access" +def _is_placeholder(secret: str) -> bool: + return secret in PLACEHOLDER_SECRETS or PLACEHOLDER_MARKER in secret.lower() + + def _user_from(claims: Claims, token: str) -> User: roles = claims.get("roles") return User( diff --git a/tests/test_secrets.py b/tests/test_secrets.py new file mode 100644 index 0000000..acc6000 --- /dev/null +++ b/tests/test_secrets.py @@ -0,0 +1,37 @@ +from __future__ import annotations + +from typing import Any + +import pytest +from conftest import TEST_SECRET, TEST_SERVICE + +from seamless_auth import Adapter + + +@pytest.mark.parametrize( + "placeholder", + [ + "dev-only-cookie-signing-key-replace-me", + "GRAB_FROM_SEAMLESS_AUTH_PORTAL_DEV", + "my-own-long-secret-value-REPLACE-ME-please", + ], +) +@pytest.mark.parametrize("option", ["cookie_secret", "service_secret"]) +def test_placeholder_secrets_are_refused(option: str, placeholder: str) -> None: + settings: dict[str, Any] = { + "auth_server_url": "https://a", + "cookie_secret": TEST_SECRET, + "service_secret": TEST_SERVICE, + option: placeholder, + } + with pytest.raises(ValueError, match=f"seamless-auth: {option} is a placeholder"): + Adapter(**settings) + + +def test_placeholder_secrets_are_allowed_in_insecure_local_mode() -> None: + Adapter( + auth_server_url="http://localhost:5312", + cookie_secret="dev-only-cookie-signing-key-replace-me", + service_secret="GRAB_FROM_SEAMLESS_AUTH_PORTAL_DEV", + insecure_cookies=True, + ) From 94e381d4f6a6165aa151a0d5a81bd676c4493f88 Mon Sep 17 00:00:00 2001 From: Brandon Corbett Date: Sat, 24 Oct 2026 21:57:25 -0400 Subject: [PATCH 3/5] feat: refuse recovery sessions in the guard and refresh after enrollment An access token from a recovery code or self-service recovery carries recovery: true, and the auth API refuses it on everything but enrolling a new factor. The guard now refuses it too, raising EnrollmentRequired (a subclass of Unauthenticated) that the bindings answer 403 enrollment_required, so an application does not treat a half-recovered account as signed in. RequireUser(allow_recovery=True), @require_auth(allow_recovery=True) and authenticate(allow_recovery=True) admit it, and User.recovery reports it. When a recovery session in cookie transport completes passkey registration or TOTP enrollment, the adapter refreshes in the same response so the access cookie drops the claim. A bearer client refreshes itself. Refs fells-code/seamless-auth-server#232 --- README.md | 24 ++++- src/seamless_auth/__init__.py | 3 +- src/seamless_auth/_adapter.py | 100 +++++++++++++++++++-- src/seamless_auth/django.py | 34 +++++-- src/seamless_auth/fastapi.py | 22 ++++- tests/test_django.py | 12 +++ tests/test_django_urls.py | 12 +++ tests/test_fastapi.py | 18 ++++ tests/test_recovery_sessions.py | 153 ++++++++++++++++++++++++++++++++ 9 files changed, 359 insertions(+), 19 deletions(-) create mode 100644 tests/test_recovery_sessions.py diff --git a/README.md b/README.md index 229791e..7b13c1d 100644 --- a/README.md +++ b/README.md @@ -58,7 +58,9 @@ def me(user: User = Depends(current_user)): The auth server reads the service token from `API_SERVICE_TOKEN` and your app reads it from `SEAMLESS_SERVICE_TOKEN`; both hold the same value. -`RequireUser` answers 401 unless the request carries a session, and returns the `User`. +`RequireUser` answers 401 unless the request carries a session, and returns the `User`. It +answers 403 for a recovery session (see [Recovery sessions](#recovery-sessions)) unless you pass +`RequireUser(auth, allow_recovery=True)`. ## Django @@ -87,6 +89,9 @@ def me(request): return JsonResponse({"id": user_of(request).id}) ``` +`@require_auth(allow_recovery=True)` admits a recovery session too (see +[Recovery sessions](#recovery-sessions)). + `SEAMLESS_AUTH` takes the keyword arguments of `Adapter` (below). `deliver` and `resolve_client_ip` may be dotted paths to callables. `require_auth` works on sync and async views. The auth routes are exempt from Django's CSRF middleware: the adapter does its own @@ -105,6 +110,23 @@ while cookies are `SameSite=None`, a state-changing request from another site (` cross-site`, or an `Origin` outside `allowed_origins`) answers 403. A bearer token is never attached by a browser, so it is not checked. +### Recovery sessions + +A user who signs in with a recovery code, or completes self-service account recovery, gets a +recovery session: its access token carries `recovery: true`, and the auth API refuses it on +everything but enrolling a new passkey or authenticator app until they do. The guard refuses it +too, with 403 `{ "error": "enrollment_required" }` (FastAPI's `RequireUser` answers `{ "detail": +"enrollment_required" }`, as it does its other errors), so an application never treats a +half-recovered account as signed in. Allow it on the routes that guide the user through +enrollment with `allow_recovery=True`, and read `User.recovery` there. +`Adapter.authenticate` raises `EnrollmentRequired`, a subclass of `Unauthenticated`. + +When a recovery session in cookie transport completes `POST /webauthn/register/finish` or +`POST /totp/enroll/verify`, the adapter refreshes the session in the same response, so the +access cookie drops the claim at once. If that refresh fails, the enrollment still answers as the +auth API did and the cookie keeps the claim until the next refresh. A bearer client holds its own +refresh token, so it calls `POST /auth/refresh` itself after enrolling. + ### Request bodies The auth routes forward a request body to the auth API as JSON, so a body must be sent as diff --git a/src/seamless_auth/__init__.py b/src/seamless_auth/__init__.py index cdcfe50..2b1d61a 100644 --- a/src/seamless_auth/__init__.py +++ b/src/seamless_auth/__init__.py @@ -10,7 +10,7 @@ :mod:`seamless_auth.django`. """ -from ._adapter import Adapter, CrossSiteRequest, Unauthenticated, User +from ._adapter import Adapter, CrossSiteRequest, EnrollmentRequired, Unauthenticated, User from ._client_ip import TrustedProxies from ._delivery import Delivery from ._http import AuthRequest, AuthResponse, Headers, ResponseCookie @@ -23,6 +23,7 @@ "AuthResponse", "CrossSiteRequest", "Delivery", + "EnrollmentRequired", "Headers", "Manifest", "ManifestError", diff --git a/src/seamless_auth/_adapter.py b/src/seamless_auth/_adapter.py index 36d4183..89e1ec3 100644 --- a/src/seamless_auth/_adapter.py +++ b/src/seamless_auth/_adapter.py @@ -70,6 +70,12 @@ # API asks for confirmation instead, so its absence is never a 401 here. _MAGIC_LINK_VERIFY = ("GET", "/magic-link/verify/{token}") +# Enrolling a factor from a recovery session lifts the API's restriction on the +# session, but the access token keeps its recovery claim until it is refreshed. +_ENROLLMENT_ROUTES = frozenset( + {("POST", "/webauthn/register/finish"), ("POST", "/totp/enroll/verify")} +) + _COOKIE_NAME = re.compile(r"^[!#$%&'*+\-.^_`|~0-9A-Za-z]+$") _COOKIE_DOMAIN = re.compile(r"^[A-Za-z0-9.\-]+$") _TOKEN = re.compile(r"^[!#$%&'*+\-.^_`|~0-9A-Za-z]+$") @@ -86,6 +92,9 @@ class User: roles: tuple[str, ...] = () email: str | None = None phone: str | None = None + #: A recovery session: signed in with a recovery code or self-service recovery, + #: and not yet enrolled in a new passkey or authenticator app. + recovery: bool = False #: The auth API access token, for calling the API on the user's behalf. token: str = field(default="", repr=False) @@ -94,6 +103,11 @@ class Unauthenticated(Exception): """A request carried no valid session.""" +class EnrollmentRequired(Unauthenticated): + """A recovery session, refused by a guard that does not allow one. Answer it + 403 ``enrollment_required``: the user is signed in only to enroll a factor.""" + + class CrossSiteRequest(Exception): """A state-changing request from another site, carrying a session cookie.""" @@ -564,9 +578,52 @@ def _manifest_route( ) return _error(502, "invalid_upstream_session") + if ( + not bearer + and (route.method, route.path) in _ENROLLMENT_ROUTES + and _is_recovery_token(_strip_bearer(authorization)) + ): + reply.set = self._refresh_after_enrollment(request.headers, forwarded, reply.set) + reply.body = data if bearer or data is None else _cookie_transport_body(route.pick, data) return reply + def _refresh_after_enrollment( + self, headers: Headers, forwarded: _Forwarded, issued: list[CookieWrite] + ) -> list[CookieWrite]: + """Rotates the session once a recovery session enrolled a factor, so the + access cookie loses the recovery claim now rather than at its next expiry. + Uses the refresh token this request already rotated to, if it did. A failure + leaves the cookies as they were: the enrollment itself succeeded.""" + refresh = next((c for c in issued if c.name == self._config.refresh_cookie_name), None) + if refresh is not None: + sub = str_claim(refresh.value, "sub") + refresh_token = str_claim(refresh.value, "refreshToken") + key = f"rotated:{refresh_token}" + else: + raw = raw_cookie(headers, self._config.refresh_cookie_name) + payload = read_cookie( + self._config, headers, self._config.refresh_cookie_name, KIND_REFRESH + ) + sub = str_claim(payload, "sub") + refresh_token = str_claim(payload, "refreshToken") + key = f"cookie:{raw}" + if not refresh_token: + return issued + service = self._sign_service(sub, {"refreshToken": refresh_token}) + outcome = self._refreshes.share( + key, lambda: self._rotate(forwarded, refresh_token, service) + ) + if outcome.session is None: + log.debug("[seamless-auth] Refresh after enrollment failed") + return issued + try: + session_id = self._verify_session(outcome.session, "access") + return session_cookies(self._config, outcome.session, session_id, True) + except (TokenError, ValueError) as err: + log.debug("[seamless-auth] Refresh after enrollment refused: %s", err) + return issued + def _issued_cookies( self, route: ManifestRoute, session: Claims, session_id: str ) -> list[CookieWrite]: @@ -683,34 +740,48 @@ def _logout_route( # The guard. - def authenticate(self, headers: Headers | Iterable[tuple[str, str]]) -> User: + def authenticate( + self, headers: Headers | Iterable[tuple[str, str]], *, allow_recovery: bool = False + ) -> User: """Verifies a request's session: the adapter's access cookie, or an auth API access token in ``Authorization: Bearer`` for clients with no cookie jar. The cookie wins when present. It does not refresh: silent refresh belongs to the auth routes, and a bearer client refreshes through - ``POST /auth/refresh`` itself. Raises :class:`Unauthenticated`.""" - return self._authenticate(_as_headers(headers))[0] + ``POST /auth/refresh`` itself. Raises :class:`Unauthenticated`, or its + subclass :class:`EnrollmentRequired` for a recovery session unless + ``allow_recovery`` is set.""" + return self._authenticate(_as_headers(headers), allow_recovery)[0] def authenticate_request( - self, method: str, headers: Headers | Iterable[tuple[str, str]] + self, + method: str, + headers: Headers | Iterable[tuple[str, str]], + *, + allow_recovery: bool = False, ) -> User: """:meth:`authenticate` for a guarded route. A session from the cookie also gets the auth routes' cross-site check, or a form on another site could act as the user: raises :class:`CrossSiteRequest` for one.""" headers = _as_headers(headers) - user, by_cookie = self._authenticate(headers) + user, by_cookie = self._authenticate(headers, allow_recovery) if by_cookie and self._check_origin(method, headers) is not None: raise CrossSiteRequest return user - def _authenticate(self, headers: Headers) -> tuple[User, bool]: + def _authenticate(self, headers: Headers, allow_recovery: bool) -> tuple[User, bool]: + user, by_cookie = self._session_of(headers) + if user.recovery and not allow_recovery: + raise EnrollmentRequired + return user, by_cookie + + def _session_of(self, headers: Headers) -> tuple[User, bool]: name = self._config.access_cookie_name if raw_cookie(headers, name): claims = read_cookie(self._config, headers, name, KIND_ACCESS) token = str_claim(claims, "token") if claims is None or not str_claim(claims, "sub") or not _is_access_token(token): raise Unauthenticated - return _user_from(claims, token), True + return _user_from(claims, token, _is_recovery_token(token)), True bearer = _bearer_token(headers) if bearer is None: @@ -725,7 +796,7 @@ def _authenticate(self, headers: Headers) -> tuple[User, bool]: # keeps it from passing for a session. if str_claim(claims, "typ") != "access" or not str_claim(claims, "sub"): raise Unauthenticated - return _user_from(claims, bearer), False + return _user_from(claims, bearer, claims.get("recovery") is True), False def _check_origin(self, method: str, headers: Headers) -> _Reply | None: """Blocks cross-site state changes while cookies are SameSite=None, which is @@ -856,17 +927,28 @@ def _is_access_token(token: str) -> bool: return bool(token) and str_claim(unverified_claims(token), "typ") == "access" +def _is_recovery_token(token: str) -> bool: + """Whether an auth API token, verified before it was stored or forwarded, is a + recovery session's. Only a literal ``true`` counts.""" + return bool(token) and (unverified_claims(token) or {}).get("recovery") is True + + +def _strip_bearer(authorization: str | None) -> str: + return authorization[7:] if authorization and authorization.startswith("Bearer ") else "" + + def _is_placeholder(secret: str) -> bool: return secret in PLACEHOLDER_SECRETS or PLACEHOLDER_MARKER in secret.lower() -def _user_from(claims: Claims, token: str) -> User: +def _user_from(claims: Claims, token: str, recovery: bool) -> User: roles = claims.get("roles") return User( id=str_claim(claims, "sub"), roles=tuple(r for r in roles if isinstance(r, str)) if isinstance(roles, list) else (), email=str_claim(claims, "email") or None, phone=str_claim(claims, "phone") or None, + recovery=recovery, token=token, ) diff --git a/src/seamless_auth/django.py b/src/seamless_auth/django.py index 3b49c5d..eb9c289 100644 --- a/src/seamless_auth/django.py +++ b/src/seamless_auth/django.py @@ -30,7 +30,7 @@ def me(request): import inspect import threading from collections.abc import Callable -from typing import Any, Literal, TypeVar, cast +from typing import Any, Literal, TypeVar, cast, overload from urllib.parse import quote, unquote from asgiref.sync import sync_to_async @@ -41,7 +41,14 @@ def me(request): from django.utils.module_loading import import_string from django.views.decorators.csrf import csrf_exempt -from ._adapter import MAX_BODY_BYTES, Adapter, CrossSiteRequest, Unauthenticated, User +from ._adapter import ( + MAX_BODY_BYTES, + Adapter, + CrossSiteRequest, + EnrollmentRequired, + Unauthenticated, + User, +) from ._http import AuthRequest, AuthResponse, Headers __all__ = [ @@ -111,15 +118,32 @@ def console_view(request: HttpRequest, path: str = "") -> HttpResponse: console_urlpatterns = [re_path(r"^(?P.*)$", console_view, name="seamless-console")] -def require_auth(view: F) -> F: +@overload +def require_auth(view: F) -> F: ... +@overload +def require_auth(*, allow_recovery: bool = False) -> Callable[[F], F]: ... + + +def require_auth(view: F | None = None, *, allow_recovery: bool = False) -> Any: """Answers 401 unless the request carries a valid session, and puts the :class:`~seamless_auth.User` on ``request.seamless_user``. A session from the cookie also gets the auth routes' cross-site check (403 on a state change from - another site). Works on sync and async views.""" + another site). A recovery session (a user who still has to enroll a new passkey + or authenticator app) answers 403 ``enrollment_required`` unless + ``@require_auth(allow_recovery=True)``. Works on sync and async views.""" + if view is None: + return lambda inner: _guard(inner, allow_recovery) + return _guard(view, allow_recovery) + +def _guard(view: F, allow_recovery: bool) -> F: def check(request: HttpRequest) -> HttpResponse | None: try: - user = get_adapter().authenticate_request(request.method or "GET", _headers(request)) + user = get_adapter().authenticate_request( + request.method or "GET", _headers(request), allow_recovery=allow_recovery + ) + except EnrollmentRequired: + return JsonResponse({"error": "enrollment_required"}, status=403) except CrossSiteRequest: return JsonResponse({"error": "cross_site_request_blocked"}, status=403) except Unauthenticated: diff --git a/src/seamless_auth/fastapi.py b/src/seamless_auth/fastapi.py index ac24b46..c9d5425 100644 --- a/src/seamless_auth/fastapi.py +++ b/src/seamless_auth/fastapi.py @@ -26,7 +26,14 @@ def me(user: User = Depends(current_user)): from starlette.concurrency import run_in_threadpool from starlette.responses import Response, StreamingResponse -from ._adapter import MAX_BODY_BYTES, Adapter, CrossSiteRequest, Unauthenticated, User +from ._adapter import ( + MAX_BODY_BYTES, + Adapter, + CrossSiteRequest, + EnrollmentRequired, + Unauthenticated, + User, +) from ._http import AuthRequest, AuthResponse, Headers __all__ = ["RequireUser", "auth_router", "console_router"] @@ -70,16 +77,25 @@ class RequireUser: returns its :class:`~seamless_auth.User`. A session from the cookie also gets the auth routes' cross-site check (403 on a state change from another site). + A recovery session (a user who still has to enroll a new passkey or + authenticator app) answers 403 ``enrollment_required`` unless + ``allow_recovery`` is set. + A plain function, so FastAPI runs it on its threadpool: the first sight of a key may fetch the auth API's key set. """ - def __init__(self, adapter: Adapter) -> None: + def __init__(self, adapter: Adapter, *, allow_recovery: bool = False) -> None: self._adapter = adapter + self._allow_recovery = allow_recovery def __call__(self, request: Request) -> User: try: - return self._adapter.authenticate_request(request.method, _headers(request)) + return self._adapter.authenticate_request( + request.method, _headers(request), allow_recovery=self._allow_recovery + ) + except EnrollmentRequired: + raise HTTPException(status_code=403, detail="enrollment_required") from None except CrossSiteRequest: raise HTTPException(status_code=403, detail="cross_site_request_blocked") from None except Unauthenticated: diff --git a/tests/test_django.py b/tests/test_django.py index 838458a..db3d8dc 100644 --- a/tests/test_django.py +++ b/tests/test_django.py @@ -134,6 +134,18 @@ def test_the_guard(client: Client) -> None: assert r.status_code == 200 +def test_the_guard_refuses_a_recovery_session_unless_allowed(client: Client) -> None: + recovery = f"Bearer {api_token('u1', 'access', recovery=True)}" + + for view in ("/api/me", "/api/me-async"): + r = client.get(view, HTTP_AUTHORIZATION=recovery) + assert (r.status_code, r.json()) == (403, {"error": "enrollment_required"}), view + r = client.get("/api/enroll", HTTP_AUTHORIZATION=recovery) + assert (r.status_code, r.json()) == (200, {"id": "u1", "recovery": True}) + assert client.get("/api/enroll-async", HTTP_AUTHORIZATION=recovery).status_code == 200 + assert client.get("/api/enroll").status_code == 401 + + def test_the_guard_on_an_async_view(client: Client) -> None: r = client.get("/api/me-async", HTTP_AUTHORIZATION=f"Bearer {api_token('u1', 'access')}") assert r.status_code == 200 and r.json() == {"id": "u1"} diff --git a/tests/test_django_urls.py b/tests/test_django_urls.py index 0f0cd43..e086563 100644 --- a/tests/test_django_urls.py +++ b/tests/test_django_urls.py @@ -17,6 +17,16 @@ async def me_async(request: HttpRequest) -> JsonResponse: return JsonResponse({"id": user_of(request).id}) +@require_auth(allow_recovery=True) +def enroll(request: HttpRequest) -> JsonResponse: + return JsonResponse({"id": user_of(request).id, "recovery": user_of(request).recovery}) + + +@require_auth(allow_recovery=True) +async def enroll_async(request: HttpRequest) -> JsonResponse: + return JsonResponse({"id": user_of(request).id}) + + @csrf_exempt @require_auth def transfer(request: HttpRequest) -> JsonResponse: @@ -26,6 +36,8 @@ def transfer(request: HttpRequest) -> JsonResponse: urlpatterns = [ path("api/me", me), path("api/me-async", me_async), + path("api/enroll", enroll), + path("api/enroll-async", enroll_async), path("api/transfer", transfer), path("auth/", include("seamless_auth.django")), path("console/", include(console_urlpatterns)), diff --git a/tests/test_fastapi.py b/tests/test_fastapi.py index f6bf7d6..370b49a 100644 --- a/tests/test_fastapi.py +++ b/tests/test_fastapi.py @@ -18,6 +18,12 @@ def app_for(adapter: Adapter) -> TestClient: def me(user: User = Depends(current_user)) -> dict[str, str]: # noqa: B008 return {"id": user.id} + @app.get("/api/enroll") + def enroll( + user: User = Depends(RequireUser(adapter, allow_recovery=True)), # noqa: B008 + ) -> dict[str, object]: + return {"id": user.id, "recovery": user.recovery} + @app.post("/api/transfer") def transfer(user: User = Depends(current_user)) -> dict[str, str]: # noqa: B008 return {"id": user.id} @@ -121,6 +127,18 @@ def test_the_guard(api: FakeApi) -> None: assert r.status_code == 200 +def test_the_guard_refuses_a_recovery_session_unless_allowed(api: FakeApi) -> None: + client = app_for(api.adapter()) + recovery = {"authorization": f"Bearer {api_token('u1', 'access', recovery=True)}"} + + r = client.get("/api/me", headers=recovery) + assert r.status_code == 403 + assert r.json() == {"detail": "enrollment_required"} + r = client.get("/api/enroll", headers=recovery) + assert (r.status_code, r.json()) == (200, {"id": "u1", "recovery": True}) + assert client.get("/api/enroll").status_code == 401 + + def test_an_async_deliver_runs_on_the_app_loop(api: FakeApi) -> None: api.on( "POST", diff --git a/tests/test_recovery_sessions.py b/tests/test_recovery_sessions.py new file mode 100644 index 0000000..2e6dd51 --- /dev/null +++ b/tests/test_recovery_sessions.py @@ -0,0 +1,153 @@ +"""Recovery sessions: the guard, and refreshing once a factor is enrolled.""" + +from __future__ import annotations + +from typing import Any + +import httpx2 +import pytest +from conftest import FakeApi, api_token, call, respond, session, signed_cookie + +from seamless_auth import EnrollmentRequired, Unauthenticated + +# Recovery sessions in the guard. + + +def recovery_cookie() -> tuple[str, str]: + token = api_token("u1", "access", sid="s-1", recovery=True) + return ("cookie", signed_cookie("seamless-access", {"sub": "u1", "token": token})) + + +def test_the_guard_refuses_a_recovery_session_from_the_cookie(api: FakeApi) -> None: + adapter = api.adapter() + + with pytest.raises(EnrollmentRequired): + adapter.authenticate([recovery_cookie()]) + with pytest.raises(Unauthenticated): + adapter.authenticate_request("GET", [recovery_cookie()]) + + user = adapter.authenticate([recovery_cookie()], allow_recovery=True) + assert (user.id, user.recovery) == ("u1", True) + + +def test_the_guard_refuses_a_recovery_session_from_a_bearer_token(api: FakeApi) -> None: + adapter = api.adapter() + headers = [("authorization", f"Bearer {api_token('u1', 'access', recovery=True)}")] + + with pytest.raises(EnrollmentRequired): + adapter.authenticate(headers) + assert adapter.authenticate_request("POST", headers, allow_recovery=True).recovery + + +@pytest.mark.parametrize("claim", [False, "true", 1, None]) +def test_only_a_literal_true_makes_a_recovery_session(api: FakeApi, claim: Any) -> None: + extra = {} if claim is None else {"recovery": claim} + token = api_token("u1", "access", **extra) + cookie = ("cookie", signed_cookie("seamless-access", {"sub": "u1", "token": token})) + adapter = api.adapter() + + assert adapter.authenticate([cookie]).recovery is False + assert adapter.authenticate([("authorization", f"Bearer {token}")]).recovery is False + + +# Refreshing after enrollment from a recovery session. + +ENROLLMENT = [ + ("POST", "/webauthn/register/finish"), + ("POST", "/totp/enroll/verify"), +] +REFRESH = signed_cookie("seamless-refresh", {"sub": "u1", "refreshToken": "r1"}) + + +@pytest.mark.parametrize(("method", "path"), ENROLLMENT) +def test_enrolling_from_a_recovery_session_refreshes_the_cookies( + api: FakeApi, method: str, path: str +) -> None: + api.on(method, path, respond(200, {"message": "Success"})) + api.on("POST", "/refresh", respond(200, session())) + + r = call(api.adapter(), method, path, body="{}", cookie=f"{recovery_cookie()[1]}; {REFRESH}") + + assert r.status == 200, r.text + assert r.body == {"message": "Success"} + assert "seamless-access" in r.cookies and "seamless-refresh" in r.cookies + refreshed = api.calls_to("POST", "/refresh") + assert len(refreshed) == 1 + assert refreshed[0].headers["authorization"] == "Bearer r1" + + +def test_enrolling_from_a_full_session_does_not_refresh(api: FakeApi) -> None: + api.on("POST", "/webauthn/register/finish", respond(200, {"message": "Success"})) + token = api_token("u1", "access", sid="s-1") + cookie = signed_cookie("seamless-access", {"sub": "u1", "token": token}) + + r = call( + api.adapter(), "POST", "/webauthn/register/finish", body="{}", cookie=f"{cookie}; {REFRESH}" + ) + + assert r.status == 200 + assert not r.cookies + assert not api.calls_to("POST", "/refresh") + + +def test_a_failed_enrollment_does_not_refresh(api: FakeApi) -> None: + api.on("POST", "/totp/enroll/verify", respond(400, {"error": "invalid_code"})) + + r = call( + api.adapter(), + "POST", + "/totp/enroll/verify", + body="{}", + cookie=f"{recovery_cookie()[1]}; {REFRESH}", + ) + + assert (r.status, r.body) == (400, {"error": "invalid_code"}) + assert not api.calls_to("POST", "/refresh") + + +def test_a_failed_refresh_after_enrollment_keeps_the_enrollment(api: FakeApi) -> None: + api.on("POST", "/webauthn/register/finish", respond(200, {"message": "Success"})) + api.on("POST", "/refresh", respond(401, {"error": "invalid_refresh_token"})) + + r = call( + api.adapter(), + "POST", + "/webauthn/register/finish", + body="{}", + cookie=f"{recovery_cookie()[1]}; {REFRESH}", + ) + + assert r.status == 200 + assert not r.cookies + + +def test_enrolling_after_a_silent_refresh_rotates_from_the_new_refresh_token( + api: FakeApi, +) -> None: + recovery_session = {**session(), "token": api_token("u1", "access", sid="s-1", recovery=True)} + answers = [recovery_session, {**session(), "refreshToken": "refresh-2"}] + api.on("POST", "/refresh", lambda _r: httpx2.Response(200, json=answers.pop(0))) + api.on("POST", "/webauthn/register/finish", respond(200, {"message": "Success"})) + + r = call(api.adapter(), "POST", "/webauthn/register/finish", body="{}", cookie=REFRESH) + + assert r.status == 200, r.text + refreshed = api.calls_to("POST", "/refresh") + assert [c.headers["authorization"] for c in refreshed] == ["Bearer r1", "Bearer refresh-u1"] + assert "seamless-access" in r.cookies and "seamless-refresh" in r.cookies + + +def test_a_bearer_client_refreshes_after_enrollment_itself(api: FakeApi) -> None: + api.on("POST", "/webauthn/register/finish", respond(200, {"message": "Success"})) + token = api_token("u1", "access", recovery=True) + + r = call( + api.adapter(), + "POST", + "/webauthn/register/finish", + body="{}", + headers={"x-seamless-auth-transport": "bearer", "authorization": f"Bearer {token}"}, + ) + + assert r.status == 200 + assert not api.calls_to("POST", "/refresh") From 9ebf903a4e33bf18513be465b11cb606b6296e1f Mon Sep 17 00:00:00 2001 From: Brandon Corbett Date: Sat, 24 Oct 2026 21:57:49 -0400 Subject: [PATCH 4/5] feat: deliver account recovery notices and serve the v0.20 recovery routes The auth API tells an account's owner when recovery opens, and hands the notices back to an adapter that owns delivery and names their kinds. - With deliver and the new deliver_account_notices=True, a manifest route marked notices (today POST /recovery/verify) asks for external delivery with x-seamless-auth-delivery-kinds: account_notice_email,account_notice_sms, delivers every item in the response's deliveries, and strips them before answering. Without the option the header is not sent and the API sends the notices itself, because it counts a returned notice as the owner being told. - Delivery gains notice, subject, text and cancel_url (hidden from repr, never logged), and purpose, which is account_recovery on a recovery code. - An unknown delivery kind is logged and skipped. A failed notice no longer stops the others, and a delivery error is logged by type only. - The manifest parser keeps the optional notices field, and the bundled manifest is synced from the API (0.19.4), which adds the recovery, recovery-code and system-config policy routes. 503 recovery_notice_failed and 403 step_up_required bodies pass through unchanged. Closes #13 --- README.md | 27 ++- src/seamless_auth/_adapter.py | 82 +++++-- src/seamless_auth/_delivery.py | 37 +++- src/seamless_auth/_manifest.py | 2 + src/seamless_auth/manifest.json | 29 ++- tests/test_recovery.py | 366 ++++++++++++++++++++++++++++++++ 6 files changed, 524 insertions(+), 19 deletions(-) create mode 100644 tests/test_recovery.py diff --git a/README.md b/README.md index 7b13c1d..2ac3821 100644 --- a/README.md +++ b/README.md @@ -151,6 +151,7 @@ JSON it would be a sign-in the user never made. The client SDKs already send JSO | `access_cookie_name`, `refresh_cookie_name` | `seamless-access`, `seamless-refresh` | Session cookie names | | `registration_cookie_name`, `pre_auth_cookie_name` | `seamless-ephemeral` | Sign-in flow cookie names | | `deliver` | none | Sends OTP codes and magic links through your own transports | +| `deliver_account_notices` | `False` | `deliver` also sends account notices to the owner (below) | | `resolve_client_ip` | the connecting peer | The end user's address. See below | | `disable_manifest_fetch` | `False` | Use only the manifest bundled with this version | | `http_client` | 15 second timeout, no redirects | Outbound `httpx2.Client` | @@ -191,7 +192,31 @@ auth = Adapter(..., deliver=deliver) ``` An `async def` callback works too; under FastAPI it runs on the application's event loop. A -delivery error answers the request with 502 `delivery_failed`. +delivery error answers the request with 502 `delivery_failed`. A delivery kind this version does +not know is logged and skipped. + +An OTP code from `POST /recovery/start` carries `purpose="account_recovery"` +(`Delivery.is_account_recovery`): word that message as an account recovery code, with a warning +to ignore it if the user did not ask, not as a sign-in code. + +#### Account notices + +When account recovery opens (`POST /recovery/verify`), the auth API tells the account's owner at +every verified address, with a link that cancels it. It sends those notices itself unless you +take them on as well: + +```python +auth = Adapter(..., deliver=deliver, deliver_account_notices=True) +``` + +Then `deliver` also receives `account_notice_email` (`to`, `subject`, `text`, `notice`, +`cancel_url`) and `account_notice_sms` (`to`, `text`, `notice`, `cancel_url`), one call per +address. The auth API counts a notice it hands back as the owner being told, so turn this on only +when `deliver` sends both kinds. `cancel_url` (and `text`, which contains it) is a credential that +cancels the recovery: send it only to `to`, and never log it. If any notice fails, every other one +is still attempted and the request answers 502 `delivery_failed`. Without the option, routes that +carry notices are called without external delivery, so the auth API sends them through its own +transports. ### Magic links diff --git a/src/seamless_auth/_adapter.py b/src/seamless_auth/_adapter.py index 89e1ec3..72d8bd5 100644 --- a/src/seamless_auth/_adapter.py +++ b/src/seamless_auth/_adapter.py @@ -29,7 +29,7 @@ session_cookies, set_cookie, ) -from ._delivery import Delivery, parse_delivery +from ._delivery import MESSAGE_KINDS, NOTICE_KINDS, Delivery, parse_delivery from ._http import AuthRequest, AuthResponse, Headers from ._jwks import BodyTooLarge, JwksCache, TokenError, read_limited from ._jwt import Claims, sign_hs256, str_claim, unverified_claims @@ -46,6 +46,7 @@ MAX_BODY_BYTES = 1 << 20 TRANSPORT_HEADER = "x-seamless-auth-transport" DELIVERY_MODE_HEADER = "x-seamless-auth-delivery-mode" +DELIVERY_KINDS_HEADER = "x-seamless-auth-delivery-kinds" # Example values from the starters and docs. Each is long enough to pass the length # check, and each is public. @@ -183,6 +184,7 @@ def __init__( registration_cookie_name: str = "seamless-ephemeral", pre_auth_cookie_name: str = "seamless-ephemeral", deliver: Deliver | None = None, + deliver_account_notices: bool = False, resolve_client_ip: ClientIpResolver | TrustedProxies | None = None, disable_manifest_fetch: bool = False, http_client: httpx2.Client | None = None, @@ -209,6 +211,8 @@ def __init__( f"seamless-auth: {option} is a placeholder value. Generate a real " "secret (it is allowed only with insecure_cookies, for local development)" ) + if deliver_account_notices and deliver is None: + raise ValueError("seamless-auth: deliver_account_notices needs a deliver callback") for name in ( access_cookie_name, refresh_cookie_name, @@ -259,6 +263,7 @@ def __init__( self._timeout: dict[str, Any] = {"timeout": UPSTREAM_TIMEOUT} if unbounded else {} _redact_upstream_urls(url) self._deliver = _sync_deliver(deliver) if deliver is not None else None + self._delivered_kinds = MESSAGE_KINDS | (NOTICE_KINDS if deliver_account_notices else set()) self._resolve_client_ip = resolve_client_ip self._jwks = JwksCache(url, self._client) self._manifest = ManifestSource(url, self._client, not disable_manifest_fetch) @@ -373,6 +378,7 @@ def _call( authorization: str | None = None, service: str | None = None, external_delivery: bool = False, + delivery_kinds: Iterable[str] = (), stream: bool = False, ) -> httpx2.Response: url = f"{self._config.auth_server_url}{path}" @@ -393,6 +399,9 @@ def _call( headers["x-seamless-client-user-agent"] = forwarded.user_agent if external_delivery: headers[DELIVERY_MODE_HEADER] = "external" + kinds = ",".join(delivery_kinds) + if kinds: + headers[DELIVERY_KINDS_HEADER] = kinds try: request = self._client.build_request( method, url, headers=headers, content=content, **self._timeout @@ -514,7 +523,11 @@ def _manifest_route( return credential authorization, cookies = credential - external = route.delivery and self._deliver is not None + # The API counts a notice it hands back as the owner being told, so it is + # asked for one only when the application has said it delivers both kinds. + # Otherwise the API sends the notice itself. + notices = route.notices and NOTICE_KINDS.issubset(self._delivered_kinds) + external = (route.delivery or notices) and self._deliver is not None try: res = self._call( forwarded, @@ -529,6 +542,7 @@ def _manifest_route( else self._proxy_authorization() ), external_delivery=external, + delivery_kinds=sorted(NOTICE_KINDS) if notices else (), stream=True, ) except _UpstreamError as err: @@ -545,21 +559,16 @@ def _manifest_route( failed.set = cookies return failed - if route.delivery and isinstance(data, dict): - # The message is the application's to send. It never goes back to the + if (route.delivery or route.notices) and isinstance(data, dict): + # Messages are the application's to send. They never go back to the # caller, in either transport. raw_delivery = data.pop("delivery", None) - if raw_delivery is not None and external and self._deliver is not None: - try: - self._deliver(parse_delivery(raw_delivery)) - except Exception as err: - log.warning( - "[seamless-auth] Delivery for %s %s failed: %s", - route.method, - route.path, - err, - ) - return _Reply(502, {"error": "delivery_failed"}, set=cookies) + raw_deliveries = data.pop("deliveries", None) + pending = [raw_delivery] if raw_delivery is not None and route.delivery else [] + if route.notices and isinstance(raw_deliveries, list): + pending.extend(raw_deliveries) + if pending and external and not self._send_deliveries(route, pending): + return _Reply(502, {"error": "delivery_failed"}, set=cookies) reply = _Reply(res.status_code, set=cookies) reply.clear = [self._config.cookie_name(held) for held in route.clears] @@ -588,6 +597,49 @@ def _manifest_route( reply.body = data if bearer or data is None else _cookie_transport_body(route.pick, data) return reply + def _send_deliveries(self, route: ManifestRoute, pending: list[Any]) -> bool: + """Hands each message to the application. Every one is attempted, so one + failed transport does not keep a notice from the account's other address. A + kind this adapter does not deliver is logged and skipped.""" + deliver = self._deliver + if deliver is None: + return True + ok = True + for raw in pending: + try: + delivery = parse_delivery(raw) + except ValueError as err: + log.warning( + "[seamless-auth] Unusable delivery from %s %s: %s", + route.method, + route.path, + err, + ) + ok = False + continue + if delivery.kind not in self._delivered_kinds: + log.warning( + "[seamless-auth] Skipping delivery kind %r from %s %s", + delivery.kind[:64], + route.method, + route.path, + ) + continue + try: + deliver(delivery) + except Exception as err: + # The type only: a message to the owner holds a cancel link, which an + # error from the transport may echo. + log.warning( + "[seamless-auth] Delivery of %s for %s %s failed: %s", + delivery.kind, + route.method, + route.path, + type(err).__name__, + ) + ok = False + return ok + def _refresh_after_enrollment( self, headers: Headers, forwarded: _Forwarded, issued: list[CookieWrite] ) -> list[CookieWrite]: diff --git a/src/seamless_auth/_delivery.py b/src/seamless_auth/_delivery.py index 4f7641e..ad9d38b 100644 --- a/src/seamless_auth/_delivery.py +++ b/src/seamless_auth/_delivery.py @@ -5,12 +5,22 @@ from dataclasses import dataclass, field from typing import Any +#: The kinds every ``deliver`` callback receives. +MESSAGE_KINDS = frozenset({"otp_email", "otp_sms", "magic_link_email", "enrollment_invite_email"}) +#: Owner notices, received only with ``deliver_account_notices=True``. +NOTICE_KINDS = frozenset({"account_notice_email", "account_notice_sms"}) + +#: The ``purpose`` of an OTP code that recovers an account rather than signs in. +PURPOSE_ACCOUNT_RECOVERY = "account_recovery" + @dataclass(frozen=True) class Delivery: - """An OTP code or magic link to send through the application's own transports.""" + """A message to send through the application's own transports: an OTP code, a + magic link, an enrollment invite, or an account notice to the account's owner.""" - #: ``otp_email``, ``otp_sms``, ``magic_link_email`` or ``enrollment_invite_email``. + #: ``otp_email``, ``otp_sms``, ``magic_link_email``, ``enrollment_invite_email``, + #: ``account_notice_email`` or ``account_notice_sms``. kind: str #: An email address or phone number. to: str @@ -18,6 +28,24 @@ class Delivery: token: str | None = field(default=None, repr=False) magic_link_url: str | None = field(default=None, repr=False) sign_in_url: str | None = None + #: ``account_recovery`` on an OTP code that recovers the account: word the + #: message as a recovery warning, not a sign-in code. + purpose: str | None = None + #: An account notice's name, such as ``account_recovery_opened``. + notice: str | None = None + #: An account notice email's subject. + subject: str | None = None + #: An account notice's message, written by the auth API. It can hold the cancel + #: link, so it is as sensitive as ``cancel_url``. + text: str | None = field(default=None, repr=False) + #: Cancels the account recovery the notice reports. It is a credential: send it + #: only to ``to`` and never log it. + cancel_url: str | None = field(default=None, repr=False) + + @property + def is_account_recovery(self) -> bool: + """An OTP code that recovers the account rather than signs in.""" + return self.purpose == PURPOSE_ACCOUNT_RECOVERY def parse_delivery(raw: Any) -> Delivery: @@ -41,4 +69,9 @@ def text(name: str) -> str | None: token=str(token) if token is not None else None, magic_link_url=text("magicLinkUrl"), sign_in_url=text("signInUrl"), + purpose=text("purpose"), + notice=text("notice"), + subject=text("subject"), + text=text("text"), + cancel_url=text("cancelUrl"), ) diff --git a/src/seamless_auth/_manifest.py b/src/seamless_auth/_manifest.py index bd181be..246a093 100644 --- a/src/seamless_auth/_manifest.py +++ b/src/seamless_auth/_manifest.py @@ -48,6 +48,7 @@ class ManifestRoute: clears: tuple[str, ...] = () pick: tuple[str, ...] | None = None delivery: bool = False + notices: bool = False def upstream_path(self, params: dict[str, str]) -> str: """The upstream path, with each ``{param}`` segment filled and escaped.""" @@ -161,6 +162,7 @@ def _parse_route(item: Any) -> ManifestRoute: clears=tuple(clears), pick=tuple(pick) if pick is not None else None, delivery=bool(item.get("delivery")), + notices=bool(item.get("notices")), ) diff --git a/src/seamless_auth/manifest.json b/src/seamless_auth/manifest.json index 936bc31..5eda92a 100644 --- a/src/seamless_auth/manifest.json +++ b/src/seamless_auth/manifest.json @@ -1,6 +1,6 @@ { "schemaVersion": 1, - "apiVersion": "0.18.0", + "apiVersion": "0.19.4", "session": { "subject": "sub", "token": "token", @@ -177,6 +177,26 @@ "credential": "registration", "issues": "session" }, + { "method": "GET", "path": "/recovery-codes", "credential": "access" }, + { "method": "POST", "path": "/recovery-codes", "credential": "access" }, + { + "method": "POST", + "path": "/recovery-codes/redeem", + "credential": "preAuth", + "issues": "session" + }, + { "method": "POST", "path": "/recovery/cancel", "credential": "none" }, + { + "method": "POST", + "path": "/recovery/complete", + "credential": "preAuth", + "issues": "session" + }, + { "method": "GET", "path": "/recovery/requests", "credential": "access" }, + { "method": "DELETE", "path": "/recovery/requests/{id}", "credential": "access" }, + { "method": "POST", "path": "/recovery/start", "credential": "preAuth", "delivery": true }, + { "method": "GET", "path": "/recovery/status", "credential": "preAuth" }, + { "method": "POST", "path": "/recovery/verify", "credential": "preAuth", "notices": true }, { "method": "POST", "path": "/refresh", "credential": "refresh", "issues": "session" }, { "method": "POST", "path": "/registration/phone", "credential": "access", "delivery": true }, { "method": "POST", "path": "/registration/phone/verify", "credential": "access" }, @@ -199,6 +219,13 @@ { "method": "POST", "path": "/system-config/oauth-providers", "credential": "access" }, { "method": "DELETE", "path": "/system-config/oauth-providers/{id}", "credential": "access" }, { "method": "PATCH", "path": "/system-config/oauth-providers/{id}", "credential": "access" }, + { "method": "GET", "path": "/system-config/policy", "credential": "access" }, + { "method": "DELETE", "path": "/system-config/policy/draft", "credential": "access" }, + { "method": "PUT", "path": "/system-config/policy/draft", "credential": "access" }, + { "method": "POST", "path": "/system-config/policy/dry-run", "credential": "access" }, + { "method": "POST", "path": "/system-config/policy/publish", "credential": "access" }, + { "method": "POST", "path": "/system-config/policy/rollback", "credential": "access" }, + { "method": "GET", "path": "/system-config/policy/versions", "credential": "access" }, { "method": "GET", "path": "/system-config/public", "credential": "none" }, { "method": "GET", "path": "/system-config/roles", "credential": "access" }, { "method": "POST", "path": "/totp/disable", "credential": "access" }, diff --git a/tests/test_recovery.py b/tests/test_recovery.py new file mode 100644 index 0000000..9259cca --- /dev/null +++ b/tests/test_recovery.py @@ -0,0 +1,366 @@ +"""Account recovery: owner notices and the v0.20 routes.""" + +from __future__ import annotations + +import json +import logging +from typing import Any + +import pytest +from conftest import ( + TEST_SECRET, + TEST_SERVICE, + FakeApi, + access, + api_token, + call, + pre_auth, + respond, + service_claims, + session, +) + +from seamless_auth import Adapter, Delivery, EnrollmentRequired, Manifest +from seamless_auth._adapter import DELIVERY_SUBJECT, PROXY_SUBJECT +from seamless_auth._manifest import bundled_manifest + +CANCEL_URL = "https://app.test/recovery/cancel#cancel-token-secret" +NOTICE_TEXT = f"Someone asked to recover your account. If this was not you, cancel it: {CANCEL_URL}" +NOTICES = [ + { + "kind": "account_notice_email", + "notice": "account_recovery_opened", + "to": "a@b.test", + "subject": "Account recovery requested", + "text": NOTICE_TEXT, + "cancelUrl": CANCEL_URL, + }, + { + "kind": "account_notice_sms", + "notice": "account_recovery_opened", + "to": "+15550100", + "text": NOTICE_TEXT, + "cancelUrl": CANCEL_URL, + }, +] +VERIFIED = { + "message": "Recovery requested", + "state": "pending", + "completesAt": "2026-10-25T00:00:00.000Z", +} +BEARER = {"x-seamless-auth-transport": "bearer", "authorization": "Bearer pre-auth"} + + +def capture() -> tuple[list[Delivery], Any]: + got: list[Delivery] = [] + return got, got.append + + +# Owner notices. + + +def test_notice_delivery_needs_a_callback() -> None: + with pytest.raises(ValueError, match="deliver_account_notices"): + Adapter( + auth_server_url="https://a", + cookie_secret=TEST_SECRET, + service_secret=TEST_SERVICE, + deliver_account_notices=True, + ) + + +def test_the_bundled_manifest_marks_recovery_verify_for_notices() -> None: + found = bundled_manifest().find("POST", "/recovery/verify") + assert found is not None and found.route.notices and not found.route.delivery + + +def test_a_manifest_carrying_notices_parses() -> None: + manifest = Manifest.parse( + json.dumps( + { + "schemaVersion": 1, + "apiVersion": "0.20.0", + "routes": [ + { + "method": "POST", + "path": "/recovery/verify", + "credential": "preAuth", + "notices": True, + }, + {"method": "POST", "path": "/recovery/start", "credential": "preAuth"}, + ], + } + ) + ) + assert [r.notices for r in manifest.routes] == [True, False] + + +@pytest.mark.parametrize("bearer", [False, True]) +def test_notices_are_delivered_and_stripped( + api: FakeApi, bearer: bool, caplog: pytest.LogCaptureFixture +) -> None: + api.on("POST", "/recovery/verify", respond(200, {**VERIFIED, "deliveries": NOTICES})) + got, deliver = capture() + adapter = api.adapter(deliver=deliver, deliver_account_notices=True) + + with caplog.at_level(logging.DEBUG): + r = call( + adapter, + "POST", + "/recovery/verify", + body='{"code":"123456"}', + cookie=None if bearer else pre_auth(), + headers=BEARER if bearer else None, + ) + + assert r.status == 200, r.text + assert r.body == VERIFIED + assert CANCEL_URL not in r.text + assert [(d.kind, d.to, d.notice) for d in got] == [ + ("account_notice_email", "a@b.test", "account_recovery_opened"), + ("account_notice_sms", "+15550100", "account_recovery_opened"), + ] + assert got[0].subject == "Account recovery requested" + assert all(d.cancel_url == CANCEL_URL and d.text == NOTICE_TEXT for d in got) + assert all(CANCEL_URL not in repr(d) for d in got) + assert "cancel-token-secret" not in caplog.text + + recorded = api.calls_to("POST", "/recovery/verify")[0] + assert recorded.headers["x-seamless-auth-delivery-mode"] == "external" + assert ( + recorded.headers["x-seamless-auth-delivery-kinds"] + == "account_notice_email,account_notice_sms" + ) + assert service_claims(recorded)["sub"] == DELIVERY_SUBJECT + + +@pytest.mark.parametrize("opted_in", [False, None]) +def test_without_notice_delivery_the_api_sends_the_notices( + api: FakeApi, opted_in: bool | None +) -> None: + api.on("POST", "/recovery/verify", respond(200, {**VERIFIED, "deliveries": NOTICES})) + got, deliver = capture() + options: dict[str, Any] = {} if opted_in is None else {"deliver": deliver} + + r = call( + api.adapter(**options), + "POST", + "/recovery/verify", + body='{"code":"123456"}', + cookie=pre_auth(), + ) + + assert r.status == 200 + assert r.body == VERIFIED + assert not got + recorded = api.calls_to("POST", "/recovery/verify")[0] + assert "x-seamless-auth-delivery-mode" not in recorded.headers + assert "x-seamless-auth-delivery-kinds" not in recorded.headers + assert service_claims(recorded)["sub"] == PROXY_SUBJECT + + +def test_a_failed_notice_is_reported_after_every_notice_is_tried( + api: FakeApi, caplog: pytest.LogCaptureFixture +) -> None: + api.on("POST", "/recovery/verify", respond(200, {**VERIFIED, "deliveries": NOTICES})) + tried: list[str] = [] + + def deliver(d: Delivery) -> None: + tried.append(d.kind) + if d.kind == "account_notice_email": + raise RuntimeError(f"smtp refused message containing {d.cancel_url}") + + adapter = api.adapter(deliver=deliver, deliver_account_notices=True) + with caplog.at_level(logging.DEBUG): + r = call(adapter, "POST", "/recovery/verify", body='{"code":"1"}', cookie=pre_auth()) + + assert (r.status, r.body) == (502, {"error": "delivery_failed"}) + assert tried == ["account_notice_email", "account_notice_sms"] + assert "cancel-token-secret" not in caplog.text + + +def test_recovery_notice_failed_passes_through(api: FakeApi) -> None: + failed = {"error": "recovery_notice_failed", "message": "The owner could not be notified"} + api.on("POST", "/recovery/verify", respond(503, failed)) + got, deliver = capture() + + r = call( + api.adapter(deliver=deliver, deliver_account_notices=True), + "POST", + "/recovery/verify", + body='{"code":"1"}', + cookie=pre_auth(), + ) + + assert (r.status, r.body) == (503, failed) + assert not got + + +def test_an_unknown_delivery_kind_is_a_logged_no_op( + api: FakeApi, caplog: pytest.LogCaptureFixture +) -> None: + api.on( + "POST", + "/recovery/verify", + respond( + 200, + {**VERIFIED, "deliveries": [{"kind": "carrier_pigeon", "to": "a@b.test"}, NOTICES[1]]}, + ), + ) + got, deliver = capture() + + with caplog.at_level(logging.WARNING): + r = call( + api.adapter(deliver=deliver, deliver_account_notices=True), + "POST", + "/recovery/verify", + body='{"code":"1"}', + cookie=pre_auth(), + ) + + assert r.status == 200 + assert [d.kind for d in got] == ["account_notice_sms"] + assert "carrier_pigeon" in caplog.text + + +def test_a_notice_on_a_delivery_route_needs_the_opt_in(api: FakeApi) -> None: + api.on( + "POST", + "/otp/generate-login-email-otp", + respond(200, {"message": "sent", "delivery": NOTICES[0]}), + ) + got, deliver = capture() + + r = call( + api.adapter(deliver=deliver), + "POST", + "/otp/generate-login-email-otp", + cookie=pre_auth(), + ) + + assert (r.status, r.body) == (200, {"message": "sent"}) + assert not got + + +def test_a_recovery_code_carries_its_purpose(api: FakeApi) -> None: + code = {"kind": "otp_email", "to": "a@b.test", "token": "654321", "purpose": "account_recovery"} + api.on("POST", "/recovery/start", respond(200, {"message": "sent", "delivery": code})) + got, deliver = capture() + + r = call( + api.adapter(deliver=deliver), + "POST", + "/recovery/start", + body='{"channel":"email"}', + cookie=pre_auth(), + ) + + assert (r.status, r.body) == (200, {"message": "sent"}) + assert got[0].is_account_recovery and got[0].purpose == "account_recovery" + assert got[0].token == "654321" + assert api.calls_to("POST", "/recovery/start")[0].headers["x-seamless-auth-delivery-mode"] == ( + "external" + ) + + +def test_a_sign_in_code_has_no_purpose(api: FakeApi) -> None: + code = {"kind": "otp_sms", "to": "+15550100", "token": 123456} + api.on( + "POST", "/otp/generate-login-phone-otp", respond(200, {"message": "sent", "delivery": code}) + ) + got, deliver = capture() + + call(api.adapter(deliver=deliver), "POST", "/otp/generate-login-phone-otp", cookie=pre_auth()) + + assert got[0].purpose is None and not got[0].is_account_recovery + + +# The v0.20 routes, from the bundled manifest. + +ACCESS_ROUTES = [ + ("GET", "/recovery-codes", "/recovery-codes"), + ("POST", "/recovery-codes", "/recovery-codes"), + ("GET", "/recovery/requests", "/recovery/requests"), + ("DELETE", "/recovery/requests/req-1", "/recovery/requests/req-1"), + ("GET", "/system-config/policy", "/system-config/policy"), + ("PUT", "/system-config/policy/draft", "/system-config/policy/draft"), + ("DELETE", "/system-config/policy/draft", "/system-config/policy/draft"), + ("POST", "/system-config/policy/publish", "/system-config/policy/publish"), + ("POST", "/system-config/policy/rollback", "/system-config/policy/rollback"), + ("GET", "/system-config/policy/versions", "/system-config/policy/versions"), + ("POST", "/system-config/policy/dry-run", "/system-config/policy/dry-run"), +] + + +@pytest.mark.parametrize(("method", "path", "upstream"), ACCESS_ROUTES) +def test_the_access_routes_pass_through( + api: FakeApi, method: str, path: str, upstream: str +) -> None: + api.on(method, upstream, respond(200, {"ok": True})) + body = "{}" if method in ("POST", "PUT") else None + + r = call(api.adapter(), method, path, body=body, cookie=access()) + + assert (r.status, r.body) == (200, {"ok": True}), r.text + assert api.calls_to(method, upstream)[0].headers["authorization"] == "Bearer t" + + +PRE_AUTH_ROUTES = [ + ("POST", "/recovery/start"), + ("POST", "/recovery/verify"), + ("GET", "/recovery/status"), + ("POST", "/recovery/complete"), +] + + +@pytest.mark.parametrize(("method", "path"), PRE_AUTH_ROUTES) +def test_the_recovery_routes_take_the_pre_auth_token(api: FakeApi, method: str, path: str) -> None: + api.on(method, path, respond(200, {"state": "pending"})) + body = "{}" if method == "POST" else None + + r = call(api.adapter(), method, path, body=body, cookie=pre_auth()) + + assert r.status == 200, r.text + assert api.calls_to(method, path)[0].headers["authorization"] == "Bearer pre-auth" + + +def test_recovery_complete_issues_a_recovery_session(api: FakeApi) -> None: + recovery = { + **session(), + "token": api_token("u1", "access", sid="s-1", recovery=True), + "recovery": True, + "nextStep": "enroll_passkey", + } + api.on("POST", "/recovery/complete", respond(200, recovery)) + adapter = api.adapter() + + r = call(adapter, "POST", "/recovery/complete", body="{}", cookie=pre_auth()) + + assert r.status == 200, r.text + assert r.body["recovery"] is True and r.body["nextStep"] == "enroll_passkey" + assert "token" not in r.body and "refreshToken" not in r.body + cookie = r.cookies["seamless-access"] + with pytest.raises(EnrollmentRequired): + adapter.authenticate([("cookie", f"seamless-access={cookie.value}")]) + + +def test_recovery_cancel_needs_no_session(api: FakeApi) -> None: + api.on("POST", "/recovery/cancel", respond(200, {"message": "ok"})) + + r = call(api.adapter(), "POST", "/recovery/cancel", body='{"cancelToken":"x"}') + + assert r.status == 200 + assert "authorization" not in api.calls_to("POST", "/recovery/cancel")[0].headers + + +def test_step_up_required_passes_through_on_admin_writes(api: FakeApi) -> None: + step_up = { + "error": "step_up_required", + "message": "Re-authenticate to continue", + "stepUp": {"required": True, "methods": ["passkey"]}, + } + api.on("POST", "/system-config/policy/publish", respond(403, step_up)) + + r = call(api.adapter(), "POST", "/system-config/policy/publish", body="{}", cookie=access()) + + assert (r.status, r.body) == (403, step_up) From 15d0fdf68e753b99ad2a26476bf85ca3311f9b0f Mon Sep 17 00:00:00 2001 From: Brandon Corbett Date: Sat, 24 Oct 2026 21:58:57 -0400 Subject: [PATCH 5/5] fix: cancel a recovery request when no owner notice is delivered The auth API opens a recovery request once it hands the notices back, and counts the owner as told. If the application delivers none of them, the owner has not been told, so the adapter now cancels the request through POST /recovery/cancel with the token from the notice's cancel link and answers 503 recovery_notice_failed. One delivered notice is enough to answer normally. The cancel link and its token are never logged. Refs #13 --- README.md | 10 ++-- src/seamless_auth/_adapter.py | 73 ++++++++++++++++++++++----- tests/test_recovery.py | 94 +++++++++++++++++++++++++++++++++-- 3 files changed, 156 insertions(+), 21 deletions(-) diff --git a/README.md b/README.md index 2ac3821..1f9c7d4 100644 --- a/README.md +++ b/README.md @@ -213,10 +213,12 @@ Then `deliver` also receives `account_notice_email` (`to`, `subject`, `text`, `n `cancel_url`) and `account_notice_sms` (`to`, `text`, `notice`, `cancel_url`), one call per address. The auth API counts a notice it hands back as the owner being told, so turn this on only when `deliver` sends both kinds. `cancel_url` (and `text`, which contains it) is a credential that -cancels the recovery: send it only to `to`, and never log it. If any notice fails, every other one -is still attempted and the request answers 502 `delivery_failed`. Without the option, routes that -carry notices are called without external delivery, so the auth API sends them through its own -transports. +cancels the recovery: send it only to `to`, and never log it. Every notice is attempted. One that +gets through is enough, and the request answers as the auth API did. If none does, the owner has +not been told, so the adapter cancels the recovery request (`POST /recovery/cancel` with the token +from the cancel link) and answers 503 `recovery_notice_failed`, as the auth API itself would. +Without the option, routes that carry notices are called without external delivery, so the auth +API sends them through its own transports. ### Magic links diff --git a/src/seamless_auth/_adapter.py b/src/seamless_auth/_adapter.py index 72d8bd5..4bf385e 100644 --- a/src/seamless_auth/_adapter.py +++ b/src/seamless_auth/_adapter.py @@ -564,11 +564,20 @@ def _manifest_route( # caller, in either transport. raw_delivery = data.pop("delivery", None) raw_deliveries = data.pop("deliveries", None) - pending = [raw_delivery] if raw_delivery is not None and route.delivery else [] - if route.notices and isinstance(raw_deliveries, list): - pending.extend(raw_deliveries) - if pending and external and not self._send_deliveries(route, pending): + if ( + route.delivery + and raw_delivery is not None + and external + and self._send_deliveries(route, [raw_delivery])[1] + ): return _Reply(502, {"error": "delivery_failed"}, set=cookies) + if notices and isinstance(raw_deliveries, list) and raw_deliveries: + delivered, _ = self._send_deliveries(route, raw_deliveries) + if not delivered: + # The API opened the request on the strength of these notices. + # Unseen by the owner, it must not be left to complete. + self._cancel_recovery(forwarded, raw_deliveries) + return _Reply(503, {"error": "recovery_notice_failed"}, set=cookies) reply = _Reply(res.status_code, set=cookies) reply.clear = [self._config.cookie_name(held) for held in route.clears] @@ -597,14 +606,16 @@ def _manifest_route( reply.body = data if bearer or data is None else _cookie_transport_body(route.pick, data) return reply - def _send_deliveries(self, route: ManifestRoute, pending: list[Any]) -> bool: - """Hands each message to the application. Every one is attempted, so one - failed transport does not keep a notice from the account's other address. A - kind this adapter does not deliver is logged and skipped.""" + def _send_deliveries(self, route: ManifestRoute, pending: list[Any]) -> tuple[int, bool]: + """Hands each message to the application, and reports how many were sent and + whether any failed. Every one is attempted, so one failed transport does not + keep a notice from the account's other address. A kind this adapter does not + deliver is logged and skipped.""" deliver = self._deliver if deliver is None: - return True - ok = True + return 0, False + sent = 0 + failed = False for raw in pending: try: delivery = parse_delivery(raw) @@ -615,7 +626,7 @@ def _send_deliveries(self, route: ManifestRoute, pending: list[Any]) -> bool: route.path, err, ) - ok = False + failed = True continue if delivery.kind not in self._delivered_kinds: log.warning( @@ -637,8 +648,36 @@ def _send_deliveries(self, route: ManifestRoute, pending: list[Any]) -> bool: route.path, type(err).__name__, ) - ok = False - return ok + failed = True + continue + sent += 1 + return sent, failed + + def _cancel_recovery(self, forwarded: _Forwarded, notices: list[Any]) -> None: + """Cancels the recovery request the notices report, with the token in the + fragment of their cancel link.""" + token = next(filter(None, (_cancel_token(raw) for raw in notices)), None) + if token is None: + log.warning("[seamless-auth] Could not cancel an unannounced recovery: no cancel token") + return + try: + res = self._call( + forwarded, + "POST", + "/recovery/cancel", + body=json.dumps({"cancelToken": token}).encode(), + service=self._proxy_authorization(), + stream=True, + ) + except _UpstreamError as err: + log.warning("[seamless-auth] Could not cancel an unannounced recovery: %s", err) + return + res.close() + if not 200 <= res.status_code < 300: + log.warning( + "[seamless-auth] Could not cancel an unannounced recovery: HTTP %s", + res.status_code, + ) def _refresh_after_enrollment( self, headers: Headers, forwarded: _Forwarded, issued: list[CookieWrite] @@ -985,6 +1024,14 @@ def _is_recovery_token(token: str) -> bool: return bool(token) and (unverified_claims(token) or {}).get("recovery") is True +def _cancel_token(raw: Any) -> str | None: + url = raw.get("cancelUrl") if isinstance(raw, dict) else None + if not isinstance(url, str): + return None + _, sep, fragment = url.partition("#") + return fragment if sep and fragment else None + + def _strip_bearer(authorization: str | None) -> str: return authorization[7:] if authorization and authorization.startswith("Bearer ") else "" diff --git a/tests/test_recovery.py b/tests/test_recovery.py index 9259cca..cd6428c 100644 --- a/tests/test_recovery.py +++ b/tests/test_recovery.py @@ -159,9 +159,7 @@ def test_without_notice_delivery_the_api_sends_the_notices( assert service_claims(recorded)["sub"] == PROXY_SUBJECT -def test_a_failed_notice_is_reported_after_every_notice_is_tried( - api: FakeApi, caplog: pytest.LogCaptureFixture -) -> None: +def test_one_delivered_notice_is_enough(api: FakeApi, caplog: pytest.LogCaptureFixture) -> None: api.on("POST", "/recovery/verify", respond(200, {**VERIFIED, "deliveries": NOTICES})) tried: list[str] = [] @@ -174,11 +172,99 @@ def deliver(d: Delivery) -> None: with caplog.at_level(logging.DEBUG): r = call(adapter, "POST", "/recovery/verify", body='{"code":"1"}', cookie=pre_auth()) - assert (r.status, r.body) == (502, {"error": "delivery_failed"}) + assert (r.status, r.body) == (200, VERIFIED) assert tried == ["account_notice_email", "account_notice_sms"] + assert not api.calls_to("POST", "/recovery/cancel") + assert "cancel-token-secret" not in caplog.text + + +@pytest.mark.parametrize("bearer", [False, True]) +def test_when_no_notice_is_delivered_the_request_is_cancelled( + api: FakeApi, bearer: bool, caplog: pytest.LogCaptureFixture +) -> None: + api.on("POST", "/recovery/verify", respond(200, {**VERIFIED, "deliveries": NOTICES})) + api.on("POST", "/recovery/cancel", respond(200, {"message": "ok"})) + tried: list[str] = [] + + def deliver(d: Delivery) -> None: + tried.append(d.kind) + raise RuntimeError(f"transport down for {d.cancel_url}") + + adapter = api.adapter(deliver=deliver, deliver_account_notices=True) + with caplog.at_level(logging.DEBUG): + r = call( + adapter, + "POST", + "/recovery/verify", + body='{"code":"1"}', + cookie=None if bearer else pre_auth(), + headers=BEARER if bearer else None, + ) + + assert (r.status, r.body) == (503, {"error": "recovery_notice_failed"}) + assert tried == ["account_notice_email", "account_notice_sms"] + cancelled = api.calls_to("POST", "/recovery/cancel") + assert len(cancelled) == 1 + assert json.loads(cancelled[0].body) == {"cancelToken": "cancel-token-secret"} + assert "authorization" not in cancelled[0].headers + assert service_claims(cancelled[0])["sub"] == PROXY_SUBJECT + assert "cancel-token-secret" not in caplog.text + + +def test_a_failed_cancel_still_answers_recovery_notice_failed( + api: FakeApi, caplog: pytest.LogCaptureFixture +) -> None: + api.on("POST", "/recovery/verify", respond(200, {**VERIFIED, "deliveries": NOTICES})) + api.on("POST", "/recovery/cancel", respond(500, {"error": "internal"})) + + def deliver(_d: Delivery) -> None: + raise RuntimeError("down") + + adapter = api.adapter(deliver=deliver, deliver_account_notices=True) + with caplog.at_level(logging.DEBUG): + r = call(adapter, "POST", "/recovery/verify", body='{"code":"1"}', cookie=pre_auth()) + + assert (r.status, r.body) == (503, {"error": "recovery_notice_failed"}) + assert "Could not cancel" in caplog.text assert "cancel-token-secret" not in caplog.text +def test_only_unknown_notice_kinds_count_as_none_delivered(api: FakeApi) -> None: + unknown = [{"kind": "account_notice_push", "to": "device-1", "cancelUrl": CANCEL_URL}] + api.on("POST", "/recovery/verify", respond(200, {**VERIFIED, "deliveries": unknown})) + api.on("POST", "/recovery/cancel", respond(200, {"message": "ok"})) + got, deliver = capture() + + r = call( + api.adapter(deliver=deliver, deliver_account_notices=True), + "POST", + "/recovery/verify", + body='{"code":"1"}', + cookie=pre_auth(), + ) + + assert (r.status, r.body) == (503, {"error": "recovery_notice_failed"}) + assert not got + assert len(api.calls_to("POST", "/recovery/cancel")) == 1 + + +def test_a_resumed_request_with_no_notices_answers_normally(api: FakeApi) -> None: + api.on("POST", "/recovery/verify", respond(200, VERIFIED)) + got, deliver = capture() + + r = call( + api.adapter(deliver=deliver, deliver_account_notices=True), + "POST", + "/recovery/verify", + body='{"code":"1"}', + cookie=pre_auth(), + ) + + assert (r.status, r.body) == (200, VERIFIED) + assert not got + assert not api.calls_to("POST", "/recovery/cancel") + + def test_recovery_notice_failed_passes_through(api: FakeApi) -> None: failed = {"error": "recovery_notice_failed", "message": "The owner could not be notified"} api.on("POST", "/recovery/verify", respond(503, failed))