fix: expiration time of the ImpersonatedCredentials token depending on the current host's timezone #932

ivan-f-n · 2022-06-23T10:45:15Z

Fix bug in ImpersonatedCredentials class where the date received as expirationTime was being incorrectly parsed, assuming the date was in the host's timezone instead of reading from the date string what timezone it was.

Fixes #931 ☕️

conventional-commit-lint-gcf · 2022-06-23T10:45:18Z

🤖 I detect that the PR title and the commit message differ and there's only one commit. To use the PR title for the commit history, you can use Github's automerge feature with squashing, or use automerge label. Good luck human!

-- conventional-commit-lint bot
https://conventionalcommits.org/

google-cla · 2022-06-23T10:45:19Z

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

TimurSadykov · 2022-06-25T02:12:41Z

@lsirac can't you please take a look, possibly something applicable to other languages

lsirac · 2022-06-27T21:22:17Z

@aeitzman

oauth2_http/javatests/com/google/auth/oauth2/ImpersonatedCredentialsTest.java

aeitzman · 2022-06-29T18:51:05Z

oauth2_http/javatests/com/google/auth/oauth2/ImpersonatedCredentialsTest.java

+
+    mockTransportFactory.transport.setTargetPrincipal(IMPERSONATED_CLIENT_EMAIL);
+    mockTransportFactory.transport.setAccessToken(ACCESS_TOKEN);
+    mockTransportFactory.transport.setExpireTime(getFormattedTime(c.getTime()));


can this just be getDefaultExpireTime() now since the time zone isn't get set differently?

It can't because we need the exact Date instance used to generate the date string to later assert it was parsed correctly

oauth2_http/javatests/com/google/auth/oauth2/ImpersonatedCredentialsTest.java

aeitzman

LGTM

TimurSadykov

credential should stay immutable

oauth2_http/javatests/com/google/auth/oauth2/ImpersonatedCredentialsTest.java

TimurSadykov · 2022-07-14T01:47:00Z

oauth2_http/java/com/google/auth/oauth2/ImpersonatedCredentials.java

@@ -110,6 +111,8 @@ public class ImpersonatedCredentials extends GoogleCredentials

  private transient HttpTransportFactory transportFactory;

+  @VisibleForTesting transient Calendar calendar = Calendar.getInstance();


This should be private and implemented via builder.

Maybe you can use the same approach as custom lifetime in ServiceAccountCredentials:

google-auth-library-java/oauth2_http/java/com/google/auth/oauth2/ServiceAccountCredentials.java

Line 666 in c21ec6c

public ServiceAccountCredentials createWithCustomLifetime(int lifetime) {

generated-files-bot · 2022-07-14T11:16:53Z

Warning: This pull request is touching the following templated files:

.kokoro/common.sh
.kokoro/release/stage.sh

…btains the given expiration time date's timezone correctly instead of using local timezone

…xpirationTime was being incorrectly parsed, assuming the date was in the default JVM timezone instead of reading from the date string what timezone it was.

… the date instead of returning dates in different timezones Expose to tests the calendar object used by the SimpleDateFormat object of the refreshAccessToken method in the ImpersonatedCredentials class That way we stop relying on returning dates in different timezones in the expireTime returned by the mockTransportFactory

… asserts

TimurSadykov

LGTM, thanks!

TimurSadykov · 2022-07-21T09:14:23Z

@ivan-f-n thanks for your contribution, please also sign Google cla, it is a quick process: https://cla.developers.google.com/

ivan-f-n · 2022-07-26T16:37:38Z

@ivan-f-n thanks for your contribution, please also sign Google cla, it is a quick process: https://cla.developers.google.com/

Hi @TimurSadykov, the CLA is currently being sorted out by my company, should have an update later this week. Thanks for the patience! 🙏

…date-parsing

ivan-f-n · 2022-08-01T08:16:11Z

Hello @lsirac, @TimurSadykov
The CLA has finally been sorted out, sorry for the delay, could we move forward with this PR?

TimurSadykov

LGTM

ivan-f-n requested a review from a team as a code owner June 23, 2022 10:45

product-auto-label bot added the size: m Pull request size is medium. label Jun 23, 2022

ivan-f-n changed the title ~~Fix expiration time of the ImpersonatedCredentials token depending on the current host's timezone~~ fix: expiration time of the ImpersonatedCredentials token depending on the current host's timezone Jun 23, 2022

TimurSadykov requested review from lsirac and TimurSadykov June 25, 2022 02:11

ivan-f-n force-pushed the impersonated-creds-date-parsing branch from be67ac7 to 43aede8 Compare June 27, 2022 09:09

aeitzman requested changes Jun 28, 2022

View reviewed changes

aeitzman reviewed Jun 29, 2022

View reviewed changes

aeitzman approved these changes Jun 30, 2022

View reviewed changes

TimurSadykov requested changes Jul 14, 2022

View reviewed changes

ivan-f-n force-pushed the impersonated-creds-date-parsing branch from 93c9f85 to a3a3138 Compare July 14, 2022 11:16

product-auto-label bot added size: xl Pull request size is extra large. and removed size: m Pull request size is medium. labels Jul 14, 2022

ivan-f-n and others added 8 commits July 14, 2022 13:43

fix: refreshAccessToken method of ImpersonatedCredentials class now o…

50d3a5c

…btains the given expiration time date's timezone correctly instead of using local timezone

Fix bug in ImpersonatedCredentials class where the date received as e…

8ad0f77

…xpirationTime was being incorrectly parsed, assuming the date was in the default JVM timezone instead of reading from the date string what timezone it was.

fix: formatting

bafd0ac

Merge branch 'main' into impersonated-creds-date-parsing

d85d431

chore: remove unnecessary method and add blank line between setup and…

4e12ce8

… asserts

Merge branch 'main' into impersonated-creds-date-parsing

5a6937c

fix: make calendar field private and modify it through builder

62d7b08

ivan-f-n force-pushed the impersonated-creds-date-parsing branch from a3a3138 to 62d7b08 Compare July 14, 2022 11:44

product-auto-label bot added size: m Pull request size is medium. and removed size: xl Pull request size is extra large. labels Jul 14, 2022

TimurSadykov approved these changes Jul 21, 2022

View reviewed changes

Merge remote-tracking branch 'upstream/main' into impersonated-creds-…

98c1516

…date-parsing

ivan-f-n requested a review from a team as a code owner July 27, 2022 07:21

Merge branch 'main' into impersonated-creds-date-parsing

76aa904

TimurSadykov approved these changes Aug 2, 2022

View reviewed changes

TimurSadykov added kokoro:force-run Add this label to force Kokoro to re-run the tests. automerge Merge the pull request once unit tests and other checks pass. labels Aug 2, 2022

kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 2, 2022

gcf-merge-on-green bot merged commit 73af08a into googleapis:main Aug 2, 2022

gcf-merge-on-green bot removed the automerge Merge the pull request once unit tests and other checks pass. label Aug 2, 2022

release-please bot mentioned this pull request Aug 2, 2022

chore(main): release 1.9.0 #956

Merged

release-please bot mentioned this pull request Aug 11, 2022

chore(main): release 1.11.0 #974

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: expiration time of the ImpersonatedCredentials token depending on the current host's timezone #932

fix: expiration time of the ImpersonatedCredentials token depending on the current host's timezone #932

ivan-f-n commented Jun 23, 2022

conventional-commit-lint-gcf bot commented Jun 23, 2022 •

edited

google-cla bot commented Jun 23, 2022

TimurSadykov commented Jun 25, 2022

lsirac commented Jun 27, 2022

aeitzman Jun 29, 2022

ivan-f-n Jun 30, 2022

aeitzman left a comment

TimurSadykov left a comment

TimurSadykov Jul 14, 2022

ivan-f-n Jul 14, 2022

generated-files-bot bot commented Jul 14, 2022

TimurSadykov left a comment

TimurSadykov commented Jul 21, 2022

ivan-f-n commented Jul 26, 2022

ivan-f-n commented Aug 1, 2022

TimurSadykov left a comment

		@@ -110,6 +111,8 @@ public class ImpersonatedCredentials extends GoogleCredentials

		private transient HttpTransportFactory transportFactory;

		@VisibleForTesting transient Calendar calendar = Calendar.getInstance();

fix: expiration time of the ImpersonatedCredentials token depending on the current host's timezone #932

fix: expiration time of the ImpersonatedCredentials token depending on the current host's timezone #932

Conversation

ivan-f-n commented Jun 23, 2022

conventional-commit-lint-gcf bot commented Jun 23, 2022 • edited

google-cla bot commented Jun 23, 2022

TimurSadykov commented Jun 25, 2022

lsirac commented Jun 27, 2022

aeitzman Jun 29, 2022

Choose a reason for hiding this comment

ivan-f-n Jun 30, 2022

Choose a reason for hiding this comment

aeitzman left a comment

Choose a reason for hiding this comment

TimurSadykov left a comment

Choose a reason for hiding this comment

TimurSadykov Jul 14, 2022

Choose a reason for hiding this comment

ivan-f-n Jul 14, 2022

Choose a reason for hiding this comment

generated-files-bot bot commented Jul 14, 2022

TimurSadykov left a comment

Choose a reason for hiding this comment

TimurSadykov commented Jul 21, 2022

ivan-f-n commented Jul 26, 2022

ivan-f-n commented Aug 1, 2022

TimurSadykov left a comment

Choose a reason for hiding this comment

conventional-commit-lint-gcf bot commented Jun 23, 2022 •

edited