From 66e090bc89714d069dea376e9ac0dc8d3a5b2606 Mon Sep 17 00:00:00 2001 From: 9pace Date: Mon, 13 Apr 2026 17:04:21 -0400 Subject: [PATCH 1/9] fix(cli): fix OAuth provider scopes and attribute mappings in generate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three bugs in auth.renderer.ts caused generate to produce broken OAuth config for social identity providers (Google, Facebook): 1. Scope field name typo: deriveProviderSpecificScopes() looked for 'authorized_scopes' but Cognito returns 'authorize_scopes'. Scopes were never collected. Added the correct field name to the lookup. 2. Scope mangling: provider scopes were filtered against VALID_SCOPES (Cognito OAuth scopes), which is the wrong namespace. Facebook's 'public_profile' was mapped to 'profile'. Removed the filter — provider scopes are now passed through as-is. 3. Attribute mapping: filterAttributeMapping() dropped keys not in MAPPED_USER_ATTRIBUTE_NAME (e.g. 'username' -> 'sub' for Google, 'username' -> 'id' for Facebook). Changed to route unknown keys into a 'custom' sub-object matching Gen2's AttributeMapping CDK interface. Caveats: - Provider scopes are no longer validated. The old validation was incorrect (wrong namespace), but there is now no validation at all — whatever the provider returns is passed through verbatim. - OIDC/SAML providers flatten standard + custom back into a single Record since their rendering path doesn't support the custom sub-object. If a custom key collides with a mapped standard key, the custom value wins. Verified end-to-end with a test app using Google and Facebook social login. Gen2 deployed with correct scopes and attribute mappings without manual post-generate edits to OAuth config. --- .../generate/amplify/auth/auth.renderer.ts | 59 +++++++++++++------ 1 file changed, 41 insertions(+), 18 deletions(-) diff --git a/packages/amplify-cli/src/commands/gen2-migration/generate/amplify/auth/auth.renderer.ts b/packages/amplify-cli/src/commands/gen2-migration/generate/amplify/auth/auth.renderer.ts index 1406f6e3e21..5fa6463f002 100644 --- a/packages/amplify-cli/src/commands/gen2-migration/generate/amplify/auth/auth.renderer.ts +++ b/packages/amplify-cli/src/commands/gen2-migration/generate/amplify/auth/auth.renderer.ts @@ -262,12 +262,12 @@ export class AuthRenderer { private static deriveExternalProviders(details?: readonly IdentityProviderType[]): { readonly oidcProviders: readonly OidcProviderConfig[]; readonly samlProvider: SamlProviderConfig | undefined; - readonly attributeMappings: Readonly>>; + readonly attributeMappings: Readonly; custom: Record }>>; readonly providerScopes: Readonly>; } { const oidcProviders: OidcProviderConfig[] = []; let samlProvider: SamlProviderConfig | undefined; - const attributeMappings: Record> = {}; + const attributeMappings: Record; custom: Record }> = {}; const providerScopes: Record = {}; if (!details) { @@ -283,21 +283,23 @@ export class AuthRenderer { authorize_url && token_url && attributes_url && jwks_uri ? { authorization: authorize_url, token: token_url, userInfo: attributes_url, jwksUri: jwks_uri } : undefined; + const oidcMapping = AttributeMapping ? AuthRenderer.filterAttributeMapping(AttributeMapping) : undefined; oidcProviders.push({ issuerUrl: oidc_issuer, name: ProviderName, endpoints, - attributeMapping: AttributeMapping ? AuthRenderer.filterAttributeMapping(AttributeMapping) : undefined, + attributeMapping: oidcMapping ? { ...oidcMapping.standard, ...oidcMapping.custom } : undefined, }); } else if (ProviderType === IdentityProviderTypeType.SAML && ProviderDetails) { const { metadataURL, metadataContent } = ProviderDetails; + const samlMapping = AttributeMapping ? AuthRenderer.filterAttributeMapping(AttributeMapping) : undefined; samlProvider = { metadata: { metadataContent: metadataURL || metadataContent, metadataType: metadataURL ? ('URL' as const) : ('FILE' as const), }, name: ProviderName, - attributeMapping: AttributeMapping ? AuthRenderer.filterAttributeMapping(AttributeMapping) : undefined, + attributeMapping: samlMapping ? { ...samlMapping.standard, ...samlMapping.custom } : undefined, }; } else { if (AttributeMapping) { @@ -311,9 +313,7 @@ export class AuthRenderer { if (ProviderDetails) { const scopes = AuthRenderer.deriveProviderSpecificScopes(ProviderDetails); if (scopes.length > 0) { - const mapped = scopes - .map((scope) => (scope === 'public_profile' ? 'profile' : scope)) - .filter((scope) => VALID_SCOPES.includes(scope)); + const mapped = scopes.filter((scope) => scope.length > 0); if (mapped.length > 0 && ProviderType) { providerScopes[ProviderType] = mapped; } @@ -445,7 +445,7 @@ export class AuthRenderer { * Extracts provider-specific scopes from provider details. */ private static deriveProviderSpecificScopes(providerDetails: Record): string[] { - const scopeFields = ['authorized_scopes', 'scope', 'scopes']; + const scopeFields = ['authorize_scopes', 'authorized_scopes', 'scope', 'scopes']; for (const field of scopeFields) { if (providerDetails[field]) { return providerDetails[field].split(/[\s,]+/).filter((scope) => scope.length > 0); @@ -457,12 +457,22 @@ export class AuthRenderer { /** * Filters attribute mappings to only known standard attributes. */ - private static filterAttributeMapping(attributeMapping: Record): Record { - return Object.fromEntries( - Object.entries(attributeMapping) - .filter(([key]) => Object.keys(MAPPED_USER_ATTRIBUTE_NAME).includes(key)) - .map(([key, value]) => [MAPPED_USER_ATTRIBUTE_NAME[key], value]), - ); + private static filterAttributeMapping(attributeMapping: Record): { + standard: Record; + custom: Record; + } { + const standard: Record = {}; + const custom: Record = {}; + + for (const [key, value] of Object.entries(attributeMapping)) { + if (key in MAPPED_USER_ATTRIBUTE_NAME) { + standard[MAPPED_USER_ATTRIBUTE_NAME[key]] = value; + } else { + custom[key] = value; + } + } + + return { standard, custom }; } // ── AST rendering helpers ──────────────────────────────────────── @@ -653,7 +663,7 @@ export class AuthRenderer { externalProviders: { readonly oidcProviders: readonly OidcProviderConfig[]; readonly samlProvider: SamlProviderConfig | undefined; - readonly attributeMappings: Readonly>>; + readonly attributeMappings: Readonly; custom: Record }>>; readonly providerScopes: Readonly>; }, callbackUrls?: readonly string[], @@ -844,7 +854,7 @@ export class AuthRenderer { private static createProviderConfig( config: Record, - attributeMapping: Record | undefined, + attributeMapping: { standard: Record; custom: Record } | undefined, ): ts.ObjectLiteralElementLike[] { const properties: ts.ObjectLiteralElementLike[] = []; @@ -870,10 +880,23 @@ export class AuthRenderer { if (attributeMapping) { const mappingProperties: ts.ObjectLiteralElementLike[] = []; - Object.entries(attributeMapping).forEach(([key, value]) => + Object.entries(attributeMapping.standard).forEach(([key, value]) => mappingProperties.push(factory.createPropertyAssignment(factory.createIdentifier(key), factory.createStringLiteral(value))), ); + if (Object.keys(attributeMapping.custom).length > 0) { + const customProperties: ts.ObjectLiteralElementLike[] = []; + Object.entries(attributeMapping.custom).forEach(([key, value]) => + customProperties.push(factory.createPropertyAssignment(factory.createIdentifier(key), factory.createStringLiteral(value))), + ); + mappingProperties.push( + factory.createPropertyAssignment( + factory.createIdentifier('custom'), + factory.createObjectLiteralExpression(customProperties, true), + ), + ); + } + properties.push( factory.createPropertyAssignment( factory.createIdentifier('attributeMapping'), @@ -888,7 +911,7 @@ export class AuthRenderer { private static createProviderPropertyAssignment( name: string, config: Record, - attributeMapping: Record | undefined, + attributeMapping: { standard: Record; custom: Record } | undefined, ): PropertyAssignment { return factory.createPropertyAssignment( factory.createIdentifier(name), From be26bd82ee96f691a81f4f618109730602ba8f36 Mon Sep 17 00:00:00 2001 From: 9pace Date: Mon, 20 Apr 2026 17:16:05 -0400 Subject: [PATCH 2/9] fix(cli): fix ARN nesting and Custom:: resolution in cfn-output-resolver MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two bugs in cfn-output-resolver.ts caused incorrect template resolution during the refactor step: 1. Phase 1 GetAtt resolution passed already-resolved ARN values through buildArn(), producing nested ARNs (e.g. arn:.../userpool/arn:...). Added a guard to short-circuit when the output value is already an ARN. 2. Phase 2 fallback resolution used PhysicalResourceId for Custom:: resources, but custom resource GetAtt attributes come from the backing Lambda's response Data — not the physical ID. These are now skipped so CloudFormation evaluates them at deploy time. --- .../refactor/resolvers/cfn-output-resolver.ts | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/packages/amplify-cli/src/commands/gen2-migration/refactor/resolvers/cfn-output-resolver.ts b/packages/amplify-cli/src/commands/gen2-migration/refactor/resolvers/cfn-output-resolver.ts index bf9bf365045..9b20f694db3 100644 --- a/packages/amplify-cli/src/commands/gen2-migration/refactor/resolvers/cfn-output-resolver.ts +++ b/packages/amplify-cli/src/commands/gen2-migration/refactor/resolvers/cfn-output-resolver.ts @@ -54,6 +54,11 @@ export function resolveOutputs(params: { if (typeof logicalId === 'string' && typeof attrName === 'string') { const outputValue = getAttLookup.get(logicalId); if (outputValue !== undefined && attrName === 'Arn') { + // getAttLookup stores runtime output values. When the output is a + // GetAtt...Arn, the runtime value is already the full ARN. Passing + // it through buildArn() would nest the ARN inside another ARN + // (e.g. arn:.../userpool/arn:.../userpool/poolId). + if (outputValue.startsWith('arn:')) return outputValue; const resourceType = templateResources[logicalId]?.Type; if (resourceType) { const arn = buildArn(resourceType, outputValue, region, accountId); @@ -67,6 +72,17 @@ export function resolveOutputs(params: { }) as Record; // Phase 2: Resolve remaining Fn::GetAtt using physical resource IDs (fallback) + // + // Known limitation: this phase assumes physical resource ID == attribute value, + // which is only true for native AWS resources where PhysicalResourceId is the + // primary identifier. It is incorrect for Custom:: resources (whose GetAtt + // attributes come from Lambda response Data, not the physical ID) and for Arn + // attributes when the physical ID is already an ARN. + // + // A more robust approach would be to only resolve GetAtts for resources that + // are being moved between stacks (since those are the only references that + // will break post-refactor), but that requires threading resource mappings + // into the resolver. cloned.Resources = walkCfnTree(cloned.Resources, (node) => { if ('Fn::GetAtt' in node && Array.isArray(node['Fn::GetAtt']) && Object.keys(node).length === 1) { const [logicalId, attrName] = node['Fn::GetAtt'] as [string, string]; @@ -78,6 +94,11 @@ export function resolveOutputs(params: { const physicalId = stackResource.PhysicalResourceId; const resourceType = stackResource.ResourceType ?? ''; + // Custom resource GetAtt attributes are returned by the backing Lambda + // in its response Data object — they bear no relation to the physical + // resource ID. Leave these unresolved so CloudFormation evaluates them. + if (resourceType.startsWith('Custom::')) return undefined; + // Kinesis streams require ARN in outputs — physical ID is the stream name, not ARN if (resourceType === 'AWS::Kinesis::Stream' && attrName === 'Arn' && !physicalId.startsWith('arn:aws:kinesis')) { throw new AmplifyError('InvalidStackError', { From 2f725c323d01ba92f19af5d003fc7a8335d2e21c Mon Sep 17 00:00:00 2001 From: 9pace Date: Wed, 22 Apr 2026 10:43:35 -0400 Subject: [PATCH 3/9] fix: set DeletionPolicy Retain on oAuth resources during lock Add a new operation to the lock step that sets DeletionPolicy: Retain on HostedUICustomResourceInputs and HostedUIProvidersCustomResourceInputs in the auth nested stack. This prevents social auth custom resources (IDPs and Domain) from being deleted during or after migration. The operation fetches the auth nested stack template, patches the DeletionPolicy for the relevant resources, and updates the stack while preserving existing parameter values. --- .../gen2-migration/_infra/cfn-template.ts | 1 + .../src/commands/gen2-migration/lock.ts | 68 ++++++++++++++++++- 2 files changed, 68 insertions(+), 1 deletion(-) diff --git a/packages/amplify-cli/src/commands/gen2-migration/_infra/cfn-template.ts b/packages/amplify-cli/src/commands/gen2-migration/_infra/cfn-template.ts index 5d544c44f80..e1b3ba4a517 100644 --- a/packages/amplify-cli/src/commands/gen2-migration/_infra/cfn-template.ts +++ b/packages/amplify-cli/src/commands/gen2-migration/_infra/cfn-template.ts @@ -36,6 +36,7 @@ export interface CFNResource { readonly Condition?: string; // DependsOn is mutable: resolvers and buildBlueprint remap dependencies on cloned templates. DependsOn?: string | string[]; + DeletionPolicy?: string; } export interface CFNParameter { diff --git a/packages/amplify-cli/src/commands/gen2-migration/lock.ts b/packages/amplify-cli/src/commands/gen2-migration/lock.ts index 4612f8c3c81..96170962c46 100644 --- a/packages/amplify-cli/src/commands/gen2-migration/lock.ts +++ b/packages/amplify-cli/src/commands/gen2-migration/lock.ts @@ -2,13 +2,25 @@ import { AmplifyMigrationStep } from './_infra/step'; import { AmplifyMigrationOperation, ValidationResult } from './_infra/operation'; import { Plan } from './_infra/plan'; import { AmplifyError } from '@aws-amplify/amplify-cli-core'; -import { SetStackPolicyCommand, GetStackPolicyCommand } from '@aws-sdk/client-cloudformation'; +import { + SetStackPolicyCommand, + GetStackPolicyCommand, + DescribeStackResourcesCommand, + DescribeStacksCommand, + GetTemplateCommand, + UpdateStackCommand, + waitUntilStackUpdateComplete, +} from '@aws-sdk/client-cloudformation'; import { UpdateAppCommand, GetAppCommand } from '@aws-sdk/client-amplify'; import { UpdateTableCommand, paginateListTables } from '@aws-sdk/client-dynamodb'; import { paginateListGraphqlApis } from '@aws-sdk/client-appsync'; +import { CFNTemplate } from './_infra/cfn-template'; const GEN2_MIGRATION_ENVIRONMENT_NAME = 'GEN2_MIGRATION_ENVIRONMENT_NAME'; +const HOSTED_UI_CUSTOM_RESOURCES = ['HostedUICustomResourceInputs', 'HostedUIProvidersCustomResourceInputs']; +const CFN_IAM_CAPABILITY = 'CAPABILITY_NAMED_IAM'; + const LOCK_STATEMENT = { Effect: 'Deny', Action: 'Update:*', @@ -69,6 +81,11 @@ export class AmplifyMigrationLockStep extends AmplifyMigrationStep { }); } + const hostedUiRetainOp = await this.buildHostedUiRetainOperation(); + if (hostedUiRetainOp) { + operations.push(hostedUiRetainOp); + } + operations.push({ validate: () => undefined, describe: async () => [`Add environment variable '${GEN2_MIGRATION_ENVIRONMENT_NAME}' (value: ${this.gen1App.envName})`], @@ -228,6 +245,55 @@ export class AmplifyMigrationLockStep extends AmplifyMigrationStep { return this._dynamoTableNames; } + private async buildHostedUiRetainOperation(): Promise { + const authResource = this.gen1App.discover().find((r) => r.category === 'auth' && r.service === 'Cognito'); + if (!authResource) return undefined; + + const nestedStackPrefix = `auth${authResource.resourceName}`; + const rootResources = await this.gen1App.clients.cloudFormation.send( + new DescribeStackResourcesCommand({ StackName: this.gen1App.rootStackName }), + ); + const authStack = (rootResources.StackResources ?? []).find( + (r) => r.ResourceType === 'AWS::CloudFormation::Stack' && r.LogicalResourceId?.startsWith(nestedStackPrefix), + ); + if (!authStack?.PhysicalResourceId) return undefined; + + const authStackId = authStack.PhysicalResourceId; + const templateResponse = await this.gen1App.clients.cloudFormation.send( + new GetTemplateCommand({ StackName: authStackId, TemplateStage: 'Original' }), + ); + if (!templateResponse.TemplateBody) return undefined; + + const template = JSON.parse(templateResponse.TemplateBody) as CFNTemplate; + const resourcesToRetain = HOSTED_UI_CUSTOM_RESOURCES.filter((id) => id in template.Resources); + if (resourcesToRetain.length === 0) return undefined; + + return { + validate: () => undefined, + describe: async () => [`Set DeletionPolicy: Retain on social auth custom resources: ${resourcesToRetain.join(', ')}`], + execute: async () => { + for (const logicalId of resourcesToRetain) { + template.Resources[logicalId].DeletionPolicy = 'Retain'; + } + const { Stacks } = await this.gen1App.clients.cloudFormation.send(new DescribeStacksCommand({ StackName: authStackId })); + const parameters = (Stacks?.[0]?.Parameters ?? []).map((p) => ({ + ParameterKey: p.ParameterKey, + UsePreviousValue: true, + })); + await this.gen1App.clients.cloudFormation.send( + new UpdateStackCommand({ + StackName: authStackId, + TemplateBody: JSON.stringify(template), + Parameters: parameters, + Capabilities: [CFN_IAM_CAPABILITY], + }), + ); + await waitUntilStackUpdateComplete({ client: this.gen1App.clients.cloudFormation, maxWaitTime: 900 }, { StackName: authStackId }); + this.logger.info(`Set DeletionPolicy: Retain on ${resourcesToRetain.join(', ')}`); + }, + }; + } + private async getExistingStackPolicy(): Promise<{ Statement: Record[] }> { const response = await this.gen1App.clients.cloudFormation.send( new GetStackPolicyCommand({ From 67571e1c30e30efa9142ec8cb8ac6ce4aa0598b3 Mon Sep 17 00:00:00 2001 From: 9pace Date: Wed, 22 Apr 2026 16:15:46 -0400 Subject: [PATCH 4/9] feat: import social auth IDPs and domain during forward refactor Gen1 creates the Cognito domain and social IDPs via a Lambda custom resource, so they aren't in the Gen1 CFN template and get left behind when the UserPool is moved to Gen2. After the move, fetch them live from Cognito and import them into the Gen2 stack via CreateChangeSet(IMPORT), matching providers to Gen2 logical IDs by ProviderName. Imported resources are written with DeletionPolicy: Retain so rollback can orphan them safely. --- .../clients/cognito-identity-provider.ts | 3 + .../auth/auth-cognito-forward.test.ts | 23 +- .../refactor/auth/auth-cognito-forward.ts | 250 +++++++++++++++++- .../commands/gen2-migration/refactor/cfn.ts | 62 +++++ 4 files changed, 333 insertions(+), 5 deletions(-) diff --git a/packages/amplify-cli/src/__tests__/commands/gen2-migration/_framework/clients/cognito-identity-provider.ts b/packages/amplify-cli/src/__tests__/commands/gen2-migration/_framework/clients/cognito-identity-provider.ts index d59bcdb3d99..3ee64acf83b 100644 --- a/packages/amplify-cli/src/__tests__/commands/gen2-migration/_framework/clients/cognito-identity-provider.ts +++ b/packages/amplify-cli/src/__tests__/commands/gen2-migration/_framework/clients/cognito-identity-provider.ts @@ -124,9 +124,12 @@ export class CognitoIdentityProviderMock { const usernameAttributes: string[] = authCliInputs.cognitoConfig.usernameAttributes ?? []; const aliasAttributes: string[] = authCliInputs.cognitoConfig.aliasAttributes ?? []; + const authMeta = this.app.meta.auth?.[authResourceName]; + const domain = authMeta?.output?.HostedUIDomain; return { UserPool: { Id: input.UserPoolId, + Domain: domain, EmailVerificationMessage: authCliInputs.cognitoConfig.emailVerificationMessage, EmailVerificationSubject: authCliInputs.cognitoConfig.emailVerificationSubject, SchemaAttributes: template.Resources.UserPool.Properties.Schema, diff --git a/packages/amplify-cli/src/__tests__/commands/gen2-migration/refactor/auth/auth-cognito-forward.test.ts b/packages/amplify-cli/src/__tests__/commands/gen2-migration/refactor/auth/auth-cognito-forward.test.ts index 9bcc6dccbe7..84b82e34474 100644 --- a/packages/amplify-cli/src/__tests__/commands/gen2-migration/refactor/auth/auth-cognito-forward.test.ts +++ b/packages/amplify-cli/src/__tests__/commands/gen2-migration/refactor/auth/auth-cognito-forward.test.ts @@ -18,7 +18,12 @@ import { ResourceMapping, } from '@aws-sdk/client-cloudformation'; import { SSMClient } from '@aws-sdk/client-ssm'; -import { CognitoIdentityProviderClient, DescribeIdentityProviderCommand } from '@aws-sdk/client-cognito-identity-provider'; +import { + CognitoIdentityProviderClient, + DescribeIdentityProviderCommand, + DescribeUserPoolCommand, + ListIdentityProvidersCommand, +} from '@aws-sdk/client-cognito-identity-provider'; import { Cfn } from '../../../../../commands/gen2-migration/refactor/cfn'; const ts = new Date(); @@ -197,8 +202,19 @@ describe('AuthCognitoForwardRefactorer.plan() — operation sequence', () => { cfnMock.on(DeleteChangeSetCommand).resolves({}); const cognitoMock = mockClient(CognitoIdentityProviderClient); + cognitoMock.on(DescribeUserPoolCommand).resolves({ + UserPool: { Id: 'us-east-1_ABC123', Domain: 'test-domain' }, + }); + cognitoMock.on(ListIdentityProvidersCommand).resolves({ + Providers: [{ ProviderName: 'Google', ProviderType: 'Google' }], + }); cognitoMock.on(DescribeIdentityProviderCommand).resolves({ - IdentityProvider: { ProviderDetails: { client_id: 'google-id', client_secret: 'google-secret' } }, + IdentityProvider: { + ProviderName: 'Google', + ProviderType: 'Google', + ProviderDetails: { client_id: 'google-id', client_secret: 'google-secret', authorize_scopes: 'openid email profile' }, + AttributeMapping: { email: 'email' }, + }, }); const clients = new (AwsClients as any)({ region: 'us-east-1' }); @@ -218,7 +234,8 @@ describe('AuthCognitoForwardRefactorer.plan() — operation sequence', () => { const ops = await refactorer.plan(); - expect(cognitoMock.commandCalls(DescribeIdentityProviderCommand)).toHaveLength(1); + // Called once by retrieveOAuthValues and once by fetchSocialAuthConfig + expect(cognitoMock.commandCalls(DescribeIdentityProviderCommand)).toHaveLength(2); expect(ops.length).toBeGreaterThanOrEqual(4); const { CreateChangeSetCommand: CreateCS } = await import('@aws-sdk/client-cloudformation'); diff --git a/packages/amplify-cli/src/commands/gen2-migration/refactor/auth/auth-cognito-forward.ts b/packages/amplify-cli/src/commands/gen2-migration/refactor/auth/auth-cognito-forward.ts index 93cc90f5192..1ced56473ed 100644 --- a/packages/amplify-cli/src/commands/gen2-migration/refactor/auth/auth-cognito-forward.ts +++ b/packages/amplify-cli/src/commands/gen2-migration/refactor/auth/auth-cognito-forward.ts @@ -1,8 +1,17 @@ -import { Output, Parameter } from '@aws-sdk/client-cloudformation'; +import { Output, Parameter, ResourceToImport } from '@aws-sdk/client-cloudformation'; +import { + DescribeUserPoolCommand, + DescribeIdentityProviderCommand, + ListIdentityProvidersCommand, +} from '@aws-sdk/client-cognito-identity-provider'; import { AmplifyError } from '@aws-amplify/amplify-cli-core'; import { retrieveOAuthValues } from '../oauth-values-retriever'; import { ForwardCategoryRefactorer } from '../workflow/forward-category-refactorer'; +import { RefactorBlueprint } from '../workflow/category-refactorer'; import { CFNResource } from '../../_infra/cfn-template'; +import { AmplifyMigrationOperation } from '../../_infra/operation'; +import { extractStackNameFromId } from '../utils'; +import CLITable from 'cli-table3'; const HOSTED_PROVIDER_META_PARAMETER_NAME = 'hostedUIProviderMeta'; const HOSTED_PROVIDER_CREDENTIALS_PARAMETER_NAME = 'hostedUIProviderCreds'; @@ -19,6 +28,7 @@ export const USER_POOL_TYPE = 'AWS::Cognito::UserPool'; export const IDENTITY_POOL_TYPE = 'AWS::Cognito::IdentityPool'; export const IDENTITY_POOL_ROLE_ATTACHMENT_TYPE = 'AWS::Cognito::IdentityPoolRoleAttachment'; export const USER_POOL_DOMAIN_TYPE = 'AWS::Cognito::UserPoolDomain'; +export const USER_POOL_IDENTITY_PROVIDER_TYPE = 'AWS::Cognito::UserPoolIdentityProvider'; export const RESOURCE_TYPES = [ USER_POOL_TYPE, @@ -26,20 +36,43 @@ export const RESOURCE_TYPES = [ IDENTITY_POOL_TYPE, IDENTITY_POOL_ROLE_ATTACHMENT_TYPE, USER_POOL_DOMAIN_TYPE, + USER_POOL_IDENTITY_PROVIDER_TYPE, ]; +interface IdpConfig { + readonly providerName: string; + readonly providerType: string; + readonly clientId: string; + readonly clientSecret: string; + readonly authorizeScopes: string; + readonly attributeMapping: Record; +} + +interface SocialAuthConfig { + readonly userPoolId: string; + readonly domain: string; + readonly providers: IdpConfig[]; +} + /** * Forward refactorer for the auth:Cognito resource. * * Moves main auth resources from Gen1 to Gen2. + * For social auth apps, imports Gen1's LambdaCallout-created IDPs and domain + * into the Gen2 stack as native CFN resources during the move phase. */ export class AuthCognitoForwardRefactorer extends ForwardCategoryRefactorer { + /** + * Returns the full set including domain and IDP types. These types don't exist in the + * Gen1 CFN template (they're created by a Lambda trigger), so they won't appear in the + * refactor mappings. They are imported into Gen2 as a separate step in move(). + */ protected resourceTypes(): string[] { return RESOURCE_TYPES; } /** - * OAuth hook: retrieves credentials and updates hostedUIProviderCreds parameter. + * OAuth hook: retrieves credentials and updates the hostedUIProviderCreds parameter. */ protected override async resolveOAuthParameters(parameters: Parameter[], outputs: Output[]): Promise { const oAuthParam = parameters.find((p) => p.ParameterKey === HOSTED_PROVIDER_META_PARAMETER_NAME); @@ -68,9 +101,104 @@ export class AuthCognitoForwardRefactorer extends ForwardCategoryRefactorer { }); } credsParam.ParameterValue = JSON.stringify(oAuthValues); + return parameters; } + /** + * Executes the standard resource refactor, then imports Gen1's + * physical domain and IDPs into the Gen2 stack as native CFN resources. + */ + protected override async move(blueprint: RefactorBlueprint): Promise { + const baseOps = await super.move(blueprint); + + const importOp = await this.buildImportSocialAuthOperation(blueprint); + if (importOp) { + return [...baseOps, importOp]; + } + + return baseOps; + } + + /** + * Builds an operation that imports Gen1's physical domain and IDPs into the + * Gen2 stack. Returns undefined if the app doesn't use social auth. + */ + private async buildImportSocialAuthOperation(blueprint: RefactorBlueprint): Promise { + const socialAuthConfig = await this.fetchSocialAuthConfig(blueprint.sourceStackId); + if (!socialAuthConfig) { + return undefined; + } + + const gen2StackId = blueprint.targetStackId; + const gen2Template = await this.cfn.fetchTemplate(gen2StackId); + const gen2IdpLogicalIds = new Map(); + let gen2DomainLogicalId: string | undefined; + + // Find the Gen2 logical IDs we'll import the physical Gen1 resources into + // We require providerName + logicalId to disambiguate between multiple providers + for (const [logicalId, resource] of Object.entries(gen2Template.Resources)) { + if (resource.Type === USER_POOL_DOMAIN_TYPE) { + gen2DomainLogicalId = logicalId; + } else if (resource.Type === USER_POOL_IDENTITY_PROVIDER_TYPE) { + const providerName = resource.Properties.ProviderName as string; + if (providerName) { + gen2IdpLogicalIds.set(providerName, logicalId); + } + } + } + + if (!gen2DomainLogicalId) { + this.debug('No Gen2 UserPoolDomain resource found — skipping import'); + return undefined; + } + + if (gen2IdpLogicalIds.size === 0) { + this.debug('No Gen2 UserPoolIdentityProvider resources found — skipping import'); + return undefined; + } + + return { + resource: this.resource, + validate: () => undefined, + describe: async () => { + const gen2StackName = extractStackNameFromId(gen2StackId); + const table = new CLITable({ + head: ['Source Physical ID', 'Target Logical ID'], + style: { head: [] }, + }); + table.push([socialAuthConfig.domain, gen2DomainLogicalId!]); + for (const provider of socialAuthConfig.providers) { + const logicalId = gen2IdpLogicalIds.get(provider.providerName); + if (logicalId) { + const label = + provider.providerType !== provider.providerName + ? `${provider.providerName} (${provider.providerType})` + : provider.providerName; + table.push([label, logicalId]); + } + } + return [`Import social auth resources into '${gen2StackName}'\n\n${table.toString()}`]; + }, + execute: async () => { + const templateForImport = await this.cfn.fetchTemplate(gen2StackId); + + const { resourcesToImport, templateAdditions } = this.buildImportSpec(socialAuthConfig, gen2DomainLogicalId!, gen2IdpLogicalIds); + + for (const [logicalId, resource] of Object.entries(templateAdditions)) { + templateForImport.Resources[logicalId] = resource; + } + + await this.cfn.importResources({ + stackName: gen2StackId, + templateBody: templateForImport, + resourcesToImport, + resource: this.resource, + }); + }, + }; + } + protected override match(sourceId: string, sourceResource: CFNResource, targetId: string, targetResource: CFNResource): boolean { if (sourceResource.Type !== targetResource.Type) { return false; @@ -101,4 +229,122 @@ export class AuthCognitoForwardRefactorer extends ForwardCategoryRefactorer { // in gen2 all auth resources are in a single auth nested stack return this.findNestedStack(this.gen2Branch, 'auth'); } + + /** + * Fetches domain and IDP config directly from Cognito. These resources are + * Lambda-created (not in the Gen1 CFN template) so the live API is the only source. + */ + private async fetchSocialAuthConfig(sourceStackId: string): Promise { + const sourceStack = await this.gen1Env.fetchStack(sourceStackId); + const userPoolId = (sourceStack.Outputs ?? []).find((o) => o.OutputKey === USER_POOL_ID_OUTPUT_KEY_NAME)?.OutputValue; + if (!userPoolId) { + return undefined; + } + + const cognitoClient = this.gen1App.clients.cognitoIdentityProvider; + + const poolResponse = await cognitoClient.send(new DescribeUserPoolCommand({ UserPoolId: userPoolId })); + const domain = poolResponse?.UserPool?.Domain; + if (!domain) { + this.debug('Gen1 UserPool has no domain — skipping social auth import'); + return undefined; + } + + const listResponse = await cognitoClient.send(new ListIdentityProvidersCommand({ UserPoolId: userPoolId })); + const providerSummaries = listResponse?.Providers ?? []; + if (providerSummaries.length === 0) { + this.debug('Gen1 UserPool has no identity providers — skipping social auth import'); + return undefined; + } + + const providers: IdpConfig[] = []; + for (const summary of providerSummaries) { + const providerName = summary.ProviderName; + if (!providerName) continue; + + const describeResponse = await cognitoClient.send( + new DescribeIdentityProviderCommand({ UserPoolId: userPoolId, ProviderName: providerName }), + ); + const idp = describeResponse.IdentityProvider; + if (!idp?.ProviderDetails) continue; + + providers.push({ + providerName, + providerType: idp.ProviderType ?? providerName, + clientId: idp.ProviderDetails.client_id ?? '', + clientSecret: idp.ProviderDetails.client_secret ?? '', + authorizeScopes: idp.ProviderDetails.authorize_scopes ?? '', + attributeMapping: (idp.AttributeMapping as Record) ?? {}, + }); + } + + this.debug(`Fetched social auth config: domain=${domain}, providers=${providers.map((p) => p.providerName).join(',')}`); + return { userPoolId, domain, providers }; + } + + /** + * Builds the CFN import spec: template additions with DeletionPolicy: Retain + * (so rollback can orphan them without deleting the physical resources) and + * resource identifiers for the import change set. + */ + private buildImportSpec( + config: SocialAuthConfig, + domainLogicalId: string, + idpLogicalIds: Map, + ): { resourcesToImport: ResourceToImport[]; templateAdditions: Record } { + const resourcesToImport: ResourceToImport[] = []; + const templateAdditions: Record = {}; + + templateAdditions[domainLogicalId] = { + Type: USER_POOL_DOMAIN_TYPE, + DeletionPolicy: 'Retain', + Properties: { + Domain: config.domain, + UserPoolId: config.userPoolId, + }, + }; + resourcesToImport.push({ + ResourceType: USER_POOL_DOMAIN_TYPE, + LogicalResourceId: domainLogicalId, + ResourceIdentifier: { + UserPoolId: config.userPoolId, + Domain: config.domain, + }, + }); + + for (const provider of config.providers) { + const logicalId = idpLogicalIds.get(provider.providerName); + if (!logicalId) { + this.debug(`No Gen2 logical ID for provider ${provider.providerName} — skipping import`); + continue; + } + + templateAdditions[logicalId] = { + Type: USER_POOL_IDENTITY_PROVIDER_TYPE, + DeletionPolicy: 'Retain', + Properties: { + UserPoolId: config.userPoolId, + ProviderName: provider.providerName, + ProviderType: provider.providerType, + ProviderDetails: { + client_id: provider.clientId, + client_secret: provider.clientSecret, + authorize_scopes: provider.authorizeScopes, + }, + AttributeMapping: provider.attributeMapping, + }, + }; + + resourcesToImport.push({ + ResourceType: USER_POOL_IDENTITY_PROVIDER_TYPE, + LogicalResourceId: logicalId, + ResourceIdentifier: { + UserPoolId: config.userPoolId, + ProviderName: provider.providerName, + }, + }); + } + + return { resourcesToImport, templateAdditions }; + } } diff --git a/packages/amplify-cli/src/commands/gen2-migration/refactor/cfn.ts b/packages/amplify-cli/src/commands/gen2-migration/refactor/cfn.ts index 613e5e8c855..50c15dd9393 100644 --- a/packages/amplify-cli/src/commands/gen2-migration/refactor/cfn.ts +++ b/packages/amplify-cli/src/commands/gen2-migration/refactor/cfn.ts @@ -14,12 +14,14 @@ import { GetTemplateCommand, Parameter, ResourceMapping, + ResourceToImport, Stack, UpdateStackCommand, UpdateStackCommandInput, waitUntilChangeSetCreateComplete, waitUntilStackCreateComplete, waitUntilStackDeleteComplete, + waitUntilStackImportComplete, waitUntilStackRefactorCreateComplete, waitUntilStackRefactorExecuteComplete, waitUntilStackUpdateComplete, @@ -279,6 +281,55 @@ export class Cfn { await waitUntilStackUpdateComplete({ client: this.client, maxWaitTime: MAX_WAIT_TIME_SECONDS }, { StackName: changeSet.StackName }); } + /** + * Imports existing physical resources into a stack via CreateChangeSet(IMPORT). + * The template must include resource definitions matching the physical state. + */ + public async importResources(params: { + readonly stackName: string; + readonly templateBody: CFNTemplate; + readonly resourcesToImport: ResourceToImport[]; + readonly resource?: DiscoveredResource; + }): Promise { + const { stackName, templateBody, resourcesToImport, resource } = params; + const displayName = extractStackNameFromId(stackName); + const changeSetName = `import-resources-${Date.now()}`; + + writeImportSnapshot({ + stackName, + templateBody: JSON.stringify(templateBody), + parameters: [], + resourcesToImport, + }); + + this.info(`Creating import changeset for ${displayName} (${resourcesToImport.length} resource(s))`, resource); + + await this.client.send( + new CreateChangeSetCommand({ + StackName: stackName, + ChangeSetName: changeSetName, + ChangeSetType: 'IMPORT', + TemplateBody: JSON.stringify(templateBody), + ResourcesToImport: resourcesToImport, + Capabilities: [CFN_IAM_CAPABILITY], + }), + ); + + this.info(`Waiting for import changeset creation: ${displayName}`, resource); + await waitUntilChangeSetCreateComplete( + { client: this.client, maxWaitTime: 120 }, + { StackName: stackName, ChangeSetName: changeSetName }, + ); + + this.info(`Executing import changeset: ${displayName}`, resource); + await this.client.send(new ExecuteChangeSetCommand({ StackName: stackName, ChangeSetName: changeSetName })); + + this.info(`Waiting for import to complete: ${displayName}`, resource); + await waitUntilStackImportComplete({ client: this.client, maxWaitTime: MAX_WAIT_TIME_SECONDS }, { StackName: stackName }); + + this.info(`Import complete: ${displayName}`, resource); + } + /** * Deletes a change set without executing it. */ @@ -431,6 +482,17 @@ interface WriteUpdateSnapshotInput { readonly parameters: Parameter[]; } +interface WriteImportSnapshotInput extends WriteUpdateSnapshotInput { + readonly resourcesToImport: ResourceToImport[]; +} + +function writeImportSnapshot(input: WriteImportSnapshotInput): void { + const stackName = extractStackNameFromId(input.stackName); + fs.writeFileSync(path.join(OUTPUT_DIRECTORY, `import.${stackName}.template.json`), formatTemplateBody(input.templateBody)); + fs.writeFileSync(path.join(OUTPUT_DIRECTORY, `import.${stackName}.parameters.json`), JSON.stringify(input.parameters, null, 2)); + fs.writeFileSync(path.join(OUTPUT_DIRECTORY, `import.${stackName}.resources.json`), JSON.stringify(input.resourcesToImport, null, 2)); +} + function writeUpdateSnapshot(input: WriteUpdateSnapshotInput): void { const stackName = extractStackNameFromId(input.stackName); fs.writeFileSync(path.join(OUTPUT_DIRECTORY, `update.${stackName}.template.json`), formatTemplateBody(input.templateBody)); From 232184ea3621867c28eac0364b164e2de9084ea7 Mon Sep 17 00:00:00 2001 From: 9pace Date: Wed, 29 Apr 2026 15:45:57 -0400 Subject: [PATCH 5/9] feat: orphan + import social auth resources during refactor Implement the Orphan + Import approach for handling OAuth/social login (Google, Facebook) during gen2-migration refactor. Instead of moving UserPoolDomain and UserPoolIdentityProvider through the holding stack (which breaks due to Fn::GetAtt references to AmplifySecretFetcherResource), orphan them from Gen2 and re-import them as native CFN resources. Forward: beforeMove sets DeletionPolicy Retain then orphans IDPs and domain from Gen2. move() imports Gen1 physical IDPs and domain into Gen2 under Gen2 logical IDs after the core StackRefactor. Rollback: move() orphans imported IDPs and domain from Gen2 after the core Gen2-to-Gen1 StackRefactor. afterMove() re-imports Gen2 original IDPs and domain after restoring P2 from the holding stack. Also fixes Ref resolution in cfn-output-resolver to fall back to PhysicalResourceId for intra-stack references not exposed via stack outputs. This resolves UserPoolClient SupportedIdentityProviders references to IDP logical IDs during the holding stack move. ADR-005 updated with Orphan + Import addendum. --- ...efactor-social-auth-idp-domain-handling.md | 478 ++++++++++++ .../generate/amplify/auth/auth.generator.ts | 65 +- .../refactor/auth/auth-cognito-forward.ts | 679 +++++++++++++----- .../refactor/auth/auth-cognito-rollback.ts | 265 ++++++- .../refactor/resolvers/cfn-output-resolver.ts | 4 +- .../resolvers/cfn-parameter-resolver.ts | 29 + .../refactor/workflow/category-refactorer.ts | 105 ++- .../workflow/forward-category-refactorer.ts | 26 +- .../workflow/rollback-category-refactorer.ts | 16 +- 9 files changed, 1430 insertions(+), 237 deletions(-) create mode 100644 packages/amplify-cli/adr/005-refactor-social-auth-idp-domain-handling.md diff --git a/packages/amplify-cli/adr/005-refactor-social-auth-idp-domain-handling.md b/packages/amplify-cli/adr/005-refactor-social-auth-idp-domain-handling.md new file mode 100644 index 00000000000..c73b59d82fc --- /dev/null +++ b/packages/amplify-cli/adr/005-refactor-social-auth-idp-domain-handling.md @@ -0,0 +1,478 @@ +# ADR-005: Social Auth IDP & Domain Handling During Refactor + +## Status + +Accepted — revised after integration testing (see Addendum) + +## Context + +When `amplify gen2-migration refactor` runs on an app with social identity providers +(Google, Facebook), the command must transfer the UserPool from the Gen1 stack to the +Gen2 stack. The UserPool carries associated resources — identity providers (IDPs) and a +UserPool domain — that differ fundamentally between Gen1 and Gen2. + +### How IDPs and Domains Differ Between Gen1 and Gen2 + +In Gen1, the Cognito domain and identity providers are **not native CloudFormation +resources**. They're created by Lambda-backed custom resources: + +``` +Gen1 auth stack: + HostedUICustomResourceInputs: Custom::LambdaCallout → creates UserPoolDomain + HostedUIProvidersCustomResourceInputs: Custom::LambdaCallout → creates Google/Facebook IDPs +``` + +CloudFormation doesn't know these are Cognito IDPs or a domain — it just knows it +invoked a Lambda. The physical IDPs and domain exist on the UserPool but are not +tracked as native CFN resources. + +In Gen2, the same resources are **native CloudFormation resources**: + +``` +Gen2 auth stack: + amplifyAuthUserPoolUserPoolDomain1F688B5B: AWS::Cognito::UserPoolDomain + amplifyAuthGoogleIdPA9736819: AWS::Cognito::UserPoolIdentityProvider + amplifyAuthFacebookIDP7CB5B5CC: AWS::Cognito::UserPoolIdentityProvider +``` + +Gen2 IDP resources reference `AmplifySecretFetcherResource` via `Fn::GetAtt` to fetch +OAuth secrets from SSM Parameter Store at deploy time: + +```json +{ + "Type": "AWS::Cognito::UserPoolIdentityProvider", + "Properties": { + "ProviderName": "Google", + "ProviderType": "Google", + "UserPoolId": { "Ref": "amplifyAuthUserPool4BA7F805" }, + "ProviderDetails": { + "client_id": { "Fn::GetAtt": ["AmplifySecretFetcherResource", "GOOGLE_CLIENT_ID"] }, + "client_secret": { "Fn::GetAtt": ["AmplifySecretFetcherResource", "GOOGLE_CLIENT_SECRET"] }, + "authorize_scopes": "openid email profile" + }, + "AttributeMapping": { "email": "email", "username": "sub" } + } +} +``` + +### Current Refactor Flow + +The auth refactor currently moves only Cognito-typed resources. The `RESOURCE_TYPES` +list determines what gets filtered: + +``` +Step 1: updateSource — resolve Gen1 template (no-op changeset) +Step 2: updateTarget — resolve Gen2 template (no-op changeset) +Step 3: beforeMove — move Gen2 resources to holding stack (6 resources) +Step 4: move — move Gen1 resources into Gen2 stack (5 resources) +``` + +**Step 3 detail: Gen2 → holding (6 resources)** + +| Gen2 Logical ID | Type | +|---|---| +| `amplifyAuthUserPool4BA7F805` | `AWS::Cognito::UserPool` | +| `amplifyAuthUserPoolUserPoolDomain1F688B5B` | `AWS::Cognito::UserPoolDomain` | +| `amplifyAuthUserPoolNativeAppClient79534448` | `AWS::Cognito::UserPoolClient` | +| `amplifyAuthUserPoolAppClient2626C6F8` | `AWS::Cognito::UserPoolClient` | +| `amplifyAuthIdentityPool3FDE84CC` | `AWS::Cognito::IdentityPool` | +| `amplifyAuthIdentityPoolRoleAttachment045F17C8` | `AWS::Cognito::IdentityPoolRoleAttachment` | + +**Step 4 detail: Gen1 → Gen2 (5 resources)** + +| Gen1 Logical ID | → Gen2 Logical ID | Type | +|---|---|---| +| `UserPool` | `amplifyAuthUserPool4BA7F805` | UserPool | +| `UserPoolClientWeb` | `amplifyAuthUserPoolAppClient2626C6F8` | UserPoolClient | +| `UserPoolClient` | `amplifyAuthUserPoolNativeAppClient79534448` | UserPoolClient | +| `IdentityPool` | `amplifyAuthIdentityPool3FDE84CC` | IdentityPool | +| `IdentityPoolRoleMap` | `amplifyAuthIdentityPoolRoleAttachment045F17C8` | IdentityPoolRoleAttachment | + +**Key observation:** 6 resources leave Gen2 but only 5 arrive from Gen1. The +`UserPoolDomain` goes to holding and nothing replaces it — Gen1's domain is managed by +`Custom::LambdaCallout`, not a native CFN resource. + +**What is NOT moved — and why it's a problem:** + +| Gen2 Logical ID | Type | Problem | +|---|---|---| +| `amplifyAuthGoogleIdPA9736819` | UserPoolIdentityProvider | Physical state: IDP on Gen2 pool. Template ref: now resolves to Gen1 pool. | +| `amplifyAuthFacebookIDP7CB5B5CC` | UserPoolIdentityProvider | Same. | +| `AmplifySecretFetcherResource` | Custom::AmplifySecretFetcherResource | Stays. No issue — fetches from SSM. | + +After the refactor, the Gen2 IDP resources' `UserPoolId` ref resolves to the Gen1 pool. +`UserPoolId` is a replacement property on `AWS::Cognito::UserPoolIdentityProvider`. +On the next Gen2 deploy, CFN attempts to CREATE new IDPs on the Gen1 pool, which already +has IDPs from LambdaCallout → `DuplicateProviderException`. The domain has a similar +conflict: CFN attempts to CREATE a new domain, but Cognito allows only one domain per +pool → `InvalidRequest`. + +### Approaches Evaluated + +**Case 1 — Do nothing.** The refactor succeeds but the next deploy fails with +`DuplicateProviderException` (IDPs) and `InvalidRequest` (domain). Social login works +immediately after refactor but all future deployments are blocked. + +**Case 2 — Override.** Add `UserPoolIdentityProvider` to `RESOURCE_TYPES` so Gen2 IDPs +move to the holding stack (8 resources total). On the next deploy, CDK synthesizes fresh +IDP and domain resources. However, the Gen1 pool already has IDPs and a domain (from +LambdaCallout), so CFN CREATE fails. The user must manually delete Gen1's IDPs and domain +via Cognito API before deploying — causing auth downtime during the gap. + +User impact of Case 2's manual deletion: +- Existing Cognito tokens (access, ID, refresh) remain valid — tokens are issued by the + UserPool, not the IDP +- Social login becomes immediately unavailable — no new sign-ins via Google/Facebook +- Refresh tokens that trigger re-authentication via the IDP will fail +- User records are NOT deleted — the identity link is preserved in user attributes +- Gap duration: typically minutes (deploy completion), but extends if deploy fails +- Domain deletion takes the hosted UI down immediately; Gen2 deploy creates a new domain + with a different prefix, requiring OAuth redirect URI updates in Google/Facebook consoles + +**Case 3 — Import.** Same holding stack move as Case 2, but after the Gen1 pool moves in, +use CloudFormation's `CreateChangeSet(ChangeSetType=IMPORT)` to adopt the Gen1 pool's +existing IDPs and domain into the Gen2 stack as native CFN resources. No deletion, no +recreation, no downtime. + +Why import is possible: Gen1's IDPs and domain are created by `Custom::LambdaCallout`. +CFN tracks them as custom resource invocations, not as `AWS::Cognito::UserPoolIdentityProvider` +or `AWS::Cognito::UserPoolDomain`. They're physically present on the pool but +**CFN-unmanaged as native types**, making them eligible for import. + +## Evidence + +### Case 2 failure (override approach) + +Experimentally verified using `cfn-import-test/override-test.mjs` against UserPool +`us-east-1_9UB4KPsWg` (app `mediavault-import-test`, ID `d1igd1gdzabina`): + +| Resource | CFN CREATE Result | +|----------|-------------------| +| `UserPoolIdentityProvider` (Google) | `HandlerErrorCode: AlreadyExists` | +| `UserPoolIdentityProvider` (Facebook) | `HandlerErrorCode: AlreadyExists` | +| `UserPoolDomain` (different prefix) | `HandlerErrorCode: InvalidRequest` | + +Both test stacks reached `ROLLBACK_COMPLETE`. Failed CREATE attempts did not damage +existing physical resources — domain and IDPs unchanged after test. + +**Conclusion**: Case 2 requires manual IDP/domain deletion before the next deploy, +causing unavoidable auth downtime. Not viable for production apps. + +### Case 3 success (import approach) + +Experimentally verified in two phases against the same UserPool: + +**Phase 2 — Raw CFN API import** (`cfn-import-test/import-test.mjs`): +- `CreateChangeSet(ChangeSetType=IMPORT)` succeeded for all three resource types +- Stack reached `IMPORT_COMPLETE`, all resources `UPDATE_COMPLETE` +- Non-destructive: `CreationDate` and `LastModifiedDate` unchanged +- Import identifier for `UserPoolDomain` requires both `{UserPoolId, Domain}` (not + just `{Domain}`) +- Template only needs `client_id`, `client_secret`, `authorize_scopes` in + ProviderDetails — auto-populated URLs (`attributes_url`, `authorize_url`, etc.) + are NOT needed + +**Phase 3 — CDK import + deploy** (`cfn-import-test/cdk-import-test/`): +- `cdk import --resource-mapping --force` succeeded (fully non-interactive) +- `cdk deploy` succeeded — only `CDKMetadata` created, imported resources untouched +- `cdk diff` post-deploy: zero differences +- IDP `LastModifiedDate` unchanged (2026-04-16) — no update API calls made +- Deploy time: ~13 seconds +- Resource mapping keys must use **CFN logical IDs**, not CDK construct paths + +**Conclusion**: Case 3 is viable. Import is non-destructive, zero-downtime, and produces +a clean state where subsequent deploys see zero drift. + +### Evidence artifacts + +| Artifact | Path | +|----------|------| +| Raw CFN import script | `oauth-workspace/cfn-import-test/import-test.mjs` | +| CDK import app | `oauth-workspace/cfn-import-test/cdk-import-test/` | +| CDK resource mapping | `oauth-workspace/cfn-import-test/cdk-import-test/resource-mapping.json` | +| Override failure script | `oauth-workspace/cfn-import-test/override-test.mjs` | +| Full test results | `oauth-workspace/cfn-import-test/full-context.md` | +| Summary | `oauth-workspace/cfn-import-test/summary.md` | +| Gen1 test app | `oauth-workspace/mediavault-import-test/` (App ID `d1igd1gdzabina`, UserPool `us-east-1_9UB4KPsWg`) | + +## Decision + +**Case 3 (Import)** is the chosen approach. + +### Implementation + +#### 1. Add `UserPoolIdentityProvider` to `RESOURCE_TYPES` + +```typescript +export const RESOURCE_TYPES = [ + USER_POOL_TYPE, + USER_POOL_CLIENT_TYPE, + IDENTITY_POOL_TYPE, + IDENTITY_POOL_ROLE_ATTACHMENT_TYPE, + USER_POOL_DOMAIN_TYPE, + USER_POOL_IDENTITY_PROVIDER_TYPE, // added +]; +``` + +Gen2 IDP resources now move to the holding stack alongside other Gen2 Cognito resources +during `beforeMove()` (8 resources instead of 6). This clears the Gen2 stack's IDP +logical IDs so the import can target them. + +#### 2. Import Gen1 IDPs and domain in `move()` (appended after standard refactor) + +`AuthCognitoForwardRefactorer` overrides `move()` to append an import operation after +the standard `CreateStackRefactor`. The import step: + +1. Fetches the Gen1 pool's domain and IDP configuration from the Cognito API + (`DescribeUserPool`, `ListIdentityProviders`, `DescribeIdentityProvider`) +2. Finds the corresponding Gen2 logical IDs in the Gen2 template (matching by + resource type and `ProviderName`) +3. Builds a CFN import changeset with `DeletionPolicy: Retain` on all imported + resources +4. Executes the import via `Cfn.importResources()` + +If the app has no social auth (no domain, no IDPs), the import step is skipped — non- +social-auth apps are completely unaffected. + +**Import identifiers:** + +| Resource Type | Identifier Keys | Example Values | +|---------------|-----------------|----------------| +| `AWS::Cognito::UserPoolDomain` | `{UserPoolId, Domain}` | `us-east-1_sL46vhIrI`, `mediavaultfresh-devenv` | +| `AWS::Cognito::UserPoolIdentityProvider` | `{UserPoolId, ProviderName}` | `us-east-1_sL46vhIrI`, `Google` | + +**Conceptual CFN API call:** + +```typescript +await cfn.send(new CreateChangeSetCommand({ + StackName: gen2AuthStackName, + ChangeSetName: 'import-social-auth-resources', + ChangeSetType: 'IMPORT', + TemplateBody: JSON.stringify(gen2TemplateWithIDPsAndDomain), + ResourcesToImport: [ + { + ResourceType: 'AWS::Cognito::UserPoolDomain', + LogicalResourceId: 'amplifyAuthUserPoolUserPoolDomain1F688B5B', + ResourceIdentifier: { UserPoolId: 'us-east-1_sL46vhIrI', Domain: 'mediavaultfresh-devenv' }, + }, + { + ResourceType: 'AWS::Cognito::UserPoolIdentityProvider', + LogicalResourceId: 'amplifyAuthGoogleIdPA9736819', + ResourceIdentifier: { UserPoolId: 'us-east-1_sL46vhIrI', ProviderName: 'Google' }, + }, + { + ResourceType: 'AWS::Cognito::UserPoolIdentityProvider', + LogicalResourceId: 'amplifyAuthFacebookIDP7CB5B5CC', + ResourceIdentifier: { UserPoolId: 'us-east-1_sL46vhIrI', ProviderName: 'Facebook' }, + }, + ], +})); +``` + +**Template requirements for import:** The template submitted with the import changeset +must include resource definitions with properties matching the physical resource state. +For IDPs, only `client_id`, `client_secret`, and `authorize_scopes` are needed in +`ProviderDetails`. For the domain, the template must use the Gen1 domain prefix (not +Gen2's auto-generated prefix). + +#### 3. Decommission safety via `DeletionPolicy: Retain` + +After the refactor moves the Gen1 pool to Gen2, Gen1's stack still contains the +LambdaCallout custom resources (`HostedUICustomResourceInputs`, +`HostedUIProvidersCustomResourceInputs`) with `DeletionPolicy: Delete`. These reference +the pool via `{Ref: UserPool}`, which `resolveSource` has already hardcoded to the +literal pool ID. When `decommission` deletes the Gen1 stack, the Lambda delete handlers +fire and call `cognito-idp:DeleteUserPoolDomain` and delete the IDPs from the pool — +which is now in Gen2. This destroys social login. + +Two layers of protection: + +- **Imported resources in Gen2**: Set `DeletionPolicy: Retain` on all imported IDP and + domain resources. If rollback removes them from the template, CloudFormation orphans + them instead of deleting the physical resources. + +- **Gen1 LambdaCallout resources**: Set `DeletionPolicy: Retain` on + `HostedUICustomResourceInputs` and `HostedUIProvidersCustomResourceInputs` in the + Gen1 stack during `updateSource()`. This prevents the Lambda delete handlers from + firing when the Gen1 stack is decommissioned. (Implemented in commit `2f725c323d`.) + +### Domain handling + +The Gen1 domain prefix (e.g., `mediavaultfresh-devenv`) differs from Gen2's +auto-generated prefix (e.g., `8f20214d49ee8819a1c1`). The import uses the Gen1 domain +value. On the next Gen2 deploy, the domain is a replacement property — if the generated +code specifies a different prefix, CloudFormation would delete and recreate the domain, +which takes the hosted UI down and changes the URL. + +The `generate` command should produce a domain override in `backend.ts`: + +```typescript +import { CfnUserPoolDomain } from 'aws-cdk-lib/aws-cognito'; + +const cfnUserPoolDomain = backend.auth.resources.userPool.node + .findChild('UserPoolDomain') + .node.defaultChild as CfnUserPoolDomain; +cfnUserPoolDomain.domain = 'mediavaultfresh-devenv'; // Gen1 domain prefix +``` + +Implementation note: `auth.generator.ts` `contributeProviderSetup()` already emits a +commented-out `tryRemoveChild("UserPoolDomain")`. This should be replaced with the +domain override escape hatch, using the Gen1 domain value from `DescribeUserPool` or +the `hostedUIDomainName` parameter. This is tracked as an open item in the generate +command. + +### Rollback considerations + +Rollback (`refactor --rollback`) for Case 3 is more complex than the standard holding +stack swap. The sequence: + +1. Remove imported IDP/domain resources from the Gen2 template. With + `DeletionPolicy: Retain`, CloudFormation orphans them (physical resources survive) + rather than deleting them. +2. Move the Gen1 pool back to Gen1 (standard rollback via holding stack swap). +3. Restore Gen2's original resources from holding → Gen2 (including Gen2 pool, + Gen2 IDPs, Gen2 domain). +4. Gen1's LambdaCallout recreates IDPs/domain on the Gen1 pool on the next Gen1 + deploy (or they may already exist if the Gen1 stack still has the custom resources). + +With `DeletionPolicy: Retain`, step 1 does not delete the physical IDPs/domain from +the pool. They become orphaned from CFN but remain functional. Social login continues +working through the rollback. This is a significant improvement over the original +design which assumed the imported resources would be deleted during rollback. + +## Consequences + +### What changes + +- `auth-cognito-forward.ts`: `RESOURCE_TYPES` includes `UserPoolIdentityProvider`. + `move()` is overridden to append the import operation after the standard refactor. + New private methods: `buildImportSocialAuthOperation()`, `fetchSocialAuthConfig()`, + `buildImportSpec()`. +- `cfn.ts`: New `importResources()` method wrapping `CreateChangeSet(IMPORT)` + + `ExecuteChangeSet` + wait. +- Rollback is more complex — see rollback considerations above. + +### What stays the same + +- The core refactor workflow phases (`resolveSource` → `updateSource` → `beforeMove` → + `move` → `afterMove`) are preserved. The import is appended to the `move()` phase, + not added as a new phase. +- Non-social-auth apps are completely unaffected — `fetchSocialAuthConfig()` returns + `undefined` and the import step is skipped. +- The holding stack pattern is preserved. Gen2's original IDPs go to holding along with + other Gen2 resources and remain available for rollback. + +### Comparison + +| | Case 1: Do Nothing | Case 2: Override | Case 3: Import | +|---|---|---|---| +| Code complexity | None | Minimal (1 line) | High (new move extension, Cognito APIs, import API) | +| Refactor succeeds | Yes | Yes | Yes | +| Auth works post-refactor | Yes | Yes | Yes | +| Next deploy succeeds | **No** | **No** (without manual IDP/domain deletion) | **Yes** (if domain/secrets match Gen1) | +| Auth downtime | None during refactor, blocked on deploy | Gap during IDP deletion/recreation (minutes) | **None** | +| Domain handling | Orphaned | Changed to Gen2 prefix (breaks redirect URIs) | Preserved (Gen1 prefix imported) | +| Rollback complexity | Simple | Simple | Higher (import reversal, mitigated by Retain) | +| Manual steps for user | None (but next deploy fails) | Delete IDPs/domain, update OAuth URIs | Override domain in generated code | + +### Risks + +- **Domain mismatch on next deploy**: If the generated `backend.ts` does not override + the domain to match Gen1's prefix, the next deploy triggers a domain replacement. + This changes the hosted UI URL, requiring updates to OAuth redirect URIs in + Google/Facebook developer consoles. + +- **Secret mismatch**: If SSM secrets at `/amplify/shared/{appId}/` contain different + credentials than what Gen1's IDPs use, the next deploy updates the IDP credentials. + The migration guide instructs users to configure SSM secrets matching their Gen1 + credentials. + +- **IdentityPool `SupportedLoginProviders`**: The Gen2 `IdentityPool` has + `SupportedLoginProviders` mapping `accounts.google.com` and `graph.facebook.com` to + client IDs via `AmplifySecretFetcherResource`. After the Gen1 IdentityPool moves in, + this property reflects Gen1's state. The next deploy would update it to whatever + `AmplifySecretFetcherResource` returns from SSM — this should be correct if the SSM + secrets match Gen1's credentials. + +### Open questions (from original analysis) + +1. **Does `UserPoolIdentityProvider` support CFN IMPORT?** YES — verified (Phase 2). +2. **Does `UserPoolDomain` support CFN IMPORT?** YES — verified. Requires both + `{UserPoolId, Domain}` (Phase 2). +3. **Can `CreateStackRefactor` move `UserPoolIdentityProvider` resources?** Not directly + tested. The implementation avoids this by using the existing `RESOURCE_TYPES` filter + in `beforeMove()`, which only moves resources that are already in the Gen2 template. +4. **Should `decommission` handle IDP/domain cleanup?** Addressed by `DeletionPolicy: + Retain` on Gen1 LambdaCallout resources — prevents delete handlers from firing. + +## Addendum: Revised Approach (Orphan + Import) + +### Problem with Original Case 3 Implementation + +The original implementation added `UserPoolIdentityProvider` and `UserPoolDomain` to `RESOURCE_TYPES`, causing them to move to the holding stack during `beforeMove()`. This failed because: + +1. **Orphaned Fn::GetAtt references**: IDP resources have `Fn::GetAtt` references to `AmplifySecretFetcherResource` (a `Custom::AmplifySecretFetcherResource` that fetches OAuth secrets from SSM). When IDPs move to the holding stack, `AmplifySecretFetcherResource` stays in Gen2 — broken references — `Template error: instance of Fn::GetAtt references undefined resource`. + +2. **StackRefactor API property validation**: Attempted fix of replacing orphaned `Fn::GetAtt` with `"PLACEHOLDER"` strings at refactor time passed structural validation but failed because the StackRefactor API (`CreateStackRefactor` + `ExecuteStackRefactor`) validates that template property values match live resource state. `"PLACEHOLDER"` doesn't match real OAuth secrets — `Resource does not match the destination resource's properties`. + +3. **IdentityPool also affected**: The IdentityPool (a core resource that MUST go through the holding stack) has `Fn::GetAtt` to `AmplifySecretFetcherResource` in its `SupportedLoginProviders` property. + +### Key Findings + +**StackRefactor API validates property match**: Contrary to initial assumption, the StackRefactor API does NOT just perform structural template validation. It checks that template resource properties match live resource state. Dummy/placeholder values are rejected. + +**CFN Import does NOT validate property match**: CloudFormation's import (`ChangeSetType: IMPORT`) only uses `ResourceIdentifier` to adopt physical resources. Template property values are metadata for future updates — dummy values are accepted. + +**CloudFormation accepts PLACEHOLDER for IdentityPool SupportedLoginProviders**: Verified via changeset experiment. A template update replacing `Fn::GetAtt` with `"PLACEHOLDER"` in `SupportedLoginProviders` is accepted (CREATE_COMPLETE, Replacement: False). This allows resolving the IdentityPool's references before it enters the holding stack. + +**NoEcho parameter handling is unnecessary**: The Gen1 `hostedUIProviderCreds` NoEcho parameter feeds only into `Custom::LambdaCallout` (stays in Gen1, not refactored). `resolveParameters()` already skips NoEcho params. `updateSource()` produces an empty changeset — the masked `"****"` value never reaches CloudFormation. + +### Revised Approach: Orphan + Import + +Instead of moving IDPs and domain through the holding stack, **orphan them from the source stack and re-import them into the target stack**. This avoids the `Fn::GetAtt` problem entirely. + +**Changes from original Case 3**: + +1. **Remove `UserPoolIdentityProvider` and `UserPoolDomain` from `RESOURCE_TYPES`**: Only 4 core types move through the holding stack: UserPool, UserPoolClient, IdentityPool, IdentityPoolRoleAttachment. + +2. **Resolve IdentityPool's `Fn::GetAtt` to PLACEHOLDER in `resolveTarget()`**: Before `updateTarget()` pushes the Gen2 template, replace the IdentityPool's `SupportedLoginProviders` `Fn::GetAtt` references to `AmplifySecretFetcherResource` with `"PLACEHOLDER"`. CloudFormation accepts this. The live IdentityPool is updated (PLACEHOLDER in `SupportedLoginProviders`), but this is restored on next deploy. + +3. **Orphan IDPs + domain from Gen2 after `super.beforeMove()`**: Remove them from the Gen2 template. Physical resources survive via `DeletionPolicy: Retain`. + +4. **Import IDPs + domain into Gen2 in `move()` after `super.move()`**: Simplified `fetchSocialAuthConfig()` — only `ListIdentityProvidersCommand` (ProviderName, ProviderType) and `DescribeUserPoolCommand` (Domain) are needed. Dummy values for ProviderDetails (`client_id: "PLACEHOLDER"`, etc.) and AttributeMapping. CFN import doesn't validate these. + +5. **Rollback**: After `super.move()` (Gen2 → Gen1), orphan the imported IDPs + domain from Gen2. After `super.afterMove()` restores holding stack resources (P2) to Gen2, import Gen2's original IDPs + domain back into Gen2. + +### Forward Flow + +``` +resolveTarget() → Replace IdentityPool Fn::GetAtt with PLACEHOLDER +updateTarget() → Push clean Gen2 template (PLACEHOLDER in SupportedLoginProviders) +beforeMove() → super.beforeMove() moves 4 core resources to holding + → Orphan IDPs + domain from Gen2 +move() → super.move() moves Gen1 core resources into Gen2 + → Import Gen1 IDPs + domain into Gen2 (dummy ProviderDetails) +afterMove() → empty (not overridden) +``` + +### Rollback Flow + +``` +move() → super.move() moves core resources Gen2 → Gen1 + → Orphan IDPs + domain from Gen2 (imported during forward move) +afterMove() → super.afterMove() restores holding stack resources (P2) to Gen2 + → Import Gen2 IDPs + domain back into Gen2 (dummy ProviderDetails) +``` + +### Pool Discovery + +Gen2 auth nested stack does not expose a `UserPoolId` output key with a stable CDK-generated name. Pool discovery uses `DescribeStackResources(gen2StackId)` filtered by `ResourceType: AWS::Cognito::UserPool` → `PhysicalResourceId`. Exactly one UserPool exists in Gen2 at each import point: + +- Forward `move()` after `super.move()` finds P1 (just moved in from Gen1). +- Rollback `afterMove()` after `super.afterMove()` finds P2 (just restored from holding). + +Same call, same filter, for both directions. + +### Describe Strategy + +Each import/orphan operation reads the Gen2 template at plan time and captures `{providerName → logicalId}` + domain logical ID into its closure — used for the describe() table and for the execute-time import spec. Execute-time work is limited to `DescribeStackResources` (pool discovery) and Cognito API calls (`DescribeUserPool`, `ListIdentityProviders`). No snapshot files are needed: the Gen2-original logical IDs are preserved through the lifecycle because forward's import re-imports Gen1 physical resources *under* the Gen2 original logical IDs. diff --git a/packages/amplify-cli/src/commands/gen2-migration/generate/amplify/auth/auth.generator.ts b/packages/amplify-cli/src/commands/gen2-migration/generate/amplify/auth/auth.generator.ts index 99c7e75bc34..ff842999454 100644 --- a/packages/amplify-cli/src/commands/gen2-migration/generate/amplify/auth/auth.generator.ts +++ b/packages/amplify-cli/src/commands/gen2-migration/generate/amplify/auth/auth.generator.ts @@ -104,7 +104,7 @@ export class AuthGenerator implements Planner { this.contributeToBackend(renderOptions); if (hasIdentityProviders) { - this.contributeProviderSetup(); + this.contributeProviderSetup(userPool.Domain); } }, }, @@ -342,16 +342,6 @@ export class AuthGenerator implements Planner { clientProps.push(factory.createPropertyAssignment('oAuth', factory.createObjectLiteralExpression(oAuthProps, true))); } - // Commented-out flows property when OAuth flows exist - if (userPoolClient.AllowedOAuthFlows?.length) { - clientProps.push( - factory.createPropertyAssignment( - factory.createIdentifier('// flows'), - factory.createArrayLiteralExpression(userPoolClient.AllowedOAuthFlows.map((flow) => factory.createStringLiteral(flow, true))), - ), - ); - } - // disableOAuth const hasOAuth = (userPoolClient.AllowedOAuthFlows?.length ?? 0) > 0; clientProps.push(factory.createPropertyAssignment('disableOAuth', hasOAuth ? factory.createFalse() : factory.createTrue())); @@ -391,7 +381,7 @@ export class AuthGenerator implements Planner { * Must run after storage overrides so it appears in the correct * position in backend.ts. */ - private contributeProviderSetup(): void { + private contributeProviderSetup(gen1Domain?: string): void { // const providerSetupResult = (backend.auth.stack.node.children.find(child => child.node.id === "amplifyAuth") as any).providerSetupResult; const findCall = factory.createCallExpression( factory.createPropertyAccessExpression( @@ -542,20 +532,51 @@ export class AuthGenerator implements Planner { ); this.backendGenerator.addStatement(forEachStatement); - // // backend.auth.resources.userPool.node.tryRemoveChild("UserPoolDomain"); - const commentedStatement = factory.createExpressionStatement( - factory.createCallExpression( + if (gen1Domain) { + this.contributeDomainOverride(gen1Domain); + } + } + + /** + * Generates an escape hatch that overrides the UserPoolDomain to match the + * Gen1 domain prefix, preventing CFN from replacing it on next deploy. + * + * Output: + * const cfnUserPoolDomain = backend.auth.resources.userPool.node + * .findChild("UserPoolDomain").node.defaultChild as CfnUserPoolDomain; + * cfnUserPoolDomain.domain = ""; + */ + private contributeDomainOverride(gen1Domain: string): void { + this.backendGenerator.addImport('aws-cdk-lib/aws-cognito', ['CfnUserPoolDomain']); + + const domainExpr = factory.createAsExpression( + factory.createPropertyAccessExpression( factory.createPropertyAccessExpression( - factory.createPropertyAccessExpression( - factory.createIdentifier('// backend.auth.resources.userPool'), - factory.createIdentifier('node'), + factory.createCallExpression( + factory.createPropertyAccessExpression( + factory.createPropertyAccessExpression( + factory.createPropertyAccessExpression( + factory.createPropertyAccessExpression( + factory.createPropertyAccessExpression(factory.createIdentifier('backend'), factory.createIdentifier('auth')), + factory.createIdentifier('resources'), + ), + factory.createIdentifier('userPool'), + ), + factory.createIdentifier('node'), + ), + factory.createIdentifier('findChild'), + ), + undefined, + [factory.createStringLiteral('UserPoolDomain')], ), - factory.createIdentifier('tryRemoveChild'), + factory.createIdentifier('node'), ), - undefined, - [factory.createStringLiteral('UserPoolDomain')], + factory.createIdentifier('defaultChild'), ), + factory.createTypeReferenceNode('CfnUserPoolDomain'), ); - this.backendGenerator.addStatement(commentedStatement); + + this.backendGenerator.addStatement(TS.constDecl('cfnUserPoolDomain', domainExpr)); + this.backendGenerator.addStatement(TS.assignProp('cfnUserPoolDomain', 'domain', gen1Domain)); } } diff --git a/packages/amplify-cli/src/commands/gen2-migration/refactor/auth/auth-cognito-forward.ts b/packages/amplify-cli/src/commands/gen2-migration/refactor/auth/auth-cognito-forward.ts index 1ced56473ed..38b9cebc3fa 100644 --- a/packages/amplify-cli/src/commands/gen2-migration/refactor/auth/auth-cognito-forward.ts +++ b/packages/amplify-cli/src/commands/gen2-migration/refactor/auth/auth-cognito-forward.ts @@ -1,21 +1,26 @@ -import { Output, Parameter, ResourceToImport } from '@aws-sdk/client-cloudformation'; +import { ResourceToImport } from '@aws-sdk/client-cloudformation'; import { + CognitoIdentityProviderClient, DescribeUserPoolCommand, - DescribeIdentityProviderCommand, ListIdentityProvidersCommand, } from '@aws-sdk/client-cognito-identity-provider'; import { AmplifyError } from '@aws-amplify/amplify-cli-core'; -import { retrieveOAuthValues } from '../oauth-values-retriever'; import { ForwardCategoryRefactorer } from '../workflow/forward-category-refactorer'; -import { RefactorBlueprint } from '../workflow/category-refactorer'; +import { ExpectedChange, RefactorBlueprint, ResolvedStack } from '../workflow/category-refactorer'; import { CFNResource } from '../../_infra/cfn-template'; import { AmplifyMigrationOperation } from '../../_infra/operation'; +import { walkCfnTree } from '../resolvers/cfn-tree-walker'; import { extractStackNameFromId } from '../utils'; +import { StackFacade } from '../stack-facade'; import CLITable from 'cli-table3'; -const HOSTED_PROVIDER_META_PARAMETER_NAME = 'hostedUIProviderMeta'; -const HOSTED_PROVIDER_CREDENTIALS_PARAMETER_NAME = 'hostedUIProviderCreds'; -const USER_POOL_ID_OUTPUT_KEY_NAME = 'UserPoolId'; +/** + * Gen2 logical ID of the Custom::AmplifySecretFetcherResource — a Lambda-backed + * custom resource that fetches OAuth client IDs/secrets from SSM at deploy time. + * Gen2 IDPs reference this via Fn::GetAtt in ProviderDetails, and the IdentityPool + * references it in SupportedLoginProviders. + */ +const AMPLIFY_SECRET_FETCHER_LOGICAL_ID = 'AmplifySecretFetcherResource'; export const GEN1_NATIVE_APP_CLIENT = 'UserPoolClient'; export const GEN1_WEB_CLIENT = 'UserPoolClientWeb'; @@ -30,89 +35,324 @@ export const IDENTITY_POOL_ROLE_ATTACHMENT_TYPE = 'AWS::Cognito::IdentityPoolRol export const USER_POOL_DOMAIN_TYPE = 'AWS::Cognito::UserPoolDomain'; export const USER_POOL_IDENTITY_PROVIDER_TYPE = 'AWS::Cognito::UserPoolIdentityProvider'; -export const RESOURCE_TYPES = [ - USER_POOL_TYPE, - USER_POOL_CLIENT_TYPE, - IDENTITY_POOL_TYPE, - IDENTITY_POOL_ROLE_ATTACHMENT_TYPE, - USER_POOL_DOMAIN_TYPE, - USER_POOL_IDENTITY_PROVIDER_TYPE, -]; +/** + * Core Cognito resource types that move through the holding stack during the + * standard refactor. UserPoolDomain and UserPoolIdentityProvider are intentionally + * excluded — they carry Fn::GetAtt references to AmplifySecretFetcherResource + * (which stays in Gen2), so they cannot go through the holding stack without + * breaking references. They are handled separately via orphan + import. + */ +export const RESOURCE_TYPES = [USER_POOL_TYPE, USER_POOL_CLIENT_TYPE, IDENTITY_POOL_TYPE, IDENTITY_POOL_ROLE_ATTACHMENT_TYPE]; -interface IdpConfig { +export interface IdpConfig { readonly providerName: string; readonly providerType: string; - readonly clientId: string; - readonly clientSecret: string; - readonly authorizeScopes: string; - readonly attributeMapping: Record; } -interface SocialAuthConfig { +export interface SocialAuthConfig { readonly userPoolId: string; readonly domain: string; readonly providers: IdpConfig[]; } +/** + * Fetches the domain and IDP config directly from Cognito for a given UserPool. + * + * CFN Import only uses ResourceIdentifier (UserPoolId + ProviderName/Domain) to + * adopt physical resources — template property values are metadata for future + * updates. We therefore fetch only identity information (ProviderName, ProviderType, + * Domain). Real client_id/client_secret/scopes/AttributeMapping are NOT needed; + * buildImportSpec() uses dummy values. The next Gen2 deploy regenerates real + * values from AmplifySecretFetcherResource. + * + * Returns undefined if the pool has no domain or no identity providers. + * + * Exported so that both forward and rollback can reuse the same logic. The same + * Cognito client works for both directions because P1 and P2 live in the same + * account/region. + */ +export async function fetchSocialAuthConfig( + cognitoClient: CognitoIdentityProviderClient, + userPoolId: string, +): Promise { + const poolResponse = await cognitoClient.send(new DescribeUserPoolCommand({ UserPoolId: userPoolId })); + const domain = poolResponse?.UserPool?.Domain; + if (!domain) { + return undefined; + } + + const listResponse = await cognitoClient.send(new ListIdentityProvidersCommand({ UserPoolId: userPoolId })); + const providerSummaries = listResponse?.Providers ?? []; + if (providerSummaries.length === 0) { + return undefined; + } + + const providers: IdpConfig[] = []; + for (const summary of providerSummaries) { + const providerName = summary.ProviderName; + if (!providerName) continue; + providers.push({ + providerName, + providerType: summary.ProviderType ?? providerName, + }); + } + + return { userPoolId, domain, providers }; +} + +/** + * Discovers the UserPool physical ID from a Gen2 auth stack via DescribeStackResources. + * + * Gen2 auth nested stack does not expose a `UserPoolId` output with a stable name + * (CDK generates hash-suffixed output names), so we rely on the resource type to + * find the single UserPool. Returns undefined if none is found. + * + * Symmetric for both directions: + * - Forward move() after super.move(): finds P1 (just moved in from Gen1). + * - Rollback afterMove() after super.afterMove(): finds P2 (just restored from holding). + */ +export async function discoverUserPoolId(facade: StackFacade, gen2StackId: string): Promise { + const resources = await facade.fetchStackResources(gen2StackId); + const userPools = resources.filter((r) => r.ResourceType === USER_POOL_TYPE); + if (userPools.length > 1) { + const stackName = extractStackNameFromId(gen2StackId); + const physicalIds = userPools.map((p) => p.PhysicalResourceId ?? '').join(', '); + throw new AmplifyError('MigrationError', { + message: `Expected exactly one UserPool in stack '${stackName}', found ${userPools.length}: ${physicalIds}`, + }); + } + return userPools[0]?.PhysicalResourceId; +} + +/** + * Builds the CFN import spec: template additions with DeletionPolicy: Retain + * (so rollback can orphan them without deleting the physical resources) and + * resource identifiers for the import change set. + * + * Uses dummy placeholder values for ProviderDetails and an empty + * AttributeMapping. CFN import does not validate property match — only the + * ResourceIdentifier (UserPoolId + ProviderName/Domain) is used to adopt the + * physical resource. The next Gen2 deploy regenerates real values via + * AmplifySecretFetcherResource. + * + * Exported so that both forward and rollback can reuse the same logic. Pure + * function — no instance state or logging. + */ +export function buildImportSpec( + config: SocialAuthConfig, + domainLogicalId: string, + idpLogicalIds: Map, +): { resourcesToImport: ResourceToImport[]; templateAdditions: Record } { + const resourcesToImport: ResourceToImport[] = []; + const templateAdditions: Record = {}; + + templateAdditions[domainLogicalId] = { + Type: USER_POOL_DOMAIN_TYPE, + DeletionPolicy: 'Retain', + Properties: { + Domain: config.domain, + UserPoolId: config.userPoolId, + }, + }; + resourcesToImport.push({ + ResourceType: USER_POOL_DOMAIN_TYPE, + LogicalResourceId: domainLogicalId, + ResourceIdentifier: { + UserPoolId: config.userPoolId, + Domain: config.domain, + }, + }); + + for (const provider of config.providers) { + const logicalId = idpLogicalIds.get(provider.providerName); + if (!logicalId) { + throw new AmplifyError('MigrationError', { + message: + `Identity provider '${provider.providerName}' exists on the UserPool but has no matching ` + + `UserPoolIdentityProvider resource in the Gen2 template. Add it to amplify/auth/resource.ts ` + + `and regenerate before refactoring.`, + }); + } + + templateAdditions[logicalId] = { + Type: USER_POOL_IDENTITY_PROVIDER_TYPE, + DeletionPolicy: 'Retain', + Properties: { + UserPoolId: config.userPoolId, + ProviderName: provider.providerName, + ProviderType: provider.providerType, + // Dummy values — CFN import does not validate property match. The next + // Gen2 deploy regenerates real values from AmplifySecretFetcherResource. + ProviderDetails: { + client_id: 'PLACEHOLDER', + client_secret: 'PLACEHOLDER', + authorize_scopes: 'PLACEHOLDER', + }, + AttributeMapping: {}, + }, + }; + + resourcesToImport.push({ + ResourceType: USER_POOL_IDENTITY_PROVIDER_TYPE, + LogicalResourceId: logicalId, + ResourceIdentifier: { + UserPoolId: config.userPoolId, + ProviderName: provider.providerName, + }, + }); + } + + return { resourcesToImport, templateAdditions }; +} + /** * Forward refactorer for the auth:Cognito resource. * - * Moves main auth resources from Gen1 to Gen2. - * For social auth apps, imports Gen1's LambdaCallout-created IDPs and domain - * into the Gen2 stack as native CFN resources during the move phase. + * Moves main auth resources (UserPool, UserPoolClient, IdentityPool, + * IdentityPoolRoleAttachment) from Gen1 to Gen2 via the holding stack. + * + * For social auth apps, the Gen2 UserPoolDomain and UserPoolIdentityProvider + * resources are orphaned from Gen2 in beforeMove() (physical resources survive + * via DeletionPolicy: Retain). After the core resources move in during move(), + * Gen1's physical domain and IDPs are imported into Gen2 as native CFN resources + * — the import operation is appended to the move() phase so that the pool is + * already in Gen2 when the import runs. */ export class AuthCognitoForwardRefactorer extends ForwardCategoryRefactorer { /** - * Returns the full set including domain and IDP types. These types don't exist in the - * Gen1 CFN template (they're created by a Lambda trigger), so they won't appear in the - * refactor mappings. They are imported into Gen2 as a separate step in move(). + * Logical IDs of AWS::Cognito::IdentityPool resources detected in the Gen2 + * template during resolveTarget(). Populated so that expectedTargetChanges() + * can declare the intentional PLACEHOLDER diff updateTarget() produces. + */ + private identityPoolLogicalIds: string[] = []; + + /** + * Returns only the core Cognito resource types. UserPoolDomain and + * UserPoolIdentityProvider are handled via the orphan + import path + * (beforeMove orphans them from Gen2, move imports Gen1's). */ protected resourceTypes(): string[] { return RESOURCE_TYPES; } /** - * OAuth hook: retrieves credentials and updates the hostedUIProviderCreds parameter. + * Resolves the Gen2 target stack template, then replaces any Fn::GetAtt to + * AmplifySecretFetcherResource inside an IdentityPool's SupportedLoginProviders + * with the literal string "PLACEHOLDER". + * + * Why: the IdentityPool is a core resource that MUST go through the holding + * stack. Its SupportedLoginProviders has Fn::GetAtt references to + * AmplifySecretFetcherResource — a Custom::AmplifySecretFetcherResource that + * stays in Gen2. If we leave the GetAtt in place, CloudFormation rejects the + * holding-stack template with + * "instance of Fn::GetAtt references undefined resource AmplifySecretFetcherResource". + * + * The StackRefactor API validates property match for non-custom ref types, but + * CloudFormation accepts the literal string "PLACEHOLDER" in an IdentityPool's + * SupportedLoginProviders values (verified via changeset experiment in ADR-005 + * Addendum: CREATE_COMPLETE, Replacement: False). The next Gen2 deploy + * regenerates the correct values from AmplifySecretFetcherResource. + * + * Scope is intentionally narrow: only Fn::GetAtt nodes whose target is + * AmplifySecretFetcherResource, only inside IdentityPool resources, only under + * SupportedLoginProviders. */ - protected override async resolveOAuthParameters(parameters: Parameter[], outputs: Output[]): Promise { - const oAuthParam = parameters.find((p) => p.ParameterKey === HOSTED_PROVIDER_META_PARAMETER_NAME); - if (!oAuthParam) return parameters; - - const userPoolId = outputs.find((o) => o.OutputKey === USER_POOL_ID_OUTPUT_KEY_NAME)?.OutputValue; - if (!userPoolId) { - throw new AmplifyError('MissingExpectedParameterError', { - message: `Auth stack output '${USER_POOL_ID_OUTPUT_KEY_NAME}' not found — required for OAuth credential retrieval`, + protected override async resolveTarget(stackId: string): Promise { + const resolved = await super.resolveTarget(stackId); + const template = resolved.resolvedTemplate; + const identityPoolLogicalIds: string[] = []; + for (const [logicalId, resource] of Object.entries(template.Resources)) { + if (resource.Type !== IDENTITY_POOL_TYPE) continue; + identityPoolLogicalIds.push(logicalId); + const slp = resource.Properties.SupportedLoginProviders; + if (!slp || typeof slp !== 'object') continue; + const walked = walkCfnTree(slp, (node) => { + if ('Fn::GetAtt' in node && Array.isArray(node['Fn::GetAtt']) && Object.keys(node).length === 1) { + const [target] = node['Fn::GetAtt'] as string[]; + if (target === AMPLIFY_SECRET_FETCHER_LOGICAL_ID) { + return 'PLACEHOLDER'; + } + } + return undefined; }); + resource.Properties.SupportedLoginProviders = walked as object; } + this.identityPoolLogicalIds = identityPoolLogicalIds; + return resolved; + } - const oAuthValues = await retrieveOAuthValues({ - ssmClient: this.gen1App.clients.ssm, - cognitoIdpClient: this.gen1App.clients.cognitoIdentityProvider, - oAuthParameter: oAuthParam, - userPoolId, - appId: this.gen1App.appId, - environmentName: this.gen1App.envName, - }); + /** + * Declares the expected changeset diff produced by resolveTarget(): the + * IdentityPool's SupportedLoginProviders values are replaced with the literal + * string 'PLACEHOLDER'. updateTarget() validate() uses this allowlist to + * accept those intentional diffs while still failing on unrelated diffs. + * + * Populated from `identityPoolLogicalIds` which resolveTarget() captures. + * Ordering: plan() calls resolveTarget before updateTarget, so this method + * is consulted after identityPoolLogicalIds is set. + */ + protected override expectedTargetChanges(): ExpectedChange[] { + return this.identityPoolLogicalIds.map((logicalId) => ({ + logicalId, + // CFN's ResourceChangeDetail.Target.Path is `/Properties/` — the leading `/Properties/` + // segment is part of Path, not separated via Attribute (Attribute is `"Properties"`, indicating the change + // is at the property level). We prefix with `/Properties/SupportedLoginProviders` so nested changes like + // `/Properties/SupportedLoginProviders/accounts.google.com` are correctly matched. + propertyPathPrefix: '/Properties/SupportedLoginProviders', + expectedAfterValueSubstring: 'PLACEHOLDER', + })); + } - const credsParam = parameters.find((p) => p.ParameterKey === HOSTED_PROVIDER_CREDENTIALS_PARAMETER_NAME); - if (!credsParam) { - throw new AmplifyError('MissingExpectedParameterError', { - message: `Auth stack parameter '${HOSTED_PROVIDER_CREDENTIALS_PARAMETER_NAME}' not found`, - }); - } - credsParam.ParameterValue = JSON.stringify(oAuthValues); + /** + * Executes the standard beforeMove (moves 4 core Cognito resources to holding), + * then appends two additional operations in order: + * + * 1. super.beforeMove() — moves the 4 core Cognito types (UserPool, + * UserPoolClient, IdentityPool, IdentityPoolRoleAttachment) to the + * holding stack. + * 2. Retain-set — adds `DeletionPolicy: Retain` to Gen2's UserPoolDomain + * and UserPoolIdentityProvider resources if any lack it. Idempotent: + * skipped entirely when every target already has Retain. Needed because + * `lock` only sets Retain on Gen1 LambdaCallout resources — Gen2 IDP + * and domain resources are out of `lock`'s scope (lock has no + * reference to the Gen2 stack). Without Retain, orphaning would + * delete the physical resources. + * 3. Orphan — validates Retain (defense in depth — the Retain-set op + * just ran), then removes the IDP and domain resources from the + * Gen2 template in a single CFN update. CloudFormation orphans + * them because of DeletionPolicy: Retain; the physical IDPs and + * domain on the pool survive. + * + * If the Gen2 stack has no IDP or domain resources (non-social-auth app), + * both the Retain-set and orphan operations are skipped. + */ + protected override async beforeMove(gen2StackId: string): Promise { + const baseOps = await super.beforeMove(gen2StackId); + + const retainOp = await this.buildRetainSocialAuthOperation(gen2StackId); + const orphanOp = await this.buildOrphanSocialAuthOperation(gen2StackId); - return parameters; + const extraOps: AmplifyMigrationOperation[] = []; + if (retainOp) extraOps.push(retainOp); + if (orphanOp) extraOps.push(orphanOp); + + return [...baseOps, ...extraOps]; } /** - * Executes the standard resource refactor, then imports Gen1's - * physical domain and IDPs into the Gen2 stack as native CFN resources. + * Executes the standard move (moves core Gen1 resources into Gen2), then + * appends an import operation that re-imports Gen1's physical UserPoolDomain + * and UserPoolIdentityProvider resources into the Gen2 stack as native CFN + * resources. + * + * The import must run AFTER super.move() completes — the core move transfers + * the Gen1 UserPool into Gen2, and the UserPool must be in the Gen2 stack + * before we can import resources that reference it by UserPoolId. */ protected override async move(blueprint: RefactorBlueprint): Promise { const baseOps = await super.move(blueprint); - const importOp = await this.buildImportSocialAuthOperation(blueprint); + const importOp = await this.buildImportSocialAuthOperation(blueprint.targetStackId); if (importOp) { return [...baseOps, importOp]; } @@ -121,16 +361,178 @@ export class AuthCognitoForwardRefactorer extends ForwardCategoryRefactorer { } /** - * Builds an operation that imports Gen1's physical domain and IDPs into the - * Gen2 stack. Returns undefined if the app doesn't use social auth. + * Builds an operation that sets `DeletionPolicy: Retain` on Gen2's + * UserPoolDomain and UserPoolIdentityProvider resources. Returns undefined + * if no such resources exist in the Gen2 template, or if every one of them + * already has Retain. + * + * This is idempotent: if a future `lock` update sets Retain on Gen2 + * resources, this operation short-circuits and issues no CFN update. Also + * re-fetches the template at execute-time (in case plan-time snapshot has + * drifted) and skips the update if every target already has Retain. + * + * Pattern borrowed from `lock.ts` `buildHostedUiRetainOperation`. Uses + * `this.cfn.update()` (which wraps UpdateStackCommand + wait + IAM + * capability) for consistency with the rest of the refactor code. Stack + * parameters are preserved via `UsePreviousValue: true` — we don't need + * to know actual values (many are NoEcho). */ - private async buildImportSocialAuthOperation(blueprint: RefactorBlueprint): Promise { - const socialAuthConfig = await this.fetchSocialAuthConfig(blueprint.sourceStackId); - if (!socialAuthConfig) { + private async buildRetainSocialAuthOperation(gen2StackId: string): Promise { + const template = await this.cfn.fetchTemplate(gen2StackId); + + const logicalIdsNeedingRetain = Object.entries(template.Resources) + .filter( + ([, r]) => + (r.Type === USER_POOL_DOMAIN_TYPE || r.Type === USER_POOL_IDENTITY_PROVIDER_TYPE) && r.DeletionPolicy !== 'Retain', + ) + .map(([id]) => id); + + if (logicalIdsNeedingRetain.length === 0) { return undefined; } - const gen2StackId = blueprint.targetStackId; + const gen2StackName = extractStackNameFromId(gen2StackId); + + return { + resource: this.resource, + validate: () => undefined, + describe: async () => [ + `Set DeletionPolicy: Retain on ${logicalIdsNeedingRetain.length} social auth resource(s) in '${gen2StackName}': ${logicalIdsNeedingRetain.join( + ', ', + )}`, + ], + execute: async () => { + // Re-fetch the template at execute time in case it has drifted since plan. + const currentTemplate = await this.cfn.fetchTemplate(gen2StackId); + + const appliedIds: string[] = []; + let retainAdded = false; + for (const id of logicalIdsNeedingRetain) { + const resource = currentTemplate.Resources[id]; + if (!resource) continue; + if (resource.DeletionPolicy !== 'Retain') { + resource.DeletionPolicy = 'Retain'; + retainAdded = true; + appliedIds.push(id); + } + } + + if (!retainAdded) { + this.info(`All social auth resources already have DeletionPolicy: Retain in '${gen2StackName}'`); + return; + } + + const stack = await this.cfn.describeStack(gen2StackId); + const parameters = (stack.Parameters ?? []).map((p) => ({ + ParameterKey: p.ParameterKey, + UsePreviousValue: true, + })); + + await this.cfn.update({ + stackName: gen2StackId, + templateBody: currentTemplate, + parameters, + resource: this.resource, + }); + + this.info(`Set DeletionPolicy: Retain on social auth resources in '${gen2StackName}': ${appliedIds.join(', ')}`); + }, + }; + } + + /** + * Builds an operation that orphans Gen2's UserPoolDomain and + * UserPoolIdentityProvider resources from the Gen2 stack. Returns undefined + * if the Gen2 template has no such resources (non-social-auth app). + * + * Validates at execute-time that every orphan target has DeletionPolicy: + * Retain (established by the prior Retain-set op in this beforeMove), then + * removes the resources from the template in a single CFN update. Retain + * guarantees the physical IDPs and domain survive the template update. + */ + private async buildOrphanSocialAuthOperation(gen2StackId: string): Promise { + const template = await this.cfn.fetchTemplate(gen2StackId); + + const logicalIdsToOrphan = Object.entries(template.Resources) + .filter(([, r]) => r.Type === USER_POOL_DOMAIN_TYPE || r.Type === USER_POOL_IDENTITY_PROVIDER_TYPE) + .map(([id]) => id); + + if (logicalIdsToOrphan.length === 0) { + return undefined; + } + + const gen2StackName = extractStackNameFromId(gen2StackId); + + return { + resource: this.resource, + // Retain is established by the preceding Retain-set op. We verify at execute time, not plan-validation + // time, because plan.validate() runs ALL operation validate() callbacks before ANY execute(). Checking + // here during plan-validation would see the pre-Retain-set state and fail on every first run. At execute + // time the Retain-set op has already durably set Retain on the targets. If the invariant is somehow + // violated (e.g. manual template edits between plan and execute), we abort before any destructive + // template mutation — missing Retain would cause the subsequent cfn.update to delete the physical + // resource. + validate: () => undefined, + describe: async () => [ + `Orphan ${logicalIdsToOrphan.length} social auth resource(s) from '${gen2StackName}': ${logicalIdsToOrphan.join(', ')}`, + ], + execute: async () => { + const currentTemplate = await this.cfn.fetchTemplate(gen2StackId); + + // Execute-time Retain verification (defense-in-depth). See the comment on validate above for why + // this runs here rather than in validate(). Orphaning without Retain would delete the physical + // UserPoolDomain / UserPoolIdentityProvider. + const missingRetain = logicalIdsToOrphan.filter( + (id) => id in currentTemplate.Resources && currentTemplate.Resources[id].DeletionPolicy !== 'Retain', + ); + if (missingRetain.length > 0) { + throw new AmplifyError('MigrationError', { + message: + `Cannot orphan social auth resources from '${gen2StackName}': the following resources are missing ` + + `DeletionPolicy: Retain and would be physically deleted: ${missingRetain.join(', ')}.`, + resolution: 'Ensure the preceding Retain-set operation succeeded, then re-run the refactor.', + }); + } + + const stack = await this.cfn.describeStack(gen2StackId); + + for (const id of logicalIdsToOrphan) { + delete currentTemplate.Resources[id]; + } + + await this.cfn.update({ + stackName: gen2StackId, + templateBody: currentTemplate, + parameters: stack.Parameters ?? [], + resource: this.resource, + }); + + this.info(`Orphaned social auth resources from '${gen2StackName}': ${logicalIdsToOrphan.join(', ')}`); + }, + }; + } + + /** + * Builds an operation that imports physical UserPoolDomain and + * UserPoolIdentityProvider resources into the Gen2 stack under the Gen2 + * original logical IDs. Returns undefined if the app doesn't use social auth + * (no domain or no IDP resources in the Gen2 template). + * + * Plan-time work: + * - Read the Gen2 template and capture `{providerName → logicalId}` and the + * domain logical ID into the operation closure. These are the Gen2 original + * logical IDs, which we reuse for the import so subsequent Gen2 deploys see + * the same IDs. + * + * Execute-time work: + * - Discover the UserPool physical ID from the Gen2 stack (P1 after + * super.move() has transferred Gen1's pool in). + * - Fetch the live domain and IDP list from Cognito. + * - Build the import spec and execute the import changeset. + */ + private async buildImportSocialAuthOperation(gen2StackId: string): Promise { + // Plan-time: capture logical IDs from the Gen2 template before any orphan + // operation runs at execute-time. const gen2Template = await this.cfn.fetchTemplate(gen2StackId); const gen2IdpLogicalIds = new Map(); let gen2DomainLogicalId: string | undefined; @@ -158,32 +560,43 @@ export class AuthCognitoForwardRefactorer extends ForwardCategoryRefactorer { return undefined; } + const domainLogicalId = gen2DomainLogicalId; + const gen2StackName = extractStackNameFromId(gen2StackId); + return { resource: this.resource, validate: () => undefined, describe: async () => { - const gen2StackName = extractStackNameFromId(gen2StackId); const table = new CLITable({ - head: ['Source Physical ID', 'Target Logical ID'], + head: ['Provider', 'Target Logical ID'], style: { head: [] }, }); - table.push([socialAuthConfig.domain, gen2DomainLogicalId!]); - for (const provider of socialAuthConfig.providers) { - const logicalId = gen2IdpLogicalIds.get(provider.providerName); - if (logicalId) { - const label = - provider.providerType !== provider.providerName - ? `${provider.providerName} (${provider.providerType})` - : provider.providerName; - table.push([label, logicalId]); - } + table.push(['(domain)', domainLogicalId]); + for (const [providerName, logicalId] of gen2IdpLogicalIds) { + table.push([providerName, logicalId]); } return [`Import social auth resources into '${gen2StackName}'\n\n${table.toString()}`]; }, execute: async () => { + // Execute-time: discover the UserPool in Gen2 (P1 after super.move()). + const userPoolId = await discoverUserPoolId(this.gen2Branch, gen2StackId); + if (!userPoolId) { + throw new AmplifyError('MigrationError', { + message: `Unable to discover UserPool in Gen2 stack '${gen2StackName}' for social auth import`, + }); + } + + const cognitoClient = this.gen1App.clients.cognitoIdentityProvider; + const socialAuthConfig = await fetchSocialAuthConfig(cognitoClient, userPoolId); + if (!socialAuthConfig) { + this.debug(`UserPool ${userPoolId} has no domain or no identity providers — skipping import`); + return; + } + + // Fetch the current (post-orphan) template. Import re-adds the resources. const templateForImport = await this.cfn.fetchTemplate(gen2StackId); - const { resourcesToImport, templateAdditions } = this.buildImportSpec(socialAuthConfig, gen2DomainLogicalId!, gen2IdpLogicalIds); + const { resourcesToImport, templateAdditions } = buildImportSpec(socialAuthConfig, domainLogicalId, gen2IdpLogicalIds); for (const [logicalId, resource] of Object.entries(templateAdditions)) { templateForImport.Resources[logicalId] = resource; @@ -229,122 +642,4 @@ export class AuthCognitoForwardRefactorer extends ForwardCategoryRefactorer { // in gen2 all auth resources are in a single auth nested stack return this.findNestedStack(this.gen2Branch, 'auth'); } - - /** - * Fetches domain and IDP config directly from Cognito. These resources are - * Lambda-created (not in the Gen1 CFN template) so the live API is the only source. - */ - private async fetchSocialAuthConfig(sourceStackId: string): Promise { - const sourceStack = await this.gen1Env.fetchStack(sourceStackId); - const userPoolId = (sourceStack.Outputs ?? []).find((o) => o.OutputKey === USER_POOL_ID_OUTPUT_KEY_NAME)?.OutputValue; - if (!userPoolId) { - return undefined; - } - - const cognitoClient = this.gen1App.clients.cognitoIdentityProvider; - - const poolResponse = await cognitoClient.send(new DescribeUserPoolCommand({ UserPoolId: userPoolId })); - const domain = poolResponse?.UserPool?.Domain; - if (!domain) { - this.debug('Gen1 UserPool has no domain — skipping social auth import'); - return undefined; - } - - const listResponse = await cognitoClient.send(new ListIdentityProvidersCommand({ UserPoolId: userPoolId })); - const providerSummaries = listResponse?.Providers ?? []; - if (providerSummaries.length === 0) { - this.debug('Gen1 UserPool has no identity providers — skipping social auth import'); - return undefined; - } - - const providers: IdpConfig[] = []; - for (const summary of providerSummaries) { - const providerName = summary.ProviderName; - if (!providerName) continue; - - const describeResponse = await cognitoClient.send( - new DescribeIdentityProviderCommand({ UserPoolId: userPoolId, ProviderName: providerName }), - ); - const idp = describeResponse.IdentityProvider; - if (!idp?.ProviderDetails) continue; - - providers.push({ - providerName, - providerType: idp.ProviderType ?? providerName, - clientId: idp.ProviderDetails.client_id ?? '', - clientSecret: idp.ProviderDetails.client_secret ?? '', - authorizeScopes: idp.ProviderDetails.authorize_scopes ?? '', - attributeMapping: (idp.AttributeMapping as Record) ?? {}, - }); - } - - this.debug(`Fetched social auth config: domain=${domain}, providers=${providers.map((p) => p.providerName).join(',')}`); - return { userPoolId, domain, providers }; - } - - /** - * Builds the CFN import spec: template additions with DeletionPolicy: Retain - * (so rollback can orphan them without deleting the physical resources) and - * resource identifiers for the import change set. - */ - private buildImportSpec( - config: SocialAuthConfig, - domainLogicalId: string, - idpLogicalIds: Map, - ): { resourcesToImport: ResourceToImport[]; templateAdditions: Record } { - const resourcesToImport: ResourceToImport[] = []; - const templateAdditions: Record = {}; - - templateAdditions[domainLogicalId] = { - Type: USER_POOL_DOMAIN_TYPE, - DeletionPolicy: 'Retain', - Properties: { - Domain: config.domain, - UserPoolId: config.userPoolId, - }, - }; - resourcesToImport.push({ - ResourceType: USER_POOL_DOMAIN_TYPE, - LogicalResourceId: domainLogicalId, - ResourceIdentifier: { - UserPoolId: config.userPoolId, - Domain: config.domain, - }, - }); - - for (const provider of config.providers) { - const logicalId = idpLogicalIds.get(provider.providerName); - if (!logicalId) { - this.debug(`No Gen2 logical ID for provider ${provider.providerName} — skipping import`); - continue; - } - - templateAdditions[logicalId] = { - Type: USER_POOL_IDENTITY_PROVIDER_TYPE, - DeletionPolicy: 'Retain', - Properties: { - UserPoolId: config.userPoolId, - ProviderName: provider.providerName, - ProviderType: provider.providerType, - ProviderDetails: { - client_id: provider.clientId, - client_secret: provider.clientSecret, - authorize_scopes: provider.authorizeScopes, - }, - AttributeMapping: provider.attributeMapping, - }, - }; - - resourcesToImport.push({ - ResourceType: USER_POOL_IDENTITY_PROVIDER_TYPE, - LogicalResourceId: logicalId, - ResourceIdentifier: { - UserPoolId: config.userPoolId, - ProviderName: provider.providerName, - }, - }); - } - - return { resourcesToImport, templateAdditions }; - } } diff --git a/packages/amplify-cli/src/commands/gen2-migration/refactor/auth/auth-cognito-rollback.ts b/packages/amplify-cli/src/commands/gen2-migration/refactor/auth/auth-cognito-rollback.ts index b5eee9d811a..28b7715945c 100644 --- a/packages/amplify-cli/src/commands/gen2-migration/refactor/auth/auth-cognito-rollback.ts +++ b/packages/amplify-cli/src/commands/gen2-migration/refactor/auth/auth-cognito-rollback.ts @@ -1,6 +1,10 @@ +import { ResourceMapping } from '@aws-sdk/client-cloudformation'; import { AmplifyError } from '@aws-amplify/amplify-cli-core'; import { CFNResource } from '../../_infra/cfn-template'; +import { AmplifyMigrationOperation } from '../../_infra/operation'; +import { RefactorBlueprint } from '../workflow/category-refactorer'; import { RollbackCategoryRefactorer } from '../workflow/rollback-category-refactorer'; +import { extractStackNameFromId } from '../utils'; import { RESOURCE_TYPES, GEN1_NATIVE_APP_CLIENT, @@ -12,18 +16,71 @@ import { IDENTITY_POOL_TYPE, IDENTITY_POOL_ROLE_ATTACHMENT_TYPE, USER_POOL_DOMAIN_TYPE, + USER_POOL_IDENTITY_PROVIDER_TYPE, + buildImportSpec, + discoverUserPoolId, + fetchSocialAuthConfig, } from './auth-cognito-forward'; +import CLITable from 'cli-table3'; + +/** + * Resource types that were imported into Gen2 by the forward step and must be + * orphaned from Gen2 during rollback. These types are NOT in RESOURCE_TYPES + * (the core refactor types), so the standard Gen2→Gen1 move does not attempt + * to move them. Instead, they are orphaned from Gen2's template after the + * core resources return to Gen1. Physical resources survive via + * DeletionPolicy: Retain (set during the forward import). + */ +const IMPORTED_RESOURCE_TYPES = [USER_POOL_DOMAIN_TYPE, USER_POOL_IDENTITY_PROVIDER_TYPE]; /** * Rollback refactorer for the auth:Cognito resource. * - * Moves main auth resources from Gen2 back to Gen1. + * Moves main auth resources (UserPool, UserPoolClient, IdentityPool, + * IdentityPoolRoleAttachment) from Gen2 back to Gen1 via holding-stack + * restoration. + * + * For social auth apps: + * - move() orphans the Gen2 UserPoolDomain and UserPoolIdentityProvider + * resources (imported during forward move) after the core Gen2→Gen1 move. + * Physical resources survive via DeletionPolicy: Retain and remain on the + * (now Gen1) UserPool. Gen1's LambdaCallout custom resources + * (HostedUICustomResourceInputs, HostedUIProvidersCustomResourceInputs) will + * recreate/update them as needed on the next Gen1 deploy. + * - afterMove() (after super.afterMove() restores P2 core resources from the + * holding stack) re-imports Gen2's original IDPs + domain back into Gen2 + * under the Gen2 original logical IDs. This mirrors forward's import step + * so rollback produces a Gen2 stack state equivalent to the pre-refactor + * state. */ export class AuthCognitoRollbackRefactorer extends RollbackCategoryRefactorer { + /** + * Returns the core Cognito resource types. UserPoolDomain and + * UserPoolIdentityProvider are intentionally excluded from the refactor move + * — they are orphaned from Gen2 separately in move() (inverse of the + * forward import step). + */ protected resourceTypes(): string[] { return RESOURCE_TYPES; } + /** + * Excludes domain and IDP resources from the Gen2→Gen1 refactor mappings. + * Even though RESOURCE_TYPES already excludes these types (so they are + * filtered out by filterResourcesByType), this filter remains as a safety + * net — if a future change adds them to RESOURCE_TYPES for the forward + * direction, rollback would still correctly exclude them from the move. + */ + protected async buildResourceMappings( + sourceResources: Map, + targetResources: Map, + sourceStackId: string, + targetStackId: string, + ): Promise { + const filtered = new Map([...sourceResources].filter(([, r]) => !IMPORTED_RESOURCE_TYPES.includes(r.Type))); + return super.buildResourceMappings(filtered, targetResources, sourceStackId, targetStackId); + } + protected async fetchSourceStackId(): Promise { return this.findNestedStack(this.gen2Branch, 'auth'); } @@ -32,6 +89,45 @@ export class AuthCognitoRollbackRefactorer extends RollbackCategoryRefactorer { return this.findNestedStack(this.gen1Env, `auth${this.resource.resourceName}`); } + /** + * Executes the standard Gen2→Gen1 resource rollback, then orphans any + * imported social auth resources (domain + IDPs) from the Gen2 stack. + */ + protected override async move(blueprint: RefactorBlueprint): Promise { + const baseOps = await super.move(blueprint); + + const orphanOp = await this.buildOrphanSocialAuthOperation(blueprint.sourceStackId); + if (orphanOp) { + return [...baseOps, orphanOp]; + } + + return baseOps; + } + + /** + * Executes the standard afterMove (restores P2 core resources from the + * holding stack into Gen2), then re-imports Gen2's original domain and IDPs + * back into Gen2. + * + * Plan-time: capture `{providerName → logicalId}` and the domain logical ID + * from the Gen2 template. Plan-time reads see the Gen2 original logical IDs + * because rollback's move() orphan runs at EXECUTE time; at plan time the + * Gen2 template still contains the imported IDP/domain resources. + * + * Execute-time: after super.afterMove() restores P2 into Gen2, discover the + * UserPool (P2), fetch domain/IDP list from Cognito, and run the import. + */ + protected override async afterMove(gen2StackId: string): Promise { + const baseOps = await super.afterMove(gen2StackId); + + const importOp = await this.buildImportSocialAuthOperation(gen2StackId); + if (importOp) { + return [...baseOps, importOp]; + } + + return baseOps; + } + protected targetLogicalId(sourceId: string, sourceResource: CFNResource): string | undefined { switch (sourceResource.Type) { case USER_POOL_CLIENT_TYPE: { @@ -51,10 +147,173 @@ export class AuthCognitoRollbackRefactorer extends RollbackCategoryRefactorer { return 'IdentityPool'; case IDENTITY_POOL_ROLE_ATTACHMENT_TYPE: return 'IdentityPoolRoleMap'; - case USER_POOL_DOMAIN_TYPE: - return 'UserPoolDomain'; default: return undefined; } } + + /** + * Builds an operation that orphans imported social auth resources from the Gen2 stack. + * Resources have DeletionPolicy: Retain (set during forward import), so removing + * them from the template does not delete the physical resources. + */ + private async buildOrphanSocialAuthOperation(gen2StackId: string): Promise { + const template = await this.cfn.fetchTemplate(gen2StackId); + + const logicalIdsToOrphan = Object.entries(template.Resources) + .filter(([, r]) => IMPORTED_RESOURCE_TYPES.includes(r.Type)) + .map(([id]) => id); + + if (logicalIdsToOrphan.length === 0) { + return undefined; + } + + const gen2StackName = extractStackNameFromId(gen2StackId); + + return { + resource: this.resource, + // Retain is established by the forward refactor's buildImportSpec (which sets DeletionPolicy: Retain on + // every imported IDP/domain resource). We verify at execute time, not plan-validation time, because + // plan.validate() runs ALL operation validate() callbacks before ANY execute(). Checking here during + // plan-validation would see state that predates the earlier execute() side effects (in a fresh rollback + // the invariant holds immediately, but keeping the check symmetric with forward avoids the anti-pattern + // of cross-operation dependencies at plan-validation time). If the invariant is somehow violated (e.g. + // manual template edits between plan and execute), we abort before any destructive template mutation + // — missing Retain would cause the subsequent cfn.update to delete the physical resource. + validate: () => undefined, + describe: async () => [ + `Orphan ${logicalIdsToOrphan.length} imported social auth resource(s) from '${gen2StackName}': ${logicalIdsToOrphan.join(', ')}`, + ], + execute: async () => { + const currentTemplate = await this.cfn.fetchTemplate(gen2StackId); + + // Execute-time Retain verification (defense-in-depth). See the comment on validate above for why + // this runs here rather than in validate(). Orphaning without Retain would delete the physical + // UserPoolDomain / UserPoolIdentityProvider. + const missingRetain = logicalIdsToOrphan.filter( + (id) => id in currentTemplate.Resources && currentTemplate.Resources[id].DeletionPolicy !== 'Retain', + ); + if (missingRetain.length > 0) { + throw new AmplifyError('MigrationError', { + message: + `Cannot orphan imported social auth resources from '${gen2StackName}': the following resources are missing ` + + `DeletionPolicy: Retain and would be physically deleted: ${missingRetain.join(', ')}.`, + resolution: 'Inspect the Gen2 template; the forward refactor should have set Retain on these resources during import.', + }); + } + + const stack = await this.cfn.describeStack(gen2StackId); + + for (const id of logicalIdsToOrphan) { + delete currentTemplate.Resources[id]; + } + + await this.cfn.update({ + stackName: gen2StackId, + templateBody: currentTemplate, + parameters: stack.Parameters ?? [], + resource: this.resource, + }); + + this.info(`Orphaned social auth resources from '${gen2StackName}': ${logicalIdsToOrphan.join(', ')}`); + }, + }; + } + + /** + * Builds an operation that re-imports Gen2's original domain and IDPs into + * the Gen2 stack under the Gen2 original logical IDs. Returns undefined if + * the Gen2 template has no such resources. + * + * Plan-time work: + * - Read the Gen2 template and capture `{providerName → logicalId}` and + * the domain logical ID into the operation closure. These are the Gen2 + * original logical IDs (preserved through forward's import under the + * same logical IDs). + * + * Execute-time work (after super.afterMove() restores P2 into Gen2): + * - Discover the UserPool (P2) from the Gen2 stack resources. + * - Fetch domain + IDP list from Cognito for P2. + * - Build the import spec and execute the import changeset. + */ + private async buildImportSocialAuthOperation(gen2StackId: string): Promise { + // Plan-time: capture logical IDs from the Gen2 template. At plan time the + // Gen2 template still contains the IDP/domain resources (imported during + // forward); rollback's move() orphan removes them at execute time, not + // plan time. + const gen2Template = await this.cfn.fetchTemplate(gen2StackId); + const gen2IdpLogicalIds = new Map(); + let gen2DomainLogicalId: string | undefined; + for (const [logicalId, resource] of Object.entries(gen2Template.Resources)) { + if (resource.Type === USER_POOL_DOMAIN_TYPE) { + gen2DomainLogicalId = logicalId; + } else if (resource.Type === USER_POOL_IDENTITY_PROVIDER_TYPE) { + const providerName = resource.Properties.ProviderName as string; + if (providerName) { + gen2IdpLogicalIds.set(providerName, logicalId); + } + } + } + + if (!gen2DomainLogicalId) { + this.debug('No Gen2 UserPoolDomain resource found — skipping rollback import'); + return undefined; + } + + if (gen2IdpLogicalIds.size === 0) { + this.debug('No Gen2 UserPoolIdentityProvider resources found — skipping rollback import'); + return undefined; + } + + const domainLogicalId = gen2DomainLogicalId; + const gen2StackName = extractStackNameFromId(gen2StackId); + + return { + resource: this.resource, + validate: () => undefined, + describe: async () => { + const table = new CLITable({ + head: ['Provider', 'Target Logical ID'], + style: { head: [] }, + }); + table.push(['(domain)', domainLogicalId]); + for (const [providerName, logicalId] of gen2IdpLogicalIds) { + table.push([providerName, logicalId]); + } + return [`Import social auth resources into '${gen2StackName}'\n\n${table.toString()}`]; + }, + execute: async () => { + // Execute-time: after super.afterMove() has restored P2 into Gen2, + // discover the UserPool and import Gen2's original IDPs + domain. + const userPoolId = await discoverUserPoolId(this.gen2Branch, gen2StackId); + if (!userPoolId) { + throw new AmplifyError('MigrationError', { + message: `Unable to discover UserPool in Gen2 stack '${gen2StackName}' for social auth re-import`, + }); + } + + const cognitoClient = this.gen1App.clients.cognitoIdentityProvider; + const socialAuthConfig = await fetchSocialAuthConfig(cognitoClient, userPoolId); + if (!socialAuthConfig) { + this.debug(`UserPool ${userPoolId} has no domain or no identity providers — skipping rollback import`); + return; + } + + const templateForImport = await this.cfn.fetchTemplate(gen2StackId); + + const { resourcesToImport, templateAdditions } = buildImportSpec(socialAuthConfig, domainLogicalId, gen2IdpLogicalIds); + + for (const [logicalId, resource] of Object.entries(templateAdditions)) { + templateForImport.Resources[logicalId] = resource; + } + + await this.cfn.importResources({ + stackName: gen2StackId, + templateBody: templateForImport, + resourcesToImport, + resource: this.resource, + }); + }, + }; + } } diff --git a/packages/amplify-cli/src/commands/gen2-migration/refactor/resolvers/cfn-output-resolver.ts b/packages/amplify-cli/src/commands/gen2-migration/refactor/resolvers/cfn-output-resolver.ts index 9b20f694db3..ff45eec541b 100644 --- a/packages/amplify-cli/src/commands/gen2-migration/refactor/resolvers/cfn-output-resolver.ts +++ b/packages/amplify-cli/src/commands/gen2-migration/refactor/resolvers/cfn-output-resolver.ts @@ -45,7 +45,9 @@ export function resolveOutputs(params: { // {"Ref": "LogicalId"} → replace with stack output value from Ref-based outputs if ('Ref' in node && typeof node.Ref === 'string' && Object.keys(node).length === 1) { const value = refLookup.get(node.Ref); - if (value !== undefined) return value; + const physicalId = stackResources.find((r) => r.LogicalResourceId === node.Ref)?.PhysicalResourceId; + const resolved = value ?? physicalId; + if (resolved !== undefined) return resolved; } // {"Fn::GetAtt": ["LogicalId", "AttrName"]} → resolve via GetAtt-based outputs + ARN builder diff --git a/packages/amplify-cli/src/commands/gen2-migration/refactor/resolvers/cfn-parameter-resolver.ts b/packages/amplify-cli/src/commands/gen2-migration/refactor/resolvers/cfn-parameter-resolver.ts index 813e5db2ae6..5a19d86b731 100644 --- a/packages/amplify-cli/src/commands/gen2-migration/refactor/resolvers/cfn-parameter-resolver.ts +++ b/packages/amplify-cli/src/commands/gen2-migration/refactor/resolvers/cfn-parameter-resolver.ts @@ -54,3 +54,32 @@ export function resolveParameters(template: CFNTemplate, parameters: Parameter[] return undefined; }) as CFNTemplate; } + +/** + * Transforms NoEcho parameters in a CloudFormation-bound Parameters array to use + * UsePreviousValue instead of the masked ParameterValue returned by DescribeStacks. + * + * Background: DescribeStacks masks NoEcho parameter values as "****". Passing that + * masked value back to CreateChangeSet / UpdateStack causes CloudFormation to treat + * it as an explicit new value. For templates that reference the NoEcho parameter + * via {Ref: } inside a Custom::LambdaCallout's Properties, this re-resolves + * the Ref to the literal string "****", triggers a Custom resource update, and crashes + * the Lambda when it JSON.parses the masked token. + * + * The fix: for each parameter whose template declaration has NoEcho: true, send + * { ParameterKey, UsePreviousValue: true } — CloudFormation uses the real stored + * value internally and the masked "****" never flows through the template. + * + * Non-NoEcho parameters pass through unchanged. + */ +export function resolveNoEchoParameters(template: CFNTemplate, parameters: Parameter[]): Parameter[] { + const templateParams = template.Parameters ?? {}; + return parameters.map((param) => { + if (!param.ParameterKey) return param; + const paramDef = templateParams[param.ParameterKey]; + if (paramDef?.NoEcho) { + return { ParameterKey: param.ParameterKey, UsePreviousValue: true }; + } + return param; + }); +} diff --git a/packages/amplify-cli/src/commands/gen2-migration/refactor/workflow/category-refactorer.ts b/packages/amplify-cli/src/commands/gen2-migration/refactor/workflow/category-refactorer.ts index 0a3cd165f87..2d981f31c8c 100644 --- a/packages/amplify-cli/src/commands/gen2-migration/refactor/workflow/category-refactorer.ts +++ b/packages/amplify-cli/src/commands/gen2-migration/refactor/workflow/category-refactorer.ts @@ -22,6 +22,72 @@ export interface ResolvedStack { readonly parameters: Parameter[]; } +/** + * Describes a CFN change detail that a refactorer intentionally produces and + * expects updateTarget() to accept. Used to narrow the updateTarget validate() + * check so it only fails on UNEXPECTED diffs. + * + * A change detail is considered expected iff: + * - detail.Target.Attribute === 'Properties' + * - change.ResourceChange.LogicalResourceId === entry.logicalId + * - detail.Target.Path starts with entry.propertyPathPrefix + * - detail.Target.AfterValue contains entry.expectedAfterValueSubstring + */ +export interface ExpectedChange { + readonly logicalId: string; + readonly propertyPathPrefix: string; + readonly expectedAfterValueSubstring: string; +} + +interface UnexpectedChangeEntry { + readonly logicalId: string; + readonly path: string; + readonly beforeValue?: string; + readonly afterValue?: string; +} + +/** + * Returns the subset of change details in the changeset that are NOT covered by + * any ExpectedChange allowlist entry. If the returned array is empty, all diffs + * are expected and validate() should pass. + */ +function filterUnexpectedChanges(changeSet: DescribeChangeSetOutput, allowlist: ExpectedChange[]): UnexpectedChangeEntry[] { + const unexpected: UnexpectedChangeEntry[] = []; + for (const change of changeSet.Changes ?? []) { + const rc = change.ResourceChange; + if (!rc) continue; + const logicalId = rc.LogicalResourceId ?? ''; + for (const detail of rc.Details ?? []) { + const target = detail.Target; + if (target?.Attribute !== 'Properties') continue; + const path = target.Path ?? ''; + const afterValue = target.AfterValue; + const matches = allowlist.some( + (entry) => + entry.logicalId === logicalId && + path.startsWith(entry.propertyPathPrefix) && + typeof afterValue === 'string' && + afterValue.includes(entry.expectedAfterValueSubstring), + ); + if (!matches) { + unexpected.push({ logicalId, path, beforeValue: target.BeforeValue, afterValue }); + } + } + } + return unexpected; +} + +/** + * Formats the unexpected entries for user display, prepending a summary header + * to the full rendered changeset report. + */ +function renderUnexpectedChanges(unexpected: UnexpectedChangeEntry[], fullReport: string): string { + const lines = unexpected.map( + (u) => ` ${u.logicalId}${u.path}: ${u.beforeValue ?? '(none)'} → ${u.afterValue ?? '(none)'}`, + ); + return `Unexpected changes:\n${lines.join('\n')}\n\nFull changeset:\n${fullReport}`; +} + /** * Mappings-only refactor plan. Templates are fetched fresh at execution time * so that sequential refactorers targeting the same stack always see current state. @@ -177,6 +243,13 @@ export abstract class CategoryRefactorer implements Planner { /** * Creates operations to update the target stack with the resolved template. * Skips if the stack was already updated by a previous refactorer. + * + * Unlike updateSource(), updateTarget allows the changeset to contain diffs + * that are covered by an explicit allowlist returned from expectedTargetChanges(). + * A refactorer that intentionally rewrites part of the target template during + * resolveTarget() (e.g. replacing an Fn::GetAtt with a literal PLACEHOLDER) + * must override expectedTargetChanges() so that its intentional diff does not + * cause validate() to fail. Anything outside the allowlist still fails. */ protected async updateTarget(target: ResolvedStack): Promise { if (this.cfn.isUpdateClaimed(target.stackId)) return []; @@ -189,7 +262,16 @@ export abstract class CategoryRefactorer implements Planner { resource: this.resource, validate: () => ({ description: `Ensure no unexpected changes to ${targetStackName}`, - run: async () => ({ valid: change?.report === undefined, report: change?.report }), + run: async () => { + if (!change) return { valid: true }; + const allowlist = this.expectedTargetChanges(); + const unexpected = filterUnexpectedChanges(change.changeSet, allowlist); + if (unexpected.length === 0) return { valid: true }; + return { + valid: false, + report: renderUnexpectedChanges(unexpected, change.report ?? ''), + }; + }, }), describe: async () => { if (!change) return []; @@ -204,6 +286,18 @@ export abstract class CategoryRefactorer implements Planner { ]; } + /** + * Returns an allowlist of CFN changes that updateTarget() should treat as expected. + * Base implementation: no expected changes — updateTarget validate() fails on any diff. + * + * Subclasses that intentionally mutate the target template during resolveTarget() + * must override this to declare the expected diff, otherwise updateTarget validate() + * will fail. + */ + protected expectedTargetChanges(): ExpectedChange[] { + return []; + } + /** * Creates a change set for the given stack and returns the described change set with a formatted report. */ @@ -317,10 +411,15 @@ export abstract class CategoryRefactorer implements Planner { run: async (): Promise => { const stack = await this.cfn.describeStack(stackId); const status = stack.StackStatus; - if (status !== 'CREATE_COMPLETE' && status !== 'UPDATE_COMPLETE') { + // CloudFormation documents UPDATE_ROLLBACK_COMPLETE as a terminal state from which new updates are permitted. + // The root-level migration validator (_infra/validations.ts) accepts it so a prior failed update does not + // prevent retrying with a fixed template; we align the per-category check with that policy. + if (status !== 'CREATE_COMPLETE' && status !== 'UPDATE_COMPLETE' && status !== 'UPDATE_ROLLBACK_COMPLETE') { return { valid: false, - report: `Stack '${stackName}' is in ${status ?? 'UNKNOWN'} state, expected CREATE_COMPLETE or UPDATE_COMPLETE`, + report: `Stack '${stackName}' is in ${ + status ?? 'UNKNOWN' + } state, expected CREATE_COMPLETE, UPDATE_COMPLETE, or UPDATE_ROLLBACK_COMPLETE`, }; } return { valid: true }; diff --git a/packages/amplify-cli/src/commands/gen2-migration/refactor/workflow/forward-category-refactorer.ts b/packages/amplify-cli/src/commands/gen2-migration/refactor/workflow/forward-category-refactorer.ts index e5b628531e4..4508193f171 100644 --- a/packages/amplify-cli/src/commands/gen2-migration/refactor/workflow/forward-category-refactorer.ts +++ b/packages/amplify-cli/src/commands/gen2-migration/refactor/workflow/forward-category-refactorer.ts @@ -1,8 +1,8 @@ -import { Output, Parameter, ResourceMapping } from '@aws-sdk/client-cloudformation'; +import { ResourceMapping } from '@aws-sdk/client-cloudformation'; import { AmplifyError } from '@aws-amplify/amplify-cli-core'; import { CFNResource } from '../../_infra/cfn-template'; import { AmplifyMigrationOperation } from '../../_infra/operation'; -import { resolveParameters } from '../resolvers/cfn-parameter-resolver'; +import { resolveNoEchoParameters, resolveParameters } from '../resolvers/cfn-parameter-resolver'; import { resolveOutputs } from '../resolvers/cfn-output-resolver'; import { resolveDependencies } from '../resolvers/cfn-dependency-resolver'; import { resolveConditions } from '../resolvers/cfn-condition-resolver'; @@ -109,9 +109,12 @@ export abstract class ForwardCategoryRefactorer extends CategoryRefactorer { const withDeps = resolveDependencies(withOutputs); const resolved = resolveConditions(withDeps, parameters); - const updatedParameters = await this.resolveOAuthParameters(parameters, outputs); + // Transform masked NoEcho parameter values to UsePreviousValue so the "****" + // returned by DescribeStacks does not flow back into CreateChangeSet / + // UpdateStack and re-resolve into the template. + const sanitizedParameters = resolveNoEchoParameters(originalTemplate, parameters); - return { stackId, resolvedTemplate: resolved, parameters: updatedParameters }; + return { stackId, resolvedTemplate: resolved, parameters: sanitizedParameters }; } /** @@ -135,7 +138,12 @@ export abstract class ForwardCategoryRefactorer extends CategoryRefactorer { accountId: this.accountId, }); - return { stackId, resolvedTemplate: resolved, parameters }; + // Transform masked NoEcho parameter values to UsePreviousValue so the "****" + // returned by DescribeStacks does not flow back into CreateChangeSet / + // UpdateStack and re-resolve into the template. + const sanitizedParameters = resolveNoEchoParameters(originalTemplate, parameters); + + return { stackId, resolvedTemplate: resolved, parameters: sanitizedParameters }; } /** @@ -214,12 +222,4 @@ export abstract class ForwardCategoryRefactorer extends CategoryRefactorer { protected async afterMove(_gen2StackId: string): Promise { return []; } - - /** - * Hook for OAuth parameter resolution. Override in auth category. - */ - // eslint-disable-next-line @typescript-eslint/no-unused-vars - protected async resolveOAuthParameters(parameters: Parameter[], _outputs: Output[]): Promise { - return parameters; - } } diff --git a/packages/amplify-cli/src/commands/gen2-migration/refactor/workflow/rollback-category-refactorer.ts b/packages/amplify-cli/src/commands/gen2-migration/refactor/workflow/rollback-category-refactorer.ts index b3574c08dc8..9ecb1132bc7 100644 --- a/packages/amplify-cli/src/commands/gen2-migration/refactor/workflow/rollback-category-refactorer.ts +++ b/packages/amplify-cli/src/commands/gen2-migration/refactor/workflow/rollback-category-refactorer.ts @@ -2,7 +2,7 @@ import { ResourceMapping } from '@aws-sdk/client-cloudformation'; import { AmplifyError } from '@aws-amplify/amplify-cli-core'; import { CFNResource } from '../../_infra/cfn-template'; import { AmplifyMigrationOperation } from '../../_infra/operation'; -import { resolveParameters } from '../resolvers/cfn-parameter-resolver'; +import { resolveNoEchoParameters, resolveParameters } from '../resolvers/cfn-parameter-resolver'; import { resolveOutputs } from '../resolvers/cfn-output-resolver'; import { resolveDependencies } from '../resolvers/cfn-dependency-resolver'; import { extractStackNameFromId } from '../utils'; @@ -74,7 +74,12 @@ export abstract class RollbackCategoryRefactorer extends CategoryRefactorer { }); const resolved = resolveDependencies(withOutputs); - return { stackId, resolvedTemplate: resolved, parameters }; + // Transform masked NoEcho parameter values to UsePreviousValue so the "****" + // returned by DescribeStacks does not flow back into CreateChangeSet / + // UpdateStack and re-resolve into the template. + const sanitizedParameters = resolveNoEchoParameters(originalTemplate, parameters); + + return { stackId, resolvedTemplate: resolved, parameters: sanitizedParameters }; } /** @@ -86,7 +91,12 @@ export abstract class RollbackCategoryRefactorer extends CategoryRefactorer { const description = await facade.fetchStack(stackId); const parameters = description.Parameters ?? []; - return { stackId, resolvedTemplate: originalTemplate, parameters }; + // Transform masked NoEcho parameter values to UsePreviousValue so the "****" + // returned by DescribeStacks does not flow back into CreateChangeSet / + // UpdateStack and re-resolve into the template. + const sanitizedParameters = resolveNoEchoParameters(originalTemplate, parameters); + + return { stackId, resolvedTemplate: originalTemplate, parameters: sanitizedParameters }; } /** From 8bbf4ce30172f5271561cc1ee19940ce08217e96 Mon Sep 17 00:00:00 2001 From: 9pace Date: Thu, 30 Apr 2026 14:25:57 -0400 Subject: [PATCH 6/9] refactor: remove SupportedLoginProviders instead of PLACEHOLDER substitution MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Generate step now emits addPropertyDeletionOverride('SupportedLoginProviders') on the IdentityPool, removing the Fn::GetAtt to AmplifySecretFetcherResource entirely. This property enables direct federation which Amplify does not use — Gen1 never sets it and social login works via UserPool CognitoIdentityProviders. Removes the resolveTarget() PLACEHOLDER override, expectedTargetChanges(), and identityPoolLogicalIds from AuthCognitoForwardRefactorer. Both forward and rollback updateSource/updateTarget now produce empty changesets for auth. ADR-005 Addendum updated to reflect this decision. --- ...efactor-social-auth-idp-domain-handling.md | 50 +++++++++-- .../generate/amplify/auth/auth.generator.ts | 37 +++++--- .../refactor/auth/auth-cognito-forward.ts | 86 +------------------ 3 files changed, 67 insertions(+), 106 deletions(-) diff --git a/packages/amplify-cli/adr/005-refactor-social-auth-idp-domain-handling.md b/packages/amplify-cli/adr/005-refactor-social-auth-idp-domain-handling.md index c73b59d82fc..249861c3d38 100644 --- a/packages/amplify-cli/adr/005-refactor-social-auth-idp-domain-handling.md +++ b/packages/amplify-cli/adr/005-refactor-social-auth-idp-domain-handling.md @@ -387,12 +387,10 @@ design which assumed the imported resources would be deleted during rollback. The migration guide instructs users to configure SSM secrets matching their Gen1 credentials. -- **IdentityPool `SupportedLoginProviders`**: The Gen2 `IdentityPool` has - `SupportedLoginProviders` mapping `accounts.google.com` and `graph.facebook.com` to - client IDs via `AmplifySecretFetcherResource`. After the Gen1 IdentityPool moves in, - this property reflects Gen1's state. The next deploy would update it to whatever - `AmplifySecretFetcherResource` returns from SSM — this should be correct if the SSM - secrets match Gen1's credentials. +- **IdentityPool `SupportedLoginProviders`**: Removed from the Gen2 template by the + generate step. Gen1 does not set this property. Amplify's social login flow uses + `CognitoIdentityProviders` (UserPool-to-IdentityPool mapping), not direct federation + via `SupportedLoginProviders`. No impact on social login functionality. ### Open questions (from original analysis) @@ -435,7 +433,34 @@ Instead of moving IDPs and domain through the holding stack, **orphan them from 1. **Remove `UserPoolIdentityProvider` and `UserPoolDomain` from `RESOURCE_TYPES`**: Only 4 core types move through the holding stack: UserPool, UserPoolClient, IdentityPool, IdentityPoolRoleAttachment. -2. **Resolve IdentityPool's `Fn::GetAtt` to PLACEHOLDER in `resolveTarget()`**: Before `updateTarget()` pushes the Gen2 template, replace the IdentityPool's `SupportedLoginProviders` `Fn::GetAtt` references to `AmplifySecretFetcherResource` with `"PLACEHOLDER"`. CloudFormation accepts this. The live IdentityPool is updated (PLACEHOLDER in `SupportedLoginProviders`), but this is restored on next deploy. +2. **Remove `SupportedLoginProviders` from the Gen2 IdentityPool via generate**: The + CDK `defineAuth` construct with `externalProviders` generates `SupportedLoginProviders` + on the IdentityPool, mapping social provider domains to client IDs via `Fn::GetAtt` → + `AmplifySecretFetcherResource`. This property enables **direct federation** — signing + in to the IdentityPool directly with a social provider token, bypassing the UserPool. + Amplify's social login flow does not use direct federation; it routes through the + UserPool's Hosted UI → UserPoolIdentityProvider → UserPool tokens → IdentityPool via + `CognitoIdentityProviders`. Gen1 never sets `SupportedLoginProviders` on the + IdentityPool, and social login works correctly without it. The `generate` command + removes it via a CDK escape hatch: + + ```typescript + const cfnIdentityPool = backend.auth.resources.cfnResources.cfnIdentityPool; + cfnIdentityPool.addPropertyDeletionOverride('SupportedLoginProviders'); + ``` + + This eliminates the `Fn::GetAtt` → `AmplifySecretFetcherResource` reference on the + IdentityPool entirely. No PLACEHOLDER substitution is needed in `resolveTarget()` + (forward) or `resolveSource()` (rollback), and no `expectedTargetChanges()` override + is needed. Both `updateTarget()` and `updateSource()` produce empty changesets for + the auth stack. + + An earlier iteration used a PLACEHOLDER approach: `resolveTarget()` replaced the + `Fn::GetAtt` with `"PLACEHOLDER"` before `updateTarget()` pushed the template. This + worked for the forward path but caused problems on rollback — the IdentityPool would + arrive in Gen1 with PLACEHOLDER in `SupportedLoginProviders`, and Gen1's template + does not declare this property nor do Gen1's LambdaCallouts manage it, so PLACEHOLDER + would persist permanently. 3. **Orphan IDPs + domain from Gen2 after `super.beforeMove()`**: Remove them from the Gen2 template. Physical resources survive via `DeletionPolicy: Retain`. @@ -443,12 +468,18 @@ Instead of moving IDPs and domain through the holding stack, **orphan them from 5. **Rollback**: After `super.move()` (Gen2 → Gen1), orphan the imported IDPs + domain from Gen2. After `super.afterMove()` restores holding stack resources (P2) to Gen2, import Gen2's original IDPs + domain back into Gen2. +6. **`DeletionPolicy: Retain` on IDPs and domain**: The generate step must also set + `DeletionPolicy: Retain` on `UserPoolDomain` and `UserPoolIdentityProvider` resources + so that CDK deploys do not strip Retain and break the orphan safety check. This is + handled by the upstream `gen2-migration` branch's existing Retain and validation + framework. + ### Forward Flow ``` -resolveTarget() → Replace IdentityPool Fn::GetAtt with PLACEHOLDER -updateTarget() → Push clean Gen2 template (PLACEHOLDER in SupportedLoginProviders) +updateTarget() → empty changeset (no SupportedLoginProviders to resolve) beforeMove() → super.beforeMove() moves 4 core resources to holding + → Set DeletionPolicy: Retain on Gen2 IDPs + domain → Orphan IDPs + domain from Gen2 move() → super.move() moves Gen1 core resources into Gen2 → Import Gen1 IDPs + domain into Gen2 (dummy ProviderDetails) @@ -458,6 +489,7 @@ afterMove() → empty (not overridden) ### Rollback Flow ``` +updateSource() → empty changeset move() → super.move() moves core resources Gen2 → Gen1 → Orphan IDPs + domain from Gen2 (imported during forward move) afterMove() → super.afterMove() restores holding stack resources (P2) to Gen2 diff --git a/packages/amplify-cli/src/commands/gen2-migration/generate/amplify/auth/auth.generator.ts b/packages/amplify-cli/src/commands/gen2-migration/generate/amplify/auth/auth.generator.ts index ff842999454..9094ce611b5 100644 --- a/packages/amplify-cli/src/commands/gen2-migration/generate/amplify/auth/auth.generator.ts +++ b/packages/amplify-cli/src/commands/gen2-migration/generate/amplify/auth/auth.generator.ts @@ -101,7 +101,7 @@ export class AuthGenerator implements Planner { await fs.mkdir(authDir, { recursive: true }); await fs.writeFile(path.join(authDir, 'resource.ts'), content, 'utf-8'); - this.contributeToBackend(renderOptions); + this.contributeToBackend(renderOptions, hasIdentityProviders); if (hasIdentityProviders) { this.contributeProviderSetup(userPool.Domain); @@ -117,7 +117,7 @@ export class AuthGenerator implements Planner { * Generates password policy overrides, identity pool config, * and user pool client overrides as post-defineBackend statements. */ - private contributeToBackend(options: AuthRenderOptions): void { + private contributeToBackend(options: AuthRenderOptions, hasIdentityProviders: boolean): void { const authIdentifier = factory.createIdentifier('auth'); this.backendGenerator.addImport('./auth/resource', ['auth']); this.backendGenerator.addDefineBackendProperty(factory.createShorthandPropertyAssignment(authIdentifier)); @@ -128,9 +128,15 @@ export class AuthGenerator implements Planner { this.contributeUserPoolOverrides(userPoolOverrides); } - // Identity pool: disable guest access + // Identity Pool Overrides + if (options.identityPool?.AllowUnauthenticatedIdentities === false || hasIdentityProviders) { + this.backendGenerator.addConstFromBackend('cfnIdentityPool', 'auth', 'resources', 'cfnResources', 'cfnIdentityPool'); + } if (options.identityPool?.AllowUnauthenticatedIdentities === false) { - this.contributeIdentityPoolOverrides(); + this.backendGenerator.addStatement(TS.assignProp('cfnIdentityPool', 'allowUnauthenticatedIdentities', false)); + } + if (hasIdentityProviders) { + this.contributeSupportedLoginProvidersOverride(); } // cfnUserPoolClient override for OAuth flows (must come before addClient) @@ -145,6 +151,21 @@ export class AuthGenerator implements Planner { } } + private contributeSupportedLoginProvidersOverride(): void { + this.backendGenerator.addStatement( + factory.createExpressionStatement( + factory.createCallExpression( + factory.createPropertyAccessExpression( + factory.createIdentifier('cfnIdentityPool'), + factory.createIdentifier('addPropertyDeletionOverride'), + ), + undefined, + [factory.createStringLiteral('SupportedLoginProviders')], + ), + ), + ); + } + /** * Generates cfnUserPool password policy and username attribute overrides. */ @@ -182,14 +203,6 @@ export class AuthGenerator implements Planner { this.backendGenerator.addStatement(TS.assignProp('cfnUserPool', 'policies', policies)); } - /** - * Generates cfnIdentityPool.allowUnauthenticatedIdentities = false. - */ - private contributeIdentityPoolOverrides(): void { - this.backendGenerator.addConstFromBackend('cfnIdentityPool', 'auth', 'resources', 'cfnResources', 'cfnIdentityPool'); - this.backendGenerator.addStatement(TS.assignProp('cfnIdentityPool', 'allowUnauthenticatedIdentities', false)); - } - /** * Generates userPool.addClient('NativeAppClient', { ... }) for the * Gen1 native app client configuration. When identity providers are diff --git a/packages/amplify-cli/src/commands/gen2-migration/refactor/auth/auth-cognito-forward.ts b/packages/amplify-cli/src/commands/gen2-migration/refactor/auth/auth-cognito-forward.ts index 38b9cebc3fa..b01e96fc705 100644 --- a/packages/amplify-cli/src/commands/gen2-migration/refactor/auth/auth-cognito-forward.ts +++ b/packages/amplify-cli/src/commands/gen2-migration/refactor/auth/auth-cognito-forward.ts @@ -6,22 +6,13 @@ import { } from '@aws-sdk/client-cognito-identity-provider'; import { AmplifyError } from '@aws-amplify/amplify-cli-core'; import { ForwardCategoryRefactorer } from '../workflow/forward-category-refactorer'; -import { ExpectedChange, RefactorBlueprint, ResolvedStack } from '../workflow/category-refactorer'; +import { RefactorBlueprint } from '../workflow/category-refactorer'; import { CFNResource } from '../../_infra/cfn-template'; import { AmplifyMigrationOperation } from '../../_infra/operation'; -import { walkCfnTree } from '../resolvers/cfn-tree-walker'; import { extractStackNameFromId } from '../utils'; import { StackFacade } from '../stack-facade'; import CLITable from 'cli-table3'; -/** - * Gen2 logical ID of the Custom::AmplifySecretFetcherResource — a Lambda-backed - * custom resource that fetches OAuth client IDs/secrets from SSM at deploy time. - * Gen2 IDPs reference this via Fn::GetAtt in ProviderDetails, and the IdentityPool - * references it in SupportedLoginProviders. - */ -const AMPLIFY_SECRET_FETCHER_LOGICAL_ID = 'AmplifySecretFetcherResource'; - export const GEN1_NATIVE_APP_CLIENT = 'UserPoolClient'; export const GEN1_WEB_CLIENT = 'UserPoolClientWeb'; @@ -219,13 +210,6 @@ export function buildImportSpec( * already in Gen2 when the import runs. */ export class AuthCognitoForwardRefactorer extends ForwardCategoryRefactorer { - /** - * Logical IDs of AWS::Cognito::IdentityPool resources detected in the Gen2 - * template during resolveTarget(). Populated so that expectedTargetChanges() - * can declare the intentional PLACEHOLDER diff updateTarget() produces. - */ - private identityPoolLogicalIds: string[] = []; - /** * Returns only the core Cognito resource types. UserPoolDomain and * UserPoolIdentityProvider are handled via the orphan + import path @@ -235,74 +219,6 @@ export class AuthCognitoForwardRefactorer extends ForwardCategoryRefactorer { return RESOURCE_TYPES; } - /** - * Resolves the Gen2 target stack template, then replaces any Fn::GetAtt to - * AmplifySecretFetcherResource inside an IdentityPool's SupportedLoginProviders - * with the literal string "PLACEHOLDER". - * - * Why: the IdentityPool is a core resource that MUST go through the holding - * stack. Its SupportedLoginProviders has Fn::GetAtt references to - * AmplifySecretFetcherResource — a Custom::AmplifySecretFetcherResource that - * stays in Gen2. If we leave the GetAtt in place, CloudFormation rejects the - * holding-stack template with - * "instance of Fn::GetAtt references undefined resource AmplifySecretFetcherResource". - * - * The StackRefactor API validates property match for non-custom ref types, but - * CloudFormation accepts the literal string "PLACEHOLDER" in an IdentityPool's - * SupportedLoginProviders values (verified via changeset experiment in ADR-005 - * Addendum: CREATE_COMPLETE, Replacement: False). The next Gen2 deploy - * regenerates the correct values from AmplifySecretFetcherResource. - * - * Scope is intentionally narrow: only Fn::GetAtt nodes whose target is - * AmplifySecretFetcherResource, only inside IdentityPool resources, only under - * SupportedLoginProviders. - */ - protected override async resolveTarget(stackId: string): Promise { - const resolved = await super.resolveTarget(stackId); - const template = resolved.resolvedTemplate; - const identityPoolLogicalIds: string[] = []; - for (const [logicalId, resource] of Object.entries(template.Resources)) { - if (resource.Type !== IDENTITY_POOL_TYPE) continue; - identityPoolLogicalIds.push(logicalId); - const slp = resource.Properties.SupportedLoginProviders; - if (!slp || typeof slp !== 'object') continue; - const walked = walkCfnTree(slp, (node) => { - if ('Fn::GetAtt' in node && Array.isArray(node['Fn::GetAtt']) && Object.keys(node).length === 1) { - const [target] = node['Fn::GetAtt'] as string[]; - if (target === AMPLIFY_SECRET_FETCHER_LOGICAL_ID) { - return 'PLACEHOLDER'; - } - } - return undefined; - }); - resource.Properties.SupportedLoginProviders = walked as object; - } - this.identityPoolLogicalIds = identityPoolLogicalIds; - return resolved; - } - - /** - * Declares the expected changeset diff produced by resolveTarget(): the - * IdentityPool's SupportedLoginProviders values are replaced with the literal - * string 'PLACEHOLDER'. updateTarget() validate() uses this allowlist to - * accept those intentional diffs while still failing on unrelated diffs. - * - * Populated from `identityPoolLogicalIds` which resolveTarget() captures. - * Ordering: plan() calls resolveTarget before updateTarget, so this method - * is consulted after identityPoolLogicalIds is set. - */ - protected override expectedTargetChanges(): ExpectedChange[] { - return this.identityPoolLogicalIds.map((logicalId) => ({ - logicalId, - // CFN's ResourceChangeDetail.Target.Path is `/Properties/` — the leading `/Properties/` - // segment is part of Path, not separated via Attribute (Attribute is `"Properties"`, indicating the change - // is at the property level). We prefix with `/Properties/SupportedLoginProviders` so nested changes like - // `/Properties/SupportedLoginProviders/accounts.google.com` are correctly matched. - propertyPathPrefix: '/Properties/SupportedLoginProviders', - expectedAfterValueSubstring: 'PLACEHOLDER', - })); - } - /** * Executes the standard beforeMove (moves 4 core Cognito resources to holding), * then appends two additional operations in order: From d56d49116a875d889d2f52fe84f3a5c1daec32bd Mon Sep 17 00:00:00 2001 From: 9pace Date: Thu, 30 Apr 2026 14:27:37 -0400 Subject: [PATCH 7/9] test: remove expectedTargetChanges PLACEHOLDER tests The resolveTarget override and expectedTargetChanges were removed in the previous commit. Remove the corresponding test describe block. --- .../auth/auth-cognito-forward.test.ts | 229 +++++++++++++++--- 1 file changed, 195 insertions(+), 34 deletions(-) diff --git a/packages/amplify-cli/src/__tests__/commands/gen2-migration/refactor/auth/auth-cognito-forward.test.ts b/packages/amplify-cli/src/__tests__/commands/gen2-migration/refactor/auth/auth-cognito-forward.test.ts index 84b82e34474..f0cc542ea00 100644 --- a/packages/amplify-cli/src/__tests__/commands/gen2-migration/refactor/auth/auth-cognito-forward.test.ts +++ b/packages/amplify-cli/src/__tests__/commands/gen2-migration/refactor/auth/auth-cognito-forward.test.ts @@ -16,8 +16,8 @@ import { ExecuteChangeSetCommand, DeleteChangeSetCommand, ResourceMapping, + UpdateStackCommand, } from '@aws-sdk/client-cloudformation'; -import { SSMClient } from '@aws-sdk/client-ssm'; import { CognitoIdentityProviderClient, DescribeIdentityProviderCommand, @@ -26,6 +26,20 @@ import { } from '@aws-sdk/client-cognito-identity-provider'; import { Cfn } from '../../../../../commands/gen2-migration/refactor/cfn'; +// Mock SDK waiters so execute-time tests don't hang on real polling. +jest.mock('@aws-sdk/client-cloudformation', () => { + const actual = jest.requireActual('@aws-sdk/client-cloudformation'); + return { + ...actual, + waitUntilStackUpdateComplete: jest.fn().mockResolvedValue({ state: 'SUCCESS' }), + waitUntilStackCreateComplete: jest.fn().mockResolvedValue({ state: 'SUCCESS' }), + waitUntilStackDeleteComplete: jest.fn().mockResolvedValue({ state: 'SUCCESS' }), + waitUntilStackRefactorCreateComplete: jest.fn().mockResolvedValue({ state: 'SUCCESS' }), + waitUntilStackRefactorExecuteComplete: jest.fn().mockResolvedValue({ state: 'SUCCESS' }), + waitUntilChangeSetCreateComplete: jest.fn().mockResolvedValue({ state: 'SUCCESS' }), + }; +}); + const ts = new Date(); const rs = ResourceStatus.CREATE_COMPLETE; @@ -102,7 +116,6 @@ describe('AuthCognitoForwardRefactorer.plan() — operation sequence', () => { beforeEach(() => { cfnMock = mockClient(CloudFormationClient); - mockClient(SSMClient); mockClient(CognitoIdentityProviderClient); }); @@ -131,7 +144,8 @@ describe('AuthCognitoForwardRefactorer.plan() — operation sequence', () => { const descriptions = await Promise.all(ops.map((op) => op.describe())); const flat = descriptions.flat(); - // Expected sequence: updateSource, updateTarget, beforeMove (holding), mainAuthMove + // Expected sequence: updateSource, updateTarget, beforeMove (holding), mainAuthMove. + // Non-OAuth: no IDP/domain in Gen2 template → no orphan op in beforeMove, no import op in afterMove. expect(flat).toHaveLength(4); expect(flat[0]).toContain('Update source'); expect(flat[1]).toContain('Update target'); @@ -139,15 +153,51 @@ describe('AuthCognitoForwardRefactorer.plan() — operation sequence', () => { expect(flat[3]).toContain('Move'); }); - it('OAuth: populates hostedUIProviderCreds when hostedUIProviderMeta parameter exists', async () => { + it('OAuth: orphans IDPs+domain in beforeMove, imports Gen1 IDPs+domain in move, does not call DescribeIdentityProvider', async () => { const oauthGen1Template: CFNTemplate = { ...gen1AuthTemplate, Parameters: { hostedUIProviderMeta: { Type: 'String' }, - hostedUIProviderCreds: { Type: 'String' }, + hostedUIProviderCreds: { Type: 'String', NoEcho: true }, }, }; + // Gen2 template extended to include the typical social-auth-app resources: + // an IdentityPool with a SupportedLoginProviders Fn::GetAtt to + // AmplifySecretFetcherResource, plus a UserPoolDomain and a Google IDP. + const oauthGen2Template: CFNTemplate = { + AWSTemplateFormatVersion: '2010-09-09', + Description: 'gen2 auth', + Resources: { + amplifyAuthUserPool12345678: { Type: 'AWS::Cognito::UserPool', Properties: {} }, + amplifyAuthIdentityPool12345678: { + Type: 'AWS::Cognito::IdentityPool', + Properties: { + SupportedLoginProviders: { + 'accounts.google.com': { 'Fn::GetAtt': ['AmplifySecretFetcherResource', 'GOOGLE_CLIENT_ID'] }, + }, + }, + }, + amplifyAuthGoogleIdP12345678: { + Type: 'AWS::Cognito::UserPoolIdentityProvider', + Properties: { + UserPoolId: { Ref: 'amplifyAuthUserPool12345678' }, + ProviderName: 'Google', + ProviderType: 'Google', + }, + }, + amplifyAuthUserPoolDomain12345678: { + Type: 'AWS::Cognito::UserPoolDomain', + Properties: { + UserPoolId: { Ref: 'amplifyAuthUserPool12345678' }, + Domain: 'gen2-auto-prefix', + }, + }, + AmplifySecretFetcherResource: { Type: 'Custom::AmplifySecretFetcherResource', Properties: {} }, + }, + Outputs: {}, + }; + // Default: no stacks found (for holding stack lookup) cfnMock.on(DescribeStacksCommand).resolves({ Stacks: [] }); @@ -185,7 +235,7 @@ describe('AuthCognitoForwardRefactorer.plan() — operation sequence', () => { Description: oauthGen1Template.Description, Parameters: [ { ParameterKey: 'hostedUIProviderMeta', ParameterValue: JSON.stringify([{ ProviderName: 'Google' }]) }, - { ParameterKey: 'hostedUIProviderCreds', ParameterValue: '[]' }, + { ParameterKey: 'hostedUIProviderCreds', ParameterValue: '****' }, ], Outputs: [{ OutputKey: 'UserPoolId', OutputValue: 'us-east-1_ABC123' }], }, @@ -195,7 +245,7 @@ describe('AuthCognitoForwardRefactorer.plan() — operation sequence', () => { Stacks: [{ StackName: 'gen2-auth-stack', StackStatus: rs, CreationTime: ts, Parameters: [], Outputs: [] }], }); cfnMock.on(GetTemplateCommand, { StackName: 'gen1-auth-stack' }).resolves({ TemplateBody: JSON.stringify(oauthGen1Template) }); - cfnMock.on(GetTemplateCommand, { StackName: 'gen2-auth-stack' }).resolves({ TemplateBody: JSON.stringify(gen2AuthTemplate) }); + cfnMock.on(GetTemplateCommand, { StackName: 'gen2-auth-stack' }).resolves({ TemplateBody: JSON.stringify(oauthGen2Template) }); cfnMock.on(CreateChangeSetCommand).resolves({}); cfnMock.on(DescribeChangeSetCommand).callsFake((input) => ({ Status: 'CREATE_COMPLETE', StackName: input.StackName, Changes: [] })); cfnMock.on(ExecuteChangeSetCommand).resolves({}); @@ -203,19 +253,11 @@ describe('AuthCognitoForwardRefactorer.plan() — operation sequence', () => { const cognitoMock = mockClient(CognitoIdentityProviderClient); cognitoMock.on(DescribeUserPoolCommand).resolves({ - UserPool: { Id: 'us-east-1_ABC123', Domain: 'test-domain' }, + UserPool: { Id: 'us-east-1_ABC123', Domain: 'gen1-prefix' }, }); cognitoMock.on(ListIdentityProvidersCommand).resolves({ Providers: [{ ProviderName: 'Google', ProviderType: 'Google' }], }); - cognitoMock.on(DescribeIdentityProviderCommand).resolves({ - IdentityProvider: { - ProviderName: 'Google', - ProviderType: 'Google', - ProviderDetails: { client_id: 'google-id', client_secret: 'google-secret', authorize_scopes: 'openid email profile' }, - AttributeMapping: { email: 'email' }, - }, - }); const clients = new (AwsClients as any)({ region: 'us-east-1' }); (clients as any).cloudFormation = new CloudFormationClient({}); @@ -234,24 +276,34 @@ describe('AuthCognitoForwardRefactorer.plan() — operation sequence', () => { const ops = await refactorer.plan(); - // Called once by retrieveOAuthValues and once by fetchSocialAuthConfig - expect(cognitoMock.commandCalls(DescribeIdentityProviderCommand)).toHaveLength(2); - expect(ops.length).toBeGreaterThanOrEqual(4); - - const { CreateChangeSetCommand: CreateCS } = await import('@aws-sdk/client-cloudformation'); - cfnMock.on(DescribeStacksCommand).resolves({ - Stacks: [{ StackName: 'gen1-auth-stack', StackStatus: 'UPDATE_COMPLETE', CreationTime: ts }], - }); - // ops[0] and ops[1] are stack status validations; ops[2] is updateSource - await ops[2].execute(); - - const createCsCalls = cfnMock.commandCalls(CreateCS); - expect(createCsCalls.length).toBeGreaterThanOrEqual(1); - const credsParam = createCsCalls[0].args[0].input.Parameters?.find( - (p: { ParameterKey?: string }) => p.ParameterKey === 'hostedUIProviderCreds', - ); - expect(credsParam?.ParameterValue).toContain('google-id'); - expect(credsParam?.ParameterValue).toContain('google-secret'); + // DescribeIdentityProvider is no longer used — the Orphan + Import approach + // imports IDPs with dummy ProviderDetails; CFN import does not validate + // property match so real secrets are not needed. + expect(cognitoMock.commandCalls(DescribeIdentityProviderCommand)).toHaveLength(0); + + const descriptions = (await Promise.all(ops.map((op) => op.describe()))).flat(); + + // beforeMove should produce the standard holding-stack move AND both the + // Retain-set op (sets DeletionPolicy: Retain on Gen2 IDPs + domain) and + // the orphan operation for the IDP + domain in the Gen2 template. + expect(descriptions.some((d) => d.includes('Set DeletionPolicy: Retain') && d.includes('social auth'))).toBe(true); + expect(descriptions.some((d) => d.includes('Orphan') && d.includes('social auth'))).toBe(true); + + // move() should produce the import operation — the import is now appended + // to move() (runs after super.move()) rather than in afterMove(), so that + // the Gen1 pool is already in Gen2 when the import executes. + expect(descriptions.some((d) => d.includes('Import social auth'))).toBe(true); + + // Ordering: Retain-set (in beforeMove) must come BEFORE orphan (also in + // beforeMove), and orphan must come BEFORE import (in move). + const retainIndex = descriptions.findIndex((d) => d.includes('Set DeletionPolicy: Retain') && d.includes('social auth')); + const orphanIndex = descriptions.findIndex((d) => d.includes('Orphan') && d.includes('social auth')); + const importIndex = descriptions.findIndex((d) => d.includes('Import social auth')); + expect(retainIndex).toBeGreaterThanOrEqual(0); + expect(orphanIndex).toBeGreaterThanOrEqual(0); + expect(importIndex).toBeGreaterThanOrEqual(0); + expect(retainIndex).toBeLessThan(orphanIndex); + expect(orphanIndex).toBeLessThan(importIndex); cognitoMock.restore(); }); @@ -354,3 +406,112 @@ describe('AuthCognitoForwardRefactorer.buildResourceMappings — UserPoolClient expect(map.get('UserPool')).toBe('amplifyAuthUserPool1234ABCD'); }); }); + +/** + * Verifies that the orphan social auth operation performs the Retain check at + * execute time (not plan-validate time) — see Fix 3 in the oauth-workspace + * agent context. The Retain invariant is established by the preceding + * Retain-set op's execute(); checking it at plan-validation time would fail + * on first run because plan.validate() runs before any execute(). + */ +describe('AuthCognitoForwardRefactorer — orphan op execute-time Retain check', () => { + let cfnMock: ReturnType; + beforeEach(() => { + cfnMock = mockClient(CloudFormationClient); + }); + afterEach(() => cfnMock.restore()); + + class TestRefactorer extends AuthCognitoForwardRefactorer { + public testBuildOrphanOp(gen2StackId: string) { + // Exposes the private method for direct testing. + return (this as unknown as { + buildOrphanSocialAuthOperation: (id: string) => Promise; + }).buildOrphanSocialAuthOperation(gen2StackId); + } + } + + function createRefactorer() { + const clients = new (AwsClients as any)({ region: 'us-east-1' }); + (clients as any).cloudFormation = new CloudFormationClient({}); + const gen1Env = new StackFacade(clients, 'gen1-root'); + const gen2Branch = new StackFacade(clients, 'gen2-root'); + return new TestRefactorer( + gen1Env, + gen2Branch, + { region: 'us-east-1', clients, appId: 'appId', envName: 'main' } as unknown as Gen1App, + '123456789', + noOpLogger(), + { category: 'auth', resourceName: 'test', service: 'Cognito', key: 'auth:Cognito' as const }, + new Cfn(new CloudFormationClient({}), noOpLogger()), + ); + } + + const idpResource = { + Type: 'AWS::Cognito::UserPoolIdentityProvider', + Properties: { ProviderName: 'Google', ProviderType: 'Google' }, + } as const; + const domainResource = { + Type: 'AWS::Cognito::UserPoolDomain', + Properties: { Domain: 'gen2-domain' }, + } as const; + + it('validate() returns undefined (check moved to execute-time)', async () => { + const templateWithoutRetain: CFNTemplate = { + AWSTemplateFormatVersion: '2010-09-09', + Description: 'gen2 auth', + Resources: { amplifyAuthGoogleIdP: idpResource, amplifyAuthUserPoolDomain: domainResource }, + Outputs: {}, + }; + cfnMock.on(GetTemplateCommand).resolves({ TemplateBody: JSON.stringify(templateWithoutRetain) }); + + const op = await createRefactorer().testBuildOrphanOp('gen2-auth-stack'); + expect(op).toBeDefined(); + expect(op!.validate()).toBeUndefined(); + }); + + it('execute() throws AmplifyError when any target is missing DeletionPolicy: Retain', async () => { + const templateMissingRetain: CFNTemplate = { + AWSTemplateFormatVersion: '2010-09-09', + Description: 'gen2 auth', + Resources: { + amplifyAuthGoogleIdP: idpResource, // no DeletionPolicy + amplifyAuthUserPoolDomain: { ...domainResource, DeletionPolicy: 'Retain' }, + }, + Outputs: {}, + }; + cfnMock.on(GetTemplateCommand).resolves({ TemplateBody: JSON.stringify(templateMissingRetain) }); + cfnMock.on(DescribeStacksCommand).resolves({ + Stacks: [{ StackName: 'gen2-auth-stack', StackStatus: rs, CreationTime: ts, Parameters: [] }], + }); + cfnMock.on(UpdateStackCommand).resolves({}); + + const op = await createRefactorer().testBuildOrphanOp('gen2-auth-stack'); + expect(op).toBeDefined(); + await expect(op!.execute()).rejects.toThrow(/DeletionPolicy: Retain/); + // Ensure we aborted BEFORE issuing the destructive UpdateStackCommand. + expect(cfnMock.commandCalls(UpdateStackCommand)).toHaveLength(0); + }); + + it('execute() succeeds when every target has DeletionPolicy: Retain', async () => { + const templateWithRetain: CFNTemplate = { + AWSTemplateFormatVersion: '2010-09-09', + Description: 'gen2 auth', + Resources: { + amplifyAuthGoogleIdP: { ...idpResource, DeletionPolicy: 'Retain' }, + amplifyAuthUserPoolDomain: { ...domainResource, DeletionPolicy: 'Retain' }, + }, + Outputs: {}, + }; + cfnMock.on(GetTemplateCommand).resolves({ TemplateBody: JSON.stringify(templateWithRetain) }); + cfnMock.on(DescribeStacksCommand).resolves({ + Stacks: [{ StackName: 'gen2-auth-stack', StackStatus: rs, CreationTime: ts, Parameters: [] }], + }); + cfnMock.on(UpdateStackCommand).resolves({}); + + const op = await createRefactorer().testBuildOrphanOp('gen2-auth-stack'); + expect(op).toBeDefined(); + await expect(op!.execute()).resolves.toBeUndefined(); + // The destructive update ran exactly once. + expect(cfnMock.commandCalls(UpdateStackCommand)).toHaveLength(1); + }); +}); From c1ede3843542d5ff520ff5cb910f0cc33d599121 Mon Sep 17 00:00:00 2001 From: 9pace Date: Thu, 30 Apr 2026 14:36:43 -0400 Subject: [PATCH 8/9] test: add tests for OAuth orphan+import, NoEcho params, and allowlist validation - auth-cognito-rollback: OAuth rollback plan, targetLogicalId exclusions, orphan Retain check - cfn-parameter-resolver: resolveNoEchoParameters coverage - category-refactorer: UPDATE_ROLLBACK_COMPLETE accepted, expectedTargetChanges allowlist - rollback-category-refactorer: formatting only - Snapshot updates for scopes, attribute mappings, and domain override --- .../amplify/auth/resource.ts | 8 + .../amplify/backend.ts | 6 +- .../auth/auth-cognito-rollback.test.ts | 271 +++++++++++++++++- .../resolvers/cfn-parameter-resolver.test.ts | 45 ++- .../workflow/category-refactorer.test.ts | 156 +++++++++- .../rollback-category-refactorer.test.ts | 6 +- 6 files changed, 483 insertions(+), 9 deletions(-) diff --git a/amplify-migration-apps/media-vault/_snapshot.post.generate/amplify/auth/resource.ts b/amplify-migration-apps/media-vault/_snapshot.post.generate/amplify/auth/resource.ts index b906304c399..68fc644d76c 100644 --- a/amplify-migration-apps/media-vault/_snapshot.post.generate/amplify/auth/resource.ts +++ b/amplify-migration-apps/media-vault/_snapshot.post.generate/amplify/auth/resource.ts @@ -12,15 +12,23 @@ export const auth = defineAuth({ google: { clientId: secret('GOOGLE_CLIENT_ID'), clientSecret: secret('GOOGLE_CLIENT_SECRET'), + scopes: ['openid', 'email', 'profile'], attributeMapping: { email: 'email', + custom: { + username: 'sub', + }, }, }, facebook: { clientId: secret('FACEBOOK_CLIENT_ID'), clientSecret: secret('FACEBOOK_CLIENT_SECRET'), + scopes: ['email', 'public_profile'], attributeMapping: { email: 'email', + custom: { + username: 'id', + }, }, }, callbackUrls: ['https://main.mediavault.amplifyapp.com/'], diff --git a/amplify-migration-apps/media-vault/_snapshot.post.generate/amplify/backend.ts b/amplify-migration-apps/media-vault/_snapshot.post.generate/amplify/backend.ts index 1c34596ad22..81cbf5bed47 100644 --- a/amplify-migration-apps/media-vault/_snapshot.post.generate/amplify/backend.ts +++ b/amplify-migration-apps/media-vault/_snapshot.post.generate/amplify/backend.ts @@ -9,6 +9,7 @@ import { Duration } from 'aws-cdk-lib'; import { OAuthScope, UserPoolClientIdentityProvider, + CfnUserPoolDomain, } from 'aws-cdk-lib/aws-cognito'; const backend = defineBackend({ @@ -78,7 +79,10 @@ Object.keys(providerSetupResult).forEach((provider) => { userPoolClient.node.addDependency(providerSetupPropertyValue); } }); -// backend.auth.resources.userPool.node.tryRemoveChild("UserPoolDomain"); +const cfnUserPoolDomain = backend.auth.resources.userPool.node.findChild( + 'UserPoolDomain' +).node.defaultChild as CfnUserPoolDomain; +cfnUserPoolDomain.domain = 'mediavault1f08412d-1f08412d-main'; const cfnGraphqlApi = backend.data.resources.cfnResources.cfnGraphqlApi; cfnGraphqlApi.additionalAuthenticationProviders = [ { diff --git a/packages/amplify-cli/src/__tests__/commands/gen2-migration/refactor/auth/auth-cognito-rollback.test.ts b/packages/amplify-cli/src/__tests__/commands/gen2-migration/refactor/auth/auth-cognito-rollback.test.ts index e2d6000e90a..9938c0462cb 100644 --- a/packages/amplify-cli/src/__tests__/commands/gen2-migration/refactor/auth/auth-cognito-rollback.test.ts +++ b/packages/amplify-cli/src/__tests__/commands/gen2-migration/refactor/auth/auth-cognito-rollback.test.ts @@ -14,9 +14,30 @@ import { CreateChangeSetCommand, DescribeChangeSetCommand, DeleteChangeSetCommand, + ExecuteChangeSetCommand, + UpdateStackCommand, } from '@aws-sdk/client-cloudformation'; +import { + CognitoIdentityProviderClient, + DescribeUserPoolCommand, + ListIdentityProvidersCommand, +} from '@aws-sdk/client-cognito-identity-provider'; import { Cfn } from '../../../../../commands/gen2-migration/refactor/cfn'; +// Mock SDK waiters so execute-time tests don't hang on real polling. +jest.mock('@aws-sdk/client-cloudformation', () => { + const actual = jest.requireActual('@aws-sdk/client-cloudformation'); + return { + ...actual, + waitUntilStackUpdateComplete: jest.fn().mockResolvedValue({ state: 'SUCCESS' }), + waitUntilStackCreateComplete: jest.fn().mockResolvedValue({ state: 'SUCCESS' }), + waitUntilStackDeleteComplete: jest.fn().mockResolvedValue({ state: 'SUCCESS' }), + waitUntilStackRefactorCreateComplete: jest.fn().mockResolvedValue({ state: 'SUCCESS' }), + waitUntilStackRefactorExecuteComplete: jest.fn().mockResolvedValue({ state: 'SUCCESS' }), + waitUntilChangeSetCreateComplete: jest.fn().mockResolvedValue({ state: 'SUCCESS' }), + }; +}); + const ts = new Date(); const rs = ResourceStatus.CREATE_COMPLETE; @@ -119,6 +140,146 @@ describe('AuthCognitoRollbackRefactorer.plan()', () => { expect(descriptions.some((d) => d.includes('Update target'))).toBe(true); expect(descriptions.some((d) => d.includes('Move'))).toBe(true); }); + + it('OAuth: orphans IDPs+domain in move, re-imports them in afterMove, orphan precedes import', async () => { + // Gen2 template still contains the IDP + domain resources imported during + // forward. Rollback's move() orphan runs at execute time — plan-time reads + // still see the resources. + const oauthGen2Template: CFNTemplate = { + AWSTemplateFormatVersion: '2010-09-09', + Description: 'gen2 auth', + Resources: { + amplifyAuthUserPool12345678: { Type: 'AWS::Cognito::UserPool', Properties: {} }, + amplifyAuthUserPoolAppClient12345678: { Type: 'AWS::Cognito::UserPoolClient', Properties: {} }, + amplifyAuthGoogleIdP12345678: { + Type: 'AWS::Cognito::UserPoolIdentityProvider', + DeletionPolicy: 'Retain', + Properties: { + UserPoolId: { Ref: 'amplifyAuthUserPool12345678' }, + ProviderName: 'Google', + ProviderType: 'Google', + }, + }, + amplifyAuthUserPoolDomain12345678: { + Type: 'AWS::Cognito::UserPoolDomain', + DeletionPolicy: 'Retain', + Properties: { + UserPoolId: { Ref: 'amplifyAuthUserPool12345678' }, + Domain: 'p2-domain', + }, + }, + }, + Outputs: {}, + }; + + cfnMock.on(DescribeStacksCommand).resolves({ Stacks: [] }); + + cfnMock.on(DescribeStackResourcesCommand, { StackName: 'gen2-root' }).resolves({ + StackResources: [ + { + LogicalResourceId: 'authStack', + ResourceType: 'AWS::CloudFormation::Stack', + PhysicalResourceId: 'gen2-auth', + Timestamp: ts, + ResourceStatus: rs, + }, + ], + }); + cfnMock.on(DescribeStackResourcesCommand, { StackName: 'gen1-root' }).resolves({ + StackResources: [ + { + LogicalResourceId: 'authtestMain', + ResourceType: 'AWS::CloudFormation::Stack', + PhysicalResourceId: 'gen1-auth', + Timestamp: ts, + ResourceStatus: rs, + }, + ], + }); + cfnMock.on(DescribeStackResourcesCommand, { StackName: 'gen1-auth' }).resolves({ StackResources: [] }); + // Gen2 DescribeStackResources — returns the UserPool at execute time. + cfnMock.on(DescribeStackResourcesCommand, { StackName: 'gen2-auth' }).resolves({ + StackResources: [ + { + LogicalResourceId: 'amplifyAuthUserPool12345678', + ResourceType: 'AWS::Cognito::UserPool', + PhysicalResourceId: 'us-east-1_P2', + Timestamp: ts, + ResourceStatus: rs, + }, + ], + }); + + cfnMock.on(DescribeStacksCommand, { StackName: 'gen2-auth' }).resolves({ + Stacks: [{ StackName: 'gen2-auth', StackStatus: rs, CreationTime: ts, Parameters: [], Outputs: [] }], + }); + cfnMock.on(DescribeStacksCommand, { StackName: 'gen1-auth' }).resolves({ + Stacks: [ + { + StackName: 'gen1-auth', + StackStatus: rs, + CreationTime: ts, + Description: gen1AuthTemplate.Description, + Parameters: [], + Outputs: [], + }, + ], + }); + + cfnMock.on(GetTemplateCommand, { StackName: 'gen2-auth' }).resolves({ TemplateBody: JSON.stringify(oauthGen2Template) }); + cfnMock.on(GetTemplateCommand, { StackName: 'gen1-auth' }).resolves({ TemplateBody: JSON.stringify(gen1AuthTemplate) }); + + cfnMock.on(CreateChangeSetCommand).resolves({}); + cfnMock.on(DescribeChangeSetCommand).resolves({ Status: 'CREATE_COMPLETE', Changes: [] }); + cfnMock.on(ExecuteChangeSetCommand).resolves({}); + cfnMock.on(DeleteChangeSetCommand).resolves({}); + + const cognitoMock = mockClient(CognitoIdentityProviderClient); + cognitoMock.on(DescribeUserPoolCommand).resolves({ + UserPool: { Id: 'us-east-1_P2', Domain: 'p2-domain' }, + }); + cognitoMock.on(ListIdentityProvidersCommand).resolves({ + Providers: [{ ProviderName: 'Google', ProviderType: 'Google' }], + }); + + const clients = new (AwsClients as any)({ region: 'us-east-1' }); + (clients as any).cloudFormation = new CloudFormationClient({}); + (clients as any).cognitoIdentityProvider = new CognitoIdentityProviderClient({}); + const cfn = new Cfn(new CloudFormationClient({}), noOpLogger()); + const refactorer = new AuthCognitoRollbackRefactorer( + new StackFacade(clients, 'gen1-root'), + new StackFacade(clients, 'gen2-root'), + { region: 'us-east-1', clients, appId: 'appId', envName: 'main' } as unknown as Gen1App, + '123', + noOpLogger(), + { category: 'auth', resourceName: 'test', service: 'Cognito', key: 'auth:Cognito' as const }, + cfn, + ); + + const ops = await refactorer.plan(); + const descriptions = (await Promise.all(ops.map((o) => o.describe()))).flat(); + + // (a) move() should produce both the core refactor move and the orphan op. + expect(descriptions.some((d) => d.includes('Orphan') && d.includes('imported social auth'))).toBe(true); + + // (a cont.) afterMove() should produce the import op (re-importing Gen2 + // original IDPs + domain back into Gen2 after super.afterMove() restores P2). + expect(descriptions.some((d) => d.includes('Import social auth'))).toBe(true); + + // (b) Ordering: orphan (in move) must come BEFORE import (in afterMove). + const orphanIndex = descriptions.findIndex((d) => d.includes('Orphan') && d.includes('imported social auth')); + const importIndex = descriptions.findIndex((d) => d.includes('Import social auth')); + expect(orphanIndex).toBeGreaterThanOrEqual(0); + expect(importIndex).toBeGreaterThanOrEqual(0); + expect(orphanIndex).toBeLessThan(importIndex); + + // (c) Import description lists Google provider and the domain logical ID. + const importDescription = descriptions[importIndex]; + expect(importDescription).toContain('Google'); + expect(importDescription).toContain('amplifyAuthUserPoolDomain12345678'); + + cognitoMock.restore(); + }); }); describe('AuthCognitoRollbackRefactorer.targetLogicalId', () => { @@ -154,8 +315,8 @@ describe('AuthCognitoRollbackRefactorer.targetLogicalId', () => { ); }); - it('maps UserPoolDomain to UserPoolDomain', () => { - expect(refactorer.testTargetLogicalId('amplifyAuthUserPoolDomain1234', 'AWS::Cognito::UserPoolDomain')).toBe('UserPoolDomain'); + it('returns undefined for UserPoolDomain (excluded from mappings by buildResourceMappings filter)', () => { + expect(refactorer.testTargetLogicalId('amplifyAuthUserPoolDomain1234', 'AWS::Cognito::UserPoolDomain')).toBeUndefined(); }); it('maps NativeAppClient to UserPoolClient', () => { @@ -176,3 +337,109 @@ describe('AuthCognitoRollbackRefactorer.targetLogicalId', () => { expect(refactorer.testTargetLogicalId('SomeResource', 'AWS::Lambda::Function')).toBeUndefined(); }); }); + +/** + * Verifies that the rollback orphan social auth operation performs the Retain + * check at execute time (not plan-validate time) — see Fix 3 in the + * oauth-workspace agent context. This mirrors the forward symmetry: both + * orphan ops defer the Retain check to execute() to avoid the plan.validate() + * anti-pattern where validate runs before any execute side-effects. + */ +describe('AuthCognitoRollbackRefactorer — orphan op execute-time Retain check', () => { + let cfnMock: ReturnType; + beforeEach(() => { + cfnMock = mockClient(CloudFormationClient); + }); + afterEach(() => cfnMock.restore()); + + class TestRefactorer extends AuthCognitoRollbackRefactorer { + public testBuildOrphanOp(gen2StackId: string) { + return (this as unknown as { + buildOrphanSocialAuthOperation: ( + id: string, + ) => Promise; + }).buildOrphanSocialAuthOperation(gen2StackId); + } + } + + function createRefactorer() { + const clients = new (AwsClients as any)({ region: 'us-east-1' }); + (clients as any).cloudFormation = new CloudFormationClient({}); + return new TestRefactorer( + new StackFacade(clients, 'gen1-root'), + new StackFacade(clients, 'gen2-root'), + { region: 'us-east-1', clients, appId: 'appId', envName: 'main' } as unknown as Gen1App, + '123456789', + noOpLogger(), + { category: 'auth', resourceName: 'test', service: 'Cognito', key: 'auth:Cognito' as const }, + new Cfn(new CloudFormationClient({}), noOpLogger()), + ); + } + + const idpResource = { + Type: 'AWS::Cognito::UserPoolIdentityProvider', + Properties: { ProviderName: 'Google', ProviderType: 'Google' }, + } as const; + const domainResource = { + Type: 'AWS::Cognito::UserPoolDomain', + Properties: { Domain: 'gen2-domain' }, + } as const; + + it('validate() returns undefined (check moved to execute-time)', async () => { + const templateWithoutRetain: CFNTemplate = { + AWSTemplateFormatVersion: '2010-09-09', + Description: 'gen2 auth', + Resources: { amplifyAuthGoogleIdP: idpResource, amplifyAuthUserPoolDomain: domainResource }, + Outputs: {}, + }; + cfnMock.on(GetTemplateCommand).resolves({ TemplateBody: JSON.stringify(templateWithoutRetain) }); + + const op = await createRefactorer().testBuildOrphanOp('gen2-auth'); + expect(op).toBeDefined(); + expect(op!.validate()).toBeUndefined(); + }); + + it('execute() throws AmplifyError when any target is missing DeletionPolicy: Retain', async () => { + const templateMissingRetain: CFNTemplate = { + AWSTemplateFormatVersion: '2010-09-09', + Description: 'gen2 auth', + Resources: { + amplifyAuthGoogleIdP: idpResource, + amplifyAuthUserPoolDomain: { ...domainResource, DeletionPolicy: 'Retain' }, + }, + Outputs: {}, + }; + cfnMock.on(GetTemplateCommand).resolves({ TemplateBody: JSON.stringify(templateMissingRetain) }); + cfnMock.on(DescribeStacksCommand).resolves({ + Stacks: [{ StackName: 'gen2-auth', StackStatus: rs, CreationTime: ts, Parameters: [] }], + }); + cfnMock.on(UpdateStackCommand).resolves({}); + + const op = await createRefactorer().testBuildOrphanOp('gen2-auth'); + expect(op).toBeDefined(); + await expect(op!.execute()).rejects.toThrow(/DeletionPolicy: Retain/); + expect(cfnMock.commandCalls(UpdateStackCommand)).toHaveLength(0); + }); + + it('execute() succeeds when every target has DeletionPolicy: Retain', async () => { + const templateWithRetain: CFNTemplate = { + AWSTemplateFormatVersion: '2010-09-09', + Description: 'gen2 auth', + Resources: { + amplifyAuthGoogleIdP: { ...idpResource, DeletionPolicy: 'Retain' }, + amplifyAuthUserPoolDomain: { ...domainResource, DeletionPolicy: 'Retain' }, + }, + Outputs: {}, + }; + cfnMock.on(GetTemplateCommand).resolves({ TemplateBody: JSON.stringify(templateWithRetain) }); + cfnMock.on(DescribeStacksCommand).resolves({ + Stacks: [{ StackName: 'gen2-auth', StackStatus: rs, CreationTime: ts, Parameters: [] }], + }); + cfnMock.on(UpdateStackCommand).resolves({}); + + const op = await createRefactorer().testBuildOrphanOp('gen2-auth'); + expect(op).toBeDefined(); + await expect(op!.execute()).resolves.toBeUndefined(); + expect(cfnMock.commandCalls(UpdateStackCommand)).toHaveLength(1); + }); +}); diff --git a/packages/amplify-cli/src/__tests__/commands/gen2-migration/refactor/resolvers/cfn-parameter-resolver.test.ts b/packages/amplify-cli/src/__tests__/commands/gen2-migration/refactor/resolvers/cfn-parameter-resolver.test.ts index e1f14412861..c2d45ffc3ff 100644 --- a/packages/amplify-cli/src/__tests__/commands/gen2-migration/refactor/resolvers/cfn-parameter-resolver.test.ts +++ b/packages/amplify-cli/src/__tests__/commands/gen2-migration/refactor/resolvers/cfn-parameter-resolver.test.ts @@ -1,4 +1,7 @@ -import { resolveParameters } from '../../../../../commands/gen2-migration/refactor/resolvers/cfn-parameter-resolver'; +import { + resolveNoEchoParameters, + resolveParameters, +} from '../../../../../commands/gen2-migration/refactor/resolvers/cfn-parameter-resolver'; import { CFNTemplate } from '../../../../../commands/gen2-migration/_infra/cfn-template'; const baseTemplate: CFNTemplate = { @@ -105,3 +108,43 @@ describe('resolveParameters', () => { expect(result.Resources.R.Properties.Zones).toEqual(['single']); }); }); + +describe('resolveNoEchoParameters', () => { + it('transforms NoEcho parameter to UsePreviousValue', () => { + const result = resolveNoEchoParameters(baseTemplate, [{ ParameterKey: 'SecretParam', ParameterValue: '****' }]); + expect(result).toEqual([{ ParameterKey: 'SecretParam', UsePreviousValue: true }]); + expect(result[0]).not.toHaveProperty('ParameterValue'); + }); + + it('passes non-NoEcho parameters through unchanged', () => { + const input = [{ ParameterKey: 'BucketNameParam', ParameterValue: 'my-bucket' }]; + const result = resolveNoEchoParameters(baseTemplate, input); + expect(result).toEqual(input); + }); + + it('handles mixed NoEcho and regular parameters', () => { + const input = [ + { ParameterKey: 'BucketNameParam', ParameterValue: 'my-bucket' }, + { ParameterKey: 'SecretParam', ParameterValue: '****' }, + { ParameterKey: 'ListParam', ParameterValue: 'a,b,c' }, + ]; + const result = resolveNoEchoParameters(baseTemplate, input); + expect(result).toEqual([ + { ParameterKey: 'BucketNameParam', ParameterValue: 'my-bucket' }, + { ParameterKey: 'SecretParam', UsePreviousValue: true }, + { ParameterKey: 'ListParam', ParameterValue: 'a,b,c' }, + ]); + }); + + it('leaves parameters without a ParameterKey alone', () => { + const input = [{ ParameterValue: 'orphan' }] as any[]; + const result = resolveNoEchoParameters(baseTemplate, input); + expect(result).toEqual(input); + }); + + it('passes through parameters with no matching template declaration', () => { + const input = [{ ParameterKey: 'UnknownParam', ParameterValue: 'value' }]; + const result = resolveNoEchoParameters(baseTemplate, input); + expect(result).toEqual(input); + }); +}); diff --git a/packages/amplify-cli/src/__tests__/commands/gen2-migration/refactor/workflow/category-refactorer.test.ts b/packages/amplify-cli/src/__tests__/commands/gen2-migration/refactor/workflow/category-refactorer.test.ts index 4bc01012d2a..343de3b7599 100644 --- a/packages/amplify-cli/src/__tests__/commands/gen2-migration/refactor/workflow/category-refactorer.test.ts +++ b/packages/amplify-cli/src/__tests__/commands/gen2-migration/refactor/workflow/category-refactorer.test.ts @@ -418,13 +418,27 @@ describe('stack status validation', () => { expect(result.report).toMatch(/gen1-storage-stack.*UPDATE_IN_PROGRESS/); }); - it('reports failure when destination stack is in UPDATE_ROLLBACK_COMPLETE', async () => { + it('passes when destination stack is in UPDATE_ROLLBACK_COMPLETE', async () => { + // UPDATE_ROLLBACK_COMPLETE is a terminal CFN state from which new updates are permitted. + // The per-category check accepts it for parity with the root-level validateDeploymentStatus + // (a prior failed update should not block retrying with a fixed template). setupWithStatuses(StackStatus.CREATE_COMPLETE, 'UPDATE_ROLLBACK_COMPLETE' as StackStatus); const ops = await createRefactorer().plan(); const validation = ops[1].validate(); const result = await validation!.run(); + expect(result.valid).toBe(true); + expect(result.report).toBeUndefined(); + }); + + it('reports failure when destination stack is in ROLLBACK_COMPLETE (non-updatable)', async () => { + setupWithStatuses(StackStatus.CREATE_COMPLETE, 'ROLLBACK_COMPLETE' as StackStatus); + const ops = await createRefactorer().plan(); + const validation = ops[1].validate(); + const result = await validation!.run(); expect(result.valid).toBe(false); - expect(result.report).toMatch(/gen2-storage-stack.*UPDATE_ROLLBACK_COMPLETE/); + expect(result.report).toMatch(/gen2-storage-stack.*ROLLBACK_COMPLETE/); + // Report should mention all three accepted terminal states. + expect(result.report).toContain('UPDATE_ROLLBACK_COMPLETE'); }); it('passes when both stacks are in CREATE_COMPLETE', async () => { @@ -455,3 +469,141 @@ describe('placeholder constants', () => { expect(MIGRATION_PLACEHOLDER_LOGICAL_ID).toBe('MigrationPlaceholder'); }); }); + +import { ExpectedChange } from '../../../../../commands/gen2-migration/refactor/workflow/category-refactorer'; + +describe('CategoryRefactorer.updateTarget validate — expected-change allowlist', () => { + let cfnMock: ReturnType; + beforeEach(() => { + cfnMock = mockClient(CloudFormationClient); + cfnMock.on(CreateChangeSetCommand).resolves({}); + cfnMock.on(DeleteChangeSetCommand).resolves({}); + cfnMock.on(DescribeStacksCommand).resolves({ Stacks: [] }); + }); + afterEach(() => cfnMock.restore()); + + /** + * Returns the updateTarget operation by running plan() against a storage + * setup where the target changeset is configured via the provided + * DescribeChangeSet mock. Uses a custom subclass that overrides + * expectedTargetChanges() so we can exercise the allowlist behavior + * without depending on auth-specific logic. + */ + async function runWithChangeSet(opts: { + changeSet: any; + allowlist: ExpectedChange[]; + }) { + setupStorageMocks(cfnMock); + // updateSource = source stack describe call: return an empty changes array. + cfnMock.on(DescribeChangeSetCommand).callsFake((input) => { + if (input.StackName === 'gen2-storage-stack') { + return { Status: 'CREATE_COMPLETE', StackName: input.StackName, Changes: [], ...opts.changeSet }; + } + return { Status: 'CREATE_COMPLETE', StackName: input.StackName, Changes: [] }; + }); + + const { gen1Env, gen2Branch, cfn, gen1App } = makeInstances(); + const Subclass = class extends StorageS3ForwardRefactorer { + protected override expectedTargetChanges(): ExpectedChange[] { + return opts.allowlist; + } + }; + const ops = await new Subclass( + gen1Env, + gen2Branch, + gen1App, + '123', + noOpLogger(), + { category: 'storage', resourceName: 'avatars', service: 'S3', key: 'storage:S3' as const }, + cfn, + ).plan(); + + // Op order in plan(): [sourceStatus, destStatus, updateSource, updateTarget, ...] + const updateTargetOp = ops[3]; + expect(updateTargetOp).toBeDefined(); + return updateTargetOp; + } + + it('passes when changeset is empty (no changes detected)', async () => { + const op = await runWithChangeSet({ + changeSet: { Changes: [] }, + allowlist: [], + }); + const result = await op.validate()!.run(); + expect(result.valid).toBe(true); + expect(result.report).toBeUndefined(); + }); + + it('passes when all diffs are on the expected allowlist', async () => { + const op = await runWithChangeSet({ + changeSet: { + Changes: [ + { + ResourceChange: { + LogicalResourceId: 'amplifyStorageBucket12345678', + Action: 'Modify', + Details: [ + { + Target: { + Attribute: 'Properties', + Name: 'Policy', + Path: '/Policy/Foo', + BeforeValue: 'X', + AfterValue: 'PLACEHOLDER-value', + }, + }, + ], + }, + }, + ], + }, + allowlist: [ + { + logicalId: 'amplifyStorageBucket12345678', + propertyPathPrefix: '/Policy', + expectedAfterValueSubstring: 'PLACEHOLDER', + }, + ], + }); + const result = await op.validate()!.run(); + expect(result.valid).toBe(true); + }); + + it('fails when an unexpected diff exists outside the allowlist', async () => { + const op = await runWithChangeSet({ + changeSet: { + Changes: [ + { + ResourceChange: { + LogicalResourceId: 'amplifyStorageBucket12345678', + Action: 'Modify', + Details: [ + { + Target: { + Attribute: 'Properties', + Name: 'BucketName', + Path: '/BucketName', + BeforeValue: 'old', + AfterValue: 'new', + }, + }, + ], + }, + }, + ], + }, + allowlist: [ + { + logicalId: 'amplifyStorageBucket12345678', + propertyPathPrefix: '/Policy', + expectedAfterValueSubstring: 'PLACEHOLDER', + }, + ], + }); + const result = await op.validate()!.run(); + expect(result.valid).toBe(false); + expect(result.report).toBeDefined(); + expect(result.report).toContain('amplifyStorageBucket12345678'); + expect(result.report).toContain('/BucketName'); + }); +}); diff --git a/packages/amplify-cli/src/__tests__/commands/gen2-migration/refactor/workflow/rollback-category-refactorer.test.ts b/packages/amplify-cli/src/__tests__/commands/gen2-migration/refactor/workflow/rollback-category-refactorer.test.ts index e0d9a53389f..f0ac172474b 100644 --- a/packages/amplify-cli/src/__tests__/commands/gen2-migration/refactor/workflow/rollback-category-refactorer.test.ts +++ b/packages/amplify-cli/src/__tests__/commands/gen2-migration/refactor/workflow/rollback-category-refactorer.test.ts @@ -185,9 +185,9 @@ describe('RollbackCategoryRefactorer.buildResourceMappings (gen1LogicalIds-based it('throws for resource with no known Gen1 logical ID', async () => { const refactorer = new TestRollbackMappingRefactorer(new Map()); - await expect(refactorer.testBuildResourceMappings(new Map([['amplifyTopic', r('AWS::SNS::Topic')]]), new Map())).rejects.toThrow( - 'Unable to determine target id of resource amplifyTopic', - ); + await expect( + refactorer.testBuildResourceMappings(new Map([['amplifyTopic', r('AWS::SNS::Topic')]]), new Map()), + ).rejects.toThrow('Unable to determine target id of resource amplifyTopic'); }); it('skips resources that already exist in target stack', async () => { From 48e22472484f9bfcfb2cb0fd9bf9560ac04a59bb Mon Sep 17 00:00:00 2001 From: 9pace Date: Thu, 30 Apr 2026 14:41:22 -0400 Subject: [PATCH 9/9] docs: use placeholder values in ADR examples --- ...efactor-social-auth-idp-domain-handling.md | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/packages/amplify-cli/adr/005-refactor-social-auth-idp-domain-handling.md b/packages/amplify-cli/adr/005-refactor-social-auth-idp-domain-handling.md index 249861c3d38..7ebffbbdc9a 100644 --- a/packages/amplify-cli/adr/005-refactor-social-auth-idp-domain-handling.md +++ b/packages/amplify-cli/adr/005-refactor-social-auth-idp-domain-handling.md @@ -144,7 +144,7 @@ or `AWS::Cognito::UserPoolDomain`. They're physically present on the pool but ### Case 2 failure (override approach) Experimentally verified using `cfn-import-test/override-test.mjs` against UserPool -`us-east-1_9UB4KPsWg` (app `mediavault-import-test`, ID `d1igd1gdzabina`): +`us-east-1_TESTPOOL` (app `mediavault-import-test`, ID `d1exampleappid`): | Resource | CFN CREATE Result | |----------|-------------------| @@ -193,7 +193,7 @@ a clean state where subsequent deploys see zero drift. | Override failure script | `oauth-workspace/cfn-import-test/override-test.mjs` | | Full test results | `oauth-workspace/cfn-import-test/full-context.md` | | Summary | `oauth-workspace/cfn-import-test/summary.md` | -| Gen1 test app | `oauth-workspace/mediavault-import-test/` (App ID `d1igd1gdzabina`, UserPool `us-east-1_9UB4KPsWg`) | +| Gen1 test app | `oauth-workspace/mediavault-import-test/` (App ID `d1exampleappid`, UserPool `us-east-1_TESTPOOL`) | ## Decision @@ -238,8 +238,8 @@ social-auth apps are completely unaffected. | Resource Type | Identifier Keys | Example Values | |---------------|-----------------|----------------| -| `AWS::Cognito::UserPoolDomain` | `{UserPoolId, Domain}` | `us-east-1_sL46vhIrI`, `mediavaultfresh-devenv` | -| `AWS::Cognito::UserPoolIdentityProvider` | `{UserPoolId, ProviderName}` | `us-east-1_sL46vhIrI`, `Google` | +| `AWS::Cognito::UserPoolDomain` | `{UserPoolId, Domain}` | `us-east-1_EXAMPLE`, `myapp-devenv` | +| `AWS::Cognito::UserPoolIdentityProvider` | `{UserPoolId, ProviderName}` | `us-east-1_EXAMPLE`, `Google` | **Conceptual CFN API call:** @@ -253,17 +253,17 @@ await cfn.send(new CreateChangeSetCommand({ { ResourceType: 'AWS::Cognito::UserPoolDomain', LogicalResourceId: 'amplifyAuthUserPoolUserPoolDomain1F688B5B', - ResourceIdentifier: { UserPoolId: 'us-east-1_sL46vhIrI', Domain: 'mediavaultfresh-devenv' }, + ResourceIdentifier: { UserPoolId: 'us-east-1_EXAMPLE', Domain: 'myapp-devenv' }, }, { ResourceType: 'AWS::Cognito::UserPoolIdentityProvider', LogicalResourceId: 'amplifyAuthGoogleIdPA9736819', - ResourceIdentifier: { UserPoolId: 'us-east-1_sL46vhIrI', ProviderName: 'Google' }, + ResourceIdentifier: { UserPoolId: 'us-east-1_EXAMPLE', ProviderName: 'Google' }, }, { ResourceType: 'AWS::Cognito::UserPoolIdentityProvider', LogicalResourceId: 'amplifyAuthFacebookIDP7CB5B5CC', - ResourceIdentifier: { UserPoolId: 'us-east-1_sL46vhIrI', ProviderName: 'Facebook' }, + ResourceIdentifier: { UserPoolId: 'us-east-1_EXAMPLE', ProviderName: 'Facebook' }, }, ], })); @@ -298,8 +298,8 @@ Two layers of protection: ### Domain handling -The Gen1 domain prefix (e.g., `mediavaultfresh-devenv`) differs from Gen2's -auto-generated prefix (e.g., `8f20214d49ee8819a1c1`). The import uses the Gen1 domain +The Gen1 domain prefix (e.g., `myapp-devenv`) differs from Gen2's +auto-generated prefix (e.g., `a1b2c3d4e5f6g7h8i9j0`). The import uses the Gen1 domain value. On the next Gen2 deploy, the domain is a replacement property — if the generated code specifies a different prefix, CloudFormation would delete and recreate the domain, which takes the hosted UI down and changes the URL. @@ -312,7 +312,7 @@ import { CfnUserPoolDomain } from 'aws-cdk-lib/aws-cognito'; const cfnUserPoolDomain = backend.auth.resources.userPool.node .findChild('UserPoolDomain') .node.defaultChild as CfnUserPoolDomain; -cfnUserPoolDomain.domain = 'mediavaultfresh-devenv'; // Gen1 domain prefix +cfnUserPoolDomain.domain = 'myapp-devenv'; // Gen1 domain prefix ``` Implementation note: `auth.generator.ts` `contributeProviderSetup()` already emits a