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.

Sign up for GitHub

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

Jump to bottom

fix(graphql-auth-transformer): fixes @auth directives for Admin UI #7035

Merged
merged 6 commits into from
May 4, 2021
Merged

fix(graphql-auth-transformer): fixes @auth directives for Admin UI #7035

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
cd12e3d
fix(graphql-auth-transformer): fixes @auth directives for Admin UI
hsspain Apr 7, 2021
9576391
fix: update auth rule for Admin UI
yuth Apr 21, 2021
0efaa98
test: add unit tests
yuth Apr 22, 2021
b941d7c
wip
yuth Apr 22, 2021
e76be9b
chore: fix connected list query not to update query fields
yuth Apr 30, 2021
11af910
chore: update docstring
yuth May 1, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions packages/graphql-auth-transformer/src/AuthRule.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,6 @@ export interface AuthRule {
operations?: ModelOperation[];
queries?: ModelQuery[];
mutations?: ModelMutation[];
// Used only for IAM provider to decide if an IAM policy needs to be generated. IAM auth with AdminUI does not need IAM policies
generateIAMPolicy?: boolean;
}
159 changes: 107 additions & 52 deletions packages/graphql-auth-transformer/src/ModelAuthTransformer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -275,23 +275,26 @@ export class ModelAuthTransformer extends Transformer {
if (this.generateIAMPolicyforAuthRole === true) {
// Sanity check to make sure we're not generating invalid policies, where no resources are defined.
if (this.authPolicyResources.size === 0) {
throw new Error('AuthRole policies should be generated, but no resources were added');
}

ctx.mergeParameters({
[ResourceConstants.PARAMETERS.AuthRoleName]: new StringParameter({
Description: 'Reference to the name of the Auth Role created for the project.',
}),
});
// When AdminUI is enabled, IAM auth is added but it does not need any policies to be generated
if (!this.isAdminUIEnabled()) {
throw new Error('AuthRole policies should be generated, but no resources were added');
}
} else {
ctx.mergeParameters({
[ResourceConstants.PARAMETERS.AuthRoleName]: new StringParameter({
Description: 'Reference to the name of the Auth Role created for the project.',
}),
});

const authPolicies = this.resources.makeIAMPolicyForRole(true, this.authPolicyResources);
const authPolicies = this.resources.makeIAMPolicyForRole(true, this.authPolicyResources);

for (let i = 0; i < authPolicies.length; i++) {
const paddedIndex = `${i + 1}`.padStart(2, '0');
const resourceName = `${ResourceConstants.RESOURCES.AuthRolePolicy}${paddedIndex}`;
ctx.mergeResources({
[resourceName]: authPolicies[i],
});
for (let i = 0; i < authPolicies.length; i++) {
const paddedIndex = `${i + 1}`.padStart(2, '0');
const resourceName = `${ResourceConstants.RESOURCES.AuthRolePolicy}${paddedIndex}`;
ctx.mergeResources({
[resourceName]: authPolicies[i],
});
}
}
}

Expand Down Expand Up @@ -347,7 +350,8 @@ export class ModelAuthTransformer extends Transformer {
const searchableDirective = def.directives.find(dir => dir.name.value === 'searchable');

// Get and validate the auth rules.
const rules = this.getAuthRulesFromDirective(directive);
const rules = this.extendAuthRulesForAdminUI(this.getAuthRulesFromDirective(directive));

// Assign default providers to rules where no provider was explicitly defined
this.ensureDefaultAuthProviderAssigned(rules);
this.validateRules(rules);
Expand All @@ -363,12 +367,11 @@ export class ModelAuthTransformer extends Transformer {
// type will be emitted as well in case of IAM.
this.propagateAuthDirectivesToNestedTypes(def, rules, ctx);

const { operationRules, queryRules } = this.splitRules(rules);

// Retrieve the configuration options for the related @model directive
const modelConfiguration = new ModelDirectiveConfiguration(modelDirective, def);

// Get the directives we need to add to the GraphQL nodes
const directives = this.getDirectivesForRules(rules, false);
const directives = this.getDirectivesForRules(rules, rules.length === 0 ? this.shouldAddDefaultAuthDirective() : false);

// Add the directives to the Type node itself
if (directives.length > 0) {
Expand All @@ -377,6 +380,8 @@ export class ModelAuthTransformer extends Transformer {

this.addTypeToResourceReferences(def.name.value, rules);

const { operationRules, queryRules } = this.splitRules(rules);

// For each operation evaluate the rules and apply the changes to the relevant resolver.
this.protectCreateMutation(
ctx,
Expand All @@ -399,13 +404,22 @@ export class ModelAuthTransformer extends Transformer {
def,
modelConfiguration,
);
this.protectGetQuery(ctx, ResolverResourceIDs.DynamoDBGetResolverResourceID(def.name.value), queryRules.get, def, modelConfiguration);
this.protectGetQuery(
ctx,
ResolverResourceIDs.DynamoDBGetResolverResourceID(def.name.value),
queryRules.get,
def,
modelConfiguration,
true,
);
this.protectListQuery(
ctx,
ResolverResourceIDs.DynamoDBListResolverResourceID(def.name.value),
queryRules.list,
def,
modelConfiguration,
undefined,
true,
);
this.protectConnections(ctx, def, operationRules.read, modelConfiguration);
this.protectQueries(ctx, def, operationRules.read, modelConfiguration);
Expand Down Expand Up @@ -650,7 +664,7 @@ Static group authorization should perform as expected.`,
// If the parent type has any rules for this operation AND
// the default provider we've to get directives including the default
// as well.
const includeDefault = this.isTypeHasRulesForOperation(parent, operationType);
const includeDefault = this.doesTypeHaveRulesForOperation(parent, operationType);
const operationDirectives = this.getDirectivesForRules(rules, includeDefault);

this.addDirectivesToOperation(ctx, ctx.getQueryTypeName(), operationName, operationDirectives);
Expand Down Expand Up @@ -756,7 +770,7 @@ Either make the field optional, set auth on the object and not the field, or dis
// If the parent type has any rules for this operation AND
// the default provider we've to get directives including the default
// as well.
const includeDefault = this.isTypeHasRulesForOperation(parent, 'create');
const includeDefault = this.doesTypeHaveRulesForOperation(parent, 'create');
const operationDirectives = this.getDirectivesForRules(rules, includeDefault);

operationName = modelConfiguration.getName('create');
Expand Down Expand Up @@ -1050,26 +1064,31 @@ operations will be generated by the CLI.`,
* @param ctx The transformer context.
* @param resolverResourceId The logical id of the get resolver.
* @param rules The auth rules to apply.
* @param parent ObjectDefinition of the type
* @param modelConfiguration auth configuration for the model
* @param updateSchema flag to indicate if protection needs to update the schema or just the resolver. When a connection is being protected
* the schema should not be updated
*/
private protectGetQuery(
ctx: TransformerContext,
resolverResourceId: string,
rules: AuthRule[],
parent: ObjectTypeDefinitionNode | null,
modelConfiguration: ModelDirectiveConfiguration,
updateSchema: boolean,
SwaySway marked this conversation as resolved.
Show resolved Hide resolved
) {
const resolver = ctx.getResource(resolverResourceId);
if (!rules || rules.length === 0 || !resolver) {
return;
} else {
let operationName: string = undefined;

if (modelConfiguration.shouldHave('get')) {
if (modelConfiguration.shouldHave('get') && updateSchema) {
operationName = modelConfiguration.getName('get');
// If the parent type has any rules for this operation AND
// the default provider we've to get directives including the default
// as well.
const includeDefault = parent !== null ? this.isTypeHasRulesForOperation(parent, 'get') : false;
const includeDefault = parent !== null ? this.doesTypeHaveRulesForOperation(parent, 'get') : this.shouldAddDefaultAuthDirective();
const operationDirectives = this.getDirectivesForRules(rules, includeDefault);

if (operationDirectives.length > 0) {
Expand Down Expand Up @@ -1176,6 +1195,7 @@ operations will be generated by the CLI.`,
parent: ObjectTypeDefinitionNode | null,
modelConfiguration: ModelDirectiveConfiguration,
explicitOperationName: string = undefined,
updateSchema: boolean = true,
yuth marked this conversation as resolved.
Show resolved Hide resolved
) {
const resolver = ctx.getResource(resolverResourceId);
if (!rules || rules.length === 0 || !resolver) {
Expand All @@ -1187,10 +1207,10 @@ operations will be generated by the CLI.`,
// If the parent type has any rules for this operation AND
// the default provider we've to get directives including the default
// as well.
const includeDefault = parent !== null ? this.isTypeHasRulesForOperation(parent, 'list') : false;
const includeDefault = parent !== null ? this.doesTypeHaveRulesForOperation(parent, 'list') : this.shouldAddDefaultAuthDirective();
const operationDirectives = this.getDirectivesForRules(rules, includeDefault);

if (operationDirectives.length > 0) {
if (operationDirectives.length > 0 && updateSchema) {
this.addDirectivesToOperation(ctx, ctx.getQueryTypeName(), operationName, operationDirectives);
}

Expand Down Expand Up @@ -1334,7 +1354,7 @@ operations will be generated by the CLI.`,
// If the parent type has any rules for this operation AND
// the default provider we've to get directives including the default
// as well.
const includeDefault = this.isTypeHasRulesForOperation(parent, 'create');
const includeDefault = this.doesTypeHaveRulesForOperation(parent, 'create');
const operationDirectives = this.getDirectivesForRules(rules, includeDefault);

if (operationDirectives.length > 0) {
Expand Down Expand Up @@ -1461,7 +1481,7 @@ operations will be generated by the CLI.`,
// If the parent type has any rules for this operation AND
// the default provider we've to get directives including the default
// as well.
const includeDefault = Boolean(!field && this.isTypeHasRulesForOperation(parent, isUpdate ? 'update' : 'delete'));
const includeDefault = Boolean(!field && this.doesTypeHaveRulesForOperation(parent, isUpdate ? 'update' : 'delete'));
const operationDirectives = this.getDirectivesForRules(rules, includeDefault);

if (operationDirectives.length > 0) {
Expand Down Expand Up @@ -1682,9 +1702,9 @@ operations will be generated by the CLI.`,
}

if (isListType(field.type)) {
this.protectListQuery(ctx, resolverResourceId, rules, null, modelConfiguration);
this.protectListQuery(ctx, resolverResourceId, rules, null, modelConfiguration, undefined, false);
} else {
this.protectGetQuery(ctx, resolverResourceId, rules, null, modelConfiguration);
this.protectGetQuery(ctx, resolverResourceId, rules, null, modelConfiguration, false);
}
}
}
Expand Down Expand Up @@ -1713,7 +1733,7 @@ operations will be generated by the CLI.`,
for (const keyWithQuery of secondaryKeyDirectivesWithQueries) {
const args = getDirectiveArguments(keyWithQuery);
const resolverResourceId = ResolverResourceIDs.ResolverResourceID(ctx.getQueryTypeName(), args.queryField);
this.protectListQuery(ctx, resolverResourceId, rules, null, modelConfiguration, args.queryField);
this.protectListQuery(ctx, resolverResourceId, rules, null, modelConfiguration, args.queryField, true);
}
}

Expand All @@ -1723,7 +1743,7 @@ operations will be generated by the CLI.`,
return;
} else {
const operationName = resolver.Properties.FieldName;
const includeDefault = def !== null ? this.isTypeHasRulesForOperation(def, 'list') : false;
const includeDefault = def !== null ? this.doesTypeHaveRulesForOperation(def, 'list') : this.shouldAddDefaultAuthDirective();
const operationDirectives = this.getDirectivesForRules(rules, includeDefault);
if (operationDirectives.length > 0) {
this.addDirectivesToOperation(ctx, ctx.getQueryTypeName(), operationName, operationDirectives);
Expand All @@ -1749,7 +1769,7 @@ operations will be generated by the CLI.`,
return;
}
const operationName = resolver.Properties.FieldName;
const includeDefault = def !== null ? this.isTypeHasRulesForOperation(def, 'list') : false;
const includeDefault = def !== null ? this.doesTypeHaveRulesForOperation(def, 'list') : this.shouldAddDefaultAuthDirective();
const operationDirectives = this.getDirectivesForRules(rules, includeDefault);
if (operationDirectives.length > 0) {
this.addDirectivesToOperation(ctx, ctx.getQueryTypeName(), operationName, operationDirectives);
Expand Down Expand Up @@ -1833,7 +1853,7 @@ operations will be generated by the CLI.`,
ctx.setResource(resolverResourceId, resolver);
} else {
// Get the directives we need to add to the GraphQL nodes
const includeDefault = parent !== null ? this.isTypeHasRulesForOperation(parent, 'get') : false;
const includeDefault = parent !== null ? this.doesTypeHaveRulesForOperation(parent, 'get') : this.shouldAddDefaultAuthDirective();
const directives = this.getDirectivesForRules(rules, includeDefault);

if (directives.length > 0) {
Expand Down Expand Up @@ -2038,14 +2058,17 @@ operations will be generated by the CLI.`,
}
}

private getDirectivesForRules(rules: AuthRule[], addDefaultIfNeeded: boolean = true): DirectiveNode[] {
if (!rules || rules.length === 0) {
return [];
private extendAuthRulesForAdminUI(rules: Readonly<AuthRule[]>): AuthRule[] {
// Check for Amplify Admin
if (this.isAdminUIEnabled()) {
return [...rules, { allow: 'private', provider: 'iam', generateIAMPolicy: false }];
}
return [...rules];
}

// Check for Amplify Admin
if (this.configuredAuthProviders.hasIAM && this.config.addAwsIamAuthInOutputSchema) {
rules.push({ allow: 'private', provider: 'iam' });
private getDirectivesForRules(rules: Readonly<AuthRule[]>, addDefaultIfNeeded: boolean = true): DirectiveNode[] {
if (!rules || rules.length === 0) {
return [];
}

const directives: DirectiveNode[] = new Array();
Expand Down Expand Up @@ -2265,7 +2288,10 @@ found '${rule.provider}' assigned.`,
};

// Get and validate the auth rules.
return getArg('rules', []) as AuthRule[];
const authRules = getArg('rules', []) as AuthRule[];

// All the IAM auth rules that are added using @auth directive need IAM policy to be generated. AuthRules added for AdminUI don't
return authRules.map(rule => (rule.provider === 'iam' ? { ...rule, generateIAMPolicy: true } : rule));
SwaySway marked this conversation as resolved.
Show resolved Hide resolved
}

private isTypeNeedsDefaultProviderAccess(def: ObjectTypeDefinitionNode): boolean {
Expand All @@ -2282,10 +2308,10 @@ found '${rule.provider}' assigned.`,
return Boolean(rules.find(r => r.provider === this.configuredAuthProviders.default));
}

private isTypeHasRulesForOperation(def: ObjectTypeDefinitionNode, operation: ModelDirectiveOperationType): boolean {
private doesTypeHaveRulesForOperation(def: ObjectTypeDefinitionNode, operation: ModelDirectiveOperationType): boolean {
const authDirective = def.directives.find(dir => dir.name.value === 'auth');
if (!authDirective) {
return false;
return this.shouldAddDefaultAuthDirective();
}

// Get and validate the auth rules.
Expand All @@ -2301,23 +2327,37 @@ found '${rule.provider}' assigned.`,

switch (operation) {
case 'create':
return hasRulesForDefaultProvider(operationRules.create);
return (
hasRulesForDefaultProvider(operationRules.create) || (operationRules.create.length === 0 && this.shouldAddDefaultAuthDirective())
);
case 'update':
return hasRulesForDefaultProvider(operationRules.update);
return (
hasRulesForDefaultProvider(operationRules.update) || (operationRules.update.length === 0 && this.shouldAddDefaultAuthDirective())
);
case 'delete':
return hasRulesForDefaultProvider(operationRules.delete);
return (
hasRulesForDefaultProvider(operationRules.delete) || (operationRules.delete.length === 0 && this.shouldAddDefaultAuthDirective())
);
case 'get':
return hasRulesForDefaultProvider(operationRules.read) || hasRulesForDefaultProvider(queryRules.get);
return (
hasRulesForDefaultProvider(operationRules.read) ||
hasRulesForDefaultProvider(queryRules.get) ||
(operationRules.read.length === 0 && queryRules.list.length === 0 && this.shouldAddDefaultAuthDirective())
);
case 'list':
return hasRulesForDefaultProvider(operationRules.read) || hasRulesForDefaultProvider(queryRules.list);
return (
hasRulesForDefaultProvider(operationRules.read) ||
hasRulesForDefaultProvider(queryRules.list) ||
(operationRules.read.length === 0 && queryRules.list.length === 0 && this.shouldAddDefaultAuthDirective())
);
}

return false;
}

private addTypeToResourceReferences(typeName: string, rules: AuthRule[]): void {
const iamPublicRules = rules.filter(r => r.allow === 'public' && r.provider === 'iam');
const iamPrivateRules = rules.filter(r => r.allow === 'private' && r.provider === 'iam');
const iamPublicRules = rules.filter(r => r.allow === 'public' && r.provider === 'iam' && r.generateIAMPolicy);
const iamPrivateRules = rules.filter(r => r.allow === 'private' && r.provider === 'iam' && r.generateIAMPolicy);

if (iamPublicRules.length > 0) {
this.unauthPolicyResources.add(`${typeName}/null`);
Expand All @@ -2329,8 +2369,8 @@ found '${rule.provider}' assigned.`,
}

private addFieldToResourceReferences(typeName: string, fieldName: string, rules: AuthRule[]): void {
const iamPublicRules = rules.filter(r => r.allow === 'public' && r.provider === 'iam');
const iamPrivateRules = rules.filter(r => r.allow === 'private' && r.provider === 'iam');
const iamPublicRules = rules.filter(r => r.allow === 'public' && r.provider === 'iam' && r.generateIAMPolicy);
const iamPrivateRules = rules.filter(r => r.allow === 'private' && r.provider === 'iam' && r.generateIAMPolicy);

if (iamPublicRules.length > 0) {
this.unauthPolicyResources.add(`${typeName}/${fieldName}`);
Expand Down Expand Up @@ -2396,6 +2436,21 @@ found '${rule.provider}' assigned.`,
}
}

private isAdminUIEnabled(): boolean {
return this.configuredAuthProviders.hasIAM && this.config.addAwsIamAuthInOutputSchema;
}

/**
* When AdminUI is enabled, all the types and operations get IAM auth. If the default auth mode is
* not IAM all the fields will need to have the default auth mode directive to ensure both IAM and deault
* auth modes are allowed to access
* default auth provider needs to be added if AdminUI is enabled and default auth type is not IAM
* @returns boolean
*/
private shouldAddDefaultAuthDirective(): boolean {
return this.isAdminUIEnabled() && this.config.authConfig.defaultAuthentication.authenticationType !== 'AWS_IAM';
}

private typeExist(type: string, ctx: TransformerContext): boolean {
return Boolean(type in ctx.nodeMap);
}
Expand Down
Loading