From 8ff6060de92e009c360c297c47c6755e01844d0c Mon Sep 17 00:00:00 2001 From: Alejandro-Vega Date: Tue, 17 Dec 2024 12:23:47 -0500 Subject: [PATCH 001/102] Centralized PBAC management based on user permissions. Also added tests --- src/config/AuthPermissions.test.ts | 78 ++++++++++ src/config/AuthPermissions.ts | 240 +++++++++++++++++++++++++++++ 2 files changed, 318 insertions(+) create mode 100644 src/config/AuthPermissions.test.ts create mode 100644 src/config/AuthPermissions.ts diff --git a/src/config/AuthPermissions.test.ts b/src/config/AuthPermissions.test.ts new file mode 100644 index 000000000..b955a4184 --- /dev/null +++ b/src/config/AuthPermissions.test.ts @@ -0,0 +1,78 @@ +import { hasPermission } from "./AuthPermissions"; + +const createUser = (role: UserRole, permissions: AuthPermissions[] = []): User => ({ + role, + permissions, + notifications: [], + _id: "user-1", + firstName: "", + lastName: "", + email: "", + dataCommons: [], + studies: [], + IDP: "nih", + userStatus: "Active", + updateAt: "", + createdAt: "", +}); + +describe("Basic Role-Based Permissions", () => { + it("should allow a Federal Lead to view the dashboard", () => { + const user = createUser("Federal Lead", ["dashboard:view"]); + expect(hasPermission(user, "dashboard", "view")).toBe(true); + }); + + it("should deny a Federal Lead to view the dashboard without the defined permissions", () => { + const user = createUser("Federal Lead", []); + expect(hasPermission(user, "dashboard", "view")).toBe(false); + }); + + it("should deny a Submitter to view the dashboard", () => { + const user = createUser("Submitter"); + expect(hasPermission(user, "dashboard", "view")).toBe(false); + }); + + it("should allow a Submitter to create a submission request", () => { + const user = createUser("Submitter", ["submission_request:create"]); + expect(hasPermission(user, "submission_request", "create")).toBe(true); + }); + + it("should deny a User to create a submission request", () => { + const user = createUser("User"); + expect(hasPermission(user, "submission_request", "create")).toBe(false); + }); +}); + +describe("Edge Cases", () => { + it("should deny permission if the user role is invalid", () => { + const user = createUser("InvalidRole" as UserRole); + expect(hasPermission(user, "dashboard", "view")).toBe(false); + }); + + it("should deny permission if the user role is null or undefined", () => { + const user = createUser(undefined); + expect(hasPermission(user, "dashboard", "view")).toBe(false); + }); + + it("should deny permission if the action is invalid", () => { + const user = createUser("Admin"); + expect(hasPermission(user, "dashboard", "invalid_action" as never)).toBe(false); + }); + + it("should deny permission if the resource is invalid", () => { + const user = createUser("Admin"); + expect(hasPermission(user, "invalid_resource" as never, "view" as never)).toBe(false); + }); +}); + +describe("Permission String Check", () => { + it("should verify that the user's permissions include the correct serialized permission key", () => { + const user = createUser("Submitter", ["submission_request:create"]); + expect(hasPermission(user, "submission_request", "create")).toBe(true); + }); + + it("should deny access if the serialized permission key is missing", () => { + const user = createUser("Submitter", []); + expect(hasPermission(user, "submission_request", "create")).toBe(false); + }); +}); diff --git a/src/config/AuthPermissions.ts b/src/config/AuthPermissions.ts new file mode 100644 index 000000000..14f83a425 --- /dev/null +++ b/src/config/AuthPermissions.ts @@ -0,0 +1,240 @@ +type PermissionCheck = + | boolean + | ((user: User, data: Permissions[Key]["dataType"]) => boolean); + +type RolesWithPermissions = { + [R in UserRole]: Partial<{ + [Key in keyof Permissions]: Partial<{ + [Action in Permissions[Key]["action"]]: PermissionCheck; + }>; + }>; +}; + +type Permissions = { + access: { + dataType: null; + action: "request"; + }; + dashboard: { + dataType: null; + action: "view"; + }; + submission_request: { + dataType: Application; + action: "view" | "create" | "submit" | "review"; + }; + data_submission: { + dataType: Submission; + action: "view" | "create" | "review" | "admin_submit" | "confirm"; + }; + user: { + dataType: null; + action: "manage"; + }; + program: { + dataType: null; + action: "manage"; + }; + study: { + dataType: null; + action: "manage"; + }; +}; + +export const ROLES = { + "Federal Lead": { + submission_request: { + view: true, + create: true, + submit: true, + review: true, + }, + dashboard: { + view: true, + }, + data_submission: { + view: true, + create: true, + review: true, + admin_submit: false, + confirm: true, + }, + access: { + request: false, + }, + user: { + manage: true, + }, + program: { + manage: true, + }, + study: { + manage: true, + }, + }, + "Data Commons Personnel": { + submission_request: { + view: true, + create: true, + submit: true, + review: true, + }, + dashboard: { + view: true, + }, + data_submission: { + view: true, + create: true, + review: true, + admin_submit: true, + confirm: true, + }, + access: { + request: false, + }, + user: { + manage: false, + }, + program: { + manage: true, + }, + study: { + manage: true, + }, + }, + Admin: { + submission_request: { + view: true, + create: false, + submit: false, + review: false, + }, + dashboard: { + view: true, + }, + data_submission: { + view: true, + create: false, + review: true, + admin_submit: true, + confirm: true, + }, + access: { + request: false, + }, + user: { + manage: true, + }, + program: { + manage: true, + }, + study: { + manage: true, + }, + }, + Submitter: { + submission_request: { + view: false, + create: true, + submit: false, + review: false, + }, + dashboard: { + view: false, + }, + data_submission: { + view: true, + create: true, + review: false, + admin_submit: false, + confirm: false, + }, + access: { + request: true, + }, + user: { + manage: false, + }, + program: { + manage: false, + }, + study: { + manage: false, + }, + }, + User: { + submission_request: { + view: false, + create: true, + submit: false, + review: false, + }, + dashboard: { + view: false, + }, + data_submission: { + view: false, + create: false, + review: false, + admin_submit: false, + confirm: false, + }, + access: { + request: true, + }, + user: { + manage: false, + }, + program: { + manage: false, + }, + study: { + manage: false, + }, + }, +} as const satisfies RolesWithPermissions; + +/** + * Determines if a user has the necessary permission to perform a specific action on a resource. + * + * @template Resource - A key of the `Permissions` type representing the resource to check. + * @param {User} user - The user object, which contains the user's role and permissions. + * @param {Resource} resource - The resource on which the action is being performed. + * @param {Permissions[Resource]["action"]} action - The action to check permission for. + * @param {Permissions[Resource]["dataType"]} [data] - Optional additional data needed for dynamic permission checks. + * @returns {boolean} - `true` if the user has permission, otherwise `false`. + * + * @example + * // Basic permission check without additional data + * const canCreate = hasPermission(user, "submission_request", "create"); + * + * @example + * // Permission check with additional data + * const canSubmit = hasPermission(user, "submission_request", "submit", applicationData); + */ +export const hasPermission = ( + user: User, + resource: Resource, + action: Permissions[Resource]["action"], + data?: Permissions[Resource]["dataType"] +): boolean => { + const { role } = user || {}; + + if (!role) { + return false; + } + + const permission = (ROLES as RolesWithPermissions)[role]?.[resource]?.[action]; + const permissionKey = `${resource}:${action}`; + + // If permission not defined, or not listed within the user permissions, then deny permission + if (permission == null || !user.permissions?.includes(permissionKey as AuthPermissions)) { + return false; + } + + if (typeof permission === "boolean") { + return permission; + } + + return !!data && permission(user, data); +}; From 70aca1f77d80bf97a7e10359f231c2fff54adaf1 Mon Sep 17 00:00:00 2001 From: Alejandro-Vega Date: Tue, 17 Dec 2024 12:28:00 -0500 Subject: [PATCH 002/102] Update roles and permission types --- src/config/AuthRoles.ts | 102 +--------------------------------------- src/types/Auth.d.ts | 46 ++++++++++++++---- 2 files changed, 39 insertions(+), 109 deletions(-) diff --git a/src/config/AuthRoles.ts b/src/config/AuthRoles.ts index 7a1fe285b..14e47ece7 100644 --- a/src/config/AuthRoles.ts +++ b/src/config/AuthRoles.ts @@ -6,110 +6,12 @@ export const Roles: UserRole[] = [ "User", "Submitter", - "Organization Owner", - "Federal Monitor", "Federal Lead", - "Data Curator", - "Data Commons POC", "Admin", + "Data Commons Personnel", ]; -/** - * Defines a list of roles that are allowed to interact with regular Validation. - */ -export const ValidateRoles: UserRole[] = [ - "Submitter", - "Organization Owner", - "Data Curator", - "Admin", -]; - -/** - * Defines a list of roles that are allowed to interact with Cross Validation. - */ -export const CrossValidateRoles: UserRole[] = ["Admin", "Data Curator"]; - -/** - * Defines a list of roles that can, at a minimum, view profiles of other users. - */ -export const CanManageUsers: UserRole[] = ["Admin", "Organization Owner"]; - -/** - * Defines a list of roles that are allowed to generate an API token. - * - * @note This also directly defines the roles that are allowed to generate a CLI config. - */ -export const GenerateApiTokenRoles: User["role"][] = ["Organization Owner", "Submitter"]; - -/** - * Defines a list of roles that are allowed to interact with the Operation Dashboard. - */ -export const DashboardRoles: UserRole[] = [ - "Admin", - "Data Curator", - "Federal Lead", - "Federal Monitor", -]; - -/** - * Defines a list of roles that are allowed to submit a Data Submission - */ -export const SubmitDataSubmissionRoles: UserRole[] = [ - "Submitter", - "Organization Owner", - "Data Curator", - "Admin", -]; - -/** - * Defines a list of roles that are allowed to view Data Submissions - * outside of their organization - */ -export const canViewOtherOrgRoles: UserRole[] = [ - "Admin", - "Data Commons POC", - "Data Curator", - "Federal Lead", - "Federal Monitor", -]; - -/** - * Defines a list of roles that can modify collaborators in a Data Submission - */ -export const canModifyCollaboratorsRoles: UserRole[] = ["Submitter", "Organization Owner"]; - -/** - * The users with permission to delete data nodes from a submission. - * - */ -export const canDeleteDataNodesRoles: UserRole[] = [ - "Submitter", - "Organization Owner", - "Data Curator", - "Admin", -]; - -/** - * Defines a list of roles that can upload metadata to a Data Submission - */ -export const canUploadMetadataRoles: UserRole[] = ["Submitter", "Organization Owner"]; - -/** - * A set of user roles that are allowed to request a role change from their profile. - */ -export const CanRequestRoleChange: UserRole[] = ["User", "Submitter", "Organization Owner"]; - -/** - * A set of user roles that are allowed to create a Submission Request form. - */ -export const CanCreateSubmissionRequest: UserRole[] = ["User", "Submitter", "Organization Owner"]; - -/** - * A set of user roles that are allowed to Submit a Submission Request. - */ -export const CanSubmitSubmissionRequestRoles: UserRole[] = ["User", "Submitter", "Federal Lead"]; - /** * A set of roles that are constrained to a set of studies. */ -export const RequiresStudiesAssigned: UserRole[] = ["Submitter", "Federal Monitor", "Federal Lead"]; +export const RequiresStudiesAssigned: UserRole[] = ["Submitter", "Federal Lead"]; diff --git a/src/types/Auth.d.ts b/src/types/Auth.d.ts index 8f4fe6178..28d8aca62 100644 --- a/src/types/Auth.d.ts +++ b/src/types/Auth.d.ts @@ -47,6 +47,14 @@ type User = { * The current account status for the user */ userStatus: "Active" | "Inactive" | "Disabled"; + /** + * The list of permissions granted to the user + */ + permissions: AuthPermissions[]; + /** + * The list of notifications the user will receive + */ + notifications: string[]; /** * The last update date of the user object * @@ -61,15 +69,35 @@ type User = { createdAt: string; }; -type UserRole = - | "User" - | "Submitter" - | "Organization Owner" - | "Federal Monitor" - | "Federal Lead" - | "Data Curator" - | "Data Commons POC" - | "Admin"; +type UserRole = "User" | "Admin" | "Data Commons Personnel" | "Federal Lead" | "Submitter"; + +type SubmissionRequestPermissions = + | "submission_request:view" + | "submission_request:create" + | "submission_request:review" + | "submission_request:submit"; + +type DataSubmissionPermissions = + | "data_submission:view" + | "data_submission:create" + | "data_submission:review" + | "data_submission:admin_submit" + | "data_submission:confirm"; + +type DashboardPermissions = "dashboard:view"; +type AccessPermissions = "access:request"; +type UserPermissions = "user:manage"; +type ProgramPermissions = "program:manage"; +type StudyPermissions = "study:manage"; + +type AuthPermissions = + | SubmissionRequestPermissions + | DataSubmissionPermissions + | DashboardPermissions + | AccessPermissions + | UserPermissions + | ProgramPermissions + | StudyPermissions; type OrgInfo = { orgID: string; From b3cf5f5a0f793eb92f858e50aabf961524ab16ba Mon Sep 17 00:00:00 2001 From: Alejandro-Vega Date: Tue, 17 Dec 2024 12:53:09 -0500 Subject: [PATCH 003/102] Update Access Request with new permission check and updated roles --- .../AccessRequest/FormDialog.test.tsx | 2 + src/components/AccessRequest/FormDialog.tsx | 2 +- src/components/AccessRequest/index.test.tsx | 43 ++++++++----------- src/components/AccessRequest/index.tsx | 4 +- 4 files changed, 22 insertions(+), 29 deletions(-) diff --git a/src/components/AccessRequest/FormDialog.test.tsx b/src/components/AccessRequest/FormDialog.test.tsx index 1d6e696fb..5fdac03aa 100644 --- a/src/components/AccessRequest/FormDialog.test.tsx +++ b/src/components/AccessRequest/FormDialog.test.tsx @@ -87,6 +87,8 @@ const mockUser: User = { userStatus: "Active", updateAt: "", createdAt: "", + permissions: ["access:request"], + notifications: [], }; type MockParentProps = { diff --git a/src/components/AccessRequest/FormDialog.tsx b/src/components/AccessRequest/FormDialog.tsx index 709bce8b4..987bc2477 100644 --- a/src/components/AccessRequest/FormDialog.tsx +++ b/src/components/AccessRequest/FormDialog.tsx @@ -71,7 +71,7 @@ type Props = { onClose: () => void; } & Omit; -const RoleOptions: UserRole[] = ["Submitter", "Organization Owner"]; +const RoleOptions: UserRole[] = ["Submitter"]; /** * Provides a dialog for users to request access to a specific role. diff --git a/src/components/AccessRequest/index.test.tsx b/src/components/AccessRequest/index.test.tsx index ba5835912..d0ca1ca98 100644 --- a/src/components/AccessRequest/index.test.tsx +++ b/src/components/AccessRequest/index.test.tsx @@ -15,7 +15,7 @@ import { ListApprovedStudiesResp, } from "../../graphql"; -const mockUser: Omit = { +const mockUser: Omit = { _id: "", firstName: "", lastName: "", @@ -27,6 +27,7 @@ const mockUser: Omit = { userStatus: "Active", updateAt: "", createdAt: "", + notifications: [], }; const mockListApprovedStudies: MockedResponse = { @@ -47,15 +48,16 @@ const mockListApprovedStudies: MockedResponse = ({ mocks, role, children }) => { +const MockParent: FC = ({ mocks, role, permissions, children }) => { const authValue: AuthContextState = useMemo( () => ({ isLoggedIn: true, status: AuthContextStatus.LOADED, - user: { ...mockUser, role }, + user: { ...mockUser, role, permissions }, }), [role] ); @@ -70,7 +72,7 @@ const MockParent: FC = ({ mocks, role, children }) => { describe("Accessibility", () => { it("should not have any violations", async () => { const { container } = render(, { - wrapper: (p) => , + wrapper: (p) => , }); expect(await axe(container)).toHaveNoViolations(); @@ -80,7 +82,14 @@ describe("Accessibility", () => { describe("Basic Functionality", () => { it("should open the dialog when the 'Request Access' button is clicked", async () => { const { getByTestId, getByRole, queryByRole } = render(, { - wrapper: (p) => , + wrapper: (p) => ( + + ), }); expect(queryByRole("dialog")).not.toBeInTheDocument(); @@ -94,34 +103,16 @@ describe("Basic Functionality", () => { describe("Implementation Requirements", () => { it("should have a button with the text content 'Request Access'", async () => { const { getByText } = render(, { - wrapper: (p) => , + wrapper: (p) => , }); expect(getByText("Request Access")).toBeInTheDocument(); expect(getByText("Request Access")).toBeEnabled(); }); - it.each(["User", "Submitter", "Organization Owner"])( - "should render the 'Request Access' button for the '%s' role", - (role) => { - const { getByTestId } = render(, { - wrapper: (p) => , - }); - - expect(getByTestId("request-access-button")).toBeInTheDocument(); - } - ); - - it.each([ - "Admin", - "Data Commons POC", - "Federal Lead", - "Federal Monitor", - "Data Curator", - "fake role" as UserRole, - ])("should not render the 'Request Access' button for the '%s' role", (role) => { + it("should not render the 'Request Access' button without the required permission", async () => { const { queryByTestId } = render(, { - wrapper: (p) => , + wrapper: (p) => , }); expect(queryByTestId("request-access-button")).not.toBeInTheDocument(); diff --git a/src/components/AccessRequest/index.tsx b/src/components/AccessRequest/index.tsx index d6e223334..e76af66ae 100644 --- a/src/components/AccessRequest/index.tsx +++ b/src/components/AccessRequest/index.tsx @@ -2,7 +2,7 @@ import { FC, memo, useState } from "react"; import { Button, styled } from "@mui/material"; import FormDialog from "./FormDialog"; import { useAuthContext } from "../Contexts/AuthContext"; -import { CanRequestRoleChange } from "../../config/AuthRoles"; +import { hasPermission } from "../../config/AuthPermissions"; const StyledButton = styled(Button)({ marginLeft: "42px", @@ -38,7 +38,7 @@ const AccessRequest: FC = (): React.ReactNode => { setDialogOpen(false); }; - if (!user?.role || !CanRequestRoleChange.includes(user.role)) { + if (!hasPermission(user, "access", "request")) { return null; } From c8afdcef5314eff3454d3c4bec050d97b274bd28 Mon Sep 17 00:00:00 2001 From: Alejandro-Vega Date: Tue, 17 Dec 2024 12:54:25 -0500 Subject: [PATCH 004/102] Update API Token Dialog with new permission check and updated roles --- src/components/APITokenDialog/index.test.tsx | 34 +++++++++++--------- src/components/APITokenDialog/index.tsx | 4 +-- 2 files changed, 21 insertions(+), 17 deletions(-) diff --git a/src/components/APITokenDialog/index.test.tsx b/src/components/APITokenDialog/index.test.tsx index 785fe9483..cc3ecbf2a 100644 --- a/src/components/APITokenDialog/index.test.tsx +++ b/src/components/APITokenDialog/index.test.tsx @@ -26,7 +26,7 @@ const baseAuthCtx: AuthContextState = { user: null, }; -const baseUser: User = { +const baseUser: Omit = { _id: "", firstName: "", lastName: "", @@ -38,20 +38,27 @@ const baseUser: User = { dataCommons: [], createdAt: "", updateAt: "", + notifications: [], }; type ParentProps = { children: React.ReactNode; mocks?: MockedResponse[]; role?: UserRole; + permissions?: AuthPermissions[]; }; -const TestParent: FC = ({ role = "Submitter", mocks = [], children }) => { +const TestParent: FC = ({ + role = "Submitter", + permissions = ["data_submission:create"], + mocks = [], + children, +}) => { const authState = useMemo( () => ({ ...baseAuthCtx, isLoggedIn: true, - user: { ...baseUser, role }, + user: { ...baseUser, role, permissions }, }), [role] ); @@ -264,18 +271,15 @@ describe("Implementation Requirements", () => { expect(mockWriteText).not.toHaveBeenCalled(); }); - it.each(["Admin", "Federal Lead", "Data Curator", "User", "fake user" as UserRole])( - "should show an error when the user role %s tries to generate a token", - async (role) => { - const { getByText } = render(, { - wrapper: (p) => , - }); + it("should show an error when the user without the required permission tries to generate a token", async () => { + const { getByText } = render(, { + wrapper: (p) => , + }); - userEvent.click(getByText(/Create Token/, { selector: "button" })); + userEvent.click(getByText(/Create Token/, { selector: "button" })); - await waitFor(() => { - expect(getByText(/Token was unable to be created./)).toBeInTheDocument(); - }); - } - ); + await waitFor(() => { + expect(getByText(/Token was unable to be created./)).toBeInTheDocument(); + }); + }); }); diff --git a/src/components/APITokenDialog/index.tsx b/src/components/APITokenDialog/index.tsx index 70e0d7ef5..ed1165f08 100644 --- a/src/components/APITokenDialog/index.tsx +++ b/src/components/APITokenDialog/index.tsx @@ -15,7 +15,7 @@ import GenericAlert, { AlertState } from "../GenericAlert"; import { ReactComponent as CopyIconSvg } from "../../assets/icons/copy_icon.svg"; import { ReactComponent as CloseIconSvg } from "../../assets/icons/close_icon.svg"; import { useAuthContext } from "../Contexts/AuthContext"; -import { GenerateApiTokenRoles } from "../../config/AuthRoles"; +import { hasPermission } from "../../config/AuthPermissions"; const StyledDialog = styled(Dialog)({ "& .MuiDialog-paper": { @@ -166,7 +166,7 @@ const APITokenDialog: FC = ({ onClose, open, ...rest }) => { }; const generateToken = async () => { - if (!GenerateApiTokenRoles.includes(user?.role)) { + if (!hasPermission(user, "data_submission", "create")) { onGenerateTokenError(); return; } From 781f0af1583aca7393b55a06049f5bd4dbc14e92 Mon Sep 17 00:00:00 2001 From: Alejandro-Vega Date: Tue, 17 Dec 2024 12:55:41 -0500 Subject: [PATCH 005/102] Update Collaborators with new permission check and updated roles --- .../CollaboratorsDialog.test.tsx | 83 +++---------------- .../Collaborators/CollaboratorsDialog.tsx | 10 +-- .../Collaborators/CollaboratorsTable.test.tsx | 2 + .../Contexts/CollaboratorsContext.test.tsx | 6 ++ 4 files changed, 23 insertions(+), 78 deletions(-) diff --git a/src/components/Collaborators/CollaboratorsDialog.test.tsx b/src/components/Collaborators/CollaboratorsDialog.test.tsx index 74b3e03bb..cee3c2218 100644 --- a/src/components/Collaborators/CollaboratorsDialog.test.tsx +++ b/src/components/Collaborators/CollaboratorsDialog.test.tsx @@ -45,6 +45,8 @@ const mockUser: User = { userStatus: "Active", updateAt: "", createdAt: "", + permissions: ["data_submission:view", "data_submission:create"], + notifications: [], }; const mockSubmission = { @@ -182,10 +184,7 @@ describe("CollaboratorsDialog Component", () => { it("calls onClose when Close button is clicked", () => { mockUseAuthContext.mockReturnValue({ - user: { - ...mockUser, - role: "Admin", - }, + user: { ...mockUser, _id: "some-other-user" } as User, status: AuthStatus.LOADED, }); @@ -367,20 +366,12 @@ describe("CollaboratorsDialog Component", () => { }); }); - it.each([ - "User", - "Admin", - "Data Curator", - "Data Commons POC", - "Federal Lead", - "Federal Monitor", - "invalid-role" as UserRole, - ])("should disable inputs when user is role %s", (role) => { + it("should disable inputs when user does not have required permissions", async () => { mockUseAuthContext.mockReturnValue({ user: { ...mockUser, - role, - }, + permissions: ["data_submission:view"], + } as User, status: AuthStatus.LOADED, }); @@ -396,67 +387,15 @@ describe("CollaboratorsDialog Component", () => { expect(getByTestId("collaborators-dialog-close-button")).toBeInTheDocument(); }); - it.each(["Submitter", "Organization Owner"])( - "should enable inputs when user is role %s", - (role) => { - mockUseAuthContext.mockReturnValue({ - user: { - ...mockUser, - role, - }, - status: AuthStatus.LOADED, - }); - - const mockOnClose = jest.fn(); - const { getByTestId, queryByTestId } = render( - - - - ); - - expect(getByTestId("collaborators-dialog-save-button")).toBeInTheDocument(); - expect(getByTestId("collaborators-dialog-cancel-button")).toBeInTheDocument(); - expect(queryByTestId("collaborators-dialog-close-button")).not.toBeInTheDocument(); - } - ); - - it("allows modification when user is Organization Owner regardless of submitterID", () => { - mockUseAuthContext.mockReturnValue({ - user: { ...mockUser, role: "Organization Owner", _id: "user-99" }, - status: AuthStatus.LOADED, - }); - - mockUseSubmissionContext.mockReturnValue({ - data: { getSubmission: { ...mockSubmission, submitterID: "user-1" } }, - }); - - const mockOnClose = jest.fn(); - const { getByTestId, queryByTestId } = render( - - - - ); - - expect(getByTestId("collaborators-dialog-save-button")).toBeInTheDocument(); - expect(getByTestId("collaborators-dialog-cancel-button")).toBeInTheDocument(); - expect(queryByTestId("collaborators-dialog-close-button")).not.toBeInTheDocument(); - }); - - it("should not allow modification when user is Organization Owner of a different organization", () => { + it("should enable inputs when user has the required permissions", async () => { mockUseAuthContext.mockReturnValue({ user: { ...mockUser, - role: "Organization Owner", - _id: "user-99", - organization: { orgID: "some-other-org" }, + permissions: ["data_submission:view", "data_submission:create"], } as User, status: AuthStatus.LOADED, }); - mockUseSubmissionContext.mockReturnValue({ - data: { getSubmission: { ...mockSubmission, submitterID: "user-1" } }, - }); - const mockOnClose = jest.fn(); const { getByTestId, queryByTestId } = render( @@ -464,8 +403,8 @@ describe("CollaboratorsDialog Component", () => { ); - expect(queryByTestId("collaborators-dialog-save-button")).not.toBeInTheDocument(); - expect(queryByTestId("collaborators-dialog-cancel-button")).not.toBeInTheDocument(); - expect(getByTestId("collaborators-dialog-close-button")).toBeInTheDocument(); + expect(getByTestId("collaborators-dialog-save-button")).toBeInTheDocument(); + expect(getByTestId("collaborators-dialog-cancel-button")).toBeInTheDocument(); + expect(queryByTestId("collaborators-dialog-close-button")).not.toBeInTheDocument(); }); }); diff --git a/src/components/Collaborators/CollaboratorsDialog.tsx b/src/components/Collaborators/CollaboratorsDialog.tsx index 08217a983..7c0c16dd1 100644 --- a/src/components/Collaborators/CollaboratorsDialog.tsx +++ b/src/components/Collaborators/CollaboratorsDialog.tsx @@ -5,8 +5,8 @@ import { ReactComponent as CloseIconSvg } from "../../assets/icons/close_icon.sv import CollaboratorsTable from "./CollaboratorsTable"; import { useCollaboratorsContext } from "../Contexts/CollaboratorsContext"; import { useSubmissionContext } from "../Contexts/SubmissionContext"; -import { canModifyCollaboratorsRoles } from "../../config/AuthRoles"; import { Status as AuthStatus, useAuthContext } from "../Contexts/AuthContext"; +import { hasPermission } from "../../config/AuthPermissions"; const StyledDialog = styled(Dialog)({ "& .MuiDialog-paper": { @@ -111,11 +111,9 @@ const CollaboratorsDialog = ({ onClose, onSave, open, ...rest }: Props) => { const isLoading = collaboratorLoading || status === AuthStatus.LOADING; const canModifyCollaborators = useMemo( () => - canModifyCollaboratorsRoles.includes(user?.role) && - (submission?.getSubmission?.submitterID === user?._id || - (user?.role === "Organization Owner" && - user?.organization?.orgID === submission?.getSubmission?.organization?._id)), - [canModifyCollaboratorsRoles, user, submission?.getSubmission] + hasPermission(user, "data_submission", "create") && + submission?.getSubmission?.submitterID === user?._id, + [user, submission?.getSubmission] ); useEffect(() => { diff --git a/src/components/Collaborators/CollaboratorsTable.test.tsx b/src/components/Collaborators/CollaboratorsTable.test.tsx index 73b8caf18..ef2529e68 100644 --- a/src/components/Collaborators/CollaboratorsTable.test.tsx +++ b/src/components/Collaborators/CollaboratorsTable.test.tsx @@ -51,6 +51,8 @@ const mockUser: User = { userStatus: "Active", updateAt: "", createdAt: "", + permissions: ["data_submission:create"], + notifications: [], }; const mockSubmission: Submission = { diff --git a/src/components/Contexts/CollaboratorsContext.test.tsx b/src/components/Contexts/CollaboratorsContext.test.tsx index eaa041f07..bff17bcc2 100644 --- a/src/components/Contexts/CollaboratorsContext.test.tsx +++ b/src/components/Contexts/CollaboratorsContext.test.tsx @@ -66,6 +66,8 @@ const mockPotentialCollaborators: User[] = [ userStatus: "Active", updateAt: "", createdAt: "", + permissions: [], + notifications: [], }, { _id: "user-2", @@ -86,6 +88,8 @@ const mockPotentialCollaborators: User[] = [ userStatus: "Active", updateAt: "", createdAt: "", + permissions: [], + notifications: [], }, { _id: "user-3", @@ -106,6 +110,8 @@ const mockPotentialCollaborators: User[] = [ userStatus: "Active", updateAt: "", createdAt: "", + permissions: [], + notifications: [], }, ]; From fc7126d50b153b1f9f8b90502c1588f2c652a5ee Mon Sep 17 00:00:00 2001 From: Alejandro-Vega Date: Tue, 17 Dec 2024 12:56:41 -0500 Subject: [PATCH 006/102] Update create data submission dialog with new permission check and updated roles --- .../DataSubmissions/CreateDataSubmissionDialog.test.tsx | 2 ++ src/components/DataSubmissions/CreateDataSubmissionDialog.tsx | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/components/DataSubmissions/CreateDataSubmissionDialog.test.tsx b/src/components/DataSubmissions/CreateDataSubmissionDialog.test.tsx index 452077c43..8e48473e1 100644 --- a/src/components/DataSubmissions/CreateDataSubmissionDialog.test.tsx +++ b/src/components/DataSubmissions/CreateDataSubmissionDialog.test.tsx @@ -71,6 +71,8 @@ const baseUser: Omit = { dataCommons: [], createdAt: "", updateAt: "", + permissions: ["data_submission:create"], + notifications: [], }; const baseAuthCtx: AuthCtxState = { diff --git a/src/components/DataSubmissions/CreateDataSubmissionDialog.tsx b/src/components/DataSubmissions/CreateDataSubmissionDialog.tsx index 94b322ac1..680314fcb 100644 --- a/src/components/DataSubmissions/CreateDataSubmissionDialog.tsx +++ b/src/components/DataSubmissions/CreateDataSubmissionDialog.tsx @@ -34,6 +34,7 @@ import BaseStyledHelperText from "../StyledFormComponents/StyledHelperText"; import Tooltip from "../Tooltip"; import { Logger, validateEmoji } from "../../utils"; import { RequiresStudiesAssigned } from "../../config/AuthRoles"; +import { hasPermission } from "../../config/AuthPermissions"; const CreateSubmissionDialog = styled(Dialog)({ "& .MuiDialog-paper": { @@ -259,7 +260,6 @@ const CreateDataSubmissionDialog: FC = ({ onCreate }) => { ); }, [shouldFetchAllStudies, allStudies, user?.studies]); - const orgOwnerOrSubmitter = user?.role === "Organization Owner" || user?.role === "Submitter"; const intention = watch("intention"); const submissionTypeOptions: RadioOption[] = [ { @@ -574,7 +574,7 @@ const CreateDataSubmissionDialog: FC = ({ onCreate }) => { - {orgOwnerOrSubmitter && ( + {hasPermission(user, "data_submission", "create") && ( Date: Tue, 17 Dec 2024 12:58:24 -0500 Subject: [PATCH 007/102] Update cross validation button with new permission check and updated roles --- .../CrossValidationButton.test.tsx | 68 ++++++++++--------- .../DataSubmissions/CrossValidationButton.tsx | 5 +- 2 files changed, 38 insertions(+), 35 deletions(-) diff --git a/src/components/DataSubmissions/CrossValidationButton.test.tsx b/src/components/DataSubmissions/CrossValidationButton.test.tsx index a615186e1..fef6581fe 100644 --- a/src/components/DataSubmissions/CrossValidationButton.test.tsx +++ b/src/components/DataSubmissions/CrossValidationButton.test.tsx @@ -84,6 +84,8 @@ const baseUser: Omit = { dataCommons: [], createdAt: "", updateAt: "", + permissions: ["data_submission:view", "data_submission:review"], + notifications: [], }; type ParentProps = { @@ -510,44 +512,46 @@ describe("Implementation Requirements", () => { } ); - it.each(["Data Curator", "Admin"])( - "should always render for the role %s with Other Submissions present", - (role) => { - const { getByTestId } = render( - - - - ); + it("should always render for user with the required permissions while other Submissions are present", () => { + const { getByTestId } = render( + + + + ); - expect(getByTestId("cross-validate-button")).toBeInTheDocument(); - } - ); + expect(getByTestId("cross-validate-button")).toBeInTheDocument(); + }); - it.each([ - "Submitter", - "Organization Owner", - "Federal Lead", - "Data Commons POC", - "fake role" as User["role"], - ])("should never render for the role %s", (role) => { + it("should never render for user without the required permissions while other Submissions are present", () => { const { getByTestId } = render( - + = ({ submission, ...props }) => { }, [crossSubmissionStatus]); if ( - !user?.role || - !CrossValidateRoles.includes(user.role) || + !hasPermission(user, "data_submission", "review", submission) || !hasOtherSubmissions || status !== "Submitted" ) { From 8734bf3ac846b4aa1c1c3761fa652a79cd30b080 Mon Sep 17 00:00:00 2001 From: Alejandro-Vega Date: Tue, 17 Dec 2024 13:00:21 -0500 Subject: [PATCH 008/102] Updated failing Data Submission tests with new permission check and updated roles --- .../DataSubmissionListFilters.test.tsx | 2 ++ .../DataSubmissionSummary.test.tsx | 2 ++ .../ExportRequestButton/index.test.tsx | 22 ++++++++----------- .../dataSubmissions/SubmittedData.test.tsx | 2 ++ 4 files changed, 15 insertions(+), 13 deletions(-) diff --git a/src/components/DataSubmissions/DataSubmissionListFilters.test.tsx b/src/components/DataSubmissions/DataSubmissionListFilters.test.tsx index e0101cc55..18a9811c2 100644 --- a/src/components/DataSubmissions/DataSubmissionListFilters.test.tsx +++ b/src/components/DataSubmissions/DataSubmissionListFilters.test.tsx @@ -51,6 +51,8 @@ const TestParent: FC = ({ createdAt: "", updateAt: "", studies: null, + permissions: ["data_submission:create"], + notifications: [], }, }), [userRole] diff --git a/src/components/DataSubmissions/DataSubmissionSummary.test.tsx b/src/components/DataSubmissions/DataSubmissionSummary.test.tsx index 6f7aea26a..e5dd1019d 100644 --- a/src/components/DataSubmissions/DataSubmissionSummary.test.tsx +++ b/src/components/DataSubmissions/DataSubmissionSummary.test.tsx @@ -36,6 +36,8 @@ const baseUser: User = { dataCommons: [], createdAt: "", updateAt: "", + permissions: ["data_submission:view"], + notifications: [], }; const baseSubmissionCtx: SubmissionCtxState = { diff --git a/src/components/ExportRequestButton/index.test.tsx b/src/components/ExportRequestButton/index.test.tsx index 3ef130c22..d38f73dc9 100644 --- a/src/components/ExportRequestButton/index.test.tsx +++ b/src/components/ExportRequestButton/index.test.tsx @@ -241,20 +241,16 @@ describe("Implementation Requirements", () => { // NOTE: This component does not currently implement any authorization, // this is just future-proofing test coverage. - it.each([ - "User", - "Submitter", - "Organization Owner", - "Federal Lead", - "Admin", - "fake role" as UserRole, - ])("should be enabled for the user role %s if they can access the page", (role) => { - const { getByTestId } = render(, { - wrapper: (p) => , - }); + it.each(["User", "Submitter", "Federal Lead", "Admin", "fake role" as UserRole])( + "should be enabled for the user role %s if they can access the page", + (role) => { + const { getByTestId } = render(, { + wrapper: (p) => , + }); - expect(getByTestId("export-submission-request-button")).toBeEnabled(); - }); + expect(getByTestId("export-submission-request-button")).toBeEnabled(); + } + ); it("should format the PDF filename as 'CRDCSubmissionPortal-Request-{studyAbbr}-{submittedDate}.pdf'", async () => { const mockFormObject: Partial = { diff --git a/src/content/dataSubmissions/SubmittedData.test.tsx b/src/content/dataSubmissions/SubmittedData.test.tsx index d6e815c9d..8b00699e8 100644 --- a/src/content/dataSubmissions/SubmittedData.test.tsx +++ b/src/content/dataSubmissions/SubmittedData.test.tsx @@ -38,6 +38,8 @@ const baseUser: User = { dataCommons: [], createdAt: "", updateAt: "", + permissions: ["data_submission:create"], + notifications: [], }; const baseAuthCtx: AuthContextState = { From 2eac021e98ba474a06d1783fbb8f09be2e26c9f0 Mon Sep 17 00:00:00 2001 From: Alejandro-Vega Date: Tue, 17 Dec 2024 13:01:14 -0500 Subject: [PATCH 009/102] Updated Data Upload with new permission check and updated roles --- .../DataSubmissions/DataUpload.test.tsx | 59 ++++++++++--------- src/components/DataSubmissions/DataUpload.tsx | 4 +- 2 files changed, 32 insertions(+), 31 deletions(-) diff --git a/src/components/DataSubmissions/DataUpload.test.tsx b/src/components/DataSubmissions/DataUpload.test.tsx index b7f7efa82..40b3af3ba 100644 --- a/src/components/DataSubmissions/DataUpload.test.tsx +++ b/src/components/DataSubmissions/DataUpload.test.tsx @@ -59,7 +59,7 @@ const baseSubmission: Omit = { collaborators: [], }; -const baseUser: User = { +const baseUser: Omit = { _id: "current-user", firstName: "", lastName: "", @@ -71,20 +71,27 @@ const baseUser: User = { dataCommons: null, createdAt: "", updateAt: "", + notifications: [], }; type ParentProps = { mocks?: MockedResponse[]; - role?: User["role"]; + role?: UserRole; + permissions?: AuthPermissions[]; children: React.ReactNode; }; -const TestParent: FC = ({ mocks = [], role = "Submitter", children }) => { +const TestParent: FC = ({ + mocks = [], + role = "Submitter", + permissions = ["data_submission:view", "data_submission:create"], + children, +}) => { const authCtxState: AuthCtxState = useMemo( () => ({ status: AuthStatus.LOADED, isLoggedIn: true, - user: { ...baseUser, role }, + user: { ...baseUser, role, permissions }, }), [role] ); @@ -299,32 +306,26 @@ describe("Implementation Requirements", () => { expect(button).toBeVisible(); }); - it.each(["Submitter", "Organization Owner"])( - "should enable the Uploader CLI download button for '%s' role", - async (role) => { - const { getByTestId } = render( - , - { wrapper: (p) => } - ); + it("should enable the Uploader CLI download button when user has required permissions", async () => { + const { getByTestId } = render( + , + { + wrapper: (p) => ( + + ), + } + ); - expect(getByTestId("uploader-cli-config-button")).toBeEnabled(); - } - ); + expect(getByTestId("uploader-cli-config-button")).toBeEnabled(); + }); - it.each([ - "Admin", - "Data Curator", - "Data Commons POC", - "Federal Lead", - "User", - "fake role" as User["role"], // NOTE: asserting that a whitelist of allowed roles is used instead of a blacklist - ])("should disable the Uploader CLI download button for '%s' role", async (role) => { + it("should disable the Uploader CLI download button when user is missing required permissions", async () => { const { getByTestId } = render( { dataType: "Metadata and Data Files", // NOTE: Required for the button to show }} />, - { wrapper: (p) => } + { wrapper: (p) => } ); expect(getByTestId("uploader-cli-config-button")).toBeDisabled(); diff --git a/src/components/DataSubmissions/DataUpload.tsx b/src/components/DataSubmissions/DataUpload.tsx index 4b021f0fa..6af73441f 100644 --- a/src/components/DataSubmissions/DataUpload.tsx +++ b/src/components/DataSubmissions/DataUpload.tsx @@ -10,7 +10,7 @@ import FlowWrapper from "./FlowWrapper"; import UploaderToolDialog from "../UploaderToolDialog"; import UploaderConfigDialog, { InputForm } from "../UploaderConfigDialog"; import { useAuthContext } from "../Contexts/AuthContext"; -import { GenerateApiTokenRoles } from "../../config/AuthRoles"; +import { hasPermission } from "../../config/AuthPermissions"; export type Props = { /** @@ -112,7 +112,7 @@ export const DataUpload: FC = ({ submission }: Props) => { setConfigDialogOpen(true)} variant="contained" From 1b7d2b5fc833de425a7751fa256ac800f04786e3 Mon Sep 17 00:00:00 2001 From: Alejandro-Vega Date: Tue, 17 Dec 2024 13:02:11 -0500 Subject: [PATCH 010/102] Updated Delete Node Data with new permission check and updated roles --- .../DeleteNodeDataButton.test.tsx | 40 ++++++++++--------- .../DataSubmissions/DeleteNodeDataButton.tsx | 4 +- 2 files changed, 24 insertions(+), 20 deletions(-) diff --git a/src/components/DataSubmissions/DeleteNodeDataButton.test.tsx b/src/components/DataSubmissions/DeleteNodeDataButton.test.tsx index c36915c06..b4d9b56b2 100644 --- a/src/components/DataSubmissions/DeleteNodeDataButton.test.tsx +++ b/src/components/DataSubmissions/DeleteNodeDataButton.test.tsx @@ -71,6 +71,8 @@ const baseUser: User = { dataCommons: [], createdAt: "", updateAt: "", + permissions: ["data_submission:view", "data_submission:create"], + notifications: [], }; type TestParentProps = { @@ -722,25 +724,27 @@ describe("Implementation Requirements", () => { } ); - it.each(["Admin", "Data Curator", "Organization Owner", "Submitter"])( - "should be visible and interactive for the role %", - (role) => { - const { getByTestId } = render( - {(user?.role === "Admin" || user?.role === "Organization Owner") && ( + {hasPermission(user, "user", "manage") && ( { )} - {user?.role === "Admin" && ( + {hasPermission(user, "program", "manage") && ( { )} - {user?.role === "Admin" && ( + {hasPermission(user, "study", "manage") && ( { )} - {user?.role && GenerateApiTokenRoles.includes(user?.role) ? ( + {hasPermission(user, "data_submission", "create") ? ( + + ); + } + + return null; }) : null} - - -
- - setClickedTitle("")} - > - User Profile - - - - - - {hasPermission(user, "user", "manage") && ( - - setClickedTitle("")} - > - Manage Users - - - )} - {hasPermission(user, "program", "manage") && ( - - setClickedTitle("")} - > - Manage Programs - - - )} - {hasPermission(user, "study", "manage") && ( - - setClickedTitle("")} - > - Manage Studies - - - )} - {hasPermission(user, "data_submission", "create", null, true) ? ( - - - - ) : null} - { - setClickedTitle(""); - handleLogout(); - }} - onKeyDown={(e) => { - if (e.key === "Enter") { - setClickedTitle(""); - handleLogout(); - } - }} - > - Logout - -
- - setOpenAPITokenDialog(false)} /> setUploaderToolOpen(false)} /> From cc0f3213e9a82cbbbb79f1a0fa72b81604b341cb Mon Sep 17 00:00:00 2001 From: Alec M Date: Tue, 31 Dec 2024 13:20:56 -0500 Subject: [PATCH 086/102] fix: Mobile navbar includes CLI Tool --- src/components/Header/components/HeaderTabletAndMobile.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/src/components/Header/components/HeaderTabletAndMobile.tsx b/src/components/Header/components/HeaderTabletAndMobile.tsx index e2369ff0a..7f0111631 100644 --- a/src/components/Header/components/HeaderTabletAndMobile.tsx +++ b/src/components/Header/components/HeaderTabletAndMobile.tsx @@ -186,6 +186,7 @@ const Header = () => { onClick: () => setUploaderToolOpen(true), id: "navbar-dropdown-item-uploader-tool", className: "navMobileSubItem action", + permissions: ["data_submission:create"], }, { name: "Logout", From 99a161ff19572b35a4e9a8e342008854fcf41bf3 Mon Sep 17 00:00:00 2001 From: Alec M Date: Tue, 31 Dec 2024 14:14:22 -0500 Subject: [PATCH 087/102] Update incorrect JSDoc --- src/utils/profileUtils.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/utils/profileUtils.ts b/src/utils/profileUtils.ts index 859a29b66..831ed92f2 100644 --- a/src/utils/profileUtils.ts +++ b/src/utils/profileUtils.ts @@ -61,8 +61,8 @@ export type ColumnizedPBACGroups = { * A utility function to group an array of PBACDefaults into columns * based on the group name. * - * If colCount is greater than the number of groups, the last column will - * aggregate the remaining groups. + * If the number of unique groups exceeds `colCount`, the function will + * aggregate the remaining groups into the last column. * * Data Structure: Array of Columns -> Array of Groups -> PBAC Defaults for the group * From 4553245ec4ad948066af5dd3c55d535161504882 Mon Sep 17 00:00:00 2001 From: Alec M Date: Tue, 31 Dec 2024 14:28:12 -0500 Subject: [PATCH 088/102] fix: Change expand icon --- src/components/PermissionPanel/index.tsx | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/components/PermissionPanel/index.tsx b/src/components/PermissionPanel/index.tsx index f2c481353..97b5c408b 100644 --- a/src/components/PermissionPanel/index.tsx +++ b/src/components/PermissionPanel/index.tsx @@ -1,5 +1,5 @@ import { useQuery } from "@apollo/client"; -import { ArrowDropDown } from "@mui/icons-material"; +import ExpandMoreIcon from "@mui/icons-material/ExpandMore"; import { Accordion, AccordionDetails, @@ -10,10 +10,10 @@ import { FormGroup, styled, Typography, + Unstable_Grid2 as Grid2, } from "@mui/material"; import { FC, memo, useEffect, useMemo, useRef } from "react"; import { useFormContext } from "react-hook-form"; -import Grid2 from "@mui/material/Unstable_Grid2"; import { useSnackbar } from "notistack"; import { cloneDeep } from "lodash"; import { @@ -44,7 +44,7 @@ const StyledAccordionSummary = styled(AccordionSummary)({ "& .MuiAccordionSummary-content.Mui-expanded": { margin: "9px 0", }, - "& .MuiAccordionSummary-expandIcon": { + "& .MuiAccordionSummary-expandIconWrapper": { color: "#356AAD", }, }); @@ -197,7 +197,7 @@ const PermissionPanel: FC = () => { return ( - }> + }> Permissions @@ -232,7 +232,7 @@ const PermissionPanel: FC = () => { - }> + }> Email Notifications From ee86b104253fdc41b3cca8488a81baa9a4c12886 Mon Sep 17 00:00:00 2001 From: Alejandro-Vega Date: Tue, 31 Dec 2024 15:08:09 -0500 Subject: [PATCH 089/102] Remove validate permission and merge with create --- .../ValidationControls.test.tsx | 8 +-- .../DataSubmissions/ValidationControls.tsx | 6 +- src/config/AuthPermissions.test.ts | 59 ------------------- src/config/AuthPermissions.ts | 28 +-------- src/types/Auth.d.ts | 1 - 5 files changed, 6 insertions(+), 96 deletions(-) diff --git a/src/components/DataSubmissions/ValidationControls.test.tsx b/src/components/DataSubmissions/ValidationControls.test.tsx index 1138ee648..e9526d9ee 100644 --- a/src/components/DataSubmissions/ValidationControls.test.tsx +++ b/src/components/DataSubmissions/ValidationControls.test.tsx @@ -84,7 +84,7 @@ const baseUser: Omit = { dataCommons: [], createdAt: "", updateAt: "", - permissions: ["data_submission:view", "data_submission:create", "data_submission:validate"], + permissions: ["data_submission:view", "data_submission:create"], notifications: [], }; @@ -214,11 +214,7 @@ describe("Basic Functionality", () => { user: { ...baseUser, role: "Submitter", - permissions: [ - "data_submission:view", - "data_submission:create", - "data_submission:validate", - ], + permissions: ["data_submission:view", "data_submission:create"], }, }} submission={{ diff --git a/src/components/DataSubmissions/ValidationControls.tsx b/src/components/DataSubmissions/ValidationControls.tsx index 388ff18bb..046a81b24 100644 --- a/src/components/DataSubmissions/ValidationControls.tsx +++ b/src/components/DataSubmissions/ValidationControls.tsx @@ -83,11 +83,11 @@ const ValidateMap: Partial< Record boolean> > = { "In Progress": (user: User, submission: Submission) => - hasPermission(user, "data_submission", "validate", submission), + hasPermission(user, "data_submission", "create", submission), Withdrawn: (user: User, submission: Submission) => - hasPermission(user, "data_submission", "validate", submission), + hasPermission(user, "data_submission", "create", submission), Rejected: (user: User, submission: Submission) => - hasPermission(user, "data_submission", "validate", submission), + hasPermission(user, "data_submission", "create", submission), Submitted: (user: User, submission: Submission) => hasPermission(user, "data_submission", "review", submission), }; diff --git a/src/config/AuthPermissions.test.ts b/src/config/AuthPermissions.test.ts index 71b447742..47efec59e 100644 --- a/src/config/AuthPermissions.test.ts +++ b/src/config/AuthPermissions.test.ts @@ -256,65 +256,6 @@ describe("data_submission:create Permission", () => { }); }); -describe("data_submission:validate Permission", () => { - const validateSubmission = { - ...baseSubmission, - _id: "submission-2", - studyID: "study-2", - dataCommons: "commons-2", - }; - - it("should allow a collaborator (no permission key needed)", () => { - const user = createUser("Submitter", []); - const submission = { - ...validateSubmission, - collaborators: [ - { - collaboratorID: user._id, - collaboratorName: "", - Organization: null, - permission: null, - }, - ], - }; - expect(hasPermission(user, "data_submission", "validate", submission)).toBe(true); - }); - - it("should allow a submitter who is the submission owner WITH 'data_submission:validate' key", () => { - const user = createUser("Submitter", ["data_submission:validate"]); - user._id = "owner-123"; - expect(hasPermission(user, "data_submission", "validate", validateSubmission)).toBe(true); - }); - - it("should allow a 'Federal Lead' with 'data_submission:validate' key if they have the matching study", () => { - const user = createUser("Federal Lead", ["data_submission:validate"]); - user.studies = [{ _id: "study-2" }]; - expect(hasPermission(user, "data_submission", "validate", validateSubmission)).toBe(true); - }); - - it("should allow 'Data Commons Personnel' with 'data_submission:validate' key if they have the matching dataCommons", () => { - const user = createUser("Data Commons Personnel", ["data_submission:validate"]); - user.dataCommons = ["commons-2"]; - expect(hasPermission(user, "data_submission", "validate", validateSubmission)).toBe(true); - }); - - it("should allow 'Admin' with 'data_submission:validate' key", () => { - const user = createUser("Admin", ["data_submission:validate"]); - expect(hasPermission(user, "data_submission", "validate", validateSubmission)).toBe(true); - }); - - it("should deny if user doesn't meet any condition", () => { - const user = createUser("User", []); - expect(hasPermission(user, "data_submission", "validate", validateSubmission)).toBe(false); - }); - - it("should return false if submission is missing or undefined", () => { - const user = createUser("Admin", ["data_submission:validate"]); - expect(hasPermission(user, "data_submission", "validate", undefined)).toBe(false); - expect(hasPermission(user, "data_submission", "validate", null)).toBe(false); - }); -}); - describe("data_submission:review Permission", () => { const reviewSubmission = { ...baseSubmission, diff --git a/src/config/AuthPermissions.ts b/src/config/AuthPermissions.ts index 1f80fb131..9b2132383 100644 --- a/src/config/AuthPermissions.ts +++ b/src/config/AuthPermissions.ts @@ -25,7 +25,7 @@ type Permissions = { }; data_submission: { dataType: Submission; - action: "view" | "create" | "validate" | "review" | "admin_submit" | "confirm"; + action: "view" | "create" | "review" | "admin_submit" | "confirm"; }; user: { dataType: null; @@ -93,32 +93,6 @@ export const PERMISSION_MAP = { return false; }, - validate: (user, submission) => { - const { role, dataCommons, studies } = user; - const hasPermissionKey = user?.permissions?.includes("data_submission:validate"); - const isSubmissionOwner = submission?.submitterID === user?._id; - const isCollaborator = submission?.collaborators?.some((c) => c.collaboratorID === user?._id); - - if (isCollaborator) { - return true; - } - // Submitters from the same study are able to view the same submissions - // Therefore, they must be the submission owner or collaborator with permission key - if (role === "Submitter" && isSubmissionOwner && hasPermissionKey) { - return true; - } - if (role === "Federal Lead" && hasPermissionKey) { - return studies?.some((s) => s._id === submission.studyID); - } - if (role === "Data Commons Personnel" && hasPermissionKey) { - return dataCommons?.some((dc) => dc === submission?.dataCommons); - } - if (role === "Admin" && hasPermissionKey) { - return true; - } - - return false; - }, review: (user, submission) => { const { role, dataCommons, studies } = user; const hasPermissionKey = user?.permissions?.includes("data_submission:review"); diff --git a/src/types/Auth.d.ts b/src/types/Auth.d.ts index c65524a00..28d8aca62 100644 --- a/src/types/Auth.d.ts +++ b/src/types/Auth.d.ts @@ -80,7 +80,6 @@ type SubmissionRequestPermissions = type DataSubmissionPermissions = | "data_submission:view" | "data_submission:create" - | "data_submission:validate" | "data_submission:review" | "data_submission:admin_submit" | "data_submission:confirm"; From e8e492c3d78b131acf5eaab587dc9835d9f27df4 Mon Sep 17 00:00:00 2001 From: Alec M Date: Tue, 31 Dec 2024 16:22:51 -0500 Subject: [PATCH 090/102] fix: Reverse CLI Tool with API Token for consistency --- .../Header/components/HeaderTabletAndMobile.tsx | 12 ++++++------ src/components/Header/components/NavbarDesktop.tsx | 12 ++++++------ 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/components/Header/components/HeaderTabletAndMobile.tsx b/src/components/Header/components/HeaderTabletAndMobile.tsx index 7f0111631..e55174b9e 100644 --- a/src/components/Header/components/HeaderTabletAndMobile.tsx +++ b/src/components/Header/components/HeaderTabletAndMobile.tsx @@ -175,16 +175,16 @@ const Header = () => { permissions: ["user:manage"], }, { - name: "API Token", - onClick: () => setOpenAPITokenDialog(true), - id: "navbar-dropdown-item-api-token", + name: "Uploader CLI Tool", + onClick: () => setUploaderToolOpen(true), + id: "navbar-dropdown-item-uploader-tool", className: "navMobileSubItem action", permissions: ["data_submission:create"], }, { - name: "Uploader CLI Tool", - onClick: () => setUploaderToolOpen(true), - id: "navbar-dropdown-item-uploader-tool", + name: "API Token", + onClick: () => setOpenAPITokenDialog(true), + id: "navbar-dropdown-item-api-token", className: "navMobileSubItem action", permissions: ["data_submission:create"], }, diff --git a/src/components/Header/components/NavbarDesktop.tsx b/src/components/Header/components/NavbarDesktop.tsx index 1b1a45c52..07b18c395 100644 --- a/src/components/Header/components/NavbarDesktop.tsx +++ b/src/components/Header/components/NavbarDesktop.tsx @@ -336,16 +336,16 @@ const NavBar = () => { permissions: ["user:manage"], }, { - name: "API Token", - onClick: () => setOpenAPITokenDialog(true), - id: "navbar-dropdown-item-api-token", + name: "Uploader CLI Tool", + onClick: () => setUploaderToolOpen(true), + id: "navbar-dropdown-item-uploader-tool", className: "navMobileSubItem action", permissions: ["data_submission:create"], }, { - name: "Uploader CLI Tool", - onClick: () => setUploaderToolOpen(true), - id: "navbar-dropdown-item-uploader-tool", + name: "API Token", + onClick: () => setOpenAPITokenDialog(true), + id: "navbar-dropdown-item-api-token", className: "navMobileSubItem action", permissions: ["data_submission:create"], }, From 0df30f8c734bafe8c509d95a7f4b698826373eba Mon Sep 17 00:00:00 2001 From: Alec M Date: Tue, 31 Dec 2024 16:24:31 -0500 Subject: [PATCH 091/102] fix: Don't clear selected menu on submenu click --- src/components/Header/components/NavbarDesktop.tsx | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/components/Header/components/NavbarDesktop.tsx b/src/components/Header/components/NavbarDesktop.tsx index 07b18c395..34fcd2c40 100644 --- a/src/components/Header/components/NavbarDesktop.tsx +++ b/src/components/Header/components/NavbarDesktop.tsx @@ -547,10 +547,7 @@ const NavBar = () => { From d483b1c94dee0eb924c504874965c8221fcdeacd Mon Sep 17 00:00:00 2001 From: Alec M Date: Tue, 31 Dec 2024 16:28:18 -0500 Subject: [PATCH 092/102] fix: Remove unused ref --- src/components/Header/components/NavbarDesktop.tsx | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/components/Header/components/NavbarDesktop.tsx b/src/components/Header/components/NavbarDesktop.tsx index 34fcd2c40..13aa1ff17 100644 --- a/src/components/Header/components/NavbarDesktop.tsx +++ b/src/components/Header/components/NavbarDesktop.tsx @@ -260,16 +260,14 @@ const StyledLoginLink = styled(Link)({ marginRight: "32px", }); -const useOutsideAlerter = (ref1, ref2) => { +const useOutsideAlerter = (ref1: React.RefObject) => { useEffect(() => { function handleClickOutside(event) { if ( !event.target || (event.target.getAttribute("class") !== "dropdownList" && ref1.current && - !ref1.current.contains(event.target) && - ref2.current && - !ref2.current.contains(event.target)) + !ref1.current.contains(event.target)) ) { const toggle = document.getElementsByClassName("navText clicked"); if (toggle[0] && !event.target.getAttribute("class")?.includes("navText clicked")) { @@ -283,7 +281,7 @@ const useOutsideAlerter = (ref1, ref2) => { return () => { document.removeEventListener("mousedown", handleClickOutside); }; - }, [ref1, ref2]); + }, [ref1]); }; const NavBar = () => { @@ -296,8 +294,7 @@ const NavBar = () => { const [uploaderToolOpen, setUploaderToolOpen] = useState(false); const [showLogoutAlert, setShowLogoutAlert] = useState(false); const [restorePath, setRestorePath] = useState(null); - const dropdownSelection = useRef(null); - const nameDropdownSelection = useRef(null); + const dropdownSelection = useRef(null); const clickableObject = HeaderLinks.filter( (item) => item.className === "navMobileItem clickable" @@ -394,7 +391,7 @@ const NavBar = () => { return linkNames.includes(correctPath); }; - useOutsideAlerter(dropdownSelection, nameDropdownSelection); + useOutsideAlerter(dropdownSelection); useEffect(() => { if (!isLoggedIn) { From d5addb50ad8bde92cbda5018071d52d9115ba26f Mon Sep 17 00:00:00 2001 From: Alec M Date: Thu, 2 Jan 2025 11:08:54 -0500 Subject: [PATCH 093/102] fix: The `useProfileFields` hook should utilize PBAC --- src/hooks/useProfileFields.test.ts | 28 ++++++++++++++++------------ src/hooks/useProfileFields.ts | 12 ++++++++---- 2 files changed, 24 insertions(+), 16 deletions(-) diff --git a/src/hooks/useProfileFields.test.ts b/src/hooks/useProfileFields.test.ts index af37195c4..a051e5109 100644 --- 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 = { _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(["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 = { _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 = { _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 = { _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 index 84d98d32d..63dae67f5 100644 --- 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> => { 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,7 +62,6 @@ 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") { @@ -65,8 +69,8 @@ const useProfileFields = ( 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"; } From a3269a5f3526438c2c581598f570e5d741ff0f80 Mon Sep 17 00:00:00 2001 From: Alejandro-Vega Date: Thu, 2 Jan 2025 14:34:40 -0500 Subject: [PATCH 094/102] Delete irrelevant tests for collaborator permissions --- .../DataSubmissions/DataUpload.test.tsx | 24 ------------ .../DeleteNodeDataButton.test.tsx | 26 ------------- .../ValidationControls.test.tsx | 39 ------------------- 3 files changed, 89 deletions(-) diff --git a/src/components/DataSubmissions/DataUpload.test.tsx b/src/components/DataSubmissions/DataUpload.test.tsx index 78bf5c417..e464d7529 100644 --- a/src/components/DataSubmissions/DataUpload.test.tsx +++ b/src/components/DataSubmissions/DataUpload.test.tsx @@ -340,30 +340,6 @@ describe("Implementation Requirements", () => { expect(getByTestId("uploader-cli-config-button")).toBeDisabled(); }); - it("should disable the Uploader CLI download button when collaborator does not have 'Can Edit' permissions", async () => { - const { getByTestId } = render( - , - { wrapper: (p) => } - ); - - expect(getByTestId("uploader-cli-config-button")).toBeDisabled(); - }); - it("should enable the Uploader CLI download button when collaborator does have 'Can Edit' permissions", async () => { const { getByTestId } = render( { expect(getByTestId("delete-node-data-button")).toBeDisabled(); }); - it("should be disabled when the collaborator does not have 'Can Edit' permissions", () => { - const { getByTestId } = render( - -