fix: use gmdate to format x-amz-date with UTC irrespective of timezone#540
Merged
vishwarajanand merged 1 commit intogoogleapis:mainfrom Mar 7, 2024
Merged
Conversation
2d6582b to
597b6ea
Compare
Contributor
Author
|
@bshaffer Hi Brent, could you review this pull request? |
Contributor
|
OP sent me this for a repro: https://screencast.googleplex.com/cast/NTM2MDMzMjAwNzUzODY4OHxhNWE1NTkwOC1iOA |
vishwarajanand
approved these changes
Mar 7, 2024
Contributor
vishwarajanand
left a comment
There was a problem hiding this comment.
This change looks good to me.
Per Aws guide on creating signed request, it appears that datetime is required in UTC time in ISO 8601.
I did some testing in my local and confirmed that this change is scoped and looks compatible with existing users php.ini configs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue
Currently the value for
x-amz-dateis generated from date function, but if user sets a different timezone other than UTC, the generated timestamp has an invalid timestamp.This causes the following
SignatureDoesNotMatcherror when authenticating with Workload Identity Federation:When the timezone is older than UTC:
When the timezone is later than UTC:
This happens because
date('Ymd\THis\Z')produces timezone-aware timestamp, but the suffix is alwaysZ(UTC).Reproduce
We can easily reproduce this issue just adding
date_default_timezone_set('America/Los_Angeles');to the code.Also the following code illustrates how
date('Ymd\THis\Z')produces invalid timestamp.How to fix this
I changed the code to use gmdate instead of date function irrespective of the timezone the user sets.
I locally changed the code and confirmed that this fix works even if a different timezone is set in the code.