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

[Abel Deuring]

This branch is my second attempt to activate permission checks for private products. The first attempt caused numerous OOPSes when it was deployed last monday. The reason was that the security adapter also refused access to most IProduct properties for inactive products, but in some cases details about inactive products are needed during page rendering, even if they are not visible for unprivileged users.

I discussed several approaches to fix this problem with sinzui; it turned out that we should for now simply allow access to the properties that are needed. Better fixes would need to long to implement.

The diff against trunk is pretty long. But r16117 simply adds the the changed from r16090 that were reverted in r16112.

tests:

./bin/test -vvt lp.registry.tests.test_product.TestProduct.test_access_to_deactivated_product
./bin/test -vvt lp.registry.tests.test_product.TestProduct.test_get_permissions
./bin/test -vvt lp.registry.tests.test_product.TestProduct.test_access_launchpad_View_proprietary_product

no lint

[Abel Deuring (adeuring)]

the "real" changes:

=== modified file 'lib/lp/registry/configure.zcml'
--- lib/lp/registry/configure.zcml 2012-10-09 10:28:02 +0000
+++ lib/lp/registry/configure.zcml 2012-10-10 18:04:36 +0000
@@ -1229,8 +1229,9 @@
         class="lp.registry.model.product.Product">
         <allow
             interface="lp.registry.interfaces.product.IProductPublic"/>
- <allow
- interface="lp.registry.interfaces.pillar.IPillar"/>
+ <require
+ permission="launchpad.View"
+ interface="lp.registry.interfaces.product.IPillar"/>
         <require
             permission="launchpad.View"
             interface="lp.registry.interfaces.product.IProductLimitedView"/>
@@ -1371,8 +1372,9 @@

         <!-- IStructuralSubscriptionTarget -->

- <require
- permission="launchpad.View"
+ <!-- XXX bug=1065162 Abel Deuring 2012-10-10:
+ This interface should require the permission launchpad.View. -->
+ <allow
             interface="lp.bugs.interfaces.structuralsubscription.IStructuralSubscriptionTargetRead" />
         <require
             permission="launchpad.AnyAllowedPerson"

=== modified file 'lib/lp/registry/interfaces/product.py'
--- lib/lp/registry/interfaces/product.py 2012-10-09 10:28:02 +0000
+++ lib/lp/registry/interfaces/product.py 2012-10-10 18:04:36 +0000
@@ -430,6 +430,24 @@
     def userCanView(user):
         """True if the given user has access to this product."""

+ # bug=1065162 Abel Deuring 2012-10-10:
+ # name and displayname should be defined in IProductLimitedView
+ name = exported(
+ ProductNameField(
+ title=_('Name'),
+ constraint=name_validator,
+ description=_(
+ "At least one lowercase letter or number, followed by "
+ "letters, numbers, dots, hyphens or pluses. "
+ "Keep this name short; it is used in URLs as shown above.")))
+
+ displayname = exported(
+ TextLine(
+ title=_('Display Name'),
+ description=_("""The name of the project as it would appear in a
+ paragraph.""")),
+ exported_as='display_name')
+

 class IProductLimitedView(
     IBugTarget, ICanGetMilestonesDirectly, IHasAppointedDriver, IHasBranches,
@@ -491,22 +509,6 @@
         "required because there might be a project driver and also a "
         "driver appointed in the overarching project group.")

- name = exported(
- ProductNameField(
- title=_('Name'),
- constraint=name_validator,
- description=_(
- "At least one lowercase letter or number, followed by "
- "letters, numbers, dots, hyphens or pluses. "
- "Keep this name short; it is used in URLs as shown above.")))
-
- displayname = exported(
- TextLine(
- title=_('Display Name'),
- description=_("""The name of the project as it would appear in a
- paragraph.""")),
- exported_as='display_name')
-
     title = exported(
         Title(
             title=_('Title'),

=== modified file 'lib/lp/registry/tests/test_product.py'
--- lib/lp/registry...

[Curtis Hovey (sinzui)]

The comment format is
XXX: Abel Deuring 2012-10-10 bug=1065162

There is still something wrong and we think are missing tests. non-registry users and anonymous users get 403 on pages that that are working with deactivated projects.
1. Start an Lp instance
2. As admin, https://bugs.launchpad.dev/redfish/+bug/15
   and verify redfish and thunderbird are in the affected table.
3. Visit https://launchpad.dev/ and verify that thunderbird is listed
4. Visit https://launchpad.dev/thunderbird/+admin
   and deactivate thunderbird
5. Visit https://launchpad.dev/
   and verify thunderbird is listed.
6. Follow the link to https://launchpad.dev/thunderbird
   and verify it says it is deactivated
7. Visit https://bugs.launchpad.dev/redfish/+bug/15
   and verify that only redfish is shown in the tasks table.

8. As no-priv, visit https://launchpad.dev/thunderbird
   and verify the page is a 404.
9. Visit https://bugs.launchpad.dev/redfish/+bug/15
   and verify that only redfish is shown in the tasks table.
   BAD: the user gets a 403!
   OH, this loads after a while...we have a async issue?

Module lp.bugs.model.structuralsubscription, line 275, in __init__
self.target_parent = target.project
Unauthorized: (<Product at 0x2b15b8d98410>, 'project', 'launchpad.View'

10. Visit https://launchpad.dev/
    and verify thunderbird is listed.
    BAD: the user gets a 403!

if IHasIcon.providedBy(context) and context.icon is not None:
Unauthorized: (<Product at 0x2b15bca1fd10>, 'icon', 'launchpad.View')

11. As anonymous, visit https://launchpad.dev/thunderbird
    and verify the page is a 404.
12. Visit https://bugs.launchpad.dev/redfish/+bug/15
    and verify that only redfish is shown in the tasks table.
    BAD: the user gets a 403!
    OH, this loads after a while...we have a async issue?

Module lp.bugs.model.structuralsubscription, line 275, in __init__
self.target_parent = target.project
Unauthorized: (<Product at 0x2b15b8d98410>, 'project', 'launchpad.View'

13. Visit https://launchpad.dev/
    ...I cannot. Lp requires me to login to see a page intended for bots and anonymous users.

This kind of error can be seen elsewhere where deactivated projects can appear in Lp. In qastaging for example we can deactivate a project listed on Lp's front page ans the page still displays for anonymous and non-registry users, but devel breaks with your branch.

I think we want to make the security checker smarter so we can land this branch to maintain the current behaviour. We can revise the checker and the interfaces in future branches. Maybe both checkAuthenticated() and checkUnauthenticated() can return true if the project is active and public. They can return true if the project is inactive and public and the user is in A, CA, R, otherwise if the project is private and user in A, CA return true

review: Needs Fixing (code)

[Abel Deuring (adeuring)]

I think we should not return True for inactive products. In that case, oridnary users will be able to see these inactive products. Or we separate the checkers for IPillar and IProductView. But as we discussed in a hangout, I had a hard time to implement this when the permission lp.View is used both for IPillar and IProductView.

[Curtis Hovey (sinzui)]

Members of ~registry, ~admins, and ~commercial-admins *do* have lp.view on deactivated projects now. They also have lp.moderate and some have lp.admin and lp.edit. So we will return true for deactivated project for about 100 people.

[Abel Deuring (adeuring)]

On 10.10.2012 21:52, Curtis Hovey wrote:
> Members of ~registry, ~admins, and ~commercial-admins *do* have lp.view on deactivated projects now. They also have lp.moderate and some have lp.admin and lp.edit. So we will return true for deactivated project for about 100 people.

yes, but my point are the permissions for ordinary users: I could simply
not find a way to use lp.View both for IPillar and for IProductView so
that an oridnary users sees a 404 error when visiting the main page of a
deactivated product

I'd be grateful for a hint how to do this.

[Curtis Hovey (sinzui)]

This is the checker used in production:

class ViewPillar(AuthorizationBase):
    usedfor = IPillar
    permission = 'launchpad.View'

    def checkUnauthenticated(self):
        return self.obj.active

    def checkAuthenticated(self, user):
        """The Admins & Commercial Admins can see inactive pillars."""
        if self.obj.active:
            return True
        else:
            return (user.in_commercial_admin or
                    user.in_admin or
                    user.in_registry_experts)

You introduced a new checker that is specific to IProduct, but is does not ever consider .active. As is said in the hangout, IPillar is not properly implemented. IDistribution.active cannot ever be false, so we know the .active rule is mostly for IProduct. I think this phrasing of rules always defers to ViewPillar for the current case that all projects are public. We only do new rule checking for private types. Unauthenticated is always false, and authenticated has to exempt A and CA from the data drive rules in userCanView()

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

    def checkAuthenticated(self, user):
        if self.obj.information_type in PUBLIC_INFORMATION_TYPES:
            return super(ViewProduct, self).checkAuthenticated(user)
        return (user.in_commercial_admin
                or user.in_admin
                or self.obj.userCanView(user))

    def checkUnauthenticated(self):
        if self.obj.information_type in PUBLIC_INFORMATION_TYPES:
            return super(ViewProduct, self).checkUnauthenticated()
        return False

[Abel Deuring (adeuring)]

On 10.10.2012 22:30, Curtis Hovey wrote:
> This is the checker used in production:
>
> class ViewPillar(AuthorizationBase):
> usedfor = IPillar
> permission = 'launchpad.View'
>
> def checkUnauthenticated(self):
> return self.obj.active
>
> def checkAuthenticated(self, user):
> """The Admins & Commercial Admins can see inactive pillars."""
> if self.obj.active:
> return True
> else:
> return (user.in_commercial_admin or
> user.in_admin or
> user.in_registry_experts)
>
> You introduced a new checker that is specific to IProduct, but is does not ever consider .active.

The one in r16090 did.

> As is said in the hangout, IPillar is not properly implemented. IDistribution.active cannot ever be false, so we know the .active rule is mostly for IProduct. I think this phrasing of rules always defers to ViewPillar for the current case that all projects are public. We only do new rule checking for private types. Unauthenticated is always false, and authenticated has to exempt A and CA from the data drive rules in userCanView()
>
> class ViewProduct(ViewPillar):
> permission = 'launchpad.View'
> usedfor = IProduct
>
> def checkAuthenticated(self, user):
> if self.obj.information_type in PUBLIC_INFORMATION_TYPES:
> return super(ViewProduct, self).checkAuthenticated(user)

...this would deny access to properties like name, displayname etc which
we need for deactivated products, so the same problem we had with r16090.

> return (user.in_commercial_admin
> or user.in_admin
> or self.obj.userCanView(user))
>
> def checkUnauthenticated(self):
> if self.obj.information_type in PUBLIC_INFORMATION_TYPES:
> return super(ViewProduct, self).checkUnauthenticated()
> return False
>

[William Grant (wgrant)]

Have you considered feature-flagging this so we can revert easily if necessary? It may also be helpful to later make projects show up as <redacted> and raise a soft OOPS as private teams do today, although that will be less helpful as a lot of artifact URLs contain the project name.

[Abel Deuring (adeuring)]

On 10.10.2012 23:53, William Grant wrote:
> Have you considered feature-flagging this so we can revert easily if necessary? It may also be helpful to later make projects show up as <redacted> and raise a soft OOPS as private teams do today, although that will be less helpful as a lot of artifact URLs contain the project name.
>

Do we have means to change ZCML configurations via feature flags? The
main issue are new permissions and a changed inheritance hierarchy for
IProduct.

[William Grant (wgrant)]

AIUI the main thing launchpad.View on IProduct is used for today is to prevent traversal to deactivated products. If you add an extra guard to prevent such traversal even if launchpad.View is held, then the launchpad.View adapter can have an emergency feature flag which grants it to everyone.

[Abel Deuring (adeuring)]

On 11.10.2012 10:40, William Grant wrote:
> AIUI the main thing launchpad.View on IProduct is used for today is to prevent traversal to deactivated products. If you add an extra guard to prevent such traversal even if launchpad.View is held, then the launchpad.View adapter can have an emergency feature flag which grants it to everyone.
>

A feature flag alone would not help much. I think the main issue is
that the same checker is used for IPillar and IProductLimitedView
(should be renamed to IProductView, but this is different issue).

Let me try to explain the problem with this branch:
lp:~adeuring/launchpad/authentication-for-private-products-test

It has my core changes: most of attributes of IProduct are defined in
IProductLimitedView and require the permission launchpad.View.

The relevant security adapter is ViewProduct.

First, the variant r16120:

The section for Product in registry/configure.zcml does not contain
any directive for IPillar.

Now do "make run", log in as <email address hidden> and try to view any
product related page. You'll get "LocationError:
(<Product at 0xba970d0>, 'active')<br />"

Or run iharness:

from lp.registry.interfaces.product import IProductSet
p = getUtility(IProductSet).getByName('firefox')
p.active

You'll get a ForbiddenAttribute exception.

So we need a directive for IPillar in registry/configure.zcml.

r16121 has

        <allow
            interface="lp.registry.interfaces.product.IPillar"/>

The iharness test now works.

Now "make run", log in as an admin and make thunderbird private, as
suggested by Curtis.

Visit https://launchpad.dev/thunderbird as an admin, as an ordinary user
and without being logged in. This works as expected: The admin sees
the regular page with the notice "this project is currently inactive."

Anonymous and <email address hidden> see a 404 page.

Now visit https://bugs.launchpad.dev/redfish/+bug/15 (this bug has a
task for thunderbird.)

Anonymous and the admin see the page just fine, but <email address hidden>
gets a "Not allowed here" error:
Unauthorized: (<Product at 0x2b0588b8d9d0>, 'project',
'launchpad.View')<br />

as noticed by Curtis.

Now let us try to remove the check for inactive products in the
security adapter, as suggested by William (out of lazyness, without a
feature flag, i just removed the check for inactive products): r16122

The bug page works fine for all users, but anonymous and
test@canoncial,com no longer get the 404 error for
https://launchpad.dev/thunderbird . Instead, they see the same page as
the admin: "This project is currently inactive."

So, no luck either.

Let us now require the permission lp.View for IPillar: r16123

Both pages, https://bugs.launchpad.dev/redfish/+bug/15 (good) and
https://launchpad.dev/thunderbird (bad) are still visible for anonymous
and <email address hidden> .

Let's reenable the check for inactive products in ViewProduct: r16124

Now anonymous and <email address hidden> get again the 404 error for
https://launchpad.dev/thunderbird -- but <email address hidden> gets
a 403 for https://bugs.launchpad.dev/redfish/+bug/15. So we are back
at the situation that required a rollback of r16090 of trunk.

I think the main problem is that the inte...

Unmerged revisions

16119. By Abel Deuring on 2012-10-10

trunk merged.

16118. By Abel Deuring on 2012-10-10

avoid OOPSes when data of inactive products is needed for users without special permissions.

=== 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-10 18:25:24 +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/model/launchpad.py'
--- lib/lp/app/model/launchpad.py	2012-10-08 10:07:11 +0000
+++ lib/lp/app/model/launchpad.py	2012-10-10 18:25:24 +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-08 10:07:11 +0000
+++ lib/lp/blueprints/browser/tests/test_specification.py	2012-10-10 18:25:24 +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 (
@@ -467,7 +468,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."""
@@ -551,9 +556,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-10 18:25:24 +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-10 18:25:24 +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-10 02:45:36 +0000
+++ lib/lp/bugs/browser/tests/test_bugtask.py	2012-10-10 18:25:24 +0000
@@ -1985,8 +1985,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()
@@ -1994,11 +1996,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-10 18:25:24 +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-10 18:25:24 +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-10 18:25:24 +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-10 18:25:24 +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-10 18:25:24 +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-08 10:07:11 +0000
+++ lib/lp/bugs/tests/test_bugs_webservice.py	2012-10-10 18:25:24 +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,12 +406,13 @@
         # to the project's bugs are private by default rule.
         project = self.factory.makeLegacyProduct(
             licenses=[License.OTHER_PROPRIETARY])
+        target_url = api_url(project)
         with person_logged_in(project.owner):
             project.setPrivateBugs(True, project.owner)
         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('Private', bug.information_type)
 
     def test_explicit_private_private_bugs_true(self):
@@ -418,12 +421,13 @@
         # user commands it.
         project = self.factory.makeLegacyProduct(
             licenses=[License.OTHER_PROPRIETARY])
+        target_url = api_url(project)
         with person_logged_in(project.owner):
             project.setPrivateBugs(True, project.owner)
         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',
             private=True)
         self.assertEqual('Private', bug.information_type)
 
@@ -433,13 +437,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)
 
 
@@ -459,7 +464,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-10 18:25:24 +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-08 10:07:11 +0000
+++ lib/lp/code/browser/tests/test_branch.py	2012-10-10 18:25:24 +0000
@@ -698,11 +698,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 10:07:11 +0000
+++ lib/lp/code/browser/tests/test_product.py	2012-10-10 18:25:24 +0000
@@ -224,8 +224,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)
@@ -383,6 +384,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
@@ -390,7 +392,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-08 10:07:11 +0000
+++ lib/lp/code/stories/branches/xx-product-branches.txt	2012-10-10 18:25:24 +0000
@@ -181,10 +181,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:
@@ -204,13 +204,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
@@ -235,7 +238,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-08 10:07:11 +0000
+++ lib/lp/code/stories/webservice/xx-code-import.txt	2012-10-10 18:25:24 +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-10 18:25:24 +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-10 18:25:24 +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-10 13:56:03 +0000
+++ lib/lp/registry/browser/tests/test_product.py	2012-10-10 18:25:24 +0000
@@ -614,8 +614,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/configure.zcml'
--- lib/lp/registry/configure.zcml	2012-10-08 10:07:11 +0000
+++ lib/lp/registry/configure.zcml	2012-10-10 18:25:24 +0000
@@ -1229,11 +1229,20 @@
         class="lp.registry.model.product.Product">
         <allow
             interface="lp.registry.interfaces.product.IProductPublic"/>
-        <allow interface="lp.bugs.interfaces.bugsummary.IBugSummaryDimension"/>
-        <allow
+        <require
+            permission="launchpad.View"
+            interface="lp.registry.interfaces.product.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 +1325,7 @@
                 translationpermission
                 translations_usage"/>
         <require
-            permission="zope.Public"
+            permission="launchpad.View"
             attributes="
                 qualifies_for_free_hosting"/>
         <require
@@ -1331,7 +1340,8 @@
 
         <!-- IHasAliases -->
 
-        <allow
+        <require
+            permission="launchpad.View"
             attributes="
                 aliases"/>
         <require
@@ -1341,14 +1351,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 +1372,18 @@
 
         <!-- IStructuralSubscriptionTarget -->
 
+        <!-- XXX bug=1065162 Abel Deuring 2012-10-10:
+             This interface should require the permission launchpad.View. -->
         <allow
             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-08 10:07:11 +0000
+++ lib/lp/registry/interfaces/product.py	2012-10-10 18:25:24 +0000
@@ -423,18 +423,42 @@
                 "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."""
+
+    # bug=1065162 Abel Deuring 2012-10-10:
+    # name and displayname should be defined in IProductLimitedView
+    name = exported(
+        ProductNameField(
+            title=_('Name'),
+            constraint=name_validator,
+            description=_(
+                "At least one lowercase letter or number, followed by "
+                "letters, numbers, dots, hyphens or pluses. "
+                "Keep this name short; it is used in URLs as shown above.")))
+
+    displayname = exported(
+        TextLine(
+            title=_('Display Name'),
+            description=_("""The name of the project as it would appear in a
+                paragraph.""")),
+        exported_as='display_name')
+
+
+class IProductLimitedView(
     IBugTarget, ICanGetMilestonesDirectly, IHasAppointedDriver, IHasBranches,
     IHasBranchVisibilityPolicy, IHasDrivers, IHasExternalBugTracker, IHasIcon,
     IHasLogo, IHasMergeProposals, IHasMilestones, IHasExpirableBugs,
     IHasMugshot, IHasOwner, IHasSprints, IHasTranslationImports,
     ITranslationPolicy, IKarmaContext, ILaunchpadUsage, IMakesAnnouncements,
-    IOfficialBugTagTargetPublic, IHasOOPSReferences, IPillar,
+    IOfficialBugTagTargetPublic, IHasOOPSReferences,
     ISpecificationTarget, IHasRecipes, IHasCodeImports, IServiceUsage):
     """Public IProduct properties."""
 
-    id = Int(title=_('The Project ID'))
-
     project = exported(
         ReferenceChoice(
             title=_('Part of'),
@@ -485,22 +509,6 @@
         "required because there might be a project driver and also a "
         "driver appointed in the overarching project group.")
 
-    name = exported(
-        ProductNameField(
-            title=_('Name'),
-            constraint=name_validator,
-            description=_(
-                "At least one lowercase letter or number, followed by "
-                "letters, numbers, dots, hyphens or pluses. "
-                "Keep this name short; it is used in URLs as shown above.")))
-
-    displayname = exported(
-        TextLine(
-            title=_('Display Name'),
-            description=_("""The name of the project as it would appear in a
-                paragraph.""")),
-        exported_as='display_name')
-
     title = exported(
         Title(
             title=_('Title'),
@@ -887,9 +895,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):
@@ -898,10 +906,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):
@@ -910,10 +918,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):
@@ -926,8 +934,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-09 13:25:28 +0000
+++ lib/lp/registry/model/product.py	2012-10-10 18:25:24 +0000
@@ -69,6 +69,7 @@
     FREE_INFORMATION_TYPES,
     InformationType,
     PRIVATE_INFORMATION_TYPES,
+    PUBLIC_INFORMATION_TYPES,
     PUBLIC_PROPRIETARY_INFORMATION_TYPES,
     PROPRIETARY_INFORMATION_TYPES,
     service_uses_launchpad,
@@ -453,7 +454,7 @@
 
     _information_type = EnumCol(
         enum=InformationType, default=InformationType.PUBLIC,
-        dbName="information_type",
+        dbName='information_type',
         storm_validator=_valid_product_information_type)
 
     def _get_information_type(self):
@@ -1557,6 +1558,31 @@
 
         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 want an actual Storm Person.
+        if IPersonRoles.providedBy(user):
+            user = user.person
+        policy = getUtility(IAccessPolicySource).find(
+            [(self, self.information_type)]).one()
+        grants_for_user = getUtility(IAccessPolicyGrantSource).find(
+            [(policy, 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-10 02:45:36 +0000
+++ lib/lp/registry/services/tests/test_sharingservice.py	2012-10-10 18:25:24 +0000
@@ -1838,12 +1838,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-10 18:25:24 +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-10 12:18:43 +0000
+++ lib/lp/registry/tests/test_product.py	2012-10-10 18:25:24 +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 (
@@ -72,6 +77,7 @@
     celebrity_logged_in,
     login,
     person_logged_in,
+    StormStatementRecorder,
     TestCase,
     TestCaseWithFactory,
     WebServiceTestCase,
@@ -467,7 +473,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
@@ -479,7 +485,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
@@ -593,6 +601,336 @@
                 CannotChangeInformationType, 'Answers is enabled.'):
                 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 = {
+        # bug=1065162 Abel Deuring 2012-10-10:
+        # name, displayname
+        # should be defined in IProductLimitedView
+        CheckerPublic: set((
+            'id', 'information_type', 'userCanView', 'private',
+            'name', 'displayname', 'bug_subscriptions', 'getSubscription',
+            'getSubscriptions', 'parent_subscription_target',
+            'target_type_display', 'userCanAlterBugSubscription',
+            'userCanAlterSubscription', 'userHasBugSubscriptions'
+            )),
+        'launchpad.View': set((
+            '_getOfficialTagClause', '_all_specifications',
+            '_valid_specifications', 'active', '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_supervisor',
+            'bug_tracking_usage', 'bugtargetdisplayname', 'bugtargetname',
+            'bugtracker', 'canUserAlterAnswerContact',
+            'checkPrivateBugsTransitionAllowed', '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', 'distrosourcepackages',
+            'downloadurl', 'driver', 'drivers', 'enable_bug_expiration',
+            'enable_bugfiling_duplicate_search', 'findReferencedOOPS',
+            'findSimilarFAQs', 'findSimilarQuestions', 'freshmeatproject',
+            'getAllowedBugInformationTypes',
+            'getAllowedSpecificationInformationTypes', 'getAnnouncement',
+            'getAnnouncements', 'getAnswerContactsForLanguage',
+            'getAnswerContactRecipients', 'getBaseBranchVisibilityRule',
+            'getBranchVisibilityRuleForBranch',
+            'getBranchVisibilityRuleForTeam',
+            'getBranchVisibilityTeamPolicies', 'getBranches',
+            'getBugSummaryContextWhereClause', 'getBugTaskWeightFunction',
+            'getCustomLanguageCode', 'getDefaultBugInformationType',
+            'getDefaultSpecificationInformationType',
+            'getEffectiveTranslationPermission', 'getExternalBugTracker',
+            'getFAQ', 'getFirstEntryToImport', 'getLinkedBugWatches',
+            'getMergeProposals', 'getMilestone', 'getMilestonesAndReleases',
+            'getQuestion', 'getQuestionLanguages', 'getPackage', 'getRelease',
+            'getSeries', 'getSpecification',
+            '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',
+            'isUsingInheritedBranchVisibilityPolicy',
+            'license_info', 'license_status', 'licenses', 'logo', 'milestones',
+            'mugshot', 'name_with_project', 'newCodeImport',
+            'obsolete_translatable_series', 'official_answers',
+            'official_anything', 'official_blueprints', 'official_bug_tags',
+            'official_codehosting', 'official_malone', 'owner',
+            'packagedInDistros', 'packagings',
+            'past_sprints', 'personHasDriverRights', 'pillar',
+            'pillar_category', 'primary_translatable', 'private_bugs',
+            'programminglang', 'project', 'qualifies_for_free_hosting',
+            'recipes', 'redeemSubscriptionVoucher', 'registrant', 'releases',
+            'remote_product', 'removeCustomLanguageCode',
+            'removeTeamFromBranchVisibilityPolicy', 'screenshotsurl',
+            'searchFAQs', 'searchQuestions', 'searchTasks', 'security_contact',
+            'series', 'setBranchVisibilityTeamPolicy', 'setPrivateBugs',
+            'sharesTranslationsWithOtherSide', 'sourceforgeproject',
+            'sourcepackages', 'specification_sharing_policy', 'specifications',
+            'sprints', 'summary', 'title',
+            'translatable_packages', 'translatable_series',
+            'translation_focus', 'translationgroup', 'translationgroups',
+            'translationpermission', 'translations_usage', 'ubuntu_packages',
+            'userCanEdit', '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_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)
+
+    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_access_to_deactivated_product(self):
+        # XXX bug=1065162 Abel Deuring 2012-10-10:
+        # The properties name, displayname, parent_subscription_target must
+        # be visible for inactive products.
+        product = self.factory.makeProduct()
+        removeSecurityProxy(product).active = False
+        user = self.factory.makePerson()
+        with person_logged_in(user):
+            product.name
+            product.displayname
+            product.parent_subscription_target
+            # Access to other attributes, including those defined in
+            # IPillar, is not possible.
+            self.assertRaises(Unauthorized, getattr, product, 'active')
+            self.assertRaises(Unauthorized, getattr, product, 'title')
+
 
 class TestProductBugInformationTypes(TestCaseWithFactory):
 

=== modified file 'lib/lp/registry/tests/test_product_webservice.py'
--- lib/lp/registry/tests/test_product_webservice.py	2012-10-08 10:07:11 +0000
+++ lib/lp/registry/tests/test_product_webservice.py	2012-10-10 18:25:24 +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.makeLegacyProduct()
+        owner = product.owner
         webservice = webservice_for_person(
             product.owner, permission=OAuthPermission.WRITE_PRIVATE)
         response = self.patch(
@@ -76,24 +80,28 @@
                 status=403,
                 body=('A current commercial subscription is required to use '
                       'proprietary branches.')))
-        self.assertIs(None, product.branch_sharing_policy)
+        with person_logged_in(owner):
+            self.assertIs(None, 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.makeLegacyProduct()
+        owner = product.owner
         webservice = webservice_for_person(
             product.owner, permission=OAuthPermission.WRITE_PRIVATE)
         response = self.patch(
@@ -102,12 +110,13 @@
                 status=403,
                 body=('A current commercial subscription is required to use '
                       'proprietary bugs.')))
-        self.assertIs(None, product.bug_sharing_policy)
+        with person_logged_in(owner):
+            self.assertIs(None, 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-10 18:25:24 +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-08 10:07:11 +0000
+++ lib/lp/security.py	2012-10-10 18:25:24 +0000
@@ -34,6 +34,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 (
@@ -430,6 +431,34 @@
         return user.isOwner(self.obj) or user.in_admin
 
 
+class ViewProduct(ViewPillar):
+    permission = 'launchpad.View'
+    usedfor = IProduct
+
+    def checkAuthenticated(self, user):
+        return (
+            super(ViewProduct, self).checkAuthenticated(user) and
+            self.obj.userCanView(user))
+
+    def checkUnauthenticated(self):
+        return (
+            self.obj.information_type in PUBLIC_INFORMATION_TYPES and
+            super(ViewProduct, self).checkUnauthenticated())
+
+
+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-08 10:07:11 +0000
+++ lib/lp/testing/factory.py	2012-10-10 18:25:24 +0000
@@ -1028,7 +1028,6 @@
                 specification_sharing_policy)
         if information_type is not None:
             naked_product.information_type = information_type
-
         return product
 
     def makeLegacyProduct(self, **kwargs):
@@ -1061,7 +1060,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:
@@ -1838,7 +1837,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-10 18:25:24 +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-10 18:25:24 +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-10 18:25:24 +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-10 18:25:24 +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()