-
Notifications
You must be signed in to change notification settings - Fork 816
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: add sharedId
in externalAuthEnable()
#7315
Conversation
Codecov Report
@@ Coverage Diff @@
## master #7315 +/- ##
=======================================
Coverage 52.63% 52.63%
=======================================
Files 511 511
Lines 25895 25893 -2
Branches 5052 5052
=======================================
- Hits 13630 13629 -1
+ Misses 11292 11291 -1
Partials 973 973
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
This commit updates externalAuthEnable() to include the auth sharedId when creating an auth resource. The sharedId is given the lowest priority in the authProps object so that it will not overwrite any previously existing sharedId. It also uses the sharedId from cognito-defaults for consistency. This change prevents the creation of "snsundefined" roles.
👋 Hi, this pull request was referenced in the v4.51.3 release! Check out the release notes here https://github.com/aws-amplify/amplify-cli/releases/tag/v4.51.3. |
This commit updates externalAuthEnable() to include the auth sharedId when creating an auth resource. The sharedId is given the lowest priority in the authProps object so that it will not overwrite any previously existing sharedId. It also uses the sharedId from cognito-defaults for consistency. This change prevents the creation of "snsundefined" roles. Co-authored-by: Colin Ihrig <colihrig@amazon.com>
Description of changes
This commit updates
externalAuthEnable()
to include the authsharedId
when creating an auth resource. ThesharedId
is given the lowest priority in theauthProps
object so that it will not overwrite any previously existingsharedId
. It also uses thesharedId
fromcognito-defaults
for consistency. This change prevents the creation ofsnsundefined
roles.Issue #, if available
N/A
Description of how you validated changes
Manual testing - create a new project, add a REST API with restricted access
Checklist
yarn test
passesBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.