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/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..7ebffbbdc9a --- /dev/null +++ b/packages/amplify-cli/adr/005-refactor-social-auth-idp-domain-handling.md @@ -0,0 +1,510 @@ +# 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_TESTPOOL` (app `mediavault-import-test`, ID `d1exampleappid`): + +| 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 `d1exampleappid`, UserPool `us-east-1_TESTPOOL`) | + +## 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_EXAMPLE`, `myapp-devenv` | +| `AWS::Cognito::UserPoolIdentityProvider` | `{UserPoolId, ProviderName}` | `us-east-1_EXAMPLE`, `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_EXAMPLE', Domain: 'myapp-devenv' }, + }, + { + ResourceType: 'AWS::Cognito::UserPoolIdentityProvider', + LogicalResourceId: 'amplifyAuthGoogleIdPA9736819', + ResourceIdentifier: { UserPoolId: 'us-east-1_EXAMPLE', ProviderName: 'Google' }, + }, + { + ResourceType: 'AWS::Cognito::UserPoolIdentityProvider', + LogicalResourceId: 'amplifyAuthFacebookIDP7CB5B5CC', + ResourceIdentifier: { UserPoolId: 'us-east-1_EXAMPLE', 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., `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. + +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 = 'myapp-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`**: 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) + +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. **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`. + +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. + +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 + +``` +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) +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 + → 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/__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..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,11 +16,30 @@ import { ExecuteChangeSetCommand, DeleteChangeSetCommand, ResourceMapping, + UpdateStackCommand, } 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'; +// 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; @@ -97,7 +116,6 @@ describe('AuthCognitoForwardRefactorer.plan() — operation sequence', () => { beforeEach(() => { cfnMock = mockClient(CloudFormationClient); - mockClient(SSMClient); mockClient(CognitoIdentityProviderClient); }); @@ -126,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'); @@ -134,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: [] }); @@ -180,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' }], }, @@ -190,15 +245,18 @@ 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({}); cfnMock.on(DeleteChangeSetCommand).resolves({}); const cognitoMock = mockClient(CognitoIdentityProviderClient); - cognitoMock.on(DescribeIdentityProviderCommand).resolves({ - IdentityProvider: { ProviderDetails: { client_id: 'google-id', client_secret: 'google-secret' } }, + cognitoMock.on(DescribeUserPoolCommand).resolves({ + UserPool: { Id: 'us-east-1_ABC123', Domain: 'gen1-prefix' }, + }); + cognitoMock.on(ListIdentityProvidersCommand).resolves({ + Providers: [{ ProviderName: 'Google', ProviderType: 'Google' }], }); const clients = new (AwsClients as any)({ region: 'us-east-1' }); @@ -218,23 +276,34 @@ describe('AuthCognitoForwardRefactorer.plan() — operation sequence', () => { const ops = await refactorer.plan(); - expect(cognitoMock.commandCalls(DescribeIdentityProviderCommand)).toHaveLength(1); - 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(); }); @@ -337,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); + }); +}); 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 () => { 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/generate/amplify/auth/auth.generator.ts b/packages/amplify-cli/src/commands/gen2-migration/generate/amplify/auth/auth.generator.ts index 99c7e75bc34..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,10 +101,10 @@ 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(); + 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 @@ -342,16 +355,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 +394,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 +545,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/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), 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({ 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..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 @@ -1,12 +1,17 @@ -import { Output, Parameter } from '@aws-sdk/client-cloudformation'; +import { ResourceToImport } from '@aws-sdk/client-cloudformation'; +import { + CognitoIdentityProviderClient, + DescribeUserPoolCommand, + 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'; - -const HOSTED_PROVIDER_META_PARAMETER_NAME = 'hostedUIProviderMeta'; -const HOSTED_PROVIDER_CREDENTIALS_PARAMETER_NAME = 'hostedUIProviderCreds'; -const USER_POOL_ID_OUTPUT_KEY_NAME = 'UserPoolId'; +import { AmplifyMigrationOperation } from '../../_infra/operation'; +import { extractStackNameFromId } from '../utils'; +import { StackFacade } from '../stack-facade'; +import CLITable from 'cli-table3'; export const GEN1_NATIVE_APP_CLIENT = 'UserPoolClient'; export const GEN1_WEB_CLIENT = 'UserPoolClientWeb'; @@ -19,56 +24,508 @@ 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'; + +/** + * 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]; + +export interface IdpConfig { + readonly providerName: string; + readonly providerType: string; +} + +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, + }, + }); + } -export const RESOURCE_TYPES = [ - USER_POOL_TYPE, - USER_POOL_CLIENT_TYPE, - IDENTITY_POOL_TYPE, - IDENTITY_POOL_ROLE_ATTACHMENT_TYPE, - USER_POOL_DOMAIN_TYPE, -]; + return { resourcesToImport, templateAdditions }; +} /** * Forward refactorer for the auth:Cognito resource. * - * Moves main auth resources from Gen1 to Gen2. + * 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 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 hostedUIProviderCreds parameter. + * 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 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 beforeMove(gen2StackId: string): Promise { + const baseOps = await super.beforeMove(gen2StackId); + + const retainOp = await this.buildRetainSocialAuthOperation(gen2StackId); + const orphanOp = await this.buildOrphanSocialAuthOperation(gen2StackId); + + const extraOps: AmplifyMigrationOperation[] = []; + if (retainOp) extraOps.push(retainOp); + if (orphanOp) extraOps.push(orphanOp); + + return [...baseOps, ...extraOps]; + } + + /** + * 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.targetStackId); + if (importOp) { + return [...baseOps, importOp]; } - 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, - }); + return baseOps; + } - 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`, - }); + /** + * 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 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 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; } - credsParam.ParameterValue = JSON.stringify(oAuthValues); - return parameters; + + 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; + + // 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; + } + + 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: 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 } = 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, + }); + }, + }; } protected override match(sourceId: string, sourceResource: CFNResource, targetId: string, targetResource: CFNResource): boolean { 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/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)); 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..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 @@ -54,6 +56,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 +74,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 +96,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', { 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 }; } /**