You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@storm.apache.org by rz...@apache.org on 2026/08/19 06:58:46 UTC
(storm) branch fix/logviewer-log-listing-user-filter created (now a67cc4772)
This is an automated email from the ASF dual-hosted git repository.
rzo1 pushed a change to branch fix/logviewer-log-listing-user-filter
in repository https://gitbox.apache.org/repos/asf/storm.git
at a67cc4772 Return only the log files the requesting user may access from the log file listing
This branch includes the following new commits:
new a67cc4772 Return only the log files the requesting user may access from the log file listing
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(storm) 01/01: Return only the log files the requesting user may access from the log file listing
Posted by rz...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rzo1 pushed a commit to branch fix/logviewer-log-listing-user-filter
in repository https://gitbox.apache.org/repos/asf/storm.git
commit a67cc47722a6a0590a51c487f4411062a4817a98
Author: Richard Zowalla <zo...@colorful-bytes.com>
AuthorDate: Wed Aug 19 08:34:56 2026 +0200
Return only the log files the requesting user may access from the log file listing
---
.../logviewer/handler/LogviewerLogPageHandler.java | 17 ++++++-
.../handler/LogviewerLogPageHandlerTest.java | 56 ++++++++++++++++++++--
2 files changed, 67 insertions(+), 6 deletions(-)
diff --git a/storm-webapp/src/main/java/org/apache/storm/daemon/logviewer/handler/LogviewerLogPageHandler.java b/storm-webapp/src/main/java/org/apache/storm/daemon/logviewer/handler/LogviewerLogPageHandler.java
index 3ffdf75f4..5917e9746 100644
--- a/storm-webapp/src/main/java/org/apache/storm/daemon/logviewer/handler/LogviewerLogPageHandler.java
+++ b/storm-webapp/src/main/java/org/apache/storm/daemon/logviewer/handler/LogviewerLogPageHandler.java
@@ -114,7 +114,7 @@ public class LogviewerLogPageHandler {
}
/**
- * Enumerate worker log files for given criteria.
+ * Enumerate worker log files for given criteria. Only the files the user is allowed to access are returned.
*
* @param user username
* @param port worker's port, null for all workers
@@ -175,8 +175,10 @@ public class LogviewerLogPageHandler {
List<String> files;
if (fileResults != null) {
+ Map<String, Boolean> authorizedPortDirs = new HashMap<>();
files = fileResults.stream()
.map(WorkerLogs::getTopologyPortWorkerLog)
+ .filter(fileStr -> isUserAllowedToAccessLog(user, fileStr, authorizedPortDirs))
.sorted().collect(toList());
} else {
files = new ArrayList<>();
@@ -185,6 +187,19 @@ public class LogviewerLogPageHandler {
return LogviewerResponseBuilder.buildSuccessJsonResponse(files, callback, origin);
}
+ /**
+ * Check whether the user may access the given "topologyId/port/fileName" worker log. The authorization only depends on the
+ * topology and the port, so the answer is cached per port directory to avoid re-reading the log metadata for every file.
+ */
+ private boolean isUserAllowedToAccessLog(String user, String fileStr, Map<String, Boolean> authorizedPortDirs) {
+ Path portDir = Paths.get(fileStr).getParent();
+ if (portDir == null) {
+ return resourceAuthorizer.isUserAllowedToAccessFile(user, fileStr);
+ }
+ return authorizedPortDirs.computeIfAbsent(portDir.toString(),
+ key -> resourceAuthorizer.isUserAllowedToAccessFile(user, fileStr));
+ }
+
/**
* Provides a worker log file to view, starting from the specified position
* or default starting position of the most recent page.
diff --git a/storm-webapp/src/test/java/org/apache/storm/daemon/logviewer/handler/LogviewerLogPageHandlerTest.java b/storm-webapp/src/test/java/org/apache/storm/daemon/logviewer/handler/LogviewerLogPageHandlerTest.java
index 2114a9d8c..b6de62009 100644
--- a/storm-webapp/src/test/java/org/apache/storm/daemon/logviewer/handler/LogviewerLogPageHandlerTest.java
+++ b/storm-webapp/src/test/java/org/apache/storm/daemon/logviewer/handler/LogviewerLogPageHandlerTest.java
@@ -22,6 +22,12 @@ import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.ArgumentMatchers.startsWith;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
import com.fasterxml.jackson.databind.ObjectMapper;
@@ -69,19 +75,22 @@ public class LogviewerLogPageHandlerTest {
new WorkerLogs(stormConf, Paths.get(rootPath), metricsRegistry), new ResourceAuthorizer(stormConf), metricsRegistry);
final Response expectedAll = LogviewerResponseBuilder.buildSuccessJsonResponse(
- List.of("topoA/port1/worker.log", "topoA/port2/worker.log", "topoB/port1/worker.log"),
+ List.of(String.join(File.separator, "topoA", "1111", "worker.log"),
+ String.join(File.separator, "topoA", "2222", "worker.log"),
+ String.join(File.separator, "topoB", "1111", "worker.log")),
null,
origin
);
final Response expectedFilterPort = LogviewerResponseBuilder.buildSuccessJsonResponse(
- List.of("topoA/port1/worker.log", "topoB/port1/worker.log"),
+ List.of(String.join(File.separator, "topoA", "1111", "worker.log"),
+ String.join(File.separator, "topoB", "1111", "worker.log")),
null,
origin
);
final Response expectedFilterTopoId = LogviewerResponseBuilder.buildSuccessJsonResponse(
- List.of("topoB/port1/worker.log"),
+ List.of(String.join(File.separator, "topoB", "1111", "worker.log")),
null,
origin
);
@@ -97,11 +106,48 @@ public class LogviewerLogPageHandlerTest {
assertEqualsJsonResponse(expectedFilterTopoId, returnedFilterTopoId, List.class);
}
+ /**
+ * list-log-files only returns the log files the user is allowed to access.
+ */
+ @Test
+ public void testListLogFilesFiltersFilesTheUserMayNotAccess() throws IOException {
+ String rootPath = Files.createTempDirectory("workers-artifacts").toFile().getCanonicalPath();
+ File file1 = new File(String.join(File.separator, rootPath, "topoA", "1111"), "worker.log");
+ File file2 = new File(String.join(File.separator, rootPath, "topoA", "1111"), "worker.log.1");
+ File file3 = new File(String.join(File.separator, rootPath, "topoB", "1111"), "worker.log");
+
+ file1.getParentFile().mkdirs();
+ file3.getParentFile().mkdirs();
+ file1.createNewFile();
+ file2.createNewFile();
+ file3.createNewFile();
+
+ String origin = "www.origin.server.net";
+ String topoAPortDir = String.join(File.separator, "topoA", "1111");
+ Map<String, Object> stormConf = Utils.readStormConfig();
+ StormMetricsRegistry metricsRegistry = new StormMetricsRegistry();
+ ResourceAuthorizer resourceAuthorizer = mock(ResourceAuthorizer.class);
+ when(resourceAuthorizer.isUserAllowedToAccessFile(anyString(), startsWith(topoAPortDir))).thenReturn(true);
+ LogviewerLogPageHandler handler = new LogviewerLogPageHandler(rootPath, rootPath,
+ new WorkerLogs(stormConf, Paths.get(rootPath), metricsRegistry), resourceAuthorizer, metricsRegistry);
+
+ final Response returned = handler.listLogFiles("user", null, null, null, origin);
+
+ List<?> files = new ObjectMapper().readValue((String) returned.getEntity(), List.class);
+
+ Utils.forceDelete(rootPath);
+
+ assertEquals(List.of(String.join(File.separator, topoAPortDir, "worker.log"),
+ String.join(File.separator, topoAPortDir, "worker.log.1")), files);
+ //The authorization only depends on the port directory, so it is checked once per port directory, not once per file.
+ verify(resourceAuthorizer, times(2)).isUserAllowedToAccessFile(anyString(), anyString());
+ }
+
private <T> void assertEqualsJsonResponse(Response expected, Response actual, Class<T> entityClass) throws IOException {
ObjectMapper objectMapper = new ObjectMapper();
T entityFromExpected = objectMapper.readValue((String) expected.getEntity(), entityClass);
- T actualFromExpected = objectMapper.readValue((String) expected.getEntity(), entityClass);
- assertEquals(entityFromExpected, actualFromExpected);
+ T entityFromActual = objectMapper.readValue((String) actual.getEntity(), entityClass);
+ assertEquals(entityFromExpected, entityFromActual);
assertEquals(expected.getStatus(), actual.getStatus());
assertTrue(expected.getHeaders().equalsIgnoreValueOrder(actual.getHeaders()));