From 4b2384374cce1381befcef1b710048078c9a2fce Mon Sep 17 00:00:00 2001 From: Leo <39062083+lsirac@users.noreply.github.com> Date: Mon, 26 Jul 2021 15:50:42 -0700 Subject: [PATCH] fix: use source credential expiration when STS does not return expires_in (#699) * feat: Adding functional tests for Service Account (#685) ServiceAccountCredentials tests for 4110 * feat: allow scopes for self signed jwt (#689) * feat: self signed jwt support * update * address comments * allow to use uri as audience * address comments * chore: release 0.27.0 (#678) :robot: I have created a release \*beep\* \*boop\* --- ## [0.27.0](https://www.github.com/googleapis/google-auth-library-java/compare/v0.26.0...v0.27.0) (2021-07-14) ### Features * add Id token support for UserCredentials ([#650](https://www.github.com/googleapis/google-auth-library-java/issues/650)) ([5a8f467](https://www.github.com/googleapis/google-auth-library-java/commit/5a8f4676630854c53aa708a9c8b960770067f858)) * add impersonation credentials to ADC ([#613](https://www.github.com/googleapis/google-auth-library-java/issues/613)) ([b9823f7](https://www.github.com/googleapis/google-auth-library-java/commit/b9823f70d7f3f7461b7de40bee06f5e7ba0e797c)) * Adding functional tests for Service Account ([#685](https://www.github.com/googleapis/google-auth-library-java/issues/685)) ([dfe118c](https://www.github.com/googleapis/google-auth-library-java/commit/dfe118c261aadf137a3cf47a7acb9892c7a6db4d)) * allow scopes for self signed jwt ([#689](https://www.github.com/googleapis/google-auth-library-java/issues/689)) ([f4980c7](https://www.github.com/googleapis/google-auth-library-java/commit/f4980c77566bbd5ef4c532acb199d7d484dbcd01)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). * test: adds integration tests for downscoping with credential access boundaries * fix: STS does not always return expires_in, fallback to source credential expiration for DownscopedCredentials * fix: review Co-authored-by: Timur Sadykov Co-authored-by: arithmetic1728 <58957152+arithmetic1728@users.noreply.github.com> Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com> --- .../auth/oauth2/DownscopedCredentials.java | 15 ++++- .../google/auth/oauth2/StsRequestHandler.java | 9 +-- .../auth/oauth2/StsTokenExchangeResponse.java | 29 ++++++---- .../oauth2/DownscopedCredentialsTest.java | 58 +++++++++++++++++-- .../google/auth/oauth2/MockStsTransport.java | 58 ++++++++++++++++++- .../auth/oauth2/StsRequestHandlerTest.java | 33 ++++++++++- 6 files changed, 175 insertions(+), 27 deletions(-) diff --git a/oauth2_http/java/com/google/auth/oauth2/DownscopedCredentials.java b/oauth2_http/java/com/google/auth/oauth2/DownscopedCredentials.java index d9e07721a..1928f2539 100644 --- a/oauth2_http/java/com/google/auth/oauth2/DownscopedCredentials.java +++ b/oauth2_http/java/com/google/auth/oauth2/DownscopedCredentials.java @@ -129,7 +129,20 @@ public AccessToken refreshAccessToken() throws IOException { .setInternalOptions(credentialAccessBoundary.toJson()) .build(); - return handler.exchangeToken().getAccessToken(); + AccessToken downscopedAccessToken = handler.exchangeToken().getAccessToken(); + + // The STS endpoint will only return the expiration time for the downscoped token if the + // original access token represents a service account. + // The downscoped token's expiration time will always match the source credential expiration. + // When no expires_in is returned, we can copy the source credential's expiration time. + if (downscopedAccessToken.getExpirationTime() == null) { + AccessToken sourceAccessToken = this.sourceCredential.getAccessToken(); + if (sourceAccessToken.getExpirationTime() != null) { + return new AccessToken( + downscopedAccessToken.getTokenValue(), sourceAccessToken.getExpirationTime()); + } + } + return downscopedAccessToken; } public GoogleCredentials getSourceCredentials() { diff --git a/oauth2_http/java/com/google/auth/oauth2/StsRequestHandler.java b/oauth2_http/java/com/google/auth/oauth2/StsRequestHandler.java index 60a230fe9..15e9611b1 100644 --- a/oauth2_http/java/com/google/auth/oauth2/StsRequestHandler.java +++ b/oauth2_http/java/com/google/auth/oauth2/StsRequestHandler.java @@ -168,13 +168,14 @@ private StsTokenExchangeResponse buildResponse(GenericData responseData) throws String issuedTokenType = OAuth2Utils.validateString(responseData, "issued_token_type", PARSE_ERROR_PREFIX); String tokenType = OAuth2Utils.validateString(responseData, "token_type", PARSE_ERROR_PREFIX); - Long expiresInSeconds = - OAuth2Utils.validateLong(responseData, "expires_in", PARSE_ERROR_PREFIX); StsTokenExchangeResponse.Builder builder = - StsTokenExchangeResponse.newBuilder( - accessToken, issuedTokenType, tokenType, expiresInSeconds); + StsTokenExchangeResponse.newBuilder(accessToken, issuedTokenType, tokenType); + if (responseData.containsKey("expires_in")) { + builder.setExpiresInSeconds( + OAuth2Utils.validateLong(responseData, "expires_in", PARSE_ERROR_PREFIX)); + } if (responseData.containsKey("refresh_token")) { builder.setRefreshToken( OAuth2Utils.validateString(responseData, "refresh_token", PARSE_ERROR_PREFIX)); diff --git a/oauth2_http/java/com/google/auth/oauth2/StsTokenExchangeResponse.java b/oauth2_http/java/com/google/auth/oauth2/StsTokenExchangeResponse.java index a16f5a329..ef0382689 100644 --- a/oauth2_http/java/com/google/auth/oauth2/StsTokenExchangeResponse.java +++ b/oauth2_http/java/com/google/auth/oauth2/StsTokenExchangeResponse.java @@ -46,8 +46,8 @@ final class StsTokenExchangeResponse { private final AccessToken accessToken; private final String issuedTokenType; private final String tokenType; - private final Long expiresInSeconds; + @Nullable private final Long expiresInSeconds; @Nullable private final String refreshToken; @Nullable private final List scopes; @@ -55,22 +55,25 @@ private StsTokenExchangeResponse( String accessToken, String issuedTokenType, String tokenType, - Long expiresInSeconds, + @Nullable Long expiresInSeconds, @Nullable String refreshToken, @Nullable List scopes) { checkNotNull(accessToken); - this.expiresInSeconds = checkNotNull(expiresInSeconds); - long expiresAtMilliseconds = System.currentTimeMillis() + expiresInSeconds * 1000L; - this.accessToken = new AccessToken(accessToken, new Date(expiresAtMilliseconds)); + + this.expiresInSeconds = expiresInSeconds; + Long expiresAtMilliseconds = + expiresInSeconds == null ? null : System.currentTimeMillis() + expiresInSeconds * 1000L; + Date date = expiresAtMilliseconds == null ? null : new Date(expiresAtMilliseconds); + this.accessToken = new AccessToken(accessToken, date); + this.issuedTokenType = checkNotNull(issuedTokenType); this.tokenType = checkNotNull(tokenType); this.refreshToken = refreshToken; this.scopes = scopes; } - public static Builder newBuilder( - String accessToken, String issuedTokenType, String tokenType, Long expiresIn) { - return new Builder(accessToken, issuedTokenType, tokenType, expiresIn); + public static Builder newBuilder(String accessToken, String issuedTokenType, String tokenType) { + return new Builder(accessToken, issuedTokenType, tokenType); } public AccessToken getAccessToken() { @@ -85,6 +88,7 @@ public String getTokenType() { return tokenType; } + @Nullable public Long getExpiresInSeconds() { return expiresInSeconds; } @@ -106,17 +110,20 @@ public static class Builder { private final String accessToken; private final String issuedTokenType; private final String tokenType; - private final Long expiresInSeconds; + @Nullable private Long expiresInSeconds; @Nullable private String refreshToken; @Nullable private List scopes; - private Builder( - String accessToken, String issuedTokenType, String tokenType, Long expiresInSeconds) { + private Builder(String accessToken, String issuedTokenType, String tokenType) { this.accessToken = accessToken; this.issuedTokenType = issuedTokenType; this.tokenType = tokenType; + } + + public StsTokenExchangeResponse.Builder setExpiresInSeconds(long expiresInSeconds) { this.expiresInSeconds = expiresInSeconds; + return this; } public StsTokenExchangeResponse.Builder setRefreshToken(String refreshToken) { diff --git a/oauth2_http/javatests/com/google/auth/oauth2/DownscopedCredentialsTest.java b/oauth2_http/javatests/com/google/auth/oauth2/DownscopedCredentialsTest.java index 333a7a5ba..c9f98604b 100644 --- a/oauth2_http/javatests/com/google/auth/oauth2/DownscopedCredentialsTest.java +++ b/oauth2_http/javatests/com/google/auth/oauth2/DownscopedCredentialsTest.java @@ -39,6 +39,7 @@ import com.google.auth.TestUtils; import com.google.auth.http.HttpTransportFactory; import java.io.IOException; +import java.util.Date; import java.util.Map; import org.junit.Test; import org.junit.runner.RunWith; @@ -85,7 +86,8 @@ public HttpTransport create() { public void refreshAccessToken() throws IOException { MockStsTransportFactory transportFactory = new MockStsTransportFactory(); - GoogleCredentials sourceCredentials = getSourceCredentials(/* canRefresh= */ true); + GoogleCredentials sourceCredentials = + getServiceAccountSourceCredentials(/* canRefresh= */ true); DownscopedCredentials downscopedCredentials = DownscopedCredentials.newBuilder() @@ -107,11 +109,40 @@ public void refreshAccessToken() throws IOException { "urn:ietf:params:oauth:token-type:access_token", query.get("requested_token_type")); } + @Test + public void refreshAccessToken_userCredentials_expectExpiresInCopied() throws IOException { + // STS only returns expires_in if the source access token belongs to a service account. + // For other source credential types, we can copy the source credentials expiration as + // the generated downscoped token will always have the same expiration time as the source + // credentials. + + MockStsTransportFactory transportFactory = new MockStsTransportFactory(); + transportFactory.transport.setReturnExpiresIn(false); + + GoogleCredentials sourceCredentials = getUserSourceCredentials(); + + DownscopedCredentials downscopedCredentials = + DownscopedCredentials.newBuilder() + .setSourceCredential(sourceCredentials) + .setCredentialAccessBoundary(CREDENTIAL_ACCESS_BOUNDARY) + .setHttpTransportFactory(transportFactory) + .build(); + + AccessToken accessToken = downscopedCredentials.refreshAccessToken(); + + assertEquals(transportFactory.transport.getAccessToken(), accessToken.getTokenValue()); + + // Validate that the expires_in has been copied from the source credential. + assertEquals( + sourceCredentials.getAccessToken().getExpirationTime(), accessToken.getExpirationTime()); + } + @Test public void refreshAccessToken_cantRefreshSourceCredentials_throws() throws IOException { MockStsTransportFactory transportFactory = new MockStsTransportFactory(); - GoogleCredentials sourceCredentials = getSourceCredentials(/* canRefresh= */ false); + GoogleCredentials sourceCredentials = + getServiceAccountSourceCredentials(/* canRefresh= */ false); DownscopedCredentials downscopedCredentials = DownscopedCredentials.newBuilder() @@ -146,7 +177,7 @@ public void builder_noCredentialAccessBoundary_throws() throws IOException { try { DownscopedCredentials.newBuilder() .setHttpTransportFactory(OAuth2Utils.HTTP_TRANSPORT_FACTORY) - .setSourceCredential(getSourceCredentials(/* canRefresh= */ true)) + .setSourceCredential(getServiceAccountSourceCredentials(/* canRefresh= */ true)) .build(); fail("Should fail as no access boundary was provided."); } catch (NullPointerException e) { @@ -156,7 +187,8 @@ public void builder_noCredentialAccessBoundary_throws() throws IOException { @Test public void builder_noTransport_defaults() throws IOException { - GoogleCredentials sourceCredentials = getSourceCredentials(/* canRefresh= */ true); + GoogleCredentials sourceCredentials = + getServiceAccountSourceCredentials(/* canRefresh= */ true); DownscopedCredentials credentials = DownscopedCredentials.newBuilder() .setSourceCredential(sourceCredentials) @@ -170,7 +202,8 @@ public void builder_noTransport_defaults() throws IOException { assertEquals(OAuth2Utils.HTTP_TRANSPORT_FACTORY, credentials.getTransportFactory()); } - private static GoogleCredentials getSourceCredentials(boolean canRefresh) throws IOException { + private static GoogleCredentials getServiceAccountSourceCredentials(boolean canRefresh) + throws IOException { GoogleCredentialsTest.MockTokenServerTransportFactory transportFactory = new GoogleCredentialsTest.MockTokenServerTransportFactory(); @@ -193,4 +226,19 @@ private static GoogleCredentials getSourceCredentials(boolean canRefresh) throws return sourceCredentials; } + + private static GoogleCredentials getUserSourceCredentials() { + GoogleCredentialsTest.MockTokenServerTransportFactory transportFactory = + new GoogleCredentialsTest.MockTokenServerTransportFactory(); + transportFactory.transport.addClient("clientId", "clientSecret"); + transportFactory.transport.addRefreshToken("refreshToken", "accessToken"); + AccessToken accessToken = new AccessToken("accessToken", new Date()); + return UserCredentials.newBuilder() + .setClientId("clientId") + .setClientSecret("clientSecret") + .setRefreshToken("refreshToken") + .setAccessToken(accessToken) + .setHttpTransportFactory(transportFactory) + .build(); + } } diff --git a/oauth2_http/javatests/com/google/auth/oauth2/MockStsTransport.java b/oauth2_http/javatests/com/google/auth/oauth2/MockStsTransport.java index 8ec189b97..1695c8450 100644 --- a/oauth2_http/javatests/com/google/auth/oauth2/MockStsTransport.java +++ b/oauth2_http/javatests/com/google/auth/oauth2/MockStsTransport.java @@ -44,21 +44,44 @@ import com.google.api.client.testing.http.MockLowLevelHttpRequest; import com.google.api.client.testing.http.MockLowLevelHttpResponse; import com.google.auth.TestUtils; +import com.google.common.base.Joiner; import java.io.IOException; +import java.util.ArrayDeque; +import java.util.Collections; +import java.util.List; import java.util.Map; +import java.util.Queue; /** Transport that mocks a basic STS endpoint. */ -public class MockStsTransport extends MockHttpTransport { +public final class MockStsTransport extends MockHttpTransport { private static final String EXPECTED_GRANT_TYPE = "urn:ietf:params:oauth:grant-type:token-exchange"; private static final String ISSUED_TOKEN_TYPE = "urn:ietf:params:oauth:token-type:access_token"; private static final String STS_URL = "https://sts.googleapis.com/v1/token"; private static final String ACCESS_TOKEN = "accessToken"; + private static final String TOKEN_TYPE = "Bearer"; private static final Long EXPIRES_IN = 3600L; + private final Queue responseErrorSequence = new ArrayDeque<>(); + private final Queue> scopeSequence = new ArrayDeque<>(); + private final Queue refreshTokenSequence = new ArrayDeque<>(); + + private boolean returnExpiresIn = true; private MockLowLevelHttpRequest request; + public void addResponseErrorSequence(IOException... errors) { + Collections.addAll(responseErrorSequence, errors); + } + + public void addRefreshTokenSequence(String... refreshTokens) { + Collections.addAll(refreshTokenSequence, refreshTokens); + } + + public void addScopeSequence(List scopes) { + Collections.addAll(scopeSequence, scopes); + } + @Override public LowLevelHttpRequest buildRequest(final String method, final String url) { this.request = @@ -69,6 +92,10 @@ public LowLevelHttpResponse execute() throws IOException { return makeErrorResponse(); } + if (!responseErrorSequence.isEmpty()) { + throw responseErrorSequence.poll(); + } + Map query = TestUtils.parseQuery(getContentAsString()); assertEquals(EXPECTED_GRANT_TYPE, query.get("grant_type")); assertNotNull(query.get("subject_token_type")); @@ -76,11 +103,20 @@ public LowLevelHttpResponse execute() throws IOException { GenericJson response = new GenericJson(); response.setFactory(new GsonFactory()); - response.put("token_type", "Bearer"); - response.put("expires_in", EXPIRES_IN); + response.put("token_type", TOKEN_TYPE); response.put("access_token", ACCESS_TOKEN); response.put("issued_token_type", ISSUED_TOKEN_TYPE); + if (returnExpiresIn) { + response.put("expires_in", EXPIRES_IN); + } + if (!refreshTokenSequence.isEmpty()) { + response.put("refresh_token", refreshTokenSequence.poll()); + } + if (!scopeSequence.isEmpty()) { + response.put("scope", Joiner.on(' ').join(scopeSequence.poll())); + } + return new MockLowLevelHttpResponse() .setContentType(Json.MEDIA_TYPE) .setContent(response.toPrettyString()); @@ -104,4 +140,20 @@ public MockLowLevelHttpRequest getRequest() { public String getAccessToken() { return ACCESS_TOKEN; } + + public String getTokenType() { + return TOKEN_TYPE; + } + + public String getIssuedTokenType() { + return ISSUED_TOKEN_TYPE; + } + + public Long getExpiresIn() { + return EXPIRES_IN; + } + + public void setReturnExpiresIn(boolean returnExpiresIn) { + this.returnExpiresIn = returnExpiresIn; + } } diff --git a/oauth2_http/javatests/com/google/auth/oauth2/StsRequestHandlerTest.java b/oauth2_http/javatests/com/google/auth/oauth2/StsRequestHandlerTest.java index 65d2bf90f..dd0cc7ce9 100644 --- a/oauth2_http/javatests/com/google/auth/oauth2/StsRequestHandlerTest.java +++ b/oauth2_http/javatests/com/google/auth/oauth2/StsRequestHandlerTest.java @@ -60,13 +60,13 @@ public final class StsRequestHandlerTest { "https://www.googleapis.com/auth/cloud-platform"; private static final String DEFAULT_REQUESTED_TOKEN_TYPE = "urn:ietf:params:oauth:token-type:access_token"; - private static final String TOKEN_URL = "https://www.sts.google.com"; + private static final String TOKEN_URL = "https://sts.googleapis.com/v1/token"; - private MockExternalAccountCredentialsTransport transport; + private MockStsTransport transport; @Before public void setup() { - transport = new MockExternalAccountCredentialsTransport(); + transport = new MockStsTransport(); } @Test @@ -248,4 +248,31 @@ public void run() throws Throwable { }); assertEquals(e, thrownException); } + + @Test + public void exchangeToken_noExpiresInReturned() throws IOException { + // Don't return expires in. This happens in the CAB flow when the subject token does not belong + // to a service account. + transport.setReturnExpiresIn(/* returnExpiresIn= */ false); + + StsTokenExchangeRequest stsTokenExchangeRequest = + StsTokenExchangeRequest.newBuilder("credential", "subjectTokenType") + .setScopes(Arrays.asList(CLOUD_PLATFORM_SCOPE)) + .build(); + + StsRequestHandler requestHandler = + StsRequestHandler.newBuilder( + TOKEN_URL, stsTokenExchangeRequest, transport.createRequestFactory()) + .build(); + + StsTokenExchangeResponse response = requestHandler.exchangeToken(); + + // Validate response. + assertEquals(transport.getAccessToken(), response.getAccessToken().getTokenValue()); + assertNull(response.getAccessToken().getExpirationTime()); + + assertEquals(transport.getTokenType(), response.getTokenType()); + assertEquals(transport.getIssuedTokenType(), response.getIssuedTokenType()); + assertNull(response.getExpiresInSeconds()); + } }