Skip to content

Commit 1267c72

Browse files
devGregAclaudeMaffooch
authored
Updates Decorators with Certain Permission Models (#14410)
* Adjust decorators for new permissions model Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Decorator and permissions updates Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Align get_object_or_404 filtering strategy with dev branch Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Fix test assertions and serializer type hint Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Enhance permission tests with detailed docstrings for clarity and maintainability --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Cody Maffucci <46459665+Maffooch@users.noreply.github.com>
1 parent 74b6b87 commit 1267c72

11 files changed

Lines changed: 1143 additions & 75 deletions

File tree

dojo/api_v2/serializers.py

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1760,9 +1760,7 @@ class FindingSerializer(serializers.ModelSerializer):
17601760
mitigated_by = serializers.PrimaryKeyRelatedField(required=False, allow_null=True, queryset=User.objects.all())
17611761
tags = TagListSerializerField(required=False)
17621762
request_response = serializers.SerializerMethodField()
1763-
accepted_risks = RiskAcceptanceSerializer(
1764-
many=True, read_only=True, source="risk_acceptance_set",
1765-
)
1763+
accepted_risks = serializers.SerializerMethodField()
17661764
push_to_jira = serializers.BooleanField(default=False)
17671765
found_by = serializers.PrimaryKeyRelatedField(
17681766
queryset=Test_Type.objects.all(), many=True,
@@ -1806,6 +1804,17 @@ def __init__(self, *args, **kwargs):
18061804
many=True, required=False, queryset=Endpoint.objects.all(),
18071805
)
18081806

1807+
@extend_schema_field(RiskAcceptanceSerializer(many=True))
1808+
def get_accepted_risks(self, obj):
1809+
request = self.context.get("request")
1810+
if request is None:
1811+
return []
1812+
if not user_has_permission(request.user, obj, Permissions.Risk_Acceptance):
1813+
return []
1814+
return RiskAcceptanceSerializer(
1815+
obj.risk_acceptance_set.all(), many=True,
1816+
).data
1817+
18091818
@extend_schema_field(serializers.DateTimeField())
18101819
def get_jira_creation(self, obj):
18111820
return jira_helper.get_jira_creation(obj)

dojo/api_v2/views.py

Lines changed: 64 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
serializers,
4646
)
4747
from dojo.api_v2.prefetch.prefetcher import _Prefetcher
48+
from dojo.authorization.authorization import user_has_permission_or_403
4849
from dojo.authorization.roles_permissions import Permissions
4950
from dojo.celery_dispatch import dojo_dispatch_task
5051
from dojo.cred.queries import get_authorized_cred_mappings
@@ -351,7 +352,11 @@ def get_queryset(self):
351352
responses={status.HTTP_200_OK: serializers.ReportGenerateSerializer},
352353
)
353354
@action(
354-
detail=True, methods=["post"], permission_classes=[IsAuthenticated],
355+
detail=True, methods=["post"],
356+
# IsAuthenticated only: report generation requires View permission,
357+
# enforced by the permission-filtered get_queryset(). The viewset's
358+
# permission_classes would check Edit (POST), which is too restrictive.
359+
permission_classes=[IsAuthenticated],
355360
)
356361
def generate_report(self, request, pk=None):
357362
endpoint = self.get_object()
@@ -475,7 +480,11 @@ def reopen(self, request, pk=None):
475480
responses={status.HTTP_200_OK: serializers.ReportGenerateSerializer},
476481
)
477482
@action(
478-
detail=True, methods=["post"], permission_classes=[IsAuthenticated],
483+
detail=True, methods=["post"],
484+
# IsAuthenticated only: report generation requires View permission,
485+
# enforced by the permission-filtered get_queryset(). The viewset's
486+
# permission_classes would check Edit (POST), which is too restrictive.
487+
permission_classes=[IsAuthenticated],
479488
)
480489
def generate_report(self, request, pk=None):
481490
engagement = self.get_object()
@@ -691,7 +700,8 @@ def download_file(self, request, file_id, pk=None):
691700
responses={status.HTTP_200_OK: serializers.EngagementUpdateJiraEpicSerializer},
692701
)
693702
@action(
694-
detail=True, methods=["post"], permission_classes=[IsAuthenticated],
703+
detail=True, methods=["post"],
704+
permission_classes=(IsAuthenticated, permissions.UserHasEngagementRelatedObjectPermission),
695705
)
696706
def update_jira_epic(self, request, pk=None):
697707
engagement = self.get_object()
@@ -1383,7 +1393,11 @@ def set_finding_as_original(self, request, pk, new_fid):
13831393
responses={status.HTTP_200_OK: serializers.ReportGenerateSerializer},
13841394
)
13851395
@action(
1386-
detail=False, methods=["post"], permission_classes=[IsAuthenticated],
1396+
detail=False, methods=["post"],
1397+
# IsAuthenticated only: report generation requires View permission,
1398+
# enforced by the permission-filtered get_queryset(). The viewset's
1399+
# permission_classes would check Edit (POST), which is too restrictive.
1400+
permission_classes=[IsAuthenticated],
13871401
)
13881402
def generate_report(self, request):
13891403
findings = self.get_queryset()
@@ -1728,38 +1742,55 @@ def batch(self, request, pk=None):
17281742
serialized_data = serializers.MetaMainSerializer(data=request.data)
17291743
if serialized_data.is_valid(raise_exception=True):
17301744
if request.method == "POST":
1731-
self.process_post(request.data)
1745+
self.process_post(request)
17321746
status_code = status.HTTP_201_CREATED
17331747
if request.method == "PATCH":
1734-
self.process_patch(request.data)
1748+
self.process_patch(request)
17351749
status_code = status.HTTP_200_OK
17361750

17371751
return Response(status=status_code, data=serialized_data.data)
17381752

1739-
def process_post(self: object, data: dict):
1740-
product = Product.objects.filter(id=data.get("product")).first()
1741-
finding = Finding.objects.filter(id=data.get("finding")).first()
1742-
endpoint = Endpoint.objects.filter(id=data.get("endpoint")).first()
1753+
def _fetch_and_authorize_parents(self, request, permission_map):
1754+
"""Fetch parent objects and verify the user has the required permissions."""
1755+
data = request.data
1756+
parents = {}
1757+
for field, (model, permission) in permission_map.items():
1758+
obj = model.objects.filter(id=data.get(field)).first()
1759+
if obj:
1760+
user_has_permission_or_403(request.user, obj, permission)
1761+
parents[field] = obj
1762+
return parents
1763+
1764+
def process_post(self, request):
1765+
data = request.data
1766+
parents = self._fetch_and_authorize_parents(request, {
1767+
"product": (Product, Permissions.Product_Edit),
1768+
"finding": (Finding, Permissions.Finding_Edit),
1769+
"endpoint": (Endpoint, Permissions.Location_Edit),
1770+
})
17431771
metalist = data.get("metadata")
17441772
for metadata in metalist:
17451773
try:
17461774
DojoMeta.objects.create(
1747-
product=product,
1748-
finding=finding,
1749-
endpoint=endpoint,
1775+
product=parents["product"],
1776+
finding=parents["finding"],
1777+
endpoint=parents["endpoint"],
17501778
name=metadata.get("name"),
17511779
value=metadata.get("value"),
17521780
)
17531781
except (IntegrityError) as ex: # this should not happen as the data was validated in the batch call
17541782
raise ValidationError(str(ex))
17551783

1756-
def process_patch(self: object, data: dict):
1757-
product = Product.objects.filter(id=data.get("product")).first()
1758-
finding = Finding.objects.filter(id=data.get("finding")).first()
1759-
endpoint = Endpoint.objects.filter(id=data.get("endpoint")).first()
1784+
def process_patch(self, request):
1785+
data = request.data
1786+
parents = self._fetch_and_authorize_parents(request, {
1787+
"product": (Product, Permissions.Product_Edit),
1788+
"finding": (Finding, Permissions.Finding_Edit),
1789+
"endpoint": (Endpoint, Permissions.Location_Edit),
1790+
})
17601791
metalist = data.get("metadata")
17611792
for metadata in metalist:
1762-
dojometa = DojoMeta.objects.filter(product=product, finding=finding, endpoint=endpoint, name=metadata.get("name"))
1793+
dojometa = DojoMeta.objects.filter(product=parents["product"], finding=parents["finding"], endpoint=parents["endpoint"], name=metadata.get("name"))
17631794
if dojometa:
17641795
try:
17651796
dojometa.update(
@@ -1815,7 +1846,11 @@ def destroy(self, request, *args, **kwargs):
18151846
responses={status.HTTP_200_OK: serializers.ReportGenerateSerializer},
18161847
)
18171848
@action(
1818-
detail=True, methods=["post"], permission_classes=[IsAuthenticated],
1849+
detail=True, methods=["post"],
1850+
# IsAuthenticated only: report generation requires View permission,
1851+
# enforced by the permission-filtered get_queryset(). The viewset's
1852+
# permission_classes would check Edit (POST), which is too restrictive.
1853+
permission_classes=[IsAuthenticated],
18191854
)
18201855
def generate_report(self, request, pk=None):
18211856
product = self.get_object()
@@ -1956,7 +1991,11 @@ def destroy(self, request, *args, **kwargs):
19561991
responses={status.HTTP_200_OK: serializers.ReportGenerateSerializer},
19571992
)
19581993
@action(
1959-
detail=True, methods=["post"], permission_classes=[IsAuthenticated],
1994+
detail=True, methods=["post"],
1995+
# IsAuthenticated only: report generation requires View permission,
1996+
# enforced by the permission-filtered get_queryset(). The viewset's
1997+
# permission_classes would check Edit (POST), which is too restrictive.
1998+
permission_classes=[IsAuthenticated],
19601999
)
19612000
def generate_report(self, request, pk=None):
19622001
product_type = self.get_object()
@@ -2143,7 +2182,11 @@ def get_serializer_class(self):
21432182
responses={status.HTTP_200_OK: serializers.ReportGenerateSerializer},
21442183
)
21452184
@action(
2146-
detail=True, methods=["post"], permission_classes=[IsAuthenticated],
2185+
detail=True, methods=["post"],
2186+
# IsAuthenticated only: report generation requires View permission,
2187+
# enforced by the permission-filtered get_queryset(). The viewset's
2188+
# permission_classes would check Edit (POST), which is too restrictive.
2189+
permission_classes=[IsAuthenticated],
21472190
)
21482191
def generate_report(self, request, pk=None):
21492192
test = self.get_object()

dojo/benchmark/views.py

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,8 @@ def update_benchmark(request, pid, _type):
4646
field = request.POST.get("field")
4747
value = request.POST.get("value")
4848
value = {"true": True, "false": False}.get(value, value)
49+
product = get_object_or_404(Product, id=pid)
50+
bench = get_object_or_404(Benchmark_Product.objects.filter(product=product), id=bench_id)
4951

5052
if field in {
5153
"enabled",
@@ -54,7 +56,6 @@ def update_benchmark(request, pid, _type):
5456
"get_notes",
5557
"delete_notes",
5658
}:
57-
bench = Benchmark_Product.objects.get(id=bench_id)
5859
if field == "enabled":
5960
bench.enabled = value
6061
elif field == "pass_fail":
@@ -90,21 +91,22 @@ def update_benchmark(request, pid, _type):
9091
@user_is_authorized(Product, Permissions.Benchmark_Edit, "pid")
9192
def update_benchmark_summary(request, pid, _type, summary):
9293
if request.method == "POST":
94+
product = get_object_or_404(Product, id=pid)
95+
benchmark_summary = get_object_or_404(Benchmark_Product_Summary.objects.filter(product=product), id=summary)
9396
field = request.POST.get("field")
9497
value = request.POST.get("value")
9598
value = {"true": True, "false": False}.get(value, value)
9699

97100
if field in {"publish", "desired_level"}:
98-
summary = Benchmark_Product_Summary.objects.get(id=summary)
99101
data = {}
100102
if field == "publish":
101-
summary.publish = value
103+
benchmark_summary.publish = value
102104
data = {"publish": value}
103105
elif field == "desired_level":
104-
summary.desired_level = value
105-
data = {"desired_level": value, "text": asvs_level(summary)}
106+
benchmark_summary.desired_level = value
107+
data = {"desired_level": value, "text": asvs_level(benchmark_summary)}
106108

107-
summary.save()
109+
benchmark_summary.save()
108110
return JsonResponse(data)
109111

110112
return redirect_to_return_url_or_else(

dojo/cred/views.py

Lines changed: 16 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ def all_cred_product(request, pid):
4747
return render(request, "dojo/view_cred_prod.html", {"product_tab": product_tab, "creds": creds, "prod": prod})
4848

4949

50-
@user_is_authorized(Cred_User, Permissions.Credential_Edit, "ttid")
50+
@user_is_configuration_authorized(Permissions.Credential_Edit)
5151
def edit_cred(request, ttid):
5252
tool_config = Cred_User.objects.get(pk=ttid)
5353
if request.method == "POST":
@@ -79,7 +79,7 @@ def edit_cred(request, ttid):
7979
})
8080

8181

82-
@user_is_authorized(Cred_User, Permissions.Credential_View, "ttid")
82+
@user_is_configuration_authorized(Permissions.Credential_View)
8383
def view_cred_details(request, ttid):
8484
cred = Cred_User.objects.get(pk=ttid)
8585
notes = cred.notes.all()
@@ -127,7 +127,7 @@ def cred(request):
127127

128128

129129
@user_is_authorized(Product, Permissions.Product_View, "pid")
130-
@user_is_authorized(Cred_User, Permissions.Credential_View, "ttid")
130+
@user_is_authorized(Cred_Mapping, Permissions.Credential_View, "ttid")
131131
def view_cred_product(request, pid, ttid):
132132
cred = get_object_or_404(
133133
Cred_Mapping.objects.select_related("cred_id"), id=ttid)
@@ -182,8 +182,8 @@ def view_cred_product(request, pid, ttid):
182182
})
183183

184184

185-
@user_is_authorized(Product, Permissions.Engagement_View, "eid")
186-
@user_is_authorized(Cred_User, Permissions.Credential_View, "ttid")
185+
@user_is_authorized(Engagement, Permissions.Engagement_View, "eid")
186+
@user_is_authorized(Cred_Mapping, Permissions.Credential_View, "ttid")
187187
def view_cred_product_engagement(request, eid, ttid):
188188
cred = get_object_or_404(
189189
Cred_Mapping.objects.select_related("cred_id"), id=ttid)
@@ -231,8 +231,8 @@ def view_cred_product_engagement(request, eid, ttid):
231231
})
232232

233233

234-
@user_is_authorized(Product, Permissions.Test_View, "tid")
235-
@user_is_authorized(Cred_User, Permissions.Credential_View, "ttid")
234+
@user_is_authorized(Test, Permissions.Test_View, "tid")
235+
@user_is_authorized(Cred_Mapping, Permissions.Credential_View, "ttid")
236236
def view_cred_engagement_test(request, tid, ttid):
237237
cred = get_object_or_404(
238238
Cred_Mapping.objects.select_related("cred_id"), id=ttid)
@@ -282,8 +282,8 @@ def view_cred_engagement_test(request, tid, ttid):
282282
})
283283

284284

285-
@user_is_authorized(Product, Permissions.Finding_View, "fid")
286-
@user_is_authorized(Cred_User, Permissions.Credential_View, "ttid")
285+
@user_is_authorized(Finding, Permissions.Finding_View, "fid")
286+
@user_is_authorized(Cred_Mapping, Permissions.Credential_View, "ttid")
287287
def view_cred_finding(request, fid, ttid):
288288
cred = get_object_or_404(
289289
Cred_Mapping.objects.select_related("cred_id"), id=ttid)
@@ -334,7 +334,7 @@ def view_cred_finding(request, fid, ttid):
334334

335335

336336
@user_is_authorized(Product, Permissions.Product_Edit, "pid")
337-
@user_is_authorized(Cred_User, Permissions.Credential_Edit, "ttid")
337+
@user_is_authorized(Cred_Mapping, Permissions.Credential_Edit, "ttid")
338338
def edit_cred_product(request, pid, ttid):
339339
cred = get_object_or_404(
340340
Cred_Mapping.objects.select_related("cred_id"), id=ttid)
@@ -362,7 +362,7 @@ def edit_cred_product(request, pid, ttid):
362362

363363

364364
@user_is_authorized(Engagement, Permissions.Engagement_Edit, "eid")
365-
@user_is_authorized(Cred_User, Permissions.Credential_Edit, "ttid")
365+
@user_is_authorized(Cred_Mapping, Permissions.Credential_Edit, "ttid")
366366
def edit_cred_product_engagement(request, eid, ttid):
367367
cred = get_object_or_404(
368368
Cred_Mapping.objects.select_related("cred_id"), id=ttid)
@@ -582,7 +582,6 @@ def new_cred_finding(request, fid):
582582
})
583583

584584

585-
@user_is_authorized(Cred_User, Permissions.Credential_Delete, "ttid")
586585
def delete_cred_controller(request, destination_url, elem_id, ttid):
587586
cred = Cred_Mapping.objects.filter(pk=ttid).first()
588587
if request.method == "POST":
@@ -662,30 +661,30 @@ def delete_cred_controller(request, destination_url, elem_id, ttid):
662661
})
663662

664663

665-
@user_is_authorized(Cred_User, Permissions.Credential_Delete, "ttid")
664+
@user_is_configuration_authorized(Permissions.Credential_Delete)
666665
def delete_cred(request, ttid):
667666
return delete_cred_controller(request, "cred", 0, ttid=ttid)
668667

669668

670669
@user_is_authorized(Product, Permissions.Product_Edit, "pid")
671-
@user_is_authorized(Cred_User, Permissions.Credential_Delete, "ttid")
670+
@user_is_authorized(Cred_Mapping, Permissions.Credential_Delete, "ttid")
672671
def delete_cred_product(request, pid, ttid):
673672
return delete_cred_controller(request, "all_cred_product", pid, ttid)
674673

675674

676675
@user_is_authorized(Engagement, Permissions.Engagement_Edit, "eid")
677-
@user_is_authorized(Cred_User, Permissions.Credential_Delete, "ttid")
676+
@user_is_authorized(Cred_Mapping, Permissions.Credential_Delete, "ttid")
678677
def delete_cred_engagement(request, eid, ttid):
679678
return delete_cred_controller(request, "view_engagement", eid, ttid)
680679

681680

682681
@user_is_authorized(Test, Permissions.Test_Edit, "tid")
683-
@user_is_authorized(Cred_User, Permissions.Credential_Delete, "ttid")
682+
@user_is_authorized(Cred_Mapping, Permissions.Credential_Delete, "ttid")
684683
def delete_cred_test(request, tid, ttid):
685684
return delete_cred_controller(request, "view_test", tid, ttid)
686685

687686

688687
@user_is_authorized(Finding, Permissions.Finding_Edit, "fid")
689-
@user_is_authorized(Cred_User, Permissions.Credential_Delete, "ttid")
688+
@user_is_authorized(Cred_Mapping, Permissions.Credential_Delete, "ttid")
690689
def delete_cred_finding(request, fid, ttid):
691690
return delete_cred_controller(request, "view_finding", fid, ttid)

dojo/engagement/views.py

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1377,7 +1377,7 @@ def edit_risk_acceptance(request, eid, raid):
13771377
# will only be called by view_risk_acceptance and edit_risk_acceptance
13781378
def view_edit_risk_acceptance(request, eid, raid, *, edit_mode=False):
13791379
risk_acceptance = get_object_or_404(Risk_Acceptance, pk=raid)
1380-
eng = get_object_or_404(Engagement, pk=eid)
1380+
eng = get_object_or_404(Engagement.objects.filter(risk_acceptance=risk_acceptance), pk=eid)
13811381

13821382
if edit_mode and not eng.product.enable_full_risk_acceptance:
13831383
raise PermissionDenied
@@ -1537,8 +1537,7 @@ def view_edit_risk_acceptance(request, eid, raid, *, edit_mode=False):
15371537
@user_is_authorized(Engagement, Permissions.Risk_Acceptance, "eid")
15381538
def expire_risk_acceptance(request, eid, raid):
15391539
risk_acceptance = get_object_or_404(prefetch_for_expiration(Risk_Acceptance.objects.all()), pk=raid)
1540-
# Validate the engagement ID exists before moving forward
1541-
get_object_or_404(Engagement, pk=eid)
1540+
get_object_or_404(Engagement.objects.filter(risk_acceptance=risk_acceptance), pk=eid)
15421541

15431542
ra_helper.expire_now(risk_acceptance)
15441543

@@ -1548,7 +1547,7 @@ def expire_risk_acceptance(request, eid, raid):
15481547
@user_is_authorized(Engagement, Permissions.Risk_Acceptance, "eid")
15491548
def reinstate_risk_acceptance(request, eid, raid):
15501549
risk_acceptance = get_object_or_404(prefetch_for_expiration(Risk_Acceptance.objects.all()), pk=raid)
1551-
eng = get_object_or_404(Engagement, pk=eid)
1550+
eng = get_object_or_404(Engagement.objects.filter(risk_acceptance=risk_acceptance), pk=eid)
15521551

15531552
if not eng.product.enable_full_risk_acceptance:
15541553
raise PermissionDenied
@@ -1561,7 +1560,7 @@ def reinstate_risk_acceptance(request, eid, raid):
15611560
@user_is_authorized(Engagement, Permissions.Risk_Acceptance, "eid")
15621561
def delete_risk_acceptance(request, eid, raid):
15631562
risk_acceptance = get_object_or_404(Risk_Acceptance, pk=raid)
1564-
eng = get_object_or_404(Engagement, pk=eid)
1563+
eng = get_object_or_404(Engagement.objects.filter(risk_acceptance=risk_acceptance), pk=eid)
15651564

15661565
ra_helper.delete(eng, risk_acceptance)
15671566

0 commit comments

Comments
 (0)