Merge lp:~stevenk/launchpad/product-limitedview-with-team-aag into lp:launchpad

[Steve Kowalik]

The LimitedView security adapter on IProduct now deals with AccessArtifactGrants that have been granted to teams.

Currently, the LimitedView security adapter for IProduct checks if a user has an AccessArtifactGrant on a bug, branch or specification for the private project. This is fine, and works well, except that it doesn't work if a team the user is a member has the AAG. As such, I've rewritten ISharingService.userHasGrantsOnPillar() to respect TeamParticipation.

I have done some drive-by on some asserts, using assert{True,False} rather than assertEqual({True,False}, and fixed one indentation error I noticed.

I have more than enough LoC credit for the +19 gain.

[William Grant (wgrant)]

review: Approve (code)

=== modified file 'lib/lp/registry/interfaces/sharingservice.py'
--- lib/lp/registry/interfaces/sharingservice.py	2012-12-02 01:51:46 +0000
+++ lib/lp/registry/interfaces/sharingservice.py	2013-01-09 05:39:20 +0000
@@ -1,4 +1,4 @@
-# Copyright 2012 Canonical Ltd.  This software is licensed under the
+# Copyright 2012-2013 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Interfaces for sharing service."""
@@ -117,7 +117,7 @@
         :return: a (bugtasks, branches, specifications) tuple
         """
 
-    def userHasGrantsOnPillar(pillar, user):
+    def checkPillarArtifactAccess(pillar, user):
         """Return True if user has any grants on pillar else return False."""
 
     @export_read_operation()

=== modified file 'lib/lp/registry/services/sharingservice.py'
--- lib/lp/registry/services/sharingservice.py	2012-12-02 03:30:18 +0000
+++ lib/lp/registry/services/sharingservice.py	2013-01-09 05:39:20 +0000
@@ -1,4 +1,4 @@
-# Copyright 2012 Canonical Ltd.  This software is licensed under the
+# Copyright 2012-2013 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Classes for pillar and artifact sharing service."""
@@ -227,10 +227,20 @@
 
         return bugtasks, branches, specifications
 
-    def userHasGrantsOnPillar(self, pillar, user):
+    def checkPillarArtifactAccess(self, pillar, user):
         """See `ISharingService`."""
-        return not self.getArtifactGrantsForPersonOnPillar(
-            pillar, user).is_empty()
+        tables = [
+            AccessPolicyGrantFlat,
+            Join(
+                TeamParticipation,
+                TeamParticipation.teamID == AccessPolicyGrantFlat.grantee_id),
+            Join(
+                AccessPolicy,
+                AccessPolicy.id == AccessPolicyGrantFlat.policy_id)]
+        return not IStore(AccessPolicyGrantFlat).using(*tables).find(
+            AccessPolicyGrantFlat,
+            AccessPolicy.product_id == pillar.id,
+            TeamParticipation.personID == user.id).is_empty()
 
     @available_with_permission('launchpad.Driver', 'pillar')
     def getSharedBugs(self, pillar, person, user):

=== modified file 'lib/lp/registry/services/tests/test_sharingservice.py'
--- lib/lp/registry/services/tests/test_sharingservice.py	2012-11-29 20:27:19 +0000
+++ lib/lp/registry/services/tests/test_sharingservice.py	2013-01-09 05:39:20 +0000
@@ -1,4 +1,4 @@
-# Copyright 2012 Canonical Ltd.  This software is licensed under the
+# Copyright 2012-2013 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
@@ -1762,19 +1762,28 @@
             self.service.sharePillarInformation(
                 product, wrong_person, product.owner,
                 {InformationType.PRIVATESECURITY: SharingPermission.ALL})
-        self.assertEqual(
-            False,
+        self.assertFalse(
             self.service.checkPillarAccess(
                 [product], InformationType.USERDATA, wrong_person))
-        self.assertEqual(
-            True,
+        self.assertTrue(
             self.service.checkPillarAccess(
                 [product], InformationType.USERDATA, right_person))
 
+    def test_checkPillarArtifactAccess_respects_teams(self):
+        owner = self.factory.makePerson()
+        product = self.factory.makeProduct(
+            information_type=InformationType.PROPRIETARY, owner=owner)
+        user = self.factory.makePerson()
+        team = self.factory.makeTeam(
+            membership_policy=TeamMembershipPolicy.MODERATED, members=[user])
+        with person_logged_in(owner):
+            bug = self.factory.makeBug(target=product)
+            bug.subscribe(team, owner)
+        self.assertTrue(self.service.checkPillarArtifactAccess(product, user))
+
     def test_checkPillarAccess_no_policy(self):
         # checkPillarAccess returns False if there's no policy.
-        self.assertEqual(
-            False,
+        self.assertFalse(
             self.service.checkPillarAccess(
                 [self.factory.makeProduct()], InformationType.PUBLIC,
                 self.factory.makePerson()))

=== modified file 'lib/lp/registry/tests/test_product.py'
--- lib/lp/registry/tests/test_product.py	2013-01-03 03:41:13 +0000
+++ lib/lp/registry/tests/test_product.py	2013-01-09 05:39:20 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2013 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
@@ -1184,7 +1184,7 @@
                 # The second access does not require another query.
                 product.description
                 self.assertEqual(
-                queries_for_first_user_access, len(recorder.queries))
+                    queries_for_first_user_access, len(recorder.queries))
 
     def test_userCanView_works_with_IPersonRoles(self):
         # userCanView() maintains a cache of users known to have the

=== modified file 'lib/lp/security.py'
--- lib/lp/security.py	2013-01-04 06:25:25 +0000
+++ lib/lp/security.py	2013-01-09 05:39:20 +0000
@@ -448,7 +448,7 @@
     def checkAuthenticated(self, user):
         return (
             super(LimitedViewProduct, self).checkAuthenticated(user) or
-            getUtility(IService, 'sharing').userHasGrantsOnPillar(
+            getUtility(IService, 'sharing').checkPillarArtifactAccess(
                 self.obj, user))