BE-761: Authenticate Cloudflare Access JWTs in the Graph API - #9235
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
PR SummaryCursor Bugbot is generating a summary for commit 7d3b7c8. Configure here. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## t/be-765-tighten-the-authentication-provider-types-and-split-the #9235 +/- ##
====================================================================================================
+ Coverage 59.94% 60.17% +0.22%
====================================================================================================
Files 1427 1430 +3
Lines 139885 140522 +637
Branches 6582 6596 +14
====================================================================================================
+ Hits 83859 84557 +698
+ Misses 54952 54887 -65
- Partials 1074 1078 +4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Adds Cloudflare Access JWT authentication to the Graph API and unifies authentication across the main and admin ports.
Changes:
- Adds JWT validation and verified-email-to-actor resolution through Kratos.
- Applies the shared Kratos, Cloudflare, and service-delegation provider chain to both APIs.
- Feature-gates destructive development endpoints and updates test clients.
Reviewed changes
Copilot reviewed 22 out of 23 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
.env |
Removes the obsolete runtime unsafe-auth flag. |
apps/hash-graph/Cargo.toml |
Forwards the unsafe-endpoints feature. |
apps/hash-graph/src/subcommand/admin_server.rs |
Builds authentication configuration for the admin server. |
apps/hash-graph/src/subcommand/server.rs |
Shares authentication setup across both servers. |
Cargo.lock |
Records dependency resolution changes. |
libs/@local/graph/api/Cargo.toml |
Defines the unsafe-endpoints feature. |
libs/@local/graph/api/src/rest/admin.rs |
Protects admin routes with shared authentication. |
libs/@local/graph/api/src/rest/auth.rs |
Constructs the shared provider chain. |
libs/@local/graph/api/src/rest/mod.rs |
Enables Cloudflare authentication on the main API. |
libs/@local/graph/authentication/Cargo.toml |
Adds JWT and async dependencies. |
libs/@local/graph/authentication/package.json |
Separates unit and integration tests. |
libs/@local/graph/authentication/src/cloudflare.rs |
Implements the Cloudflare Access provider. |
libs/@local/graph/authentication/src/jwt.rs |
Provides reusable JWKS-backed JWT validation. |
libs/@local/graph/authentication/src/kratos/identity.rs |
Resolves verified emails to Graph actors. |
libs/@local/graph/authentication/src/kratos/mod.rs |
Exports Kratos identity resolution. |
libs/@local/graph/authentication/src/lib.rs |
Exposes Cloudflare and JWT modules. |
libs/@local/graph/authentication/src/provider.rs |
Supports optional providers in chains. |
libs/@local/graph/authentication/src/request.rs |
Adds Access-specific authentication errors. |
libs/@local/graph/authentication/tests/kratos.rs |
Adds live Kratos contract tests. |
tests/graph/http/.httpyac.config.js |
Injects service credentials for both Graph ports. |
tests/graph/http/tests/reset-database.http |
Adds delegated actor headers to reset requests. |
tests/hash-backend-integration/src/tests/admin-server.ts |
Authenticates integration admin requests. |
tests/hash-playwright/tests/shared/delete-user.ts |
Authenticates Playwright user deletion. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Adds Cloudflare Access as a third credential and puts the admin API behind the same provider chain as the main API, so both ports authenticate identically. The Access provider recognizes only `Cf-Access-Jwt-Assertion`; `Authorization: Bearer` stays reserved for the OAuth work. The token's email claim is resolved through the Kratos admin API, which returns identities whose matching address is still unverified, so requiring `verified` there is what keeps an unverified address from authenticating as its owner. The plain `Cf-Access-Authenticated-User-Email` header carries no signature and is never consulted. The admin API drops its own JWT extractors and header-based actor resolution in favour of the shared chain. The bulk destructive endpoints move behind the `unsafe-dev-endpoints` cargo feature instead of a runtime flag, so the released image cannot serve them at all: the Dockerfile builds without the feature, while local and CI builds enable it. Callers that reach the admin API now present the service credential. JWT validation moves out of the API crate into the authentication crate, where the providers live. Its JWKS cache is covered by counting fetches at a fake endpoint, which pins the refresh cooldown that stops a crafted `kid` from driving one fetch per request.
Review of the Access path turned up one live gap and several classification faults. `aud` and `iss` were configured but not required. `set_audience` and `set_issuer` compare a claim only when it is present, and only `exp` is required by default, so a token omitting either — or setting it to a non-string — verified with no audience binding at all. Since every application of a Cloudflare Access team is signed by the same keys, `aud` is what scopes a token to this one. `nbf` was likewise unvalidated. The admin API no longer accepts an end-user session. It authenticates with the operator chain (Access, then service delegation), because its handlers do not authorize beyond requiring some actor: operators arrive through Access, internal services through the shared secret. That also drops the Kratos session configuration from the admin server entirely. The looked-up email travelled in a query string, and a `reqwest::Error` renders the URL it failed on, so a Kratos timeout put an address into an error-level log line and a Sentry event. A JWKS entry that cannot be turned into a verification key was reported as an invalid token, which answered 401 at debug level and hid a provider outage behind "your token is invalid"; it is now a provider fault. A failed JWKS fetch is negatively cached for the cooldown, so an outage no longer buys one outbound attempt per request. The three JWT duration flags reject zero, which previously disabled the refresh cooldown silently.
4889222 to
5eed42c
Compare
…oundary The email resolver extends the admin URL with a path, which a cannot-be-a-base URL does not support, so `HASH_KRATOS_ADMIN_URL=mailto:…` aborted the process inside the resolver instead of failing to start. The session provider already validates its URL this way. The suite only asserted successful admin calls with the injected secret, so the middleware could have been removed, or accepted a wrong secret, without a test failing. It now covers an omitted secret, a wrong secret, and the open health route on the admin port.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 25 out of 26 changed files in this pull request and generated no new comments.
Suppressed comments (1)
libs/@local/graph/authentication/src/jwt.rs:488
- This test never observes how many outbound requests were made: even if
last_failureandcheck_failure_cooldownare removed, all five validations still returnErr, so the outage-amplification regression it is meant to prevent would pass. Use a counting JWKS handler that returns 500 and assert it was fetched exactly once during the cooldown.
let mut failures = 0_u32;
for attempt in 0..5 {
if validator
.validate(&token_with_key_id(&format!("crafted-{attempt}")))
.await
.is_err()
Benchmark results
|
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| resolve_policies_for_actor | user: empty, selectivity: high, policies: 2002 | Flame Graph | |
| resolve_policies_for_actor | user: empty, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: empty, selectivity: medium, policies: 1002 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: high, policies: 3314 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: medium, policies: 1527 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: high, policies: 2078 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: medium, policies: 1033 | Flame Graph |
policy_resolution_medium
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| resolve_policies_for_actor | user: empty, selectivity: high, policies: 102 | Flame Graph | |
| resolve_policies_for_actor | user: empty, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: empty, selectivity: medium, policies: 52 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: high, policies: 269 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: medium, policies: 108 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: high, policies: 133 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: medium, policies: 63 | Flame Graph |
policy_resolution_none
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| resolve_policies_for_actor | user: empty, selectivity: high, policies: 2 | Flame Graph | |
| resolve_policies_for_actor | user: empty, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: empty, selectivity: medium, policies: 2 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: high, policies: 8 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: medium, policies: 3 | Flame Graph |
policy_resolution_small
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| resolve_policies_for_actor | user: empty, selectivity: high, policies: 52 | Flame Graph | |
| resolve_policies_for_actor | user: empty, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: empty, selectivity: medium, policies: 26 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: high, policies: 94 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: medium, policies: 27 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: high, policies: 66 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: medium, policies: 29 | Flame Graph |
read_scaling_complete
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| entity_by_id;one_depth | 1 entities | Flame Graph | |
| entity_by_id;one_depth | 10 entities | Flame Graph | |
| entity_by_id;one_depth | 25 entities | Flame Graph | |
| entity_by_id;one_depth | 5 entities | Flame Graph | |
| entity_by_id;one_depth | 50 entities | Flame Graph | |
| entity_by_id;two_depth | 1 entities | Flame Graph | |
| entity_by_id;two_depth | 10 entities | Flame Graph | |
| entity_by_id;two_depth | 25 entities | Flame Graph | |
| entity_by_id;two_depth | 5 entities | Flame Graph | |
| entity_by_id;two_depth | 50 entities | Flame Graph | |
| entity_by_id;zero_depth | 1 entities | Flame Graph | |
| entity_by_id;zero_depth | 10 entities | Flame Graph | |
| entity_by_id;zero_depth | 25 entities | Flame Graph | |
| entity_by_id;zero_depth | 5 entities | Flame Graph | |
| entity_by_id;zero_depth | 50 entities | Flame Graph |
read_scaling_linkless
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| entity_by_id | 1 entities | Flame Graph | |
| entity_by_id | 10 entities | Flame Graph | |
| entity_by_id | 100 entities | Flame Graph | |
| entity_by_id | 1000 entities | Flame Graph | |
| entity_by_id | 10000 entities | Flame Graph |
representative_read_entity
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/block/v/1
|
Flame Graph | |
| entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/book/v/1
|
Flame Graph | |
| entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/building/v/1
|
Flame Graph | |
| entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/organization/v/1
|
Flame Graph | |
| entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/page/v/2
|
Flame Graph | |
| entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/person/v/1
|
Flame Graph | |
| entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/playlist/v/1
|
Flame Graph | |
| entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/song/v/1
|
Flame Graph | |
| entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/uk-address/v/1
|
Flame Graph |
representative_read_entity_type
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| get_entity_type_by_id | Account ID: bf5a9ef5-dc3b-43cf-a291-6210c0321eba
|
Flame Graph |
representative_read_multiple_entities
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| entity_by_property | traversal_paths=0 | 0 | |
| entity_by_property | traversal_paths=255 | 1,resolve_depths=inherit:1;values:255;properties:255;links:127;link_dests:126;type:true | |
| entity_by_property | traversal_paths=2 | 1,resolve_depths=inherit:0;values:0;properties:0;links:0;link_dests:0;type:false | |
| entity_by_property | traversal_paths=2 | 1,resolve_depths=inherit:0;values:0;properties:0;links:1;link_dests:0;type:true | |
| entity_by_property | traversal_paths=2 | 1,resolve_depths=inherit:0;values:0;properties:2;links:1;link_dests:0;type:true | |
| entity_by_property | traversal_paths=2 | 1,resolve_depths=inherit:0;values:2;properties:2;links:1;link_dests:0;type:true | |
| link_by_source_by_property | traversal_paths=0 | 0 | |
| link_by_source_by_property | traversal_paths=255 | 1,resolve_depths=inherit:1;values:255;properties:255;links:127;link_dests:126;type:true | |
| link_by_source_by_property | traversal_paths=2 | 1,resolve_depths=inherit:0;values:0;properties:0;links:0;link_dests:0;type:false | |
| link_by_source_by_property | traversal_paths=2 | 1,resolve_depths=inherit:0;values:0;properties:0;links:1;link_dests:0;type:true | |
| link_by_source_by_property | traversal_paths=2 | 1,resolve_depths=inherit:0;values:0;properties:2;links:1;link_dests:0;type:true | |
| link_by_source_by_property | traversal_paths=2 | 1,resolve_depths=inherit:0;values:2;properties:2;links:1;link_dests:0;type:true |
scenarios
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| full_test | query-limited | Flame Graph | |
| full_test | query-unlimited | Flame Graph | |
| linked_queries | query-limited | Flame Graph | |
| linked_queries | query-unlimited | Flame Graph |
🌟 What is the purpose of this PR?
Adds Cloudflare Access as a third credential alongside the Kratos session and the service delegation, and replaces the admin API's own authentication scheme with the shared providers.
The two ports do not authenticate identically: the admin API deliberately runs without the session provider. Its handlers erase entities and delete users while authorizing nothing beyond "some actor", so operators reach it through Access and internal services through the shared secret — an end-user session is not a credential there.
This is what lets an operator reach the admin API through Cloudflare Access with their own identity, rather than through a header the server has to trust.
🔗 Related links
🚫 Blocked by
🔍 What does this change?
Cf-Access-Jwt-Assertion.Authorization: Bearerstays reserved for the OAuth work in BE-728, so a Bearer token is not silently accepted here.emailclaim resolves to an actor through the Kratos admin API. A credentials-identifier lookup also returns identities whose matching address is still unverified, so requiringverifiedis what stops someone registering an address they do not control and authenticating as its owner. The unsignedCf-Access-Authenticated-User-Emailheader is never consulted.AdminActorIdand its JWT extractors, and runs the operator chain — Access JWT, then service delegation./healthstays open; everything else authenticates, and a Kratos session does not./snapshot,/accounts,/data-types,/property-types,/entity-types) move behind theunsafe-dev-endpointscargo feature, replacing the runtime flag. The Dockerfile builds without it, so a released image cannot serve them at all; local and CI builds enable it.audandissrather than only comparing them when present, and validatesnbf.jsonwebtokenrequires justexpby default, so a token omittingaud— or carrying it with the wrong type — verified before this;audis what scopes a token to this application, since every application of an Access team is signed by the same keys.reqwesterror renders the URL it failed on, which put the address into error-level logs and Sentry on any Kratos fault.0, which silently disabled the refresh cooldown or made every fetch time out.--kratos-admin-urlis rejected at startup instead of aborting the process inside the resolver.Pre-Merge Checklist 🚀
🚢 Has this modified a publishable library?
This PR:
📜 Does this require a change to the docs?
The changes in this PR:
🕸️ Does this require a change to the Turbo Graph?
The changes in this PR:
turbo.json's have been updated to reflect thisThe JWT validator accepts a single audience. An Access application with more than one AUD tag would need the configuration widened to a list — a small change, but not one this PR makes since nothing needs it yet.
The admin routes carry no OpenAPI annotations, so they stay absent from the generated spec as before. Worth folding into BE-757 rather than here.
Removing the
Authorization: Bearerfallback is a behaviour change for anyone who reached the admin API that way. Nothing in the repo does, but a hand-rolled client might.A Kratos session no longer authenticates on the admin API. Anything that reached it with a session must present the service credential instead; the callers in this repo already do.
The Access path does not read the identity's Kratos
state, while the session path requires an active session. Deactivating an identity would therefore not revoke Access-based entry. Latent today, since users are deleted rather than deactivated — tracked as a follow-up rather than fixed here, because it needs a decision on whetherstategates authentication or only the session.🐾 Next steps
🛡 What tests cover this?
kidfrom driving one fetch per request.tests/kratos.rsrun the identity lookup against the live Kratos of the compose stack, covering a verified address, an unverified one, an identity without a Graph actor, an unknown address, and case-insensitive matching. These exist because the unit tests serve their responses from a hand-written fake, which only holds as long as the fake matches Kratos./healthstays open.aud/issfails, not only a wrong one — the mutation that removes the requirement turns those cases red.❓ How to test this?
cargo nextest run --package hash-graph-authentication --package hash-graph-api --all-featuresyarn workspace @rust/hash-graph-authentication test:integrationfor the Kratos contract teststests/graph/http/:sh test.sh--all-featuresand check thatDELETE /data-typeson the admin port answers 404 whilePOST /users/deletestill answers 401