fix(cli): generate admin queries auth wiring by hariharan077 · Pull Request #14909 · aws-amplify/amplify-cli

hariharan077 · 2026-06-09T10:42:54Z

Description of changes

When Gen1 AdminQueries REST APIs are generated for Gen2 migration, the generated REST API resource needs to preserve the Cognito behavior that AdminQueries depends on. This updates the REST API generator to detect AdminQueries APIs and emit the missing pieces:

API-level CORS preflight options so the proxy resource handles OPTIONS
a Cognito User Pools authorizer using the Authorization header
Cognito authorization method options with the aws.cognito.signin.user.admin scope
explicit ANY methods on the root/proxy resources using the Cognito method options
Cognito admin/list IAM permissions for the backing AdminQueries Lambda, scoped to the Gen2 user pool and the Gen1 user pool id when available

Regular REST API generation is left unchanged unless the API is AdminQueries or uses an AdminQueries* backing function.

Description of how you validated changes

PATH="$PWD/node_modules/.bin:$PATH" corepack yarn workspace @aws-amplify/cli-internal test src/__tests__/commands/gen2-migration/generate/amplify/rest-api/rest-api.generator.test.ts --runInBand --no-coverage
PATH="$PWD/node_modules/.bin:$PATH" corepack yarn prettier --check packages/amplify-cli/src/commands/gen2-migration/generate/amplify/rest-api/rest-api.generator.ts packages/amplify-cli/src/commands/gen2-migration/generate/amplify/rest-api/rest-api.renderer.ts packages/amplify-cli/src/__tests__/commands/gen2-migration/generate/amplify/rest-api/rest-api.generator.test.ts
PATH="$PWD/node_modules/.bin:$PATH" corepack yarn eslint packages/amplify-cli/src/commands/gen2-migration/generate/amplify/rest-api/rest-api.generator.ts packages/amplify-cli/src/commands/gen2-migration/generate/amplify/rest-api/rest-api.renderer.ts packages/amplify-cli/src/__tests__/commands/gen2-migration/generate/amplify/rest-api/rest-api.generator.test.ts --max-warnings=1500
git diff --check
PATH="$PWD/node_modules/.bin:$PATH" corepack yarn sanitize-migration-apps
PATH="$PWD/node_modules/.bin:$PATH" corepack yarn build-tests-changed

Note: the first build-tests-changed run hit an NX daemon restart message; rerunning the same command succeeded. Local git hooks call a plain yarn binary directly, while this environment exposes Yarn through Corepack, so commit/push hooks were bypassed after running the hook commands above manually.

Checklist

PR description included
Tests are changed or added

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

sharonyajain · 2026-08-25T14:01:12Z

+      [factory.createIdentifier(apiVarName)],
+    );
+
+    return factory.createTemplateExpression(factory.createTemplateHead('arn:aws:cognito-idp:'), [


The generated Gen1 user-pool ARN hardcodes the aws partition. The sibling Gen2 pool ARN uses backend.auth.resources.userPool.userPoolArn (a partition-correct token), so only the Gen1 ARN is partition-blind.

return factory.createTemplateExpression(factory.createTemplateHead('arn:aws:cognito-idp:'), [

Why it matters: cause: emitted ARN begins arn:aws:cognito-idp: regardless of partition -> mechanism: in aws-us-gov / aws-cn the real user-pool ARN uses arn:aws-us-gov: / arn:aws-cn:, so the policy resource never matches -> consequence: the AdminQueries Lambda's IAM statement grants nothing for the Gen1 pool, so admin operations targeting the Gen1 pool fail with AccessDenied after migration in those partitions.

Suggestion: We can emit the ARN via the stack's partition-aware formatter instead of a literal prefix, e.g. Stack.of(<api>).formatArn({ service: 'cognito-idp', resource: 'userpool', resourceName: <gen1UserPoolId> }), or interpolate Stack.of(<api>).partition.

Fixed in b78acb6. The Gen1 pool ARN now uses Stack.of(AdminQueriesApi).formatArn(...), allowing CDK to supply the correct partition, region, and account. The complete output snapshot verifies this.

sharonyajain · 2026-08-25T14:01:12Z

@@ -1073,4 +1073,52 @@ describe('RestApiGenerator', () => {
    const output = writtenFile('resource.ts');
    expect(output).toContain('myapiApi');


The single new test covers only the happy path: an AdminQueries-named API, gen1UserPoolId present, one /{proxy+} path. Two branches this change introduces are untested.

it('renders Cognito authorizer and admin permissions for AdminQueries API', async () => {

Why it matters: cause: no test exercises (a) the gen1UserPoolId absent branch, where the IAM resources array should contain only the Gen2 pool ARN, or (b) the generator's startsWith('AdminQueries') branch for a non-AdminQueries-named API -> mechanism: a future edit that breaks either branch (e.g. emits a malformed ARN when the Gen1 pool id is missing, or mis-detects admin APIs) compiles and passes CI -> consequence: the regression ships to migrating users undetected.

Suggestion: Adding two cases: an AdminQueries API with no Gen1 user pool (assert the IAM statement has exactly the Gen2 userPoolArn and no arn:...:userpool/ literal), and a non-AdminQueries-named API whose backing function starts with AdminQueries (assert admin wiring is applied).

Added both edge cases. Missing UserPoolId now warns and emits only the Gen2 pool ARN. The prefix case is intentionally negative because the later least-privilege finding shows that enabling admin wiring for unrelated AdminQueries* Lambdas is unsafe.

sharonyajain · 2026-08-25T14:01:12Z

+    if (apiName === 'AdminQueries') {
+      return Array.from(functions);
+    }
+    return Array.from(functions).filter((name) => name.startsWith('AdminQueries'));


For any API not named AdminQueries, a backing function whose name merely starts with AdminQueries flips the whole API into admin mode (isAdminQueriesApi true). Detection is name-prefix based with no other signal.

return Array.from(functions).filter((name) => name.startsWith('AdminQueries'));

Why it matters: cause: a user function named e.g. AdminQueriesReports on an unrelated REST API matches the prefix -> mechanism: renderPaths applies the Cognito aws.cognito.signin.user.admin-scope method options to ALL of that API's paths and renderAdminQueriesLambdaPolicies grants that Lambda 11 cognito-idp:Admin*/List* actions -> consequence: (1) the API silently starts requiring the admin scope so existing callers without it get 401, and (2) a Lambda that never needed Cognito admin powers is over-privileged (least-privilege violation). Input is the developer's own config, not attacker-controlled, so this is a footgun, not an external exploit.

Suggestion: Tighten detection beyond a bare name prefix (e.g. match the exact AdminQueries API name, or require the migration marker that identifies the CLI-owned AdminQueries feature) so unrelated user resources cannot inherit admin auth + admin IAM.

Fixed by requiring the exact AdminQueries API, canonical Cognito and Lambda dependencies from amplify-meta.json, and a matching Lambda referenced by an API path.

sharonyajain · 2026-08-25T14:01:12Z

+            [
+              factory.createPropertyAssignment(
+                'cognitoUserPools',
+                factory.createArrayLiteralExpression([TS.propAccess('backend', 'auth', 'resources', 'userPool')]),


The admin wiring emits backend.auth.resources.userPool (authorizer, line 303) and backend.auth.resources.userPool.userPoolArn (Lambda policy, line 578) unconditionally whenever isAdminQueriesApi is true. It never consults the hasAuth flag the renderer is constructed with, even though the sibling Gen1-policy attachment at line 97 (if (this.hasPathAuth(restApi) && this.hasAuth)) already gates auth-dependent emission on hasAuth.

factory.createArrayLiteralExpression([TS.propAccess('backend', 'auth', 'resources', 'userPool')]),

Why it matters: cause: isAdminQueriesApi is driven purely by the startsWith('AdminQueries') function-name match, independent of whether the Gen1 app has any Cognito auth -> mechanism: an app with NO Cognito (e.g. an API-key/IAM REST API whose backing Lambda is merely named AdminQueries*) has no auth resource migrated into defineBackend, so the generated resource.ts references a backend.auth that does not exist -> consequence: the generated backend fails to type-check (Property 'auth' does not exist on type 'Backend') or throws at synth (Cannot read properties of undefined (reading 'resources')), breaking the ENTIRE migrated backend.ts, not just this one API. Low probability (needs the name coincidence on a no-Cognito app) but high blast radius.

Suggestion: Guarding the admin wiring on this.hasAuth (only treat an API as AdminQueries when the app actually has Cognito auth) and/or fold a hasAuth check into isAdminQueriesApi, mirroring the existing this.hasAuth gate at line 97.

Fixed. AdminQueries generation requires the referenced Cognito resource, and every renderer path—including Lambda policy emission—is gated by hasAuth. The no-auth regression verifies there is no authorizer, backend.auth, or Cognito admin policy.

Require canonical AdminQueries dependencies before generating auth. Generate IAM policies only for verified metadata. Build Gen1 user pool ARNs with CDK's partition-aware formatter. Warn when the referenced pool is unavailable. Cover prefix collisions, absent auth, missing pool IDs, and full output snapshots. Validate with the cli-internal build and full unit suite. --- Prompt: do it. make sure you verify and test if its all working by reproducing the error and the patch before pushing and then comment and reply whatever is necessary you have my approval

fix(cli): generate admin queries auth wiring

20318fc

hariharan077 requested a review from a team as a code owner June 9, 2026 10:42

hariharan077 mentioned this pull request Jun 9, 2026

(gen2-migration): generate incorrectly handles Admin queries in auth definition #14757

Open

2 tasks

sharonyajain self-assigned this Aug 25, 2026

sharonyajain reviewed Aug 26, 2026

View reviewed changes

Navigation Menu

Uh oh!

fix(cli): generate admin queries auth wiring - #14909

fix(cli): generate admin queries auth wiring#14909
hariharan077 wants to merge 2 commits into
aws-amplify:devfrom
hariharan077:fix-14757-admin-queries-rest-api

hariharan077 commented Jun 9, 2026

Uh oh!

sharonyajain Aug 25, 2026

Uh oh!

hariharan077 Aug 26, 2026

Uh oh!

sharonyajain Aug 25, 2026

Uh oh!

hariharan077 Aug 26, 2026

Uh oh!

sharonyajain Aug 25, 2026

Uh oh!

hariharan077 Aug 26, 2026

Uh oh!

sharonyajain Aug 25, 2026

Uh oh!

hariharan077 Aug 26, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		@@ -1073,4 +1073,52 @@ describe('RestApiGenerator', () => {
		const output = writtenFile('resource.ts');
		expect(output).toContain('myapiApi');

Uh oh!

Conversation

hariharan077 commented Jun 9, 2026

Description of changes

Description of how you validated changes

Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants