Repository navigation
fix: Origin allowlist with Sec-Fetch-Site, and verify the cookie's inner access token - #18
Merged
Merged
Conversation
…ner access token A same-site request is held to allowed_origins when the adopter set one, and the guard now verifies the auth API access token inside the access cookie against the JWKS, requiring a matching sub and taking roles and recovery from the verified token. Closes #15
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ports the v0.20 JS adapter hardening (fells-code/seamless-auth-server#234, issues #230 and #231 there) to the Python adapter. Both the FastAPI (
RequireUser) and Django (require_auth) bindings callAdapter.authenticate_request, so the change lives once in the core (_adapter.py) and both get it. Each binding has a test for it.Origin check (server#230)
Still only while cookies are
SameSite=Noneand only for methods other than GET, HEAD and OPTIONS:Sec-Fetch-Site: cross-sitegives 403cross_site_request_blocked, as before.Sec-Fetch-Site: same-site: whenallowed_originsis set, theOriginmust be present and on the list, otherwise 403. A missingOriginis refused. Without an allowlist the request passes, as before.same-origin,noneand any other value pass, as before.Sec-Fetch-Site, nothing changes.The allowlist match is now one helper (
_is_allowed_origin) that refuses anOriginthat does not normalize toscheme://host. An unparseableOrigintherefore cannot match an unparseable allowlist entry. Before this change that case was already safe withoutSec-Fetch-Site, and is now covered on thesame-sitepath too.Verify the cookie's inner access token (server#231)
In the guard's cookie path, after the HS256 cookie verifies, the API token inside it now goes through the same verification the bearer path uses (
_verified_access_token): RS256 against the auth server JWKS,iss=auth_server_issuer,aud=audience, exp and nbf,typ: access, and a non-emptysub. The innersubmust equal the cookie'ssub, otherwise the answer is 401.rolesandrecoverycome from the verified token.emailandphonestill come from the cookie. In this adapterauth_server_urlis required andaudiencedefaults to the issuer, so the check always runs. It fails closed: a JWKS fetch failure, or an empty issuer or audience, means 401. This replaces the unverified_is_access_tokenread.Cookie lifetime vs the inner token's exp
The access cookie (its
Max-Ageand the HS256exp) is sized from the session response'sttl. The API sets that from the sameaccess_token_ttlit uses for the access token'sexp(sessionIssuance.tsand the/refreshcontroller in seamless-auth-api;signAccessTokenusessetExpirationTime(access_token_ttl)). So the cookie outlives the inner token only by the round trip between the API signing the token and the adapter signing the cookie, plus any clock difference between the two hosts. The 5 second leeway absorbs both. Every place that writes an access cookie (sign-in routes, silent refresh,POST /auth/refresh, the refresh after recovery enrollment) writes it from a fresh, just-verified token. No path re-signs a cookie around an old token. A session that is valid now does not start failing. In the last seconds of a cookie's life, a skewed clock could produce a 401 slightly before the browser drops the cookie. The browser would then refresh through the auth routes as it does at expiry.Other places that read the access cookie
_credential_for(manifest auth routes such as/users/me, the admin routes and logout): forwards the inner token to the API asBearer, and the API verifies it. No claim is trusted locally, so unchanged._optional_pre_auth(magic link verify): forwards the ephemeral cookie's token to the API. Unchanged._is_recovery_tokenon the enrollment routes: read only after the API answered 2xx to that same token, so it has been verified upstream. Unchanged._silent_refreshand_refresh_after_enrollment: read the refresh cookie, not the access cookie. Each new access cookie goes through_verify_session(JWKS) before it is issued. Unchanged.console(admin dashboard proxy): serves static assets and reads no cookie. Unchanged.User.roles, which now comes from the verified token.Tests
tests/test_adapter.pymirroring the JSoriginGuard.test.js. It covers same-site with an allowlist (allowed, other, missing andnullOrigin), same-site without an allowlist, cross-site always blocked, same-origin always passing, Lax cookies unaffected, GET and OPTIONS ungated, and unparseable Origins against an unparseable allowlist. Guard-level same-site tests are intests/test_guard.py.["admin"]with token roles["user"]give("user",). Recovery is taken from the token. JWKS unreachable or 5xx gives 401. An empty audience gives 401 without a fetch. FastAPI and Django guard tests cover a forged and a mismatched cookie. Existing guard tests already signed real RS256 inner tokens from the test JWKS. Theaccess()fixture with an opaque token is only used on auth routes, where the token is forwarded upstream._adapter.py, 22 of the new tests fail.Checks
README "The guard" section is updated.
Closes #15