Current Users or ALL Users getting deleted in an Update (#906)

* fixed: when updating a user permission by DELETING a user from a certain role, it drops ALL the users. Also if you add a user to a role it will still keep the role on previously added users

* updated the code so that if datbase selected is empty we show feedback there is nothing to fetch and do not provide labels and also enable to save users to role even though db is empty

---------

Co-authored-by: Niels de Jong <[email protected]>

Author: AleSim94

src/extensions/rbac/RBACManagementModal.tsx

@@ -32,6 +32,7 @@ export const RBACManagementModal = ({ open, handleClose, currentRole, createNoti
   const [allowCompleted, setAllowCompleted] = useState(false);
   const [usersCompleted, setUsersCompleted] = useState(false);
   const [failed, setFailed] = useState(false);
+  const [isDatabaseEmpty, setIsDatabaseEmpty] = useState(false);
 
   useEffect(() => {
     if (!open) {
@@ -75,35 +76,48 @@ export const RBACManagementModal = ({ open, handleClose, currentRole, createNoti
 
   const handleDatabaseSelect = (selectedOption) => {
     setSelectedDatabase(selectedOption.value);
-    retrieveLabelsList(driver, selectedOption.value, (records) => parseLabelsList(selectedOption.value, records));
+    setLabels([]);
+    setAllowList([]);
+    setDenyList([]);
+    retrieveLabelsList(driver, selectedOption.value, (records) => {
+      if (records.length === 0) {
+        setIsDatabaseEmpty(true);
+      } else {
+        parseLabelsList(selectedOption.value, records);
+        setIsDatabaseEmpty(false);
+      }
+    });
   };
 
   const handleSave = async () => {
     createNotification('Updating', `Access for role '${currentRole}' is being updated, please wait...`);
-    console.log(selectedUsers);
-    updateUsers(
-      driver,
-      currentRole,
-      neo4jUsers,
-      selectedUsers,
-      () => setUsersCompleted(true),
-      (failReason) => setFailed(`Operation 'ROLE-USER ASSIGNMENT' failed.\n Reason: ${failReason}`)
-    );
-
-    if (selectedDatabase) {
-      const nonFixedDenyList = denyList.filter((n) => !fixedDenyList.includes(n));
-      const nonFixedAllowList = allowList.filter((n) => !fixedDenyList.includes(n));
-      updatePrivileges(
+    try {
+      await updateUsers(
         driver,
-        selectedDatabase,
         currentRole,
-        labels,
-        nonFixedDenyList,
-        Operation.DENY,
-        () => setDenyCompleted(true),
-        (failReason) => setFailed(`Operation 'DENY LABEL ACCESS' failed.\n Reason: ${failReason}`)
-      ).then(() => {
-        updatePrivileges(
+        neo4jUsers,
+        selectedUsers,
+        () => setUsersCompleted(true),
+        (failReason) => setFailed(`Operation 'ROLE-USER ASSIGNMENT' failed.\n Reason: ${failReason}`)
+      );
+
+      if (selectedDatabase && labels.length > 0) {
+        // Check if there are labels to update
+        const nonFixedDenyList = denyList.filter((n) => !fixedDenyList.includes(n));
+        const nonFixedAllowList = allowList.filter((n) => !fixedDenyList.includes(n));
+
+        await updatePrivileges(
+          driver,
+          selectedDatabase,
+          currentRole,
+          labels,
+          nonFixedDenyList,
+          Operation.DENY,
+          () => setDenyCompleted(true),
+          (failReason) => setFailed(`Operation 'DENY LABEL ACCESS' failed.\n Reason: ${failReason}`)
+        );
+
+        await updatePrivileges(
           driver,
           selectedDatabase,
           currentRole,
@@ -113,14 +127,18 @@ export const RBACManagementModal = ({ open, handleClose, currentRole, createNoti
           () => setAllowCompleted(true),
           (failReason) => setFailed(`Operation 'ALLOW LABEL ACCESS' failed.\n Reason: ${failReason}`)
         );
-      });
-    } else {
-      // Since there is no database selected, we don't run the DENY/ALLOW queries.
-      // We just mark them as completed so the success message shows up.
-      setDenyCompleted(true);
-      setAllowCompleted(true);
+      } else {
+        // Since there is no database or labels selected, we don't run the DENY/ALLOW queries.
+        // We just mark them as completed so the success message shows up.
+        setDenyCompleted(true);
+        setAllowCompleted(true);
+      }
+    } catch (error) {
+      // Handle any errors that occur during the update process
+      createNotification('error', `An error occurred: ${error.message}`);
+    } finally {
+      handleClose();
     }
-    handleClose();
   };
 
   return (
@@ -171,8 +189,13 @@ export const RBACManagementModal = ({ open, handleClose, currentRole, createNoti
               onChange: handleDatabaseSelect,
             }}
           />
+          {selectedDatabase && isDatabaseEmpty && (
+            <p style={{ color: 'red' }}>
+              This database is currently empty. Please select a different database or add labels to manage access.
+            </p>
+          )}
         </div>
-        {selectedDatabase && loaded && (
+        {selectedDatabase && !isDatabaseEmpty && loaded && (
           <>
             <br />
             <div style={{ display: 'flex', justifyContent: 'space-between' }}>

src/extensions/rbac/RBACUtils.ts

@@ -217,15 +217,24 @@ export const retrieveNeo4jUsers = (driver, currentRole, setNeo4jUsers, setRoleUs
  * @param setLabels callback to update the list of labels.
  */
 export function retrieveLabelsList(driver, database: any, setLabels: (records: any) => void) {
-  runCypherQuery(
-    driver,
-    database.value,
-    'CALL db.labels()',
-    {},
-    1000,
-    () => {},
-    (records) => setLabels(records)
-  );
+  let labelsSet = false; // Flag to track if setLabels was called
+
+  // Wrapper around the original setLabels to set the flag when called
+  const wrappedSetLabels = (records) => {
+    labelsSet = true;
+    setLabels(records);
+  };
+
+  runCypherQuery(driver, database, 'CALL db.labels()', {}, 1000, () => {}, wrappedSetLabels)
+    .then(() => {
+      if (!labelsSet) {
+        setLabels([]);
+      }
+    })
+    .catch((error) => {
+      console.error('Error retrieving labels:', error);
+      setLabels([]);
+    });
 }
 
 /**
@@ -265,34 +274,36 @@ export async function updateUsers(driver, currentRole, allUsers, selectedUsers,
     `REVOKE ROLE ${currentRole} FROM ${escapedAllUsers}`,
     {},
     1000,
-    (status) => {
+    async (status) => {
       globalStatus = status;
+      if (globalStatus == QueryStatus.NO_DATA || globalStatus == QueryStatus.COMPLETE) {
+        //  TODO: Neo4j is very slow in updating after the previous query, even though it is technically a finished query.
+        // We build in an artificial delay... This must be improved the future.
+        setTimeout(async () => {
+          if (selectedUsers.length > 0) {
+            const escapedSelectedUsers = selectedUsers.map((user) => `\`${user}\``).join(',');
+            await runCypherQuery(
+              driver,
+              'system',
+              `GRANT ROLE ${currentRole} TO ${escapedSelectedUsers};`,
+              {},
+              1000,
+              (status) => {
+                if (status == QueryStatus.NO_DATA || QueryStatus.COMPLETE) {
+                  onSuccess();
+                }
+              }
+            );
+          } else {
+            onSuccess();
+          }
+        }, 2000);
+      }
     },
     (records) => {
       if (records && records[0] && records[0].error) {
         onFail(records[0].error);
       }
     }
   );
-  if (globalStatus == QueryStatus.NO_DATA || globalStatus == QueryStatus.COMPLETE) {
-    //  TODO: Neo4j is very slow in updating after the previous query, even though it is technically a finished query.
-    // We build in an artificial delay...
-    if (selectedUsers.length > 0) {
-      const escapedSelectedUsers = selectedUsers.map((user) => `\`${user}\``).join(',');
-      await runCypherQuery(
-        driver,
-        'system',
-        `GRANT ROLE ${currentRole} TO ${escapedSelectedUsers}`,
-        {},
-        1000,
-        (status) => {
-          if (status == QueryStatus.NO_DATA || QueryStatus.COMPLETE) {
-            onSuccess();
-          }
-        }
-      );
-    } else {
-      onSuccess();
-    }
-  }
 }