diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7b57d93..96824e9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -84,6 +84,9 @@ jobs: - name: Conformance env: SEAMLESS_API_DIR: ${{ github.workspace }}/seamless-auth-api + # This adapter ships the content-type gate, so hold it to the pending + # contract (seamless-cli verify/harness/lib/pendingContracts.ts). + SEAMLESS_VERIFY_CONTRACTS: content-type-gate run: node seamless-cli/dist/index.js verify --adapter-url=http://localhost:8080 - name: Reference app log if: always() diff --git a/AGENTS.md b/AGENTS.md index ef21533..5112c35 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -67,8 +67,8 @@ current Django, so both are covered. Do not use language or standard library fea routes, credential resolution (with silent refresh), session verification, refresh, logout, the guard, the cross-site check. - `src/seamless_auth/_console.py`: the admin dashboard proxy (`Adapter.console`). -- `src/seamless_auth/_refresh.py`: refresh sharing (one result per refresh token for 5 seconds, - across threads). +- `src/seamless_auth/_refresh.py`: refresh sharing across threads (one rotation per refresh token + while it runs, and for cookie transport only, its result for 5 seconds after). - `src/seamless_auth/_manifest.py`: parsing, matching, the live and bundled manifest. - `src/seamless_auth/_cookies.py`, `_jwt.py`, `_jwks.py`: HS256 cookies and service tokens, RS256 against the API's JWKS. diff --git a/README.md b/README.md index c10c76d..c04b608 100644 --- a/README.md +++ b/README.md @@ -134,20 +134,27 @@ and audience, in date, `typ: access`, and the same `sub` as the cookie. `User.ro claims of its own even if the cookie secret leaks. The first request with a new key fetches the key set, and when it cannot be fetched the guard answers 401. -A request the guard admits on the session cookie also gets the auth routes' cross-site check, -while cookies are `SameSite=None` and only for methods other than `GET`, `HEAD` and `OPTIONS`: +A request the guard admits on the session cookie also gets the auth routes' cross-site check, for +methods other than `GET`, `HEAD` and `OPTIONS`, whatever the `same_site` setting (a sibling +subdomain is same-site, so `SameSite=Lax` or `Strict` still sends it the cookies): +- `Origin: null` (in any case, with any surrounding space) answers 403, checked first, whatever + `Sec-Fetch-Site` and `allowed_origins` say. - `Sec-Fetch-Site: cross-site` answers 403. -- `Sec-Fetch-Site: same-site` also covers a sibling subdomain, so when `allowed_origins` is set the - request must carry an `Origin` on that list, or it answers 403. Without `allowed_origins` it - passes. - `Sec-Fetch-Site: same-origin` (or `none`) passes. -- With no `Sec-Fetch-Site`, an `Origin: null` answers 403, and when `allowed_origins` is set any - other `Origin` must be on it. A request with neither header passes. - -A bearer token is never attached by a browser, so it is not checked. For the same reason the auth -routes skip the check on a bearer-transport request (`x-seamless-auth-transport: bearer`), which -reads no cookies. +- `Sec-Fetch-Site: same-site` also covers a sibling subdomain, and a request with no + `Sec-Fetch-Site` could come from anywhere, so for both the `Origin` decides. When + `allowed_origins` is set, the request must carry an `Origin` on that list, or it answers 403. Without `allowed_origins` it passes, and on the auth routes the body rule below + is what stops a form. +- A request with neither `Sec-Fetch-Site` nor `Origin` is not a browser form, so it passes, with + or without `allowed_origins`. On the auth routes the body rule below still applies to it. +- `allowed_origins=[]` is strict: no `Origin` is on it, so a same-site request, or one with an + `Origin` and no `Sec-Fetch-Site`, answers 403. + +A bearer token is never attached by a browser, so the guard does not check a request it admits on +one. The auth routes check every state change, bearer transport (`x-seamless-auth-transport: +bearer`) included, as the other adapters do. A native client sends neither `Sec-Fetch-Site` nor +`Origin`, so it passes. `User.organization_id` is the active organization, from the `org_id` claim of the verified access token, or `None` when the session has none. Like `User.roles`, it never comes from the cookie. @@ -200,7 +207,13 @@ refresh token, so it calls `POST /auth/refresh` itself after enrolling. The auth routes forward a request body to the auth API as JSON, so a body must be sent as `application/json` (or a `+json` type). Anything else answers 415 `unsupported_media_type`: a cross-site form can post a `text/plain` body shaped like JSON with no CORS preflight, and read as -JSON it would be a sign-in the user never made. The client SDKs already send JSON. +JSON it would be a sign-in the user never made. + +A state change on an auth route (any method but `GET`, `HEAD` and `OPTIONS`) needs that content +type even with no body, or else an `x-seamless-auth-transport` header (`cookie` or `bearer`). +Either one forces a CORS preflight, which a form on a sibling subdomain cannot send, so a body-less +form post answers 415 too. `POST /auth/refresh`, `DELETE /auth/logout` and +`DELETE /auth/logout/all` are exempt, though they still get the cross-site check. ## Options @@ -215,14 +228,14 @@ JSON it would be a sign-in the user never made. The client SDKs already send JSO | `cookie_domain` | none | Cookie `Domain` | | `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` | +| `allowed_origins` | none | The only origins allowed to change state from a same-site page or without fetch metadata; `[]` allows none | | `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` | +| `http_client` | 15 second timeout | Outbound `httpx2.Client`. Redirects are never followed, whatever it is set to | 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 @@ -494,12 +507,22 @@ def confirm(cnf, request): rs = ResourceServer(issuer=..., audience=..., confirm=confirm) ``` +The URL is built from the request, and its host from a `Host` header the client chooses. Under +FastAPI, pass the API's public origin so a proof's `htu` is checked against it instead: +`RequireBearer(rs, base_url="https://api.example.com")`. Without it, put Starlette's +`TrustedHostMiddleware` in front of the app, and behind a proxy trust its forwarded headers only +from that proxy. Under Django the URL comes from `build_absolute_uri()`, which `ALLOWED_HOSTS` +already guards. + ## How it runs The adapter is synchronous and thread-safe: one `Adapter` serves the whole process. Under FastAPI the bindings run it on Starlette's threadpool, so it never blocks the event loop. Under Django it -runs in the request thread, on WSGI or ASGI. Refresh sharing (one rotation per refresh token, -handed to every parallel request for 5 seconds) works across threads. +runs in the request thread, on WSGI or ASGI. Refresh sharing works across threads: requests that +present one refresh token while its rotation runs share that rotation, in either transport. For +cookie transport only, a request presenting the same refresh cookie within 5 seconds after gets +the same rotated session, so several tabs with one expired session all succeed. A bearer client +presenting a spent refresh token always reaches the auth API. ## The manifest diff --git a/pyproject.toml b/pyproject.toml index 61dce83..444b17f 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -34,7 +34,7 @@ dependencies = [ # maintained continuation of httpx (Pydantic), which Starlette also moved to. "httpx2>=2.13,<3", # HS256 cookies and service tokens, RS256 verification against the API's JWKS. - "PyJWT[crypto]>=2.10,<3", + "PyJWT[crypto]>=2.10.1,<3", ] [project.optional-dependencies] diff --git a/src/seamless_auth/_adapter.py b/src/seamless_auth/_adapter.py index 6ff8120..8108050 100644 --- a/src/seamless_auth/_adapter.py +++ b/src/seamless_auth/_adapter.py @@ -71,6 +71,7 @@ # recognises by that flow's pre-auth token. The token is optional: without it the # API asks for confirmation instead, so its absence is never a 401 here. _MAGIC_LINK_VERIFY = ("GET", "/magic-link/verify/{token}") +_SAFE_METHODS = ("GET", "HEAD", "OPTIONS") # 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. @@ -144,7 +145,7 @@ class Config: cookie_domain: str | None secure_cookies: bool same_site: str - allowed_origins: tuple[str, ...] + allowed_origins: tuple[str, ...] | None access_cookie_name: str registration_cookie_name: str refresh_cookie_name: str @@ -196,7 +197,7 @@ def __init__( cookie_domain: str | None = None, insecure_cookies: bool = False, same_site: str | None = None, - allowed_origins: Iterable[str] = (), + allowed_origins: Iterable[str] | None = None, access_cookie_name: str = "seamless-access", refresh_cookie_name: str = "seamless-refresh", # Shares the registration cookie on purpose: registration and sign-in never @@ -265,14 +266,15 @@ def __init__( cookie_domain=cookie_domain, secure_cookies=not insecure_cookies, same_site=same_site, - allowed_origins=tuple(allowed_origins), + allowed_origins=None if allowed_origins is None else tuple(allowed_origins), access_cookie_name=access_cookie_name, registration_cookie_name=registration_cookie_name, refresh_cookie_name=refresh_cookie_name, pre_auth_cookie_name=pre_auth_cookie_name, ) # A client that follows redirects would carry the service token to wherever - # the auth API pointed it. + # the auth API pointed it, so every request also says not to: a supplied + # client may have been built to follow them. self._client = http_client or httpx2.Client( timeout=UPSTREAM_TIMEOUT, follow_redirects=False ) @@ -327,12 +329,11 @@ def handle(self, request: AuthRequest) -> AuthResponse: def _handle(self, request: AuthRequest) -> AuthResponse: bearer = (request.headers.get(TRANSPORT_HEADER) or "").lower() == "bearer" - # Bearer transport reads no cookies, so a cross-site request has nothing of the - # user's to ride on. A custom header also needs a CORS preflight to be sent. - if not bearer: - blocked = self._check_origin(request.method, request.headers) - if blocked is not None: - return self._write(blocked, bearer=False) + # Both transports, as the other adapters: a native bearer client sends + # neither fetch metadata nor an Origin, so it passes. + blocked = self._check_origin(request.method, request.headers) + if blocked is not None: + return self._write(blocked, bearer=bearer) forwarded = self._forwarded(request) method = request.method.upper() @@ -429,7 +430,7 @@ def _call( request = self._client.build_request( method, url, headers=headers, content=content, **self._timeout ) - return self._client.send(request, stream=stream) + return self._client.send(request, stream=stream, follow_redirects=False) except (httpx2.HTTPError, httpx2.InvalidURL, UnicodeError, ValueError) as err: # The type only: the text of these holds the URL, and a path or query # can hold a magic-link token or an OAuth code. @@ -496,6 +497,13 @@ def _credential_for( return _error(401, f"Invalid or expired {name} cookie") return _error(401, f'Missing required cookie "{name}"') + def _unreadable_access_cookie(self, headers: Headers) -> bool: + name = self._config.access_cookie_name + return ( + raw_cookie(headers, name) is not None + and read_cookie(self._config, headers, name, cookie_kind("access")) is None + ) + def _optional_pre_auth(self, headers: Headers, bearer: bool) -> str | None: if bearer: token = _bearer_token(headers) @@ -532,8 +540,13 @@ def _manifest_route( body = request.body if request.body and request.body.strip() else None # The body goes upstream as JSON, so it has to be JSON. A cross-site form can # send a text/plain body shaped like JSON with no CORS preflight; read as - # JSON, that would be a sign-in the victim never made. - if body is not None and not _is_explicit_json(request.headers.get("content-type")): + # JSON, that would be a sign-in the victim never made. A state change with no + # body needs a JSON content type or the transport header all the same: either + # one forces a preflight, which a form on a sibling subdomain cannot pass. + if route.method not in _SAFE_METHODS and not ( + _is_explicit_json(request.headers.get("content-type")) + or (body is None and request.headers.get(TRANSPORT_HEADER) is not None) + ): return _error(415, "unsupported_media_type") authorization: str | None @@ -726,7 +739,7 @@ def _refresh_after_enrollment( return issued service = self._sign_service(sub, {"refreshToken": refresh_token}) outcome = self._refreshes.share( - key, lambda: self._rotate(forwarded, refresh_token, service) + key, lambda: self._rotate(forwarded, refresh_token, service), keep=True ) if outcome.session is None: log.debug("[seamless-auth] Refresh after enrollment failed") @@ -782,7 +795,7 @@ def _silent_refresh( service = self._sign_service(str_claim(payload, "sub"), {"refreshToken": refresh_token}) outcome = self._refreshes.share( - f"cookie:{raw}", lambda: self._rotate(forwarded, refresh_token, service) + f"cookie:{raw}", lambda: self._rotate(forwarded, refresh_token, service), keep=True ) if outcome.session is None: return outcome.error or f"refresh answered {outcome.status}" @@ -800,7 +813,7 @@ def _refresh_route(self, headers: Headers, forwarded: _Forwarded, bearer: bool) return _error(401, "refresh token required") service = self._proxy_authorization() outcome = self._refreshes.share( - f"bearer:{token}", lambda: self._rotate(forwarded, token, service) + f"bearer:{token}", lambda: self._rotate(forwarded, token, service), keep=False ) if outcome.session is None: if outcome.error is not None: @@ -835,8 +848,16 @@ def _logout_route( ] credential = self._credential_for(headers, forwarded, "access", bearer) if isinstance(credential, _Reply): - credential.clear = clears - return credential + if not bearer and self._unreadable_access_cookie(headers): + # Credential resolution never refreshes past an access cookie that + # fails verification, so nothing told the auth API. Its /logout takes + # an access token only, so the refresh cookie is spent for one. + refreshed = self._silent_refresh(headers, forwarded) + if isinstance(refreshed, tuple): + credential = (f"Bearer {str_claim(refreshed[0], 'token')}", []) + if isinstance(credential, _Reply): + credential.clear = clears + return credential try: res = self._call( forwarded, @@ -957,32 +978,30 @@ def _verified_access_token(self, token: str) -> Claims: return claims def _check_origin(self, method: str, headers: Headers) -> _Reply | None: - """Blocks cross-site state changes while cookies are SameSite=None, which is - when the browser would otherwise attach them to such a request.""" - if self._config.same_site != "None": - return None - if method.upper() in ("GET", "HEAD", "OPTIONS"): + """Blocks cross-site state changes, whatever the SameSite setting: a sibling + subdomain is same-site, so SameSite=Lax or Strict still sends it the cookies.""" + if method.upper() in _SAFE_METHODS: return None blocked = _error(403, "cross_site_request_blocked") + # An opaque origin (a sandboxed frame, a data: URL) is refused before + # anything else, whatever the fetch metadata says. origin = headers.get("origin") - site = headers.get("sec-fetch-site") - if site is not None: - site = site.lower() - if site == "cross-site": - return blocked - # same-site also covers a sibling subdomain, so an adopter who listed - # allowed origins gets the Origin held to that list. - if site != "same-site" or not self._config.allowed_origins: - return None - return None if origin is not None and self._is_allowed_origin(origin) else blocked - if origin is None: - return None - if origin == "null": + if origin is not None and origin.strip().lower() == "null": return blocked - if not self._config.allowed_origins: + site = (headers.get("sec-fetch-site") or "").lower() + if site == "cross-site": + return blocked + if site in ("same-origin", "none"): + return None + # same-site also covers a sibling subdomain, and a request with no fetch + # metadata could come from anywhere, so the Origin is what decides. With + # neither header it is no browser form, and the body gate still applies. + if origin is None and not site: + return None + if self._config.allowed_origins is None: return None - return None if self._is_allowed_origin(origin) else blocked + return None if origin is not None and self._is_allowed_origin(origin) else blocked def _is_allowed_origin(self, origin: str) -> bool: normalized = _normalize_origin(origin) @@ -990,7 +1009,8 @@ def _is_allowed_origin(self, origin: str) -> bool: if not normalized: return False return any( - _normalize_origin(allowed) == normalized for allowed in self._config.allowed_origins + _normalize_origin(allowed) == normalized + for allowed in self._config.allowed_origins or () ) diff --git a/src/seamless_auth/_console.py b/src/seamless_auth/_console.py index f5c88f0..3d9130b 100644 --- a/src/seamless_auth/_console.py +++ b/src/seamless_auth/_console.py @@ -107,7 +107,7 @@ def handle(self, method: str, subpath: str, query: str = "") -> AuthResponse: try: for _ in range(MAX_REDIRECTS + 1): request = client.build_request(method, url, timeout=UPSTREAM_TIMEOUT) - response = client.send(request, stream=True) + response = client.send(request, stream=True, follow_redirects=False) if not response.is_redirect: break follow = response.next_request diff --git a/src/seamless_auth/_jwks.py b/src/seamless_auth/_jwks.py index 18b3de7..a6851ea 100644 --- a/src/seamless_auth/_jwks.py +++ b/src/seamless_auth/_jwks.py @@ -41,6 +41,15 @@ class Jwk: key: RSAPublicKey params: dict[str, Any] + def check_rs256_signing(self) -> None: + """Refuses a key its JWK publishes for another use or algorithm.""" + if self.params.get("kty") != "RSA": + raise TokenError("not an RSA key") + if "use" in self.params and self.params["use"] != "sig": + raise TokenError("key is not for signatures") + if "alg" in self.params and self.params["alg"] != "RS256": + raise TokenError("key is for another algorithm") + class JwksCache: """The key set at ``url``, fetched on first use and kept fresh.""" @@ -52,33 +61,66 @@ def __init__(self, url: str, client: httpx2.Client) -> None: self._keys: dict[str, Jwk] | None = None self._fetched_at: float | None = None self._attempted_at: float | None = None + # Set while a fetch is in flight; waiters block on it, never on the lock. + self._fetching: threading.Event | None = None def key(self, kid: str) -> Jwk: - with self._lock: - now = time.monotonic() - fresh = self._fetched_at is not None and now - self._fetched_at < MAX_AGE - if fresh and self._keys is not None and kid in self._keys: - return self._keys[kid] - - backoff = REFETCH_INTERVAL if self._keys is not None else FAILED_FETCH_BACKOFF - if self._attempted_at is None or now - self._attempted_at >= backoff: - self._attempted_at = now - try: - self._keys = self._fetch() - self._fetched_at = time.monotonic() - except Exception as err: - # A stale key set keeps verifying while the API cannot be - # reached, rather than signing every user out. - if self._keys is None: - raise TokenError(f"jwks: {err}") from err - log.warning("[seamless-auth] Could not refresh the JWKS (%s).", err) + # The lock guards the cache only and is never held across the network: a + # slow JWKS endpoint must not queue verifications of keys already known. + while True: + with self._lock: + now = time.monotonic() + keys = self._keys + fresh = self._fetched_at is not None and now - self._fetched_at < MAX_AGE + if keys is not None and kid in keys and (fresh or self._fetching is not None): + return keys[kid] + in_flight = self._fetching + if in_flight is None: + backoff = REFETCH_INTERVAL if keys is not None else FAILED_FETCH_BACKOFF + if self._attempted_at is not None and now - self._attempted_at < backoff: + if keys is None or kid not in keys: + raise TokenError(f"unknown key id {kid!r}") + return keys[kid] + self._attempted_at = now + self._fetching = threading.Event() + if in_flight is not None: + # Single flight: wait for the fetch another caller started, then + # look again (within the backoff, so this cannot start a second). + in_flight.wait() + continue + return self._refresh(kid) - if self._keys is None or kid not in self._keys: - raise TokenError(f"unknown key id {kid!r}") - return self._keys[kid] + def _refresh(self, kid: str) -> Jwk: + fetched: dict[str, Jwk] | None = None + error: Exception | None = None + try: + fetched = self._fetch() + except Exception as err: + error = err + finally: + # Released whatever happens, or every waiter would block for good. + with self._lock: + if fetched is not None: + self._keys = fetched + self._fetched_at = time.monotonic() + done, self._fetching = self._fetching, None + keys = self._keys + if done is not None: + done.set() + if error is not None: + # A stale key set keeps verifying while the API cannot be reached, + # rather than signing every user out. + if keys is None: + raise TokenError(f"jwks: {error}") from error + log.warning("[seamless-auth] Could not refresh the JWKS (%s).", error) + if keys is None or kid not in keys: + raise TokenError(f"unknown key id {kid!r}") + return keys[kid] def _fetch(self) -> dict[str, Jwk]: - with self._client.stream("GET", self._url, timeout=FETCH_TIMEOUT) as res: + with self._client.stream( + "GET", self._url, timeout=FETCH_TIMEOUT, follow_redirects=False + ) as res: if res.status_code != 200: raise TokenError(f"HTTP {res.status_code}") body = read_limited(res, MAX_JWKS_BYTES) @@ -109,11 +151,12 @@ def verify(self, token: str, issuer: str, audience: str) -> Claims: if header.get("alg") != "RS256": raise TokenError("unexpected algorithm") kid = header.get("kid") - key = self.key(kid if isinstance(kid, str) else "").key + jwk = self.key(kid if isinstance(kid, str) else "") + jwk.check_rs256_signing() try: claims: Claims = jwt.decode( token, - key, + jwk.key, algorithms=["RS256"], issuer=issuer, audience=audience, diff --git a/src/seamless_auth/_manifest.py b/src/seamless_auth/_manifest.py index 246a093..d40533d 100644 --- a/src/seamless_auth/_manifest.py +++ b/src/seamless_auth/_manifest.py @@ -243,7 +243,9 @@ def get(self) -> Manifest: def _load(self) -> Manifest: headers = {"accept": "application/json"} - with self._client.stream("GET", self._url, headers=headers, timeout=FETCH_TIMEOUT) as res: + with self._client.stream( + "GET", self._url, headers=headers, timeout=FETCH_TIMEOUT, follow_redirects=False + ) as res: if res.status_code != 200: raise ManifestError(f"HTTP {res.status_code}") return Manifest.parse(read_limited(res, MAX_MANIFEST_BYTES)) diff --git a/src/seamless_auth/_refresh.py b/src/seamless_auth/_refresh.py index 7e4a25e..7a4b888 100644 --- a/src/seamless_auth/_refresh.py +++ b/src/seamless_auth/_refresh.py @@ -1,5 +1,5 @@ -"""Refresh sharing: one rotation per refresh token, handed to every caller -presenting it for a few seconds.""" +"""Refresh sharing: one rotation per refresh token at a time, and for cookie +transport, handed to later callers presenting it for a few seconds.""" from __future__ import annotations @@ -12,9 +12,10 @@ from ._jwt import Claims -# A refresh token presented again within this window gets the rotation it already +# A refresh cookie presented again within this window gets the rotation it already # got, so parallel requests from several tabs with one expired session all succeed -# instead of tripping the auth API's reuse detection. +# instead of tripping the auth API's reuse detection. Bearer callers never get it: a +# bearer client replaying a spent token goes to the API, which revokes the family. REUSE_WINDOW = 5.0 # How long a caller waits on another's rotation before giving up. FLIGHT_TIMEOUT = 30.0 @@ -43,10 +44,14 @@ def __init__(self) -> None: self._flights: dict[str, _Flight] = {} self._ids = itertools.count(1) - def share(self, key: str, rotate: Callable[[], RefreshOutcome]) -> RefreshOutcome: - """Runs ``rotate`` once per key at a time, and hands a successful outcome - to every caller with the same key for the reuse window afterwards. A - failure is not kept, so each later caller sees it from the auth API itself.""" + def share( + self, key: str, rotate: Callable[[], RefreshOutcome], *, keep: bool + ) -> RefreshOutcome: + """Runs ``rotate`` once per key at a time, handing its outcome to every + caller that arrives while it runs. With ``keep`` (cookie transport) a + successful outcome also goes to callers with the same key for the reuse + window afterwards. A failure is never kept, so each later caller sees it + from the auth API itself.""" with self._lock: now = time.monotonic() for stale in [ @@ -72,7 +77,7 @@ def share(self, key: str, rotate: Callable[[], RefreshOutcome]) -> RefreshOutcom finally: with self._lock: if self._flights.get(key) is flight: - if outcome.session is not None: + if keep and outcome.session is not None: flight.finished = time.monotonic() else: del self._flights[key] diff --git a/src/seamless_auth/_resource_server.py b/src/seamless_auth/_resource_server.py index 9918df2..933d5d7 100644 --- a/src/seamless_auth/_resource_server.py +++ b/src/seamless_auth/_resource_server.py @@ -181,7 +181,7 @@ def _keys(self) -> JwksCache: def _discover(self) -> str: url = self.issuer.rstrip("/") + DISCOVERY_PATH - with self._client.stream("GET", url, timeout=FETCH_TIMEOUT) as res: + with self._client.stream("GET", url, timeout=FETCH_TIMEOUT, follow_redirects=False) as res: if res.status_code != 200: raise TokenError(f"HTTP {res.status_code}") body = read_limited(res, MAX_DISCOVERY_BYTES) @@ -212,12 +212,7 @@ def _verify(self, token: str, request: BearerRequest | None) -> VerifiedUser: raise TokenError("critical header parameters are not supported") jwk = self._keys().key(kid) - if jwk.params.get("kty") != "RSA": - raise TokenError("not an RSA key") - if "use" in jwk.params and jwk.params["use"] != "sig": - raise TokenError("key is not for signatures") - if "alg" in jwk.params and jwk.params["alg"] != "RS256": - raise TokenError("key is for another algorithm") + jwk.check_rs256_signing() signing_input = f"{segments[0]}.{segments[1]}".encode("ascii") try: signed = _RS256.verify(signing_input, jwk.key, _b64decode(segments[2])) diff --git a/src/seamless_auth/fastapi.py b/src/seamless_auth/fastapi.py index 520e569..44cfbac 100644 --- a/src/seamless_auth/fastapi.py +++ b/src/seamless_auth/fastapi.py @@ -37,7 +37,7 @@ def orders(user: VerifiedUser = Depends(RequireBearer(rs, require_scope("orders: from __future__ import annotations from collections.abc import Iterable -from urllib.parse import quote +from urllib.parse import quote, urlsplit from fastapi import APIRouter, FastAPI, HTTPException, Request from starlette.background import BackgroundTask @@ -184,17 +184,42 @@ class RequireBearer: """A dependency that verifies the request's OpenID Provider access token, runs ``guards`` in order, and returns the :class:`~seamless_auth.VerifiedUser`. + ``base_url`` is this API's public origin, such as ``https://api.example.com``. + The ``confirm`` hook's URL (a DPoP proof's ``htu``) takes its scheme and host + from it rather than from the request, whose ``Host`` header the client chooses. + Without it the URL is Starlette's, so put ``TrustedHostMiddleware`` (and, behind + a proxy, trusted forwarded headers) in front of the app. + A plain function, so FastAPI runs it on its threadpool: the first request may fetch the provider's discovery document and key set. """ - def __init__(self, server: ResourceServer, *guards: Guard) -> None: + def __init__(self, server: ResourceServer, *guards: Guard, base_url: str | None = None) -> None: self._server = server self._guards = guards + self._origin: tuple[str, str] | None = None + if base_url is not None: + parts = urlsplit(base_url) if isinstance(base_url, str) else None + if ( + parts is None + or parts.scheme not in ("http", "https") + or not parts.netloc + or parts.path not in ("", "/") + or parts.query + or parts.fragment + ): + raise ValueError("seamless-auth: base_url must be an http(s) origin") + self._origin = (parts.scheme, parts.netloc) + + def _url(self, request: Request) -> str: + if self._origin is None: + return str(request.url) + scheme, netloc = self._origin + return str(request.url.replace(scheme=scheme, netloc=netloc)) def __call__(self, request: Request) -> VerifiedUser: values = request.headers.getlist("authorization") - bearer = BearerRequest(request.method, _headers(request), lambda: str(request.url)) + bearer = BearerRequest(request.method, _headers(request), lambda: self._url(request)) try: # More than one Authorization header is not one token. user = self._server.authenticate(values[0] if len(values) == 1 else None, bearer) diff --git a/tests/conftest.py b/tests/conftest.py index f371fed..b7e08ef 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -184,13 +184,16 @@ def call( headers: dict[str, str] | None = None, peer: str | None = None, ) -> Reply: - """Sends a request to the adapter's auth routes. A body goes as JSON, as the - client SDKs send it, unless ``content_type`` says otherwise.""" + """Sends a request to the adapter's auth routes. A body, or a state change with + none, goes as JSON, as the client SDKs send it, unless ``content_type`` or + ``headers`` say otherwise.""" path, _, query = path.partition("?") items = list((headers or {}).items()) if cookie: items.append(("cookie", cookie)) - if body is not None and content_type: + unsafe = method.upper() not in ("GET", "HEAD", "OPTIONS") + named = any(k.lower() == "content-type" for k, _ in items) + if (body is not None or unsafe) and content_type and not named: items.append(("content-type", content_type)) res = adapter.handle( AuthRequest( diff --git a/tests/test_adapter.py b/tests/test_adapter.py index abfe8b8..d35d1ad 100644 --- a/tests/test_adapter.py +++ b/tests/test_adapter.py @@ -221,19 +221,118 @@ def test_post_refresh_rotates_both_cookies_and_returns_no_tokens(api: FakeApi) - assert "seamless-access" in r.cookies and "seamless-refresh" in r.cookies -def test_bearer_refresh_is_shared_for_one_refresh_token(api: FakeApi) -> None: +BEARER_REFRESH = {"x-seamless-auth-transport": "bearer", "authorization": "Bearer r1"} + + +def _start(n: int, run: Any) -> tuple[list[threading.Thread], list[Any]]: + results: list[Any] = [None] * n + + def one(i: int) -> None: + results[i] = run() + + threads = [threading.Thread(target=one, args=(i,)) for i in range(n)] + for t in threads: + t.start() + return threads, results + + +def _gated_refresh(api: FakeApi) -> tuple[threading.Event, threading.Event]: + started = threading.Event() + gate = threading.Event() + + def refresh(_req: httpx2.Request) -> httpx2.Response: + started.set() + gate.wait(2) + return httpx2.Response(200, json=session()) + + api.on("POST", "/refresh", refresh) + return started, gate + + +def test_bearer_refresh_in_flight_is_shared_for_one_refresh_token(api: FakeApi) -> None: + started, gate = _gated_refresh(api) + adapter = api.adapter() + + first: list[Any] = [] + leader = threading.Thread( + target=lambda: first.append(call(adapter, "POST", "/refresh", headers=BEARER_REFRESH)) + ) + leader.start() + assert started.wait(2) + threads, results = _start(2, lambda: call(adapter, "POST", "/refresh", headers=BEARER_REFRESH)) + gate.set() + leader.join() + for t in threads: + t.join() + + assert first[0].status == 200 + assert first[0].body["refreshToken"] == "refresh-u1" + for r in results: + assert r.body == first[0].body + assert len(api.calls_to("POST", "/refresh")) == 1 + assert not first[0].cookies + + +def test_a_bearer_replay_after_its_rotation_finished_reaches_the_api(api: FakeApi) -> None: api.on("POST", "/refresh", respond(200, session())) adapter = api.adapter() - headers = {"x-seamless-auth-transport": "bearer", "authorization": "Bearer r1"} - first = call(adapter, "POST", "/refresh", headers=headers) - second = call(adapter, "POST", "/refresh", headers=headers) + first = call(adapter, "POST", "/refresh", headers=BEARER_REFRESH) + api.on("POST", "/refresh", respond(401, {"error": "refresh_token_reused"})) + replay = call(adapter, "POST", "/refresh", headers=BEARER_REFRESH) assert first.status == 200 - assert first.body["refreshToken"] == "refresh-u1" - assert second.body == first.body + assert replay.status == 401 + assert replay.body == {"error": "refresh_token_reused"} + assert len(api.calls_to("POST", "/refresh")) == 2 + assert not adapter._refreshes._flights + + +def test_a_cookie_replay_inside_the_window_gets_the_same_rotation(api: FakeApi) -> None: + api.on("POST", "/refresh", respond(200, session())) + adapter = api.adapter() + + first = call(adapter, "POST", "/refresh", cookie=REFRESH) + replay = call(adapter, "POST", "/refresh", cookie=REFRESH) + restored = call(adapter, "GET", "/users/me", cookie=REFRESH) + + assert first.status == 200 and replay.status == 200 + for name in ("seamless-access", "seamless-refresh"): + assert replay.cookies[name].value == first.cookies[name].value, name + assert restored.cookies[name].value == first.cookies[name].value, name assert len(api.calls_to("POST", "/refresh")) == 1 - assert not first.cookies + + +def test_a_cookie_replay_after_the_window_reaches_the_api( + api: FakeApi, monkeypatch: pytest.MonkeyPatch +) -> None: + clock = [1000.0] + monkeypatch.setattr("seamless_auth._refresh.time.monotonic", lambda: clock[0]) + api.on("POST", "/refresh", respond(200, session())) + adapter = api.adapter() + + first = call(adapter, "POST", "/refresh", cookie=REFRESH) + clock[0] += 5.5 + api.on("POST", "/refresh", respond(401, {"error": "refresh_token_reused"})) + replay = call(adapter, "POST", "/refresh", cookie=REFRESH) + + assert first.status == 200 + assert replay.status == 401 + assert replay.cleared("seamless-access") and replay.cleared("seamless-refresh") + assert len(api.calls_to("POST", "/refresh")) == 2 + assert not adapter._refreshes._flights + + +def test_a_bearer_replay_inside_the_cookie_window_still_reaches_the_api(api: FakeApi) -> None: + api.on("POST", "/refresh", respond(200, session())) + adapter = api.adapter() + + cookie = call(adapter, "POST", "/refresh", cookie=REFRESH) + first = call(adapter, "POST", "/refresh", headers=BEARER_REFRESH) + replay = call(adapter, "POST", "/refresh", headers=BEARER_REFRESH) + + assert cookie.status == 200 and first.status == 200 and replay.status == 200 + assert len(api.calls_to("POST", "/refresh")) == 3 def test_bearer_refresh_without_a_token_is_401(api: FakeApi) -> None: @@ -531,38 +630,67 @@ def test_cross_site_state_changes_are_blocked(api: FakeApi) -> None: assert r.status == 400 -def test_bearer_transport_skips_the_origin_check(api: FakeApi) -> None: +def test_a_native_bearer_client_passes_the_origin_check(api: FakeApi) -> None: api.on("POST", "/login", respond(400, {"error": "invalid_request"})) adapter = api.adapter(allowed_origins=["https://app.example"]) - for headers in ({"sec-fetch-site": "cross-site"}, {"origin": "null"}): - r = call( - adapter, - "POST", - "/login", - body="{}", - headers={"x-seamless-auth-transport": "bearer", **headers}, - ) - assert r.status == 400, r.text - r = call(adapter, "POST", "/login", body="{}", headers=headers) - assert (r.status, r.body) == (403, {"error": "cross_site_request_blocked"}) + # A native client sends neither fetch metadata nor an Origin. + r = call(adapter, "POST", "/login", body="{}", headers={"x-seamless-auth-transport": "bearer"}) + assert r.status == 400, r.text + r = call( + adapter, + "POST", + "/login", + body="{}", + headers={"x-seamless-auth-transport": "bearer", "origin": "https://evil.example"}, + ) + assert (r.status, r.body) == (403, {"error": "cross_site_request_blocked"}) + + +@pytest.mark.parametrize("headers", [{"sec-fetch-site": "cross-site"}, {"origin": "null"}]) +def test_bearer_transport_gets_the_origin_check(api: FakeApi, headers: dict[str, str]) -> None: + api.on("POST", "/recovery-codes", respond(200, {"ok": True})) + adapter = api.adapter() + + r = call( + adapter, + "POST", + "/recovery-codes", + content_type=None, + headers={"x-seamless-auth-transport": "bearer", "authorization": "Bearer t", **headers}, + ) + + assert (r.status, r.body) == (403, {"error": "cross_site_request_blocked"}), r.text + assert not r.cookies, "a bearer reply writes no cookies" + assert not api.calls_to("POST", "/recovery-codes") APP = "https://app.example" +SIBLING = "https://evil.app.example" ORIGIN_CASES = [ # (allowlist, same_site, method, sec-fetch-site, origin, blocked) - ((), "None", "POST", "cross-site", None, True), - ((), "None", "POST", "same-origin", None, False), - ((), "None", "POST", "same-site", None, False), - ((), "None", "POST", "same-site", "https://evil.example", False), - ((), "None", "POST", None, None, False), - ((), "None", "POST", None, "null", True), - ((), "None", "POST", None, "https://evil.example", False), - ((), "None", "GET", "cross-site", None, False), - ((), "None", "OPTIONS", "cross-site", None, False), - ((), "Lax", "POST", "cross-site", None, False), - ((APP,), "Lax", "POST", "same-site", "https://evil.example", False), + (None, "None", "POST", "cross-site", None, True), + (None, "None", "POST", "same-origin", None, False), + # Without an allowlist the body gate is what stops a sibling's form. + (None, "None", "POST", "same-site", None, False), + (None, "None", "POST", "same-site", "https://evil.example", False), + (None, "None", "POST", "same-site", "null", True), + (None, "None", "POST", None, None, False), + (None, "None", "POST", None, "null", True), + (None, "None", "POST", None, "https://evil.example", False), + (None, "None", "GET", "cross-site", None, False), + (None, "None", "OPTIONS", "cross-site", None, False), + # Whatever the SameSite setting. + (None, "Lax", "POST", "cross-site", None, True), + (None, "Strict", "POST", "cross-site", None, True), + (None, "Lax", "POST", None, "null", True), + ((APP,), "Lax", "POST", "same-site", "https://evil.example", True), + ((APP,), "Lax", "POST", None, "https://evil.example", True), + ((APP,), "Strict", "POST", "same-site", APP, False), ((APP,), "None", "POST", None, APP, False), ((APP,), "None", "POST", None, "https://evil.example", True), + # Neither header is no browser form, so it passes; the body gate still applies. + ((APP,), "None", "POST", None, None, False), + ((APP,), "Lax", "POST", None, None, False), # A sibling subdomain is same-site, so the header alone cannot tell it from the app. ((APP,), "None", "POST", "same-site", "https://evil.example", True), ((APP,), "None", "POST", "same-site", None, True), @@ -575,6 +703,21 @@ def test_bearer_transport_skips_the_origin_check(api: FakeApi) -> None: ((APP,), "None", "POST", "cross-site", APP, True), ((APP,), "None", "POST", "Cross-Site", APP, True), ((APP,), "None", "GET", "same-site", "https://evil.example", False), + # A configured empty allowlist is strict: only same-origin requests pass. + ((), "None", "POST", "same-site", APP, True), + ((), "Lax", "POST", "same-site", APP, True), + ((), "None", "POST", None, APP, True), + ((), "None", "POST", None, None, False), + ((), "None", "POST", "same-origin", None, False), + ((), "None", "GET", "same-site", APP, False), + # Origin: null is refused first, whatever the fetch metadata or allowlist says. + (None, "None", "POST", "same-origin", "null", True), + (None, "None", "POST", "none", "null", True), + ((APP,), "None", "POST", "same-origin", "null", True), + (None, "Lax", "POST", "same-origin", " NULL ", True), + (None, "None", "POST", None, "Null", True), + (None, "None", "POST", "same-site", " null", True), + (None, "None", "GET", "same-origin", "null", False), # Neither normalizes to an origin, so neither can match the other. (("not an origin",), "None", "POST", "same-site", "also not one", True), (("not an origin",), "None", "POST", None, "also not one", True), @@ -582,7 +725,7 @@ def test_bearer_transport_skips_the_origin_check(api: FakeApi) -> None: ] -OriginCase = tuple[tuple[str, ...], str, str, str | None, str | None, bool] +OriginCase = tuple[tuple[str, ...] | None, str, str, str | None, str | None, bool] @pytest.mark.parametrize("case", ORIGIN_CASES) @@ -601,6 +744,115 @@ def test_the_origin_check(api: FakeApi, case: OriginCase) -> None: assert r.status != 403, r.text +# Cookie-authenticated routes a forged form POST can drive with no body at all. +FORM_POST_TARGETS = ["/recovery-codes", "/totp/enroll/start", "/admin/applications/app-1/secret"] + + +@pytest.mark.parametrize("path", FORM_POST_TARGETS) +@pytest.mark.parametrize("site", ["same-site", None]) +@pytest.mark.parametrize("same_site", ["Lax", "None"]) +def test_a_body_less_form_post_does_not_reach_a_cookie_route( + api: FakeApi, path: str, site: str | None, same_site: str +) -> None: + api.on("POST", path, respond(200, {"ok": True})) + adapter = api.adapter(same_site=same_site) + # What a sibling subdomain's
sends: no body, a form + # content type, and either same-site fetch metadata or none at all. + headers = {"content-type": "application/x-www-form-urlencoded", "origin": SIBLING} + if site is not None: + headers["sec-fetch-site"] = site + + r = call(adapter, "POST", path, cookie=access(), headers=headers) + + assert r.status in (403, 415), r.text + assert not api.calls_to("POST", path) + + +@pytest.mark.parametrize("same_site", ["Lax", "Strict", "None"]) +@pytest.mark.parametrize( + ("method", "path"), [("POST", "/refresh"), ("DELETE", "/logout"), ("DELETE", "/logout/all")] +) +def test_refresh_and_logout_get_the_origin_check_and_keep_the_cookies( + api: FakeApi, same_site: str, method: str, path: str +) -> None: + api.on("POST", "/refresh", respond(200, session())) + api.on("DELETE", "/logout", respond(204, None)) + api.on("DELETE", "/logout/all", respond(204, None)) + adapter = api.adapter(allowed_origins=[APP], same_site=same_site) + for headers in ( + {"sec-fetch-site": "cross-site"}, + {"sec-fetch-site": "same-site", "origin": SIBLING}, + {"origin": SIBLING}, + ): + r = call(adapter, method, path, content_type=None, cookie=REFRESH, headers=headers) + assert (r.status, r.body) == (403, {"error": "cross_site_request_blocked"}), headers + assert not r.cookies, "a refused request must not clear the session" + assert not api.calls_to(method, path) + + # Exempt from the content-type gate: the client sends these with no body. With + # neither fetch metadata nor an Origin it passes the origin check too. + r = call(adapter, method, path, content_type=None, cookie=REFRESH) + assert r.status not in (403, 415), r.text + + +def test_a_state_change_with_no_body_needs_json_or_the_transport_header(api: FakeApi) -> None: + api.on("POST", "/recovery-codes", respond(200, {"ok": True})) + api.on("GET", "/recovery-codes", respond(200, {"ok": True})) + adapter = api.adapter() + + for content_type in (None, "text/plain", "application/x-www-form-urlencoded"): + r = call(adapter, "POST", "/recovery-codes", content_type=content_type, cookie=access()) + assert (r.status, r.body) == (415, {"error": "unsupported_media_type"}), content_type + assert not api.calls_to("POST", "/recovery-codes") + # Passing the origin check with neither header does not skip the body gate. + strict = api.adapter(allowed_origins=[APP]) + r = call(strict, "POST", "/recovery-codes", content_type=None, cookie=access()) + assert r.status == 415 + assert not api.calls_to("POST", "/recovery-codes") + + assert call(adapter, "POST", "/recovery-codes", cookie=access()).status == 200 + for transport in ("cookie", "bearer"): + r = call( + adapter, + "POST", + "/recovery-codes", + content_type=None, + cookie=access(), + headers={"x-seamless-auth-transport": transport, "authorization": "Bearer t"}, + ) + assert r.status == 200, (transport, r.text) + # The header stands in for a content type only when there is no body to read. + r = call( + adapter, + "POST", + "/recovery-codes", + body="{}", + content_type="text/plain", + cookie=access(), + headers={"x-seamless-auth-transport": "cookie"}, + ) + assert r.status == 415 + r = call(adapter, "GET", "/recovery-codes", content_type=None, cookie=access()) + assert r.status == 200 + + +@pytest.mark.parametrize("site", ["same-site", None, "cross-site"]) +@pytest.mark.parametrize("same_site", ["Lax", "Strict"]) +def test_the_allowlist_holds_whatever_the_same_site_setting( + api: FakeApi, site: str | None, same_site: str +) -> None: + api.on("POST", "/recovery-codes", respond(200, {"ok": True})) + adapter = api.adapter(allowed_origins=[APP], same_site=same_site) + headers = {"origin": SIBLING} + if site is not None: + headers["sec-fetch-site"] = site + + r = call(adapter, "POST", "/recovery-codes", body="{}", cookie=access(), headers=headers) + + assert (r.status, r.body) == (403, {"error": "cross_site_request_blocked"}) + assert not api.calls_to("POST", "/recovery-codes") + + def test_the_live_manifest_adds_routes(api: FakeApi) -> None: api.manifest = { "schemaVersion": 1, @@ -920,3 +1172,37 @@ def test_a_client_without_a_timeout_gets_one(api: FakeApi) -> None: assert adapter._timeout == {"timeout": 15.0} # The test client keeps httpx2's default timeout, so it is left alone. assert api.adapter()._timeout == {} + + +def test_a_caller_client_that_follows_redirects_does_not_carry_the_service_token_away( + api: FakeApi, +) -> None: + hosts: list[str] = [] + + def handle(request: httpx2.Request) -> httpx2.Response: + hosts.append(request.url.host) + return api.handle(request) + + api.on( + "POST", + "/login", + lambda _r: httpx2.Response(307, headers={"location": "http://evil.test/steal"}), + ) + client = httpx2.Client(transport=httpx2.MockTransport(handle), follow_redirects=True) + + r = call(api.adapter(http_client=client), "POST", "/login", body="{}") + + assert "evil.test" not in hosts + assert r.status == 307 + + +def test_logout_with_an_unreadable_access_cookie_still_ends_the_session(api: FakeApi) -> None: + api.on("POST", "/refresh", respond(200, session())) + api.on("DELETE", "/logout", respond(200, {"message": "Success"})) + + r = call(api.adapter(), "DELETE", "/logout", cookie=f"{access()}x; {REFRESH}") + + assert len(api.calls_to("DELETE", "/logout")) == 1 + assert r.status == 204 + for name in ("seamless-access", "seamless-ephemeral", "seamless-refresh"): + assert r.cleared(name), name diff --git a/tests/test_console.py b/tests/test_console.py index 2269b6f..0cade8c 100644 --- a/tests/test_console.py +++ b/tests/test_console.py @@ -151,3 +151,19 @@ def test_the_fastapi_console_router(api: FakeApi) -> None: assert client.get("/console/%2e%2e/jwks.json").status_code == 400 assert client.post("/console/x").status_code == 405 assert all("cookie" not in c.headers for c in api.calls) + + +def test_a_caller_client_that_follows_redirects_keeps_the_console_inside_itself( + api: FakeApi, +) -> None: + api.on( + "GET", + "/console/escape", + lambda _r: httpx2.Response(302, headers={"location": "/.well-known/jwks.json"}), + ) + client = httpx2.Client(transport=httpx2.MockTransport(api.handle), follow_redirects=True) + + r = read(api.adapter(http_client=client).console("GET", "/escape")) + + assert r.status == 502 + assert not api.calls_to("GET", "/.well-known/jwks.json") diff --git a/tests/test_fastapi.py b/tests/test_fastapi.py index 0196ec3..7ac12cf 100644 --- a/tests/test_fastapi.py +++ b/tests/test_fastapi.py @@ -184,7 +184,7 @@ async def deliver(delivery: Delivery) -> None: client = app_for(api.adapter(deliver=deliver)) client.cookies.set("seamless-ephemeral", pre_auth().split("=", 1)[1]) - r = client.post("/auth/magic-link") + r = client.post("/auth/magic-link", headers={"content-type": "application/json"}) assert r.status_code == 200 assert r.json() == {"message": "sent"} diff --git a/tests/test_guard.py b/tests/test_guard.py index ba1ded9..15d8d9d 100644 --- a/tests/test_guard.py +++ b/tests/test_guard.py @@ -368,6 +368,27 @@ def post(*headers: tuple[str, str]) -> str: ) +@pytest.mark.parametrize("same_site", ["Lax", "Strict"]) +def test_the_guard_checks_the_origin_whatever_the_same_site_setting( + api: FakeApi, same_site: str +) -> None: + adapter = api.adapter(allowed_origins=["https://app.example"], same_site=same_site) + session = session_cookie(ACCESS) + + def post(*headers: tuple[str, str]) -> str: + return adapter.authenticate_request("POST", [session, *headers]).id + + with pytest.raises(CrossSiteRequest): + post(("sec-fetch-site", "cross-site")) + with pytest.raises(CrossSiteRequest): + post(("sec-fetch-site", "same-site"), ("origin", "https://evil.app.example")) + with pytest.raises(CrossSiteRequest): + post(("origin", "https://evil.app.example")) + assert post(("sec-fetch-site", "same-site"), ("origin", "https://app.example")) == "u1" + # Neither header: not a browser form. + assert post() == "u1" + + ROLE_CASES = [ # (granted, required, ok) (["admin"], ["admin"], True), diff --git a/tests/test_jwks.py b/tests/test_jwks.py new file mode 100644 index 0000000..10e9e85 --- /dev/null +++ b/tests/test_jwks.py @@ -0,0 +1,124 @@ +from __future__ import annotations + +import contextlib +import threading +import time + +import httpx2 +import pytest +from conftest import API_URL, JWKS, api_token + +from seamless_auth._jwks import REFETCH_INTERVAL, JwksCache, TokenError + +JWKS_URL = f"{API_URL}/.well-known/jwks.json" + + +class SlowJwks: + """A JWKS endpoint whose fetches after the first hang until released.""" + + def __init__(self) -> None: + self.fetches = 0 + self.started = threading.Event() + self.release = threading.Event() + + def handle(self, _request: httpx2.Request) -> httpx2.Response: + self.fetches += 1 + if self.fetches > 1: + self.started.set() + self.release.wait(10) + return httpx2.Response(200, json=JWKS) + + +def test_a_cached_kid_does_not_wait_on_a_fetch_in_progress() -> None: + endpoint = SlowJwks() + cache = JwksCache(JWKS_URL, httpx2.Client(transport=httpx2.MockTransport(endpoint.handle))) + token = api_token("u1", "access") + cache.verify(token, API_URL, API_URL) + # Past the refetch interval, so an unknown kid is allowed a refetch. + cache._attempted_at = time.monotonic() - REFETCH_INTERVAL - 1 + + unknown = threading.Thread(target=lambda: _swallow(cache, "random-kid")) + unknown.start() + try: + assert endpoint.started.wait(5), "the refetch for the unknown kid never started" + + done = threading.Event() + verified = threading.Thread( + target=lambda: (cache.verify(token, API_URL, API_URL), done.set()) + ) + verified.start() + assert done.wait(1), "a token with a cached, fresh kid waited on the slow fetch" + verified.join(5) + finally: + endpoint.release.set() + unknown.join(10) + + +def test_concurrent_misses_share_one_fetch() -> None: + endpoint = SlowJwks() + cache = JwksCache(JWKS_URL, httpx2.Client(transport=httpx2.MockTransport(endpoint.handle))) + cache.verify(api_token("u1", "access"), API_URL, API_URL) + cache._attempted_at = time.monotonic() - REFETCH_INTERVAL - 1 + + threads = [threading.Thread(target=lambda: _swallow(cache, "random-kid")) for _ in range(5)] + for t in threads: + t.start() + try: + assert endpoint.started.wait(5) + time.sleep(0.2) + finally: + endpoint.release.set() + for t in threads: + t.join(10) + assert endpoint.fetches == 2, "the misses refetched once between them" + + +def _swallow(cache: JwksCache, kid: str) -> None: + with contextlib.suppress(TokenError): + cache.key(kid) + + +def test_a_failed_fetch_releases_its_waiters() -> None: + fetches = 0 + + def failing(_request: httpx2.Request) -> httpx2.Response: + nonlocal fetches + fetches += 1 + return httpx2.Response(503) + + cache = JwksCache(JWKS_URL, httpx2.Client(transport=httpx2.MockTransport(failing))) + + for _ in range(2): + with pytest.raises(TokenError): + cache.key("k1") + assert fetches == 1, "a second miss inside the backoff does not fetch again" + assert cache._fetching is None + + +def test_a_caller_client_that_follows_redirects_does_not_fetch_keys_elsewhere() -> None: + hosts: list[str] = [] + + def handle(request: httpx2.Request) -> httpx2.Response: + hosts.append(request.url.host) + if request.url.host == "evil.test": + return httpx2.Response(200, json=JWKS) + return httpx2.Response(302, headers={"location": "http://evil.test/jwks.json"}) + + client = httpx2.Client(transport=httpx2.MockTransport(handle), follow_redirects=True) + cache = JwksCache(JWKS_URL, client) + + with pytest.raises(TokenError): + cache.verify(api_token("u1", "access"), API_URL, API_URL) + assert "evil.test" not in hosts + + +@pytest.mark.parametrize(("member", "value"), [("use", "enc"), ("alg", "RS512")]) +def test_a_key_published_for_another_use_does_not_verify(member: str, value: str) -> None: + jwks = {"keys": [{**JWKS["keys"][0], member: value}]} + client = httpx2.Client( + transport=httpx2.MockTransport(lambda _r: httpx2.Response(200, json=jwks)) + ) + cache = JwksCache(JWKS_URL, client) + + with pytest.raises(TokenError): + cache.verify(api_token("u1", "access"), API_URL, API_URL) diff --git a/tests/test_verify_only_frameworks.py b/tests/test_verify_only_frameworks.py index 38e1d85..178350d 100644 --- a/tests/test_verify_only_frameworks.py +++ b/tests/test_verify_only_frameworks.py @@ -25,7 +25,7 @@ from fastapi import Depends, FastAPI from fastapi.testclient import TestClient -from seamless_auth import VerifiedUser, require_scope +from seamless_auth import BearerRequest, VerifiedUser, require_scope from seamless_auth.django import _reset_resource_server from seamless_auth.fastapi import RequireBearer, add_bearer_error_handler @@ -170,3 +170,37 @@ def x(user: VerifiedUser = Depends(RequireBearer(server))) -> dict[str, str]: # r = TestClient(app).get("/x", headers=[("authorization", auth), ("authorization", auth)]) assert r.status_code == 401 assert r.json() == {"error": "missing_token"} + + +def test_fastapi_confirm_hook_url_does_not_come_from_the_host_header() -> None: + seen: list[str] = [] + + def confirm(cnf: Any, request: BearerRequest | None) -> bool: + assert request is not None + seen.append(request.url) + return True + + server = Provider().server(confirm=confirm) + app = FastAPI() + + bearer = RequireBearer(server, base_url="https://api.example") + + @app.get("/orders") + def orders(user: VerifiedUser = Depends(bearer)) -> dict[str, str]: # noqa: B008 + return {"sub": user.sub} + + r = TestClient(app, base_url="http://evil.test").get( + "/orders?page=2", + headers={"authorization": f"Bearer {VECTORS['tokens']['cnf_bound']}", "host": "evil.test"}, + ) + + assert r.status_code == 200 + assert seen == ["https://api.example/orders?page=2"] + + +@pytest.mark.parametrize( + "base_url", ["api.example", "ftp://api.example", "https://api.example/v1", "https://"] +) +def test_fastapi_base_url_must_be_an_origin(base_url: str) -> None: + with pytest.raises(ValueError, match="base_url"): + RequireBearer(Provider().server(), base_url=base_url) diff --git a/uv.lock b/uv.lock index 736e199..550c6ad 100644 --- a/uv.lock +++ b/uv.lock @@ -924,7 +924,7 @@ requires-dist = [ { name = "django", marker = "extra == 'django'", specifier = ">=5.2" }, { name = "fastapi", marker = "extra == 'fastapi'", specifier = ">=0.115" }, { name = "httpx2", specifier = ">=2.13,<3" }, - { name = "pyjwt", extras = ["crypto"], specifier = ">=2.10,<3" }, + { name = "pyjwt", extras = ["crypto"], specifier = ">=2.10.1,<3" }, ] provides-extras = ["fastapi", "django"]