Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(graphql-auth-transformer): extend isOptional subscription owner check #7765

Merged
merged 9 commits into from
Jul 30, 2021
25 changes: 19 additions & 6 deletions packages/graphql-auth-transformer/src/ModelAuthTransformer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
this.addSubscriptionOwnerArgument(ctx, resolver, ownerRules, makeNonNull);
}
}
Expand All @@ -1944,6 +1941,22 @@ operations will be generated by the CLI.`,
ctx.mapResourceToStack(parent.name.value, resolverResourceId);
}

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
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(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: maybe this function should return a boolean every time, so that it can return the value you want instead of having to do this.isSubscriptionOwnerArgumentOptional(rules) ? false : true?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated to Boolean, makes it cleaner, thanks!

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) {
let subscription = ctx.getSubscription();
let createField: FieldDefinitionNode = subscription.fields.find(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 allowedOwner
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 allowedOwner
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) => {
Expand Down