Merge pull request #5881 from freedomofpress/fix-5755

Correct untranslated messages

securedrop/bin/dev-shell

@@ -79,7 +79,7 @@ function docker_run() {
            -e LOADDATA_ARGS \
            -e LC_ALL=C.UTF-8 \
            -e LANG=C.UTF-8 \
-           -e PAGE_LAYOUT_LOCALES \
+           -e TEST_LOCALES \
            -e PATH \
            -e BASE_OS=$BASE_OS \
            --user "${USER:-root}" \

securedrop/bin/run-test

@@ -33,13 +33,15 @@ fi
 
 mkdir -p "../test-results"
 
-: "${PAGE_LAYOUT_LOCALES:=en_US,ar}"
-export PAGE_LAYOUT_LOCALES
+: "${TEST_LOCALES:="ar en_US"}"
+export TEST_LOCALES
 export TOR_FORCE_NET_CONFIG=0
 
 
 pytest \
     --force-flaky --max-runs=3 \
+    -rx \
+    --no-success-flaky-report \
     --page-layout \
     --durations 10 \
     --junitxml=../test-results/junit.xml \

securedrop/bin/translation-test

@@ -36,7 +36,7 @@ printf "Running tests in these locales: %s\n" "$LOCALES"
 # separately avoids this.
 for locale in $LOCALES
 do
-  PAGE_LAYOUT_LOCALES=$locale pytest \
+  TEST_LOCALES=$locale pytest \
     -v \
     --force-flaky --max-runs=3 \
     --page-layout \

securedrop/journalist_app/__init__.py

@@ -94,9 +94,8 @@ def create_app(config: 'SDConfig') -> Flask:
 
     @app.errorhandler(CSRFError)
     def handle_csrf_error(e: CSRFError) -> 'Response':
-        # render the message first to ensure it's localized.
-        msg = gettext('You have been logged out due to inactivity.')
         session.clear()
+        msg = gettext('You have been logged out due to inactivity.')
         flash(msg, 'error')
         return redirect(url_for('main.login'))
 

securedrop/journalist_app/admin.py

@@ -52,8 +52,8 @@ def manage_config() -> Union[str, werkzeug.Response]:
                 f.save(custom_logo_filepath)
                 flash(gettext("Image updated."), "logo-success")
             except Exception:
-                flash("Unable to process the image file."
-                      " Try another one.", "logo-error")
+                # Translators: This error is shown when an uploaded image cannot be used.
+                flash(gettext("Unable to process the image file. Try another one."), "logo-error")
             finally:
                 return redirect(url_for("admin.manage_config") + "#config-logoimage")
         else:
@@ -131,13 +131,16 @@ def add_user() -> Union[str, werkzeug.Response]:
                 form_valid = False
             except InvalidUsernameException as e:
                 form_valid = False
-                flash('Invalid username: ' + str(e), "error")
+                # Translators: Here, "{message}" explains the problem with the username.
+                flash(gettext('Invalid username: {message}').format(message=e), "error")
             except IntegrityError as e:
                 db.session.rollback()
                 form_valid = False
                 if "UNIQUE constraint failed: journalists.username" in str(e):
-                    flash(gettext('Username "{user}" already taken.'.format(
-                        user=username)), "error")
+                    flash(
+                        gettext('Username "{username}" already taken.').format(username=username),
+                        "error"
+                    )
                 else:
                     flash(gettext("An error occurred saving this user"
                                   " to the database."
@@ -214,18 +217,20 @@ def edit_user(user_id: int) -> Union[str, werkzeug.Response]:
                 try:
                     Journalist.check_username_acceptable(new_username)
                 except InvalidUsernameException as e:
-                    flash('Invalid username: ' + str(e), 'error')
+                    flash(gettext('Invalid username: {message}').format(message=e), "error")
                     return redirect(url_for("admin.edit_user",
                                             user_id=user_id))
 
                 if new_username == user.username:
                     pass
                 elif Journalist.query.filter_by(
                         username=new_username).one_or_none():
-                    flash(gettext(
-                        'Username "{user}" already taken.').format(
-                            user=new_username),
-                        "error")
+                    flash(
+                        gettext('Username "{username}" already taken.').format(
+                            username=new_username
+                        ),
+                        "error"
+                    )
                     return redirect(url_for("admin.edit_user",
                                             user_id=user_id))
                 else:
@@ -236,15 +241,16 @@ def edit_user(user_id: int) -> Union[str, werkzeug.Response]:
                 Journalist.check_name_acceptable(first_name)
                 user.first_name = first_name
             except FirstOrLastNameError as e:
-                flash(gettext('Name not updated: {}'.format(e)), "error")
+                # Translators: Here, "{message}" explains the problem with the name.
+                flash(gettext('Name not updated: {message}').format(message=e), "error")
                 return redirect(url_for("admin.edit_user", user_id=user_id))
 
             try:
                 last_name = request.form['last_name']
                 Journalist.check_name_acceptable(last_name)
                 user.last_name = last_name
             except FirstOrLastNameError as e:
-                flash(gettext('Name not updated: {}'.format(e)), "error")
+                flash(gettext('Name not updated: {message}').format(message=e), "error")
                 return redirect(url_for("admin.edit_user", user_id=user_id))
 
             user.is_admin = bool(request.form.get('is_admin'))

securedrop/journalist_app/utils.py

@@ -99,7 +99,7 @@ def validate_user(
             InvalidPasswordLength) as e:
         current_app.logger.error("Login for '{}' failed: {}".format(
             username, e))
-        login_flashed_msg = error_message if error_message else gettext('<b>Login failed.</b>')
+        login_flashed_msg = error_message if error_message else gettext('Login failed.')
 
         if isinstance(e, LoginThrottledException):
             login_flashed_msg += " "
@@ -123,7 +123,7 @@ def validate_user(
             except Exception:
                 pass
 
-        flash(Markup(login_flashed_msg), "error")
+        flash(login_flashed_msg, "error")
         return None
 
 
@@ -414,7 +414,7 @@ def set_name(user: Journalist, first_name: Optional[str], last_name: Optional[st
         db.session.commit()
         flash(gettext('Name updated.'), "success")
     except FirstOrLastNameError as e:
-        flash(gettext('Name not updated: {}'.format(e)), "error")
+        flash(gettext('Name not updated: {message}').format(message=e), "error")
 
 
 def set_diceware_password(user: Journalist, password: Optional[str]) -> bool:
@@ -437,11 +437,20 @@ def set_diceware_password(user: Journalist, password: Optional[str]) -> bool:
         return False
 
     # using Markup so the HTML isn't escaped
-    flash(Markup("<p>" + gettext(
-        "Password updated. Don't forget to "
-        "save it in your KeePassX database. New password:") +
-        ' <span><code>{}</code></span></p>'.format(password)),
-        'success')
+    flash(
+        Markup(
+            "<p>{message} <span><code>{password}</code></span></p>".format(
+                message=Markup.escape(
+                    gettext(
+                        "Password updated. Don't forget to save it in your KeePassX database. "
+                        "New password:"
+                    )
+                ),
+                password=Markup.escape("" if password is None else password)
+            )
+        ),
+        'success'
+    )
     return True
 
 

securedrop/models.py

@@ -12,6 +12,7 @@
 from io import BytesIO
 
 from flask import current_app, url_for
+from flask_babel import gettext, ngettext
 from itsdangerous import TimedJSONWebSignatureSerializer, BadData
 from jinja2 import Markup
 from passlib.hash import argon2
@@ -342,10 +343,8 @@ def __init__(self, msg: str) -> None:
 class InvalidNameLength(FirstOrLastNameError):
     """Raised when attempting to create a Journalist with an invalid name length."""
 
-    def __init__(self, name: str) -> None:
-        name_len = len(name)
-        msg = "Name too long (len={})".format(name_len)
-        super(InvalidNameLength, self).__init__(msg)
+    def __init__(self) -> None:
+        super(InvalidNameLength, self).__init__(gettext("Name too long"))
 
 
 class LoginThrottledException(Exception):
@@ -380,11 +379,9 @@ def __init__(self, passphrase: str) -> None:
 
     def __str__(self) -> str:
         if self.passphrase_len > Journalist.MAX_PASSWORD_LEN:
-            return "Password too long (len={})".format(self.passphrase_len)
+            return "Password is too long."
         if self.passphrase_len < Journalist.MIN_PASSWORD_LEN:
-            return "Password needs to be at least {} characters".format(
-                Journalist.MIN_PASSWORD_LEN
-            )
+            return "Password is too short."
         return ""   # return empty string that can be appended harmlessly
 
 
@@ -496,18 +493,25 @@ def set_name(self, first_name: Optional[str], last_name: Optional[str]) -> None:
     def check_username_acceptable(cls, username: str) -> None:
         if len(username) < cls.MIN_USERNAME_LEN:
             raise InvalidUsernameException(
-                        'Username "{}" must be at least {} characters long.'
-                        .format(username, cls.MIN_USERNAME_LEN))
+                ngettext(
+                    'Must be at least {num} character long.',
+                    'Must be at least {num} characters long.',
+                    cls.MIN_USERNAME_LEN
+                ).format(num=cls.MIN_USERNAME_LEN)
+            )
         if username in cls.INVALID_USERNAMES:
             raise InvalidUsernameException(
+                gettext(
                     "This username is invalid because it is reserved "
-                    "for internal use by the software.")
+                    "for internal use by the software."
+                )
+            )
 
     @classmethod
     def check_name_acceptable(cls, name: str) -> None:
         # Enforce a reasonable maximum length for names
         if len(name) > cls.MAX_NAME_LEN:
-            raise InvalidNameLength(name)
+            raise InvalidNameLength()
 
     @classmethod
     def check_password_acceptable(cls, password: str) -> None:
@@ -675,13 +679,11 @@ def login(cls,
         try:
             user = Journalist.query.filter_by(username=username).one()
         except NoResultFound:
-            raise InvalidUsernameException(
-                "invalid username '{}'".format(username))
+            raise InvalidUsernameException(gettext("Invalid username"))
 
         if user.username in Journalist.INVALID_USERNAMES and \
                 user.uuid in Journalist.INVALID_USERNAMES:
-            raise InvalidUsernameException(
-                "Invalid username")
+            raise InvalidUsernameException(gettext("Invalid username"))
 
         if LOGIN_HARDENING:
             cls.throttle_login(user)
@@ -876,9 +878,9 @@ def get_current(cls) -> "InstanceConfig":
     def check_name_acceptable(cls, name: str) -> None:
         # Enforce a reasonable maximum length for names
         if name is None or len(name) == 0:
-            raise InvalidNameLength(name)
+            raise InvalidNameLength()
         if len(name) > cls.MAX_ORG_NAME_LEN:
-            raise InvalidNameLength(name)
+            raise InvalidNameLength()
 
     @classmethod
     def set_organization_name(cls, name: str) -> None:

securedrop/source_app/__init__.py

@@ -3,7 +3,7 @@
 from typing import Optional
 
 import werkzeug
-from flask import (Flask, render_template, flash, Markup, request, g, session,
+from flask import (Flask, render_template, escape, flash, Markup, request, g, session,
                    url_for, redirect)
 from flask_babel import gettext
 from flask_assets import Environment
@@ -127,14 +127,21 @@ def check_tor2web() -> None:
         # ignore_static here so we only flash a single message warning
         # about Tor2Web, corresponding to the initial page load.
         if 'X-tor2web' in request.headers:
-            flash(Markup(gettext(
-                '<strong>WARNING:&nbsp;</strong> '
-                'You appear to be using Tor2Web. '
-                'This <strong>&nbsp;does not&nbsp;</strong> '
-                'provide anonymity. '
-                '<a href="{url}">Why is this dangerous?</a>')
-                .format(url=url_for('info.tor2web_warning'))),
-                "banner-warning")
+            flash(
+                Markup(
+                    '<strong>{}</strong>&nbsp;{}&nbsp;<a href="{}">{}</a>'.format(
+                        escape(gettext("WARNING:")),
+                        escape(
+                            gettext(
+                                'You appear to be using Tor2Web, which does not provide anonymity.'
+                            )
+                        ),
+                        url_for('info.tor2web_warning'),
+                        escape(gettext('Why is this dangerous?')),
+                    )
+                ),
+                "banner-warning"
+            )
 
     @app.before_request
     @ignore_static

securedrop/tests/conftest.py

@@ -32,6 +32,7 @@
 import models
 from source_app import create_app as create_source_app
 from . import utils
+from .utils import i18n
 
 # The PID file for the redis worker is hard-coded below.
 # Ideally this constant would be provided by a test harness.
@@ -130,6 +131,8 @@ def config(gpg_key_dir: Path) -> Generator[SDConfig, None, None]:
         config.DATABASE_FILE = str(sqlite_db_path)
         subprocess.check_call(["sqlite3", config.DATABASE_FILE, ".databases"])
 
+        config.SUPPORTED_LOCALES = i18n.get_test_locales()
+
         yield config
 
 

securedrop/tests/pageslayout/functional_test.py

@@ -30,8 +30,8 @@
 
 
 def list_locales():
-    if "PAGE_LAYOUT_LOCALES" in os.environ:
-        locales = os.environ["PAGE_LAYOUT_LOCALES"].split(",")
+    if "TEST_LOCALES" in os.environ:
+        locales = os.environ["TEST_LOCALES"].split()
     else:
         locales = ["en_US"]
     return locales