fix(cli): gen2-migration inject required standard attrs into social IdP mappings - #14983
Open
sharonyajain wants to merge 1 commit into
Open
sharonyajain wants to merge 1 commit into
sharonyajain wants to merge 1 commit into
Conversation
…dP mappings
A Gen1 user pool can carry phone_number as a username attribute (Cognito
treats a username attribute as implicitly required) while its social IdP
attribute mappings only cover email. Cognito rejects any federated IdP whose
attributeMapping omits a required standard attribute ("The attribute mapping
is missing required attributes [phone_number]"), so the generated Gen2 app
fails to deploy and CloudFormation rolls the auth nested stack back.
The auth renderer now derives the pool's required standard attributes
(UsernameAttributes union AliasAttributes, filtered to standard attributes it
knows how to map) and injects each into every social provider's attribute
mapping as an identity mapping when the Gen1 metadata did not already provide
one. This preserves Gen1 login behavior (phone_number stays a username
attribute) and removes no capability.
Regenerated the media-vault migration-app snapshot to reflect the injected
mappings, and added a regression test that fails without the fix.
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.
Description of changes
When
gen2-migration generatemigrates a Gen1 app whose Cognito user pool hasphone_numberas a username attribute and also uses social sign-in (Google/Facebook), the generated Gen2 app fails to deploy:Cognito treats a username attribute as implicitly required, and then requires every federated identity provider's
attributeMappingto include that attribute. The migration renderer copiesphone_numberforward as a username attribute but generates the social IdPattributeMappingfrom the Gen1 provider metadata only (which typically maps justemail+ a custom username claim), sophone_numberis absent and CloudFormation rolls the auth nested stack back. This surfaced as thel_gen2_migration_media_vaultE2E shard failing on every deploy retry.Approach → change: the auth renderer now derives the pool's required standard attributes (
UsernameAttributes∪AliasAttributes, filtered to standard attributes it knows how to map viaMAPPED_USER_ATTRIBUTE_NAME) and injects each into every social provider'sattributeMappingas an identity mapping (e.g.phoneNumber: 'phone_number') only when the Gen1 metadata did not already provide one. This is a general rule derived from the pool config, applied to all providers — not a per-app or per-provider special case. It preserves Gen1 login behavior (phone_numberstays a username attribute) and removes no capability; it only adds the mapping Cognito already requires.This is independent of the store-locator circular-dependency fix in #14981 — it is a separate, pre-existing migration-codegen gap for the required-standard-attribute + social-IdP combination.
no linked issue: surfaced via the media-vault E2E shard, not a filed GitHub issue
Issue #, if available
N/A
Description of how you validated changes
auth.generator.test.ts) that generates from a pool withUsernameAttributes: ['email', 'phone_number']+ Google/Facebook IdPs mapping only email; it assertsphoneNumber: 'phone_number'is injected into both provider mappings, the Gen1 custom username claims are preserved, andphone_numberremains a pool username attribute. Verified the test fails without the fix (Expected 2, Received 0) and passes with it.gen2-migration/generatesuite green: 23 suites / 260 tests / 103 snapshots, including the gatedmedia-vault snapshotcomparison (regenerated_snapshot.post.generate/amplify/auth/resource.ts).tsc --noEmitclean, prettier clean, eslint 0 errors.Checklist
yarn testpassesBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.