Merge lp:~adeuring/launchpad/product-sharing-sec-adapter into lp:launchpad

[Abel Deuring]

require the permission launchpad.View for formerly public properties of IProduct; replace the permission launchpad.AnyAllowedPerson instead of launchpad.AnyPerson

This branch changes the security configuration for IProduct.

Access to most properties of IProduct now requires the permission
launchpad.LimitedView or launchpad.AnyAllowedPerson. The only still
publicly available properties are id, information_type and a new method
userCanView()

Details:

- A new interface class IProductLimitedView, which defines most
  properties that were previously defined in IProductPublic

- IProductPublic now defines only the properties id, information_type
  and userCanView()

- registry/configure.zcml now requires the permissions launchpad.LimitedView
  or launchpad.AnyAllowedPerson for most "partial interfaces" of IProduct.

- new security adapters ViewProduct (for the permission lp.LimitedView)
  and ChangeProduct (for the permission lp.AnyAllowedPerson).
  These security adapters check if a project is public; if so, they allow
  access for any logged in person; ViewProduct also allws access for
  anonymous users in this case. For non-public projects, these adapters
  use the new method IProduct.userCanView() to check if the current user
  may be given access.

- the implementation of userCanView() is obviously incomplete: Only
  a project owner can currently access data of a private project.
  I will change this in a follow-up branch so that userCanView() will
  check if the user has an access policy grant for the given product.
  (The implementation of this change would have made the diff of this
  branch a bit too large ;)

  The implementation of IProduct.information_type is also still
  incomplete (the DB patch that adds a column Product.information_type
  has not yet been applied). Products are by default public; only some
  new tests set information_type to a non-public state, but this
  change is obviously not permanent. Hence there on risk that properties
  of any existing product become accidentally inaccessible.

- the tests test_get_permissions() and test_set_permissions() may look a
  bit excessive, but having the permissions neede for all properties
  documented in the dictionaries expected_[gs]et_permissions helped
  me to check that I did not leave any property publicly accessible.

- I had to change some websevice tests: webservice_for_person() calls
  endInteraction(), so we don't have any user/principal defined when
  this function finishes. This mean that access to an attribute of a
  product will fail, if no new interaction is created. This is why the
  changes store for example product.owner in alocal variable right after
  the product has been generated. For assertions that access
  product attributes, I used the context manager person_logged_in().

tests:

bin/test -vvt lp.registry.tests.test_product.TestProduct.test_.et_permissions
bin/test -vvt lp.registry.tests.test_product.TestProduct.test_access_launchpad_LimitedView_proprietary_product
bin/test -vvt lp.registry.tests.test_product.TestProduct.test_access_launchpad_LimitedView_public_product
bin/test -vvt lp.registry.tests.test_product.TestProduct.test_access_launchpad_AnyAllowedPerson_proprietary_product
bin/test -vvt lp.registry.tests.test_product.TestProduct.test_access_launchpad_AnyAllowedPerson_public_product

no lint.

[Francesco Banconi (frankban)]

Looks good Abel, thank you. Some minor details follow.

256 + def check_permissions(self, expected_permissions, used_permissions,
257 + type_):

There is a check_permissions function in lp.blueprints.tests.test_specification, which is very similar. Maybe we could abstract them out.

420 + def test_access_launchpad_View_proprietary_product(self):
421 + # Only people with grants for a prviate product can access
422 + # attributes protected by the permission launchapd.View.

Typos: private, launchpad. The same below in test_access_launchpad_AnyAllowedPerson_proprietary_product and in
test_set_launchpad_AnyAllowedPerson_proprietary_product.

439 + def test_access_launchpad_AnyAllowedPerson_public_product(self):
440 + # Only logged in persons hav access to properties of public products

Typo: have

475 + def test_set_launchpad_AnyAllowedPerson_public_product(self):
476 + # Only logged in users can set attributes protected by the
477 + # permission launchapd.AnyAllowedPerson.

Typo: launchpad

review: Approve

=== modified file 'lib/lp/registry/configure.zcml'
--- lib/lp/registry/configure.zcml	2012-09-25 04:29:34 +0000
+++ lib/lp/registry/configure.zcml	2012-10-02 14:25:30 +0000
@@ -1229,11 +1229,17 @@
         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.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"
@@ -1311,7 +1317,7 @@
                 translationpermission
                 translations_usage"/>
         <require
-            permission="zope.Public"
+            permission="launchpad.View"
             attributes="
                 qualifies_for_free_hosting"/>
         <require
@@ -1326,7 +1332,8 @@
 
         <!-- IHasAliases -->
 
-        <allow
+        <require
+            permission="launchpad.View"
             attributes="
                 aliases"/>
         <require
@@ -1336,14 +1343,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"/>
@@ -1354,15 +1364,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-09-28 06:15:58 +0000
+++ lib/lp/registry/interfaces/product.py	2012-10-02 14:25:30 +0000
@@ -422,7 +422,22 @@
                 "Not applicable to 'Other/Proprietary'.")))
 
 
-class IProductPublic(
+class IProductPublic(Interface):
+
+    id = Int(title=_('The Project ID'))
+
+    information_type = exported(
+        Choice(
+            title=_('Information Type'), vocabulary=InformationType,
+            required=True, readonly=True,
+            description=_(
+                'The type of of data contained in this project.')))
+
+    def userCanView(user):
+        """True if the given user has access to this product."""
+
+
+class IProductLimitedView(
     IBugTarget, ICanGetMilestonesDirectly, IHasAppointedDriver, IHasBranches,
     IHasBranchVisibilityPolicy, IHasDrivers, IHasExternalBugTracker, IHasIcon,
     IHasLogo, IHasMergeProposals, IHasMilestones,
@@ -432,8 +447,6 @@
     ISpecificationTarget, IHasRecipes, IHasCodeImports, IServiceUsage):
     """Public IProduct properties."""
 
-    id = Int(title=_('The Project ID'))
-
     project = exported(
         ReferenceChoice(
             title=_('Part of'),
@@ -450,13 +463,6 @@
                 'and security policy will apply to this project.')),
         exported_as='project_group')
 
-    information_type = exported(
-        Choice(
-            title=_('Information Type'), vocabulary=InformationType,
-            required=True, readonly=True,
-            description=_(
-                'The type of of data contained in this project.')))
-
     owner = exported(
         PersonChoice(
             title=_('Maintainer'),
@@ -893,9 +899,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):
@@ -904,10 +910,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):
@@ -916,10 +922,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):
@@ -932,7 +938,7 @@
 class IProduct(
     IHasBugSupervisor, IProductEditRestricted,
     IProductModerateRestricted, IProductDriverRestricted,
-    IProductPublic, IQuestionTarget, IRootContext,
+    IProductLimitedView, IProductPublic, IQuestionTarget, IRootContext,
     IStructuralSubscriptionTarget):
     """A Product.
 

=== modified file 'lib/lp/registry/model/product.py'
--- lib/lp/registry/model/product.py	2012-09-28 19:59:35 +0000
+++ lib/lp/registry/model/product.py	2012-10-02 14:25:30 +0000
@@ -69,6 +69,7 @@
     FREE_INFORMATION_TYPES,
     InformationType,
     PRIVATE_INFORMATION_TYPES,
+    PUBLIC_INFORMATION_TYPES,
     service_uses_launchpad,
     ServiceUsage,
     )
@@ -413,15 +414,17 @@
         """
         pass
 
-    @property
-    def information_type(self):
-        """See `IProduct`
-
-        Place holder for a db column.
-        XXX: rharding 2012-09-10 bug=1048720: Waiting on db patch to connect
-        into place.
-        """
-        pass
+    # Place holder for a db column.
+    # XXX: rharding 2012-09-10 bug=1048720: Waiting on db patch to connect
+    # into place.
+    def _get_information_type(self):
+        return getattr(
+            self, '_information_type', InformationType.PUBLIC)
+
+    def _set_information_type(self, value):
+        self._information_type = value
+
+    information_type = property(_get_information_type, _set_information_type)
 
     security_contact = None
 
@@ -1518,6 +1521,12 @@
 
         return weight_function
 
+    def userCanView(self, user):
+        """See `IProductPublic`."""
+        if self.information_type in PUBLIC_INFORMATION_TYPES:
+            return True
+        return user.inTeam(self.owner)
+
 
 class ProductSet:
     implements(IProductSet)

=== modified file 'lib/lp/registry/tests/test_product.py'
--- lib/lp/registry/tests/test_product.py	2012-09-28 06:15:58 +0000
+++ lib/lp/registry/tests/test_product.py	2012-10-02 14:25:30 +0000
@@ -11,6 +11,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
 
@@ -390,6 +394,256 @@
         expected = [InformationType.PROPRIETARY]
         self.assertContentEqual(expected, [policy.type for policy in aps])
 
+    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(('id', 'information_type', 'userCanView',)),
+        'launchpad.View': set((
+            '_getOfficialTagClause', 'active', 'active_or_packaged_series',
+            'aliases', 'all_milestones', 'all_specifications',
+            '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',
+            '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', 'displayname', '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', 'getSubscription',
+            'getSubscriptions', 'getSupportedLanguages', 'getTimeline',
+            'getTopContributors', 'getTopContributorsGroupedByCategory',
+            'getTranslationGroups', 'getTranslationImportQueueEntries',
+            'getTranslators', 'getUsedBugTagsWithOpenCounts',
+            'getVersionSortedSeries', 'has_any_specifications',
+            'has_current_commercial_subscription',
+            'has_custom_language_codes', 'has_milestones', 'homepage_content',
+            'homepageurl', 'icon', 'invitesTranslationEdits',
+            'invitesTranslationSuggestions',
+            'isUsingInheritedBranchVisibilityPolicy',
+            'latest_completed_specifications', 'latest_specifications',
+            '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',
+            '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', 'target_type_display', 'title',
+            'translatable_packages', 'translatable_series',
+            'translation_focus', 'translationgroup', 'translationgroups',
+            'translationpermission', 'translations_usage', 'ubuntu_packages',
+            'userCanAlterBugSubscription', 'userCanAlterSubscription',
+            'userCanEdit', 'userHasBugSubscriptions', 'uses_launchpad',
+            'valid_specifications', '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', 'license_info',
+                'licenses', 'logo', 'mugshot', 'official_answers',
+                'official_blueprints', 'official_codehosting', 'owner',
+                '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.factory.makeProduct(
+            information_type=InformationType.PROPRIETARY)
+        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(removeSecurityProxy(product).owner):
+            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.factory.makeProduct(
+            information_type=InformationType.PROPRIETARY)
+        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(removeSecurityProxy(product).owner):
+            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.factory.makeProduct(
+            information_type=InformationType.PROPRIETARY)
+        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(removeSecurityProxy(product).owner):
+            setattr(product, 'date_next_suggest_packaging', 'foo')
+
 
 class TestProductBugInformationTypes(TestCaseWithFactory):
 

=== modified file 'lib/lp/registry/tests/test_product_webservice.py'
--- lib/lp/registry/tests/test_product_webservice.py	2012-09-19 05:15:39 +0000
+++ lib/lp/registry/tests/test_product_webservice.py	2012-10-02 14:25:30 +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/security.py'
--- lib/lp/security.py	2012-09-19 04:09:06 +0000
+++ lib/lp/security.py	2012-10-02 14:25:30 +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,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-02 07:08:13 +0000
+++ lib/lp/testing/factory.py	2012-10-02 14:25:30 +0000
@@ -964,7 +964,8 @@
         title=None, summary=None, official_malone=None,
         translations_usage=None, bug_supervisor=None, private_bugs=False,
         driver=None, icon=None, bug_sharing_policy=None,
-        branch_sharing_policy=None, specification_sharing_policy=None):
+        branch_sharing_policy=None, specification_sharing_policy=None,
+        information_type=None):
         """Create and return a new, arbitrary Product."""
         if owner is None:
             owner = self.makePerson()
@@ -1020,6 +1021,8 @@
         if specification_sharing_policy:
             naked_product.setSpecificationSharingPolicy(
                 specification_sharing_policy)
+        if information_type is not None:
+            naked_product.information_type = information_type
 
         return product
 

=== modified file 'lib/lp/testing/pages.py'
--- lib/lp/testing/pages.py	2012-08-14 23:27:07 +0000
+++ lib/lp/testing/pages.py	2012-10-02 14:25:30 +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():