From dfe554096cae5605f8f01a9006dda87a18f4abf7 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Wed, 2 Oct 2024 14:33:02 -0400 Subject: [PATCH] Do not URL-decode refresh header (#332) * apply fix * autolint * Add test * autolint * clean up test; still fails * polish test more; still 404 * autolint * also mock path * autolint --------- Co-authored-by: Michael Collado <40346148+collado-mike@users.noreply.github.com> --- .../service/auth/DefaultOAuth2ApiService.java | 6 +-- .../PolarisApplicationIntegrationTest.java | 41 +++++++++++++++++++ 2 files changed, 43 insertions(+), 4 deletions(-) diff --git a/polaris-service/src/main/java/org/apache/polaris/service/auth/DefaultOAuth2ApiService.java b/polaris-service/src/main/java/org/apache/polaris/service/auth/DefaultOAuth2ApiService.java index 49854ee86..ff1a57dfd 100644 --- a/polaris-service/src/main/java/org/apache/polaris/service/auth/DefaultOAuth2ApiService.java +++ b/polaris-service/src/main/java/org/apache/polaris/service/auth/DefaultOAuth2ApiService.java @@ -23,8 +23,6 @@ import com.fasterxml.jackson.annotation.JsonTypeName; import jakarta.ws.rs.core.Response; import jakarta.ws.rs.core.SecurityContext; -import java.net.URLDecoder; -import java.nio.charset.Charset; import org.apache.commons.codec.binary.Base64; import org.apache.hadoop.hdfs.web.oauth2.OAuth2Constants; import org.apache.iceberg.rest.auth.OAuth2Properties; @@ -80,8 +78,8 @@ public Response getToken( } LOGGER.debug("Found credentials in auth header - treating as client_credentials"); String[] parts = credentials.split(":", 2); - clientId = URLDecoder.decode(parts[0], Charset.defaultCharset()); - clientSecret = URLDecoder.decode(parts[1], Charset.defaultCharset()); + clientId = parts[0]; + clientSecret = parts[1]; } TokenResponse tokenResponse = switch (subjectTokenType) { diff --git a/polaris-service/src/test/java/org/apache/polaris/service/PolarisApplicationIntegrationTest.java b/polaris-service/src/test/java/org/apache/polaris/service/PolarisApplicationIntegrationTest.java index 3795bdd76..268f17f03 100644 --- a/polaris-service/src/test/java/org/apache/polaris/service/PolarisApplicationIntegrationTest.java +++ b/polaris-service/src/test/java/org/apache/polaris/service/PolarisApplicationIntegrationTest.java @@ -22,6 +22,8 @@ import static org.apache.polaris.service.throttling.RequestThrottlingErrorResponse.RequestThrottlingErrorType.REQUEST_TOO_LARGE; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.when; import io.dropwizard.testing.ConfigOverride; import io.dropwizard.testing.ResourceHelpers; @@ -57,8 +59,13 @@ import org.apache.iceberg.exceptions.RESTException; import org.apache.iceberg.hadoop.HadoopFileIO; import org.apache.iceberg.io.ResolvingFileIO; +import org.apache.iceberg.rest.HTTPClient; +import org.apache.iceberg.rest.RESTClient; import org.apache.iceberg.rest.RESTSessionCatalog; +import org.apache.iceberg.rest.auth.AuthConfig; +import org.apache.iceberg.rest.auth.ImmutableAuthConfig; import org.apache.iceberg.rest.auth.OAuth2Properties; +import org.apache.iceberg.rest.auth.OAuth2Util; import org.apache.iceberg.types.Types; import org.apache.iceberg.util.EnvironmentUtil; import org.apache.polaris.core.admin.model.AwsStorageConfigInfo; @@ -88,6 +95,7 @@ import org.junit.jupiter.api.extension.ExtendWith; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.testcontainers.shaded.com.google.common.collect.ImmutableMap; @ExtendWith({ DropwizardExtensionsSupport.class, @@ -699,4 +707,37 @@ public void testRequestBodyTooLarge() { .equals(REQUEST_TOO_LARGE)); } } + + @Test + public void testRefreshToken() throws IOException { + String path = + String.format("http://localhost:%d/api/catalog/v1/oauth/tokens", EXT.getLocalPort()); + try (RESTClient client = + HTTPClient.builder(ImmutableMap.of()) + .withHeader(REALM_PROPERTY_KEY, realm) + .uri(path) + .build()) { + String credentialString = + snowmanCredentials.clientId() + ":" + snowmanCredentials.clientSecret(); + var authConfig = + AuthConfig.builder().credential(credentialString).scope("PRINCIPAL_ROLE:ALL").build(); + ImmutableAuthConfig configSpy = spy(authConfig); + when(configSpy.expiresAtMillis()).thenReturn(0L); + assertThat(configSpy.expiresAtMillis()).isEqualTo(0L); + when(configSpy.oauth2ServerUri()).thenReturn(path); + + var parentSession = new OAuth2Util.AuthSession(Map.of(), configSpy); + var session = + OAuth2Util.AuthSession.fromAccessToken(client, null, userToken, 0L, parentSession); + + OAuth2Util.AuthSession sessionSpy = spy(session); + when(sessionSpy.expiresAtMillis()).thenReturn(0L); + assertThat(sessionSpy.expiresAtMillis()).isEqualTo(0L); + assertThat(sessionSpy.token()).isEqualTo(userToken); + + sessionSpy.refresh(client); + assertThat(sessionSpy.credential()).isNotNull(); + assertThat(sessionSpy.credential()).isNotEqualTo(userToken); + } + } }