Merge lp:~adeuring/launchpad/product-lp-limitedview-2 into lp:launchpad

[Abel Deuring]

Require the permission launchpad.LimitedView for IProduct attributes needed to render a bug page.

This branch changes the permision required to access a number
of IProduct attributes from launchpad.View to
launchpad.LimitedView.

The changes are necessary to let a user with an artifact grant on
a bug of a private product view the main bug page. This is tested in
test_bugtask_view_user_with_grant_on_bug_for_private_product().

For some attributes, this means that an attribute definition
is moved from the interface class IProductView to IProductLimitedView.

Other attributes are defined in classes which IProductView inherited from.
If these classes defined just the attributes required to let the test pass
(IHasLogo and IHasOnwer) or if the attributes do not provide any data that
might be considered "really confidential", those of ILaunchpadUsage,
these classes are inherited by IProductLimitedView.

Permissions for IBugTarget and IStructuralSubscriptionTarget are now
defined attribute by attribute.

These changes are not sufficient to let a user with a grant on a bug
view bug pages: LP's traversal machinery still return a 404 error
because it checks if the current user has the permission lp.View for
the bug's target. I'll fix this in another branch.

tests:

./bin/test bugs -vvt test_bugtask_view_user_with_grant_on_bug_for_private_product
./bin/test registry -vvt lp.registry.tests.test_product.TestProduct.test_access
./bin/test registry -vvt lp.registry.tests.test_product.TestProduct.test_.et_permissions

no lint

[j.c.sackett (jcsackett)]

Abel--

This looks ok, just one nitpick you can address before landing.

#16: It seems odd to have a test with no assertions--I believe this may throw
off some test machinery. At a minimum, probably worth testing that the bug
does show in contents:

    contents = view.render()
    self.assertTrue(bug.title in contents)

review: Approve

=== modified file 'lib/lp/bugs/browser/tests/test_bug_views.py'
--- lib/lp/bugs/browser/tests/test_bug_views.py	2012-11-11 23:39:34 +0000
+++ lib/lp/bugs/browser/tests/test_bug_views.py	2012-11-12 11:53:43 +0000
@@ -28,6 +28,7 @@
 from zope.security.proxy import removeSecurityProxy
 
 from lp.app.enums import InformationType
+from lp.app.interfaces.services import IService
 from lp.bugs.adapters.bugchange import BugAttachmentChange
 from lp.registry.enums import BugSharingPolicy
 from lp.registry.interfaces.accesspolicy import (
@@ -499,6 +500,28 @@
         self.assertEqual(
             u'Private', soup.find('label', text="Private"))
 
+    def test_bugtask_view_user_with_grant_on_bug_for_private_product(self):
+        # The regular bug view is properly rendered even if the user
+        # does not have permissions to view every detail of a product.
+        owner = self.factory.makePerson()
+        product = self.factory.makeProduct(
+            owner=owner,
+            information_type=InformationType.PROPRIETARY)
+        user = self.factory.makePerson()
+        with person_logged_in(owner):
+            bug = self.factory.makeBug(
+                target=product, information_type=InformationType.PROPRIETARY)
+            getUtility(IService, 'sharing').ensureAccessGrants(
+                [user], owner, bugs=[bug])
+            launchbag = getUtility(IOpenLaunchBag)
+            launchbag.add(bug)
+            launchbag.add(bug.default_bugtask)
+        with person_logged_in(user):
+            view = create_initialized_view(
+                bug.default_bugtask, name=u'+index', principal=user)
+            contents = view.render()
+            self.assertTrue(bug.title in contents)
+
 
 class TestBugTextViewPrivateTeams(TestCaseWithFactory):
     """ Test for rendering BugTextView with private team artifacts.

=== modified file 'lib/lp/registry/configure.zcml'
--- lib/lp/registry/configure.zcml	2012-11-11 23:39:34 +0000
+++ lib/lp/registry/configure.zcml	2012-11-12 11:53:43 +0000
@@ -1356,6 +1356,25 @@
             set_schema="lp.registry.interfaces.product.IProductModerateRestricted"
             set_attributes="active"/>
 
+        <!-- IBugTarget -->
+
+        <require
+            permission="launchpad.LimitedView"
+            attributes="
+                bugtargetdisplayname"/>
+
+        <require
+            permission="launchpad.View"
+            attributes="
+                bug_reported_acknowledgement
+                bug_reporting_guidelines
+                bugtargetname
+                createBug
+                enable_bugfiling_duplicate_search
+                getBugTaskWeightFunction
+                pillar
+                searchTasks"/>
+
         <!-- IHasAliases -->
 
         <require
@@ -1391,8 +1410,19 @@
         <!-- IStructuralSubscriptionTarget -->
 
         <require
+            permission="launchpad.LimitedView"
+            attributes="
+                parent_subscription_target" />
+        <require
             permission="launchpad.View"
-            interface="lp.bugs.interfaces.structuralsubscription.IStructuralSubscriptionTargetRead" />
+            attributes="
+                bug_subscriptions
+                getSubscription
+                getSubscriptions
+                target_type_display
+                userCanAlterBugSubscription
+                userCanAlterSubscription
+                userHasBugSubscriptions" />
         <require
             permission="launchpad.AnyAllowedPerson"
             interface="lp.bugs.interfaces.structuralsubscription.IStructuralSubscriptionTargetWrite" />

=== modified file 'lib/lp/registry/interfaces/product.py'
--- lib/lp/registry/interfaces/product.py	2012-11-11 23:39:34 +0000
+++ lib/lp/registry/interfaces/product.py	2012-11-12 11:53:43 +0000
@@ -428,11 +428,27 @@
         """True if the given user has access to this product."""
 
 
-class IProductLimitedView(Interface):
+class IProductLimitedView(IHasLogo, IHasOwner, ILaunchpadUsage):
     """Attributes that must be visible for person with artifact grants
     on bugs, branches or specifications for the product.
     """
 
+    displayname = exported(
+        TextLine(
+            title=_('Display Name'),
+            description=_("""The name of the project as it would appear in a
+                paragraph.""")),
+        exported_as='display_name')
+
+    logo = exported(
+        LogoImageUpload(
+            title=_("Logo"), required=False,
+            default_image_resource='/@@/product-logo',
+            description=_(
+                "An image of exactly 64x64 pixels that will be displayed in "
+                "the heading of all pages related to this project. It should "
+                "be no bigger than 50kb in size.")))
+
     name = exported(
         ProductNameField(
             title=_('Name'),
@@ -442,16 +458,14 @@
                 "letters, numbers, dots, hyphens or pluses. "
                 "Keep this name short; it is used in URLs as shown above.")))
 
-
-class IProductView(
-    IBugTarget, ICanGetMilestonesDirectly, IHasAppointedDriver, IHasBranches,
-    IHasDrivers, IHasExternalBugTracker, IHasIcon,
-    IHasLogo, IHasMergeProposals, IHasMilestones, IHasExpirableBugs,
-    IHasMugshot, IHasOwner, IHasSprints, IHasTranslationImports,
-    ITranslationPolicy, IKarmaContext, ILaunchpadUsage, IMakesAnnouncements,
-    IOfficialBugTagTargetPublic, IHasOOPSReferences,
-    ISpecificationTarget, IHasRecipes, IHasCodeImports, IServiceUsage):
-    """Public IProduct properties."""
+    owner = exported(
+        PersonChoice(
+            title=_('Maintainer'),
+            required=True,
+            vocabulary='ValidPillarOwner',
+            description=_("The restricted team, moderated team, or person "
+                          "who maintains the project information in "
+                          "Launchpad.")))
 
     project = exported(
         ReferenceChoice(
@@ -469,14 +483,21 @@
                 'and security policy will apply to this project.')),
         exported_as='project_group')
 
-    owner = exported(
-        PersonChoice(
-            title=_('Maintainer'),
-            required=True,
-            vocabulary='ValidPillarOwner',
-            description=_("The restricted team, moderated team, or person "
-                          "who maintains the project information in "
-                          "Launchpad.")))
+    title = exported(
+        Title(
+            title=_('Title'),
+            description=_("The project title. Should be just a few words.")))
+
+
+class IProductView(
+    ICanGetMilestonesDirectly, IHasAppointedDriver, IHasBranches,
+    IHasDrivers, IHasExternalBugTracker, IHasIcon,
+    IHasMergeProposals, IHasMilestones, IHasExpirableBugs,
+    IHasMugshot, IHasSprints, IHasTranslationImports,
+    ITranslationPolicy, IKarmaContext, IMakesAnnouncements,
+    IOfficialBugTagTargetPublic, IHasOOPSReferences,
+    ISpecificationTarget, IHasRecipes, IHasCodeImports, IServiceUsage):
+    """Public IProduct properties."""
 
     registrant = exported(
         PublicPersonChoice(
@@ -503,18 +524,6 @@
         "required because there might be a project driver and also a "
         "driver appointed in the overarching project group.")
 
-    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'),
-            description=_("The project title. Should be just a few words.")))
-
     summary = exported(
         Summary(
             title=_('Summary'),
@@ -614,15 +623,6 @@
                 "will be displayed next to the project name everywhere in "
                 "Launchpad that we refer to the project and link to it.")))
 
-    logo = exported(
-        LogoImageUpload(
-            title=_("Logo"), required=False,
-            default_image_resource='/@@/product-logo',
-            description=_(
-                "An image of exactly 64x64 pixels that will be displayed in "
-                "the heading of all pages related to this project. It should "
-                "be no bigger than 50kb in size.")))
-
     mugshot = exported(
         MugshotImageUpload(
             title=_("Brand"), required=False,
@@ -917,7 +917,7 @@
 
 
 class IProduct(
-    IHasBugSupervisor, IProductEditRestricted,
+    IBugTarget, IHasBugSupervisor, IProductEditRestricted,
     IProductModerateRestricted, IProductDriverRestricted, IProductView,
     IProductLimitedView, IProductPublic, IQuestionTarget, IRootContext,
     IStructuralSubscriptionTarget, IInformationType, IPillar):

=== modified file 'lib/lp/registry/tests/test_product.py'
--- lib/lp/registry/tests/test_product.py	2012-11-11 23:39:34 +0000
+++ lib/lp/registry/tests/test_product.py	2012-11-12 11:53:43 +0000
@@ -551,7 +551,11 @@
         CheckerPublic: set((
             'active', 'id', 'information_type', 'pillar_category', 'private',
             'userCanView',)),
-        'launchpad.LimitedView': set(('name', )),
+        'launchpad.LimitedView': set((
+            'bugtargetdisplayname', 'displayname', 'enable_bug_expiration',
+            'logo', 'name', 'official_answers', 'official_anything',
+            'official_blueprints', 'official_codehosting', 'official_malone',
+            'owner', 'parent_subscription_target', 'project', 'title', )),
         'launchpad.View': set((
             '_getOfficialTagClause', '_all_specifications',
             '_valid_specifications', 'active_or_packaged_series',
@@ -561,16 +565,15 @@
             'blueprints_usage', 'branch_sharing_policy',
             'bug_reported_acknowledgement', 'bug_reporting_guidelines',
             'bug_sharing_policy', 'bug_subscriptions', 'bug_supervisor',
-            'bug_tracking_usage', 'bugtargetdisplayname', 'bugtargetname',
-            'bugtracker', 'canUserAlterAnswerContact',
-            'codehosting_usage',
+            'bug_tracking_usage', 'bugtargetname',
+            'bugtracker', 'canUserAlterAnswerContact', 'codehosting_usage',
             'coming_sprints', 'commercial_subscription',
             'commercial_subscription_is_due', 'createBug',
             'createCustomLanguageCode', 'custom_language_codes',
             'date_next_suggest_packaging', 'datecreated', 'description',
             'development_focus', 'development_focusID',
-            'direct_answer_contacts', 'displayname', 'distrosourcepackages',
-            'downloadurl', 'driver', 'drivers', 'enable_bug_expiration',
+            'direct_answer_contacts', 'distrosourcepackages',
+            'downloadurl', 'driver', 'drivers',
             'enable_bugfiling_duplicate_search', 'findReferencedOOPS',
             'findSimilarFAQs', 'findSimilarQuestions', 'freshmeatproject',
             'getAllowedBugInformationTypes',
@@ -594,15 +597,13 @@
             'has_custom_language_codes', 'has_milestones', 'homepage_content',
             'homepageurl', 'icon', 'invitesTranslationEdits',
             'invitesTranslationSuggestions',
-            'license_info', 'license_status', 'licenses', 'logo', 'milestones',
+            'license_info', 'license_status', 'licenses', 'milestones',
             'mugshot', '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',
+            'obsolete_translatable_series', 'official_bug_tags',
+            'packagedInDistros', 'packagings',
             'past_sprints', 'personHasDriverRights', 'pillar',
             'primary_translatable', 'private_bugs',
-            'programminglang', 'project', 'qualifies_for_free_hosting',
+            'programminglang', 'qualifies_for_free_hosting',
             'recipes', 'redeemSubscriptionVoucher', 'registrant', 'releases',
             'remote_product', 'removeCustomLanguageCode',
             'screenshotsurl',
@@ -610,7 +611,7 @@
             'series',
             'sharesTranslationsWithOtherSide', 'sourceforgeproject',
             'sourcepackages', 'specification_sharing_policy', 'specifications',
-            'sprints', 'summary', 'target_type_display', 'title',
+            'sprints', 'summary', 'target_type_display',
             'translatable_packages', 'translatable_series',
             'translation_focus', 'translationgroup', 'translationgroups',
             'translationpermission', 'translations_usage', 'ubuntu_packages',