Merge pull request #2885 from HHS/OPS-2870/Group-and-extra-checks-for…

…-API-endpoints

Ops 2870/group and extra checks for api endpoints

backend/data_tools/data/user_data.json5

@@ -50,6 +50,8 @@
         "PUT_BUDGET_LINE_ITEM",
         "PATCH_BUDGET_LINE_ITEM",
         "POST_BUDGET_LINE_ITEM",
+        // Delete permissions are only for local, not for prod. Users should not have delete BLI permissions in prod.
+        "DELETE_BUDGET_LINE_ITEM",
 
         "GET_SERVICES_COMPONENT",
         "PUT_SERVICES_COMPONENT",
@@ -172,11 +174,15 @@
         "PUT_AGREEMENT",
         "PATCH_AGREEMENT",
         "POST_AGREEMENT",
+      // Delete permissions are only for local, not for prod. Users should not have delete Agreement permissions in prod.
+        "DELETE_AGREEMENT",
 
         "GET_BUDGET_LINE_ITEM",
         "PUT_BUDGET_LINE_ITEM",
         "PATCH_BUDGET_LINE_ITEM",
         "POST_BUDGET_LINE_ITEM",
+      // Delete permissions are only for local, not for prod. Users should not have delete BLI permissions in prod.
+        "DELETE_BUDGET_LINE_ITEM",
 
         "GET_SERVICES_COMPONENT",
         "PUT_SERVICES_COMPONENT",

backend/ops_api/ops/auth/authorization_providers.py

@@ -1,7 +1,5 @@
-from typing import Callable, Optional
-
 from flask import current_app
-from flask_jwt_extended import current_user, get_jwt_identity
+from flask_jwt_extended import get_jwt_identity
 from sqlalchemy import select
 
 from models.users import User
@@ -30,17 +28,3 @@ def _check_role(permission_type: PermissionType, permission: Permission) -> bool
     auth_gateway = AuthorizationGateway(BasicAuthorizationProvider())
     identity = get_jwt_identity()
     return auth_gateway.is_authorized(identity, f"{permission_type.name}_{permission.name}".upper())
-
-
-def _check_groups(groups: Optional[list[str]]) -> bool:
-    auth_group = False
-    if groups is not None:
-        auth_group = len(set(groups) & {g.name for g in current_user.groups}) > 0
-    return auth_group
-
-
-def _check_extra(extra_check: Optional[Callable[..., bool]], args, kwargs) -> bool:
-    valid = False
-    if extra_check is not None:
-        valid = extra_check(*args, **kwargs)
-    return valid

backend/ops_api/ops/auth/decorators.py

@@ -7,7 +7,7 @@
 
 from models import User, UserStatus
 from ops_api.ops.auth.auth_types import Permission, PermissionType
-from ops_api.ops.auth.authorization_providers import _check_extra, _check_groups, _check_role
+from ops_api.ops.auth.authorization_providers import _check_role
 from ops_api.ops.auth.exceptions import ExtraCheckError, InvalidUserSessionError, NotActiveUserError
 from ops_api.ops.auth.utils import (
     deactivate_all_user_sessions,
@@ -81,11 +81,7 @@ def __call__(self, func: Callable) -> Callable:
         @error_simulator
         def wrapper(*args, **kwargs) -> Response:
             try:
-                if (
-                    _check_role(self.permission_type, self.permission)
-                    or _check_groups(self.groups)
-                    or _check_extra(self.extra_check, args, kwargs)
-                ):
+                if _check_role(self.permission_type, self.permission):
                     response = func(*args, **kwargs)
 
                 else:

backend/ops_api/ops/resources/budget_line_items.py

@@ -1,10 +1,9 @@
 from __future__ import annotations
 
-from functools import partial
 from typing import Optional
 
 from flask import Response, current_app, request
-from flask_jwt_extended import get_jwt_identity
+from flask_jwt_extended import get_current_user, verify_jwt_in_request
 from sqlalchemy import inspect, select
 from sqlalchemy.exc import SQLAlchemyError
 from typing_extensions import Any
@@ -18,6 +17,7 @@
     Division,
     OpsEventType,
     Portfolio,
+    User,
 )
 from ops_api.ops.auth.auth_types import Permission, PermissionType
 from ops_api.ops.auth.decorators import is_authorized
@@ -38,42 +38,6 @@
 ENDPOINT_STRING = "/budget-line-items"
 
 
-def bli_associated_with_agreement(self, id: int, permission_type: PermissionType) -> bool:
-    jwt_identity = get_jwt_identity()
-    budget_line_item: BudgetLineItem = current_app.db_session.get(BudgetLineItem, id)
-    try:
-        agreement = budget_line_item.agreement
-    except AttributeError as e:
-        # No BLI found in the DB. Erroring out.
-        raise ExtraCheckError({}) from e
-
-    if agreement is None:
-        # We are faking a validation check at this point. We know there is no agreement associated with the BLI.
-        # This is made to emulate the validation check from a marshmallow schema.
-        if permission_type == PermissionType.PUT:
-            raise ExtraCheckError(
-                {
-                    "_schema": ["BLI must have an Agreement when status is not DRAFT"],
-                    "agreement_id": ["Missing data for required field."],
-                }
-            )
-        elif permission_type == PermissionType.PATCH:
-            raise ExtraCheckError({"_schema": ["BLI must have an Agreement when status is not DRAFT"]})
-        else:
-            raise ExtraCheckError({})
-
-    oidc_ids = set()
-    if agreement.created_by_user:
-        oidc_ids.add(str(agreement.created_by_user.oidc_id))
-    if agreement.project_officer:
-        oidc_ids.add(str(agreement.project_officer.oidc_id))
-    oidc_ids |= set(str(tm.oidc_id) for tm in agreement.team_members)
-
-    ret = jwt_identity in oidc_ids
-
-    return ret
-
-
 def get_division_for_budget_line_item(bli_id: int) -> Optional[Division]:
     division = (
         current_app.db_session.query(Division)
@@ -93,6 +57,54 @@ def __init__(self, model: BaseModel):
         self._put_schema = POSTRequestBodySchema()
         self._patch_schema = PATCHRequestBodySchema()
 
+    def bli_associated_with_agreement(self, id: int, permission_type: PermissionType) -> bool:
+        """
+        In order to edit a budget line, the user must be authenticated and meet on of these conditions:
+            -  The user is the agreement creator.
+            -  The user is the project officer of the agreement.
+            -  The user is a team member on the agreement.
+            -  The user is a budget team member.
+        """
+        verify_jwt_in_request()
+        user = get_current_user()
+        if not user:
+            return False
+        budget_line_item: BudgetLineItem = current_app.db_session.get(BudgetLineItem, id)
+        try:
+            agreement = budget_line_item.agreement
+        except AttributeError as e:
+            # No BLI found in the DB. Erroring out.
+            raise ExtraCheckError({}) from e
+
+        if agreement is None:
+            # We are faking a validation check at this point. We know there is no agreement associated with the BLI.
+            # This is made to emulate the validation check from a marshmallow schema.
+            if permission_type == PermissionType.PUT:
+                raise ExtraCheckError(
+                    {
+                        "_schema": ["BLI must have an Agreement when status is not DRAFT"],
+                        "agreement_id": ["Missing data for required field."],
+                    }
+                )
+            elif permission_type == PermissionType.PATCH:
+                raise ExtraCheckError({"_schema": ["BLI must have an Agreement when status is not DRAFT"]})
+            else:
+                raise ExtraCheckError({})
+
+        oidc_ids = set()
+        if agreement.created_by_user:
+            oidc_ids.add(str(agreement.created_by_user.oidc_id))
+        if agreement.created_by:
+            user = current_app.db_session.get(User, agreement.created_by)
+            oidc_ids.add(str(user.oidc_id))
+        if agreement.project_officer:
+            oidc_ids.add(str(agreement.project_officer.oidc_id))
+        oidc_ids |= set(str(tm.oidc_id) for tm in agreement.team_members)
+
+        ret = str(user.oidc_id) in oidc_ids or "BUDGET_TEAM" in [role.name for role in user.roles]
+
+        return ret
+
     def _get_item_with_try(self, id: int) -> Response:
         try:
             item = self._get_item(id)
@@ -219,19 +231,19 @@ def _update(self, id, method, schema) -> Response:
     @is_authorized(
         PermissionType.PUT,
         Permission.BUDGET_LINE_ITEM,
-        extra_check=partial(bli_associated_with_agreement, permission_type=PermissionType.PUT),
-        groups=["Budget Team", "Admins"],
     )
     def put(self, id: int) -> Response:
+        if not self.bli_associated_with_agreement(id, PermissionType.PUT):
+            return make_response_with_headers({}, 403)
         return self._update(id, "PUT", self._put_schema)
 
     @is_authorized(
         PermissionType.PATCH,
         Permission.BUDGET_LINE_ITEM,
-        extra_check=partial(bli_associated_with_agreement, permission_type=PermissionType.PATCH),
-        groups=["Budget Team", "Admins"],
     )
     def patch(self, id: int) -> Response:
+        if not self.bli_associated_with_agreement(id, PermissionType.PATCH):
+            return make_response_with_headers({}, 403)
         return self._update(id, "PATCH", self._patch_schema)
 
     def update_and_commit_budget_line_item(self, data, id):
@@ -243,10 +255,11 @@ def update_and_commit_budget_line_item(self, data, id):
     @is_authorized(
         PermissionType.DELETE,
         Permission.BUDGET_LINE_ITEM,
-        extra_check=partial(bli_associated_with_agreement, permission_type=PermissionType.DELETE),
-        groups=["Budget Team", "Admins"],
     )
     def delete(self, id: int) -> Response:
+        if not self.bli_associated_with_agreement(id, PermissionType.DELETE):
+            return make_response_with_headers({}, 403)
+
         with OpsEventHandler(OpsEventType.DELETE_BLI) as meta:
             bli: BudgetLineItem = self._get_item(id)
 

backend/ops_api/ops/resources/procurement_steps.py

@@ -1,5 +1,3 @@
-from functools import partial
-
 import marshmallow_dataclass as mmdc
 from flask import Response, current_app, request
 from flask_jwt_extended import current_user, get_jwt_identity
@@ -51,45 +49,6 @@ def get_current_user_id():
     return current_user.id
 
 
-# TODO: considering refactoring to DRYer along with similar code in services_component.py and budget_line_items.py
-def step_associated_with_agreement(self, id: int, permission_type: PermissionType) -> bool:
-    jwt_identity = get_jwt_identity()
-    step: ProcurementStep = current_app.db_session.get(ProcurementStep, id)
-    try:
-        # should there be a step.agreement ?
-        agreement_id = step.agreement_id
-        agreement: Agreement = current_app.db_session.get(Agreement, agreement_id)
-    except AttributeError as e:
-        # No step found in the DB. Erroring out.
-        raise ExtraCheckError({}) from e
-
-    if agreement is None:
-        # We are faking a validation check at this point. We know there is no agreement associated with the SC.
-        # This is made to emulate the validation check from a marshmallow schema.
-        if permission_type == PermissionType.PUT:
-            raise ExtraCheckError(
-                {
-                    "_schema": ["ProcurementStep must have an Agreement"],
-                    "contract_agreement_id": ["Missing data for required field."],
-                }
-            )
-        elif permission_type == PermissionType.PATCH:
-            raise ExtraCheckError({"_schema": ["ProcurementStep must have an Agreement"]})
-        else:
-            raise ExtraCheckError({})
-
-    oidc_ids = set()
-    if agreement.created_by_user:
-        oidc_ids.add(str(agreement.created_by_user.oidc_id))
-    if agreement.project_officer:
-        oidc_ids.add(str(agreement.project_officer.oidc_id))
-    oidc_ids |= set(str(tm.oidc_id) for tm in agreement.team_members)
-
-    ret = jwt_identity in oidc_ids
-
-    return ret
-
-
 # Base Procurement Step APIs
 
 
@@ -145,6 +104,43 @@ def __init__(self, model: BaseModel = ProcurementStep):
         self._response_schema = mmdc.class_schema(ProcurementStepResponse)()
         self._patch_schema = mmdc.class_schema(ProcurementStepRequest)(dump_only=["type"])
 
+    def step_associated_with_agreement(self, id: int, permission_type: PermissionType) -> bool:
+        jwt_identity = get_jwt_identity()
+        step: ProcurementStep = current_app.db_session.get(ProcurementStep, id)
+        try:
+            # should there be a step.agreement ?
+            agreement_id = step.agreement_id
+            agreement: Agreement = current_app.db_session.get(Agreement, agreement_id)
+        except AttributeError as e:
+            # No step found in the DB. Erroring out.
+            raise ExtraCheckError({}) from e
+
+        if agreement is None:
+            # We are faking a validation check at this point. We know there is no agreement associated with the SC.
+            # This is made to emulate the validation check from a marshmallow schema.
+            if permission_type == PermissionType.PUT:
+                raise ExtraCheckError(
+                    {
+                        "_schema": ["ProcurementStep must have an Agreement"],
+                        "contract_agreement_id": ["Missing data for required field."],
+                    }
+                )
+            elif permission_type == PermissionType.PATCH:
+                raise ExtraCheckError({"_schema": ["ProcurementStep must have an Agreement"]})
+            else:
+                raise ExtraCheckError({})
+
+        oidc_ids = set()
+        if agreement.created_by_user:
+            oidc_ids.add(str(agreement.created_by_user.oidc_id))
+        if agreement.project_officer:
+            oidc_ids.add(str(agreement.project_officer.oidc_id))
+        oidc_ids |= set(str(tm.oidc_id) for tm in agreement.team_members)
+
+        ret = jwt_identity in oidc_ids
+
+        return ret
+
     def _update(self, id, method, schema) -> Response:
         message_prefix = f"{request.method} to {request.path}"
         with OpsEventHandler(OpsEventType.UPDATE_PROCUREMENT_ACQUISITION_PLANNING) as meta:
@@ -165,10 +161,10 @@ def _update(self, id, method, schema) -> Response:
     @is_authorized(
         PermissionType.PATCH,
         Permission.WORKFLOW,
-        extra_check=partial(step_associated_with_agreement, permission_type=PermissionType.PATCH),
-        groups=["Budget Team", "Admins"],
     )
     def patch(self, id: int) -> Response:
+        if not self.step_associated_with_agreement(id, PermissionType.PATCH):
+            return make_response_with_headers({}, 403)
         return self._update(id, "PATCH", self._patch_schema)