From 1d432d5abecd7bada955f2a4265ac1f89d27fb66 Mon Sep 17 00:00:00 2001 From: Nassim Tabchiche Date: Fri, 24 Jan 2025 10:57:21 +0100 Subject: [PATCH 1/4] Raise if user is not approver when trying to approve/reject/revoke acceptance --- backend/core/views.py | 37 +++++++++++++++++++++++++++++++------ 1 file changed, 31 insertions(+), 6 deletions(-) diff --git a/backend/core/views.py b/backend/core/views.py index a5fc46dd6..a945810cb 100644 --- a/backend/core/views.py +++ b/backend/core/views.py @@ -70,6 +70,7 @@ from rest_framework.response import Response from rest_framework.utils.serializer_helpers import ReturnDict from rest_framework.views import APIView +from rest_framework.exceptions import PermissionDenied from weasyprint import HTML @@ -1710,20 +1711,44 @@ def to_review(self, request): @action(detail=True, methods=["post"], name="Accept risk acceptance") def accept(self, request, pk): - if request.user == self.get_object().approver: - self.get_object().set_state("accepted") + if request.user != self.get_object().approver: + logger.error( + "Only the approver can accept the risk acceptance", + user=request.user, + approver=self.get_object().approver, + ) + raise PermissionDenied( + {"error": "Only the approver can accept the risk acceptance"} + ) + self.get_object().set_state("accepted") return Response({"results": "state updated to accepted"}) @action(detail=True, methods=["post"], name="Reject risk acceptance") def reject(self, request, pk): - if request.user == self.get_object().approver: - self.get_object().set_state("rejected") + if request.user != self.get_object().approver: + logger.error( + "Only the approver can reject the risk rejectance", + user=request.user, + approver=self.get_object().approver, + ) + raise PermissionDenied( + {"error": "Only the approver can reject the risk rejectance"} + ) + self.get_object().set_state("rejected") return Response({"results": "state updated to rejected"}) @action(detail=True, methods=["post"], name="Revoke risk acceptance") def revoke(self, request, pk): - if request.user == self.get_object().approver: - self.get_object().set_state("revoked") + if request.user != self.get_object().approver: + logger.error( + "Only the approver can revoke the risk revokeance", + user=request.user, + approver=self.get_object().approver, + ) + raise PermissionDenied( + {"error": "Only the approver can revoke the risk revokeance"} + ) + self.get_object().set_state("revoked") return Response({"results": "state updated to revoked"}) @action(detail=False, methods=["get"], name="Get waiting risk acceptances") From 7379d807f53f5142393b1c587dfb7ad428c08a1a Mon Sep 17 00:00:00 2001 From: Nassim Tabchiche Date: Fri, 24 Jan 2025 10:57:35 +0100 Subject: [PATCH 2/4] Add reject and revoke acceptance exceptions --- backend/core/permissions.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/backend/core/permissions.py b/backend/core/permissions.py index 0ffed5017..04bbe772d 100644 --- a/backend/core/permissions.py +++ b/backend/core/permissions.py @@ -38,12 +38,17 @@ def has_object_permission(self, request: Request, view, obj): ): return True perm = Permission.objects.get(codename=_codename) + # special case of risk acceptance approval - if ( - request.parser_context["request"]._request.resolver_match.url_name - == "risk-acceptances-accept" - ): + if request.parser_context and request.parser_context[ + "request" + ]._request.resolver_match.url_name in [ + "risk-acceptances-accept", + "risk-acceptances-reject", + "risk-acceptances-revoke", + ]: perm = Permission.objects.get(codename="approve_riskacceptance") + return RoleAssignment.is_access_allowed( user=request.user, perm=perm, From c2f212ad1f0caf408b4556af91730e3b70b58060 Mon Sep 17 00:00:00 2001 From: Nassim Tabchiche Date: Fri, 24 Jan 2025 16:43:47 +0100 Subject: [PATCH 3/4] refactor: move permission overrides to the view --- backend/core/permissions.py | 18 ++++++++---------- backend/core/views.py | 6 ++++++ 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/backend/core/permissions.py b/backend/core/permissions.py index 04bbe772d..8ea3d2db5 100644 --- a/backend/core/permissions.py +++ b/backend/core/permissions.py @@ -37,17 +37,15 @@ def has_object_permission(self, request: Request, view, obj): obj, "is_published", False ): return True - perm = Permission.objects.get(codename=_codename) - # special case of risk acceptance approval - if request.parser_context and request.parser_context[ - "request" - ]._request.resolver_match.url_name in [ - "risk-acceptances-accept", - "risk-acceptances-reject", - "risk-acceptances-revoke", - ]: - perm = Permission.objects.get(codename="approve_riskacceptance") + # Check for view action permission overrides + current_action = getattr(view, "action", None) + + if current_action: + permission_overrides = getattr(view, "permission_overrides", {}) + _codename = permission_overrides.get(current_action, _codename) + + perm = Permission.objects.get(codename=_codename) return RoleAssignment.is_access_allowed( user=request.user, diff --git a/backend/core/views.py b/backend/core/views.py index a945810cb..c212f0fea 100644 --- a/backend/core/views.py +++ b/backend/core/views.py @@ -1676,6 +1676,12 @@ class RiskAcceptanceViewSet(BaseModelViewSet): API endpoint that allows risk acceptance to be viewed or edited. """ + permission_overrides = { + "accept": "approve_riskacceptance", + "reject": "approve_riskacceptance", + "revoke": "approve_riskacceptance", + } + model = RiskAcceptance serializer_class = RiskAcceptanceWriteSerializer filterset_fields = ["folder", "state", "approver", "risk_scenarios"] From 7f5c9ee3a86cd16cd2dfcdd643f3a3e8c2e909e5 Mon Sep 17 00:00:00 2001 From: Nassim Tabchiche Date: Fri, 24 Jan 2025 19:18:36 +0100 Subject: [PATCH 4/4] Fix log messages --- backend/core/views.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/backend/core/views.py b/backend/core/views.py index c212f0fea..79fed3023 100644 --- a/backend/core/views.py +++ b/backend/core/views.py @@ -1733,12 +1733,12 @@ def accept(self, request, pk): def reject(self, request, pk): if request.user != self.get_object().approver: logger.error( - "Only the approver can reject the risk rejectance", + "Only the approver can reject the risk acceptance", user=request.user, approver=self.get_object().approver, ) raise PermissionDenied( - {"error": "Only the approver can reject the risk rejectance"} + {"error": "Only the approver can reject the risk acceptance"} ) self.get_object().set_state("rejected") return Response({"results": "state updated to rejected"}) @@ -1747,12 +1747,12 @@ def reject(self, request, pk): def revoke(self, request, pk): if request.user != self.get_object().approver: logger.error( - "Only the approver can revoke the risk revokeance", + "Only the approver can revoke the risk acceptance", user=request.user, approver=self.get_object().approver, ) raise PermissionDenied( - {"error": "Only the approver can revoke the risk revokeance"} + {"error": "Only the approver can revoke the risk acceptance"} ) self.get_object().set_state("revoked") return Response({"results": "state updated to revoked"})