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(@aws-amplify/core): automatically set clock offset based on CognitoIdentityCredentials systemClockOffset #4977

Closed
wants to merge 2 commits into from

Conversation

Jun711
Copy link

@Jun711 Jun711 commented Feb 25, 2020

Issue #, if available:
fix #4312, #2014, #3719 based on #4844

Description of changes:
Turn on correctClockSkew option provided by AWS.CognitoIdentityCredentials in Amplify Credentials class
Use DateUtils to setClockOffset using systemClockOffset calculated by AWS.CognitoIdentityCredentials
in aws-sdk

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

@codecov
Copy link

codecov bot commented Feb 25, 2020

Codecov Report

Merging #4977 into master will decrease coverage by 0.02%.
The diff coverage is 33.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4977      +/-   ##
==========================================
- Coverage   76.38%   76.35%   -0.03%     
==========================================
  Files         175      175              
  Lines        9668     9674       +6     
  Branches     1924     1926       +2     
==========================================
+ Hits         7385     7387       +2     
- Misses       2144     2148       +4     
  Partials      139      139
Impacted Files Coverage Δ
packages/core/src/Credentials.ts 33.17% <33.33%> (ø) ⬆️

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 a577935...6da6492. Read the comment docs.

@warrenmcquinn
Copy link

Thank you for your work @Jun711 -- I hope this is merged quickly! This is a bug that desperately needs a fix.

@ericclemmons ericclemmons added this to the Auth Clockdrift milestone Mar 19, 2020
@elgordino
Copy link

Is there a schedule for when this might be merged into a release?

@GeorgeBellTMH
Copy link

Any ETA?

@ericclemmons
Copy link
Contributor

Unfortunately, this PR can't be accepted as-is because of Amplify now using https://github.com/aws/aws-sdk-js-v3 (#3365), which automatically accounts for clock drift: aws/aws-sdk-js-v3#459

I'm reproducing various scenarios (e.g. #2014 (comment)) to determine how to resolve this with the latest version of Amplify.

@ericclemmons
Copy link
Contributor

Please see my comment in #2014 (comment). I need to do more validation to resolve the conflicts in this PR updated to match aws-amplify@latest's Credentials.ts.

If you have use-cases and steps to reproduce, I can get this updated to finally resolve it.

@ericclemmons
Copy link
Contributor

Thanks for your PR! We've since updated our libraries and I've pulled your changes forward into #5869.

This will be released shortly!

@github-actions
Copy link

This pull request has been automatically locked since there hasn't been any recent activity after it was closed. Please open a new issue for related bugs.

Looking for a help forum? We recommend joining the Amplify Community Discord server *-help channels or Discussions for those types of questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Occasional 504 and 403(service worker) errors when using Api with Signature V4
6 participants