← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] ~cjwatson/launchpad:project-series-expensive-exports into launchpad:master

 

Colin Watson has proposed merging ~cjwatson/launchpad:project-series-expensive-exports into launchpad:master.

Commit message:
Disallow ProductSeries translation exports to most users

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1998037 in Launchpad itself: "Translation download not working"
  https://bugs.launchpad.net/launchpad/+bug/1998037

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/433813

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.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:project-series-expensive-exports into launchpad:master.
diff --git a/lib/lp/registry/security.py b/lib/lp/registry/security.py
index 17c201f..14866a0 100644
--- a/lib/lp/registry/security.py
+++ b/lib/lp/registry/security.py
@@ -273,6 +273,39 @@ class DownloadFullSourcePackageTranslations(OnlyRosettaExpertsAndAdmins):
         )
 
 
+class DownloadFullProductSeriesTranslations(OnlyRosettaExpertsAndAdmins):
+    """Restrict full `ProductSeries` translation downloads.
+
+    Some product series contain a large number of templates, and requests
+    for those can swamp the export queue.  Most translators probably only
+    need individual files.
+    """
+
+    permission = "launchpad.ExpensiveRequest"
+    usedfor = IProductSeries
+
+    def checkAuthenticated(self, user):
+        """Define who may download these translations.
+
+        Admins and Translations admins have access, as does the owner of
+        the translation group (if applicable) and distribution uploaders.
+        """
+        translation_group = self.obj.product.translationgroup
+        return (
+            # User is admin of some relevant kind.
+            OnlyRosettaExpertsAndAdmins.checkAuthenticated(self, user)
+            # User is the owner of the product, or the release manager of
+            # the series.
+            or user.isOwner(self.obj.product)
+            or user.isDriver(self.obj)
+            # User is owner of applicable translation group.
+            or (
+                translation_group is not None
+                and user.inTeam(translation_group.owner)
+            )
+        )
+
+
 class EditProductRelease(EditByOwnersOrAdmins):
     permission = "launchpad.Edit"
     usedfor = IProductRelease
diff --git a/lib/lp/translations/browser/configure.zcml b/lib/lp/translations/browser/configure.zcml
index 6c3e0a6..61fcbc1 100644
--- a/lib/lp/translations/browser/configure.zcml
+++ b/lib/lp/translations/browser/configure.zcml
@@ -600,7 +600,7 @@
     <browser:page
         name="+export"
         for="lp.registry.interfaces.productseries.IProductSeries"
-        permission="launchpad.AnyPerson"
+        permission="launchpad.ExpensiveRequest"
         template="../templates/translations-export.pt"
         class="lp.translations.browser.productseries.ProductSeriesTranslationsExportView"/>
     <browser:page
diff --git a/lib/lp/translations/browser/product.py b/lib/lp/translations/browser/product.py
index 844252f..71fc174 100644
--- a/lib/lp/translations/browser/product.py
+++ b/lib/lp/translations/browser/product.py
@@ -55,6 +55,9 @@ class ProductTranslationsMenu(NavigationMenu):
         enabled = (
             service_uses_launchpad(self.context.translations_usage)
             and preferred_series is not None
+            and check_permission(
+                "launchpad.ExpensiveRequest", preferred_series
+            )
         )
         link = ""
         if enabled:
diff --git a/lib/lp/translations/browser/productseries.py b/lib/lp/translations/browser/productseries.py
index 34bed51..8a6d4e4 100644
--- a/lib/lp/translations/browser/productseries.py
+++ b/lib/lp/translations/browser/productseries.py
@@ -93,6 +93,7 @@ class ProductSeriesTranslationsMenuMixIn:
         """Return a link to upload translations."""
         return Link("+translations-upload", "Upload", site="translations")
 
+    @enabled_with_permission("launchpad.ExpensiveRequest")
     def translationdownload(self):
         """Return a link to download the translations."""
         return Link("+export", "Download", site="translations")
diff --git a/lib/lp/translations/stories/productseries/xx-productseries-translation-export.rst b/lib/lp/translations/stories/productseries/xx-productseries-translation-export.rst
index d5e1561..e599501 100644
--- a/lib/lp/translations/stories/productseries/xx-productseries-translation-export.rst
+++ b/lib/lp/translations/stories/productseries/xx-productseries-translation-export.rst
@@ -9,42 +9,141 @@ all templates for the product series as well as all their translations.
 Where to request
 ----------------
 
-Any authenticated user can request a tarball export for a productseries.  The
-option is presented in the sidebar as "Download translations" on the
-productseries' translations page.
+For qualified users (see below), the option is presented in the sidebar as
+"Download translations" on the productseries' translations page.
 
-    >>> user_browser.open(
-    ...     "http://translations.launchpad.test/evolution/trunk/";
-    ... )
-    >>> user_browser.getLink("download").click()
-    >>> print(user_browser.url)
+Mark is a qualified user.
+
+    >>> browser = setupBrowser(auth="Basic mark@xxxxxxxxxxx:test")
+    >>> browser.open("http://translations.launchpad.test/evolution/trunk/";)
+    >>> download = browser.getLink("download")
+    >>> download_url = download.url
+    >>> download.click()
+    >>> print(browser.url)
     http://translations.launchpad.test/evolution/trunk/+export
 
 Another way of getting there is by going to the product's +translate page.
 It will select a series of the product as its primary translation target, and
 offer a download link for that series.
 
-    >>> user_browser.open(
+    >>> browser.open(
     ...     "http://translations.launchpad.test/evolution/+translations";
     ... )
-    >>> user_browser.getLink("download").click()
-    >>> print(user_browser.url)
+    >>> browser.getLink("download").click()
+    >>> print(browser.url)
     http://translations.launchpad.test/evolution/trunk/+export
 
 
-Anonymous access
-----------------
+Authorization
+-------------
 
-To prevent spambot activity from becoming a problem, the download option is
-not available to anonymous visitors.
+The option to download an entire productseries' translations is restricted
+to users who are involved in certain ways, in order to keep load to a
+reasonable level.
 
-    >>> anon_browser.open(
-    ...     "http://translations.launchpad.test/evolution/trunk/";
-    ... )
-    >>> anon_browser.getLink("download")
-    Traceback (most recent call last):
+    >>> from zope.security.interfaces import Unauthorized
+    >>> from zope.testbrowser.browser import LinkNotFoundError
+
+    >>> def can_download_translations(browser):
+    ...     """Can browser download full series translations?
     ...
-    zope.testbrowser.browser.LinkNotFoundError
+    ...     Checks for the "Download" link on a series.
+    ...     Also attempts direct access to the same series' download
+    ...     page and sees that the two have consistent access rules.
+    ...     """
+    ...     browser.open(
+    ...         "http://translations.launchpad.test/evolution/trunk/";
+    ...     )
+    ...     try:
+    ...         browser.getLink("download").click()
+    ...     except LinkNotFoundError:
+    ...         see_link = False
+    ...     else:
+    ...         see_link = True
+    ...
+    ...     try:
+    ...         browser.open(download_url)
+    ...     except Unauthorized:
+    ...         have_access = False
+    ...     else:
+    ...         have_access = True
+    ...
+    ...     if have_access != see_link:
+    ...         if have_access:
+    ...             return "Download link not shown, but direct URL works."
+    ...         else:
+    ...             return "Download link shown to unauthorized user."
+    ...
+    ...     return have_access
+    ...
+
+An arbitrary user visiting the series' translations page does not see the
+download link for the full series, and cannot download.
+
+    >>> can_download_translations(user_browser)
+    False
+
+It's the same for anonymous visitors.
+
+    >>> can_download_translations(anon_browser)
+    False
+
+An administrator, of course, can download the full translations.
+
+    >>> can_download_translations(admin_browser)
+    True
+
+A translations expert can download the full translations.
+
+    >>> translations_admin_browser = setupRosettaExpertBrowser()
+    >>> can_download_translations(translations_admin_browser)
+    True
+
+An owner of an unrelated translation group cannot download translations.
+
+    >>> from lp.testing.pages import setupBrowserForUser
+
+    >>> login("foo.bar@xxxxxxxxxxxxx")
+    >>> group_owner = factory.makePerson()
+    >>> translators = factory.makeTeam(group_owner)
+    >>> group = factory.makeTranslationGroup(translators)
+    >>> logout()
+    >>> group_owner_browser = setupBrowserForUser(group_owner)
+    >>> can_download_translations(group_owner_browser)
+    False
+
+But if the translation group is in charge of translations for this product,
+then the translation group owner can download translations.
+
+    >>> from zope.component import getUtility
+
+    >>> from lp.registry.interfaces.product import IProductSet
+
+    >>> login("foo.bar@xxxxxxxxxxxxx")
+    >>> evolution = getUtility(IProductSet).getByName("evolution")
+    >>> evolution.translationgroup = group
+    >>> logout()
+    >>> can_download_translations(group_owner_browser)
+    True
+
+The owner of the product can download translations.
+
+    >>> login("foo.bar@xxxxxxxxxxxxx")
+    >>> evolution_owner = evolution.owner
+    >>> logout()
+    >>> evolution_owner_browser = setupBrowserForUser(evolution_owner)
+    >>> can_download_translations(evolution_owner_browser)
+    True
+
+The release manager of the product series can download translations.
+
+    >>> login("foo.bar@xxxxxxxxxxxxx")
+    >>> trunk_driver = factory.makePerson()
+    >>> evolution.getSeries("trunk").driver = trunk_driver
+    >>> logout()
+    >>> trunk_driver_browser = setupBrowserForUser(trunk_driver)
+    >>> can_download_translations(trunk_driver_browser)
+    True
 
 
 Making the request
@@ -53,7 +152,7 @@ Making the request
 The logged-in user sees a page that lets them select an export format, and
 request the download.
 
-    >>> print(user_browser.title)
+    >>> print(browser.title)
     Download : Series trunk : Translations...
 
 
@@ -62,32 +161,32 @@ File format
 
 The request must specify a file format.
 
-    >>> user_browser.getControl("Format:").clear()
-    >>> user_browser.getControl("Request Download").click()
+    >>> browser.getControl("Format:").clear()
+    >>> browser.getControl("Request Download").click()
 
-    >>> print_feedback_messages(user_browser.contents)
+    >>> print_feedback_messages(browser.contents)
     Please select a valid format for download.
 
 The most usual and most well-supported format is PO.
 
-    >>> user_browser.getControl("Format:").value = ["PO"]
-    >>> user_browser.getControl("Request Download").click()
+    >>> browser.getControl("Format:").value = ["PO"]
+    >>> browser.getControl("Request Download").click()
 
-    >>> print(user_browser.url)
+    >>> print(browser.url)
     http://translations.launchpad.test/evolution/trunk
 
-    >>> print_feedback_messages(user_browser.contents)
+    >>> print_feedback_messages(browser.contents)
     Your request has been received. Expect to receive an email shortly.
 
 An alternative is MO.
 
-    >>> user_browser.getLink("download").click()
-    >>> user_browser.getControl("Format:").value = ["PO"]
-    >>> user_browser.getControl("Request Download").click()
-    >>> print(user_browser.url)
+    >>> browser.getLink("download").click()
+    >>> browser.getControl("Format:").value = ["PO"]
+    >>> browser.getControl("Request Download").click()
+    >>> print(browser.url)
     http://translations.launchpad.test/evolution/trunk
 
-    >>> print_feedback_messages(user_browser.contents)
+    >>> print_feedback_messages(browser.contents)
     Your request has been received. Expect to receive an email shortly.
 
 
@@ -97,17 +196,15 @@ Nothing to export
 Where there are no translation files to be exported, the user is not offered
 the option to download any.
 
-    >>> user_browser.open(
-    ...     "http://translations.launchpad.test/bzr/trunk/+export";
-    ... )
-    >>> print_feedback_messages(user_browser.contents)
+    >>> browser.open("http://translations.launchpad.test/bzr/trunk/+export";)
+    >>> print_feedback_messages(browser.contents)
     There are no translations to download in Bazaar trunk series.
 
 On +translate pages for products that do not have any translations, the action
 link for "Download translations" is hidden.
 
-    >>> user_browser.open("http://translations.launchpad.test/bzr/";)
-    >>> user_browser.getLink("download")
+    >>> browser.open("http://translations.launchpad.test/bzr/";)
+    >>> browser.getLink("download")
     Traceback (most recent call last):
     ...
     zope.testbrowser.browser.LinkNotFoundError
diff --git a/lib/lp/translations/stories/standalone/xx-product-export.rst b/lib/lp/translations/stories/standalone/xx-product-export.rst
index 90c3d6d..73edc9c 100644
--- a/lib/lp/translations/stories/standalone/xx-product-export.rst
+++ b/lib/lp/translations/stories/standalone/xx-product-export.rst
@@ -2,10 +2,13 @@ Downloading Product Series Translations
 =======================================
 
 Products and product series that use Translations offer complete
-translation downloads.
+translation downloads to qualified users.  (See
+``../productseries/xx-productseries-translation-export.rst`` for more
+details on who counts as "qualified".)
 
-    >>> user_browser.open("http://translations.launchpad.test/evolution";)
-    >>> download = user_browser.getLink("download")
+    >>> browser = setupBrowser(auth="Basic mark@xxxxxxxxxxx:test")
+    >>> browser.open("http://translations.launchpad.test/evolution";)
+    >>> download = browser.getLink("download")
 
 For products, that option downloads translations for the series that is
 currently the preferred translation target.
@@ -17,21 +20,19 @@ currently the preferred translation target.
 Another way of getting that same export would be to browse to the series
 first and requesting a download there.
 
-    >>> user_browser.open(
-    ...     "http://translations.launchpad.test/evolution/trunk";
-    ... )
-    >>> user_browser.getLink("download").click()
-    >>> user_browser.url
+    >>> browser.open("http://translations.launchpad.test/evolution/trunk";)
+    >>> browser.getLink("download").click()
+    >>> browser.url
     'http://translations.launchpad.test/evolution/trunk/+export'
 
 The translations export is implemented by the same machinery that does
 it for source packages (tested and documented separately).
 
-    >>> print(user_browser.title)
+    >>> print(browser.title)
     Download : Series trunk : Translations...
 
-    >>> user_browser.getControl("Request Download").click()
-    >>> print_feedback_messages(user_browser.contents)
+    >>> browser.getControl("Request Download").click()
+    >>> print_feedback_messages(browser.contents)
     Your request has been received.  Expect to receive an email shortly.
 
 
@@ -49,8 +50,8 @@ Use the DB classes directly to avoid having to setup a zope interaction
     >>> product = Product.byName("evolution")
     >>> product.translations_usage = ServiceUsage.NOT_APPLICABLE
     >>> product.sync()
-    >>> user_browser.open("http://translations.launchpad.test/evolution";)
-    >>> user_browser.getLink("download")
+    >>> browser.open("http://translations.launchpad.test/evolution";)
+    >>> browser.getLink("download")
     Traceback (most recent call last):
     ...
     zope.testbrowser.browser.LinkNotFoundError
@@ -59,8 +60,8 @@ Restore previous state for subsequent tests, and verify.
 
     >>> product.translations_usage = ServiceUsage.LAUNCHPAD
     >>> product.sync()
-    >>> user_browser.open("http://translations.launchpad.test/evolution";)
-    >>> user_browser.getLink("download") is not None
+    >>> browser.open("http://translations.launchpad.test/evolution";)
+    >>> browser.getLink("download") is not None
     True
 
 
@@ -75,6 +76,16 @@ Only logged-in users get the option to request downloads.
     ...
     zope.testbrowser.browser.LinkNotFoundError
 
+Unqualified users (see
+``../productseries/xx-productseries-translation-export.rst``) do not get the
+option to request downloads.
+
+    >>> user_browser.open("http://translations.launchpad.test/evolution";)
+    >>> user_browser.getLink("download")
+    Traceback (most recent call last):
+    ...
+    zope.testbrowser.browser.LinkNotFoundError
+
 We can't see its placeholder in non-development mode:
 
     >>> from lp.services.config import config
@@ -96,3 +107,8 @@ Even "hacking the URL" to the download option will fail.
     Traceback (most recent call last):
     ...
     zope.security.interfaces.Unauthorized: ...
+
+    >>> user_browser.open(download_url)
+    Traceback (most recent call last):
+    ...
+    zope.security.interfaces.Unauthorized: ...
diff --git a/lib/lp/translations/templates/product-translations.pt b/lib/lp/translations/templates/product-translations.pt
index 61aa335..ad79952 100644
--- a/lib/lp/translations/templates/product-translations.pt
+++ b/lib/lp/translations/templates/product-translations.pt
@@ -59,10 +59,14 @@
                               >trunk</tal:target>.
                   <span tal:condition="context/required:launchpad.AnyPerson">
                     You can also
-                    <a tal:attributes="href target/fmt:url:translations/+export"
-                       >download</a>
-                    <tal:owner-or-admin condition="admin_user">
+                    <tal:expensive condition="target/required:launchpad.ExpensiveRequest">
+                      <a tal:attributes="href target/fmt:url:translations/+export"
+                         >download</a>
+                      <tal:owner-or-admin condition="admin_user">
                       or
+                      </tal:owner-or-admin>
+                    </tal:expensive>
+                    <tal:owner-or-admin condition="admin_user">
                       <a tal:attributes="
                           href target/fmt:url:translations/+translations-upload"
                        >upload</a>
diff --git a/lib/lp/translations/templates/productseries-translations.pt b/lib/lp/translations/templates/productseries-translations.pt
index 056d016..18c5535 100644
--- a/lib/lp/translations/templates/productseries-translations.pt
+++ b/lib/lp/translations/templates/productseries-translations.pt
@@ -186,10 +186,12 @@
                 <a tal:attributes="href context/menu:navigation/translationupload/url"
                    tal:condition="context/required:launchpad.Edit"
                    class="add sprite">upload</a>
-                or
-                <a tal:attributes="href context/menu:navigation/translationdownload/url"
-                   tal:condition="context/required:launchpad.AnyPerson"
-                   class="download sprite">download</a>
+                <tal:expensive condition="context/required:launchpad.ExpensiveRequest">
+                  or
+                  <a tal:attributes="href context/menu:navigation/translationdownload/url"
+                     tal:condition="context/required:launchpad.AnyPerson"
+                     class="download sprite">download</a>
+                </tal:expensive>
                 translation tarballs.
               </p>
             </div>