Merge ~cjwatson/launchpad:project-series-expensive-exports into launchpad:master
- Git
- lp:~cjwatson/launchpad
- project-series-expensive-exports
- Merge into master
Proposed by
Colin Watson
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | Colin Watson | ||||
Approved revision: | 20d7835caaae618a968ab5906b386d93ed260c66 | ||||
Merge reported by: | Otto Co-Pilot | ||||
Merged at revision: | not available | ||||
Proposed branch: | ~cjwatson/launchpad:project-series-expensive-exports | ||||
Merge into: | launchpad:master | ||||
Diff against target: |
465 lines (+218/-63) 8 files modified
lib/lp/registry/security.py (+33/-0) lib/lp/translations/browser/configure.zcml (+1/-1) lib/lp/translations/browser/product.py (+3/-0) lib/lp/translations/browser/productseries.py (+1/-0) lib/lp/translations/stories/productseries/xx-productseries-translation-export.rst (+137/-40) lib/lp/translations/stories/standalone/xx-product-export.rst (+31/-15) lib/lp/translations/templates/product-translations.pt (+7/-3) lib/lp/translations/templates/productseries-translations.pt (+5/-4) |
||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Andrey Fedoseev (community) | Approve | ||
Review via email: mp+433813@code.launchpad.net |
Commit message
Disallow ProductSeries translation exports to most users
Description of the change
Exporting translations for an entire `ProductSeries` can be benign, but in some cases (e.g. `/linuxmint/latest` on production) it can be very expensive; and users who aren't admins or responsible for the project in question should generally export an individual `POTemplate` or `POFile` instead. Restrict this operation to admins, owners of the project, release managers of the project series, or owners of applicable translation groups, in much the same way as we already do for translation exports of entire source packages.
To post a comment you must log in.
Revision history for this message
Andrey Fedoseev (andrey-fedoseev) : | # |
review:
Approve
- 20d7835... by Colin Watson
-
Remove unnecessary condition
Revision history for this message
Colin Watson (cjwatson) : | # |
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | diff --git a/lib/lp/registry/security.py b/lib/lp/registry/security.py |
2 | index 17c201f..14866a0 100644 |
3 | --- a/lib/lp/registry/security.py |
4 | +++ b/lib/lp/registry/security.py |
5 | @@ -273,6 +273,39 @@ class DownloadFullSourcePackageTranslations(OnlyRosettaExpertsAndAdmins): |
6 | ) |
7 | |
8 | |
9 | +class DownloadFullProductSeriesTranslations(OnlyRosettaExpertsAndAdmins): |
10 | + """Restrict full `ProductSeries` translation downloads. |
11 | + |
12 | + Some product series contain a large number of templates, and requests |
13 | + for those can swamp the export queue. Most translators probably only |
14 | + need individual files. |
15 | + """ |
16 | + |
17 | + permission = "launchpad.ExpensiveRequest" |
18 | + usedfor = IProductSeries |
19 | + |
20 | + def checkAuthenticated(self, user): |
21 | + """Define who may download these translations. |
22 | + |
23 | + Admins and Translations admins have access, as does the owner of |
24 | + the translation group (if applicable) and distribution uploaders. |
25 | + """ |
26 | + translation_group = self.obj.product.translationgroup |
27 | + return ( |
28 | + # User is admin of some relevant kind. |
29 | + OnlyRosettaExpertsAndAdmins.checkAuthenticated(self, user) |
30 | + # User is the owner of the product, or the release manager of |
31 | + # the series. |
32 | + or user.isOwner(self.obj.product) |
33 | + or user.isDriver(self.obj) |
34 | + # User is owner of applicable translation group. |
35 | + or ( |
36 | + translation_group is not None |
37 | + and user.inTeam(translation_group.owner) |
38 | + ) |
39 | + ) |
40 | + |
41 | + |
42 | class EditProductRelease(EditByOwnersOrAdmins): |
43 | permission = "launchpad.Edit" |
44 | usedfor = IProductRelease |
45 | diff --git a/lib/lp/translations/browser/configure.zcml b/lib/lp/translations/browser/configure.zcml |
46 | index 6c3e0a6..61fcbc1 100644 |
47 | --- a/lib/lp/translations/browser/configure.zcml |
48 | +++ b/lib/lp/translations/browser/configure.zcml |
49 | @@ -600,7 +600,7 @@ |
50 | <browser:page |
51 | name="+export" |
52 | for="lp.registry.interfaces.productseries.IProductSeries" |
53 | - permission="launchpad.AnyPerson" |
54 | + permission="launchpad.ExpensiveRequest" |
55 | template="../templates/translations-export.pt" |
56 | class="lp.translations.browser.productseries.ProductSeriesTranslationsExportView"/> |
57 | <browser:page |
58 | diff --git a/lib/lp/translations/browser/product.py b/lib/lp/translations/browser/product.py |
59 | index 844252f..71fc174 100644 |
60 | --- a/lib/lp/translations/browser/product.py |
61 | +++ b/lib/lp/translations/browser/product.py |
62 | @@ -55,6 +55,9 @@ class ProductTranslationsMenu(NavigationMenu): |
63 | enabled = ( |
64 | service_uses_launchpad(self.context.translations_usage) |
65 | and preferred_series is not None |
66 | + and check_permission( |
67 | + "launchpad.ExpensiveRequest", preferred_series |
68 | + ) |
69 | ) |
70 | link = "" |
71 | if enabled: |
72 | diff --git a/lib/lp/translations/browser/productseries.py b/lib/lp/translations/browser/productseries.py |
73 | index 34bed51..8a6d4e4 100644 |
74 | --- a/lib/lp/translations/browser/productseries.py |
75 | +++ b/lib/lp/translations/browser/productseries.py |
76 | @@ -93,6 +93,7 @@ class ProductSeriesTranslationsMenuMixIn: |
77 | """Return a link to upload translations.""" |
78 | return Link("+translations-upload", "Upload", site="translations") |
79 | |
80 | + @enabled_with_permission("launchpad.ExpensiveRequest") |
81 | def translationdownload(self): |
82 | """Return a link to download the translations.""" |
83 | return Link("+export", "Download", site="translations") |
84 | diff --git a/lib/lp/translations/stories/productseries/xx-productseries-translation-export.rst b/lib/lp/translations/stories/productseries/xx-productseries-translation-export.rst |
85 | index d5e1561..e599501 100644 |
86 | --- a/lib/lp/translations/stories/productseries/xx-productseries-translation-export.rst |
87 | +++ b/lib/lp/translations/stories/productseries/xx-productseries-translation-export.rst |
88 | @@ -9,42 +9,141 @@ all templates for the product series as well as all their translations. |
89 | Where to request |
90 | ---------------- |
91 | |
92 | -Any authenticated user can request a tarball export for a productseries. The |
93 | -option is presented in the sidebar as "Download translations" on the |
94 | -productseries' translations page. |
95 | +For qualified users (see below), the option is presented in the sidebar as |
96 | +"Download translations" on the productseries' translations page. |
97 | |
98 | - >>> user_browser.open( |
99 | - ... "http://translations.launchpad.test/evolution/trunk/" |
100 | - ... ) |
101 | - >>> user_browser.getLink("download").click() |
102 | - >>> print(user_browser.url) |
103 | +Mark is a qualified user. |
104 | + |
105 | + >>> browser = setupBrowser(auth="Basic mark@example.com:test") |
106 | + >>> browser.open("http://translations.launchpad.test/evolution/trunk/") |
107 | + >>> download = browser.getLink("download") |
108 | + >>> download_url = download.url |
109 | + >>> download.click() |
110 | + >>> print(browser.url) |
111 | http://translations.launchpad.test/evolution/trunk/+export |
112 | |
113 | Another way of getting there is by going to the product's +translate page. |
114 | It will select a series of the product as its primary translation target, and |
115 | offer a download link for that series. |
116 | |
117 | - >>> user_browser.open( |
118 | + >>> browser.open( |
119 | ... "http://translations.launchpad.test/evolution/+translations" |
120 | ... ) |
121 | - >>> user_browser.getLink("download").click() |
122 | - >>> print(user_browser.url) |
123 | + >>> browser.getLink("download").click() |
124 | + >>> print(browser.url) |
125 | http://translations.launchpad.test/evolution/trunk/+export |
126 | |
127 | |
128 | -Anonymous access |
129 | ----------------- |
130 | +Authorization |
131 | +------------- |
132 | |
133 | -To prevent spambot activity from becoming a problem, the download option is |
134 | -not available to anonymous visitors. |
135 | +The option to download an entire productseries' translations is restricted |
136 | +to users who are involved in certain ways, in order to keep load to a |
137 | +reasonable level. |
138 | |
139 | - >>> anon_browser.open( |
140 | - ... "http://translations.launchpad.test/evolution/trunk/" |
141 | - ... ) |
142 | - >>> anon_browser.getLink("download") |
143 | - Traceback (most recent call last): |
144 | + >>> from zope.security.interfaces import Unauthorized |
145 | + >>> from zope.testbrowser.browser import LinkNotFoundError |
146 | + |
147 | + >>> def can_download_translations(browser): |
148 | + ... """Can browser download full series translations? |
149 | ... |
150 | - zope.testbrowser.browser.LinkNotFoundError |
151 | + ... Checks for the "Download" link on a series. |
152 | + ... Also attempts direct access to the same series' download |
153 | + ... page and sees that the two have consistent access rules. |
154 | + ... """ |
155 | + ... browser.open( |
156 | + ... "http://translations.launchpad.test/evolution/trunk/" |
157 | + ... ) |
158 | + ... try: |
159 | + ... browser.getLink("download").click() |
160 | + ... except LinkNotFoundError: |
161 | + ... see_link = False |
162 | + ... else: |
163 | + ... see_link = True |
164 | + ... |
165 | + ... try: |
166 | + ... browser.open(download_url) |
167 | + ... except Unauthorized: |
168 | + ... have_access = False |
169 | + ... else: |
170 | + ... have_access = True |
171 | + ... |
172 | + ... if have_access != see_link: |
173 | + ... if have_access: |
174 | + ... return "Download link not shown, but direct URL works." |
175 | + ... else: |
176 | + ... return "Download link shown to unauthorized user." |
177 | + ... |
178 | + ... return have_access |
179 | + ... |
180 | + |
181 | +An arbitrary user visiting the series' translations page does not see the |
182 | +download link for the full series, and cannot download. |
183 | + |
184 | + >>> can_download_translations(user_browser) |
185 | + False |
186 | + |
187 | +It's the same for anonymous visitors. |
188 | + |
189 | + >>> can_download_translations(anon_browser) |
190 | + False |
191 | + |
192 | +An administrator, of course, can download the full translations. |
193 | + |
194 | + >>> can_download_translations(admin_browser) |
195 | + True |
196 | + |
197 | +A translations expert can download the full translations. |
198 | + |
199 | + >>> translations_admin_browser = setupRosettaExpertBrowser() |
200 | + >>> can_download_translations(translations_admin_browser) |
201 | + True |
202 | + |
203 | +An owner of an unrelated translation group cannot download translations. |
204 | + |
205 | + >>> from lp.testing.pages import setupBrowserForUser |
206 | + |
207 | + >>> login("foo.bar@canonical.com") |
208 | + >>> group_owner = factory.makePerson() |
209 | + >>> translators = factory.makeTeam(group_owner) |
210 | + >>> group = factory.makeTranslationGroup(translators) |
211 | + >>> logout() |
212 | + >>> group_owner_browser = setupBrowserForUser(group_owner) |
213 | + >>> can_download_translations(group_owner_browser) |
214 | + False |
215 | + |
216 | +But if the translation group is in charge of translations for this product, |
217 | +then the translation group owner can download translations. |
218 | + |
219 | + >>> from zope.component import getUtility |
220 | + |
221 | + >>> from lp.registry.interfaces.product import IProductSet |
222 | + |
223 | + >>> login("foo.bar@canonical.com") |
224 | + >>> evolution = getUtility(IProductSet).getByName("evolution") |
225 | + >>> evolution.translationgroup = group |
226 | + >>> logout() |
227 | + >>> can_download_translations(group_owner_browser) |
228 | + True |
229 | + |
230 | +The owner of the product can download translations. |
231 | + |
232 | + >>> login("foo.bar@canonical.com") |
233 | + >>> evolution_owner = evolution.owner |
234 | + >>> logout() |
235 | + >>> evolution_owner_browser = setupBrowserForUser(evolution_owner) |
236 | + >>> can_download_translations(evolution_owner_browser) |
237 | + True |
238 | + |
239 | +The release manager of the product series can download translations. |
240 | + |
241 | + >>> login("foo.bar@canonical.com") |
242 | + >>> trunk_driver = factory.makePerson() |
243 | + >>> evolution.getSeries("trunk").driver = trunk_driver |
244 | + >>> logout() |
245 | + >>> trunk_driver_browser = setupBrowserForUser(trunk_driver) |
246 | + >>> can_download_translations(trunk_driver_browser) |
247 | + True |
248 | |
249 | |
250 | Making the request |
251 | @@ -53,7 +152,7 @@ Making the request |
252 | The logged-in user sees a page that lets them select an export format, and |
253 | request the download. |
254 | |
255 | - >>> print(user_browser.title) |
256 | + >>> print(browser.title) |
257 | Download : Series trunk : Translations... |
258 | |
259 | |
260 | @@ -62,32 +161,32 @@ File format |
261 | |
262 | The request must specify a file format. |
263 | |
264 | - >>> user_browser.getControl("Format:").clear() |
265 | - >>> user_browser.getControl("Request Download").click() |
266 | + >>> browser.getControl("Format:").clear() |
267 | + >>> browser.getControl("Request Download").click() |
268 | |
269 | - >>> print_feedback_messages(user_browser.contents) |
270 | + >>> print_feedback_messages(browser.contents) |
271 | Please select a valid format for download. |
272 | |
273 | The most usual and most well-supported format is PO. |
274 | |
275 | - >>> user_browser.getControl("Format:").value = ["PO"] |
276 | - >>> user_browser.getControl("Request Download").click() |
277 | + >>> browser.getControl("Format:").value = ["PO"] |
278 | + >>> browser.getControl("Request Download").click() |
279 | |
280 | - >>> print(user_browser.url) |
281 | + >>> print(browser.url) |
282 | http://translations.launchpad.test/evolution/trunk |
283 | |
284 | - >>> print_feedback_messages(user_browser.contents) |
285 | + >>> print_feedback_messages(browser.contents) |
286 | Your request has been received. Expect to receive an email shortly. |
287 | |
288 | An alternative is MO. |
289 | |
290 | - >>> user_browser.getLink("download").click() |
291 | - >>> user_browser.getControl("Format:").value = ["PO"] |
292 | - >>> user_browser.getControl("Request Download").click() |
293 | - >>> print(user_browser.url) |
294 | + >>> browser.getLink("download").click() |
295 | + >>> browser.getControl("Format:").value = ["PO"] |
296 | + >>> browser.getControl("Request Download").click() |
297 | + >>> print(browser.url) |
298 | http://translations.launchpad.test/evolution/trunk |
299 | |
300 | - >>> print_feedback_messages(user_browser.contents) |
301 | + >>> print_feedback_messages(browser.contents) |
302 | Your request has been received. Expect to receive an email shortly. |
303 | |
304 | |
305 | @@ -97,17 +196,15 @@ Nothing to export |
306 | Where there are no translation files to be exported, the user is not offered |
307 | the option to download any. |
308 | |
309 | - >>> user_browser.open( |
310 | - ... "http://translations.launchpad.test/bzr/trunk/+export" |
311 | - ... ) |
312 | - >>> print_feedback_messages(user_browser.contents) |
313 | + >>> browser.open("http://translations.launchpad.test/bzr/trunk/+export") |
314 | + >>> print_feedback_messages(browser.contents) |
315 | There are no translations to download in Bazaar trunk series. |
316 | |
317 | On +translate pages for products that do not have any translations, the action |
318 | link for "Download translations" is hidden. |
319 | |
320 | - >>> user_browser.open("http://translations.launchpad.test/bzr/") |
321 | - >>> user_browser.getLink("download") |
322 | + >>> browser.open("http://translations.launchpad.test/bzr/") |
323 | + >>> browser.getLink("download") |
324 | Traceback (most recent call last): |
325 | ... |
326 | zope.testbrowser.browser.LinkNotFoundError |
327 | diff --git a/lib/lp/translations/stories/standalone/xx-product-export.rst b/lib/lp/translations/stories/standalone/xx-product-export.rst |
328 | index 90c3d6d..73edc9c 100644 |
329 | --- a/lib/lp/translations/stories/standalone/xx-product-export.rst |
330 | +++ b/lib/lp/translations/stories/standalone/xx-product-export.rst |
331 | @@ -2,10 +2,13 @@ Downloading Product Series Translations |
332 | ======================================= |
333 | |
334 | Products and product series that use Translations offer complete |
335 | -translation downloads. |
336 | +translation downloads to qualified users. (See |
337 | +``../productseries/xx-productseries-translation-export.rst`` for more |
338 | +details on who counts as "qualified".) |
339 | |
340 | - >>> user_browser.open("http://translations.launchpad.test/evolution") |
341 | - >>> download = user_browser.getLink("download") |
342 | + >>> browser = setupBrowser(auth="Basic mark@example.com:test") |
343 | + >>> browser.open("http://translations.launchpad.test/evolution") |
344 | + >>> download = browser.getLink("download") |
345 | |
346 | For products, that option downloads translations for the series that is |
347 | currently the preferred translation target. |
348 | @@ -17,21 +20,19 @@ currently the preferred translation target. |
349 | Another way of getting that same export would be to browse to the series |
350 | first and requesting a download there. |
351 | |
352 | - >>> user_browser.open( |
353 | - ... "http://translations.launchpad.test/evolution/trunk" |
354 | - ... ) |
355 | - >>> user_browser.getLink("download").click() |
356 | - >>> user_browser.url |
357 | + >>> browser.open("http://translations.launchpad.test/evolution/trunk") |
358 | + >>> browser.getLink("download").click() |
359 | + >>> browser.url |
360 | 'http://translations.launchpad.test/evolution/trunk/+export' |
361 | |
362 | The translations export is implemented by the same machinery that does |
363 | it for source packages (tested and documented separately). |
364 | |
365 | - >>> print(user_browser.title) |
366 | + >>> print(browser.title) |
367 | Download : Series trunk : Translations... |
368 | |
369 | - >>> user_browser.getControl("Request Download").click() |
370 | - >>> print_feedback_messages(user_browser.contents) |
371 | + >>> browser.getControl("Request Download").click() |
372 | + >>> print_feedback_messages(browser.contents) |
373 | Your request has been received. Expect to receive an email shortly. |
374 | |
375 | |
376 | @@ -49,8 +50,8 @@ Use the DB classes directly to avoid having to setup a zope interaction |
377 | >>> product = Product.byName("evolution") |
378 | >>> product.translations_usage = ServiceUsage.NOT_APPLICABLE |
379 | >>> product.sync() |
380 | - >>> user_browser.open("http://translations.launchpad.test/evolution") |
381 | - >>> user_browser.getLink("download") |
382 | + >>> browser.open("http://translations.launchpad.test/evolution") |
383 | + >>> browser.getLink("download") |
384 | Traceback (most recent call last): |
385 | ... |
386 | zope.testbrowser.browser.LinkNotFoundError |
387 | @@ -59,8 +60,8 @@ Restore previous state for subsequent tests, and verify. |
388 | |
389 | >>> product.translations_usage = ServiceUsage.LAUNCHPAD |
390 | >>> product.sync() |
391 | - >>> user_browser.open("http://translations.launchpad.test/evolution") |
392 | - >>> user_browser.getLink("download") is not None |
393 | + >>> browser.open("http://translations.launchpad.test/evolution") |
394 | + >>> browser.getLink("download") is not None |
395 | True |
396 | |
397 | |
398 | @@ -75,6 +76,16 @@ Only logged-in users get the option to request downloads. |
399 | ... |
400 | zope.testbrowser.browser.LinkNotFoundError |
401 | |
402 | +Unqualified users (see |
403 | +``../productseries/xx-productseries-translation-export.rst``) do not get the |
404 | +option to request downloads. |
405 | + |
406 | + >>> user_browser.open("http://translations.launchpad.test/evolution") |
407 | + >>> user_browser.getLink("download") |
408 | + Traceback (most recent call last): |
409 | + ... |
410 | + zope.testbrowser.browser.LinkNotFoundError |
411 | + |
412 | We can't see its placeholder in non-development mode: |
413 | |
414 | >>> from lp.services.config import config |
415 | @@ -96,3 +107,8 @@ Even "hacking the URL" to the download option will fail. |
416 | Traceback (most recent call last): |
417 | ... |
418 | zope.security.interfaces.Unauthorized: ... |
419 | + |
420 | + >>> user_browser.open(download_url) |
421 | + Traceback (most recent call last): |
422 | + ... |
423 | + zope.security.interfaces.Unauthorized: ... |
424 | diff --git a/lib/lp/translations/templates/product-translations.pt b/lib/lp/translations/templates/product-translations.pt |
425 | index 61aa335..ad79952 100644 |
426 | --- a/lib/lp/translations/templates/product-translations.pt |
427 | +++ b/lib/lp/translations/templates/product-translations.pt |
428 | @@ -59,10 +59,14 @@ |
429 | >trunk</tal:target>. |
430 | <span tal:condition="context/required:launchpad.AnyPerson"> |
431 | You can also |
432 | - <a tal:attributes="href target/fmt:url:translations/+export" |
433 | - >download</a> |
434 | - <tal:owner-or-admin condition="admin_user"> |
435 | + <tal:expensive condition="target/required:launchpad.ExpensiveRequest"> |
436 | + <a tal:attributes="href target/fmt:url:translations/+export" |
437 | + >download</a> |
438 | + <tal:owner-or-admin condition="admin_user"> |
439 | or |
440 | + </tal:owner-or-admin> |
441 | + </tal:expensive> |
442 | + <tal:owner-or-admin condition="admin_user"> |
443 | <a tal:attributes=" |
444 | href target/fmt:url:translations/+translations-upload" |
445 | >upload</a> |
446 | diff --git a/lib/lp/translations/templates/productseries-translations.pt b/lib/lp/translations/templates/productseries-translations.pt |
447 | index 056d016..c3ef832 100644 |
448 | --- a/lib/lp/translations/templates/productseries-translations.pt |
449 | +++ b/lib/lp/translations/templates/productseries-translations.pt |
450 | @@ -186,10 +186,11 @@ |
451 | <a tal:attributes="href context/menu:navigation/translationupload/url" |
452 | tal:condition="context/required:launchpad.Edit" |
453 | class="add sprite">upload</a> |
454 | - or |
455 | - <a tal:attributes="href context/menu:navigation/translationdownload/url" |
456 | - tal:condition="context/required:launchpad.AnyPerson" |
457 | - class="download sprite">download</a> |
458 | + <tal:expensive condition="context/required:launchpad.ExpensiveRequest"> |
459 | + or |
460 | + <a tal:attributes="href context/menu:navigation/translationdownload/url" |
461 | + class="download sprite">download</a> |
462 | + </tal:expensive> |
463 | translation tarballs. |
464 | </p> |
465 | </div> |