Merge lp:~adeuring/launchpad/authentication-for-private-products-2 into lp:launchpad

[Abel Deuring]

policy grant based access checks for private products.

Another attempt to activate permission checks for private products.

pre-imp calls with suínzui and flacoste

The first attempt, merged in r16090, reviewed here
https://code.launchpad.net/~adeuring/launchpad/correct-permission-check-for-iproduct/+merge/127518
and here
https://code.launchpad.net/~adeuring/launchpad/product-sharing-sec-adapter/+merge/127473
had to be reverted becaused is caused lots of "permission denied"
errors.

The causes of these error:

(1) Nearly all properties of IProduct required the permission
launchpad.View . Before, only IPillar was guarded by this
permission, most other attributes were public.

(2) The security adapter, class ViewProduct, inherited from
the checker for IPillar, class ViewPillar. And ViewProduct
further limited the access so that only people with policy
grants for private products get acces to those product. But
it also denided access to most properties of inactive public
products, by also calling ViewPillar.check[Un]Authenticated()

(3) Lots of pages retrieve "related products", for example
person pages, or bug pages. It often happens that attributes
like product.name, product.displayname or subscription related
stuff, were accessed -- even for inactive products. Data for
inactive products is later dropped, but obviously at a very
late stage during rendering. It seems that inactive products
are quite often filtered out at a very late stage of page
rendering.

This branch fixes these problems by again allowing access to
all attributes that require the permisison launchpad.View
for inactive public products, by not checking the flag
product.active flag in ProductView.check[Un]authenticated()

This has again another unwanted side effect:

All users can see inactive public products, while we want to
present a 404 page in this case. The reason:

lp.app.browser.launchpad.Launchpad.traverse() checks if
the user has the permission launchpad.View for a given pillar.

This check worked before because launchpad.View was only
required for IPillar (see class ViewPillar), so all these
eventually unnecessary accesses to product properties were
possible even if a user did not have the permission lp.View
-- but with the new permission checker ViewProduct in place,
users would see inactive products like admins, just with a
warning "this project ic inactive".

So we simply can't call check_permission('lp.View', product)
in Launchpad.traverse() any longer but need to explicitly check
for inactive products.

Note that this "special rule" for products will be needed anyway
once we allow artifact grants for private products: This will
require another permission, launchpad.LimitedView, so that users
with grants for artifacts can traverse to branches or bugs.
Aand the traverse() method should then check if users

tests:

./bin/test -vvt lib/lp/registry/stories/pillar/xx-pillar-deactivation.txt
./bin/test -vvt lp.app.browser.tests.test_launchpad.TestProductTraversal
./bin/test -vvt lp.registry.tests.test_product.TestProduct.test_access_launchpad_View_proprietary_product
./bin/test -vvt lp.registry.tests.test_product.TestProduct.test_access_launchpad_View_public_inactive_product

no lint.

The complete diff against trunk is quite large.

r16117 is just a "rollback of the rollback" from r16112.

The relevant changes are in r16118. The diff r16117..16118:

=== modified file 'lib/lp/app/browser/launchpad.py'
--- lib/lp/app/browser/launchpad.py 2012-07-09 13:18:34 +0000
+++ lib/lp/app/browser/launchpad.py 2012-10-12 13:02:05 +0000
@@ -105,9 +105,11 @@
 from lp.registry.interfaces.pillar import IPillarNameSet
 from lp.registry.interfaces.product import (
     InvalidProductName,
+ IProduct,
     IProductSet,
     )
 from lp.registry.interfaces.projectgroup import IProjectGroupSet
+from lp.registry.interfaces.role import IPersonRoles
 from lp.registry.interfaces.sourcepackagename import ISourcePackageNameSet
 from lp.services.config import config
 from lp.services.helpers import intOrZero
@@ -763,6 +765,29 @@

         pillar = getUtility(IPillarNameSet).getByName(
             name, ignore_inactive=False)
+
+ if (pillar is not None and IProduct.providedBy(pillar) and
+ not pillar.active):
+ # Emergency brake for public but inactive products:
+ # These products should not be shown to ordinary users.
+ # The root problem is that many views iterate over products,
+ # inactive products included, and access attributes like
+ # name, displayname or call canonical_url(product) --
+ # and finally throw the data away, if the product is
+ # inactive. So we cannot make these attributes inaccessible
+ # for inactive public products. On the other hand, we
+ # require the permission launchpad.View to protect private
+ # products.
+ # This means that we cannot simply check if the current
+ # user has the permission launchpad.View for an inactive
+ # product.
+ user = getUtility(ILaunchBag).user
+ if user is None:
+ return None
+ user = IPersonRoles(user)
+ if (not user.in_commercial_admin and not user.in_admin and
+ not user.in_registry_experts):
+ return None
         if pillar is not None and check_permission('launchpad.View', pillar):
             if pillar.name != name:
                 # This pillar was accessed through one of its aliases, so we

=== modified file 'lib/lp/app/browser/tests/test_launchpad.py'
--- lib/lp/app/browser/tests/test_launchpad.py 2012-09-17 15:19:10 +0000
+++ lib/lp/app/browser/tests/test_launchpad.py 2012-10-12 13:02:05 +0000
@@ -20,8 +20,12 @@
 from lp.app.enums import InformationType
 from lp.app.errors import GoneError
 from lp.app.interfaces.launchpad import ILaunchpadCelebrities
+from lp.app.interfaces.services import IService
 from lp.code.interfaces.linkedbranch import ICanHasLinkedBranch
-from lp.registry.enums import PersonVisibility
+from lp.registry.enums import (
+ PersonVisibility,
+ SharingPermission,
+ )
 from lp.registry.interfaces.person import IPersonSet
 from lp.services.identity.interfaces.account import AccountStatus
 from lp.services.webapp import canonical_url
@@ -468,3 +472,108 @@
             reg.name for reg in iter_view_registrations(macros.__class__))
         self.assertIn('+base-layout-macros', names)
         self.assertNotIn('+related-pages', names)
+
+
+class TestProductTraversal(TestCaseWithFactory, TraversalMixin):
+
+ layer = DatabaseFunctionalLayer
+
+ def setUp(self):
+ super(TestProductTraversal, self).setUp()
+ self.active_public_product = self.factory.makeProduct()
+ self.inactive_public_product = self.factory.makeProduct()
+ removeSecurityProxy(self.inactive_public_product).active = False
+ self.proprietary_product_owner = self.factory.makePerson()
+ self.active_proprietary_product = self.factory.makeProduct(
+ owner=self.proprietary_product_owner,
+ information_type=InformationType.PROPRIETARY)
+ self.inactive_proprietary_product = self.factory.makeProduct(
+ owner=self.proprietary_product_owner,
+ information_type=InformationType.PROPRIETARY)
+ removeSecurityProxy(self.inactive_proprietary_product).active = False
+
+ def traverse_to_active_public_product(self):
+ segment = self.active_public_product.name
+ self.traverse(segment, segment)
+
+ def traverse_to_inactive_public_product(self):
+ segment = removeSecurityProxy(self.inactive_public_product).name
+ self.traverse(segment, segment)
+
+ def traverse_to_active_proprietary_product(self):
+ segment = removeSecurityProxy(self.active_proprietary_product).name
+ self.traverse(segment, segment)
+
+ def traverse_to_inactive_proprietary_product(self):
+ segment = removeSecurityProxy(self.inactive_proprietary_product).name
+ self.traverse(segment, segment)
+
+ def test_access_for_anon(self):
+ # Anonymous users can see only public active products.
+ with person_logged_in(ANONYMOUS):
+ self.traverse_to_active_public_product()
+ # Access to other products raises a NotFound error.
+ self.assertRaises(
+ NotFound, self.traverse_to_inactive_public_product)
+ self.assertRaises(
+ NotFound, self.traverse_to_active_proprietary_product)
+ self.assertRaises(
+ NotFound, self.traverse_to_inactive_proprietary_product)
+
+ def test_access_for_ordinary_users(self):
+ # Ordinary logged in users can see only public active products.
+ with person_logged_in(self.factory.makePerson()):
+ self.traverse_to_active_public_product()
+ # Access to other products raises a NotFound error.
+ self.assertRaises(
+ NotFound, self.traverse_to_inactive_public_product)
+ self.assertRaises(
+ NotFound, self.traverse_to_active_proprietary_product)
+ self.assertRaises(
+ NotFound, self.traverse_to_inactive_proprietary_product)
+
+ def test_access_for_person_with_pillar_grant(self):
+ # Persons with a policy grant for a proprietary product can
+ # access this product, if it is active.
+ user = self.factory.makePerson()
+ with person_logged_in(self.proprietary_product_owner):
+ getUtility(IService, 'sharing').sharePillarInformation(
+ self.active_proprietary_product, user,
+ self.proprietary_product_owner,
+ {InformationType.PROPRIETARY: SharingPermission.ALL})
+ getUtility(IService, 'sharing').sharePillarInformation(
+ self.inactive_proprietary_product, user,
+ self.proprietary_product_owner,
+ {InformationType.PROPRIETARY: SharingPermission.ALL})
+ with person_logged_in(user):
+ self.traverse_to_active_public_product()
+ self.assertRaises(
+ NotFound, self.traverse_to_inactive_public_product)
+ self.traverse_to_active_proprietary_product()
+ self.assertRaises(
+ NotFound, self.traverse_to_inactive_proprietary_product)
+
+ def check_admin_access(self, user):
+ with person_logged_in(user):
+ self.traverse_to_active_public_product()
+ self.traverse_to_inactive_public_product()
+ self.traverse_to_active_proprietary_product()
+ self.traverse_to_inactive_proprietary_product()
+
+ def test_access_for_persons_with_special_permissions(self):
+ # Admins have access all products, including inactive and propretary
+ # products.
+ self.check_admin_access(getUtility(IPersonSet).getByName('name16'))
+ # Registry experts can access to all products.
+ registry_expert = self.factory.makePerson()
+ registry = getUtility(ILaunchpadCelebrities).registry_experts
+ with person_logged_in(registry.teamowner):
+ registry.addMember(registry_expert, registry.teamowner)
+ self.check_admin_access(registry_expert)
+ # Commercial admins have access to all products.
+ commercial_admin = self.factory.makePerson()
+ commercial_admins = getUtility(ILaunchpadCelebrities).commercial_admin
+ with person_logged_in(commercial_admins.teamowner):
+ commercial_admins.addMember(
+ commercial_admin, commercial_admins.teamowner)
+ self.check_admin_access(registry_expert)

=== modified file 'lib/lp/registry/model/product.py'
--- lib/lp/registry/model/product.py 2012-10-09 10:28:02 +0000
+++ lib/lp/registry/model/product.py 2012-10-12 13:02:05 +0000
@@ -1548,13 +1548,21 @@
             return False
         if user.id in self._known_viewers:
             return True
- # We want an actual Storm Person.
+ # We need the plain Storm Person object for the SQL query below
+ # but an IPersonRoles object for the team membership checks.
         if IPersonRoles.providedBy(user):
- user = user.person
+ plain_user = user.person
+ else:
+ plain_user = user
+ user = IPersonRoles(user)
+ if (user.in_commercial_admin or user.in_admin or
+ user.in_registry_experts):
+ self._known_viewers.add(user.id)
+ return True
         policy = getUtility(IAccessPolicySource).find(
             [(self, self.information_type)]).one()
         grants_for_user = getUtility(IAccessPolicyGrantSource).find(
- [(policy, user)])
+ [(policy, plain_user)])
         if grants_for_user.is_empty():
             return False
         self._known_viewers.add(user.id)

=== modified file 'lib/lp/registry/tests/test_product.py'
--- lib/lp/registry/tests/test_product.py 2012-10-09 10:28:02 +0000
+++ lib/lp/registry/tests/test_product.py 2012-10-12 13:02:05 +0000
@@ -59,11 +59,13 @@
     IAccessPolicySource,
     )
 from lp.registry.interfaces.oopsreferences import IHasOOPSReferences
+from lp.registry.interfaces.person import IPersonSet
 from lp.registry.interfaces.product import (
     IProduct,
     IProductSet,
     License,
     )
+from lp.registry.interfaces.role import IPersonRoles
 from lp.registry.interfaces.series import SeriesStatus
 from lp.registry.model.product import (
     Product,
@@ -741,6 +743,24 @@
             for attribute_name in names:
                 getattr(product, attribute_name)

+ def test_access_launchpad_View_public_inactive_product(self):
+ # Everybody, including anonymous users, has access to
+ # properties of public but inactvie products that require
+ # the permission launchpad.View.
+ product = self.factory.makeProduct()
+ removeSecurityProxy(product).active = False
+ names = self.expected_get_permissions['launchpad.View']
+ with person_logged_in(None):
+ for attribute_name in names:
+ getattr(product, attribute_name)
+ ordinary_user = self.factory.makePerson()
+ with person_logged_in(ordinary_user):
+ for attribute_name in names:
+ getattr(product, attribute_name)
+ with person_logged_in(product.owner):
+ for attribute_name in names:
+ getattr(product, attribute_name)
+
     def test_access_launchpad_View_proprietary_product(self):
         # Only people with grants for a private product can access
         # attributes protected by the permission launchpad.View.
@@ -770,6 +790,26 @@
         with person_logged_in(ordinary_user):
             for attribute_name in names:
                 getattr(product, attribute_name)
+ # Admins can access proprietary products.
+ with person_logged_in(getUtility(IPersonSet).getByName('name16')):
+ for attribute_name in names:
+ getattr(product, attribute_name)
+ registry_expert = self.factory.makePerson()
+ registry = getUtility(ILaunchpadCelebrities).registry_experts
+ with person_logged_in(registry.teamowner):
+ registry.addMember(registry_expert, registry.teamowner)
+ with person_logged_in(registry_expert):
+ for attribute_name in names:
+ getattr(product, attribute_name)
+ # Commercial admins have access to all products.
+ commercial_admin = self.factory.makePerson()
+ commercial_admins = getUtility(ILaunchpadCelebrities).commercial_admin
+ with person_logged_in(commercial_admins.teamowner):
+ commercial_admins.addMember(
+ commercial_admin, commercial_admins.teamowner)
+ with person_logged_in(commercial_admin):
+ for attribute_name in names:
+ getattr(product, attribute_name)

     def test_access_launchpad_AnyAllowedPerson_public_product(self):
         # Only logged in persons have access to properties of public products
@@ -882,6 +922,16 @@
                 self.assertEqual(
                 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
+ # permission to access a product.
+ product = self.createProduct(
+ information_type=InformationType.PROPRIETARY,
+ license=License.OTHER_PROPRIETARY)
+ user = self.factory.makePerson()
+ product.userCanView(user)
+ product.userCanView(IPersonRoles(user))
+

 class TestProductBugInformationTypes(TestCaseWithFactory):

=== modified file 'lib/lp/security.py'
--- lib/lp/security.py 2012-10-09 10:28:02 +0000
+++ lib/lp/security.py 2012-10-12 13:02:05 +0000
@@ -14,13 +14,8 @@
 from operator import methodcaller

 from storm.expr import (
- And,
- Exists,
- Or,
     Select,
- SQL,
     Union,
- With,
     )
 from zope.component import (
     getUtility,
@@ -177,7 +172,6 @@
     )
 from lp.registry.interfaces.wikiname import IWikiName
 from lp.registry.model.person import Person
-from lp.registry.model.teammembership import TeamParticipation
 from lp.services.config import config
 from lp.services.database.lpstorm import IStore
 from lp.services.identity.interfaces.account import IAccount
@@ -431,19 +425,15 @@
         return user.isOwner(self.obj) or user.in_admin

-class ViewProduct(ViewPillar):
+class ViewProduct(AuthorizationBase):
     permission = 'launchpad.View'
     usedfor = IProduct

     def checkAuthenticated(self, user):
- return (
- super(ViewProduct, self).checkAuthenticated(user) and
- self.obj.userCanView(user))
+ return self.obj.userCanView(user)

     def checkUnauthenticated(self):
- return (
- self.obj.information_type in PUBLIC_INFORMATION_TYPES and
- super(ViewProduct, self).checkUnauthenticated())
+ return self.obj.information_type in PUBLIC_INFORMATION_TYPES

 class ChangeProduct(ViewProduct):

[Curtis Hovey (sinzui)]

This looks good. Thank you. I think one test can be simplified (and it looks like it is testing the wrong user at the end). this test duplicates code performed by 'celebrity_logged_in'. We also want to avoid sampledata since Foo Bar is more than just an Admin.

203 + def test_access_for_persons_with_special_permissions(self):
204 + # Admins have access all products, including inactive and propretary
205 + # products.
206 + self.check_admin_access(getUtility(IPersonSet).getByName('name16'))
207 + # Registry experts can access to all products.
208 + registry_expert = self.factory.makePerson()
209 + registry = getUtility(ILaunchpadCelebrities).registry_experts
210 + with person_logged_in(registry.teamowner):
211 + registry.addMember(registry_expert, registry.teamowner)
212 + self.check_admin_access(registry_expert)
213 + # Commercial admins have access to all products.
214 + commercial_admin = self.factory.makePerson()
215 + commercial_admins = getUtility(ILaunchpadCelebrities).commercial_admin
216 + with person_logged_in(commercial_admins.teamowner):
217 + commercial_admins.addMember(
218 + commercial_admin, commercial_admins.teamowner)
219 + self.check_admin_access(registry_expert)

Could be
    def test_access_for_persons_with_special_permissions(self):
        # Admins have access all products, including inactive and propretary
        # products.
        with celebrity_logged_in('admin') as admin:
            self.check_admin_access(admin)
        with celebrity_logged_in('registry_experts') as registry_expert:
            self.check_admin_access(registry_expert)
        with celebrity_logged_in('commercial_admins') as commercial_admin:
            self.check_admin_access(commercial_admin)

review: Approve (code)

[William Grant (wgrant)]

I'd still like to see an emergency feature flag in userCanView, in case we end up with the initial private projects on production breaking something unexpected. But otherwise this looks pretty good, particularly the improvements around handling deactivated products.

969 + if (user.in_commercial_admin or user.in_admin or
970 + user.in_registry_experts):

There are registry admins from outside Canonical, so ~registry must not be able to see all private projects.

Also, you can also easily fix bug #1061933 while you're here.

review: Approve (code)

=== modified file 'lib/lp/answers/tests/test_question_webservice.py'
--- lib/lp/answers/tests/test_question_webservice.py	2012-10-08 10:07:11 +0000
+++ lib/lp/answers/tests/test_question_webservice.py	2012-10-15 15:39:22 +0000
@@ -10,6 +10,7 @@
 from simplejson import dumps
 import transaction
 from zope.component import getUtility
+from zope.security.proxy import removeSecurityProxy
 
 from lp.answers.errors import (
     AddAnswerContactError,
@@ -83,6 +84,7 @@
         with celebrity_logged_in('admin'):
             self.question = self.factory.makeQuestion(
                 title="This is a question")
+            self.target_name = self.question.target.name
 
         self.webservice = LaunchpadWebServiceCaller(
             'launchpad-library', 'salgado-change-anything')
@@ -105,7 +107,7 @@
     def test_GET_xhtml_representation(self):
         # A question's xhtml representation is available on the api.
         response = self.webservice.get(
-            '/%s/+question/%d' % (self.question.target.name,
+            '/%s/+question/%d' % (self.target_name,
                 self.question.id),
             'application/xhtml+xml')
         self.assertEqual(response.status, 200)
@@ -119,7 +121,7 @@
         new_title = "No, this is a question"
 
         question_json = self.webservice.get(
-            '/%s/+question/%d' % (self.question.target.name,
+            '/%s/+question/%d' % (self.target_name,
                 self.question.id)).jsonBody()
 
         response = self.webservice.patch(
@@ -156,7 +158,7 @@
         # End any open lplib instance.
         logout()
         lp = launchpadlib_for("test", user)
-        return ws_object(lp, self.question)
+        return ws_object(lp, removeSecurityProxy(self.question))
 
     def _set_visibility(self, question):
         """Method to set visibility; needed for assertRaises."""

=== modified file 'lib/lp/app/browser/launchpad.py'
--- lib/lp/app/browser/launchpad.py	2012-07-09 13:18:34 +0000
+++ lib/lp/app/browser/launchpad.py	2012-10-15 15:39:22 +0000
@@ -105,9 +105,11 @@
 from lp.registry.interfaces.pillar import IPillarNameSet
 from lp.registry.interfaces.product import (
     InvalidProductName,
+    IProduct,
     IProductSet,
     )
 from lp.registry.interfaces.projectgroup import IProjectGroupSet
+from lp.registry.interfaces.role import IPersonRoles
 from lp.registry.interfaces.sourcepackagename import ISourcePackageNameSet
 from lp.services.config import config
 from lp.services.helpers import intOrZero
@@ -763,6 +765,29 @@
 
         pillar = getUtility(IPillarNameSet).getByName(
             name, ignore_inactive=False)
+
+        if (pillar is not None and IProduct.providedBy(pillar) and
+            not pillar.active):
+            # Emergency brake for public but inactive products:
+            # These products should not be shown to ordinary users.
+            # The root problem is that many views iterate over products,
+            # inactive products included, and access attributes like
+            # name, displayname or call canonical_url(product) --
+            # and finally throw the data away, if the product is
+            # inactive. So we cannot make these attributes inaccessible
+            # for inactive public products. On the other hand, we
+            # require the permission launchpad.View to protect private
+            # products.
+            # This means that we cannot simply check if the current
+            # user has the permission launchpad.View for an inactive
+            # product.
+            user = getUtility(ILaunchBag).user
+            if user is None:
+                return None
+            user = IPersonRoles(user)
+            if (not user.in_commercial_admin and not user.in_admin and
+                not user.in_registry_experts):
+                return None
         if pillar is not None and check_permission('launchpad.View', pillar):
             if pillar.name != name:
                 # This pillar was accessed through one of its aliases, so we

=== modified file 'lib/lp/app/browser/tests/test_launchpad.py'
--- lib/lp/app/browser/tests/test_launchpad.py	2012-09-17 15:19:10 +0000
+++ lib/lp/app/browser/tests/test_launchpad.py	2012-10-15 15:39:22 +0000
@@ -20,8 +20,12 @@
 from lp.app.enums import InformationType
 from lp.app.errors import GoneError
 from lp.app.interfaces.launchpad import ILaunchpadCelebrities
+from lp.app.interfaces.services import IService
 from lp.code.interfaces.linkedbranch import ICanHasLinkedBranch
-from lp.registry.enums import PersonVisibility
+from lp.registry.enums import (
+    PersonVisibility,
+    SharingPermission,
+    )
 from lp.registry.interfaces.person import IPersonSet
 from lp.services.identity.interfaces.account import AccountStatus
 from lp.services.webapp import canonical_url
@@ -33,6 +37,7 @@
 from lp.services.webapp.url import urlappend
 from lp.testing import (
     ANONYMOUS,
+    celebrity_logged_in,
     login,
     login_person,
     person_logged_in,
@@ -468,3 +473,101 @@
             reg.name for reg in iter_view_registrations(macros.__class__))
         self.assertIn('+base-layout-macros', names)
         self.assertNotIn('+related-pages', names)
+
+
+class TestProductTraversal(TestCaseWithFactory, TraversalMixin):
+
+    layer = DatabaseFunctionalLayer
+
+    def setUp(self):
+        super(TestProductTraversal, self).setUp()
+        self.active_public_product = self.factory.makeProduct()
+        self.inactive_public_product = self.factory.makeProduct()
+        removeSecurityProxy(self.inactive_public_product).active = False
+        self.proprietary_product_owner = self.factory.makePerson()
+        self.active_proprietary_product = self.factory.makeProduct(
+            owner=self.proprietary_product_owner,
+            information_type=InformationType.PROPRIETARY)
+        self.inactive_proprietary_product = self.factory.makeProduct(
+            owner=self.proprietary_product_owner,
+            information_type=InformationType.PROPRIETARY)
+        removeSecurityProxy(self.inactive_proprietary_product).active = False
+
+    def traverse_to_active_public_product(self):
+        segment = self.active_public_product.name
+        self.traverse(segment, segment)
+
+    def traverse_to_inactive_public_product(self):
+        segment = removeSecurityProxy(self.inactive_public_product).name
+        self.traverse(segment, segment)
+
+    def traverse_to_active_proprietary_product(self):
+        segment = removeSecurityProxy(self.active_proprietary_product).name
+        self.traverse(segment, segment)
+
+    def traverse_to_inactive_proprietary_product(self):
+        segment = removeSecurityProxy(self.inactive_proprietary_product).name
+        self.traverse(segment, segment)
+
+    def test_access_for_anon(self):
+        # Anonymous users can see only public active products.
+        with person_logged_in(ANONYMOUS):
+            self.traverse_to_active_public_product()
+            # Access to other products raises a NotFound error.
+            self.assertRaises(
+                NotFound, self.traverse_to_inactive_public_product)
+            self.assertRaises(
+                NotFound, self.traverse_to_active_proprietary_product)
+            self.assertRaises(
+                NotFound, self.traverse_to_inactive_proprietary_product)
+
+    def test_access_for_ordinary_users(self):
+        # Ordinary logged in users can see only public active products.
+        with person_logged_in(self.factory.makePerson()):
+            self.traverse_to_active_public_product()
+            # Access to other products raises a NotFound error.
+            self.assertRaises(
+                NotFound, self.traverse_to_inactive_public_product)
+            self.assertRaises(
+                NotFound, self.traverse_to_active_proprietary_product)
+            self.assertRaises(
+                NotFound, self.traverse_to_inactive_proprietary_product)
+
+    def test_access_for_person_with_pillar_grant(self):
+        # Persons with a policy grant for a proprietary product can
+        # access this product, if it is active.
+        user = self.factory.makePerson()
+        with person_logged_in(self.proprietary_product_owner):
+            getUtility(IService, 'sharing').sharePillarInformation(
+                self.active_proprietary_product, user,
+                self.proprietary_product_owner,
+                {InformationType.PROPRIETARY: SharingPermission.ALL})
+            getUtility(IService, 'sharing').sharePillarInformation(
+                self.inactive_proprietary_product, user,
+                self.proprietary_product_owner,
+                {InformationType.PROPRIETARY: SharingPermission.ALL})
+        with person_logged_in(user):
+            self.traverse_to_active_public_product()
+            self.assertRaises(
+                NotFound, self.traverse_to_inactive_public_product)
+            self.traverse_to_active_proprietary_product()
+            self.assertRaises(
+                NotFound, self.traverse_to_inactive_proprietary_product)
+
+    def check_admin_access(self):
+        self.traverse_to_active_public_product()
+        self.traverse_to_inactive_public_product()
+        self.traverse_to_active_proprietary_product()
+        self.traverse_to_inactive_proprietary_product()
+
+    def test_access_for_persons_with_special_permissions(self):
+        # Admins have access all products, including inactive and propretary
+        # products.
+        with celebrity_logged_in('admin'):
+            self.check_admin_access()
+        # Registry experts can access to all products.
+        with celebrity_logged_in('registry_experts'):
+            self.check_admin_access()
+        # Commercial admins have access to all products.
+        with celebrity_logged_in('commercial_admin'):
+            self.check_admin_access()

=== modified file 'lib/lp/app/model/launchpad.py'
--- lib/lp/app/model/launchpad.py	2012-10-08 10:07:11 +0000
+++ lib/lp/app/model/launchpad.py	2012-10-15 15:39:22 +0000
@@ -43,7 +43,7 @@
 
 
 class InformationTypeMixin:
-    """"Common functionality for classes implementing IInformationType."""
+    """Common functionality for classes implementing IInformationType."""
 
     @property
     def private(self):

=== modified file 'lib/lp/blueprints/browser/tests/test_specification.py'
--- lib/lp/blueprints/browser/tests/test_specification.py	2012-10-11 12:41:43 +0000
+++ lib/lp/blueprints/browser/tests/test_specification.py	2012-10-15 15:39:22 +0000
@@ -40,6 +40,7 @@
     IProductSeries,
     )
 from lp.services.features.testing import FeatureFixture
+from lp.services.webapp.interaction import ANONYMOUS
 from lp.services.webapp.interfaces import BrowserNotificationLevel
 from lp.services.webapp.publisher import canonical_url
 from lp.testing import (
@@ -480,7 +481,11 @@
             control = browser.getControl(information_type.title)
             if not control.selected:
                 control.click()
-            return product.getSpecification(self.submitSpec(browser))
+            specification_name = self.submitSpec(browser)
+            # Using the browser terminated the interaction, but we need
+            # an interaction in order to access a product.
+            with person_logged_in(ANONYMOUS):
+                return product.getSpecification(specification_name)
 
     def test_supplied_information_types(self):
         """Creating honours information types."""
@@ -564,9 +569,11 @@
         Useful because we need to follow to product from a
         ProductSeries.
         """
-        if IProductSeries.providedBy(target):
-            return target.product.getSpecification(name)
-        return target.getSpecification(name)
+        # We need an interaction in order to access a product.
+        with person_logged_in(ANONYMOUS):
+            if IProductSeries.providedBy(target):
+                return target.product.getSpecification(name)
+            return target.getSpecification(name)
 
     def submitSpec(self, browser):
         """Submit a Specification via a browser."""

=== modified file 'lib/lp/blueprints/browser/tests/test_views.py'
--- lib/lp/blueprints/browser/tests/test_views.py	2012-10-08 10:07:11 +0000
+++ lib/lp/blueprints/browser/tests/test_views.py	2012-10-15 15:39:22 +0000
@@ -66,9 +66,9 @@
         collector = QueryCollector()
         collector.register()
         self.addCleanup(collector.unregister)
+        url = canonical_url(target) + "/+assignments"
         viewer = self.factory.makePerson()
         browser = self.getUserBrowser(user=viewer)
-        url = canonical_url(target) + "/+assignments"
         # Seed the cookie cache and any other cross-request state we may gain
         # in future.  See lp.services.webapp.serssion: _get_secret.
         browser.open(url)

=== modified file 'lib/lp/blueprints/tests/test_webservice.py'
--- lib/lp/blueprints/tests/test_webservice.py	2012-10-08 10:07:11 +0000
+++ lib/lp/blueprints/tests/test_webservice.py	2012-10-15 15:39:22 +0000
@@ -7,6 +7,7 @@
 
 import transaction
 from zope.security.management import endInteraction
+from zope.security.proxy import removeSecurityProxy
 
 from lp.blueprints.enums import SpecificationDefinitionStatus
 from lp.services.webapp.interaction import ANONYMOUS
@@ -42,8 +43,9 @@
         return result
 
     def getPillarOnWebservice(self, pillar_obj):
+        pillar_name = pillar_obj.name
         launchpadlib = self.getLaunchpadlib()
-        return launchpadlib.load(pillar_obj.name)
+        return launchpadlib.load(pillar_name)
 
 
 class SpecificationAttributeWebserviceTests(SpecificationWebserviceTestCase):
@@ -74,9 +76,9 @@
     def test_representation_contains_target(self):
         spec = self.factory.makeSpecification(
             product=self.factory.makeProduct())
-        spec_target = spec.target
+        spec_target_name = spec.target.name
         spec_webservice = self.getSpecOnWebservice(spec)
-        self.assertEqual(spec_target.name, spec_webservice.target.name)
+        self.assertEqual(spec_target_name, spec_webservice.target.name)
 
     def test_representation_contains_title(self):
         spec = self.factory.makeSpecification(title='Foo')
@@ -271,7 +273,7 @@
         # setup a new one.
         endInteraction()
         lplib = launchpadlib_for('lplib-test', person=None, version='devel')
-        ws_product = ws_object(lplib, product)
+        ws_product = ws_object(lplib, removeSecurityProxy(product))
         self.assertNamesOfSpecificationsAre(
             ["spec1", "spec2"], ws_product.all_specifications)
 

=== modified file 'lib/lp/bugs/browser/tests/test_bugtask.py'
--- lib/lp/bugs/browser/tests/test_bugtask.py	2012-10-15 02:32:30 +0000
+++ lib/lp/bugs/browser/tests/test_bugtask.py	2012-10-15 15:39:22 +0000
@@ -2024,8 +2024,10 @@
 
     def test_rendered_query_counts_constant_with_many_bugtasks(self):
         product = self.factory.makeProduct()
+        url = canonical_url(product, view_name='+bugs')
         bug = self.factory.makeBug(target=product)
         buggy_product = self.factory.makeProduct()
+        buggy_url = canonical_url(buggy_product, view_name='+bugs')
         for _ in range(10):
             self.factory.makeBug(target=buggy_product)
         recorder = QueryCollector()
@@ -2033,11 +2035,9 @@
         self.addCleanup(recorder.unregister)
         self.invalidate_caches(bug)
         # count with single task
-        url = canonical_url(product, view_name='+bugs')
         self.getUserBrowser(url)
         self.assertThat(recorder, HasQueryCount(LessThan(35)))
         # count with many tasks
-        buggy_url = canonical_url(buggy_product, view_name='+bugs')
         self.getUserBrowser(buggy_url)
         self.assertThat(recorder, HasQueryCount(LessThan(35)))
 

=== modified file 'lib/lp/bugs/stories/bug-also-affects/xx-bug-also-affects.txt'
--- lib/lp/bugs/stories/bug-also-affects/xx-bug-also-affects.txt	2012-10-08 10:07:11 +0000
+++ lib/lp/bugs/stories/bug-also-affects/xx-bug-also-affects.txt	2012-10-15 15:39:22 +0000
@@ -219,6 +219,7 @@
     >>> other_product = factory.makeProduct(
     ...     official_malone=True,
     ...     bug_sharing_policy=BugSharingPolicy.PROPRIETARY)
+    >>> other_product_name = other_product.name
     >>> params = CreateBugParams(
     ...     title="a test private bug",
     ...     comment="a description of the bug",
@@ -229,7 +230,7 @@
 
     >>> browser.open(canonical_url(private_bug, rootsite='bugs'))
     >>> browser.getLink(url='+choose-affected-product').click()
-    >>> browser.getControl(name='field.product').value = other_product.name
+    >>> browser.getControl(name='field.product').value = other_product_name
     >>> browser.getControl('Continue').click()
     >>> print browser.url
     http://bugs.launchpad.dev/proprietary-product/+bug/16/+choose-affected-product

=== modified file 'lib/lp/bugs/stories/bugs/xx-bug-personal-subscriptions-advanced-features.txt'
--- lib/lp/bugs/stories/bugs/xx-bug-personal-subscriptions-advanced-features.txt	2012-10-08 10:07:11 +0000
+++ lib/lp/bugs/stories/bugs/xx-bug-personal-subscriptions-advanced-features.txt	2012-10-15 15:39:22 +0000
@@ -9,8 +9,9 @@
     >>> login(USER_EMAIL)
     >>> bug = factory.makeBug()
     >>> task = bug.default_bugtask
+    >>> url = canonical_url(task, view_name='+subscribe')
     >>> logout()
-    >>> user_browser.open(canonical_url(task, view_name='+subscribe'))
+    >>> user_browser.open(url)
     >>> bug_notification_level_control = user_browser.getControl(
     ...     name='field.bug_notification_level')
     >>> for control in bug_notification_level_control.controls:

=== modified file 'lib/lp/bugs/stories/patches-view/patches-view.txt'
--- lib/lp/bugs/stories/patches-view/patches-view.txt	2012-10-08 10:07:11 +0000
+++ lib/lp/bugs/stories/patches-view/patches-view.txt	2012-10-15 15:39:22 +0000
@@ -255,9 +255,11 @@
     ...         bugtask.transitionToImportance(importance, ubuntu_distro.owner)
     ...     if status is not None:
     ...         bugtask.transitionToStatus(status, ubuntu_distro.owner)
+    >>> login(ANONYMOUS)
     >>> patchy_product_series = patchy_product.getSeries('trunk')
     >>> make_bugtask(bug=bug_a, target=patchy_product_series)
     >>> make_bugtask(bug=bug_c, target=patchy_product_series)
+    >>> logout()
     >>> anon_browser.open(
     ...     'https://bugs.launchpad.dev/patchy-product-1/trunk/+patches')
     >>> show_patches_view(anon_browser.contents)

=== modified file 'lib/lp/bugs/stories/structural-subscriptions/xx-advanced-features.txt'
--- lib/lp/bugs/stories/structural-subscriptions/xx-advanced-features.txt	2012-10-08 10:07:11 +0000
+++ lib/lp/bugs/stories/structural-subscriptions/xx-advanced-features.txt	2012-10-15 15:39:22 +0000
@@ -8,8 +8,8 @@
     >>> from lp.testing.sampledata import USER_EMAIL
     >>> login(USER_EMAIL)
     >>> product = factory.makeProduct()
+    >>> url = canonical_url(product, view_name='+subscriptions')
     >>> logout()
-    >>> user_browser.open(
-    ...     canonical_url(product, view_name='+subscriptions'))
+    >>> user_browser.open(url)
     >>> user_browser.getLink("Add a subscription")
     <Link text='Add a subscription' url='.../+subscriptions#'>

=== modified file 'lib/lp/bugs/stories/webservice/xx-bug.txt'
--- lib/lp/bugs/stories/webservice/xx-bug.txt	2012-10-08 10:07:11 +0000
+++ lib/lp/bugs/stories/webservice/xx-bug.txt	2012-10-15 15:39:22 +0000
@@ -1463,13 +1463,14 @@
     >>> from lp.testing.sampledata import ADMIN_EMAIL
     >>> login(ADMIN_EMAIL)
     >>> target = factory.makeProduct()
+    >>> target_name = target.name
     >>> bug = factory.makeBug(target=target)
     >>> bug = removeSecurityProxy(bug)
     >>> date = bug.date_last_updated - timedelta(days=6)
     >>> logout()
 
     >>> pprint_collection(webservice.named_get(
-    ...     '/%s' % target.name, 'searchTasks',
+    ...     '/%s' % target_name, 'searchTasks',
     ...     modified_since=u'%s' % date ).jsonBody())
     start: 0
     total_size: 1
@@ -1481,12 +1482,13 @@
     >>> from lp.bugs.interfaces.bugtarget import IBugTarget
     >>> login(ADMIN_EMAIL)
     >>> target = IBugTarget(factory.makeProduct())
+    >>> target_name = target.name
     >>> task = factory.makeBugTask(target=target)
     >>> date = task.datecreated - timedelta(days=8)
     >>> logout()
 
     >>> pprint_collection(webservice.named_get(
-    ...     '/%s' % target.name, 'searchTasks',
+    ...     '/%s' % target_name, 'searchTasks',
     ...     created_since=u'%s' % date).jsonBody())
     start: 0
     total_size: 1
@@ -1497,7 +1499,7 @@
 
     >>> before_date = task.datecreated + timedelta(days=8)
     >>> pprint_collection(webservice.named_get(
-    ...     '/%s' % target.name, 'searchTasks',
+    ...     '/%s' % target_name, 'searchTasks',
     ...     created_before=u'%s' % before_date).jsonBody())
     start: 0
     total_size: 1

=== modified file 'lib/lp/bugs/tests/test_bugs_webservice.py'
--- lib/lp/bugs/tests/test_bugs_webservice.py	2012-10-09 08:39:54 +0000
+++ lib/lp/bugs/tests/test_bugs_webservice.py	2012-10-15 15:39:22 +0000
@@ -355,12 +355,13 @@
     def test_add_duplicate_bugtask_for_project_gives_bad_request(self):
         bug = self.factory.makeBug()
         product = self.factory.makeProduct()
+        product_url = api_url(product)
         self.factory.makeBugTask(bug=bug, target=product)
 
         launchpad = launchpadlib_for('test', bug.owner)
         lp_bug = launchpad.load(api_url(bug))
         self.assertRaises(
-            BadRequest, lp_bug.addTask, target=api_url(product))
+            BadRequest, lp_bug.addTask, target=product_url)
 
     def test_add_invalid_bugtask_to_proprietary_bug_gives_bad_request(self):
         # Test we get an error when we attempt to invalidly add a bug task to
@@ -371,6 +372,7 @@
             bug_sharing_policy=BugSharingPolicy.PROPRIETARY)
         product2 = self.factory.makeProduct(
             bug_sharing_policy=BugSharingPolicy.PROPRIETARY)
+        product2_url = api_url(product2)
         bug = self.factory.makeBug(
             target=product1, owner=owner,
             information_type=InformationType.PROPRIETARY)
@@ -379,7 +381,7 @@
         launchpad = launchpadlib_for('test', owner)
         lp_bug = launchpad.load(api_url(bug))
         self.assertRaises(
-            BadRequest, lp_bug.addTask, target=api_url(product2))
+            BadRequest, lp_bug.addTask, target=product2_url)
 
     def test_add_attachment_with_bad_filename_raises_exception(self):
         # Test that addAttachment raises BadRequest when the filename given
@@ -404,13 +406,14 @@
         # to the project's bug sharing policy.
         project = self.factory.makeProduct(
             licenses=[License.OTHER_PROPRIETARY])
+        target_url = api_url(project)
         with person_logged_in(project.owner):
             project.setBugSharingPolicy(
                 BugSharingPolicy.PROPRIETARY_OR_PUBLIC)
         webservice = launchpadlib_for('test', 'salgado')
         bugs_collection = webservice.load('/bugs')
         bug = bugs_collection.createBug(
-            target=api_url(project), title='title', description='desc')
+            target=target_url, title='title', description='desc')
         self.assertEqual('Proprietary', bug.information_type)
 
 
@@ -430,7 +433,7 @@
 
     def test_subscribe_does_not_update(self):
         # Calling subscribe over the API does not update date_last_updated.
-        (bug, owner, webservice)  = self.make_old_bug()
+        (bug, owner, webservice) = self.make_old_bug()
         subscriber = self.factory.makePerson()
         date_last_updated = bug.date_last_updated
         api_sub = api_url(subscriber)

=== modified file 'lib/lp/bugs/tests/test_searchtasks_webservice.py'
--- lib/lp/bugs/tests/test_searchtasks_webservice.py	2012-10-08 10:07:11 +0000
+++ lib/lp/bugs/tests/test_searchtasks_webservice.py	2012-10-15 15:39:22 +0000
@@ -54,6 +54,7 @@
         self.owner = self.factory.makePerson()
         with person_logged_in(self.owner):
             self.product = self.factory.makeProduct()
+        self.product_name = self.product.name
         self.bug = self.factory.makeBug(
             target=self.product,
             information_type=InformationType.PRIVATESECURITY)
@@ -62,7 +63,7 @@
 
     def search(self, api_version, **kwargs):
         return self.webservice.named_get(
-            '/%s' % self.product.name, 'searchTasks',
+            '/%s' % self.product_name, 'searchTasks',
             api_version=api_version, **kwargs).jsonBody()
 
     def test_linked_blueprints_in_devel(self):
@@ -102,7 +103,7 @@
     def test_search_with_wrong_orderby(self):
         # Calling searchTasks() with a wrong order_by is a Bad Request.
         response = self.webservice.named_get(
-            '/%s' % self.product.name, 'searchTasks',
+            '/%s' % self.product_name, 'searchTasks',
             api_version='devel', order_by='date_created')
         self.assertEqual(400, response.status)
         self.assertRaisesWithContent(

=== modified file 'lib/lp/code/browser/tests/test_branch.py'
--- lib/lp/code/browser/tests/test_branch.py	2012-10-09 00:05:04 +0000
+++ lib/lp/code/browser/tests/test_branch.py	2012-10-15 15:39:22 +0000
@@ -694,11 +694,11 @@
             base_url = canonical_url(branch, rootsite='code')
             product_url = canonical_url(product, rootsite='code')
         url = '%s/+subscription/%s' % (base_url, subscriber.name)
+        expected_title = "Code : %s" % product.displayname
         browser = self._getBrowser(user=subscriber)
         browser.open(url)
         browser.getControl('Unsubscribe').click()
         self.assertEqual(product_url, browser.url)
-        expected_title = "Code : %s" % product.displayname
         self.assertEqual(expected_title, browser.title)
 
 

=== modified file 'lib/lp/code/browser/tests/test_product.py'
--- lib/lp/code/browser/tests/test_product.py	2012-10-08 14:27:31 +0000
+++ lib/lp/code/browser/tests/test_product.py	2012-10-15 15:39:22 +0000
@@ -222,8 +222,9 @@
         login_person(product.owner)
         product.development_focus.branch = code_import.branch
         self.assertEqual(ServiceUsage.EXTERNAL, product.codehosting_usage)
+        product_url = canonical_url(product, rootsite='code')
         logout()
-        browser = self.getUserBrowser(canonical_url(product, rootsite='code'))
+        browser = self.getUserBrowser(product_url)
         login(ANONYMOUS)
         content = find_tag_by_id(browser.contents, 'external')
         text = extract_text(content)
@@ -379,6 +380,7 @@
 
     def test_is_public(self):
         product = self.factory.makeProduct()
+        product_displayname = product.displayname
         branch = self.factory.makeProductBranch(product=product)
         login_person(product.owner)
         product.development_focus.branch = branch
@@ -386,7 +388,7 @@
         text = extract_text(find_tag_by_id(browser.contents, 'privacy'))
         expected = (
             "New branches for %(name)s are Public.*"
-            % dict(name=product.displayname))
+            % dict(name=product_displayname))
         self.assertTextMatchesExpressionIgnoreWhitespace(expected, text)
 
 

=== modified file 'lib/lp/code/stories/branches/xx-product-branches.txt'
--- lib/lp/code/stories/branches/xx-product-branches.txt	2012-10-11 04:14:37 +0000
+++ lib/lp/code/stories/branches/xx-product-branches.txt	2012-10-15 15:39:22 +0000
@@ -180,10 +180,10 @@
     >>> factory = LaunchpadObjectFactory()
     >>> login(ANONYMOUS)
     >>> product = getUtility(IProductSet).getByName('firefox')
+    >>> owner = product.owner
     >>> old_branch = product.development_focus.branch
     >>> ignored = login_person(product.owner)
     >>> product.development_focus.branch = None
-    >>> logout()
     >>> def print_links(browser):
     ...     links = find_tag_by_id(browser.contents, 'involvement')
     ...     if links is None:
@@ -203,13 +203,16 @@
 
     >>> print product.codehosting_usage.name
     UNKNOWN
+    >>> logout()
     >>> admin_browser.open('http://code.launchpad.dev/firefox')
     >>> print_links(admin_browser)
     None
 
     >>> setup_code_hosting('firefox')
+    >>> login(ANONYMOUS)
     >>> print product.codehosting_usage.name
     LAUNCHPAD
+    >>> logout()
     >>> admin_browser.open('http://code.launchpad.dev/firefox')
     >>> print_links(admin_browser)
     Import a branch
@@ -233,7 +236,7 @@
 If the product specifies that it officially uses Launchpad code, then
 the 'Import a branch' button is still shown.
 
-    >>> ignored = login_person(product.owner)
+    >>> ignored = login_person(owner)
     >>> product.development_focus.branch = old_branch
     >>> logout()
     >>> browser.open('http://code.launchpad.dev/firefox')

=== modified file 'lib/lp/code/stories/webservice/xx-code-import.txt'
--- lib/lp/code/stories/webservice/xx-code-import.txt	2012-10-09 00:10:04 +0000
+++ lib/lp/code/stories/webservice/xx-code-import.txt	2012-10-15 15:39:22 +0000
@@ -17,6 +17,7 @@
     >>> other_person = factory.makePerson(name='other-person')
     >>> removeSecurityProxy(person).join(team)
     >>> product = factory.makeProduct(name='scruff')
+    >>> product_name = product.name
     >>> svn_branch_url = "http://svn.domain.com/source"
     >>> code_import = removeSecurityProxy(factory.makeProductCodeImport(
     ...    registrant=person, product=product, branch_name='import',
@@ -122,7 +123,7 @@
 
 We can create an import using the API by calling a method on the project.
 
-    >>> product_url = '/' + product.name
+    >>> product_url = '/' + product_name
     >>> new_remote_url = factory.getUniqueURL()
     >>> response = import_webservice.named_post(product_url, 'newCodeImport',
     ...    branch_name='new-import', rcs_type='Git',
@@ -149,7 +150,7 @@
 
 If we must we can create a CVS import.
 
-    >>> product_url = '/' + product.name
+    >>> product_url = '/' + product_name
     >>> new_remote_url = factory.getUniqueURL()
     >>> response = import_webservice.named_post(product_url, 'newCodeImport',
     ...    branch_name='cvs-import', rcs_type='Concurrent Versions System',

=== modified file 'lib/lp/registry/browser/tests/test_milestone.py'
--- lib/lp/registry/browser/tests/test_milestone.py	2012-10-08 10:07:11 +0000
+++ lib/lp/registry/browser/tests/test_milestone.py	2012-10-15 15:39:22 +0000
@@ -234,12 +234,13 @@
         super(TestProjectMilestoneIndexQueryCount, self).setUp()
         self.owner = self.factory.makePerson(name='product-owner')
         self.product = self.factory.makeProduct(owner=self.owner)
+        self.product_owner = self.product.owner
         login_person(self.product.owner)
         self.milestone = self.factory.makeMilestone(
             productseries=self.product.development_focus)
 
     def add_bug(self, count):
-        login_person(self.product.owner)
+        login_person(self.product_owner)
         for i in range(count):
             bug = self.factory.makeBug(target=self.product)
             bug.bugtasks[0].transitionToMilestone(
@@ -284,6 +285,7 @@
         # increasing the cap.
         page_query_limit = 37
         product = self.factory.makeProduct()
+        product_owner = product.owner
         login_person(product.owner)
         milestone = self.factory.makeMilestone(
             productseries=product.development_focus)
@@ -316,7 +318,7 @@
         with_1_private_bug = collector.count
         with_1_queries = ["%s: %s" % (pos, stmt[3]) for (pos, stmt) in
             enumerate(collector.queries)]
-        login_person(product.owner)
+        login_person(product_owner)
         bug2 = self.factory.makeBug(
             target=product, information_type=InformationType.USERDATA,
             owner=product.owner)

=== modified file 'lib/lp/registry/browser/tests/test_pillar_sharing.py'
--- lib/lp/registry/browser/tests/test_pillar_sharing.py	2012-10-08 10:07:11 +0000
+++ lib/lp/registry/browser/tests/test_pillar_sharing.py	2012-10-15 15:39:22 +0000
@@ -243,10 +243,10 @@
 
     def test_sharing_menu(self):
         url = canonical_url(self.pillar)
-        browser = setupBrowserForUser(user=self.driver)
-        browser.open(url)
-        soup = BeautifulSoup(browser.contents)
         sharing_url = canonical_url(self.pillar, view_name='+sharing')
+        browser = setupBrowserForUser(user=self.driver)
+        browser.open(url)
+        soup = BeautifulSoup(browser.contents)
         sharing_menu = soup.find('a', {'href': sharing_url})
         self.assertIsNotNone(sharing_menu)
 

=== modified file 'lib/lp/registry/browser/tests/test_product.py'
--- lib/lp/registry/browser/tests/test_product.py	2012-10-12 20:17:12 +0000
+++ lib/lp/registry/browser/tests/test_product.py	2012-10-15 15:39:22 +0000
@@ -631,8 +631,8 @@
     def test_headers(self):
         """The headers for the RDF view of a product should be as expected."""
         product = self.factory.makeProduct()
+        content_disposition = 'attachment; filename="%s.rdf"' % product.name
         browser = self.getViewBrowser(product, view_name='+rdf')
-        content_disposition = 'attachment; filename="%s.rdf"' % product.name
         self.assertEqual(
             content_disposition, browser.headers['Content-disposition'])
         self.assertEqual(

=== modified file 'lib/lp/registry/browser/tests/test_productseries_views.py'
--- lib/lp/registry/browser/tests/test_productseries_views.py	2012-10-11 16:27:29 +0000
+++ lib/lp/registry/browser/tests/test_productseries_views.py	2012-10-15 15:39:22 +0000
@@ -8,12 +8,14 @@
 
 import soupmatchers
 from testtools.matchers import Not
+from zope.security.proxy import removeSecurityProxy
 
 from lp.app.enums import InformationType
 from lp.bugs.interfaces.bugtask import (
     BugTaskStatus,
     BugTaskStatusSearch,
     )
+from lp.services.webapp import canonical_url
 from lp.testing import (
     BrowserTestCase,
     person_logged_in,
@@ -60,6 +62,11 @@
         browser = self.getViewBrowser(series)
         self.assertThat(browser.contents, soupmatchers.HTMLContains(tag))
 
+    def getBrowser(self, series, view_name=None):
+        series = removeSecurityProxy(series)
+        url = canonical_url(series, view_name=view_name)
+        return self.getUserBrowser(url, series.product.owner)
+
     def test_package_proprietary_error(self):
         """Packaging a proprietary product produces an error."""
         product = self.factory.makeProduct(
@@ -68,7 +75,7 @@
         ubuntu_series = self.factory.makeUbuntuDistroSeries()
         sp = self.factory.makeSourcePackage(distroseries=ubuntu_series,
                                             publish=True)
-        browser = self.getViewBrowser(productseries, '+ubuntupkg')
+        browser = self.getBrowser(productseries, '+ubuntupkg')
         browser.getControl('Source Package Name').value = (
             sp.sourcepackagename.name)
         browser.getControl(ubuntu_series.displayname).selected = True
@@ -84,7 +91,7 @@
         product = self.factory.makeProduct(
             information_type=InformationType.PROPRIETARY)
         series = self.factory.makeProductSeries(product=product)
-        browser = self.getViewBrowser(series)
+        browser = self.getBrowser(series)
         tag = soupmatchers.Tag(
             'portlet-packages', True, attrs={'id': 'portlet-packages'})
         self.assertThat(browser.contents, Not(soupmatchers.HTMLContains(tag)))

=== modified file 'lib/lp/registry/browser/tests/test_sourcepackage_views.py'
--- lib/lp/registry/browser/tests/test_sourcepackage_views.py	2012-10-11 20:33:03 +0000
+++ lib/lp/registry/browser/tests/test_sourcepackage_views.py	2012-10-15 15:39:22 +0000
@@ -297,12 +297,16 @@
 
     def test_error_on_proprietary_product(self):
         """Packaging cannot be created for PROPRIETARY products"""
+        product_owner = self.factory.makePerson()
+        product_name = 'proprietary-product'
         product = self.factory.makeProduct(
+            name=product_name, owner=product_owner,
             information_type=InformationType.PROPRIETARY)
         ubuntu_series = self.factory.makeUbuntuDistroSeries()
         sp = self.factory.makeSourcePackage(distroseries=ubuntu_series)
-        browser = self.getViewBrowser(sp, '+edit-packaging')
-        browser.getControl('Project').value = product.name
+        browser = self.getViewBrowser(
+            sp, '+edit-packaging', user=product_owner)
+        browser.getControl('Project').value = product_name
         browser.getControl('Continue').click()
         self.assertIn(
             'Only Public projects can be packaged, not Proprietary.',
@@ -310,14 +314,18 @@
 
     def test_error_on_proprietary_productseries(self):
         """Packaging cannot be created for PROPRIETARY productseries"""
-        product = self.factory.makeProduct()
+        product_owner = self.factory.makePerson()
+        product_name = 'proprietary-product'
+        product = self.factory.makeProduct(
+            name=product_name, owner=product_owner)
         series = self.factory.makeProductSeries(product=product)
         ubuntu_series = self.factory.makeUbuntuDistroSeries()
         sp = self.factory.makeSourcePackage(distroseries=ubuntu_series)
-        browser = self.getViewBrowser(sp, '+edit-packaging')
-        browser.getControl('Project').value = product.name
+        browser = self.getViewBrowser(
+            sp, '+edit-packaging', user=product_owner)
+        browser.getControl('Project').value = product_name
         browser.getControl('Continue').click()
-        with person_logged_in(product.owner):
+        with person_logged_in(product_owner):
             product.information_type = InformationType.PROPRIETARY
         browser.getControl(series.displayname).selected = True
         browser.getControl('Change').click()

=== modified file 'lib/lp/registry/configure.zcml'
--- lib/lp/registry/configure.zcml	2012-10-08 10:07:11 +0000
+++ lib/lp/registry/configure.zcml	2012-10-15 15:39:22 +0000
@@ -1229,11 +1229,19 @@
         class="lp.registry.model.product.Product">
         <allow
             interface="lp.registry.interfaces.product.IProductPublic"/>
-        <allow interface="lp.bugs.interfaces.bugsummary.IBugSummaryDimension"/>
         <allow
+            interface="lp.registry.interfaces.pillar.IPillar"/>
+        <require
+            permission="launchpad.View"
+            interface="lp.registry.interfaces.product.IProductLimitedView"/>
+        <require
+            permission="launchpad.View"
+            interface="lp.bugs.interfaces.bugsummary.IBugSummaryDimension"/>
+        <require
+            permission="launchpad.View"
             interface="lp.translations.interfaces.customlanguagecode.IHasCustomLanguageCodes"/>
         <require
-            permission="launchpad.View"
+            permission="launchpad.AnyAllowedPerson"
             set_attributes="date_next_suggest_packaging"/>
         <require
             permission="launchpad.Driver"
@@ -1316,7 +1324,7 @@
                 translationpermission
                 translations_usage"/>
         <require
-            permission="zope.Public"
+            permission="launchpad.View"
             attributes="
                 qualifies_for_free_hosting"/>
         <require
@@ -1331,7 +1339,8 @@
 
         <!-- IHasAliases -->
 
-        <allow
+        <require
+            permission="launchpad.View"
             attributes="
                 aliases"/>
         <require
@@ -1341,14 +1350,17 @@
 
         <!-- IQuestionTarget -->
 
-        <allow interface="lp.answers.interfaces.questiontarget.IQuestionTargetPublic"/>
-        <require
-          permission="launchpad.AnyPerson"
+        <require
+            permission="launchpad.View"
+            interface="lp.answers.interfaces.questiontarget.IQuestionTargetPublic"/>
+        <require
+          permission="launchpad.AnyAllowedPerson"
           interface="lp.answers.interfaces.questiontarget.IQuestionTargetView"/>
 
         <!-- IFAQTarget -->
 
-        <allow
+        <require
+            permission="launchpad.View"
             interface="lp.answers.interfaces.faqcollection.IFAQCollection"
             attributes="
                 findSimilarFAQs"/>
@@ -1359,15 +1371,17 @@
 
         <!-- IStructuralSubscriptionTarget -->
 
-        <allow
+        <require
+            permission="launchpad.View"
             interface="lp.bugs.interfaces.structuralsubscription.IStructuralSubscriptionTargetRead" />
         <require
-            permission="launchpad.AnyPerson"
+            permission="launchpad.AnyAllowedPerson"
             interface="lp.bugs.interfaces.structuralsubscription.IStructuralSubscriptionTargetWrite" />
 
         <!-- IHasBugSupervisor -->
 
-        <allow
+        <require
+            permission="launchpad.View"
             attributes="
                 bug_supervisor"/>
         <require

=== modified file 'lib/lp/registry/interfaces/product.py'
--- lib/lp/registry/interfaces/product.py	2012-10-11 07:45:33 +0000
+++ lib/lp/registry/interfaces/product.py	2012-10-15 15:39:22 +0000
@@ -420,18 +420,24 @@
                 "Not applicable to 'Other/Proprietary'.")))
 
 
-class IProductPublic(
+class IProductPublic(Interface):
+
+    id = Int(title=_('The Project ID'))
+
+    def userCanView(user):
+        """True if the given user has access to this product."""
+
+
+class IProductLimitedView(
     IBugTarget, ICanGetMilestonesDirectly, IHasAppointedDriver, IHasBranches,
-    IHasDrivers, IHasExternalBugTracker, IHasIcon, IHasLogo,
-    IHasMergeProposals, IHasMilestones, IHasExpirableBugs, IHasMugshot,
-    IHasOwner, IHasSprints, IHasTranslationImports, ITranslationPolicy,
-    IKarmaContext, ILaunchpadUsage, IMakesAnnouncements,
-    IOfficialBugTagTargetPublic, IHasOOPSReferences, IPillar,
+    IHasDrivers, IHasExternalBugTracker, IHasIcon,
+    IHasLogo, IHasMergeProposals, IHasMilestones, IHasExpirableBugs,
+    IHasMugshot, IHasOwner, IHasSprints, IHasTranslationImports,
+    ITranslationPolicy, IKarmaContext, ILaunchpadUsage, IMakesAnnouncements,
+    IOfficialBugTagTargetPublic, IHasOOPSReferences,
     ISpecificationTarget, IHasRecipes, IHasCodeImports, IServiceUsage):
     """Public IProduct properties."""
 
-    id = Int(title=_('The Project ID'))
-
     project = exported(
         ReferenceChoice(
             title=_('Part of'),
@@ -868,9 +874,9 @@
 class IProductEditRestricted(IOfficialBugTagTargetRestricted):
     """`IProduct` properties which require launchpad.Edit permission."""
 
-    @mutator_for(IProductPublic['bug_sharing_policy'])
+    @mutator_for(IProductLimitedView['bug_sharing_policy'])
     @operation_parameters(bug_sharing_policy=copy_field(
-        IProductPublic['bug_sharing_policy']))
+        IProductLimitedView['bug_sharing_policy']))
     @export_write_operation()
     @operation_for_version("devel")
     def setBugSharingPolicy(bug_sharing_policy):
@@ -879,10 +885,10 @@
         Checks authorization and entitlement.
         """
 
-    @mutator_for(IProductPublic['branch_sharing_policy'])
+    @mutator_for(IProductLimitedView['branch_sharing_policy'])
     @operation_parameters(
         branch_sharing_policy=copy_field(
-            IProductPublic['branch_sharing_policy']))
+            IProductLimitedView['branch_sharing_policy']))
     @export_write_operation()
     @operation_for_version("devel")
     def setBranchSharingPolicy(branch_sharing_policy):
@@ -891,10 +897,10 @@
         Checks authorization and entitlement.
         """
 
-    @mutator_for(IProductPublic['specification_sharing_policy'])
+    @mutator_for(IProductLimitedView['specification_sharing_policy'])
     @operation_parameters(
         specification_sharing_policy=copy_field(
-            IProductPublic['specification_sharing_policy']))
+            IProductLimitedView['specification_sharing_policy']))
     @export_write_operation()
     @operation_for_version("devel")
     def setSpecificationSharingPolicy(specification_sharing_policy):
@@ -907,8 +913,8 @@
 class IProduct(
     IHasBugSupervisor, IProductEditRestricted,
     IProductModerateRestricted, IProductDriverRestricted,
-    IProductPublic, IQuestionTarget, IRootContext,
-    IStructuralSubscriptionTarget, IInformationType):
+    IProductLimitedView, IProductPublic, IQuestionTarget, IRootContext,
+    IStructuralSubscriptionTarget, IInformationType, IPillar):
     """A Product.
 
     The Launchpad Registry describes the open source world as ProjectGroups

=== modified file 'lib/lp/registry/model/product.py'
--- lib/lp/registry/model/product.py	2012-10-12 18:23:18 +0000
+++ lib/lp/registry/model/product.py	2012-10-15 15:39:22 +0000
@@ -69,6 +69,7 @@
     InformationType,
     PRIVATE_INFORMATION_TYPES,
     PROPRIETARY_INFORMATION_TYPES,
+    PUBLIC_INFORMATION_TYPES,
     PUBLIC_PROPRIETARY_INFORMATION_TYPES,
     service_uses_launchpad,
     ServiceUsage,
@@ -155,6 +156,7 @@
     LicenseStatus,
     )
 from lp.registry.interfaces.productrelease import IProductReleaseSet
+from lp.registry.interfaces.role import IPersonRoles
 from lp.registry.model.announcement import MakesAnnouncements
 from lp.registry.model.commercialsubscription import CommercialSubscription
 from lp.registry.model.distribution import Distribution
@@ -1519,6 +1521,39 @@
 
         return weight_function
 
+    @cachedproperty
+    def _known_viewers(self):
+        """A set of known persons able to view this product."""
+        return set()
+
+    def userCanView(self, user):
+        """See `IProductPublic`."""
+        if self.information_type in PUBLIC_INFORMATION_TYPES:
+            return True
+        if user is None:
+            return False
+        if user.id in self._known_viewers:
+            return True
+        # We need the plain Storm Person object for the SQL query below
+        # but an IPersonRoles object for the team membership checks.
+        if IPersonRoles.providedBy(user):
+            plain_user = user.person
+        else:
+            plain_user = user
+            user = IPersonRoles(user)
+        if (user.in_commercial_admin or user.in_admin or
+            user.in_registry_experts):
+            self._known_viewers.add(user.id)
+            return True
+        policy = getUtility(IAccessPolicySource).find(
+            [(self, self.information_type)]).one()
+        grants_for_user = getUtility(IAccessPolicyGrantSource).find(
+            [(policy, plain_user)])
+        if grants_for_user.is_empty():
+            return False
+        self._known_viewers.add(user.id)
+        return True
+
 
 def get_precached_products(products, need_licences=False, need_projects=False,
                         need_series=False, need_releases=False,

=== modified file 'lib/lp/registry/services/tests/test_sharingservice.py'
--- lib/lp/registry/services/tests/test_sharingservice.py	2012-10-11 04:14:37 +0000
+++ lib/lp/registry/services/tests/test_sharingservice.py	2012-10-15 15:39:22 +0000
@@ -1837,12 +1837,14 @@
             api_method, api_version='devel', **kwargs).jsonBody()
 
     def _getPillarGranteeData(self):
-        pillar_uri = canonical_url(self.pillar, force_local_path=True)
+        pillar_uri = canonical_url(
+            removeSecurityProxy(self.pillar), force_local_path=True)
         return self._named_get(
             'getPillarGranteeData', pillar=pillar_uri)
 
     def _sharePillarInformation(self, pillar):
-        pillar_uri = canonical_url(pillar, force_local_path=True)
+        pillar_uri = canonical_url(
+            removeSecurityProxy(pillar), force_local_path=True)
         return self._named_post(
             'sharePillarInformation', pillar=pillar_uri,
             grantee=self.grantee_uri,

=== modified file 'lib/lp/registry/tests/test_pillaraffiliation.py'
--- lib/lp/registry/tests/test_pillaraffiliation.py	2012-10-08 10:07:11 +0000
+++ lib/lp/registry/tests/test_pillaraffiliation.py	2012-10-15 15:39:22 +0000
@@ -150,7 +150,7 @@
         Store.of(product).invalidate()
         with StormStatementRecorder() as recorder:
             IHasAffiliation(product).getAffiliationBadges([person])
-        self.assertThat(recorder, HasQueryCount(Equals(5)))
+        self.assertThat(recorder, HasQueryCount(Equals(4)))
 
     def test_distro_affiliation_query_count(self):
         # Only 2 business queries are expected, selects from:

=== modified file 'lib/lp/registry/tests/test_product.py'
--- lib/lp/registry/tests/test_product.py	2012-10-11 14:05:29 +0000
+++ lib/lp/registry/tests/test_product.py	2012-10-15 15:39:22 +0000
@@ -15,6 +15,10 @@
 import transaction
 from zope.component import getUtility
 from zope.lifecycleevent.interfaces import IObjectModifiedEvent
+from zope.security.checker import (
+    CheckerPublic,
+    getChecker,
+    )
 from zope.security.interfaces import Unauthorized
 from zope.security.proxy import removeSecurityProxy
 
@@ -44,6 +48,7 @@
     BugSharingPolicy,
     EXCLUSIVE_TEAM_POLICY,
     INCLUSIVE_TEAM_POLICY,
+    SharingPermission,
     SpecificationSharingPolicy,
     )
 from lp.registry.errors import (
@@ -56,11 +61,13 @@
     IAccessPolicySource,
     )
 from lp.registry.interfaces.oopsreferences import IHasOOPSReferences
+from lp.registry.interfaces.person import IPersonSet
 from lp.registry.interfaces.product import (
     IProduct,
     IProductSet,
     License,
     )
+from lp.registry.interfaces.role import IPersonRoles
 from lp.registry.interfaces.series import SeriesStatus
 from lp.registry.model.product import (
     Product,
@@ -72,6 +79,7 @@
     celebrity_logged_in,
     login,
     person_logged_in,
+    StormStatementRecorder,
     TestCase,
     TestCaseWithFactory,
     WebServiceTestCase,
@@ -387,7 +395,7 @@
             license=License.OTHER_PROPRIETARY)
         self.assertEqual(InformationType.EMBARGOED, product.information_type)
         # Owner can set information_type
-        with person_logged_in(product.owner):
+        with person_logged_in(removeSecurityProxy(product).owner):
             product.information_type = InformationType.PROPRIETARY
         self.assertEqual(InformationType.PROPRIETARY, product.information_type)
         # Database persists information_type value
@@ -399,7 +407,9 @@
 
     def test_product_information_type_default(self):
         # Default information_type is PUBLIC
-        product = self.createProduct()
+        owner = self.factory.makePerson()
+        product = getUtility(IProductSet).createProduct(
+            owner, 'fnord', 'Fnord', 'Fnord', 'test 1', 'test 2')
         self.assertEqual(InformationType.PUBLIC, product.information_type)
 
     invalid_information_types = [info_type for info_type in
@@ -525,6 +535,349 @@
                 CannotChangeInformationType, 'Some series are packaged.'):
                 product.information_type = InformationType.PROPRIETARY
 
+    def check_permissions(self, expected_permissions, used_permissions,
+                          type_):
+        expected = set(expected_permissions.keys())
+        self.assertEqual(
+            expected, set(used_permissions.values()),
+            'Unexpected %s permissions' % type_)
+        for permission in expected_permissions:
+            attribute_names = set(
+                name for name, value in used_permissions.items()
+                if value == permission)
+            self.assertEqual(
+                expected_permissions[permission], attribute_names,
+                'Unexpected set of attributes with %s permission %s:\n'
+                'Defined but not expected: %s\n'
+                'Expected but not defined: %s'
+                % (
+                    type_, permission,
+                    sorted(
+                        attribute_names - expected_permissions[permission]),
+                    sorted(
+                        expected_permissions[permission] - attribute_names)))
+
+    expected_get_permissions = {
+        CheckerPublic: set((
+            'active', 'id', 'information_type', 'pillar_category', 'private',
+            'userCanView',)),
+        'launchpad.View': set((
+            '_getOfficialTagClause', '_all_specifications',
+            '_valid_specifications', 'active_or_packaged_series',
+            'aliases', 'all_milestones',
+            'allowsTranslationEdits', 'allowsTranslationSuggestions',
+            'announce', 'answer_contacts', 'answers_usage', 'autoupdate',
+            'blueprints_usage', 'branch_sharing_policy',
+            'bug_reported_acknowledgement', 'bug_reporting_guidelines',
+            'bug_sharing_policy', 'bug_subscriptions', 'bug_supervisor',
+            'bug_tracking_usage', 'bugtargetdisplayname', 'bugtargetname',
+            'bugtracker', 'canUserAlterAnswerContact',
+            'codehosting_usage',
+            'coming_sprints', 'commercial_subscription',
+            'commercial_subscription_is_due', 'createBug',
+            'createCustomLanguageCode', 'custom_language_codes',
+            'date_next_suggest_packaging', 'datecreated', 'description',
+            'development_focus', 'development_focusID',
+            'direct_answer_contacts', 'displayname', 'distrosourcepackages',
+            'downloadurl', 'driver', 'drivers', 'enable_bug_expiration',
+            'enable_bugfiling_duplicate_search', 'findReferencedOOPS',
+            'findSimilarFAQs', 'findSimilarQuestions', 'freshmeatproject',
+            'getAllowedBugInformationTypes',
+            'getAllowedSpecificationInformationTypes', 'getAnnouncement',
+            'getAnnouncements', 'getAnswerContactsForLanguage',
+            'getAnswerContactRecipients', 'getBranches',
+            'getBugSummaryContextWhereClause', 'getBugTaskWeightFunction',
+            'getCustomLanguageCode', 'getDefaultBugInformationType',
+            'getDefaultSpecificationInformationType',
+            'getEffectiveTranslationPermission', 'getExternalBugTracker',
+            'getFAQ', 'getFirstEntryToImport', 'getLinkedBugWatches',
+            'getMergeProposals', 'getMilestone', 'getMilestonesAndReleases',
+            'getQuestion', 'getQuestionLanguages', 'getPackage', 'getRelease',
+            'getSeries', 'getSpecification', 'getSubscription',
+            'getSubscriptions', 'getSupportedLanguages', 'getTimeline',
+            'getTopContributors', 'getTopContributorsGroupedByCategory',
+            'getTranslationGroups', 'getTranslationImportQueueEntries',
+            'getTranslators', 'getUsedBugTagsWithOpenCounts',
+            'getVersionSortedSeries',
+            'has_current_commercial_subscription',
+            'has_custom_language_codes', 'has_milestones', 'homepage_content',
+            'homepageurl', 'icon', 'invitesTranslationEdits',
+            'invitesTranslationSuggestions',
+            'license_info', 'license_status', 'licenses', 'logo', 'milestones',
+            'mugshot', 'name', 'name_with_project', 'newCodeImport',
+            'obsolete_translatable_series', 'official_answers',
+            'official_anything', 'official_blueprints', 'official_bug_tags',
+            'official_codehosting', 'official_malone', 'owner',
+            'parent_subscription_target', 'packagedInDistros', 'packagings',
+            'past_sprints', 'personHasDriverRights', 'pillar',
+            'primary_translatable', 'private_bugs',
+            'programminglang', 'project', 'qualifies_for_free_hosting',
+            'recipes', 'redeemSubscriptionVoucher', 'registrant', 'releases',
+            'remote_product', 'removeCustomLanguageCode',
+            'screenshotsurl',
+            'searchFAQs', 'searchQuestions', 'searchTasks', 'security_contact',
+            'series',
+            'sharesTranslationsWithOtherSide', 'sourceforgeproject',
+            'sourcepackages', 'specification_sharing_policy', 'specifications',
+            'sprints', 'summary', 'target_type_display', 'title',
+            'translatable_packages', 'translatable_series',
+            'translation_focus', 'translationgroup', 'translationgroups',
+            'translationpermission', 'translations_usage', 'ubuntu_packages',
+            'userCanAlterBugSubscription', 'userCanAlterSubscription',
+            'userCanEdit', 'userHasBugSubscriptions', 'uses_launchpad',
+            'wikiurl')),
+        'launchpad.AnyAllowedPerson': set((
+            'addAnswerContact', 'addBugSubscription',
+            'addBugSubscriptionFilter', 'addSubscription',
+            'createQuestionFromBug', 'newQuestion', 'removeAnswerContact',
+            'removeBugSubscription')),
+        'launchpad.Append': set(('newFAQ', )),
+        'launchpad.Driver': set(('newSeries', )),
+        'launchpad.Edit': set((
+            'addOfficialBugTag', 'removeOfficialBugTag',
+            'setBranchSharingPolicy', 'setBugSharingPolicy',
+            'setSpecificationSharingPolicy')),
+        'launchpad.Moderate': set((
+            'is_permitted', 'license_approved', 'project_reviewed',
+            'reviewer_whiteboard', 'setAliases')),
+        }
+
+    def test_get_permissions(self):
+        product = self.factory.makeProduct()
+        checker = getChecker(product)
+        self.check_permissions(
+            self.expected_get_permissions, checker.get_permissions, 'get')
+
+    def test_set_permissions(self):
+        expected_set_permissions = {
+            'launchpad.BugSupervisor': set((
+                'bug_reported_acknowledgement', 'bug_reporting_guidelines',
+                'bugtracker', 'enable_bug_expiration',
+                'enable_bugfiling_duplicate_search', 'official_bug_tags',
+                'official_malone', 'remote_product')),
+            'launchpad.Edit': set((
+                'answers_usage', 'blueprints_usage', 'bug_supervisor',
+                'bug_tracking_usage', 'codehosting_usage',
+                'commercial_subscription', 'description', 'development_focus',
+                'displayname', 'downloadurl', 'driver', 'freshmeatproject',
+                'homepage_content', 'homepageurl', 'icon', 'information_type',
+                'license_info', 'licenses', 'logo', 'mugshot',
+                'official_answers', 'official_blueprints',
+                'official_codehosting', 'owner', 'private',
+                'programminglang', 'project', 'redeemSubscriptionVoucher',
+                'releaseroot', 'screenshotsurl', 'sourceforgeproject',
+                'summary', 'title', 'uses_launchpad', 'wikiurl')),
+            'launchpad.Moderate': set((
+                'active', 'autoupdate', 'license_approved', 'name',
+                'project_reviewed', 'registrant', 'reviewer_whiteboard')),
+            'launchpad.TranslationsAdmin': set((
+                'translation_focus', 'translationgroup',
+                'translationpermission', 'translations_usage')),
+            'launchpad.AnyAllowedPerson': set((
+                'date_next_suggest_packaging', )),
+            }
+        product = self.factory.makeProduct()
+        checker = getChecker(product)
+        self.check_permissions(
+            expected_set_permissions, checker.set_permissions, 'set')
+
+    def test_access_launchpad_View_public_product(self):
+        # Everybody, including anonymous users, has access to
+        # properties of public products that require the permission
+        # launchpad.View
+        product = self.factory.makeProduct()
+        names = self.expected_get_permissions['launchpad.View']
+        with person_logged_in(None):
+            for attribute_name in names:
+                getattr(product, attribute_name)
+        ordinary_user = self.factory.makePerson()
+        with person_logged_in(ordinary_user):
+            for attribute_name in names:
+                getattr(product, attribute_name)
+        with person_logged_in(product.owner):
+            for attribute_name in names:
+                getattr(product, attribute_name)
+
+    def test_access_launchpad_View_public_inactive_product(self):
+        # Everybody, including anonymous users, has access to
+        # properties of public but inactvie products that require
+        # the permission launchpad.View.
+        product = self.factory.makeProduct()
+        removeSecurityProxy(product).active = False
+        names = self.expected_get_permissions['launchpad.View']
+        with person_logged_in(None):
+            for attribute_name in names:
+                getattr(product, attribute_name)
+        ordinary_user = self.factory.makePerson()
+        with person_logged_in(ordinary_user):
+            for attribute_name in names:
+                getattr(product, attribute_name)
+        with person_logged_in(product.owner):
+            for attribute_name in names:
+                getattr(product, attribute_name)
+
+    def test_access_launchpad_View_proprietary_product(self):
+        # Only people with grants for a private product can access
+        # attributes protected by the permission launchpad.View.
+        product = self.createProduct(
+            information_type=InformationType.PROPRIETARY,
+            license=License.OTHER_PROPRIETARY)
+        owner = removeSecurityProxy(product).owner
+        names = self.expected_get_permissions['launchpad.View']
+        with person_logged_in(None):
+            for attribute_name in names:
+                self.assertRaises(
+                    Unauthorized, getattr, product, attribute_name)
+        ordinary_user = self.factory.makePerson()
+        with person_logged_in(ordinary_user):
+            for attribute_name in names:
+                self.assertRaises(
+                    Unauthorized, getattr, product, attribute_name)
+        with person_logged_in(owner):
+            for attribute_name in names:
+                getattr(product, attribute_name)
+        # A user with a policy grant for the product can access attributes
+        # of a private product.
+        with person_logged_in(owner):
+            getUtility(IService, 'sharing').sharePillarInformation(
+                product, ordinary_user, owner,
+                {InformationType.PROPRIETARY: SharingPermission.ALL})
+        with person_logged_in(ordinary_user):
+            for attribute_name in names:
+                getattr(product, attribute_name)
+        # Admins can access proprietary products.
+        with celebrity_logged_in('admin'):
+            for attribute_name in names:
+                getattr(product, attribute_name)
+        with celebrity_logged_in('registry_experts'):
+            for attribute_name in names:
+                getattr(product, attribute_name)
+        # Commercial admins have access to all products.
+        with celebrity_logged_in('commercial_admin'):
+            for attribute_name in names:
+                getattr(product, attribute_name)
+
+    def test_access_launchpad_AnyAllowedPerson_public_product(self):
+        # Only logged in persons have access to properties of public products
+        # that require the permission launchpad.AnyAllowedPerson.
+        product = self.factory.makeProduct()
+        names = self.expected_get_permissions['launchpad.AnyAllowedPerson']
+        with person_logged_in(None):
+            for attribute_name in names:
+                self.assertRaises(
+                    Unauthorized, getattr, product, attribute_name)
+        ordinary_user = self.factory.makePerson()
+        with person_logged_in(ordinary_user):
+            for attribute_name in names:
+                getattr(product, attribute_name)
+        with person_logged_in(product.owner):
+            for attribute_name in names:
+                getattr(product, attribute_name)
+
+    def test_access_launchpad_AnyAllowedPerson_proprietary_product(self):
+        # Only people with grants for a private product can access
+        # attributes protected by the permission launchpad.AnyAllowedPerson.
+        product = self.createProduct(
+            information_type=InformationType.PROPRIETARY,
+            license=License.OTHER_PROPRIETARY)
+        owner = removeSecurityProxy(product).owner
+        names = self.expected_get_permissions['launchpad.AnyAllowedPerson']
+        with person_logged_in(None):
+            for attribute_name in names:
+                self.assertRaises(
+                    Unauthorized, getattr, product, attribute_name)
+        ordinary_user = self.factory.makePerson()
+        with person_logged_in(ordinary_user):
+            for attribute_name in names:
+                self.assertRaises(
+                    Unauthorized, getattr, product, attribute_name)
+        with person_logged_in(owner):
+            for attribute_name in names:
+                getattr(product, attribute_name)
+        # A user with a policy grant for the product can access attributes
+        # of a private product.
+        with person_logged_in(owner):
+            getUtility(IService, 'sharing').sharePillarInformation(
+                product, ordinary_user, owner,
+                {InformationType.PROPRIETARY: SharingPermission.ALL})
+        with person_logged_in(ordinary_user):
+            for attribute_name in names:
+                getattr(product, attribute_name)
+
+    def test_set_launchpad_AnyAllowedPerson_public_product(self):
+        # Only logged in users can set attributes protected by the
+        # permission launchpad.AnyAllowedPerson.
+        product = self.factory.makeProduct()
+        with person_logged_in(None):
+            self.assertRaises(
+                Unauthorized, setattr, product, 'date_next_suggest_packaging',
+                'foo')
+        ordinary_user = self.factory.makePerson()
+        with person_logged_in(ordinary_user):
+            setattr(product, 'date_next_suggest_packaging', 'foo')
+        with person_logged_in(product.owner):
+            setattr(product, 'date_next_suggest_packaging', 'foo')
+
+    def test_set_launchpad_AnyAllowedPerson_proprietary_product(self):
+        # Only people with grants for a private product can set
+        # attributes protected by the permission launchpad.AnyAllowedPerson.
+        product = self.createProduct(
+            information_type=InformationType.PROPRIETARY,
+            license=License.OTHER_PROPRIETARY)
+        owner = removeSecurityProxy(product).owner
+        with person_logged_in(None):
+            self.assertRaises(
+                Unauthorized, setattr, product, 'date_next_suggest_packaging',
+                'foo')
+        ordinary_user = self.factory.makePerson()
+        with person_logged_in(ordinary_user):
+            self.assertRaises(
+                Unauthorized, setattr, product, 'date_next_suggest_packaging',
+                'foo')
+        with person_logged_in(owner):
+            setattr(product, 'date_next_suggest_packaging', 'foo')
+        # A user with a policy grant for the product can access attributes
+        # of a private product.
+        with person_logged_in(owner):
+            getUtility(IService, 'sharing').sharePillarInformation(
+                product, ordinary_user, owner,
+                {InformationType.PROPRIETARY: SharingPermission.ALL})
+        with person_logged_in(ordinary_user):
+            setattr(product, 'date_next_suggest_packaging', 'foo')
+
+    def test_userCanView_caches_known_users(self):
+        # userCanView() maintains a cache of users known to have the
+        # permission to access a product.
+        product = self.createProduct(
+            information_type=InformationType.PROPRIETARY,
+            license=License.OTHER_PROPRIETARY)
+        owner = removeSecurityProxy(product).owner
+        user = self.factory.makePerson()
+        with person_logged_in(owner):
+            getUtility(IService, 'sharing').sharePillarInformation(
+                product, user, owner,
+                {InformationType.PROPRIETARY: SharingPermission.ALL})
+        with person_logged_in(user):
+            with StormStatementRecorder() as recorder:
+                # The first access to a property of the product from
+                # a user requires a DB query.
+                product.homepageurl
+                queries_for_first_user_access = len(recorder.queries)
+                # The second access does not require another query.
+                product.description
+                self.assertEqual(
+                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
+        # permission to access a product.
+        product = self.createProduct(
+            information_type=InformationType.PROPRIETARY,
+            license=License.OTHER_PROPRIETARY)
+        user = self.factory.makePerson()
+        product.userCanView(user)
+        product.userCanView(IPersonRoles(user))
+
 
 class TestProductBugInformationTypes(TestCaseWithFactory):
 

=== modified file 'lib/lp/registry/tests/test_product_webservice.py'
--- lib/lp/registry/tests/test_product_webservice.py	2012-10-11 04:18:37 +0000
+++ lib/lp/registry/tests/test_product_webservice.py	2012-10-15 15:39:22 +0000
@@ -17,6 +17,7 @@
 from lp.services.webapp.publisher import canonical_url
 from lp.testing import TestCaseWithFactory
 from lp.testing.layers import DatabaseFunctionalLayer
+from lp.testing import person_logged_in
 from lp.testing.pages import (
     LaunchpadWebServiceCaller,
     webservice_for_person,
@@ -47,27 +48,30 @@
     layer = DatabaseFunctionalLayer
 
     def patch(self, webservice, obj, **data):
+        with person_logged_in(webservice.user):
+            path = URI(canonical_url(obj)).path
         return webservice.patch(
-            URI(canonical_url(obj)).path,
-            'application/json', json.dumps(data),
-            api_version='devel')
+            path, 'application/json', json.dumps(data), api_version='devel')
 
     def test_branch_sharing_policy_can_be_set(self):
         # branch_sharing_policy can be set via the API.
         product = self.factory.makeProduct()
+        owner = product.owner
         self.factory.makeCommercialSubscription(product=product)
         webservice = webservice_for_person(
-            product.owner, permission=OAuthPermission.WRITE_PRIVATE)
+            owner, permission=OAuthPermission.WRITE_PRIVATE)
         response = self.patch(
             webservice, product, branch_sharing_policy='Proprietary')
         self.assertEqual(209, response.status)
-        self.assertEqual(
-            BranchSharingPolicy.PROPRIETARY, product.branch_sharing_policy)
+        with person_logged_in(owner):
+            self.assertEqual(
+                BranchSharingPolicy.PROPRIETARY, product.branch_sharing_policy)
 
     def test_branch_sharing_policy_non_commercial(self):
         # An API attempt to set a commercial-only branch_sharing_policy
         # on a non-commercial project returns Forbidden.
         product = self.factory.makeProduct()
+        owner = product.owner
         webservice = webservice_for_person(
             product.owner, permission=OAuthPermission.WRITE_PRIVATE)
         response = self.patch(
@@ -76,25 +80,29 @@
                 status=403,
                 body=('A current commercial subscription is required to use '
                       'proprietary branches.')))
-        self.assertEqual(
-            BranchSharingPolicy.PUBLIC, product.branch_sharing_policy)
+        with person_logged_in(owner):
+            self.assertEqual(
+                BranchSharingPolicy.PUBLIC, product.branch_sharing_policy)
 
     def test_bug_sharing_policy_can_be_set(self):
         # bug_sharing_policy can be set via the API.
         product = self.factory.makeProduct()
+        owner = product.owner
         self.factory.makeCommercialSubscription(product=product)
         webservice = webservice_for_person(
             product.owner, permission=OAuthPermission.WRITE_PRIVATE)
         response = self.patch(
             webservice, product, bug_sharing_policy='Proprietary')
         self.assertEqual(209, response.status)
-        self.assertEqual(
-            BugSharingPolicy.PROPRIETARY, product.bug_sharing_policy)
+        with person_logged_in(owner):
+            self.assertEqual(
+                BugSharingPolicy.PROPRIETARY, product.bug_sharing_policy)
 
     def test_bug_sharing_policy_non_commercial(self):
         # An API attempt to set a commercial-only bug_sharing_policy
         # on a non-commercial project returns Forbidden.
         product = self.factory.makeProduct()
+        owner = product.owner
         webservice = webservice_for_person(
             product.owner, permission=OAuthPermission.WRITE_PRIVATE)
         response = self.patch(
@@ -103,13 +111,14 @@
                 status=403,
                 body=('A current commercial subscription is required to use '
                       'proprietary bugs.')))
-        self.assertEqual(
-            BugSharingPolicy.PUBLIC, product.bug_sharing_policy)
+        with person_logged_in(owner):
+            self.assertEqual(
+                BugSharingPolicy.PUBLIC, product.bug_sharing_policy)
 
     def fetch_product(self, webservice, product, api_version):
-        return webservice.get(
-            canonical_url(product, force_local_path=True),
-            api_version=api_version).jsonBody()
+        with person_logged_in(webservice.user):
+            url = canonical_url(product, force_local_path=True)
+        return webservice.get(url, api_version=api_version).jsonBody()
 
     def test_security_contact_exported(self):
         # security_contact is exported for 1.0, but not for other versions.

=== modified file 'lib/lp/registry/tests/test_subscribers.py'
--- lib/lp/registry/tests/test_subscribers.py	2012-10-08 10:07:11 +0000
+++ lib/lp/registry/tests/test_subscribers.py	2012-10-15 15:39:22 +0000
@@ -195,7 +195,10 @@
         # If there is no request, there is no reason to show a message in
         # the browser.
         product, user = self.make_product_user([License.GNU_GPL_V2])
-        notification = LicenseNotification(product)
+        # Using the proxied product leads to an exeception when
+        # notification.display() below is called because the permission
+        # checks product require an interaction.
+        notification = LicenseNotification(removeSecurityProxy(product))
         logout()
         result = notification.display()
         self.assertIs(False, result)

=== modified file 'lib/lp/security.py'
--- lib/lp/security.py	2012-10-09 08:39:54 +0000
+++ lib/lp/security.py	2012-10-15 15:39:22 +0000
@@ -29,6 +29,7 @@
 from lp.answers.interfaces.questionmessage import IQuestionMessage
 from lp.answers.interfaces.questionsperson import IQuestionsPerson
 from lp.answers.interfaces.questiontarget import IQuestionTarget
+from lp.app.enums import PUBLIC_INFORMATION_TYPES
 from lp.app.interfaces.launchpad import ILaunchpadCelebrities
 from lp.app.interfaces.security import IAuthorization
 from lp.app.security import (
@@ -424,6 +425,30 @@
         return user.isOwner(self.obj) or user.in_admin
 
 
+class ViewProduct(AuthorizationBase):
+    permission = 'launchpad.View'
+    usedfor = IProduct
+
+    def checkAuthenticated(self, user):
+        return self.obj.userCanView(user)
+
+    def checkUnauthenticated(self):
+        return self.obj.information_type in PUBLIC_INFORMATION_TYPES
+
+
+class ChangeProduct(ViewProduct):
+    """Used for attributes of IProduct that are accessible to any logged
+    in user for public product but only to persons with access grants
+    for private products.
+    """
+
+    permission = 'launchpad.AnyAllowedPerson'
+    usedfor = IProduct
+
+    def checkUnauthenticated(self):
+        return False
+
+
 class EditProduct(EditByOwnersOrAdmins):
     usedfor = IProduct
 

=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py	2012-10-11 04:21:07 +0000
+++ lib/lp/testing/factory.py	2012-10-15 15:39:22 +0000
@@ -1025,7 +1025,6 @@
                 specification_sharing_policy)
         if information_type is not None:
             naked_product.information_type = information_type
-
         return product
 
     def makeProductSeries(self, product=None, name=None, owner=None,
@@ -1044,7 +1043,7 @@
         if product is None:
             product = self.makeProduct()
         if owner is None:
-            owner = product.owner
+            owner = removeSecurityProxy(product).owner
         if name is None:
             name = self.getUniqueString()
         if summary is None:
@@ -1821,7 +1820,8 @@
 
         if owner is None:
             owner = self.makePerson()
-        return removeSecurityProxy(bug).addTask(owner, target)
+        return removeSecurityProxy(bug).addTask(
+            owner, removeSecurityProxy(target))
 
     def makeBugNomination(self, bug=None, target=None):
         """Create and return a BugNomination.

=== modified file 'lib/lp/testing/pages.py'
--- lib/lp/testing/pages.py	2012-10-08 10:07:11 +0000
+++ lib/lp/testing/pages.py	2012-10-15 15:39:22 +0000
@@ -730,7 +730,9 @@
     request_token.review(person, permission, context)
     access_token = request_token.createAccessToken()
     logout()
-    return LaunchpadWebServiceCaller(consumer_key, access_token.key)
+    service = LaunchpadWebServiceCaller(consumer_key, access_token.key)
+    service.user = person
+    return service
 
 
 def setupDTCBrowser():

=== modified file 'lib/lp/translations/browser/tests/test_noindex.py'
--- lib/lp/translations/browser/tests/test_noindex.py	2012-10-08 10:07:11 +0000
+++ lib/lp/translations/browser/tests/test_noindex.py	2012-10-15 15:39:22 +0000
@@ -46,7 +46,12 @@
         # Using create_initialized_view for distroseries causes an error when
         # rendering the view due to the way the view is registered and menus
         # are adapted.  Getting the contents via a browser does work.
-        self.user_browser.open(self.url)
+        #
+        # Retrieve the URL before the user_browser is created. Products
+        # can only be access with an active interaction, and getUserBrowser()
+        # closes the current interaction.
+        url = self.url
+        self.user_browser.open(url)
         return self.user_browser.contents
 
     def getRobotsDirective(self):

=== modified file 'lib/lp/translations/stories/importqueue/xx-entry-details.txt'
--- lib/lp/translations/stories/importqueue/xx-entry-details.txt	2012-10-08 10:07:11 +0000
+++ lib/lp/translations/stories/importqueue/xx-entry-details.txt	2012-10-15 15:39:22 +0000
@@ -15,6 +15,7 @@
     >>> queue = TranslationImportQueue()
     >>> product = factory.makeProduct(
     ...     translations_usage=ServiceUsage.LAUNCHPAD)
+    >>> product_displayname = product.displayname
     >>> trunk = product.getSeries('trunk')
     >>> uploader = factory.makePerson()
     >>> entry = queue.addOrUpdateEntry(
@@ -34,7 +35,7 @@
 
 The details include the project the entry is for, and who uploaded it.
 
-    >>> product.displayname in details_text
+    >>> product_displayname in details_text
     True
 
     # Must remove the security proxy because IPerson.displayname is protected.

=== modified file 'lib/lp/translations/stories/webservice/xx-potemplate.txt'
--- lib/lp/translations/stories/webservice/xx-potemplate.txt	2012-10-08 10:07:11 +0000
+++ lib/lp/translations/stories/webservice/xx-potemplate.txt	2012-10-15 15:39:22 +0000
@@ -71,13 +71,14 @@
 
     >>> login('[e-mail]')
     >>> productseries = factory.makeProductSeries()
+    >>> product_name = productseries.product.name
     >>> potemplate_1 = factory.makePOTemplate(productseries=productseries)
     >>> potemplate_2 = factory.makePOTemplate(productseries=productseries)
     >>> potemplate_count = 2
     >>> logout()
     >>> all_translation_templates = anon_webservice.named_get(
     ...     '/%s/%s' % (
-    ...         productseries.product.name,
+    ...         product_name,
     ...         productseries.name),
     ...     'getTranslationTemplates'
     ...     ).jsonBody()