Skip to content

Commit

Permalink
fix: ensure policy resource name when pushing REST APIs (#7192)
Browse files Browse the repository at this point in the history
* 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: #7190

* 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.

* 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.

* fix: add AdminQueries to multiple REST API test

* fix: don't throw on malformed path

Co-authored-by: Colin Ihrig <colihrig@amazon.com>
  • Loading branch information
cjihrig and cjihrig-aws authored Apr 29, 2021
1 parent 8f36c94 commit fc77006
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 5 deletions.
27 changes: 27 additions & 0 deletions packages/amplify-e2e-core/src/categories/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1275,3 +1275,30 @@ export function addAuthUserPoolOnlyNoOAuth(cwd: string, settings: AddAuthUserPoo
});
});
}

export function updateAuthAddAdminQueries(projectDir: string, groupName: string = 'adminQueriesGroup'): Promise<void> {
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();
}
});
});
}
2 changes: 2 additions & 0 deletions packages/amplify-e2e-tests/src/__tests__/api_2.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
initJSProjectWithProfile,
listAttachedRolePolicies,
listRolePolicies,
updateAuthAddAdminQueries,
} from 'amplify-e2e-core';
import * as path from 'path';
import { existsSync } from 'fs';
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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;
}

Expand All @@ -239,6 +235,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 ?? {};
Expand All @@ -248,6 +245,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;
}
Expand All @@ -271,8 +274,18 @@ function updateExistingApiCfn(context: $TSContext, api: $TSObject): void {
}
}

if (Array.isArray(api.params.paths)) {
api.params.paths.forEach(path => {
if (!path.policyResourceName) {
path.policyResourceName = String(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);
}
}

0 comments on commit fc77006

Please sign in to comment.