Merge pull request #7585 from ckan/serve-i18n-files-from-storage-folder

Store JS translation files in the CKAN storage folder

changes/7585.misc

@@ -0,0 +1 @@
+Store JS translation files in the storage folder rather than the source, to avoid permission problems

ckan/lib/i18n.py

@@ -54,6 +54,7 @@
 from ckan.plugins.interfaces import ITranslation
 from ckan.types import Request
 from ckan.common import config
+from ckan.lib.io import get_ckan_temp_directory
 
 log = logging.getLogger(__name__)
 
@@ -64,9 +65,6 @@
 # CKAN root directory
 _CKAN_DIR = os.path.abspath(os.path.join(os.path.dirname(__file__), u'..'))
 
-# Output directory for generated JavaScript translations
-_JS_TRANSLATIONS_DIR = os.path.join(_CKAN_DIR, u'public', u'base', u'i18n')
-
 
 def get_ckan_i18n_dir() -> str:
     path = config.get(u'ckan.i18n_directory') or os.path.join(
@@ -77,6 +75,14 @@ def get_ckan_i18n_dir() -> str:
     return path
 
 
+def get_js_translations_dir() -> str:
+    storage_path = config["ckan.storage_path"] or get_ckan_temp_directory()
+
+    js_translations_path = os.path.join(storage_path, "i18n", "js")
+
+    return js_translations_path
+
+
 def get_locales_from_config() -> set[str]:
     ''' despite the name of this function it gets the locales defined by
     the config AND also the locals available subject to the config. '''
@@ -308,6 +314,12 @@ def _build_js_translation(
         f.write(s)
 
 
+def _check_js_translations_dest_dir() -> None:
+    dest_dir = get_js_translations_dir()
+    if not os.path.isdir(dest_dir):
+        os.makedirs(dest_dir)
+
+
 def build_js_translations() -> None:
     '''
     Build JavaScript translation files.
@@ -319,6 +331,9 @@ def build_js_translations() -> None:
     '''
     log.debug(u'Generating JavaScript translations')
     ckan_i18n_dir = get_ckan_i18n_dir()
+    dest_dir = get_js_translations_dir()
+    _check_js_translations_dest_dir()
+
     # Collect all language codes (an extension might add support for a
     # language that isn't supported by CKAN core, yet).
     langs = set()
@@ -357,7 +372,7 @@ def build_js_translations() -> None:
             continue
 
         latest = max(os.path.getmtime(fn) for fn in po_files)
-        dest_file = os.path.join(_JS_TRANSLATIONS_DIR, lang + u'.js')
+        dest_file = os.path.join(dest_dir, lang + u'.js')
 
         if (not os.path.isfile(dest_file) or
                 os.path.getmtime(dest_file) < latest):

ckan/lib/io.py

@@ -1,12 +1,18 @@
 # encoding: utf-8
 
-u'''
+"""
 Utility functions for I/O.
-'''
+"""
 
+import hashlib
+import os
 import sys
+import tempfile
 from typing import Any, Union
 
+from ckan.common import config
+from ckan.exceptions import CkanConfigurationException
+
 
 _FILESYSTEM_ENCODING = str(
     sys.getfilesystemencoding() or sys.getdefaultencoding()
@@ -53,3 +59,29 @@ def decode_path(p: Union[bytes, Any]) -> str:
     if not isinstance(p, bytes):
         raise TypeError(u'Can only decode bytes, not {}'.format(type(p)))
     return p.decode(_FILESYSTEM_ENCODING)
+
+
+def get_ckan_temp_directory() -> str:
+    """
+    Returns the path to a securely created temporary directory that can be used
+    to store internal generated files like webassets, i18n files, etc
+
+    It is used as fallback when `ckan.storage_path` is not defined.
+    """
+
+    if not config.get("SECRET_KEY"):
+        # This function can be called before the configuration is validated,
+        # so we need to do this check here
+        raise CkanConfigurationException(
+            "Invalid configuration values provided:\n"
+            "SECRET_KEY: Missing value")
+    unique_suffix = hashlib.sha256(
+        config["SECRET_KEY"].encode()).hexdigest()[:10]
+    directory_name = f"ckan_{unique_suffix}"
+
+    path = os.path.join(tempfile.gettempdir(), directory_name)
+
+    if not os.path.exists(path):
+        os.mkdir(path, mode=0o700)
+
+    return path

ckan/lib/webassets_tools.py

@@ -2,7 +2,6 @@
 
 import logging
 import os
-import tempfile
 from typing import Any
 from typing_extensions import Literal, TypedDict, assert_never
 
@@ -11,6 +10,7 @@
 from webassets.loaders import YAMLLoader
 
 from ckan.common import config, g
+from ckan.lib.io import get_ckan_temp_directory
 
 
 log = logging.getLogger(__name__)
@@ -207,7 +207,9 @@ def get_webassets_path() -> str:
     webassets_path = config["ckan.webassets.path"]
 
     if not webassets_path:
-        storage_path = config["ckan.storage_path"] or tempfile.gettempdir()
+        storage_path = config.get(
+            "ckan.storage_path"
+        ) or get_ckan_temp_directory()
 
         if storage_path:
             webassets_path = os.path.join(storage_path, "webassets")

ckan/tests/cli/test_asset.py

@@ -12,9 +12,7 @@ def test_build_and_clean(cli, ckan_config, tmpdir, monkeypatch):
     """
     monkeypatch.setitem(ckan_config, u'ckan.storage_path', str(tmpdir))
     cli.invoke(ckan, [u'asset', u'build'])
-    assert len(tmpdir.listdir()) == 1
-    webassets_folder = tmpdir.listdir()[0]
-    assert webassets_folder.basename == u'webassets'
+    webassets_folder = [d for d in tmpdir.listdir() if d.basename == "webassets"][0]
     for folder in webassets_folder.listdir():
         if not folder.isdir():
             continue

ckan/tests/lib/test_i18n.py

@@ -1,6 +1,6 @@
 # encoding: utf-8
 
-u"""
+"""
 Tests for ``ckan.lib.i18n``.
 """
 
@@ -9,6 +9,7 @@
 import os.path
 import shutil
 import tempfile
+from unittest import mock
 
 import pytest
 from ckan.lib import i18n
@@ -17,12 +18,13 @@
 
 
 HERE = os.path.abspath(os.path.dirname(__file__))
-I18N_DIR = os.path.join(HERE, u"_i18n_build_js_translations")
-I18N_DUMMY_DIR = os.path.join(HERE, u"_i18n_dummy_es")
+I18N_DIR = os.path.join(HERE, "_i18n_build_js_translations")
+I18N_DUMMY_DIR = os.path.join(HERE, "_i18n_dummy_es")
+I18N_TEMP_DIR = tempfile.mkdtemp()
 
 
 class JSTranslationsTestPlugin(plugins.SingletonPlugin, DefaultTranslation):
-    u"""
+    """
     CKAN plugin for testing JavaScript translations from extensions.
 
     Registered in ``setup.py`` as ``test_js_translations_plugin``.
@@ -33,48 +35,47 @@ def i18n_directory(self):
         return I18N_DIR
 
     def i18n_domain(self):
-        return u"ckanext-test_js_translations"
+        return "ckanext-test_js_translations"
+
 
+@pytest.fixture
+def temp_i18n_dir():
+    yield
+    shutil.rmtree(I18N_TEMP_DIR, ignore_errors=True)
 
-@pytest.mark.ckan_config(u"ckan.plugins", u"test_js_translations_plugin")
-@pytest.mark.usefixtures(u"with_plugins")
+
+@pytest.mark.ckan_config("ckan.plugins", "test_js_translations_plugin")
+@pytest.mark.usefixtures("with_plugins", "temp_i18n_dir")
 class TestBuildJSTranslations(object):
-    u"""
+    """
     Tests for ``ckan.lib.i18n.build_js_translations``.
     """
 
-    def setup(self):
-        self.temp_dir = tempfile.mkdtemp()
-
-    def teardown(self):
-        shutil.rmtree(self.temp_dir, ignore_errors=True)
+    temp_dir = I18N_TEMP_DIR
 
     def build_js_translations(self):
-        u"""
+        """
         Build JS translations in temporary directory.
         """
-        old_translations_dir = i18n._JS_TRANSLATIONS_DIR
-        i18n._JS_TRANSLATIONS_DIR = self.temp_dir
-        try:
+        with mock.patch("ckan.lib.i18n.get_js_translations_dir", return_value=self.temp_dir):
+
             return i18n.build_js_translations()
-        finally:
-            i18n._JS_TRANSLATIONS_DIR = old_translations_dir
 
     def test_output_is_valid(self):
-        u"""
+        """
         Test that the generated JS files are valid.
         """
 
         def check_file(path):
-            with codecs.open(path, u"r", encoding=u"utf-8") as f:
+            with codecs.open(path, "r", encoding="utf-8") as f:
                 data = json.load(f)
-            assert data[u""].get(u"domain", None) == u"ckan"
+            assert data[""].get("domain", None) == "ckan"
 
         self.build_js_translations()
         files = os.listdir(self.temp_dir)
 
         # Check that all locales have been generated
-        assert set(i18n.get_locales()).difference([u"en"]) == set(
+        assert set(i18n.get_locales()).difference(["en"]) == set(
             os.path.splitext(fn)[0] for fn in files
         )
 
@@ -83,7 +84,7 @@ def check_file(path):
             check_file(os.path.join(self.temp_dir, filename))
 
     def test_regenerate_only_if_necessary(self):
-        u"""
+        """
         Test that translation files are only generated when necessary.
         """
         self.build_js_translations()
@@ -117,24 +118,24 @@ def test_regenerate_only_if_necessary(self):
             assert new_mtime == mtimes[filename]
 
     def test_translations_from_extensions(self):
-        u"""
+        """
         Test that translations from extensions are taken into account.
         """
         self.build_js_translations()
-        filename = os.path.join(self.temp_dir, u"de.js")
-        with codecs.open(filename, u"r", encoding=u"utf-8") as f:
+        filename = os.path.join(self.temp_dir, "de.js")
+        with codecs.open(filename, "r", encoding="utf-8") as f:
             de = json.load(f)
 
         # Check overriding a JS translation from CKAN core
-        assert u"Loading..." in de
-        assert de[u"Loading..."] == [None, u"foo"]
+        assert "Loading..." in de
+        assert de["Loading..."] == [None, "foo"]
 
         # Check introducing a new JS translation
-        assert u"Test JS Translations 1" in de
-        assert de[u"Test JS Translations 1"] == [None, u"bar"]
+        assert "Test JS Translations 1" in de
+        assert de["Test JS Translations 1"] == [None, "bar"]
 
         # Check that non-JS strings are not exported
-        assert u"Test JS Translations 2" not in de
+        assert "Test JS Translations 2" not in de
 
 
 @pytest.mark.ckan_config("ckan.plugins", "test_blueprint_plugin")

ckan/tests/lib/test_io.py

@@ -1,12 +1,11 @@
-# encoding: utf-8
-
+import hashlib
 import io
 import os.path
 import shutil
 import tempfile
 import pytest
 
-
+from ckan.common import config
 import ckan.lib.io as ckan_io
 
 
@@ -40,3 +39,10 @@ def test_decode_encode_path():
         assert ckan_io.decode_path(filenames[0]) == filename
     finally:
         shutil.rmtree(temp_dir)
+
+
+def test_get_ckan_temp_directory():
+    suffix = hashlib.sha256(config["SECRET_KEY"].encode()).hexdigest()[:10]
+    expected_folder = f"ckan_{suffix}"
+
+    assert ckan_io.get_ckan_temp_directory() == os.path.join(tempfile.gettempdir(), expected_folder)

ckan/views/api.py

@@ -18,7 +18,7 @@
 from ckan.common import json, _, g, request, current_user
 from ckan.lib.helpers import url_for
 from ckan.lib.base import render
-from ckan.lib.i18n import get_locales_from_config
+from ckan.lib.i18n import get_locales_from_config, get_js_translations_dir
 
 from ckan.lib.navl.dictization_functions import DataError
 from ckan.logic import get_action, ValidationError, NotFound, NotAuthorized
@@ -473,12 +473,12 @@ def i18n_js_translations(
     if lang not in get_locales_from_config():
         return _finish_bad_request('Unknown locale: {}'.format(lang))
 
-    ckan_path = os.path.join(os.path.dirname(__file__), u'..')
-    source = os.path.abspath(os.path.join(ckan_path, u'public',
-                             u'base', u'i18n', u'{0}.js'.format(lang)))
+    js_translations_folder = get_js_translations_dir()
+
+    source = os.path.join(js_translations_folder, f"{lang}.js")
     if not os.path.exists(source):
-        return u'{}'
-    translations = json.load(io.open(source, u'r', encoding='utf-8'))
+        return "{}"
+    translations = json.load(io.open(source, "r", encoding="utf-8"))
     return _finish_ok(translations)