From fe2d61d5a44344042ec1994d0943cfc7977fbdcd Mon Sep 17 00:00:00 2001 From: Qiang Zhao Date: Tue, 24 Oct 2023 19:08:21 +0800 Subject: [PATCH] [fix][proxy] Move status endpoint out of auth coverage (#21428) --- .../proxy/server/ProxyServiceStarter.java | 6 +++-- .../apache/pulsar/proxy/server/WebServer.java | 27 ++++++++++++++++++- .../server/ProxyWithJwtAuthorizationTest.java | 24 +++++++++++++++++ 3 files changed, 54 insertions(+), 3 deletions(-) diff --git a/pulsar-proxy/src/main/java/org/apache/pulsar/proxy/server/ProxyServiceStarter.java b/pulsar-proxy/src/main/java/org/apache/pulsar/proxy/server/ProxyServiceStarter.java index ee8f648182dac..7427331641318 100644 --- a/pulsar-proxy/src/main/java/org/apache/pulsar/proxy/server/ProxyServiceStarter.java +++ b/pulsar-proxy/src/main/java/org/apache/pulsar/proxy/server/ProxyServiceStarter.java @@ -253,9 +253,11 @@ public static void addWebServerHandlers(WebServer server, ProxyConfiguration config, ProxyService service, BrokerDiscoveryProvider discoveryProvider) throws Exception { + // We can make 'status.html' publicly accessible without authentication since + // it does not contain any sensitive data. + server.addRestResource("/", VipStatus.ATTRIBUTE_STATUS_FILE_PATH, config.getStatusFilePath(), + VipStatus.class, false); if (config.isEnableProxyStatsEndpoints()) { - server.addRestResource("/", VipStatus.ATTRIBUTE_STATUS_FILE_PATH, config.getStatusFilePath(), - VipStatus.class); server.addRestResource("/proxy-stats", ProxyStats.ATTRIBUTE_PULSAR_PROXY_NAME, service, ProxyStats.class); if (service != null) { diff --git a/pulsar-proxy/src/main/java/org/apache/pulsar/proxy/server/WebServer.java b/pulsar-proxy/src/main/java/org/apache/pulsar/proxy/server/WebServer.java index edbcfe0847c4e..b95bbcab08b11 100644 --- a/pulsar-proxy/src/main/java/org/apache/pulsar/proxy/server/WebServer.java +++ b/pulsar-proxy/src/main/java/org/apache/pulsar/proxy/server/WebServer.java @@ -239,7 +239,31 @@ private static void popularServletParams(ServletHolder servletHolder, ProxyConfi } } + /** + * Add a REST resource to the servlet context with authentication coverage. + * + * @see WebServer#addRestResource(String, String, Object, Class, boolean) + * + * @param basePath The base path for the resource. + * @param attribute An attribute associated with the resource. + * @param attributeValue The value of the attribute. + * @param resourceClass The class representing the resource. + */ public void addRestResource(String basePath, String attribute, Object attributeValue, Class resourceClass) { + addRestResource(basePath, attribute, attributeValue, resourceClass, true); + } + + /** + * Add a REST resource to the servlet context. + * + * @param basePath The base path for the resource. + * @param attribute An attribute associated with the resource. + * @param attributeValue The value of the attribute. + * @param resourceClass The class representing the resource. + * @param requireAuthentication A boolean indicating whether authentication is required for this resource. + */ + public void addRestResource(String basePath, String attribute, Object attributeValue, + Class resourceClass, boolean requireAuthentication) { ResourceConfig config = new ResourceConfig(); config.register(resourceClass); config.register(JsonMapperProvider.class); @@ -247,7 +271,8 @@ public void addRestResource(String basePath, String attribute, Object attributeV servletHolder.setAsyncSupported(true); // This method has not historically checked for existing paths, so we don't check here either. The // method call is added to reduce code duplication. - addServlet(basePath, servletHolder, Collections.singletonList(Pair.of(attribute, attributeValue)), true, false); + addServlet(basePath, servletHolder, Collections.singletonList(Pair.of(attribute, attributeValue)), + requireAuthentication, false); } public int getExternalServicePort() { diff --git a/pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyWithJwtAuthorizationTest.java b/pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyWithJwtAuthorizationTest.java index 500d940930e23..37465b21322bc 100644 --- a/pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyWithJwtAuthorizationTest.java +++ b/pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyWithJwtAuthorizationTest.java @@ -116,6 +116,7 @@ protected void setup() throws Exception { proxyConfig.setBrokerClientAuthenticationPlugin(AuthenticationToken.class.getName()); proxyConfig.setBrokerClientAuthenticationParameters(PROXY_TOKEN); proxyConfig.setAuthenticationProviders(providers); + proxyConfig.setStatusFilePath("./src/test/resources/vip_status.html"); AuthenticationService authService = new AuthenticationService(PulsarConfigurationLoader.convertFrom(proxyConfig)); @@ -405,6 +406,29 @@ public void testProxyAuthorizationWithPrefixSubscriptionAuthMode() throws Except log.info("-- Exiting {} test --", methodName); } + @Test + void testGetStatus() throws Exception { + log.info("-- Starting {} test --", methodName); + final PulsarResources resource = new PulsarResources(new ZKMetadataStore(mockZooKeeper), + new ZKMetadataStore(mockZooKeeperGlobal)); + final AuthenticationService authService = new AuthenticationService( + PulsarConfigurationLoader.convertFrom(proxyConfig)); + final WebServer webServer = new WebServer(proxyConfig, authService); + ProxyServiceStarter.addWebServerHandlers(webServer, proxyConfig, proxyService, + new BrokerDiscoveryProvider(proxyConfig, resource)); + webServer.start(); + @Cleanup + final Client client = javax.ws.rs.client.ClientBuilder + .newClient(new ClientConfig().register(LoggingFeature.class)); + try { + final Response r = client.target(webServer.getServiceUri()).path("/status.html").request().get(); + Assert.assertEquals(r.getStatus(), Response.Status.OK.getStatusCode()); + } finally { + webServer.stop(); + } + log.info("-- Exiting {} test --", methodName); + } + @Test void testGetMetrics() throws Exception { log.info("-- Starting {} test --", methodName);