Skip to content

Commit 0df0520

Browse files
stsewdstsewd
authored
API V3: add IsAuthenticated to permissions (#10452)
This isn't a security vulnerability, we were filtering by the current user. This was just causing errors, since we were expecting a real user in some queries. These views are used in .com only, so I have added tests there. Ref https://read-the-docs.sentry.io/issues/4261898141/?alert_rule_id=242443&alert_timestamp=1687234764178&alert_type=email&environment=production&project=161479&referrer=alert_email
1 parent 463e106 commit 0df0520

9 files changed

+250
-171
lines changed

‎readthedocs/api/v3/tests/test_builds.py

+39-24
Original file line numberDiff line numberDiff line change
@@ -19,26 +19,36 @@
1919
class BuildsEndpointTests(APIEndpointMixin):
2020

2121
def test_projects_builds_list(self):
22-
self.client.credentials(HTTP_AUTHORIZATION=f'Token {self.token.key}')
23-
response = self.client.get(
24-
reverse(
25-
'projects-builds-list',
26-
kwargs={
27-
'parent_lookup_project__slug': self.project.slug,
28-
}),
22+
url = reverse(
23+
"projects-builds-list",
24+
kwargs={
25+
"parent_lookup_project__slug": self.project.slug,
26+
},
2927
)
28+
29+
self.client.logout()
30+
response = self.client.get(url)
31+
self.assertEqual(response.status_code, 401)
32+
33+
self.client.credentials(HTTP_AUTHORIZATION=f"Token {self.token.key}")
34+
response = self.client.get(url)
3035
self.assertEqual(response.status_code, 200)
3136

3237
def test_projects_builds_detail(self):
33-
self.client.credentials(HTTP_AUTHORIZATION=f'Token {self.token.key}')
34-
response = self.client.get(
35-
reverse(
36-
'projects-builds-detail',
37-
kwargs={
38-
'parent_lookup_project__slug': self.project.slug,
39-
'build_pk': self.build.pk,
40-
}),
38+
url = reverse(
39+
"projects-builds-detail",
40+
kwargs={
41+
"parent_lookup_project__slug": self.project.slug,
42+
"build_pk": self.build.pk,
43+
},
4144
)
45+
46+
self.client.logout()
47+
response = self.client.get(url)
48+
self.assertEqual(response.status_code, 401)
49+
50+
self.client.credentials(HTTP_AUTHORIZATION=f"Token {self.token.key}")
51+
response = self.client.get(url)
4252
self.assertEqual(response.status_code, 200)
4353

4454
self.assertDictEqual(
@@ -47,16 +57,21 @@ def test_projects_builds_detail(self):
4757
)
4858

4959
def test_projects_versions_builds_list_post(self):
50-
self.client.credentials(HTTP_AUTHORIZATION=f'Token {self.token.key}')
51-
self.assertEqual(self.project.builds.count(), 1)
52-
response = self.client.post(
53-
reverse(
54-
'projects-versions-builds-list',
55-
kwargs={
56-
'parent_lookup_project__slug': self.project.slug,
57-
'parent_lookup_version__slug': self.version.slug,
58-
}),
60+
url = reverse(
61+
"projects-versions-builds-list",
62+
kwargs={
63+
"parent_lookup_project__slug": self.project.slug,
64+
"parent_lookup_version__slug": self.version.slug,
65+
},
5966
)
67+
68+
self.client.logout()
69+
response = self.client.post(url)
70+
self.assertEqual(response.status_code, 401)
71+
72+
self.client.credentials(HTTP_AUTHORIZATION=f"Token {self.token.key}")
73+
self.assertEqual(self.project.builds.count(), 1)
74+
response = self.client.post(url)
6075
self.assertEqual(response.status_code, 202)
6176
self.assertEqual(self.project.builds.count(), 2)
6277

‎readthedocs/api/v3/tests/test_environmentvariables.py

+13-9
Original file line numberDiff line numberDiff line change
@@ -132,15 +132,19 @@ def test_projects_environmentvariables_list_post(self):
132132
)
133133

134134
def test_projects_environmentvariables_detail_delete(self):
135-
self.assertEqual(self.project.environmentvariable_set.count(), 1)
136-
self.client.credentials(HTTP_AUTHORIZATION=f'Token {self.token.key}')
137-
response = self.client.delete(
138-
reverse(
139-
'projects-environmentvariables-detail',
140-
kwargs={
141-
'parent_lookup_project__slug': self.project.slug,
142-
'environmentvariable_pk': self.environmentvariable.pk,
143-
}),
135+
url = reverse(
136+
"projects-environmentvariables-detail",
137+
kwargs={
138+
"parent_lookup_project__slug": self.project.slug,
139+
"environmentvariable_pk": self.environmentvariable.pk,
140+
},
144141
)
142+
self.client.logout()
143+
response = self.client.delete(url)
144+
self.assertEqual(response.status_code, 401)
145+
146+
self.assertEqual(self.project.environmentvariable_set.count(), 1)
147+
self.client.credentials(HTTP_AUTHORIZATION=f"Token {self.token.key}")
148+
response = self.client.delete(url)
145149
self.assertEqual(response.status_code, 204)
146150
self.assertEqual(self.project.environmentvariable_set.count(), 0)

‎readthedocs/api/v3/tests/test_projects.py

+67-43
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,14 @@
1818
class ProjectsEndpointTests(APIEndpointMixin):
1919

2020
def test_projects_list(self):
21+
url = reverse("projects-list")
22+
23+
self.client.logout()
24+
response = self.client.get(url)
25+
self.assertEqual(response.status_code, 401)
26+
2127
self.client.credentials(HTTP_AUTHORIZATION=f'Token {self.token.key}')
22-
response = self.client.get(
23-
reverse('projects-list'),
24-
)
28+
response = self.client.get(url)
2529
self.assertEqual(response.status_code, 200)
2630
self.assertDictEqual(
2731
response.json(),
@@ -96,21 +100,26 @@ def test_projects_list_filter_miss(self):
96100
)
97101

98102
def test_own_projects_detail(self):
99-
self.client.credentials(HTTP_AUTHORIZATION=f'Token {self.token.key}')
100-
response = self.client.get(
101-
reverse(
102-
'projects-detail',
103-
kwargs={
104-
'project_slug': self.project.slug,
105-
}),
106-
{
107-
'expand': (
108-
'active_versions,'
109-
'active_versions.last_build,'
110-
'active_versions.last_build.config'
111-
),
103+
url = reverse(
104+
"projects-detail",
105+
kwargs={
106+
"project_slug": self.project.slug,
112107
},
113108
)
109+
data = {
110+
"expand": (
111+
"active_versions,"
112+
"active_versions.last_build,"
113+
"active_versions.last_build.config"
114+
),
115+
}
116+
117+
self.client.logout()
118+
response = self.client.get(url, data)
119+
self.assertEqual(response.status_code, 401)
120+
121+
self.client.credentials(HTTP_AUTHORIZATION=f"Token {self.token.key}")
122+
response = self.client.get(url, data)
114123
self.assertEqual(response.status_code, 200)
115124

116125
self.assertDictEqual(
@@ -173,15 +182,20 @@ def test_own_projects_detail_privacy_levels_enabled(self):
173182

174183
def test_projects_superproject(self):
175184
self._create_subproject()
176-
self.client.credentials(HTTP_AUTHORIZATION=f'Token {self.token.key}')
177-
response = self.client.get(
178-
reverse(
179-
'projects-superproject',
180-
kwargs={
181-
'project_slug': self.subproject.slug,
182-
},
183-
),
185+
186+
url = reverse(
187+
"projects-superproject",
188+
kwargs={
189+
"project_slug": self.subproject.slug,
190+
},
184191
)
192+
193+
self.client.logout()
194+
response = self.client.get(url)
195+
self.assertEqual(response.status_code, 401)
196+
197+
self.client.credentials(HTTP_AUTHORIZATION=f"Token {self.token.key}")
198+
response = self.client.get(url)
185199
self.assertEqual(response.status_code, 200)
186200

187201
self.assertDictEqual(
@@ -247,9 +261,14 @@ def test_import_project(self):
247261
"programming_language": "py",
248262
"tags": ["test tag", "template tag"],
249263
}
264+
url = reverse("projects-list")
265+
266+
self.client.logout()
267+
response = self.client.post(url, data)
268+
self.assertEqual(response.status_code, 401)
250269

251270
self.client.credentials(HTTP_AUTHORIZATION=f'Token {self.token.key}')
252-
response = self.client.post(reverse('projects-list'), data)
271+
response = self.client.post(url, data)
253272
self.assertEqual(response.status_code, 201)
254273

255274
query = Project.objects.filter(slug='test-project')
@@ -381,17 +400,19 @@ def test_update_project(self):
381400
"single_version": True,
382401
"external_builds_enabled": True,
383402
}
403+
url = reverse(
404+
"projects-detail",
405+
kwargs={
406+
"project_slug": self.project.slug,
407+
},
408+
)
409+
410+
self.client.logout()
411+
response = self.client.put(url, data)
412+
self.assertEqual(response.status_code, 401)
384413

385414
self.client.credentials(HTTP_AUTHORIZATION=f'Token {self.token.key}')
386-
response = self.client.put(
387-
reverse(
388-
'projects-detail',
389-
kwargs={
390-
'project_slug': self.project.slug,
391-
},
392-
),
393-
data,
394-
)
415+
response = self.client.put(url, data)
395416
self.assertEqual(response.status_code, 204)
396417

397418
self.project.refresh_from_db()
@@ -422,16 +443,19 @@ def test_partial_update_project(self):
422443
"tags": ["partial tags", "updated"],
423444
}
424445

425-
self.client.credentials(HTTP_AUTHORIZATION=f'Token {self.token.key}')
426-
response = self.client.patch(
427-
reverse(
428-
'projects-detail',
429-
kwargs={
430-
'project_slug': self.project.slug,
431-
},
432-
),
433-
data,
446+
url = reverse(
447+
"projects-detail",
448+
kwargs={
449+
"project_slug": self.project.slug,
450+
},
434451
)
452+
453+
self.client.logout()
454+
response = self.client.patch(url, data)
455+
self.assertEqual(response.status_code, 401)
456+
457+
self.client.credentials(HTTP_AUTHORIZATION=f"Token {self.token.key}")
458+
response = self.client.patch(url, data)
435459
self.assertEqual(response.status_code, 204)
436460

437461
self.project.refresh_from_db()

‎readthedocs/api/v3/tests/test_redirects.py

+26-20
Original file line numberDiff line numberDiff line change
@@ -170,23 +170,25 @@ def test_projects_redirects_type_sphinx_html_list_post(self):
170170

171171

172172
def test_projects_redirects_detail_put(self):
173+
url = reverse(
174+
"projects-redirects-detail",
175+
kwargs={
176+
"parent_lookup_project__slug": self.project.slug,
177+
"redirect_pk": self.redirect.pk,
178+
},
179+
)
173180
data = {
174181
'from_url': '/changed/',
175182
'to_url': '/toanother/',
176183
'type': 'page',
177184
}
178185

186+
self.client.logout()
187+
response = self.client.put(url, data)
188+
self.assertEqual(response.status_code, 401)
189+
179190
self.client.credentials(HTTP_AUTHORIZATION=f'Token {self.token.key}')
180-
response = self.client.put(
181-
reverse(
182-
'projects-redirects-detail',
183-
kwargs={
184-
'parent_lookup_project__slug': self.project.slug,
185-
'redirect_pk': self.redirect.pk,
186-
},
187-
),
188-
data,
189-
)
191+
response = self.client.put(url, data)
190192
self.assertEqual(response.status_code, 200)
191193

192194
response_json = response.json()
@@ -197,16 +199,20 @@ def test_projects_redirects_detail_put(self):
197199
)
198200

199201
def test_projects_redirects_detail_delete(self):
200-
self.assertEqual(self.project.redirects.count(), 1)
201-
self.client.credentials(HTTP_AUTHORIZATION=f'Token {self.token.key}')
202-
response = self.client.delete(
203-
reverse(
204-
'projects-redirects-detail',
205-
kwargs={
206-
'parent_lookup_project__slug': self.project.slug,
207-
'redirect_pk': self.redirect.pk,
208-
},
209-
),
202+
url = reverse(
203+
"projects-redirects-detail",
204+
kwargs={
205+
"parent_lookup_project__slug": self.project.slug,
206+
"redirect_pk": self.redirect.pk,
207+
},
210208
)
209+
210+
self.client.logout()
211+
response = self.client.delete(url)
212+
self.assertEqual(response.status_code, 401)
213+
214+
self.assertEqual(self.project.redirects.count(), 1)
215+
self.client.credentials(HTTP_AUTHORIZATION=f"Token {self.token.key}")
216+
response = self.client.delete(url)
211217
self.assertEqual(response.status_code, 204)
212218
self.assertEqual(self.project.redirects.count(), 0)

‎readthedocs/api/v3/tests/test_remoteorganizations.py

+8-4
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,14 @@ def setUp(self):
3636
)
3737

3838
def test_remote_organization_list(self):
39-
self.client.credentials(HTTP_AUTHORIZATION=f'Token {self.token.key}')
40-
response = self.client.get(
41-
reverse('remoteorganizations-list')
42-
)
39+
url = reverse("remoteorganizations-list")
40+
41+
self.client.logout()
42+
response = self.client.get(url)
43+
self.assertEqual(response.status_code, 401)
44+
45+
self.client.credentials(HTTP_AUTHORIZATION=f"Token {self.token.key}")
46+
response = self.client.get(url)
4347
self.assertEqual(response.status_code, 200)
4448

4549
self.assertDictEqual(

‎readthedocs/api/v3/tests/test_remoterepositories.py

+9-10
Original file line numberDiff line numberDiff line change
@@ -66,16 +66,15 @@ def setUp(self):
6666
)
6767

6868
def test_remote_repository_list(self):
69-
self.client.credentials(HTTP_AUTHORIZATION=f'Token {self.token.key}')
70-
response = self.client.get(
71-
reverse('remoterepositories-list'),
72-
{
73-
'expand': (
74-
'projects,'
75-
'remote_organization'
76-
)
77-
}
78-
)
69+
url = reverse("remoterepositories-list")
70+
data = {"expand": ("projects," "remote_organization")}
71+
72+
self.client.logout()
73+
response = self.client.get(url, data)
74+
self.assertEqual(response.status_code, 401)
75+
76+
self.client.credentials(HTTP_AUTHORIZATION=f"Token {self.token.key}")
77+
response = self.client.get(url, data)
7978
self.assertEqual(response.status_code, 200)
8079

8180
self.assertDictEqual(

0 commit comments

Comments
 (0)