fix(cli): fix OAuth provider scopes and attribute mappings in generate#14777
Draft
9pace wants to merge 9 commits intogen2-migrationfrom
Draft
fix(cli): fix OAuth provider scopes and attribute mappings in generate#147779pace wants to merge 9 commits intogen2-migrationfrom
9pace wants to merge 9 commits intogen2-migrationfrom
Conversation
Three bugs in auth.renderer.ts caused generate to produce broken OAuth config for social identity providers (Google, Facebook): 1. Scope field name typo: deriveProviderSpecificScopes() looked for 'authorized_scopes' but Cognito returns 'authorize_scopes'. Scopes were never collected. Added the correct field name to the lookup. 2. Scope mangling: provider scopes were filtered against VALID_SCOPES (Cognito OAuth scopes), which is the wrong namespace. Facebook's 'public_profile' was mapped to 'profile'. Removed the filter — provider scopes are now passed through as-is. 3. Attribute mapping: filterAttributeMapping() dropped keys not in MAPPED_USER_ATTRIBUTE_NAME (e.g. 'username' -> 'sub' for Google, 'username' -> 'id' for Facebook). Changed to route unknown keys into a 'custom' sub-object matching Gen2's AttributeMapping CDK interface. Caveats: - Provider scopes are no longer validated. The old validation was incorrect (wrong namespace), but there is now no validation at all — whatever the provider returns is passed through verbatim. - OIDC/SAML providers flatten standard + custom back into a single Record since their rendering path doesn't support the custom sub-object. If a custom key collides with a mapped standard key, the custom value wins. Verified end-to-end with a test app using Google and Facebook social login. Gen2 deployed with correct scopes and attribute mappings without manual post-generate edits to OAuth config.
1ffface to
66e090b
Compare
Two bugs in cfn-output-resolver.ts caused incorrect template resolution during the refactor step: 1. Phase 1 GetAtt resolution passed already-resolved ARN values through buildArn(), producing nested ARNs (e.g. arn:.../userpool/arn:...). Added a guard to short-circuit when the output value is already an ARN. 2. Phase 2 fallback resolution used PhysicalResourceId for Custom:: resources, but custom resource GetAtt attributes come from the backing Lambda's response Data — not the physical ID. These are now skipped so CloudFormation evaluates them at deploy time.
on oAuth resources during lock Add a new operation to the lock step that sets DeletionPolicy: Retain on HostedUICustomResourceInputs and HostedUIProvidersCustomResourceInputs in the auth nested stack. This prevents social auth custom resources (IDPs and Domain) from being deleted during or after migration. The operation fetches the auth nested stack template, patches the DeletionPolicy for the relevant resources, and updates the stack while preserving existing parameter values.
Gen1 creates the Cognito domain and social IDPs via a Lambda custom resource, so they aren't in the Gen1 CFN template and get left behind when the UserPool is moved to Gen2. After the move, fetch them live from Cognito and import them into the Gen2 stack via CreateChangeSet(IMPORT), matching providers to Gen2 logical IDs by ProviderName. Imported resources are written with DeletionPolicy: Retain so rollback can orphan them safely.
97a0f06 to
67571e1
Compare
Implement the Orphan + Import approach for handling OAuth/social login (Google, Facebook) during gen2-migration refactor. Instead of moving UserPoolDomain and UserPoolIdentityProvider through the holding stack (which breaks due to Fn::GetAtt references to AmplifySecretFetcherResource), orphan them from Gen2 and re-import them as native CFN resources. Forward: beforeMove sets DeletionPolicy Retain then orphans IDPs and domain from Gen2. move() imports Gen1 physical IDPs and domain into Gen2 under Gen2 logical IDs after the core StackRefactor. Rollback: move() orphans imported IDPs and domain from Gen2 after the core Gen2-to-Gen1 StackRefactor. afterMove() re-imports Gen2 original IDPs and domain after restoring P2 from the holding stack. Also fixes Ref resolution in cfn-output-resolver to fall back to PhysicalResourceId for intra-stack references not exposed via stack outputs. This resolves UserPoolClient SupportedIdentityProviders references to IDP logical IDs during the holding stack move. ADR-005 updated with Orphan + Import addendum.
…itution
Generate step now emits addPropertyDeletionOverride('SupportedLoginProviders')
on the IdentityPool, removing the Fn::GetAtt to AmplifySecretFetcherResource
entirely. This property enables direct federation which Amplify does not use —
Gen1 never sets it and social login works via UserPool CognitoIdentityProviders.
Removes the resolveTarget() PLACEHOLDER override, expectedTargetChanges(), and
identityPoolLogicalIds from AuthCognitoForwardRefactorer. Both forward and
rollback updateSource/updateTarget now produce empty changesets for auth.
ADR-005 Addendum updated to reflect this decision.
The resolveTarget override and expectedTargetChanges were removed in the previous commit. Remove the corresponding test describe block.
… validation - auth-cognito-rollback: OAuth rollback plan, targetLogicalId exclusions, orphan Retain check - cfn-parameter-resolver: resolveNoEchoParameters coverage - category-refactorer: UPDATE_ROLLBACK_COMPLETE accepted, expectedTargetChanges allowlist - rollback-category-refactorer: formatting only - Snapshot updates for scopes, attribute mappings, and domain override
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
authorized_scopes→authorize_scopes(Cognito's actual field name)VALID_SCOPESfilter that mangled provider scopes (e.g. Facebook'spublic_profile→profile)username→sub/id) intocustomsub-object instead of dropping themBefore (generated
resource.ts)Scopes missing entirely.
username→sub/idmapping dropped.After (generated
resource.ts)Caveats
customsub-objectTest plan