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
1=== modified file 'lib/lp/registry/configure.zcml'
2--- lib/lp/registry/configure.zcml 2010-07-09 14:58:42 +0000
3+++ lib/lp/registry/configure.zcml 2010-07-29 02:07:45 +0000
4@@ -465,7 +465,7 @@
5 <class
6 class="lp.registry.model.commercialsubscription.CommercialSubscription">
7 <require
8- permission="launchpad.View"
9+ permission="zope.Public"
10 interface="lp.registry.interfaces.commercialsubscription.ICommercialSubscription"/>
11 <require
12 permission="launchpad.Commercial"
13
14=== modified file 'lib/lp/registry/doc/commercialsubscription.txt'
15--- lib/lp/registry/doc/commercialsubscription.txt 2010-04-03 17:07:26 +0000
16+++ lib/lp/registry/doc/commercialsubscription.txt 2010-07-29 02:07:45 +0000
17@@ -1,4 +1,6 @@
18-= Commercial Subscriptions =
19+========================
20+Commercial Subscriptions
21+========================
22
23 The CommercialSubscription class is used to track whether a project,
24 which does not qualify for free hosting, has an unexpired subscription.
25@@ -32,6 +34,43 @@
26 >>> print bzr.commercial_subscription.sales_system_id
27 asdf123
28
29+Commercial subscriptions have zope.Public permissions for reading.
30+
31+ >>> from canonical.launchpad.webapp.authorization import check_permission
32+ >>> login(ANONYMOUS)
33+ >>> check_permission('zope.Public', bzr.commercial_subscription)
34+ True
35+ >>> print bzr.commercial_subscription.product.name
36+ bzr
37+
38+For modification, launchpad.Commerical is required. Anonymous users,
39+regular users, and the project owner all are denied.
40+
41+ >>> check_permission('launchpad.Commercial', bzr.commercial_subscription)
42+ False
43+
44+ >>> login('no-priv@canonical.com')
45+ >>> check_permission('launchpad.Commercial', bzr.commercial_subscription)
46+ False
47+
48+ >>> login_person(owner)
49+ >>> check_permission('launchpad.Commercial', bzr.commercial_subscription)
50+ False
51+
52+A member of the commercial admins team does have modification privileges.
53+
54+ >>> from canonical.launchpad.interfaces.launchpad import (
55+ ... ILaunchpadCelebrities)
56+ >>> celebs = getUtility(ILaunchpadCelebrities)
57+ >>> commercial_admin = celebs.commercial_admin
58+ >>> login('commercial-member@canonical.com')
59+ >>> commercial_member = getUtility(ILaunchBag).user
60+ >>> commercial_member.inTeam(commercial_admin)
61+ True
62+ >>> check_permission('launchpad.Commercial', bzr.commercial_subscription)
63+ True
64+
65+
66 The expiration date can be extended by redeeming another voucher. In
67 real life this would be done nearer to the expiration date. For
68 testing consistency, let's force the start and expiry dates to a known
69@@ -40,6 +79,7 @@
70 >>> from zope.security.proxy import removeSecurityProxy
71 >>> from datetime import date, datetime, timedelta
72 >>> from pytz import UTC
73+ >>> login('no-priv@canonical.com')
74 >>> now = datetime(2008, 1, 10, tzinfo=UTC)
75 >>> subscription = removeSecurityProxy(bzr.commercial_subscription)
76 >>> subscription.date_starts = datetime(2008,1,15,tzinfo=UTC)
77@@ -321,7 +361,9 @@
78 >>> bzr.qualifies_for_free_hosting, bzr.commercial_subscription_is_due
79 (True, False)
80
81-= Product License Reviews =
82+=======================
83+Product License Reviews
84+=======================
85
86 The forReview() method allows searching for products whose license needs to
87 be reviewed. You can search by text in the Product table's full text index.
88@@ -486,8 +528,6 @@
89 ... print product.name
90 bzr
91
92-
93-
94 You can search for products based on the date of that
95 its commercial subscription was modified, which only
96 happens when a voucher is redeemed.
97@@ -530,9 +570,8 @@
98
99 Anonymous users cannot access 'forReview'.
100
101- >>> from canonical.launchpad.webapp.authorization import check_permission
102 >>> login(ANONYMOUS)
103- >>> check_permission('launchpad.Commercial', product_set)
104+ >>> check_permission('launchpad.ProjectReview', product_set)
105 False
106 >>> gnome = product_set.forReview(search_text='gnome')
107 Traceback (most recent call last):
108@@ -553,10 +592,7 @@
109 access it.
110
111 >>> login('foo.bar@canonical.com')
112- >>> from canonical.launchpad.interfaces.launchpad import (
113- ... ILaunchpadCelebrities)
114 >>> registry_member = factory.makePerson()
115- >>> celebs = getUtility(ILaunchpadCelebrities)
116 >>> registry = celebs.registry_experts
117 >>> ignored = registry.addMember(registry_member, registry.teamowner)
118 >>> logout()