HHS / OPRE-OPS

ACF's OPRE OPS product. Code name Unicorn.
Other
12 stars 3 forks source link

šŸ› Non-Agreement team members can submit budget line status change #2789

Open fpigeonjr opened 1 week ago

fpigeonjr commented 1 week ago

Expected Behavior

Only Agreement team members should be able to submit budget lines for a status change on the ReviewAgreement page.

Current Behavior

Non-team members are able to submit budget lines for a status change on the ReviewAgreement page, which should not be allowed.

Possible Cause

There might be a lack of proper access control or user role validation on the ReviewAgreement page or in the backend API that handles budget line status changes.

Steps to Reproduce

  1. Log in as basic user
  2. Navigate to the ReviewAgreement page
  3. Attempt to submit a budget line for a status change
  4. Observe that the submission is successful when it should be restricted

Context

This issue affects the security and integrity of the budget review process. It allows non-team members to make changes that should be limited to team members only. This was observed in the localdev environment of OPS.

See also #2725

Detailed Description

The ReviewAgreement page is currently allowing non-team members to submit budget lines for status changes. Only authorized team members should have the ability to make these changes. The system is not correctly validating the user's role or permissions before allowing the submission of budget line status changes.

Possible Implementation

  1. Implement a role-based access control (RBAC) check on the frontend to disable the submission option for non-team members.
  2. Add a middleware or decorator on the backend API to verify the user's role before processing any budget line status change requests.
  3. Audit all existing budget line status changes to identify any unauthorized modifications made by non-team members.
  4. Add logging for all attempts to change budget line statuses, including user information and whether the attempt was successful or blocked.
fpigeonjr commented 5 days ago

here is a snippet on the hook I am using for permissions on the FE:

// src/hooks/agreement.hooks.js
export const useIsUserAllowedToEditAgreement = (agreementId) => {
    // TODO: add check if user is on the Budget Team
    const { data: agreement } = useGetAgreementByIdQuery(agreementId);
    const loggedInUserId = useSelector((state) => state?.auth?.activeUser?.id);

    const isUserTheProjectOfficer = agreement?.project_officer_id === loggedInUserId;
    const isUserTheAgreementCreator = agreement?.created_by === loggedInUserId;
    const isUserATeamMember = agreement?.team_members?.some((teamMember) => teamMember.id === loggedInUserId);
    const isUserCreatorOfAnyBudgetLines = agreement?.budget_line_items?.some(
        (bli) => bli.created_by === loggedInUserId
    );
    const isUserAllowedToEditAgreement =
        isUserTheProjectOfficer || isUserTheAgreementCreator || isUserATeamMember || isUserCreatorOfAnyBudgetLines;

    return isUserAllowedToEditAgreement;
};