Merge lp:~bac/launchpad/bug-607733 into lp:launchpad

[Brad Crittenden]

Change view permissions for ICommercialSubscription to be zope.Public to allow anonymous users to view a project's index page when that project has a commercial-use subscription.

= Summary =

Projects with commercial subscriptions cannot be viewed by anonymous
users, which is horrible.

== Proposed fix ==

Change ICommercialSubscription read permission from launchpad.View to
zope.Public.

== Pre-implementation notes ==

None

== Implementation details ==

As above.

== Tests ==

bin/test -vvt xx-product-index.txt

== Demo and Q/A ==

Logout and look at https://launchpad.net/swift

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/registry/configure.zcml
  lib/lp/registry/stories/product/xx-product-index.txt

[Curtis Hovey (sinzui)]

I Brad.

As I said on IRC. I see a permission change on the model, but a test of the view, which I know has its own permission settings. I expected to see a test of the model permissions, perhaps check_permission('zope.Public', subscription).

review: Needs Fixing (code)

[Brad Crittenden (bac)]

Good suggestion Curtis.

Here are the changes:
http://pastebin.ubuntu.com/470431/

[Curtis Hovey (sinzui)]

The additional tests look good. I think the story test is pointless--why does an annonymous user need to read when a license expires. Is there an action he needs to take. I think the browser tests should be removed.

review: Approve (code)

[Brad Crittenden (bac)]

The browser test was driven by the bug I was solving -- an anonymous user was unable to see that data and it triggered a login request. Proving the test passed after modifying the permission was useful and I feel a decent safeguard against a future regression.

=== modified file 'lib/lp/registry/configure.zcml'
--- lib/lp/registry/configure.zcml	2010-07-09 14:58:42 +0000
+++ lib/lp/registry/configure.zcml	2010-07-29 02:07:45 +0000
@@ -465,7 +465,7 @@
     <class
         class="lp.registry.model.commercialsubscription.CommercialSubscription">
         <require
-            permission="launchpad.View"
+            permission="zope.Public"
             interface="lp.registry.interfaces.commercialsubscription.ICommercialSubscription"/>
         <require
             permission="launchpad.Commercial"

=== modified file 'lib/lp/registry/doc/commercialsubscription.txt'
--- lib/lp/registry/doc/commercialsubscription.txt	2010-04-03 17:07:26 +0000
+++ lib/lp/registry/doc/commercialsubscription.txt	2010-07-29 02:07:45 +0000
@@ -1,4 +1,6 @@
-= Commercial Subscriptions =
+========================
+Commercial Subscriptions
+========================
 
 The CommercialSubscription class is used to track whether a project,
 which does not qualify for free hosting, has an unexpired subscription.
@@ -32,6 +34,43 @@
     >>> print bzr.commercial_subscription.sales_system_id
     asdf123
 
+Commercial subscriptions have zope.Public permissions for reading.
+
+    >>> from canonical.launchpad.webapp.authorization import check_permission
+    >>> login(ANONYMOUS)
+    >>> check_permission('zope.Public', bzr.commercial_subscription)
+    True
+    >>> print bzr.commercial_subscription.product.name
+    bzr
+
+For modification, launchpad.Commerical is required.  Anonymous users,
+regular users, and the project owner all are denied.
+
+    >>> check_permission('launchpad.Commercial', bzr.commercial_subscription)
+    False
+
+    >>> login('[e-mail]')
+    >>> check_permission('launchpad.Commercial', bzr.commercial_subscription)
+    False
+
+    >>> login_person(owner)
+    >>> check_permission('launchpad.Commercial', bzr.commercial_subscription)
+    False
+
+A member of the commercial admins team does have modification privileges.
+
+    >>> from canonical.launchpad.interfaces.launchpad import (
+    ...     ILaunchpadCelebrities)
+    >>> celebs = getUtility(ILaunchpadCelebrities)
+    >>> commercial_admin = celebs.commercial_admin
+    >>> login('[e-mail]')
+    >>> commercial_member = getUtility(ILaunchBag).user
+    >>> commercial_member.inTeam(commercial_admin)
+    True
+    >>> check_permission('launchpad.Commercial', bzr.commercial_subscription)
+    True
+
+
 The expiration date can be extended by redeeming another voucher.  In
 real life this would be done nearer to the expiration date.  For
 testing consistency, let's force the start and expiry dates to a known
@@ -40,6 +79,7 @@
     >>> from zope.security.proxy import removeSecurityProxy
     >>> from datetime import date, datetime, timedelta
     >>> from pytz import UTC
+    >>> login('[e-mail]')
     >>> now = datetime(2008, 1, 10, tzinfo=UTC)
     >>> subscription = removeSecurityProxy(bzr.commercial_subscription)
     >>> subscription.date_starts = datetime(2008,1,15,tzinfo=UTC)
@@ -321,7 +361,9 @@
     >>> bzr.qualifies_for_free_hosting, bzr.commercial_subscription_is_due
     (True, False)
 
-= Product License Reviews =
+=======================
+Product License Reviews
+=======================
 
 The forReview() method allows searching for products whose license needs to
 be reviewed. You can search by text in the Product table's full text index.
@@ -486,8 +528,6 @@
     ...     print product.name
     bzr
 
-
-
 You can search for products based on the date of that
 its commercial subscription was modified, which only
 happens when a voucher is redeemed.
@@ -530,9 +570,8 @@
 
 Anonymous users cannot access 'forReview'.
 
-    >>> from canonical.launchpad.webapp.authorization import check_permission
     >>> login(ANONYMOUS)
-    >>> check_permission('launchpad.Commercial', product_set)
+    >>> check_permission('launchpad.ProjectReview', product_set)
     False
     >>> gnome = product_set.forReview(search_text='gnome')
     Traceback (most recent call last):
@@ -553,10 +592,7 @@
 access it.
 
     >>> login('[e-mail]')
-    >>> from canonical.launchpad.interfaces.launchpad import (
-    ...     ILaunchpadCelebrities)
     >>> registry_member = factory.makePerson()
-    >>> celebs = getUtility(ILaunchpadCelebrities)
     >>> registry = celebs.registry_experts
     >>> ignored = registry.addMember(registry_member, registry.teamowner)
     >>> logout()