From bcb889ff00d4b09edbbab9908f233a4f850d9289 Mon Sep 17 00:00:00 2001 From: lazpavel <85319655+lazpavel@users.noreply.github.com> Date: Thu, 8 Jul 2021 14:19:26 -0400 Subject: [PATCH 1/4] Add .circleci/config.yml From e77ebc2d7d6970e24b8f6e61e5fc5fc49c386a1b Mon Sep 17 00:00:00 2001 From: Pavel Lazar Date: Tue, 20 Jul 2021 21:49:16 -0400 Subject: [PATCH 2/4] fix(graphql-auth-transformer): extend isOption subscription owner check --- .../src/ModelAuthTransformer.ts | 23 ++++++-- .../__tests__/OwnerAuthTransformer.test.ts | 58 +++++++++++++++++++ 2 files changed, 75 insertions(+), 6 deletions(-) diff --git a/packages/graphql-auth-transformer/src/ModelAuthTransformer.ts b/packages/graphql-auth-transformer/src/ModelAuthTransformer.ts index 920b858e08a..844d9c8c0c1 100644 --- a/packages/graphql-auth-transformer/src/ModelAuthTransformer.ts +++ b/packages/graphql-auth-transformer/src/ModelAuthTransformer.ts @@ -1921,17 +1921,14 @@ operations will be generated by the CLI.`, // check if owner is enabled in auth const ownerRules = rules.filter(rule => rule.allow === OWNER_AUTH_STRATEGY); const needsDefaultOwnerField = ownerRules.find(rule => !rule.ownerField); - // if there are any public, private, and/or static group rules then the owner argument is optional - const needsOwnerArgument = rules.find( - rule => (rule.allow === GROUPS_AUTH_STRATEGY && !rule.groupsField) || rule.allow === 'private' || rule.allow === 'public', - ); + if (ownerRules) { // if there is an owner rule without ownerField add the owner field in the type if (needsDefaultOwnerField) { this.addOwner(ctx, parent.name.value); } - // If static group is specified in any of the rules then it would specify the owner arg(s) as optional - const makeNonNull = needsOwnerArgument ? false : true; + + const makeNonNull = this.isSubscriptionOwnerArgumentOptional(rules) ? false : true; this.addSubscriptionOwnerArgument(ctx, resolver, ownerRules, makeNonNull); } } @@ -1944,6 +1941,20 @@ operations will be generated by the CLI.`, ctx.mapResourceToStack(parent.name.value, resolverResourceId); } + private isSubscriptionOwnerArgumentOptional(rules: AuthRule[]) { + const ownerRules = rules.filter(rule => rule.allow === OWNER_AUTH_STRATEGY); + + // if there are multiple owner rules then the owner argument is optional + if (ownerRules.length > 1) { + return true; + } + + // if there are any public, private, and/or static group rules then the owner argument is optional + return rules.find( + rule => (rule.allow === GROUPS_AUTH_STRATEGY && !rule.groupsField) || rule.allow === 'private' || rule.allow === 'public', + ); + } + private addSubscriptionOwnerArgument(ctx: TransformerContext, resolver: Resolver, ownerRules: AuthRule[], makeNonNull: boolean = false) { let subscription = ctx.getSubscription(); let createField: FieldDefinitionNode = subscription.fields.find( diff --git a/packages/graphql-auth-transformer/src/__tests__/OwnerAuthTransformer.test.ts b/packages/graphql-auth-transformer/src/__tests__/OwnerAuthTransformer.test.ts index a0314833991..02fc00693c8 100644 --- a/packages/graphql-auth-transformer/src/__tests__/OwnerAuthTransformer.test.ts +++ b/packages/graphql-auth-transformer/src/__tests__/OwnerAuthTransformer.test.ts @@ -91,6 +91,64 @@ test('Test OwnerField with Subscriptions', () => { ); }); +test('Test multiple owner rules with Subscriptions', () => { + const validSchema = ` + type Post @model + @auth(rules: [ + { allow: owner }, + { allow: owner, ownerField: "editors", operations: [read, update] } + ]) + { + id: ID! + title: String + owner: String + editors: [String] + }`; + const transformer = new GraphQLTransform({ + featureFlags, + transformers: [ + new DynamoDBModelTransformer(), + new ModelAuthTransformer({ + authConfig: { + defaultAuthentication: { + authenticationType: 'AMAZON_COGNITO_USER_POOLS', + }, + additionalAuthenticationProviders: [], + }, + }), + ], + }); + const out = transformer.transform(validSchema); + expect(out).toBeDefined(); + + // expect 'owner' and 'editors' as arguments for subscription operations + expect(out.schema).toContain('onCreatePost(owner: String, editors: String)'); + expect(out.schema).toContain('onUpdatePost(owner: String, editors: String)'); + expect(out.schema).toContain('onDeletePost(owner: String, editors: String)'); + + // expect logic in the resolvers to check for owner args as an allowerOwner + expect(out.resolvers['Subscription.onCreatePost.res.vtl']).toContain( + '#set( $allowedOwners0 = $util.defaultIfNull($ctx.args.owner, null) )', + ); + expect(out.resolvers['Subscription.onUpdatePost.res.vtl']).toContain( + '#set( $allowedOwners0 = $util.defaultIfNull($ctx.args.owner, null) )', + ); + expect(out.resolvers['Subscription.onDeletePost.res.vtl']).toContain( + '#set( $allowedOwners0 = $util.defaultIfNull($ctx.args.owner, null) )', + ); + + // expect logic in the resolvers to check for editors args as an allowerOwner + expect(out.resolvers['Subscription.onCreatePost.res.vtl']).toContain( + '#set( $allowedOwners1 = $util.defaultIfNull($ctx.args.editors, null) )', + ); + expect(out.resolvers['Subscription.onUpdatePost.res.vtl']).toContain( + '#set( $allowedOwners1 = $util.defaultIfNull($ctx.args.editors, null) )', + ); + expect(out.resolvers['Subscription.onDeletePost.res.vtl']).toContain( + '#set( $allowedOwners1 = $util.defaultIfNull($ctx.args.editors, null) )', + ); +}); + describe('add missing implicit owner fields to type', () => { let ff: FeatureFlagProvider; const runTransformer = (validSchema: string) => { From c6352ac04df43e6139a61aca85cac6d57dd81c47 Mon Sep 17 00:00:00 2001 From: Pavel Lazar Date: Tue, 20 Jul 2021 22:05:26 -0400 Subject: [PATCH 3/4] fix(graphql-auth-transformer): fixed typo in unit test --- .../src/__tests__/OwnerAuthTransformer.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/graphql-auth-transformer/src/__tests__/OwnerAuthTransformer.test.ts b/packages/graphql-auth-transformer/src/__tests__/OwnerAuthTransformer.test.ts index 02fc00693c8..edc49c5568a 100644 --- a/packages/graphql-auth-transformer/src/__tests__/OwnerAuthTransformer.test.ts +++ b/packages/graphql-auth-transformer/src/__tests__/OwnerAuthTransformer.test.ts @@ -126,7 +126,7 @@ test('Test multiple owner rules with Subscriptions', () => { expect(out.schema).toContain('onUpdatePost(owner: String, editors: String)'); expect(out.schema).toContain('onDeletePost(owner: String, editors: String)'); - // expect logic in the resolvers to check for owner args as an allowerOwner + // expect logic in the resolvers to check for owner args as an allowedOwner expect(out.resolvers['Subscription.onCreatePost.res.vtl']).toContain( '#set( $allowedOwners0 = $util.defaultIfNull($ctx.args.owner, null) )', ); @@ -137,7 +137,7 @@ test('Test multiple owner rules with Subscriptions', () => { '#set( $allowedOwners0 = $util.defaultIfNull($ctx.args.owner, null) )', ); - // expect logic in the resolvers to check for editors args as an allowerOwner + // expect logic in the resolvers to check for editors args as an allowedOwner expect(out.resolvers['Subscription.onCreatePost.res.vtl']).toContain( '#set( $allowedOwners1 = $util.defaultIfNull($ctx.args.editors, null) )', ); From fd87c3716ae631558bd976f1cb824778c1e5ad33 Mon Sep 17 00:00:00 2001 From: Pavel Lazar Date: Fri, 23 Jul 2021 17:46:24 -0400 Subject: [PATCH 4/4] chore(graphql-auth-transformer): change isSubscriptionOwnerArgumentOptional return type to Boolean --- .../graphql-auth-transformer/src/ModelAuthTransformer.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/graphql-auth-transformer/src/ModelAuthTransformer.ts b/packages/graphql-auth-transformer/src/ModelAuthTransformer.ts index 844d9c8c0c1..cf2fab44ac3 100644 --- a/packages/graphql-auth-transformer/src/ModelAuthTransformer.ts +++ b/packages/graphql-auth-transformer/src/ModelAuthTransformer.ts @@ -1928,7 +1928,7 @@ operations will be generated by the CLI.`, this.addOwner(ctx, parent.name.value); } - const makeNonNull = this.isSubscriptionOwnerArgumentOptional(rules) ? false : true; + const makeNonNull = !this.isSubscriptionOwnerArgumentOptional(rules); this.addSubscriptionOwnerArgument(ctx, resolver, ownerRules, makeNonNull); } } @@ -1941,7 +1941,7 @@ operations will be generated by the CLI.`, ctx.mapResourceToStack(parent.name.value, resolverResourceId); } - private isSubscriptionOwnerArgumentOptional(rules: AuthRule[]) { + private isSubscriptionOwnerArgumentOptional(rules: AuthRule[]): Boolean { const ownerRules = rules.filter(rule => rule.allow === OWNER_AUTH_STRATEGY); // if there are multiple owner rules then the owner argument is optional @@ -1952,7 +1952,9 @@ operations will be generated by the CLI.`, // if there are any public, private, and/or static group rules then the owner argument is optional return rules.find( rule => (rule.allow === GROUPS_AUTH_STRATEGY && !rule.groupsField) || rule.allow === 'private' || rule.allow === 'public', - ); + ) + ? true + : false; } private addSubscriptionOwnerArgument(ctx: TransformerContext, resolver: Resolver, ownerRules: AuthRule[], makeNonNull: boolean = false) {