Skip to content

Commit 00e88f2

Browse files
authored
Harden metadata API object authorization (#15380)
* Harden metadata API object authorization Authorize the target object supplied on every mutating verb of the metadata endpoint, not only on a subset, and keep the authorized metadata queryset available to non-superusers. Adds a regression test. * lint: sort imports in metadata authz regression test * Restore original verb-map comment, add one-line clarification
1 parent e6d0dcd commit 00e88f2

3 files changed

Lines changed: 91 additions & 2 deletions

File tree

dojo/authorization/api_permissions.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -255,6 +255,8 @@ def has_permission(self, request, view):
255255
"GET": "get_permission",
256256
"POST": "post_permission",
257257
# PATCH is generally not used here, but this endpoint is sorta odd...
258+
# ...it accepts PUT and PATCH alike, so both must authorize the target.
259+
"PUT": "put_permission",
258260
"PATCH": "put_permission",
259261
}
260262
for request_method, permission_type in method_to_permission_map.items():

dojo/authorization/query_registrations.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -225,11 +225,12 @@ def _get_authorized_dojo_meta(permission):
225225
return DojoMeta.objects.none()
226226
if _is_unrestricted(user, permission_to_action(permission)):
227227
return DojoMeta.objects.all()
228+
# _authorized_product_ids already includes products reachable via product-type
229+
# membership, so the product / finding / endpoint clauses below cover product-type
230+
# authorization on their own.
228231
authorized_products = _authorized_product_ids(user)
229-
authorized_product_types = _authorized_product_type_ids(user)
230232
return DojoMeta.objects.filter(
231233
Q(product__id__in=authorized_products)
232-
| Q(product_type__id__in=authorized_product_types)
233234
| Q(finding__test__engagement__product__id__in=authorized_products)
234235
| Q(endpoint__product__id__in=authorized_products),
235236
)
Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
"""
2+
Regression tests for cross-tenant authorization on the metadata REST API.
3+
4+
A user with edit on a source product must not be able to re-parent a
5+
metadata record onto a product they have no access to, and this must
6+
hold on every mutating verb (both PUT and PATCH). Also guards that
7+
listing metadata stays available to ordinary (non-superuser) users.
8+
"""
9+
from django.urls import reverse
10+
from rest_framework.authtoken.models import Token
11+
from rest_framework.test import APIClient
12+
13+
from dojo.authorization.models import Product_Member, Role
14+
from dojo.models import Dojo_User, DojoMeta, Product, Product_Type
15+
16+
from .dojo_test_case import DojoTestCase
17+
from .test_permissions_audit import LegacyAuthMirrorMixin
18+
19+
PASSWORD = "testTEST1234!@#$"
20+
21+
22+
class TestMetadataCrossTenantAuthorization(LegacyAuthMirrorMixin, DojoTestCase):
23+
24+
@classmethod
25+
def setUpTestData(cls):
26+
owner_role = Role.objects.get(name="Owner")
27+
28+
cls.pt_src = Product_Type.objects.create(name="Meta Authz SRC PT")
29+
cls.pt_outside = Product_Type.objects.create(name="Meta Authz OUTSIDE PT")
30+
cls.product_src = Product.objects.create(
31+
name="Meta Authz Src Product", description="src", prod_type=cls.pt_src,
32+
)
33+
cls.product_outside = Product.objects.create(
34+
name="Meta Authz Outside Product", description="out", prod_type=cls.pt_outside,
35+
)
36+
37+
cls.user = Dojo_User.objects.create_user(
38+
username="meta_authz_user", password=PASSWORD, is_active=True,
39+
)
40+
# Edit on the source product only; no membership on product_outside.
41+
Product_Member.objects.create(
42+
product=cls.product_src, user=cls.user, role=owner_role,
43+
)
44+
45+
cls.meta = DojoMeta.objects.create(
46+
product=cls.product_src, name="k", value="v",
47+
)
48+
cls.token = Token.objects.create(user=cls.user)
49+
50+
def _client(self):
51+
client = APIClient()
52+
client.credentials(HTTP_AUTHORIZATION=f"Token {self.token.key}")
53+
return client
54+
55+
def _relink_payload(self):
56+
return {"product": self.product_outside.id, "name": "k", "value": "v"}
57+
58+
def _assert_rejected(self, response):
59+
self.assertIn(
60+
response.status_code, [400, 403],
61+
msg=f"Expected 400/403, got {response.status_code}: {response.content[:500]!r}",
62+
)
63+
64+
def test_put_relink_to_unauthorized_product_blocked(self):
65+
r = self._client().put(
66+
reverse("metadata-detail", args=(self.meta.id,)),
67+
self._relink_payload(),
68+
format="json",
69+
)
70+
self._assert_rejected(r)
71+
self.meta.refresh_from_db()
72+
self.assertEqual(self.meta.product_id, self.product_src.id)
73+
74+
def test_patch_relink_to_unauthorized_product_blocked(self):
75+
r = self._client().patch(
76+
reverse("metadata-detail", args=(self.meta.id,)),
77+
self._relink_payload(),
78+
format="json",
79+
)
80+
self._assert_rejected(r)
81+
self.meta.refresh_from_db()
82+
self.assertEqual(self.meta.product_id, self.product_src.id)
83+
84+
def test_metadata_list_available_to_non_superuser(self):
85+
r = self._client().get(reverse("metadata-list"))
86+
self.assertEqual(r.status_code, 200, r.content[:500])

0 commit comments

Comments
 (0)