Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: ComputeEngineCredentials.createScoped should invalidate existing AccessToken #1428

Merged
merged 18 commits into from
Aug 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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
Expand All @@ -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());
}
lqiu96 marked this conversation as resolved.
Show resolved Hide resolved

@Test
public void create_scoped_correctMargins() {
GoogleCredentials credentials =
Expand All @@ -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();
Expand All @@ -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();
Expand Down Expand Up @@ -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 =
Expand All @@ -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();
Expand All @@ -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 =
Expand All @@ -538,7 +586,7 @@ public LowLevelHttpResponse execute() throws IOException {
}
};

transportFactory.transport.setAccessToken(accessToken);
transportFactory.transport.setAccessToken(ACCESS_TOKEN);
transportFactory.transport.setServiceAccountEmail(defaultAccountEmail);

ComputeEngineCredentials credentials =
Expand All @@ -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 =
Expand All @@ -579,7 +626,7 @@ public LowLevelHttpResponse execute() throws IOException {
}
};

transportFactory.transport.setAccessToken(accessToken);
transportFactory.transport.setAccessToken(ACCESS_TOKEN);
transportFactory.transport.setServiceAccountEmail(defaultAccountEmail);

ComputeEngineCredentials credentials =
Expand Down Expand Up @@ -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 =
Expand All @@ -828,7 +874,7 @@ public LowLevelHttpResponse execute() throws IOException {
}
};

transportFactory.transport.setAccessToken(accessToken);
transportFactory.transport.setAccessToken(ACCESS_TOKEN);
transportFactory.transport.setServiceAccountEmail(defaultAccountEmail);

ComputeEngineCredentials credentials =
Expand Down Expand Up @@ -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() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand All @@ -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");

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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<>();
}
Comment on lines +82 to +84
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: initialize in the constructor

scopesToAccessToken.put(scopes, accessToken);
}

public void setRequestStatusCode(Integer requestStatusCode) {
Expand All @@ -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())) {
lqiu96 marked this conversation as resolved.
Show resolved Hide resolved
return getMockRequestForTokenEndpoint(url);
} else if (isGetServiceAccountsUrl(url)) {
return getMockRequestForServiceAccount(url);
Expand Down Expand Up @@ -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();
Expand Down