Skip to content

Commit 1e38a56

Browse files
rakeshkkyhasura-bot
rakeshkky
authored and
hasura-bot
committed
jsonapi: Check relationship's output type permission while building catalog; fixes OpenAPI schema generation (#1434)
<!-- The PR description should answer 2 important questions: --> ### 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 <!-- How is it trying to accomplish it (what are the implementation steps)? --> Include the relationship in the map after checking the role's accessibility to the type. V3_GIT_ORIGIN_REV_ID: 5c7b95d7118a665783d1b765198648d876ad684a
1 parent 4df58b5 commit 1e38a56

5 files changed

+433
-13
lines changed

‎v3/changelog.md

+4
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,10 @@ customizations:
4444

4545
### Fixed
4646

47+
- Fixed the `include` query parameter and `included` response field in JSON:API
48+
OpenAPI schema generation. These now honor type permissions for the role in
49+
relationship fields.
50+
4751
- Fixed a bug where commands with array return types would not build when header
4852
forwarding was in effect.
4953

‎v3/crates/jsonapi/src/catalog/object_types.rs

+55-13
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@ use crate::types::ObjectTypeWarning;
33
use hasura_authn_core::Role;
44
use indexmap::IndexMap;
55
use metadata_resolve::{
6-
ObjectTypeWithRelationships, Qualified, QualifiedBaseType, QualifiedTypeName,
7-
QualifiedTypeReference, ScalarTypeRepresentation,
6+
unwrap_custom_type_name, ObjectTypeWithRelationships, Qualified, QualifiedBaseType,
7+
QualifiedTypeName, QualifiedTypeReference, ScalarTypeRepresentation,
88
};
99
use open_dds::types::{CustomTypeName, InbuiltType};
1010
use std::collections::BTreeMap;
@@ -46,20 +46,49 @@ pub fn build_object_type(
4646
// Relationships
4747
let mut type_relationships = IndexMap::new();
4848
for (_, relationship_field) in &object_type.relationship_fields {
49-
let target = match &relationship_field.target {
50-
metadata_resolve::RelationshipTarget::Model(model) => RelationshipTarget::Model {
51-
object_type: model.target_typename.clone(),
52-
relationship_type: model.relationship_type.clone(),
53-
},
49+
// Only track the relationship if its output type is accessible to the role.
50+
let mut target = None;
51+
match &relationship_field.target {
52+
metadata_resolve::RelationshipTarget::Model(model) => {
53+
if object_type_permission_access(role, &model.target_typename, object_types) {
54+
target = Some(RelationshipTarget::Model {
55+
object_type: model.target_typename.clone(),
56+
relationship_type: model.relationship_type.clone(),
57+
});
58+
}
59+
}
5460
metadata_resolve::RelationshipTarget::ModelAggregate(model_aggregate) => {
55-
let target_type = model_aggregate.target_typename.clone();
56-
RelationshipTarget::ModelAggregate(target_type)
61+
if object_type_permission_access(
62+
role,
63+
&model_aggregate.target_typename,
64+
object_types,
65+
) {
66+
target = Some(RelationshipTarget::ModelAggregate(
67+
model_aggregate.target_typename.clone(),
68+
));
69+
}
70+
}
71+
metadata_resolve::RelationshipTarget::Command(command) => {
72+
let track_this_relationship = if let Some(target_object_type) =
73+
unwrap_custom_type_name(&command.target_type)
74+
{
75+
// For command relationship of a custom type, check if type exists.
76+
object_type_permission_access(role, target_object_type, object_types)
77+
} else {
78+
// The output type of this command relationship is not a custom type; it is a built-in type (scalar).
79+
// Track it.
80+
true
81+
};
82+
if track_this_relationship {
83+
target = Some(RelationshipTarget::Command {
84+
type_reference: command.target_type.clone(),
85+
});
86+
}
5787
}
58-
metadata_resolve::RelationshipTarget::Command(command) => RelationshipTarget::Command {
59-
type_reference: command.target_type.clone(),
60-
},
6188
};
62-
type_relationships.insert(relationship_field.relationship_name.clone(), target);
89+
if let Some(target) = target {
90+
type_relationships.insert(relationship_field.relationship_name.clone(), target);
91+
}
6392
}
6493

6594
Ok(ObjectType {
@@ -68,6 +97,19 @@ pub fn build_object_type(
6897
})
6998
}
7099

100+
// Check if object_type is accessible to given role
101+
fn object_type_permission_access(
102+
role: &Role,
103+
type_name: &Qualified<CustomTypeName>,
104+
object_types: &BTreeMap<Qualified<CustomTypeName>, ObjectTypeWithRelationships>,
105+
) -> bool {
106+
let mut accessible = false;
107+
if let Some(object_type) = object_types.get(type_name) {
108+
accessible = object_type.type_output_permissions.contains_key(role);
109+
}
110+
accessible
111+
}
112+
71113
// turn an OpenDD type into a type representation
72114
fn type_from_type_representation(
73115
qualified_type_reference: &QualifiedTypeReference,

‎v3/crates/jsonapi/tests/snapshots/jsonapi_golden_tests__generated_openapi_for_role_user_1.snap

+230
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,138 @@ expression: generated_openapi
176176
}
177177
}
178178
}
179+
},
180+
"included": {
181+
"type": "array",
182+
"items": {
183+
"type": "object",
184+
"anyOf": [
185+
{
186+
"type": "object",
187+
"required": [
188+
"id",
189+
"_type",
190+
"attributes"
191+
],
192+
"properties": {
193+
"_type": {
194+
"enum": [
195+
"default_Artist"
196+
]
197+
},
198+
"attributes": {
199+
"type": "object",
200+
"properties": {
201+
"Name": {
202+
"type": "object"
203+
}
204+
}
205+
},
206+
"id": {
207+
"type": "string"
208+
},
209+
"relationships": {
210+
"type": "object",
211+
"properties": {
212+
"Albums": {
213+
"type": "object",
214+
"required": [
215+
"data"
216+
],
217+
"properties": {
218+
"data": {
219+
"type": "array",
220+
"items": {
221+
"type": "object",
222+
"required": [
223+
"id",
224+
"_type"
225+
],
226+
"properties": {
227+
"_type": {
228+
"enum": [
229+
"default_Album"
230+
]
231+
},
232+
"id": {
233+
"type": "string"
234+
}
235+
}
236+
}
237+
}
238+
}
239+
}
240+
}
241+
}
242+
}
243+
},
244+
{
245+
"type": "object",
246+
"required": [
247+
"id",
248+
"_type",
249+
"attributes"
250+
],
251+
"properties": {
252+
"_type": {
253+
"enum": [
254+
"default_Track"
255+
]
256+
},
257+
"attributes": {
258+
"type": "object",
259+
"properties": {
260+
"AlbumId": {
261+
"type": "object"
262+
},
263+
"Milliseconds": {
264+
"type": "object"
265+
},
266+
"Name": {
267+
"type": "object"
268+
},
269+
"TrackId": {
270+
"type": "object"
271+
}
272+
}
273+
},
274+
"id": {
275+
"type": "string"
276+
},
277+
"relationships": {
278+
"type": "object",
279+
"properties": {
280+
"Album": {
281+
"type": "object",
282+
"required": [
283+
"data"
284+
],
285+
"properties": {
286+
"data": {
287+
"type": "object",
288+
"required": [
289+
"id",
290+
"_type"
291+
],
292+
"properties": {
293+
"_type": {
294+
"enum": [
295+
"default_Album"
296+
]
297+
},
298+
"id": {
299+
"type": "string"
300+
}
301+
}
302+
}
303+
}
304+
}
305+
}
306+
}
307+
}
308+
}
309+
]
310+
}
179311
}
180312
}
181313
}
@@ -353,6 +485,68 @@ expression: generated_openapi
353485
"items": {
354486
"type": "object",
355487
"anyOf": [
488+
{
489+
"type": "object",
490+
"required": [
491+
"id",
492+
"_type",
493+
"attributes"
494+
],
495+
"properties": {
496+
"_type": {
497+
"enum": [
498+
"default_Author"
499+
]
500+
},
501+
"attributes": {
502+
"type": "object",
503+
"properties": {
504+
"author_id": {
505+
"type": "object"
506+
},
507+
"last_name": {
508+
"type": "string"
509+
}
510+
}
511+
},
512+
"id": {
513+
"type": "string"
514+
},
515+
"relationships": {
516+
"type": "object",
517+
"properties": {
518+
"articles": {
519+
"type": "object",
520+
"required": [
521+
"data"
522+
],
523+
"properties": {
524+
"data": {
525+
"type": "array",
526+
"items": {
527+
"type": "object",
528+
"required": [
529+
"id",
530+
"_type"
531+
],
532+
"properties": {
533+
"_type": {
534+
"enum": [
535+
"default_Article"
536+
]
537+
},
538+
"id": {
539+
"type": "string"
540+
}
541+
}
542+
}
543+
}
544+
}
545+
}
546+
}
547+
}
548+
}
549+
},
356550
{
357551
"type": "object",
358552
"required": [
@@ -421,6 +615,42 @@ expression: generated_openapi
421615
}
422616
}
423617
},
618+
"default_Artist": {
619+
"type": "object",
620+
"properties": {
621+
"Name": {
622+
"type": "object"
623+
}
624+
}
625+
},
626+
"default_Author": {
627+
"type": "object",
628+
"properties": {
629+
"author_id": {
630+
"type": "object"
631+
},
632+
"last_name": {
633+
"type": "string"
634+
}
635+
}
636+
},
637+
"default_Track": {
638+
"type": "object",
639+
"properties": {
640+
"AlbumId": {
641+
"type": "object"
642+
},
643+
"Milliseconds": {
644+
"type": "object"
645+
},
646+
"Name": {
647+
"type": "object"
648+
},
649+
"TrackId": {
650+
"type": "object"
651+
}
652+
}
653+
},
424654
"default_commandArticle": {
425655
"type": "object",
426656
"properties": {

0 commit comments

Comments
 (0)