Skip to content

Commit 71c9779

Browse files
feat(composition): add DNF conjunction argument merge strategy (#8817)
Current merge policies for `@authenticated`, `@requiresScopes` and `@policy` were inconsistent. If single subgraph declared a field with one of the directives then it would restrict access to this supergraph field regardless which subgraph would resolve this field (results in AND rule for any applied auth directive, i.e. `@authenticated` AND `@policy` is required to access this field). If the same auth directive (`@requiresScopes`/`@policy`) were applied across the subgraphs then the resulting supergraph field could be resolved by fullfilling either one of the subgraph requirements (resulting in OR rule, i.e. either `@policy` 1 or `@policy` 2 has to be true to access the field). While arguably this allowed for easier schema evolution, it did result in weakening the security requirements. Since `@policy` and `@requiresScopes` values are represent boolean conditions in Disjunctive Normal Form, we can merge them conjunctively to get the final auth requirements, i.e. ```graphql type T @authenticated { # requires scopes (A1 AND A2) OR A3 secret: String @requiresScopes(scopes: [["A1", "A2"], ["A3"]]) } type T { # requires scopes B1 OR B2 secret: String @requiresScopes(scopes: [["B1"], ["B2"]] } type T @authenticated { secret: String @requiresScopes( scopes: [ ["A1", "A2", "B1"], ["A1", "A2", "B2"], ["A3", "B1"], ["A3", "B2"] ]) } ``` This algorithm also deduplicates redundant requirements, e.g. ```graphql type T { # requires A1 AND A2 scopes to access secret: String @requiresScopes(scopes: [["A1", "A2"]]) } type T { # requires only A1 scope to access secret: String @requiresScopes(scopes: [["A1"]]) } type T { # requires only A1 scope to access as A2 is redundant secret: String @requiresScopes(scopes: [["A1"]]) } ``` Partial backport of apollographql/federation#3321 and apollographql/federation#3343 Co-authored-by: Sachin D. Shinde <sachin@apollographql.com>
1 parent d1e2efb commit 71c9779

1 file changed

Lines changed: 319 additions & 0 deletions

File tree

apollo-federation/src/schema/argument_composition_strategies.rs

Lines changed: 319 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,13 @@
1+
use std::collections::BTreeSet;
12
use std::sync::LazyLock;
23

4+
use apollo_compiler::Node;
35
use apollo_compiler::ast::Value;
46
use apollo_compiler::collections::HashSet;
57
use apollo_compiler::collections::IndexSet;
68
use apollo_compiler::schema::Type;
79
use apollo_compiler::ty;
10+
use itertools::Itertools;
811

912
use crate::schema::FederationSchema;
1013

@@ -19,6 +22,7 @@ pub(crate) enum ArgumentCompositionStrategy {
1922
NullableAnd,
2023
NullableMax,
2124
NullableUnion,
25+
DnfConjunction,
2226
}
2327

2428
pub(crate) static MAX_STRATEGY: LazyLock<MaxArgumentCompositionStrategy> =
@@ -37,6 +41,8 @@ pub(crate) static NULLABLE_MAX_STRATEGY: LazyLock<NullableMaxArgumentComposition
3741
LazyLock::new(NullableMaxArgumentCompositionStrategy::new);
3842
pub(crate) static NULLABLE_UNION_STRATEGY: LazyLock<NullableUnionArgumentCompositionStrategy> =
3943
LazyLock::new(|| NullableUnionArgumentCompositionStrategy {});
44+
pub(crate) static DNF_CONJUNCTION_STRATEGY: LazyLock<DnfConjunctionArgumentCompositionStrategy> =
45+
LazyLock::new(|| DnfConjunctionArgumentCompositionStrategy {});
4046

4147
impl ArgumentCompositionStrategy {
4248
fn get_impl(&self) -> &dyn ArgumentComposition {
@@ -49,6 +55,7 @@ impl ArgumentCompositionStrategy {
4955
Self::NullableAnd => &*NULLABLE_AND_STRATEGY,
5056
Self::NullableMax => &*NULLABLE_MAX_STRATEGY,
5157
Self::NullableUnion => &*NULLABLE_UNION_STRATEGY,
58+
Self::DnfConjunction => &*DNF_CONJUNCTION_STRATEGY,
5259
}
5360
}
5461

@@ -132,6 +139,20 @@ fn support_any_array(ty: &Type) -> Result<(), String> {
132139
}
133140
}
134141

142+
/// Support for doubly nested non-nullable list types of any non-nullable type `[[Foo!]!]!`.
143+
/// PORT_NOTE: This differs slightly from JS behavior, which would allow the innermost type to be
144+
/// nullable.
145+
fn support_any_non_null_nested_array(ty: &Type) -> Result<(), String> {
146+
if matches!(ty, Type::NonNullList(_))
147+
&& matches!(ty.item_type(), Type::NonNullList(_))
148+
&& ty.item_type().item_type().is_non_null()
149+
{
150+
Ok(())
151+
} else {
152+
Err("non-nullable doubly nested list of any type".to_string())
153+
}
154+
}
155+
135156
fn max_int_value<'a>(values: impl Iterator<Item = &'a Value>) -> Value {
136157
values
137158
.filter_map(|val| match val {
@@ -193,6 +214,148 @@ fn merge_nullable_values(
193214
merge_values(&values).into()
194215
}
195216

217+
/// Performs conjunction of 2d arrays that represent conditions in Disjunctive Normal Form.
218+
///
219+
/// Each 2D array is interpreted as follows
220+
/// * Inner array is interpreted as the conjunction (an AND) of the conditions in the array.
221+
/// * Outer array is interpreted as the disjunction (an OR) of the inner arrays.
222+
///
223+
/// Algorithm
224+
/// * filter out duplicate entries to limit the amount of necessary computations
225+
/// * calculate cartesian product of the arrays to find all possible combinations
226+
/// * simplify combinations by dropping duplicate conditions (i.e. p ^ p = p, p ^ q = q ^ p)
227+
/// * eliminate entries that are subsumed by others (i.e. (p ^ q) subsumes (p ^ q ^ r))
228+
///
229+
/// PORT_NOTE: While JS has a poor representation for sets, that's not true in Rust, and accordingly
230+
/// data structures here have been changed to account for that. As part of this change, this
231+
/// function will now always deduplicate elements of a conjunction, whereas the JS code would only
232+
/// sometimes deduplicate that.
233+
fn dnf_conjunction(values: &[Value]) -> Value {
234+
// Copy the 2D arrays to sort them and remove duplicates. Note that we assume the arrays here
235+
// are lists-of-lists-of-values, as GraphQL validation should have already verified this.
236+
let mut filtered = values
237+
.iter()
238+
.flat_map(Value::as_list)
239+
.map(|disjunction| {
240+
// Normally for DNF, you'd consider [] to be always false and [[]] to be always true,
241+
// and code that uses any()/all() needs no special-casing to work with these
242+
// definitions. However, router special-cases [] to also mean true, and so if we're
243+
// about to do any evaluation on DNFs, we need to do these conversions beforehand.
244+
if disjunction.is_empty() {
245+
std::iter::once(Default::default()).collect()
246+
} else {
247+
disjunction
248+
.iter()
249+
.map(Node::as_ref)
250+
.flat_map(Value::as_list)
251+
.map(|conjunction| conjunction.iter().cloned().map(DnfMember::from).collect())
252+
.collect()
253+
}
254+
})
255+
.collect::<IndexSet<BTreeSet<BTreeSet<DnfMember>>>>()
256+
.into_iter();
257+
258+
// Initialize with the first entry.
259+
let Some(first) = filtered
260+
.next()
261+
.map(|first| first.into_iter().collect::<IndexSet<BTreeSet<DnfMember>>>())
262+
else {
263+
// Should never be the case this is empty, but if it occurs, we effectively echo the input.
264+
return Value::List(vec![]);
265+
};
266+
267+
// Perform cartesian product to find all possible entries
268+
let result = filtered.fold(first, |result_disjunction, current_disjunction| {
269+
let accumulated = result_disjunction
270+
.into_iter()
271+
.cartesian_product(current_disjunction.iter())
272+
.map(|(result_conjunction, current_conjunction)| {
273+
result_conjunction
274+
.union(current_conjunction)
275+
.cloned()
276+
.collect()
277+
})
278+
.collect::<IndexSet<BTreeSet<DnfMember>>>();
279+
deduplicate_subsumed_values(accumulated)
280+
});
281+
Value::List(
282+
result
283+
.into_iter()
284+
.map(|conjunction| {
285+
Node::new(Value::List(
286+
conjunction.into_iter().map(Node::<Value>::from).collect(),
287+
))
288+
})
289+
.collect(),
290+
)
291+
}
292+
293+
/// Deduplicate subsumed values from 2D arrays.
294+
///
295+
/// Given that
296+
/// - outer array implies OR requirements
297+
/// - inner array implies AND requirements
298+
///
299+
/// We can filter out any inner arrays that fully contain other inner arrays, i.e.
300+
/// A OR B OR (A AND B) OR (A AND B AND C) => A OR B
301+
fn deduplicate_subsumed_values(
302+
mut value: IndexSet<BTreeSet<DnfMember>>,
303+
) -> IndexSet<BTreeSet<DnfMember>> {
304+
// We first sort by length as the longer ones might be dropped
305+
value.sort_by_key(BTreeSet::len);
306+
307+
value
308+
.into_iter()
309+
.fold(Default::default(), |mut result, candidate| {
310+
// if `r` is a subset of a `candidate` then it means `candidate` is redundant
311+
if !result.iter().any(|r| r.is_subset(&candidate)) {
312+
result.insert(candidate);
313+
}
314+
result
315+
})
316+
}
317+
318+
#[derive(Clone, Debug)]
319+
struct DnfMember(Node<str>, Node<Value>);
320+
321+
impl PartialEq for DnfMember {
322+
fn eq(&self, other: &Self) -> bool {
323+
self.0.eq(&other.0)
324+
}
325+
}
326+
327+
impl Eq for DnfMember {}
328+
329+
impl std::hash::Hash for DnfMember {
330+
fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
331+
self.0.hash(state);
332+
}
333+
}
334+
335+
impl PartialOrd for DnfMember {
336+
fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
337+
Some(self.cmp(other))
338+
}
339+
}
340+
341+
impl Ord for DnfMember {
342+
fn cmp(&self, other: &Self) -> std::cmp::Ordering {
343+
self.0.cmp(&other.0)
344+
}
345+
}
346+
347+
impl From<Node<Value>> for DnfMember {
348+
fn from(node: Node<Value>) -> Self {
349+
Self(node.to_string().into(), node)
350+
}
351+
}
352+
353+
impl From<DnfMember> for Node<Value> {
354+
fn from(node: DnfMember) -> Self {
355+
node.1
356+
}
357+
}
358+
196359
// MAX
197360
#[derive(Clone)]
198361
pub(crate) struct MaxArgumentCompositionStrategy {
@@ -435,3 +598,159 @@ impl ArgumentComposition for NullableUnionArgumentCompositionStrategy {
435598
merge_nullable_values(values, |values| union_list_values(values.iter().copied()))
436599
}
437600
}
601+
602+
// DNF_CONJUNCTION
603+
#[derive(Clone)]
604+
pub(crate) struct DnfConjunctionArgumentCompositionStrategy {}
605+
606+
impl ArgumentComposition for DnfConjunctionArgumentCompositionStrategy {
607+
fn name(&self) -> &str {
608+
"DNF_CONJUNCTION"
609+
}
610+
611+
fn is_type_supported(&self, _schema: &FederationSchema, ty: &Type) -> Result<(), String> {
612+
support_any_non_null_nested_array(ty)
613+
}
614+
615+
fn merge_values(&self, values: &[Value]) -> Option<Value> {
616+
dnf_conjunction(values).into()
617+
}
618+
}
619+
620+
#[cfg(test)]
621+
mod tests {
622+
use std::collections::BTreeSet;
623+
624+
use apollo_compiler::Node;
625+
use apollo_compiler::ast::Type;
626+
use apollo_compiler::ast::Value;
627+
use apollo_compiler::collections::IndexSet;
628+
629+
use crate::schema::argument_composition_strategies::ArgumentComposition;
630+
use crate::schema::argument_composition_strategies::DnfConjunctionArgumentCompositionStrategy;
631+
use crate::schema::argument_composition_strategies::DnfMember;
632+
use crate::schema::argument_composition_strategies::deduplicate_subsumed_values;
633+
use crate::schema::argument_composition_strategies::support_any_non_null_nested_array;
634+
635+
#[test]
636+
fn verify_support_any_non_null_nested_array() {
637+
for unsupported_type in [
638+
"String",
639+
"String!",
640+
"[String]",
641+
"[String!]",
642+
"[String]!",
643+
"[String!]!",
644+
"[[String]]",
645+
"[[String!]]",
646+
"[[String]!]",
647+
"[[String!]!]",
648+
"[[String]]!",
649+
"[[String!]]!",
650+
"[[String]!]!", // this one is incorrectly allowed by JS
651+
] {
652+
let _type = Type::parse(unsupported_type, "schema.graphql").expect("valid type");
653+
assert!(support_any_non_null_nested_array(&_type).is_err());
654+
}
655+
656+
for supported_type in ["[[String!]!]!", "[[Foo!]!]!"] {
657+
let _type = Type::parse(supported_type, "schema.graphql").expect("valid type");
658+
assert!(support_any_non_null_nested_array(&_type).is_ok());
659+
}
660+
}
661+
662+
#[test]
663+
fn verify_deduplicate_subsumed_values() {
664+
let value = parse_for_deduplicate_subsumed_values(vec![
665+
vec!["A", "B", "C"],
666+
vec!["A", "B"],
667+
vec!["A"],
668+
]);
669+
let result = deduplicate_subsumed_values(value);
670+
assert_eq!(
671+
parse_for_deduplicate_subsumed_values(vec![vec!["A"]]),
672+
result
673+
);
674+
675+
let value = parse_for_deduplicate_subsumed_values(vec![
676+
vec!["A", "B"],
677+
vec!["A", "B", "C"],
678+
vec!["A", "B", "C", "D"],
679+
vec!["A", "B", "D"],
680+
vec!["A", "B"],
681+
vec!["A", "D"],
682+
vec!["A", "B"],
683+
]);
684+
let result = deduplicate_subsumed_values(value);
685+
assert_eq!(
686+
parse_for_deduplicate_subsumed_values(vec![vec!["A", "B"], vec!["A", "D"]]),
687+
result
688+
);
689+
}
690+
691+
#[test]
692+
fn dnf_conjunction_of_empty_values() {
693+
let strategy = DnfConjunctionArgumentCompositionStrategy {};
694+
let value = strategy
695+
.merge_values(&[])
696+
.expect("successfully computed DNF conjunction value");
697+
assert_eq!(parse_into_ast_value_list(vec![]), value);
698+
}
699+
700+
#[test]
701+
fn dnf_conjunction_of_multiple_lists() {
702+
let strategy = DnfConjunctionArgumentCompositionStrategy {};
703+
let values = parse_into_ast_vec_value_list(vec![
704+
vec![vec!["C", "B", "D"], vec!["B", "A"]],
705+
vec![vec!["A", "D"]],
706+
vec![vec!["A"]],
707+
vec![vec!["A"], vec!["B"]],
708+
vec![vec!["C", "B"]],
709+
vec![vec!["A", "D"]],
710+
vec![vec!["A", "A"]],
711+
]);
712+
let result = strategy
713+
.merge_values(&values)
714+
.expect("computed DNF conjunction value");
715+
assert_eq!(
716+
parse_into_ast_value_list(vec![vec!["A", "B", "C", "D"]]),
717+
result
718+
);
719+
}
720+
721+
fn parse_into_ast_vec_value_list(values: Vec<Vec<Vec<&str>>>) -> Vec<Value> {
722+
let mut result = vec![];
723+
// each outer_array is a specific directive application value of [[Policy!]!]! and/or [[Scope!]!]!
724+
for outer_array in values {
725+
result.push(parse_into_ast_value_list(outer_array));
726+
}
727+
result
728+
}
729+
730+
fn parse_into_ast_value_list(value: Vec<Vec<&str>>) -> Value {
731+
// outer array is interpreted as the disjunction (an OR) of the inner arrays.
732+
let mut disjunctions = vec![];
733+
for inner_array in value {
734+
// inner array is interpreted as the conjunction (an AND) of the conditions in the array.
735+
let mut conjunctions = vec![];
736+
for value in inner_array {
737+
conjunctions.push(Node::new(Value::String(value.to_string())));
738+
}
739+
disjunctions.push(Node::new(Value::List(conjunctions)));
740+
}
741+
Value::List(disjunctions)
742+
}
743+
744+
fn parse_for_deduplicate_subsumed_values(
745+
value: Vec<Vec<&str>>,
746+
) -> IndexSet<BTreeSet<DnfMember>> {
747+
parse_into_ast_value_list(value)
748+
.as_list()
749+
.expect("Test unexpectedly provided a non-list value")
750+
.iter()
751+
.map(Node::as_ref)
752+
.flat_map(Value::as_list)
753+
.map(|conjunction| conjunction.iter().cloned().map(DnfMember::from).collect())
754+
.collect()
755+
}
756+
}

0 commit comments

Comments
 (0)