Merge lp:~wgrant/launchpad/question-content-lockdown into lp:launchpad

Proposed by William Grant
Status: Merged
Merged at revision: 17859
Proposed branch: lp:~wgrant/launchpad/question-content-lockdown
Merge into: lp:launchpad
Diff against target: 261 lines (+89/-30)
6 files modified
lib/lp/answers/browser/tests/views.txt (+16/-9)
lib/lp/answers/configure.zcml (+5/-1)
lib/lp/answers/doc/notifications.txt (+2/-2)
lib/lp/answers/stories/question-edit.txt (+17/-14)
lib/lp/answers/tests/test_question.py (+35/-1)
lib/lp/security.py (+14/-3)
To merge this branch: bzr merge lp:~wgrant/launchpad/question-content-lockdown
Reviewer Review Type Date Requested Status
Colin Watson (community) Approve
Review via email: mp+279069@code.launchpad.net

Commit message

Lock down Question.title/description edits from random users.

Description of the change

Lock down Question.title/description edits from random users.

Question content doesn't usually evolve through the life cycle as bug content does, and there is no audit trail, so letting any user edit the title and description is mostly useless and very abusable. Those fields are now behind launchpad.Edit, which is a union of launchpad.Owner (creator) and launchpad.Append (target owner, ~admins, and now ~registry).

To post a comment you must log in.
Revision history for this message
Colin Watson (cjwatson) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/answers/browser/tests/views.txt'
2--- lib/lp/answers/browser/tests/views.txt 2015-09-28 11:13:42 +0000
3+++ lib/lp/answers/browser/tests/views.txt 2015-12-01 11:38:41 +0000
4@@ -447,12 +447,6 @@
5
6 >>> view = getMultiAdapter((question_three, request), name='+edit')
7 >>> view.initialize()
8- >>> question_three.title
9- u'Better Title'
10-
11- >>> question_three.description
12- u'A better description.'
13-
14 >>> print question_three.distribution.name
15 ubuntu
16
17@@ -462,9 +456,16 @@
18 >>> print question_three.product
19 None
20
21-Since a user must have launchpad.Moderator privilege to change the
22-assignee and launchpad.Admin privilege to change status whiteboard, the
23-values are unchanged.
24+Since a user must have launchpad.Edit (question creator or target answer
25+contact) to change the title or description, launchpad.Append (target
26+answer contact) to change the assignee and launchpad.Admin (target
27+owner) to change status whiteboard, the values are unchanged.
28+
29+ >>> question_three.title
30+ u'Firefox is slow and consumes too much RAM'
31+
32+ >>> question_three.description
33+ u"I'm running on a 486 with 32 MB ram. And Firefox is slow! What should I do?"
34
35 >>> question_three.assignee is None
36 True
37@@ -489,6 +490,12 @@
38 >>> request.method = 'POST'
39 >>> view = getMultiAdapter((question_three, request), name='+edit')
40 >>> view.initialize()
41+ >>> question_three.title
42+ u'Better Title'
43+
44+ >>> question_three.description
45+ u'A better description.'
46+
47 >>> print question_three.assignee.displayname
48 Foo Bar
49
50
51=== modified file 'lib/lp/answers/configure.zcml'
52--- lib/lp/answers/configure.zcml 2015-09-25 02:33:15 +0000
53+++ lib/lp/answers/configure.zcml 2015-12-01 11:38:41 +0000
54@@ -116,7 +116,7 @@
55 attributes="requestInfo giveAnswer expireQuestion addComment
56 addCommentWithoutNotify linkFAQ subscribe unsubscribe
57 setCommentVisibility"
58- set_attributes="title description datedue target language"
59+ set_attributes="datedue target language"
60 />
61 <require
62 permission="launchpad.Append"
63@@ -124,6 +124,10 @@
64 set_attributes="assignee"
65 />
66 <require
67+ permission="launchpad.Edit"
68+ set_attributes="title description"
69+ />
70+ <require
71 permission="launchpad.Admin"
72 attributes="setStatus"
73 set_attributes="priority whiteboard"
74
75=== modified file 'lib/lp/answers/doc/notifications.txt'
76--- lib/lp/answers/doc/notifications.txt 2015-09-25 03:02:28 +0000
77+++ lib/lp/answers/doc/notifications.txt 2015-12-01 11:38:41 +0000
78@@ -88,8 +88,7 @@
79 >>> from lazr.lifecycle.event import ObjectModifiedEvent
80 >>> from lazr.lifecycle.snapshot import Snapshot
81
82- >>> login('no-priv@canonical.com')
83- >>> no_priv = getUtility(ILaunchBag).user
84+ >>> login('test@canonical.com')
85 >>> unmodified_question = Snapshot(
86 ... ubuntu_question, providing=providedBy(ubuntu_question))
87 >>> ubuntu_question.title = "Installer doesn't work on a Mac"
88@@ -147,6 +146,7 @@
89 Changing the assignee will trigger a notification.
90
91 >>> login('foo.bar@canonical.com')
92+ >>> no_priv = getUtility(IPersonSet).getByName('no-priv')
93 >>> unmodified_question = Snapshot(
94 ... ubuntu_question, providing=providedBy(ubuntu_question))
95 >>> ubuntu_question.assignee = no_priv
96
97=== modified file 'lib/lp/answers/stories/question-edit.txt'
98--- lib/lp/answers/stories/question-edit.txt 2012-12-11 02:19:13 +0000
99+++ lib/lp/answers/stories/question-edit.txt 2015-12-01 11:38:41 +0000
100@@ -1,7 +1,9 @@
101 = Editing Questions =
102
103-To edit the title and description of question, one uses the 'Edit Question'
104-menu item. You need to be logged in to perform that action:
105+To edit the title and description of question, one uses the 'Edit
106+Question' menu item. You need to be logged in to see the edit form, and
107+only the question creator or an answer contact can change the title and
108+description.
109
110 >>> anon_browser.open('http://launchpad.dev/firefox/+question/2')
111 >>> anon_browser.getLink('Edit question').click()
112@@ -9,14 +11,15 @@
113 ...
114 Unauthorized...
115
116- >>> user_browser.open('http://launchpad.dev/firefox/+question/2')
117- >>> user_browser.getLink('Edit question').click()
118- >>> print user_browser.url
119+ >>> test_browser = setupBrowser(auth='Basic test@canonical.com:test')
120+ >>> test_browser.open('http://launchpad.dev/firefox/+question/2')
121+ >>> test_browser.getLink('Edit question').click()
122+ >>> print test_browser.url
123 http://answers.launchpad.dev/firefox/+question/2/+edit
124
125 There is a cancel link should the user decide otherwise:
126
127- >>> print user_browser.getLink('Cancel').url
128+ >>> print test_browser.getLink('Cancel').url
129 http://answers.launchpad.dev/firefox/+question/2
130
131 When we post the form, we should be redirected back to the question page.
132@@ -24,17 +27,17 @@
133 >>> description = (
134 ... "Hi! I'm trying to learn about SVG but I can't get it to work at "
135 ... "all in firefox. Maybe there is a plugin? Help! Thanks. Mark")
136- >>> user_browser.getControl('Description').value = description
137+ >>> test_browser.getControl('Description').value = description
138 >>> summary = "Problem showing the SVG demo on W3C web site"
139- >>> user_browser.getControl('Summary').value = summary
140- >>> user_browser.getControl('Save Changes').click()
141+ >>> test_browser.getControl('Summary').value = summary
142+ >>> test_browser.getControl('Save Changes').click()
143
144- >>> print user_browser.url
145+ >>> print test_browser.url
146 http://answers.launchpad.dev/firefox/+question/2
147
148 And viewing that page should show the updated information.
149
150- >>> soup = find_main_content(user_browser.contents)
151+ >>> soup = find_main_content(test_browser.contents)
152 >>> print soup.first('div', 'report').renderContents().strip()
153 <p>Hi! I&#x27;m trying to learn about SVG but I can&#x27;t get it to
154 work at all in firefox. Maybe there is a plugin? Help! Thanks.
155@@ -49,11 +52,11 @@
156 ... print extract_text(
157 ... find_tag_by_id(browser.contents, 'question-status'))
158
159- >>> user_browser.open('http://launchpad.dev/ubuntu/+question/3')
160- >>> print_question_status(user_browser)
161+ >>> test_browser.open('http://launchpad.dev/ubuntu/+question/3')
162+ >>> print_question_status(test_browser)
163 Status: Invalid
164
165- >>> user_browser.getLink('Edit question')
166+ >>> test_browser.getLink('Edit question')
167 <Link...>
168
169
170
171=== modified file 'lib/lp/answers/tests/test_question.py'
172--- lib/lp/answers/tests/test_question.py 2015-01-29 14:14:01 +0000
173+++ lib/lp/answers/tests/test_question.py 2015-12-01 11:38:41 +0000
174@@ -3,12 +3,46 @@
175
176 __metaclass__ = type
177
178+from testtools.testcase import ExpectedException
179+from zope.security.interfaces import Unauthorized
180 from zope.security.proxy import removeSecurityProxy
181
182-from lp.testing import TestCaseWithFactory
183+from lp.testing import (
184+ admin_logged_in,
185+ anonymous_logged_in,
186+ celebrity_logged_in,
187+ person_logged_in,
188+ TestCaseWithFactory,
189+ )
190 from lp.testing.layers import DatabaseFunctionalLayer
191
192
193+class TestQuestionSecurity(TestCaseWithFactory):
194+
195+ layer = DatabaseFunctionalLayer
196+
197+ def test_title_and_description_writes(self):
198+ question = self.factory.makeQuestion()
199+ with anonymous_logged_in():
200+ with ExpectedException(Unauthorized):
201+ question.title = 'foo anon'
202+ with ExpectedException(Unauthorized):
203+ question.description = 'foo anon'
204+ with person_logged_in(self.factory.makePerson()):
205+ with ExpectedException(Unauthorized):
206+ question.title = 'foo random'
207+ with ExpectedException(Unauthorized):
208+ question.description = 'foo random'
209+ with person_logged_in(question.owner):
210+ question.title = question.description = 'foo owner'
211+ with person_logged_in(question.target.owner):
212+ question.title = question.description = 'foo target owner'
213+ with admin_logged_in():
214+ question.title = question.description = 'foo admin'
215+ with celebrity_logged_in('registry_experts'):
216+ question.title = question.description = 'foo registry'
217+
218+
219 class TestQuestionSearch(TestCaseWithFactory):
220 layer = DatabaseFunctionalLayer
221
222
223=== modified file 'lib/lp/security.py'
224--- lib/lp/security.py 2015-11-26 15:46:38 +0000
225+++ lib/lp/security.py 2015-12-01 11:38:41 +0000
226@@ -1969,15 +1969,16 @@
227 super(ViewTranslationTemplatesBuild, self).__init__(obj, obj.branch)
228
229
230-class AdminQuestion(AdminByAdminsTeam):
231+class AdminQuestion(AuthorizationBase):
232 permission = 'launchpad.Admin'
233 usedfor = IQuestion
234
235 def checkAuthenticated(self, user):
236 """Allow only admins and owners of the question pillar target."""
237 context = self.obj.product or self.obj.distribution
238- return (AdminByAdminsTeam.checkAuthenticated(self, user) or
239- user.inTeam(context.owner))
240+ return (
241+ user.in_admin or user.in_registry_experts
242+ or user.inTeam(context.owner))
243
244
245 class AppendQuestion(AdminQuestion):
246@@ -2012,6 +2013,16 @@
247 return user.inTeam(self.obj.owner)
248
249
250+class EditQuestion(AuthorizationBase):
251+ permission = 'launchpad.Edit'
252+ usedfor = IQuestion
253+
254+ def checkAuthenticated(self, user):
255+ return (
256+ AppendQuestion(self.obj).checkAuthenticated(user)
257+ or QuestionOwner(self.obj).checkAuthenticated(user))
258+
259+
260 class ViewQuestion(AnonymousAuthorization):
261 usedfor = IQuestion
262