diff --git a/Cargo.lock b/Cargo.lock index 1e53e14dcec..4df7bcc8759 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6293,6 +6293,7 @@ dependencies = [ "md-5 0.11.0", "mime", "opentelemetry", + "percent-encoding", "problematic", "reqwest 0.13.4", "rstest", @@ -6304,6 +6305,7 @@ dependencies = [ "simple-mermaid", "time", "tokio", + "tokio-postgres", "tokio-util", "tower", "tower-http", diff --git a/Cargo.toml b/Cargo.toml index c38982ad7d0..7ad6f6cd81d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -256,6 +256,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 538ab5e7c4b..6aa0eeb32e1 100644 --- a/libs/@blockprotocol/type-system/rust/src/knowledge/entity/id.rs +++ b/libs/@blockprotocol/type-system/rust/src/knowledge/entity/id.rs @@ -77,6 +77,7 @@ 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)] pub struct EntityId { pub web_id: WebId, 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..856b9e7d3af 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 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/Cargo.toml b/libs/@local/graph/api/Cargo.toml index 84d137118f7..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"] } @@ -92,6 +93,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..28efe8fa585 100644 --- a/libs/@local/graph/api/src/rest/caller.rs +++ b/libs/@local/graph/api/src/rest/caller.rs @@ -1,14 +1,12 @@ -use aide::{ - axum::{ApiRouter, routing::get_with}, - transform::TransformOperation, -}; +use aide::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::{self, Route, Routes}, }; /// The resolved caller. @@ -51,18 +49,32 @@ fn document_authenticated_caller(operation: TransformOperation<'_>) -> Transform .summary("Get the authenticated caller") } -pub(super) fn routes() -> ApiRouter { - ApiRouter::new() - .api_route( +fn authenticated_caller_route() -> Route { + routing::get( + authenticated_caller::>, + document_authenticated_caller, + ) +} + +pub(super) fn routes() -> Routes { + Routes::new() + .route( "/caller", - get_with(caller::>, document_caller), + routing::get(caller::>, document_caller), ) - .api_route( + .route( + "/authenticated-caller", + authenticated_caller_route::(), + ) + .tag("Caller") +} + +pub(super) fn authenticated_routes() -> Routes +{ + Routes::new() + .route( "/authenticated-caller", - get_with( - authenticated_caller::>, - document_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 a9148963b51..46743656a9e 100644 --- a/libs/@local/graph/api/src/rest/credentials.rs +++ b/libs/@local/graph/api/src/rest/credentials.rs @@ -77,6 +77,12 @@ 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`], which documents anonymous access that +/// [`Audience::Public`] refuses before a handler runs. +pub(super) trait AdmitsAnonymous: Credentials {} + pub(super) struct MaybeActor(Option, PhantomData C>); impl From> for Option { @@ -85,7 +91,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,8 +101,15 @@ impl FromRequestParts for MaybeActor { } } -impl OperationInput 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 e4ba0fd4c3a..96dae543538 100644 --- a/libs/@local/graph/api/src/rest/extract/mod.rs +++ b/libs/@local/graph/api/src/rest/extract/mod.rs @@ -18,13 +18,18 @@ 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") +)] +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)] @@ -32,7 +37,6 @@ pub(in crate::rest) enum MessagePart { PathParameters, QueryParameters, RequestBody, - ResponseBody, } impl MessagePart { @@ -44,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 b45a460abbe..4f1fcd0659c 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") +)] 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..f0556cce19c 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,11 @@ 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::{self, Routes}, + test_utils::NoCredentials, +}; #[derive(Deserialize, JsonSchema)] struct Subject { @@ -500,7 +504,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(), @@ -508,9 +512,9 @@ fn echo_document() { ..Info::default() }, || { - ApiRouter::new().api_route( + Routes::new().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/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/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/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/middleware/mod.rs b/libs/@local/graph/api/src/rest/middleware/mod.rs index d99336a2502..38fa4be46c3 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,11 @@ 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`, as its [`Audience`] requires. fn attach_api(&self, api: Api) -> Router { let Api { audience, @@ -153,8 +154,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::(router, &self.public_provider, None, challenge) + } + Audience::Internal => { + self.attach::>(router, &self.internal_provider, None, challenge) + } } } @@ -178,7 +183,7 @@ where apis: impl IntoIterator, documentation: Router, ) -> Router { - let mut router = self.attach( + let mut router = self.attach::>( legacy_routes, &self.legacy_provider, Some(Bootstrap { @@ -211,15 +216,15 @@ 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. if !routes.has_routes() { @@ -236,7 +241,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 7df57ab2c37..290482ed70c 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}; @@ -214,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, ) @@ -245,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")), @@ -317,7 +326,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, ) @@ -462,7 +471,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, ) @@ -520,6 +529,120 @@ async fn panic_problem_document() { ); } +/// 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( + &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" + ); +} + +/// An anonymous request to the entity route is answered `401` before the store is read. +#[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" + ); +} + +/// 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() { + 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); @@ -642,7 +765,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| { @@ -664,10 +787,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(), ); @@ -675,7 +799,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 9d15784e35e..f2fe49664e0 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,6 +11,8 @@ pub mod probe; mod public; pub mod rate_limit; mod router; +mod routing; +mod state; pub mod telemetry; #[cfg(test)] @@ -26,27 +20,34 @@ pub(crate) mod test_utils; use aide::openapi::OpenApi; use axum::Router; +use hash_graph_store::pool::StorePool; use hash_middleware::authentication::Challenge; 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 one is answered `401`. Public, + /// Callers presenting a Kratos session. A request without one is served anonymously, one + /// presenting an API token is answered `401`. Internal, } /// 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 +62,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..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,8 @@ #[cfg(test)] mod tests; -use alloc::collections::BTreeSet; +use alloc::{borrow::Cow, collections::BTreeSet}; +use core::mem; use std::collections::HashSet; use aide::{ @@ -17,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}; +use super::{Api, credentials::Credentials, extract::MessagePart, middleware, routing::Routes}; /// Builds an API from its routes and generates the document describing them. /// @@ -36,21 +38,22 @@ use super::{Api, credentials::Credentials, extract::MessagePart, middleware}; /// - 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. +/// - 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 -pub(super) fn build( +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 { +) -> Api { #[expect( clippy::wildcard_enum_match_arm, reason = "Every documentation error apart from the inferred-response conflicts is a \ @@ -67,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); @@ -80,6 +83,8 @@ pub(super) fn build( assert_security_schemes_declared(&mut document); assert_parameters_readable(&mut document); assert_read_through_extractors(&mut document); + assert_operation_ids_unique(&document); + remove_unreferenced_schemas(&mut document); Api { audience: C::AUDIENCE, challenge: C::CHALLENGE, @@ -212,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 @@ -244,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)) @@ -279,21 +283,108 @@ 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 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 &paths.paths { + let Some(item) = item.as_item() else { + continue; + }; + for (method, operation) in item.iter() { + 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 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 + .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(schema_name) + { + references.push(name.into_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 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 { @@ -322,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 5fa3986a28a..0bcc9a61db3 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,9 @@ use problematic::{Problem, ProblemDetails, ProblemType, ProblemVariant, Rejectio use super::reference_responses; use crate::rest::{ Api, - extract::{Path, Query}, + extract::{Json, Path, Query}, 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>() } @@ -129,16 +135,16 @@ 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(), || { - 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>>() }, @@ -159,13 +165,16 @@ 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(), || { - 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, @@ -175,15 +184,18 @@ 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(), || { - 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, @@ -228,13 +240,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( + Routes::new().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 +331,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( + Routes::new().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,16 +388,17 @@ 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(), || { - 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, ), ) }, @@ -390,16 +409,17 @@ 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(), || { - 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, ), ) }, @@ -410,29 +430,34 @@ 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)), + || { + Routes::new().route( + "/entities", + routing::post(async |body: bytes::Bytes| Json::<_, 200>(body.len()), named), + ) + }, |document| document, ); } fn build_with_parameter(parameter: Parameter) { - let _: Api = openapi::build::( + let _: Api = openapi::build::( "/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) }, ), ) @@ -478,12 +503,126 @@ 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::( +#[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(), - || ApiRouter::new().api_route("/entities", get(async || axum::Json(0_u8))), + || { + let route = || { + routing::get( + async || Json::<_, 200>(0_u8), + |operation| operation.id("getEntity").summary("Get an entity"), + ) + }; + Routes::new() + .route("/first", route()) + .route("/second", route()) + }, |document| document, ); } + +/// Path parameters whose field has a component schema of its own. +#[derive(serde::Deserialize, schemars::JsonSchema)] +struct EntityPath { + entity: EntityUuid, +} + +/// A response whose field has a component schema of its own. +#[derive(serde::Serialize, schemars::JsonSchema)] +struct EntityResponse { + entity: EntityName, +} + +/// A name that only responses reference. +#[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(), + || { + Routes::new().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" + ); +} + +/// 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/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/entity.rs b/libs/@local/graph/api/src/rest/public/entities/v1/entity.rs new file mode 100644 index 00000000000..280f1f0872b --- /dev/null +++ b/libs/@local/graph/api/src/rest/public/entities/v1/entity.rs @@ -0,0 +1,220 @@ +//! `GET /{entityId}`: one entity by its ID. + +use alloc::{borrow::Cow, collections::BTreeSet}; +use core::error::Error; + +use aide::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, Schema, SchemaGenerator, json_schema}; +use serde::{Deserialize, Serialize}; +use type_system::{ + knowledge::entity::{EntityId, id::EntityUuid}, + ontology::VersionedUrl, + principal::actor_group::WebId, +}; +use uuid::Uuid; + +use crate::rest::{ + ApiState, + credentials::Actor, + extract::{Json, Path}, + public::Credentials, + 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 DraftEntityIdError; + +impl TryFrom for LiveEntityId { + type Error = DraftEntityIdError; + + fn try_from(entity_id: EntityId) -> Result { + if entity_id.draft_id.is_some() { + return Err(DraftEntityIdError); + } + 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`. + entity_id: LiveEntityId, +} + +/// An entity the caller may view. +#[derive(Serialize, JsonSchema)] +struct Entity { + /// What identifies and classifies the entity. + metadata: EntityMetadata, +} + +/// What identifies and classifies an entity. +#[derive(Serialize, JsonSchema)] +#[serde(rename_all = "camelCase")] +struct EntityMetadata { + /// The ID of the entity. + entity_id: LiveEntityId, + /// The entity types the entity is an instance of, without the types they inherit from. + entity_type_ids: BTreeSet, +} + +/// 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 { + #[serde(skip)] + entity_id: LiveEntityId, +} + +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: LiveEntityId::new( + WebId::new(Uuid::from_u128(0x8D86C8C3_D66D_43D1_859B_676C3BCAEADC)), + EntityUuid::new(Uuid::from_u128(0xBE2C5B2F_E3E1_4F26_A0C4_0F8B5E9C4A1D)), + ), + }) + } +} + +/// The ways [`get_entity`] refuses a request its extractors accept. +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(LiveEntityId), + #[display("entity `{_0}` is archived")] + Archived(LiveEntityId), + #[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) | GetEntityError::Archived(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 mut store = state + .acquire() + .await + .change_context(GetEntityError::Unavailable)?; + let entity = store + .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)))?; + if entity.metadata.archived { + return Err(Report::new(GetEntityError::Archived(entity_id)).into()); + } + + Ok(Json(Entity { + metadata: EntityMetadata { + entity_id, + entity_type_ids: entity.metadata.entity_type_ids.into_iter().collect(), + }, + })) +} + +fn document_get_entity(operation: TransformOperation<'_>) -> TransformOperation<'_> { + operation + .id("getEntity") + .summary("Get an entity") + .description("Returns the entity's ID and the entity types of its current edition.") +} + +pub(super) fn routes() -> Routes> +where + S: StorePool + Send + Sync + 'static, +{ + Routes::new() + .route( + "/{entityId}", + routing::get(get_entity::, document_get_entity), + ) + .tag("Entity") +} diff --git a/libs/@local/graph/api/src/rest/public/entities/v1.rs b/libs/@local/graph/api/src/rest/public/entities/v1/mod.rs similarity index 50% rename from libs/@local/graph/api/src/rest/public/entities/v1.rs rename to libs/@local/graph/api/src/rest/public/entities/v1/mod.rs index 70738544670..6ffc2a4104f 100644 --- a/libs/@local/graph/api/src/rest/public/entities/v1.rs +++ b/libs/@local/graph/api/src/rest/public/entities/v1/mod.rs @@ -1,9 +1,15 @@ +mod entity; + use aide::openapi::Info; +use hash_graph_store::pool::StorePool; -use crate::rest::{Api, caller, documentation, openapi, public}; +use crate::rest::{Api, ApiState, documentation, openapi, public}; -pub(super) fn api() -> Api { - openapi::build::( +pub(super) fn api() -> Api> +where + S: StorePool + Send + Sync + 'static, +{ + openapi::build::( "/entities/v1", Info { title: "Entities".to_owned(), @@ -11,7 +17,7 @@ pub(super) fn api() -> Api { description: Some(documentation::OVERVIEW.to_owned()), ..Info::default() }, - caller::routes::, + 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..1e18a7e0e25 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::authenticated_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..015ff63e85b 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,18 +84,18 @@ 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. - 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)) @@ -104,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 new file mode 100644 index 00000000000..8efcf819cda --- /dev/null +++ b/libs/@local/graph/api/src/rest/routing.rs @@ -0,0 +1,144 @@ +//! [`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. [`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::{ + ApiRouter, + routing::{self, ApiMethodRouter}, + }, + operation::OperationHandler, + transform::TransformOperation, +}; +use axum::handler::Handler; +use problematic::Rejection; + +use super::extract::Json; + +/// 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 +{ +} + +/// 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 +/// +/// 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<'_>, +) -> Route +where + H: Handler + OperationHandler, + I: OperationInput, + O: Outcome, + S: Clone + Send + Sync + 'static, + T: 'static, +{ + Route(routing::get_with(handler, |operation| { + identified(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") +)] +pub(in crate::rest) fn post( + handler: H, + transform: impl FnOnce(TransformOperation<'_>) -> TransformOperation<'_>, +) -> Route +where + H: Handler + OperationHandler, + I: OperationInput, + O: Outcome, + S: Clone + Send + Sync + 'static, + T: 'static, +{ + Route(routing::post_with(handler, |operation| { + identified(transform(operation)) + })) +} + +/// 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 + .as_deref() + .expect("the operation should have an `operationId`"); + assert!( + inner.summary.is_some(), + "the operation `{id}` should have a summary" + ); + operation +} + +#[cfg(test)] +mod tests { + use super::{Route, get}; + use crate::rest::extract::Json; + + #[test] + #[should_panic(expected = "the operation should have an `operationId`")] + fn get_missing_id() { + let _: Route<()> = 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 _: 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 69d0e6d4027..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::get}, - openapi::{Info, ReferenceOr, SecurityScheme}, -}; +use aide::openapi::{Info, ReferenceOr, SecurityScheme}; use axum::{Router, body::Body}; use error_stack::Report; use hash_graph_authentication::{ @@ -42,9 +39,10 @@ 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::{self, Route, Routes}, }; const SERVICE_SECRET: &str = "hash-svc-test-secret"; @@ -113,13 +111,33 @@ struct Advertised { api_token: bool, } -/// Assembles `/test/optional` and `/test/required` behind the credentials `C`, with the provider -/// chains the router composes. +/// 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| { + operation + .id("getRequiredCaller") + .summary("Get the authenticated caller") + }, + ) +} + +/// 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") + }) +} + +/// 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() -> Routes, ) -> (Router, Advertised) { let public_provider = Arc::new(ApiTokenProvider::new( UnreadableTokens, @@ -149,19 +167,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", get(echo_caller::>)) - .api_route( - "/required", - get(async |actor: Actor| echo_caller(Some(actor.into())).await), - ) - }, - |document| document, - ); + let api = openapi::build::("/test", Info::default(), build_routes, |document| document); let schemes = api .document() .components @@ -202,19 +208,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"], || { + Routes::new().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"], || { + Routes::new() + .route("/optional", optional_route::()) + .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() -> 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); + 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 { @@ -247,7 +273,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 0896f5edbc0..2d85dff9ca6 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,9 @@ mod authentication; +use alloc::sync::Arc; + use aide::{ OperationInput, - axum::{ApiRouter, routing::get}, openapi::{Info, SecurityScheme}, }; use axum::{ @@ -10,14 +11,26 @@ 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 self::authentication::{assert_anonymous_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::{self, Routes}, +}; pub(super) async fn request(router: &Router, path: &str) -> Response { router @@ -39,8 +52,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 +94,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"))), + || { + Routes::new().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..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 @@ -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,13 +129,13 @@ } } }, - "ServiceUnavailable": { + "NotFound": { "content": { "application/problem+json": { "example": { - "detail": "authentication is temporarily unavailable", - "status": 503, - "title": "Service Unavailable", + "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": { @@ -82,10 +146,10 @@ { "properties": { "status": { - "const": 503 + "const": 404 }, "title": { - "const": "Service Unavailable" + "const": "Not Found" }, "type": { "const": "about:blank" @@ -96,11 +160,117 @@ ] } ], - "title": "Service Unavailable" + "title": "Not Found" + } + } + }, + "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.", + "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": { + "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": "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.", @@ -277,102 +447,53 @@ } }, "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.", + "Entity": { + "description": "An entity the caller may view.", "properties": { - "actor": { + "metadata": { "allOf": [ { - "$ref": "#/components/schemas/ActorId" + "$ref": "#/components/schemas/EntityMetadata" } ], - "description": "The resolved actor." + "description": "What identifies and classifies the entity." } }, "required": [ - "actor" + "metadata" ], "type": "object" }, - "CallerResponse": { - "description": "The resolved caller.", + "EntityId": { + "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": { - "actor": { - "anyOf": [ - { - "$ref": "#/components/schemas/ActorId" - }, + "entityId": { + "allOf": [ { - "type": "null" + "$ref": "#/components/schemas/EntityId" } ], - "description": "The resolved actor, or `null` for an anonymous request." + "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", + "uniqueItems": true } }, "required": [ - "actor" + "entityId", + "entityTypeIds" ], "type": "object" }, @@ -408,6 +529,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 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": { @@ -425,85 +556,33 @@ }, "openapi": "3.1.0", "paths": { - "/entities/v1/authenticated-caller": { + "/entities/v1/{entityId}": { "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": [ + "description": "Returns the entity's ID and the entity types of its current edition.", + "operationId": "getEntity", + "parameters": [ { - "apiToken": [] + "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. An ID that names a draft is answered with `400`." + }, + "style": "simple" } ], - "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" + "$ref": "#/components/schemas/Entity" } } }, - "description": "The resolved caller.", + "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.", @@ -532,9 +611,15 @@ } } }, + "400": { + "$ref": "#/components/responses/BadRequest" + }, "401": { "$ref": "#/components/responses/Unauthorized" }, + "404": { + "$ref": "#/components/responses/NotFound" + }, "429": { "$ref": "#/components/responses/TooManyRequests" }, @@ -546,14 +631,13 @@ } }, "security": [ - {}, { "apiToken": [] } ], - "summary": "Get the caller", + "summary": "Get an entity", "tags": [ - "Caller" + "Entity" ] } } 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/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 03991513101..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::{ @@ -1976,38 +1976,38 @@ where } async fn get_entity_by_id( - &self, + &mut self, actor_id: Option, entity_id: EntityId, transaction_time: Option>, decision_time: Option>, - ) -> Result> { - let policy_components = PolicyComponents::builder(self, actor_id) - .with_action(ActionName::ViewEntity, MergePolicies::Yes) - .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(); - - Read::::read_one( - self, - &filters, - Some(&temporal_axes), - entity_id.draft_id.is_some(), - ) - .await + ) -> Result, Report> { + 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, + }, + // At most one edition matches: both temporal axes resolve to a single point in time, + // 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, + include_drafts: entity_id.draft_id.is_some(), + include_permissions: false, + }; + let response = self.query_entities(actor_id, params).await?; + Ok(response.entities.into_iter().next()) } #[tracing::instrument(level = "info", skip(self, params))] 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 a544a151aa7..a8159ce323a 100644 --- a/libs/@local/graph/store/rust/src/entity/store.rs +++ b/libs/@local/graph/store/rust/src/entity/store.rs @@ -903,14 +903,26 @@ 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. Reads a draft only when `entity_id` + /// 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>, decision_time: Option>, - ) -> impl Future>> + Send; + ) -> impl Future, Report>> + Send; fn patch_entity( &mut self, @@ -971,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 @@ -986,7 +998,13 @@ pub trait EntityStore { params.first_transaction_time, params.first_decision_time, ) - .await?; + .await? + .ok_or_else(|| { + Report::new(QueryError).attach(format!( + "entity `{}` is not visible", + params.first_entity_id + )) + })?; let second_entity = self .get_entity_by_id( Some(actor_id), @@ -994,7 +1012,13 @@ pub trait EntityStore { params.second_transaction_time, params.second_decision_time, ) - .await?; + .await? + .ok_or_else(|| { + Report::new(QueryError).attach(format!( + "entity `{}` is not visible", + 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..d6c82ff7093 100644 --- a/libs/@local/graph/type-fetcher/src/store.rs +++ b/libs/@local/graph/type-fetcher/src/store.rs @@ -1673,12 +1673,12 @@ where } async fn get_entity_by_id( - &self, + &mut self, actor_id: Option, 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/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) + } +} diff --git a/tests/graph/integration/postgres/drafts.rs b/tests/graph/integration/postgres/drafts.rs index 1725cc5f560..1969e59e894 100644 --- a/tests/graph/integration/postgres/drafts.rs +++ b/tests/graph/integration/postgres/drafts.rs @@ -75,10 +75,11 @@ 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 - .is_ok() + .expect("should query the entity") + .is_some() } #[tokio::test] @@ -112,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 @@ -158,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 @@ -224,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 @@ -292,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(); @@ -355,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 @@ -418,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 @@ -474,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) = @@ -543,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 @@ -601,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/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() { diff --git a/tests/graph/integration/postgres/entity.rs b/tests/graph/integration/postgres/entity.rs index f0d3ca56202..a981042173a 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,110 @@ async fn update() { let entity_v2 = response_v2.entities.pop().expect("no entity found"); assert_eq!(entity_v2.properties.properties(), page_v2.properties()); } + +/// 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..20a8cbe3201 100644 --- a/tests/graph/integration/postgres/lib.rs +++ b/tests/graph/integration/postgres/lib.rs @@ -862,12 +862,12 @@ impl EntityStore for DatabaseApi<'_> { } async fn get_entity_by_id( - &self, + &mut self, actor_id: Option, 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" 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..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 @@ -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,25 +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 } = entity; + const [ownerResponse, otherResponse] = await Promise.all([ + requestWithToken(`/entities/v1/${entityId}`, ownerToken), + requestWithToken(`/entities/v1/${entityId}`, otherToken), + ]); + + expect(ownerResponse.status).toBe(200); + expect(await ownerResponse.json()).toMatchObject({ + metadata: { entityId, entityTypeIds: [ownerEntityType.schema.$id] }, + }); + expect(otherResponse.status).toBe(404); + }); + + 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/authenticated-caller", + `/entities/v1/${entity.entityId}`, token, ); - expect(response.status).toBe(200); - expect(await response.json()).toEqual({ - actor: { actorType: "user", id: owner.accountId }, - }); + expect(response.status).toBe(404); }); - it("rejects a revoked token instead of serving the request anonymously", async () => { + it("rejects a revoked token", async () => { const { token, apiToken } = await createApiToken( graphApi, { actorId: owner.accountId }, @@ -128,12 +186,18 @@ 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( "application/problem+json", ); + expect(response.headers.get("www-authenticate")).toContain( + 'error="invalid_token"', + ); }); it("does not accept a token on the legacy API", async () => {