From 1e38a569ce56434e0cd6743e42a15da2f21a98ad Mon Sep 17 00:00:00 2001 From: Rakesh Emmadi <12475069+rakeshkky@users.noreply.github.com> Date: Tue, 10 Dec 2024 06:27:42 +0530 Subject: [PATCH] jsonapi: Check relationship's output type permission while building catalog; fixes OpenAPI schema generation (#1434) ### What While generating the json:api catalog for a given role, include relationships for an object type only if the relationship's output type is accessible to the role. This would enable proper open api schema generation and permission check during json:api validation phase. ### How Include the relationship in the map after checking the role's accessibility to the type. V3_GIT_ORIGIN_REV_ID: 5c7b95d7118a665783d1b765198648d876ad684a --- v3/changelog.md | 4 + v3/crates/jsonapi/src/catalog/object_types.rs | 68 +++++- ...ts__generated_openapi_for_role_user_1.snap | 230 ++++++++++++++++++ ...ts__generated_openapi_for_role_user_2.snap | 109 +++++++++ v3/crates/jsonapi/tests/static/metadata.json | 35 +++ 5 files changed, 433 insertions(+), 13 deletions(-) diff --git a/v3/changelog.md b/v3/changelog.md index 7bb5545802d40..50ce8a57af160 100644 --- a/v3/changelog.md +++ b/v3/changelog.md @@ -44,6 +44,10 @@ customizations: ### Fixed +- Fixed the `include` query parameter and `included` response field in JSON:API + OpenAPI schema generation. These now honor type permissions for the role in + relationship fields. + - Fixed a bug where commands with array return types would not build when header forwarding was in effect. diff --git a/v3/crates/jsonapi/src/catalog/object_types.rs b/v3/crates/jsonapi/src/catalog/object_types.rs index c7526a913619a..89c5c83bcbc20 100644 --- a/v3/crates/jsonapi/src/catalog/object_types.rs +++ b/v3/crates/jsonapi/src/catalog/object_types.rs @@ -3,8 +3,8 @@ use crate::types::ObjectTypeWarning; use hasura_authn_core::Role; use indexmap::IndexMap; use metadata_resolve::{ - ObjectTypeWithRelationships, Qualified, QualifiedBaseType, QualifiedTypeName, - QualifiedTypeReference, ScalarTypeRepresentation, + unwrap_custom_type_name, ObjectTypeWithRelationships, Qualified, QualifiedBaseType, + QualifiedTypeName, QualifiedTypeReference, ScalarTypeRepresentation, }; use open_dds::types::{CustomTypeName, InbuiltType}; use std::collections::BTreeMap; @@ -46,20 +46,49 @@ pub fn build_object_type( // Relationships let mut type_relationships = IndexMap::new(); for (_, relationship_field) in &object_type.relationship_fields { - let target = match &relationship_field.target { - metadata_resolve::RelationshipTarget::Model(model) => RelationshipTarget::Model { - object_type: model.target_typename.clone(), - relationship_type: model.relationship_type.clone(), - }, + // Only track the relationship if its output type is accessible to the role. + let mut target = None; + match &relationship_field.target { + metadata_resolve::RelationshipTarget::Model(model) => { + if object_type_permission_access(role, &model.target_typename, object_types) { + target = Some(RelationshipTarget::Model { + object_type: model.target_typename.clone(), + relationship_type: model.relationship_type.clone(), + }); + } + } metadata_resolve::RelationshipTarget::ModelAggregate(model_aggregate) => { - let target_type = model_aggregate.target_typename.clone(); - RelationshipTarget::ModelAggregate(target_type) + if object_type_permission_access( + role, + &model_aggregate.target_typename, + object_types, + ) { + target = Some(RelationshipTarget::ModelAggregate( + model_aggregate.target_typename.clone(), + )); + } + } + metadata_resolve::RelationshipTarget::Command(command) => { + let track_this_relationship = if let Some(target_object_type) = + unwrap_custom_type_name(&command.target_type) + { + // For command relationship of a custom type, check if type exists. + object_type_permission_access(role, target_object_type, object_types) + } else { + // The output type of this command relationship is not a custom type; it is a built-in type (scalar). + // Track it. + true + }; + if track_this_relationship { + target = Some(RelationshipTarget::Command { + type_reference: command.target_type.clone(), + }); + } } - metadata_resolve::RelationshipTarget::Command(command) => RelationshipTarget::Command { - type_reference: command.target_type.clone(), - }, }; - type_relationships.insert(relationship_field.relationship_name.clone(), target); + if let Some(target) = target { + type_relationships.insert(relationship_field.relationship_name.clone(), target); + } } Ok(ObjectType { @@ -68,6 +97,19 @@ pub fn build_object_type( }) } +// Check if object_type is accessible to given role +fn object_type_permission_access( + role: &Role, + type_name: &Qualified, + object_types: &BTreeMap, ObjectTypeWithRelationships>, +) -> bool { + let mut accessible = false; + if let Some(object_type) = object_types.get(type_name) { + accessible = object_type.type_output_permissions.contains_key(role); + } + accessible +} + // turn an OpenDD type into a type representation fn type_from_type_representation( qualified_type_reference: &QualifiedTypeReference, diff --git a/v3/crates/jsonapi/tests/snapshots/jsonapi_golden_tests__generated_openapi_for_role_user_1.snap b/v3/crates/jsonapi/tests/snapshots/jsonapi_golden_tests__generated_openapi_for_role_user_1.snap index bceacc7e85078..8cb529abd2083 100644 --- a/v3/crates/jsonapi/tests/snapshots/jsonapi_golden_tests__generated_openapi_for_role_user_1.snap +++ b/v3/crates/jsonapi/tests/snapshots/jsonapi_golden_tests__generated_openapi_for_role_user_1.snap @@ -176,6 +176,138 @@ expression: generated_openapi } } } + }, + "included": { + "type": "array", + "items": { + "type": "object", + "anyOf": [ + { + "type": "object", + "required": [ + "id", + "_type", + "attributes" + ], + "properties": { + "_type": { + "enum": [ + "default_Artist" + ] + }, + "attributes": { + "type": "object", + "properties": { + "Name": { + "type": "object" + } + } + }, + "id": { + "type": "string" + }, + "relationships": { + "type": "object", + "properties": { + "Albums": { + "type": "object", + "required": [ + "data" + ], + "properties": { + "data": { + "type": "array", + "items": { + "type": "object", + "required": [ + "id", + "_type" + ], + "properties": { + "_type": { + "enum": [ + "default_Album" + ] + }, + "id": { + "type": "string" + } + } + } + } + } + } + } + } + } + }, + { + "type": "object", + "required": [ + "id", + "_type", + "attributes" + ], + "properties": { + "_type": { + "enum": [ + "default_Track" + ] + }, + "attributes": { + "type": "object", + "properties": { + "AlbumId": { + "type": "object" + }, + "Milliseconds": { + "type": "object" + }, + "Name": { + "type": "object" + }, + "TrackId": { + "type": "object" + } + } + }, + "id": { + "type": "string" + }, + "relationships": { + "type": "object", + "properties": { + "Album": { + "type": "object", + "required": [ + "data" + ], + "properties": { + "data": { + "type": "object", + "required": [ + "id", + "_type" + ], + "properties": { + "_type": { + "enum": [ + "default_Album" + ] + }, + "id": { + "type": "string" + } + } + } + } + } + } + } + } + } + ] + } } } } @@ -353,6 +485,68 @@ expression: generated_openapi "items": { "type": "object", "anyOf": [ + { + "type": "object", + "required": [ + "id", + "_type", + "attributes" + ], + "properties": { + "_type": { + "enum": [ + "default_Author" + ] + }, + "attributes": { + "type": "object", + "properties": { + "author_id": { + "type": "object" + }, + "last_name": { + "type": "string" + } + } + }, + "id": { + "type": "string" + }, + "relationships": { + "type": "object", + "properties": { + "articles": { + "type": "object", + "required": [ + "data" + ], + "properties": { + "data": { + "type": "array", + "items": { + "type": "object", + "required": [ + "id", + "_type" + ], + "properties": { + "_type": { + "enum": [ + "default_Article" + ] + }, + "id": { + "type": "string" + } + } + } + } + } + } + } + } + } + }, { "type": "object", "required": [ @@ -421,6 +615,42 @@ expression: generated_openapi } } }, + "default_Artist": { + "type": "object", + "properties": { + "Name": { + "type": "object" + } + } + }, + "default_Author": { + "type": "object", + "properties": { + "author_id": { + "type": "object" + }, + "last_name": { + "type": "string" + } + } + }, + "default_Track": { + "type": "object", + "properties": { + "AlbumId": { + "type": "object" + }, + "Milliseconds": { + "type": "object" + }, + "Name": { + "type": "object" + }, + "TrackId": { + "type": "object" + } + } + }, "default_commandArticle": { "type": "object", "properties": { diff --git a/v3/crates/jsonapi/tests/snapshots/jsonapi_golden_tests__generated_openapi_for_role_user_2.snap b/v3/crates/jsonapi/tests/snapshots/jsonapi_golden_tests__generated_openapi_for_role_user_2.snap index 4e2d1e44580b5..ce6cbf3b4ce69 100644 --- a/v3/crates/jsonapi/tests/snapshots/jsonapi_golden_tests__generated_openapi_for_role_user_2.snap +++ b/v3/crates/jsonapi/tests/snapshots/jsonapi_golden_tests__generated_openapi_for_role_user_2.snap @@ -173,6 +173,99 @@ expression: generated_openapi } } } + }, + "included": { + "type": "array", + "items": { + "type": "object", + "anyOf": [ + { + "type": "object", + "required": [ + "id", + "_type", + "attributes" + ], + "properties": { + "_type": { + "enum": [ + "default_Author" + ] + }, + "attributes": { + "type": "object", + "properties": { + "author_id": { + "type": "object" + } + } + }, + "id": { + "type": "string" + }, + "relationships": { + "type": "object", + "properties": { + "articles": { + "type": "object", + "required": [ + "data" + ], + "properties": { + "data": { + "type": "array", + "items": { + "type": "object", + "required": [ + "id", + "_type" + ], + "properties": { + "_type": { + "enum": [ + "default_Article" + ] + }, + "id": { + "type": "string" + } + } + } + } + } + } + } + } + } + }, + { + "type": "object", + "required": [ + "id", + "_type", + "attributes" + ], + "properties": { + "_type": { + "enum": [ + "default_commandAuthor" + ] + }, + "attributes": { + "type": "object", + "properties": { + "id": { + "type": "integer" + } + } + }, + "id": { + "type": "string" + } + } + } + ] + } } } } @@ -195,6 +288,22 @@ expression: generated_openapi "type": "string" } } + }, + "default_Author": { + "type": "object", + "properties": { + "author_id": { + "type": "object" + } + } + }, + "default_commandAuthor": { + "type": "object", + "properties": { + "id": { + "type": "integer" + } + } } } } diff --git a/v3/crates/jsonapi/tests/static/metadata.json b/v3/crates/jsonapi/tests/static/metadata.json index cad409b4c34b6..b3a07ab44d1ce 100644 --- a/v3/crates/jsonapi/tests/static/metadata.json +++ b/v3/crates/jsonapi/tests/static/metadata.json @@ -2261,6 +2261,18 @@ "output": { "allowedFields": ["author_id", "first_name"] } + }, + { + "role": "user_1", + "output": { + "allowedFields": ["author_id", "last_name"] + } + }, + { + "role": "user_2", + "output": { + "allowedFields": ["author_id"] + } } ] } @@ -2558,6 +2570,12 @@ "output": { "allowedFields": ["ArtistId", "Name"] } + }, + { + "role": "user_1", + "output": { + "allowedFields": ["Name"] + } } ] } @@ -4245,6 +4263,17 @@ "UnitPrice" ] } + }, + { + "role": "user_1", + "output": { + "allowedFields": [ + "AlbumId", + "Name", + "TrackId", + "Milliseconds" + ] + } } ] } @@ -5053,6 +5082,12 @@ "output": { "allowedFields": ["id", "first_name", "last_name"] } + }, + { + "role": "user_2", + "output": { + "allowedFields": ["id"] + } } ] }