Use Exists expression for relationship predicates in permissions (#612)

A follow-up to https://github.com/hasura/v3-engine/pull/605.

Similar to the above linked PR, apply the `Exists` NDC expression to all
relationship predicates in permission filters. This avoids tracking
relationship paths and their usage in comparison targets.

V3_GIT_ORIGIN_REV_ID: 09784b7ea04f054f2183e593da6e6cc327a5c853
This commit is contained in:
Rakesh Emmadi 2024-05-27 18:17:40 +05:30 committed by hasura-bot
parent e4c5d7a752
commit da27c29ee8
9 changed files with 254 additions and 66 deletions

View File

@ -0,0 +1,23 @@
[
{
"data": {
"Artist": [
{
"Name": "AC/DC"
}
]
}
},
{
"data": {
"Artist": [
{
"Name": "AC/DC"
},
{
"Name": "The Rolling Stones"
}
]
}
}
]

View File

@ -0,0 +1,148 @@
{
"version": "v2",
"subgraphs": [
{
"name": "default",
"objects": [
{
"definition": {
"modelName": "Albums",
"permissions": [
{
"role": "admin",
"select": {
"filter": null
}
},
{
"role": "user",
"select": {
"filter": null
}
}
]
},
"version": "v1",
"kind": "ModelPermissions"
},
{
"definition": {
"modelName": "Tracks",
"permissions": [
{
"role": "admin",
"select": {
"filter": null
}
},
{
"role": "user",
"select": {
"filter": null
}
}
]
},
"version": "v1",
"kind": "ModelPermissions"
},
{
"definition": {
"modelName": "Artists",
"permissions": [
{
"role": "admin",
"select": {
"filter": {
"relationship": {
"name": "Albums",
"predicate": {
"and": [
{
"relationship": {
"name": "Tracks",
"predicate": {
"fieldComparison": {
"field": "Name",
"operator": "_ilike",
"value": {
"literal": "%Rock%"
}
}
}
}
},
{
"fieldComparison": {
"field": "Title",
"operator": "_ilike",
"value": {
"literal": "%Rock%"
}
}
}
]
}
}
}
}
},
{
"role": "user",
"select": {
"filter": {
"and": [
{
"relationship": {
"name": "Albums",
"predicate": {
"and": [
{
"relationship": {
"name": "Tracks",
"predicate": {
"fieldComparison": {
"field": "Name",
"operator": "_ilike",
"value": {
"literal": "%Rock%"
}
}
}
}
}
]
}
}
},
{
"relationship": {
"name": "Albums",
"predicate": {
"and": [
{
"fieldComparison": {
"field": "Title",
"operator": "_ilike",
"value": {
"literal": "%Rock%"
}
}
}
]
}
}
}
]
}
}
}
]
},
"version": "v1",
"kind": "ModelPermissions"
}
]
}
]
}

View File

@ -0,0 +1,5 @@
query MyQuery {
Artist {
Name
}
}

View File

@ -0,0 +1,8 @@
[
{
"x-hasura-role": "admin"
},
{
"x-hasura-role": "user"
}
]

View File

@ -717,6 +717,38 @@
"version": "v1",
"kind": "TypePermissions"
},
{
"definition": {
"sourceType": "Artist",
"name": "Albums",
"target": {
"model": {
"name": "Albums",
"relationshipType": "Array"
}
},
"mapping": [
{
"source": {
"fieldPath": [
{
"fieldName": "ArtistId"
}
]
},
"target": {
"modelField": [
{
"fieldName": "ArtistId"
}
]
}
}
]
},
"version": "v1",
"kind": "Relationship"
},
{
"definition": {
"sourceType": "Album",

View File

@ -908,6 +908,24 @@ fn test_model_select_many_relationship_predicate_array_nested() -> anyhow::Resul
)
}
// Nested Array relationship with multiple fields
#[test]
fn test_model_select_many_relationship_predicate_array_nested_multiple_fields() -> anyhow::Result<()>
{
let test_path_string =
"execute/models/select_many/relationship_predicates/array/nested_multiple_fields";
let common_metadata_path_string = "execute/common_metadata/postgres_connector_schema.json";
let boolean_exp_rel_metadata_path_string =
"execute/models/select_many/relationship_predicates/common_metadata.json";
common::test_execution_expectation(
test_path_string,
&[
common_metadata_path_string,
boolean_exp_rel_metadata_path_string,
],
)
}
// Tests using relationships in predicates
// Object relationship
#[test]

View File

@ -266,21 +266,3 @@ fn build_is_null_expression(
})
}
}
pub fn build_path_elements(
relationship_paths: &Vec<NDCRelationshipName>,
) -> Vec<ndc_models::PathElement> {
let mut path_elements = Vec::new();
for path in relationship_paths {
path_elements.push(ndc_models::PathElement {
relationship: path.0.clone(),
arguments: BTreeMap::new(),
// 'AND' predicate indicates that the column can be accessed
// by joining all the relationships paths provided
predicate: Some(Box::new(ndc_models::Expression::And {
expressions: Vec::new(),
})),
})
}
path_elements
}

View File

@ -63,12 +63,10 @@ pub(crate) fn model_selection_ir<'s>(
match permissions_predicate {
metadata_resolve::FilterPermission::AllowAll => {}
metadata_resolve::FilterPermission::Filter(predicate) => {
let permissions_predicate_relationship_paths = Vec::new();
let mut permissions_predicate_relationships = BTreeMap::new();
let processed_model_perdicate = permissions::process_model_predicate(
predicate,
session_variables,
permissions_predicate_relationship_paths,
&mut permissions_predicate_relationships,
usage_counts,
)?;

View File

@ -75,7 +75,6 @@ pub(crate) fn get_argument_presets(
pub(crate) fn process_model_predicate<'s>(
model_predicate: &'s metadata_resolve::ModelPredicate,
session_variables: &SessionVariables,
mut relationship_paths: Vec<NDCRelationshipName>,
relationships: &mut BTreeMap<NDCRelationshipName, LocalModelRelationshipInfo<'s>>,
usage_counts: &mut UsagesCounts,
) -> Result<ndc_models::Expression, error::Error> {
@ -87,7 +86,6 @@ pub(crate) fn process_model_predicate<'s>(
} => Ok(make_permission_unary_boolean_expression(
ndc_column.clone(),
*operator,
&relationship_paths,
)?),
metadata_resolve::ModelPredicate::BinaryFieldComparison {
field: _,
@ -101,17 +99,11 @@ pub(crate) fn process_model_predicate<'s>(
operator,
value,
session_variables,
&relationship_paths,
usage_counts,
)?),
metadata_resolve::ModelPredicate::Not(predicate) => {
let expr = process_model_predicate(
predicate,
session_variables,
relationship_paths,
relationships,
usage_counts,
)?;
let expr =
process_model_predicate(predicate, session_variables, relationships, usage_counts)?;
Ok(ndc_models::Expression::Not {
expression: Box::new(expr),
})
@ -119,30 +111,14 @@ pub(crate) fn process_model_predicate<'s>(
metadata_resolve::ModelPredicate::And(predicates) => {
let exprs = predicates
.iter()
.map(|p| {
process_model_predicate(
p,
session_variables,
relationship_paths.clone(),
relationships,
usage_counts,
)
})
.map(|p| process_model_predicate(p, session_variables, relationships, usage_counts))
.collect::<Result<Vec<_>, error::Error>>()?;
Ok(ndc_models::Expression::And { expressions: exprs })
}
metadata_resolve::ModelPredicate::Or(predicates) => {
let exprs = predicates
.iter()
.map(|p| {
process_model_predicate(
p,
session_variables,
relationship_paths.clone(),
relationships,
usage_counts,
)
})
.map(|p| process_model_predicate(p, session_variables, relationships, usage_counts))
.collect::<Result<Vec<_>, error::Error>>()?;
Ok(ndc_models::Expression::Or { expressions: exprs })
}
@ -150,12 +126,14 @@ pub(crate) fn process_model_predicate<'s>(
relationship_info,
predicate,
} => {
// Add the target model being used in the usage counts
count_model(&relationship_info.target_model_name, usage_counts);
let relationship_name = (NDCRelationshipName::new(
&relationship_info.source_type,
&relationship_info.relationship_name,
))?;
relationship_paths.push(relationship_name.clone());
relationships.insert(
relationship_name.clone(),
LocalModelRelationshipInfo {
@ -170,16 +148,18 @@ pub(crate) fn process_model_predicate<'s>(
},
);
// Add the target model being used in the usage counts
count_model(&relationship_info.target_model_name, usage_counts);
let relationship_predicate =
process_model_predicate(predicate, session_variables, relationships, usage_counts)?;
process_model_predicate(
predicate,
session_variables,
relationship_paths,
relationships,
usage_counts,
)
let exists_in_relationship = ndc_models::ExistsInCollection::Related {
relationship: relationship_name.0,
arguments: BTreeMap::new(),
};
Ok(ndc_models::Expression::Exists {
in_collection: exists_in_relationship,
predicate: Some(Box::new(relationship_predicate)),
})
}
}
}
@ -190,10 +170,8 @@ fn make_permission_binary_boolean_expression(
operator: &str,
value_expression: &metadata_resolve::ValueExpression,
session_variables: &SessionVariables,
relationship_paths: &Vec<NDCRelationshipName>,
usage_counts: &mut UsagesCounts,
) -> Result<ndc_models::Expression, error::Error> {
let path_elements = super::filter::build_path_elements(relationship_paths);
let ndc_expression_value = make_value_from_value_expression(
value_expression,
argument_type,
@ -203,7 +181,7 @@ fn make_permission_binary_boolean_expression(
Ok(ndc_models::Expression::BinaryComparisonOperator {
column: ndc_models::ComparisonTarget::Column {
name: ndc_column,
path: path_elements,
path: Vec::new(),
},
operator: operator.to_owned(),
value: ndc_models::ComparisonValue::Scalar {
@ -215,13 +193,11 @@ fn make_permission_binary_boolean_expression(
fn make_permission_unary_boolean_expression(
ndc_column: String,
operator: ndc_models::UnaryComparisonOperator,
relationship_paths: &Vec<NDCRelationshipName>,
) -> Result<ndc_models::Expression, error::Error> {
let path_elements = super::filter::build_path_elements(relationship_paths);
Ok(ndc_models::Expression::UnaryComparisonOperator {
column: ndc_models::ComparisonTarget::Column {
name: ndc_column,
path: path_elements,
path: Vec::new(),
},
operator,
})
@ -245,13 +221,11 @@ pub(crate) fn make_value_from_value_expression(
typecast_session_variable(value, value_type)
}
metadata_resolve::ValueExpression::BooleanExpression(model_predicate) => {
let relationship_paths = Vec::new();
let mut relationships = BTreeMap::new();
let ndc_expression = process_model_predicate(
model_predicate,
session_variables,
relationship_paths,
&mut relationships,
usage_counts,
)?;