fix(acls): inconsistency between READ and READ_CONFIG actions for NOD…

…E resource (#1521) close #1519
tchiotludo · Jun 29, 2023 · 72a41cc · 72a41cc
1 parent cfc50e2
commit 72a41cc
Show file tree

Hide file tree

Showing 7 changed files with 40 additions and 18 deletions.
diff --git a/application.example.yml b/application.example.yml
@@ -187,6 +187,12 @@ akhq:
   # Auth & Roles (optional)
   security:
     roles:
+      node-read:
+        - resources: [ "NODE" ]
+          actions: [ "READ", "READ_CONFIG" ]
+      node-admin:
+        - resources: [ "NODE" ]
+          actions: [ "READ", "READ_CONFIG", "ALTER_CONFIG" ]
       topic-read:
         - resources: [ "TOPIC", "TOPIC_DATA" ]
           actions: [ "READ" ]
@@ -223,6 +229,7 @@ akhq:
     # Groups definition
     groups:
       admin:
+        - role: node-admin
         - role: topic-admin
         - role: connect-admin
         - role: registry-admin

diff --git a/client/src/containers/Node/NodeDetail/Node.jsx b/client/src/containers/Node/NodeDetail/Node.jsx
@@ -13,17 +13,24 @@ class Node extends Component {
     data: [],
     clusterId: this.props.match.params.clusterId,
     selectedNode: this.props.match.params.nodeId,
-    selectedTab: 'configs'
+    selectedTab: 'logs',
+    roles: JSON.parse(sessionStorage.getItem('roles'))
   };
 
   tabs = ['configs', 'logs'];
 
   componentDidMount() {
     const { clusterId, nodeId } = this.props.match.params;
-    const tabSelected = getSelectedTab(this.props, this.tabs);
+    const { roles } = this.state;
+    let tabSelected = getSelectedTab(this.props, this.tabs);
+
+    if (tabSelected === 'configs' && roles.NODE && !roles.NODE.includes('READ_CONFIG')) {
+      tabSelected = 'logs';
+    }
+
     this.setState(
       {
-        selectedTab: tabSelected ? tabSelected : 'configs'
+        selectedTab: tabSelected
       },
       () => {
         this.props.history.replace(`/ui/${clusterId}/node/${nodeId}/${this.state.selectedTab}`);
@@ -75,20 +82,22 @@ class Node extends Component {
   }
 
   render() {
-    const { selectedNode, clusterId } = this.state;
+    const { selectedNode, clusterId, roles } = this.state;
     return (
       <div>
         <Header title={`Node ${selectedNode}`} history={this.props.history} />
         <div className="tabs-container">
           <ul className="nav nav-tabs" role="tablist">
-            <li className="nav-item">
-              <Link
-                to={`/ui/${clusterId}/node/${selectedNode}/configs`}
-                className={this.tabClassName('configs')}
-              >
-                Configs
-              </Link>
-            </li>
+            {roles.NODE && roles.NODE.includes('READ_CONFIG') && (
+              <li className="nav-item">
+                <Link
+                  to={`/ui/${clusterId}/node/${selectedNode}/configs`}
+                  className={this.tabClassName('configs')}
+                >
+                  Configs
+                </Link>
+              </li>
+            )}
             <li className="nav-item">
               <Link
                 to={`/ui/${clusterId}/node/${selectedNode}/logs`}

diff --git a/client/src/containers/SideBar/Sidebar.jsx b/client/src/containers/SideBar/Sidebar.jsx
@@ -336,6 +336,7 @@ class Sidebar extends Component {
           </NavItem>
           {roles &&
             roles.NODE &&
+            roles.NODE.includes('READ') &&
             this.renderMenuItem('fa fa-fw fa-laptop', constants.NODE, 'Nodes')}
           {roles &&
             roles.TOPIC &&

diff --git a/client/src/utils/Routes.js b/client/src/utils/Routes.js
@@ -110,8 +110,7 @@ class Routes extends Root {
     let clusterId = this.state.clusterId;
     const roles = JSON.parse(sessionStorage.getItem('roles'));
     if (roles && roles.TOPIC && roles.TOPIC.includes('READ')) return `/ui/${clusterId}/topic`;
-    else if (roles && roles.NODE && roles.NODE.includes('READ_CONFIG'))
-      return `/ui/${clusterId}/node`;
+    else if (roles && roles.NODE && roles.NODE.includes('READ')) return `/ui/${clusterId}/node`;
     else if (roles && roles.CONSUMER_GROUP && roles.CONSUMER_GROUP.includes('READ'))
       return `/ui/${clusterId}/group`;
     else if (roles && roles.ACL && roles.ACL.includes('READ')) return `/ui/${clusterId}/acls`;
@@ -177,10 +176,10 @@ class Routes extends Root {
                 <Route exact path="/ui/:clusterId/tail" component={Tail} />
               )}
 
-              {roles && roles.NODE && roles.NODE.includes('READ_CONFIG') && (
+              {roles && roles.NODE && roles.NODE.includes('READ') && (
                 <Route exact path="/ui/:clusterId/node" component={NodesList} />
               )}
-              {roles && roles.NODE && roles.NODE.includes('READ_CONFIG') && (
+              {roles && roles.NODE && roles.NODE.includes('READ') && (
                 <Route exact path="/ui/:clusterId/node/:nodeId/:tab?" component={NodeDetails} />
               )}
 

diff --git a/src/main/java/org/akhq/controllers/AkhqController.java b/src/main/java/org/akhq/controllers/AkhqController.java
@@ -202,6 +202,10 @@ public Connection.UiOptions options(String cluster) {
     private List<AuthUser.AuthPermissions> expandRoles(List<Group> groupBindings) {
         SecurityProperties securityProperties = applicationContext.getBean(SecurityProperties.class);
 
+        if (securityProperties.getRoles() == null) {
+            throw new RuntimeException("Roles has not been defined properly. Please check the documentation");
+        }
+
         return groupBindings.stream()
             .map(binding -> securityProperties.getRoles().entrySet().stream()
                 .filter(role -> role.getKey().equals(binding.getRole()))

diff --git a/src/main/java/org/akhq/controllers/NodeController.java b/src/main/java/org/akhq/controllers/NodeController.java
@@ -20,7 +20,7 @@
 import java.util.concurrent.ExecutionException;
 import java.util.stream.Collectors;
 
-@AKHQSecured(resource = Role.Resource.NODE, action = Role.Action.READ_CONFIG)
+@AKHQSecured(resource = Role.Resource.NODE, action = Role.Action.READ)
 @Controller
 public class NodeController extends AbstractController {
     private final ClusterRepository clusterRepository;
@@ -108,6 +108,7 @@ public List<LogDir> nodeLog(String cluster, Integer nodeId) throws ExecutionExce
     }
 
     @Get("api/{cluster}/node/{nodeId}/configs")
+    @AKHQSecured(resource = Role.Resource.NODE, action = Role.Action.READ_CONFIG)
     @Operation(tags = {"node"}, summary = "List all configs for a node")
     public List<Config> nodeConfig(String cluster, Integer nodeId) throws ExecutionException, InterruptedException {
         checkIfClusterAndResourceAllowed(cluster, nodeId.toString());

diff --git a/src/main/resources/application.yml b/src/main/resources/application.yml
@@ -153,7 +153,7 @@ akhq:
           actions: [ "READ", "CREATE", "UPDATE", "DELETE", "DELETE_VERSION" ]
       node-admin:
         - resources: [ "NODE" ]
-          actions: [ "READ_CONFIG", "ALTER_CONFIG" ]
+          actions: [ "READ", "READ_CONFIG", "ALTER_CONFIG" ]
       acl-reader:
         - resources: [ "ACL" ]
           actions: [ "READ" ]
@@ -166,6 +166,7 @@ akhq:
           # patterns: [ ".*" ]
           # clusters: [ ".*" ]
       admin:
+        - role: node-admin
         - role: topic-admin
         - role: topic-data-admin
         - role: consumer-group-admin