fix: ComputeEngineCredentials.createScoped should invalidate existing AccessToken (#1428)

Fixes #1387 ☕️


As described in the original issue, this looks like a regression introduced in googleapis/google-auth-library-java@7e26861 when migrating from deprecated constructor to use builder. 
[Access token](https://cloud.google.com/docs/authentication/token-types#access-contents) is scoped and should be invalidated when scope changes.

This PR include changes:
- ComputeEngineCredentials.createScoped() should invalidate existing AccessToken 
- ComputeEngineCredentials.createScoped(newScopes, newDefaultScopes) should respect universe domain settings.
- For testing, update to mock transport to support returning access token when url is scoped. Should return a different access token compared to default url with no scopes.

Author: zhumin8

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

@@ -48,6 +48,7 @@
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Joiner;
 import com.google.common.base.MoreObjects.ToStringHelper;
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.google.errorprone.annotations.CanIgnoreReturnValue;
 import java.io.BufferedReader;
@@ -155,21 +156,19 @@ private ComputeEngineCredentials(ComputeEngineCredentials.Builder builder) {
   /** Clones the compute engine account with the specified scopes. */
   @Override
   public GoogleCredentials createScoped(Collection<String> newScopes) {
-    ComputeEngineCredentials.Builder builder =
-        this.toBuilder().setHttpTransportFactory(transportFactory).setScopes(newScopes);
-    return new ComputeEngineCredentials(builder);
+    return createScoped(newScopes, ImmutableList.of());
   }
 
   /** Clones the compute engine account with the specified scopes and default scopes. */
   @Override
   public GoogleCredentials createScoped(
       Collection<String> newScopes, Collection<String> newDefaultScopes) {
-    ComputeEngineCredentials.Builder builder =
-        ComputeEngineCredentials.newBuilder()
-            .setHttpTransportFactory(transportFactory)
-            .setScopes(newScopes)
-            .setDefaultScopes(newDefaultScopes);
-    return new ComputeEngineCredentials(builder);
+    return this.toBuilder()
+        .setHttpTransportFactory(transportFactory)
+        .setScopes(newScopes)
+        .setDefaultScopes(newDefaultScopes)
+        .setAccessToken(null)
+        .build();
   }
 
   /**

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

@@ -427,8 +427,8 @@ public boolean createScopedRequired() {
   }
 
   /**
-   * If the credentials support scopes, creates a copy of the identity with the specified scopes;
-   * otherwise, returns the same instance.
+   * If the credentials support scopes, creates a copy of the identity with the specified scopes,
+   * invalidates the existing scoped access token; otherwise, return the same instance.
    *
    * @param scopes Collection of scopes to request.
    * @return GoogleCredentials with requested scopes.

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

@@ -63,7 +63,9 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Queue;
+import java.util.stream.Collectors;
 import java.util.stream.IntStream;
+import java.util.stream.Stream;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
@@ -111,6 +113,16 @@ public class ComputeEngineCredentialsTest extends BaseSerializationTest {
           + "iTElDRU5TRV8xIiwNCiAgICAgICAiTElDRU5TRV8yIg0KICAgIF0NCiAgfSwNCiAgImlhdCI6IDE1NjQ1MTU4OTY"
           + "sDQogICJpc3MiOiAiaHR0cHM6Ly9hY2NvdW50cy5nb29nbGUuY29tIiwNCiAgInN1YiI6ICIxMTIxNzkwNjI3MjA"
           + "zOTEzMDU4ODUiDQp9.redacted";
+  private static final String ACCESS_TOKEN = "1/MkSJoj1xsli0AccessToken_NKPY2";
+  private static final List<String> SCOPES = Arrays.asList("foo", "bar");
+  private static final String ACCESS_TOKEN_WITH_SCOPES = "1/MkSJoj1xsli0AccessTokenScoped_NKPY2";
+  private static final Map<String, String> SCOPE_TO_ACCESS_TOKEN_MAP =
+      Stream.of(
+              new String[][] {
+                {"default", ACCESS_TOKEN},
+                {SCOPES.toString().replaceAll("\\s", ""), ACCESS_TOKEN_WITH_SCOPES},
+              })
+          .collect(Collectors.toMap(data -> data[0], data -> data[1]));
 
   @Test
   public void buildTokenUrlWithScopes_null_scopes() {
@@ -215,7 +227,7 @@ public void buildScoped_explicitUniverse() throws IOException {
         (ComputeEngineCredentials) credentials.createScoped(Arrays.asList("foo"));
 
     assertEquals("some-universe", scopedCredentials.getUniverseDomain());
-    assertEquals(true, scopedCredentials.isExplicitUniverseDomain());
+    assertTrue(scopedCredentials.isExplicitUniverseDomain());
   }
 
   @Test
@@ -228,6 +240,33 @@ public void createScoped_defaultScopes() {
     assertEquals("foo", scopes.toArray()[0]);
   }
 
+  @Test
+  public void buildScoped_quotaProjectId() throws IOException {
+    ComputeEngineCredentials credentials =
+        ComputeEngineCredentials.newBuilder()
+            .setScopes(null)
+            .setQuotaProjectId("some-project-id")
+            .build();
+    ComputeEngineCredentials scopedCredentials =
+        (ComputeEngineCredentials) credentials.createScoped(Arrays.asList("foo"));
+
+    assertEquals("some-project-id", scopedCredentials.getQuotaProjectId());
+  }
+
+  @Test
+  public void buildDefaultScoped_explicitUniverse() throws IOException {
+    ComputeEngineCredentials credentials =
+        ComputeEngineCredentials.newBuilder()
+            .setScopes(null)
+            .setUniverseDomain("some-universe")
+            .build();
+    ComputeEngineCredentials scopedCredentials =
+        (ComputeEngineCredentials) credentials.createScoped(null, Arrays.asList("foo"));
+
+    assertEquals("some-universe", scopedCredentials.getUniverseDomain());
+    assertTrue(scopedCredentials.isExplicitUniverseDomain());
+  }
+
   @Test
   public void create_scoped_correctMargins() {
     GoogleCredentials credentials =
@@ -240,21 +279,37 @@ public void create_scoped_correctMargins() {
 
   @Test
   public void getRequestMetadata_hasAccessToken() throws IOException {
-    String accessToken = "1/MkSJoj1xsli0AccessToken_NKPY2";
     MockMetadataServerTransportFactory transportFactory = new MockMetadataServerTransportFactory();
-    transportFactory.transport.setAccessToken(accessToken);
     ComputeEngineCredentials credentials =
         ComputeEngineCredentials.newBuilder().setHttpTransportFactory(transportFactory).build();
     Map<String, List<String>> metadata = credentials.getRequestMetadata(CALL_URI);
 
-    TestUtils.assertContainsBearerToken(metadata, accessToken);
+    TestUtils.assertContainsBearerToken(metadata, ACCESS_TOKEN);
+  }
+
+  @Test
+  public void getRequestMetadata_shouldInvalidateAccessTokenWhenScoped_newAccessTokenFromRefresh()
+      throws IOException {
+    MockMetadataServerTransportFactory transportFactory = new MockMetadataServerTransportFactory();
+    ComputeEngineCredentials credentials =
+        ComputeEngineCredentials.newBuilder().setHttpTransportFactory(transportFactory).build();
+    Map<String, List<String>> metadata = credentials.getRequestMetadata(CALL_URI);
+
+    TestUtils.assertContainsBearerToken(metadata, ACCESS_TOKEN);
+
+    assertNotNull(credentials.getAccessToken());
+    ComputeEngineCredentials scopedCredentialCopy =
+        (ComputeEngineCredentials) credentials.createScoped(SCOPES);
+    assertNull(scopedCredentialCopy.getAccessToken());
+    Map<String, List<String>> metadataForCopiedCredentials =
+        scopedCredentialCopy.getRequestMetadata(CALL_URI);
+    TestUtils.assertContainsBearerToken(metadataForCopiedCredentials, ACCESS_TOKEN_WITH_SCOPES);
+    TestUtils.assertNotContainsBearerToken(metadataForCopiedCredentials, ACCESS_TOKEN);
   }
 
   @Test
   public void getRequestMetadata_missingServiceAccount_throws() {
-    String accessToken = "1/MkSJoj1xsli0AccessToken_NKPY2";
     MockMetadataServerTransportFactory transportFactory = new MockMetadataServerTransportFactory();
-    transportFactory.transport.setAccessToken(accessToken);
     transportFactory.transport.setRequestStatusCode(HttpStatusCodes.STATUS_CODE_NOT_FOUND);
     ComputeEngineCredentials credentials =
         ComputeEngineCredentials.newBuilder().setHttpTransportFactory(transportFactory).build();
@@ -271,9 +326,7 @@ public void getRequestMetadata_missingServiceAccount_throws() {
 
   @Test
   public void getRequestMetadata_serverError_throws() {
-    String accessToken = "1/MkSJoj1xsli0AccessToken_NKPY2";
     MockMetadataServerTransportFactory transportFactory = new MockMetadataServerTransportFactory();
-    transportFactory.transport.setAccessToken(accessToken);
     transportFactory.transport.setRequestStatusCode(HttpStatusCodes.STATUS_CODE_SERVER_ERROR);
     ComputeEngineCredentials credentials =
         ComputeEngineCredentials.newBuilder().setHttpTransportFactory(transportFactory).build();
@@ -481,11 +534,9 @@ public LowLevelHttpResponse execute() throws IOException {
   @Test
   public void sign_sameAs() throws IOException {
     MockMetadataServerTransportFactory transportFactory = new MockMetadataServerTransportFactory();
-    final String accessToken = "1/MkSJoj1xsli0AccessToken_NKPY2";
     String defaultAccountEmail = "mail@mail.com";
     byte[] expectedSignature = {0xD, 0xE, 0xA, 0xD};
 
-    transportFactory.transport.setAccessToken(accessToken);
     transportFactory.transport.setServiceAccountEmail(defaultAccountEmail);
     transportFactory.transport.setSignature(expectedSignature);
     ComputeEngineCredentials credentials =
@@ -497,10 +548,8 @@ public void sign_sameAs() throws IOException {
   @Test
   public void sign_getAccountFails() throws IOException {
     MockMetadataServerTransportFactory transportFactory = new MockMetadataServerTransportFactory();
-    final String accessToken = "1/MkSJoj1xsli0AccessToken_NKPY2";
     byte[] expectedSignature = {0xD, 0xE, 0xA, 0xD};
 
-    transportFactory.transport.setAccessToken(accessToken);
     transportFactory.transport.setSignature(expectedSignature);
     ComputeEngineCredentials credentials =
         ComputeEngineCredentials.newBuilder().setHttpTransportFactory(transportFactory).build();
@@ -517,7 +566,6 @@ public void sign_getAccountFails() throws IOException {
   @Test
   public void sign_accessDenied_throws() {
     MockMetadataServerTransportFactory transportFactory = new MockMetadataServerTransportFactory();
-    final String accessToken = "1/MkSJoj1xsli0AccessToken_NKPY2";
     String defaultAccountEmail = "mail@mail.com";
 
     transportFactory.transport =
@@ -538,7 +586,7 @@ public LowLevelHttpResponse execute() throws IOException {
           }
         };
 
-    transportFactory.transport.setAccessToken(accessToken);
+    transportFactory.transport.setAccessToken(ACCESS_TOKEN);
     transportFactory.transport.setServiceAccountEmail(defaultAccountEmail);
 
     ComputeEngineCredentials credentials =
@@ -558,7 +606,6 @@ public LowLevelHttpResponse execute() throws IOException {
   @Test
   public void sign_serverError_throws() {
     MockMetadataServerTransportFactory transportFactory = new MockMetadataServerTransportFactory();
-    final String accessToken = "1/MkSJoj1xsli0AccessToken_NKPY2";
     String defaultAccountEmail = "mail@mail.com";
 
     transportFactory.transport =
@@ -579,7 +626,7 @@ public LowLevelHttpResponse execute() throws IOException {
           }
         };
 
-    transportFactory.transport.setAccessToken(accessToken);
+    transportFactory.transport.setAccessToken(ACCESS_TOKEN);
     transportFactory.transport.setServiceAccountEmail(defaultAccountEmail);
 
     ComputeEngineCredentials credentials =
@@ -808,7 +855,6 @@ public void getUniverseDomain_fromMetadata_non404error_throws() throws IOExcepti
   @Test
   public void sign_emptyContent_throws() {
     MockMetadataServerTransportFactory transportFactory = new MockMetadataServerTransportFactory();
-    String accessToken = "1/MkSJoj1xsli0AccessToken_NKPY2";
     String defaultAccountEmail = "mail@mail.com";
 
     transportFactory.transport =
@@ -828,7 +874,7 @@ public LowLevelHttpResponse execute() throws IOException {
           }
         };
 
-    transportFactory.transport.setAccessToken(accessToken);
+    transportFactory.transport.setAccessToken(ACCESS_TOKEN);
     transportFactory.transport.setServiceAccountEmail(defaultAccountEmail);
 
     ComputeEngineCredentials credentials =
@@ -930,7 +976,8 @@ public void idTokenWithAudience_license() throws IOException {
 
   static class MockMetadataServerTransportFactory implements HttpTransportFactory {
 
-    MockMetadataServerTransport transport = new MockMetadataServerTransport();
+    MockMetadataServerTransport transport =
+        new MockMetadataServerTransport(SCOPE_TO_ACCESS_TOKEN_MAP);
 
     @Override
     public HttpTransport create() {

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

@@ -46,7 +46,6 @@
 import com.google.api.client.testing.http.MockLowLevelHttpRequest;
 import com.google.auth.TestUtils;
 import com.google.auth.http.HttpTransportFactory;
-import com.google.auth.oauth2.ComputeEngineCredentialsTest.MockMetadataServerTransportFactory;
 import java.io.BufferedReader;
 import java.io.ByteArrayInputStream;
 import java.io.File;
@@ -306,7 +305,6 @@ public void getDefaultCredentials_appEngineSkipWorks_retrievesCloudShellCredenti
   @Test
   public void getDefaultCredentials_compute_providesToken() throws IOException {
     MockMetadataServerTransportFactory transportFactory = new MockMetadataServerTransportFactory();
-    transportFactory.transport.setAccessToken(ACCESS_TOKEN);
     TestDefaultCredentialsProvider testProvider = new TestDefaultCredentialsProvider();
 
     GoogleCredentials defaultCredentials = testProvider.getDefaultCredentials(transportFactory);
@@ -331,7 +329,6 @@ public void getDefaultCredentials_cloudshell() throws IOException {
   @Test
   public void getDefaultCredentials_cloudshell_withComputCredentialsPresent() throws IOException {
     MockMetadataServerTransportFactory transportFactory = new MockMetadataServerTransportFactory();
-    transportFactory.transport.setAccessToken(ACCESS_TOKEN);
     TestDefaultCredentialsProvider testProvider = new TestDefaultCredentialsProvider();
     testProvider.setEnv(DefaultCredentialsProvider.CLOUD_SHELL_ENV_VAR, "4");
 
@@ -463,7 +460,6 @@ public void getDefaultCredentials_quota_project() throws IOException {
   @Test
   public void getDefaultCredentials_compute_quotaProject() throws IOException {
     MockMetadataServerTransportFactory transportFactory = new MockMetadataServerTransportFactory();
-    transportFactory.transport.setAccessToken(ACCESS_TOKEN);
     TestDefaultCredentialsProvider testProvider = new TestDefaultCredentialsProvider();
     testProvider.setEnv(
         DefaultCredentialsProvider.QUOTA_PROJECT_ENV_VAR, QUOTA_PROJECT_FROM_ENVIRONMENT);
@@ -887,4 +883,14 @@ public HttpTransport create() {
       return transport;
     }
   }
+
+  static class MockMetadataServerTransportFactory implements HttpTransportFactory {
+
+    MockMetadataServerTransport transport = new MockMetadataServerTransport(ACCESS_TOKEN);
+
+    @Override
+    public HttpTransport create() {
+      return transport;
+    }
+  }
 }

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

@@ -38,20 +38,22 @@
 import com.google.api.client.testing.http.MockHttpTransport;
 import com.google.api.client.testing.http.MockLowLevelHttpRequest;
 import com.google.api.client.testing.http.MockLowLevelHttpResponse;
+import com.google.common.base.Splitter;
 import com.google.common.io.BaseEncoding;
 import java.io.IOException;
 import java.io.UnsupportedEncodingException;
 import java.net.MalformedURLException;
 import java.net.URL;
 import java.net.URLDecoder;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 
 /** Transport that simulates the GCE metadata server for access tokens. */
 public class MockMetadataServerTransport extends MockHttpTransport {
 
-  private String accessToken;
-
+  // key are scopes as in request url string following "?scopes="
+  private Map<String, String> scopesToAccessToken;
   private Integer requestStatusCode;
 
   private String serviceAccountEmail;
@@ -62,8 +64,25 @@ public class MockMetadataServerTransport extends MockHttpTransport {
 
   public MockMetadataServerTransport() {}
 
+  public MockMetadataServerTransport(String accessToken) {
+    setAccessToken(accessToken);
+  }
+
+  public MockMetadataServerTransport(Map<String, String> accessTokenMap) {
+    for (String scope : accessTokenMap.keySet()) {
+      setAccessToken(scope, accessTokenMap.get(scope));
+    }
+  }
+
   public void setAccessToken(String accessToken) {
-    this.accessToken = accessToken;
+    setAccessToken("default", accessToken);
+  }
+
+  public void setAccessToken(String scopes, String accessToken) {
+    if (scopesToAccessToken == null) {
+      scopesToAccessToken = new HashMap<>();
+    }
+    scopesToAccessToken.put(scopes, accessToken);
   }
 
   public void setRequestStatusCode(Integer requestStatusCode) {
@@ -84,7 +103,7 @@ public void setIdToken(String idToken) {
 
   @Override
   public LowLevelHttpRequest buildRequest(String method, String url) throws IOException {
-    if (url.equals(ComputeEngineCredentials.getTokenServerEncodedUrl())) {
+    if (url.startsWith(ComputeEngineCredentials.getTokenServerEncodedUrl())) {
       return getMockRequestForTokenEndpoint(url);
     } else if (isGetServiceAccountsUrl(url)) {
       return getMockRequestForServiceAccount(url);
@@ -164,7 +183,15 @@ public LowLevelHttpResponse execute() throws IOException {
         // Create the JSON response
         GenericJson refreshContents = new GenericJson();
         refreshContents.setFactory(OAuth2Utils.JSON_FACTORY);
-        refreshContents.put("access_token", accessToken);
+
+        List<String> urlParsed = Splitter.on("?scopes=").splitToList(url);
+        if (urlParsed.size() == 1) {
+          // no scopes specified, return access token correspoinding to default scopes
+          refreshContents.put("access_token", scopesToAccessToken.get("default"));
+        } else {
+          refreshContents.put(
+              "access_token", scopesToAccessToken.get("[" + urlParsed.get(1) + "]"));
+        }
         refreshContents.put("expires_in", 3600000);
         refreshContents.put("token_type", "Bearer");
         String refreshText = refreshContents.toPrettyString();