Merge pull request #577 from CBIIT/CRDCDH-2102-V2

CRDCDH-2102 fix: The `useProfileFields` hook should utilize PBAC
CBIIT · Jan 2, 2025 · 9db2a7c · 9db2a7c
2 parents 8109792 + d5addb5
commit 9db2a7c
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 16 deletions.
diff --git a/src/hooks/useProfileFields.test.ts b/src/hooks/useProfileFields.test.ts
@@ -7,19 +7,23 @@ describe("Users View", () => {
     jest.clearAllMocks();
   });
 
-  it("should return UNLOCKED for role and userStatus when viewing users as an Admin", () => {
-    const user = { _id: "User-A", role: "Admin" } as User;
-    const profileOf: Pick<User, "_id" | "role"> = { _id: "I-Am-User-B", role: "Submitter" };
+  // NOTE: This is mostly a sanity check to ensure we're ignoring the signed-in user's role
+  it.each<UserRole>(["Admin", "Data Commons Personnel", "Federal Lead", "Submitter", "User"])(
+    "should return UNLOCKED for role, status, and PBAC when viewing users with management permission (%s)",
+    (role) => {
+      const user = { _id: "User-A", role, permissions: ["user:manage"] } as User;
+      const profileOf: Pick<User, "_id" | "role"> = { _id: "I-Am-User-B", role: "Submitter" };
 
-    jest.spyOn(Auth, "useAuthContext").mockReturnValue({ user } as Auth.ContextState);
+      jest.spyOn(Auth, "useAuthContext").mockReturnValue({ user } as Auth.ContextState);
 
-    const { result } = renderHook(() => useProfileFields(profileOf, "users"));
+      const { result } = renderHook(() => useProfileFields(profileOf, "users"));
 
-    expect(result.current.role).toBe("UNLOCKED");
-    expect(result.current.userStatus).toBe("UNLOCKED");
-    expect(result.current.permissions).toBe("UNLOCKED");
-    expect(result.current.notifications).toBe("UNLOCKED");
-  });
+      expect(result.current.role).toBe("UNLOCKED");
+      expect(result.current.userStatus).toBe("UNLOCKED");
+      expect(result.current.permissions).toBe("UNLOCKED");
+      expect(result.current.notifications).toBe("UNLOCKED");
+    }
+  );
 
   it("should return READ_ONLY for all standard fields when a Submitter views the page", () => {
     const user = { _id: "User-A", role: "Submitter" } as User;
@@ -44,7 +48,7 @@ describe("Users View", () => {
     ["UNLOCKED", "Federal Lead"],
     ["UNLOCKED", "Submitter"],
   ])("should return %s for the studies field on the users page for role %s", (state, role) => {
-    const user = { _id: "User-A", role: "Admin" } as User;
+    const user = { _id: "User-A", role: "Admin", permissions: ["user:manage"] } as User;
     const profileOf: Pick<User, "_id" | "role"> = { _id: "I-Am-User-B", role };
 
     jest.spyOn(Auth, "useAuthContext").mockReturnValue({ user } as Auth.ContextState);
@@ -63,7 +67,7 @@ describe("Users View", () => {
     ["HIDDEN", "fake role" as UserRole],
     ["UNLOCKED", "Data Commons Personnel"], // NOTE: accepts Data Commons
   ])("should return %s for the dataCommons field on the users page for role %s", (state, role) => {
-    const user = { _id: "User-A", role: "Admin" } as User;
+    const user = { _id: "User-A", role: "Admin", permissions: ["user:manage"] } as User;
     const profileOf: Pick<User, "_id" | "role"> = { _id: "I-Am-User-B", role };
 
     jest.spyOn(Auth, "useAuthContext").mockReturnValue({ user } as Auth.ContextState);

diff --git a/src/hooks/useProfileFields.ts b/src/hooks/useProfileFields.ts
@@ -1,5 +1,7 @@
 import { useAuthContext } from "../components/Contexts/AuthContext";
+import { hasPermission } from "../config/AuthPermissions";
 import { RequiresStudiesAssigned } from "../config/AuthRoles";
+
 /**
  * Constrains the fields that this hook supports generating states for
  */
@@ -47,6 +49,9 @@ const useProfileFields = (
   viewType: "users" | "profile"
 ): Readonly<Partial<ProfileFields>> => {
   const { user } = useAuthContext();
+  const canManage = hasPermission(user, "user", "manage");
+
+  const isSelf: boolean = user?._id === profileOf?._id;
   const fields: ProfileFields = {
     firstName: "READ_ONLY",
     lastName: "READ_ONLY",
@@ -57,16 +62,15 @@ const useProfileFields = (
     permissions: "HIDDEN",
     notifications: "HIDDEN",
   };
-  const isSelf: boolean = user?._id === profileOf?._id;
 
   // Editable for the current user viewing their own profile
   if (isSelf && viewType === "profile") {
     fields.firstName = "UNLOCKED";
     fields.lastName = "UNLOCKED";
   }
 
-  // Editable for Admin viewing Manage Users
-  if (user?.role === "Admin" && viewType === "users") {
+  // Editable for user with permission to Manage Users
+  if (canManage && viewType === "users") {
     fields.role = "UNLOCKED";
     fields.userStatus = "UNLOCKED";
     fields.permissions = "UNLOCKED";
@@ -78,7 +82,7 @@ const useProfileFields = (
 
   // Only applies to Data Commons Personnel
   if (profileOf?.role === "Data Commons Personnel") {
-    fields.dataCommons = user?.role === "Admin" && viewType === "users" ? "UNLOCKED" : "READ_ONLY";
+    fields.dataCommons = canManage && viewType === "users" ? "UNLOCKED" : "READ_ONLY";
   } else {
     fields.dataCommons = "HIDDEN";
   }