From 287f808cf34870295e4032995fc083ac204b64c3 Mon Sep 17 00:00:00 2001
From: Bryan Pan <bryanpan342@gmail.com>
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
+