From 1325685cb29388d8b1b970a44576ea6a95ff61f0 Mon Sep 17 00:00:00 2001 From: Tim Diekmann Date: Wed, 14 Oct 2026 22:18:46 +0200 Subject: [PATCH 01/13] Return `None` from `get_entity_by_id` for a missing or hidden entity --- .../store/postgres/knowledge/entity/mod.rs | 7 +- .../graph/store/rust/src/entity/store.rs | 24 +++- libs/@local/graph/type-fetcher/src/store.rs | 2 +- tests/graph/integration/postgres/drafts.rs | 3 +- tests/graph/integration/postgres/entity.rs | 113 +++++++++++++++++- tests/graph/integration/postgres/lib.rs | 2 +- .../integration/postgres/partial_updates.rs | 21 ++-- 7 files changed, 153 insertions(+), 19 deletions(-) diff --git a/libs/@local/graph/postgres-store/src/store/postgres/knowledge/entity/mod.rs b/libs/@local/graph/postgres-store/src/store/postgres/knowledge/entity/mod.rs index a5f1ffb9196..f850e3a2c9d 100644 --- a/libs/@local/graph/postgres-store/src/store/postgres/knowledge/entity/mod.rs +++ b/libs/@local/graph/postgres-store/src/store/postgres/knowledge/entity/mod.rs @@ -1980,7 +1980,7 @@ where entity_id: EntityId, transaction_time: Option>, decision_time: Option>, - ) -> Result> { + ) -> Result, Report> { let policy_components = PolicyComponents::builder(self, actor_id) .with_action(ActionName::ViewEntity, MergePolicies::Yes) .await @@ -2000,13 +2000,14 @@ where } .resolve(); - Read::::read_one( + let entities = Read::::read( self, &filters, Some(&temporal_axes), entity_id.draft_id.is_some(), ) - .await + .await?; + core::pin::pin!(entities).try_next().await } #[tracing::instrument(level = "info", skip(self, params))] diff --git a/libs/@local/graph/store/rust/src/entity/store.rs b/libs/@local/graph/store/rust/src/entity/store.rs index a544a151aa7..62cf632325a 100644 --- a/libs/@local/graph/store/rust/src/entity/store.rs +++ b/libs/@local/graph/store/rust/src/entity/store.rs @@ -903,14 +903,20 @@ pub trait EntityStore { /// Get the latest edition of the [`Entity`], or the one live at the given timestamps. /// - /// [`None`] reads as the public actor. + /// An `actor_id` of [`None`] reads as the public actor. Returns [`None`] if the actor may not + /// view the entity or no edition of it is live at `transaction_time` and `decision_time`, each + /// of which defaults to now. + /// + /// # Errors + /// + /// - [`QueryError`] if the store cannot be queried fn get_entity_by_id( &self, actor_id: Option, entity_id: EntityId, transaction_time: Option>, decision_time: Option>, - ) -> impl Future>> + Send; + ) -> impl Future, Report>> + Send; fn patch_entity( &mut self, @@ -986,7 +992,11 @@ pub trait EntityStore { params.first_transaction_time, params.first_decision_time, ) - .await?; + .await? + .ok_or_else(|| { + Report::new(QueryError) + .attach(format!("entity `{}` was not found", params.first_entity_id)) + })?; let second_entity = self .get_entity_by_id( Some(actor_id), @@ -994,7 +1004,13 @@ pub trait EntityStore { params.second_transaction_time, params.second_decision_time, ) - .await?; + .await? + .ok_or_else(|| { + Report::new(QueryError).attach(format!( + "entity `{}` was not found", + params.second_entity_id + )) + })?; let property_diff = first_entity .properties diff --git a/libs/@local/graph/type-fetcher/src/store.rs b/libs/@local/graph/type-fetcher/src/store.rs index cdd313ea314..22f26e86d23 100644 --- a/libs/@local/graph/type-fetcher/src/store.rs +++ b/libs/@local/graph/type-fetcher/src/store.rs @@ -1678,7 +1678,7 @@ where entity_id: EntityId, transaction_time: Option>, decision_time: Option>, - ) -> Result> { + ) -> Result, Report> { self.store .get_entity_by_id(actor_id, entity_id, transaction_time, decision_time) .await diff --git a/tests/graph/integration/postgres/drafts.rs b/tests/graph/integration/postgres/drafts.rs index 1725cc5f560..0c5183c4256 100644 --- a/tests/graph/integration/postgres/drafts.rs +++ b/tests/graph/integration/postgres/drafts.rs @@ -78,7 +78,8 @@ fn charles() -> PropertyObject { async fn check_entity_exists(api: &DatabaseApi<'_>, id: EntityId) -> bool { api.get_entity_by_id(Some(api.account_id), id, None, None) .await - .is_ok() + .expect("should query the entity") + .is_some() } #[tokio::test] diff --git a/tests/graph/integration/postgres/entity.rs b/tests/graph/integration/postgres/entity.rs index f0d3ca56202..ad62a71e757 100644 --- a/tests/graph/integration/postgres/entity.rs +++ b/tests/graph/integration/postgres/entity.rs @@ -15,7 +15,7 @@ use hash_graph_temporal_versioning::{ClosedTemporalBound, LimitedTemporalBound, use hash_graph_test_data::{data_type, entity, entity_type, property_type}; use type_system::{ knowledge::{ - entity::provenance::ProvidedEntityEditionProvenance, + entity::{EntityId, id::EntityUuid, provenance::ProvidedEntityEditionProvenance}, property::{ Property, PropertyObject, PropertyObjectWithMetadata, PropertyPatchOperation, PropertyPath, PropertyWithMetadata, @@ -25,8 +25,9 @@ use type_system::{ principal::{actor::ActorType, actor_group::WebId}, provenance::{OriginProvenance, OriginType}, }; +use uuid::Uuid; -use crate::DatabaseTestWrapper; +use crate::{DatabaseApi, DatabaseTestWrapper}; #[tokio::test] async fn insert() { @@ -561,3 +562,111 @@ async fn update() { let entity_v2 = response_v2.entities.pop().expect("no entity found"); assert_eq!(entity_v2.properties.properties(), page_v2.properties()); } + +/// Seeds the organization type and creates an organization in the account's web, under no +/// policy of its own. +async fn create_organization(api: &mut DatabaseApi<'_>) -> EntityId { + let organization: PropertyObject = + serde_json::from_str(entity::ORGANIZATION_V1).expect("could not parse entity"); + api.create_entity( + api.account_id, + CreateEntityParams { + web_id: WebId::new(api.account_id), + entity_uuid: None, + decision_time: None, + entity_type_ids: HashSet::from([VersionedUrl { + base_url: BaseUrl::new( + "https://blockprotocol.org/@alice/types/entity-type/organization/".to_owned(), + ) + .expect("couldn't construct Base URL"), + version: OntologyTypeVersion { + major: OntologyTypeMajorVersion::MIN, + pre_release: None, + }, + }]), + properties: PropertyObjectWithMetadata::from_parts(organization, None) + .expect("could not create property with metadata object"), + confidence: None, + link_data: None, + draft: false, + policies: Vec::new(), + provenance: ProvidedEntityEditionProvenance { + actor_type: ActorType::User, + origin: OriginProvenance::from_empty_type(OriginType::Api), + sources: Vec::new(), + }, + read_only: false, + }, + ) + .await + .expect("could not create entity") + .metadata + .record_id + .entity_id +} + +/// An entity the actor may not view reads as absent, not as an error. +#[tokio::test] +async fn get_entity_by_id_unpermitted() { + let mut database = DatabaseTestWrapper::new().await; + let mut api = database + .seed( + [data_type::VALUE_V1, data_type::TEXT_V1], + [property_type::NAME_V1], + [entity_type::ORGANIZATION_V1], + ) + .await + .expect("could not seed database"); + let entity_id = create_organization(&mut api).await; + + let owned = api + .get_entity_by_id(Some(api.account_id), entity_id, None, None) + .await + .expect("the owner's read should succeed"); + assert_eq!( + owned.map(|entity| entity.metadata.record_id.entity_id), + Some(entity_id), + "the owner should see the entity in its web" + ); + + let hidden = api + .get_entity_by_id(None, entity_id, None, None) + .await + .expect("the public read should succeed"); + assert!( + hidden.is_none(), + "the public actor should not see an entity no policy permits it to view" + ); +} + +/// An entity that does not exist reads as absent, not as an error. +#[tokio::test] +async fn get_entity_by_id_missing() { + let mut database = DatabaseTestWrapper::new().await; + let mut api = database + .seed( + [data_type::VALUE_V1, data_type::TEXT_V1], + [property_type::NAME_V1], + [entity_type::ORGANIZATION_V1], + ) + .await + .expect("could not seed database"); + let entity_id = create_organization(&mut api).await; + + let missing = api + .get_entity_by_id( + Some(api.account_id), + EntityId { + entity_uuid: EntityUuid::new(Uuid::new_v4()), + ..entity_id + }, + None, + None, + ) + .await + .expect("the read should succeed"); + assert!( + missing.is_none(), + "an entity UUID nothing was created under should read as absent" + ); +} diff --git a/tests/graph/integration/postgres/lib.rs b/tests/graph/integration/postgres/lib.rs index b2022e29137..ec11c7c24bc 100644 --- a/tests/graph/integration/postgres/lib.rs +++ b/tests/graph/integration/postgres/lib.rs @@ -867,7 +867,7 @@ impl EntityStore for DatabaseApi<'_> { entity_id: EntityId, transaction_time: Option>, decision_time: Option>, - ) -> Result> { + ) -> Result, Report> { self.store .get_entity_by_id(actor_id, entity_id, transaction_time, decision_time) .await diff --git a/tests/graph/integration/postgres/partial_updates.rs b/tests/graph/integration/postgres/partial_updates.rs index 1eb9e01e816..08dcb71d891 100644 --- a/tests/graph/integration/postgres/partial_updates.rs +++ b/tests/graph/integration/postgres/partial_updates.rs @@ -654,7 +654,8 @@ async fn patch_entities_input_order() { None, ) .await - .expect("should read the patched entity"); + .expect("should read the patched entity") + .expect("the patched entity should exist"); assert_eq!( stored.metadata.record_id.edition_id, patched.metadata.record_id.edition_id, "should persist the returned edition" @@ -702,7 +703,8 @@ async fn patch_entities_missing_entity() { let stored = api .get_entity_by_id(Some(api.account_id), entity_id, None, None) .await - .expect("should read the original entity after rollback"); + .expect("should read the original entity after rollback") + .expect("the original entity should exist"); assert_eq!(stored, original, "should roll back the earlier patch"); } @@ -785,7 +787,8 @@ async fn patch_entities_duplicate_ids() { let stored = api .get_entity_by_id(Some(api.account_id), entity_id, None, None) .await - .expect("should read the final entity"); + .expect("should read the final entity") + .expect("the final entity should exist"); assert_eq!(stored, second, "should persist the combined patch result"); } @@ -844,7 +847,8 @@ async fn patch_entities_live_then_publish() { let stored = api .get_entity_by_id(Some(api.account_id), live_id, None, None) .await - .expect("should read the published entity"); + .expect("should read the published entity") + .expect("the published entity should exist"); assert_eq!(stored, published, "should persist the published edition"); let history = Read::::read_vec(&api.store, &[], None, true) .await @@ -898,7 +902,8 @@ async fn patch_entities_publish_existing_decision_start() { let stored = api .get_entity_by_id(Some(api.account_id), live_id, None, None) .await - .expect("should read the published entity"); + .expect("should read the published entity") + .expect("the published entity should exist"); assert_eq!(stored, published, "should persist the published edition"); assert_eq!( published.properties, draft.properties, @@ -992,7 +997,8 @@ async fn patch_entities_validation_index() { None, ) .await - .expect("should read the original entity after rollback"); + .expect("should read the original entity after rollback") + .expect("the original entity should exist"); assert_eq!(stored, original, "should roll back the whole invalid batch"); } } @@ -1039,7 +1045,8 @@ async fn patch_entities_unchanged_then_changed() { None, ) .await - .expect("should read the changed entity"); + .expect("should read the changed entity") + .expect("the changed entity should exist"); assert_eq!( stored, patched_changed, "should persist the patch after the unchanged entity" From 577f1056c89e82c631b096727ed630a1c7fb79d7 Mon Sep 17 00:00:00 2001 From: Tim Diekmann Date: Wed, 14 Oct 2026 22:18:55 +0200 Subject: [PATCH 02/13] Derive a JSON schema for `EntityId` --- .../type-system/rust/src/knowledge/entity/id.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/libs/@blockprotocol/type-system/rust/src/knowledge/entity/id.rs b/libs/@blockprotocol/type-system/rust/src/knowledge/entity/id.rs index 538ab5e7c4b..c3c00bec5a3 100644 --- a/libs/@blockprotocol/type-system/rust/src/knowledge/entity/id.rs +++ b/libs/@blockprotocol/type-system/rust/src/knowledge/entity/id.rs @@ -77,7 +77,16 @@ impl From for Uuid { } } +/// An entity's web ID and UUID, joined by `~`, optionally followed by `~` and a draft ID. #[derive(Debug, Clone, Copy, Hash, PartialEq, Eq)] +#[cfg_attr( + feature = "schemars", + derive(schemars::JsonSchema), + schemars( + with = "String", + example = "8d86c8c3-d66d-43d1-859b-676c3bcaeadc~be2c5b2f-e3e1-4f26-a0c4-0f8b5e9c4a1d" + ) +)] pub struct EntityId { pub web_id: WebId, pub entity_uuid: EntityUuid, From 5717beaee7c66a312b361d364e1f6fbe80feb310 Mon Sep 17 00:00:00 2001 From: Tim Diekmann Date: Wed, 14 Oct 2026 22:19:02 +0200 Subject: [PATCH 03/13] Add `GET /entities/v1/{entityId}` to the public Graph API --- Cargo.lock | 1 + libs/@local/graph/api/Cargo.toml | 1 + libs/@local/graph/api/src/rest/caller.rs | 12 +- libs/@local/graph/api/src/rest/extract/mod.rs | 7 +- .../graph/api/src/rest/extract/query.rs | 4 + .../snapshots/extract-example.snap.json | 29 -- .../graph/api/src/rest/extract/tests.rs | 8 +- .../@local/graph/api/src/rest/internal/mod.rs | 6 +- .../graph/api/src/rest/middleware/tests.rs | 59 ++- libs/@local/graph/api/src/rest/mod.rs | 84 ++++- libs/@local/graph/api/src/rest/openapi/mod.rs | 104 +++++- .../graph/api/src/rest/openapi/tests.rs | 136 ++++++- .../graph/api/src/rest/public/entities/mod.rs | 9 +- .../graph/api/src/rest/public/entities/v1.rs | 17 - .../api/src/rest/public/entities/v1/entity.rs | 151 ++++++++ .../api/src/rest/public/entities/v1/mod.rs | 23 ++ libs/@local/graph/api/src/rest/public/mod.rs | 10 +- .../graph/api/src/rest/public/types/mod.rs | 2 +- .../graph/api/src/rest/public/types/v1.rs | 6 +- libs/@local/graph/api/src/rest/router.rs | 9 +- libs/@local/graph/api/src/rest/routing.rs | 121 +++++++ .../api/src/rest/test_utils/authentication.rs | 22 +- .../graph/api/src/rest/test_utils/mod.rs | 48 ++- .../snapshots/openapi/entities-v1.snap.json | 340 ++++++++++++++++++ libs/@local/middleware/src/problem.rs | 31 ++ 25 files changed, 1126 insertions(+), 114 deletions(-) delete mode 100644 libs/@local/graph/api/src/rest/public/entities/v1.rs create mode 100644 libs/@local/graph/api/src/rest/public/entities/v1/entity.rs create mode 100644 libs/@local/graph/api/src/rest/public/entities/v1/mod.rs create mode 100644 libs/@local/graph/api/src/rest/routing.rs diff --git a/Cargo.lock b/Cargo.lock index 8965ac3df19..d256b525438 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6320,6 +6320,7 @@ dependencies = [ "simple-mermaid", "time", "tokio", + "tokio-postgres", "tokio-util", "tower", "tower-http", diff --git a/libs/@local/graph/api/Cargo.toml b/libs/@local/graph/api/Cargo.toml index 84d137118f7..d57c9525588 100644 --- a/libs/@local/graph/api/Cargo.toml +++ b/libs/@local/graph/api/Cargo.toml @@ -92,6 +92,7 @@ hash-middleware = { workspace = true, features = ["test-utils"] } insta = { workspace = true } rstest = { workspace = true } tokio = { workspace = true, features = ["macros", "rt-multi-thread", "test-util"] } +tokio-postgres = { workspace = true } tower = { workspace = true, features = ["util"] } uuid = { workspace = true, features = ["v4"] } diff --git a/libs/@local/graph/api/src/rest/caller.rs b/libs/@local/graph/api/src/rest/caller.rs index b45a8555e79..f738c819eb8 100644 --- a/libs/@local/graph/api/src/rest/caller.rs +++ b/libs/@local/graph/api/src/rest/caller.rs @@ -1,7 +1,4 @@ -use aide::{ - axum::{ApiRouter, routing::get_with}, - transform::TransformOperation, -}; +use aide::{axum::ApiRouter, transform::TransformOperation}; use schemars::JsonSchema; use serde::Serialize; use type_system::principal::actor::ActorId; @@ -9,6 +6,7 @@ use type_system::principal::actor::ActorId; use super::{ credentials::{Actor, Credentials, MaybeActor}, extract::Json, + routing, }; /// The resolved caller. @@ -51,15 +49,15 @@ fn document_authenticated_caller(operation: TransformOperation<'_>) -> Transform .summary("Get the authenticated caller") } -pub(super) fn routes() -> ApiRouter { +pub(super) fn routes() -> ApiRouter { ApiRouter::new() .api_route( "/caller", - get_with(caller::>, document_caller), + routing::get(caller::>, document_caller), ) .api_route( "/authenticated-caller", - get_with( + routing::get( authenticated_caller::>, document_authenticated_caller, ), diff --git a/libs/@local/graph/api/src/rest/extract/mod.rs b/libs/@local/graph/api/src/rest/extract/mod.rs index e4ba0fd4c3a..fee86a64d86 100644 --- a/libs/@local/graph/api/src/rest/extract/mod.rs +++ b/libs/@local/graph/api/src/rest/extract/mod.rs @@ -18,7 +18,12 @@ mod tests; use aide::openapi::Operation; use serde_json::Value; -pub(in crate::rest) use self::{json::Json, path::Path, query::Query}; +#[cfg_attr( + not(test), + expect(unused_imports, reason = "only the tests read query parameters so far") +)] +pub(in crate::rest) use self::query::Query; +pub(in crate::rest) use self::{json::Json, path::Path}; /// The part of a request or response that [`Json`], [`Path`] or [`Query`] reads or writes. /// diff --git a/libs/@local/graph/api/src/rest/extract/query.rs b/libs/@local/graph/api/src/rest/extract/query.rs index b45a460abbe..dbca62601a3 100644 --- a/libs/@local/graph/api/src/rest/extract/query.rs +++ b/libs/@local/graph/api/src/rest/extract/query.rs @@ -72,6 +72,10 @@ impl Expose for QueryRejection { /// serde hands flattened fields their values as strings, which only a string field accepts. /// /// [`openapi::build`]: crate::rest::openapi::build +#[cfg_attr( + not(test), + expect(dead_code, reason = "only the tests read query parameters so far") +)] pub(in crate::rest) struct Query(pub T); impl FromRequestParts for Query diff --git a/libs/@local/graph/api/src/rest/extract/snapshots/extract-example.snap.json b/libs/@local/graph/api/src/rest/extract/snapshots/extract-example.snap.json index 5a2ed5b84e7..949c000b41a 100644 --- a/libs/@local/graph/api/src/rest/extract/snapshots/extract-example.snap.json +++ b/libs/@local/graph/api/src/rest/extract/snapshots/extract-example.snap.json @@ -641,35 +641,6 @@ } }, "schemas": { - "EchoPath": { - "description": "The label the response repeats.", - "properties": { - "label": { - "description": "Repeated back as `label`.", - "type": "string" - } - }, - "required": [ - "label" - ], - "type": "object" - }, - "EchoQuery": { - "description": "How often the response repeats the value.", - "properties": { - "repeat": { - "description": "How many copies of `value` the response carries. Defaults to one.", - "format": "uint8", - "maximum": 255, - "minimum": 0, - "type": [ - "integer", - "null" - ] - } - }, - "type": "object" - }, "EchoRequest": { "description": "The value the response repeats.", "properties": { diff --git a/libs/@local/graph/api/src/rest/extract/tests.rs b/libs/@local/graph/api/src/rest/extract/tests.rs index 2b288841878..b98557364fd 100644 --- a/libs/@local/graph/api/src/rest/extract/tests.rs +++ b/libs/@local/graph/api/src/rest/extract/tests.rs @@ -3,7 +3,7 @@ use std::io; use aide::{ axum::{ ApiRouter, - routing::{get, post, post_with}, + routing::{get, post}, }, openapi::{Info, OpenApi, Operation, PathItem, Response, StatusCode}, }; @@ -17,7 +17,7 @@ use serde_json::{Value, json}; use tower::ServiceExt as _; use super::{Json, Path, Query, json::content_status}; -use crate::rest::{openapi, test_utils::NoCredentials}; +use crate::rest::{Api, openapi, routing, test_utils::NoCredentials}; #[derive(Deserialize, JsonSchema)] struct Subject { @@ -500,7 +500,7 @@ async fn echo( /// snapshot shows how its rejections join the responses of the middleware. #[test] fn echo_document() { - let api = openapi::build::( + let api: Api = openapi::build::( "/example", Info { title: "Extractor example".to_owned(), @@ -510,7 +510,7 @@ fn echo_document() { || { ApiRouter::new().api_route( "/echo/{label}", - post_with(echo, |operation| { + routing::post(echo, |operation| { operation.id("echo").summary("Echo a request").description( "Returns the label from the path and the value from the body, repeated as \ often as the query parameter `repeat` asks.", diff --git a/libs/@local/graph/api/src/rest/internal/mod.rs b/libs/@local/graph/api/src/rest/internal/mod.rs index 7144c5aa00f..49a85e7b66c 100644 --- a/libs/@local/graph/api/src/rest/internal/mod.rs +++ b/libs/@local/graph/api/src/rest/internal/mod.rs @@ -8,8 +8,8 @@ use super::{Api, caller, documentation, openapi}; /// The path the internal API, its documentation and the legacy document are served under. pub(super) const PREFIX: &str = "/_api"; -pub(super) fn api() -> Api { - openapi::build::( +pub(super) fn api() -> Api { + openapi::build::( PREFIX, Info { title: "Internal".to_owned(), @@ -17,7 +17,7 @@ pub(super) fn api() -> Api { description: Some(documentation::OVERVIEW.to_owned()), ..Info::default() }, - caller::routes::, + caller::routes::, document, ) } diff --git a/libs/@local/graph/api/src/rest/middleware/tests.rs b/libs/@local/graph/api/src/rest/middleware/tests.rs index 7df57ab2c37..1bfd1f3f475 100644 --- a/libs/@local/graph/api/src/rest/middleware/tests.rs +++ b/libs/@local/graph/api/src/rest/middleware/tests.rs @@ -9,7 +9,7 @@ use hash_middleware::{ authentication::{ AuthenticatedActorId, AuthenticationMetrics, provider::StaticAuthenticationProvider, }, - problem::InternalServerError, + problem::{InternalServerError, ServiceUnavailable}, rate_limit::{ClientIpSource, RateLimitConfig, RateLimitMode, RateLimiters}, }; use hash_secret::Secret; @@ -17,6 +17,7 @@ use http::{ HeaderValue, Method, Request, StatusCode, header::{ALLOW, AUTHORIZATION, CACHE_CONTROL, CONTENT_TYPE, WWW_AUTHENTICATE}, }; +use problematic::Rejected; use serde_json::json; use tower::ServiceExt as _; use type_system::principal::actor::{ActorId, ActorType}; @@ -520,6 +521,62 @@ async fn panic_problem_document() { ); } +/// The entity route reads the store it was handed, and answers `500` when it cannot reach it. +#[tokio::test] +async fn entity_store_unreachable() { + let router = middleware( + &config(10, 10), + StaticAuthenticationProvider::Verified(ActorId::new(Uuid::from_u128(1), ActorType::User)), + StaticAuthenticationProvider::Unreachable, + StaticAuthenticationProvider::Unreachable, + ) + .assemble(Router::new(), apis(), Router::new()); + + let response = send( + &router, + "/entities/v1/8d86c8c3-d66d-43d1-859b-676c3bcaeadc~be2c5b2f-e3e1-4f26-a0c4-0f8b5e9c4a1d", + ) + .await; + assert_eq!( + response.status(), + StatusCode::SERVICE_UNAVAILABLE, + "the route should answer 503 when it cannot reach the store" + ); + assert!( + response.extensions().get::().is_some(), + "the 503 should be the route's rejection, which a panic does not leave" + ); + assert_eq!( + response_json(response).await["detail"], + json!(ServiceUnavailable.to_string()), + "the problem document should not name the store" + ); +} + +/// The entity route requires an actor, so an anonymous request is answered `401` before it reads +/// the store. +#[tokio::test] +async fn entity_anonymous() { + let router = middleware( + &config(10, 10), + StaticAuthenticationProvider::NotRecognized, + StaticAuthenticationProvider::Unreachable, + StaticAuthenticationProvider::Unreachable, + ) + .assemble(Router::new(), apis(), Router::new()); + + let response = send( + &router, + "/entities/v1/8d86c8c3-d66d-43d1-859b-676c3bcaeadc~be2c5b2f-e3e1-4f26-a0c4-0f8b5e9c4a1d", + ) + .await; + assert_eq!( + response.status(), + StatusCode::UNAUTHORIZED, + "an anonymous request should be answered 401" + ); +} + #[tokio::test] async fn apis_and_legacy_share_one_actor_budget() { let actor = ActorId::new(Uuid::from_u128(1), ActorType::User); diff --git a/libs/@local/graph/api/src/rest/mod.rs b/libs/@local/graph/api/src/rest/mod.rs index 9d15784e35e..8097f3dd3b4 100644 --- a/libs/@local/graph/api/src/rest/mod.rs +++ b/libs/@local/graph/api/src/rest/mod.rs @@ -2,14 +2,6 @@ pub mod authentication; mod caller; mod credentials; mod documentation; -#[cfg_attr( - not(test), - expect( - dead_code, - unused_imports, - reason = "only the tests read requests through these extractors" - ) -)] mod extract; mod internal; pub mod legacy; @@ -19,14 +11,20 @@ pub mod probe; mod public; pub mod rate_limit; mod router; +mod routing; pub mod telemetry; #[cfg(test)] pub(crate) mod test_utils; +use alloc::sync::Arc; + use aide::openapi::OpenApi; use axum::Router; +use error_stack::Report; +use hash_graph_store::pool::StorePool; use hash_middleware::authentication::Challenge; +use hash_temporal_client::TemporalClient; pub use self::router::{Dependencies, router}; @@ -37,16 +35,60 @@ pub(crate) enum Audience { Internal, } +/// What the handlers of the public and internal APIs read through [`State`]. +/// +/// [`State`]: axum::extract::State +pub(crate) struct ApiState { + store: Arc, + temporal_client: Option>, +} + +impl ApiState { + pub(crate) const fn new(store: Arc, temporal_client: Option>) -> Self { + Self { + store, + temporal_client, + } + } +} + +impl ApiState { + /// Acquires a store from the pool. + /// + /// The store starts the workflows that follow a write, such as computing embeddings, through + /// the Temporal client. + /// + /// # Errors + /// + /// Returns the pool's error if no store can be acquired. + pub(crate) fn acquire( + &self, + ) -> impl Future, Report>> + Send { + self.store.acquire(self.temporal_client.clone()) + } +} + +impl Clone for ApiState { + fn clone(&self) -> Self { + Self { + store: Arc::clone(&self.store), + temporal_client: self.temporal_client.clone(), + } + } +} + /// An API's routes, nested under its prefix, with the document describing them. -pub(crate) struct Api { +/// +/// The routes read the state `T`, which [`with_state`](Self::with_state) hands them. +pub(crate) struct Api { audience: Audience, challenge: Challenge, prefix: &'static str, - router: Router, + router: Router, document: OpenApi, } -impl Api { +impl Api { pub(crate) const fn prefix(&self) -> &'static str { self.prefix } @@ -61,6 +103,22 @@ impl Api { } } -pub(crate) fn apis() -> impl Iterator { - public::apis().chain([internal::api()]) +impl Api { + /// The API whose routes read `state`. + pub(crate) fn with_state(self, state: T) -> Api { + Api { + audience: self.audience, + challenge: self.challenge, + prefix: self.prefix, + router: self.router.with_state(state), + document: self.document, + } + } +} + +pub(crate) fn apis() -> impl Iterator>> +where + S: StorePool + Send + Sync + 'static, +{ + public::apis::().chain([internal::api()]) } diff --git a/libs/@local/graph/api/src/rest/openapi/mod.rs b/libs/@local/graph/api/src/rest/openapi/mod.rs index e9854613846..c494da42e0a 100644 --- a/libs/@local/graph/api/src/rest/openapi/mod.rs +++ b/libs/@local/graph/api/src/rest/openapi/mod.rs @@ -2,6 +2,7 @@ mod tests; use alloc::collections::BTreeSet; +use core::mem; use std::collections::HashSet; use aide::{ @@ -19,7 +20,7 @@ use convert_case::{Case, Casing as _}; use indexmap::IndexMap; use serde_json::Value; -use super::{Api, credentials::Credentials, extract::MessagePart, middleware}; +use super::{Api, credentials::Credentials, extract::MessagePart, middleware, routing}; /// Builds an API from its routes and generates the document describing them. /// @@ -41,16 +42,19 @@ use super::{Api, credentials::Credentials, extract::MessagePart, middleware}; /// parameter. /// - The problem variants of an operation cannot be documented, such as when the status of a /// variant already has a response that documents none. +/// - An operation was registered other than through [`routing`], or shares its `operationId` with +/// another operation of the document. /// /// [`Json`]: crate::rest::extract::Json /// [`Path`]: crate::rest::extract::Path /// [`Query`]: crate::rest::extract::Query -pub(super) fn build( +/// [`routing`]: crate::rest::routing +pub(super) fn build( prefix: &'static str, info: Info, - build_routes: impl FnOnce() -> ApiRouter, + build_routes: impl FnOnce() -> ApiRouter, transform: impl FnOnce(TransformOpenApi<'_>) -> TransformOpenApi<'_>, -) -> Api { +) -> Api { #[expect( clippy::wildcard_enum_match_arm, reason = "Every documentation error apart from the inferred-response conflicts is a \ @@ -80,6 +84,10 @@ pub(super) fn build( assert_security_schemes_declared(&mut document); assert_parameters_readable(&mut document); assert_read_through_extractors(&mut document); + // The tests of the checks above register their operations through aide directly, so this + // check runs after them. + assert_routed(&mut document); + remove_unreferenced_schemas(&mut document); Api { audience: C::AUDIENCE, challenge: C::CHALLENGE, @@ -294,6 +302,94 @@ fn assert_read_through_extractors(document: &mut OpenApi) { } } +/// Checks that [`routing`] registered every operation and that no two operations share an +/// `operationId`, and removes the marks. +/// +/// [`routing`]: crate::rest::routing +fn assert_routed(document: &mut OpenApi) { + let Some(paths) = &mut document.paths else { + return; + }; + let mut ids = HashSet::new(); + for (path, item) in &mut paths.paths { + let Some(item) = item.as_item_mut() else { + continue; + }; + for (method, operation) in iter_operations_mut(item) { + assert!( + operation + .extensions + .shift_remove(routing::EXTENSION) + .is_some(), + "{method} {path} should be registered through `rest::routing`, which admits only \ + handlers answering with `rest::extract::Json` or a `Rejection`" + ); + if let Some(id) = &operation.operation_id { + assert!( + ids.insert(id.clone()), + "{method} {path} should have an `operationId` of its own, another operation \ + already has `{id}`" + ); + } + } + } +} + +/// Removes the component schemas that nothing else in the document references, directly or +/// through other component schemas. +/// +/// Aide adds a component for the struct a handler reads its path or query parameters into, though +/// the parameters inline its fields. +fn remove_unreferenced_schemas(document: &mut OpenApi) { + let Some(components) = &mut document.components else { + return; + }; + let schemas = mem::take(&mut components.schemas); + let mut pending = Vec::new(); + schema_references( + &serde_json::to_value(&*document).expect("the document should be a JSON value"), + &mut pending, + ); + let mut referenced = HashSet::new(); + while let Some(name) = pending.pop() { + if let Some(schema) = schemas.get(name.as_str()) + && referenced.insert(name) + { + schema_references(schema.json_schema.as_value(), &mut pending); + } + } + if let Some(components) = &mut document.components { + components.schemas = schemas + .into_iter() + .filter(|(name, _)| referenced.contains(name)) + .collect(); + } +} + +/// Appends the name of every component schema that `value` references to `references`. +fn schema_references(value: &Value, references: &mut Vec) { + match value { + Value::Object(object) => { + if let Some(name) = object + .get("$ref") + .and_then(Value::as_str) + .and_then(|reference| reference.strip_prefix("#/components/schemas/")) + { + references.push(name.to_owned()); + } + for member in object.values() { + schema_references(member, references); + } + } + Value::Array(elements) => { + for element in elements { + schema_references(element, references); + } + } + Value::Null | Value::Bool(_) | Value::Number(_) | Value::String(_) => {} + } +} + /// The schema of `parameter`, unless it describes its value by media type instead. fn parameter_schema(parameter: &ParameterData) -> Option<&Value> { if let ParameterSchemaOrContent::Schema(schema) = ¶meter.format { diff --git a/libs/@local/graph/api/src/rest/openapi/tests.rs b/libs/@local/graph/api/src/rest/openapi/tests.rs index 5fa3986a28a..775995178c5 100644 --- a/libs/@local/graph/api/src/rest/openapi/tests.rs +++ b/libs/@local/graph/api/src/rest/openapi/tests.rs @@ -1,4 +1,4 @@ -use alloc::borrow::Cow; +use alloc::{borrow::Cow, collections::BTreeSet}; use core::assert_matches; use aide::{ @@ -18,8 +18,8 @@ use problematic::{Problem, ProblemDetails, ProblemType, ProblemVariant, Rejectio use super::reference_responses; use crate::rest::{ Api, - extract::{Path, Query}, - middleware, openapi, + extract::{Json, Path, Query}, + middleware, openapi, routing, test_utils::NoCredentials, }; @@ -129,7 +129,7 @@ fn component_names_keep_to_alphanumeric_keys() { #[test] #[should_panic(expected = "should generate without errors")] fn build_duplicate_response() { - let _: Api = openapi::build::( + let _: Api = openapi::build::( "/test", Info::default(), || { @@ -159,7 +159,7 @@ struct DraftPath { #[test] #[should_panic(expected = "should document one path parameter per placeholder")] fn build_path_placeholder_undocumented() { - let _: Api = openapi::build::( + let _: Api = openapi::build::( "/test", Info::default(), || { @@ -175,7 +175,7 @@ fn build_path_placeholder_undocumented() { #[test] #[should_panic(expected = "should document one path parameter per placeholder")] fn build_path_parameter_without_placeholder() { - let _: Api = openapi::build::( + let _: Api = openapi::build::( "/test", Info::default(), || { @@ -228,13 +228,16 @@ fn build_entity_route(read: fn(T) -> String) where T: serde::de::DeserializeOwned + schemars::JsonSchema + Send + 'static, { - let _: Api = openapi::build::( + let _: Api = openapi::build::( "/test", Info::default(), || { ApiRouter::new().api_route( "/entities/{entity}", - get(async move |Path(parameters): Path| read(parameters)), + routing::get( + async move |Path(parameters): Path| Json::<_, 200>(read(parameters)), + |operation| operation.id("getEntity").summary("Get an entity"), + ), ) }, |document| document, @@ -316,13 +319,16 @@ fn build_query_route(read: fn(T) -> String) where T: serde::de::DeserializeOwned + schemars::JsonSchema + Send + 'static, { - let _: Api = openapi::build::( + let _: Api = openapi::build::( "/test", Info::default(), || { ApiRouter::new().api_route( "/entities", - get(async move |Query(parameters): Query| read(parameters)), + routing::get( + async move |Query(parameters): Query| Json::<_, 200>(read(parameters)), + |operation| operation.id("listEntities").summary("List entities"), + ), ) }, |document| document, @@ -370,7 +376,7 @@ fn build_query_parameter_values_accepted() { #[test] #[should_panic(expected = "should read its path parameter `entity` through `rest::extract::Path`")] fn build_axum_path() { - let _: Api = openapi::build::( + let _: Api = openapi::build::( "/test", Info::default(), || { @@ -390,7 +396,7 @@ fn build_axum_path() { #[test] #[should_panic(expected = "should read its query parameter `ids` through `rest::extract::Query`")] fn build_axum_extra_query() { - let _: Api = openapi::build::( + let _: Api = openapi::build::( "/test", Info::default(), || { @@ -410,7 +416,7 @@ fn build_axum_extra_query() { #[test] #[should_panic(expected = "should read its request body through `rest::extract::Json`")] fn build_bytes_body() { - let _: Api = openapi::build::( + let _: Api = openapi::build::( "/test", Info::default(), || ApiRouter::new().api_route("/entities", post(async |body: bytes::Bytes| body)), @@ -419,7 +425,7 @@ fn build_bytes_body() { } fn build_with_parameter(parameter: Parameter) { - let _: Api = openapi::build::( + let _: Api = openapi::build::( "/test", Info::default(), move || { @@ -480,10 +486,110 @@ fn build_cookie_parameter() { #[test] #[should_panic(expected = "should answer with JSON through `rest::extract::Json`")] fn build_axum_json_response() { - let _: Api = openapi::build::( + let _: Api = openapi::build::( "/test", Info::default(), || ApiRouter::new().api_route("/entities", get(async || axum::Json(0_u8))), |document| document, ); } + +#[test] +#[should_panic(expected = "should be registered through `rest::routing`")] +fn build_unrouted() { + let _: Api = openapi::build::( + "/test", + Info::default(), + || { + ApiRouter::new().api_route( + "/entities", + get_with( + async || Json::<_, 200>(0_u8), + |operation| operation.id("listEntities").summary("List entities"), + ), + ) + }, + |document| document, + ); +} + +#[test] +#[should_panic( + expected = "should have an `operationId` of its own, another operation already has `getEntity`" +)] +fn build_duplicate_operation_id() { + let _: Api = openapi::build::( + "/test", + Info::default(), + || { + let route = || { + routing::get( + async || Json::<_, 200>(0_u8), + |operation| operation.id("getEntity").summary("Get an entity"), + ) + }; + ApiRouter::new() + .api_route("/first", route()) + .api_route("/second", route()) + }, + |document| document, + ); +} + +/// Path parameters with a field whose schema other schemas reference. +#[derive(serde::Deserialize, schemars::JsonSchema)] +struct EntityPath { + entity: EntityUuid, +} + +/// A response with a field whose schema other schemas reference. +#[derive(serde::Serialize, schemars::JsonSchema)] +struct EntityResponse { + entity: EntityName, +} + +/// A name the path and the response share. +#[derive(serde::Serialize, schemars::JsonSchema)] +struct EntityName(String); + +/// The schemas of parameter structs go, the schemas that the operations, the response components +/// or other kept schemas reference stay. +#[test] +fn build_unreferenced_schemas() { + let api: Api = openapi::build::( + "/test", + Info::default(), + || { + ApiRouter::new().api_route( + "/entities/{entity}", + routing::get( + async |Path(EntityPath { entity }): Path| { + Json::<_, 200>(EntityResponse { + entity: EntityName(entity.0), + }) + }, + |operation| operation.id("getEntity").summary("Get an entity"), + ), + ) + }, + |document| document, + ); + + let schemas = api + .document() + .components + .as_ref() + .map_or_else(BTreeSet::new, |components| { + components.schemas.keys().map(String::as_str).collect() + }); + assert_eq!( + schemas, + BTreeSet::from([ + "EntityName", + "EntityResponse", + "EntityUuid", + "ProblemDetails" + ]), + "only the schemas the document references should remain" + ); +} diff --git a/libs/@local/graph/api/src/rest/public/entities/mod.rs b/libs/@local/graph/api/src/rest/public/entities/mod.rs index dd16c159b96..288cff87f14 100644 --- a/libs/@local/graph/api/src/rest/public/entities/mod.rs +++ b/libs/@local/graph/api/src/rest/public/entities/mod.rs @@ -1,7 +1,12 @@ mod v1; -use crate::rest::Api; +use hash_graph_store::pool::StorePool; -pub(super) fn api() -> [Api; 1] { +use crate::rest::{Api, ApiState}; + +pub(super) fn api() -> [Api>; 1] +where + S: StorePool + Send + Sync + 'static, +{ [v1::api()] } diff --git a/libs/@local/graph/api/src/rest/public/entities/v1.rs b/libs/@local/graph/api/src/rest/public/entities/v1.rs deleted file mode 100644 index 70738544670..00000000000 --- a/libs/@local/graph/api/src/rest/public/entities/v1.rs +++ /dev/null @@ -1,17 +0,0 @@ -use aide::openapi::Info; - -use crate::rest::{Api, caller, documentation, openapi, public}; - -pub(super) fn api() -> Api { - openapi::build::( - "/entities/v1", - Info { - title: "Entities".to_owned(), - version: "1".to_owned(), - description: Some(documentation::OVERVIEW.to_owned()), - ..Info::default() - }, - caller::routes::, - public::document, - ) -} diff --git a/libs/@local/graph/api/src/rest/public/entities/v1/entity.rs b/libs/@local/graph/api/src/rest/public/entities/v1/entity.rs new file mode 100644 index 00000000000..50238806da4 --- /dev/null +++ b/libs/@local/graph/api/src/rest/public/entities/v1/entity.rs @@ -0,0 +1,151 @@ +//! `GET /{entityId}`: one entity by its ID. + +use alloc::borrow::Cow; +use core::error::Error; + +use aide::{axum::ApiRouter, transform::TransformOperation}; +use axum::extract::State; +use error_stack::{Report, ResultExt as _}; +use hash_graph_store::{entity::EntityStore as _, pool::StorePool}; +use hash_middleware::problem::{InternalServerError, ServiceUnavailable}; +use http::StatusCode; +use problematic::{Answer, Expose, Problem, ProblemType, ProblemVariant, Rejection, Variant}; +use schemars::JsonSchema; +use serde::{Deserialize, Serialize}; +use type_system::{ + knowledge::entity::{EntityId, id::EntityUuid}, + principal::actor_group::WebId, +}; +use uuid::Uuid; + +use crate::rest::{ + ApiState, + credentials::Actor, + extract::{Json, Path}, + public::Credentials, + routing, +}; + +/// The path of one entity. +#[derive(Deserialize, JsonSchema)] +#[serde(rename_all = "camelCase")] +struct EntityPath { + /// The ID of the entity to read. + entity_id: EntityId, +} + +/// An entity the caller may view. +#[derive(Serialize, JsonSchema)] +#[serde(rename_all = "camelCase")] +struct Entity { + /// The ID of the entity. + entity_id: EntityId, +} + +/// The entity does not exist, or the caller may not view it. +#[derive(Serialize, JsonSchema, derive_more::Display)] +#[display("No entity `{entity_id}` is visible to the caller.")] +struct EntityNotFound { + #[serde(skip)] + entity_id: EntityId, +} + +impl ProblemVariant for EntityNotFound { + const TYPE: ProblemType = ProblemType { + type_uri: Cow::Borrowed("about:blank"), + title: Cow::Borrowed("Not Found"), + status: StatusCode::NOT_FOUND, + }; + + fn example() -> Option { + Some(Self { + entity_id: EntityId { + web_id: WebId::new(Uuid::from_u128(0x8D86_C8C3_D66D_43D1_859B_676C_3BCA_EADC)), + entity_uuid: EntityUuid::new(Uuid::from_u128( + 0xBE2C_5B2F_E3E1_4F26_A0C4_0F8B_5E9C_4A1D, + )), + draft_id: None, + }, + }) + } +} + +/// The ways `GET /{entityId}` refuses a request. +struct GetEntityProblem; + +impl Problem for GetEntityProblem { + const VARIANTS: &'static [Variant] = &[ + Variant::of::(), + Variant::of::(), + Variant::of::(), + ]; +} + +#[derive(Debug, derive_more::Display)] +enum GetEntityError { + #[display("no entity `{_0}` is visible to the caller")] + NotFound(EntityId), + #[display("no store could be acquired")] + Unavailable, + #[display("the store could not be read")] + Store, +} + +impl Error for GetEntityError {} + +impl Expose for Report { + fn expose(&self) -> Answer<'_, GetEntityProblem> { + match self.current_context() { + GetEntityError::NotFound(entity_id) => Answer::new(EntityNotFound { + entity_id: *entity_id, + }), + GetEntityError::Unavailable => Answer::new(ServiceUnavailable), + GetEntityError::Store => Answer::new(InternalServerError), + } + } +} + +async fn get_entity( + State(state): State>, + actor: Actor, + Path(EntityPath { entity_id }): Path, +) -> Result, Rejection> +where + S: StorePool + Send + Sync + 'static, +{ + let store = state + .acquire() + .await + .change_context(GetEntityError::Unavailable)?; + let entity = store + .get_entity_by_id(Some(actor.into()), entity_id, None, None) + .await + .change_context(GetEntityError::Store)? + .ok_or_else(|| Report::new(GetEntityError::NotFound(entity_id)))?; + + Ok(Json(Entity { + entity_id: entity.metadata.record_id.entity_id, + })) +} + +fn document_get_entity(operation: TransformOperation<'_>) -> TransformOperation<'_> { + operation + .id("getEntity") + .summary("Get an entity") + .description( + "Returns the current edition of the entity, or of the draft the ID names. Answers \ + `404` both when the entity does not exist and when the caller may not view it.", + ) +} + +pub(super) fn routes() -> ApiRouter> +where + S: StorePool + Send + Sync + 'static, +{ + ApiRouter::new() + .api_route( + "/{entityId}", + routing::get(get_entity::, document_get_entity), + ) + .with_path_items(|path| path.tag("Entity")) +} diff --git a/libs/@local/graph/api/src/rest/public/entities/v1/mod.rs b/libs/@local/graph/api/src/rest/public/entities/v1/mod.rs new file mode 100644 index 00000000000..4f1af2b33cd --- /dev/null +++ b/libs/@local/graph/api/src/rest/public/entities/v1/mod.rs @@ -0,0 +1,23 @@ +mod entity; + +use aide::openapi::Info; +use hash_graph_store::pool::StorePool; + +use crate::rest::{Api, ApiState, caller, documentation, openapi, public}; + +pub(super) fn api() -> Api> +where + S: StorePool + Send + Sync + 'static, +{ + openapi::build::( + "/entities/v1", + Info { + title: "Entities".to_owned(), + version: "1".to_owned(), + description: Some(documentation::OVERVIEW.to_owned()), + ..Info::default() + }, + || caller::routes::().merge(entity::routes::()), + public::document, + ) +} diff --git a/libs/@local/graph/api/src/rest/public/mod.rs b/libs/@local/graph/api/src/rest/public/mod.rs index a4b57625c17..f21183749a6 100644 --- a/libs/@local/graph/api/src/rest/public/mod.rs +++ b/libs/@local/graph/api/src/rest/public/mod.rs @@ -4,12 +4,16 @@ mod entities; mod types; use aide::transform::TransformOpenApi; +use hash_graph_store::pool::StorePool; use self::credentials::Credentials; -use super::Api; +use super::{Api, ApiState}; -pub(super) fn apis() -> impl Iterator { - entities::api().into_iter().chain(types::api()) +pub(super) fn apis() -> impl Iterator>> +where + S: StorePool + Send + Sync + 'static, +{ + entities::api::().into_iter().chain(types::api()) } const fn document(document: TransformOpenApi<'_>) -> TransformOpenApi<'_> { diff --git a/libs/@local/graph/api/src/rest/public/types/mod.rs b/libs/@local/graph/api/src/rest/public/types/mod.rs index dd16c159b96..21d9594d1a7 100644 --- a/libs/@local/graph/api/src/rest/public/types/mod.rs +++ b/libs/@local/graph/api/src/rest/public/types/mod.rs @@ -2,6 +2,6 @@ mod v1; use crate::rest::Api; -pub(super) fn api() -> [Api; 1] { +pub(super) fn api() -> [Api; 1] { [v1::api()] } diff --git a/libs/@local/graph/api/src/rest/public/types/v1.rs b/libs/@local/graph/api/src/rest/public/types/v1.rs index bffbe231a8e..e3508251fcf 100644 --- a/libs/@local/graph/api/src/rest/public/types/v1.rs +++ b/libs/@local/graph/api/src/rest/public/types/v1.rs @@ -2,8 +2,8 @@ use aide::openapi::Info; use crate::rest::{Api, caller, documentation, openapi, public}; -pub(super) fn api() -> Api { - openapi::build::( +pub(super) fn api() -> Api { + openapi::build::( "/types/v1", Info { title: "Types".to_owned(), @@ -11,7 +11,7 @@ pub(super) fn api() -> Api { description: Some(documentation::OVERVIEW.to_owned()), ..Info::default() }, - caller::routes::, + caller::routes::, public::document, ) } diff --git a/libs/@local/graph/api/src/rest/router.rs b/libs/@local/graph/api/src/rest/router.rs index 85d6913e45d..c45638f7453 100644 --- a/libs/@local/graph/api/src/rest/router.rs +++ b/libs/@local/graph/api/src/rest/router.rs @@ -17,6 +17,7 @@ use tower::ServiceBuilder; use type_system::ontology::json_schema::DomainValidator; use super::{ + ApiState, authentication::{self, api_token::ApiTokenRejection}, documentation, legacy::{self, ApiConfig, QueryLogger, RestApiStore, entity::ClusteringContext, hashql}, @@ -83,7 +84,13 @@ where rate_limiters: RateLimiters::start(&rate_limit_config, &dependencies.meter), }; - let apis = super::apis().collect::>(); + let state = ApiState::new( + Arc::clone(&dependencies.store), + dependencies.temporal_client.clone(), + ); + let apis = super::apis::() + .map(|api| api.with_state(state.clone())) + .collect::>(); let documentation = documentation::routes(&apis); // The tracing layer sits inside the Sentry layers: the errors it logs reach Sentry through the // request's hub, which attaches the request to them. diff --git a/libs/@local/graph/api/src/rest/routing.rs b/libs/@local/graph/api/src/rest/routing.rs new file mode 100644 index 00000000000..48132390695 --- /dev/null +++ b/libs/@local/graph/api/src/rest/routing.rs @@ -0,0 +1,121 @@ +//! [`get`] and [`post`] register the operations of the public and internal APIs. +//! +//! They accept only a handler answering with [`Json`] or with `Result, Rejection<_>>`, +//! so a successful response is JSON that [`Json`] documents and an error is a problem details +//! response that the [`Problem`](problematic::Problem) of the [`Rejection`] documents. A handler +//! answering with anything else, such as a bare `StatusCode`, fails to compile. Each marks the +//! operations it registers, and [`openapi::build`] panics for an operation registered any other +//! way. +//! +//! [`openapi::build`]: crate::rest::openapi::build + +use aide::{ + OperationInput, OperationOutput, + axum::routing::{self, ApiMethodRouter}, + operation::OperationHandler, + transform::TransformOperation, +}; +use axum::handler::Handler; +use problematic::Rejection; +use serde_json::Value; + +use super::extract::Json; + +/// The key of the extension that marks an operation [`get`] or [`post`] registered, which +/// [`openapi::build`] removes again. +/// +/// [`openapi::build`]: crate::rest::openapi::build +pub(in crate::rest) const EXTENSION: &str = "x-hash-routed"; + +/// A response a handler of the public and internal APIs answers with. +pub(in crate::rest) trait Outcome: OperationOutput {} + +impl Outcome for Json where Self: OperationOutput {} + +impl Outcome for Result, Rejection> where + Self: OperationOutput +{ +} + +/// Routes `GET` requests to `handler`, documented by `transform`. +/// +/// # Panics +/// +/// Panics if `transform` leaves the operation without an `operationId` or a `summary`. +pub(in crate::rest) fn get( + handler: H, + transform: impl FnOnce(TransformOperation<'_>) -> TransformOperation<'_>, +) -> ApiMethodRouter +where + H: Handler + OperationHandler, + I: OperationInput, + O: Outcome, + S: Clone + Send + Sync + 'static, + T: 'static, +{ + routing::get_with(handler, |operation| registered(transform(operation))) +} + +/// Routes `POST` requests to `handler`, documented by `transform`. +/// +/// # Panics +/// +/// Panics if `transform` leaves the operation without an `operationId` or a `summary`. +#[cfg_attr( + not(test), + expect(dead_code, reason = "only the tests route `POST` requests so far") +)] +pub(in crate::rest) fn post( + handler: H, + transform: impl FnOnce(TransformOperation<'_>) -> TransformOperation<'_>, +) -> ApiMethodRouter +where + H: Handler + OperationHandler, + I: OperationInput, + O: Outcome, + S: Clone + Send + Sync + 'static, + T: 'static, +{ + routing::post_with(handler, |operation| registered(transform(operation))) +} + +/// Checks that `operation` has an `operationId` and a `summary`, and marks it. +fn registered(mut operation: TransformOperation<'_>) -> TransformOperation<'_> { + let inner = operation.inner_mut(); + let id = inner + .operation_id + .as_deref() + .expect("the operation should have an `operationId`"); + assert!( + inner.summary.is_some(), + "the operation `{id}` should have a summary" + ); + inner + .extensions + .insert(EXTENSION.to_owned(), Value::Bool(true)); + operation +} + +#[cfg(test)] +mod tests { + use super::get; + use crate::rest::extract::Json; + + #[test] + #[should_panic(expected = "the operation should have an `operationId`")] + fn get_missing_id() { + let _: aide::axum::routing::ApiMethodRouter = get( + async || Json::<_, 200>(true), + |operation| operation.summary("Answer"), + ); + } + + #[test] + #[should_panic(expected = "the operation `answer` should have a summary")] + fn get_missing_summary() { + let _: aide::axum::routing::ApiMethodRouter = get( + async || Json::<_, 200>(true), + |operation| operation.id("answer"), + ); + } +} diff --git a/libs/@local/graph/api/src/rest/test_utils/authentication.rs b/libs/@local/graph/api/src/rest/test_utils/authentication.rs index 69d0e6d4027..5525dda5645 100644 --- a/libs/@local/graph/api/src/rest/test_utils/authentication.rs +++ b/libs/@local/graph/api/src/rest/test_utils/authentication.rs @@ -3,7 +3,7 @@ use core::{num::NonZeroU32, ops::ControlFlow, pin::pin}; use std::collections::HashMap; use aide::{ - axum::{ApiRouter, routing::get}, + axum::ApiRouter, openapi::{Info, ReferenceOr, SecurityScheme}, }; use axum::{Router, body::Body}; @@ -44,7 +44,7 @@ use super::{echo_caller, response_json}; use crate::rest::{ credentials::{API_TOKEN, Actor, Credentials, MaybeActor}, middleware::Middleware, - openapi, + openapi, routing, }; const SERVICE_SECRET: &str = "hash-svc-test-secret"; @@ -149,15 +149,27 @@ fn caller_router( rate_limit_actor_per_hour: NonZeroU32::MAX, rate_limit_actor_burst: NonZeroU32::MAX, }; - let api = openapi::build::( + let api = openapi::build::( "/test", Info::default(), || { ApiRouter::new() - .api_route("/optional", get(echo_caller::>)) + .api_route( + "/optional", + routing::get(echo_caller::>, |operation| { + operation.id("getOptionalCaller").summary("Get the caller") + }), + ) .api_route( "/required", - get(async |actor: Actor| echo_caller(Some(actor.into())).await), + routing::get( + async |actor: Actor| echo_caller(Some(actor.into())).await, + |operation| { + operation + .id("getRequiredCaller") + .summary("Get the authenticated caller") + }, + ), ) }, |document| document, diff --git a/libs/@local/graph/api/src/rest/test_utils/mod.rs b/libs/@local/graph/api/src/rest/test_utils/mod.rs index 0896f5edbc0..0d05db3513a 100644 --- a/libs/@local/graph/api/src/rest/test_utils/mod.rs +++ b/libs/@local/graph/api/src/rest/test_utils/mod.rs @@ -1,8 +1,10 @@ mod authentication; +use alloc::sync::Arc; + use aide::{ OperationInput, - axum::{ApiRouter, routing::get}, + axum::ApiRouter, openapi::{Info, SecurityScheme}, }; use axum::{ @@ -10,14 +12,20 @@ use axum::{ body::{Body, to_bytes}, response::Response, }; +use hash_graph_postgres_store::store::{ + DatabaseConnectionInfo, DatabasePoolConfig, DatabaseType, PostgresStorePool, + PostgresStoreSettings, +}; use hash_middleware::authentication::Challenge; +use hash_secret::Secret; use http::Request; use serde_json::Value; +use tokio_postgres::NoTls; use tower::ServiceExt as _; pub(super) use self::authentication::assert_authentication; pub(super) use super::caller::caller as echo_caller; -use super::{Api, Audience, credentials::Credentials, openapi}; +use super::{Api, ApiState, Audience, credentials::Credentials, extract::Json, openapi, routing}; pub(super) async fn request(router: &Router, path: &str) -> Response { router @@ -39,8 +47,30 @@ pub(super) async fn response_json(response: Response) -> Value { serde_json::from_slice(&body).expect("the response body should be JSON") } +/// The public and internal APIs, reading a store whose every `acquire` fails. pub(crate) fn apis() -> Vec { - super::apis().collect() + let state = ApiState::new(Arc::new(unconnected_store()), None); + super::apis::() + .map(|api| api.with_state(state.clone())) + .collect() +} + +/// A store pool pointed at a Unix socket that does not exist, so every `acquire` fails. +fn unconnected_store() -> PostgresStorePool { + PostgresStorePool::new( + &DatabaseConnectionInfo::new( + DatabaseType::Postgres, + "graph-api-test".to_owned(), + Secret::from_string(String::new()), + "/no-graph-api-test-postgres".to_owned(), + 5432, + "graph-api-test".to_owned(), + ), + &DatabasePoolConfig::default(), + NoTls, + PostgresStoreSettings::default(), + ) + .expect("the pool should be built without connecting") } /// A public credential set documenting no security scheme. @@ -59,10 +89,18 @@ impl Credentials for NoCredentials { /// A public API answering `GET {prefix}/test`. pub(super) fn api(prefix: &'static str) -> Api { - openapi::build::( + openapi::build::( prefix, Info::default(), - || ApiRouter::new().api_route("/test", get(async || String::from("ok"))), + || { + ApiRouter::new().api_route( + "/test", + routing::get( + async || Json::<_, 200>(String::from("ok")), + |operation| operation.id("test").summary("Answer `ok`"), + ), + ) + }, |document| document, ) } diff --git a/libs/@local/graph/api/tests/snapshots/openapi/entities-v1.snap.json b/libs/@local/graph/api/tests/snapshots/openapi/entities-v1.snap.json index d7de45cf7ee..dc0fbe0f458 100644 --- a/libs/@local/graph/api/tests/snapshots/openapi/entities-v1.snap.json +++ b/libs/@local/graph/api/tests/snapshots/openapi/entities-v1.snap.json @@ -1,6 +1,70 @@ { "components": { "responses": { + "BadRequest": { + "content": { + "application/problem+json": { + "example": { + "detail": "Invalid URL: Cannot parse `limit` with value `many` to a `u8`", + "status": 400, + "title": "Bad Request", + "type": "about:blank" + }, + "schema": { + "allOf": [ + { + "$ref": "#/components/schemas/ProblemDetails" + }, + { + "properties": { + "status": { + "const": 400 + }, + "title": { + "const": "Bad Request" + }, + "type": { + "const": "about:blank" + } + }, + "required": [ + "type" + ] + } + ], + "title": "Bad Request" + } + } + }, + "description": "A path parameter does not match its documented schema.", + "headers": { + "Cache-Control": { + "description": "Always `private, no-store`: the response depends on the caller, so no cache stores it.", + "example": "private, no-store", + "required": true, + "schema": { + "type": "string" + }, + "style": "simple" + }, + "RateLimit": { + "description": "The requests each budget in `RateLimit-Policy` has left after this request, as its `r`. A crossed budget reports `r=0`.", + "example": "\"actor\";r=499", + "schema": { + "type": "string" + }, + "style": "simple" + }, + "RateLimit-Policy": { + "description": "The budgets the request was checked against, one item each, as draft-ietf-httpapi-ratelimit-headers-11 defines them. `q` is the burst a budget admits at once, and `w` the whole seconds an exhausted budget takes to refill, so `q` requests per `w` seconds stay within its sustained rate.", + "example": "\"actor\";q=500;w=50", + "schema": { + "type": "string" + }, + "style": "simple" + } + } + }, "InternalServerError": { "content": { "application/problem+json": { @@ -65,6 +129,70 @@ } } }, + "NotFound": { + "content": { + "application/problem+json": { + "example": { + "detail": "No entity `8d86c8c3-d66d-43d1-859b-676c3bcaeadc~be2c5b2f-e3e1-4f26-a0c4-0f8b5e9c4a1d` is visible to the caller.", + "status": 404, + "title": "Not Found", + "type": "about:blank" + }, + "schema": { + "allOf": [ + { + "$ref": "#/components/schemas/ProblemDetails" + }, + { + "properties": { + "status": { + "const": 404 + }, + "title": { + "const": "Not Found" + }, + "type": { + "const": "about:blank" + } + }, + "required": [ + "type" + ] + } + ], + "title": "Not Found" + } + } + }, + "description": "The entity does not exist, or the caller may not view it.", + "headers": { + "Cache-Control": { + "description": "Always `private, no-store`: the response depends on the caller, so no cache stores it.", + "example": "private, no-store", + "required": true, + "schema": { + "type": "string" + }, + "style": "simple" + }, + "RateLimit": { + "description": "The requests each budget in `RateLimit-Policy` has left after this request, as its `r`. A crossed budget reports `r=0`.", + "example": "\"actor\";r=499", + "schema": { + "type": "string" + }, + "style": "simple" + }, + "RateLimit-Policy": { + "description": "The budgets the request was checked against, one item each, as draft-ietf-httpapi-ratelimit-headers-11 defines them. `q` is the burst a budget admits at once, and `w` the whole seconds an exhausted budget takes to refill, so `q` requests per `w` seconds stay within its sustained rate.", + "example": "\"actor\";q=500;w=50", + "schema": { + "type": "string" + }, + "style": "simple" + } + } + }, "ServiceUnavailable": { "content": { "application/problem+json": { @@ -376,6 +504,30 @@ ], "type": "object" }, + "Entity": { + "description": "An entity the caller may view.", + "properties": { + "entityId": { + "allOf": [ + { + "$ref": "#/components/schemas/EntityId" + } + ], + "description": "The ID of the entity." + } + }, + "required": [ + "entityId" + ], + "type": "object" + }, + "EntityId": { + "description": "An entity's web ID and UUID, joined by `~`, optionally followed by `~` and a draft ID.", + "examples": [ + "8d86c8c3-d66d-43d1-859b-676c3bcaeadc~be2c5b2f-e3e1-4f26-a0c4-0f8b5e9c4a1d" + ], + "type": "string" + }, "ProblemDetails": { "properties": { "detail": { @@ -556,6 +708,194 @@ "Caller" ] } + }, + "/entities/v1/{entityId}": { + "get": { + "description": "Returns the current edition of the entity, or of the draft the ID names. Answers `404` both when the entity does not exist and when the caller may not view it.", + "operationId": "getEntity", + "parameters": [ + { + "description": "The ID of the entity to read.", + "in": "path", + "name": "entityId", + "required": true, + "schema": { + "$ref": "#/components/schemas/EntityId", + "description": "The ID of the entity to read." + }, + "style": "simple" + } + ], + "responses": { + "200": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Entity" + } + } + }, + "description": "An entity the caller may view.", + "headers": { + "Cache-Control": { + "description": "Always `private, no-store`: the response depends on the caller, so no cache stores it.", + "example": "private, no-store", + "required": true, + "schema": { + "type": "string" + }, + "style": "simple" + }, + "RateLimit": { + "description": "The requests each budget in `RateLimit-Policy` has left after this request, as its `r`. A crossed budget reports `r=0`.", + "example": "\"actor\";r=499", + "schema": { + "type": "string" + }, + "style": "simple" + }, + "RateLimit-Policy": { + "description": "The budgets the request was checked against, one item each, as draft-ietf-httpapi-ratelimit-headers-11 defines them. `q` is the burst a budget admits at once, and `w` the whole seconds an exhausted budget takes to refill, so `q` requests per `w` seconds stay within its sustained rate.", + "example": "\"actor\";q=500;w=50", + "schema": { + "type": "string" + }, + "style": "simple" + } + } + }, + "400": { + "$ref": "#/components/responses/BadRequest" + }, + "401": { + "$ref": "#/components/responses/Unauthorized" + }, + "404": { + "$ref": "#/components/responses/NotFound" + }, + "429": { + "$ref": "#/components/responses/TooManyRequests" + }, + "500": { + "$ref": "#/components/responses/InternalServerError" + }, + "503": { + "content": { + "application/problem+json": { + "examples": { + "about:blank": { + "summary": "Service Unavailable: The service is temporarily unavailable.", + "value": { + "detail": "The service is temporarily unavailable.", + "status": 503, + "title": "Service Unavailable", + "type": "about:blank" + } + }, + "about:blank (2)": { + "summary": "Service Unavailable: Authentication is temporarily unavailable.", + "value": { + "detail": "authentication is temporarily unavailable", + "status": 503, + "title": "Service Unavailable", + "type": "about:blank" + } + } + }, + "schema": { + "anyOf": [ + { + "allOf": [ + { + "$ref": "#/components/schemas/ProblemDetails" + }, + { + "properties": { + "status": { + "const": 503 + }, + "title": { + "const": "Service Unavailable" + }, + "type": { + "const": "about:blank" + } + }, + "required": [ + "type" + ] + } + ], + "title": "Service Unavailable: The service is temporarily unavailable." + }, + { + "allOf": [ + { + "$ref": "#/components/schemas/ProblemDetails" + }, + { + "properties": { + "status": { + "const": 503 + }, + "title": { + "const": "Service Unavailable" + }, + "type": { + "const": "about:blank" + } + }, + "required": [ + "type" + ] + } + ], + "title": "Service Unavailable: Authentication is temporarily unavailable." + } + ] + } + } + }, + "description": "- Service Unavailable: The service is temporarily unavailable.\n- Service Unavailable: Authentication is temporarily unavailable.", + "headers": { + "Cache-Control": { + "description": "Always `private, no-store`: the response depends on the caller, so no cache stores it.", + "example": "private, no-store", + "required": true, + "schema": { + "type": "string" + }, + "style": "simple" + }, + "RateLimit": { + "description": "The requests each budget in `RateLimit-Policy` has left after this request, as its `r`. A crossed budget reports `r=0`.", + "example": "\"actor\";r=499", + "schema": { + "type": "string" + }, + "style": "simple" + }, + "RateLimit-Policy": { + "description": "The budgets the request was checked against, one item each, as draft-ietf-httpapi-ratelimit-headers-11 defines them. `q` is the burst a budget admits at once, and `w` the whole seconds an exhausted budget takes to refill, so `q` requests per `w` seconds stay within its sustained rate.", + "example": "\"actor\";q=500;w=50", + "schema": { + "type": "string" + }, + "style": "simple" + } + } + } + }, + "security": [ + { + "apiToken": [] + } + ], + "summary": "Get an entity", + "tags": [ + "Entity" + ] + } } } } \ No newline at end of file diff --git a/libs/@local/middleware/src/problem.rs b/libs/@local/middleware/src/problem.rs index 1cef5d96879..3076c134e1c 100644 --- a/libs/@local/middleware/src/problem.rs +++ b/libs/@local/middleware/src/problem.rs @@ -35,3 +35,34 @@ impl ProblemVariant for InternalServerError { Some(Self) } } + +/// The `detail` of a service unavailable error, and its description in the documentation. +const UNAVAILABLE_DETAIL: &str = "The service is temporarily unavailable."; + +/// The problem variant for a dependency the service cannot reach right now. +/// +/// The client receives `503 Service Unavailable` of type `about:blank`, with a `detail` that names +/// neither the dependency nor the error. List it in the [`Problem`](problematic::Problem) of an +/// endpoint that reads a dependency, and answer with it through +/// [`Answer::new`](problematic::Answer::new) when the dependency cannot be reached. +#[derive(Debug, Clone, Copy, serde::Serialize, schemars::JsonSchema)] +#[schemars(description = UNAVAILABLE_DETAIL)] +pub struct ServiceUnavailable; + +impl fmt::Display for ServiceUnavailable { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + formatter.write_str(UNAVAILABLE_DETAIL) + } +} + +impl ProblemVariant for ServiceUnavailable { + const TYPE: ProblemType = ProblemType { + type_uri: Cow::Borrowed("about:blank"), + title: Cow::Borrowed("Service Unavailable"), + status: StatusCode::SERVICE_UNAVAILABLE, + }; + + fn example() -> Option { + Some(Self) + } +} From d10bef5847d766f683647f08ddc976955e30acaf Mon Sep 17 00:00:00 2001 From: Tim Diekmann Date: Thu, 15 Oct 2026 11:38:11 +0200 Subject: [PATCH 04/13] Mask protected properties in `get_entity_by_id` --- .../store/postgres/knowledge/entity/mod.rs | 45 ++++++++++--------- .../postgres/email_filter_protection.rs | 29 ++++++++++++ 2 files changed, 52 insertions(+), 22 deletions(-) diff --git a/libs/@local/graph/postgres-store/src/store/postgres/knowledge/entity/mod.rs b/libs/@local/graph/postgres-store/src/store/postgres/knowledge/entity/mod.rs index f850e3a2c9d..bd349349450 100644 --- a/libs/@local/graph/postgres-store/src/store/postgres/knowledge/entity/mod.rs +++ b/libs/@local/graph/postgres-store/src/store/postgres/knowledge/entity/mod.rs @@ -1986,28 +1986,29 @@ where .await .change_context(QueryError)?; - let mut filters = vec![Filter::for_entity_by_entity_id(entity_id)]; - - let filter = Filter::::for_view_entity(&policy_components); - filters.push(filter); - - let temporal_axes = QueryTemporalAxesUnresolved::TransactionTime { - pinned: PinnedTemporalAxisUnresolved::new(decision_time), - variable: VariableTemporalAxisUnresolved::new( - transaction_time.map(TemporalBound::Inclusive), - transaction_time.map(LimitedTemporalBound::Inclusive), - ), - } - .resolve(); - - let entities = Read::::read( - self, - &filters, - Some(&temporal_axes), - entity_id.draft_id.is_some(), - ) - .await?; - core::pin::pin!(entities).try_next().await + let params = QueryEntitiesParams { + filter: Filter::for_entity_by_entity_id(entity_id), + temporal_axes: QueryTemporalAxesUnresolved::TransactionTime { + pinned: PinnedTemporalAxisUnresolved::new(decision_time), + variable: VariableTemporalAxisUnresolved::new( + transaction_time.map(TemporalBound::Inclusive), + transaction_time.map(LimitedTemporalBound::Inclusive), + ), + }, + sorting: EntityQuerySorting { + paths: Vec::new(), + cursor: None, + }, + limit: 1, + conversions: Vec::new(), + include_entity_types: None, + include_drafts: entity_id.draft_id.is_some(), + include_permissions: false, + }; + let response = self + .read_entities_impl(¶ms, ¶ms.temporal_axes.resolve(), &policy_components) + .await?; + Ok(response.entities.into_iter().next()) } #[tracing::instrument(level = "info", skip(self, params))] diff --git a/tests/graph/integration/postgres/email_filter_protection.rs b/tests/graph/integration/postgres/email_filter_protection.rs index 6c3120d7a6c..2e808271418 100644 --- a/tests/graph/integration/postgres/email_filter_protection.rs +++ b/tests/graph/integration/postgres/email_filter_protection.rs @@ -2725,6 +2725,35 @@ async fn user_email_masked_in_response() { ); } +/// Reading a User entity by its ID masks its email like a query does. +#[tokio::test] +async fn user_email_masked_by_id() { + let mut database = DatabaseTestWrapper::new().await; + let mut api = seed_with_user_only(&mut database).await; + + let user = api.create_user("hidden@example.com", "alice").await; + + let entity = api + .get_entity_by_id( + Some(api.account_id), + user.metadata.record_id.entity_id, + None, + None, + ) + .await + .expect("should read the User entity") + .expect("should see the User entity"); + + assert!( + !has_email_property(&entity), + "User entity should NOT have email property when read by ID (masked)" + ); + assert!( + !has_email_metadata(&entity), + "User entity should NOT have email property metadata when read by ID (masked)" + ); +} + /// When querying without email filter, Invitation entity SHOULD have email in response. #[tokio::test] async fn invitation_email_visible_in_response() { From cd08988ac2d1271fb1ced71cee3ad95387ae42f3 Mon Sep 17 00:00:00 2001 From: Tim Diekmann Date: Thu, 15 Oct 2026 12:58:10 +0200 Subject: [PATCH 05/13] Reject draft IDs and archived entities in `GET /entities/v1/{entityId}` --- .../graph/api/src/rest/middleware/tests.rs | 31 ++++++++++++++++ .../api/src/rest/public/entities/v1/entity.rs | 35 +++++++++++++++---- .../snapshots/openapi/entities-v1.snap.json | 11 +++--- 3 files changed, 66 insertions(+), 11 deletions(-) diff --git a/libs/@local/graph/api/src/rest/middleware/tests.rs b/libs/@local/graph/api/src/rest/middleware/tests.rs index 1bfd1f3f475..3a10e417215 100644 --- a/libs/@local/graph/api/src/rest/middleware/tests.rs +++ b/libs/@local/graph/api/src/rest/middleware/tests.rs @@ -577,6 +577,37 @@ async fn entity_anonymous() { ); } +/// The entity route reads no drafts, so a draft's ID is answered `400` before it reads the store. +#[tokio::test] +async fn entity_draft_id() { + let router = middleware( + &config(10, 10), + StaticAuthenticationProvider::Verified(ActorId::new(Uuid::from_u128(1), ActorType::User)), + StaticAuthenticationProvider::Unreachable, + StaticAuthenticationProvider::Unreachable, + ) + .assemble(Router::new(), apis(), Router::new()); + + let response = send( + &router, + "/entities/v1/8d86c8c3-d66d-43d1-859b-676c3bcaeadc~be2c5b2f-e3e1-4f26-a0c4-0f8b5e9c4a1d~\ + 3f0c3b6e-5d2a-4c8e-9b1f-7a6d5e4c3b2a", + ) + .await; + assert_eq!( + response.status(), + StatusCode::BAD_REQUEST, + "a draft's ID should be answered 400" + ); + let detail = response_json(response).await["detail"].clone(); + assert!( + detail + .as_str() + .is_some_and(|detail| detail.contains("the entity ID names a draft")), + "the problem document should say that the ID names a draft, got {detail}" + ); +} + #[tokio::test] async fn apis_and_legacy_share_one_actor_budget() { let actor = ActorId::new(Uuid::from_u128(1), ActorType::User); diff --git a/libs/@local/graph/api/src/rest/public/entities/v1/entity.rs b/libs/@local/graph/api/src/rest/public/entities/v1/entity.rs index 50238806da4..a743e97110e 100644 --- a/libs/@local/graph/api/src/rest/public/entities/v1/entity.rs +++ b/libs/@local/graph/api/src/rest/public/entities/v1/entity.rs @@ -11,7 +11,7 @@ use hash_middleware::problem::{InternalServerError, ServiceUnavailable}; use http::StatusCode; use problematic::{Answer, Expose, Problem, ProblemType, ProblemVariant, Rejection, Variant}; use schemars::JsonSchema; -use serde::{Deserialize, Serialize}; +use serde::{Deserialize, Deserializer, Serialize, de}; use type_system::{ knowledge::entity::{EntityId, id::EntityUuid}, principal::actor_group::WebId, @@ -31,9 +31,23 @@ use crate::rest::{ #[serde(rename_all = "camelCase")] struct EntityPath { /// The ID of the entity to read. + #[serde(deserialize_with = "live_entity_id")] + #[schemars( + with = "String", + example = "8d86c8c3-d66d-43d1-859b-676c3bcaeadc~be2c5b2f-e3e1-4f26-a0c4-0f8b5e9c4a1d" + )] entity_id: EntityId, } +/// Reads an entity ID that names no draft. +fn live_entity_id<'de, D: Deserializer<'de>>(deserializer: D) -> Result { + let entity_id = EntityId::deserialize(deserializer)?; + if entity_id.draft_id.is_some() { + return Err(de::Error::custom("the entity ID names a draft")); + } + Ok(entity_id) +} + /// An entity the caller may view. #[derive(Serialize, JsonSchema)] #[serde(rename_all = "camelCase")] @@ -42,7 +56,7 @@ struct Entity { entity_id: EntityId, } -/// The entity does not exist, or the caller may not view it. +/// The entity does not exist, is archived, or the caller may not view it. #[derive(Serialize, JsonSchema, derive_more::Display)] #[display("No entity `{entity_id}` is visible to the caller.")] struct EntityNotFound { @@ -85,6 +99,8 @@ impl Problem for GetEntityProblem { enum GetEntityError { #[display("no entity `{_0}` is visible to the caller")] NotFound(EntityId), + #[display("entity `{_0}` is archived")] + Archived(EntityId), #[display("no store could be acquired")] Unavailable, #[display("the store could not be read")] @@ -96,9 +112,11 @@ impl Error for GetEntityError {} impl Expose for Report { fn expose(&self) -> Answer<'_, GetEntityProblem> { match self.current_context() { - GetEntityError::NotFound(entity_id) => Answer::new(EntityNotFound { - entity_id: *entity_id, - }), + GetEntityError::NotFound(entity_id) | GetEntityError::Archived(entity_id) => { + Answer::new(EntityNotFound { + entity_id: *entity_id, + }) + } GetEntityError::Unavailable => Answer::new(ServiceUnavailable), GetEntityError::Store => Answer::new(InternalServerError), } @@ -122,6 +140,9 @@ where .await .change_context(GetEntityError::Store)? .ok_or_else(|| Report::new(GetEntityError::NotFound(entity_id)))?; + if entity.metadata.archived { + return Err(Report::new(GetEntityError::Archived(entity_id)).into()); + } Ok(Json(Entity { entity_id: entity.metadata.record_id.entity_id, @@ -133,8 +154,8 @@ fn document_get_entity(operation: TransformOperation<'_>) -> TransformOperation< .id("getEntity") .summary("Get an entity") .description( - "Returns the current edition of the entity, or of the draft the ID names. Answers \ - `404` both when the entity does not exist and when the caller may not view it.", + "Returns the current edition of the entity. Answers `404` when the entity does not \ + exist, is archived, or the caller may not view it.", ) } diff --git a/libs/@local/graph/api/tests/snapshots/openapi/entities-v1.snap.json b/libs/@local/graph/api/tests/snapshots/openapi/entities-v1.snap.json index dc0fbe0f458..44bcdefff14 100644 --- a/libs/@local/graph/api/tests/snapshots/openapi/entities-v1.snap.json +++ b/libs/@local/graph/api/tests/snapshots/openapi/entities-v1.snap.json @@ -164,7 +164,7 @@ } } }, - "description": "The entity does not exist, or the caller may not view it.", + "description": "The entity does not exist, is archived, or the caller may not view it.", "headers": { "Cache-Control": { "description": "Always `private, no-store`: the response depends on the caller, so no cache stores it.", @@ -711,7 +711,7 @@ }, "/entities/v1/{entityId}": { "get": { - "description": "Returns the current edition of the entity, or of the draft the ID names. Answers `404` both when the entity does not exist and when the caller may not view it.", + "description": "Returns the current edition of the entity. Answers `404` when the entity does not exist, is archived, or the caller may not view it.", "operationId": "getEntity", "parameters": [ { @@ -720,8 +720,11 @@ "name": "entityId", "required": true, "schema": { - "$ref": "#/components/schemas/EntityId", - "description": "The ID of the entity to read." + "description": "The ID of the entity to read.", + "examples": [ + "8d86c8c3-d66d-43d1-859b-676c3bcaeadc~be2c5b2f-e3e1-4f26-a0c4-0f8b5e9c4a1d" + ], + "type": "string" }, "style": "simple" } From d8f26d53e9f481cfa42f1add8e25d65c90c3f9eb Mon Sep 17 00:00:00 2001 From: Tim Diekmann Date: Thu, 15 Oct 2026 20:17:52 +0200 Subject: [PATCH 06/13] Return the entity's ID and types under `metadata` in `GET /entities/v1/{entityId}` --- .../rust/src/knowledge/entity/id.rs | 2 +- .../type-system/rust/src/ontology/id/mod.rs | 24 +++++++++ .../api/src/rest/public/entities/v1/entity.rs | 32 +++++++++--- .../snapshots/openapi/entities-v1.snap.json | 52 +++++++++++++++---- 4 files changed, 91 insertions(+), 19 deletions(-) diff --git a/libs/@blockprotocol/type-system/rust/src/knowledge/entity/id.rs b/libs/@blockprotocol/type-system/rust/src/knowledge/entity/id.rs index c3c00bec5a3..fde09798935 100644 --- a/libs/@blockprotocol/type-system/rust/src/knowledge/entity/id.rs +++ b/libs/@blockprotocol/type-system/rust/src/knowledge/entity/id.rs @@ -77,7 +77,7 @@ impl From for Uuid { } } -/// An entity's web ID and UUID, joined by `~`, optionally followed by `~` and a draft ID. +/// An entity's web ID and UUID, joined by `~`. #[derive(Debug, Clone, Copy, Hash, PartialEq, Eq)] #[cfg_attr( feature = "schemars", diff --git a/libs/@blockprotocol/type-system/rust/src/ontology/id/mod.rs b/libs/@blockprotocol/type-system/rust/src/ontology/id/mod.rs index 70869710ba6..003fab9466d 100644 --- a/libs/@blockprotocol/type-system/rust/src/ontology/id/mod.rs +++ b/libs/@blockprotocol/type-system/rust/src/ontology/id/mod.rs @@ -31,6 +31,8 @@ //! ); //! ``` +#[cfg(feature = "schemars")] +use alloc::borrow::Cow; #[cfg(feature = "postgres")] use core::error::Error; use core::{ @@ -47,6 +49,8 @@ pub use error::{ use error_stack::Report; #[cfg(feature = "postgres")] use postgres_types::{FromSql, IsNull, ToSql, Type}; +#[cfg(feature = "schemars")] +use schemars::{JsonSchema, Schema, SchemaGenerator, json_schema}; use serde::{Deserialize, Deserializer, Serialize, Serializer, de}; use url::Url; #[cfg(feature = "utoipa")] @@ -775,6 +779,26 @@ impl ToSchema<'_> for VersionedUrl { } } +#[cfg(feature = "schemars")] +impl JsonSchema for VersionedUrl { + fn schema_name() -> Cow<'static, str> { + Cow::Borrowed("VersionedUrl") + } + + fn json_schema(_: &mut SchemaGenerator) -> Schema { + json_schema!({ + "type": "string", + "title": "Versioned URL", + "description": "The versioned URL of a SemType ontology type (the $id of the \ + schema). It must be of the form `${baseUrl}v/${version}`, where \ + `version` is currently a major version number.", + "format": "uri", + "maxLength": 2048, + "examples": ["https://hash.ai/@h/types/entity-type/user/v/1"], + }) + } +} + /// An identifier for an ontology type record consisting of a base URL and version. /// /// This type provides a structured representation of an ontology type identifier diff --git a/libs/@local/graph/api/src/rest/public/entities/v1/entity.rs b/libs/@local/graph/api/src/rest/public/entities/v1/entity.rs index a743e97110e..287dd331920 100644 --- a/libs/@local/graph/api/src/rest/public/entities/v1/entity.rs +++ b/libs/@local/graph/api/src/rest/public/entities/v1/entity.rs @@ -14,6 +14,7 @@ use schemars::JsonSchema; use serde::{Deserialize, Deserializer, Serialize, de}; use type_system::{ knowledge::entity::{EntityId, id::EntityUuid}, + ontology::VersionedUrl, principal::actor_group::WebId, }; use uuid::Uuid; @@ -32,10 +33,6 @@ use crate::rest::{ struct EntityPath { /// The ID of the entity to read. #[serde(deserialize_with = "live_entity_id")] - #[schemars( - with = "String", - example = "8d86c8c3-d66d-43d1-859b-676c3bcaeadc~be2c5b2f-e3e1-4f26-a0c4-0f8b5e9c4a1d" - )] entity_id: EntityId, } @@ -50,10 +47,19 @@ fn live_entity_id<'de, D: Deserializer<'de>>(deserializer: D) -> Result, } /// The entity does not exist, is archived, or the caller may not view it. @@ -74,9 +80,9 @@ impl ProblemVariant for EntityNotFound { fn example() -> Option { Some(Self { entity_id: EntityId { - web_id: WebId::new(Uuid::from_u128(0x8D86_C8C3_D66D_43D1_859B_676C_3BCA_EADC)), + web_id: WebId::new(Uuid::from_u128(0x8D86C8C3_D66D_43D1_859B_676C3BCAEADC)), entity_uuid: EntityUuid::new(Uuid::from_u128( - 0xBE2C_5B2F_E3E1_4F26_A0C4_0F8B_5E9C_4A1D, + 0xBE2C5B2F_E3E1_4F26_A0C4_0F8B5E9C4A1D, )), draft_id: None, }, @@ -144,8 +150,18 @@ where return Err(Report::new(GetEntityError::Archived(entity_id)).into()); } + let mut entity_type_ids = entity + .metadata + .entity_type_ids + .into_iter() + .collect::>(); + entity_type_ids.sort(); + Ok(Json(Entity { - entity_id: entity.metadata.record_id.entity_id, + metadata: EntityMetadata { + entity_id: entity.metadata.record_id.entity_id, + entity_type_ids, + }, })) } diff --git a/libs/@local/graph/api/tests/snapshots/openapi/entities-v1.snap.json b/libs/@local/graph/api/tests/snapshots/openapi/entities-v1.snap.json index 44bcdefff14..dc064b77764 100644 --- a/libs/@local/graph/api/tests/snapshots/openapi/entities-v1.snap.json +++ b/libs/@local/graph/api/tests/snapshots/openapi/entities-v1.snap.json @@ -507,27 +507,52 @@ "Entity": { "description": "An entity the caller may view.", "properties": { - "entityId": { + "metadata": { "allOf": [ { - "$ref": "#/components/schemas/EntityId" + "$ref": "#/components/schemas/EntityMetadata" } ], - "description": "The ID of the entity." + "description": "What identifies and classifies the entity." } }, "required": [ - "entityId" + "metadata" ], "type": "object" }, "EntityId": { - "description": "An entity's web ID and UUID, joined by `~`, optionally followed by `~` and a draft ID.", + "description": "An entity's web ID and UUID, joined by `~`.", "examples": [ "8d86c8c3-d66d-43d1-859b-676c3bcaeadc~be2c5b2f-e3e1-4f26-a0c4-0f8b5e9c4a1d" ], "type": "string" }, + "EntityMetadata": { + "description": "What identifies and classifies an entity.", + "properties": { + "entityId": { + "allOf": [ + { + "$ref": "#/components/schemas/EntityId" + } + ], + "description": "The ID of the entity." + }, + "entityTypeIds": { + "description": "The entity types the entity is an instance of, without the types they inherit from.", + "items": { + "$ref": "#/components/schemas/VersionedUrl" + }, + "type": "array" + } + }, + "required": [ + "entityId", + "entityTypeIds" + ], + "type": "object" + }, "ProblemDetails": { "properties": { "detail": { @@ -560,6 +585,16 @@ ], "title": "Problem Details", "type": "object" + }, + "VersionedUrl": { + "description": "The versioned URL of a SemType ontology type (the $id of the schema). It must be of the form `${baseUrl}v/${version}`, where `version` is currently a major version number.", + "examples": [ + "https://hash.ai/@h/types/entity-type/user/v/1" + ], + "format": "uri", + "maxLength": 2048, + "title": "Versioned URL", + "type": "string" } }, "securitySchemes": { @@ -720,11 +755,8 @@ "name": "entityId", "required": true, "schema": { - "description": "The ID of the entity to read.", - "examples": [ - "8d86c8c3-d66d-43d1-859b-676c3bcaeadc~be2c5b2f-e3e1-4f26-a0c4-0f8b5e9c4a1d" - ], - "type": "string" + "$ref": "#/components/schemas/EntityId", + "description": "The ID of the entity to read." }, "style": "simple" } From 264113126be3a886ebba42c15afd29a9b2039e81 Mon Sep 17 00:00:00 2001 From: Tim Diekmann Date: Thu, 15 Oct 2026 20:18:15 +0200 Subject: [PATCH 07/13] Require an API token on the public APIs --- libs/@local/graph/api/src/rest/caller.rs | 34 +- libs/@local/graph/api/src/rest/credentials.rs | 11 +- .../api/src/rest/internal/credentials.rs | 6 +- .../graph/api/src/rest/middleware/mod.rs | 25 +- .../graph/api/src/rest/middleware/tests.rs | 41 +- libs/@local/graph/api/src/rest/mod.rs | 2 + .../api/src/rest/public/entities/v1/mod.rs | 4 +- .../graph/api/src/rest/public/types/v1.rs | 2 +- .../api/src/rest/test_utils/authentication.rs | 88 ++-- .../graph/api/src/rest/test_utils/mod.rs | 2 +- .../snapshots/openapi/entities-v1.snap.json | 422 +++--------------- .../snapshots/openapi/types-v1.snap.json | 87 ---- .../graph/authentication/api-token.test.ts | 15 +- 13 files changed, 222 insertions(+), 517 deletions(-) diff --git a/libs/@local/graph/api/src/rest/caller.rs b/libs/@local/graph/api/src/rest/caller.rs index f738c819eb8..3eb37039378 100644 --- a/libs/@local/graph/api/src/rest/caller.rs +++ b/libs/@local/graph/api/src/rest/caller.rs @@ -1,10 +1,13 @@ -use aide::{axum::ApiRouter, transform::TransformOperation}; +use aide::{ + axum::{ApiRouter, routing::ApiMethodRouter}, + transform::TransformOperation, +}; use schemars::JsonSchema; use serde::Serialize; use type_system::principal::actor::ActorId; use super::{ - credentials::{Actor, Credentials, MaybeActor}, + credentials::{Actor, AdmitsAnonymous, Credentials, MaybeActor}, extract::Json, routing, }; @@ -49,7 +52,16 @@ fn document_authenticated_caller(operation: TransformOperation<'_>) -> Transform .summary("Get the authenticated caller") } -pub(super) fn routes() -> ApiRouter { +fn authenticated_caller_route() +-> ApiMethodRouter { + routing::get( + authenticated_caller::>, + document_authenticated_caller, + ) +} + +/// `/caller` and `/authenticated-caller`. +pub(super) fn routes() -> ApiRouter { ApiRouter::new() .api_route( "/caller", @@ -57,10 +69,18 @@ pub(super) fn routes() -> ApiR ) .api_route( "/authenticated-caller", - routing::get( - authenticated_caller::>, - document_authenticated_caller, - ), + authenticated_caller_route::(), + ) + .with_path_items(|path| path.tag("Caller")) +} + +/// `/authenticated-caller`. +pub(super) fn authenticated_routes() +-> ApiRouter { + ApiRouter::new() + .api_route( + "/authenticated-caller", + authenticated_caller_route::(), ) .with_path_items(|path| path.tag("Caller")) } diff --git a/libs/@local/graph/api/src/rest/credentials.rs b/libs/@local/graph/api/src/rest/credentials.rs index a9148963b51..d92f2f1a13a 100644 --- a/libs/@local/graph/api/src/rest/credentials.rs +++ b/libs/@local/graph/api/src/rest/credentials.rs @@ -77,6 +77,13 @@ impl OperationInput for Actor { } } +/// Credentials of an API that serves a request without a credential anonymously. +/// +/// Only such an API reads its caller through [`MaybeActor`]. The public APIs answer a request +/// without a credential with `401` before a handler runs, so an optional caller would document +/// access they refuse. +pub(super) trait AdmitsAnonymous: Credentials {} + pub(super) struct MaybeActor(Option, PhantomData C>); impl From> for Option { @@ -85,7 +92,7 @@ impl From> for Option { } } -impl FromRequestParts for MaybeActor { +impl FromRequestParts for MaybeActor { type Rejection = AuthenticationRejection; async fn from_request_parts(parts: &mut Parts, state: &S) -> Result { @@ -95,7 +102,7 @@ impl FromRequestParts for MaybeActor { } } -impl OperationInput for MaybeActor { +impl OperationInput for MaybeActor { fn operation_input(ctx: &mut GenContext, operation: &mut Operation) { let _: TransformOperation<'_> = TransformOperation::new(operation).security_requirement_multi([]); diff --git a/libs/@local/graph/api/src/rest/internal/credentials.rs b/libs/@local/graph/api/src/rest/internal/credentials.rs index c1d05b1340b..cb11a350a50 100644 --- a/libs/@local/graph/api/src/rest/internal/credentials.rs +++ b/libs/@local/graph/api/src/rest/internal/credentials.rs @@ -40,6 +40,8 @@ impl credentials::Credentials for Credentials { } } +impl credentials::AdmitsAnonymous for Credentials {} + impl OperationInput for Credentials { fn operation_input(_ctx: &mut GenContext, operation: &mut Operation) { let _: TransformOperation<'_> = TransformOperation::new(operation) @@ -51,10 +53,10 @@ impl OperationInput for Credentials { #[cfg(test)] mod tests { use super::Credentials; - use crate::rest::test_utils::assert_authentication; + use crate::rest::test_utils::assert_anonymous_authentication; #[tokio::test] async fn credentials_resolve_callers() { - assert_authentication::().await; + assert_anonymous_authentication::().await; } } diff --git a/libs/@local/graph/api/src/rest/middleware/mod.rs b/libs/@local/graph/api/src/rest/middleware/mod.rs index d99336a2502..38297f0dbd3 100644 --- a/libs/@local/graph/api/src/rest/middleware/mod.rs +++ b/libs/@local/graph/api/src/rest/middleware/mod.rs @@ -15,7 +15,7 @@ use axum::{Router, response::Response}; use hash_middleware::{ authentication::{ AuthenticationLayer, AuthenticationMetrics, Bootstrap, Challenge, - provider::AuthenticationProvider, + provider::{AuthenticationProvider, Caller}, }, problem::InternalServerError, rate_limit::{CallerLimitLayer, IpGateLayer, RateLimiters}, @@ -141,10 +141,14 @@ pub(super) struct Middleware { impl Middleware where - P: AuthenticationProvider> + 'static, + P: AuthenticationProvider + 'static, I: AuthenticationProvider> + 'static, L: AuthenticationProvider> + 'static, { + /// Attaches authentication and the caller budget to `api`. + /// + /// The public APIs answer a request that carries no credential with `401` before routing it to + /// a handler, the internal API serves it anonymously. fn attach_api(&self, api: Api) -> Router { let Api { audience, @@ -153,8 +157,12 @@ where .. } = api; match audience { - Audience::Public => self.attach(router, &self.public_provider, None, challenge), - Audience::Internal => self.attach(router, &self.internal_provider, None, challenge), + Audience::Public => { + self.attach::<_, ActorId>(router, &self.public_provider, None, challenge) + } + Audience::Internal => { + self.attach::<_, Option>(router, &self.internal_provider, None, challenge) + } } } @@ -178,7 +186,7 @@ where apis: impl IntoIterator, documentation: Router, ) -> Router { - let mut router = self.attach( + let mut router = self.attach::<_, Option>( legacy_routes, &self.legacy_provider, Some(Bootstrap { @@ -211,7 +219,7 @@ where /// /// The bootstrap secret also exempts its presenters from the caller budget, which a bootstrap /// request naming no actor can pass only by that exemption. - fn attach( + fn attach( &self, routes: Router, provider: &Arc, @@ -219,7 +227,8 @@ where challenge: Challenge, ) -> Router where - A: AuthenticationProvider> + 'static, + A: AuthenticationProvider + 'static, + C: Caller + 'static, { // `route_layer` panics on a router without routes. if !routes.has_routes() { @@ -236,7 +245,7 @@ where .as_ref() .map(|bootstrap| Pin::clone(&bootstrap.service_secret)), }) - .route_layer(AuthenticationLayer::<_, Option> { + .route_layer(AuthenticationLayer::<_, C> { provider: Arc::clone(provider), metrics: Arc::clone(&self.authentication_metrics), bootstrap, diff --git a/libs/@local/graph/api/src/rest/middleware/tests.rs b/libs/@local/graph/api/src/rest/middleware/tests.rs index 3a10e417215..8b70d313781 100644 --- a/libs/@local/graph/api/src/rest/middleware/tests.rs +++ b/libs/@local/graph/api/src/rest/middleware/tests.rs @@ -215,7 +215,7 @@ async fn fallback_answers_problem_document() { async fn cache_control_gate_rejection() { let router = middleware( &config(1, 10), - StaticAuthenticationProvider::NotRecognized, + StaticAuthenticationProvider::Verified(ActorId::new(Uuid::from_u128(1), ActorType::User)), StaticAuthenticationProvider::NotRecognized, StaticAuthenticationProvider::NotRecognized, ) @@ -318,7 +318,7 @@ async fn fallback_draws_on_address_gate() { async fn method_not_allowed_problem_document() { let router = middleware( &config(10, 10), - StaticAuthenticationProvider::NotRecognized, + StaticAuthenticationProvider::Verified(ActorId::new(Uuid::from_u128(1), ActorType::User)), StaticAuthenticationProvider::NotRecognized, StaticAuthenticationProvider::NotRecognized, ) @@ -463,7 +463,7 @@ async fn method_not_allowed_draws_on_address_gate() { async fn method_not_allowed_draws_on_caller_budget() { let router = middleware( &config(10, 1), - StaticAuthenticationProvider::NotRecognized, + StaticAuthenticationProvider::Verified(ActorId::new(Uuid::from_u128(1), ActorType::User)), StaticAuthenticationProvider::NotRecognized, StaticAuthenticationProvider::NotRecognized, ) @@ -577,6 +577,34 @@ async fn entity_anonymous() { ); } +/// A public API answers a request without a credential with `401` even on a route that reads no +/// caller. +#[tokio::test] +async fn public_api_anonymous() { + let router = middleware( + &config(10, 10), + StaticAuthenticationProvider::NotRecognized, + StaticAuthenticationProvider::NotRecognized, + StaticAuthenticationProvider::NotRecognized, + ) + .assemble(Router::new(), [test_utils::api("/first")], Router::new()); + + let response = send(&router, "/first/test").await; + assert_eq!( + response.status(), + StatusCode::UNAUTHORIZED, + "an anonymous request should be answered 401" + ); + assert!( + response + .headers() + .get(WWW_AUTHENTICATE) + .and_then(|value| value.to_str().ok()) + .is_some_and(|value| value.starts_with("Bearer")), + "the 401 should challenge with `Bearer`" + ); +} + /// The entity route reads no drafts, so a draft's ID is answered `400` before it reads the store. #[tokio::test] async fn entity_draft_id() { @@ -730,7 +758,7 @@ async fn service_secret_exempt_on_legacy_only() { ) .assemble( Router::new().route("/legacy", get(async || "ok")), - [test_utils::api("/first")], + [internal::api()], Router::new(), ); let with_secret = |path: &str, peer: IpAddr| { @@ -752,10 +780,11 @@ async fn service_secret_exempt_on_legacy_only() { "the legacy routes should not charge the service secret" ); } + let caller = format!("{}/caller", internal::PREFIX); let mut statuses = Vec::new(); for _ in 0..2 { statuses.push( - send_request(&router, with_secret("/first/test", client(2))) + send_request(&router, with_secret(&caller, client(2))) .await .status(), ); @@ -763,7 +792,7 @@ async fn service_secret_exempt_on_legacy_only() { assert_eq!( statuses, [StatusCode::OK, StatusCode::TOO_MANY_REQUESTS], - "an API should charge the service secret's presenter like any anonymous caller" + "the internal API should charge the service secret's presenter like any anonymous caller" ); } diff --git a/libs/@local/graph/api/src/rest/mod.rs b/libs/@local/graph/api/src/rest/mod.rs index 8097f3dd3b4..8c5491ca4bf 100644 --- a/libs/@local/graph/api/src/rest/mod.rs +++ b/libs/@local/graph/api/src/rest/mod.rs @@ -31,7 +31,9 @@ pub use self::router::{Dependencies, router}; /// The callers an API admits, which selects the provider chain authenticating its requests. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub(crate) enum Audience { + /// Callers presenting an API token. A request without a credential is answered `401`. Public, + /// The HASH frontend and services. A request without a credential is served anonymously. Internal, } diff --git a/libs/@local/graph/api/src/rest/public/entities/v1/mod.rs b/libs/@local/graph/api/src/rest/public/entities/v1/mod.rs index 4f1af2b33cd..6ffc2a4104f 100644 --- a/libs/@local/graph/api/src/rest/public/entities/v1/mod.rs +++ b/libs/@local/graph/api/src/rest/public/entities/v1/mod.rs @@ -3,7 +3,7 @@ mod entity; use aide::openapi::Info; use hash_graph_store::pool::StorePool; -use crate::rest::{Api, ApiState, caller, documentation, openapi, public}; +use crate::rest::{Api, ApiState, documentation, openapi, public}; pub(super) fn api() -> Api> where @@ -17,7 +17,7 @@ where description: Some(documentation::OVERVIEW.to_owned()), ..Info::default() }, - || caller::routes::().merge(entity::routes::()), + entity::routes::, public::document, ) } diff --git a/libs/@local/graph/api/src/rest/public/types/v1.rs b/libs/@local/graph/api/src/rest/public/types/v1.rs index e3508251fcf..1e18a7e0e25 100644 --- a/libs/@local/graph/api/src/rest/public/types/v1.rs +++ b/libs/@local/graph/api/src/rest/public/types/v1.rs @@ -11,7 +11,7 @@ pub(super) fn api() -> Api { description: Some(documentation::OVERVIEW.to_owned()), ..Info::default() }, - caller::routes::, + caller::authenticated_routes::, public::document, ) } diff --git a/libs/@local/graph/api/src/rest/test_utils/authentication.rs b/libs/@local/graph/api/src/rest/test_utils/authentication.rs index 5525dda5645..72267b3d738 100644 --- a/libs/@local/graph/api/src/rest/test_utils/authentication.rs +++ b/libs/@local/graph/api/src/rest/test_utils/authentication.rs @@ -3,7 +3,7 @@ use core::{num::NonZeroU32, ops::ControlFlow, pin::pin}; use std::collections::HashMap; use aide::{ - axum::ApiRouter, + axum::{ApiRouter, routing::ApiMethodRouter}, openapi::{Info, ReferenceOr, SecurityScheme}, }; use axum::{Router, body::Body}; @@ -42,7 +42,7 @@ use uuid::Uuid; use super::{echo_caller, response_json}; use crate::rest::{ - credentials::{API_TOKEN, Actor, Credentials, MaybeActor}, + credentials::{API_TOKEN, Actor, AdmitsAnonymous, Credentials, MaybeActor}, middleware::Middleware, openapi, routing, }; @@ -113,13 +113,33 @@ struct Advertised { api_token: bool, } -/// Assembles `/test/optional` and `/test/required` behind the credentials `C`, with the provider -/// chains the router composes. +/// Routes `GET` to the caller, which it reads through [`Actor`]. +fn required_route() -> ApiMethodRouter { + routing::get( + async |actor: Actor| echo_caller(Some(actor.into())).await, + |operation| { + operation + .id("getRequiredCaller") + .summary("Get the authenticated caller") + }, + ) +} + +/// Routes `GET` to the caller, which it reads through [`MaybeActor`]. +fn optional_route() -> ApiMethodRouter { + routing::get(echo_caller::>, |operation| { + operation.id("getOptionalCaller").summary("Get the caller") + }) +} + +/// Assembles the routes `build_routes` returns under `/test` behind the credentials `C`, with the +/// provider chains the router composes. /// /// Returns the router and the schemes the generated document advertises. fn caller_router( operator: ActorId, session_actor: ActorId, + build_routes: impl FnOnce() -> ApiRouter, ) -> (Router, Advertised) { let public_provider = Arc::new(ApiTokenProvider::new( UnreadableTokens, @@ -149,31 +169,7 @@ fn caller_router( rate_limit_actor_per_hour: NonZeroU32::MAX, rate_limit_actor_burst: NonZeroU32::MAX, }; - let api = openapi::build::( - "/test", - Info::default(), - || { - ApiRouter::new() - .api_route( - "/optional", - routing::get(echo_caller::>, |operation| { - operation.id("getOptionalCaller").summary("Get the caller") - }), - ) - .api_route( - "/required", - routing::get( - async |actor: Actor| echo_caller(Some(actor.into())).await, - |operation| { - operation - .id("getRequiredCaller") - .summary("Get the authenticated caller") - }, - ), - ) - }, - |document| document, - ); + let api = openapi::build::("/test", Info::default(), build_routes, |document| document); let schemes = api .document() .components @@ -214,19 +210,39 @@ enum Outcome { Rejected(StatusCode), } -/// Sends each credential to the caller routes of `C` and checks the resolved actor. +/// Checks the resolved actor of each credential on `/test/required`, which reads it through +/// [`Actor`]. +pub(in crate::rest) async fn assert_authentication() { + assert_routes::(&["/test/required"], || { + ApiRouter::new().api_route("/required", required_route::()) + }) + .await; +} + +/// Checks the resolved actor of each credential on `/test/optional`, which reads it through +/// [`MaybeActor`], and on `/test/required`. +pub(in crate::rest) async fn assert_anonymous_authentication() { + assert_routes::(&["/test/optional", "/test/required"], || { + ApiRouter::new() + .api_route("/optional", optional_route::()) + .api_route("/required", required_route::()) + }) + .await; +} + +/// Sends each credential to `paths` behind `C` and checks the resolved actor. /// /// A session token resolves an actor exactly when `C` documents the session token header. Where /// `C` documents the API token scheme, a bearer API token is looked up, which the unreadable store /// answers with 503; elsewhere it is not accepted. An API token without the `Bearer` scheme is not /// accepted either way, and an API token fails the request even where anonymous callers are -/// served. A service delegation and a Cloudflare Access JWT resolve as anonymous everywhere. Every -/// `401` challenges with `C::CHALLENGE`. This ties the provider chain the audience selects to the -/// schemes the document states. -pub(in crate::rest) async fn assert_authentication() { +/// served. A service delegation and a Cloudflare Access JWT resolve as anonymous everywhere, which +/// `/test/required` answers with `401`. Every `401` challenges with `C::CHALLENGE`. This ties the +/// provider chain the audience selects to the schemes the document states. +async fn assert_routes(paths: &[&str], build_routes: impl FnOnce() -> ApiRouter) { let operator = ActorId::new(Uuid::from_u128(1), ActorType::Machine); let session_actor = ActorId::new(Uuid::from_u128(2), ActorType::User); - let (router, advertised) = caller_router::(operator, session_actor); + let (router, advertised) = caller_router::(operator, session_actor, build_routes); let session = Outcome::Caller(advertised.session.then_some(session_actor)); let api_token = api_token(); let bearer_api_token = if advertised.api_token { @@ -259,7 +275,7 @@ pub(in crate::rest) async fn assert_authentication() { Outcome::Rejected(StatusCode::UNAUTHORIZED), ), ] { - for path in ["/test/optional", "/test/required"] { + for &path in paths { let mut request = Request::builder().uri(path); for (name, value) in &headers { request = request.header(*name, value); diff --git a/libs/@local/graph/api/src/rest/test_utils/mod.rs b/libs/@local/graph/api/src/rest/test_utils/mod.rs index 0d05db3513a..ff32a034c62 100644 --- a/libs/@local/graph/api/src/rest/test_utils/mod.rs +++ b/libs/@local/graph/api/src/rest/test_utils/mod.rs @@ -23,7 +23,7 @@ use serde_json::Value; use tokio_postgres::NoTls; use tower::ServiceExt as _; -pub(super) use self::authentication::assert_authentication; +pub(super) use self::authentication::{assert_anonymous_authentication, assert_authentication}; pub(super) use super::caller::caller as echo_caller; use super::{Api, ApiState, Audience, credentials::Credentials, extract::Json, openapi, routing}; diff --git a/libs/@local/graph/api/tests/snapshots/openapi/entities-v1.snap.json b/libs/@local/graph/api/tests/snapshots/openapi/entities-v1.snap.json index dc064b77764..c03d3c5263f 100644 --- a/libs/@local/graph/api/tests/snapshots/openapi/entities-v1.snap.json +++ b/libs/@local/graph/api/tests/snapshots/openapi/entities-v1.snap.json @@ -196,39 +196,81 @@ "ServiceUnavailable": { "content": { "application/problem+json": { - "example": { - "detail": "authentication is temporarily unavailable", - "status": 503, - "title": "Service Unavailable", - "type": "about:blank" + "examples": { + "about:blank": { + "summary": "Service Unavailable: The service is temporarily unavailable.", + "value": { + "detail": "The service is temporarily unavailable.", + "status": 503, + "title": "Service Unavailable", + "type": "about:blank" + } + }, + "about:blank (2)": { + "summary": "Service Unavailable: Authentication is temporarily unavailable.", + "value": { + "detail": "authentication is temporarily unavailable", + "status": 503, + "title": "Service Unavailable", + "type": "about:blank" + } + } }, "schema": { - "allOf": [ + "anyOf": [ { - "$ref": "#/components/schemas/ProblemDetails" + "allOf": [ + { + "$ref": "#/components/schemas/ProblemDetails" + }, + { + "properties": { + "status": { + "const": 503 + }, + "title": { + "const": "Service Unavailable" + }, + "type": { + "const": "about:blank" + } + }, + "required": [ + "type" + ] + } + ], + "title": "Service Unavailable: The service is temporarily unavailable." }, { - "properties": { - "status": { - "const": 503 - }, - "title": { - "const": "Service Unavailable" + "allOf": [ + { + "$ref": "#/components/schemas/ProblemDetails" }, - "type": { - "const": "about:blank" + { + "properties": { + "status": { + "const": 503 + }, + "title": { + "const": "Service Unavailable" + }, + "type": { + "const": "about:blank" + } + }, + "required": [ + "type" + ] } - }, - "required": [ - "type" - ] + ], + "title": "Service Unavailable: Authentication is temporarily unavailable." } - ], - "title": "Service Unavailable" + ] } } }, - "description": "Authentication is temporarily unavailable.", + "description": "- Service Unavailable: The service is temporarily unavailable.\n- Service Unavailable: Authentication is temporarily unavailable.", "headers": { "Cache-Control": { "description": "Always `private, no-store`: the response depends on the caller, so no cache stores it.", @@ -405,105 +447,6 @@ } }, "schemas": { - "ActorId": { - "description": "An actor's type and UUID.", - "oneOf": [ - { - "properties": { - "actorType": { - "const": "user", - "type": "string" - }, - "id": { - "description": "The UUID of a user.", - "format": "uuid", - "type": "string" - } - }, - "required": [ - "actorType", - "id" - ], - "title": "User", - "type": "object" - }, - { - "properties": { - "actorType": { - "const": "machine", - "type": "string" - }, - "id": { - "description": "The UUID of a machine actor.", - "format": "uuid", - "type": "string" - } - }, - "required": [ - "actorType", - "id" - ], - "title": "Machine", - "type": "object" - }, - { - "properties": { - "actorType": { - "const": "ai", - "type": "string" - }, - "id": { - "description": "The UUID of an AI agent.", - "format": "uuid", - "type": "string" - } - }, - "required": [ - "actorType", - "id" - ], - "title": "AI", - "type": "object" - } - ] - }, - "AuthenticatedCallerResponse": { - "description": "The resolved caller.", - "properties": { - "actor": { - "allOf": [ - { - "$ref": "#/components/schemas/ActorId" - } - ], - "description": "The resolved actor." - } - }, - "required": [ - "actor" - ], - "type": "object" - }, - "CallerResponse": { - "description": "The resolved caller.", - "properties": { - "actor": { - "anyOf": [ - { - "$ref": "#/components/schemas/ActorId" - }, - { - "type": "null" - } - ], - "description": "The resolved actor, or `null` for an anonymous request." - } - }, - "required": [ - "actor" - ], - "type": "object" - }, "Entity": { "description": "An entity the caller may view.", "properties": { @@ -612,138 +555,6 @@ }, "openapi": "3.1.0", "paths": { - "/entities/v1/authenticated-caller": { - "get": { - "operationId": "getAuthenticatedCaller", - "responses": { - "200": { - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/AuthenticatedCallerResponse" - } - } - }, - "description": "The resolved caller.", - "headers": { - "Cache-Control": { - "description": "Always `private, no-store`: the response depends on the caller, so no cache stores it.", - "example": "private, no-store", - "required": true, - "schema": { - "type": "string" - }, - "style": "simple" - }, - "RateLimit": { - "description": "The requests each budget in `RateLimit-Policy` has left after this request, as its `r`. A crossed budget reports `r=0`.", - "example": "\"actor\";r=499", - "schema": { - "type": "string" - }, - "style": "simple" - }, - "RateLimit-Policy": { - "description": "The budgets the request was checked against, one item each, as draft-ietf-httpapi-ratelimit-headers-11 defines them. `q` is the burst a budget admits at once, and `w` the whole seconds an exhausted budget takes to refill, so `q` requests per `w` seconds stay within its sustained rate.", - "example": "\"actor\";q=500;w=50", - "schema": { - "type": "string" - }, - "style": "simple" - } - } - }, - "401": { - "$ref": "#/components/responses/Unauthorized" - }, - "429": { - "$ref": "#/components/responses/TooManyRequests" - }, - "500": { - "$ref": "#/components/responses/InternalServerError" - }, - "503": { - "$ref": "#/components/responses/ServiceUnavailable" - } - }, - "security": [ - { - "apiToken": [] - } - ], - "summary": "Get the authenticated caller", - "tags": [ - "Caller" - ] - } - }, - "/entities/v1/caller": { - "get": { - "description": "Returns the resolved actor, or `actor: null` for an anonymous request.", - "operationId": "getCaller", - "responses": { - "200": { - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/CallerResponse" - } - } - }, - "description": "The resolved caller.", - "headers": { - "Cache-Control": { - "description": "Always `private, no-store`: the response depends on the caller, so no cache stores it.", - "example": "private, no-store", - "required": true, - "schema": { - "type": "string" - }, - "style": "simple" - }, - "RateLimit": { - "description": "The requests each budget in `RateLimit-Policy` has left after this request, as its `r`. A crossed budget reports `r=0`.", - "example": "\"actor\";r=499", - "schema": { - "type": "string" - }, - "style": "simple" - }, - "RateLimit-Policy": { - "description": "The budgets the request was checked against, one item each, as draft-ietf-httpapi-ratelimit-headers-11 defines them. `q` is the burst a budget admits at once, and `w` the whole seconds an exhausted budget takes to refill, so `q` requests per `w` seconds stay within its sustained rate.", - "example": "\"actor\";q=500;w=50", - "schema": { - "type": "string" - }, - "style": "simple" - } - } - }, - "401": { - "$ref": "#/components/responses/Unauthorized" - }, - "429": { - "$ref": "#/components/responses/TooManyRequests" - }, - "500": { - "$ref": "#/components/responses/InternalServerError" - }, - "503": { - "$ref": "#/components/responses/ServiceUnavailable" - } - }, - "security": [ - {}, - { - "apiToken": [] - } - ], - "summary": "Get the caller", - "tags": [ - "Caller" - ] - } - }, "/entities/v1/{entityId}": { "get": { "description": "Returns the current edition of the entity. Answers `404` when the entity does not exist, is archived, or the caller may not view it.", @@ -815,110 +626,7 @@ "$ref": "#/components/responses/InternalServerError" }, "503": { - "content": { - "application/problem+json": { - "examples": { - "about:blank": { - "summary": "Service Unavailable: The service is temporarily unavailable.", - "value": { - "detail": "The service is temporarily unavailable.", - "status": 503, - "title": "Service Unavailable", - "type": "about:blank" - } - }, - "about:blank (2)": { - "summary": "Service Unavailable: Authentication is temporarily unavailable.", - "value": { - "detail": "authentication is temporarily unavailable", - "status": 503, - "title": "Service Unavailable", - "type": "about:blank" - } - } - }, - "schema": { - "anyOf": [ - { - "allOf": [ - { - "$ref": "#/components/schemas/ProblemDetails" - }, - { - "properties": { - "status": { - "const": 503 - }, - "title": { - "const": "Service Unavailable" - }, - "type": { - "const": "about:blank" - } - }, - "required": [ - "type" - ] - } - ], - "title": "Service Unavailable: The service is temporarily unavailable." - }, - { - "allOf": [ - { - "$ref": "#/components/schemas/ProblemDetails" - }, - { - "properties": { - "status": { - "const": 503 - }, - "title": { - "const": "Service Unavailable" - }, - "type": { - "const": "about:blank" - } - }, - "required": [ - "type" - ] - } - ], - "title": "Service Unavailable: Authentication is temporarily unavailable." - } - ] - } - } - }, - "description": "- Service Unavailable: The service is temporarily unavailable.\n- Service Unavailable: Authentication is temporarily unavailable.", - "headers": { - "Cache-Control": { - "description": "Always `private, no-store`: the response depends on the caller, so no cache stores it.", - "example": "private, no-store", - "required": true, - "schema": { - "type": "string" - }, - "style": "simple" - }, - "RateLimit": { - "description": "The requests each budget in `RateLimit-Policy` has left after this request, as its `r`. A crossed budget reports `r=0`.", - "example": "\"actor\";r=499", - "schema": { - "type": "string" - }, - "style": "simple" - }, - "RateLimit-Policy": { - "description": "The budgets the request was checked against, one item each, as draft-ietf-httpapi-ratelimit-headers-11 defines them. `q` is the burst a budget admits at once, and `w` the whole seconds an exhausted budget takes to refill, so `q` requests per `w` seconds stay within its sustained rate.", - "example": "\"actor\";q=500;w=50", - "schema": { - "type": "string" - }, - "style": "simple" - } - } + "$ref": "#/components/responses/ServiceUnavailable" } }, "security": [ diff --git a/libs/@local/graph/api/tests/snapshots/openapi/types-v1.snap.json b/libs/@local/graph/api/tests/snapshots/openapi/types-v1.snap.json index 04ba358b0aa..06d42ae9184 100644 --- a/libs/@local/graph/api/tests/snapshots/openapi/types-v1.snap.json +++ b/libs/@local/graph/api/tests/snapshots/openapi/types-v1.snap.json @@ -356,26 +356,6 @@ ], "type": "object" }, - "CallerResponse": { - "description": "The resolved caller.", - "properties": { - "actor": { - "anyOf": [ - { - "$ref": "#/components/schemas/ActorId" - }, - { - "type": "null" - } - ], - "description": "The resolved actor, or `null` for an anonymous request." - } - }, - "required": [ - "actor" - ], - "type": "object" - }, "ProblemDetails": { "properties": { "detail": { @@ -489,73 +469,6 @@ "Caller" ] } - }, - "/types/v1/caller": { - "get": { - "description": "Returns the resolved actor, or `actor: null` for an anonymous request.", - "operationId": "getCaller", - "responses": { - "200": { - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/CallerResponse" - } - } - }, - "description": "The resolved caller.", - "headers": { - "Cache-Control": { - "description": "Always `private, no-store`: the response depends on the caller, so no cache stores it.", - "example": "private, no-store", - "required": true, - "schema": { - "type": "string" - }, - "style": "simple" - }, - "RateLimit": { - "description": "The requests each budget in `RateLimit-Policy` has left after this request, as its `r`. A crossed budget reports `r=0`.", - "example": "\"actor\";r=499", - "schema": { - "type": "string" - }, - "style": "simple" - }, - "RateLimit-Policy": { - "description": "The budgets the request was checked against, one item each, as draft-ietf-httpapi-ratelimit-headers-11 defines them. `q` is the burst a budget admits at once, and `w` the whole seconds an exhausted budget takes to refill, so `q` requests per `w` seconds stay within its sustained rate.", - "example": "\"actor\";q=500;w=50", - "schema": { - "type": "string" - }, - "style": "simple" - } - } - }, - "401": { - "$ref": "#/components/responses/Unauthorized" - }, - "429": { - "$ref": "#/components/responses/TooManyRequests" - }, - "500": { - "$ref": "#/components/responses/InternalServerError" - }, - "503": { - "$ref": "#/components/responses/ServiceUnavailable" - } - }, - "security": [ - {}, - { - "apiToken": [] - } - ], - "summary": "Get the caller", - "tags": [ - "Caller" - ] - } } } } \ No newline at end of file diff --git a/tests/hash-backend-integration/src/tests/graph/authentication/api-token.test.ts b/tests/hash-backend-integration/src/tests/graph/authentication/api-token.test.ts index a990791a0df..1bd39acf07f 100644 --- a/tests/hash-backend-integration/src/tests/graph/authentication/api-token.test.ts +++ b/tests/hash-backend-integration/src/tests/graph/authentication/api-token.test.ts @@ -105,15 +105,11 @@ describe("API tokens", () => { { name: "public API in integration test" }, ); - const response = await requestWithToken( - "/entities/v1/authenticated-caller", - token, - ); + const entityId = owner.entity.metadata.recordId.entityId; + const response = await requestWithToken(`/entities/v1/${entityId}`, token); expect(response.status).toBe(200); - expect(await response.json()).toEqual({ - actor: { actorType: "user", id: owner.accountId }, - }); + expect(await response.json()).toMatchObject({ metadata: { entityId } }); }); it("rejects a revoked token instead of serving the request anonymously", async () => { @@ -128,7 +124,10 @@ describe("API tokens", () => { apiToken.tokenId, ); - const response = await requestWithToken("/entities/v1/caller", token); + const response = await requestWithToken( + `/entities/v1/${owner.entity.metadata.recordId.entityId}`, + token, + ); expect(response.status).toBe(401); expect(response.headers.get("content-type")).toBe( From 3ce174a4295d9fab44d52ab157930f7e442aba71 Mon Sep 17 00:00:00 2001 From: Tim Diekmann Date: Thu, 15 Oct 2026 22:09:10 +0200 Subject: [PATCH 08/13] Address the review of the public entities API --- Cargo.lock | 1 + Cargo.toml | 1 + .../rust/src/knowledge/entity/id.rs | 2 +- .../type-system/rust/src/ontology/id/mod.rs | 2 +- libs/@local/graph/api/Cargo.toml | 1 + libs/@local/graph/api/src/rest/caller.rs | 32 ++-- libs/@local/graph/api/src/rest/credentials.rs | 12 +- .../@local/graph/api/src/rest/extract/json.rs | 1 - libs/@local/graph/api/src/rest/extract/mod.rs | 12 +- .../graph/api/src/rest/extract/query.rs | 2 +- .../graph/api/src/rest/extract/tests.rs | 8 +- .../graph/api/src/rest/middleware/mod.rs | 5 +- .../graph/api/src/rest/middleware/tests.rs | 21 ++- libs/@local/graph/api/src/rest/mod.rs | 53 +----- libs/@local/graph/api/src/rest/openapi/mod.rs | 101 ++++++------ .../graph/api/src/rest/openapi/tests.rs | 155 +++++++++++------- .../api/src/rest/public/entities/v1/entity.rs | 34 ++-- libs/@local/graph/api/src/rest/router.rs | 28 ++-- libs/@local/graph/api/src/rest/routing.rs | 73 ++++++--- libs/@local/graph/api/src/rest/state.rs | 47 ++++++ .../api/src/rest/test_utils/authentication.rs | 28 ++-- .../graph/api/src/rest/test_utils/mod.rs | 11 +- .../snapshots/openapi/entities-v1.snap.json | 13 +- .../postgres-store/tests/deletion/purge.rs | 8 + .../graph/store/rust/src/entity/store.rs | 15 +- tests/graph/integration/postgres/entity.rs | 3 +- .../graph/authentication/api-token.test.ts | 87 ++++++++-- 27 files changed, 446 insertions(+), 310 deletions(-) create mode 100644 libs/@local/graph/api/src/rest/state.rs diff --git a/Cargo.lock b/Cargo.lock index 93e36e0880d..aa68230c1b3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6309,6 +6309,7 @@ dependencies = [ "md-5 0.11.0", "mime", "opentelemetry", + "percent-encoding", "problematic", "reqwest 0.13.4", "rstest", diff --git a/Cargo.toml b/Cargo.toml index 0b6a1ab2180..3ac695605be 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -260,6 +260,7 @@ opentelemetry-semantic-conventions = { version = "0.32.1", default-features = fa opentelemetry_sdk = { version = "0.32.1", default-features = false } owo-colors = { version = "4.2.3", default-features = false } oxc = { version = "0.95.0", default-features = false } +percent-encoding = { version = "2.3.2", default-features = false } pin-project = { version = "1.1.10", default-features = false } pin-project-lite = { version = "0.2.16", default-features = false } plist = { version = "1.8.0", default-features = false } diff --git a/libs/@blockprotocol/type-system/rust/src/knowledge/entity/id.rs b/libs/@blockprotocol/type-system/rust/src/knowledge/entity/id.rs index fde09798935..d5894a270cb 100644 --- a/libs/@blockprotocol/type-system/rust/src/knowledge/entity/id.rs +++ b/libs/@blockprotocol/type-system/rust/src/knowledge/entity/id.rs @@ -77,7 +77,7 @@ impl From for Uuid { } } -/// An entity's web ID and UUID, joined by `~`. +/// An entity's web ID and UUID joined by `~`, followed by `~` and a draft ID when it names a draft. #[derive(Debug, Clone, Copy, Hash, PartialEq, Eq)] #[cfg_attr( feature = "schemars", diff --git a/libs/@blockprotocol/type-system/rust/src/ontology/id/mod.rs b/libs/@blockprotocol/type-system/rust/src/ontology/id/mod.rs index 003fab9466d..856b9e7d3af 100644 --- a/libs/@blockprotocol/type-system/rust/src/ontology/id/mod.rs +++ b/libs/@blockprotocol/type-system/rust/src/ontology/id/mod.rs @@ -791,7 +791,7 @@ impl JsonSchema for VersionedUrl { "title": "Versioned URL", "description": "The versioned URL of a SemType ontology type (the $id of the \ schema). It must be of the form `${baseUrl}v/${version}`, where \ - `version` is currently a major version number.", + `version` is a major version number.", "format": "uri", "maxLength": 2048, "examples": ["https://hash.ai/@h/types/entity-type/user/v/1"], diff --git a/libs/@local/graph/api/Cargo.toml b/libs/@local/graph/api/Cargo.toml index d57c9525588..d2f4ea160bf 100644 --- a/libs/@local/graph/api/Cargo.toml +++ b/libs/@local/graph/api/Cargo.toml @@ -72,6 +72,7 @@ include_dir = { workspace = true } indexmap = { workspace = true } md-5 = { workspace = true } mime = { workspace = true } +percent-encoding = { workspace = true, features = ["alloc"] } reqwest = { workspace = true } scalar_api_reference = { workspace = true } schemars = { workspace = true, features = ["uuid1"] } diff --git a/libs/@local/graph/api/src/rest/caller.rs b/libs/@local/graph/api/src/rest/caller.rs index 3eb37039378..28efe8fa585 100644 --- a/libs/@local/graph/api/src/rest/caller.rs +++ b/libs/@local/graph/api/src/rest/caller.rs @@ -1,7 +1,4 @@ -use aide::{ - axum::{ApiRouter, routing::ApiMethodRouter}, - transform::TransformOperation, -}; +use aide::transform::TransformOperation; use schemars::JsonSchema; use serde::Serialize; use type_system::principal::actor::ActorId; @@ -9,7 +6,7 @@ use type_system::principal::actor::ActorId; use super::{ credentials::{Actor, AdmitsAnonymous, Credentials, MaybeActor}, extract::Json, - routing, + routing::{self, Route, Routes}, }; /// The resolved caller. @@ -52,35 +49,32 @@ fn document_authenticated_caller(operation: TransformOperation<'_>) -> Transform .summary("Get the authenticated caller") } -fn authenticated_caller_route() --> ApiMethodRouter { +fn authenticated_caller_route() -> Route { routing::get( authenticated_caller::>, document_authenticated_caller, ) } -/// `/caller` and `/authenticated-caller`. -pub(super) fn routes() -> ApiRouter { - ApiRouter::new() - .api_route( +pub(super) fn routes() -> Routes { + Routes::new() + .route( "/caller", routing::get(caller::>, document_caller), ) - .api_route( + .route( "/authenticated-caller", authenticated_caller_route::(), ) - .with_path_items(|path| path.tag("Caller")) + .tag("Caller") } -/// `/authenticated-caller`. -pub(super) fn authenticated_routes() --> ApiRouter { - ApiRouter::new() - .api_route( +pub(super) fn authenticated_routes() -> Routes +{ + Routes::new() + .route( "/authenticated-caller", authenticated_caller_route::(), ) - .with_path_items(|path| path.tag("Caller")) + .tag("Caller") } diff --git a/libs/@local/graph/api/src/rest/credentials.rs b/libs/@local/graph/api/src/rest/credentials.rs index d92f2f1a13a..46743656a9e 100644 --- a/libs/@local/graph/api/src/rest/credentials.rs +++ b/libs/@local/graph/api/src/rest/credentials.rs @@ -79,9 +79,8 @@ impl OperationInput for Actor { /// Credentials of an API that serves a request without a credential anonymously. /// -/// Only such an API reads its caller through [`MaybeActor`]. The public APIs answer a request -/// without a credential with `401` before a handler runs, so an optional caller would document -/// access they refuse. +/// Only such an API reads its caller through [`MaybeActor`], which documents anonymous access that +/// [`Audience::Public`] refuses before a handler runs. pub(super) trait AdmitsAnonymous: Credentials {} pub(super) struct MaybeActor(Option, PhantomData C>); @@ -104,6 +103,13 @@ impl FromRequestParts for MaybeActor { impl OperationInput for MaybeActor { fn operation_input(ctx: &mut GenContext, operation: &mut Operation) { + const { + assert!( + matches!(C::AUDIENCE, Audience::Internal), + "`AdmitsAnonymous` should only be implemented for credentials of \ + `Audience::Internal`" + ); + } let _: TransformOperation<'_> = TransformOperation::new(operation).security_requirement_multi([]); C::operation_input(ctx, operation); diff --git a/libs/@local/graph/api/src/rest/extract/json.rs b/libs/@local/graph/api/src/rest/extract/json.rs index 6d84f0cc01d..f5ef0b4effc 100644 --- a/libs/@local/graph/api/src/rest/extract/json.rs +++ b/libs/@local/graph/api/src/rest/extract/json.rs @@ -277,7 +277,6 @@ impl OperationOutput for Json { ) -> Option { // Documents the answer to a body that fails to serialize on the operation itself. Rejection::::inferred_responses(ctx, operation); - MessagePart::ResponseBody.mark(operation); axum::Json::::operation_response(ctx, operation) } diff --git a/libs/@local/graph/api/src/rest/extract/mod.rs b/libs/@local/graph/api/src/rest/extract/mod.rs index fee86a64d86..96dae543538 100644 --- a/libs/@local/graph/api/src/rest/extract/mod.rs +++ b/libs/@local/graph/api/src/rest/extract/mod.rs @@ -20,16 +20,16 @@ use serde_json::Value; #[cfg_attr( not(test), - expect(unused_imports, reason = "only the tests read query parameters so far") + expect(unused_imports, reason = "only the tests read query parameters") )] pub(in crate::rest) use self::query::Query; pub(in crate::rest) use self::{json::Json, path::Path}; -/// The part of a request or response that [`Json`], [`Path`] or [`Query`] reads or writes. +/// The part of a request that [`Json`], [`Path`] or [`Query`] reads. /// -/// Each extractor marks the operations it documents with the parts it reads or writes, and -/// [`openapi::build`] requires the mark for every path parameter, query parameter, request body and -/// JSON response an operation documents. +/// Each extractor marks the operations it documents with the parts it reads, and +/// [`openapi::build`] requires the mark for every path parameter, query parameter and request body +/// an operation documents. /// /// [`openapi::build`]: crate::rest::openapi::build #[derive(Debug, Clone, Copy)] @@ -37,7 +37,6 @@ pub(in crate::rest) enum MessagePart { PathParameters, QueryParameters, RequestBody, - ResponseBody, } impl MessagePart { @@ -49,7 +48,6 @@ impl MessagePart { Self::PathParameters => "x-hash-path-parameters", Self::QueryParameters => "x-hash-query-parameters", Self::RequestBody => "x-hash-request-body", - Self::ResponseBody => "x-hash-response-body", } } diff --git a/libs/@local/graph/api/src/rest/extract/query.rs b/libs/@local/graph/api/src/rest/extract/query.rs index dbca62601a3..4f1fcd0659c 100644 --- a/libs/@local/graph/api/src/rest/extract/query.rs +++ b/libs/@local/graph/api/src/rest/extract/query.rs @@ -74,7 +74,7 @@ impl Expose for QueryRejection { /// [`openapi::build`]: crate::rest::openapi::build #[cfg_attr( not(test), - expect(dead_code, reason = "only the tests read query parameters so far") + expect(dead_code, reason = "only the tests read query parameters") )] pub(in crate::rest) struct Query(pub T); diff --git a/libs/@local/graph/api/src/rest/extract/tests.rs b/libs/@local/graph/api/src/rest/extract/tests.rs index b98557364fd..f0556cce19c 100644 --- a/libs/@local/graph/api/src/rest/extract/tests.rs +++ b/libs/@local/graph/api/src/rest/extract/tests.rs @@ -17,7 +17,11 @@ use serde_json::{Value, json}; use tower::ServiceExt as _; use super::{Json, Path, Query, json::content_status}; -use crate::rest::{Api, openapi, routing, test_utils::NoCredentials}; +use crate::rest::{ + Api, openapi, + routing::{self, Routes}, + test_utils::NoCredentials, +}; #[derive(Deserialize, JsonSchema)] struct Subject { @@ -508,7 +512,7 @@ fn echo_document() { ..Info::default() }, || { - ApiRouter::new().api_route( + Routes::new().route( "/echo/{label}", routing::post(echo, |operation| { operation.id("echo").summary("Echo a request").description( diff --git a/libs/@local/graph/api/src/rest/middleware/mod.rs b/libs/@local/graph/api/src/rest/middleware/mod.rs index 38297f0dbd3..6e9046a15a2 100644 --- a/libs/@local/graph/api/src/rest/middleware/mod.rs +++ b/libs/@local/graph/api/src/rest/middleware/mod.rs @@ -145,10 +145,7 @@ where I: AuthenticationProvider> + 'static, L: AuthenticationProvider> + 'static, { - /// Attaches authentication and the caller budget to `api`. - /// - /// The public APIs answer a request that carries no credential with `401` before routing it to - /// a handler, the internal API serves it anonymously. + /// Attaches authentication and the caller budget to `api`, as its [`Audience`] requires. fn attach_api(&self, api: Api) -> Router { let Api { audience, diff --git a/libs/@local/graph/api/src/rest/middleware/tests.rs b/libs/@local/graph/api/src/rest/middleware/tests.rs index 8b70d313781..290482ed70c 100644 --- a/libs/@local/graph/api/src/rest/middleware/tests.rs +++ b/libs/@local/graph/api/src/rest/middleware/tests.rs @@ -246,18 +246,26 @@ async fn cache_control_gate_rejection() { async fn cache_control_fallback() { let router = middleware( &config(10, 10), - StaticAuthenticationProvider::NotRecognized, + StaticAuthenticationProvider::Verified(ActorId::new(Uuid::from_u128(1), ActorType::User)), StaticAuthenticationProvider::NotRecognized, StaticAuthenticationProvider::NotRecognized, ) .assemble(Router::new(), [test_utils::api("/first")], Router::new()); - for request in [ - request_to("/does-not-exist"), - request_with(Method::DELETE, "/first/test"), + for (request, status) in [ + (request_to("/does-not-exist"), StatusCode::NOT_FOUND), + ( + request_with(Method::DELETE, "/first/test"), + StatusCode::METHOD_NOT_ALLOWED, + ), ] { let path = request.uri().path().to_owned(); let response = send_request(&router, request).await; + assert_eq!( + response.status(), + status, + "{path} should be answered {status}" + ); assert_eq!( response.headers().get(CACHE_CONTROL), Some(&HeaderValue::from_static("private, no-store")), @@ -521,7 +529,7 @@ async fn panic_problem_document() { ); } -/// The entity route reads the store it was handed, and answers `500` when it cannot reach it. +/// The entity route reads the store it was handed, and answers `503` when it cannot acquire one. #[tokio::test] async fn entity_store_unreachable() { let router = middleware( @@ -553,8 +561,7 @@ async fn entity_store_unreachable() { ); } -/// The entity route requires an actor, so an anonymous request is answered `401` before it reads -/// the store. +/// An anonymous request to the entity route is answered `401` before the store is read. #[tokio::test] async fn entity_anonymous() { let router = middleware( diff --git a/libs/@local/graph/api/src/rest/mod.rs b/libs/@local/graph/api/src/rest/mod.rs index 8c5491ca4bf..f2fe49664e0 100644 --- a/libs/@local/graph/api/src/rest/mod.rs +++ b/libs/@local/graph/api/src/rest/mod.rs @@ -12,73 +12,30 @@ mod public; pub mod rate_limit; mod router; mod routing; +mod state; pub mod telemetry; #[cfg(test)] pub(crate) mod test_utils; -use alloc::sync::Arc; - use aide::openapi::OpenApi; use axum::Router; -use error_stack::Report; use hash_graph_store::pool::StorePool; use hash_middleware::authentication::Challenge; -use hash_temporal_client::TemporalClient; pub use self::router::{Dependencies, router}; +pub(crate) use self::state::ApiState; /// The callers an API admits, which selects the provider chain authenticating its requests. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub(crate) enum Audience { - /// Callers presenting an API token. A request without a credential is answered `401`. + /// Callers presenting an API token. A request without one is answered `401`. Public, - /// The HASH frontend and services. A request without a credential is served anonymously. + /// Callers presenting a Kratos session. A request without one is served anonymously, one + /// presenting an API token is answered `401`. Internal, } -/// What the handlers of the public and internal APIs read through [`State`]. -/// -/// [`State`]: axum::extract::State -pub(crate) struct ApiState { - store: Arc, - temporal_client: Option>, -} - -impl ApiState { - pub(crate) const fn new(store: Arc, temporal_client: Option>) -> Self { - Self { - store, - temporal_client, - } - } -} - -impl ApiState { - /// Acquires a store from the pool. - /// - /// The store starts the workflows that follow a write, such as computing embeddings, through - /// the Temporal client. - /// - /// # Errors - /// - /// Returns the pool's error if no store can be acquired. - pub(crate) fn acquire( - &self, - ) -> impl Future, Report>> + Send { - self.store.acquire(self.temporal_client.clone()) - } -} - -impl Clone for ApiState { - fn clone(&self) -> Self { - Self { - store: Arc::clone(&self.store), - temporal_client: self.temporal_client.clone(), - } - } -} - /// An API's routes, nested under its prefix, with the document describing them. /// /// The routes read the state `T`, which [`with_state`](Self::with_state) hands them. diff --git a/libs/@local/graph/api/src/rest/openapi/mod.rs b/libs/@local/graph/api/src/rest/openapi/mod.rs index c494da42e0a..4a68de1b2f0 100644 --- a/libs/@local/graph/api/src/rest/openapi/mod.rs +++ b/libs/@local/graph/api/src/rest/openapi/mod.rs @@ -1,7 +1,7 @@ #[cfg(test)] mod tests; -use alloc::collections::BTreeSet; +use alloc::{borrow::Cow, collections::BTreeSet}; use core::mem; use std::collections::HashSet; @@ -18,9 +18,10 @@ use aide::{ }; use convert_case::{Case, Casing as _}; use indexmap::IndexMap; +use percent_encoding::percent_decode_str; use serde_json::Value; -use super::{Api, credentials::Credentials, extract::MessagePart, middleware, routing}; +use super::{Api, credentials::Credentials, extract::MessagePart, middleware, routing::Routes}; /// Builds an API from its routes and generates the document describing them. /// @@ -37,22 +38,20 @@ use super::{Api, credentials::Credentials, extract::MessagePart, middleware, rou /// - The path parameters an operation documents are not exactly the placeholders of its path, or /// one of them is optional, a sequence or a map. /// - A query parameter is a map or a sequence of anything but single values. -/// - An operation documents a path parameter, a query parameter, a request body or a JSON response -/// that [`Json`], [`Path`] or [`Query`] did not read or write, or documents a header or cookie -/// parameter. +/// - An operation documents a path parameter, a query parameter or a request body that [`Json`], +/// [`Path`] or [`Query`] did not read, or documents a header or cookie parameter. /// - The problem variants of an operation cannot be documented, such as when the status of a /// variant already has a response that documents none. -/// - An operation was registered other than through [`routing`], or shares its `operationId` with -/// another operation of the document. +/// - Two operations of the document share an `operationId`. +/// - The document references a component schema it does not declare. /// /// [`Json`]: crate::rest::extract::Json /// [`Path`]: crate::rest::extract::Path /// [`Query`]: crate::rest::extract::Query -/// [`routing`]: crate::rest::routing pub(super) fn build( prefix: &'static str, info: Info, - build_routes: impl FnOnce() -> ApiRouter, + build_routes: impl FnOnce() -> Routes, transform: impl FnOnce(TransformOpenApi<'_>) -> TransformOpenApi<'_>, ) -> Api { #[expect( @@ -71,7 +70,7 @@ pub(super) fn build( ..OpenApi::default() }; let router = ApiRouter::new() - .nest(prefix, build_routes()) + .nest(prefix, build_routes().into_router()) .finish_api_with(&mut document, |mut document| { for (name, scheme) in C::schemes() { document = document.security_scheme(name, scheme); @@ -84,9 +83,7 @@ pub(super) fn build( assert_security_schemes_declared(&mut document); assert_parameters_readable(&mut document); assert_read_through_extractors(&mut document); - // The tests of the checks above register their operations through aide directly, so this - // check runs after them. - assert_routed(&mut document); + assert_operation_ids_unique(&document); remove_unreferenced_schemas(&mut document); Api { audience: C::AUDIENCE, @@ -220,10 +217,10 @@ fn assert_parameters_readable(document: &mut OpenApi) { } /// Checks that [`Json`], [`Path`] and [`Query`] read every path parameter, query parameter and -/// request body and write every JSON response an operation documents, and removes their marks. +/// request body an operation documents, and removes their marks. /// -/// Axum's own extractors and `axum::Json` answer a failure with plain text. An operation that -/// documents a header or cookie parameter panics. +/// Axum's own extractors answer a failure with plain text. An operation that documents a header or +/// cookie parameter panics. /// /// [`Json`]: crate::rest::extract::Json /// [`Path`]: crate::rest::extract::Path @@ -252,7 +249,6 @@ fn assert_read_through_extractors(document: &mut OpenApi) { let path_read = marked(MessagePart::PathParameters); let query_read = marked(MessagePart::QueryParameters); let body_read = marked(MessagePart::RequestBody); - let response_written = marked(MessagePart::ResponseBody); for parameter in shared .iter() .chain(operation.parameters.iter().filter_map(ReferenceOr::as_item)) @@ -287,43 +283,21 @@ fn assert_read_through_extractors(document: &mut OpenApi) { "{method} {path} should read its request body through `rest::extract::Json`, \ whose rejections are problem details" ); - let answers_json = operation - .responses - .iter() - .flat_map(|responses| responses.responses.values().chain(&responses.default)) - .filter_map(ReferenceOr::as_item) - .any(|response| response.content.contains_key("application/json")); - assert!( - !answers_json || response_written, - "{method} {path} should answer with JSON through `rest::extract::Json`, which \ - answers a body that fails to serialize with problem details" - ); } } } -/// Checks that [`routing`] registered every operation and that no two operations share an -/// `operationId`, and removes the marks. -/// -/// [`routing`]: crate::rest::routing -fn assert_routed(document: &mut OpenApi) { - let Some(paths) = &mut document.paths else { +/// Checks that no two operations of `document` share an `operationId`. +fn assert_operation_ids_unique(document: &OpenApi) { + let Some(paths) = &document.paths else { return; }; let mut ids = HashSet::new(); - for (path, item) in &mut paths.paths { - let Some(item) = item.as_item_mut() else { + for (path, item) in &paths.paths { + let Some(item) = item.as_item() else { continue; }; - for (method, operation) in iter_operations_mut(item) { - assert!( - operation - .extensions - .shift_remove(routing::EXTENSION) - .is_some(), - "{method} {path} should be registered through `rest::routing`, which admits only \ - handlers answering with `rest::extract::Json` or a `Rejection`" - ); + for (method, operation) in item.iter() { if let Some(id) = &operation.operation_id { assert!( ids.insert(id.clone()), @@ -352,11 +326,17 @@ fn remove_unreferenced_schemas(document: &mut OpenApi) { ); let mut referenced = HashSet::new(); while let Some(name) = pending.pop() { - if let Some(schema) = schemas.get(name.as_str()) - && referenced.insert(name) - { - schema_references(schema.json_schema.as_value(), &mut pending); + if referenced.contains(&name) { + continue; } + let schema = schemas.get(&name).unwrap_or_else(|| { + panic!( + "the document should declare every component schema it references, `{name}` is \ + missing" + ) + }); + schema_references(schema.json_schema.as_value(), &mut pending); + referenced.insert(name); } if let Some(components) = &mut document.components { components.schemas = schemas @@ -373,9 +353,9 @@ fn schema_references(value: &Value, references: &mut Vec) { if let Some(name) = object .get("$ref") .and_then(Value::as_str) - .and_then(|reference| reference.strip_prefix("#/components/schemas/")) + .and_then(schema_name) { - references.push(name.to_owned()); + references.push(name.into_owned()); } for member in object.values() { schema_references(member, references); @@ -390,6 +370,21 @@ fn schema_references(value: &Value, references: &mut Vec) { } } +/// The name of the component schema `reference` points to, if it points to one. +/// +/// A reference names the schema by a JSON pointer token in a URI fragment: `~` and `/` as `~0` and +/// `~1`, and the characters a fragment does not allow percent-encoded. +fn schema_name(reference: &str) -> Option> { + let token = percent_decode_str(reference.strip_prefix("#/components/schemas/")?) + .decode_utf8() + .ok()?; + Some(if token.contains('~') { + Cow::Owned(token.replace("~1", "/").replace("~0", "~")) + } else { + token + }) +} + /// The schema of `parameter`, unless it describes its value by media type instead. fn parameter_schema(parameter: &ParameterData) -> Option<&Value> { if let ParameterSchemaOrContent::Schema(schema) = ¶meter.format { @@ -418,8 +413,8 @@ fn resolved<'schema>( schema .get("$ref") .and_then(Value::as_str) - .and_then(|reference| reference.strip_prefix("#/components/schemas/")) - .and_then(|name| schemas?.get(name)) + .and_then(schema_name) + .and_then(|name| schemas?.get(name.as_ref())) .map(|component| component.json_schema.as_value()) }; let Some(next) = next else { diff --git a/libs/@local/graph/api/src/rest/openapi/tests.rs b/libs/@local/graph/api/src/rest/openapi/tests.rs index 775995178c5..0bcc9a61db3 100644 --- a/libs/@local/graph/api/src/rest/openapi/tests.rs +++ b/libs/@local/graph/api/src/rest/openapi/tests.rs @@ -19,7 +19,8 @@ use super::reference_responses; use crate::rest::{ Api, extract::{Json, Path, Query}, - middleware, openapi, routing, + middleware, openapi, + routing::{self, Routes}, test_utils::NoCredentials, }; @@ -42,6 +43,11 @@ impl Problem for QueryProblem { const VARIANTS: &'static [Variant] = &[Variant::of::()]; } +/// Gives `operation` the `operationId` and summary [`routing`] requires. +fn named(operation: TransformOperation<'_>) -> TransformOperation<'_> { + operation.id("getEntities").summary("Get entities") +} + fn unsupported_query(operation: TransformOperation<'_>) -> TransformOperation<'_> { operation.response::<400, Rejection>() } @@ -133,12 +139,12 @@ fn build_duplicate_response() { "/test", Info::default(), || { - ApiRouter::new().api_route( + Routes::new().route( "/duplicate", - get_with( - || async {}, + routing::get( + async || Json::<_, 200>(true), |operation| { - operation + named(operation) .response::<400, ProblemDetails<'static>>() .response::<400, ProblemDetails<'static>>() }, @@ -163,9 +169,12 @@ fn build_path_placeholder_undocumented() { "/test", Info::default(), || { - ApiRouter::new().api_route( + Routes::new().route( "/entities/{entity}", - get(async |Path((entity,)): Path<(String,)>| entity), + routing::get( + async |Path((entity,)): Path<(String,)>| Json::<_, 200>(entity), + named, + ), ) }, |document| document, @@ -179,11 +188,14 @@ fn build_path_parameter_without_placeholder() { "/test", Info::default(), || { - ApiRouter::new().api_route( + Routes::new().route( "/entities/{entity}", - get(async |Path(DraftPath { entity, draft }): Path| { - format!("{entity}/{draft}") - }), + routing::get( + async |Path(DraftPath { entity, draft }): Path| { + Json::<_, 200>(format!("{entity}/{draft}")) + }, + named, + ), ) }, |document| document, @@ -232,7 +244,7 @@ where "/test", Info::default(), || { - ApiRouter::new().api_route( + Routes::new().route( "/entities/{entity}", routing::get( async move |Path(parameters): Path| Json::<_, 200>(read(parameters)), @@ -323,7 +335,7 @@ where "/test", Info::default(), || { - ApiRouter::new().api_route( + Routes::new().route( "/entities", routing::get( async move |Query(parameters): Query| Json::<_, 200>(read(parameters)), @@ -380,12 +392,13 @@ fn build_axum_path() { "/test", Info::default(), || { - ApiRouter::new().api_route( + Routes::new().route( "/entities/{entity}", - get( + routing::get( async |axum::extract::Path(NewtypePath { entity }): axum::extract::Path< NewtypePath, - >| entity.0, + >| Json::<_, 200>(entity.0), + named, ), ) }, @@ -400,12 +413,13 @@ fn build_axum_extra_query() { "/test", Info::default(), || { - ApiRouter::new().api_route( + Routes::new().route( "/entities", - get( + routing::get( async |axum_extra::extract::Query(ValuesQuery { ids, limit }): axum_extra::extract::Query< ValuesQuery, - >| format!("{ids:?} {limit:?}"), + >| Json::<_, 200>(format!("{ids:?} {limit:?}")), + named, ), ) }, @@ -419,7 +433,12 @@ fn build_bytes_body() { let _: Api = openapi::build::( "/test", Info::default(), - || ApiRouter::new().api_route("/entities", post(async |body: bytes::Bytes| body)), + || { + Routes::new().route( + "/entities", + routing::post(async |body: bytes::Bytes| Json::<_, 200>(body.len()), named), + ) + }, |document| document, ); } @@ -429,16 +448,16 @@ fn build_with_parameter(parameter: Parameter) { "/test", Info::default(), move || { - ApiRouter::new().api_route( + Routes::new().route( "/entities", - get_with( - || async {}, + routing::get( + async || Json::<_, 200>(true), move |mut operation| { operation .inner_mut() .parameters .push(ReferenceOr::Item(parameter)); - operation + named(operation) }, ), ) @@ -483,36 +502,6 @@ fn build_cookie_parameter() { }); } -#[test] -#[should_panic(expected = "should answer with JSON through `rest::extract::Json`")] -fn build_axum_json_response() { - let _: Api = openapi::build::( - "/test", - Info::default(), - || ApiRouter::new().api_route("/entities", get(async || axum::Json(0_u8))), - |document| document, - ); -} - -#[test] -#[should_panic(expected = "should be registered through `rest::routing`")] -fn build_unrouted() { - let _: Api = openapi::build::( - "/test", - Info::default(), - || { - ApiRouter::new().api_route( - "/entities", - get_with( - async || Json::<_, 200>(0_u8), - |operation| operation.id("listEntities").summary("List entities"), - ), - ) - }, - |document| document, - ); -} - #[test] #[should_panic( expected = "should have an `operationId` of its own, another operation already has `getEntity`" @@ -528,27 +517,27 @@ fn build_duplicate_operation_id() { |operation| operation.id("getEntity").summary("Get an entity"), ) }; - ApiRouter::new() - .api_route("/first", route()) - .api_route("/second", route()) + Routes::new() + .route("/first", route()) + .route("/second", route()) }, |document| document, ); } -/// Path parameters with a field whose schema other schemas reference. +/// Path parameters whose field has a component schema of its own. #[derive(serde::Deserialize, schemars::JsonSchema)] struct EntityPath { entity: EntityUuid, } -/// A response with a field whose schema other schemas reference. +/// A response whose field has a component schema of its own. #[derive(serde::Serialize, schemars::JsonSchema)] struct EntityResponse { entity: EntityName, } -/// A name the path and the response share. +/// A name that only responses reference. #[derive(serde::Serialize, schemars::JsonSchema)] struct EntityName(String); @@ -560,7 +549,7 @@ fn build_unreferenced_schemas() { "/test", Info::default(), || { - ApiRouter::new().api_route( + Routes::new().route( "/entities/{entity}", routing::get( async |Path(EntityPath { entity }): Path| { @@ -593,3 +582,47 @@ fn build_unreferenced_schemas() { "only the schemas the document references should remain" ); } + +/// A response whose schema name a reference has to encode. +#[derive(serde::Serialize, schemars::JsonSchema)] +#[schemars(rename = "Entity page/v1")] +struct EntityPage { + entity: EntityName, +} + +/// A schema whose name contains characters a reference encodes stays, as do the schemas it +/// references. +#[test] +fn build_encoded_schema_name() { + let api: Api = openapi::build::( + "/test", + Info::default(), + || { + Routes::new().route( + "/entities", + routing::get( + async || { + Json::<_, 200>(EntityPage { + entity: EntityName(String::new()), + }) + }, + named, + ), + ) + }, + |document| document, + ); + + let schemas = api + .document() + .components + .as_ref() + .map_or_else(BTreeSet::new, |components| { + components.schemas.keys().map(String::as_str).collect() + }); + assert!( + schemas.contains("Entity page/v1") && schemas.contains("EntityName"), + "the schema with the encoded name and the schemas it references should remain, got \ + {schemas:?}" + ); +} diff --git a/libs/@local/graph/api/src/rest/public/entities/v1/entity.rs b/libs/@local/graph/api/src/rest/public/entities/v1/entity.rs index 287dd331920..c433ca27606 100644 --- a/libs/@local/graph/api/src/rest/public/entities/v1/entity.rs +++ b/libs/@local/graph/api/src/rest/public/entities/v1/entity.rs @@ -1,9 +1,9 @@ //! `GET /{entityId}`: one entity by its ID. -use alloc::borrow::Cow; +use alloc::{borrow::Cow, collections::BTreeSet}; use core::error::Error; -use aide::{axum::ApiRouter, transform::TransformOperation}; +use aide::transform::TransformOperation; use axum::extract::State; use error_stack::{Report, ResultExt as _}; use hash_graph_store::{entity::EntityStore as _, pool::StorePool}; @@ -24,14 +24,14 @@ use crate::rest::{ credentials::Actor, extract::{Json, Path}, public::Credentials, - routing, + routing::{self, Routes}, }; /// The path of one entity. #[derive(Deserialize, JsonSchema)] #[serde(rename_all = "camelCase")] struct EntityPath { - /// The ID of the entity to read. + /// The ID of the entity to read. An ID that names a draft is answered with `400`. #[serde(deserialize_with = "live_entity_id")] entity_id: EntityId, } @@ -59,7 +59,7 @@ struct EntityMetadata { /// The ID of the entity. entity_id: EntityId, /// The entity types the entity is an instance of, without the types they inherit from. - entity_type_ids: Vec, + entity_type_ids: BTreeSet, } /// The entity does not exist, is archived, or the caller may not view it. @@ -90,7 +90,7 @@ impl ProblemVariant for EntityNotFound { } } -/// The ways `GET /{entityId}` refuses a request. +/// The ways [`get_entity`] refuses a request its extractors accept. struct GetEntityProblem; impl Problem for GetEntityProblem { @@ -150,17 +150,10 @@ where return Err(Report::new(GetEntityError::Archived(entity_id)).into()); } - let mut entity_type_ids = entity - .metadata - .entity_type_ids - .into_iter() - .collect::>(); - entity_type_ids.sort(); - Ok(Json(Entity { metadata: EntityMetadata { entity_id: entity.metadata.record_id.entity_id, - entity_type_ids, + entity_type_ids: entity.metadata.entity_type_ids.into_iter().collect(), }, })) } @@ -169,20 +162,17 @@ fn document_get_entity(operation: TransformOperation<'_>) -> TransformOperation< operation .id("getEntity") .summary("Get an entity") - .description( - "Returns the current edition of the entity. Answers `404` when the entity does not \ - exist, is archived, or the caller may not view it.", - ) + .description("Returns the entity's ID and the entity types of its current edition.") } -pub(super) fn routes() -> ApiRouter> +pub(super) fn routes() -> Routes> where S: StorePool + Send + Sync + 'static, { - ApiRouter::new() - .api_route( + Routes::new() + .route( "/{entityId}", routing::get(get_entity::, document_get_entity), ) - .with_path_items(|path| path.tag("Entity")) + .tag("Entity") } diff --git a/libs/@local/graph/api/src/rest/router.rs b/libs/@local/graph/api/src/rest/router.rs index c45638f7453..015ff63e85b 100644 --- a/libs/@local/graph/api/src/rest/router.rs +++ b/libs/@local/graph/api/src/rest/router.rs @@ -92,16 +92,10 @@ where .map(|api| api.with_state(state.clone())) .collect::>(); let documentation = documentation::routes(&apis); - // The tracing layer sits inside the Sentry layers: the errors it logs reach Sentry through the - // request's hub, which attaches the request to them. - let mut router = middleware - .assemble(legacy::routes::(), apis, documentation) - .layer(telemetry::layer()) - .layer( - ServiceBuilder::new() - .layer(NewSentryLayer::new_from_top()) - .layer(SentryHttpLayer::default().enable_transaction()), - ) + + // Only the legacy routes read dependencies as extensions, the public and internal APIs read + // them through `ApiState`. + let mut legacy_routes = legacy::routes::() .layer(Extension(dependencies.store)) .layer(Extension(Arc::new(dependencies.postgres))) .layer(Extension(dependencies.temporal_client)) @@ -111,11 +105,21 @@ where .layer(Extension(dependencies.api_tokens)) .layer(Extension(dependencies.compiler)) .layer(Extension(dependencies.clustering)); - if let Some(query_logger) = dependencies.query_logger { - router = router.layer(Extension(query_logger)); + legacy_routes = legacy_routes.layer(Extension(query_logger)); } + // The tracing layer sits inside the Sentry layers: the errors it logs reach Sentry through the + // request's hub, which attaches the request to them. + let router = middleware + .assemble(legacy_routes, apis, documentation) + .layer(telemetry::layer()) + .layer( + ServiceBuilder::new() + .layer(NewSentryLayer::new_from_top()) + .layer(SentryHttpLayer::default().enable_transaction()), + ); + merge_probe(router) } diff --git a/libs/@local/graph/api/src/rest/routing.rs b/libs/@local/graph/api/src/rest/routing.rs index 48132390695..8efcf819cda 100644 --- a/libs/@local/graph/api/src/rest/routing.rs +++ b/libs/@local/graph/api/src/rest/routing.rs @@ -1,32 +1,28 @@ -//! [`get`] and [`post`] register the operations of the public and internal APIs. +//! [`get`] and [`post`] create the routes of the public and internal APIs, and [`Routes`] collects +//! them for [`openapi::build`]. //! //! They accept only a handler answering with [`Json`] or with `Result, Rejection<_>>`, //! so a successful response is JSON that [`Json`] documents and an error is a problem details //! response that the [`Problem`](problematic::Problem) of the [`Rejection`] documents. A handler -//! answering with anything else, such as a bare `StatusCode`, fails to compile. Each marks the -//! operations it registers, and [`openapi::build`] panics for an operation registered any other -//! way. +//! answering with anything else, such as a bare `StatusCode`, fails to compile. [`Routes`] accepts +//! no other route, so a handler reaches an API only through [`get`] or [`post`]. //! //! [`openapi::build`]: crate::rest::openapi::build use aide::{ OperationInput, OperationOutput, - axum::routing::{self, ApiMethodRouter}, + axum::{ + ApiRouter, + routing::{self, ApiMethodRouter}, + }, operation::OperationHandler, transform::TransformOperation, }; use axum::handler::Handler; use problematic::Rejection; -use serde_json::Value; use super::extract::Json; -/// The key of the extension that marks an operation [`get`] or [`post`] registered, which -/// [`openapi::build`] removes again. -/// -/// [`openapi::build`]: crate::rest::openapi::build -pub(in crate::rest) const EXTENSION: &str = "x-hash-routed"; - /// A response a handler of the public and internal APIs answers with. pub(in crate::rest) trait Outcome: OperationOutput {} @@ -37,6 +33,32 @@ impl Outcome for Result, Rejection> { } +/// A route [`get`] or [`post`] created. +pub(in crate::rest) struct Route(ApiMethodRouter); + +/// The routes of an API, each created by [`get`] or [`post`]. +pub(in crate::rest) struct Routes(ApiRouter); + +impl Routes { + pub(in crate::rest) fn new() -> Self { + Self(ApiRouter::new()) + } + + /// Serves `route` at `path`. + pub(in crate::rest) fn route(self, path: &str, route: Route) -> Self { + Self(self.0.api_route(path, route.0)) + } + + /// Tags the operations of every route added before this call with `tag`. + pub(in crate::rest) fn tag(self, tag: &str) -> Self { + Self(self.0.with_path_items(|item| item.tag(tag))) + } + + pub(in crate::rest) fn into_router(self) -> ApiRouter { + self.0 + } +} + /// Routes `GET` requests to `handler`, documented by `transform`. /// /// # Panics @@ -45,7 +67,7 @@ impl Outcome for Result, Rejection> pub(in crate::rest) fn get( handler: H, transform: impl FnOnce(TransformOperation<'_>) -> TransformOperation<'_>, -) -> ApiMethodRouter +) -> Route where H: Handler + OperationHandler, I: OperationInput, @@ -53,7 +75,9 @@ where S: Clone + Send + Sync + 'static, T: 'static, { - routing::get_with(handler, |operation| registered(transform(operation))) + Route(routing::get_with(handler, |operation| { + identified(transform(operation)) + })) } /// Routes `POST` requests to `handler`, documented by `transform`. @@ -63,12 +87,12 @@ where /// Panics if `transform` leaves the operation without an `operationId` or a `summary`. #[cfg_attr( not(test), - expect(dead_code, reason = "only the tests route `POST` requests so far") + expect(dead_code, reason = "only the tests route `POST` requests") )] pub(in crate::rest) fn post( handler: H, transform: impl FnOnce(TransformOperation<'_>) -> TransformOperation<'_>, -) -> ApiMethodRouter +) -> Route where H: Handler + OperationHandler, I: OperationInput, @@ -76,11 +100,13 @@ where S: Clone + Send + Sync + 'static, T: 'static, { - routing::post_with(handler, |operation| registered(transform(operation))) + Route(routing::post_with(handler, |operation| { + identified(transform(operation)) + })) } -/// Checks that `operation` has an `operationId` and a `summary`, and marks it. -fn registered(mut operation: TransformOperation<'_>) -> TransformOperation<'_> { +/// Checks that `operation` has an `operationId` and a `summary`. +fn identified(mut operation: TransformOperation<'_>) -> TransformOperation<'_> { let inner = operation.inner_mut(); let id = inner .operation_id @@ -90,21 +116,18 @@ fn registered(mut operation: TransformOperation<'_>) -> TransformOperation<'_> { inner.summary.is_some(), "the operation `{id}` should have a summary" ); - inner - .extensions - .insert(EXTENSION.to_owned(), Value::Bool(true)); operation } #[cfg(test)] mod tests { - use super::get; + use super::{Route, get}; use crate::rest::extract::Json; #[test] #[should_panic(expected = "the operation should have an `operationId`")] fn get_missing_id() { - let _: aide::axum::routing::ApiMethodRouter = get( + let _: Route<()> = get( async || Json::<_, 200>(true), |operation| operation.summary("Answer"), ); @@ -113,7 +136,7 @@ mod tests { #[test] #[should_panic(expected = "the operation `answer` should have a summary")] fn get_missing_summary() { - let _: aide::axum::routing::ApiMethodRouter = get( + let _: Route<()> = get( async || Json::<_, 200>(true), |operation| operation.id("answer"), ); diff --git a/libs/@local/graph/api/src/rest/state.rs b/libs/@local/graph/api/src/rest/state.rs new file mode 100644 index 00000000000..c8e3fbb9656 --- /dev/null +++ b/libs/@local/graph/api/src/rest/state.rs @@ -0,0 +1,47 @@ +use alloc::sync::Arc; + +use error_stack::Report; +use hash_graph_store::pool::StorePool; +use hash_temporal_client::TemporalClient; + +/// The store pool and Temporal client a handler reads through [`State`]. +/// +/// [`State`]: axum::extract::State +pub(crate) struct ApiState { + store: Arc, + temporal_client: Option>, +} + +impl ApiState { + pub(crate) const fn new(store: Arc, temporal_client: Option>) -> Self { + Self { + store, + temporal_client, + } + } +} + +impl ApiState { + /// Acquires a store from the pool. + /// + /// The store starts the workflows that follow a write, such as computing embeddings, through + /// the Temporal client. + /// + /// # Errors + /// + /// Returns the pool's error if no store can be acquired. + pub(crate) fn acquire( + &self, + ) -> impl Future, Report>> + Send { + self.store.acquire(self.temporal_client.clone()) + } +} + +impl Clone for ApiState { + fn clone(&self) -> Self { + Self { + store: Arc::clone(&self.store), + temporal_client: self.temporal_client.clone(), + } + } +} diff --git a/libs/@local/graph/api/src/rest/test_utils/authentication.rs b/libs/@local/graph/api/src/rest/test_utils/authentication.rs index 72267b3d738..ecf0639394a 100644 --- a/libs/@local/graph/api/src/rest/test_utils/authentication.rs +++ b/libs/@local/graph/api/src/rest/test_utils/authentication.rs @@ -2,10 +2,7 @@ use alloc::sync::Arc; use core::{num::NonZeroU32, ops::ControlFlow, pin::pin}; use std::collections::HashMap; -use aide::{ - axum::{ApiRouter, routing::ApiMethodRouter}, - openapi::{Info, ReferenceOr, SecurityScheme}, -}; +use aide::openapi::{Info, ReferenceOr, SecurityScheme}; use axum::{Router, body::Body}; use error_stack::Report; use hash_graph_authentication::{ @@ -44,7 +41,8 @@ use super::{echo_caller, response_json}; use crate::rest::{ credentials::{API_TOKEN, Actor, AdmitsAnonymous, Credentials, MaybeActor}, middleware::Middleware, - openapi, routing, + openapi, + routing::{self, Route, Routes}, }; const SERVICE_SECRET: &str = "hash-svc-test-secret"; @@ -113,8 +111,8 @@ struct Advertised { api_token: bool, } -/// Routes `GET` to the caller, which it reads through [`Actor`]. -fn required_route() -> ApiMethodRouter { +/// A `GET` route answering with the caller it reads through [`Actor`]. +fn required_route() -> Route<()> { routing::get( async |actor: Actor| echo_caller(Some(actor.into())).await, |operation| { @@ -125,8 +123,8 @@ fn required_route() -> ApiMethodRouter { ) } -/// Routes `GET` to the caller, which it reads through [`MaybeActor`]. -fn optional_route() -> ApiMethodRouter { +/// A `GET` route answering with the caller it reads through [`MaybeActor`]. +fn optional_route() -> Route<()> { routing::get(echo_caller::>, |operation| { operation.id("getOptionalCaller").summary("Get the caller") }) @@ -139,7 +137,7 @@ fn optional_route() -> ApiMethodRouter { fn caller_router( operator: ActorId, session_actor: ActorId, - build_routes: impl FnOnce() -> ApiRouter, + build_routes: impl FnOnce() -> Routes, ) -> (Router, Advertised) { let public_provider = Arc::new(ApiTokenProvider::new( UnreadableTokens, @@ -214,7 +212,7 @@ enum Outcome { /// [`Actor`]. pub(in crate::rest) async fn assert_authentication() { assert_routes::(&["/test/required"], || { - ApiRouter::new().api_route("/required", required_route::()) + Routes::new().route("/required", required_route::()) }) .await; } @@ -223,9 +221,9 @@ pub(in crate::rest) async fn assert_authentication() { /// [`MaybeActor`], and on `/test/required`. pub(in crate::rest) async fn assert_anonymous_authentication() { assert_routes::(&["/test/optional", "/test/required"], || { - ApiRouter::new() - .api_route("/optional", optional_route::()) - .api_route("/required", required_route::()) + Routes::new() + .route("/optional", optional_route::()) + .route("/required", required_route::()) }) .await; } @@ -239,7 +237,7 @@ pub(in crate::rest) async fn assert_anonymous_authentication /// served. A service delegation and a Cloudflare Access JWT resolve as anonymous everywhere, which /// `/test/required` answers with `401`. Every `401` challenges with `C::CHALLENGE`. This ties the /// provider chain the audience selects to the schemes the document states. -async fn assert_routes(paths: &[&str], build_routes: impl FnOnce() -> ApiRouter) { +async fn assert_routes(paths: &[&str], build_routes: impl FnOnce() -> Routes) { let operator = ActorId::new(Uuid::from_u128(1), ActorType::Machine); let session_actor = ActorId::new(Uuid::from_u128(2), ActorType::User); let (router, advertised) = caller_router::(operator, session_actor, build_routes); diff --git a/libs/@local/graph/api/src/rest/test_utils/mod.rs b/libs/@local/graph/api/src/rest/test_utils/mod.rs index ff32a034c62..2d85dff9ca6 100644 --- a/libs/@local/graph/api/src/rest/test_utils/mod.rs +++ b/libs/@local/graph/api/src/rest/test_utils/mod.rs @@ -4,7 +4,6 @@ use alloc::sync::Arc; use aide::{ OperationInput, - axum::ApiRouter, openapi::{Info, SecurityScheme}, }; use axum::{ @@ -25,7 +24,13 @@ use tower::ServiceExt as _; pub(super) use self::authentication::{assert_anonymous_authentication, assert_authentication}; pub(super) use super::caller::caller as echo_caller; -use super::{Api, ApiState, Audience, credentials::Credentials, extract::Json, openapi, routing}; +use super::{ + Api, ApiState, Audience, + credentials::Credentials, + extract::Json, + openapi, + routing::{self, Routes}, +}; pub(super) async fn request(router: &Router, path: &str) -> Response { router @@ -93,7 +98,7 @@ pub(super) fn api(prefix: &'static str) -> Api { prefix, Info::default(), || { - ApiRouter::new().api_route( + Routes::new().route( "/test", routing::get( async || Json::<_, 200>(String::from("ok")), diff --git a/libs/@local/graph/api/tests/snapshots/openapi/entities-v1.snap.json b/libs/@local/graph/api/tests/snapshots/openapi/entities-v1.snap.json index c03d3c5263f..b89e7a57371 100644 --- a/libs/@local/graph/api/tests/snapshots/openapi/entities-v1.snap.json +++ b/libs/@local/graph/api/tests/snapshots/openapi/entities-v1.snap.json @@ -465,7 +465,7 @@ "type": "object" }, "EntityId": { - "description": "An entity's web ID and UUID, joined by `~`.", + "description": "An entity's web ID and UUID joined by `~`, followed by `~` and a draft ID when it names a draft.", "examples": [ "8d86c8c3-d66d-43d1-859b-676c3bcaeadc~be2c5b2f-e3e1-4f26-a0c4-0f8b5e9c4a1d" ], @@ -487,7 +487,8 @@ "items": { "$ref": "#/components/schemas/VersionedUrl" }, - "type": "array" + "type": "array", + "uniqueItems": true } }, "required": [ @@ -530,7 +531,7 @@ "type": "object" }, "VersionedUrl": { - "description": "The versioned URL of a SemType ontology type (the $id of the schema). It must be of the form `${baseUrl}v/${version}`, where `version` is currently a major version number.", + "description": "The versioned URL of a SemType ontology type (the $id of the schema). It must be of the form `${baseUrl}v/${version}`, where `version` is a major version number.", "examples": [ "https://hash.ai/@h/types/entity-type/user/v/1" ], @@ -557,17 +558,17 @@ "paths": { "/entities/v1/{entityId}": { "get": { - "description": "Returns the current edition of the entity. Answers `404` when the entity does not exist, is archived, or the caller may not view it.", + "description": "Returns the entity's ID and the entity types of its current edition.", "operationId": "getEntity", "parameters": [ { - "description": "The ID of the entity to read.", + "description": "The ID of the entity to read. An ID that names a draft is answered with `400`.", "in": "path", "name": "entityId", "required": true, "schema": { "$ref": "#/components/schemas/EntityId", - "description": "The ID of the entity to read." + "description": "The ID of the entity to read. An ID that names a draft is answered with `400`." }, "style": "simple" } diff --git a/libs/@local/graph/postgres-store/tests/deletion/purge.rs b/libs/@local/graph/postgres-store/tests/deletion/purge.rs index acdc469730d..963f3318abb 100644 --- a/libs/@local/graph/postgres-store/tests/deletion/purge.rs +++ b/libs/@local/graph/postgres-store/tests/deletion/purge.rs @@ -794,6 +794,14 @@ async fn query_after_purge_returns_empty() { // If the tombstone caused a read error, this would panic instead of returning 0 assert_eq!(count_entities(&api, entity_id, false).await, 0); assert_eq!(count_entities(&api, entity_id, true).await, 0); + assert!( + api.store + .get_entity_by_id(Some(api.account_id), entity_id, None, None) + .await + .expect("should read a purged entity without an error") + .is_none(), + "a purged entity should read as absent" + ); } /// Verifies provenance records the deleting actor, not the creating actor. diff --git a/libs/@local/graph/store/rust/src/entity/store.rs b/libs/@local/graph/store/rust/src/entity/store.rs index 62cf632325a..adae83ecf80 100644 --- a/libs/@local/graph/store/rust/src/entity/store.rs +++ b/libs/@local/graph/store/rust/src/entity/store.rs @@ -903,9 +903,10 @@ pub trait EntityStore { /// Get the latest edition of the [`Entity`], or the one live at the given timestamps. /// - /// An `actor_id` of [`None`] reads as the public actor. Returns [`None`] if the actor may not - /// view the entity or no edition of it is live at `transaction_time` and `decision_time`, each - /// of which defaults to now. + /// An `actor_id` of [`None`] reads as the public actor. Reads a draft only when `entity_id` + /// names one, and masks protected properties as [`query_entities`](Self::query_entities) + /// does. Returns [`None`] if the actor may not view the entity or no such edition is live + /// at `transaction_time` and `decision_time`, each of which defaults to now. /// /// # Errors /// @@ -994,8 +995,10 @@ pub trait EntityStore { ) .await? .ok_or_else(|| { - Report::new(QueryError) - .attach(format!("entity `{}` was not found", params.first_entity_id)) + Report::new(QueryError).attach(format!( + "entity `{}` is not visible", + params.first_entity_id + )) })?; let second_entity = self .get_entity_by_id( @@ -1007,7 +1010,7 @@ pub trait EntityStore { .await? .ok_or_else(|| { Report::new(QueryError).attach(format!( - "entity `{}` was not found", + "entity `{}` is not visible", params.second_entity_id )) })?; diff --git a/tests/graph/integration/postgres/entity.rs b/tests/graph/integration/postgres/entity.rs index ad62a71e757..a981042173a 100644 --- a/tests/graph/integration/postgres/entity.rs +++ b/tests/graph/integration/postgres/entity.rs @@ -563,8 +563,7 @@ async fn update() { assert_eq!(entity_v2.properties.properties(), page_v2.properties()); } -/// Seeds the organization type and creates an organization in the account's web, under no -/// policy of its own. +/// Creates an organization in the account's web, under no policy of its own. async fn create_organization(api: &mut DatabaseApi<'_>) -> EntityId { let organization: PropertyObject = serde_json::from_str(entity::ORGANIZATION_V1).expect("could not parse entity"); diff --git a/tests/hash-backend-integration/src/tests/graph/authentication/api-token.test.ts b/tests/hash-backend-integration/src/tests/graph/authentication/api-token.test.ts index 1bd39acf07f..20b8cb55680 100644 --- a/tests/hash-backend-integration/src/tests/graph/authentication/api-token.test.ts +++ b/tests/hash-backend-integration/src/tests/graph/authentication/api-token.test.ts @@ -2,6 +2,9 @@ import { beforeAll, describe, expect, it } from "vitest"; import { deleteKratosIdentity } from "@apps/hash-api/src/auth/ory-kratos"; import { ensureSystemGraphIsInitialized } from "@apps/hash-api/src/graph/ensure-system-graph-is-initialized"; +import { generateSystemEntityTypeSchema } from "@apps/hash-api/src/graph/ensure-system-graph-is-initialized/migrate-ontology-types/util"; +import { createEntity } from "@apps/hash-api/src/graph/knowledge/primitive/entity"; +import { createEntityType } from "@apps/hash-api/src/graph/ontology/primitive/entity-type"; import { getRequiredEnv } from "@local/hash-backend-utils/environment"; import { Logger } from "@local/hash-backend-utils/logger"; import { @@ -9,10 +12,12 @@ import { listApiTokens, revokeApiToken, } from "@local/hash-graph-sdk/api-token"; +import { generateTypeId } from "@local/hash-isomorphic-utils/ontology-types"; import { createTestImpureGraphContext, createTestUser } from "../../util"; import type { User } from "@apps/hash-api/src/graph/knowledge/system-types/user"; +import type { EntityTypeWithMetadata, WebId } from "@blockprotocol/type-system"; const logger = new Logger({ environment: "test", @@ -33,6 +38,7 @@ const requestWithToken = (path: string, token: string) => describe("API tokens", () => { let owner: User; let otherUser: User; + let ownerEntityType: EntityTypeWithMetadata; beforeAll(async () => { await ensureSystemGraphIsInitialized({ @@ -44,6 +50,25 @@ describe("API tokens", () => { owner = await createTestUser(graphContext, "apitokenowner", logger); otherUser = await createTestUser(graphContext, "apitokenother", logger); + ownerEntityType = await createEntityType( + graphContext, + { actorId: owner.accountId }, + { + webId: owner.accountId as WebId, + schema: generateSystemEntityTypeSchema({ + entityTypeId: generateTypeId({ + kind: "entity-type", + title: "API token test", + webShortname: owner.shortname!, + }), + title: "API token test", + description: "An entity type only the API token tests use.", + properties: [], + outgoingLinks: [], + }), + }, + ); + return async () => { await Promise.all( [owner, otherUser].map((user) => @@ -98,21 +123,58 @@ describe("API tokens", () => { ).toBe("revoked"); }); - it("authenticates a token as its owner on the public API", async () => { - const { token } = await createApiToken( - graphApi, - { actorId: owner.accountId }, - { name: "public API in integration test" }, - ); + it("reads an entity on the public API as the token's owner", async () => { + const authentication = { actorId: owner.accountId }; + const entity = await createEntity(graphContext, authentication, { + webId: owner.accountId as WebId, + properties: { value: {} }, + entityTypeIds: [ownerEntityType.schema.$id], + }); + const [{ token: ownerToken }, { token: otherToken }] = await Promise.all([ + createApiToken(graphApi, authentication, { + name: "owner on the public API in integration test", + }), + createApiToken( + graphApi, + { actorId: otherUser.accountId }, + { name: "other user on the public API in integration test" }, + ), + ]); - const entityId = owner.entity.metadata.recordId.entityId; - const response = await requestWithToken(`/entities/v1/${entityId}`, token); + const { entityId } = entity; + const [ownerResponse, otherResponse] = await Promise.all([ + requestWithToken(`/entities/v1/${entityId}`, ownerToken), + requestWithToken(`/entities/v1/${entityId}`, otherToken), + ]); - expect(response.status).toBe(200); - expect(await response.json()).toMatchObject({ metadata: { entityId } }); + expect(ownerResponse.status).toBe(200); + expect(await ownerResponse.json()).toMatchObject({ + metadata: { entityId }, + }); + expect(otherResponse.status).toBe(404); }); - it("rejects a revoked token instead of serving the request anonymously", async () => { + it("answers an archived entity with 404 on the public API", async () => { + const authentication = { actorId: owner.accountId }; + const entity = await createEntity(graphContext, authentication, { + webId: owner.accountId as WebId, + properties: { value: {} }, + entityTypeIds: [ownerEntityType.schema.$id], + }); + await entity.archive(graphApi, authentication, graphContext.provenance); + const { token } = await createApiToken(graphApi, authentication, { + name: "archived entity on the public API in integration test", + }); + + const response = await requestWithToken( + `/entities/v1/${entity.entityId}`, + token, + ); + + expect(response.status).toBe(404); + }); + + it("rejects a revoked token", async () => { const { token, apiToken } = await createApiToken( graphApi, { actorId: owner.accountId }, @@ -133,6 +195,9 @@ describe("API tokens", () => { expect(response.headers.get("content-type")).toBe( "application/problem+json", ); + expect(response.headers.get("www-authenticate")).toContain( + 'error="invalid_token"', + ); }); it("does not accept a token on the legacy API", async () => { From b0bdc5071796988ed4fdeece8d36f8495bd7d90c Mon Sep 17 00:00:00 2001 From: Tim Diekmann Date: Sun, 18 Oct 2026 16:38:34 +0200 Subject: [PATCH 09/13] Introduce `LiveEntityId` and read `get_entity_by_id` through `query_entities` --- .../rust/src/knowledge/entity/id.rs | 8 -- .../graph/api/src/rest/legacy/entity/mod.rs | 2 +- .../api/src/rest/public/entities/v1/entity.rs | 96 +++++++++++++------ .../snapshots/openapi/entities-v1.snap.json | 2 +- .../store/postgres/knowledge/entity/mod.rs | 14 +-- .../graph/store/rust/src/entity/store.rs | 17 ++-- libs/@local/graph/type-fetcher/src/store.rs | 2 +- tests/graph/integration/postgres/drafts.rs | 34 ++++--- tests/graph/integration/postgres/lib.rs | 2 +- 9 files changed, 108 insertions(+), 69 deletions(-) diff --git a/libs/@blockprotocol/type-system/rust/src/knowledge/entity/id.rs b/libs/@blockprotocol/type-system/rust/src/knowledge/entity/id.rs index d5894a270cb..6aa0eeb32e1 100644 --- a/libs/@blockprotocol/type-system/rust/src/knowledge/entity/id.rs +++ b/libs/@blockprotocol/type-system/rust/src/knowledge/entity/id.rs @@ -79,14 +79,6 @@ impl From for Uuid { /// An entity's web ID and UUID joined by `~`, followed by `~` and a draft ID when it names a draft. #[derive(Debug, Clone, Copy, Hash, PartialEq, Eq)] -#[cfg_attr( - feature = "schemars", - derive(schemars::JsonSchema), - schemars( - with = "String", - example = "8d86c8c3-d66d-43d1-859b-676c3bcaeadc~be2c5b2f-e3e1-4f26-a0c4-0f8b5e9c4a1d" - ) -)] pub struct EntityId { pub web_id: WebId, pub entity_uuid: EntityUuid, diff --git a/libs/@local/graph/api/src/rest/legacy/entity/mod.rs b/libs/@local/graph/api/src/rest/legacy/entity/mod.rs index fec68a7dd50..7722be7a80d 100644 --- a/libs/@local/graph/api/src/rest/legacy/entity/mod.rs +++ b/libs/@local/graph/api/src/rest/legacy/entity/mod.rs @@ -761,7 +761,7 @@ where query_logger.capture(Some(actor_id), OpenApiQuery::DiffEntity(¶ms)); } - let store = store_pool + let mut store = store_pool .acquire(temporal_client.0) .await .map_err(report_to_response)?; diff --git a/libs/@local/graph/api/src/rest/public/entities/v1/entity.rs b/libs/@local/graph/api/src/rest/public/entities/v1/entity.rs index c433ca27606..6098522da9d 100644 --- a/libs/@local/graph/api/src/rest/public/entities/v1/entity.rs +++ b/libs/@local/graph/api/src/rest/public/entities/v1/entity.rs @@ -10,8 +10,8 @@ use hash_graph_store::{entity::EntityStore as _, pool::StorePool}; use hash_middleware::problem::{InternalServerError, ServiceUnavailable}; use http::StatusCode; use problematic::{Answer, Expose, Problem, ProblemType, ProblemVariant, Rejection, Variant}; -use schemars::JsonSchema; -use serde::{Deserialize, Deserializer, Serialize, de}; +use schemars::{JsonSchema, Schema, SchemaGenerator, json_schema}; +use serde::{Deserialize, Serialize}; use type_system::{ knowledge::entity::{EntityId, id::EntityUuid}, ontology::VersionedUrl, @@ -27,22 +27,67 @@ use crate::rest::{ routing::{self, Routes}, }; +/// An entity's web ID and UUID, joined by `~`. +/// +/// Unlike [`EntityId`], it never names a draft. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, derive_more::Display)] +#[serde(try_from = "EntityId", into = "EntityId")] +struct LiveEntityId(EntityId); + +impl LiveEntityId { + const fn new(web_id: WebId, entity_uuid: EntityUuid) -> Self { + Self(EntityId { + web_id, + entity_uuid, + draft_id: None, + }) + } +} + +/// The entity ID names a draft. +#[derive(Debug, derive_more::Display)] +#[display("the entity ID names a draft")] +struct DraftEntityId; + +impl TryFrom for LiveEntityId { + type Error = DraftEntityId; + + fn try_from(entity_id: EntityId) -> Result { + if entity_id.draft_id.is_some() { + return Err(DraftEntityId); + } + Ok(Self(entity_id)) + } +} + +impl From for EntityId { + fn from(entity_id: LiveEntityId) -> Self { + entity_id.0 + } +} + +impl JsonSchema for LiveEntityId { + fn schema_name() -> Cow<'static, str> { + Cow::Borrowed("EntityId") + } + + fn json_schema(_: &mut SchemaGenerator) -> Schema { + json_schema!({ + "type": "string", + "description": "An entity's web ID and UUID, joined by `~`.", + "examples": [ + "8d86c8c3-d66d-43d1-859b-676c3bcaeadc~be2c5b2f-e3e1-4f26-a0c4-0f8b5e9c4a1d", + ], + }) + } +} + /// The path of one entity. #[derive(Deserialize, JsonSchema)] #[serde(rename_all = "camelCase")] struct EntityPath { /// The ID of the entity to read. An ID that names a draft is answered with `400`. - #[serde(deserialize_with = "live_entity_id")] - entity_id: EntityId, -} - -/// Reads an entity ID that names no draft. -fn live_entity_id<'de, D: Deserializer<'de>>(deserializer: D) -> Result { - let entity_id = EntityId::deserialize(deserializer)?; - if entity_id.draft_id.is_some() { - return Err(de::Error::custom("the entity ID names a draft")); - } - Ok(entity_id) + entity_id: LiveEntityId, } /// An entity the caller may view. @@ -57,7 +102,7 @@ struct Entity { #[serde(rename_all = "camelCase")] struct EntityMetadata { /// The ID of the entity. - entity_id: EntityId, + entity_id: LiveEntityId, /// The entity types the entity is an instance of, without the types they inherit from. entity_type_ids: BTreeSet, } @@ -67,7 +112,7 @@ struct EntityMetadata { #[display("No entity `{entity_id}` is visible to the caller.")] struct EntityNotFound { #[serde(skip)] - entity_id: EntityId, + entity_id: LiveEntityId, } impl ProblemVariant for EntityNotFound { @@ -79,13 +124,10 @@ impl ProblemVariant for EntityNotFound { fn example() -> Option { Some(Self { - entity_id: EntityId { - web_id: WebId::new(Uuid::from_u128(0x8D86C8C3_D66D_43D1_859B_676C3BCAEADC)), - entity_uuid: EntityUuid::new(Uuid::from_u128( - 0xBE2C5B2F_E3E1_4F26_A0C4_0F8B5E9C4A1D, - )), - draft_id: None, - }, + entity_id: LiveEntityId::new( + WebId::new(Uuid::from_u128(0x8D86C8C3_D66D_43D1_859B_676C3BCAEADC)), + EntityUuid::new(Uuid::from_u128(0xBE2C5B2F_E3E1_4F26_A0C4_0F8B5E9C4A1D)), + ), }) } } @@ -104,9 +146,9 @@ impl Problem for GetEntityProblem { #[derive(Debug, derive_more::Display)] enum GetEntityError { #[display("no entity `{_0}` is visible to the caller")] - NotFound(EntityId), + NotFound(LiveEntityId), #[display("entity `{_0}` is archived")] - Archived(EntityId), + Archived(LiveEntityId), #[display("no store could be acquired")] Unavailable, #[display("the store could not be read")] @@ -137,12 +179,12 @@ async fn get_entity( where S: StorePool + Send + Sync + 'static, { - let store = state + let mut store = state .acquire() .await .change_context(GetEntityError::Unavailable)?; let entity = store - .get_entity_by_id(Some(actor.into()), entity_id, None, None) + .get_entity_by_id(Some(actor.into()), EntityId::from(entity_id), None, None) .await .change_context(GetEntityError::Store)? .ok_or_else(|| Report::new(GetEntityError::NotFound(entity_id)))?; @@ -152,7 +194,7 @@ where Ok(Json(Entity { metadata: EntityMetadata { - entity_id: entity.metadata.record_id.entity_id, + entity_id, entity_type_ids: entity.metadata.entity_type_ids.into_iter().collect(), }, })) diff --git a/libs/@local/graph/api/tests/snapshots/openapi/entities-v1.snap.json b/libs/@local/graph/api/tests/snapshots/openapi/entities-v1.snap.json index b89e7a57371..311681772c5 100644 --- a/libs/@local/graph/api/tests/snapshots/openapi/entities-v1.snap.json +++ b/libs/@local/graph/api/tests/snapshots/openapi/entities-v1.snap.json @@ -465,7 +465,7 @@ "type": "object" }, "EntityId": { - "description": "An entity's web ID and UUID joined by `~`, followed by `~` and a draft ID when it names a draft.", + "description": "An entity's web ID and UUID, joined by `~`.", "examples": [ "8d86c8c3-d66d-43d1-859b-676c3bcaeadc~be2c5b2f-e3e1-4f26-a0c4-0f8b5e9c4a1d" ], diff --git a/libs/@local/graph/postgres-store/src/store/postgres/knowledge/entity/mod.rs b/libs/@local/graph/postgres-store/src/store/postgres/knowledge/entity/mod.rs index bd349349450..1a500e1ed71 100644 --- a/libs/@local/graph/postgres-store/src/store/postgres/knowledge/entity/mod.rs +++ b/libs/@local/graph/postgres-store/src/store/postgres/knowledge/entity/mod.rs @@ -1975,17 +1975,12 @@ where } async fn get_entity_by_id( - &self, + &mut self, actor_id: Option, entity_id: EntityId, transaction_time: Option>, decision_time: Option>, ) -> Result, Report> { - let policy_components = PolicyComponents::builder(self, actor_id) - .with_action(ActionName::ViewEntity, MergePolicies::Yes) - .await - .change_context(QueryError)?; - let params = QueryEntitiesParams { filter: Filter::for_entity_by_entity_id(entity_id), temporal_axes: QueryTemporalAxesUnresolved::TransactionTime { @@ -1999,15 +1994,16 @@ where paths: Vec::new(), cursor: None, }, + // At most one edition matches: both temporal axes resolve to a single point in time, + // and `include_drafts` is set only when `entity_id` names a draft, which the filter + // then pins. Including drafts for an ID without one would match its drafts as well. limit: 1, conversions: Vec::new(), include_entity_types: None, include_drafts: entity_id.draft_id.is_some(), include_permissions: false, }; - let response = self - .read_entities_impl(¶ms, ¶ms.temporal_axes.resolve(), &policy_components) - .await?; + let response = self.query_entities(actor_id, params).await?; Ok(response.entities.into_iter().next()) } diff --git a/libs/@local/graph/store/rust/src/entity/store.rs b/libs/@local/graph/store/rust/src/entity/store.rs index adae83ecf80..a8159ce323a 100644 --- a/libs/@local/graph/store/rust/src/entity/store.rs +++ b/libs/@local/graph/store/rust/src/entity/store.rs @@ -904,15 +904,20 @@ pub trait EntityStore { /// Get the latest edition of the [`Entity`], or the one live at the given timestamps. /// /// An `actor_id` of [`None`] reads as the public actor. Reads a draft only when `entity_id` - /// names one, and masks protected properties as [`query_entities`](Self::query_entities) - /// does. Returns [`None`] if the actor may not view the entity or no such edition is live - /// at `transaction_time` and `decision_time`, each of which defaults to now. + /// names one. Returns [`None`] if the actor may not view the entity or no such edition is + /// live at `transaction_time` and `decision_time`, each of which defaults to now. + /// + /// Reads through [`query_entities`], so it masks protected properties the same way and runs + /// against a single consistent snapshot of the store. This requires mutable access to the + /// store. /// /// # Errors /// /// - [`QueryError`] if the store cannot be queried + /// + /// [`query_entities`]: Self::query_entities fn get_entity_by_id( - &self, + &mut self, actor_id: Option, entity_id: EntityId, transaction_time: Option>, @@ -978,12 +983,12 @@ pub trait EntityStore { ) -> impl Future>> + Send; fn diff_entity( - &self, + &mut self, actor_id: ActorId, params: DiffEntityParams, ) -> impl Future, Report>> + Send where - Self: Sync, + Self: Send, { async move { let first_entity = self diff --git a/libs/@local/graph/type-fetcher/src/store.rs b/libs/@local/graph/type-fetcher/src/store.rs index 22f26e86d23..d6c82ff7093 100644 --- a/libs/@local/graph/type-fetcher/src/store.rs +++ b/libs/@local/graph/type-fetcher/src/store.rs @@ -1673,7 +1673,7 @@ where } async fn get_entity_by_id( - &self, + &mut self, actor_id: Option, entity_id: EntityId, transaction_time: Option>, diff --git a/tests/graph/integration/postgres/drafts.rs b/tests/graph/integration/postgres/drafts.rs index 0c5183c4256..1969e59e894 100644 --- a/tests/graph/integration/postgres/drafts.rs +++ b/tests/graph/integration/postgres/drafts.rs @@ -75,7 +75,7 @@ fn charles() -> PropertyObject { } #[must_use] -async fn check_entity_exists(api: &DatabaseApi<'_>, id: EntityId) -> bool { +async fn check_entity_exists(api: &mut DatabaseApi<'_>, id: EntityId) -> bool { api.get_entity_by_id(Some(api.account_id), id, None, None) .await .expect("should query the entity") @@ -113,7 +113,7 @@ async fn initial_draft() { .await .expect("could not create entity"); assert!(entity.metadata.record_id.entity_id.draft_id.is_some()); - assert!(check_entity_exists(&api, entity.metadata.record_id.entity_id).await); + assert!(check_entity_exists(&mut api, entity.metadata.record_id.entity_id).await); assert!(entity.metadata.record_id.entity_id.draft_id.is_some()); assert!( entity @@ -159,7 +159,7 @@ async fn initial_draft() { updated_entity.metadata.record_id.entity_id, entity.metadata.record_id.entity_id ); - assert!(check_entity_exists(&api, updated_entity.metadata.record_id.entity_id).await); + assert!(check_entity_exists(&mut api, updated_entity.metadata.record_id.entity_id).await); assert!( updated_entity .metadata @@ -225,8 +225,8 @@ async fn initial_draft() { .is_none() ); - assert!(!check_entity_exists(&api, updated_entity.metadata.record_id.entity_id).await); - assert!(check_entity_exists(&api, updated_live_entity.metadata.record_id.entity_id).await); + assert!(!check_entity_exists(&mut api, updated_entity.metadata.record_id.entity_id).await); + assert!(check_entity_exists(&mut api, updated_live_entity.metadata.record_id.entity_id).await); assert!( updated_live_entity .metadata @@ -293,7 +293,7 @@ async fn no_initial_draft() { .await .expect("could not create entity"); assert!(entity.metadata.record_id.entity_id.draft_id.is_none()); - assert!(check_entity_exists(&api, entity.metadata.record_id.entity_id).await); + assert!(check_entity_exists(&mut api, entity.metadata.record_id.entity_id).await); let ClosedTemporalBound::Inclusive(undraft_transaction_time) = entity.metadata.temporal_versioning.transaction_time.start(); @@ -356,8 +356,8 @@ async fn no_initial_draft() { .draft_id .is_some() ); - assert!(check_entity_exists(&api, entity.metadata.record_id.entity_id).await); - assert!(check_entity_exists(&api, updated_entity.metadata.record_id.entity_id).await); + assert!(check_entity_exists(&mut api, entity.metadata.record_id.entity_id).await); + assert!(check_entity_exists(&mut api, updated_entity.metadata.record_id.entity_id).await); assert!( (updated_entity .metadata @@ -419,8 +419,10 @@ async fn no_initial_draft() { .draft_id .is_none() ); - assert!(!check_entity_exists(&api, updated_entity.metadata.record_id.entity_id).await); - assert!(check_entity_exists(&api, updated_live_entity.metadata.record_id.entity_id).await); + assert!(!check_entity_exists(&mut api, updated_entity.metadata.record_id.entity_id).await); + assert!( + check_entity_exists(&mut api, updated_live_entity.metadata.record_id.entity_id).await + ); assert!( (updated_live_entity .metadata @@ -475,7 +477,7 @@ async fn multiple_drafts() { .await .expect("could not create entity"); assert!(entity.metadata.record_id.entity_id.draft_id.is_none()); - assert!(check_entity_exists(&api, entity.metadata.record_id.entity_id).await); + assert!(check_entity_exists(&mut api, entity.metadata.record_id.entity_id).await); let ClosedTemporalBound::Inclusive(undraft_transaction_time) = entity.metadata.temporal_versioning.transaction_time.start(); let ClosedTemporalBound::Inclusive(undraft_decision_time) = @@ -544,8 +546,8 @@ async fn multiple_drafts() { .draft_id .is_some() ); - assert!(check_entity_exists(&api, entity.metadata.record_id.entity_id).await); - assert!(check_entity_exists(&api, updated_entity.metadata.record_id.entity_id).await); + assert!(check_entity_exists(&mut api, entity.metadata.record_id.entity_id).await); + assert!(check_entity_exists(&mut api, updated_entity.metadata.record_id.entity_id).await); assert!( (updated_entity .metadata @@ -602,8 +604,10 @@ async fn multiple_drafts() { entity.metadata.record_id.entity_id, updated_live_entity.metadata.record_id.entity_id ); - assert!(!check_entity_exists(&api, draft).await); - assert!(check_entity_exists(&api, updated_live_entity.metadata.record_id.entity_id).await); + assert!(!check_entity_exists(&mut api, draft).await); + assert!( + check_entity_exists(&mut api, updated_live_entity.metadata.record_id.entity_id).await + ); assert!( (updated_live_entity diff --git a/tests/graph/integration/postgres/lib.rs b/tests/graph/integration/postgres/lib.rs index ec11c7c24bc..20a8cbe3201 100644 --- a/tests/graph/integration/postgres/lib.rs +++ b/tests/graph/integration/postgres/lib.rs @@ -862,7 +862,7 @@ impl EntityStore for DatabaseApi<'_> { } async fn get_entity_by_id( - &self, + &mut self, actor_id: Option, entity_id: EntityId, transaction_time: Option>, From a5a0bfd3515507a9a7ec10b9fef9b49eff2a097d Mon Sep 17 00:00:00 2001 From: Tim Diekmann Date: Sun, 18 Oct 2026 17:47:39 +0200 Subject: [PATCH 10/13] Address the second review of the public entities API --- .../src/store/postgres/knowledge/entity/mod.rs | 6 ++++-- .../src/tests/graph/authentication/api-token.test.ts | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/libs/@local/graph/postgres-store/src/store/postgres/knowledge/entity/mod.rs b/libs/@local/graph/postgres-store/src/store/postgres/knowledge/entity/mod.rs index 1a500e1ed71..95c7a4e5b0c 100644 --- a/libs/@local/graph/postgres-store/src/store/postgres/knowledge/entity/mod.rs +++ b/libs/@local/graph/postgres-store/src/store/postgres/knowledge/entity/mod.rs @@ -1995,8 +1995,10 @@ where cursor: None, }, // At most one edition matches: both temporal axes resolve to a single point in time, - // and `include_drafts` is set only when `entity_id` names a draft, which the filter - // then pins. Including drafts for an ID without one would match its drafts as well. + // at which the exclusion constraints on `entity_temporal_metadata` admit one live + // edition and one edition per draft. `include_drafts` is set only when `entity_id` + // names a draft, which the filter then pins. Including drafts for an ID without one + // would match its drafts as well. limit: 1, conversions: Vec::new(), include_entity_types: None, diff --git a/tests/hash-backend-integration/src/tests/graph/authentication/api-token.test.ts b/tests/hash-backend-integration/src/tests/graph/authentication/api-token.test.ts index 20b8cb55680..1f3a42f019b 100644 --- a/tests/hash-backend-integration/src/tests/graph/authentication/api-token.test.ts +++ b/tests/hash-backend-integration/src/tests/graph/authentication/api-token.test.ts @@ -149,7 +149,7 @@ describe("API tokens", () => { expect(ownerResponse.status).toBe(200); expect(await ownerResponse.json()).toMatchObject({ - metadata: { entityId }, + metadata: { entityId, entityTypeIds: [ownerEntityType.schema.$id] }, }); expect(otherResponse.status).toBe(404); }); From 5c1c6a7770fb8eec3b49cc809199282e69e5911b Mon Sep 17 00:00:00 2001 From: Tim Diekmann Date: Mon, 19 Oct 2026 12:49:59 +0200 Subject: [PATCH 11/13] Infer the authentication provider type in `attach` --- libs/@local/graph/api/src/rest/middleware/mod.rs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/libs/@local/graph/api/src/rest/middleware/mod.rs b/libs/@local/graph/api/src/rest/middleware/mod.rs index 6e9046a15a2..38fa4be46c3 100644 --- a/libs/@local/graph/api/src/rest/middleware/mod.rs +++ b/libs/@local/graph/api/src/rest/middleware/mod.rs @@ -155,10 +155,10 @@ where } = api; match audience { Audience::Public => { - self.attach::<_, ActorId>(router, &self.public_provider, None, challenge) + self.attach::(router, &self.public_provider, None, challenge) } Audience::Internal => { - self.attach::<_, Option>(router, &self.internal_provider, None, challenge) + self.attach::>(router, &self.internal_provider, None, challenge) } } } @@ -183,7 +183,7 @@ where apis: impl IntoIterator, documentation: Router, ) -> Router { - let mut router = self.attach::<_, Option>( + let mut router = self.attach::>( legacy_routes, &self.legacy_provider, Some(Bootstrap { @@ -216,15 +216,14 @@ where /// /// The bootstrap secret also exempts its presenters from the caller budget, which a bootstrap /// request naming no actor can pass only by that exemption. - fn attach( + fn attach( &self, routes: Router, - provider: &Arc, + provider: &Arc + 'static>, bootstrap: Option, challenge: Challenge, ) -> Router where - A: AuthenticationProvider + 'static, C: Caller + 'static, { // `route_layer` panics on a router without routes. From 5db89208b34935de503c688a43915bc7da93b041 Mon Sep 17 00:00:00 2001 From: Tim Diekmann Date: Mon, 19 Oct 2026 18:29:05 +0200 Subject: [PATCH 12/13] Rename `DraftEntityId` to `DraftEntityIdError` --- libs/@local/graph/api/src/rest/public/entities/v1/entity.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/libs/@local/graph/api/src/rest/public/entities/v1/entity.rs b/libs/@local/graph/api/src/rest/public/entities/v1/entity.rs index 6098522da9d..280f1f0872b 100644 --- a/libs/@local/graph/api/src/rest/public/entities/v1/entity.rs +++ b/libs/@local/graph/api/src/rest/public/entities/v1/entity.rs @@ -47,14 +47,14 @@ impl LiveEntityId { /// The entity ID names a draft. #[derive(Debug, derive_more::Display)] #[display("the entity ID names a draft")] -struct DraftEntityId; +struct DraftEntityIdError; impl TryFrom for LiveEntityId { - type Error = DraftEntityId; + type Error = DraftEntityIdError; fn try_from(entity_id: EntityId) -> Result { if entity_id.draft_id.is_some() { - return Err(DraftEntityId); + return Err(DraftEntityIdError); } Ok(Self(entity_id)) } From 742217e4b2f730d343082dd1c8f9e483b2749e69 Mon Sep 17 00:00:00 2001 From: Tim Diekmann Date: Mon, 19 Oct 2026 19:39:06 +0200 Subject: [PATCH 13/13] Remove the `Read` import that `patch_entities` no longer needs here --- .../postgres-store/src/store/postgres/knowledge/entity/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/@local/graph/postgres-store/src/store/postgres/knowledge/entity/mod.rs b/libs/@local/graph/postgres-store/src/store/postgres/knowledge/entity/mod.rs index dd9fb9beafa..231abc0610d 100644 --- a/libs/@local/graph/postgres-store/src/store/postgres/knowledge/entity/mod.rs +++ b/libs/@local/graph/postgres-store/src/store/postgres/knowledge/entity/mod.rs @@ -38,7 +38,7 @@ use hash_graph_store::{ CheckPermissionError, ClusterError, DeletionError, InsertionError, QueryError, UpdateError, }, filter::{Filter, FilterExpression, FilterExpressionList, ParameterList}, - query::{QueryResult as _, Read}, + query::QueryResult as _, subgraph::{ Subgraph, SubgraphRecord as _, edges::{