diff --git a/docs/guide/bearer-tokens.md b/docs/guide/bearer-tokens.md index 2eb4a8d..2d31b8c 100644 --- a/docs/guide/bearer-tokens.md +++ b/docs/guide/bearer-tokens.md @@ -19,7 +19,7 @@ icon: lucide/key-round ## Prefixes -`prefix="user_"` makes the source only accept tokens starting with `user_`. The check is a string comparison made before your validator runs, so a token with the wrong prefix is refused without hashing it or querying the database. Under `MultiAuth`, it counts as no credential for that source and the next source is tried. +`prefix="user_"` makes the source only accept tokens starting with `user_`. The check is a string comparison made before your validator runs, so a token with the wrong prefix is refused without hashing it or querying the database. Under `MultiAuth`, it counts as no credential for that source and the next source is tried. If no source takes it, the request is refused, even with a valid cookie. Give each kind of token its own prefix and source, and combine them with [`MultiAuth`](multiauth.md): each validator only ever sees its own tokens. Prefixes also help secret scanners spot leaked tokens. diff --git a/docs/guide/custom-sources.md b/docs/guide/custom-sources.md index 2d92998..acf4f04 100644 --- a/docs/guide/custom-sources.md +++ b/docs/guide/custom-sources.md @@ -20,4 +20,6 @@ When no built-in source reads your credential, subclass [`AuthSource`][fastapi_m The scheme passed to `super().__init__()` is only used for OpenAPI. Your source works with `MultiAuth`, `optional()`, WebSockets, logging and auditing like the built-in ones. +If your source reads the `Authorization` header, set the class attribute `authorization_scheme` to the scheme it reads, lowercased, such as `"bearer"`, or to `"*"` for any scheme. `MultiAuth` then refuses a header of that scheme that no source takes, instead of using the cookie sent alongside it. Without it, a token your source takes is refused when a built-in source of the same scheme, such as a prefixed `HTTPBearerAuth`, is combined with it and does not take that token. + To check [scopes](scopes.md), override `authenticate_scoped()`, keeping its keyword-only `request` parameter. To use [`fresh()`](step-up.md) or [`stepped_up()`](step-up.md#strong-login-stepped_up) without a reader, override `authenticated_at()` or `authenticated_acr()`. See the [API reference](../reference.md#fastapi_multiauth.AuthSource). diff --git a/docs/guide/logging.md b/docs/guide/logging.md index 267173d..23c4ac8 100644 --- a/docs/guide/logging.md +++ b/docs/guide/logging.md @@ -34,6 +34,7 @@ Records never contain the credential, its hash, or the identity. Correlate with | `authenticated` | accepted | | `anonymous` | an `optional()` route served a caller without a credential | | `missing_credential` | no credential on a route that requires one | +| `unclaimed_authorization` | a token no `MultiAuth` source takes, such as one without the expected prefix | | `invalid_signature` | a signed cookie, JWT or webhook failed its signature check | | `expired` | a signed cookie past its `ttl`, a JWT past its `exp`, or a webhook timestamp outside its `tolerance` | | `malformed` | a Basic header, JWT header or webhook signature header that does not parse | diff --git a/docs/guide/multiauth.md b/docs/guide/multiauth.md index 41bf072..cacfb16 100644 --- a/docs/guide/multiauth.md +++ b/docs/guide/multiauth.md @@ -15,6 +15,7 @@ icon: lucide/layers Sources are tried **in order**, and the first one that finds a credential decides: - If that credential is invalid, the request is refused. It does **not** fall through to the next source. +- A token no source takes, such as one without the expected prefix, is refused too: the cookie sent alongside it is never used instead. - With no credential at all, the 401 lists every source's challenge. - All sources appear in `/docs`. diff --git a/docs/security-model.md b/docs/security-model.md index 30062a0..74fb1cc 100644 --- a/docs/security-model.md +++ b/docs/security-model.md @@ -9,7 +9,7 @@ What the library takes care of, and what stays with you. To report a vulnerabili ## What the library handles - **Tampering**: cookies are signed (HMAC-SHA256), JWTs and ID tokens are verified, OAuth `state` is checked against CSRF. -- **Confusion between sources**: a cookie signed for one cookie name is never accepted under another, a JWKS-backed `JWTValidator` requires an `audience`, and a JWT whose `typ` or `token_use` claim marks it as an ID or refresh token is refused as not an access token. Providers that leave their ID tokens unmarked are told apart by `audience`, or by the `typ` header you ask `JWTValidator` for. +- **Confusion between sources**: a cookie signed for one cookie name is never accepted under another, a token no `MultiAuth` source takes never falls back to the cookie sent alongside it, a JWKS-backed `JWTValidator` requires an `audience`, and a JWT whose `typ` or `token_use` claim marks it as an ID or refresh token is refused as not an access token. Providers that leave their ID tokens unmarked are told apart by `audience`, or by the `typ` header you ask `JWTValidator` for. - **Downgrades**: OAuth, OIDC and JWKS URLs must be HTTPS, and `HS*` algorithms are refused against a provider's keys. - **Cross-site requests**: a cookie source [refuses requests sent from another origin](guide/cookie-sessions.md#csrf) unless you pass `csrf=False`. - **Stolen tokens**: a [DPoP](guide/jwt-and-oauth2.md#sender-constrained-tokens-dpop)-bound token is refused without a fresh proof signed by its key. diff --git a/src/fastapi_multiauth/abc.py b/src/fastapi_multiauth/abc.py index 572deb8..d969218 100644 --- a/src/fastapi_multiauth/abc.py +++ b/src/fastapi_multiauth/abc.py @@ -265,7 +265,8 @@ class AuthSource(ABC): Subclass it to read a credential the built-in sources do not handle. Implement `extract()` and `authenticate()`; override `www_authenticate()`, `authenticated_at()`, `authenticated_acr()` or `authenticate_scoped()` - when the source needs them. + when the source needs them, and set `authorization_scheme` on a source + that reads the `Authorization` header. Args: scheme: A `fastapi.security` scheme describing the source in OpenAPI. @@ -273,6 +274,11 @@ class AuthSource(ABC): """ scheme: SecurityBase | None + authorization_scheme: str | None = None + """The `Authorization` scheme this source reads, lowercased: `"*"` for any + scheme, `None` when it does not read the header. Under `MultiAuth`, a + header of a scheme a source reads, which no such source takes, is refused + instead of passing to a cookie or an API key.""" _optional: bool = False _scope_catalogue: dict[str, str] | None = None """The scopes this source publishes to OpenAPI, `None` when it has none.""" diff --git a/src/fastapi_multiauth/sources/basic.py b/src/fastapi_multiauth/sources/basic.py index 7e45aa1..b906588 100644 --- a/src/fastapi_multiauth/sources/basic.py +++ b/src/fastapi_multiauth/sources/basic.py @@ -43,6 +43,8 @@ class HTTPBasicAuth(ValidatedAuthSource): **kwargs: Keyword arguments passed to the validator on every call. """ + authorization_scheme = "basic" + def __init__( self, validator: Callable[..., Any], diff --git a/src/fastapi_multiauth/sources/bearer.py b/src/fastapi_multiauth/sources/bearer.py index 770f063..8e2848d 100644 --- a/src/fastapi_multiauth/sources/bearer.py +++ b/src/fastapi_multiauth/sources/bearer.py @@ -22,6 +22,7 @@ class _BearerSource(ValidatedAuthSource): """Extraction shared by every source carrying an `Authorization: Bearer` token.""" + authorization_scheme = "bearer" _prefix: str | None = None def __init__( diff --git a/src/fastapi_multiauth/sources/dpop.py b/src/fastapi_multiauth/sources/dpop.py index c99d184..f512b50 100644 --- a/src/fastapi_multiauth/sources/dpop.py +++ b/src/fastapi_multiauth/sources/dpop.py @@ -129,6 +129,7 @@ class HTTPDPoPAuth(ValidatedAuthSource): NO_REPLAY_CHECK: ClassVar[NoReplayCheck] = NoReplayCheck.NONE """Explicit `replay_check` opt-out.""" + authorization_scheme = "dpop" def __init__( self, diff --git a/src/fastapi_multiauth/sources/header.py b/src/fastapi_multiauth/sources/header.py index 3c8e821..473804e 100644 --- a/src/fastapi_multiauth/sources/header.py +++ b/src/fastapi_multiauth/sources/header.py @@ -30,6 +30,8 @@ def __init__( **kwargs: Any, ) -> None: self._name = name + if name.lower() == "authorization": + self.authorization_scheme = "*" super().__init__( validator, APIKeyHeader( diff --git a/src/fastapi_multiauth/sources/multi.py b/src/fastapi_multiauth/sources/multi.py index 48e9984..0864bf3 100644 --- a/src/fastapi_multiauth/sources/multi.py +++ b/src/fastapi_multiauth/sources/multi.py @@ -24,7 +24,7 @@ _unenforceable_scopes_error, _wiring_error, ) -from ..utils import challenge_headers, log_decision +from ..utils import authorization_scheme, challenge_headers, log_decision logger = logging.getLogger(__name__) @@ -47,7 +47,8 @@ class MultiAuth: Sources are tried in order, and the first one that finds a credential decides: an invalid credential is refused, not passed to the next - source. All sources appear in OpenAPI. + source. A token no source takes, such as one without the expected + prefix, is refused too. All sources appear in OpenAPI. Args: *sources: The sources to try, in order. @@ -129,6 +130,11 @@ def __init__(self, *sources: AuthSource) -> None: self._catalogued = tuple( source for source in sources if source._scope_catalogue is not None ) + self._authorization_readers = tuple( + (index, source) + for index, source in enumerate(sources) + if source.authorization_scheme is not None + ) def www_authenticate(self) -> str | None: """Return the challenges of all sources, joined, or `None` when none has one.""" @@ -269,20 +275,35 @@ async def _ensure_fresh(self, request: HTTPConnection, window: _Freshness) -> No for source in self._sources: if await source.extract(request) is not None: raise _wiring_error(self) - raise self._missing_credential() - - def _missing_credential(self) -> UnauthorizedError: - """Log and build the 401 for a request no source found a credential in.""" - log_decision( - logger, - logging.INFO, - "missing_credential", - source="MultiAuth", - scheme=None, - status=401, - ) + raise self._unauthorized("missing_credential", logging.INFO) + + def _unauthorized(self, reason: str, level: int) -> UnauthorizedError: + """Log and build a 401 carrying every source's challenge.""" + log_decision(logger, level, reason, source="MultiAuth", scheme=None, status=401) return UnauthorizedError(headers=challenge_headers(self.www_authenticate())) + async def _unclaimed_authorization( + self, request: HTTPConnection, taker: int + ) -> bool: + """Whether a source reads the `Authorization` header, yet none takes it. + + `taker` is the index of the source that found a credential, which does + not read the header, or the number of sources when none found one. + """ + if not self._authorization_readers: + return False + scheme = authorization_scheme(request) + if scheme is None: + return False + declined = False + for index, source in self._authorization_readers: + if source.authorization_scheme not in (scheme, "*"): + continue + if index > taker and await source.extract(request) is not None: + return False + declined = True + return declined + def _with_sources(self, sources: Iterable[AuthSource]) -> "MultiAuth": """A new `MultiAuth` over `sources`, carrying this one's flags.""" clone = MultiAuth(*sources) @@ -307,15 +328,23 @@ async def dispatch(self, request: HTTPConnection, scopes: list[str]) -> Any: for source in self._sources: credential = await source.extract(request) if credential is not None: + if source.authorization_scheme is None and ( + await self._unclaimed_authorization( + request, self._sources.index(source) + ) + ): + raise self._unauthorized("unclaimed_authorization", logging.WARNING) return await source._authenticate_with_challenge( credential, scopes, request=request ) + if await self._unclaimed_authorization(request, len(self._sources)): + raise self._unauthorized("unclaimed_authorization", logging.WARNING) if self._optional: log_decision( logger, logging.DEBUG, "anonymous", source="MultiAuth", scheme=None ) return None - raise self._missing_credential() + raise self._unauthorized("missing_credential", logging.INFO) async def __call__(self, **kwargs: Any) -> Any: """Run as a FastAPI dependency.""" diff --git a/src/fastapi_multiauth/sources/signature.py b/src/fastapi_multiauth/sources/signature.py index b5c68c0..60bad17 100644 --- a/src/fastapi_multiauth/sources/signature.py +++ b/src/fastapi_multiauth/sources/signature.py @@ -151,6 +151,8 @@ def __init__( ) ] self._header = header + if header.lower() == "authorization": + self.authorization_scheme = "*" self._prefix = prefix self._signature_key = signature_key self._timestamp_key = timestamp_key diff --git a/src/fastapi_multiauth/utils.py b/src/fastapi_multiauth/utils.py index 860a45f..48549ca 100644 --- a/src/fastapi_multiauth/utils.py +++ b/src/fastapi_multiauth/utils.py @@ -7,6 +7,7 @@ import inspect import logging import math +import re import secrets import string import time @@ -464,6 +465,19 @@ def step_up_challenge( return challenge_headers(f"{challenge}{separator}{', '.join(params)}") +_SCHEME = re.compile(r"[!#$%&'*+\-.^_`|~0-9A-Za-z]+") + + +def authorization_scheme(request: HTTPConnection) -> str | None: + """Return the lowercased scheme of the `Authorization` header, `None` without one. + + The scheme is the leading run of token characters (RFC 7230 ยง3.2.6), + whatever follows it. + """ + match = _SCHEME.match(request.headers.get("Authorization", "")) + return match.group().lower() if match else None + + def authorization_credential(request: HTTPConnection, scheme: str) -> str | None: """Extract the value of an `Authorization: ` header. diff --git a/tests/test_multiauth.py b/tests/test_multiauth.py index 8f662c1..2d533fe 100644 --- a/tests/test_multiauth.py +++ b/tests/test_multiauth.py @@ -102,6 +102,7 @@ from fastapi_multiauth.sources.dpop import _jwk_thumbprint from fastapi_multiauth.utils import ( VerifiedBinding, + authorization_credential, ensure_async, first_use, record_verified_jkt, @@ -468,6 +469,186 @@ async def tracking_validator(credential: str) -> dict: assert response.status_code == 401 assert second_called == [] # cookie validator was never called + @staticmethod + def _token_and_cookie(*, cookie_first: bool = False): + """A prefixed bearer for alice and a cookie for bob, and the cookie's calls.""" + cookie_calls: list[str] = [] + + async def token_validator(token: str) -> dict: + if token != "user_alice": + raise UnauthorizedError() + return {"user": "alice"} + + async def bob_cookie(value: str) -> dict: + cookie_calls.append(value) + return {"user": "bob"} + + bearer = HTTPBearerAuth(token_validator, prefix="user_") + cookie = APIKeyCookieAuth("session", bob_cookie) + sources = (cookie, bearer) if cookie_first else (bearer, cookie) + return _client(MultiAuth(*sources)), cookie_calls + + def test_token_wins_over_a_cookie_for_another_user(self): + client, _ = self._token_and_cookie() + response = client.get( + "/me", + headers={ + "Authorization": "Bearer user_alice", + **_cookie_header({"session": "bob"}), + }, + ) + assert response.json() == {"user": "alice"} + + def test_unclaimed_token_does_not_fall_back_to_the_cookie(self, caplog): + caplog.set_level(logging.WARNING, logger="fastapi_multiauth") + client, cookie_calls = self._token_and_cookie() + for authorization in ("Bearer pat_alice", "Bearer alice", "Bearer ", "bearer"): + caplog.clear() + response = client.get( + "/me", + headers={ + "Authorization": authorization, + **_cookie_header({"session": "bob"}), + }, + ) + assert response.status_code == 401 + assert response.headers["www-authenticate"] == "Bearer" + assert [(r.auth_reason, r.auth_status) for r in caplog.records] == [ + ("unclaimed_authorization", 401) + ] + assert cookie_calls == [] + + def test_scheme_no_source_reads_falls_back_to_the_cookie(self): + client, _ = self._token_and_cookie() + for authorization in ("Basic YTpi", "Token user_alice"): + response = client.get( + "/me", + headers={ + "Authorization": authorization, + **_cookie_header({"session": "bob"}), + }, + ) + assert response.json() == {"user": "bob"} + + client = _client( + MultiAuth( + APIKeyHeaderAuth("X-API-Key", simple_validator), + APIKeyCookieAuth("session", cookie_validator), + ) + ) + response = client.get( + "/me", + headers={ + "Authorization": "Bearer alice", + **_cookie_header({"session": VALID_COOKIE}), + }, + ) + assert response.json() == {"session": VALID_COOKIE} + + def test_order_still_decides_between_two_accepted_credentials(self): + client, _ = self._token_and_cookie(cookie_first=True) + cookie = _cookie_header({"session": "bob"}) + response = client.get( + "/me", headers={"Authorization": "Bearer user_alice", **cookie} + ) + assert response.json() == {"user": "bob"} + response = client.get( + "/me", headers={"Authorization": "Bearer alice", **cookie} + ) + assert response.status_code == 401 + + def test_malformed_separator_does_not_fall_back_to_the_cookie(self): + client, cookie_calls = self._token_and_cookie() + for authorization in ("Bearer\tpat_alice", "Bearer,pat_alice", "Bearer="): + response = client.get( + "/me", + headers={ + "Authorization": authorization, + **_cookie_header({"session": "bob"}), + }, + ) + assert response.status_code == 401 + assert cookie_calls == [] + + response = client.get( + "/me", + headers={"Authorization": "=", **_cookie_header({"session": "bob"})}, + ) + assert response.json() == {"user": "bob"} + + def test_unclaimed_token_is_refused_past_a_source_reading_another_header(self): + client = _client( + MultiAuth( + APIKeyCookieAuth("session", cookie_validator), + APIKeyHeaderAuth("X-API-Key", simple_validator), + HTTPBearerAuth(simple_validator, prefix="user_"), + ) + ) + response = client.get( + "/me", + headers={ + "Authorization": "Bearer alice", + **_cookie_header({"session": VALID_COOKIE}), + }, + ) + assert response.status_code == 401 + + def test_unclaimed_dpop_header_does_not_fall_back_to_the_cookie(self): + client = _client( + MultiAuth( + HTTPDPoPAuth( + simple_validator, replay_check=HTTPDPoPAuth.NO_REPLAY_CHECK + ), + APIKeyCookieAuth("session", cookie_validator), + ) + ) + cookie = _cookie_header({"session": VALID_COOKIE}) + response = client.get("/me", headers={"Authorization": "DPoP", **cookie}) + assert response.status_code == 401 + response = client.get("/me", headers={"Authorization": "Bearer x", **cookie}) + assert response.json() == {"session": VALID_COOKIE} + + def test_header_source_on_authorization_takes_the_header(self): + async def raw(value: str) -> dict: + return {"raw": value} + + prefixed = HTTPBearerAuth(simple_validator, prefix="user_") + header = APIKeyHeaderAuth("Authorization", raw) + for sources in ((prefixed, header), (header, prefixed)): + client = _client(MultiAuth(*sources)) + response = client.get("/me", headers={"Authorization": "Bearer legacy"}) + assert response.json() == {"raw": "Bearer legacy"} + + def test_custom_source_reading_authorization(self): + class _OpaqueAuth(AuthSource): + authorization_scheme = "bearer" + + async def extract(self, request: HTTPConnection) -> str | None: + token = authorization_credential(request, "bearer") + return token if token and token.startswith("opaque_") else None + + async def authenticate(self, credential: str) -> dict: + return {"opaque": credential} + + cookie = _cookie_header({"session": VALID_COOKIE}) + client = _client( + MultiAuth( + HTTPBearerAuth(simple_validator, prefix="user_"), + _OpaqueAuth(), + APIKeyCookieAuth("session", cookie_validator), + ) + ) + response = client.get( + "/me", headers={"Authorization": "Bearer opaque_1", **cookie} + ) + assert response.json() == {"opaque": "opaque_1"} + response = client.get( + "/me", headers={"Authorization": "Bearer other", **cookie} + ) + assert response.status_code == 401 + response = client.get("/me", headers={"Authorization": "Basic YTpi", **cookie}) + assert response.json() == {"session": VALID_COOKIE} + def test_prefix_routes_to_correct_source(self): """Prefix-based dispatch: only the matching source's validator is called.""" user_calls: list[str] = [] @@ -3399,6 +3580,17 @@ def test_multiauth_none_only_when_no_source_matches(self): } assert client.get("/maybe", headers={"X-API-Key": "wrong"}).status_code == 401 + def test_multiauth_unclaimed_token_is_not_anonymous(self): + auth = MultiAuth( + HTTPBearerAuth(simple_validator, prefix="user_"), + APIKeyCookieAuth("session", cookie_validator), + ) + client = TestClient(_app(self._routes(auth))) + + response = client.get("/maybe", headers={"Authorization": "Bearer alice"}) + assert response.status_code == 401 + assert client.get("/maybe").json() == {"user": None} + def test_scoped_optional_route_fails_for_every_caller(self): """An anonymous caller cannot satisfy scopes, so the combination is refused.""" @@ -6254,6 +6446,23 @@ def _stamped(body: bytes, t: int | str | None = None) -> tuple[str, str]: t = str(int(time.time()) if t is None else t) return t, _hmac_hex(WEBHOOK_SECRET, f"{t}.".encode() + body) + def test_on_authorization_takes_the_header_under_multiauth(self): + body = b"payload" + source = HMACSignatureAuth( + WEBHOOK_SECRET, header="Authorization", prefix="Bearer " + ) + client = self._client( + MultiAuth(HTTPBearerAuth(simple_validator, prefix="user_"), source) + ) + response = client.post( + "/hook", + content=body, + headers={"Authorization": f"Bearer {_hmac_hex(WEBHOOK_SECRET, body)}"}, + ) + assert response.json() == {"body": "payload"} + other = HMACSignatureAuth(WEBHOOK_SECRET, header="X-Signature") + assert other.authorization_scheme is None + def test_github_layout(self): client = self._client(HMACSignatureAuth(self.GITHUB_SECRET, **self.GITHUB)) upper = "sha256=" + self.GITHUB_SIGNATURE.removeprefix("sha256=").upper()