Skip to content

Commit

Permalink
fix: expiration time of the ImpersonatedCredentials token depending o…
Browse files Browse the repository at this point in the history
…n the current host's timezone (#932)

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 ☕️
  • Loading branch information
ivan-f-n committed Aug 2, 2022
1 parent acc1ce3 commit 73af08a
Show file tree
Hide file tree
Showing 2 changed files with 102 additions and 3 deletions.
Expand Up @@ -55,6 +55,7 @@
import java.text.SimpleDateFormat;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Calendar;
import java.util.Collection;
import java.util.Date;
import java.util.List;
Expand Down Expand Up @@ -91,7 +92,7 @@ public class ImpersonatedCredentials extends GoogleCredentials
implements ServiceAccountSigner, IdTokenProvider, QuotaProjectIdProvider {

private static final long serialVersionUID = -2133257318957488431L;
private static final String RFC3339 = "yyyy-MM-dd'T'HH:mm:ss'Z'";
private static final String RFC3339 = "yyyy-MM-dd'T'HH:mm:ssX";
private static final int TWELVE_HOURS_IN_SECONDS = 43200;
private static final int DEFAULT_LIFETIME_IN_SECONDS = 3600;
private static final String CLOUD_PLATFORM_SCOPE =
Expand All @@ -110,6 +111,8 @@ public class ImpersonatedCredentials extends GoogleCredentials

private transient HttpTransportFactory transportFactory;

private transient Calendar calendar;

/**
* @param sourceCredentials the source credential used to acquire the impersonated credentials. It
* should be either a user account credential or a service account credential.
Expand Down Expand Up @@ -429,6 +432,25 @@ public GoogleCredentials createScoped(Collection<String> scopes) {
.build();
}

/**
* Clones the impersonated credentials with a new calendar.
*
* @param calendar the calendar that will be used by the new ImpersonatedCredentials instance when
* parsing the received expiration time of the refreshed access token
* @return the cloned impersonated credentials with the given custom calendar
*/
public ImpersonatedCredentials createWithCustomCalendar(Calendar calendar) {
return toBuilder()
.setScopes(this.scopes)
.setLifetime(this.lifetime)
.setDelegates(this.delegates)
.setHttpTransportFactory(this.transportFactory)
.setQuotaProjectId(this.quotaProjectId)
.setIamEndpointOverride(this.iamEndpointOverride)
.setCalendar(calendar)
.build();
}

@Override
protected Map<String, List<String>> getAdditionalHeaders() {
Map<String, List<String>> headers = super.getAdditionalHeaders();
Expand All @@ -451,6 +473,7 @@ private ImpersonatedCredentials(Builder builder) {
this.quotaProjectId = builder.quotaProjectId;
this.iamEndpointOverride = builder.iamEndpointOverride;
this.transportFactoryClassName = this.transportFactory.getClass().getName();
this.calendar = builder.getCalendar();
if (this.delegates == null) {
this.delegates = new ArrayList<String>();
}
Expand Down Expand Up @@ -512,6 +535,7 @@ public AccessToken refreshAccessToken() throws IOException {
OAuth2Utils.validateString(responseData, "expireTime", "Expected to find an expireTime");

DateFormat format = new SimpleDateFormat(RFC3339);
format.setCalendar(calendar);
try {
Date date = format.parse(expireTime);
return new AccessToken(accessToken, date);
Expand Down Expand Up @@ -606,6 +630,7 @@ public static class Builder extends GoogleCredentials.Builder {
private HttpTransportFactory transportFactory;
private String quotaProjectId;
private String iamEndpointOverride;
private Calendar calendar = Calendar.getInstance();

protected Builder() {}

Expand Down Expand Up @@ -678,6 +703,15 @@ public Builder setIamEndpointOverride(String iamEndpointOverride) {
return this;
}

public Builder setCalendar(Calendar calendar) {
this.calendar = calendar;
return this;
}

public Calendar getCalendar() {
return this.calendar;
}

public ImpersonatedCredentials build() {
return new ImpersonatedCredentials(this);
}
Expand Down
Expand Up @@ -60,14 +60,17 @@
import java.io.InputStream;
import java.nio.charset.Charset;
import java.security.PrivateKey;
import java.text.DateFormat;
import java.text.SimpleDateFormat;
import java.time.temporal.ChronoUnit;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Calendar;
import java.util.Date;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.TimeZone;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

Expand Down Expand Up @@ -118,7 +121,7 @@ class ImpersonatedCredentialsTest extends BaseSerializationTest {
private static final int INVALID_LIFETIME = 43210;
private static JsonFactory JSON_FACTORY = GsonFactory.getDefaultInstance();

private static final String RFC3339 = "yyyy-MM-dd'T'HH:mm:ss'Z'";
private static final String RFC3339 = "yyyy-MM-dd'T'HH:mm:ssX";
public static final String DEFAULT_IMPERSONATION_URL =
"https://iamcredentials.googleapis.com/v1/projects/-/serviceAccounts/"
+ IMPERSONATED_CLIENT_EMAIL
Expand Down Expand Up @@ -562,6 +565,56 @@ void refreshAccessToken_delegates_success() throws IOException, IllegalStateExce
assertEquals(ACCESS_TOKEN, targetCredentials.refreshAccessToken().getTokenValue());
}

@Test
void refreshAccessToken_GMT_dateParsedCorrectly() throws IOException, IllegalStateException {
Calendar c = Calendar.getInstance();
c.add(Calendar.SECOND, VALID_LIFETIME);

mockTransportFactory.transport.setTargetPrincipal(IMPERSONATED_CLIENT_EMAIL);
mockTransportFactory.transport.setAccessToken(ACCESS_TOKEN);
mockTransportFactory.transport.setExpireTime(getFormattedTime(c.getTime()));
ImpersonatedCredentials targetCredentials =
ImpersonatedCredentials.create(
sourceCredentials,
IMPERSONATED_CLIENT_EMAIL,
null,
IMMUTABLE_SCOPES_LIST,
VALID_LIFETIME,
mockTransportFactory)
.createWithCustomCalendar(
// Set system timezone to GMT
Calendar.getInstance(TimeZone.getTimeZone("GMT")));

assertEquals(
c.getTime().toInstant().truncatedTo(ChronoUnit.SECONDS).toEpochMilli(),
targetCredentials.refreshAccessToken().getExpirationTimeMillis());
}

@Test
void refreshAccessToken_nonGMT_dateParsedCorrectly() throws IOException, IllegalStateException {
Calendar c = Calendar.getInstance();
c.add(Calendar.SECOND, VALID_LIFETIME);

mockTransportFactory.transport.setTargetPrincipal(IMPERSONATED_CLIENT_EMAIL);
mockTransportFactory.transport.setAccessToken(ACCESS_TOKEN);
mockTransportFactory.transport.setExpireTime(getFormattedTime(c.getTime()));
ImpersonatedCredentials targetCredentials =
ImpersonatedCredentials.create(
sourceCredentials,
IMPERSONATED_CLIENT_EMAIL,
null,
IMMUTABLE_SCOPES_LIST,
VALID_LIFETIME,
mockTransportFactory)
.createWithCustomCalendar(
// Set system timezone to one different than GMT
Calendar.getInstance(TimeZone.getTimeZone("America/Los_Angeles")));

assertEquals(
c.getTime().toInstant().truncatedTo(ChronoUnit.SECONDS).toEpochMilli(),
targetCredentials.refreshAccessToken().getExpirationTimeMillis());
}

@Test
void refreshAccessToken_invalidDate() throws IllegalStateException {

Expand Down Expand Up @@ -926,7 +979,19 @@ void serialize() throws IOException, ClassNotFoundException {
public static String getDefaultExpireTime() {
Calendar c = Calendar.getInstance();
c.add(Calendar.SECOND, VALID_LIFETIME);
return new SimpleDateFormat(RFC3339).format(c.getTime());
return getFormattedTime(c.getTime());
}

/**
* Given a {@link Date}, it will return a string of the date formatted like
* <b>yyyy-MM-dd'T'HH:mm:ss'Z'</b>
*/
private static String getFormattedTime(final Date date) {
// Set timezone to GMT since that's the TZ used in the response from the service impersonation
// token exchange
final DateFormat formatter = new SimpleDateFormat(RFC3339);
formatter.setTimeZone(TimeZone.getTimeZone("GMT"));
return formatter.format(date);
}

private String generateErrorJson(
Expand Down

0 comments on commit 73af08a

Please sign in to comment.