fix(@aws-amplify/core): AWS.config.systemClockOffset for signing requ… #4251

thiagohirata · 2019-10-24T19:36:11Z

…ests

Issue #, if available:
#2014 #3719 #3699

Description of changes:
Signer calls AWS.util.date.getDate() instead of new Date() - making it possible to correct the signature and headers for the correct date when the system time is wrong by:

import { AWS } from '@aws-amplify/core'
AWS.config = new AWS.Config({
    systemClockOffset: CLOCK_OFFSET
})

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

…ests

codecov · 2019-10-28T14:57:53Z

Codecov Report

Merging #4251 into master will not change coverage.
The diff coverage is 100%.

@@           Coverage Diff           @@
##           master    #4251   +/-   ##
=======================================
  Coverage   78.72%   78.72%           
=======================================
  Files         165      165           
  Lines        9032     9032           
  Branches     1872     1820   -52     
=======================================
  Hits         7110     7110           
- Misses       1783     1789    +6     
+ Partials      139      133    -6

Impacted Files	Coverage Δ
packages/core/src/Signer.ts	`93.04% <100%> (ø)`	⬆️
packages/core/src/OAuthHelper/GoogleOAuth.ts	`31.25% <0%> (ø)`	⬆️
...ackages/pubsub/src/Providers/AWSAppSyncProvider.ts	`66.66% <0%> (ø)`	⬆️
...rc/Providers/AmazonAIConvertPredictionsProvider.ts	`61.23% <0%> (ø)`	⬆️
packages/xr/src/Providers/SumerianProvider.ts	`47.55% <0%> (ø)`	⬆️
packages/core/src/OAuthHelper/FacebookOAuth.ts	`34.09% <0%> (ø)`	⬆️
packages/auth/src/OAuth/OAuth.ts	`48.48% <0%> (ø)`	⬆️

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 e61d906...b3f68e3. Read the comment docs.

thiagohirata · 2019-11-14T22:51:19Z

Hi, @Amplifiyer!
Any chance checking this out?

Amplifiyer · 2019-11-15T18:05:46Z

Hi, @Amplifiyer!
Any chance checking this out?

@thiagohirata thanks for submitting the PR. I have a few questions about the implementation and the intended usage.

How are customers going to calculate CLOCK_OFFSET and pass it to AWS.Config?
Would it be better to calculate this offset in the signer itself? Such as by reading the data header in the response and calculating the clock drift and setting it again on the retry?

We are also looking to migrate towards aws-sdk V3 which is in preview right now. V3 version doesn't export a global AWS variable or currently has any ways to provide a clockSkew value. This change will then need to be migrated as well when we migrate to V3 of aws-sdk.

thiagohirata · 2019-11-21T17:16:58Z

Hi, @Amplifiyer

I am not sure if the current version of aws-sdk provides an automatic way to recalculate itself the clock skew. I will check.

Making this process automatic sounds good, but I was expecting that amplify-js would follow the same behaviour as aws-sdk.

This is a current issue here in Brazil - we had in two consecutive years changes in daylight saving time rules (the current rule is that there is no daylight saving time), and there is a bunch of devices with outdated databases. With users manually adjusting the time, but not changing the internal timezone offset, all signatures are automatically expired.

I made a webservice myself to calculate the skew of the client. The requests made with aws-sdk work when I provide this option, but there is no way to make amplify-js work.

Thanks!

dorsegal · 2019-12-25T15:13:03Z

@Amplifiyer
We applied the fix manually and it worked great.
Any chance this merge will happen?

dorsegal · 2019-12-25T16:53:17Z

We found a way to calculate clock offset using aws amplify:

Auth.currentAuthenticatedUser({
      bypassCache: true // this will get the latest clockDrift
    }).then(user => {
        AWS.config = new AWS.Config({
          systemClockOffset: -(user.signInUserSession.clockDrift * 1000)
      })
}

thiagohirata · 2019-12-30T19:07:33Z

Hi @Amplifiyer!

@thiagohirata thanks for submitting the PR. I have a few questions about the implementation and the intended usage.

How are customers going to calculate CLOCK_OFFSET and pass it to AWS.Config?

I created a webservice that returns the server timestamp, and the client compares it with the browser's date. @dorsegal provided another way.

Would it be better to calculate this offset in the signer itself? Such as by reading the data header in the response and calculating the clock drift and setting it again on the retry?

Maybe, but I personally think that it is better to give more alternatives to the user.

We are also looking to migrate towards aws-sdk V3 which is in preview right now. V3 version doesn't export a global AWS variable or currently has any ways to provide a clockSkew value. This change will then need to be migrated as well when we migrate to V3 of aws-sdk.

Yes, but I don't think this is a blocking reason for not merging this PR, as this solves a problem with current aws-sdk verson - don't you agree?

houmark · 2020-01-27T05:11:10Z

Dear AWS team,

Instead of searching for the perfect long term solution and consider eight different options, when you cannot come up with a solution that solves this issue better than the already ready PR, please respect your customers and their valuable time, by merging the already ready PR. Then you can always improve on the solution later when you come up with a better solution long term.

This issue is potentially affecting thousands of users in the wild and developers using Amplify can spend hours individually searching for why a rare bug like this happens, while you could solve this with a click of a button. There's nothing complex about this PR in any way, so the risk of regression is about non-existent.

LGTM!!!

computationalcore · 2020-02-04T02:47:58Z

@houmark is absolutely correct. In my case, it is literally affecting thousands of users in a published application. Please, merge it and release ASAP.

Amplifiyer · 2020-02-04T04:50:50Z

I'll look into this PR tomorrow and update.

computationalcore · 2020-02-04T13:42:30Z

I'll look into this PR tomorrow and update.

Ok, thanks @Amplifiyer

Jun711 · 2020-02-04T19:21:55Z

This potentially affects a lot of AWS customers who use AWS_IAM for API authorization.
Some paid users may report to us but new users may just leave our website because the website is not working as expected.

Jun711 · 2020-02-04T21:11:49Z

@Amplifiyer @thiagohirata
I wonder if you could explain what the possible cause of this case. We have asked the user to turn on set time automatically on a windows machine but there is still drift issue. Maybe set time automatically doesn't set time correctly immediately?

Error message is

Signature expired: 20200204T203226Z is now earlier than 
20200204T203234Z (20200204T203734Z - 5 min.) .

in har file

Request
"startedDateTime": "2020-02-04T20:32:26.801Z",
"time": 134.58600000012666,

header

"name": "x-amz-date",
"value": "20200204T203226Z"

Response

 "name": "date",
 "value": "Tue, 04 Feb 2020 20:37:34 GMT"

Amplifiyer · 2020-02-05T23:33:54Z

We discussed internally how to ensure that future changes (integrating with aws-sdk-js-v3) doesn't break this, we are making small changes(#4844) to this PR to abstract out the AWS global variable and exposing a util to handle the clock skew.

We are working on releasing this as soon as possible.

ramanasak · 2020-04-02T09:32:41Z

Hi, @Amplifiyer

How to give a customized header to Sign-in screen, I have used signUpConfig for sign up screen its working fine but for the sign-in screen, it's not working,

const signUpConfig = {
//header: 'My Customized Sign Up',
header: ,
hideAllDefaults: true,
defaultCountryCode: '1',
Label:{color:'red'},

};
const SigninConfig = {
//header: 'My Customized Sign Up',
header: ,
hideAllDefaults: true,
defaultCountryCode: '1',
Label:{color:'red'},
};

export default withAuthenticator((connect(mapStateToProps, mapDispatchToProps)(App)),{ signUpConfig },true,[],null,MyTheme);

I am stuck here please help thanks in advance.

MedElba · 2021-06-08T21:33:36Z

Hi everyone!

I'm using amplify on a React Native application and iOS gives sometimes this 403 errors, this cannot even be resolved by a Signout and Signin! I've seen you wrote about the clockdrift problem and that you merged the fix! I'm using the latest version but it is still present! Any idea on how to solve?
Really urgent for a in-production iPhone Application.

@dorsegal Is it still the same issue?

Thank you very much!

github-actions · 2022-06-09T02:12:44Z

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.

fix(@aws-amplify/core): AWS.config.systemClockOffset for signing requ…

2f8333b

…ests

thiagohirata force-pushed the fix/core/#2014 branch from 0933c5a to 2f8333b Compare October 28, 2019 14:48

sammartinez requested a review from Amplifiyer November 1, 2019 19:20

Merge branch 'master' into fix/core/aws-amplify#2014

b3f68e3

thiagohirata force-pushed the fix/core/#2014 branch from 99f2e62 to b3f68e3 Compare November 5, 2019 19:06

dorsegal approved these changes Dec 25, 2019

View reviewed changes

dorsegal mentioned this pull request Dec 26, 2019

correctClockSkew for API #2014

Closed

ericclemmons added this to the Auth Clockdrift milestone Feb 4, 2020

houmark mentioned this pull request Feb 4, 2020

API.get Sometimes gets 403. #3567

Closed

ericclemmons mentioned this pull request Feb 5, 2020

fix(@aws-amplify/core): Support clock skew #4844

Merged

ericclemmons closed this in #4844 Feb 6, 2020

github-actions bot locked as resolved and limited conversation to collaborators Jun 9, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(@aws-amplify/core): AWS.config.systemClockOffset for signing requ… #4251

fix(@aws-amplify/core): AWS.config.systemClockOffset for signing requ… #4251

thiagohirata commented Oct 24, 2019 •

edited

Loading

codecov bot commented Oct 28, 2019 •

edited

Loading

thiagohirata commented Nov 14, 2019

Amplifiyer commented Nov 15, 2019

thiagohirata commented Nov 21, 2019

dorsegal commented Dec 25, 2019 •

edited

Loading

dorsegal commented Dec 25, 2019

thiagohirata commented Dec 30, 2019

houmark commented Jan 27, 2020

computationalcore commented Feb 4, 2020

Amplifiyer commented Feb 4, 2020

computationalcore commented Feb 4, 2020

Jun711 commented Feb 4, 2020 •

edited

Loading

Jun711 commented Feb 4, 2020

Amplifiyer commented Feb 5, 2020

ramanasak commented Apr 2, 2020

MedElba commented Jun 8, 2021

github-actions bot commented Jun 9, 2022

fix(@aws-amplify/core): AWS.config.systemClockOffset for signing requ… #4251

fix(@aws-amplify/core): AWS.config.systemClockOffset for signing requ… #4251

Conversation

thiagohirata commented Oct 24, 2019 • edited Loading

codecov bot commented Oct 28, 2019 • edited Loading

Codecov Report

thiagohirata commented Nov 14, 2019

Amplifiyer commented Nov 15, 2019

thiagohirata commented Nov 21, 2019

dorsegal commented Dec 25, 2019 • edited Loading

dorsegal commented Dec 25, 2019

thiagohirata commented Dec 30, 2019

houmark commented Jan 27, 2020

computationalcore commented Feb 4, 2020

Amplifiyer commented Feb 4, 2020

computationalcore commented Feb 4, 2020

Jun711 commented Feb 4, 2020 • edited Loading

Jun711 commented Feb 4, 2020

Amplifiyer commented Feb 5, 2020

ramanasak commented Apr 2, 2020

MedElba commented Jun 8, 2021

github-actions bot commented Jun 9, 2022

thiagohirata commented Oct 24, 2019 •

edited

Loading

codecov bot commented Oct 28, 2019 •

edited

Loading

dorsegal commented Dec 25, 2019 •

edited

Loading

Jun711 commented Feb 4, 2020 •

edited

Loading