Skip to content

Commit

Permalink
Remove Option from ModelWithPermissions (hasura#529)
Browse files Browse the repository at this point in the history 
<!-- Thank you for submitting this PR! :) -->

## Description

Much as per hasura/v3-engine#524 for
`CommandWithPermission`, this removes the `Option` from
`ModelWithPermissions`. Again, select permissions are opt-in, so
`Some(HashMap::new())` is the same as `None`, so we can remove this
layer of indirection, which I believe was only there because we were
building up the `Model` type incrementally.

Functional no-op.

V3_GIT_ORIGIN_REV_ID: fcb6479f32b001380ae24db6439e96339a868692
  • Loading branch information
@danieljharvey @hasura-bot
danieljharvey authored and hasura-bot committed May 3, 2024
1 parent 66990c0 commit aae750a
Show file tree
Hide file tree
Showing 5 changed files with 91 additions and 121 deletions.
29 changes: 11 additions & 18 deletions v3/crates/engine/src/schema/model_arguments.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,27 +84,20 @@ pub fn build_model_argument_fields(
gql_schema::DeprecationStatus::NotDeprecated,
);

// if we have select_permissions, then work out which arguments to include in the schema
match &model.select_permissions {
// if no permissions apply, allow the argument through
None => Ok((field_name, builder.allow_all_namespaced(input_field, None))),
Some(permissions_by_namespace) => {
let mut namespaced_annotations = HashMap::new();
let mut namespaced_annotations = HashMap::new();

for (namespace, permission) in permissions_by_namespace {
// if there is a preset for this argument, remove it from the schema
// so the user cannot provide one
if permission.argument_presets.get(argument_name).is_none() {
namespaced_annotations.insert(namespace.clone(), None);
}
}

Ok((
field_name,
builder.conditional_namespaced(input_field, namespaced_annotations),
))
for (namespace, permission) in &model.select_permissions {
// if there is a preset for this argument, remove it from the schema
// so the user cannot provide one
if permission.argument_presets.get(argument_name).is_none() {
namespaced_annotations.insert(namespace.clone(), None);
}
}

Ok((
field_name,
builder.conditional_namespaced(input_field, namespaced_annotations),
))
})
.collect()
}
Expand Down
167 changes: 73 additions & 94 deletions v3/crates/engine/src/schema/permissions.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,43 +20,38 @@ pub(crate) fn get_select_permissions_namespace_annotations(
) -> Result<HashMap<Role, Option<types::NamespaceAnnotation>>, schema::Error> {
let mut permissions: HashMap<Role, Option<types::NamespaceAnnotation>> = model
.select_permissions
.as_ref()
.map(|permissions| {
permissions
.iter()
.map(|(role, select_permission)| {
(
role.clone(),
Some(types::NamespaceAnnotation::Model {
filter: select_permission.filter.clone(),
argument_presets: types::ArgumentPresets {
argument_presets: select_permission
.argument_presets
.iter()
.map(|(arg_name, preset)| {
(
ArgumentNameAndPath {
ndc_argument_name: model
.model
.source
.as_ref()
.and_then(|model_source| {
model_source.argument_mappings.get(arg_name)
})
.cloned(),
field_path: vec![],
},
preset.clone(),
)
})
.collect(),
},
}),
)
})
.collect()
.iter()
.map(|(role, select_permission)| {
(
role.clone(),
Some(types::NamespaceAnnotation::Model {
filter: select_permission.filter.clone(),
argument_presets: types::ArgumentPresets {
argument_presets: select_permission
.argument_presets
.iter()
.map(|(arg_name, preset)| {
(
ArgumentNameAndPath {
ndc_argument_name: model
.model
.source
.as_ref()
.and_then(|model_source| {
model_source.argument_mappings.get(arg_name)
})
.cloned(),
field_path: vec![],
},
preset.clone(),
)
})
.collect(),
},
}),
)
})
.unwrap_or_default();
.collect();

// if any of model argument's input type has field presets defined, add
// them to model argument preset annotations as well. if there is no
Expand Down Expand Up @@ -453,33 +448,25 @@ pub(crate) fn get_node_field_namespace_permissions(
) -> HashMap<Role, metadata_resolve::FilterPermission> {
let mut permissions = HashMap::new();

match &model.select_permissions {
// Model doesn't have any select permissions, so no `FilterPermission` can be obtained
None => {}
Some(select_permissions) => {
for (role, type_output_permission) in
&object_type_representation.type_output_permissions
{
let is_global_id_field_accessible = object_type_representation
.object_type
.global_id_fields
.iter()
.all(|field_name| type_output_permission.allowed_fields.contains(field_name));

if is_global_id_field_accessible {
let select_permission = select_permissions.get(role).map(|s| s.filter.clone());

match select_permission {
// Select permission doesn't exist for the role, so no `FilterPermission` can
// be obtained.
None => {}
Some(select_permission) => {
permissions.insert(role.clone(), select_permission);
}
}
};
for (role, type_output_permission) in &object_type_representation.type_output_permissions {
let is_global_id_field_accessible = object_type_representation
.object_type
.global_id_fields
.iter()
.all(|field_name| type_output_permission.allowed_fields.contains(field_name));

if is_global_id_field_accessible {
let select_permission = model.select_permissions.get(role).map(|s| s.filter.clone());

match select_permission {
// Select permission doesn't exist for the role, so no `FilterPermission` can
// be obtained.
None => {}
Some(select_permission) => {
permissions.insert(role.clone(), select_permission);
}
}
}
};
}

permissions
Expand All @@ -492,39 +479,31 @@ pub(crate) fn get_entities_field_namespace_permissions(
) -> HashMap<Role, metadata_resolve::FilterPermission> {
let mut permissions = HashMap::new();

match &model.select_permissions {
// Model doesn't have any select permissions, so no `FilterPermission` can be obtained
None => {}
Some(select_permissions) => {
for (role, type_output_permission) in
&object_type_representation.type_output_permissions
{
if let Some(apollo_federation_config) = &object_type_representation
.object_type
.apollo_federation_config
{
let is_all_keys_field_accessible =
apollo_federation_config.keys.iter().all(|key_fields| {
key_fields.fields.iter().all(|field_name| {
type_output_permission.allowed_fields.contains(field_name)
})
});

if is_all_keys_field_accessible {
let select_permission =
select_permissions.get(role).map(|s| s.filter.clone());

match select_permission {
// Select permission doesn't exist for the role, so no `FilterPermission` can
// be obtained.
None => {}
Some(select_permission) => {
permissions.insert(role.clone(), select_permission);
}
}
};
for (role, type_output_permission) in &object_type_representation.type_output_permissions {
if let Some(apollo_federation_config) = &object_type_representation
.object_type
.apollo_federation_config
{
let is_all_keys_field_accessible =
apollo_federation_config.keys.iter().all(|key_fields| {
key_fields.fields.iter().all(|field_name| {
type_output_permission.allowed_fields.contains(field_name)
})
});

if is_all_keys_field_accessible {
let select_permission =
model.select_permissions.get(role).map(|s| s.filter.clone());

match select_permission {
// Select permission doesn't exist for the role, so no `FilterPermission` can
// be obtained.
None => {}
Some(select_permission) => {
permissions.insert(role.clone(), select_permission);
}
}
}
};
}
}

Expand Down
8 changes: 4 additions & 4 deletions v3/crates/metadata-resolve/src/stages/model_permissions/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ pub fn resolve(
model_name.clone(),
ModelWithPermissions {
model: model.clone(),
select_permissions: None,
select_permissions: HashMap::new(),
},
)
})
Expand All @@ -67,16 +67,16 @@ pub fn resolve(
model_name: model_name.clone(),
})?;

if model.select_permissions.is_none() {
let select_permissions = Some(resolve_model_select_permissions(
if model.select_permissions.is_empty() {
let select_permissions = resolve_model_select_permissions(
&model.model,
subgraph,
permissions,
data_connectors,
object_types,
models, // This is required to get the model for the relationship target
boolean_expression_types,
)?);
)?;

model.select_permissions = select_permissions;
} else {
Expand Down
2 changes: 1 addition & 1 deletion v3/crates/metadata-resolve/src/stages/model_permissions/types.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use serde::{Deserialize, Serialize};
#[derive(Serialize, Deserialize, Clone, Debug, PartialEq)]
pub struct ModelWithPermissions {
pub model: models::Model,
pub select_permissions: Option<HashMap<Role, SelectPermission>>,
pub select_permissions: HashMap<Role, SelectPermission>,
}

#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq)]
Expand Down
6 changes: 2 additions & 4 deletions v3/crates/metadata-resolve/src/stages/roles/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,8 @@ pub fn resolve(
}
}
for model in models.values() {
if let Some(select_permissions) = &model.select_permissions {
for role in select_permissions.keys() {
roles.push(role.clone());
}
for role in model.select_permissions.keys() {
roles.push(role.clone());
}
}
for command in commands.values() {
Expand Down

0 comments on commit aae750a

Please sign in to comment.