Skip to content

Commit ae9d47b

Browse files
devGregAclaudeMaffooch
authored
refactor(locations): consistent object lookups in endpoint views (#15173)
* refactor(locations): use a shared helper for endpoint view object lookups The endpoint (Location) views each resolved their object with an ad-hoc get_object_or_404 (and one Location.objects.get). Route them all through a single helper that fetches the Location from the standard location queryset, consistent with the list and host views. Adds regression coverage for the endpoint view lookups. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(locations): assert real cross-product denial status (400) for endpoint views The cross-product tests asserted 404, but these views are guarded by the AuthorizationMiddleware object check (URL_PERMISSIONS -> (object, Location, ...)), and DefectDojo renders PermissionDenied via custom_unauthorized_view as HTTP 400 app-wide. Assert the actual denial status so the suite passes under V3_FEATURE_LOCATIONS; the view-level get_authorized_locations lookup remains as defense-in-depth. Security property (deny + object unchanged) is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: Cody Maffucci <46459665+Maffooch@users.noreply.github.com>
1 parent 73b9a68 commit ae9d47b

2 files changed

Lines changed: 111 additions & 6 deletions

File tree

dojo/url/ui/views.py

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,20 @@
4343
logger = logging.getLogger(__name__)
4444

4545

46+
def _get_location_or_404(request, location_id, permission):
47+
"""
48+
Resolve a Location for the endpoint views via the shared authorized queryset.
49+
50+
Keeps object retrieval in these views consistent with the list/host views, which
51+
already scope Location lookups through ``get_authorized_locations``. A lookup that
52+
falls outside the queryset returns 404, matching object retrieval elsewhere.
53+
"""
54+
return get_object_or_404(
55+
get_authorized_locations(permission, Location.objects.all(), request.user),
56+
id=location_id,
57+
)
58+
59+
4660
def view_endpoint(request: HttpRequest, location_id: int):
4761
return process_endpoint_view(request, location_id, host_view=False)
4862

@@ -95,7 +109,7 @@ def process_endpoint_view(request: HttpRequest, location_id: int, *, host_view=F
95109
- host_view: Boolean indicating if host view is enabled.
96110
97111
"""
98-
location = get_object_or_404(Location, id=location_id)
112+
location = _get_location_or_404(request, location_id, "view")
99113
if location.location_type != URL.get_location_type():
100114
messages.add_message(
101115
request,
@@ -288,7 +302,7 @@ def process_endpoints_view(request, *, host_view=False, vulnerable=False):
288302

289303
def edit_endpoint(request, location_id):
290304
# Retrieve the Location object by ID and add breadcrumb for editing
291-
location = get_object_or_404(Location, id=location_id)
305+
location = _get_location_or_404(request, location_id, "edit")
292306
add_breadcrumb(parent=location, title="Edit", top_level=False, request=request)
293307
# Initialize the URLForm with the current URL instance for editing
294308
form = URLForm(instance=location.url)
@@ -366,7 +380,7 @@ def add_endpoint_to_finding(request, finding_id):
366380

367381
def delete_endpoint(request, location_id):
368382
# Retrieve the Location object by primary key and initialize the delete form
369-
location = get_object_or_404(Location, pk=location_id)
383+
location = _get_location_or_404(request, location_id, "delete")
370384
form = DeleteEndpointForm(instance=location)
371385
# Handle POST request for deleting an endpoint and its relationships
372386
if request.method == "POST":
@@ -399,7 +413,7 @@ def delete_endpoint(request, location_id):
399413

400414
def manage_meta_data(request, location_id):
401415
# Retrieve the Location object by ID and filter its associated metadata
402-
location = Location.objects.get(id=location_id)
416+
location = _get_location_or_404(request, location_id, "edit")
403417
meta_data_query = DojoMeta.objects.filter(location=location)
404418
# Map the foreign key for the formset to the location
405419
form_mapping = {"location": location}
@@ -624,10 +638,10 @@ def migrate_endpoints_view(request):
624638

625639

626640
def endpoint_report(request, location_id):
627-
location = get_object_or_404(Location, id=location_id)
641+
location = _get_location_or_404(request, location_id, "view")
628642
return generate_report(request, location, host_view=False)
629643

630644

631645
def endpoint_host_report(request, location_id):
632-
location = get_object_or_404(Location, id=location_id)
646+
location = _get_location_or_404(request, location_id, "view")
633647
return generate_report(request, location, host_view=True)
Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
from django.urls import reverse
2+
3+
from dojo.authorization.roles_permissions import Roles
4+
from dojo.location.models import Location, LocationProductReference
5+
from dojo.location.status import ProductLocationStatus
6+
from dojo.models import (
7+
Dojo_User,
8+
Product,
9+
Product_Member,
10+
Product_Type,
11+
Role,
12+
User,
13+
)
14+
from dojo.url.models import URL
15+
from unittests.dojo_test_case import DojoTestCase, skip_unless_v3
16+
17+
18+
@skip_unless_v3
19+
class LocationEndpointViewCrossProductAuthzTest(DojoTestCase):
20+
21+
"""
22+
The endpoint (Location) UI views resolve objects by location_id.
23+
24+
A user authorized for one product must not be able to read, edit, or delete a
25+
Location that belongs only to a different product.
26+
"""
27+
28+
@classmethod
29+
def setUpTestData(cls):
30+
prod_type, _ = Product_Type.objects.get_or_create(name="LOC-XProd PT")
31+
writer_role = Role.objects.get(id=Roles.Writer)
32+
33+
cls.product_a = Product.objects.create(name="LOC-XProd Product A", description="A", prod_type=prod_type)
34+
cls.product_b = Product.objects.create(name="LOC-XProd Product B", description="B", prod_type=prod_type)
35+
36+
# Alice is authorized only for Product A. Legacy authorization is membership-based
37+
# via authorized_users, so mirror the Product_Member row onto that M2M.
38+
cls.alice = User.objects.create_user(
39+
username="loc_xprod_alice",
40+
password="not-a-real-secret", # noqa: S106 - test fixture user
41+
)
42+
Product_Member.objects.create(user=cls.alice, product=cls.product_a, role=writer_role)
43+
cls.product_a.authorized_users.add(Dojo_User.objects.get(pk=cls.alice.pk))
44+
45+
# A URL location that belongs only to Product B (Alice must not reach it).
46+
cls.location_b = URL.create_location_from_value("https://private.example.test/secret").location
47+
LocationProductReference.objects.create(
48+
location=cls.location_b, product=cls.product_b, status=ProductLocationStatus.Active,
49+
)
50+
# A URL location that belongs to Product A (Alice may reach it).
51+
cls.location_a = URL.create_location_from_value("https://a.example.test/ok").location
52+
LocationProductReference.objects.create(
53+
location=cls.location_a, product=cls.product_a, status=ProductLocationStatus.Active,
54+
)
55+
56+
def setUp(self):
57+
super().setUp()
58+
self.client.force_login(self.alice)
59+
60+
# A cross-product request is denied by the AuthorizationMiddleware object check
61+
# (URL_PERMISSIONS maps these views to ("object", Location, ...)). DefectDojo renders
62+
# PermissionDenied via dojo.views.custom_unauthorized_view, which returns HTTP 400
63+
# app-wide, so the denied status here is 400. The view-level get_authorized_locations
64+
# lookup is defense-in-depth behind that middleware check.
65+
DENIED_STATUS = 400
66+
67+
def test_view_endpoint_cross_product_is_denied(self):
68+
response = self.client.get(reverse("view_endpoint", kwargs={"location_id": self.location_b.id}))
69+
self.assertEqual(self.DENIED_STATUS, response.status_code)
70+
71+
def test_view_endpoint_own_product_is_allowed(self):
72+
response = self.client.get(reverse("view_endpoint", kwargs={"location_id": self.location_a.id}))
73+
self.assertEqual(200, response.status_code)
74+
75+
def test_edit_endpoint_cross_product_is_denied_and_unchanged(self):
76+
original_host = self.location_b.url.host
77+
response = self.client.post(
78+
reverse("edit_endpoint", kwargs={"location_id": self.location_b.id}),
79+
{"protocol": "https", "host": "changed.example.test"},
80+
)
81+
self.assertEqual(self.DENIED_STATUS, response.status_code)
82+
self.location_b.url.refresh_from_db()
83+
self.assertEqual(original_host, self.location_b.url.host)
84+
85+
def test_delete_endpoint_cross_product_is_denied_and_persists(self):
86+
response = self.client.post(
87+
reverse("delete_endpoint", kwargs={"location_id": self.location_b.id}),
88+
{"id": self.location_b.id},
89+
)
90+
self.assertEqual(self.DENIED_STATUS, response.status_code)
91+
self.assertTrue(Location.objects.filter(pk=self.location_b.id).exists())

0 commit comments

Comments
 (0)