fix: ImpersonatedCredentials does not use Calendar for expiration (#1908)

* chore: Add ObsoleteApi annotation

* chore: Add tests for serializing ImpersonatedCredentials

* chore: Restore old code

* chore: Update error message

* chore: Fix failing test issue

* chore: Remove old tests referecing obsolete api functionality

Original-PR: googleapis/google-auth-library-java#1908

Author: lqiu96

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

@@ -45,6 +45,7 @@
 import com.google.api.client.json.JsonObjectParser;
 import com.google.api.client.util.GenericData;
 import com.google.api.core.InternalApi;
+import com.google.api.core.ObsoleteApi;
 import com.google.auth.CredentialTypeForMetrics;
 import com.google.auth.ServiceAccountSigner;
 import com.google.auth.http.HttpCredentialsAdapter;
@@ -59,9 +60,9 @@
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.ObjectInputStream;
-import java.text.DateFormat;
-import java.text.ParseException;
-import java.text.SimpleDateFormat;
+import java.time.DateTimeException;
+import java.time.Instant;
+import java.time.format.DateTimeFormatter;
 import java.util.ArrayList;
 import java.util.Calendar;
 import java.util.Collection;
@@ -101,7 +102,6 @@ public class ImpersonatedCredentials extends GoogleCredentials
     implements ServiceAccountSigner, IdTokenProvider {
 
   private static final long serialVersionUID = -2133257318957488431L;
-  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 GoogleCredentials sourceCredentials;
@@ -510,12 +510,16 @@ public CredentialTypeForMetrics getMetricsCredentialType() {
   }
 
   /**
-   * Clones the impersonated credentials with a new calendar.
+   * This method is marked obsolete. There is no alternative to setting a custom calendar for the
+   * Credential.
+   *
+   * <p>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
    */
+  @ObsoleteApi("This method is obsolete and will be removed in a future release.")
   public ImpersonatedCredentials createWithCustomCalendar(Calendar calendar) {
     return toBuilder()
         .setScopes(this.scopes)
@@ -660,14 +664,23 @@ public AccessToken refreshAccessToken() throws IOException {
     String expireTime =
         OAuth2Utils.validateString(responseData, "expireTime", "Expected to find an expireTime");
 
-    DateFormat format = new SimpleDateFormat(RFC3339);
-    format.setCalendar(calendar);
+    Instant expirationInstant;
     try {
-      Date date = format.parse(expireTime);
-      return new AccessToken(accessToken, date);
-    } catch (ParseException pe) {
-      throw new IOException("Error parsing expireTime: " + pe.getMessage());
+      if (calendar != null) {
+        // For backward compatibility, if a custom calendar is set, use its timezone
+        // and convert it to an Instant
+        expirationInstant =
+            Instant.from(
+                DateTimeFormatter.ISO_INSTANT
+                    .withZone(calendar.getTimeZone().toZoneId())
+                    .parse(expireTime));
+      } else {
+        expirationInstant = Instant.parse(expireTime);
+      }
+    } catch (DateTimeException e) {
+      throw new IOException("Error parsing expireTime: " + expireTime, e);
     }
+    return new AccessToken(accessToken, Date.from(expirationInstant));
   }
 
   /**
@@ -883,7 +896,17 @@ public Builder setIamEndpointOverride(String iamEndpointOverride) {
       return this;
     }
 
+    /**
+     * This method is marked obsolete. There is no alternative to setting a custom calendar for the
+     * Credential.
+     *
+     * <p>Sets the calendar to be used for parsing the expiration time.
+     *
+     * @param calendar the calendar to use
+     * @return the builder
+     */
     @CanIgnoreReturnValue
+    @ObsoleteApi("This method is obsolete and will be removed in a future release.")
     public Builder setCalendar(Calendar calendar) {
       this.calendar = calendar;
       return this;
@@ -903,6 +926,15 @@ public Builder setReadTimeout(int readTimeout) {
       return this;
     }
 
+    /**
+     * This method is marked obsolete. There is no alternative to getting a custom calendar for the
+     * Credential.
+     *
+     * <p>Returns the calendar to be used for parsing the expiration time.
+     *
+     * @return the calendar
+     */
+    @ObsoleteApi("This method is obsolete and will be removed in a future release.")
     public Calendar getCalendar() {
       return this.calendar;
     }

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

@@ -44,6 +44,7 @@
 import static org.mockito.Mockito.when;
 
 import com.google.api.client.http.HttpStatusCodes;
+import com.google.api.client.http.HttpTransport;
 import com.google.api.client.json.GenericJson;
 import com.google.api.client.json.JsonFactory;
 import com.google.api.client.json.JsonGenerator;
@@ -54,23 +55,22 @@
 import com.google.auth.Credentials;
 import com.google.auth.ServiceAccountSigner.SigningException;
 import com.google.auth.TestUtils;
+import com.google.auth.http.HttpTransportFactory;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.io.InputStream;
 import java.nio.charset.Charset;
 import java.security.PrivateKey;
-import java.text.DateFormat;
-import java.text.SimpleDateFormat;
+import java.time.Instant;
 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.TimeZone;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
@@ -122,8 +122,6 @@ class ImpersonatedCredentialsTest extends BaseSerializationTest {
   private static final int INVALID_LIFETIME = 43210;
   private static final JsonFactory JSON_FACTORY = GsonFactory.getDefaultInstance();
 
-  private static final String RFC3339 = "yyyy-MM-dd'T'HH:mm:ssX";
-
   private static final String TEST_UNIVERSE_DOMAIN = "test.xyz";
   private static final String OLD_IMPERSONATION_URL =
       "https://iamcredentials.googleapis.com/v1/projects/-/serviceAccounts/"
@@ -653,61 +651,9 @@ 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.getTransport().setTargetPrincipal(IMPERSONATED_CLIENT_EMAIL);
-    mockTransportFactory.getTransport().setAccessToken(ACCESS_TOKEN);
-    mockTransportFactory.getTransport().setExpireTime(getFormattedTime(c.getTime()));
-    mockTransportFactory.getTransport().addStatusCodeAndMessage(HttpStatusCodes.STATUS_CODE_OK, "");
-    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")));
-
-    assertTrue(
-        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.getTransport().setTargetPrincipal(IMPERSONATED_CLIENT_EMAIL);
-    mockTransportFactory.getTransport().setAccessToken(ACCESS_TOKEN);
-    mockTransportFactory.getTransport().setExpireTime(getFormattedTime(c.getTime()));
-    mockTransportFactory.getTransport().addStatusCodeAndMessage(HttpStatusCodes.STATUS_CODE_OK, "");
-    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")));
-
-    assertTrue(
-        c.getTime().toInstant().truncatedTo(ChronoUnit.SECONDS).toEpochMilli()
-            == targetCredentials.refreshAccessToken().getExpirationTimeMillis());
-  }
-
   @Test
   void refreshAccessToken_invalidDate() throws IllegalStateException {
-    String expectedMessage = "Unparseable date";
+    String expectedMessage = "Error parsing expireTime: ";
     mockTransportFactory.getTransport().setTargetPrincipal(IMPERSONATED_CLIENT_EMAIL);
     mockTransportFactory.getTransport().setAccessToken("foo");
     mockTransportFactory.getTransport().setExpireTime("1973-09-29T15:01:23");
@@ -1297,22 +1243,69 @@ void serialize() throws IOException, ClassNotFoundException {
     assertSame(deserializedCredentials.clock, Clock.SYSTEM);
   }
 
-  public static String getDefaultExpireTime() {
-    Calendar c = Calendar.getInstance();
-    c.add(Calendar.SECOND, VALID_LIFETIME);
-    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>
+   * A stateful {@link HttpTransportFactory} that provides a shared {@link
+   * MockIAMCredentialsServiceTransport} instance.
+   *
+   * <p>This is necessary for serialization tests because {@link ImpersonatedCredentials} stores the
+   * factory's class name and re-instantiates it via reflection during deserialization. A standard
+   * factory would create a fresh, unconfigured transport upon re-instantiation, causing refreshed
+   * token requests to fail. Using a static transport ensures the mock configuration persists across
+   * serialization boundaries.
    */
-  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);
+  public static class StatefulMockIAMTransportFactory implements HttpTransportFactory {
+    private static final MockIAMCredentialsServiceTransport TRANSPORT =
+        new MockIAMCredentialsServiceTransport(GoogleCredentials.GOOGLE_DEFAULT_UNIVERSE);
+
+    @Override
+    public HttpTransport create() {
+      return TRANSPORT;
+    }
+
+    public static MockIAMCredentialsServiceTransport getTransport() {
+      return TRANSPORT;
+    }
+  }
+
+  @Test
+  void refreshAccessToken_afterSerialization_success() throws IOException, ClassNotFoundException {
+    // This test ensures that credentials can still refresh after being serialized.
+    // ImpersonatedCredentials only serializes the transport factory's class name.
+    // Upon deserialization, it creates a new instance of that factory via reflection.
+    // StatefulMockIAMTransportFactory uses a static transport instance so that the
+    // configuration we set here (token, expiration) is available to the new factory instance.
+    MockIAMCredentialsServiceTransport transport = StatefulMockIAMTransportFactory.getTransport();
+    transport.setTargetPrincipal(IMPERSONATED_CLIENT_EMAIL);
+    transport.setAccessToken(ACCESS_TOKEN);
+
+    transport.setExpireTime(getDefaultExpireTime());
+    transport.addStatusCodeAndMessage(HttpStatusCodes.STATUS_CODE_OK, "", true);
+
+    // Use a source credential that doesn't need refresh
+    AccessToken sourceToken =
+        new AccessToken("source-token", new Date(System.currentTimeMillis() + 3600000));
+    GoogleCredentials sourceCredentials = GoogleCredentials.create(sourceToken);
+
+    ImpersonatedCredentials targetCredentials =
+        ImpersonatedCredentials.create(
+            sourceCredentials,
+            IMPERSONATED_CLIENT_EMAIL,
+            null,
+            IMMUTABLE_SCOPES_LIST,
+            VALID_LIFETIME,
+            new StatefulMockIAMTransportFactory());
+
+    ImpersonatedCredentials deserializedCredentials = serializeAndDeserialize(targetCredentials);
+
+    // This should not throw NPE. The transient 'calendar' field being null after
+    // deserialization is now handled by using java.time.Instant for parsing.
+    AccessToken token = deserializedCredentials.refreshAccessToken();
+    assertNotNull(token);
+    assertEquals(ACCESS_TOKEN, token.getTokenValue());
+  }
+
+  public static String getDefaultExpireTime() {
+    return Instant.now().plusSeconds(VALID_LIFETIME).truncatedTo(ChronoUnit.SECONDS).toString();
   }
 
   private String generateErrorJson(