From 827c7b8df81fdae38826c94f7ac7698a8887001a Mon Sep 17 00:00:00 2001 From: Josue Ruiz Date: Mon, 13 Jul 2020 17:06:05 -0700 Subject: [PATCH] fix(graphql-auth-transformer): allow auth progation to recursive types (#4788) allow auth progation to recursive types - once auth has been added to a non model type it will not re-add auth to said type re #4631 --- .../src/ModelAuthTransformer.ts | 40 ++++++++-------- .../src/__tests__/MultiAuth.test.ts | 46 ++++++++++++++++++- .../graphql-auth-transformer/src/constants.ts | 1 + 3 files changed, 67 insertions(+), 20 deletions(-) diff --git a/packages/graphql-auth-transformer/src/ModelAuthTransformer.ts b/packages/graphql-auth-transformer/src/ModelAuthTransformer.ts index 28d0f08f3ab..b1b01c71060 100644 --- a/packages/graphql-auth-transformer/src/ModelAuthTransformer.ts +++ b/packages/graphql-auth-transformer/src/ModelAuthTransformer.ts @@ -41,7 +41,7 @@ import { import { Expression, print, raw, iff, forEach, set, ref, list, compoundExpression, newline, comment, not } from 'graphql-mapping-template'; import { ModelDirectiveConfiguration, ModelDirectiveOperationType, ModelSubscriptionLevel } from './ModelDirectiveConfiguration'; -import { OWNER_AUTH_STRATEGY, GROUPS_AUTH_STRATEGY, DEFAULT_OWNER_FIELD } from './constants'; +import { OWNER_AUTH_STRATEGY, GROUPS_AUTH_STRATEGY, DEFAULT_OWNER_FIELD, AUTH_NON_MODEL_TYPES } from './constants'; /** * Implements the ModelAuthTransformer. @@ -265,6 +265,9 @@ export class ModelAuthTransformer extends Transformer { ctx.mergeOutputs(template.Outputs); ctx.mergeConditions(template.Conditions); this.updateAPIAuthentication(ctx); + if (!ctx.metadata.has(AUTH_NON_MODEL_TYPES)) { + ctx.metadata.set(AUTH_NON_MODEL_TYPES, new Set()); + } }; public after = (ctx: TransformerContext): void => { @@ -521,38 +524,37 @@ Static group authorization should perform as expected.`, }; private propagateAuthDirectivesToNestedTypes(type: ObjectTypeDefinitionNode, rules: AuthRule[], ctx: TransformerContext) { + const seenNonModelTypes: Set = ctx.metadata.get(AUTH_NON_MODEL_TYPES); + const nonModelTypePredicate = (fieldType: TypeDefinitionNode): TypeDefinitionNode | undefined => { if (fieldType) { if (fieldType.kind !== 'ObjectTypeDefinition') { return undefined; } - const typeModel = fieldType.directives.find(dir => dir.name.value === 'model'); return typeModel !== undefined ? undefined : fieldType; } - return fieldType; }; - const nonModelFieldTypes = type.fields.map(f => ctx.getType(getBaseType(f.type)) as TypeDefinitionNode).filter(nonModelTypePredicate); - for (const nonModelFieldType of nonModelFieldTypes) { - const directives = this.getDirectivesForRules(rules, false); - - // Add the directives to the Type node itself - if (directives.length > 0) { - this.extendTypeWithDirectives(ctx, nonModelFieldType.name.value, directives); - } - - const hasIAM = directives.filter(directive => directive.name.value === 'aws_iam') || this.configuredAuthProviders.default === 'iam'; + if (!seenNonModelTypes.has(nonModelFieldType.name.value)) { + // add to the set of seen non model types + seenNonModelTypes.add(nonModelFieldType.name.value); + const directives = this.getDirectivesForRules(rules, false); + // Add the directives to the Type node itself + if (directives.length > 0) { + this.extendTypeWithDirectives(ctx, nonModelFieldType.name.value, directives); + } + const hasIAM = directives.filter(directive => directive.name.value === 'aws_iam') || this.configuredAuthProviders.default === 'iam'; + if (hasIAM) { + this.unauthPolicyResources.add(`${nonModelFieldType.name.value}/null`); + this.authPolicyResources.add(`${nonModelFieldType.name.value}/null`); + } - if (hasIAM) { - this.unauthPolicyResources.add(`${nonModelFieldType.name.value}/null`); - this.authPolicyResources.add(`${nonModelFieldType.name.value}/null`); + // Recursively process the nested types if there is any + this.propagateAuthDirectivesToNestedTypes(nonModelFieldType, rules, ctx); } - - // Recursively process the nested types if there is any - this.propagateAuthDirectivesToNestedTypes(nonModelFieldType, rules, ctx); } } diff --git a/packages/graphql-auth-transformer/src/__tests__/MultiAuth.test.ts b/packages/graphql-auth-transformer/src/__tests__/MultiAuth.test.ts index 411c1951315..7abe4492f47 100644 --- a/packages/graphql-auth-transformer/src/__tests__/MultiAuth.test.ts +++ b/packages/graphql-auth-transformer/src/__tests__/MultiAuth.test.ts @@ -1,4 +1,4 @@ -import { ObjectTypeDefinitionNode, parse, DocumentNode, Kind } from 'graphql'; +import { ObjectTypeDefinitionNode, FieldDefinitionNode, parse, DocumentNode, Kind } from 'graphql'; import { GraphQLTransform } from 'graphql-transformer-core'; import { DynamoDBModelTransformer } from 'graphql-dynamodb-transformer'; import { ModelConnectionTransformer } from 'graphql-connection-transformer'; @@ -66,6 +66,7 @@ const multiAuthDirective = const ownerAuthDirective = '@auth(rules: [{allow: owner}])'; const ownerWithIAMAuthDirective = '@auth(rules: [{allow: owner, provider: iam }])'; const ownerRestrictedPublicAuthDirective = '@auth(rules: [{allow: owner},{allow: public, operations: [read]}])'; +const ownerRestrictedIAMPrivateAuthDirective = '@auth(rules: [{allow: owner},{allow: private, operations: [read], provider: iam }])'; const groupsAuthDirective = '@auth(rules: [{allow: groups}])'; const groupsWithApiKeyAuthDirective = '@auth(rules: [{allow: groups}, {allow: public, operations: [read]}])'; const groupsWithProviderAuthDirective = '@auth(rules: [{allow: groups, provider: iam }])'; @@ -140,6 +141,21 @@ const getSchemaWithNonModelField = (authDirective: string) => { }`; }; +const getSchemaWithRecursiveNonModelField = (authDirective: string) => { + return ` + type Post @model ${authDirective} { + id: ID! + title: String! + tags: [Tag] + } + + type Tag { + id: ID + tags: [Tag] + } + `; +}; + const getTransformer = (authConfig: AppSyncAuthConfiguration) => new GraphQLTransform({ transformers: [new DynamoDBModelTransformer(), new ModelConnectionTransformer(), new ModelAuthTransformer({ authConfig })], @@ -168,6 +184,21 @@ const expectTwo = (fieldOrType, directiveNames) => { expect(fieldOrType.directives.find(d => d.name.value === directiveNames[1])).toBeDefined(); }; +const expectMultiple = (fieldOrType: ObjectTypeDefinitionNode | FieldDefinitionNode, directiveNames: string[]) => { + expect(directiveNames).toBeDefined(); + expect(directiveNames).toHaveLength(directiveNames.length); + expect(fieldOrType.directives.length).toEqual(directiveNames.length); + directiveNames.forEach(directiveName => { + expect(fieldOrType.directives).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + name: expect.objectContaining({ value: directiveName }), + }), + ]), + ); + }); +}; + const getField = (type, name) => type.fields.find(f => f.name.value === name); describe('Validation tests', () => { @@ -461,6 +492,19 @@ describe('Type directive transformation tests', () => { } }); + test(`Recursive types without @model`, () => { + const schema = getSchemaWithRecursiveNonModelField(ownerRestrictedIAMPrivateAuthDirective); + const transformer = getTransformer(withAuthModes(userPoolsDefaultConfig, ['AWS_IAM'])); + + const out = transformer.transform(schema); + const schemaDoc = parse(out.schema); + + const tagType = getObjectType(schemaDoc, 'Tag'); + const expectedDirectiveNames = [userPoolsDirectiveName, iamDirectiveName]; + + expectMultiple(tagType, expectedDirectiveNames); + }); + test(`Nested types without @model getting directives applied (cognito default, iam additional)`, () => { const schema = getSchemaWithNonModelField(privateAndPrivateIAMDirective); const transformer = getTransformer(withAuthModes(userPoolsDefaultConfig, ['AWS_IAM'])); diff --git a/packages/graphql-auth-transformer/src/constants.ts b/packages/graphql-auth-transformer/src/constants.ts index c6b27888723..38eaeca1209 100644 --- a/packages/graphql-auth-transformer/src/constants.ts +++ b/packages/graphql-auth-transformer/src/constants.ts @@ -7,3 +7,4 @@ export const DEFAULT_GROUP_CLAIM = 'cognito:groups'; export const ON_CREATE_FIELD = 'onCreate'; export const ON_UPDATE_FIELD = 'onUpdate'; export const ON_DELETE_FIELD = 'onDelete'; +export const AUTH_NON_MODEL_TYPES = 'authNonModelTypes';