feat(composition): add DNF conjunction argument merge strategy

[dariuszkuc]

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.

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.

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

[apollo-librarian]

✅ Docs preview has no changes

The preview was not built because there were no changes.

Build ID: 281f14853ea318e70a2317f4
Build Logs: View logs

[github-actions]

@dariuszkuc, please consider creating a changeset entry in /.changesets/. These instructions describe the process and tooling.

[dariuszkuc]

arguably this should be extremely rare so probably doesn't matter

[dariuszkuc]

Port of https://github.com/apollographql/federation/blob/main/internals-js/src/argumentCompositionStrategies.ts#L78

[sachindshinde]

There were a few things below, but I summarized the main bits. Instead of writing out the specifics, I went ahead and filed a PR into this PR addressing the feedback, let me know if that PR looks good/makes sense.

[sachindshinde]

It's fine if you want to fix the code here, but it may be worth leaving a port note that it diverges from JS behavior.

[sachindshinde]

This can be simplified to

Suggested change

	if ty.is_non_null()
	&& ty.is_list()
	&& ty.item_type().is_non_null()
	&& ty.item_type().is_list()
	&& ty.item_type().item_type().is_non_null()
	{
	if matches!(ty, Type::NonNullList(_))
	&& matches!(ty.item_type(), Type::NonNullList(_))
	&& ty.item_type().item_type().is_non_null()
	{

[sachindshinde]

For this function, the JS version had to jump through hoops because JS's support for sets is bad. In Rust, we can just use actual sets, e.g. a IndexSet<BTreeSet<BTreeSet<_>>> (with some new type around Node<Value> that appropriately implements Ord). This representation also allows the code to avoid having the if let Value::List(_) = value unwrapping and Value::List rewrapping everywhere.

[sachindshinde]

We can avoid this expect() if we move the // should never be the case block down here.

[sachindshinde]

This particular representation (a vector and a set that deduplicates it) is a holdover from limitations in JS; we can just use a set around Node<Value> instead (it'll avoid a vector clone down below).