Skip to content
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

Sign in with Apple - add and update auth #7265

Merged
merged 13 commits into from
May 12, 2021
Merged

Sign in with Apple - add and update auth #7265

merged 13 commits into from
May 12, 2021

Conversation

letsbelopez
Copy link
Contributor

@letsbelopez letsbelopez commented May 5, 2021

Description of changes

Updated the --headless add and update auth flow to take Sign in with Apple in the headless payload and update the Cognito user pool.

Issue #, if available

Description of how you validated changes

I use a test web project and passed in the new SIWA headless payloads with the add and update commands:
cat MyAuthTemplate.addauth.json | jq -c '.' | amplify add auth --headless

I verified the changes in Cognito console.

Checklist

  • PR description included
  • yarn test passes
  • Tests are changed or added
  • Relevant documentation is changed or added (and PR referenced) The relevant headless documentation points to the headless schema package, which is updated in the PR.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@codecov-commenter
Copy link

codecov-commenter commented May 5, 2021

Codecov Report

Merging #7265 (68fe947) into master (d28dd1c) will decrease coverage by 0.07%.
The diff coverage is 1.92%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #7265      +/-   ##
==========================================
- Coverage   52.32%   52.24%   -0.08%     
==========================================
  Files         499      499              
  Lines       25546    25585      +39     
  Branches     5006     5014       +8     
==========================================
  Hits        13367    13367              
- Misses      11230    11261      +31     
- Partials      949      957       +8     
Impacted Files Coverage Δ
...ider-utils/awscloudformation/assets/string-maps.js 85.71% <ø> (ø)
.../src/provider-utils/awscloudformation/constants.ts 100.00% <ø> (ø)
...auth/src/provider-utils/awscloudformation/index.js 4.34% <0.00%> (-0.40%) ⬇️
...udformation/service-walkthroughs/auth-questions.js 3.37% <0.00%> (-0.11%) ⬇️
...s/awscloudformation/utils/auth-request-adaptors.ts 40.95% <0.00%> (-2.05%) ⬇️
...gory-auth/src/provider-utils/supported-services.ts 100.00% <ø> (ø)
...c/provider-utils/awscloudformation/import/index.ts 5.01% <11.11%> (-0.08%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d28dd1c...68fe947. Read the comment docs.

@letsbelopez letsbelopez marked this pull request as ready for review May 5, 2021 22:13
@letsbelopez letsbelopez requested a review from a team as a code owner May 5, 2021 22:13
Copy link
Contributor

@edwardfoyle edwardfoyle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One small nit, but LGTM

Copy link
Contributor

@SwaySway SwaySway left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code lgtm - can you also add some tests to verify the add auth with siwa and headless pull with siwa works as expected?

@lgtm-com
Copy link

lgtm-com bot commented May 9, 2021

This pull request introduces 1 alert when merging d522c14 into 5c918be - view on LGTM.com

new alerts:

  • 1 for Incomplete URL substring sanitization

Co-authored-by: Edward Foyle <foyleef@amazon.com>
@lgtm-com
Copy link

lgtm-com bot commented May 10, 2021

This pull request introduces 1 alert when merging bbb18ad into 5c918be - view on LGTM.com

new alerts:

  • 1 for Incomplete URL substring sanitization

@lgtm-com
Copy link

lgtm-com bot commented May 10, 2021

This pull request introduces 1 alert when merging 08eeba9 into d28dd1c - view on LGTM.com

new alerts:

  • 1 for Incomplete URL substring sanitization

Copy link
Contributor

@SwaySway SwaySway left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@lgtm-com
Copy link

lgtm-com bot commented May 11, 2021

This pull request introduces 1 alert when merging 68fe947 into d28dd1c - view on LGTM.com

new alerts:

  • 1 for Incomplete URL substring sanitization

@letsbelopez letsbelopez changed the title Sign in with Apple - add and update auth headless Sign in with Apple - add and update auth May 11, 2021
@lgtm-com
Copy link

lgtm-com bot commented May 11, 2021

This pull request introduces 1 alert when merging 2a58958 into d28dd1c - view on LGTM.com

new alerts:

  • 1 for Incomplete URL substring sanitization

@letsbelopez letsbelopez requested a review from ammarkarachi May 11, 2021 22:16
@ammarkarachi ammarkarachi merged commit 9f5e659 into aws-amplify:master May 12, 2021
@github-actions
Copy link

👋 Hi, this pull request was referenced in the v4.51.0 release!

Check out the release notes here https://github.com/aws-amplify/amplify-cli/releases/tag/v4.51.0.

@github-actions github-actions bot added the referenced-in-release Issues referenced in a published release changelog label May 14, 2021
ammarkarachi pushed a commit to ammarkarachi/amplify-cli that referenced this pull request May 18, 2021
SwaySway pushed a commit that referenced this pull request May 18, 2021
* Revert "feat: Support for Apple Sign In (#7265)"
cjihrig pushed a commit to ctjlewis/amplify-cli that referenced this pull request Jul 12, 2021
cjihrig pushed a commit to ctjlewis/amplify-cli that referenced this pull request Jul 12, 2021
* Revert "feat: Support for Apple Sign In (aws-amplify#7265)"
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
referenced-in-release Issues referenced in a published release changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants