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

fix(auth): Catch password reset Cognito API error response #11274

Merged
merged 2 commits into from
Jun 26, 2023

Conversation

oschwartz10612
Copy link

@oschwartz10612 oschwartz10612 commented Apr 20, 2023

Description of changes

This is a pretty simple fix I think. Auth.currentAuthenticatedUser does not currently catch if the password is reset in Cognito. It currently will just return the current user even though the Cognito API call returns 400 with the following message.

{"__type":"PasswordResetRequiredException","message":"Password reset required for the user"}

We need to catch this message I believe in the same way that the token revoke is handled.

Description of how you validated changes

Tested in a personal demo application.

Checklist

  • PR description included
  • yarn test passes
  • Tests are changed or added
  • Relevant documentation is changed or added (and PR referenced)

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

Sorry, something went wrong.

@oschwartz10612 oschwartz10612 requested a review from a team as a code owner April 20, 2023 15:51
@cwomack cwomack added bug Something isn't working Auth Related to Auth components/category external-contributor labels Jun 6, 2023
erinleigh90
erinleigh90 previously approved these changes Jun 21, 2023
@erinleigh90 erinleigh90 dismissed their stale review June 21, 2023 22:35

Dismissing my review as I need to replicate and test first - then will add again.

@erinleigh90
Copy link
Contributor

@oschwartz10612 Can you update your branch to reflect the changes in main? Code looks good and checks out locally, so we will merge once the branch is caught up and checks are passing. Thank you!

@oschwartz10612
Copy link
Author

Hi @erinleigh90, thanks for reviewing! I just updated the branch!

@codecov-commenter
Copy link

Codecov Report

Merging #11274 (fc71a45) into main (3e5e6f9) will increase coverage by 0.13%.
The diff coverage is n/a.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@            Coverage Diff             @@
##             main   #11274      +/-   ##
==========================================
+ Coverage   83.23%   83.37%   +0.13%     
==========================================
  Files         274      294      +20     
  Lines       20523    20867     +344     
  Branches     4436     4492      +56     
==========================================
+ Hits        17083    17398     +315     
- Misses       3152     3182      +30     
+ Partials      288      287       -1     

see 56 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Contributor

@erinleigh90 erinleigh90 left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you!

Copy link
Member

@israx israx left a comment

Choose a reason for hiding this comment

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

Hey @oschwartz10612 . Thanks for putting this together.

LGTM!

@erinleigh90 erinleigh90 merged commit 18012b0 into aws-amplify:main Jun 26, 2023
@oschwartz10612 oschwartz10612 deleted the catch-password-reset branch June 26, 2023 17:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Auth Related to Auth components/category bug Something isn't working external-contributor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants