Launchpad itself

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

  1. bug-607733
  2. Merge into devel
Proposed by Brad Crittenden
Status: Merged
Approved by: Curtis Hovey
Approved revision: no longer in the source branch.
Merged at revision: 11252
Proposed branch: lp:~bac/launchpad/bug-607733
Merge into: lp:launchpad
Diff against target: 118 lines (+46/-10)
2 files modified
lib/lp/registry/configure.zcml (+1/-1)
lib/lp/registry/doc/commercialsubscription.txt (+45/-9)
To merge this branch: bzr merge lp:~bac/launchpad/bug-607733
Reviewer Review Type Date Requested Status
Curtis Hovey (community) code Approve
Edwin Grubbs Pending
Review via email: mp+31195@code.launchpad.net

Commit message

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.

Description of the change

= 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

To post a comment you must log in.
Revision history for this message
Curtis Hovey (sinzui) wrote :

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)
Revision history for this message
Brad Crittenden (bac) wrote :

Good suggestion Curtis.

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

Revision history for this message
Curtis Hovey (sinzui) wrote :

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)
Revision history for this message
Brad Crittenden (bac) wrote :

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.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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 @@
465 <class465 <class
466 class="lp.registry.model.commercialsubscription.CommercialSubscription">466 class="lp.registry.model.commercialsubscription.CommercialSubscription">
467 <require467 <require
468 permission="launchpad.View"468 permission="zope.Public"
469 interface="lp.registry.interfaces.commercialsubscription.ICommercialSubscription"/>469 interface="lp.registry.interfaces.commercialsubscription.ICommercialSubscription"/>
470 <require470 <require
471 permission="launchpad.Commercial"471 permission="launchpad.Commercial"
472472
=== 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 @@
1= Commercial Subscriptions =1========================
2Commercial Subscriptions
3========================
24
3The CommercialSubscription class is used to track whether a project,5The CommercialSubscription class is used to track whether a project,
4which does not qualify for free hosting, has an unexpired subscription.6which does not qualify for free hosting, has an unexpired subscription.
@@ -32,6 +34,43 @@
32 >>> print bzr.commercial_subscription.sales_system_id34 >>> print bzr.commercial_subscription.sales_system_id
33 asdf12335 asdf123
3436
37Commercial subscriptions have zope.Public permissions for reading.
38
39 >>> from canonical.launchpad.webapp.authorization import check_permission
40 >>> login(ANONYMOUS)
41 >>> check_permission('zope.Public', bzr.commercial_subscription)
42 True
43 >>> print bzr.commercial_subscription.product.name
44 bzr
45
46For modification, launchpad.Commerical is required. Anonymous users,
47regular users, and the project owner all are denied.
48
49 >>> check_permission('launchpad.Commercial', bzr.commercial_subscription)
50 False
51
52 >>> login('no-priv@canonical.com')
53 >>> check_permission('launchpad.Commercial', bzr.commercial_subscription)
54 False
55
56 >>> login_person(owner)
57 >>> check_permission('launchpad.Commercial', bzr.commercial_subscription)
58 False
59
60A member of the commercial admins team does have modification privileges.
61
62 >>> from canonical.launchpad.interfaces.launchpad import (
63 ... ILaunchpadCelebrities)
64 >>> celebs = getUtility(ILaunchpadCelebrities)
65 >>> commercial_admin = celebs.commercial_admin
66 >>> login('commercial-member@canonical.com')
67 >>> commercial_member = getUtility(ILaunchBag).user
68 >>> commercial_member.inTeam(commercial_admin)
69 True
70 >>> check_permission('launchpad.Commercial', bzr.commercial_subscription)
71 True
72
73
35The expiration date can be extended by redeeming another voucher. In74The expiration date can be extended by redeeming another voucher. In
36real life this would be done nearer to the expiration date. For75real life this would be done nearer to the expiration date. For
37testing consistency, let's force the start and expiry dates to a known76testing consistency, let's force the start and expiry dates to a known
@@ -40,6 +79,7 @@
40 >>> from zope.security.proxy import removeSecurityProxy79 >>> from zope.security.proxy import removeSecurityProxy
41 >>> from datetime import date, datetime, timedelta80 >>> from datetime import date, datetime, timedelta
42 >>> from pytz import UTC81 >>> from pytz import UTC
82 >>> login('no-priv@canonical.com')
43 >>> now = datetime(2008, 1, 10, tzinfo=UTC)83 >>> now = datetime(2008, 1, 10, tzinfo=UTC)
44 >>> subscription = removeSecurityProxy(bzr.commercial_subscription)84 >>> subscription = removeSecurityProxy(bzr.commercial_subscription)
45 >>> subscription.date_starts = datetime(2008,1,15,tzinfo=UTC)85 >>> subscription.date_starts = datetime(2008,1,15,tzinfo=UTC)
@@ -321,7 +361,9 @@
321 >>> bzr.qualifies_for_free_hosting, bzr.commercial_subscription_is_due361 >>> bzr.qualifies_for_free_hosting, bzr.commercial_subscription_is_due
322 (True, False)362 (True, False)
323363
324= Product License Reviews =364=======================
365Product License Reviews
366=======================
325367
326The forReview() method allows searching for products whose license needs to368The forReview() method allows searching for products whose license needs to
327be reviewed. You can search by text in the Product table's full text index.369be reviewed. You can search by text in the Product table's full text index.
@@ -486,8 +528,6 @@
486 ... print product.name528 ... print product.name
487 bzr529 bzr
488530
489
490
491You can search for products based on the date of that531You can search for products based on the date of that
492its commercial subscription was modified, which only532its commercial subscription was modified, which only
493happens when a voucher is redeemed.533happens when a voucher is redeemed.
@@ -530,9 +570,8 @@
530570
531Anonymous users cannot access 'forReview'.571Anonymous users cannot access 'forReview'.
532572
533 >>> from canonical.launchpad.webapp.authorization import check_permission
534 >>> login(ANONYMOUS)573 >>> login(ANONYMOUS)
535 >>> check_permission('launchpad.Commercial', product_set)574 >>> check_permission('launchpad.ProjectReview', product_set)
536 False575 False
537 >>> gnome = product_set.forReview(search_text='gnome')576 >>> gnome = product_set.forReview(search_text='gnome')
538 Traceback (most recent call last):577 Traceback (most recent call last):
@@ -553,10 +592,7 @@
553access it.592access it.
554593
555 >>> login('foo.bar@canonical.com')594 >>> login('foo.bar@canonical.com')
556 >>> from canonical.launchpad.interfaces.launchpad import (
557 ... ILaunchpadCelebrities)
558 >>> registry_member = factory.makePerson()595 >>> registry_member = factory.makePerson()
559 >>> celebs = getUtility(ILaunchpadCelebrities)
560 >>> registry = celebs.registry_experts596 >>> registry = celebs.registry_experts
561 >>> ignored = registry.addMember(registry_member, registry.teamowner)597 >>> ignored = registry.addMember(registry_member, registry.teamowner)
562 >>> logout()598 >>> logout()