Skip to content

Commit 73b9a68

Browse files
devGregAclaude
andauthored
Align questionnaire relink routes with questionnaire permissions (#15192)
The questionnaire relink routes (engagement_empty_survey and existing_engagement_empty_survey) now use the same permission mapping as the other questionnaire management routes, and engagement_empty_survey reuses the source-engagement edit check already used by the existing-engagement view. This keeps questionnaire handling consistent across the survey routes. Adds unit coverage for the relink routes. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent bb1e60f commit 73b9a68

3 files changed

Lines changed: 152 additions & 0 deletions

File tree

dojo/authorization/url_permissions.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -289,6 +289,12 @@
289289
"view_empty_survey": [("config", "dojo.view_engagement_survey")],
290290
"delete_empty_questionnaire": [("config", "dojo.delete_engagement_survey")],
291291
"delete_general_questionnaire": [("config", "dojo.delete_engagement_survey")],
292+
# Relinking an answered questionnaire to an engagement changes the
293+
# questionnaire, so it requires the questionnaire-change permission. The
294+
# views also verify the user can edit the source engagement (if any) before
295+
# moving the answers out of it.
296+
"engagement_empty_survey": [("config", "dojo.change_engagement_survey")],
297+
"existing_engagement_empty_survey": [("config", "dojo.change_engagement_survey")],
292298
}
293299

294300

dojo/survey/ui/views.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -818,6 +818,10 @@ def engagement_empty_survey(request, esid):
818818
if form.is_valid():
819819
product = form.cleaned_data.get("product")
820820
user_has_permission_or_403(request.user, product, "add")
821+
# If the questionnaire is already linked to an engagement, make sure
822+
# the user can edit that source engagement before moving its answers.
823+
if survey.engagement:
824+
user_has_permission_or_403(request.user, survey.engagement, "edit")
821825
engagement = Engagement(
822826
product_id=product.id,
823827
target_start=tz.now().date(),
Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,142 @@
1+
"""
2+
Regression tests for authorization on the "empty questionnaire" relink routes
3+
(``dojo/survey/ui/views.py``).
4+
5+
``engagement_empty_survey`` and ``existing_engagement_empty_survey`` relink an
6+
``Answered_Survey`` to an engagement. They were missing from
7+
``URL_PERMISSIONS``, so the ``AuthorizationMiddleware`` applied no check: any
8+
authenticated user could relink -- and thereby read -- another tenant's
9+
answered questionnaire by enumerating ``Answered_Survey`` primary keys, since
10+
only the attacker-chosen destination product was authorized.
11+
12+
These tests fix the contract:
13+
14+
* the relink routes require the questionnaire-change configuration permission;
15+
* even with that permission, a user cannot move a questionnaire out of a source
16+
engagement they are not allowed to edit;
17+
* the legitimate flow (claiming an unlinked questionnaire into a product you
18+
own) still works.
19+
"""
20+
from django.contrib.auth.models import Permission
21+
from django.urls import reverse
22+
from django.utils import timezone
23+
24+
from dojo.models import (
25+
Answered_Survey,
26+
Dojo_User,
27+
Engagement,
28+
Engagement_Survey,
29+
Product,
30+
Product_Type,
31+
)
32+
from unittests.dojo_test_case import DojoTestCase
33+
34+
35+
class EmptyQuestionnaireRelinkAuthorizationTests(DojoTestCase):
36+
37+
"""
38+
Two products and an attacker with no access to the victim tenant, plus a
39+
questionnaire manager who holds the configuration permission but is not a
40+
member of the victim product.
41+
"""
42+
43+
@classmethod
44+
def setUpTestData(cls):
45+
cls.prod_type = Product_Type.objects.create(name="qn_authz_pt")
46+
cls.victim_product = Product.objects.create(
47+
name="qn_authz_victim", description="v", prod_type=cls.prod_type,
48+
)
49+
cls.attacker_product = Product.objects.create(
50+
name="qn_authz_attacker", description="a", prod_type=cls.prod_type,
51+
)
52+
cls.manager_product = Product.objects.create(
53+
name="qn_authz_manager", description="m", prod_type=cls.prod_type,
54+
)
55+
56+
# Attacker: authenticated, member of only the attacker product, without
57+
# the questionnaire-change configuration permission.
58+
cls.attacker = Dojo_User.objects.create(username="qn_authz_attacker_user", is_active=True)
59+
cls.attacker_product.authorized_users.add(cls.attacker)
60+
61+
# Manager: holds the questionnaire-change configuration permission and
62+
# can add engagements to their own product, but has no access to the
63+
# victim product.
64+
cls.manager = Dojo_User.objects.create(username="qn_authz_manager_user", is_active=True)
65+
cls.manager_product.authorized_users.add(cls.manager)
66+
cls.manager.user_permissions.add(
67+
Permission.objects.get(
68+
content_type__app_label="dojo",
69+
codename="change_engagement_survey",
70+
),
71+
)
72+
73+
cls.template = Engagement_Survey.objects.create(name="qn_authz_template", description="t")
74+
75+
# Victim's answered questionnaire, linked to an engagement in the victim
76+
# product neither the attacker nor the manager can access.
77+
cls.victim_engagement = Engagement.objects.create(
78+
name="qn_authz_victim_eng",
79+
product=cls.victim_product,
80+
target_start=timezone.now().date(),
81+
target_end=timezone.now().date(),
82+
)
83+
cls.victim_survey = Answered_Survey.objects.create(
84+
survey=cls.template, engagement=cls.victim_engagement,
85+
)
86+
87+
# An unlinked ("empty") questionnaire response -- the legitimate object
88+
# of the relink workflow.
89+
cls.unlinked_survey = Answered_Survey.objects.create(survey=cls.template, engagement=None)
90+
91+
# ------------------------------------------------------------------
92+
# Middleware configuration gate: an authenticated user without the
93+
# questionnaire-change permission is rejected before the view runs.
94+
# ------------------------------------------------------------------
95+
def test_new_engagement_relink_get_denied_without_config_perm(self):
96+
self.client.force_login(self.attacker)
97+
response = self.client.get(reverse("engagement_empty_survey", args=(self.victim_survey.id,)))
98+
self.assertEqual(response.status_code, 400)
99+
100+
def test_new_engagement_relink_post_denied_without_config_perm(self):
101+
self.client.force_login(self.attacker)
102+
response = self.client.post(
103+
reverse("engagement_empty_survey", args=(self.victim_survey.id,)),
104+
data={"product": self.attacker_product.id},
105+
)
106+
self.assertEqual(response.status_code, 400)
107+
self.victim_survey.refresh_from_db()
108+
self.assertEqual(self.victim_survey.engagement_id, self.victim_engagement.id)
109+
110+
def test_existing_engagement_relink_get_denied_without_config_perm(self):
111+
self.client.force_login(self.attacker)
112+
response = self.client.get(reverse("existing_engagement_empty_survey", args=(self.victim_survey.id,)))
113+
self.assertEqual(response.status_code, 400)
114+
115+
# ------------------------------------------------------------------
116+
# View-level source check: even with the configuration permission, a user
117+
# cannot relink a questionnaire out of a source engagement they cannot edit.
118+
# ------------------------------------------------------------------
119+
def test_manager_cannot_relink_from_unauthorized_source_engagement(self):
120+
self.client.force_login(self.manager)
121+
response = self.client.post(
122+
reverse("engagement_empty_survey", args=(self.victim_survey.id,)),
123+
data={"product": self.manager_product.id},
124+
)
125+
self.assertEqual(response.status_code, 400)
126+
self.victim_survey.refresh_from_db()
127+
self.assertEqual(self.victim_survey.engagement_id, self.victim_engagement.id)
128+
129+
# ------------------------------------------------------------------
130+
# Positive control: the legitimate claim-an-unlinked-questionnaire flow
131+
# still works for a permitted user.
132+
# ------------------------------------------------------------------
133+
def test_manager_can_relink_unlinked_questionnaire_into_own_product(self):
134+
self.client.force_login(self.manager)
135+
response = self.client.post(
136+
reverse("engagement_empty_survey", args=(self.unlinked_survey.id,)),
137+
data={"product": self.manager_product.id},
138+
)
139+
self.assertEqual(response.status_code, 302)
140+
self.unlinked_survey.refresh_from_db()
141+
self.assertIsNotNone(self.unlinked_survey.engagement_id)
142+
self.assertEqual(self.unlinked_survey.engagement.product_id, self.manager_product.id)

0 commit comments

Comments
 (0)