Reorder filters and let RealmIdResolver throw NotAuthorized

apache · Jan 22, 2025 · 9c45494 · 9c45494
1 parent 3491e5f
commit 9c45494
Show file tree

Hide file tree

Showing 19 changed files with 266 additions and 70 deletions.
diff --git a/integration-tests/src/main/java/org/apache/polaris/service/it/env/PolarisApiEndpoints.java b/integration-tests/src/main/java/org/apache/polaris/service/it/env/PolarisApiEndpoints.java
@@ -28,7 +28,11 @@
  */
 public final class PolarisApiEndpoints implements Serializable {
 
-  public static String REALM_HEADER = "realm";
+  /**
+   * The header name for the realm ID. Tests must make sure that Polaris is configured with this
+   * header name.
+   */
+  public static String REALM_HEADER = "Polaris-Realm";
 
   private final URI baseUri;
   private final String realm;

diff --git a/integration-tests/src/main/java/org/apache/polaris/service/it/env/RestApi.java b/integration-tests/src/main/java/org/apache/polaris/service/it/env/RestApi.java
@@ -22,6 +22,7 @@
 import jakarta.ws.rs.client.Invocation;
 import jakarta.ws.rs.client.WebTarget;
 import java.net.URI;
+import java.util.HashMap;
 import java.util.Map;
 
 /** Base class for API helper classes. */
@@ -48,6 +49,19 @@ public Invocation.Builder request(String path, Map<String, String> templateValue
 
   public Invocation.Builder request(
       String path, Map<String, String> templateValues, Map<String, String> queryParams) {
+    Map<String, String> headers = new HashMap<>();
+    headers.put(PolarisApiEndpoints.REALM_HEADER, endpoints.realm());
+    if (authToken != null) {
+      headers.put("Authorization", "Bearer " + authToken);
+    }
+    return request(path, templateValues, queryParams, headers);
+  }
+
+  public Invocation.Builder request(
+      String path,
+      Map<String, String> templateValues,
+      Map<String, String> queryParams,
+      Map<String, String> headers) {
     WebTarget target = client.target(uri).path(path);
     for (Map.Entry<String, String> entry : templateValues.entrySet()) {
       target = target.resolveTemplate(entry.getKey(), entry.getValue());
@@ -56,10 +70,7 @@ public Invocation.Builder request(
       target = target.queryParam(entry.getKey(), entry.getValue());
     }
     Invocation.Builder request = target.request("application/json");
-    request = request.header(PolarisApiEndpoints.REALM_HEADER, endpoints.realm());
-    if (authToken != null) {
-      request = request.header("Authorization", "Bearer " + authToken);
-    }
+    headers.forEach(request::header);
     return request;
   }
 }
diff --git a/...s/src/main/java/org/apache/polaris/service/it/test/PolarisApplicationIntegrationTest.java b/...s/src/main/java/org/apache/polaris/service/it/test/PolarisApplicationIntegrationTest.java
@@ -28,6 +28,7 @@
 import jakarta.ws.rs.client.Entity;
 import jakarta.ws.rs.client.Invocation;
 import jakarta.ws.rs.core.Response;
+import jakarta.ws.rs.core.Response.Status;
 import java.io.IOException;
 import java.nio.file.Files;
 import java.nio.file.Path;
@@ -64,6 +65,7 @@
 import org.apache.polaris.core.admin.model.Catalog;
 import org.apache.polaris.core.admin.model.CatalogProperties;
 import org.apache.polaris.core.admin.model.CatalogRole;
+import org.apache.polaris.core.admin.model.Catalogs;
 import org.apache.polaris.core.admin.model.ExternalCatalog;
 import org.apache.polaris.core.admin.model.FileStorageConfigInfo;
 import org.apache.polaris.core.admin.model.PolarisCatalog;
@@ -85,6 +87,8 @@
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.TestInfo;
 import org.junit.jupiter.api.extension.ExtendWith;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
 
 /**
  * @implSpec This test expects the server to be configured with the following features configured:
@@ -97,6 +101,8 @@
  *     </ul>
  *     The server must also be configured to reject request body sizes larger than 1MB (1000000
  *     bytes).
+ *     <p>The server must also be configured with the following realms: POLARIS (default), and
+ *     OTHER.
  */
 @ExtendWith(PolarisIntegrationTestExtension.class)
 public class PolarisApplicationIntegrationTest {
@@ -657,4 +663,52 @@ public void testRequestBodyTooLarge() throws Exception {
               });
     }
   }
+
+  @Test
+  public void testNoRealmHeader() {
+    try (Response response =
+        managementApi
+            .request(
+                "v1/catalogs", Map.of(), Map.of(), Map.of("Authorization", "Bearer " + authToken))
+            .get()) {
+      assertThat(response.getStatus()).isEqualTo(Status.OK.getStatusCode());
+      Catalogs roles = response.readEntity(Catalogs.class);
+      assertThat(roles.getCatalogs()).extracting(Catalog::getName).contains(internalCatalogName);
+    }
+  }
+
+  @ParameterizedTest
+  @ValueSource(strings = {"POLARIS", "OTHER"})
+  public void testRealmHeaderValid(String realmId) {
+    try (Response response =
+        managementApi
+            .request(
+                "v1/catalogs",
+                Map.of(),
+                Map.of(),
+                Map.of("Authorization", "Bearer " + authToken, REALM_HEADER, realmId))
+            .get()) {
+      assertThat(response.getStatus()).isEqualTo(Status.OK.getStatusCode());
+      Catalogs roles = response.readEntity(Catalogs.class);
+      if ("POLARIS".equals(realmId)) {
+        assertThat(roles.getCatalogs()).extracting(Catalog::getName).contains(internalCatalogName);
+      } else {
+        assertThat(roles.getCatalogs()).isEmpty();
+      }
+    }
+  }
+
+  @Test
+  public void testRealmHeaderInvalid() {
+    try (Response response =
+        managementApi
+            .request(
+                "v1/catalogs",
+                Map.of(),
+                Map.of(),
+                Map.of("Authorization", "Bearer " + authToken, REALM_HEADER, "INVALID"))
+            .get()) {
+      assertThat(response.getStatus()).isEqualTo(Status.UNAUTHORIZED.getStatusCode());
+    }
+  }
 }
diff --git a/quarkus/defaults/src/main/resources/application-it.properties b/quarkus/defaults/src/main/resources/application-it.properties
@@ -37,6 +37,8 @@ polaris.features.defaults."INITIALIZE_DEFAULT_CATALOG_FILEIO_FOR_it"=true
 polaris.features.defaults."SKIP_CREDENTIAL_SUBSCOPING_INDIRECTION"=true
 polaris.features.defaults."SUPPORTED_CATALOG_STORAGE_TYPES"=["FILE","S3","GCS","AZURE"]
 
+polaris.realm-context.realms=POLARIS,OTHER
+
 polaris.storage.gcp.token=token
 polaris.storage.gcp.lifespan=PT1H
 
diff --git a/...vice/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusFilterPriorities.java b/...vice/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusFilterPriorities.java
@@ -0,0 +1,26 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.service.quarkus.config;
+
+import org.apache.polaris.service.config.PolarisFilterPriorities;
+
+public final class QuarkusFilterPriorities {
+  public static final int MDC_FILTER = PolarisFilterPriorities.REALM_ID_FILTER + 1;
+  public static final int TRACING_FILTER = PolarisFilterPriorities.REALM_ID_FILTER + 2;
+}
diff --git a/...kus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusProducers.java b/...kus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusProducers.java
@@ -21,7 +21,6 @@
 import io.quarkus.runtime.StartupEvent;
 import io.smallrye.common.annotation.Identifier;
 import io.smallrye.context.SmallRyeManagedExecutor;
-import io.vertx.core.http.HttpServerRequest;
 import jakarta.enterprise.context.ApplicationScoped;
 import jakarta.enterprise.context.RequestScoped;
 import jakarta.enterprise.event.Observes;
@@ -30,9 +29,9 @@
 import jakarta.enterprise.inject.Instance;
 import jakarta.enterprise.inject.Produces;
 import jakarta.inject.Singleton;
+import jakarta.ws.rs.container.ContainerRequestContext;
 import jakarta.ws.rs.core.Context;
 import java.time.Clock;
-import java.util.HashMap;
 import org.apache.polaris.core.PolarisConfigurationStore;
 import org.apache.polaris.core.PolarisDefaultDiagServiceImpl;
 import org.apache.polaris.core.PolarisDiagnostics;
@@ -53,6 +52,7 @@
 import org.apache.polaris.service.catalog.io.FileIOFactory;
 import org.apache.polaris.service.config.RealmEntityManagerFactory;
 import org.apache.polaris.service.context.RealmContextConfiguration;
+import org.apache.polaris.service.context.RealmIdFilter;
 import org.apache.polaris.service.context.RealmIdResolver;
 import org.apache.polaris.service.persistence.InMemoryPolarisMetaStoreManagerFactory;
 import org.apache.polaris.service.quarkus.auth.QuarkusAuthenticationConfiguration;
@@ -100,13 +100,8 @@ public PolarisDiagnostics polarisDiagnostics() {
 
   @Produces
   @RequestScoped
-  public RealmId realmId(@Context HttpServerRequest request, RealmIdResolver realmIdResolver) {
-    return realmIdResolver.resolveRealmContext(
-        request.absoluteURI(),
-        request.method().name(),
-        request.path(),
-        request.headers().entries().stream()
-            .collect(HashMap::new, (m, e) -> m.put(e.getKey(), e.getValue()), HashMap::putAll));
+  public RealmId realmId(@Context ContainerRequestContext request) {
+    return (RealmId) request.getProperty(RealmIdFilter.REALM_ID_KEY);
   }
 
   @Produces

diff --git a/...ice/src/main/java/org/apache/polaris/service/quarkus/logging/QuarkusLoggingMDCFilter.java b/...ice/src/main/java/org/apache/polaris/service/quarkus/logging/QuarkusLoggingMDCFilter.java
@@ -18,55 +18,42 @@
  */
 package org.apache.polaris.service.quarkus.logging;
 
-import io.quarkus.vertx.web.RouteFilter;
-import io.vertx.ext.web.RoutingContext;
+import static org.apache.polaris.service.context.RealmIdFilter.REALM_ID_KEY;
+
+import jakarta.annotation.Priority;
 import jakarta.enterprise.context.ApplicationScoped;
 import jakarta.inject.Inject;
+import jakarta.ws.rs.container.ContainerRequestContext;
+import jakarta.ws.rs.container.ContainerRequestFilter;
+import jakarta.ws.rs.container.PreMatching;
+import jakarta.ws.rs.ext.Provider;
 import org.apache.polaris.core.context.RealmId;
+import org.apache.polaris.service.quarkus.config.QuarkusFilterPriorities;
 import org.slf4j.MDC;
 
+@PreMatching
 @ApplicationScoped
-public class QuarkusLoggingMDCFilter {
-
-  public static final int PRIORITY = RouteFilter.DEFAULT_PRIORITY + 100;
+@Priority(QuarkusFilterPriorities.MDC_FILTER)
+@Provider
+public class QuarkusLoggingMDCFilter implements ContainerRequestFilter {
 
-  private static final String REQUEST_ID_KEY = "requestId";
-  private static final String REALM_ID_KEY = "realmId";
-
-  @Inject RealmId realmId;
+  public static final String REQUEST_ID_KEY = "requestId";
 
   @Inject QuarkusLoggingConfiguration loggingConfiguration;
 
-  public static String requestId(RoutingContext rc) {
-    return rc.get(REQUEST_ID_KEY);
-  }
-
-  public static String realmId(RoutingContext rc) {
-    return rc.get(REALM_ID_KEY);
-  }
-
-  @RouteFilter(value = PRIORITY)
-  public void applyMDCContext(RoutingContext rc) {
+  @Override
+  public void filter(ContainerRequestContext rc) {
     // The request scope is active here, so any MDC values set here will be propagated to
     // threads handling the request.
     // Also put the MDC values in the request context for use by other filters and handlers
     loggingConfiguration.mdc().forEach(MDC::put);
-    loggingConfiguration.mdc().forEach(rc::put);
-    var requestId = rc.request().getHeader(loggingConfiguration.requestIdHeaderName());
+    loggingConfiguration.mdc().forEach(rc::setProperty);
+    var requestId = rc.getHeaderString(loggingConfiguration.requestIdHeaderName());
     if (requestId != null) {
       MDC.put(REQUEST_ID_KEY, requestId);
-      rc.put(REQUEST_ID_KEY, requestId);
+      rc.setProperty(REQUEST_ID_KEY, requestId);
     }
+    RealmId realmId = (RealmId) rc.getProperty(REALM_ID_KEY);
     MDC.put(REALM_ID_KEY, realmId.id());
-    rc.put(REALM_ID_KEY, realmId.id());
-    // Do not explicitly remove the MDC values from the request context with an end handler,
-    // as this could remove MDC context still in use in TaskExecutor threads
-    //    rc.addEndHandler(
-    //        (v) -> {
-    //          MDC.remove(REQUEST_ID_MDC_KEY);
-    //          MDC.remove(REALM_ID_MDC_KEY);
-    //          loggingConfiguration.mdc().keySet().forEach(MDC::remove);
-    //        });
-    rc.next();
   }
 }
diff --git a/...rvice/src/main/java/org/apache/polaris/service/quarkus/metrics/RealmIdTagContributor.java b/...rvice/src/main/java/org/apache/polaris/service/quarkus/metrics/RealmIdTagContributor.java
@@ -38,12 +38,17 @@ public class RealmIdTagContributor implements HttpServerMetricsTagsContributor {
   public Tags contribute(Context context) {
     // FIXME request scope does not work here, so we have to resolve the realm context manually
     HttpServerRequest request = context.request();
-    RealmId realmId = resolveRealmContext(request);
-    return Tags.of(TAG_REALM, realmId.id());
+    try {
+      RealmId realmId = resolveRealmContext(request);
+      return Tags.of(TAG_REALM, realmId.id());
+    } catch (Exception ignored) {
+      // ignore, the RealmIdFilter will handle the error
+      return Tags.empty();
+    }
   }
 
   private RealmId resolveRealmContext(HttpServerRequest request) {
-    return realmIdResolver.resolveRealmContext(
+    return realmIdResolver.resolveRealmId(
         request.absoluteURI(),
         request.method().name(),
         request.path(),

diff --git a/...ervice/src/main/java/org/apache/polaris/service/quarkus/tracing/QuarkusTracingFilter.java b/...ervice/src/main/java/org/apache/polaris/service/quarkus/tracing/QuarkusTracingFilter.java
@@ -19,32 +19,40 @@
 package org.apache.polaris.service.quarkus.tracing;
 
 import io.opentelemetry.api.trace.Span;
-import io.quarkus.vertx.web.RouteFilter;
-import io.vertx.ext.web.RoutingContext;
+import jakarta.annotation.Priority;
 import jakarta.enterprise.context.ApplicationScoped;
+import jakarta.ws.rs.container.ContainerRequestContext;
+import jakarta.ws.rs.container.ContainerRequestFilter;
+import jakarta.ws.rs.container.PreMatching;
+import jakarta.ws.rs.ext.Provider;
+import org.apache.polaris.core.context.RealmId;
+import org.apache.polaris.service.context.RealmIdFilter;
+import org.apache.polaris.service.quarkus.config.QuarkusFilterPriorities;
 import org.apache.polaris.service.quarkus.logging.QuarkusLoggingMDCFilter;
 import org.eclipse.microprofile.config.inject.ConfigProperty;
 
+@PreMatching
 @ApplicationScoped
-public class QuarkusTracingFilter {
+@Priority(QuarkusFilterPriorities.TRACING_FILTER)
+@Provider
+public class QuarkusTracingFilter implements ContainerRequestFilter {
 
   public static final String REQUEST_ID_ATTRIBUTE = "polaris.request.id";
   public static final String REALM_ID_ATTRIBUTE = "polaris.realm";
 
   @ConfigProperty(name = "quarkus.otel.sdk.disabled")
   boolean sdkDisabled;
 
-  @RouteFilter(QuarkusLoggingMDCFilter.PRIORITY - 1)
-  public void applySpanAttributes(RoutingContext rc) {
+  @Override
+  public void filter(ContainerRequestContext rc) {
     if (!sdkDisabled) {
       Span span = Span.current();
-      String requestId = QuarkusLoggingMDCFilter.requestId(rc);
-      String realmId = QuarkusLoggingMDCFilter.realmId(rc);
+      String requestId = (String) rc.getProperty(QuarkusLoggingMDCFilter.REQUEST_ID_KEY);
+      RealmId realmId = (RealmId) rc.getProperty(RealmIdFilter.REALM_ID_KEY);
       if (requestId != null) {
         span.setAttribute(REQUEST_ID_ATTRIBUTE, requestId);
       }
-      span.setAttribute(REALM_ID_ATTRIBUTE, realmId);
+      span.setAttribute(REALM_ID_ATTRIBUTE, realmId.id());
     }
-    rc.next();
   }
 }
diff --git a/...rc/test/java/org/apache/polaris/service/quarkus/it/QuarkusApplicationIntegrationTest.java b/...rc/test/java/org/apache/polaris/service/quarkus/it/QuarkusApplicationIntegrationTest.java
@@ -48,6 +48,7 @@ public static class Profile implements QuarkusTestProfile {
     public Map<String, String> getConfigOverrides() {
       return Map.of(
           "quarkus.http.limits.max-body-size", "1000000",
+          "polaris.realm-context.realms", "POLARIS,OTHER",
           "polaris.features.defaults.\"ALLOW_OVERLAPPING_CATALOG_URLS\"", "true",
           "polaris.features.defaults.\"SKIP_CREDENTIAL_SUBSCOPING_INDIRECTION\"", "true");
     }

diff --git a/...ice/src/testFixtures/java/org/apache/polaris/service/quarkus/it/QuarkusServerManager.java b/...ice/src/testFixtures/java/org/apache/polaris/service/quarkus/it/QuarkusServerManager.java
@@ -28,15 +28,15 @@
 
 public class QuarkusServerManager implements PolarisServerManager {
 
-  private static final String TEST_REALM = "POLARIS";
+  private static final String DEFAULT_REALM_ID = "POLARIS";
 
   @Override
   public Server serverForContext(ExtensionContext context) {
     return new Server() {
 
       @Override
       public String realmId() {
-        return TEST_REALM;
+        return DEFAULT_REALM_ID;
       }
 
       @Override