Merge lp:~salgado/offspring/private-projects-views into lp:~linaro-automation/offspring/linaro

[Guilherme Salgado]

Make all project and build views return a 404 if the user is not allowed to see that private project/build.

[James Tunnicliffe (dooferlad)]

Looks good.

review: Approve

[Guilherme Salgado (salgado)]

In fact this was merged into our integration branch (lp:~linaro-infrastructure/offspring/private-builds)

=== modified file 'Makefile'
--- Makefile	2011-10-10 14:33:25 +0000
+++ Makefile	2011-10-10 14:33:25 +0000
@@ -36,6 +36,12 @@
 	tar --directory=lib/offspring/web/media/js/ -zxf .download-cache/SmartClientRuntime-7.0.tgz
 	make
 
+web-run: web
+	./bin/offspring-web runserver --settings=offspring.web.settings_devel
+
+web-shell: web
+	./bin/offspring-web shell --settings=offspring.web.settings_devel
+
 install-test-runner: .virtualenv .download-cache
 	$(PIP_INSTALL) nose
 
 
=== modified file 'lib/offspring/web/queuemanager/tests/__init__.py'
--- lib/offspring/web/queuemanager/tests/__init__.py	2011-10-10 14:33:25 +0000
+++ lib/offspring/web/queuemanager/tests/__init__.py	2011-10-10 14:33:25 +0000
@@ -1,1 +1,2 @@
 from .test_models import *
+from .test_views import *
 
=== modified file 'lib/offspring/web/queuemanager/tests/factory.py'
--- lib/offspring/web/queuemanager/tests/factory.py	2011-10-10 14:33:25 +0000
+++ lib/offspring/web/queuemanager/tests/factory.py	2011-10-10 14:33:25 +0000
@@ -37,10 +37,8 @@
 
     def makeUser(self):
         userid = password = self.getUniqueString()
-        user = User.objects.create_user(
+        return User.objects.create_user(
             userid, self.getUniqueEmailAddress(), password)
-        user.save()
-        return user
 
     def makeProject(self, name=None, title=None, is_private=False,
                     project_group=None, owner=None, access_groups=[]):
@@ -68,11 +66,13 @@
         group.save()
         return group
 
-    def makeBuildResult(self, project=None):
+    def makeBuildResult(self, project=None, name=None, result=None):
+        if name is None:
+            name = self.getUniqueString()
         if project is None:
             project = self.makeProject()
             project.save()
-        result = BuildResult(project=project)
+        result = BuildResult(name=name, project=project, result=result)
         result.save()
         return result
 
 
=== added file 'lib/offspring/web/queuemanager/tests/test_views.py'
--- lib/offspring/web/queuemanager/tests/test_views.py	1970-01-01 00:00:00 +0000
+++ lib/offspring/web/queuemanager/tests/test_views.py	2011-10-10 14:33:25 +0000
@@ -0,0 +1,201 @@
+from django.core.urlresolvers import reverse
+from django.contrib.auth.models import (
+    AnonymousUser,
+    Group,
+    Permission,
+    )
+from django.http import Http404
+from django.test import TestCase
+
+from offspring.enums import ProjectBuildStates
+from offspring.web.queuemanager.models import (
+    Project,
+    )
+from offspring.web.queuemanager.views import get_possibly_private_object
+from offspring.web.queuemanager.tests.factory import factory
+
+
+class Test_get_possibly_private_object(TestCase):
+
+    def test_public_object(self):
+        obj = factory.makeProject(is_private=False)
+        obj2 = get_possibly_private_object(
+            AnonymousUser(), Project, pk=obj.pk)
+        self.assertEqual(obj.pk, obj2.pk)
+
+    def test_private_object_for_owner(self):
+        obj = factory.makeProject(is_private=True)
+        obj2 = get_possibly_private_object(obj.owner, Project, pk=obj.pk)
+        self.assertEqual(obj.pk, obj2.pk)
+
+    def test_private_object_for_user_with_no_access(self):
+        obj = factory.makeProject(is_private=True)
+        user = factory.makeUser()
+        self.assertRaises(
+            Http404, get_possibly_private_object, user, Project, pk=obj.pk)
+
+
+class ProjectViewTestsMixin(object):
+    view_path = None
+
+    def get_expected_page_heading(self, project):
+        raise NotImplementedError()
+
+    def grant_necessary_permission_to(self, user):
+        pass
+
+    def test_private_project_visible(self):
+        project = factory.makeProject(is_private=True)
+        user = project.owner
+        self.grant_necessary_permission_to(user)
+        self.assertTrue(
+            self.client.login(username=user.username, password=user.username))
+        response = self.client.get(
+            reverse(self.view_path, args=[project.name]))
+        self.assertContains(
+            response, self.get_expected_page_heading(project),
+            status_code=200, msg_prefix=response.content)
+
+    def test_private_project_invisible(self):
+        project = factory.makeProject(is_private=True)
+        user = factory.makeUser()
+        self.grant_necessary_permission_to(user)
+        self.assertTrue(
+            self.client.login(username=user.username, password=user.username))
+        response = self.client.get(
+            reverse(self.view_path, args=[project.name]))
+        self.assertEqual(404, response.status_code)
+
+
+class ProjectDetailsViewTests(TestCase, ProjectViewTestsMixin):
+    view_path = 'offspring.web.queuemanager.views.project_details'
+
+    def get_expected_page_heading(self, project):
+        return 'Project Details for %s' % project.title.capitalize()
+
+    def test_public_project(self):
+        project = factory.makeProject(is_private=False)
+        response = self.client.get(
+            reverse(self.view_path, args=[project.name]))
+        self.assertContains(
+            response, self.get_expected_page_heading(project),
+            status_code=200, msg_prefix=response.content)
+
+
+class ProjectEditViewTests(TestCase, ProjectViewTestsMixin):
+    view_path = 'offspring.web.queuemanager.views.project_edit'
+
+    def get_expected_page_heading(self, project):
+        return 'Update details for %s' % project.title.capitalize()
+
+    def grant_necessary_permission_to(self, user):
+        grant_permission_to_user(user, 'change_project')
+
+    def test_public_project(self):
+        project = factory.makeProject(is_private=False)
+        user = factory.makeUser()
+        self.grant_necessary_permission_to(user)
+        self.assertTrue(
+            self.client.login(username=user.username, password=user.username))
+        response = self.client.get(
+            reverse(self.view_path, args=[project.name]))
+        self.assertContains(
+            response, self.get_expected_page_heading(project),
+            status_code=200, msg_prefix=response.content)
+
+
+class BuildViewTests(TestCase):
+
+    def test_public_build(self):
+        project = factory.makeProject(is_private=False)
+        build = factory.makeBuildResult(project=project)
+        response = self.client.get(
+            reverse('offspring.web.queuemanager.views.build_details',
+                    args=[project.name, build.name]))
+        self.assertContains(
+            response, 'Build Details', status_code=200,
+            msg_prefix=response.content)
+
+    def test_private_build_visible(self):
+        project = factory.makeProject(is_private=True)
+        build = factory.makeBuildResult(project=project)
+        user = project.owner
+        self.assertTrue(
+            self.client.login(username=user.username, password=user.username))
+        response = self.client.get(
+            reverse('offspring.web.queuemanager.views.build_details',
+                    args=[project.name, build.name]))
+        self.assertContains(
+            response, 'Build Details', status_code=200,
+            msg_prefix=response.content)
+
+    def test_private_build_invisible(self):
+        project = factory.makeProject(is_private=True)
+        build = factory.makeBuildResult(project=project)
+        self.assertTrue(make_user_and_login(self.client))
+        response = self.client.get(
+            reverse('offspring.web.queuemanager.views.build_details',
+                    args=[project.name, build.name]))
+        self.assertEqual(404, response.status_code)
+
+
+class BuildPublishViewTests(TestCase):
+
+    def test_public_build(self):
+        project = factory.makeProject(is_private=False)
+        build = factory.makeBuildResult(
+            project=project, result=ProjectBuildStates.SUCCESS)
+        user = factory.makeUser()
+        grant_permission_to_user(user, 'add_release')
+        self.assertTrue(
+            self.client.login(username=user.username, password=user.username))
+        response = self.client.get(
+            reverse('offspring.web.queuemanager.views.build_publish',
+                    args=[project.name, build.name]))
+        self.assertContains(
+            response, 'Release %s build' % project.title.capitalize(),
+            status_code=200, msg_prefix=response.content)
+
+    def test_private_build_visible(self):
+        project = factory.makeProject(is_private=True)
+        build = factory.makeBuildResult(
+            project=project, result=ProjectBuildStates.SUCCESS)
+        user = build.project.owner
+        grant_permission_to_user(user, 'add_release')
+        self.assertTrue(
+            self.client.login(username=user.username, password=user.username))
+        response = self.client.get(
+            reverse('offspring.web.queuemanager.views.build_publish',
+                    args=[project.name, build.name]))
+        self.assertContains(
+            response, 'Release %s build' % project.title.capitalize(),
+            status_code=200, msg_prefix=response.content)
+
+    def test_private_build_invisible(self):
+        project = factory.makeProject(is_private=True)
+        build = factory.makeBuildResult(
+            project=project, result=ProjectBuildStates.SUCCESS)
+        user = factory.makeUser()
+        grant_permission_to_user(user, 'add_release')
+        self.assertTrue(
+            self.client.login(username=user.username, password=user.username))
+        response = self.client.get(
+            reverse('offspring.web.queuemanager.views.build_publish',
+                    args=[project.name, build.name]))
+        self.assertEqual(404, response.status_code)
+
+
+def make_user_and_login(client):
+    user = factory.makeUser()
+    return client.login(username=user.username, password=user.username)
+
+
+def grant_permission_to_user(user, permission):
+    group = Group(name=user.username)
+    group.save()
+    group.permissions.add(Permission.objects.get(codename=permission))
+    group.save()
+    user.groups.add(group)
+    user.save()
+    return user
+
 
=== modified file 'lib/offspring/web/queuemanager/views.py'
--- lib/offspring/web/queuemanager/views.py	2011-10-10 14:33:25 +0000
+++ lib/offspring/web/queuemanager/views.py	2011-10-10 14:33:25 +0000
@@ -16,12 +16,13 @@
 from django.http import (
     HttpResponse,
     HttpResponseRedirect,
-    HttpResponseForbidden
+    HttpResponseForbidden,
+    Http404,
 )
 from django.middleware import csrf
 from django.shortcuts import (
     render_to_response,
-    get_object_or_404
+    get_object_or_404,
 )
 from django.template import (
     Context,
@@ -57,6 +58,19 @@
     Release
 )
 
+
+def get_possibly_private_object(user, klass, *args, **kwargs):
+    """Use get_object_or_404() to return a possibly private object.
+
+    If the object does not exist or the user doesn't have permission to see
+    it, raise Http404.
+    """
+    obj = get_object_or_404(klass, *args, **kwargs)
+    if not obj.is_visible_to(user):
+        raise Http404('No matches for the given query')
+    return obj
+
+
 #XXX: Reduce horrible duplication that occurs in these functions (mostly done).
 #TODO: Thanks to reduced duplication, lets start using more generic views.
 
@@ -118,8 +132,11 @@
 
 
 def build_details(request, projectName, buildName):
-    b = get_object_or_404(
-        BuildResult.objects.select_related('builder', 'project', 'requestor'),
+    user_visible_builds = BuildResult.all_objects.accessible_by_user(
+        request.user)
+    b = get_possibly_private_object(
+        request.user,
+        user_visible_builds.select_related('builder', 'project', 'requestor'),
         project__pk=projectName, name=buildName)
     pageData = { 
         'pillar' : 'builds',
@@ -134,14 +151,18 @@
 
 @login_required
 def build_publish(request, projectName, buildName):
-    p = get_object_or_404(Project, pk=projectName)
-    b = get_object_or_404(
-        BuildResult, project=p, name=buildName, 
+    user = request.user
+    p = get_possibly_private_object(user, Project, pk=projectName)
+    # We use get_possibly_private_object() here just for consistency as it's
+    # not really needed because the above will fail if the user doesn't have
+    # the rights to see this build.
+    b = get_possibly_private_object(
+        user, BuildResult, project=p, name=buildName, 
         result=ProjectBuildStates.SUCCESS, release=None)
-    if request.user.has_perm('queuemanager.add_release'):
+    if user.has_perm('queuemanager.add_release'):
         r = Release()
         r.build = b
-        r.creator = request.user
+        r.creator = user
         if request.method == 'POST':
             f = ReleaseForm(request.POST, instance=r)
             if f.is_valid():
@@ -175,21 +196,35 @@
 project_create = permission_required('queuemanager.add_project')(project_create)
 
 def project_edit(request, projectName):
-    return update_object(
-        request,
-        form_class=EditProjectForm,
-        login_required=True,
-        template_name='queuemanager/project_edit.html',
-        template_object_name='project',
-        extra_context={ 'pillar' : 'projects' },
-        object_id=projectName,
-    )
+    # XXX: This nasty hack is because update_object uses (via lookup_object)
+    # .objects directly when it should insted use the default manager
+    # (https://code.djangoproject.com/ticket/17021).
+    from django.views.generic import create_update
+    orig_lookup = create_update.lookup_object
+    def lookup(model, obj_id, slug, slug_field):
+        return get_possibly_private_object(
+            request.user, Project, pk=projectName)
+    create_update.lookup_object = lookup
+    try:
+        response = update_object(
+            request,
+            form_class=EditProjectForm,
+            login_required=True,
+            template_name='queuemanager/project_edit.html',
+            template_object_name='project',
+            extra_context={ 'pillar' : 'projects' },
+            object_id=projectName,
+        )
+    finally:
+        create_update.lookup_object = orig_lookup
+    return response
 project_edit = permission_required('queuemanager.change_project')(project_edit)
 
 def project_details(request, projectName):
-    p = get_object_or_404(
-        Project.objects.select_related('project_group', 'launchpad_project'), 
-        pk=projectName)
+    user_visible_objects = Project.all_objects.accessible_by_user(
+        request.user)
+    p = get_possibly_private_object(
+        request.user, user_visible_objects, pk=projectName)
     project_build_results = BuildResult.objects.filter(
         project = p).exclude(result = None)
     build_stats = {