Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🐛 Non-Agreement team members can submit budget line status change #2789

Open
fpigeonjr opened this issue Sep 12, 2024 · 1 comment
Open
Assignees
Labels
bug Something isn't working Dev Backend Ready Backend, Pipeline, or other UX related work is refined, and ready for Developers

Comments

@fpigeonjr
Copy link
Contributor

fpigeonjr commented Sep 12, 2024

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 fpigeonjr added the bug Something isn't working label Sep 12, 2024
@fpigeonjr fpigeonjr changed the title 🐛 Non-team members can submit budget line status changes on ReviewAgreement page 🐛 Non-Agreement team members can submit budget line status changes on ReviewAgreement page Sep 12, 2024
@fpigeonjr fpigeonjr added Dev Frontend Ready Designers have created and finalized the designs, story has been refined, and ready for a Developer Dev Backend Ready Backend, Pipeline, or other UX related work is refined, and ready for Developers labels Sep 12, 2024
@fpigeonjr fpigeonjr changed the title 🐛 Non-Agreement team members can submit budget line status changes on ReviewAgreement page 🐛 Non-Agreement team members can submit budget line status change Sep 13, 2024
@fpigeonjr fpigeonjr removed the Dev Frontend Ready Designers have created and finalized the designs, story has been refined, and ready for a Developer label Sep 13, 2024
@fpigeonjr
Copy link
Contributor Author

fpigeonjr commented Sep 16, 2024

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;
};

@Santi-3rd Santi-3rd self-assigned this Sep 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Dev Backend Ready Backend, Pipeline, or other UX related work is refined, and ready for Developers
Projects
None yet
Development

When branches are created from issues, their pull requests are automatically linked.

2 participants