Merge lp:~salgado/offspring/private-projects into lp:offspring

[Guilherme Salgado]

Add support for private projects on models of web.queuemanager

It adds a ._is_private attribute to Project, together with a few helper methods to decide who's able to see a given private project and to query only Projects a given user is allowed to see. This also works for classes that have a foreign key (direct or indirect) to Project, as we defer the checks to the class that provides the ._is_private attribute.

The default model manager (.objects) now returns only public objects, to reduce the risk of leaking private data. Subsequent branches will update all the callsites to use the privacy-aware API (.all_objects.accessible_by_user(user)) to lookup objects. This code, together with the callsite updates, is already running on offspring.linaro.org

In order to try this you'll need to get http://pypi.python.org/pypi/django-group-access and place it on your .download-cache/

[lp:~salgado/offspring/private-projects]

48. By Guilherme Salgado on 2011-11-30

merge trunk

[Kevin McDermott (bigkevmcd)]

#1

+BUILDRESULTS_DIRECTORY = '/srv/offspring.com/www/builds/'

Is this the right path?

#2

Where is the migration used?

#3

+ def test_get_projects_filters_private_objects(self):
No docstring.

#4

+class ReleaseTests(
+ TestCase, ReleaseOrLexbuilderOrBuildResultOrBuildRequestTestsMixin):
+ model = Release
+
+ def factoryMethod(self, project):
+ return factory.make_release(
+ build=factory.make_build_result(project=project))
+
+
+class LexbuilderTests(
+ TestCase, ReleaseOrLexbuilderOrBuildResultOrBuildRequestTestsMixin):
+ model = Lexbuilder
+
+ def factoryMethod(self, project):
+ return factory.make_lexbuilder(
+ current_job=factory.make_build_result(project=project))
+
+
+class BuildResultTests(
+ TestCase, ReleaseOrLexbuilderOrBuildResultOrBuildRequestTestsMixin):
+ model = BuildResult
+ factoryMethod = factory.make_build_result
+
+
+class BuildRequestTests(
+ TestCase, ReleaseOrLexbuilderOrBuildResultOrBuildRequestTestsMixin):
+ model = BuildRequest
+ factoryMethod = factory.make_build_request

Couldn't this be done as a parent class, something like BasePrivateModelTestCase or something?

#5

+class ProjectTests(TestCase):

There are no docstrings explaining any of the tests...

#6

+from .test_models import *

Any idea why we actually need this? (I know we do, but is there a way we can drop it?) My concern is that missing tests from there won't necessarily affect localised test runs, but could be missed from just running test-web.

Also...see PEP8

    - Relative imports for intra-package imports are highly discouraged.
      Always use the absolute package path for all imports.
      Even now that PEP 328 [7] is fully implemented in Python 2.5,
      its style of explicit relative imports is actively discouraged;
      absolute imports are more portable and usually more readable.

#7

=== added directory 'lib/offspring/web/queuemanager/sql'
=== added file 'lib/offspring/web/queuemanager/sql/project.postgresql_psycopg2.sql'

What are these used for? Is there a speed benefit from using sqlite in tests?

Personally, I'd prefer to run the tests against the database we use in production i.e. postgresql and not have SQL files in separate directories for this purpose...

#8

+class Release(models.Model, MaybePrivateMixin):
+class BuildResult(models.Model, MaybePrivateMixin):
+class Lexbuilder(models.Model, MaybePrivateMixin):

+ # Using PublicOnlyObjectsManager here we ensure that any oversights when
+ # updating existing uses of Release.objects won't leak anything related to
+ # private projects.

How about defining a base class for these MaybePrivateMixin objects rather than redeclaring he same thing and comments each time, and just subclass for the various models that use it?

#9

+ hostname = os.popen('hostname').read().strip()
+ config.set('DEFAULT', 'hostname', hostname)
+ config.set('DEFAULT', 'username', os.environ['USERNAME'])

I reimplemented this in add-path-independence, but note that you can use os.uname to get the hostname...

#10

+base_dir: /srv

-development_pool: %(base_dir)s/builds/
-production_pool: %...

review: Approve

[Kevin McDermott (bigkevmcd)]

review: Needs Fixing

[Guilherme Salgado (salgado)]

On Wed, 2011-11-30 at 14:57 +0000, Kevin McDermott wrote:
> Review: Approve
>
> #1
>
> +BUILDRESULTS_DIRECTORY = '/srv/offspring.com/www/builds/'
>
> Is this the right path?

This is part of path-independence; I've already removed it from this
branch but it looks like you merged an old revision when you reviewed.

>
> #2
>
> Where is the migration used?

It's not; Offspring doesn't use south (Cody had concerns with it) so the
migration scripts have to be applied manually.

>
> #3
>
> + def test_get_projects_filters_private_objects(self):
> No docstring.

Done

> #4
>
> +class ReleaseTests(
> + TestCase,
> ReleaseOrLexbuilderOrBuildResultOrBuildRequestTestsMixin):
> + model = Release
> +
> + def factoryMethod(self, project):
> + return factory.make_release(
> + build=factory.make_build_result(project=project))
> +
> +
> +class LexbuilderTests(
> + TestCase,
> ReleaseOrLexbuilderOrBuildResultOrBuildRequestTestsMixin):
> + model = Lexbuilder
> +
> + def factoryMethod(self, project):
> + return factory.make_lexbuilder(
> + current_job=factory.make_build_result(project=project))
> +
> +
> +class BuildResultTests(
> + TestCase,
> ReleaseOrLexbuilderOrBuildResultOrBuildRequestTestsMixin):
> + model = BuildResult
> + factoryMethod = factory.make_build_result
> +
> +
> +class BuildRequestTests(
> + TestCase,
> ReleaseOrLexbuilderOrBuildResultOrBuildRequestTestsMixin):
> + model = BuildRequest
> + factoryMethod = factory.make_build_request
>
> Couldn't this be done as a parent class, something like
> BasePrivateModelTestCase or something?

I'm not sure I understand. What would we put in the parent class? Both
the model and the factory method are different on every class and they
already share a parent class which has the actual tests.

>
> #5
>
> +class ProjectTests(TestCase):
>
> There are no docstrings explaining any of the tests...

I'm not convinced using docstrings in test methods is a good idea as
nobody is going to be using these tests methods (i.e. they're not an
API), but when I have a test which is not trivial I write some comments
around the interesting lines. I do that for a couple tests here, but I
don't think the others need any comments as they'd just be the test's
name with s/_/ /

> #6
>
> +from .test_models import *
>
> Any idea why we actually need this? (I know we do, but is there a way
> we can drop it?) My concern is that missing tests from there won't
> necessarily affect localised test runs, but could be missed from just
> running test-web.

Django requires it.

> Also...see PEP8
>
> - Relative imports for intra-package imports are highly
> discouraged.
> Always use the absolute package path for all imports.
> Even now that PEP 328 [7] is fully implemented in Python 2.5,
> its style of explicit relative imports is actively discouraged;
> absolute imports are more portable and usually more readable.

Ok, changed.

>
> #7
>
> === added directory 'lib/offspring/web/queuemanager/sql'
> === added file
> 'lib/offspring/web/queuemanager/sql/project.postgresql_psycopg2.sql'
>
> What are these used for? I...

[Kevin McDermott (bigkevmcd)]

> Where is the migration used?
>
> It's not; Offspring doesn't use south (Cody had concerns with it) so the
> migration scripts have to be applied manually.

Ok, I think we should definitely use South for migrations, it's in use successfully elsewhere in Canonical. Unless there are good reasons for not using it, then we can migrate that way over time.

At that point, we can fix this migration :-)

> >
> > Couldn't this be done as a parent class, something like
> > BasePrivateModelTestCase or something?
>
> I'm not sure I understand. What would we put in the parent class? Both
> the model and the factory method are different on every class and they
> already share a parent class which has the actual tests.

After having read more on Nose's test finding (which I don't particularly like), it seems that this would be doomed to failure because it finds all TestCase subclasses, so it wouldn't work :-(

>
> I'm not convinced using docstrings in test methods is a good idea as
> nobody is going to be using these tests methods (i.e. they're not an
> API), but when I have a test which is not trivial I write some comments
> around the interesting lines. I do that for a couple tests here, but I
> don't think the others need any comments as they'd just be the test's
> name with s/_/ /

Instead of comments, these should be docstrings...

But, the problem with tests without docstrings is that they don't convey anything of what they're actually doing.

def test_myfunction(self):
    self.assertEqual(7, my_function(1, 1))

Doesn't mean anything, but if the test passes, it's fine?

> Django requires it.

Why does Django require it? We removed it from Hexr recently and the tests still run :-)

> >
> > #7
> >
> > === added directory 'lib/offspring/web/queuemanager/sql'
> > === added file
> > 'lib/offspring/web/queuemanager/sql/project.postgresql_psycopg2.sql'
> >
> > What are these used for? Is there a speed benefit from using sqlite
> > in tests?
>
> Yes, it's incredibly faster to run using sqlite3, but there's also a
> test-web-postgres target to run the test-suite using postgres.
>
Sure, I see that, what a difference sqlite3 makes.

Ok, so is there some way we can deal with this in either South or in Django itself, so that it's not just in an SQL file?

This doesn't have to be fixed right away, but having bits of SQL lying around that may or may not be run isn't good...

> That's tricky because the Managers have to be declared on a models.Model
> instance and if I make MaybePrivateMixin a subclass of that, Project
> will end up inheriting from it twice: via MaybePrivateMixin and
> AccessGroupMixin, and that seems to case some weird issues. I'm sure I
> could create two separate mixins to workaround this but then I'm not
> sure it's worth it anymore.

Sure, I just hate to see duplication of code...maybe we can come up with something in a future refactoring...

> That was done in another branch, so I'd rather not do it here as it's
> not related to those changes I'm doing and would cause yet another
> conflict later on when I merge this back into the linaro branch.

Ok...seems reasonable...we do need to make sure we don't get caught by the Django/psyco...

[lp:~salgado/offspring/private-projects]

49. By Guilherme Salgado on 2011-12-01

Lots of docstrings for tests

[Kevin McDermott (bigkevmcd)]

Thanks for adding the docstrings...

pep8 throws a fit and lint chucks out at least one likely error :-)

But they're not in your branch, so let's not introduce any more noise :-)

review: Approve

=== modified file 'Makefile'
--- Makefile	2011-09-21 22:44:51 +0000
+++ Makefile	2011-12-01 15:15:28 +0000
@@ -46,7 +46,10 @@
 	./.virtualenv/bin/nosetests offspring.master
 
 test-web: web install-test-runner
-	./bin/offspring-web test --settings=offspring.web.settings_test
+	./bin/offspring-web test queuemanager --settings=offspring.web.settings_test
+
+test-web-postgres: web install-test-runner
+	./bin/offspring-web test queuemanager --settings=offspring.web.settings_devel
 
 test: master slave web install-test-runner test-web
 	./.virtualenv/bin/nosetests -e 'queuemanager.*'

=== modified file 'lib/offspring/web/queuemanager/models.py'
--- lib/offspring/web/queuemanager/models.py	2011-09-27 18:42:21 +0000
+++ lib/offspring/web/queuemanager/models.py	2011-12-01 15:15:28 +0000
@@ -13,12 +13,16 @@
 import math
 
 from django.conf import settings
-from django.contrib.auth.models import User
+from django.contrib.auth.models import AnonymousUser, User
 from django.db import (
     connection,
     models
 )
 
+from django_group_access.models import (
+    AccessGroup,
+    AccessGroupMixin,
+    AccessManager)
 from offspring.enums import ProjectBuildStates
 
 
@@ -73,9 +77,9 @@
     def get_absolute_url(self):
         return "/project-groups/%s/" % self.name
 
-    @property
-    def projects(self):
-        return Project.objects.filter(project_group=self)
+    def get_projects(self, user):
+        return Project.all_objects.accessible_by_user(user).filter(
+            project_group=self)
 
     @property
     def builder(self): 
@@ -83,11 +87,10 @@
 
     @property
     def is_active(self):
-        return bool(Project.objects.filter(project_group=self).exclude(is_active=False).count() != 0)
-
-    @property
-    def needs_build(self):
-        return BuildRequest.objects.filter(project__project_group=self).exists()
+        # Can safely use .all_objects here because we're not returning any
+        # objects so there's no risk of leaking private stuff.
+        projects = Project.all_objects.filter(project_group=self)
+        return projects.exclude(is_active=False).count() > 0
 
     @property
     def average_build_time(self):
@@ -111,7 +114,81 @@
         return "UNKNOWN"
 
 
-class Project(models.Model):
+class PublicOnlyObjectsManager(models.Manager):
+    """A model manager which only returns public objects."""
+
+    def get_query_set(self):
+        return super(PublicOnlyObjectsManager, self).get_query_set().filter(
+            models.Q(**{self.model._get_privacy_attr(): False}))
+
+
+class AccessControlledObjectsManager(AccessManager):
+    """A model manager which provides an extra API for querying objects a
+    given user is allowed to see.
+
+    We extend AccessManager because we want the access-control filters to be
+    applied only for private objects, but also because we allow AnonymousUsers
+    to be passed to accessible_by_user().
+    """
+
+    def _get_accessible_by_user_filter_rules(self, user):
+        rules = super(AccessControlledObjectsManager,
+                      self)._get_accessible_by_user_filter_rules(user)
+        return rules | models.Q(**{self.model._get_privacy_attr(): False})
+
+    def accessible_by_user(self, user):
+        if isinstance(user, AnonymousUser):
+            # If we're passed in an AnonymousUser we must not attempt to apply
+            # the usual constraints because they rely on the given user
+            # existing in the DB and that's not the case with AnonymousUsers.
+            query = models.Q(**{self.model._get_privacy_attr(): False})
+            return super(
+                AccessControlledObjectsManager, self).get_query_set().filter(
+                    query)
+        else:
+            return super(
+                AccessControlledObjectsManager, self).accessible_by_user(user)
+
+
+class MaybePrivateMixin(object):
+    """A model which may be private or linked to a private one.
+
+    If the model itself is private, it's expected to have an '_is_private'
+    boolean attribute. Otherwise it must have an 'access_relation' attribute
+    pointing to the linked model which has the '_is_private' attribute.
+    """
+    _privacy_attr_name = '_is_private'
+    _is_visible_method_name = '_is_visible_to'
+
+    @classmethod
+    def _get_privacy_attr(self):
+        privacy_attr = self._privacy_attr_name
+        if getattr(self, 'access_relation', None) is not None:
+            privacy_attr = self.access_relation + '__' + privacy_attr
+        return privacy_attr
+
+    def _get_access_controlled_object(self):
+        if getattr(self, 'access_relation', None) is None:
+            return self
+        names = self.access_relation.split('__')
+        obj = self
+        while len(names):
+            name = names.pop(0)
+            obj = getattr(obj, name)
+        return obj
+
+    @property
+    def is_private(self):
+        obj = self._get_access_controlled_object()
+        return getattr(obj, self._privacy_attr_name)
+
+    def is_visible_to(self, user):
+        is_visible_to = getattr(self._get_access_controlled_object(),
+                                self._is_visible_method_name)
+        return is_visible_to(user)
+
+
+class Project(AccessGroupMixin, MaybePrivateMixin):
     #XXX: This should probably be managed in the database.
     STATUS_CHOICES = (
         (u'devel', u'Active Development'),
@@ -121,7 +198,15 @@
         (u'experimental', u'Experimental'),
         (u'obsolete', u'Obsolete'),
     )
+    all_objects = AccessControlledObjectsManager()
+    # Using PublicOnlyObjectsManager here we ensure that any oversights when
+    # updating existing uses of Project.objects won't leak private projects.
+    objects = PublicOnlyObjectsManager()
 
+    _is_private = models.BooleanField(default=False, db_column='is_private')
+    # We allow projects without an owner for backwards compatibility but
+    # there's a DB constraint to ensure private projects always have an owner.
+    owner = models.ForeignKey(User, blank=True, null=True)
     name = models.SlugField('codename', max_length=200, primary_key=True, unique=True)
     title = models.CharField('project title', max_length=30)
     project_group = models.ForeignKey(ProjectGroup, blank=True, null=True)
@@ -144,6 +229,16 @@
     def __unicode__(self):
         return self.display_name()
 
+    def _is_visible_to(self, user):
+        if not self._is_private or user == self.owner:
+            return True
+        if isinstance(user, AnonymousUser):
+            # Private objects are never visible to anonymous users.
+            return False
+        access_groups = set(self.access_groups.all())
+        user_groups = AccessGroup.objects.filter(members=user)
+        return len(access_groups.intersection(user_groups)) > 0
+
     def get_absolute_url(self):
         return "/projects/%s/" % self.name
 
@@ -191,7 +286,7 @@
             return u""
 
 
-class Lexbuilder(models.Model):
+class Lexbuilder(models.Model, MaybePrivateMixin):
     name = models.SlugField(max_length=200, unique=True)
     uri = models.CharField(max_length=200)
     current_state = models.CharField(max_length=200, default="UNKNOWN", editable=False)
@@ -204,6 +299,13 @@
     current_job = models.ForeignKey("BuildResult", db_column="current_job_id", editable=False, blank=True, null=True)
     notes = models.TextField('whiteboard', blank=True, null=True)
 
+    access_relation = 'current_job__project'
+    all_objects = AccessControlledObjectsManager()
+    # Using PublicOnlyObjectsManager here we ensure that any oversights when
+    # updating existing uses of Lexbuilder.objects won't leak anything related
+    # to private projects.
+    objects = PublicOnlyObjectsManager()
+
     class Meta:
         db_table = "lexbuilders"
 
@@ -265,7 +367,7 @@
         return cursor.fetchone()[0]
 
 
-class BuildResult(models.Model):
+class BuildResult(models.Model, MaybePrivateMixin):
     name = models.CharField('build name', max_length=200, blank=True, null=True, editable=False)
     project = models.ForeignKey(Project, db_column="project_name", editable=False)
     builder = models.ForeignKey(Lexbuilder, blank=True, null=True, editable=False)
@@ -278,6 +380,13 @@
     dispatched_at = models.DateTimeField('date dispatched', editable=False, null=True, blank=True) 
     notes = models.CharField(max_length=200, null=True, blank=True)
 
+    access_relation = 'project'
+    all_objects = AccessControlledObjectsManager()
+    # Using PublicOnlyObjectsManager here we ensure that any oversights when
+    # updating existing uses of BuildResult.objects won't leak anything
+    # related to private projects.
+    objects = PublicOnlyObjectsManager()
+
     class Meta:
         db_table = "buildresults"
         get_latest_by = "started_at"
@@ -325,13 +434,20 @@
     recentBuildFailures = staticmethod(recentBuildFailures)
 
 
-class BuildRequest(models.Model):
+class BuildRequest(models.Model, MaybePrivateMixin):
     project = models.ForeignKey(Project, db_column="project_name")
     requestor = models.ForeignKey(User, db_column="requestor_id", blank=True, null=True)
     created_at = models.DateTimeField('date created', auto_now_add=True)
     score = models.IntegerField(default=10)
     reason = models.TextField('request reason', blank=True, max_length=200)
 
+    access_relation = 'project'
+    all_objects = AccessControlledObjectsManager()
+    # Using PublicOnlyObjectsManager here we ensure that any oversights when
+    # updating existing uses of BuildRequest.objects won't leak anything
+    # related to private projects.
+    objects = PublicOnlyObjectsManager()
+
     class Meta:
         db_table = "buildrequests"
 
@@ -450,7 +566,7 @@
         return "%s subscribed to %s" % (self.user, self.project)
 
 
-class Release(models.Model):
+class Release(models.Model, MaybePrivateMixin):
     STATUS_PENDING, STATUS_PUBLISHED, STATUS_REMOVED, STATUS_MISSING, STATUS_ERROR = range(5)
     STATUS_CHOICES = (
         (STATUS_PENDING, 'Pending Publication'),
@@ -468,6 +584,13 @@
         ("GM", "Gold Master"),
     )
 
+    access_relation = 'build__project'
+    all_objects = AccessControlledObjectsManager()
+    # Using PublicOnlyObjectsManager here we ensure that any oversights when
+    # updating existing uses of Release.objects won't leak anything related to
+    # private projects.
+    objects = PublicOnlyObjectsManager()
+
     build = models.OneToOneField(BuildResult, primary_key=True, unique=True)
     name = models.CharField('release name', max_length=200)
     milestone = models.ForeignKey(LaunchpadProjectMilestone, null=True, blank=True)

=== added directory 'lib/offspring/web/queuemanager/sql'
=== added file 'lib/offspring/web/queuemanager/sql/project.postgresql_psycopg2.sql'
--- lib/offspring/web/queuemanager/sql/project.postgresql_psycopg2.sql	1970-01-01 00:00:00 +0000
+++ lib/offspring/web/queuemanager/sql/project.postgresql_psycopg2.sql	2011-12-01 15:15:28 +0000
@@ -0,0 +1,5 @@
+-- XXX: This doesn't apply when running tests because sqlite3 doesn't support
+-- addition of new constraints to existing tables.  Need to decide whether
+-- it's preferred to just ignore this in our tests or use postgresql to run
+-- the tests as well.
+ALTER TABLE projects ADD CONSTRAINT "private_project_requires_owner" CHECK (is_private IS FALSE OR owner_id IS NOT NULL);

=== modified file 'lib/offspring/web/queuemanager/tests/__init__.py'
--- lib/offspring/web/queuemanager/tests/__init__.py	2011-11-30 14:01:37 +0000
+++ lib/offspring/web/queuemanager/tests/__init__.py	2011-12-01 15:15:28 +0000
@@ -0,0 +1,1 @@
+from offspring.web.queuemanager.tests.test_models import *

=== modified file 'lib/offspring/web/queuemanager/tests/factory.py'
--- lib/offspring/web/queuemanager/tests/factory.py	2011-11-30 14:01:37 +0000
+++ lib/offspring/web/queuemanager/tests/factory.py	2011-12-01 15:15:28 +0000
@@ -1,6 +1,7 @@
 from itertools import count
 
 from django.contrib.auth.models import User
+from django_group_access.models import AccessGroup
 
 from offspring.web.queuemanager.models import (
     BuildRequest,
@@ -41,7 +42,8 @@
         return User.objects.create_user(
             userid, self.get_unique_email_address(), password)
 
-    def make_project(self, name=None, title=None):
+    def make_project(self, name=None, title=None, is_private=False,
+                     project_group=None, owner=None, access_groups=[]):
         """
         Create a Project with the given name and title. If any of those is not
         given we use an arbitrary value.
@@ -50,7 +52,15 @@
             name = self.get_unique_string()
         if title is None:
             title = self.get_unique_string()
-        return Project.objects.create(name=name, title=title)
+        if owner is None:
+            owner = self.make_user()
+        project = Project.objects.create(
+            name=name, title=title, _is_private=is_private, owner=owner,
+            project_group=project_group)
+        for group in access_groups:
+            project.access_groups.add(group)
+        project.save()
+        return project
 
     def make_project_group(self, name=None, title=None):
         """
@@ -108,5 +118,16 @@
             project = self.make_project()
         return BuildRequest.objects.create(project=project)
 
+    def make_access_group(self, users):
+        """
+        Create an AccessGroup containing the given users.
+        """
+        name = self.get_unique_string()
+        group = AccessGroup.objects.create(name=name)
+        for user in users:
+            group.members.add(user)
+        group.save()
+        return group
+
 
 factory = ObjectFactory()

=== added file 'lib/offspring/web/queuemanager/tests/test_models.py'
--- lib/offspring/web/queuemanager/tests/test_models.py	1970-01-01 00:00:00 +0000
+++ lib/offspring/web/queuemanager/tests/test_models.py	2011-12-01 15:15:28 +0000
@@ -0,0 +1,306 @@
+from operator import attrgetter
+
+from django.contrib.auth.models import AnonymousUser
+from django.test import TestCase
+
+from offspring.web.queuemanager.models import (
+    BuildRequest,
+    BuildResult,
+    Lexbuilder,
+    Project,
+    Release)
+from offspring.web.queuemanager.tests.factory import factory
+
+
+class ProjectTests(TestCase):
+
+    def test_default_manager(self):
+        """
+        Make sure .all_objects is the default manager.
+        
+        If that's not the case the admin UI will exclude all private objects
+        and we would break model validation of unique fields if the user
+        reuses a value from an object they're not allowed to see.
+        """
+        self.assertEquals(Project.all_objects, Project._default_manager)
+
+    def test_is_private_for_public_project(self):
+        """
+        Check that Project.is_private returns False for public projects.
+        """
+        project = factory.make_project(is_private=False)
+        self.assertEqual(False, project.is_private)
+
+    def test_is_private_for_private_project(self):
+        """
+        Check that Project.is_private returns True for private projects.
+        """
+        private_project = factory.make_project(is_private=True)
+        self.assertEqual(True, private_project.is_private)
+
+    def test_is_visible_to_anyone_if_public(self):
+        """
+        Check that public projects are visible to anyone.
+        """
+        project = factory.make_project(is_private=False)
+        self.assertTrue(project.is_visible_to(AnonymousUser()))
+
+    def test_is_not_visible_to_anyone_if_private(self):
+        """
+        Check that private projects are not visible to anonymous users.
+        """
+        project = factory.make_project(is_private=True)
+        self.assertFalse(project.is_visible_to(AnonymousUser()))
+
+    def test_is_visible_to_owner_if_private(self):
+        """
+        Check that private projects are visible to their owners.
+        """
+        project = factory.make_project(is_private=True)
+        self.assertTrue(project.is_visible_to(project.owner))
+
+    def test_is_visible_to_member_of_access_group_if_private(self):
+        """
+        Check that private projects are visible to members of their access
+        groups.
+        """
+        user = factory.make_user()
+        group = factory.make_access_group([user])
+        project = factory.make_project(is_private=True, access_groups=[group])
+        self.assertTrue(project.is_visible_to(user))
+
+    def test_dot_objects_model_manager_only_returns_public_projects(self):
+        """
+        Check that the .objects model manager return only public projects.
+        """
+        project = factory.make_project(is_private=False)
+        project2 = factory.make_project(is_private=True)
+        self.assertEqual([project], list(Project.objects.all()))
+
+    def test_all_objects_model_manager(self):
+        """
+        Check that Project.all_objects includes private objects.
+
+        Project.all_objects() is a separate model manager which returns
+        private objects as well. Its .all() method will return public and
+        private objects regardless of whether or not the current user has
+        access to them. It is to be used with caution.
+        """
+        project = factory.make_project(is_private=False)
+        project2 = factory.make_project(is_private=True)
+        self.assertEqual([project, project2], list(Project.all_objects.all()))
+
+    def test_accessible_by_user_with_anonymous_user(self):
+        """
+        Check that accessible_by_user() can be given an anonymous user as
+        argument.
+        """
+        project = factory.make_project(is_private=True)
+        project2 = factory.make_project(is_private=False)
+        self.assertEqual(True, project.is_private)
+        self.assertEqual(False, project2.is_private)
+        objects = Project.all_objects.accessible_by_user(AnonymousUser())
+        self.assertEqual([project2], list(objects))
+
+    def test_accessible_by_user_with_public_project(self):
+        """
+        Check that accessible_by_user() returns public projects regardless of
+        the given user.
+        """
+        user = factory.make_user()
+        project = factory.make_project(is_private=False)
+        self.assertEqual(False, project.is_private)
+        objects = Project.all_objects.accessible_by_user(user)
+        self.assertEqual([project], list(objects))
+
+    def test_accessible_by_user_with_private_project(self):
+        """
+        Check that accessible_by_user() returns only the private projects the
+        given user is allowed to see.
+        """
+        user = factory.make_user()
+        group = factory.make_access_group([user])
+        project = factory.make_project(is_private=True, access_groups=[group])
+        # This second project is private but has no access groups set up, so
+        # the user cannot see it.
+        project2 = factory.make_project(is_private=True)
+        self.assertEqual(True, project.is_private)
+        self.assertEqual(True, project2.is_private)
+        objects = Project.all_objects.accessible_by_user(user)
+        self.assertEqual([project], list(objects))
+
+
+class ReleaseOrLexbuilderOrBuildResultOrBuildRequestTestsMixin(object):
+    """A mixin with tests that work for the following models:
+
+        Release
+        Lexbuilder
+        BuildResult
+        BuildRequest
+
+    You just need to mix this with TestCase in your subclass and define
+    factoryMethod and model.
+    """
+    model = None
+
+    def factoryMethod(self, project):
+        raise NotImplementedError()
+
+    def test_default_manager(self):
+        """
+        Make sure .all_objects is the default manager.
+        
+        If that's not the case the admin UI will exclude all private objects
+        and we would break model validation of unique fields if the user
+        reuses a value from an object they're not allowed to see.
+        """
+        self.assertEquals(self.model.all_objects, self.model._default_manager)
+
+    def test_is_private_for_public_object(self):
+        """
+        Check that .is_private returns False for public objects.
+        """
+        public_object = self.factoryMethod(
+            factory.make_project(is_private=False))
+        self.assertFalse(public_object.is_private)
+
+    def test_is_private_for_private_object(self):
+        """
+        Check that .is_private returns True for private objects.
+        """
+        private_object = self.factoryMethod(
+            factory.make_project(is_private=True))
+        self.assertTrue(private_object.is_private)
+
+    def test_is_visible_to_anyone_if_public(self):
+        """
+        Check that public objects are visible to anyone.
+        """
+        public_object = self.factoryMethod(
+            factory.make_project(is_private=False))
+        self.assertTrue(public_object.is_visible_to(AnonymousUser()))
+
+    def test_is_not_visible_to_anyone_if_private(self):
+        """
+        Check that private objects are not visible to anonymous users.
+        """
+        private_object = self.factoryMethod(
+            factory.make_project(is_private=True))
+        self.assertFalse(private_object.is_visible_to(AnonymousUser()))
+
+    def test_is_visible_to_owner_if_private(self):
+        """
+        Check that private objects are visible to their project's owners.
+        """
+        # The object we're dealing with inherits the access-control rules from
+        # the project it's related to, so the owner that matters is the
+        # project owner.
+        owner = factory.make_user()
+        private_object = self.factoryMethod(
+            factory.make_project(is_private=True, owner=owner))
+        self.assertTrue(private_object.is_visible_to(owner))
+
+    def test_is_visible_to_member_of_access_group_if_private(self):
+        """
+        Check that private objects are visible to members of their project's
+        access groups.
+        """
+        user = factory.make_user()
+        group = factory.make_access_group([user])
+        private_object = self.factoryMethod(
+            factory.make_project(is_private=True, access_groups=[group]))
+        self.assertTrue(private_object.is_visible_to(user))
+
+    def test_dot_objects_model_manager_only_returns_public_objects(self):
+        """
+        Check that the .objects model manager return only public objects.
+        """
+        private_object = self.factoryMethod(
+            factory.make_project(is_private=True))
+        public_object = self.factoryMethod(
+            factory.make_project(is_private=False))
+        self.assertEqual([public_object], list(self.model.objects.all()))
+
+    def test_all_objects_model_manager(self):
+        """
+        Check that .all_objects includes private objects.
+
+        self.model.all_objects() is a separate model manager which returns
+        private objects as well. Its .all() method will return public and
+        private objects regardless of whether or not the current user has
+        access to them. It is to be used with caution.
+        """
+        public_object = self.factoryMethod(
+            project=factory.make_project(is_private=False))
+        private_object = self.factoryMethod(
+            project=factory.make_project(is_private=True))
+        self.assertEqual([public_object, private_object],
+                         list(self.model.all_objects.all()))
+
+    def test_accessible_by_user(self):
+        """
+        Check that accessible_by_user() returns only the private objects the
+        user is allowed to see.
+        """
+        user = factory.make_user()
+        group = factory.make_access_group([user])
+        visible_object = self.factoryMethod(
+            factory.make_project(is_private=True, access_groups=[group]))
+        # This object is linked to a private project which has no access
+        # groups set up, so the user cannot see it.
+        invisible_object = self.factoryMethod(
+            project=factory.make_project(is_private=True))
+        self.assertEqual(
+            [visible_object],
+            list(self.model.all_objects.accessible_by_user(user)))
+
+
+class ReleaseTests(
+    TestCase, ReleaseOrLexbuilderOrBuildResultOrBuildRequestTestsMixin):
+    model = Release
+
+    def factoryMethod(self, project):
+        return factory.make_release(
+            build=factory.make_build_result(project=project))
+
+
+class LexbuilderTests(
+        TestCase, ReleaseOrLexbuilderOrBuildResultOrBuildRequestTestsMixin):
+    model = Lexbuilder
+
+    def factoryMethod(self, project):
+        return factory.make_lexbuilder(
+            current_job=factory.make_build_result(project=project))
+
+
+class BuildResultTests(
+        TestCase, ReleaseOrLexbuilderOrBuildResultOrBuildRequestTestsMixin):
+    model = BuildResult
+    factoryMethod = factory.make_build_result
+
+
+class BuildRequestTests(
+        TestCase, ReleaseOrLexbuilderOrBuildResultOrBuildRequestTestsMixin):
+    model = BuildRequest
+    factoryMethod = factory.make_build_request
+
+
+class ProjectGroupTests(TestCase):
+
+    def test_get_projects_filters_private_objects(self):
+        """
+        ProjectGroup.get_projects() will return only the private projects the
+        given user is allowed to see.
+        """
+        user = factory.make_user()
+        group = factory.make_project_group()
+        public_project = factory.make_project(
+            is_private=False, project_group=group)
+        private_project_visible_to_user = factory.make_project(
+            is_private=True, project_group=group, owner=user)
+        private_project = factory.make_project(
+            is_private=True, project_group=group)
+        self.assertEqual(
+            sorted([public_project, private_project_visible_to_user],
+                   key=attrgetter('name')),
+            sorted(group.get_projects(user), key=attrgetter('name')))

=== modified file 'lib/offspring/web/queuemanager/views.py'
--- lib/offspring/web/queuemanager/views.py	2011-08-13 06:28:28 +0000
+++ lib/offspring/web/queuemanager/views.py	2011-12-01 15:15:28 +0000
@@ -241,6 +241,7 @@
 
     pageData = { 
         'projectGroup' : pg, 
+        'projects': pg.get_projects(request.user),
         'pillar' : 'projects', 
         'build_stats' : build_stats,
         'projectgroup_build_results' : projectgroup_build_results,

=== modified file 'lib/offspring/web/settings.py'
--- lib/offspring/web/settings.py	2011-08-11 00:16:09 +0000
+++ lib/offspring/web/settings.py	2011-12-01 15:15:28 +0000
@@ -96,6 +96,7 @@
     'djcelery',
     'djkombu',
     'piston',
+    'django_group_access',
     # Offspring Apps
     'offspring.web.queuemanager',
 )

=== modified file 'lib/offspring/web/templates/queuemanager/projectgroup_details.html'
--- lib/offspring/web/templates/queuemanager/projectgroup_details.html	2010-11-29 08:27:24 +0000
+++ lib/offspring/web/templates/queuemanager/projectgroup_details.html	2011-12-01 15:15:28 +0000
@@ -32,7 +32,7 @@
             <h3>Project</h3></td><td></td><td><h3>Last Build</h3></td><td width="30%"><h3>Date</h3></td><td width="15%"><h3>Result</h3>
         </td>
     </tr>
-    {% for project in projectGroup.projects %}
+    {% for project in projects %}
     <tr>
         <td>
             {% if project.needs_build %}<img src="/assets/images/33.png" title="Pending Build">{% else %}{% ifequal project.latest_build.result "FAILED" %}<img src="/assets/images/34.png" title="Build Failure">{% else %} {% ifequal project.latest_build.result "COMPLETED" %} <img src="/assets/images/35.png" title="Build Successful"> {% else %} <img src="/assets/images/36.png" title="Not yet built">{% endifequal %}{% endifequal %}{% endif %}</td><td> <a href="{% url offspring.web.queuemanager.views.project_details project.name %}">{{ project.title }}</a>

=== added directory 'migration'
=== added file 'migration/001-project-privacy.sql'
--- migration/001-project-privacy.sql	1970-01-01 00:00:00 +0000
+++ migration/001-project-privacy.sql	2011-12-01 15:15:28 +0000
@@ -0,0 +1,10 @@
+ALTER TABLE projects ADD COLUMN "is_private" boolean NOT NULL default FALSE;
+ALTER TABLE projects ADD COLUMN "owner_id" integer REFERENCES "auth_user" ("id") DEFERRABLE INITIALLY DEFERRED;
+ALTER TABLE projects ADD CONSTRAINT "private_project_requires_owner" CHECK (is_private IS FALSE OR owner_id IS NOT NULL);
+CREATE TABLE "projects_access_groups" (
+	    "id" serial NOT NULL PRIMARY KEY,
+	    "project_id" varchar(200) NOT NULL,
+	    "accessgroup_id" integer NOT NULL REFERENCES "django_group_access_accessgroup" ("id") DEFERRABLE INITIALLY DEFERRED,
+	    UNIQUE ("project_id", "accessgroup_id")
+);
+ALTER TABLE "projects_access_groups" ADD CONSTRAINT "project_id_refs_name_44bd562d" FOREIGN KEY ("project_id") REFERENCES "projects" ("name") DEFERRABLE INITIALLY DEFERRED;

=== modified file 'requirements/requirements.web.txt'
--- requirements/requirements.web.txt	2011-08-11 00:16:09 +0000
+++ requirements/requirements.web.txt	2011-12-01 15:15:28 +0000
@@ -11,3 +11,4 @@
 celery>=2.3.1
 django-celery>=2.3.0
 django-kombu>=0.9.4
+django-group-access>=0.0.1