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.

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

fix(appsync): strongly type expires prop in apiKeyConfig #9122

Merged
merged 43 commits into from
Sep 9, 2020
Merged
Show file tree
Hide file tree
Changes from 42 commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
ae65030
apiKeyConfig expire works using Expires from @aws-cdk/core
BryanPan342 Jul 16, 2020
3699019
Merge branch 'master' into appsync-expires
BryanPan342 Jul 17, 2020
5a72efe
remove unused variable
BryanPan342 Jul 17, 2020
72b5999
update integ
BryanPan342 Jul 17, 2020
9397fe4
address suggestions for expire
BryanPan342 Jul 21, 2020
7ac8c88
run integ
BryanPan342 Jul 21, 2020
8803379
adjust integ to set up for a year
BryanPan342 Jul 21, 2020
25dc05f
move utility to core
BryanPan342 Jul 28, 2020
93b26be
linter stop it
BryanPan342 Jul 28, 2020
8d7d415
linter fix
BryanPan342 Jul 28, 2020
d0b3632
Merge branch 'master' into appsync-expires
BryanPan342 Jul 28, 2020
0b1bf97
fix bug in s3-deployments
BryanPan342 Jul 28, 2020
924a543
Merge branch 'appsync-expires' of https://github.com/BryanPan342/aws-…
BryanPan342 Jul 28, 2020
771ee1c
lint
BryanPan342 Jul 28, 2020
2f3f29d
change less/greaterThan to isBefore/After
BryanPan342 Jul 28, 2020
8f695a2
Merge branch 'master' into appsync-expires
BryanPan342 Jul 29, 2020
70245f3
clean up asEpoch
BryanPan342 Aug 7, 2020
a006853
update test
BryanPan342 Aug 7, 2020
02193d2
Merge branch 'master' into appsync-expires
BryanPan342 Aug 17, 2020
7014424
fix build
BryanPan342 Aug 17, 2020
9c079c9
Merge branch 'master' into appsync-expires
BryanPan342 Aug 17, 2020
52bb994
fix breaking changes
BryanPan342 Aug 17, 2020
794a88d
Merge branch 'master' into appsync-expires
BryanPan342 Aug 20, 2020
6cc9c0c
Merge branch 'master' into appsync-expires
BryanPan342 Aug 21, 2020
8a89736
Merge branch 'master' into appsync-expires
BryanPan342 Aug 31, 2020
b9845fa
fix merge conflicts
BryanPan342 Aug 31, 2020
f3c77e3
refactor(appsync): graphQLApi to graphqlApi for better snakecasing
BryanPan342 Aug 31, 2020
51edc01
Revert "refactor(appsync): graphQLApi to graphqlApi for better snakec…
BryanPan342 Aug 31, 2020
884e185
Merge remote-tracking branch 'upstream/master'
BryanPan342 Aug 31, 2020
9ad9d36
Merge remote-tracking branch 'upstream/master'
BryanPan342 Sep 1, 2020
d9ebafb
Merge remote-tracking branch 'upstream/master'
BryanPan342 Sep 1, 2020
64db87e
Merge remote-tracking branch 'upstream/master'
BryanPan342 Sep 1, 2020
b3622b0
Merge remote-tracking branch 'upstream/master'
BryanPan342 Sep 1, 2020
c61f436
address suggestions
BryanPan342 Sep 2, 2020
4cdd244
Merge remote-tracking branch 'upstream/master'
BryanPan342 Sep 2, 2020
5070407
Merge branch 'master' into appsync-expires
BryanPan342 Sep 2, 2020
859c14d
fix merge conflicts
BryanPan342 Sep 2, 2020
6608704
Merge remote-tracking branch 'upstream/master'
BryanPan342 Sep 3, 2020
f85ce81
Merge remote-tracking branch 'upstream/master'
BryanPan342 Sep 4, 2020
e8cfc0c
Merge remote-tracking branch 'upstream/master'
BryanPan342 Sep 8, 2020
1423bc6
Merge remote-tracking branch 'upstream/master'
BryanPan342 Sep 8, 2020
d38aa2a
Merge branch 'master' into appsync-expires
BryanPan342 Sep 8, 2020
7af4dab
Merge branch 'master' into appsync-expires
mergify[bot] Sep 9, 2020
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
21 changes: 8 additions & 13 deletions packages/@aws-cdk/aws-appsync/lib/graphqlapi.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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;
}

/**
Expand Down Expand Up @@ -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,
Expand Down
65 changes: 65 additions & 0 deletions packages/@aws-cdk/aws-appsync/test/appsync-auth.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', {
Expand Down
12 changes: 12 additions & 0 deletions packages/@aws-cdk/aws-appsync/test/appsync.auth.graphql
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
type test {
id: Int!
version: String!
}

type Query {
getTests: [ test! ]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is this related to this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just for the integrity test to see if the API Key works for query/mutation

}

type Mutation {
addTest(version: String!): test!
}
186 changes: 186 additions & 0 deletions packages/@aws-cdk/aws-appsync/test/integ.auth-apikey.expected.json
Original file line number Diff line number Diff line change
@@ -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"
}
}
}
63 changes: 63 additions & 0 deletions packages/@aws-cdk/aws-appsync/test/integ.auth-apikey.ts
Original file line number Diff line number Diff line change
@@ -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(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is this related to this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as the comment above

just for the integrity test to see if the API Key works for query/mutation

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eladb o i see.. i just need to check if the apikey exist is that right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think its important to test the connection else i can just remove the integ test all together

});

testDS.createResolver({
typeName: 'Mutation',
fieldName: 'addTest',
requestMappingTemplate: MappingTemplate.dynamoDbPutItem(PrimaryKey.partition('id').auto(), Values.projecting('test')),
responseMappingTemplate: MappingTemplate.dynamoDbResultItem(),
});

app.synth();
Loading