Skip to content

Commit 8bcf818

Browse files
devGregAclaude
andcommitted
test(authorization): adapt RBAC-flavored tests to legacy semantics
Apply the same legacy-semantics rewrite to the next layer of test fallout — files that exercise risk-acceptance, finding-template, user-API, and user-queries paths through the authorization layer. Notable changes: * test_apply_finding_template: replace test_reader_role_cannot_X / test_writer_role_can_X with test_member_can_X / test_non_member_cannot_X. Legacy collapses Reader/Writer/Maintainer to one bit (membership in authorized_users), so the role pair is meaningless under legacy. * test_apiv2_user: rename allows_global_owner → denies_global_owner. Global_Role(role=Owner) is inert under legacy; resetting another user's API token requires is_superuser, not a Global_Role. * test_bulk_risk_acceptance_api: Writer needs to be added to product.authorized_users to actually have access. Reader's "forbidden" tests now expect 404 (DRF returns not-found when the per-object queryset is empty, hiding existence rather than 403). * test_user_queries: rewrite TestGetAuthorizedUsersForProductType / TestGetAuthorizedUsersForProductAndProductType to assert legacy semantics — the queries are gated on the calling user's staff/superuser status, not on the listed users' RBAC roles. Two real bugs surfaced and fixed: * dojo/authorization/authorization.py: Risk_Acceptance branch crashed with AttributeError because there is no engagement_id field — Risk_Acceptance is reachable from Engagement via the reverse M2M engagement.risk_acceptance. Walk through engagement_set.first() the way pre-2020 code did at e7805aa~. * dojo/authorization/query_registrations.py: _get_authorized_users_for_product_type and _get_authorized_users_for_product_and_product_type crashed when callers passed users=None (the test suite does this idiomatically). Default to Dojo_User.objects.all() in that case. After this commit: 182 of 182 authorization-touching tests pass (was 132/182). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent fb505f8 commit 8bcf818

6 files changed

Lines changed: 120 additions & 112 deletions

File tree

dojo/authorization/authorization.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -153,8 +153,12 @@ def _user_authorized_for(user: Dojo_User, obj: Model, action: Action) -> bool:
153153
return _user_authorized_for(user, obj.test.engagement.product, action)
154154

155155
if isinstance(obj, Risk_Acceptance):
156-
if obj.engagement_id is not None:
157-
return _user_authorized_for(user, obj.engagement.product, action)
156+
# Risk_Acceptance is reachable from Engagement via the reverse M2M
157+
# `engagement.risk_acceptance`. Pre-2020 followed the same path
158+
# (see dojo/user/helper.py at e7805aa14~).
159+
engagement = obj.engagement_set.first()
160+
if engagement is not None:
161+
return _user_authorized_for(user, engagement.product, action)
158162
return False
159163

160164
if isinstance(obj, Location):

dojo/authorization/query_registrations.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -609,6 +609,8 @@ def _get_authorized_users(permission, user=None):
609609

610610

611611
def _get_authorized_users_for_product_type(users, product_type, permission):
612+
if users is None:
613+
users = Dojo_User.objects.all()
612614
user = get_current_user()
613615
if user is None or getattr(user, "is_anonymous", False):
614616
return users.none()
@@ -621,6 +623,8 @@ def _get_authorized_users_for_product_type(users, product_type, permission):
621623

622624

623625
def _get_authorized_users_for_product_and_product_type(users, product, permission):
626+
if users is None:
627+
users = Dojo_User.objects.all()
624628
user = get_current_user()
625629
if user is None or getattr(user, "is_anonymous", False):
626630
return users.none()

unittests/test_apiv2_user.py

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -201,8 +201,12 @@ def test_user_reset_api_token_denies_non_privileged(self):
201201
r = nonpriv_client.post(url)
202202
self.assertEqual(r.status_code, 403, r.content[:1000])
203203

204-
def test_user_reset_api_token_allows_global_owner(self):
205-
# Create a global-owner user (not superuser)
204+
def test_user_reset_api_token_denies_global_owner_legacy(self):
205+
"""
206+
Legacy: Global_Role(role=Owner) is inert. Resetting another
207+
user's API token requires is_superuser; a global-owner who isn't
208+
a superuser is treated like any non-privileged user.
209+
"""
206210
password = "testTEST1234!@#$"
207211
r = self.client.post(reverse("user-list"), {
208212
"username": "api-user-global-owner",
@@ -240,4 +244,4 @@ def test_user_reset_api_token_allows_global_owner(self):
240244

241245
url = "{}{}/reset_api_token/".format(reverse("user-list"), target_id)
242246
r = go_client.post(url)
243-
self.assertEqual(r.status_code, 204, r.content[:1000])
247+
self.assertEqual(r.status_code, 403, r.content[:1000])

unittests/test_apply_finding_template.py

Lines changed: 33 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
from dojo.finding import views
1717
from dojo.finding.helper import save_endpoints_template, save_vulnerability_ids_template
1818
from dojo.models import (
19+
Dojo_User,
1920
Engagement,
2021
Finding,
2122
Finding_Template,
@@ -107,9 +108,14 @@ def create_user(is_staff):
107108

108109
@staticmethod
109110
def create_user_with_role(product, role_name, *, is_staff=False):
110-
"""Create a user with a specific role on a product"""
111+
"""
112+
Create a user with a specific role on a product. Under the
113+
legacy authorization model the role is informational only —
114+
callers needing actual access must add the user to
115+
product.authorized_users.
116+
"""
111117
user_count = User.objects.count()
112-
user = User()
118+
user = Dojo_User()
113119
user.is_staff = is_staff
114120
user.is_superuser = False
115121
user.username = f"TestUser{role_name}{user_count}"
@@ -219,41 +225,48 @@ def test_unauthorized_apply_template_to_finding_fails(self):
219225
"impact": "template impact"},
220226
)
221227

222-
def test_reader_role_cannot_apply_template(self):
223-
"""Test that a Reader role user (read-only) cannot apply template"""
224-
reader_user = FindingTemplateTestUtil.create_user_with_role(
225-
self.finding.test.engagement.product, "Reader", is_staff=False,
226-
)
228+
def test_authorized_user_can_apply_template(self):
229+
"""
230+
Legacy: any user in product.authorized_users can apply a template
231+
(Reader/Writer/Maintainer/Owner all collapse to one bit of access).
232+
"""
233+
product = self.finding.test.engagement.product
234+
member = FindingTemplateTestUtil.create_user_with_role(product, "Writer", is_staff=False)
235+
product.authorized_users.add(member)
227236
request = FindingTemplateTestUtil.create_post_request(
228-
reader_user, self.apply_template_url,
237+
member, self.apply_template_url,
229238
data={"title": "Finding for Testing Apply Template functionality",
230239
"cwe": "89",
231240
"severity": "High",
232241
"description": "Finding for Testing Apply Template Functionality",
233242
"mitigation": "template mitigation",
234243
"impact": "template impact"},
235244
)
236-
with impersonate(reader_user), self.assertRaises(PermissionDenied):
237-
views.apply_template_to_finding(request, fid=self.finding.id, tid=self.template.id)
245+
with impersonate(member):
246+
result = views.apply_template_to_finding(request, fid=self.finding.id, tid=self.template.id)
247+
self.assertEqual(302, result.status_code)
248+
self.assertEqual(f"/finding/{self.finding.id}", result.url)
238249

239-
def test_writer_role_can_apply_template(self):
240-
"""Test that a Writer role user (non-staff) can apply template"""
241-
writer_user = FindingTemplateTestUtil.create_user_with_role(
242-
self.finding.test.engagement.product, "Writer", is_staff=False,
243-
)
250+
def test_non_member_cannot_apply_template(self):
251+
"""
252+
Legacy: a user with no authorized_users membership and no staff
253+
flag is denied — covers the case test_reader_role_cannot_apply_template
254+
used to assert under RBAC.
255+
"""
256+
product = self.finding.test.engagement.product
257+
outsider = FindingTemplateTestUtil.create_user_with_role(product, "Reader", is_staff=False)
258+
# Deliberately NOT added to product.authorized_users.
244259
request = FindingTemplateTestUtil.create_post_request(
245-
writer_user, self.apply_template_url,
260+
outsider, self.apply_template_url,
246261
data={"title": "Finding for Testing Apply Template functionality",
247262
"cwe": "89",
248263
"severity": "High",
249264
"description": "Finding for Testing Apply Template Functionality",
250265
"mitigation": "template mitigation",
251266
"impact": "template impact"},
252267
)
253-
with impersonate(writer_user):
254-
result = views.apply_template_to_finding(request, fid=self.finding.id, tid=self.template.id)
255-
self.assertEqual(302, result.status_code)
256-
self.assertEqual(f"/finding/{self.finding.id}", result.url)
268+
with impersonate(outsider), self.assertRaises(PermissionDenied):
269+
views.apply_template_to_finding(request, fid=self.finding.id, tid=self.template.id)
257270

258271
def test_apply_template_to_finding_with_illegal_finding_fails(self):
259272
with self.assertRaises(Http404):

unittests/test_bulk_risk_acceptance_api.py

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
)
1111
from dojo.authorization.roles_permissions import Roles
1212
from dojo.models import (
13+
Dojo_User,
1314
Engagement,
1415
Finding,
1516
Product,
@@ -166,8 +167,9 @@ def setUpTestData(cls):
166167
target_end=datetime.datetime(2000, 2, 1, tzinfo=datetime.UTC),
167168
)
168169

169-
# Writer user: has Risk_Acceptance permission, NOT is_staff
170-
cls.writer = User.objects.create(username="rbac_writer", is_staff=False)
170+
# Writer user: a member of both products (legacy: any member of
171+
# authorized_users can perform risk acceptance).
172+
cls.writer = Dojo_User.objects.create(username="rbac_writer", is_staff=False)
171173
cls.writer_token = Token.objects.create(user=cls.writer)
172174
Product_Member.objects.create(
173175
product=cls.product_enabled, user=cls.writer,
@@ -177,9 +179,13 @@ def setUpTestData(cls):
177179
product=cls.product_disabled, user=cls.writer,
178180
role=Role.objects.get(id=Roles.Writer),
179181
)
182+
cls.product_enabled.authorized_users.add(cls.writer)
183+
cls.product_disabled.authorized_users.add(cls.writer)
180184

181-
# Reader user: does NOT have Risk_Acceptance permission, NOT is_staff
182-
cls.reader = User.objects.create(username="rbac_reader", is_staff=False)
185+
# Reader user: NOT in authorized_users — under legacy this is the
186+
# only way to deny access (Reader/Writer/Maintainer/Owner are all
187+
# the same bit). The Product_Member row is informational only.
188+
cls.reader = Dojo_User.objects.create(username="rbac_reader", is_staff=False)
183189
cls.reader_token = Token.objects.create(user=cls.reader)
184190
Product_Member.objects.create(
185191
product=cls.product_enabled, user=cls.reader,
@@ -243,25 +249,28 @@ def test_writer_can_accept_risks_on_findings(self):
243249
)
244250
self.assertEqual(result.status_code, 201)
245251

246-
# --- Reader (no Risk_Acceptance) is forbidden ---
252+
# --- Non-member (reader is not in product.authorized_users under
253+
# legacy) sees the object as not-found because get_authorized_*
254+
# filters return an empty queryset, hiding existence rather than
255+
# leaking it via 403. ---
247256

248-
def test_reader_forbidden_on_engagement(self):
257+
def test_non_member_not_found_on_engagement(self):
249258
client = self._client_for(self.reader_token)
250259
result = client.post(
251260
reverse("engagement-accept-risks", kwargs={"pk": self.engagement_enabled.id}),
252261
data=self._accepted_risks(["CVE-2024-3"]),
253262
format="json",
254263
)
255-
self.assertEqual(result.status_code, 403)
264+
self.assertEqual(result.status_code, 404)
256265

257-
def test_reader_forbidden_on_test(self):
266+
def test_non_member_not_found_on_test(self):
258267
client = self._client_for(self.reader_token)
259268
result = client.post(
260269
reverse("test-accept-risks", kwargs={"pk": self.test_enabled.id}),
261270
data=self._accepted_risks(["CVE-2024-4"]),
262271
format="json",
263272
)
264-
self.assertEqual(result.status_code, 403)
273+
self.assertEqual(result.status_code, 404)
265274

266275
def test_reader_gets_empty_result_on_findings(self):
267276
client = self._client_for(self.reader_token)

0 commit comments

Comments
 (0)