Stop echoing the request Origin with allow-credentials in logviewer JSON responses

Author: rzo1

storm-webapp/src/main/java/org/apache/storm/daemon/logviewer/utils/LogviewerResponseBuilder.java

@@ -35,8 +35,6 @@
 import java.io.InputStream;
 import java.io.OutputStream;
 import java.nio.file.Files;
-import java.util.HashMap;
-import java.util.Map;
 
 import org.apache.storm.daemon.common.JsonResponseBuilder;
 import org.apache.storm.daemon.ui.UIHelpers;
@@ -61,11 +59,13 @@ public static Response buildSuccessHtmlResponse(String content) {
      *
      * @param entity entity object to represent it as JSON
      * @param callback callbackParameterName for JSONP
-     * @param origin origin
+     * @param origin origin of the request, not echoed back in the response
      */
     public static Response buildSuccessJsonResponse(Object entity, String callback, String origin) {
-        return new JsonResponseBuilder().setData(entity).setCallback(callback)
-                .setHeaders(LogviewerResponseBuilder.getHeadersForSuccessResponse(origin)).build();
+        // The request origin is deliberately not reflected back: pairing a caller supplied
+        // Access-Control-Allow-Origin with Access-Control-Allow-Credentials would let browsers
+        // hand the response to any site. Keep the default Access-Control-Allow-Origin: * instead.
+        return new JsonResponseBuilder().setData(entity).setCallback(callback).build();
     }
 
     /**
@@ -136,13 +136,6 @@ public static Response buildExceptionJsonResponse(Exception ex, String callback)
                 .setCallback(callback).setStatus(statusCode).build();
     }
 
-    private static Map<String, Object> getHeadersForSuccessResponse(String origin) {
-        Map<String, Object> headers = new HashMap<>();
-        headers.put("Access-Control-Allow-Origin", origin);
-        headers.put("Access-Control-Allow-Credentials", "true");
-        return headers;
-    }
-
     private static String buildUnauthorizedUserHtml(String user) {
         String content = "User '" + escapeHtml4(user) + "' is not authorized.";
         return body(h2(content)).render();

storm-webapp/src/test/java/org/apache/storm/daemon/logviewer/utils/LogviewerResponseBuilderTest.java

@@ -0,0 +1,46 @@
+/*
+ * 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.storm.daemon.logviewer.utils;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.nullValue;
+
+import jakarta.ws.rs.core.Response;
+
+import java.util.Collections;
+
+import org.junit.jupiter.api.Test;
+
+public class LogviewerResponseBuilderTest {
+
+    /**
+     * A success response must keep the documented Access-Control-Allow-Origin: * and must not
+     * echo the request origin back, nor allow credentials.
+     */
+    @Test
+    public void testSuccessJsonResponseDoesNotEchoRequestOrigin() {
+        Response response = LogviewerResponseBuilder.buildSuccessJsonResponse(
+                Collections.singletonMap("someKey", "someValue"), null, "http://other.example.com");
+
+        assertThat(response.getStatus(), is(200));
+        assertThat(response.getHeaderString("Access-Control-Allow-Origin"), is("*"));
+        assertThat(response.getHeaderString("Access-Control-Allow-Credentials"), is(nullValue()));
+    }
+}