From 64e5f6e88d19a10a2ce88234b409284e59495647 Mon Sep 17 00:00:00 2001 From: Colin Ihrig Date: Tue, 27 Apr 2021 15:21:07 -0400 Subject: [PATCH 1/5] fix: ensure policy resource name when pushing REST APIs It is possible that older REST APIs have never been updated to include the policyResourceName field on paths. This commit ensures that the field is present. Refs: https://github.com/aws-amplify/amplify-cli/issues/7190 --- .../src/utils/consolidate-apigw-policies.ts | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/packages/amplify-provider-awscloudformation/src/utils/consolidate-apigw-policies.ts b/packages/amplify-provider-awscloudformation/src/utils/consolidate-apigw-policies.ts index 480b1067894..33c1730164c 100644 --- a/packages/amplify-provider-awscloudformation/src/utils/consolidate-apigw-policies.ts +++ b/packages/amplify-provider-awscloudformation/src/utils/consolidate-apigw-policies.ts @@ -239,6 +239,7 @@ function updateExistingApiCfn(context: $TSContext, api: $TSObject): void { const resourceDir = getResourceDirPath(context, 'api', resourceName); const cfnTemplate = path.join(resourceDir, `${resourceName}-cloudformation-template.json`); const paramsFile = path.join(resourceDir, 'parameters.json'); + const apiParamsFile = path.join(resourceDir, API_PARAMS_FILE); const cfn: any = JSONUtilities.readJson(cfnTemplate, { throwIfNotExist: false }) ?? {}; const parameterJson = JSONUtilities.readJson(paramsFile, { throwIfNotExist: false }) ?? {}; const parameters = cfn?.Parameters ?? {}; @@ -271,8 +272,24 @@ function updateExistingApiCfn(context: $TSContext, api: $TSObject): void { } } + if (Array.isArray(api.params.paths)) { + api.params.paths.forEach(path => { + if (!path.policyResourceName) { + if (typeof path.name !== 'string') { + const err = new Error(`Malformed parameters file for REST API ${resourceName}`); + err.stack = undefined; + throw err; + } + + path.policyResourceName = path.name.replace(/{[a-zA-Z0-9\-]+}/g, '*'); + modified = true; + } + }); + } + if (modified) { JSONUtilities.writeJson(cfnTemplate, cfn); JSONUtilities.writeJson(paramsFile, parameterJson); + JSONUtilities.writeJson(apiParamsFile, api.params); } } From 340953317b4ee61c4dbe4348ccdafeadd7ff1b73 Mon Sep 17 00:00:00 2001 From: Colin Ihrig Date: Wed, 28 Apr 2021 14:00:29 -0400 Subject: [PATCH 2/5] fix: process CFN and parameters.json separately When removing the auth and unauth roles from REST APIs, the CFN template and the parameters.json file should be processed separately in case they have gotten out of sync. Otherwise, the generated root stack is likely to have errors. --- .../src/utils/consolidate-apigw-policies.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/amplify-provider-awscloudformation/src/utils/consolidate-apigw-policies.ts b/packages/amplify-provider-awscloudformation/src/utils/consolidate-apigw-policies.ts index 33c1730164c..107406e9535 100644 --- a/packages/amplify-provider-awscloudformation/src/utils/consolidate-apigw-policies.ts +++ b/packages/amplify-provider-awscloudformation/src/utils/consolidate-apigw-policies.ts @@ -249,6 +249,12 @@ function updateExistingApiCfn(context: $TSContext, api: $TSObject): void { for (const parameterName in parameters) { if (parameterName === AUTH_ROLE_NAME || parameterName === UNAUTH_ROLE_NAME) { delete parameters[parameterName]; + modified = true; + } + } + + for (const parameterName in parameterJson as any) { + if (parameterName === AUTH_ROLE_NAME || parameterName === UNAUTH_ROLE_NAME) { delete parameterJson[parameterName]; modified = true; } From 7fdf68843c6b2da897d1418167436c32f32d4140 Mon Sep 17 00:00:00 2001 From: Colin Ihrig Date: Wed, 28 Apr 2021 14:48:36 -0400 Subject: [PATCH 3/5] fix: ignore AdminQueries during REST API consolidation This commit moves the check for AdminQueries from consolidateApiGatewayPolicies() to loadApiWithPrivacyParams(), as the latter is also called from push-resources.ts. --- .../src/utils/consolidate-apigw-policies.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/packages/amplify-provider-awscloudformation/src/utils/consolidate-apigw-policies.ts b/packages/amplify-provider-awscloudformation/src/utils/consolidate-apigw-policies.ts index 107406e9535..a6a10abf603 100644 --- a/packages/amplify-provider-awscloudformation/src/utils/consolidate-apigw-policies.ts +++ b/packages/amplify-provider-awscloudformation/src/utils/consolidate-apigw-policies.ts @@ -178,10 +178,6 @@ export function consolidateApiGatewayPolicies(context: $TSContext, stackName: st const apis = amplifyMeta?.api ?? {}; Object.keys(apis).forEach(resourceName => { - if (resourceName === 'AdminQueries') { - return; - } - const resource = apis[resourceName]; const apiParams = loadApiWithPrivacyParams(context, resourceName, resource); @@ -220,7 +216,7 @@ function createApiGatewayAuthResources(context: $TSContext, stackName: string, a } export function loadApiWithPrivacyParams(context: $TSContext, name: string, resource: any): object | undefined { - if (resource.providerPlugin !== ProviderName || resource.service !== 'API Gateway') { + if (resource.providerPlugin !== ProviderName || resource.service !== 'API Gateway' || name === 'AdminQueries') { return; } From e916768f77f9b4e64f69f1ef6f619c414cf31164 Mon Sep 17 00:00:00 2001 From: Colin Ihrig Date: Wed, 28 Apr 2021 16:39:04 -0400 Subject: [PATCH 4/5] fix: add AdminQueries to multiple REST API test --- .../amplify-e2e-core/src/categories/auth.ts | 27 +++++++++++++++++++ .../src/__tests__/api_2.test.ts | 2 ++ 2 files changed, 29 insertions(+) diff --git a/packages/amplify-e2e-core/src/categories/auth.ts b/packages/amplify-e2e-core/src/categories/auth.ts index 217d645f46e..a229f34df8a 100644 --- a/packages/amplify-e2e-core/src/categories/auth.ts +++ b/packages/amplify-e2e-core/src/categories/auth.ts @@ -1275,3 +1275,30 @@ export function addAuthUserPoolOnlyNoOAuth(cwd: string, settings: AddAuthUserPoo }); }); } + +export function updateAuthAddAdminQueries(projectDir: string, groupName: string = 'adminQueriesGroup'): Promise { + return new Promise((resolve, reject) => { + spawn(getCLIPath(), ['update', 'auth'], { cwd: projectDir, stripColors: true }) + .wait('What do you want to do?') + .send(KEY_DOWN_ARROW) + .send(KEY_DOWN_ARROW) + .send(KEY_DOWN_ARROW) + .send(KEY_DOWN_ARROW) + .sendCarriageReturn() // Create or update Admin queries API + .wait('Do you want to restrict access to the admin queries API to a specific Group') + .sendConfirmYes() + .wait('Select the group to restrict access with') + .sendCarriageReturn() // Enter a custom group + .wait('Provide a group name') + .send(groupName) + .sendCarriageReturn() + .sendEof() + .run((err: Error) => { + if (err) { + reject(err); + } else { + resolve(); + } + }); + }); +} diff --git a/packages/amplify-e2e-tests/src/__tests__/api_2.test.ts b/packages/amplify-e2e-tests/src/__tests__/api_2.test.ts index 0f03ad294e6..c849897c88b 100644 --- a/packages/amplify-e2e-tests/src/__tests__/api_2.test.ts +++ b/packages/amplify-e2e-tests/src/__tests__/api_2.test.ts @@ -5,6 +5,7 @@ import { initJSProjectWithProfile, listAttachedRolePolicies, listRolePolicies, + updateAuthAddAdminQueries, } from 'amplify-e2e-core'; import * as path from 'path'; import { existsSync } from 'fs'; @@ -402,6 +403,7 @@ describe('amplify add api (REST)', () => { allowGuestUsers: false, }); await addRestApi(projRoot, { existingLambda: true }); + await updateAuthAddAdminQueries(projRoot); await amplifyPushUpdate(projRoot); const amplifyMeta = getProjectMeta(projRoot); From 31a082468691fd582ad1fb77625c27429522e88c Mon Sep 17 00:00:00 2001 From: Colin Ihrig Date: Thu, 29 Apr 2021 09:26:30 -0400 Subject: [PATCH 5/5] fix: don't throw on malformed path --- .../src/utils/consolidate-apigw-policies.ts | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/packages/amplify-provider-awscloudformation/src/utils/consolidate-apigw-policies.ts b/packages/amplify-provider-awscloudformation/src/utils/consolidate-apigw-policies.ts index a6a10abf603..297a28c0c7a 100644 --- a/packages/amplify-provider-awscloudformation/src/utils/consolidate-apigw-policies.ts +++ b/packages/amplify-provider-awscloudformation/src/utils/consolidate-apigw-policies.ts @@ -277,13 +277,7 @@ function updateExistingApiCfn(context: $TSContext, api: $TSObject): void { if (Array.isArray(api.params.paths)) { api.params.paths.forEach(path => { if (!path.policyResourceName) { - if (typeof path.name !== 'string') { - const err = new Error(`Malformed parameters file for REST API ${resourceName}`); - err.stack = undefined; - throw err; - } - - path.policyResourceName = path.name.replace(/{[a-zA-Z0-9\-]+}/g, '*'); + path.policyResourceName = String(path.name).replace(/{[a-zA-Z0-9\-]+}/g, '*'); modified = true; } });