Browse Source

FE: Refactor RBAC tests, fix "subset" permissions issues.

Roman Zabaluev 1 year ago
parent
commit
7fb68e6ba6

+ 2 - 0
kafka-ui-react-app/src/components/common/ActionComponent/__tests__/fixtures.ts

@@ -7,11 +7,13 @@ export const clusterName = 'local';
 export const validPermission = {
   resource: ResourceType.TOPIC,
   action: Action.CREATE,
+  value: 'topic',
 };
 
 export const invalidPermission = {
   resource: ResourceType.SCHEMA,
   action: Action.DELETE,
+  value: 'test',
 };
 
 const roles = [

+ 61 - 39
kafka-ui-react-app/src/lib/__test__/permission.spec.ts

@@ -1,9 +1,5 @@
-import {
-  isPermitted,
-  isPermittedToCreate,
-  modifyRolesData,
-} from 'lib/permissions';
-import { Action, ResourceType } from 'generated-sources';
+import {isPermitted, isPermittedToCreate, modifyRolesData,} from 'lib/permissions';
+import {Action, ResourceType} from 'generated-sources';
 
 describe('Permission Helpers', () => {
   const clusterName1 = 'local';
@@ -14,6 +10,7 @@ describe('Permission Helpers', () => {
       clusters: [clusterName1],
       resource: ResourceType.TOPIC,
       actions: [Action.VIEW, Action.CREATE],
+      value: '.*',
     },
     {
       clusters: [clusterName1],
@@ -24,11 +21,18 @@ describe('Permission Helpers', () => {
       clusters: [clusterName1, clusterName2],
       resource: ResourceType.SCHEMA,
       actions: [Action.VIEW],
+      value: '.*',
     },
     {
       clusters: [clusterName1, clusterName2],
       resource: ResourceType.CONNECT,
       actions: [Action.VIEW],
+      value: '.*',
+    },
+    {
+      clusters: [clusterName1],
+      resource: ResourceType.APPLICATIONCONFIG,
+      actions: [Action.EDIT],
     },
     {
       clusters: [clusterName1],
@@ -39,6 +43,7 @@ describe('Permission Helpers', () => {
       clusters: [clusterName1],
       resource: ResourceType.CONSUMER,
       actions: [Action.DELETE],
+      value: '.*',
     },
     {
       clusters: [clusterName1],
@@ -46,6 +51,16 @@ describe('Permission Helpers', () => {
       actions: [Action.EDIT, Action.DELETE, Action.CREATE],
       value: '123.*',
     },
+    {
+      clusters: [clusterName1],
+      resource: ResourceType.ACL,
+      actions: [Action.VIEW],
+    },
+    {
+      clusters: [clusterName1],
+      resource: ResourceType.AUDIT,
+      actions: [Action.VIEW],
+    },
     {
       clusters: [clusterName1, clusterName2],
       resource: ResourceType.TOPIC,
@@ -58,6 +73,12 @@ describe('Permission Helpers', () => {
       value: '.*',
       actions: [Action.EDIT, Action.DELETE],
     },
+    {
+      clusters: [clusterName1, clusterName2],
+      resource: ResourceType.TOPIC,
+      value: 'bobross.*',
+      actions: [Action.VIEW, Action.MESSAGES_READ],
+    },
   ];
 
   const roles = modifyRolesData(userPermissionsMock);
@@ -100,11 +121,11 @@ describe('Permission Helpers', () => {
 
       expect(result.size).toBe(2);
 
-      expect(cluster1Map?.size).toBe(6);
+      expect(cluster1Map?.size).toBe(9);
       expect(cluster2Map?.size).toBe(3);
 
       // clusterMap1
-      expect(cluster1Map?.get(ResourceType.TOPIC)).toHaveLength(3);
+      expect(cluster1Map?.get(ResourceType.TOPIC)).toHaveLength(4);
       expect(cluster1Map?.get(ResourceType.SCHEMA)).toHaveLength(2);
       expect(cluster1Map?.get(ResourceType.CONSUMER)).toHaveLength(1);
       expect(cluster1Map?.get(ResourceType.CLUSTERCONFIG)).toHaveLength(1);
@@ -177,33 +198,23 @@ describe('Permission Helpers', () => {
       ).toBeFalsy();
     });
 
-    it('should check if the isPermitted returns the correct value without name values', () => {
+    it('should check if the isPermitted returns the correct value without resource values (exempt list)', () => {
       expect(
         isPermitted({
           roles,
           clusterName: clusterName1,
-          resource: ResourceType.TOPIC,
-          action: Action.VIEW,
+          resource: ResourceType.KSQL,
+          action: Action.EXECUTE,
           rbacFlag: true,
         })
       ).toBeTruthy();
 
-      expect(
-        isPermitted({
-          roles,
-          clusterName: clusterName2,
-          resource: ResourceType.TOPIC,
-          action: Action.VIEW,
-          rbacFlag: true,
-        })
-      ).toBeFalsy();
-
       expect(
         isPermitted({
           roles,
           clusterName: clusterName1,
-          resource: ResourceType.SCHEMA,
-          action: Action.VIEW,
+          resource: ResourceType.CLUSTERCONFIG,
+          action: Action.EDIT,
           rbacFlag: true,
         })
       ).toBeTruthy();
@@ -212,7 +223,7 @@ describe('Permission Helpers', () => {
         isPermitted({
           roles,
           clusterName: clusterName1,
-          resource: ResourceType.CLUSTERCONFIG,
+          resource: ResourceType.APPLICATIONCONFIG,
           action: Action.EDIT,
           rbacFlag: true,
         })
@@ -222,8 +233,8 @@ describe('Permission Helpers', () => {
         isPermitted({
           roles,
           clusterName: clusterName1,
-          resource: ResourceType.KSQL,
-          action: Action.EXECUTE,
+          resource: ResourceType.ACL,
+          action: Action.VIEW,
           rbacFlag: true,
         })
       ).toBeTruthy();
@@ -231,22 +242,22 @@ describe('Permission Helpers', () => {
       expect(
         isPermitted({
           roles,
-          clusterName: clusterName2,
-          resource: ResourceType.KSQL,
-          action: Action.EXECUTE,
+          clusterName: clusterName1,
+          resource: ResourceType.AUDIT,
+          action: Action.VIEW,
           rbacFlag: true,
         })
-      ).toBeFalsy();
+      ).toBeTruthy();
 
       expect(
         isPermitted({
           roles,
-          clusterName: clusterName2,
-          resource: ResourceType.SCHEMA,
+          clusterName: clusterName1,
+          resource: ResourceType.TOPIC,
           action: Action.VIEW,
           rbacFlag: true,
         })
-      ).toBeTruthy();
+      ).toBeFalsy();
 
       expect(
         isPermitted({
@@ -256,17 +267,17 @@ describe('Permission Helpers', () => {
           action: Action.VIEW,
           rbacFlag: true,
         })
-      ).toBeTruthy();
+      ).toBeFalsy();
 
       expect(
         isPermitted({
           roles,
-          clusterName: clusterName2,
-          resource: ResourceType.CONNECT,
+          clusterName: clusterName1,
+          resource: ResourceType.CONSUMER,
           action: Action.VIEW,
           rbacFlag: true,
         })
-      ).toBeTruthy();
+      ).toBeFalsy();
 
       expect(
         isPermitted({
@@ -276,7 +287,7 @@ describe('Permission Helpers', () => {
           action: Action.VIEW,
           rbacFlag: true,
         })
-      ).toBeTruthy();
+      ).toBeFalsy();
     });
 
     it('should check if the isPermitted returns the correct value with name values', () => {
@@ -445,7 +456,7 @@ describe('Permission Helpers', () => {
           value: '123456',
           rbacFlag: true,
         })
-      ).toBeFalsy();
+      ).toBeTruthy();
 
       expect(
         isPermitted({
@@ -468,6 +479,17 @@ describe('Permission Helpers', () => {
           rbacFlag: true,
         })
       ).toBeTruthy();
+
+      expect(
+        isPermitted({
+          roles,
+          clusterName: clusterName1,
+          resource: ResourceType.TOPIC,
+          action: [Action.MESSAGES_READ],
+          value: 'bobross-test',
+          rbacFlag: true,
+        })
+      ).toBeTruthy();
     });
 
     it('should check the rbac flag and works with permissions accordingly', () => {

+ 30 - 27
kafka-ui-react-app/src/lib/permissions.ts

@@ -1,9 +1,17 @@
-import { Action, UserPermission, ResourceType } from 'generated-sources';
+import { Action, ResourceType, UserPermission } from 'generated-sources';
 
 export type RolesType = UserPermission[];
 
 export type RolesModifiedTypes = Map<string, Map<ResourceType, RolesType>>;
 
+const ResourceExemptList: ResourceType[] = [
+  ResourceType.KSQL,
+  ResourceType.CLUSTERCONFIG,
+  ResourceType.APPLICATIONCONFIG,
+  ResourceType.ACL,
+  ResourceType.AUDIT,
+];
+
 export function modifyRolesData(
   data?: RolesType
 ): Map<string, Map<ResourceType, RolesType>> {
@@ -83,32 +91,27 @@ export function isPermitted({
   if (!clusterMap) return false;
 
   // short circuit
-  const resourceData = clusterMap.get(resource);
-  if (!resourceData) return false;
-
-  return (
-    resourceData.findIndex((item) => {
-      let valueCheck = true;
-      if (item.value) {
-        valueCheck = false;
-
-        if (value) valueCheck = new RegExp(item.value).test(value);
-      }
-
-      // short circuit
-      if (!valueCheck) return false;
-
-      if (!Array.isArray(action)) {
-        return item.actions.includes(action);
-      }
-
-      // every given action should be found in that resource
-      return action.every(
-        (currentAction) =>
-          item.actions.findIndex((element) => element === currentAction) !== -1
-      );
-    }) !== -1
-  );
+  const resourcePermissions = clusterMap.get(resource);
+  if (!resourcePermissions) return false;
+
+  const valueMatches = (
+    regexp: string | undefined,
+    val: string | undefined
+  ) => {
+    if (!val) return false;
+    if (!regexp) return true;
+    return new RegExp(regexp).test(val);
+  };
+
+  const actions = Array.isArray(action) ? action : [action];
+
+  return actions.every((a) => {
+    return resourcePermissions.some((item) => {
+      if (!item.actions.includes(a)) return false;
+      if (ResourceExemptList.includes(resource)) return true;
+      return valueMatches(item.value, value);
+    });
+  });
 }
 
 /**