Skip to content

Commit

Permalink
fix(appsync): add dependency between apikey and schema (#9737)
Browse files Browse the repository at this point in the history 
Added dependency between the CfnApiKey and CfnSchema. The dependency here is to prevent a `ConcurrencyModificationError` as seen in #8168. We allow this dependency to exist because from referencing the [docs](https://docs.aws.amazon.com/appsync/latest/APIReference/API_CreateApiKey.html#API_CreateApiKey_Errors) there shouldn't be any issue between creating an api key before or after schema creation. 

Also make ApiKeyConfig correctly configure the ApiKey when used in `additionalAuthorizationModes`.

Fixes #9736
Fixes #8168 

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
@BryanPan342
BryanPan342 authored Aug 19, 2020
1 parent d6278b3 commit 4448794
Show file tree
Hide file tree
Showing 5 changed files with 190 additions and 100 deletions.
115 changes: 39 additions & 76 deletions packages/@aws-cdk/aws-appsync/lib/graphqlapi.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -397,21 +397,21 @@ export class GraphQLApi extends GraphqlApiBase {

/**
* the configured API key, if present
*
* @default - no api key
*/
public get apiKey(): string | undefined {
return this._apiKey;
}
public readonly apiKey?: string;

private schemaMode: SchemaDefinition;
private api: CfnGraphQLApi;
private _apiKey?: string;
private _apiKey?: CfnApiKey;

constructor(scope: Construct, id: string, props: GraphQLApiProps) {
super(scope, id);

this.validateAuthorizationProps(props);
const defaultAuthorizationType =
props.authorizationConfig?.defaultAuthorization?.authorizationType ||
props.authorizationConfig?.defaultAuthorization?.authorizationType ??
AuthorizationType.API_KEY;

let apiLogsRole;
Expand Down Expand Up @@ -462,22 +462,24 @@ export class GraphQLApi extends GraphqlApiBase {
this.graphQlUrl = this.api.attrGraphQlUrl;
this.name = this.api.name;
this.schemaMode = props.schemaDefinition;
this.schema = this.defineSchema(props.schemaDefinitionFile);

if (
defaultAuthorizationType === AuthorizationType.API_KEY ||
if (defaultAuthorizationType === AuthorizationType.API_KEY ||
props.authorizationConfig?.additionalAuthorizationModes?.some(
(authMode) => authMode.authorizationType === AuthorizationType.API_KEY
)
(authMode) => authMode.authorizationType === AuthorizationType.API_KEY)
) {
const apiKeyConfig: ApiKeyConfig = props.authorizationConfig
?.defaultAuthorization?.apiKeyConfig || {
name: 'DefaultAPIKey',
description: 'Default API Key created by CDK',
};
// create a variable for apiKeyConfig if one has been specified by the user
// first check is for default authorization
// second check is for additional authorization modes
const apiKeyConfig = props.authorizationConfig?.defaultAuthorization?.apiKeyConfig ??
props.authorizationConfig?.additionalAuthorizationModes?.
find((mode: AuthorizationMode) => {
return mode.authorizationType === AuthorizationType.API_KEY && mode.apiKeyConfig;
})?.apiKeyConfig;
this._apiKey = this.createAPIKey(apiKeyConfig);
this._apiKey.addDependsOn(this.schema);
this.apiKey = this._apiKey.attrApiKey;
}

this.schema = this.defineSchema(props.schemaDefinitionFile);
}

/**
Expand Down Expand Up @@ -531,65 +533,27 @@ export class GraphQLApi extends GraphqlApiBase {
}

private validateAuthorizationProps(props: GraphQLApiProps) {
const defaultAuthorizationType =
props.authorizationConfig?.defaultAuthorization?.authorizationType ||
AuthorizationType.API_KEY;
const defaultMode = props.authorizationConfig?.defaultAuthorization ?? {
authorizationType: AuthorizationType.API_KEY,
};
const additionalModes = props.authorizationConfig?.additionalAuthorizationModes ?? [];
const allModes = [defaultMode, ...additionalModes];

if (
defaultAuthorizationType === AuthorizationType.OIDC &&
!props.authorizationConfig?.defaultAuthorization?.openIdConnectConfig
) {
throw new Error('Missing default OIDC Configuration');
}
allModes.map((mode) => {
if(mode.authorizationType === AuthorizationType.OIDC && !mode.openIdConnectConfig){
throw new Error('Missing default OIDC Configuration');
}
if(mode.authorizationType === AuthorizationType.USER_POOL && !mode.userPoolConfig){
throw new Error('Missing default OIDC Configuration');
}
});

if (
defaultAuthorizationType === AuthorizationType.USER_POOL &&
!props.authorizationConfig?.defaultAuthorization?.userPoolConfig
) {
throw new Error('Missing default User Pool Configuration');
if(allModes.filter((mode) => mode.authorizationType === AuthorizationType.API_KEY).length > 1){
throw new Error('You can\'t duplicate API_KEY configuration. See https://docs.aws.amazon.com/appsync/latest/devguide/security.html');
}

if (props.authorizationConfig?.additionalAuthorizationModes) {
props.authorizationConfig.additionalAuthorizationModes.forEach(
(authorizationMode) => {
if (
authorizationMode.authorizationType === AuthorizationType.API_KEY &&
defaultAuthorizationType === AuthorizationType.API_KEY
) {
throw new Error(
"You can't duplicate API_KEY in additional authorization config. See https://docs.aws.amazon.com/appsync/latest/devguide/security.html",
);
}

if (
authorizationMode.authorizationType === AuthorizationType.IAM &&
defaultAuthorizationType === AuthorizationType.IAM
) {
throw new Error(
"You can't duplicate IAM in additional authorization config. See https://docs.aws.amazon.com/appsync/latest/devguide/security.html",
);
}

if (
authorizationMode.authorizationType === AuthorizationType.OIDC &&
!authorizationMode.openIdConnectConfig
) {
throw new Error(
'Missing OIDC Configuration inside an additional authorization mode',
);
}

if (
authorizationMode.authorizationType ===
AuthorizationType.USER_POOL &&
!authorizationMode.userPoolConfig
) {
throw new Error(
'Missing User Pool Configuration inside an additional authorization mode',
);
}
},
);
if(allModes.filter((mode) => mode.authorizationType === AuthorizationType.IAM).length > 1){
throw new Error('You can\'t duplicate IAM configuration. See https://docs.aws.amazon.com/appsync/latest/devguide/security.html');
}
}

Expand Down Expand Up @@ -625,9 +589,9 @@ export class GraphQLApi extends GraphqlApiBase {
};
}

private createAPIKey(config: ApiKeyConfig) {
private createAPIKey(config?: ApiKeyConfig) {
let expires: number | undefined;
if (config.expires) {
if (config?.expires) {
expires = new Date(config.expires).valueOf();
const days = (d: number) =>
Date.now() + Duration.days(d).toMilliseconds();
Expand All @@ -636,12 +600,11 @@ export class GraphQLApi extends GraphqlApiBase {
}
expires = Math.round(expires / 1000);
}
const key = new CfnApiKey(this, `${config.name || 'DefaultAPIKey'}ApiKey`, {
return new CfnApiKey(this, `${config?.name || 'Default'}ApiKey`, {
expires,
description: config.description || 'Default API Key created by CDK',
description: config?.description,
apiId: this.apiId,
});
return key.attrApiKey;
}

private formatAdditionalAuthorizationModes(
Expand Down
121 changes: 121 additions & 0 deletions packages/@aws-cdk/aws-appsync/test/appsync-apikey.test.ts
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import '@aws-cdk/assert/jest';
import * as path from 'path';
import * as cognito from '@aws-cdk/aws-cognito';
import * as cdk from '@aws-cdk/core';
import * as appsync from '../lib';

Expand Down Expand Up @@ -82,4 +83,124 @@ describe('AppSync Authorization Config', () => {
// THEN
expect(stack).not.toHaveResource('AWS::AppSync::ApiKey');
});

test('appsync creates configured api key with additionalAuthorizationModes', () => {
// GIVEN
const stack = new cdk.Stack();

// WHEN
new appsync.GraphQLApi(stack, 'api', {
name: 'api',
schemaDefinition: appsync.SchemaDefinition.FILE,
schemaDefinitionFile: path.join(__dirname, 'appsync.test.graphql'),
authorizationConfig: {
defaultAuthorization: {
authorizationType: appsync.AuthorizationType.IAM,
},
additionalAuthorizationModes: [{
authorizationType: appsync.AuthorizationType.API_KEY,
apiKeyConfig: {
description: 'Custom Description',
},
}],
},
});

// THEN
expect(stack).toHaveResourceLike('AWS::AppSync::ApiKey', {
Description: 'Custom Description',
});
});

test('appsync creates configured api key with additionalAuthorizationModes (not as first element)', () => {
// GIVEN
const stack = new cdk.Stack();
const userPool = new cognito.UserPool(stack, 'myPool');

// WHEN
new appsync.GraphQLApi(stack, 'api', {
name: 'api',
schemaDefinition: appsync.SchemaDefinition.FILE,
schemaDefinitionFile: path.join(__dirname, 'appsync.test.graphql'),
authorizationConfig: {
defaultAuthorization: {
authorizationType: appsync.AuthorizationType.IAM,
},
additionalAuthorizationModes: [
{
authorizationType: appsync.AuthorizationType.USER_POOL,
userPoolConfig: {
userPool,
},
},
{
authorizationType: appsync.AuthorizationType.API_KEY,
apiKeyConfig: {
description: 'Custom Description',
},
},
],
},
});

// THEN
expect(stack).toHaveResourceLike('AWS::AppSync::ApiKey', {
Description: 'Custom Description',
});
});

test('appsync fails when multiple API_KEY auth modes', () => {
// GIVEN
const stack = new cdk.Stack();

// WHEN
const when = () => {
new appsync.GraphQLApi(stack, 'api', {
name: 'api',
schemaDefinition: appsync.SchemaDefinition.FILE,
schemaDefinitionFile: path.join(__dirname, 'appsync.test.graphql'),
authorizationConfig: {
defaultAuthorization: {
authorizationType: appsync.AuthorizationType.API_KEY,
},
additionalAuthorizationModes: [{
authorizationType: appsync.AuthorizationType.API_KEY,
}],
},
});
};

// THEN
expect(when).toThrowError('You can\'t duplicate API_KEY configuration. See https://docs.aws.amazon.com/appsync/latest/devguide/security.html');
});

test('appsync fails when multiple API_KEY auth modes in additionalXxx', () => {
// GIVEN
const stack = new cdk.Stack();

// WHEN
const when = () => {
new appsync.GraphQLApi(stack, 'api', {
name: 'api',
schemaDefinition: appsync.SchemaDefinition.FILE,
schemaDefinitionFile: path.join(__dirname, 'appsync.test.graphql'),
authorizationConfig: {
defaultAuthorization: {
authorizationType: appsync.AuthorizationType.IAM,
},
additionalAuthorizationModes: [
{
authorizationType: appsync.AuthorizationType.API_KEY,
},
{
authorizationType: appsync.AuthorizationType.API_KEY,
},
],
},
});
};

// THEN
expect(when).toThrowError('You can\'t duplicate API_KEY configuration. See https://docs.aws.amazon.com/appsync/latest/devguide/security.html');
});
});
18 changes: 10 additions & 8 deletions packages/@aws-cdk/aws-appsync/test/integ.api-import.expected.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,29 +8,31 @@
"Name": "baseApi"
}
},
"baseApiDefaultAPIKeyApiKey4804ACE5": {
"Type": "AWS::AppSync::ApiKey",
"baseApiSchemaB12C7BB0": {
"Type": "AWS::AppSync::GraphQLSchema",
"Properties": {
"ApiId": {
"Fn::GetAtt": [
"baseApiCDA4D43A",
"ApiId"
]
},
"Description": "Default API Key created by CDK"
"Definition": "type test {\n version: String!\n}\n\ntype Query {\n getTests: [ test! ]!\n}\n\ntype Mutation {\n addTest(version: String!): test\n}\n"
}
},
"baseApiSchemaB12C7BB0": {
"Type": "AWS::AppSync::GraphQLSchema",
"baseApiDefaultApiKeyA3A0A16A": {
"Type": "AWS::AppSync::ApiKey",
"Properties": {
"ApiId": {
"Fn::GetAtt": [
"baseApiCDA4D43A",
"ApiId"
]
},
"Definition": "type test {\n version: String!\n}\n\ntype Query {\n getTests: [ test! ]!\n}\n\ntype Mutation {\n addTest(version: String!): test\n}\n"
}
}
},
"DependsOn": [
"baseApiSchemaB12C7BB0"
]
}
},
"Outputs": {
Expand Down
18 changes: 10 additions & 8 deletions packages/@aws-cdk/aws-appsync/test/integ.graphql-schema.expected.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,29 +7,31 @@
"Name": "api"
}
},
"codefirstapiDefaultAPIKeyApiKeyB73DF94B": {
"Type": "AWS::AppSync::ApiKey",
"codefirstapiSchema148B6CDE": {
"Type": "AWS::AppSync::GraphQLSchema",
"Properties": {
"ApiId": {
"Fn::GetAtt": [
"codefirstapi1A3CC7D2",
"ApiId"
]
},
"Description": "Default API Key created by CDK"
"Definition": "type Planet {\n name: String\n diameter: Int\n rotationPeriod: Int\n orbitalPeriod: Int\n gravity: String\n population: [String]\n climates: [String]\n terrains: [String]\n surfaceWater: Float\n created: String\n edited: String\n id: ID!\n}\ntype Species {\n name: String\n classification: String\n designation: String\n averageHeight: Float\n averageLifespan: Int\n eyeColors: [String]\n hairColors: [String]\n skinColors: [String]\n language: String\n homeworld: Planet\n created: String\n edited: String\n id: ID!\n}\n\ntype Query {\n getPlanets: [Planet]\n}\n"
}
},
"codefirstapiSchema148B6CDE": {
"Type": "AWS::AppSync::GraphQLSchema",
"codefirstapiDefaultApiKey89863A80": {
"Type": "AWS::AppSync::ApiKey",
"Properties": {
"ApiId": {
"Fn::GetAtt": [
"codefirstapi1A3CC7D2",
"ApiId"
]
},
"Definition": "type Planet {\n name: String\n diameter: Int\n rotationPeriod: Int\n orbitalPeriod: Int\n gravity: String\n population: [String]\n climates: [String]\n terrains: [String]\n surfaceWater: Float\n created: String\n edited: String\n id: ID!\n}\ntype Species {\n name: String\n classification: String\n designation: String\n averageHeight: Float\n averageLifespan: Int\n eyeColors: [String]\n hairColors: [String]\n skinColors: [String]\n language: String\n homeworld: Planet\n created: String\n edited: String\n id: ID!\n}\n\ntype Query {\n getPlanets: [Planet]\n}\n"
}
}
},
"DependsOn": [
"codefirstapiSchema148B6CDE"
]
},
"codefirstapiplanetsServiceRole2F4AA8E7": {
"Type": "AWS::IAM::Role",
Expand Down
Loading

0 comments on commit 4448794

Please sign in to comment.