Skip to content

Commit

Permalink
fix: auth vnext validation fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
SwaySway committed Oct 26, 2021
1 parent 8991e78 commit 5eb6a0e
Show file tree
Hide file tree
Showing 5 changed files with 91 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ test('test access control on object and field', () => {
const studentOwnerRole = 'userPools:owner:studentID';
const studentTypeFields = ['studentID', 'name', 'email', 'ssn'];
const acm = new AccessControlMatrix({
name: 'Student',
resources: studentTypeFields,
operations: MODEL_OPERATIONS,
});
Expand Down Expand Up @@ -90,6 +91,7 @@ test('test access control only on field', () => {
const studentOwnerRole = 'userPools:owner:studentID';
const studentTypeFields = ['studentID', 'name', 'email', 'ssn'];
const acm = new AccessControlMatrix({
name: 'Student',
resources: studentTypeFields,
operations: MODEL_OPERATIONS,
});
Expand All @@ -110,3 +112,21 @@ test('test access control only on field', () => {
expect(acm.isAllowed(studentOwnerRole, 'ssn', 'update')).toBe(false);
expect(acm.isAllowed(studentOwnerRole, 'ssn', 'delete')).toBe(false);
});

test('that that adding the a role again without a resource is not allowed', () => {
const blogOwnerRole = 'userPools:owner';
const blogFields = ['id', 'owner', 'name', 'content'];
const acm = new AccessControlMatrix({
name: 'Blog',
resources: blogFields,
operations: MODEL_OPERATIONS,
});
acm.setRole({ role: blogOwnerRole, operations: MODEL_OPERATIONS });
for (let field of blogFields) {
expect(acm.isAllowed(blogOwnerRole, field, 'create')).toBe(true);
expect(acm.isAllowed(blogOwnerRole, field, 'read')).toBe(true);
expect(acm.isAllowed(blogOwnerRole, field, 'update')).toBe(true);
expect(acm.isAllowed(blogOwnerRole, field, 'delete')).toBe(true);
}
expect(() => acm.setRole({ role: blogOwnerRole, operations: ['read'] })).toThrow(`@auth ${blogOwnerRole} already exists for Blog`);
});
Original file line number Diff line number Diff line change
Expand Up @@ -95,3 +95,47 @@ test('validation on @auth on a non-@model type', () => {
});
expect(() => transformer.transform(invalidSchema)).toThrowError('Types annotated with @auth must also be annotated with @model.');
});

test('empty groups list', () => {
const authConfig: AppSyncAuthConfiguration = {
defaultAuthentication: {
authenticationType: 'AMAZON_COGNITO_USER_POOLS',
},
additionalAuthenticationProviders: [],
};
const invalidSchema = `
type Post @model @auth(rules: [{ allow: groups, groups: [] }]) {
id: ID!
title: String!
group: String
createdAt: String
updatedAt: String
}`;
const transformer = new GraphQLTransform({
authConfig,
transformers: [new ModelTransformer(), new AuthTransformer()],
});
expect(() => transformer.transform(invalidSchema)).toThrowError('@auth rules using groups cannot have an empty list');
});

test('no @auth rules list', () => {
const authConfig: AppSyncAuthConfiguration = {
defaultAuthentication: {
authenticationType: 'AMAZON_COGNITO_USER_POOLS',
},
additionalAuthenticationProviders: [],
};
const invalidSchema = `
type Post @model @auth(rules: []) {
id: ID!
title: String!
group: String
createdAt: String
updatedAt: String
}`;
const transformer = new GraphQLTransform({
authConfig,
transformers: [new ModelTransformer(), new AuthTransformer()],
});
expect(() => transformer.transform(invalidSchema)).toThrowError('@auth on Post does not have any auth rules.');
});
15 changes: 11 additions & 4 deletions packages/amplify-graphql-auth-transformer/src/accesscontrol/acm.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import assert from 'assert';
import { InvalidDirectiveError, TransformerContractError } from '@aws-amplify/graphql-transformer-core';

type ACMConfig = {
resources: string[];
operations: string[];
name: string;
};

type SetRoleInput = {
Expand Down Expand Up @@ -31,12 +33,14 @@ type ResourceOperationInput = {
* - Operations
*/
export class AccessControlMatrix {
private name: string;
private roles: Array<string>;
private operations: Array<string>;
private resources: Array<string>;
private matrix: Array<Array<Array<boolean>>>;

constructor(config: ACMConfig) {
this.name = config.name;
this.operations = config.operations;
this.resources = config.resources;
this.matrix = new Array();
Expand All @@ -52,10 +56,12 @@ export class AccessControlMatrix {
this.roles.push(input.role);
this.matrix.push(allowedVector);
assert(this.roles.length === this.matrix.length, 'Roles are not aligned with Roles added in Matrix');
} else {
} else if (this.roles.includes(role) && resource) {
allowedVector = this.getResourceOperationMatrix({ operations, resource, role });
const roleIndex = this.roles.indexOf(role);
this.matrix[roleIndex] = allowedVector;
} else {
throw new InvalidDirectiveError(`@auth ${role} already exists for ${this.name}`);
}
}

Expand Down Expand Up @@ -142,14 +148,15 @@ export class AccessControlMatrix {
*/
private validate(input: ValidateInput) {
if (input.resource && !this.resources.includes(input.resource)) {
throw Error(`Resource: ${input.resource} is not configued in the ACM`);
throw new TransformerContractError(`Resource: ${input.resource} is not configued in the ACM`);
}
if (input.role && !this.roles.includes(input.role)) {
throw Error(`Role: ${input.role} does not exist in ACM.`);
throw new TransformerContractError(`Role: ${input.role} does not exist in ACM.`);
}
if (input.operations) {
input.operations.forEach(operation => {
if (this.operations.indexOf(operation) === -1) throw Error(`Operation: ${operation} does not exist in the ACM.`);
if (this.operations.indexOf(operation) === -1)
throw new TransformerContractError(`Operation: ${operation} does not exist in the ACM.`);
});
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,9 +141,10 @@ export class AuthTransformer extends TransformerAuthBase implements TransformerA
const rules: AuthRule[] = authDir.getArguments<{ rules: Array<AuthRule> }>({ rules: [] }).rules;
ensureAuthRuleDefaults(rules);
// validate rules
validateRules(rules, this.configuredAuthProviders);
validateRules(rules, this.configuredAuthProviders, def.name.value);
// create access control for object
const acm = new AccessControlMatrix({
name: def.name.value,
operations: MODEL_OPERATIONS,
resources: collectFieldNames(def),
});
Expand Down Expand Up @@ -191,7 +192,7 @@ Static group authorization should perform as expected.`,
const authDir = new DirectiveWrapper(directive);
const rules: AuthRule[] = authDir.getArguments<{ rules: Array<AuthRule> }>({ rules: [] }).rules;
ensureAuthRuleDefaults(rules);
validateFieldRules(rules, isParentTypeBuiltinType, modelDirective !== undefined, this.configuredAuthProviders);
validateFieldRules(rules, isParentTypeBuiltinType, modelDirective !== undefined, this.configuredAuthProviders, field.name.value);

// regardless if a model directive is used we generate the policy for iam auth
this.setAuthPolicyFlag(rules);
Expand All @@ -205,6 +206,7 @@ Static group authorization should perform as expected.`,
if (!this.modelDirectiveConfig.has(typeName)) {
this.modelDirectiveConfig.set(typeName, getModelConfig(modelDirective, typeName, context.isProjectUsingDataStore()));
acm = new AccessControlMatrix({
name: parent.name.value,
operations: MODEL_OPERATIONS,
resources: collectFieldNames(parent),
});
Expand All @@ -221,6 +223,7 @@ Static group authorization should perform as expected.`,
const staticRules = rules.filter((rule: AuthRule) => rule.allow !== 'owner' && !rule.groupsField);
const typeFieldName = `${typeName}:${fieldName}`;
const acm = new AccessControlMatrix({
name: typeFieldName,
operations: ['read'],
resources: [typeFieldName],
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,10 @@ found '${rule.provider}' assigned.`,
}
};

export const validateRules = (rules: AuthRule[], configuredAuthProviders: ConfiguredAuthProviders) => {
export const validateRules = (rules: AuthRule[], configuredAuthProviders: ConfiguredAuthProviders, typeName: string) => {
if (rules.length === 0) {
throw new InvalidDirectiveError(`@auth on ${typeName} does not have any auth rules.`);
}
for (const rule of rules) {
validateRuleAuthStrategy(rule, configuredAuthProviders);
commonRuleValidation(rule);
Expand All @@ -84,7 +87,11 @@ export const validateFieldRules = (
isParentTypeBuiltinType: boolean,
parentHasModelDirective: boolean,
authProviderConfig: ConfiguredAuthProviders,
fieldName: string,
) => {
if (rules.length === 0) {
throw new InvalidDirectiveError(`@auth on ${fieldName} does not have any auth rules.`);
}
for (const rule of rules) {
validateRuleAuthStrategy(rule, authProviderConfig);

Expand Down Expand Up @@ -120,4 +127,7 @@ export const commonRuleValidation = (rule: AuthRule) => {
if (groupsField && groups) {
throw new InvalidDirectiveError('This rule has groupsField and groups, please use one or the other');
}
if (allow === 'groups' && groups && groups.length < 1) {
throw new InvalidDirectiveError('@auth rules using groups cannot have an empty list');
}
};

0 comments on commit 5eb6a0e

Please sign in to comment.