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

rzo1 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/storm.git


The following commit(s) were added to refs/heads/master by this push:
     new 9a6490a0f Filter the topology history by the authenticated caller 
rather than the requested user name (#9003)
9a6490a0f is described below

commit 9a6490a0f22300ce268efacbe6a9b02d9b15dc0e
Author: Richard Zowalla <[email protected]>
AuthorDate: Mon Aug 24 10:06:23 2026 +0200

    Filter the topology history by the authenticated caller rather than the 
requested user name (#9003)
---
 .../auth/authorizer/SimpleACLAuthorizer.java       |  1 +
 .../auth/authorizer/SimpleACLAuthorizerTest.java   |  5 ++
 .../org/apache/storm/daemon/nimbus/Nimbus.java     | 52 ++++++++++++++--
 .../org/apache/storm/daemon/nimbus/NimbusTest.java | 69 ++++++++++++++++++++++
 4 files changed, 121 insertions(+), 6 deletions(-)

diff --git 
a/storm-client/src/jvm/org/apache/storm/security/auth/authorizer/SimpleACLAuthorizer.java
 
b/storm-client/src/jvm/org/apache/storm/security/auth/authorizer/SimpleACLAuthorizer.java
index d95965eaa..baaba221d 100644
--- 
a/storm-client/src/jvm/org/apache/storm/security/auth/authorizer/SimpleACLAuthorizer.java
+++ 
b/storm-client/src/jvm/org/apache/storm/security/auth/authorizer/SimpleACLAuthorizer.java
@@ -43,6 +43,7 @@ public class SimpleACLAuthorizer implements IAuthorizer {
         "getNimbusConf",
         "listBlobs",
         "getClusterInfo",
+        "getTopologyHistory",
         "getLeader",
         "isTopologyNameAllowed",
         "getTopologySummaries",
diff --git 
a/storm-client/test/jvm/org/apache/storm/security/auth/authorizer/SimpleACLAuthorizerTest.java
 
b/storm-client/test/jvm/org/apache/storm/security/auth/authorizer/SimpleACLAuthorizerTest.java
index b2c05c385..7d67b034c 100644
--- 
a/storm-client/test/jvm/org/apache/storm/security/auth/authorizer/SimpleACLAuthorizerTest.java
+++ 
b/storm-client/test/jvm/org/apache/storm/security/auth/authorizer/SimpleACLAuthorizerTest.java
@@ -83,6 +83,11 @@ public class SimpleACLAuthorizerTest {
         assertTrue(authorizer.permit(new ReqContext(userA), "getClusterInfo", 
new HashMap<>()));
         assertTrue(authorizer.permit(new ReqContext(userB), "getClusterInfo", 
new HashMap<>()));
 
+        assertTrue(authorizer.permit(new ReqContext(adminUser), 
"getTopologyHistory", new HashMap<>()));
+        assertFalse(authorizer.permit(new ReqContext(supervisorUser), 
"getTopologyHistory", new HashMap<>()));
+        assertTrue(authorizer.permit(new ReqContext(userA), 
"getTopologyHistory", new HashMap<>()));
+        assertTrue(authorizer.permit(new ReqContext(userB), 
"getTopologyHistory", new HashMap<>()));
+
         assertTrue(authorizer.permit(new ReqContext(adminUser), 
"getSupervisorPageInfo", new HashMap<>()));
         assertFalse(authorizer.permit(new ReqContext(supervisorUser), 
"getSupervisorPageInfo", new HashMap<>()));
         assertTrue(authorizer.permit(new ReqContext(userA), 
"getSupervisorPageInfo", new HashMap<>()));
diff --git 
a/storm-server/src/main/java/org/apache/storm/daemon/nimbus/Nimbus.java 
b/storm-server/src/main/java/org/apache/storm/daemon/nimbus/Nimbus.java
index 3064c6ebe..db2957804 100644
--- a/storm-server/src/main/java/org/apache/storm/daemon/nimbus/Nimbus.java
+++ b/storm-server/src/main/java/org/apache/storm/daemon/nimbus/Nimbus.java
@@ -3008,6 +3008,44 @@ public class Nimbus implements Iface, Shutdownable, 
DaemonCommon {
         return !userGroups.isEmpty();
     }
 
+    /**
+     * Get the user whose topology history is to be returned.
+     *
+     * <p>The history is filtered for the caller authenticated on this 
request, not for the user named in the RPC
+     * argument. Only an admin (the ui daemon is expected to be one, see 
SECURITY.md) may ask for the history of
+     * somebody else, because it serves the endpoint on behalf of its own 
authenticated web users.
+     *
+     * @param user the user asked for by the caller
+     * @param adminUsers the configured admin users
+     * @param adminGroups the configured admin groups
+     * @return the user to filter the history with, the argument unchanged if 
security is off
+     *
+     * @throws AuthorizationException if a non admin caller asked for somebody 
else's history
+     * @throws IOException on any error while looking up the caller's groups
+     */
+    private String topologyHistoryUser(String user, Collection<String> 
adminUsers,
+                                       Collection<String> adminGroups) throws 
AuthorizationException, IOException {
+        Principal principal = ReqContext.context().principal();
+        if (principal == null) {
+            //security is off, there is no caller to filter by
+            return user;
+        }
+        String callerPrincipal = principal.getName();
+        String callerUser = principalToLocal.toLocal(principal);
+        if (adminUsers.contains(callerPrincipal) || 
adminUsers.contains(callerUser) || isUserPartOf(callerUser, adminGroups)) {
+            return user;
+        }
+        if (user != null && !user.equals(callerPrincipal) && 
!user.equals(callerUser)) {
+            //Only an admin may ask for somebody else's history. Fall back to 
the caller's own
+            //rather than failing the call: a UI that is not in nimbus.admins 
asks on behalf of
+            //its web users, and answering with the caller's history keeps 
that page working.
+            LOG.warn("{} is not an admin and asked for the topology history of 
{}, returning its own history instead. "
+                + "Add {} to {} if it should be able to read the history of 
other users.",
+                callerUser, user, callerPrincipal, Config.NIMBUS_ADMINS);
+        }
+        return callerUser;
+    }
+
     private List<String> readTopologyHistory(String user, Collection<String> 
adminUsers) throws IOException {
         LocalState state = topologyHistoryState;
         List<LSTopoHistory> topoHistoryList = state.getTopoHistoryList();
@@ -4894,25 +4932,27 @@ public class Nimbus implements Iface, Shutdownable, 
DaemonCommon {
     @Override
     public TopologyHistoryInfo getTopologyHistory(String user) throws 
AuthorizationException, TException {
         try {
+            checkAuthorization(null, null, "getTopologyHistory");
             List<String> adminUsers = (List<String>) 
conf.getOrDefault(Config.NIMBUS_ADMINS, Collections.emptyList());
             List<String> adminGroups = (List<String>) 
conf.getOrDefault(Config.NIMBUS_ADMINS_GROUPS, Collections.emptyList());
+            String historyUser = topologyHistoryUser(user, adminUsers, 
adminGroups);
             IStormClusterState state = stormClusterState;
             List<String> assignedIds = state.assignments(null);
             Set<String> ret = new HashSet<>();
-            boolean isAdmin = adminUsers.contains(user);
+            boolean isAdmin = adminUsers.contains(historyUser);
             for (String topoId : assignedIds) {
                 Map<String, Object> topoConf = tryReadTopoConf(topoId, 
topoCache);
                 topoConf = Utils.merge(conf, topoConf);
                 List<String> groups = 
ServerConfigUtils.getTopoLogsGroups(topoConf);
                 List<String> topoLogUsers = 
ServerConfigUtils.getTopoLogsUsers(topoConf);
-                if (user == null || isAdmin
-                    || isUserPartOf(user, groups)
-                    || isUserPartOf(user, adminGroups)
-                    || topoLogUsers.contains(user)) {
+                if (historyUser == null || isAdmin
+                    || isUserPartOf(historyUser, groups)
+                    || isUserPartOf(historyUser, adminGroups)
+                    || topoLogUsers.contains(historyUser)) {
                     ret.add(topoId);
                 }
             }
-            ret.addAll(readTopologyHistory(user, adminUsers));
+            ret.addAll(readTopologyHistory(historyUser, adminUsers));
             return new TopologyHistoryInfo(new ArrayList<>(ret));
         } catch (Exception e) {
             LOG.warn("Get topology history. (user='{}')", user, e);
diff --git 
a/storm-server/src/test/java/org/apache/storm/daemon/nimbus/NimbusTest.java 
b/storm-server/src/test/java/org/apache/storm/daemon/nimbus/NimbusTest.java
index a6c0771f1..49baebbd2 100644
--- a/storm-server/src/test/java/org/apache/storm/daemon/nimbus/NimbusTest.java
+++ b/storm-server/src/test/java/org/apache/storm/daemon/nimbus/NimbusTest.java
@@ -21,6 +21,8 @@ package org.apache.storm.daemon.nimbus;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
+import java.util.ArrayList;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
@@ -29,6 +31,8 @@ import java.util.Optional;
 import java.util.Set;
 import javax.security.auth.Subject;
 
+import javax.security.auth.Subject;
+
 import net.minidev.json.JSONValue;
 import org.apache.commons.io.FileUtils;
 import org.apache.storm.Config;
@@ -42,6 +46,8 @@ import org.apache.storm.generated.InvalidTopologyException;
 import org.apache.storm.generated.KeyNotFoundException;
 import org.apache.storm.generated.ListBlobsResult;
 import org.apache.storm.generated.RebalanceOptions;
+import org.apache.storm.generated.ReadableBlobMeta;
+import org.apache.storm.generated.SettableBlobMeta;
 import org.apache.storm.generated.StormTopology;
 import org.apache.storm.metric.StormMetricsRegistry;
 import org.apache.storm.nimbus.ILeaderElector;
@@ -51,6 +57,7 @@ import 
org.apache.storm.scheduler.resource.strategies.priority.DefaultScheduling
 import 
org.apache.storm.scheduler.resource.strategies.scheduling.DefaultResourceAwareStrategy;
 import 
org.apache.storm.scheduler.resource.strategies.scheduling.GenericResourceAwareStrategyOld;
 import 
org.apache.storm.scheduler.resource.strategies.scheduling.RoundRobinResourceAwareStrategy;
+import org.apache.storm.security.auth.DefaultPrincipalToLocal;
 import org.apache.storm.security.auth.IAuthorizer;
 import org.apache.storm.security.auth.IGroupMappingServiceProvider;
 import org.apache.storm.security.auth.ReqContext;
@@ -59,8 +66,10 @@ import 
org.apache.storm.security.auth.authorizer.DenyAuthorizer;
 import org.apache.storm.testing.TestWordSpout;
 import org.apache.storm.thrift.TException;
 import org.apache.storm.topology.TopologyBuilder;
+import org.apache.storm.utils.ConfigUtils;
 import org.apache.storm.utils.ServerUtils;
 import org.apache.storm.utils.Time;
+import org.apache.storm.utils.Utils;
 import org.apache.storm.utils.WrappedAuthorizationException;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
@@ -86,6 +95,7 @@ import static org.mockito.Mockito.when;
 
 class NimbusTest {
     private static final String BLOB_FILE_KEY = "file-key";
+    private static final String TOPO_ID = "topology1-1-1";
 
     @Mock
     private StormMetricsRegistry metricRegistry;
@@ -333,4 +343,63 @@ class NimbusTest {
             ReqContext.reset();
         }
     }
+
+    @Test
+    void testGetTopologyHistoryFiltersByTheAuthenticatedCaller() throws 
Exception {
+        Map<String, Object> conf = new HashMap<>();
+        conf.put(DaemonConfig.NIMBUS_MONITOR_FREQ_SECS, 10);
+        conf.put(Config.STORM_PRINCIPAL_TO_LOCAL_PLUGIN, 
DefaultPrincipalToLocal.class.getName());
+        conf.put(Config.NIMBUS_ADMINS, Collections.singletonList("admin"));
+        nimbus = new Nimbus(conf, iNimbus, stormClusterState, nimbusInfo, 
localBlobStore, leaderElector, groupMapper, metricRegistry);
+
+        Map<String, Object> topoConf = new HashMap<>();
+        topoConf.put(Config.TOPOLOGY_NAME, "topology1");
+        topoConf.put(Config.TOPOLOGY_USERS, 
Collections.singletonList("alice"));
+        
when(stormClusterState.assignments(null)).thenReturn(Collections.singletonList(TOPO_ID));
+        
when(localBlobStore.readBlob(eq(ConfigUtils.masterStormConfKey(TOPO_ID)), 
any()))
+            .thenReturn(Utils.toCompressedJsonConf(topoConf));
+        
when(localBlobStore.getBlobMeta(eq(ConfigUtils.masterStormConfKey(TOPO_ID)), 
any()))
+            .thenReturn(new ReadableBlobMeta(new SettableBlobMeta(new 
ArrayList<>()), 0));
+
+        try {
+            setCaller("bob");
+            // asking for somebody else's history is only for admins, the ui 
daemon is expected to be one.
+            // a caller that is not an admin gets its own history back rather 
than an error, so a ui that
+            // was left out of nimbus.admins keeps serving the page instead of 
failing it
+            
assertTrue(nimbus.getTopologyHistory("alice").get_topo_ids().isEmpty());
+            // and no user argument at all is the caller's own history, not 
everybody's
+            
assertTrue(nimbus.getTopologyHistory(null).get_topo_ids().isEmpty());
+
+            setCaller("alice");
+            assertEquals(Collections.singletonList(TOPO_ID), 
nimbus.getTopologyHistory(null).get_topo_ids());
+
+            setCaller("admin");
+            assertEquals(Collections.singletonList(TOPO_ID), 
nimbus.getTopologyHistory("alice").get_topo_ids());
+            
assertTrue(nimbus.getTopologyHistory("bob").get_topo_ids().isEmpty());
+        } finally {
+            ReqContext.reset();
+        }
+    }
+
+    @Test
+    void testGetTopologyHistoryIsAuthorized() throws Exception {
+        Map<String, Object> conf = new HashMap<>();
+        conf.put(DaemonConfig.NIMBUS_MONITOR_FREQ_SECS, 10);
+        conf.put(Config.STORM_PRINCIPAL_TO_LOCAL_PLUGIN, 
DefaultPrincipalToLocal.class.getName());
+        conf.put(DaemonConfig.NIMBUS_AUTHORIZER, 
DenyAuthorizer.class.getName());
+        nimbus = new Nimbus(conf, iNimbus, stormClusterState, nimbusInfo, 
localBlobStore, leaderElector, groupMapper, metricRegistry);
+
+        try {
+            setCaller("bob");
+            assertThrows(AuthorizationException.class, () -> 
nimbus.getTopologyHistory("bob"));
+        } finally {
+            ReqContext.reset();
+        }
+    }
+
+    private static void setCaller(String user) {
+        Subject subject = new Subject();
+        subject.getPrincipals().add(new SingleUserPrincipal(user));
+        ReqContext.context().setSubject(subject);
+    }
 }

Reply via email to