Skip to content

Commit

Permalink
fix: use managedpolicies and slice them (#2883)
Browse files Browse the repository at this point in the history 
Use managed policies instead of inline policies for GraphQL API policy generation for auth and unauth roles. Also take into account the policy size and create multiple policies of needed. This means that 6144 bytes long can be 1 policy and 10 Managed Policies can be attached to a single role and that 10 can be raised to 20 by AWS Support, which raises the maximum policy size to ~120kb, which is 10 times the currently supported size.

This PR does not solve the policy size issue for API Gateway (#2084), but since sizes are adding up, perhaps customers can be unblocked by this change.
  • Loading branch information
@yuth
Attila Hajdrik authored and yuth committed Nov 30, 2019
1 parent 0449e88 commit fa0f2ed
Show file tree
Hide file tree
Showing 5 changed files with 3,017 additions and 2,858 deletions.
43 changes: 19 additions & 24 deletions packages/graphql-auth-transformer/src/ModelAuthTransformer.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import {
ResolverResourceIDs,
isListType,
getBaseType,
getDirectiveArgument,
makeDirective,
makeNamedType,
makeInputValueDefinition,
Expand All @@ -38,23 +37,7 @@ import {
makeField,
ModelResourceIDs,
} from 'graphql-transformer-common';
import {
Expression,
print,
raw,
iff,
forEach,
set,
ref,
list,
compoundExpression,
or,
newline,
comment,
and,
not,
parens,
} from 'graphql-mapping-template';
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';
Expand Down Expand Up @@ -296,9 +279,15 @@ export class ModelAuthTransformer extends Transformer {
}),
});

ctx.mergeResources({
[ResourceConstants.RESOURCES.AuthRolePolicy]: 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],
});
}
}

if (this.generateIAMPolicyforUnauthRole === true) {
Expand All @@ -313,9 +302,15 @@ export class ModelAuthTransformer extends Transformer {
}),
});

ctx.mergeResources({
[ResourceConstants.RESOURCES.UnauthRolePolicy]: this.resources.makeIAMPolicyForRole(false, this.unauthPolicyResources),
});
const unauthPolicies = this.resources.makeIAMPolicyForRole(false, this.unauthPolicyResources);

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

Expand Down
76 changes: 74 additions & 2 deletions packages/graphql-auth-transformer/src/__tests__/MultiAuth.test.ts
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { ObjectTypeDefinitionNode, parse, DocumentNode, Kind } from 'graphql';
import { GraphQLTransform } from 'graphql-transformer-core';
import { ResourceConstants } from 'graphql-transformer-common';
import { DynamoDBModelTransformer } from 'graphql-dynamodb-transformer';
import { ModelConnectionTransformer } from 'graphql-connection-transformer';
import { ModelAuthTransformer, AppSyncAuthConfiguration, AppSyncAuthMode } from '../ModelAuthTransformer';
Expand Down Expand Up @@ -320,7 +319,7 @@ describe('Type directive transformation tests', () => {

const out = transformer.transform(schema);

expect(out.rootStack.Resources[ResourceConstants.RESOURCES.UnauthRolePolicy]).toBeDefined();
expect(out.rootStack.Resources.UnauthRolePolicy01).toBeDefined();
});

test(`Field level @auth is propagated to type and the type related operations`, () => {
Expand Down Expand Up @@ -438,3 +437,76 @@ describe('Type directive transformation tests', () => {
// expect (expectOne(modelPostConnectionType, 'aws_cognito_user_pools'));
// });
});

describe(`Policy slicing tests`, () => {
test(`'For the long Todo type there should be 2 auth role managed policies generated`, () => {
const schema = `
type TodoWithExtraLongLongLongLongLongLongLongLongLongLongLongLongLongLongLongName @model(subscriptions:null) @auth(rules:[{allow: private, provider: iam}])
{
id: ID!
namenamenamenamenamenamenamenamenamenamenamenamenamenamename001: String! @auth(rules:[{allow: private, provider: iam}])
namenamenamenamenamenamenamenamenamenamenamenamenamenamename002: String! @auth(rules:[{allow: private, provider: iam}])
namenamenamenamenamenamenamenamenamenamenamenamenamenamename003: String! @auth(rules:[{allow: private, provider: iam}])
namenamenamenamenamenamenamenamenamenamenamenamenamenamename004: String! @auth(rules:[{allow: private, provider: iam}])
namenamenamenamenamenamenamenamenamenamenamenamenamenamename005: String! @auth(rules:[{allow: private, provider: iam}])
namenamenamenamenamenamenamenamenamenamenamenamenamenamename006: String! @auth(rules:[{allow: private, provider: iam}])
namenamenamenamenamenamenamenamenamenamenamenamenamenamename007: String! @auth(rules:[{allow: private, provider: iam}])
namenamenamenamenamenamenamenamenamenamenamenamenamenamename008: String! @auth(rules:[{allow: private, provider: iam}])
namenamenamenamenamenamenamenamenamenamenamenamenamenamename009: String! @auth(rules:[{allow: private, provider: iam}])
namenamenamenamenamenamenamenamenamenamenamenamenamenamename010: String! @auth(rules:[{allow: private, provider: iam}])
namenamenamenamenamenamenamenamenamenamenamenamenamenamename011: String! @auth(rules:[{allow: private, provider: iam}])
namenamenamenamenamenamenamenamenamenamenamenamenamenamename012: String! @auth(rules:[{allow: private, provider: iam}])
namenamenamenamenamenamenamenamenamenamenamenamenamenamename013: String! @auth(rules:[{allow: private, provider: iam}])
namenamenamenamenamenamenamenamenamenamenamenamenamenamename014: String! @auth(rules:[{allow: private, provider: iam}])
namenamenamenamenamenamenamenamenamenamenamenamenamenamename015: String! @auth(rules:[{allow: private, provider: iam}])
namenamenamenamenamenamenamenamenamenamenamenamenamenamename016: String! @auth(rules:[{allow: private, provider: iam}])
namenamenamenamenamenamenamenamenamenamenamenamenamenamename017: String! @auth(rules:[{allow: private, provider: iam}])
namenamenamenamenamenamenamenamenamenamenamenamenamenamename018: String! @auth(rules:[{allow: private, provider: iam}])
namenamenamenamenamenamenamenamenamenamenamenamenamenamename019: String! @auth(rules:[{allow: private, provider: iam}])
namenamenamenamenamenamenamenamenamenamenamenamenamenamename020: String! @auth(rules:[{allow: private, provider: iam}])
namenamenamenamenamenamenamenamenamenamenamenamenamenamename021: String! @auth(rules:[{allow: private, provider: iam}])
namenamenamenamenamenamenamenamenamenamenamenamenamenamename022: String! @auth(rules:[{allow: private, provider: iam}])
namenamenamenamenamenamenamenamenamenamenamenamenamenamename023: String! @auth(rules:[{allow: private, provider: iam}])
namenamenamenamenamenamenamenamenamenamenamenamenamenamename024: String! @auth(rules:[{allow: private, provider: iam}])
namenamenamenamenamenamenamenamenamenamenamenamenamenamename025: String! @auth(rules:[{allow: private, provider: iam}])
namenamenamenamenamenamenamenamenamenamenamenamenamenamename026: String! @auth(rules:[{allow: private, provider: iam}])
namenamenamenamenamenamenamenamenamenamenamenamenamenamename027: String! @auth(rules:[{allow: private, provider: iam}])
namenamenamenamenamenamenamenamenamenamenamenamenamenamename028: String! @auth(rules:[{allow: private, provider: iam}])
namenamenamenamenamenamenamenamenamenamenamenamenamenamename029: String! @auth(rules:[{allow: private, provider: iam}])
namenamenamenamenamenamenamenamenamenamenamenamenamenamename030: String! @auth(rules:[{allow: private, provider: iam}])
namenamenamenamenamenamenamenamenamenamenamenamenamenamename031: String! @auth(rules:[{allow: private, provider: iam}])
namenamenamenamenamenamenamenamenamenamenamenamenamenamename032: String! @auth(rules:[{allow: private, provider: iam}])
namenamenamenamenamenamenamenamenamenamenamenamenamenamename033: String! @auth(rules:[{allow: private, provider: iam}])
namenamenamenamenamenamenamenamenamenamenamenamenamenamename034: String! @auth(rules:[{allow: private, provider: iam}])
namenamenamenamenamenamenamenamenamenamenamenamenamenamename035: String! @auth(rules:[{allow: private, provider: iam}])
namenamenamenamenamenamenamenamenamenamenamenamenamenamename036: String! @auth(rules:[{allow: private, provider: iam}])
namenamenamenamenamenamenamenamenamenamenamenamenamenamename037: String! @auth(rules:[{allow: private, provider: iam}])
namenamenamenamenamenamenamenamenamenamenamenamenamenamename038: String! @auth(rules:[{allow: private, provider: iam}])
namenamenamenamenamenamenamenamenamenamenamenamenamenamename039: String! @auth(rules:[{allow: private, provider: iam}])
namenamenamenamenamenamenamenamenamenamenamenamenamenamename040: String! @auth(rules:[{allow: private, provider: iam}])
namenamenamenamenamenamenamenamenamenamenamenamenamenamename041: String! @auth(rules:[{allow: private, provider: iam}])
namenamenamenamenamenamenamenamenamenamenamenamenamenamename042: String! @auth(rules:[{allow: private, provider: iam}])
namenamenamenamenamenamenamenamenamenamenamenamenamenamename043: String! @auth(rules:[{allow: private, provider: iam}])
namenamenamenamenamenamenamenamenamenamenamenamenamenamename044: String! @auth(rules:[{allow: private, provider: iam}])
namenamenamenamenamenamenamenamenamenamenamenamenamenamename045: String! @auth(rules:[{allow: private, provider: iam}])
namenamenamenamenamenamenamenamenamenamenamenamenamenamename046: String! @auth(rules:[{allow: private, provider: iam}])
namenamenamenamenamenamenamenamenamenamenamenamenamenamename047: String! @auth(rules:[{allow: private, provider: iam}])
namenamenamenamenamenamenamenamenamenamenamenamenamenamename048: String! @auth(rules:[{allow: private, provider: iam}])
namenamenamenamenamenamenamenamenamenamenamenamenamenamename049: String! @auth(rules:[{allow: private, provider: iam}])
namenamenamenamenamenamenamenamenamenamenamenamenamenamename050: String! @auth(rules:[{allow: private, provider: iam}])
description: String
}
`;
const authConfig = withAuthModes(apiKeyDefaultConfig, ['AMAZON_COGNITO_USER_POOLS', 'AWS_IAM']);
const transformer = getTransformer(authConfig);
const out = transformer.transform(schema);

expect(out.rootStack.Resources.AuthRolePolicy01).toBeTruthy();
expect(out.rootStack.Resources.AuthRolePolicy01.Properties.PolicyDocument.Statement[0].Resource.length).toEqual(27);
expect(out.rootStack.Resources.AuthRolePolicy02).toBeTruthy();
expect(out.rootStack.Resources.AuthRolePolicy02.Properties.PolicyDocument.Statement[0].Resource.length).toEqual(27);
expect(out.rootStack.Resources.AuthRolePolicy03).toBeTruthy();
expect(out.rootStack.Resources.AuthRolePolicy03.Properties.PolicyDocument.Statement[0].Resource.length).toEqual(2);
expect(out.rootStack.Resources.UnauthRolePolicy01).toBeFalsy();
});
});
74 changes: 54 additions & 20 deletions packages/graphql-auth-transformer/src/resources.ts
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import Template from 'cloudform-types/types/template';
import Policy from 'cloudform-types/types/iam/policy';
import { AppSync, Fn, StringParameter, Refs, NumberParameter, IAM, Value } from 'cloudform-types';
import { AuthRule, AuthProvider } from './AuthRule';
import {
Expand Down Expand Up @@ -37,6 +36,7 @@ import * as Transformer from './ModelAuthTransformer';
import { FieldDefinitionNode } from 'graphql';

import { DEFAULT_OWNER_FIELD, DEFAULT_IDENTITY_FIELD, DEFAULT_GROUPS_FIELD, DEFAULT_GROUP_CLAIM } from './constants';
import ManagedPolicy from 'cloudform-types/types/iam/managedPolicy';

function replaceIfUsername(identityClaim: string): string {
return identityClaim === 'username' ? 'cognito:username' : identityClaim;
Expand Down Expand Up @@ -973,9 +973,34 @@ identityClaim: "${rule.identityField || rule.identityClaim || DEFAULT_IDENTITY_F
: ResourceConstants.SNIPPETS.IsStaticGroupAuthorizedVariable;
}

public makeIAMPolicyForRole(isAuthPolicy: Boolean, resources: Set<string>): Policy {
public makeIAMPolicyForRole(isAuthPolicy: Boolean, resources: Set<string>): ManagedPolicy[] {
const policies = new Array<ManagedPolicy>();
const authPiece = isAuthPolicy ? 'auth' : 'unauth';
const policyResources: object[] = [];
let policyResources: object[] = [];
let resourceSize = 0;

// 6144 bytes is the maximum policy payload size, but there is structural overhead, hence the 6000 bytes
const MAX_BUILT_SIZE_BYTES = 6000;
// The overhead is the amount of static policy arn contents like region, accountid, etc.
const RESOURCE_OVERHEAD = 90;

const createPolicy = newPolicyResources =>
new IAM.ManagedPolicy({
Roles: [
//HACK double casting needed because it cannot except Ref
({ Ref: `${authPiece}RoleName` } as unknown) as Value<string>,
],
PolicyDocument: {
Version: '2012-10-17',
Statement: [
{
Effect: 'Allow',
Action: ['appsync:GraphQL'],
Resource: newPolicyResources,
},
],
},
});

for (const resource of resources) {
// We always have 2 parts, no need to check
Expand All @@ -991,6 +1016,8 @@ identityClaim: "${rule.identityField || rule.identityClaim || DEFAULT_IDENTITY_F
fieldName: resourceParts[1],
})
);

resourceSize += RESOURCE_OVERHEAD + resourceParts[0].length + resourceParts[1].length;
} else {
policyResources.push(
Fn.Sub('arn:aws:appsync:${AWS::Region}:${AWS::AccountId}:apis/${apiId}/types/${typeName}/*', {
Expand All @@ -1000,26 +1027,33 @@ identityClaim: "${rule.identityField || rule.identityClaim || DEFAULT_IDENTITY_F
typeName: resourceParts[0],
})
);

resourceSize += RESOURCE_OVERHEAD + resourceParts[0].length;
}

//
// Check policy size and if needed create a new one and clear the resources, reset
// accumulated size
//

if (resourceSize > MAX_BUILT_SIZE_BYTES) {
const policy = createPolicy(policyResources.slice(0, policyResources.length - 1));

policies.push(policy);

// Remove all but the last item
policyResources = policyResources.slice(-1);
resourceSize = 0;
}
}

return new IAM.Policy({
PolicyName: `appsync-${authPiece}role-policy`,
Roles: [
//HACK double casting needed because it cannot except Ref
({ Ref: `${authPiece}RoleName` } as unknown) as Value<string>,
],
PolicyDocument: {
Version: '2012-10-17',
Statement: [
{
Effect: 'Allow',
Action: ['appsync:GraphQL'],
Resource: policyResources,
},
],
},
});
if (policyResources.length > 0) {
const policy = createPolicy(policyResources);

policies.push(policy);
}

return policies;
}

/**
Expand Down
Loading

0 comments on commit fa0f2ed

Please sign in to comment.