fix: stricter merge rules for @requiresScopes and @Policy

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"]])
}
```

Author: dariuszkuc

.changeset/tasty-snails-invent.md

@@ -0,0 +1,59 @@
+---
+"@apollo/composition": patch
+"@apollo/federation-internals": patch
+---
+
+Stricter merge rules for @requiresScopes and @policy
+
+Current merge policies for `@authenticated`, `@requiresScopes` and `@policy` were inconsistent.
+
+If a shared field uses the same authorization directives across subgraphs, composition merges them using `OR` logic. However, if a shared field uses different authorization directives across subgraphs composition merges them using `AND` logic. This simplified schema evolution, but weakened security requirements. Therefore, the behavior has been changed to always apply `AND` logic to authorization directives applied to the same field across subgraphs.
+
+Since `@policy` and `@requiresScopes` values represent boolean conditions in Disjunctive Normal Form, we can merge them conjunctively to get the final auth requirements. For example:
+
+```graphql
+# subgraph A
+type T @authenticated {
+  # requires scopes (A1 AND A2) OR A3
+  secret: String @requiresScopes(scopes: [["A1", "A2"], ["A3"]])
+}
+
+# subgraph B
+type T {
+  # requires scopes B1 OR B2
+  secret: String @requiresScopes(scopes: [["B1"], ["B2"]]
+}
+
+# composed supergraph
+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
+# subgraph A
+type T {
+  # requires A1 AND A2 scopes to access
+  secret: String @requiresScopes(scopes: [["A1", "A2"]])
+}
+
+# subgraph B
+type T {
+  # requires only A1 scope to access
+  secret: String @requiresScopes(scopes: [["A1"]])
+}
+
+# composed supergraph
+type T {
+  # requires only A1 scope to access as A2 is redundant
+  secret: String @requiresScopes(scopes: [["A1"]])
+}
+```

composition-js/src/__tests__/compose.directiveArgumentMergeStrategies.test.ts

@@ -159,6 +159,22 @@ describe('composition of directive with non-trivial argument strategies', () =>
     resultValues: {
       t: ['foo', 'bar'], k: ['v1', 'v2'], b: ['x'],
     },
+  },
+  {
+    name: 'dnf_conjunction',
+    // [[String!]!]!
+    type: (schema: Schema) => new NonNullType(new ListType(
+        new NonNullType(new ListType(
+            new NonNullType(schema.stringType())))
+    )),
+    compositionStrategy: ARGUMENT_COMPOSITION_STRATEGIES.DNF_CONJUNCTION,
+    argValues: {
+      s1: { t: [['foo'], ['bar']], k: [['v1']] },
+      s2: { t: [['foo'], ['bar'], ['baz']], k: [['v2', 'v3']], b: [['x']] },
+    },
+    resultValues: {
+      t: [['bar'], ['foo']], k: [['v1', 'v2', 'v3']], b: [['x']],
+    },
   }])('works for $name', ({ name, type, compositionStrategy, argValues, resultValues }) => {
     createTestFeature({
       url: 'https://specs.apollo.dev',

composition-js/src/__tests__/compose.test.ts

@@ -4560,14 +4560,14 @@ describe('composition', () => {
       expect(result2.schema.type('A')?.hasAppliedDirective(directiveName.slice(1))).toBeTruthy();
     });
 
-    it.each(testsToRun)('merges ${directiveName} lists (simple union)', ({ directiveName, argName }) => {
+    it.each(testsToRun)('merges $directiveName lists (outer sum)', ({ directiveName, argName }) => {
       const a1 = {
         typeDefs: gql`
           type Query {
             a: A
           }
 
-          type A ${directiveName}(${argName}: ["a"]) @key(fields: "id") {
+          type A ${directiveName}(${argName}: [["a1", "a2"], ["a3"]]) @key(fields: "id") {
             id: String!
             a1: String
           }
@@ -4576,7 +4576,7 @@ describe('composition', () => {
       };
       const a2 = {
         typeDefs: gql`
-          type A ${directiveName}(${argName}: ["b"]) @key(fields: "id") {
+          type A ${directiveName}(${argName}: [["b1"], ["b2"]]) @key(fields: "id") {
             id: String!
             a2: String
           }
@@ -4589,18 +4589,24 @@ describe('composition', () => {
       expect(
         result.schema.type('A')
           ?.appliedDirectivesOf(directiveName.slice(1))
-          ?.[0]?.arguments()?.[argName]).toStrictEqual(['a', 'b']
+          ?.[0]?.arguments()?.[argName]).toStrictEqual(
+              [
+                ['a3', 'b1'],
+                ['a3', 'b2'],
+                ['a1', 'a2', 'b1'],
+                ['a1', 'a2', 'b2']
+              ]
       );
     });
 
-    it.each(testsToRun)('merges ${directiveName} lists (deduplicates intersecting scopes)', ({ directiveName, argName }) => {
+    it.each(testsToRun)('merges $directiveName lists (deduplicates redundant scopes)', ({ directiveName, argName }) => {
       const a1 = {
         typeDefs: gql`
           type Query {
             a: A
           }
 
-          type A ${directiveName}(${argName}: ["a", "b"]) @key(fields: "id") {
+          type A ${directiveName}(${argName}: [["a"], ["c"]]) @key(fields: "id") {
             id: String!
             a1: String
           }
@@ -4609,28 +4615,41 @@ describe('composition', () => {
       };
       const a2 = {
         typeDefs: gql`
-          type A ${directiveName}(${argName}: ["b", "c"]) @key(fields: "id") {
+          type A ${directiveName}(${argName}: [["a"], ["b"], ["c"]]) @key(fields: "id") {
             id: String!
             a2: String
           }
         `,
         name: 'a2',
       };
+      const a3 = {
+        typeDefs: gql`
+          type A ${directiveName}(${argName}: [["a"], ["b", "c"]]) @key(fields: "id") {
+            id: String!
+            a3: String
+          }
+        `,
+        name: 'a3',
+      };
 
-      const result = composeAsFed2Subgraphs([a1, a2]);
+      const result = composeAsFed2Subgraphs([a1, a2, a3]);
       assertCompositionSuccess(result);
       expect(
         result.schema.type('A')
           ?.appliedDirectivesOf(directiveName.slice(1))
-          ?.[0]?.arguments()?.[argName]).toStrictEqual(['a', 'b', 'c']
+          ?.[0]?.arguments()?.[argName]).toStrictEqual(
+            [
+              ['a'],
+              ['b', 'c'],
+            ]
       );
     });
 
     it.each(testsToRun)('${directiveName} has correct definition in the supergraph', ({ directiveName, argName, argType, identity }) => {
       const a = {
         typeDefs: gql`
           type Query {
-            x: Int ${directiveName}(${argName}: ["a", "b"])
+            x: Int ${directiveName}(${argName}: [["a"], ["b"]])
           }
         `,
         name: 'a',

internals-js/src/argumentCompositionStrategies.ts

@@ -1,4 +1,4 @@
-import { InputType, NonNullType, Schema, isListType, isNonNullType } from "./definitions"
+import {InputType, NonNullType, Schema, isListType, isNonNullType} from "./definitions"
 import { sameType } from "./types";
 import { valueEquals } from "./values";
 
@@ -19,6 +19,14 @@ function supportFixedTypes(types: (schema: Schema) => InputType[]): TypeSupportV
   };
 }
 
+function supportAnyNonNullNestedArray(): TypeSupportValidator {
+  return (_, type) =>
+    isNonNullType(type) && isListType(type.ofType)
+      && isNonNullType(type.ofType.ofType) && isListType(type.ofType.ofType.ofType)
+        ? { valid: true }
+        : { valid: false, supportedMsg: 'non nullable nested list types of any type' }
+}
+
 function supportAnyNonNullArray(): TypeSupportValidator {
   return (_, type) => isNonNullType(type) && isListType(type.ofType)
     ? { valid: true }
@@ -54,6 +62,104 @@ function unionValues(values: any[]): any {
   }, []);
 }
 
+/**
+ * Performs conjunction of 2d arrays that represent conditions in Disjunctive Normal Form.
+ *
+ * * Each inner array is interpreted as the conjunction of the conditions in the array.
+ * * The top-level array is interpreted as the disjunction of the inner arrays
+ *
+ * Algorithm
+ * * filter out duplicate entries to limit the amount of necessary computations
+ * * calculate cartesian product of the arrays to find all possible combinations
+ *   * simplify combinations by dropping duplicate conditions (i.e. p ^ p = p, p ^ q = q ^ p)
+ * * eliminate entries that are subsumed by others (i.e. (p ^ q) subsumes (p ^ q ^ r))
+ */
+function dnfConjunction<T>(values: T[][][]): T[][] {
+  // should never be the case
+  if (values.length == 0) {
+    return [];
+  }
+
+  // we first filter out duplicate values from candidates
+  // this avoids exponential computation of exactly the same conditions
+  const filtered = filterNestedArrayDuplicates(values);
+
+  // initialize with first entry
+  let result: T[][] = filtered[0];
+  // perform cartesian product to find all possible entries
+  for (let i = 1; i < filtered.length; i++) {
+    const current = filtered[i];
+    const accumulator: T[][] = [];
+    const seen = new Set<string>;
+
+    for (const accElement of result) {
+      for (const currentElement of current) {
+        // filter out elements that are already present in accElement
+        const filteredElement = currentElement.filter((e) => !accElement.includes(e));
+        const candidate = [...accElement, ...filteredElement].sort();
+        const key = JSON.stringify(candidate);
+        // only add entries which has not been seen yet
+        if (!seen.has(key)) {
+          seen.add(key);
+          accumulator.push(candidate);
+        }
+      }
+    }
+    // Now we need to deduplicate the results. Given that
+    // - outer array implies OR requirements
+    // - inner array implies AND requirements
+    // We can filter out any inner arrays that fully contain other inner arrays, i.e.
+    //   A OR B OR (A AND B) OR (A AND B AND C) => A OR B
+    result = deduplicateSubsumedValues(accumulator);
+  }
+  return result;
+}
+
+function filterNestedArrayDuplicates<T>(values: T[][][]): T[][][] {
+  const filtered: T[][][] = [];
+  const seen = new Set<string>;
+  values.forEach((value) => {
+    value.sort();
+    const key = JSON.stringify(value);
+    if (!seen.has(key)) {
+      seen.add(key);
+      filtered.push(value);
+    }
+  });
+  return filtered;
+}
+
+function deduplicateSubsumedValues<T>(values: T[][]): T[][] {
+  const result: T[][] = [];
+  // we first sort by length as the longer ones might be dropped
+  values.sort((first, second) => {
+    if (first.length < second.length) {
+      return -1;
+    } else if (first.length > second.length) {
+      return 1;
+    } else {
+      return 0;
+    }
+  });
+
+  for (const candidate of values) {
+    const entry = new Set(candidate);
+    let redundant = false;
+    for (const r of result) {
+      if (r.every(e => entry.has(e))) {
+        // if `r` is a subset of a `candidate` then it means `candidate` is redundant
+        redundant = true;
+        break;
+      }
+    }
+
+    if (!redundant) {
+      result.push(candidate);
+    }
+  }
+  return result;
+}
+
 export const ARGUMENT_COMPOSITION_STRATEGIES = {
   MAX: {
     name: 'MAX',
@@ -95,7 +201,8 @@ export const ARGUMENT_COMPOSITION_STRATEGIES = {
       schema.booleanType(),
       new NonNullType(schema.booleanType())
     ]),
-    mergeValues: mergeNullableValues(
+    mergeValues:
+        mergeNullableValues(
       (values: boolean[]) => values.every((v) => v)
     ),
   },
@@ -113,5 +220,10 @@ export const ARGUMENT_COMPOSITION_STRATEGIES = {
     name: 'NULLABLE_UNION',
     isTypeSupported: supportAnyArray(),
     mergeValues: mergeNullableValues(unionValues),
+  },
+  DNF_CONJUNCTION: {
+    name: 'DNF_CONJUNCTION',
+    isTypeSupported: supportAnyNonNullNestedArray(),
+    mergeValues: dnfConjunction
   }
 }

internals-js/src/specs/policySpec.ts

@@ -42,7 +42,7 @@ export class PolicySpecDefinition extends FeatureDefinition {
           assert(PolicyType, () => `Expected "${policyName}" to be defined`);
           return new NonNullType(new ListType(new NonNullType(new ListType(new NonNullType(PolicyType)))));
         },
-        compositionStrategy: ARGUMENT_COMPOSITION_STRATEGIES.UNION,
+        compositionStrategy: ARGUMENT_COMPOSITION_STRATEGIES.DNF_CONJUNCTION,
       }],
       locations: [
         DirectiveLocation.FIELD_DEFINITION,

internals-js/src/specs/requiresScopesSpec.ts

@@ -43,7 +43,7 @@ export class RequiresScopesSpecDefinition extends FeatureDefinition {
           assert(scopeType, () => `Expected "${scopeName}" to be defined`);
           return new NonNullType(new ListType(new NonNullType(new ListType(new NonNullType(scopeType)))));
         },
-        compositionStrategy: ARGUMENT_COMPOSITION_STRATEGIES.UNION,
+        compositionStrategy: ARGUMENT_COMPOSITION_STRATEGIES.DNF_CONJUNCTION,
       }],
       locations: [
         DirectiveLocation.FIELD_DEFINITION,