From 6adbf7e86927de969aedab665b6a3a8dbf3a6095 Mon Sep 17 00:00:00 2001 From: dariuszkuc <9501705+dariuszkuc@users.noreply.github.com> Date: Thu, 9 Oct 2025 16:58:46 -0500 Subject: [PATCH 1/5] fix: restrict usage of auth directives on interfaces Restricts usage of `@authenticated`, `@policy` and `@requiresScopes` from being applied on interfaces, interface objects and their fields. GraphQL spec currently does not define any interface inheritance rules and developers have to explicitly redefine all interface fields on their implementations. At runtime, GraphQL servers cannot return abstract types and always return concrete output types. Due to the above, applying auth directives on the interfaces may lead to unexpected runtime behavior as they won't have any effect at runtime. --- .changeset/rotten-trainers-taste.md | 10 ++++ composition-js/src/__tests__/compose.test.ts | 60 ------------------- .../src/__tests__/subgraphValidation.test.ts | 59 ++++++++++++++++++ internals-js/src/error.ts | 7 +++ internals-js/src/federation.ts | 41 ++++++++++++- internals-js/src/specs/authenticatedSpec.ts | 5 ++ internals-js/src/specs/policySpec.ts | 6 +- internals-js/src/specs/requiresScopesSpec.ts | 6 +- 8 files changed, 131 insertions(+), 63 deletions(-) create mode 100644 .changeset/rotten-trainers-taste.md diff --git a/.changeset/rotten-trainers-taste.md b/.changeset/rotten-trainers-taste.md new file mode 100644 index 000000000..18fe4c2d5 --- /dev/null +++ b/.changeset/rotten-trainers-taste.md @@ -0,0 +1,10 @@ +--- +"@apollo/composition": patch +"@apollo/federation-internals": patch +--- + +Restrict usage of auth directives on interfaces + +Restricts usage of `@authenticated`, `@policy` and `@requiresScopes` from being applied on interfaces, interface objects and their fields. + +GraphQL spec currently does not define any interface inheritance rules and developers have to explicitly redefine all interface fields on their implementations. At runtime, GraphQL servers cannot return abstract types and always return concrete output types. Due to the above, applying auth directives on the interfaces may lead to unexpected runtime behavior as they won't have any effect at runtime. diff --git a/composition-js/src/__tests__/compose.test.ts b/composition-js/src/__tests__/compose.test.ts index 3cc57c12f..13875f781 100644 --- a/composition-js/src/__tests__/compose.test.ts +++ b/composition-js/src/__tests__/compose.test.ts @@ -4092,32 +4092,6 @@ describe('composition', () => { name: 'on-object', }; - const onInterface = { - typeDefs: gql` - type Query { - interface: AuthenticatedInterface! - } - - interface AuthenticatedInterface @authenticated { - field: Int! - } - `, - name: 'on-interface', - }; - - const onInterfaceObject = { - typeDefs: gql` - type AuthenticatedInterfaceObject - @interfaceObject - @key(fields: "id") - @authenticated - { - id: String! - } - `, - name: 'on-interface-object', - } - const onScalar = { typeDefs: gql` scalar AuthenticatedScalar @authenticated @@ -4181,8 +4155,6 @@ describe('composition', () => { const result = composeAsFed2Subgraphs([ onObject, - onInterface, - onInterfaceObject, onScalar, onEnum, onRootField, @@ -4193,8 +4165,6 @@ describe('composition', () => { const authenticatedElements = [ "AuthenticatedObject", - "AuthenticatedInterface", - "AuthenticatedInterfaceObject", "AuthenticatedScalar", "AuthenticatedEnum", "Query.authenticatedRootField", @@ -4328,32 +4298,6 @@ describe('composition', () => { name: 'on-object', }; - const onInterface = { - typeDefs: gql` - type Query { - interface: ScopedInterface! - } - - interface ScopedInterface ${directiveName}(${argName}: ["interface"]) { - field: Int! - } - `, - name: 'on-interface', - }; - - const onInterfaceObject = { - typeDefs: gql` - type ScopedInterfaceObject - @interfaceObject - @key(fields: "id") - ${directiveName}(${argName}: ["interfaceObject"]) - { - id: String! - } - `, - name: 'on-interface-object', - } - const onScalar = { typeDefs: gql` scalar ScopedScalar ${directiveName}(${argName}: ["scalar"]) @@ -4417,8 +4361,6 @@ describe('composition', () => { const result = composeAsFed2Subgraphs([ onObject, - onInterface, - onInterfaceObject, onScalar, onEnum, onRootField, @@ -4429,8 +4371,6 @@ describe('composition', () => { const scopedElements = [ "ScopedObject", - "ScopedInterface", - "ScopedInterfaceObject", "ScopedScalar", "ScopedEnum", "Query.scopedRootField", diff --git a/internals-js/src/__tests__/subgraphValidation.test.ts b/internals-js/src/__tests__/subgraphValidation.test.ts index c153093bc..cb5e14d50 100644 --- a/internals-js/src/__tests__/subgraphValidation.test.ts +++ b/internals-js/src/__tests__/subgraphValidation.test.ts @@ -1667,3 +1667,62 @@ describe('@listSize', () => { ]); }); }); + +describe('authentication validations', () => { + it.each([ + { + directiveName: '@authenticated', + directiveString: '@authenticated', + }, + { + directiveName: '@requiresScopes', + directiveString: '@requiresScopes(scopes: [["scope1"]])', + }, + { + directiveName: '@policy', + directiveString: '@policy(policies: [["policy1"]])', + }, + ])( + `rejects $directiveName applications on interfaces and interface objects`, + ({ directiveName, directiveString }) => { + const doc = gql` + type Query { + i: I + o: O + } + + interface I ${directiveString} { + x: Int ${directiveString} + } + + type T implements I { + x: Int + } + + type O @key(fields: "id") @interfaceObject ${directiveString} { + id: ID! + y: Int ${directiveString} + } + `; + + expect(buildForErrors(doc, { includeAllImports: true })).toStrictEqual([ + [ + 'AUTHENTICATION_APPLIED_ON_INTERFACE', + `[S] Invalid use of ${directiveName} on field "I.x": ${directiveName} cannot be applied on interfaces, interface objects or their fields`, + ], + [ + 'AUTHENTICATION_APPLIED_ON_INTERFACE', + `[S] Invalid use of ${directiveName} on interface "I": ${directiveName} cannot be applied on interfaces, interface objects or their fields`, + ], + [ + 'AUTHENTICATION_APPLIED_ON_INTERFACE', + `[S] Invalid use of ${directiveName} on field "O.y": ${directiveName} cannot be applied on interfaces, interface objects or their fields`, + ], + [ + 'AUTHENTICATION_APPLIED_ON_INTERFACE', + `[S] Invalid use of ${directiveName} on interface object "O": ${directiveName} cannot be applied on interfaces, interface objects or their fields`, + ], + ]); + }, + ); +}); diff --git a/internals-js/src/error.ts b/internals-js/src/error.ts index 3f96dccbc..00fa286bc 100644 --- a/internals-js/src/error.ts +++ b/internals-js/src/error.ts @@ -633,6 +633,12 @@ const MAX_VALIDATION_SUBGRAPH_PATHS_EXCEEDED = makeCodeDefinition( { addedIn: '2.8.0' }, ); +const AUTHENTICATION_APPLIED_ON_INTERFACE = makeCodeDefinition( + 'AUTHENTICATION_APPLIED_ON_INTERFACE', + 'The @authenticated, @requiresScopes and @policy directive cannot be applied on interface, interface object or their fields.', + { addedIn: '2.9.4' }, +); + export const ERROR_CATEGORIES = { DIRECTIVE_FIELDS_MISSING_EXTERNAL, DIRECTIVE_UNSUPPORTED_ON_INTERFACE, @@ -734,6 +740,7 @@ export const ERRORS = { LIST_SIZE_INVALID_SIZED_FIELD, LIST_SIZE_INVALID_SLICING_ARGUMENT, MAX_VALIDATION_SUBGRAPH_PATHS_EXCEEDED, + AUTHENTICATION_APPLIED_ON_INTERFACE, }; const codeDefByCode = Object.values(ERRORS).reduce((obj: {[code: string]: ErrorCodeDefinition}, codeDef: ErrorCodeDefinition) => { obj[codeDef.code] = codeDef; return obj; }, {}); diff --git a/internals-js/src/federation.ts b/internals-js/src/federation.ts index cfeb98467..9998bc6a9 100644 --- a/internals-js/src/federation.ts +++ b/internals-js/src/federation.ts @@ -37,7 +37,7 @@ import { isWrapperType, possibleRuntimeTypes, isIntType, - Type, + Type, isFieldDefinition, } from "./definitions"; import { assert, MultiMap, printHumanReadableList, OrderedMap, mapValues, assertUnreachable } from "./utils"; import { SDLValidationRule } from "graphql/validation/ValidationContext"; @@ -1838,6 +1838,9 @@ export class FederationBlueprint extends SchemaBlueprint { validateSizedFieldsAreValidLists(application, parent, errorCollector); } + // Validate @authenticated, @requireScopes and @policy + validateNoAuthenticationOnInterfaces(metadata, errorCollector); + return errorCollector; } @@ -2889,3 +2892,39 @@ function withoutNonExternalLeafFields(selectionSet: SelectionSet): SelectionSet return undefined; }); } + +function validateNoAuthenticationOnInterfaces(metadata: FederationMetadata, errorCollector: GraphQLError[]) { + const authenticatedDirective = metadata.authenticatedDirective(); + const requiresScopesDirective = metadata.requiresScopesDirective(); + const policyDirective = metadata.policyDirective(); + [authenticatedDirective, requiresScopesDirective, policyDirective].forEach((directive) => { + for (const application of directive.applications()) { + const element = application.parent; + function isAppliedOnInterface(type: Type) { + return isInterfaceType(type) || isInterfaceObjectType(baseType(type)); + } + function isAppliedOnInterfaceField(elem: SchemaElement) { + return isFieldDefinition(elem) && isAppliedOnInterface(elem.parent); + } + + if (isAppliedOnInterface(element) || isAppliedOnInterfaceField(element)) { + let kind = ''; + switch (element.kind) { + case 'FieldDefinition': + kind = 'field'; + break; + case 'InterfaceType': + kind = 'interface'; + break; + case 'ObjectType': + kind = 'interface object'; + break; + } + errorCollector.push(ERRORS.AUTHENTICATION_APPLIED_ON_INTERFACE.err( + `Invalid use of @${directive.name} on ${kind} "${element.coordinate}": @${directive.name} cannot be applied on interfaces, interface objects or their fields`, + {nodes: sourceASTs(application, element.parent)}, + )); + } + } + }); +} diff --git a/internals-js/src/specs/authenticatedSpec.ts b/internals-js/src/specs/authenticatedSpec.ts index 3cc62d114..b6efe6001 100644 --- a/internals-js/src/specs/authenticatedSpec.ts +++ b/internals-js/src/specs/authenticatedSpec.ts @@ -8,6 +8,7 @@ import { } from "./coreSpec"; import { createDirectiveSpecification } from "../directiveAndTypeSpecification"; import { registerKnownFeature } from "../knownCoreFeatures"; +import {DirectiveDefinition, Schema} from "../definitions"; export class AuthenticatedSpecDefinition extends FeatureDefinition { public static readonly directiveName = "authenticated"; @@ -37,6 +38,10 @@ export class AuthenticatedSpecDefinition extends FeatureDefinition { })); } + authenticatedDirective(schema: Schema): DirectiveDefinition | undefined { + return this.directive(schema, AuthenticatedSpecDefinition.directiveName); + } + get defaultCorePurpose(): CorePurpose { return 'SECURITY'; } diff --git a/internals-js/src/specs/policySpec.ts b/internals-js/src/specs/policySpec.ts index 112c77f78..5dff405c3 100644 --- a/internals-js/src/specs/policySpec.ts +++ b/internals-js/src/specs/policySpec.ts @@ -6,7 +6,7 @@ import { FeatureUrl, FeatureVersion, } from "./coreSpec"; -import { ListType, NonNullType } from "../definitions"; +import {DirectiveDefinition, ListType, NonNullType, Schema} from "../definitions"; import { createDirectiveSpecification, createScalarTypeSpecification } from "../directiveAndTypeSpecification"; import { registerKnownFeature } from "../knownCoreFeatures"; import { ARGUMENT_COMPOSITION_STRATEGIES } from "../argumentCompositionStrategies"; @@ -56,6 +56,10 @@ export class PolicySpecDefinition extends FeatureDefinition { })); } + policyDirective(schema: Schema): DirectiveDefinition | undefined { + return this.directive(schema, PolicySpecDefinition.directiveName); + } + get defaultCorePurpose(): CorePurpose { return 'SECURITY'; } diff --git a/internals-js/src/specs/requiresScopesSpec.ts b/internals-js/src/specs/requiresScopesSpec.ts index cc26ef1c0..50e0d3365 100644 --- a/internals-js/src/specs/requiresScopesSpec.ts +++ b/internals-js/src/specs/requiresScopesSpec.ts @@ -6,7 +6,7 @@ import { FeatureUrl, FeatureVersion, } from "./coreSpec"; -import { ListType, NonNullType } from "../definitions"; +import {DirectiveDefinition, ListType, NonNullType, Schema} from "../definitions"; import { createDirectiveSpecification, createScalarTypeSpecification } from "../directiveAndTypeSpecification"; import { registerKnownFeature } from "../knownCoreFeatures"; import { ARGUMENT_COMPOSITION_STRATEGIES } from "../argumentCompositionStrategies"; @@ -57,6 +57,10 @@ export class RequiresScopesSpecDefinition extends FeatureDefinition { })); } + requiresScopesDirective(schema: Schema): DirectiveDefinition | undefined { + return this.directive(schema, RequiresScopesSpecDefinition.directiveName); + } + get defaultCorePurpose(): CorePurpose { return 'SECURITY'; } From 7730c03e128be6754b9e40c086d5cb5c4685ac66 Mon Sep 17 00:00:00 2001 From: dariuszkuc <9501705+dariuszkuc@users.noreply.github.com> Date: Wed, 15 Oct 2025 20:36:04 -0500 Subject: [PATCH 2/5] fix: transitive auth requirements on @requires and @fromContext Adds new `postMergeValidation` check to ensure that all fields that depends on data from other parts of the supergraph through `@requires` and/or `@fromContext` directives explicitly specify matching `@authenticated`, `@requiresScopes` and/or `@policy` auth requirements, e.g. ```graphql type T @key(fields: "id") { id: ID! extra: String @external # we need explicit @authenticated as it is needed to access extra requiresExtra: String @requires(fields: "extra") @authenticated } type T @key(fields: "id") { id: ID! extra: String @authenticated } ``` --- .changeset/eleven-maps-look.md | 22 + .../src/__tests__/compose.auth.test.ts | 713 ++++++++++++++++++ composition-js/src/merging/merge.ts | 362 +++++++++ internals-js/src/error.ts | 7 + internals-js/src/specs/policySpec.ts | 2 +- internals-js/src/specs/requiresScopesSpec.ts | 2 +- 6 files changed, 1106 insertions(+), 2 deletions(-) create mode 100644 .changeset/eleven-maps-look.md create mode 100644 composition-js/src/__tests__/compose.auth.test.ts diff --git a/.changeset/eleven-maps-look.md b/.changeset/eleven-maps-look.md new file mode 100644 index 000000000..e4243095d --- /dev/null +++ b/.changeset/eleven-maps-look.md @@ -0,0 +1,22 @@ +--- +"@apollo/composition": patch +"@apollo/federation-internals": patch +--- + +Fix transitive auth requirements on `@requires` and `@fromcontext` + +Adds new `postMergeValidation` check to ensure that all fields that depends on data from other parts of the supergraph through `@requires` and/or `@fromContext` directives explicitly specify matching `@authenticated`, `@requiresScopes` and/or `@policy` auth requirements, e.g. + +```graphql +type T @key(fields: "id") { + id: ID! + extra: String @external + # we need explicit `@authenticated` as it is needed to access extra + requiresExtra: String @requires(fields: "extra") @authenticated +} + +type T @key(fields: "id") { + id: ID! + extra: String @authenticated +} +``` \ No newline at end of file diff --git a/composition-js/src/__tests__/compose.auth.test.ts b/composition-js/src/__tests__/compose.auth.test.ts new file mode 100644 index 000000000..953b3c9c0 --- /dev/null +++ b/composition-js/src/__tests__/compose.auth.test.ts @@ -0,0 +1,713 @@ +import gql from 'graphql-tag'; +import { + assertCompositionSuccess, + composeAsFed2Subgraphs, +} from "./testHelper"; + +describe('authorization tests', () => { + describe("@requires", () => { + it('works with explicit auth', () => { + const subgraph1 = { + name: 'Subgraph1', + url: 'https://Subgraph1', + typeDefs: gql` + type Query { + t: T + } + + type T @key(fields: "id") { + id: ID + extra: String @external + requiresExtra: String @requires(fields: "extra") @authenticated + } + ` + } + + const subgraph2 = { + name: 'Subgraph2', + url: 'https://Subgraph2', + typeDefs: gql` + type T @key(fields: "id") { + id: ID + extra: String @authenticated + } + ` + } + + const result = composeAsFed2Subgraphs([subgraph1, subgraph2]); + assertCompositionSuccess(result); + }) + + it('works with auth on the type', () => { + const subgraph1 = { + name: 'Subgraph1', + url: 'https://Subgraph1', + typeDefs: gql` + type Query { + t: T + } + + type T @key(fields: "id") @policy(policies: [["P1"]]) { + id: ID + extra: String @external + requiresExtra: String @requires(fields: "extra") + } + ` + } + + const subgraph2 = { + name: 'Subgraph2', + url: 'https://Subgraph2', + typeDefs: gql` + type T @key(fields: "id") { + id: ID + extra: String @policy(policies: [["P1"]]) + } + ` + } + + const result = composeAsFed2Subgraphs([subgraph1, subgraph2]); + assertCompositionSuccess(result); + }) + + it('works with valid subset of auth', () => { + const subgraph1 = { + name: 'Subgraph1', + url: 'https://Subgraph1', + typeDefs: gql` + type Query { + t: T + } + + type T @key(fields: "id") { + id: ID + extra: String @external + requiresExtra: String @requires(fields: "extra") @requiresScopes(scopes: [["S2", "S1"]]) + } + ` + } + + const subgraph2 = { + name: 'Subgraph2', + url: 'https://Subgraph2', + typeDefs: gql` + type T @key(fields: "id") { + id: ID + extra: String @requiresScopes(scopes: [["S1", "S2"], ["S3"]]) + } + ` + } + + const result = composeAsFed2Subgraphs([subgraph1, subgraph2]); + assertCompositionSuccess(result); + }) + + it('works with auth on nested selection', () => { + const subgraph1 = { + name: 'Subgraph1', + url: 'https://Subgraph1', + typeDefs: gql` + type Query { + t: T + } + + type T @key(fields: "id") @authenticated { + id: ID + extra: I @external + requiresExtra: String @requires(fields: "extra { i ... on I1 { i1 } ... on I2 { i2 } }") + @requiresScopes(scopes: [["S1"]["S2"]]) @policy(policies: [["P1"]]) + } + + interface I { + i: String + } + + type I1 implements I @external { + i: String + i1: String + } + + type I2 implements I @external { + i: String + i2: Int + } + ` + } + + const subgraph2 = { + name: 'Subgraph2', + url: 'https://Subgraph2', + typeDefs: gql` + type T @key(fields: "id") { + id: ID + extra: I @authenticated + } + + interface I { + i: String + } + + type I1 implements I { + i: String @requiresScopes(scopes: [["S1"]]) + i1: String @requiresScopes(scopes: [["S2"]]) + } + + type I2 implements I { + i: String + i2: Int @policy(policies: [["P1"]]) + } + ` + } + + const result = composeAsFed2Subgraphs([subgraph1, subgraph2]); + assertCompositionSuccess(result); + }) + + it('does not work when missing auth', () => { + const subgraph1 = { + name: 'Subgraph1', + url: 'https://Subgraph1', + typeDefs: gql` + type Query { + t: T + } + + type T @key(fields: "id") { + id: ID + extra: String @external + requiresExtra: String @requires(fields: "extra") + } + ` + } + + const subgraph2 = { + name: 'Subgraph2', + url: 'https://Subgraph2', + typeDefs: gql` + type T @key(fields: "id") { + id: ID + extra: String @authenticated + } + ` + } + + const result = composeAsFed2Subgraphs([subgraph1, subgraph2]); + expect(result.schema).toBeUndefined(); + expect(result.errors?.length).toBe(1); + expect(result.errors?.[0].message).toBe( + '[Subgraph1] Field "T.requiresExtra" does not specify necessary @authenticated, @requiresScopes and/or ' + + '@policy auth requirements to access the transitive field T.extra data from @requires selection set.' + ); + }) + + it('does not work with subset of auth', () => { + const subgraph1 = { + name: 'Subgraph1', + url: 'https://Subgraph1', + typeDefs: gql` + type Query { + t: T + } + + type T @key(fields: "id") { + id: ID + extra: String @external + requiresExtra: String @requires(fields: "extra") @requiresScopes(scopes: [["S1"]]) + } + ` + } + + const subgraph2 = { + name: 'Subgraph2', + url: 'https://Subgraph2', + typeDefs: gql` + type T @key(fields: "id") { + id: ID + extra: String @requiresScopes(scopes: [["S1", "S2"]]) + } + ` + } + + const result = composeAsFed2Subgraphs([subgraph1, subgraph2]); + expect(result.schema).toBeUndefined(); + expect(result.errors?.length).toBe(1); + expect(result.errors?.[0].message).toBe( + '[Subgraph1] Field "T.requiresExtra" does not specify necessary @authenticated, @requiresScopes and/or @policy ' + + 'auth requirements to access the transitive field T.extra data from @requires selection set.' + ); + }) + + it('does not work when missing auth on a nested selection', () => { + const subgraph1 = { + name: 'Subgraph1', + url: 'https://Subgraph1', + typeDefs: gql` + type Query { + t: T + } + + type T @key(fields: "id") { + id: ID + extra: I @external + requiresExtra: String @requires(fields: "extra { i ... on I1 { i1 } ... on I2 { i2 } }") + } + + interface I { + i: String + } + + type I1 implements I @external { + i: String + i1: String + } + + type I2 implements I @external { + i: String + i2: Int + } + ` + } + + const subgraph2 = { + name: 'Subgraph2', + url: 'https://Subgraph2', + typeDefs: gql` + type T @key(fields: "id") { + id: ID + extra: I + } + + interface I { + i: String + } + + type I1 implements I { + i: String + i1: String + } + + type I2 implements I { + i: String + i2: Int @policy(policies: [["P1"]]) + } + ` + } + + const result = composeAsFed2Subgraphs([subgraph1, subgraph2]); + expect(result.schema).toBeUndefined(); + expect(result.errors?.length).toBe(1); + expect(result.errors?.[0].message).toBe( + '[Subgraph1] Field "T.requiresExtra" does not specify necessary @authenticated, @requiresScopes and/or @policy ' + + 'auth requirements to access the transitive field I2.i2 data from @requires selection set.' + ); + }) + + it('does not work when missing explicit auth on an interface field selection', () => { + const subgraph1 = { + name: 'Subgraph1', + url: 'https://Subgraph1', + typeDefs: gql` + type Query { + t: T + } + + type T @key(fields: "id") { + id: ID + extra: I @external + requiresExtra: String @requires(fields: "extra { i }") + } + + interface I { + i: String + } + + type I1 implements I @external { + i: String + } + ` + } + + const subgraph2 = { + name: 'Subgraph2', + url: 'https://Subgraph2', + typeDefs: gql` + type T @key(fields: "id") { + id: ID + extra: I + } + + interface I { + i: String + } + + type I1 implements I { + i: String @requiresScopes(scopes: [["S1"]]) + } + ` + } + + const result = composeAsFed2Subgraphs([subgraph1, subgraph2]); + expect(result.schema).toBeUndefined(); + expect(result.errors?.length).toBe(1); + expect(result.errors?.[0].message).toBe( + '[Subgraph1] Field "T.requiresExtra" does not specify necessary @authenticated, @requiresScopes and/or @policy ' + + 'auth requirements to access the transitive field I1.i data from @requires selection set.' + ); + }) + + it('does not work when missing inherited auth on a interface field selection', () => { + const subgraph1 = { + name: 'Subgraph1', + url: 'https://Subgraph1', + typeDefs: gql` + type Query { + t: T + } + + type T @key(fields: "id") { + id: ID + extra: I @external + requiresExtra: String @requires(fields: "extra { i }") + } + + interface I { + i: String + } + + type I1 implements I @external { + i: String + } + ` + } + + const subgraph2 = { + name: 'Subgraph2', + url: 'https://Subgraph2', + typeDefs: gql` + type T @key(fields: "id") { + id: ID + extra: I + } + + interface I { + i: String + } + + type I1 implements I @authenticated { + i: String + } + ` + } + + const result = composeAsFed2Subgraphs([subgraph1, subgraph2]); + expect(result.schema).toBeUndefined(); + expect(result.errors?.length).toBe(1); + expect(result.errors?.[0].message).toBe( + '[Subgraph1] Field "T.requiresExtra" does not specify necessary @authenticated, @requiresScopes and/or @policy ' + + 'auth requirements to access the transitive interface I data from @requires selection set.' + ); + }) + + it('does not work when missing auth on type condition in a field selection', () => { + const subgraph1 = { + name: 'Subgraph1', + url: 'https://Subgraph1', + typeDefs: gql` + type Query { + t: T + } + + type T @key(fields: "id") { + id: ID + extra: I @external + requiresExtra: String @requires(fields: "extra { ... on I1 { i1 } ... on I2 { i2 }}") + } + + interface I { + i: String + } + + type I1 implements I @external { + i: String + i1: Int + } + + type I2 implements I @external { + i: String + i2: String + } + ` + } + + const subgraph2 = { + name: 'Subgraph2', + url: 'https://Subgraph2', + typeDefs: gql` + type T @key(fields: "id") { + id: ID + extra: I + } + + interface I { + i: String + } + + type I1 implements I @requiresScopes(scopes: [["S1"]]) { + i: String + i1: Int + } + + type I2 implements I { + i: String + i2: String + } + ` + } + + const result = composeAsFed2Subgraphs([subgraph1, subgraph2]); + expect(result.schema).toBeUndefined(); + expect(result.errors?.length).toBe(1); + expect(result.errors?.[0].message).toBe( + '[Subgraph1] Field "T.requiresExtra" does not specify necessary @authenticated, @requiresScopes and/or @policy' + + ' auth requirements to access the transitive data in inline fragment type condition I1 from @requires selection set.' + ); + }) + }); + describe("@context", () => { + it('works with explicit auth', () => { + const subgraph1 = { + name: 'Subgraph1', + url: 'https://Subgraph1', + typeDefs: gql` + type Query { + t: T! + } + + type T @key(fields: "id") @context(name: "context") { + id: ID! + u: U! + prop: String! @authenticated + } + + type U @key(fields: "id") { + id: ID! + field(a: String @fromContext(field: "$context { prop }")): Int! @authenticated + } + ` + } + + const subgraph2 = { + name: 'Subgraph2', + url: 'https://Subgraph2', + typeDefs: gql` + type Query { + a: Int! + } + + type U @key(fields: "id") { + id: ID! + } + ` + } + + const result = composeAsFed2Subgraphs([subgraph1, subgraph2]); + assertCompositionSuccess(result); + }) + + it('works with explicit auth and multiple contexts', () => { + const subgraph1 = { + name: "Subgraph1", + utl: "https://Subgraph1", + typeDefs: gql` + type Query { + foo: Foo! + bar: Bar! + } + + type Foo @key(fields: "id") @context(name: "context") { + id: ID! + u: U! + prop: String! @requiresScopes(scopes: [["S1"]]) + } + + type Bar @key(fields: "id") @context(name: "context") { + id: ID! + u: U! + prop: String! @requiresScopes(scopes: [["S2"]]) + } + + type U @key(fields: "id") { + id: ID! + field(a: String @fromContext(field: "$context { prop }")): Int! @requiresScopes(scopes: [["S1"], ["S2"]]) + } + `, + }; + + const subgraph2 = { + name: "Subgraph2", + utl: "https://Subgraph2", + typeDefs: gql` + type Query { + a: Int! + } + + type U @key(fields: "id") { + id: ID! + } + `, + }; + + const result = composeAsFed2Subgraphs([subgraph1, subgraph2]); + assertCompositionSuccess(result); + }) + + it('works with explicit auth and multiple contexts using type conditions', () => { + const subgraph1 = { + name: "Subgraph1", + utl: "https://Subgraph1", + typeDefs: gql` + type Query { + foo: Foo! + bar: Bar! + } + + type Foo @key(fields: "id") @context(name: "context") { + id: ID! + u: U! + prop: String! @requiresScopes(scopes: [["S1"]]) + } + + type Bar @key(fields: "id") @context(name: "context") { + id: ID! + u: U! + prop2: String! @policy(policies: [["P1"]]) + } + + type U @key(fields: "id") { + id: ID! + field( + a: String + @fromContext( + field: "$context ... on Foo { prop } ... on Bar { prop2 }" + ) + ): Int! @requiresScopes(scopes: [["S1"]]) @policy(policies: [["P1"]]) + } + `, + }; + + const subgraph2 = { + name: "Subgraph2", + utl: "https://Subgraph2", + typeDefs: gql` + type Query { + a: Int! + } + + type U @key(fields: "id") { + id: ID! + } + `, + }; + + const result = composeAsFed2Subgraphs([subgraph1, subgraph2]); + assertCompositionSuccess(result); + }) + + it('does not work with missing auth', () => { + const subgraph1 = { + name: 'Subgraph1', + url: 'https://Subgraph1', + typeDefs: gql` + type Query { + t: T! + } + + type T @key(fields: "id") @context(name: "context") { + id: ID! + u: U! + prop: String! @authenticated + } + + type U @key(fields: "id") { + id: ID! + field(a: String @fromContext(field: "$context { prop }")): Int! + } + ` + } + + const subgraph2 = { + name: 'Subgraph2', + url: 'https://Subgraph2', + typeDefs: gql` + type Query { + a: Int! + } + + type U @key(fields: "id") { + id: ID! + } + ` + } + + const result = composeAsFed2Subgraphs([subgraph1, subgraph2]); + expect(result.schema).toBeUndefined(); + expect(result.errors?.length).toBe(1); + expect(result.errors?.[0].message).toBe( + '[Subgraph1] Field "U.field" does not specify necessary @authenticated, @requiresScopes and/or @policy ' + + 'auth requirements to access the transitive field T.prop data from @fromContext selection set.' + ); + }) + + it('does not work with missing auth on one of the contexts', () => { + const subgraph1 = { + name: "Subgraph1", + utl: "https://Subgraph1", + typeDefs: gql` + type Query { + foo: Foo! + bar: Bar! + } + + type Foo @key(fields: "id") @context(name: "context") @authenticated { + id: ID! + u: U! + prop: String! + } + + type Bar @key(fields: "id") @context(name: "context") { + id: ID! + u: U! + prop: String! + } + + type U @key(fields: "id") { + id: ID! + field(a: String @fromContext(field: "$context { prop }")): Int! + } + `, + }; + + const subgraph2 = { + name: "Subgraph2", + utl: "https://Subgraph2", + typeDefs: gql` + type Query { + a: Int! + } + + type U @key(fields: "id") { + id: ID! + } + `, + }; + + const result = composeAsFed2Subgraphs([subgraph1, subgraph2]); + expect(result.schema).toBeUndefined(); + expect(result.errors?.length).toBe(1); + expect(result.errors?.[0].message).toBe( + '[Subgraph1] Field "U.field" does not specify necessary @authenticated, @requiresScopes and/or @policy auth ' + + 'requirements to access the transitive data in context Subgraph1__context from @fromContext selection set.' + ); + }) + }); +}); diff --git a/composition-js/src/merging/merge.ts b/composition-js/src/merging/merge.ts index d6f7d4915..2c383bec1 100644 --- a/composition-js/src/merging/merge.ts +++ b/composition-js/src/merging/merge.ts @@ -86,6 +86,19 @@ import { inaccessibleIdentity, FeatureDefinitions, CONNECT_VERSIONS, + AuthenticatedSpecDefinition, + RequiresScopesSpecDefinition, + PolicySpecDefinition, + ScalarType, + AUTHENTICATED_VERSIONS, + REQUIRES_SCOPES_VERSIONS, + POLICY_VERSIONS, + parseSelectionSet, + FieldSelection, + SelectionSet, + JoinFieldDirectiveArguments, + ContextSpecDefinition, + CONTEXT_VERSIONS, } from "@apollo/federation-internals"; import { ASTNode, GraphQLError, DirectiveLocation } from "graphql"; import { @@ -382,6 +395,8 @@ class Merger { private schemaToImportNameToFeatureUrl = new Map>(); private fieldsWithFromContext: Set; private fieldsWithOverride: Set; + private fieldsWithRequires: Set; + private coordinatesWithAuth: Set; constructor(readonly subgraphs: Subgraphs, readonly options: CompositionOptions) { this.latestFedVersionUsed = this.getLatestFederationVersionUsed(); @@ -389,6 +404,8 @@ class Merger { this.linkSpec = LINK_VERSIONS.getMinimumRequiredVersion(this.latestFedVersionUsed); this.fieldsWithFromContext = this.getFieldsWithFromContextDirective(); this.fieldsWithOverride = this.getFieldsWithOverrideDirective(); + this.fieldsWithRequires = this.getFieldsWithRequiresDirective(); + this.coordinatesWithAuth = this.getCoordinatesWithAuth(); this.names = subgraphs.names(); this.composeDirectiveManager = new ComposeDirectiveManager( @@ -3417,6 +3434,21 @@ class Merger { } } } + + // auth verification on the supergraph + // need to verify usage of @requires on fields that require authorization + if (this.coordinatesWithAuth.size > 0) { + const authValidator = new AuthValidator(this.merged, this.joinSpec, this.subgraphNamesToJoinSpecName); + for (const coordinate of this.fieldsWithRequires) { + const errors = authValidator.validateRequiresFieldSet(coordinate); + this.errors.push(...errors); + } + + for (const coordinate of this.fieldsWithFromContext) { + const errors = authValidator.validateFromContext(coordinate); + this.errors.push(...errors); + } + } } private updateInaccessibleErrorsWithLinkToSubgraphs( @@ -3586,6 +3618,45 @@ class Merger { ); } + private getFieldsWithRequiresDirective(): Set { + return this.getFieldsWithAppliedDirective( + (subgraph: Subgraph) => subgraph.metadata().requiresDirective(), + (application: Directive>) => { + const field = application.parent; + assert(isFieldDefinition(field), () => `Expected ${application.parent} to be a field`); + return field; + } + ); + } + + private getCoordinatesWithAuth(): Set { + const coordinates = new Set(); + for (const subgraph of this.subgraphs) { + const authenticatedDirective = subgraph.metadata().authenticatedDirective(); + const requiresScopesDirective = subgraph.metadata().requiresScopesDirective(); + const policyDirective = subgraph.metadata().policyDirective(); + + for (const authDirective of [authenticatedDirective, requiresScopesDirective, policyDirective]) { + if (isFederationDirectiveDefinedInSchema(authDirective)) { + for (const application of authDirective.applications()) { + const parent = application.parent; + if (parent instanceof FieldDefinition) { + coordinates.add(parent.coordinate); + } else if (parent instanceof ObjectType) { + coordinates.add(parent.coordinate) + } else if (parent instanceof ScalarType) { + coordinates.add(parent.coordinate); + } else if (parent instanceof EnumType) { + coordinates.add(parent.coordinate); + } + } + } + } + } + + return coordinates; + } + private getFieldsWithAppliedDirective( getDirective: (subgraph: Subgraph) => Post20FederationDirectiveDefinition, getField: (application: Directive>) => FieldDefinition, @@ -3606,3 +3677,294 @@ class Merger { return fields; } } + +class AuthValidator { + schema: Schema; + joinSpecNamesToSubgraphNames: Map; + joinFieldDirective: DirectiveDefinition; + authenticatedDirective?: DirectiveDefinition; + requiresScopesDirective?: DirectiveDefinition<{ scopes: string[][] }>; + policyDirective?: DirectiveDefinition<{ policies: string[][] }>; + contexts: Map; + + constructor(schema: Schema, joinSpec: JoinSpecDefinition, subgraphNamesToJoinSpecName: Map) { + this.schema = schema; + this.joinFieldDirective = joinSpec.fieldDirective(this.schema); + this.joinSpecNamesToSubgraphNames = new Map(Array.from( + subgraphNamesToJoinSpecName, entry => entry.reverse() as [string, string]) + ); + + const authenticatedFeature = this.schema.coreFeatures?.getByIdentity(AuthenticatedSpecDefinition.identity); + const authenticatedSpec = authenticatedFeature && AUTHENTICATED_VERSIONS.find(authenticatedFeature.url.version); + this.authenticatedDirective = authenticatedSpec?.authenticatedDirective(this.schema); + + const requiresScopesFeature = this.schema.coreFeatures?.getByIdentity(RequiresScopesSpecDefinition.identity); + const requiresScopesSpec = requiresScopesFeature && REQUIRES_SCOPES_VERSIONS.find(requiresScopesFeature.url.version); + this.requiresScopesDirective = requiresScopesSpec?.requiresScopesDirective(this.schema); + + const policyFeature = this.schema.coreFeatures?.getByIdentity(PolicySpecDefinition.identity); + const policySpec = policyFeature && POLICY_VERSIONS.find(policyFeature.url.version); + this.policyDirective = policySpec?.policyDirective(this.schema); + + const contextFeature = this.schema.coreFeatures?.getByIdentity(ContextSpecDefinition.identity); + const contextSpec = contextFeature && CONTEXT_VERSIONS.find(contextFeature.url.version); + const contextDirective = contextSpec?.contextDirective(this.schema); + + const contextApplications = Array.from(contextDirective?.applications() ?? []); + this.contexts = new Map(); + contextApplications.forEach((context) => { + const contextName = context.arguments().name; + const contextType = context.parent as CompositeType; + const types = this.contexts.get(contextName) ?? []; + types.push(contextType.name); + this.contexts.set(contextName, types); + }); + } + + validateRequiresFieldSet(coordinate: string): GraphQLError[] { + const fieldCoordinate = coordinate.split('.'); + assert(fieldCoordinate && fieldCoordinate.length == 2,'Valid coordinate for field with @requires'); + const type = this.schema.type(fieldCoordinate[0]); + assert(type instanceof ObjectType, 'Type exists in the schema'); + const field = type.field(fieldCoordinate[1]); + assert(field instanceof FieldDefinition, 'Field exists in the schema'); + + const authRequirementOnRequires = new AuthRequirements(coordinate, '@requires'); + authRequirementOnRequires.type = this.authRequirementsOnElement(type); + authRequirementOnRequires.field = this.authRequirementsOnElement(field); + + const errors: GraphQLError[] = [] + const joinDirectivesOnRequires = field.appliedDirectivesOf(this.joinFieldDirective); + for (const joinDirectiveOnRequires of joinDirectivesOnRequires) { + const requiresFieldSet = joinDirectiveOnRequires.arguments().requires!; + const requiresSelectionSet = parseSelectionSet({parentType: type, source: requiresFieldSet}); + try { + this.verifyAuthRequirementsOnSelectionSet(authRequirementOnRequires, requiresSelectionSet); + } catch (e) { + if (!(e instanceof GraphQLError)) { + throw e; + } + // target subgraph info should always be provided but just in case + const enumSubgraphValue = joinDirectiveOnRequires.arguments().graph; + const subgraph = enumSubgraphValue ? this.joinSpecNamesToSubgraphNames.get(enumSubgraphValue) : undefined; + if (subgraph) { + errors.push(addSubgraphToError(e, subgraph)); + } else { + errors.push(e); + } + } + } + + return errors; + } + + validateFromContext(coordinate: string): GraphQLError[] { + const fieldCoordinate = coordinate.split('.'); + assert(fieldCoordinate && fieldCoordinate.length == 2,'Valid coordinate for field with @fromContext'); + const type = this.schema.type(fieldCoordinate[0]); + assert(type instanceof ObjectType, 'Type exists in the schema'); + const field = type.field(fieldCoordinate[1]); + assert(field instanceof FieldDefinition, 'Field exists in the schema'); + + const authRequirementOnContext = new AuthRequirements(coordinate, '@fromContext'); + authRequirementOnContext.type = this.authRequirementsOnElement(type); + authRequirementOnContext.field = this.authRequirementsOnElement(field); + + const errors: GraphQLError[] = [] + const joinDirectivesOnFromContext = field.appliedDirectivesOf(this.joinFieldDirective); + for (const joinDirectiveOnFromContext of joinDirectivesOnFromContext) { + const contexts = joinDirectiveOnFromContext.arguments().contextArguments!; + for (const context of contexts) { + const name = context.context; + const contextSelection = context.selection; + + const targetTypeNames = this.contexts.get(name); + assert(targetTypeNames, 'Contexts exists'); + for (const targetTypeName of targetTypeNames) { + // we need to verify against all possible contexts + const targetType = this.schema.type(targetTypeName) as CompositeType; + assert(targetType, 'Context references valid type in the schema'); + try { + const requirementsOnContextType = this.authRequirementsOnElement(targetType); + if (!authRequirementOnContext.satisfies(requirementsOnContextType)) { + const msg = `Field "${field.coordinate}" does not specify necessary @authenticated, @requiresScopes ` + + `and/or @policy auth requirements to access the transitive data in context ${name} from @fromContext selection set.`; + throw ERRORS.MISSING_TRANSITIVE_AUTH_REQUIREMENTS.err(msg); + } + + const contextSelectionSet = parseSelectionSet({parentType: targetType, source: contextSelection}); + this.verifyAuthRequirementsOnSelectionSet(authRequirementOnContext, contextSelectionSet); + } catch (e) { + if (!(e instanceof GraphQLError)) { + throw e; + } + // target subgraph info should always be provided but just in case + const enumSubgraphValue = joinDirectiveOnFromContext.arguments().graph; + const subgraph = enumSubgraphValue ? this.joinSpecNamesToSubgraphNames.get(enumSubgraphValue) : undefined; + if (subgraph) { + errors.push(addSubgraphToError(e, subgraph)); + } else { + errors.push(e); + } + } + } + } + } + return errors; + } + + authRequirementsOnElement(element: NamedType | FieldDefinition): AuthRequirementsOnElement | undefined { + const requirements = new AuthRequirementsOnElement(); + if (this.authenticatedDirective) { + const appliedDirective = element.appliedDirectivesOf(this.authenticatedDirective)?.[0]; + if (appliedDirective) { + requirements.isAuthenticated = true; + } + } + + if (this.requiresScopesDirective) { + const appliedDirective = element.appliedDirectivesOf(this.requiresScopesDirective)?.[0]; + if (appliedDirective) { + const { scopes } = appliedDirective.arguments(); + requirements.scopes = scopes; + } + } + + if (this.policyDirective) { + const appliedDirective = element.appliedDirectivesOf(this.policyDirective)?.[0]; + if (appliedDirective) { + const { policies } = appliedDirective.arguments(); + requirements.policies = policies; + } + } + + if (requirements.isAuthenticated || requirements.scopes || requirements.policies) { + return requirements; + } else { + return; + } + } + + verifyAuthRequirementsOnSelectionSet(authRequirements: AuthRequirements, selectionSet: SelectionSet) { + const parentType = this.schema.type(selectionSet.parentType.name); + for (const selection of selectionSet.selections()) { + if (selection instanceof FieldSelection) { + if (parentType instanceof InterfaceType) { + // if we are referencing an interface field we need to check against all implementations + for (const implType of parentType.possibleRuntimeTypes()) { + const requirementsOnImplType = this.authRequirementsOnElement(implType); + if (!authRequirements.satisfies(requirementsOnImplType)) { + const msg = `Field "${authRequirements.fieldCoordinate}" does not specify necessary @authenticated, @requiresScopes ` + + `and/or @policy auth requirements to access the transitive interface ${parentType} data from ${authRequirements.directive} selection set.`; + throw ERRORS.MISSING_TRANSITIVE_AUTH_REQUIREMENTS.err(msg); + } + // re-check the implementation auth condition + this.verifyAuthOnFieldSelection(implType, selection, authRequirements); + } + } + + if (parentType instanceof ObjectType) { + this.verifyAuthOnFieldSelection(parentType, selection, authRequirements); + } + + if (selection.selectionSet) { + this.verifyAuthRequirementsOnSelectionSet(authRequirements, selection.selectionSet); + } + } else { + const condition = selection.element.typeCondition; + if (condition) { + const requirementsOnCondition = this.authRequirementsOnElement(condition); + if (!authRequirements.satisfies(requirementsOnCondition)) { + const msg = `Field "${authRequirements.fieldCoordinate}" does not specify necessary @authenticated, @requiresScopes ` + + `and/or @policy auth requirements to access the transitive data in inline fragment type condition ${condition} from ${authRequirements.directive} selection set.`; + throw ERRORS.MISSING_TRANSITIVE_AUTH_REQUIREMENTS.err(msg); + } + } + this.verifyAuthRequirementsOnSelectionSet(authRequirements, selection.selectionSet); + } + } + } + + private verifyAuthOnFieldSelection(parentType: ObjectType, selection: FieldSelection, authRequirements: AuthRequirements) { + const field = parentType.field(selection.element.name); + assert(field, 'field exists in the schema'); + const fieldAuthReqs = this.authRequirementsOnElement(field); + const returnType = baseType(field.type!); + const fieldReturnAuthReqs = this.authRequirementsOnElement(returnType); + + if (!authRequirements.satisfies(fieldAuthReqs) || !authRequirements.satisfies(fieldReturnAuthReqs)) { + const msg = `Field "${authRequirements.fieldCoordinate}" does not specify necessary @authenticated, @requiresScopes ` + + `and/or @policy auth requirements to access the transitive field ${field.coordinate} data from ${authRequirements.directive} selection set.`; + throw ERRORS.MISSING_TRANSITIVE_AUTH_REQUIREMENTS.err(msg); + } + } +} + +class AuthRequirements { + fieldCoordinate: string; + directive: string; + type?: AuthRequirementsOnElement; + field?: AuthRequirementsOnElement; + + constructor(coordinate: string, directive: string) { + this.fieldCoordinate = coordinate; + this.directive = directive; + } + + satisfies(authOnElement: AuthRequirementsOnElement | undefined): boolean { + if (authOnElement) { + const typeSatisfiesRequirements = this.type && this.type.satisfies(authOnElement); + const fieldSatisfiesRequirements = this.field && this.field.satisfies(authOnElement); + if (!typeSatisfiesRequirements && !fieldSatisfiesRequirements) { + return false; + } + } + return true; + } +} + +class AuthRequirementsOnElement { + isAuthenticated: boolean = false; + scopes?: string[][]; + policies?: string[][]; + + satisfies(other: AuthRequirementsOnElement): boolean { + const authenticatedSatisfied = this.isAuthenticated || !other.isAuthenticated; + const scopesSatisfied = this.isSubset(this.scopes, other.scopes); + const policiesSatisfied = this.isSubset(this.policies, other.policies); + return authenticatedSatisfied && scopesSatisfied && policiesSatisfied; + } + + isSubset(first: string[][] | undefined, second: string[][] | undefined): boolean { + if (second) { + if (first) { + // outer elements follow OR rules so we just need one element to match + // inner elements follow AND rules so ALL has to match + return first.some((firstInner) => second.some((secondInner) => { + const s = new Set(secondInner); + return firstInner.length == secondInner.length && firstInner.every((elem) => s.has(elem)); + })); + } else { + // we don't specify any auth requirements but second one requires it + return false; + } + } else { + // second one does not specify any auth requirements so we are good + return true; + } + } + + toString(): string { + let result = `{ is_authenticated: ${this.isAuthenticated}`; + if (this.scopes) { + result += `, scopes: [${this.scopes.join(',')}]`; + } + + if (this.policies) { + result += `, policies: [${this.policies.join(',')}]`; + } + + result += ' }'; + return result; + } +} diff --git a/internals-js/src/error.ts b/internals-js/src/error.ts index 00fa286bc..cbb59291c 100644 --- a/internals-js/src/error.ts +++ b/internals-js/src/error.ts @@ -639,6 +639,12 @@ const AUTHENTICATION_APPLIED_ON_INTERFACE = makeCodeDefinition( { addedIn: '2.9.4' }, ); +const MISSING_TRANSITIVE_AUTH_REQUIREMENTS = makeCodeDefinition( + 'MISSING_TRANSITIVE_AUTH_REQUIREMENTS', + 'Field missing transitive @authenticated, @requiresScopes and/or @policy auth requirements needed to access dependent data.', + { addedIn: '2.9.4' }, +) + export const ERROR_CATEGORIES = { DIRECTIVE_FIELDS_MISSING_EXTERNAL, DIRECTIVE_UNSUPPORTED_ON_INTERFACE, @@ -741,6 +747,7 @@ export const ERRORS = { LIST_SIZE_INVALID_SLICING_ARGUMENT, MAX_VALIDATION_SUBGRAPH_PATHS_EXCEEDED, AUTHENTICATION_APPLIED_ON_INTERFACE, + MISSING_TRANSITIVE_AUTH_REQUIREMENTS, }; const codeDefByCode = Object.values(ERRORS).reduce((obj: {[code: string]: ErrorCodeDefinition}, codeDef: ErrorCodeDefinition) => { obj[codeDef.code] = codeDef; return obj; }, {}); diff --git a/internals-js/src/specs/policySpec.ts b/internals-js/src/specs/policySpec.ts index 5dff405c3..ae96c626c 100644 --- a/internals-js/src/specs/policySpec.ts +++ b/internals-js/src/specs/policySpec.ts @@ -56,7 +56,7 @@ export class PolicySpecDefinition extends FeatureDefinition { })); } - policyDirective(schema: Schema): DirectiveDefinition | undefined { + policyDirective(schema: Schema): DirectiveDefinition<{policies: string[][]}> | undefined { return this.directive(schema, PolicySpecDefinition.directiveName); } diff --git a/internals-js/src/specs/requiresScopesSpec.ts b/internals-js/src/specs/requiresScopesSpec.ts index 50e0d3365..9eb9d3f7c 100644 --- a/internals-js/src/specs/requiresScopesSpec.ts +++ b/internals-js/src/specs/requiresScopesSpec.ts @@ -57,7 +57,7 @@ export class RequiresScopesSpecDefinition extends FeatureDefinition { })); } - requiresScopesDirective(schema: Schema): DirectiveDefinition | undefined { + requiresScopesDirective(schema: Schema): DirectiveDefinition<{scopes: string[][]}> | undefined { return this.directive(schema, RequiresScopesSpecDefinition.directiveName); } From 2a20dc38dfc40e0b618d5cc826f18a19ddb91aff Mon Sep 17 00:00:00 2001 From: dariuszkuc <9501705+dariuszkuc@users.noreply.github.com> Date: Fri, 17 Oct 2025 11:03:14 -0500 Subject: [PATCH 3/5] 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"]]) } ``` --- .changeset/tasty-snails-invent.md | 59 +++++++++ ...e.directiveArgumentMergeStrategies.test.ts | 16 +++ composition-js/src/__tests__/compose.test.ts | 39 ++++-- .../src/argumentCompositionStrategies.ts | 116 +++++++++++++++++- internals-js/src/specs/policySpec.ts | 2 +- internals-js/src/specs/requiresScopesSpec.ts | 2 +- 6 files changed, 220 insertions(+), 14 deletions(-) create mode 100644 .changeset/tasty-snails-invent.md diff --git a/.changeset/tasty-snails-invent.md b/.changeset/tasty-snails-invent.md new file mode 100644 index 000000000..5a0ee32b7 --- /dev/null +++ b/.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"]]) +} +``` \ No newline at end of file diff --git a/composition-js/src/__tests__/compose.directiveArgumentMergeStrategies.test.ts b/composition-js/src/__tests__/compose.directiveArgumentMergeStrategies.test.ts index c83466e93..ce48b9681 100644 --- a/composition-js/src/__tests__/compose.directiveArgumentMergeStrategies.test.ts +++ b/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', diff --git a/composition-js/src/__tests__/compose.test.ts b/composition-js/src/__tests__/compose.test.ts index 13875f781..a65d14fe3 100644 --- a/composition-js/src/__tests__/compose.test.ts +++ b/composition-js/src/__tests__/compose.test.ts @@ -4421,14 +4421,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 } @@ -4437,7 +4437,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 } @@ -4450,18 +4450,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 } @@ -4470,20 +4476,33 @@ 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'], + ] ); }); @@ -4491,7 +4510,7 @@ describe('composition', () => { const a = { typeDefs: gql` type Query { - x: Int ${directiveName}(${argName}: ["a", "b"]) + x: Int ${directiveName}(${argName}: [["a"], ["b"]]) } `, name: 'a', diff --git a/internals-js/src/argumentCompositionStrategies.ts b/internals-js/src/argumentCompositionStrategies.ts index f9d60ef3f..5a9c77180 100644 --- a/internals-js/src/argumentCompositionStrategies.ts +++ b/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(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; + + 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(values: T[][][]): T[][][] { + const filtered: T[][][] = []; + const seen = new Set; + values.forEach((value) => { + value.sort(); + const key = JSON.stringify(value); + if (!seen.has(key)) { + seen.add(key); + filtered.push(value); + } + }); + return filtered; +} + +function deduplicateSubsumedValues(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 } } diff --git a/internals-js/src/specs/policySpec.ts b/internals-js/src/specs/policySpec.ts index ae96c626c..378cd4fbc 100644 --- a/internals-js/src/specs/policySpec.ts +++ b/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, diff --git a/internals-js/src/specs/requiresScopesSpec.ts b/internals-js/src/specs/requiresScopesSpec.ts index 9eb9d3f7c..6d93a9848 100644 --- a/internals-js/src/specs/requiresScopesSpec.ts +++ b/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, From d221ac04c3ee00a3c7a671d9d56e2cfa36943b49 Mon Sep 17 00:00:00 2001 From: dariuszkuc <9501705+dariuszkuc@users.noreply.github.com> Date: Wed, 22 Oct 2025 17:54:50 -0500 Subject: [PATCH 4/5] fix: automatically propagate auth requirements from type to interface in supergraph Propagate auth requirements from polymorphic types upwards to their interfaces. This ensures consistent router runtime behavior regardless of the router version. --- .changeset/chilled-baboons-whisper.md | 8 + .../src/__tests__/compose.auth.test.ts | 400 +++++++++++++++++- composition-js/src/merging/merge.ts | 102 ++++- 3 files changed, 482 insertions(+), 28 deletions(-) create mode 100644 .changeset/chilled-baboons-whisper.md diff --git a/.changeset/chilled-baboons-whisper.md b/.changeset/chilled-baboons-whisper.md new file mode 100644 index 000000000..e600459be --- /dev/null +++ b/.changeset/chilled-baboons-whisper.md @@ -0,0 +1,8 @@ +--- +"@apollo/composition": patch +"@apollo/federation-internals": patch +--- + +Automatically propagate authorization requirements from implementing type to interface in the supergraph. + +Authorization requirements now automatically propagate from implementing types to interfaces during composition. Direct auth specifications on interfaces are no longer allowed. Interface access requires satisfying ALL implementing types' requirements (`AND` rule), with these requirements included in the supergraph for backward compatibility with older routers. \ No newline at end of file diff --git a/composition-js/src/__tests__/compose.auth.test.ts b/composition-js/src/__tests__/compose.auth.test.ts index 953b3c9c0..270845735 100644 --- a/composition-js/src/__tests__/compose.auth.test.ts +++ b/composition-js/src/__tests__/compose.auth.test.ts @@ -3,6 +3,7 @@ import { assertCompositionSuccess, composeAsFed2Subgraphs, } from "./testHelper"; +import {InterfaceType} from "@apollo/federation-internals"; describe('authorization tests', () => { describe("@requires", () => { @@ -196,11 +197,11 @@ describe('authorization tests', () => { expect(result.errors?.length).toBe(1); expect(result.errors?.[0].message).toBe( '[Subgraph1] Field "T.requiresExtra" does not specify necessary @authenticated, @requiresScopes and/or ' + - '@policy auth requirements to access the transitive field T.extra data from @requires selection set.' + '@policy auth requirements to access the transitive field "T.extra" data from @requires selection set.' ); }) - it('does not work with subset of auth', () => { + it('does not work with invalid subset of auth', () => { const subgraph1 = { name: 'Subgraph1', url: 'https://Subgraph1', @@ -233,7 +234,7 @@ describe('authorization tests', () => { expect(result.errors?.length).toBe(1); expect(result.errors?.[0].message).toBe( '[Subgraph1] Field "T.requiresExtra" does not specify necessary @authenticated, @requiresScopes and/or @policy ' + - 'auth requirements to access the transitive field T.extra data from @requires selection set.' + 'auth requirements to access the transitive field "T.extra" data from @requires selection set.' ); }) @@ -298,7 +299,7 @@ describe('authorization tests', () => { expect(result.errors?.length).toBe(1); expect(result.errors?.[0].message).toBe( '[Subgraph1] Field "T.requiresExtra" does not specify necessary @authenticated, @requiresScopes and/or @policy ' + - 'auth requirements to access the transitive field I2.i2 data from @requires selection set.' + 'auth requirements to access the transitive field "I2.i2" data from @requires selection set.' ); }) @@ -351,7 +352,7 @@ describe('authorization tests', () => { expect(result.errors?.length).toBe(1); expect(result.errors?.[0].message).toBe( '[Subgraph1] Field "T.requiresExtra" does not specify necessary @authenticated, @requiresScopes and/or @policy ' + - 'auth requirements to access the transitive field I1.i data from @requires selection set.' + 'auth requirements to access the transitive field "I1.i" data from @requires selection set.' ); }) @@ -404,7 +405,7 @@ describe('authorization tests', () => { expect(result.errors?.length).toBe(1); expect(result.errors?.[0].message).toBe( '[Subgraph1] Field "T.requiresExtra" does not specify necessary @authenticated, @requiresScopes and/or @policy ' + - 'auth requirements to access the transitive interface I data from @requires selection set.' + 'auth requirements to access the transitive field "T.extra" data from @requires selection set.' ); }) @@ -469,7 +470,7 @@ describe('authorization tests', () => { expect(result.errors?.length).toBe(1); expect(result.errors?.[0].message).toBe( '[Subgraph1] Field "T.requiresExtra" does not specify necessary @authenticated, @requiresScopes and/or @policy' + - ' auth requirements to access the transitive data in inline fragment type condition I1 from @requires selection set.' + ' auth requirements to access the transitive field "T.extra" data from @requires selection set.' ); }) }); @@ -654,7 +655,7 @@ describe('authorization tests', () => { expect(result.errors?.length).toBe(1); expect(result.errors?.[0].message).toBe( '[Subgraph1] Field "U.field" does not specify necessary @authenticated, @requiresScopes and/or @policy ' + - 'auth requirements to access the transitive field T.prop data from @fromContext selection set.' + 'auth requirements to access the transitive field "T.prop" data from @fromContext selection set.' ); }) @@ -710,4 +711,387 @@ describe('authorization tests', () => { ); }) }); + describe("interfaces", () => { + it('propagates @authenticated from type', () => { + const subgraph1 = { + name: 'Subgraph1', + url: 'https://Subgraph1', + typeDefs: gql` + type Query { + i: I! + } + + interface I { + id: ID + } + + type T implements I @key(fields: "id") @authenticated { + id: ID + value1: String + } + + type U implements I @key(fields: "id") { + id: ID + value2: String + } + ` + } + + const subgraph2 = { + name: 'Subgraph2', + url: 'https://Subgraph2', + typeDefs: gql` + type Query { + t: T + } + + type T @key(fields: "id") { + id: ID! + other: Int + } + ` + } + + const result = composeAsFed2Subgraphs([subgraph1, subgraph2]); + assertCompositionSuccess(result); + expect( + result.schema.type('I')?.appliedDirectivesOf("authenticated")?.[0] + ); + }) + + it('propagates @requiresScopes from type', () => { + const subgraph1 = { + name: 'Subgraph1', + url: 'https://Subgraph1', + typeDefs: gql` + type Query { + i: I! + } + + interface I { + id: ID + } + + type T implements I @key(fields: "id") @requiresScopes(scopes: [["S1"], ["S2"]]) { + id: ID + vT: String + } + + type U implements I @key(fields: "id") @requiresScopes(scopes: [["S1"], ["S2", "S3"]]) { + id: ID + vU: String + } + + type V implements I @key(fields: "id") { + id: ID + vV: String + } + ` + } + + const subgraph2 = { + name: 'Subgraph2', + url: 'https://Subgraph2', + typeDefs: gql` + type Query { + t: T + } + + type T @key(fields: "id") { + id: ID! + other: Int + } + ` + } + + const result = composeAsFed2Subgraphs([subgraph1, subgraph2]); + assertCompositionSuccess(result); + expect( + result.schema.type('I') + ?.appliedDirectivesOf("requiresScopes") + ?.[0]?.arguments()?.["scopes"]).toStrictEqual( + [ + ['S1'], + ['S2', 'S3'], + ] + ); + }) + + it('propagates @policy from type', () => { + const subgraph1 = { + name: 'Subgraph1', + url: 'https://Subgraph1', + typeDefs: gql` + type Query { + i: I! + } + + interface I { + id: ID + } + + type T implements I @key(fields: "id") @policy(policies: [["P1"]]) { + id: ID + vT: String + } + + type U implements I @key(fields: "id") @policy(policies: [["P2"]]) { + id: ID + vU: String + } + + type V implements I @key(fields: "id") { + id: ID + vV: String + } + ` + } + + const subgraph2 = { + name: 'Subgraph2', + url: 'https://Subgraph2', + typeDefs: gql` + type Query { + t: T + } + + type T @key(fields: "id") { + id: ID! + other: Int + } + ` + } + + const result = composeAsFed2Subgraphs([subgraph1, subgraph2]); + assertCompositionSuccess(result); + expect( + result.schema.type('I') + ?.appliedDirectivesOf("policy") + ?.[0]?.arguments()?.["policies"]).toStrictEqual( + [ + ['P1', 'P2'], + ] + ); + }) + + it('propagates @authenticated from fields', () => { + const subgraph1 = { + name: 'Subgraph1', + url: 'https://Subgraph1', + typeDefs: gql` + type Query { + i: I! + } + + interface I { + id: ID + i1: Int + i2: String + i3: String + } + + type T1 implements I @key(fields: "id") { + id: ID + i1: Int + i2: String @shareable + i3: String + value1: String + } + + type T2 implements I @key(fields: "id") { + id: ID + i1: Int @authenticated + i2: String + i3: String + value2: String + } + ` + } + + const subgraph2 = { + name: 'Subgraph2', + url: 'https://Subgraph2', + typeDefs: gql` + type Query { + t: T1 + } + + type T1 @key(fields: "id") { + id: ID! + i2: String @shareable @authenticated + other: Int + } + ` + } + + const result = composeAsFed2Subgraphs([subgraph1, subgraph2]); + assertCompositionSuccess(result); + const i = result.schema.type('I'); + expect(i).toBeDefined(); + expect(i).toBeInstanceOf(InterfaceType); + const field1 = (i as InterfaceType).field("i1"); + const field2 = (i as InterfaceType).field("i2"); + expect(field1?.appliedDirectivesOf("authenticated")?.[0]).toBeDefined(); + expect(field2?.appliedDirectivesOf("authenticated")?.[0]).toBeDefined(); + }) + + it('propagates @requiresScopes from field', () => { + const subgraph1 = { + name: 'Subgraph1', + url: 'https://Subgraph1', + typeDefs: gql` + type Query { + i: I! + } + + interface I { + id: ID + i1: Int + i2: String + i3: String + } + + type T1 implements I @key(fields: "id") { + id: ID + i1: Int @requiresScopes(scopes: [["S1"]]) + i2: String @shareable + i3: String + value1: String + } + + type T2 implements I @key(fields: "id") { + id: ID + i1: Int @requiresScopes(scopes: [["S1", "S2"]]) + i2: String + i3: String + value2: String + } + + type T3 implements I @key(fields: "id") { + id: ID + i1: Int + i2: String + i3: String + value2: String + } + ` + } + + const subgraph2 = { + name: 'Subgraph2', + url: 'https://Subgraph2', + typeDefs: gql` + type Query { + t: T1 + } + + type T1 @key(fields: "id") { + id: ID! + i2: String @shareable @requiresScopes(scopes: [["S3"]]) + other: Int + } + ` + } + + const result = composeAsFed2Subgraphs([subgraph1, subgraph2]); + assertCompositionSuccess(result); + const i = result.schema.type('I'); + expect(i).toBeDefined(); + expect(i).toBeInstanceOf(InterfaceType); + const field1 = (i as InterfaceType).field("i1"); + expect(field1?.appliedDirectivesOf("requiresScopes") + ?.[0]?.arguments()?.["scopes"]).toStrictEqual( + [ + ['S1', 'S2'], + ] + ); + const field2 = (i as InterfaceType).field("i2"); + expect(field2?.appliedDirectivesOf("requiresScopes") + ?.[0]?.arguments()?.["scopes"]).toStrictEqual( + [ + ['S3'], + ] + ); + }) + + it('propagates @policy on field', () => { + const subgraph1 = { + name: 'Subgraph1', + url: 'https://Subgraph1', + typeDefs: gql` + type Query { + i: I! + } + + interface I { + id: ID + i1: Int + i2: String + i3: String + } + + type T1 implements I @key(fields: "id") { + id: ID + i1: Int @policy(policies: [["P1"], ["P2"]]) + i2: String @shareable + i3: String + value1: String + } + + type T2 implements I @key(fields: "id") { + id: ID + i1: Int @policy(policies: [["P1"], ["P2", "P3"]]) + i2: String + i3: String + value2: String + } + + type T3 implements I @key(fields: "id") { + id: ID + i1: Int + i2: String + i3: String + value2: String + } + ` + } + + const subgraph2 = { + name: 'Subgraph2', + url: 'https://Subgraph2', + typeDefs: gql` + type Query { + t: T1 + } + + type T1 @key(fields: "id") { + id: ID! + i2: String @shareable @policy(policies: [["P4"]]) + other: Int + } + ` + } + + const result = composeAsFed2Subgraphs([subgraph1, subgraph2]); + assertCompositionSuccess(result); + const i = result.schema.type('I'); + expect(i).toBeDefined(); + expect(i).toBeInstanceOf(InterfaceType); + const field1 = (i as InterfaceType).field("i1"); + expect(field1?.appliedDirectivesOf("policy") + ?.[0]?.arguments()?.["policies"]).toStrictEqual( + [ + ['P1'], + ['P2', 'P3'], + ] + ); + const field2 = (i as InterfaceType).field("i2"); + expect(field2?.appliedDirectivesOf("policy") + ?.[0]?.arguments()?.["policies"]).toStrictEqual( + [ + ['P4'], + ] + ); + }) + }); }); diff --git a/composition-js/src/merging/merge.ts b/composition-js/src/merging/merge.ts index 2c383bec1..a81bbd9e5 100644 --- a/composition-js/src/merging/merge.ts +++ b/composition-js/src/merging/merge.ts @@ -396,7 +396,8 @@ class Merger { private fieldsWithFromContext: Set; private fieldsWithOverride: Set; private fieldsWithRequires: Set; - private coordinatesWithAuth: Set; + private authEnabled: boolean; + private interfacesWithAuthRequirements: Set; constructor(readonly subgraphs: Subgraphs, readonly options: CompositionOptions) { this.latestFedVersionUsed = this.getLatestFederationVersionUsed(); @@ -405,7 +406,9 @@ class Merger { this.fieldsWithFromContext = this.getFieldsWithFromContextDirective(); this.fieldsWithOverride = this.getFieldsWithOverrideDirective(); this.fieldsWithRequires = this.getFieldsWithRequiresDirective(); - this.coordinatesWithAuth = this.getCoordinatesWithAuth(); + const { authEnabled, interfacesWithAuthRequirements } = this.getAuthRequirements(); + this.authEnabled = authEnabled; + this.interfacesWithAuthRequirements = interfacesWithAuthRequirements; this.names = subgraphs.names(); this.composeDirectiveManager = new ComposeDirectiveManager( @@ -751,6 +754,8 @@ class Merger { // we want to make sure everything is ready. this.addMissingInterfaceObjectFieldsToImplementations(); + this.propagateAuthToInterfaces(); + // If we already encountered errors, `this.merged` is probably incomplete. Let's not risk adding errors that // are only an artifact of that incompleteness as it's confusing. if (this.errors.length === 0) { @@ -3437,7 +3442,7 @@ class Merger { // auth verification on the supergraph // need to verify usage of @requires on fields that require authorization - if (this.coordinatesWithAuth.size > 0) { + if (this.authEnabled) { const authValidator = new AuthValidator(this.merged, this.joinSpec, this.subgraphNamesToJoinSpecName); for (const coordinate of this.fieldsWithRequires) { const errors = authValidator.validateRequiresFieldSet(coordinate); @@ -3629,8 +3634,9 @@ class Merger { ); } - private getCoordinatesWithAuth(): Set { - const coordinates = new Set(); + private getAuthRequirements(): { authEnabled: boolean, interfacesWithAuthRequirements: Set } { + const interfacesWithAuthRequirements = new Set(); + let authEnabled = false; for (const subgraph of this.subgraphs) { const authenticatedDirective = subgraph.metadata().authenticatedDirective(); const requiresScopesDirective = subgraph.metadata().requiresScopesDirective(); @@ -3638,23 +3644,30 @@ class Merger { for (const authDirective of [authenticatedDirective, requiresScopesDirective, policyDirective]) { if (isFederationDirectiveDefinedInSchema(authDirective)) { + authEnabled = authEnabled || authDirective.applications().size > 0; + for (const application of authDirective.applications()) { const parent = application.parent; - if (parent instanceof FieldDefinition) { - coordinates.add(parent.coordinate); - } else if (parent instanceof ObjectType) { - coordinates.add(parent.coordinate) - } else if (parent instanceof ScalarType) { - coordinates.add(parent.coordinate); - } else if (parent instanceof EnumType) { - coordinates.add(parent.coordinate); + if (parent instanceof ObjectType) { + // capture interfaces that will need to be updated later on + parent.interfaces().forEach( + (intf) => interfacesWithAuthRequirements.add(intf.name) + ); + } else if (parent instanceof FieldDefinition) { + // we only allow auth on object fields + const parentObjectType = parent.parent; + if (isObjectType(parentObjectType)) { + parentObjectType.interfaces().forEach( + (intf) => interfacesWithAuthRequirements.add(intf.name) + ); + } } } } } } - return coordinates; + return { authEnabled, interfacesWithAuthRequirements }; } private getFieldsWithAppliedDirective( @@ -3676,9 +3689,58 @@ class Merger { } return fields; } + + propagateAuthToInterfaces() { + if (this.authEnabled) { + const interfacesThatNeedAuth = this.merged.interfaceTypes() + .filter((intf) => this.interfacesWithAuthRequirements.has(intf.name)); + + if (interfacesThatNeedAuth.length > 0) { + const authenticatedFeature = this.merged.coreFeatures?.getByIdentity(AuthenticatedSpecDefinition.identity); + const authenticatedSpec = authenticatedFeature && AUTHENTICATED_VERSIONS.find(authenticatedFeature.url.version); + const authenticatedDirective = authenticatedSpec?.authenticatedDirective(this.merged); + + const requiresScopesFeature = this.merged.coreFeatures?.getByIdentity(RequiresScopesSpecDefinition.identity); + const requiresScopesSpec = requiresScopesFeature && REQUIRES_SCOPES_VERSIONS.find(requiresScopesFeature.url.version); + const requiresScopesDirective = requiresScopesSpec?.requiresScopesDirective(this.merged); + + const policyFeature = this.merged.coreFeatures?.getByIdentity(PolicySpecDefinition.identity); + const policySpec = policyFeature && POLICY_VERSIONS.find(policyFeature.url.version); + const policyDirective = policySpec?.policyDirective(this.merged); + + for (const intf of interfacesThatNeedAuth) { + const impls = intf.possibleRuntimeTypes(); + const implSources = sourcesFromArray(>impls); + if (authenticatedDirective) { + this.mergeAppliedDirective(authenticatedDirective.name, implSources, intf); + } + if (requiresScopesDirective) { + this.mergeAppliedDirective(requiresScopesDirective.name, implSources, intf); + } + if (policyDirective) { + this.mergeAppliedDirective(policyDirective.name, implSources, intf); + } + + for (const intfField of intf.fields()) { + const implFields = impls.map((impl) => impl.field(intfField.name)); + const fieldSources = sourcesFromArray(implFields); + if (authenticatedDirective) { + this.mergeAppliedDirective(authenticatedDirective.name, fieldSources, intfField); + } + if (requiresScopesDirective) { + this.mergeAppliedDirective(requiresScopesDirective.name, fieldSources, intfField); + } + if (policyDirective) { + this.mergeAppliedDirective(policyDirective.name, fieldSources, intfField); + } + } + } + } + } + } } -class AuthValidator { +export class AuthValidator { schema: Schema; joinSpecNamesToSubgraphNames: Map; joinFieldDirective: DirectiveDefinition; @@ -3813,7 +3875,7 @@ class AuthValidator { return errors; } - authRequirementsOnElement(element: NamedType | FieldDefinition): AuthRequirementsOnElement | undefined { + private authRequirementsOnElement(element: NamedType | FieldDefinition): AuthRequirementsOnElement | undefined { const requirements = new AuthRequirementsOnElement(); if (this.authenticatedDirective) { const appliedDirective = element.appliedDirectivesOf(this.authenticatedDirective)?.[0]; @@ -3845,7 +3907,7 @@ class AuthValidator { } } - verifyAuthRequirementsOnSelectionSet(authRequirements: AuthRequirements, selectionSet: SelectionSet) { + private verifyAuthRequirementsOnSelectionSet(authRequirements: AuthRequirements, selectionSet: SelectionSet) { const parentType = this.schema.type(selectionSet.parentType.name); for (const selection of selectionSet.selections()) { if (selection instanceof FieldSelection) { @@ -3855,7 +3917,7 @@ class AuthValidator { const requirementsOnImplType = this.authRequirementsOnElement(implType); if (!authRequirements.satisfies(requirementsOnImplType)) { const msg = `Field "${authRequirements.fieldCoordinate}" does not specify necessary @authenticated, @requiresScopes ` - + `and/or @policy auth requirements to access the transitive interface ${parentType} data from ${authRequirements.directive} selection set.`; + + `and/or @policy auth requirements to access the transitive interface "${parentType}" data from ${authRequirements.directive} selection set.`; throw ERRORS.MISSING_TRANSITIVE_AUTH_REQUIREMENTS.err(msg); } // re-check the implementation auth condition @@ -3876,7 +3938,7 @@ class AuthValidator { const requirementsOnCondition = this.authRequirementsOnElement(condition); if (!authRequirements.satisfies(requirementsOnCondition)) { const msg = `Field "${authRequirements.fieldCoordinate}" does not specify necessary @authenticated, @requiresScopes ` - + `and/or @policy auth requirements to access the transitive data in inline fragment type condition ${condition} from ${authRequirements.directive} selection set.`; + + `and/or @policy auth requirements to access the transitive data in inline fragment type condition "${condition}" from ${authRequirements.directive} selection set.`; throw ERRORS.MISSING_TRANSITIVE_AUTH_REQUIREMENTS.err(msg); } } @@ -3894,7 +3956,7 @@ class AuthValidator { if (!authRequirements.satisfies(fieldAuthReqs) || !authRequirements.satisfies(fieldReturnAuthReqs)) { const msg = `Field "${authRequirements.fieldCoordinate}" does not specify necessary @authenticated, @requiresScopes ` - + `and/or @policy auth requirements to access the transitive field ${field.coordinate} data from ${authRequirements.directive} selection set.`; + + `and/or @policy auth requirements to access the transitive field "${field.coordinate}" data from ${authRequirements.directive} selection set.`; throw ERRORS.MISSING_TRANSITIVE_AUTH_REQUIREMENTS.err(msg); } } From d41bad4fdf801435404111738ade4912e71746ba Mon Sep 17 00:00:00 2001 From: dariuszkuc <9501705+dariuszkuc@users.noreply.github.com> Date: Tue, 4 Nov 2025 09:25:00 -0600 Subject: [PATCH 5/5] fix cspell and prettier check --- .cspell/cspell-dict.txt | 3 +++ composition-js/src/merging/merge.ts | 1 - 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/.cspell/cspell-dict.txt b/.cspell/cspell-dict.txt index 05c50a5ff..9af90407f 100644 --- a/.cspell/cspell-dict.txt +++ b/.cspell/cspell-dict.txt @@ -103,6 +103,7 @@ iface impelementation implemenations implementationt +impls inacessible incopatibilities ineffectient @@ -112,6 +113,7 @@ insteances instrospection Inteface Intelli +intf invalidities isnodelike issuels @@ -191,6 +193,7 @@ Renamers reoptimized repeateable reponse +Reqs resovable Reviwed Rhai diff --git a/composition-js/src/merging/merge.ts b/composition-js/src/merging/merge.ts index a81bbd9e5..209ead5a5 100644 --- a/composition-js/src/merging/merge.ts +++ b/composition-js/src/merging/merge.ts @@ -89,7 +89,6 @@ import { AuthenticatedSpecDefinition, RequiresScopesSpecDefinition, PolicySpecDefinition, - ScalarType, AUTHENTICATED_VERSIONS, REQUIRES_SCOPES_VERSIONS, POLICY_VERSIONS,