From 287f808cf34870295e4032995fc083ac204b64c3 Mon Sep 17 00:00:00 2001 From: Bryan Pan Date: Wed, 9 Sep 2020 15:30:22 -0700 Subject: [PATCH] fix(appsync): strongly type `expires` prop in apiKeyConfig (#9122) **[ISSUE]** `apiKeyConfig` has prop `expires` that has unclear documentation/not strongly typed and is prone to user errors. **[APPROACH]** Force `expires` to take `Expiration` class from `core` and will be able to output api key configurations easily through `Expiration` static functions: `after(...)`, `fromString(...)`, ` atDate(...)`, `atTimeStamp(...)`. Fixes #8698 BREAKING CHANGE: force `apiKeyConfig` require a Expiration class instead of string - **appsync**: Parameter `apiKeyConfig` takes `Expiration` class instead of `string` ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .../@aws-cdk/aws-appsync/lib/graphqlapi.ts | 21 +- .../aws-appsync/test/appsync-auth.test.ts | 65 ++++++ .../aws-appsync/test/appsync.auth.graphql | 12 ++ .../test/integ.auth-apikey.expected.json | 186 ++++++++++++++++++ .../aws-appsync/test/integ.auth-apikey.ts | 63 ++++++ .../test/verify.integ.auth-apikey.sh | 37 ++++ 6 files changed, 371 insertions(+), 13 deletions(-) create mode 100644 packages/@aws-cdk/aws-appsync/test/appsync.auth.graphql create mode 100644 packages/@aws-cdk/aws-appsync/test/integ.auth-apikey.expected.json create mode 100644 packages/@aws-cdk/aws-appsync/test/integ.auth-apikey.ts create mode 100644 packages/@aws-cdk/aws-appsync/test/verify.integ.auth-apikey.sh diff --git a/packages/@aws-cdk/aws-appsync/lib/graphqlapi.ts b/packages/@aws-cdk/aws-appsync/lib/graphqlapi.ts index f3ae3546ff130..3b99fbaed592c 100644 --- a/packages/@aws-cdk/aws-appsync/lib/graphqlapi.ts +++ b/packages/@aws-cdk/aws-appsync/lib/graphqlapi.ts @@ -1,6 +1,6 @@ import { IUserPool } from '@aws-cdk/aws-cognito'; import { ManagedPolicy, Role, ServicePrincipal, Grant, IGrantable } from '@aws-cdk/aws-iam'; -import { CfnResource, Construct, Duration, IResolvable, Stack } from '@aws-cdk/core'; +import { CfnResource, Construct, Duration, Expiration, IResolvable, Stack } from '@aws-cdk/core'; import { CfnApiKey, CfnGraphQLApi, CfnGraphQLSchema } from './appsync.generated'; import { IGraphqlApi, GraphqlApiBase } from './graphqlapi-base'; import { Schema } from './schema'; @@ -111,12 +111,13 @@ export interface ApiKeyConfig { readonly description?: string; /** - * The time from creation time after which the API key expires, using RFC3339 representation. + * The time from creation time after which the API key expires. * It must be a minimum of 1 day and a maximum of 365 days from date of creation. * Rounded down to the nearest hour. - * @default - 7 days from creation time + * + * @default - 7 days rounded down to nearest hour */ - readonly expires?: string; + readonly expires?: Expiration; } /** @@ -556,16 +557,10 @@ export class GraphqlApi extends GraphqlApiBase { } private createAPIKey(config?: ApiKeyConfig) { - let expires: number | undefined; - if (config?.expires) { - expires = new Date(config.expires).valueOf(); - const days = (d: number) => - Date.now() + Duration.days(d).toMilliseconds(); - if (expires < days(1) || expires > days(365)) { - throw Error('API key expiration must be between 1 and 365 days.'); - } - expires = Math.round(expires / 1000); + if (config?.expires?.isBefore(Duration.days(1)) || config?.expires?.isAfter(Duration.days(365))) { + throw Error('API key expiration must be between 1 and 365 days.'); } + const expires = config?.expires ? config?.expires.toEpoch() : undefined; return new CfnApiKey(this, `${config?.name || 'Default'}ApiKey`, { expires, description: config?.description, diff --git a/packages/@aws-cdk/aws-appsync/test/appsync-auth.test.ts b/packages/@aws-cdk/aws-appsync/test/appsync-auth.test.ts index 5815908198feb..fbac8fcd07350 100644 --- a/packages/@aws-cdk/aws-appsync/test/appsync-auth.test.ts +++ b/packages/@aws-cdk/aws-appsync/test/appsync-auth.test.ts @@ -88,6 +88,71 @@ describe('AppSync API Key Authorization', () => { }); }); + test('apiKeyConfig creates default with valid expiration date', () => { + const expirationDate: number = cdk.Expiration.after(cdk.Duration.days(10)).toEpoch(); + + // WHEN + new appsync.GraphqlApi(stack, 'API', { + name: 'apiKeyUnitTest', + schema: appsync.Schema.fromAsset(path.join(__dirname, 'appsync.auth.graphql')), + authorizationConfig: { + defaultAuthorization: { + authorizationType: appsync.AuthorizationType.API_KEY, + apiKeyConfig: { + expires: cdk.Expiration.after(cdk.Duration.days(10)), + }, + }, + }, + }); + // THEN + expect(stack).toHaveResourceLike('AWS::AppSync::ApiKey', { + ApiId: { 'Fn::GetAtt': ['API62EA1CFF', 'ApiId'] }, + Expires: expirationDate, + }); + }); + + test('apiKeyConfig fails if expire argument less than a day', () => { + // WHEN + const when = () => { + new appsync.GraphqlApi(stack, 'API', { + name: 'apiKeyUnitTest', + schema: appsync.Schema.fromAsset(path.join(__dirname, 'appsync.auth.graphql')), + authorizationConfig: { + defaultAuthorization: { + authorizationType: appsync.AuthorizationType.API_KEY, + apiKeyConfig: { + expires: cdk.Expiration.after(cdk.Duration.hours(1)), + }, + }, + }, + }); + }; + + // THEN + expect(when).toThrowError('API key expiration must be between 1 and 365 days.'); + }); + + test('apiKeyConfig fails if expire argument greater than 365 day', () => { + // WHEN + const when = () => { + new appsync.GraphqlApi(stack, 'API', { + name: 'apiKeyUnitTest', + schema: appsync.Schema.fromAsset(path.join(__dirname, 'appsync.auth.graphql')), + authorizationConfig: { + defaultAuthorization: { + authorizationType: appsync.AuthorizationType.API_KEY, + apiKeyConfig: { + expires: cdk.Expiration.after(cdk.Duration.days(366)), + }, + }, + }, + }); + }; + + // THEN + expect(when).toThrowError('API key expiration must be between 1 and 365 days.'); + }); + test('appsync creates configured api key with additionalAuthorizationModes (not as first element)', () => { // WHEN new appsync.GraphqlApi(stack, 'api', { diff --git a/packages/@aws-cdk/aws-appsync/test/appsync.auth.graphql b/packages/@aws-cdk/aws-appsync/test/appsync.auth.graphql new file mode 100644 index 0000000000000..e59469dedfd10 --- /dev/null +++ b/packages/@aws-cdk/aws-appsync/test/appsync.auth.graphql @@ -0,0 +1,12 @@ +type test { + id: Int! + version: String! +} + +type Query { + getTests: [ test! ] +} + +type Mutation { + addTest(version: String!): test! +} \ No newline at end of file diff --git a/packages/@aws-cdk/aws-appsync/test/integ.auth-apikey.expected.json b/packages/@aws-cdk/aws-appsync/test/integ.auth-apikey.expected.json new file mode 100644 index 0000000000000..17fec6fbed787 --- /dev/null +++ b/packages/@aws-cdk/aws-appsync/test/integ.auth-apikey.expected.json @@ -0,0 +1,186 @@ +{ + "Resources": { + "ApiF70053CD": { + "Type": "AWS::AppSync::GraphQLApi", + "Properties": { + "AuthenticationType": "API_KEY", + "Name": "Integ_Test_APIKey" + } + }, + "ApiSchema510EECD7": { + "Type": "AWS::AppSync::GraphQLSchema", + "Properties": { + "ApiId": { + "Fn::GetAtt": [ + "ApiF70053CD", + "ApiId" + ] + }, + "Definition": "type test {\n id: Int!\n version: String!\n}\n\ntype Query {\n getTests: [ test! ]\n}\n\ntype Mutation {\n addTest(version: String!): test!\n}" + } + }, + "ApiDefaultApiKeyF991C37B": { + "Type": "AWS::AppSync::ApiKey", + "Properties": { + "ApiId": { + "Fn::GetAtt": [ + "ApiF70053CD", + "ApiId" + ] + }, + "Expires": 1626566400 + }, + "DependsOn": [ + "ApiSchema510EECD7" + ] + }, + "ApitestDataSourceServiceRoleACBC3F3D": { + "Type": "AWS::IAM::Role", + "Properties": { + "AssumeRolePolicyDocument": { + "Statement": [ + { + "Action": "sts:AssumeRole", + "Effect": "Allow", + "Principal": { + "Service": "appsync.amazonaws.com" + } + } + ], + "Version": "2012-10-17" + } + } + }, + "ApitestDataSourceServiceRoleDefaultPolicy897CD912": { + "Type": "AWS::IAM::Policy", + "Properties": { + "PolicyDocument": { + "Statement": [ + { + "Action": [ + "dynamodb:BatchGetItem", + "dynamodb:GetRecords", + "dynamodb:GetShardIterator", + "dynamodb:Query", + "dynamodb:GetItem", + "dynamodb:Scan", + "dynamodb:BatchWriteItem", + "dynamodb:PutItem", + "dynamodb:UpdateItem", + "dynamodb:DeleteItem" + ], + "Effect": "Allow", + "Resource": [ + { + "Fn::GetAtt": [ + "TestTable5769773A", + "Arn" + ] + }, + { + "Ref": "AWS::NoValue" + } + ] + } + ], + "Version": "2012-10-17" + }, + "PolicyName": "ApitestDataSourceServiceRoleDefaultPolicy897CD912", + "Roles": [ + { + "Ref": "ApitestDataSourceServiceRoleACBC3F3D" + } + ] + } + }, + "ApitestDataSource96AE54D5": { + "Type": "AWS::AppSync::DataSource", + "Properties": { + "ApiId": { + "Fn::GetAtt": [ + "ApiF70053CD", + "ApiId" + ] + }, + "Name": "testDataSource", + "Type": "AMAZON_DYNAMODB", + "DynamoDBConfig": { + "AwsRegion": { + "Ref": "AWS::Region" + }, + "TableName": { + "Ref": "TestTable5769773A" + } + }, + "ServiceRoleArn": { + "Fn::GetAtt": [ + "ApitestDataSourceServiceRoleACBC3F3D", + "Arn" + ] + } + } + }, + "ApitestDataSourceQuerygetTestsResolverA3BBB672": { + "Type": "AWS::AppSync::Resolver", + "Properties": { + "ApiId": { + "Fn::GetAtt": [ + "ApiF70053CD", + "ApiId" + ] + }, + "FieldName": "getTests", + "TypeName": "Query", + "DataSourceName": "testDataSource", + "Kind": "UNIT", + "RequestMappingTemplate": "{\"version\" : \"2017-02-28\", \"operation\" : \"Scan\"}", + "ResponseMappingTemplate": "$util.toJson($ctx.result.items)" + }, + "DependsOn": [ + "ApiSchema510EECD7", + "ApitestDataSource96AE54D5" + ] + }, + "ApitestDataSourceMutationaddTestResolver36203D6B": { + "Type": "AWS::AppSync::Resolver", + "Properties": { + "ApiId": { + "Fn::GetAtt": [ + "ApiF70053CD", + "ApiId" + ] + }, + "FieldName": "addTest", + "TypeName": "Mutation", + "DataSourceName": "testDataSource", + "Kind": "UNIT", + "RequestMappingTemplate": "\n #set($input = $ctx.args.test)\n \n {\n \"version\": \"2017-02-28\",\n \"operation\": \"PutItem\",\n \"key\" : {\n \"id\" : $util.dynamodb.toDynamoDBJson($util.autoId())\n },\n \"attributeValues\": $util.dynamodb.toMapValuesJson($input)\n }", + "ResponseMappingTemplate": "$util.toJson($ctx.result)" + }, + "DependsOn": [ + "ApiSchema510EECD7", + "ApitestDataSource96AE54D5" + ] + }, + "TestTable5769773A": { + "Type": "AWS::DynamoDB::Table", + "Properties": { + "KeySchema": [ + { + "AttributeName": "id", + "KeyType": "HASH" + } + ], + "AttributeDefinitions": [ + { + "AttributeName": "id", + "AttributeType": "S" + } + ], + "BillingMode": "PAY_PER_REQUEST" + }, + "UpdateReplacePolicy": "Delete", + "DeletionPolicy": "Delete" + } + } +} \ No newline at end of file diff --git a/packages/@aws-cdk/aws-appsync/test/integ.auth-apikey.ts b/packages/@aws-cdk/aws-appsync/test/integ.auth-apikey.ts new file mode 100644 index 0000000000000..5ddcb9abd1dbb --- /dev/null +++ b/packages/@aws-cdk/aws-appsync/test/integ.auth-apikey.ts @@ -0,0 +1,63 @@ +import { join } from 'path'; +import { AttributeType, BillingMode, Table } from '@aws-cdk/aws-dynamodb'; +import { App, RemovalPolicy, Stack, Expiration } from '@aws-cdk/core'; +import { AuthorizationType, GraphqlApi, MappingTemplate, PrimaryKey, Schema, Values } from '../lib'; + +/* + * Creates an Appsync GraphQL API with API_KEY authorization. + * Testing for API_KEY Authorization. + * + * Stack verification steps: + * Deploy stack, get api-key and endpoint. + * Check if authorization occurs with empty get. + * + * -- bash verify.integ.auth-apikey.sh --start -- deploy stack -- + * -- aws appsync list-graphql-apis -- obtain api id && endpoint -- + * -- aws appsync list-api-keys --api-id [API ID] -- obtain api key -- + * -- bash verify.integ.auth-apikey.sh --check [APIKEY] [ENDPOINT] -- check if fails/success -- + * -- bash verify.integ.auth-apikey.sh --clean -- clean dependencies/stack -- + */ + +const app = new App(); +const stack = new Stack(app, 'aws-appsync-integ'); + +const api = new GraphqlApi(stack, 'Api', { + name: 'Integ_Test_APIKey', + schema: Schema.fromAsset(join(__dirname, 'appsync.auth.graphql')), + authorizationConfig: { + defaultAuthorization: { + authorizationType: AuthorizationType.API_KEY, + apiKeyConfig: { + // Generate a timestamp that's 365 days ahead, use atTimestamp so integ test doesn't fail + expires: Expiration.atTimestamp(1626566400000), + }, + }, + }, +}); + +const testTable = new Table(stack, 'TestTable', { + billingMode: BillingMode.PAY_PER_REQUEST, + partitionKey: { + name: 'id', + type: AttributeType.STRING, + }, + removalPolicy: RemovalPolicy.DESTROY, +}); + +const testDS = api.addDynamoDbDataSource('testDataSource', testTable); + +testDS.createResolver({ + typeName: 'Query', + fieldName: 'getTests', + requestMappingTemplate: MappingTemplate.dynamoDbScanTable(), + responseMappingTemplate: MappingTemplate.dynamoDbResultList(), +}); + +testDS.createResolver({ + typeName: 'Mutation', + fieldName: 'addTest', + requestMappingTemplate: MappingTemplate.dynamoDbPutItem(PrimaryKey.partition('id').auto(), Values.projecting('test')), + responseMappingTemplate: MappingTemplate.dynamoDbResultItem(), +}); + +app.synth(); \ No newline at end of file diff --git a/packages/@aws-cdk/aws-appsync/test/verify.integ.auth-apikey.sh b/packages/@aws-cdk/aws-appsync/test/verify.integ.auth-apikey.sh new file mode 100644 index 0000000000000..2102f10d627e4 --- /dev/null +++ b/packages/@aws-cdk/aws-appsync/test/verify.integ.auth-apikey.sh @@ -0,0 +1,37 @@ +#!/bin/bash + +function error { + printf "\e[91;5;81m$@\e[0m\n" +} + +function usage { + echo "###############################################################################" + echo "# run 'verify.integ.auth-apikey.sh --start' to deploy #" + echo "# run 'verify.integ.auth-apikey.sh --check [APIKEY] [ENDPOINT]' to run check #" + echo "# run 'verify.integ.auth-apikey.sh --clean' to clean up stack #" + echo "###############################################################################" +} + +if [[ "$1" == "--start" ]]; then + cdk deploy --app "node integ.auth-apikey.js" +elif [[ "$1" == "--check" ]]; then + if [[ -z $2 || -z $3 ]]; then + error "Error: --check flag requires [APIKEY] [ENDPOINT]" + usage + exit 1 + fi + echo THIS TEST SHOULD FAIL + curl -XPOST -H "Content-Type:application/graphql" -H "x-api-key:garbage" -d '{ "query": "query { getTests { id version } }" }" }' $3 + echo "" + echo "" + echo THIS TEST SHOULD SUCCEED + curl -XPOST -H "Content-Type:application/graphql" -H "x-api-key:$2" -d '{ "query": "query { getTests { id version } }" }" }' $3 + echo "" +elif [[ "$1" == "--clean" ]];then + cdk destroy --app "node integ.auth-apikey.js" +else + error "Error: use flags --start, --check, --clean" + usage + exit 1 +fi +