(helix) branch master updated: Improve CORS filter making it configurable

[jx...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

asf-gitbox-commits pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/helix.git


The following commit(s) were added to refs/heads/master by this push:
     new becac6891 Improve CORS filter making it configurable
becac6891 is described below

commit becac68913a25b98ffa36454668487c12d425f05
Author: Junkai Xue <jx...@apache.org>
AuthorDate: Tue Jun 23 14:45:52 2026 -0700

    Improve CORS filter making it configurable
---
 .../helix/rest/server/filters/CORSFilter.java      |  84 ++++++++++--
 .../helix/rest/server/filters/TestCORSFilter.java  | 142 +++++++++++++++++++++
 2 files changed, 213 insertions(+), 13 deletions(-)

diff --git a/helix-rest/src/main/java/org/apache/helix/rest/server/filters/CORSFilter.java b/helix-rest/src/main/java/org/apache/helix/rest/server/filters/CORSFilter.java
index dac0ea133..48c8d7660 100644
--- a/helix-rest/src/main/java/org/apache/helix/rest/server/filters/CORSFilter.java
+++ b/helix-rest/src/main/java/org/apache/helix/rest/server/filters/CORSFilter.java
@@ -20,6 +20,11 @@ package org.apache.helix.rest.server.filters;
  */
 
 import java.io.IOException;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Set;
+import java.util.stream.Collectors;
 import javax.ws.rs.container.ContainerRequestContext;
 import javax.ws.rs.container.ContainerRequestFilter;
 import javax.ws.rs.container.ContainerResponseContext;
@@ -30,24 +35,65 @@ import javax.ws.rs.ext.Provider;
 @Provider
 public class CORSFilter implements ContainerRequestFilter, ContainerResponseFilter {
 
+  /**
+   * Comma-separated list of origins allowed to make cross-origin requests. A single {@code "*"}
+   * enables anonymous wildcard access (without credentials). When unset, no cross-origin requests
+   * are permitted.
+   */
+  public static final String ALLOWED_ORIGINS_PROPERTY = "cors.allowed.origins";
+
+  private static final String WILDCARD = "*";
+  // Helix REST only exposes these HTTP methods; do not reflect arbitrary requested methods.
+  private static final String ALLOWED_METHODS = "GET, POST, PUT, DELETE, OPTIONS, HEAD";
+
+  private final Set<String> _allowedOrigins;
+  private final boolean _allowAnyOrigin;
+
+  public CORSFilter() {
+    this(System.getProperty(ALLOWED_ORIGINS_PROPERTY));
+  }
+
+  CORSFilter(String allowedOriginsConfig) {
+    Set<String> origins = new HashSet<>();
+    if (allowedOriginsConfig != null) {
+      origins = Arrays.stream(allowedOriginsConfig.split(","))
+          .map(String::trim)
+          .filter(origin -> !origin.isEmpty())
+          .collect(Collectors.toSet());
+    }
+    _allowAnyOrigin = origins.contains(WILDCARD);
+    _allowedOrigins = Collections.unmodifiableSet(origins);
+  }
+
+  private boolean isAllowedOrigin(String origin) {
+    return origin != null && (_allowAnyOrigin || _allowedOrigins.contains(origin));
+  }
+
   @Override
   public void filter(ContainerRequestContext request) throws IOException {
     // handle preflight
     if (request.getMethod().equalsIgnoreCase("OPTIONS")) {
       Response.ResponseBuilder builder = Response.ok();
+      String origin = request.getHeaderString("Origin");
 
-      // NOTE: Allow whichever HTTP Methods requested in the incoming request because the cluster
-      // administrator must be able to trigger such operations
-      // Helix REST uses GET, PUT, DELETE, POST
-      String requestMethods = request.getHeaderString("Access-Control-Request-Method");
-      if (requestMethods != null) {
-        builder.header("Access-Control-Allow-Methods", requestMethods);
-      }
+      if (isAllowedOrigin(origin)) {
+        if (_allowAnyOrigin && !_allowedOrigins.contains(origin)) {
+          // Wildcard, anonymous access only: never advertise credentials with "*".
+          builder.header("Access-Control-Allow-Origin", WILDCARD);
+        } else {
+          // Echo the specific allowed origin and permit credentials.
+          builder.header("Access-Control-Allow-Origin", origin);
+          builder.header("Access-Control-Allow-Credentials", "true");
+          builder.header("Vary", "Origin");
+        }
+
+        builder.header("Access-Control-Allow-Methods", ALLOWED_METHODS);
 
-      // NOTE: All headers must be allowed for preflight
-      String allowHeaders = request.getHeaderString("Access-Control-Request-Headers");
-      if (allowHeaders != null) {
-        builder.header("Access-Control-Allow-Headers", allowHeaders);
+        // Reflect requested headers only for an already-validated allowed origin.
+        String allowHeaders = request.getHeaderString("Access-Control-Request-Headers");
+        if (allowHeaders != null) {
+          builder.header("Access-Control-Allow-Headers", allowHeaders);
+        }
       }
 
       request.abortWith(builder.build());
@@ -58,7 +104,19 @@ public class CORSFilter implements ContainerRequestFilter, ContainerResponseFilt
   public void filter(ContainerRequestContext request,
         ContainerResponseContext response) throws IOException {
     // handle origin
-    response.getHeaders().putSingle("Access-Control-Allow-Origin", "*");
-    response.getHeaders().putSingle("Access-Control-Allow-Credentials", "true");
+    String origin = request.getHeaderString("Origin");
+    if (!isAllowedOrigin(origin)) {
+      return;
+    }
+
+    if (_allowAnyOrigin && !_allowedOrigins.contains(origin)) {
+      // Wildcard, anonymous access only: never advertise credentials with "*".
+      response.getHeaders().putSingle("Access-Control-Allow-Origin", WILDCARD);
+    } else {
+      // Echo the specific allowed origin and permit credentials.
+      response.getHeaders().putSingle("Access-Control-Allow-Origin", origin);
+      response.getHeaders().putSingle("Access-Control-Allow-Credentials", "true");
+      response.getHeaders().add("Vary", "Origin");
+    }
   }
 }
diff --git a/helix-rest/src/test/java/org/apache/helix/rest/server/filters/TestCORSFilter.java b/helix-rest/src/test/java/org/apache/helix/rest/server/filters/TestCORSFilter.java
new file mode 100644
index 000000000..e3b690d2d
--- /dev/null
+++ b/helix-rest/src/test/java/org/apache/helix/rest/server/filters/TestCORSFilter.java
@@ -0,0 +1,142 @@
+package org.apache.helix.rest.server.filters;
+
+/*
+ * 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.
+ */
+
+import java.io.IOException;
+import javax.ws.rs.container.ContainerRequestContext;
+import javax.ws.rs.container.ContainerResponseContext;
+import javax.ws.rs.core.MultivaluedHashMap;
+import javax.ws.rs.core.MultivaluedMap;
+import javax.ws.rs.core.Response;
+
+import org.mockito.ArgumentCaptor;
+import org.testng.Assert;
+import org.testng.annotations.Test;
+
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+public class TestCORSFilter {
+  private static final String ALLOWED_ORIGIN = "https://admin.example.com";
+  private static final String DISALLOWED_ORIGIN = "https://evil.example.com";
+
+  @Test
+  public void testResponseAllowedOriginEchoesOriginAndCredentials() throws IOException {
+    CORSFilter filter = new CORSFilter(ALLOWED_ORIGIN);
+    ContainerRequestContext request = mockRequest("GET", ALLOWED_ORIGIN);
+    MultivaluedMap<String, Object> headers = new MultivaluedHashMap<>();
+    ContainerResponseContext response = mock(ContainerResponseContext.class);
+    when(response.getHeaders()).thenReturn(headers);
+
+    filter.filter(request, response);
+
+    Assert.assertEquals(headers.getFirst("Access-Control-Allow-Origin"), ALLOWED_ORIGIN);
+    Assert.assertEquals(headers.getFirst("Access-Control-Allow-Credentials"), "true");
+    Assert.assertTrue(headers.get("Vary").contains("Origin"));
+  }
+
+  @Test
+  public void testResponseDisallowedOriginAddsNoHeaders() throws IOException {
+    CORSFilter filter = new CORSFilter(ALLOWED_ORIGIN);
+    ContainerRequestContext request = mockRequest("GET", DISALLOWED_ORIGIN);
+    MultivaluedMap<String, Object> headers = new MultivaluedHashMap<>();
+    ContainerResponseContext response = mock(ContainerResponseContext.class);
+    when(response.getHeaders()).thenReturn(headers);
+
+    filter.filter(request, response);
+
+    Assert.assertTrue(headers.isEmpty());
+  }
+
+  @Test
+  public void testResponseWildcardNeverEmitsCredentials() throws IOException {
+    CORSFilter filter = new CORSFilter("*");
+    ContainerRequestContext request = mockRequest("GET", DISALLOWED_ORIGIN);
+    MultivaluedMap<String, Object> headers = new MultivaluedHashMap<>();
+    ContainerResponseContext response = mock(ContainerResponseContext.class);
+    when(response.getHeaders()).thenReturn(headers);
+
+    filter.filter(request, response);
+
+    Assert.assertEquals(headers.getFirst("Access-Control-Allow-Origin"), "*");
+    Assert.assertNull(headers.getFirst("Access-Control-Allow-Credentials"));
+  }
+
+  @Test
+  public void testUnconfiguredDeniesAllOrigins() throws IOException {
+    CORSFilter filter = new CORSFilter(null);
+    ContainerRequestContext request = mockRequest("GET", ALLOWED_ORIGIN);
+    MultivaluedMap<String, Object> headers = new MultivaluedHashMap<>();
+    ContainerResponseContext response = mock(ContainerResponseContext.class);
+    when(response.getHeaders()).thenReturn(headers);
+
+    filter.filter(request, response);
+
+    Assert.assertTrue(headers.isEmpty());
+  }
+
+  @Test
+  public void testPreflightAllowedOriginRestrictsMethods() throws IOException {
+    CORSFilter filter = new CORSFilter(ALLOWED_ORIGIN);
+    ContainerRequestContext request = mockRequest("OPTIONS", ALLOWED_ORIGIN);
+    when(request.getHeaderString("Access-Control-Request-Method")).thenReturn("TRACE");
+    when(request.getHeaderString("Access-Control-Request-Headers")).thenReturn("X-Custom");
+
+    Response response = capturePreflight(filter, request);
+
+    Assert.assertEquals(response.getStatus(), 200);
+    Assert.assertEquals(response.getHeaderString("Access-Control-Allow-Origin"), ALLOWED_ORIGIN);
+    Assert.assertEquals(response.getHeaderString("Access-Control-Allow-Credentials"), "true");
+    String allowedMethods = response.getHeaderString("Access-Control-Allow-Methods");
+    Assert.assertNotNull(allowedMethods);
+    Assert.assertFalse(allowedMethods.contains("TRACE"));
+    Assert.assertTrue(allowedMethods.contains("GET"));
+    Assert.assertEquals(response.getHeaderString("Access-Control-Allow-Headers"), "X-Custom");
+  }
+
+  @Test
+  public void testPreflightDisallowedOriginEmitsNoCorsHeaders() throws IOException {
+    CORSFilter filter = new CORSFilter(ALLOWED_ORIGIN);
+    ContainerRequestContext request = mockRequest("OPTIONS", DISALLOWED_ORIGIN);
+    when(request.getHeaderString("Access-Control-Request-Method")).thenReturn("GET");
+
+    Response response = capturePreflight(filter, request);
+
+    Assert.assertNull(response.getHeaderString("Access-Control-Allow-Origin"));
+    Assert.assertNull(response.getHeaderString("Access-Control-Allow-Methods"));
+    Assert.assertNull(response.getHeaderString("Access-Control-Allow-Credentials"));
+  }
+
+  private ContainerRequestContext mockRequest(String method, String origin) {
+    ContainerRequestContext request = mock(ContainerRequestContext.class);
+    when(request.getMethod()).thenReturn(method);
+    when(request.getHeaderString("Origin")).thenReturn(origin);
+    return request;
+  }
+
+  private Response capturePreflight(CORSFilter filter, ContainerRequestContext request)
+      throws IOException {
+    filter.filter(request);
+    ArgumentCaptor<Response> captor = ArgumentCaptor.forClass(Response.class);
+    verify(request).abortWith(captor.capture());
+    return captor.getValue();
+  }
+}