Merge lp:~salgado/offspring/private-projects-views into lp:~linaro-automation/offspring/linaro
- private-projects-views
- Merge into linaro
Proposed by
Guilherme Salgado
Status: | Merged |
---|---|
Approved by: | James Tunnicliffe |
Approved revision: | no longer in the source branch. |
Merge reported by: | Guilherme Salgado |
Merged at revision: | not available |
Proposed branch: | lp:~salgado/offspring/private-projects-views |
Merge into: | lp:~linaro-automation/offspring/linaro |
Prerequisite: | lp:~salgado/offspring/private-projects |
Diff against target: |
387 lines (+269/-26) 5 files modified
Makefile (+6/-0) lib/offspring/web/queuemanager/tests/__init__.py (+1/-0) lib/offspring/web/queuemanager/tests/factory.py (+5/-5) lib/offspring/web/queuemanager/tests/test_views.py (+201/-0) lib/offspring/web/queuemanager/views.py (+56/-21) |
To merge this branch: | bzr merge lp:~salgado/offspring/private-projects-views |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
James Tunnicliffe (community) | Approve | ||
Review via email:
|
Commit message
Description of the change
Make all project and build views return a 404 if the user is not allowed to see that private project/build.
To post a comment you must log in.
Revision history for this message
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Guilherme Salgado (salgado) wrote : | # |
In fact this was merged into our integration branch (lp:~linaro-infrastructure/offspring/private-builds)
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | === modified file 'Makefile' |
2 | --- Makefile 2011-10-10 14:33:25 +0000 |
3 | +++ Makefile 2011-10-10 14:33:25 +0000 |
4 | @@ -36,6 +36,12 @@ |
5 | tar --directory=lib/offspring/web/media/js/ -zxf .download-cache/SmartClientRuntime-7.0.tgz |
6 | make |
7 | |
8 | +web-run: web |
9 | + ./bin/offspring-web runserver --settings=offspring.web.settings_devel |
10 | + |
11 | +web-shell: web |
12 | + ./bin/offspring-web shell --settings=offspring.web.settings_devel |
13 | + |
14 | install-test-runner: .virtualenv .download-cache |
15 | $(PIP_INSTALL) nose |
16 | |
17 | |
18 | === modified file 'lib/offspring/web/queuemanager/tests/__init__.py' |
19 | --- lib/offspring/web/queuemanager/tests/__init__.py 2011-10-10 14:33:25 +0000 |
20 | +++ lib/offspring/web/queuemanager/tests/__init__.py 2011-10-10 14:33:25 +0000 |
21 | @@ -1,1 +1,2 @@ |
22 | from .test_models import * |
23 | +from .test_views import * |
24 | |
25 | === modified file 'lib/offspring/web/queuemanager/tests/factory.py' |
26 | --- lib/offspring/web/queuemanager/tests/factory.py 2011-10-10 14:33:25 +0000 |
27 | +++ lib/offspring/web/queuemanager/tests/factory.py 2011-10-10 14:33:25 +0000 |
28 | @@ -37,10 +37,8 @@ |
29 | |
30 | def makeUser(self): |
31 | userid = password = self.getUniqueString() |
32 | - user = User.objects.create_user( |
33 | + return User.objects.create_user( |
34 | userid, self.getUniqueEmailAddress(), password) |
35 | - user.save() |
36 | - return user |
37 | |
38 | def makeProject(self, name=None, title=None, is_private=False, |
39 | project_group=None, owner=None, access_groups=[]): |
40 | @@ -68,11 +66,13 @@ |
41 | group.save() |
42 | return group |
43 | |
44 | - def makeBuildResult(self, project=None): |
45 | + def makeBuildResult(self, project=None, name=None, result=None): |
46 | + if name is None: |
47 | + name = self.getUniqueString() |
48 | if project is None: |
49 | project = self.makeProject() |
50 | project.save() |
51 | - result = BuildResult(project=project) |
52 | + result = BuildResult(name=name, project=project, result=result) |
53 | result.save() |
54 | return result |
55 | |
56 | |
57 | === added file 'lib/offspring/web/queuemanager/tests/test_views.py' |
58 | --- lib/offspring/web/queuemanager/tests/test_views.py 1970-01-01 00:00:00 +0000 |
59 | +++ lib/offspring/web/queuemanager/tests/test_views.py 2011-10-10 14:33:25 +0000 |
60 | @@ -0,0 +1,201 @@ |
61 | +from django.core.urlresolvers import reverse |
62 | +from django.contrib.auth.models import ( |
63 | + AnonymousUser, |
64 | + Group, |
65 | + Permission, |
66 | + ) |
67 | +from django.http import Http404 |
68 | +from django.test import TestCase |
69 | + |
70 | +from offspring.enums import ProjectBuildStates |
71 | +from offspring.web.queuemanager.models import ( |
72 | + Project, |
73 | + ) |
74 | +from offspring.web.queuemanager.views import get_possibly_private_object |
75 | +from offspring.web.queuemanager.tests.factory import factory |
76 | + |
77 | + |
78 | +class Test_get_possibly_private_object(TestCase): |
79 | + |
80 | + def test_public_object(self): |
81 | + obj = factory.makeProject(is_private=False) |
82 | + obj2 = get_possibly_private_object( |
83 | + AnonymousUser(), Project, pk=obj.pk) |
84 | + self.assertEqual(obj.pk, obj2.pk) |
85 | + |
86 | + def test_private_object_for_owner(self): |
87 | + obj = factory.makeProject(is_private=True) |
88 | + obj2 = get_possibly_private_object(obj.owner, Project, pk=obj.pk) |
89 | + self.assertEqual(obj.pk, obj2.pk) |
90 | + |
91 | + def test_private_object_for_user_with_no_access(self): |
92 | + obj = factory.makeProject(is_private=True) |
93 | + user = factory.makeUser() |
94 | + self.assertRaises( |
95 | + Http404, get_possibly_private_object, user, Project, pk=obj.pk) |
96 | + |
97 | + |
98 | +class ProjectViewTestsMixin(object): |
99 | + view_path = None |
100 | + |
101 | + def get_expected_page_heading(self, project): |
102 | + raise NotImplementedError() |
103 | + |
104 | + def grant_necessary_permission_to(self, user): |
105 | + pass |
106 | + |
107 | + def test_private_project_visible(self): |
108 | + project = factory.makeProject(is_private=True) |
109 | + user = project.owner |
110 | + self.grant_necessary_permission_to(user) |
111 | + self.assertTrue( |
112 | + self.client.login(username=user.username, password=user.username)) |
113 | + response = self.client.get( |
114 | + reverse(self.view_path, args=[project.name])) |
115 | + self.assertContains( |
116 | + response, self.get_expected_page_heading(project), |
117 | + status_code=200, msg_prefix=response.content) |
118 | + |
119 | + def test_private_project_invisible(self): |
120 | + project = factory.makeProject(is_private=True) |
121 | + user = factory.makeUser() |
122 | + self.grant_necessary_permission_to(user) |
123 | + self.assertTrue( |
124 | + self.client.login(username=user.username, password=user.username)) |
125 | + response = self.client.get( |
126 | + reverse(self.view_path, args=[project.name])) |
127 | + self.assertEqual(404, response.status_code) |
128 | + |
129 | + |
130 | +class ProjectDetailsViewTests(TestCase, ProjectViewTestsMixin): |
131 | + view_path = 'offspring.web.queuemanager.views.project_details' |
132 | + |
133 | + def get_expected_page_heading(self, project): |
134 | + return 'Project Details for %s' % project.title.capitalize() |
135 | + |
136 | + def test_public_project(self): |
137 | + project = factory.makeProject(is_private=False) |
138 | + response = self.client.get( |
139 | + reverse(self.view_path, args=[project.name])) |
140 | + self.assertContains( |
141 | + response, self.get_expected_page_heading(project), |
142 | + status_code=200, msg_prefix=response.content) |
143 | + |
144 | + |
145 | +class ProjectEditViewTests(TestCase, ProjectViewTestsMixin): |
146 | + view_path = 'offspring.web.queuemanager.views.project_edit' |
147 | + |
148 | + def get_expected_page_heading(self, project): |
149 | + return 'Update details for %s' % project.title.capitalize() |
150 | + |
151 | + def grant_necessary_permission_to(self, user): |
152 | + grant_permission_to_user(user, 'change_project') |
153 | + |
154 | + def test_public_project(self): |
155 | + project = factory.makeProject(is_private=False) |
156 | + user = factory.makeUser() |
157 | + self.grant_necessary_permission_to(user) |
158 | + self.assertTrue( |
159 | + self.client.login(username=user.username, password=user.username)) |
160 | + response = self.client.get( |
161 | + reverse(self.view_path, args=[project.name])) |
162 | + self.assertContains( |
163 | + response, self.get_expected_page_heading(project), |
164 | + status_code=200, msg_prefix=response.content) |
165 | + |
166 | + |
167 | +class BuildViewTests(TestCase): |
168 | + |
169 | + def test_public_build(self): |
170 | + project = factory.makeProject(is_private=False) |
171 | + build = factory.makeBuildResult(project=project) |
172 | + response = self.client.get( |
173 | + reverse('offspring.web.queuemanager.views.build_details', |
174 | + args=[project.name, build.name])) |
175 | + self.assertContains( |
176 | + response, 'Build Details', status_code=200, |
177 | + msg_prefix=response.content) |
178 | + |
179 | + def test_private_build_visible(self): |
180 | + project = factory.makeProject(is_private=True) |
181 | + build = factory.makeBuildResult(project=project) |
182 | + user = project.owner |
183 | + self.assertTrue( |
184 | + self.client.login(username=user.username, password=user.username)) |
185 | + response = self.client.get( |
186 | + reverse('offspring.web.queuemanager.views.build_details', |
187 | + args=[project.name, build.name])) |
188 | + self.assertContains( |
189 | + response, 'Build Details', status_code=200, |
190 | + msg_prefix=response.content) |
191 | + |
192 | + def test_private_build_invisible(self): |
193 | + project = factory.makeProject(is_private=True) |
194 | + build = factory.makeBuildResult(project=project) |
195 | + self.assertTrue(make_user_and_login(self.client)) |
196 | + response = self.client.get( |
197 | + reverse('offspring.web.queuemanager.views.build_details', |
198 | + args=[project.name, build.name])) |
199 | + self.assertEqual(404, response.status_code) |
200 | + |
201 | + |
202 | +class BuildPublishViewTests(TestCase): |
203 | + |
204 | + def test_public_build(self): |
205 | + project = factory.makeProject(is_private=False) |
206 | + build = factory.makeBuildResult( |
207 | + project=project, result=ProjectBuildStates.SUCCESS) |
208 | + user = factory.makeUser() |
209 | + grant_permission_to_user(user, 'add_release') |
210 | + self.assertTrue( |
211 | + self.client.login(username=user.username, password=user.username)) |
212 | + response = self.client.get( |
213 | + reverse('offspring.web.queuemanager.views.build_publish', |
214 | + args=[project.name, build.name])) |
215 | + self.assertContains( |
216 | + response, 'Release %s build' % project.title.capitalize(), |
217 | + status_code=200, msg_prefix=response.content) |
218 | + |
219 | + def test_private_build_visible(self): |
220 | + project = factory.makeProject(is_private=True) |
221 | + build = factory.makeBuildResult( |
222 | + project=project, result=ProjectBuildStates.SUCCESS) |
223 | + user = build.project.owner |
224 | + grant_permission_to_user(user, 'add_release') |
225 | + self.assertTrue( |
226 | + self.client.login(username=user.username, password=user.username)) |
227 | + response = self.client.get( |
228 | + reverse('offspring.web.queuemanager.views.build_publish', |
229 | + args=[project.name, build.name])) |
230 | + self.assertContains( |
231 | + response, 'Release %s build' % project.title.capitalize(), |
232 | + status_code=200, msg_prefix=response.content) |
233 | + |
234 | + def test_private_build_invisible(self): |
235 | + project = factory.makeProject(is_private=True) |
236 | + build = factory.makeBuildResult( |
237 | + project=project, result=ProjectBuildStates.SUCCESS) |
238 | + user = factory.makeUser() |
239 | + grant_permission_to_user(user, 'add_release') |
240 | + self.assertTrue( |
241 | + self.client.login(username=user.username, password=user.username)) |
242 | + response = self.client.get( |
243 | + reverse('offspring.web.queuemanager.views.build_publish', |
244 | + args=[project.name, build.name])) |
245 | + self.assertEqual(404, response.status_code) |
246 | + |
247 | + |
248 | +def make_user_and_login(client): |
249 | + user = factory.makeUser() |
250 | + return client.login(username=user.username, password=user.username) |
251 | + |
252 | + |
253 | +def grant_permission_to_user(user, permission): |
254 | + group = Group(name=user.username) |
255 | + group.save() |
256 | + group.permissions.add(Permission.objects.get(codename=permission)) |
257 | + group.save() |
258 | + user.groups.add(group) |
259 | + user.save() |
260 | + return user |
261 | + |
262 | |
263 | === modified file 'lib/offspring/web/queuemanager/views.py' |
264 | --- lib/offspring/web/queuemanager/views.py 2011-10-10 14:33:25 +0000 |
265 | +++ lib/offspring/web/queuemanager/views.py 2011-10-10 14:33:25 +0000 |
266 | @@ -16,12 +16,13 @@ |
267 | from django.http import ( |
268 | HttpResponse, |
269 | HttpResponseRedirect, |
270 | - HttpResponseForbidden |
271 | + HttpResponseForbidden, |
272 | + Http404, |
273 | ) |
274 | from django.middleware import csrf |
275 | from django.shortcuts import ( |
276 | render_to_response, |
277 | - get_object_or_404 |
278 | + get_object_or_404, |
279 | ) |
280 | from django.template import ( |
281 | Context, |
282 | @@ -57,6 +58,19 @@ |
283 | Release |
284 | ) |
285 | |
286 | + |
287 | +def get_possibly_private_object(user, klass, *args, **kwargs): |
288 | + """Use get_object_or_404() to return a possibly private object. |
289 | + |
290 | + If the object does not exist or the user doesn't have permission to see |
291 | + it, raise Http404. |
292 | + """ |
293 | + obj = get_object_or_404(klass, *args, **kwargs) |
294 | + if not obj.is_visible_to(user): |
295 | + raise Http404('No matches for the given query') |
296 | + return obj |
297 | + |
298 | + |
299 | #XXX: Reduce horrible duplication that occurs in these functions (mostly done). |
300 | #TODO: Thanks to reduced duplication, lets start using more generic views. |
301 | |
302 | @@ -118,8 +132,11 @@ |
303 | |
304 | |
305 | def build_details(request, projectName, buildName): |
306 | - b = get_object_or_404( |
307 | - BuildResult.objects.select_related('builder', 'project', 'requestor'), |
308 | + user_visible_builds = BuildResult.all_objects.accessible_by_user( |
309 | + request.user) |
310 | + b = get_possibly_private_object( |
311 | + request.user, |
312 | + user_visible_builds.select_related('builder', 'project', 'requestor'), |
313 | project__pk=projectName, name=buildName) |
314 | pageData = { |
315 | 'pillar' : 'builds', |
316 | @@ -134,14 +151,18 @@ |
317 | |
318 | @login_required |
319 | def build_publish(request, projectName, buildName): |
320 | - p = get_object_or_404(Project, pk=projectName) |
321 | - b = get_object_or_404( |
322 | - BuildResult, project=p, name=buildName, |
323 | + user = request.user |
324 | + p = get_possibly_private_object(user, Project, pk=projectName) |
325 | + # We use get_possibly_private_object() here just for consistency as it's |
326 | + # not really needed because the above will fail if the user doesn't have |
327 | + # the rights to see this build. |
328 | + b = get_possibly_private_object( |
329 | + user, BuildResult, project=p, name=buildName, |
330 | result=ProjectBuildStates.SUCCESS, release=None) |
331 | - if request.user.has_perm('queuemanager.add_release'): |
332 | + if user.has_perm('queuemanager.add_release'): |
333 | r = Release() |
334 | r.build = b |
335 | - r.creator = request.user |
336 | + r.creator = user |
337 | if request.method == 'POST': |
338 | f = ReleaseForm(request.POST, instance=r) |
339 | if f.is_valid(): |
340 | @@ -175,21 +196,35 @@ |
341 | project_create = permission_required('queuemanager.add_project')(project_create) |
342 | |
343 | def project_edit(request, projectName): |
344 | - return update_object( |
345 | - request, |
346 | - form_class=EditProjectForm, |
347 | - login_required=True, |
348 | - template_name='queuemanager/project_edit.html', |
349 | - template_object_name='project', |
350 | - extra_context={ 'pillar' : 'projects' }, |
351 | - object_id=projectName, |
352 | - ) |
353 | + # XXX: This nasty hack is because update_object uses (via lookup_object) |
354 | + # .objects directly when it should insted use the default manager |
355 | + # (https://code.djangoproject.com/ticket/17021). |
356 | + from django.views.generic import create_update |
357 | + orig_lookup = create_update.lookup_object |
358 | + def lookup(model, obj_id, slug, slug_field): |
359 | + return get_possibly_private_object( |
360 | + request.user, Project, pk=projectName) |
361 | + create_update.lookup_object = lookup |
362 | + try: |
363 | + response = update_object( |
364 | + request, |
365 | + form_class=EditProjectForm, |
366 | + login_required=True, |
367 | + template_name='queuemanager/project_edit.html', |
368 | + template_object_name='project', |
369 | + extra_context={ 'pillar' : 'projects' }, |
370 | + object_id=projectName, |
371 | + ) |
372 | + finally: |
373 | + create_update.lookup_object = orig_lookup |
374 | + return response |
375 | project_edit = permission_required('queuemanager.change_project')(project_edit) |
376 | |
377 | def project_details(request, projectName): |
378 | - p = get_object_or_404( |
379 | - Project.objects.select_related('project_group', 'launchpad_project'), |
380 | - pk=projectName) |
381 | + user_visible_objects = Project.all_objects.accessible_by_user( |
382 | + request.user) |
383 | + p = get_possibly_private_object( |
384 | + request.user, user_visible_objects, pk=projectName) |
385 | project_build_results = BuildResult.objects.filter( |
386 | project = p).exclude(result = None) |
387 | build_stats = { |
Looks good.