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>
apache · Oct 2, 2024 · dfe5540 · dfe5540
1 parent 99e7e69
commit dfe5540
Show file tree

Hide file tree

Showing 2 changed files with 43 additions and 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
@@ -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/...s-service/src/test/java/org/apache/polaris/service/PolarisApplicationIntegrationTest.java b/...s-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);
+ }
+ }
 }