Merge lp:~salgado/offspring/make-piston-handlers-privacy-aware into lp:offspring

[Guilherme Salgado]

Update the piston handlers to include private objects the user is allowed to see.

[lp:~salgado/offspring/make-piston-handlers-privacy-aware]

105. By Guilherme Salgado on 2011-12-02

Plenty of docstrings for tests

[Kevin McDermott (bigkevmcd)]

Looks good with some small fixes...thanks for adding so many tests!

=== modified file 'lib/offspring/web/queuemanager/handlers.py'

#1

+ project = projects.get(pk=projectName)
+ build = BuildResult.all_objects.accessible_by_user(
+ request.user).get(project=project, name=buildName)
         except:
             return rc.NOT_FOUND

Please change this "bare except" to list the exceptions expected...there are couple of cases of this...

#2

+# TODO: Must be made privacy-aware.

Isn't that what this branch does, or is there something missing, that's not captured in the comment?

#3

+ p = Project.all_objects.accessible_by_user(request.user).get(
+ pk=projectName)

s/p/project/

#4

+from .test_handlers import *

"Relative imports for intra-package imports are highly discouraged."

In any case, merge from trunk, and you won't need to add the import, as nose will find your tests without the import (or run your tests twice if you add it ;-)

=== added file 'lib/offspring/web/queuemanager/tests/helpers.py'

#5

+from django.contrib.auth.models import (
+ Group,
+ Permission,
+ )

Will all fit on one line...

#6

+def grant_permission_to_user(user, permission):

No docstring...

#7

+def make_build(is_private):

No docstring...but, it'd be nicer if this was at the top of the file, so that you know what it does when you're reading the tests that use it?

The docstring should be fixed, but moving it in the file is a personal preference...

#8

+ """
+ Check that the builds of private projects are not visible to members
+ of the project's access group.
+ """
+ # If the user has no rights to see a given project, they'll get an
+ # empty list of builds, just like if the project didn't exist.

Wouldn't the comment be better as part of the docstring?

#9

+ def assert_build_is_visible_to_user(self, build, user):

I appreciate that this might appear to go against my PEP8 requirement, but...

"New modules and packages (including third party frameworks) should be written to these standards, but where an existing library has a different style, internal consistency is preferred."

So, this would look nicer if it was assertBuildIsVisibleToUser

Which would make it clearer that it was intended to be an extension to TestCase's assertions :-)

There are a few cases like this...

#10

assert_create_return_code

+ def assert_create_return_code(self, project, user, return_code):

How about:

+ def assertBuildHandlerCreation(self, project, user, return_code):

?

There are a couple of trivial conflicts with trunk...

review: Needs Fixing

[Kevin McDermott (bigkevmcd)]

#11

- return RC.NOT_FOUND
+ return rc.NOT_FOUND

There is no test for this change...

[Guilherme Salgado (salgado)]

On Wed, 2011-12-14 at 14:12 +0000, Kevin McDermott wrote:
> Review: Needs Fixing
>
> Looks good with some small fixes...thanks for adding so many tests!
>
> === modified file 'lib/offspring/web/queuemanager/handlers.py'
>
> #1
>
> + project = projects.get(pk=projectName)
> + build = BuildResult.all_objects.accessible_by_user(
> + request.user).get(project=project, name=buildName)
> except:
> return rc.NOT_FOUND
>
> Please change this "bare except" to list the exceptions expected...there are couple of cases of this...

Ok, fixed all of them.

> #2
>
> +# TODO: Must be made privacy-aware.
>
> Isn't that what this branch does, or is there something missing, that's not captured in the comment?

This one is a bit more involved and this branch was already big enough,
so I did it in a separate one.

> #3
>
> + p = Project.all_objects.accessible_by_user(request.user).get(
> + pk=projectName)
>
> s/p/project/

Fixed

>
> #4
>
> +from .test_handlers import *
>
> "Relative imports for intra-package imports are highly discouraged."
>
> In any case, merge from trunk, and you won't need to add the import, as nose will find your tests without the import (or run your tests twice if you add it ;-)

Oh, cool, got rid of that.

>
>
> === added file 'lib/offspring/web/queuemanager/tests/helpers.py'
>
> #5
>
> +from django.contrib.auth.models import (
> + Group,
> + Permission,
> + )
>
> Will all fit on one line...
>
> #6
>
> +def grant_permission_to_user(user, permission):
>
> No docstring...

Fixed both

>
> #7
>
> +def make_build(is_private):
>
> No docstring...but, it'd be nicer if this was at the top of the file,
> so that you know what it does when you're reading the tests that use
> it?

I prefer it at the bottom as I think one should not have to read the
implementation of helper methods to understand what a test does, and I
think in this case it's pretty clear what it does by its name alone.

>
> The docstring should be fixed, but moving it in the file is a personal
> preference...

Ok

>
> #8
>
> + """
> + Check that the builds of private projects are not visible to members
> + of the project's access group.
> + """
> + # If the user has no rights to see a given project, they'll get an
> + # empty list of builds, just like if the project didn't exist.
>
> Wouldn't the comment be better as part of the docstring?

Maybe; moved it there.

>
> #9
>
> + def assert_build_is_visible_to_user(self, build, user):
>
> I appreciate that this might appear to go against my PEP8 requirement, but...
>
> "New modules and packages (including third party frameworks) should be written to these standards, but where an existing library has a different style, internal consistency is preferred."
>
> So, this would look nicer if it was assertBuildIsVisibleToUser
>
> Which would make it clearer that it was intended to be an extension to TestCase's assertions :-)
>
> There are a few cases like this...

Makes sense; fixed them all

>
> #10
>
> assert_create_return_code
>
> + def assert_create_ret...

[Guilherme Salgado (salgado)]

> #11
>
> - return RC.NOT_FOUND
> + return rc.NOT_FOUND
>
> There is no test for this change...

Indeed, but when I tried to write one I realized it's impossible to exercise this piece of code as the code in the try block is only filtering the results so it won't raise an exception if something is not found. I've removed the try/except around the code as the real protection against not found build requests is further down and that is tested.

[lp:~salgado/offspring/make-piston-handlers-privacy-aware]

106. By Guilherme Salgado on 2011-12-14

A few changes suggested by Kevin

107. By Guilherme Salgado on 2011-12-14

merge trunk

108. By Guilherme Salgado on 2011-12-14

A few more changes suggested by Kevin

109. By Guilherme Salgado on 2011-12-14

Remove base_dir from config/offspring.cfg or else the tree root will not be used

110. By Guilherme Salgado on 2011-12-14

Fix one last thing Keving mentioned

[Kevin McDermott (bigkevmcd)]

Thanks for the fixes.

review: Approve

=== modified file 'config/offspring.cfg'
--- config/offspring.cfg	2011-11-30 14:15:52 +0000
+++ config/offspring.cfg	2011-12-14 16:08:38 +0000
@@ -7,7 +7,8 @@
 #===============================================================================
 # Offspring Global Defaults
 #
-# base_dir: Base directory of offspring install
+# base_dir: Base directory of offspring install. If ommitted, use the root of
+#     the Offspring tree.
 # bin_dir: Directory to offspring binaries
 # config_dir: Directory to offspring configuration files
 # admin_email: E-mail admin notifications should be sent to by default
@@ -21,7 +22,6 @@
 #===============================================================================
 
 [DEFAULT]
-base_dir: /srv
 bin_dir: %(base_dir)s/bin
 config_dir: %(base_dir)s/config
 log_dir: %(base_dir)s/logs
@@ -159,8 +159,8 @@
 
 [publisher]
 db: postgres://offspring:temp1234@localhost/offspring
-development_pool: /srv/builds/
-production_pool: /srv/partners/
+development_pool: %(base_dir)s/builds/
+production_pool: %(base_dir)s/partners/
 signer_directory: 
 signer_files: 
 signer_results: 

=== modified file 'lib/offspring/web/queuemanager/handlers.py'
--- lib/offspring/web/queuemanager/handlers.py	2011-08-13 07:24:06 +0000
+++ lib/offspring/web/queuemanager/handlers.py	2011-12-14 16:08:38 +0000
@@ -43,10 +43,12 @@
     allowed_methods = ('GET')
 
     def read(self, request, projectName, buildName):
+        projects = Project.all_objects.accessible_by_user(request.user)
         try:
-            project = Project.objects.get(pk=projectName)
-            build = BuildResult.objects.get(project=project, name=buildName)
-        except: 
+            project = projects.get(pk=projectName)
+            build = BuildResult.all_objects.accessible_by_user(
+                request.user).get(project=project, name=buildName)
+        except (BuildResult.DoesNotExist, Project.DoesNotExist):
             return rc.NOT_FOUND
         manifestPath = glob.glob(build.result_directory + '/images/*/*.manifest')[0]
         return [ {'package' : package, 'version' : version} for package, version in BuildManifest(manifestPath).packages.iteritems() ]
@@ -56,15 +58,18 @@
     allowed_methods = ('GET')
 
     def read(self, request, projectName, buildName):
+        projects = Project.all_objects.accessible_by_user(request.user)
+        builds = BuildResult.all_objects.accessible_by_user(request.user)
         try:
-            project = Project.objects.get(pk=projectName)
-            base_build = BuildResult.objects.get(project=project, name=buildName)
-        except:
+            project = projects.get(pk=projectName)
+            base_build = builds.get(project=project, name=buildName)
+        except (BuildResult.DoesNotExist, Project.DoesNotExist):
             return rc.NOT_FOUND
         if request.GET.has_key('build'):
             try:
-                target_build = BuildResult.objects.get(project=project, name=request.GET['build'])
-            except:
+                target_build = builds.get(
+                    project=project, name=request.GET['build'])
+            except BuildResult.DoesNotExist:
                 return rc.NOT_FOUND
         else:
             return rc.BAD_REQUEST
@@ -81,6 +86,7 @@
         return output
 
 
+# TODO: Must be made privacy-aware.
 class ProjectNotificationSubscriptionHandler(BaseHandler):
     allowed_methods = ('GET', 'POST', 'DELETE')
     model = ProjectNotificationSubscription
@@ -138,7 +144,7 @@
         'reason', 'result', 'notes', 'is_released')
 
     def read(self, request, projectName=None, buildName=None, builderName=None):
-        results = BuildResult.objects.all()
+        results = BuildResult.all_objects.accessible_by_user(request.user)
         if projectName is not None:
             results = results.filter(project__name=projectName)
         if buildName is not None:
@@ -155,11 +161,11 @@
 
     def read(self, request, projectName=None, request_id=None):
         try:
+            requests = BuildRequest.all_objects.accessible_by_user(
+                request.user)
             if projectName is not None:
-                requests = BuildRequest.objects.filter(project__name=projectName)
-            else:
-                requests = BuildRequest.objects
-        except:
+                requests = requests.filter(project__name=projectName)
+        except BuildRequest.DoesNotExist:
             return rc.NOT_FOUND
         if request_id is not None:
             return requests.get(pk=request_id)
@@ -169,18 +175,15 @@
     @method_decorator(login_required)
     def delete(self, request, projectName=None, request_id=None):
         if request.user.has_perm("queuemanager.can_build"):
-            try:
-                if projectName is not None:
-                    requests = BuildRequest.objects.filter(project__name=projectName)
-                else:
-                    requests = BuildRequest.objects
-            except:
-                return RC.NOT_FOUND
+            requests = BuildRequest.all_objects.accessible_by_user(
+                request.user)
+            if projectName is not None:
+                requests = requests.filter(project__name=projectName)
             if request_id is not None:
                 try:
                     requests.get(pk=request_id).delete()
                     return rc.DELETED
-                except:
+                except BuildRequest.DoesNotExist:
                     return rc.NOT_FOUND
             else:
                 try:
@@ -195,14 +198,18 @@
     def create(self, request, projectName):
         if request.user.has_perm("queuemanager.can_build"):
             try:
-                p = Project.objects.get(pk=projectName)
+                project = Project.all_objects.accessible_by_user(
+                    request.user).get(pk=projectName)
                 if request.POST.has_key('build_reason'):
-                    request = BuildRequest.queue_build(p, request.user, reason=request.POST['build_reason'], scoreBonus=25)
+                    reason = request.POST['build_reason']
+                    request = BuildRequest.queue_build(
+                        project, request.user, reason=reason, scoreBonus=25)
                 else:
-                    request = BuildRequest.queue_build(p, request.user, scoreBonus=25)
+                    request = BuildRequest.queue_build(
+                        project, request.user, scoreBonus=25)
                 request.save()
                 return rc.CREATED
-            except:
+            except (Project.DoesNotExist, BuildRequest.DoesNotExist):
                 return rc.BAD_REQUEST
         else:
             return rc.FORBIDDEN
@@ -217,8 +224,9 @@
         'created_at', 'published_at', 'status', 'checklist_url', 'notes')
 
     def read(self, request, projectName):
+        objects = Release.all_objects.accessible_by_user(request.user)
         try:
-            releases = Release.objects.filter(build__project__name=projectName)
-        except:
+            releases = objects.filter(build__project__name=projectName)
+        except Release.DoesNotExist:
             return rc.NOT_FOUND
         return releases

=== modified file 'lib/offspring/web/queuemanager/models.py'
--- lib/offspring/web/queuemanager/models.py	2011-12-13 08:53:34 +0000
+++ lib/offspring/web/queuemanager/models.py	2011-12-14 16:08:38 +0000
@@ -13,7 +13,6 @@
 )
 import math
 
-from django.conf import settings
 from django.contrib.auth.models import AnonymousUser, User
 from django.db import (
     connection,
@@ -24,9 +23,12 @@
     AccessGroup,
     AccessGroupMixin,
     AccessManager)
+from offspring.config import get_configuration
 from offspring.enums import ProjectBuildStates
 
 
+config = get_configuration()
+
 ARCH_CHOICES = (
     (u'i386', u'i386'),
     (u'amd64', u'amd64'),
@@ -422,7 +424,7 @@
     def result_directory(self):
         if self.name:
             try:
-                base_dir = getattr(settings, 'BUILDRESULTS_DIRECTORY', '/srv/builds/')
+                base_dir = config.get('builder', 'result_dir')
                 buildDate, buildId = self.name.rsplit('-', 1)
             except:
                 return None

=== modified file 'lib/offspring/web/queuemanager/tests/factory.py'
--- lib/offspring/web/queuemanager/tests/factory.py	2011-12-02 13:55:56 +0000
+++ lib/offspring/web/queuemanager/tests/factory.py	2011-12-14 16:08:38 +0000
@@ -73,7 +73,8 @@
             title = self.get_unique_string()
         return ProjectGroup.objects.create(name=name, title=title)
 
-    def make_build_result(self, project=None, name=None, result=None):
+    def make_build_result(self, project=None, name=None, result=None,
+                          builder=None):
         """
         Create a BuildResult with the given project or a newly created one if
         none is given.
@@ -83,7 +84,7 @@
         if project is None:
             project = self.make_project()
         return BuildResult.objects.create(
-            project=project, name=name, result=result)
+            project=project, name=name, result=result, builder=builder)
 
     def make_release(self, build=None, name=None, creator=None):
         """

=== added file 'lib/offspring/web/queuemanager/tests/helpers.py'
--- lib/offspring/web/queuemanager/tests/helpers.py	1970-01-01 00:00:00 +0000
+++ lib/offspring/web/queuemanager/tests/helpers.py	2011-12-14 16:08:38 +0000
@@ -0,0 +1,13 @@
+from django.contrib.auth.models import Group, Permission
+
+
+def grant_permission_to_user(user, permission_codename):
+    """Grant the permission with the given codename to the given user."""
+    group = Group(name=user.username)
+    group.save()
+    group.permissions.add(
+        Permission.objects.get(codename=permission_codename))
+    group.save()
+    user.groups.add(group)
+    user.save()
+    return user

=== added file 'lib/offspring/web/queuemanager/tests/test_handlers.py'
--- lib/offspring/web/queuemanager/tests/test_handlers.py	1970-01-01 00:00:00 +0000
+++ lib/offspring/web/queuemanager/tests/test_handlers.py	2011-12-14 16:08:38 +0000
@@ -0,0 +1,404 @@
+import errno
+import os
+
+from django.contrib.auth import authenticate, login
+from django.http import HttpRequest, HttpResponse
+from django.test import TestCase
+from django.contrib.sessions.middleware import SessionMiddleware
+
+from piston.utils import rc
+
+from offspring.web.queuemanager.models import BuildRequest
+from offspring.web.queuemanager.handlers import (
+    BuildManifestComparisonHandler,
+    BuildManifestHandler,
+    BuildRequestHandler,
+    BuildResultHandler,
+    ReleaseHandler,
+    )
+from offspring.web.queuemanager.tests.factory import factory
+from offspring.web.queuemanager.tests.helpers import grant_permission_to_user
+
+
+class BaseHandlerTestCase(TestCase):
+    """
+    A TestCase with some custom setup and helpers for testing Piston handlers.
+    """
+
+    def setUp(self):
+        super(BaseHandlerTestCase, self).setUp()
+        self.request = HttpRequest()
+        self.request.method = 'GET'
+        # Set request.user and process the request manually using
+        # SessionMiddleware so that we can use login(request, user) in tests.
+        self.request.user = None
+        SessionMiddleware().process_request(self.request)
+
+    def _login(self, user):
+        user = authenticate(username=user.username, password=user.username)
+        login(self.request, user)
+        return user
+
+
+class BuildResultHandlerTests(BaseHandlerTestCase):
+
+    def test_builds_for_inexistent_project(self):
+        """
+        Check that no builds are returned if there's no project with the given
+        name.
+        """
+        results = BuildResultHandler().read(
+            self.request, projectName='not-existent')
+        self.assertEqual(0, results.count())
+
+    def test_inexistent_build(self):
+        """
+        Check that no builds are returned if there's no build with the given
+        name.
+        """
+        results = BuildResultHandler().read(
+            self.request, buildName='not-existent')
+        self.assertEqual(0, results.count())
+
+    def test_public_project_builds(self):
+        """
+        Check that the builds of public projects are visible to anyone.
+        """
+        build = make_build(is_private=False)
+        self.assertBuildIsVisibleToUser(build, factory.make_user())
+
+    def test_private_project_build_is_visible_to_project_owner(self):
+        """
+        Check that the builds of private projects are visible to the project's
+        owner.
+        """
+        build = make_build(is_private=True)
+        self.assertBuildIsVisibleToUser(build, build.project.owner)
+
+    def test_private_project_builds_invisible_to_user(self):
+        """
+        Check that the builds of private projects are not visible to members
+        of the project's access group.
+
+        If the user has no rights to see a given project, they'll get an
+        empty list of builds, just like if the project didn't exist.
+        """
+        build = make_build(is_private=True)
+        self.assertBuildIsNotVisibleToUser(build, factory.make_user())
+
+    def assertBuildIsVisibleToUser(self, build, user):
+        """
+        Assert that the given build is visible to the given user.
+        """
+        user = self._login(user)
+        handler = BuildResultHandler()
+
+        # It is visible when looking at all project builds...
+        results = handler.read(self.request, projectName=build.project.name)
+        self.assertEqual(1, results.count())
+        self.assertEqual([build], list(results))
+
+        # when looking at one specific build ...
+        results = handler.read(self.request, buildName=build.name)
+        self.assertEqual(1, results.count())
+        self.assertEqual([build], list(results))
+
+        # and when looking at all builds of a builder.
+        results = handler.read(self.request, builderName=build.builder.name)
+        self.assertEqual(1, results.count())
+        self.assertEqual([build], list(results))
+
+    def assertBuildIsNotVisibleToUser(self, build, user):
+        """
+        Assert that the given build is not visible to the given user.
+        """
+        user = self._login(user)
+        handler = BuildResultHandler()
+
+        # It is *not* visible when looking at all project builds...
+        results = handler.read(self.request, projectName=build.project.name)
+        self.assertEqual(0, results.count())
+
+        # when looking at one specific build ...
+        results = handler.read(self.request, buildName=build.name)
+        self.assertEqual(0, results.count())
+
+        # and when looking at all builds of a builder.
+        results = handler.read(self.request, builderName=build.builder.name)
+        self.assertEqual(0, results.count())
+
+
+class BuildRequestHandlerTests(BaseHandlerTestCase):
+
+    def test_public_build_request_is_visible_to_anyone(self):
+        """
+        Check that build requests of public projects are visible to anyone.
+        """
+        job = factory.make_build_request(
+            project=factory.make_project(is_private=False))
+        results = BuildRequestHandler().read(
+            self.request, projectName=job.project.name, request_id=job.id)
+        self.assertEqual(job, results)
+
+    def test_private_build_request_is_visible_to_owner(self):
+        """
+        Check that build requests of private projects are visible to the
+        project's owner.
+        """
+        job = factory.make_build_request(
+            project=factory.make_project(is_private=True))
+        self._login(job.project.owner)
+        results = BuildRequestHandler().read(
+            self.request, projectName=job.project.name, request_id=job.id)
+        self.assertEqual(job, results)
+
+    def test_private_build_request_is_not_visible_to_others(self):
+        """
+        Check that build requests of private projects are not visible if the
+        user doesn't have the rights to see the project.
+        """
+        job = factory.make_build_request(
+            project=factory.make_project(is_private=True))
+        self._login(factory.make_user())
+        self.assertRaises(
+            BuildRequest.DoesNotExist,
+            BuildRequestHandler().read,
+            self.request, projectName=job.project.name, request_id=job.id)
+
+    def test_new_build_request_on_public_project(self):
+        """
+        Check that anybody can request a build of a public project.
+        """
+        project = factory.make_project(is_private=False)
+        self.assertBuildHandlerCreation(
+            project, factory.make_user(), rc.CREATED.status_code)
+
+    def test_owner_requests_new_build_on_private_project(self):
+        """
+        Check that the project owner can request the build of a private
+        project.
+        """
+        project = factory.make_project(is_private=True)
+        self.assertBuildHandlerCreation(
+            project, project.owner, rc.CREATED.status_code)
+
+    def test_other_user_requests_new_build_on_private_project(self):
+        """
+        Check that users who can't see a project cannot request a build
+        of it.
+        """
+        project = factory.make_project(is_private=True)
+        self.assertBuildHandlerCreation(
+            project, factory.make_user(), rc.BAD_REQUEST.status_code)
+
+    def test_delete_build_request_on_public_project(self):
+        """
+        Check that anybody can cancel a build request of a public project.
+        """
+        job = factory.make_build_request(
+            project=factory.make_project(is_private=False))
+        self.assertBuildHandlerDeletion(
+            job, factory.make_user(), rc.DELETED.status_code)
+
+    def test_owner_deletes_build_request_on_private_project(self):
+        """
+        Check that the project owner can cancel a build request of a private
+        project.
+        """
+        job = factory.make_build_request(
+            project=factory.make_project(is_private=True))
+        self.assertBuildHandlerDeletion(
+            job, job.project.owner, rc.DELETED.status_code)
+
+    def test_other_user_deletes_build_request_on_private_project(self):
+        """
+        Check that users who can't see a project cannot cancel a build
+        request of it.
+        """
+        job = factory.make_build_request(
+            project=factory.make_project(is_private=True))
+        self.assertBuildHandlerDeletion(
+            job, factory.make_user(), rc.NOT_FOUND.status_code)
+
+    def assertBuildHandlerCreation(self, project, user, return_code):
+        """
+        Assert that BuildRequestHandler.create() returns the given code.
+
+        Also grant the given user the 'can_build' permission and logs it in.
+        """
+        grant_permission_to_user(user, 'can_build')
+        self._login(user)
+        result = BuildRequestHandler().create(
+            self.request, projectName=project.name)
+        self.assertEqual(return_code, result.status_code)
+
+    def assertBuildHandlerDeletion(self, job, user, return_code):
+        """
+        Assert that BuildRequestHandler.delete() returns the given code.
+
+        Also grant the given user the 'can_build' permission and logs it in.
+        """
+        grant_permission_to_user(user, 'can_build')
+        self._login(user)
+        result = BuildRequestHandler().delete(
+            self.request, projectName=job.project.name, request_id=job.id)
+        self.assertEqual(return_code, result.status_code)
+
+
+class ReleaseHandlerTests(BaseHandlerTestCase):
+
+    def test_public_project_release(self):
+        """
+        Check that releases of public projects are visible to anyone.
+        """
+        release = factory.make_release(make_build(is_private=False))
+        results = ReleaseHandler().read(
+            self.request, projectName=release.build.project.name)
+        self.assertEqual(1, results.count())
+        self.assertEqual([release], list(results))
+
+    def test_private_project_release_is_visible_to_owner(self):
+        """
+        Check that releases of private projects are visible to the project's
+        owner.
+        """
+        release = factory.make_release(make_build(is_private=True))
+        self._login(release.build.project.owner)
+        results = ReleaseHandler().read(
+            self.request, projectName=release.build.project.name)
+        self.assertEqual(1, results.count())
+        self.assertEqual([release], list(results))
+
+    def test_private_project_release_is_not_visible_to_others(self):
+        """
+        Check that releases of private projects are not visible to users who
+        don't have the rights to see the project.
+        """
+        release = factory.make_release(make_build(is_private=True))
+        user = self._login(factory.make_user())
+        self.assertFalse(release.is_visible_to(user))
+        results = ReleaseHandler().read(
+            self.request, projectName=release.build.project.name)
+        self.assertEqual(0, results.count())
+
+
+class BuildManifestHandlerTests(BaseHandlerTestCase):
+
+    def test_public_project_build_manifest(self):
+        """
+        Check that build manifests of public projects are visible to anyone.
+        """
+        build = make_build(is_private=False)
+        make_build_manifest_file(build)
+        results = BuildManifestHandler().read(
+            self.request, build.project.name, build.name)
+        self.assertEqual([{'package': 'foo', 'version': '1.2'}], results)
+
+    def test_private_project_build_manifest_is_visible_to_owner(self):
+        """
+        Check that build manifests of private projects are visible to the
+        project's owner.
+        """
+        build = make_build(is_private=True)
+        make_build_manifest_file(build)
+        self._login(build.project.owner)
+        results = BuildManifestHandler().read(
+            self.request, build.project.name, build.name)
+        self.assertEqual([{'package': 'foo', 'version': '1.2'}], results)
+
+    def test_private_project_build_manifest_returns_404_for_others(self):
+        """
+        Check that build manifests of private projects are not visible to
+        users who don't have the rights to see the project.
+        """
+        build = make_build(is_private=True)
+        make_build_manifest_file(build)
+        user = self._login(factory.make_user())
+        self.assertFalse(build.project.is_visible_to(user))
+        results = BuildManifestHandler().read(
+            self.request, build.project.name, build.name)
+        self.assertTrue(isinstance(results, HttpResponse))
+        self.assertEqual(404, results.status_code)
+
+
+class BuildManifestComparisonHandlerTests(BaseHandlerTestCase):
+
+    def test_public_projects_build_manifest_comparison(self):
+        """
+        Check that anyone can get a build comparison between builds of a
+        public project.
+        """
+        build = make_build(is_private=False)
+        build2 = factory.make_build_result(
+            project=build.project, builder=factory.make_lexbuilder())
+        make_build_manifest_file(build)
+        make_build_manifest_file(build2)
+        self.request.GET['build'] = build2.name
+        results = BuildManifestComparisonHandler().read(
+            self.request, build.project.name, build.name)
+        self.assertEqual(
+            [{'changeType': 'no_change', 'target_version': '1.2',
+              'base_version': '1.2', 'package': 'foo'}],
+            results)
+
+    def test_private_build_manifest_comparison_is_visible_to_owner(self):
+        """
+        Check that the project owner can get a build comparison between builds
+        of a private project.
+        """
+        build = make_build(is_private=True)
+        build2 = factory.make_build_result(
+            project=build.project, builder=factory.make_lexbuilder())
+        make_build_manifest_file(build)
+        make_build_manifest_file(build2)
+        self._login(build.project.owner)
+        self.request.GET['build'] = build2.name
+        results = BuildManifestComparisonHandler().read(
+            self.request, build.project.name, build.name)
+        self.assertEqual(
+            [{'changeType': 'no_change', 'target_version': '1.2',
+              'base_version': '1.2', 'package': 'foo'}],
+            results)
+
+    def test_private_build_manifest_comparison_returns_404_for_others(self):
+        """
+        Check that those who don't have the rights to see a private project
+        cannot get a build comparison between builds of that project.
+        """
+        build = make_build(is_private=True)
+        build2 = factory.make_build_result(
+            project=build.project, builder=factory.make_lexbuilder())
+        make_build_manifest_file(build)
+        make_build_manifest_file(build2)
+        self._login(factory.make_user())
+        self.request.GET['build'] = build2.name
+        results = BuildManifestComparisonHandler().read(
+            self.request, build.project.name, build.name)
+        self.assertTrue(isinstance(results, HttpResponse))
+        self.assertEqual(404, results.status_code)
+
+
+def make_build(is_private):
+    """Create a new BuildResult associated to a newly created project.
+
+    If is_private is True, the project is marked as private, otherwise it is
+    marked as public.
+    """
+    return factory.make_build_result(
+        project=factory.make_project(is_private=is_private),
+        builder=factory.make_lexbuilder())
+
+
+def make_build_manifest_file(build):
+    """
+    Create a manifest file where BuildManifestHandler expects to find it.
+    """
+    manifest_dir = os.path.join(build.result_directory, 'images', 'build')
+    try:
+        os.makedirs(manifest_dir)
+    except OSError, exc:
+        if exc.errno != errno.EEXIST:
+            raise
+    path = os.path.join(manifest_dir, 'build.manifest')
+    with open(path, 'w') as fd:
+        fd.write('foo 1.2')

=== modified file 'lib/offspring/web/queuemanager/tests/test_views.py'
--- lib/offspring/web/queuemanager/tests/test_views.py	2011-12-14 12:13:02 +0000
+++ lib/offspring/web/queuemanager/tests/test_views.py	2011-12-14 16:08:38 +0000
@@ -2,8 +2,7 @@
 # GNU Affero General Public License version 3 (see the file LICENSE).
 import urllib
 
-from django.contrib.auth.models import AnonymousUser, Group, Permission
-from django.contrib.contenttypes.models import ContentType
+from django.contrib.auth.models import AnonymousUser
 from django.core.urlresolvers import reverse
 from django.http import Http404
 from django.test import TestCase
@@ -15,6 +14,7 @@
 from offspring.web.queuemanager.forms import LaunchpadProjectForm
 
 from offspring.web.queuemanager.tests.factory import factory
+from offspring.web.queuemanager.tests.helpers import grant_permission_to_user
 
 
 class TestGetPossiblyPrivateObject(TestCase):
@@ -417,18 +417,6 @@
     return client.login(username=user.username, password=user.username)
 
 
-def grant_permission_to_user(user, permission_codename):
-    """Grant the permission with the given codename to the given user."""
-    group = Group(name=user.username)
-    group.save()
-    group.permissions.add(
-        Permission.objects.get(codename=permission_codename))
-    group.save()
-    user.groups.add(group)
-    user.save()
-    return user
-
-
 class LaunchpadProjectViewsTests(TestCase):
 
     def setUp(self):
@@ -451,8 +439,8 @@
 
     def test_get_launchpad_project_add_url_gets_correct_form(self):
         """
-        If the logged in user has the correct permissions, a GET request should
-        get the LaunchpadProjectForm.
+        If the logged in user has the correct permissions, a GET request
+        should get the LaunchpadProjectForm.
         """
         grant_permission_to_user(self.user, "add_launchpadproject")
         self.client.login(username=self.user.username,

=== modified file 'lib/offspring/web/settings.py'
--- lib/offspring/web/settings.py	2011-11-18 15:45:06 +0000
+++ lib/offspring/web/settings.py	2011-12-14 16:08:38 +0000
@@ -131,6 +131,3 @@
 CELERY_RESULT_BACKEND = 'djcelery.backends.database.DatabaseBackend'
 CELERYBEAT_SCHEDULER = 'djcelery.schedulers.DatabaseScheduler'
 djcelery.setup_loader()
-
-# Path to build results
-BUILDRESULTS_DIRECTORY = '/srv/builds/'

=== modified file 'lib/offspring/web/settings_production.py'
--- lib/offspring/web/settings_production.py	2011-11-30 14:15:52 +0000
+++ lib/offspring/web/settings_production.py	2011-12-14 16:08:38 +0000
@@ -16,6 +16,3 @@
 EMAIL_HOST = 'localhost'
 
 MEDIA_URL = 'https://offspring.com/assets/'
-
-# Path to build results
-BUILDRESULTS_DIRECTORY = '/srv/offspring.com/www/builds/'