From f99b21165d244a43da226b4fa9141968194ea553 Mon Sep 17 00:00:00 2001 From: Kushal Das Date: Wed, 6 Mar 2019 18:56:16 +0530 Subject: [PATCH 01/47] Updates Dockerfile for Python3 environment --- securedrop/dockerfiles/xenial/Dockerfile | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/securedrop/dockerfiles/xenial/Dockerfile b/securedrop/dockerfiles/xenial/Dockerfile index 3402b026f5..6ccbc54e77 100644 --- a/securedrop/dockerfiles/xenial/Dockerfile +++ b/securedrop/dockerfiles/xenial/Dockerfile @@ -6,13 +6,12 @@ ARG USER_ID ENV USER_ID ${USER_ID:-0} # If running grsecurity kernel on the host, Memprotect must be disabled on mono-sgen in the container -RUN apt-get update && \ - apt-get install -y paxctl && \ +RUN apt-get update && apt-get install -y paxctl && \ { apt-get install -y libgtk2.0 || echo 'libgtk2.0 was not installed'; } && \ paxctl -cm /usr/bin/mono-sgen && dpkg-reconfigure mono-runtime-sgen -RUN apt-get install -y devscripts vim \ - python-pip libpython2.7-dev libssl-dev secure-delete \ +RUN apt-get install -y devscripts \ + python3-pip libpython3.5-dev libssl-dev secure-delete \ gnupg2 ruby redis-server firefox git xvfb haveged curl \ gettext paxctl x11vnc enchant libffi-dev sqlite3 gettext sudo \ libgtk2.0 @@ -28,11 +27,11 @@ RUN gem install sass -v 3.4.23 COPY requirements requirements -RUN pip install -r requirements/securedrop-app-code-requirements.txt && \ - pip install -r requirements/test-requirements.txt +RUN pip3 install -r requirements/securedrop-app-code-requirements.txt && \ + pip3 install -r requirements/test-requirements.txt # Fixes #4036 pybabel requires latest version of setuptools -RUN pip install --upgrade setuptools +RUN pip3 install --upgrade setuptools RUN if test $USER_NAME != root ; then useradd --no-create-home --home-dir /tmp --uid $USER_ID $USER_NAME && echo "$USER_NAME ALL=(ALL) NOPASSWD:ALL" >> /etc/sudoers ; fi From 5b7d08d9245388da712fdeaf02901cdf17943375 Mon Sep 17 00:00:00 2001 From: Kushal Das Date: Wed, 6 Mar 2019 18:58:02 +0530 Subject: [PATCH 02/47] Uses py.test executable for tests --- securedrop/bin/run-test | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/securedrop/bin/run-test b/securedrop/bin/run-test index 04cba44fa4..d06b146fbb 100755 --- a/securedrop/bin/run-test +++ b/securedrop/bin/run-test @@ -27,7 +27,7 @@ mkdir -p "/tmp/test-results/logs" : "${PAGE_LAYOUT_LOCALES:=en_US,ar,fr_FR}" export PAGE_LAYOUT_LOCALES -pytest \ +py.test \ --page-layout \ --durations 10 \ --junitxml=/tmp/test-results/junit.xml \ From 5432a48573cee6ed0ccd644381b4776d2cac7046 Mon Sep 17 00:00:00 2001 From: Kushal Das Date: Wed, 6 Mar 2019 18:58:22 +0530 Subject: [PATCH 03/47] Uses Python3 to create dev config --- securedrop/Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/securedrop/Makefile b/securedrop/Makefile index 2bd973ffcb..27c8cada92 100644 --- a/securedrop/Makefile +++ b/securedrop/Makefile @@ -40,10 +40,10 @@ test-config: ## Generate the test config journalist_secret_key=$(shell head -c 32 /dev/urandom | base64) \ scrypt_id_pepper=$(shell head -c 32 /dev/urandom | base64) \ scrypt_gpg_pepper=$(shell head -c 32 /dev/urandom | base64) \ - python -c 'import os; from jinja2 import Environment, FileSystemLoader; \ + python3 -c 'import os; from jinja2 import Environment, FileSystemLoader; \ env = Environment(loader=FileSystemLoader(".")); \ ctx = {"securedrop_app_gpg_fingerprint": "65A1B5FF195B56353CC63DFFCC40EF1228271441"}; \ - ctx.update(dict((k, {"stdout":v}) for k,v in os.environ.iteritems())); \ + ctx.update(dict((k, {"stdout":v}) for k,v in os.environ.items())); \ ctx = open("config.py", "w").write(env.get_template("config.py.example").render(ctx))' @echo >> config.py @echo "SUPPORTED_LOCALES = ['ar', 'de_DE', 'es_ES', 'en_US', 'el', 'fr_FR', 'it_IT', 'nb_NO', 'nl', 'pt_BR', 'tr', 'zh_Hant']" >> config.py From cbe3f251c86b3a45a2c9b1b874af763ea2d9734c Mon Sep 17 00:00:00 2001 From: Kushal Das Date: Wed, 6 Mar 2019 19:00:10 +0530 Subject: [PATCH 04/47] Uses BytesIO for svg In Python3 we need to use BytesIO and then later convert bytes to string for viewing svg QR code on the browser. --- securedrop/models.py | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/securedrop/models.py b/securedrop/models.py index 74420e9046..99896c0228 100644 --- a/securedrop/models.py +++ b/securedrop/models.py @@ -9,12 +9,7 @@ # Using svg because it doesn't require additional dependencies import qrcode.image.svg import uuid - -# Find the best implementation available on this platform -try: - from cStringIO import StringIO -except ImportError: - from StringIO import StringIO # type: ignore +from io import BytesIO from flask import current_app, url_for from itsdangerous import TimedJSONWebSignatureSerializer, BadData @@ -470,9 +465,9 @@ def shared_secret_qrcode(self): qr.add_data(uri) img = qr.make_image() - svg_out = StringIO() + svg_out = BytesIO() img.save(svg_out) - return Markup(svg_out.getvalue()) + return Markup(svg_out.getvalue().decode('utf-8')) @property def formatted_otp_secret(self): From 2490ba766aed58040723bd3c789f8a34157220f2 Mon Sep 17 00:00:00 2001 From: Kushal Das Date: Wed, 6 Mar 2019 19:14:00 +0530 Subject: [PATCH 05/47] Uses Python3 interpreter --- securedrop/create-dev-data.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/securedrop/create-dev-data.py b/securedrop/create-dev-data.py index ab4e886710..15e5da936a 100755 --- a/securedrop/create-dev-data.py +++ b/securedrop/create-dev-data.py @@ -1,4 +1,4 @@ -#!/usr/bin/env python +#!/usr/bin/env python3 # -*- coding: utf-8 -*- import datetime From 88f30498ba426cb15514dd24ef1d080142f177e8 Mon Sep 17 00:00:00 2001 From: Kushal Das Date: Wed, 6 Mar 2019 19:17:10 +0530 Subject: [PATCH 06/47] Returns str instead of bytes in Python3 --- securedrop/crypto_util.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/securedrop/crypto_util.py b/securedrop/crypto_util.py index 61f1e93f45..a284151aa7 100644 --- a/securedrop/crypto_util.py +++ b/securedrop/crypto_util.py @@ -162,7 +162,7 @@ def hash_codename(self, codename, salt=None): salt = self.scrypt_id_pepper return b32encode(scrypt.hash(clean(codename), salt, - **self.scrypt_params)) + **self.scrypt_params)).decode('utf-8') def genkeypair(self, name, secret): """Generate a GPG key through batch file key generation. A source's From c00fdc797f74c00770aabac7ca37a3bac64d7fdd Mon Sep 17 00:00:00 2001 From: Kushal Das Date: Wed, 6 Mar 2019 19:18:20 +0530 Subject: [PATCH 07/47] Mass update from 2to3 tool of the source --- securedrop/i18n.py | 2 +- securedrop/i18n_tool.py | 25 +++++++++---------- securedrop/journalist_app/__init__.py | 2 +- securedrop/journalist_app/admin.py | 2 +- securedrop/journalist_app/forms.py | 2 +- securedrop/manage.py | 36 +++++++++++++-------------- securedrop/management/run.py | 11 ++++---- securedrop/secure_tempfile.py | 8 +++--- securedrop/source_app/info.py | 2 +- securedrop/template_filters.py | 2 +- 10 files changed, 45 insertions(+), 47 deletions(-) diff --git a/securedrop/i18n.py b/securedrop/i18n.py index d76cc05d01..9535f315b0 100644 --- a/securedrop/i18n.py +++ b/securedrop/i18n.py @@ -80,7 +80,7 @@ def get_locale(config): """ locale = None accept_languages = [] - for l in request.accept_languages.values(): + for l in list(request.accept_languages.values()): if '-' in l: sep = '-' else: diff --git a/securedrop/i18n_tool.py b/securedrop/i18n_tool.py index b0d7689e9b..3272b66beb 100755 --- a/securedrop/i18n_tool.py +++ b/securedrop/i18n_tool.py @@ -1,4 +1,4 @@ -#!/usr/bin/env python +#!/usr/bin/env python3 # -*- coding: utf-8 -*- import argparse @@ -124,9 +124,8 @@ def translate_desktop(self, args): log.warning("desktop translations are already up to date") if args.compile: - pos = filter(lambda f: f.endswith('.po'), - os.listdir(args.translations_dir)) - linguas = map(lambda l: l[:-3], pos) + pos = [f for f in os.listdir(args.translations_dir) if f.endswith('.po')] + linguas = [l[:-3] for l in pos] content = "\n".join(linguas) + "\n" open(join(args.translations_dir, 'LINGUAS'), 'w').write(content) @@ -201,7 +200,7 @@ def require_git_email_name(git_dir): 'git -C {d} config --get user.email > /dev/null'.format( d=git_dir)) if subprocess.call(cmd, shell=True): # nosec - if u'docker' in io.open('/proc/1/cgroup').read(): + if 'docker' in io.open('/proc/1/cgroup').read(): log.error("remember ~/.gitconfig does not exist " "in the dev-shell Docker container, " "only .git/config does") @@ -209,7 +208,7 @@ def require_git_email_name(git_dir): return True def update_docs(self, args): - l10n_content = u'.. GENERATED BY i18n_tool.py DO NOT EDIT:\n\n' + l10n_content = '.. GENERATED BY i18n_tool.py DO NOT EDIT:\n\n' for (code, info) in sorted(I18NTool.SUPPORTED_LANGUAGES.items()): l10n_content += '* ' + info['name'] + ' (``' + code + '``)\n' includes = join(args.documentation_dir, 'includes') @@ -240,7 +239,7 @@ def set_update_docs_parser(self, subps): def update_from_weblate(self, args): self.ensure_i18n_remote(args) - codes = I18NTool.SUPPORTED_LANGUAGES.keys() + codes = list(I18NTool.SUPPORTED_LANGUAGES.keys()) if args.supported_languages: codes = args.supported_languages.split(',') for code in codes: @@ -288,23 +287,23 @@ def upstream_commit(self, args, code): diffs = git('--no-pager', '-C', args.root, 'diff', '--name-only', '--cached').stdout for path in diffs.strip().split('\n'): - previous_message = unicode(git( + previous_message = str(git( '--no-pager', '-C', args.root, 'log', '-n', '1', path, _encoding='utf-8')) - m = re.search(u'copied from (\w+)', previous_message) + m = re.search('copied from (\w+)', previous_message) if m: origin = m.group(1) else: origin = '' - git_authors = unicode(git( + git_authors = str(git( '--no-pager', '-C', args.root, 'log', '--format=%aN', '{}..i18n/i18n'.format(origin), '--', path, _encoding='utf-8')) - git_authors = git_authors.strip().split(u'\n') + git_authors = git_authors.strip().split('\n') authors |= set(git_authors) current = git('-C', args.root, 'rev-parse', 'i18n/i18n').stdout info = I18NTool.SUPPORTED_LANGUAGES[code] - message = textwrap.dedent(u""" + message = textwrap.dedent(""" l10n: updated {code} {name} localizers: {authors} @@ -313,7 +312,7 @@ def upstream_commit(self, args, code): copied from {current} """.format(remote=args.url, name=info['name'], - authors=u", ".join(authors), + authors=", ".join(authors), code=code, current=current)) git('-C', args.root, 'commit', '-m', message) diff --git a/securedrop/journalist_app/__init__.py b/securedrop/journalist_app/__init__.py index b5b32267fa..fd4d2cd436 100644 --- a/securedrop/journalist_app/__init__.py +++ b/securedrop/journalist_app/__init__.py @@ -94,7 +94,7 @@ def handle_csrf_error(e): def _handle_http_exception(error): # Workaround for no blueprint-level 404/5 error handlers, see: # https://github.com/pallets/flask/issues/503#issuecomment-71383286 - handler = app.error_handler_spec['api'][error.code].values()[0] + handler = list(app.error_handler_spec['api'][error.code].values())[0] if request.path.startswith('/api/') and handler: return handler(error) diff --git a/securedrop/journalist_app/admin.py b/securedrop/journalist_app/admin.py index 1680e4e0c4..5d81ce6658 100644 --- a/securedrop/journalist_app/admin.py +++ b/securedrop/journalist_app/admin.py @@ -42,7 +42,7 @@ def manage_config(): finally: return redirect(url_for("admin.manage_config")) else: - for field, errors in form.errors.items(): + for field, errors in list(form.errors.items()): for error in errors: flash(error, "logo-error") return render_template("config.html", form=form) diff --git a/securedrop/journalist_app/forms.py b/securedrop/journalist_app/forms.py index d866264480..8bf79e94c8 100644 --- a/securedrop/journalist_app/forms.py +++ b/securedrop/journalist_app/forms.py @@ -46,7 +46,7 @@ class NewUserForm(FlaskForm): class ReplyForm(FlaskForm): message = TextAreaField( - u'Message', + 'Message', id="content-area", validators=[ InputRequired(message=gettext( diff --git a/securedrop/manage.py b/securedrop/manage.py index f23f24b112..448653772a 100755 --- a/securedrop/manage.py +++ b/securedrop/manage.py @@ -1,4 +1,4 @@ -#!/usr/bin/env python +#!/usr/bin/env python3 # -*- coding: utf-8 -*- import argparse @@ -110,11 +110,11 @@ def add_journalist(args): def _get_username(): while True: - username = raw_input('Username: ') + username = input('Username: ') try: Journalist.check_username_acceptable(username) except InvalidUsernameException as e: - print('Invalid username: ' + str(e)) + print(('Invalid username: ' + str(e))) else: return username @@ -122,7 +122,7 @@ def _get_username(): def _get_yubikey_usage(): '''Function used to allow for test suite mocking''' while True: - answer = raw_input('Will this user be using a YubiKey [HOTP]? ' + answer = input('Will this user be using a YubiKey [HOTP]? ' '(y/N): ').lower().strip() if answer in ('y', 'yes'): return True @@ -148,21 +148,21 @@ def _add_user(is_admin=False): print("Note: Passwords are now autogenerated.") password = _make_password() - print("This user's password is: {}".format(password)) + print(("This user's password is: {}".format(password))) is_hotp = _get_yubikey_usage() otp_secret = None if is_hotp: while True: - otp_secret = raw_input( + otp_secret = input( "Please configure this user's YubiKey and enter the " "secret: ") if otp_secret: tmp_str = otp_secret.replace(" ", "") if len(tmp_str) != 40: - print("The length of the secret is not correct. " + print(("The length of the secret is not correct. " "Expected 40 characters, but received {0}. " - "Try again.".format(len(tmp_str))) + "Try again.".format(len(tmp_str)))) continue if otp_secret: break @@ -180,11 +180,11 @@ def _add_user(is_admin=False): print('ERROR: That username is already taken!') else: exc_type, exc_value, exc_traceback = sys.exc_info() - print(repr(traceback.format_exception(exc_type, exc_value, - exc_traceback))) + print((repr(traceback.format_exception(exc_type, exc_value, + exc_traceback)))) return 1 else: - print('User "{}" successfully added'.format(username)) + print(('User "{}" successfully added'.format(username))) if not otp_secret: # Print the QR code for FreeOTP print('\nScan the QR code below with FreeOTP:\n') @@ -194,26 +194,26 @@ def _add_user(is_admin=False): qr.add_data(uri) sys.stdout = codecs.getwriter("utf-8")(sys.stdout) qr.print_ascii(tty=sys.stdout.isatty()) - print('\nIf the barcode does not render correctly, try ' + print(('\nIf the barcode does not render correctly, try ' "changing your terminal's font (Monospace for Linux, " 'Menlo for OS X). If you are using iTerm on Mac OS X, ' 'you will need to change the "Non-ASCII Font", which ' "is your profile\'s Text settings.\n\nCan't scan the " 'barcode? Enter following shared secret manually:' - '\n{}\n'.format(user.formatted_otp_secret)) + '\n{}\n'.format(user.formatted_otp_secret))) return 0 def _get_username_to_delete(): - return raw_input('Username to delete: ') + return input('Username to delete: ') def _get_delete_confirmation(user): - confirmation = raw_input('Are you sure you want to delete user ' + confirmation = input('Are you sure you want to delete user ' '"{}" (y/n)?'.format(user)) if confirmation.lower() != 'y': - print('Confirmation not received: user "{}" was NOT ' - 'deleted'.format(user)) + print(('Confirmation not received: user "{}" was NOT ' + 'deleted'.format(user))) return False return True @@ -248,7 +248,7 @@ def delete_user(args): else: raise e - print('User "{}" successfully deleted'.format(username)) + print(('User "{}" successfully deleted'.format(username))) return 0 diff --git a/securedrop/management/run.py b/securedrop/management/run.py index 1f971b67aa..16e4fbb018 100644 --- a/securedrop/management/run.py +++ b/securedrop/management/run.py @@ -99,7 +99,7 @@ def monitor(self): self.last_proc = proc line = proc.stdout.readline() - sys.stdout.write(line) + sys.stdout.write(line.decode('utf-8')) sys.stdout.flush() if any(proc.poll() is not None for proc in self.procs): @@ -143,8 +143,7 @@ def run(args): # pragma: no cover * https://stackoverflow.com/q/22565606/837471 """ - print \ -""" + print(""" ____ ____ /\\ _`\\ /\\ _`\\ \\ \\,\\L\\_\\ __ ___ __ __ _ __ __\\ \\ \\/\\ \\ _ __ ___ _____ @@ -154,14 +153,14 @@ def run(args): # pragma: no cover \\/_____/\\/____/\\/____/ \\/___/ \\/_/ \\/____/ \\/___/ \\/_/ \\/___/ \\ \\ \\/ \\ \\_\\ \\/_/ -""" # noqa +""") # noqa procs = [ lambda: DevServerProcess('Source Interface', - ['python', 'source.py'], + ['python3', 'source.py'], 'blue'), lambda: DevServerProcess('Journalist Interface', - ['python', 'journalist.py'], + ['python3', 'journalist.py'], 'cyan'), lambda: DevServerProcess('SASS Compiler', ['sass', '--watch', 'sass:static/css'], diff --git a/securedrop/secure_tempfile.py b/securedrop/secure_tempfile.py index 4a52da7135..93ccaf2968 100644 --- a/securedrop/secure_tempfile.py +++ b/securedrop/secure_tempfile.py @@ -47,7 +47,7 @@ def __init__(self, store_dir): """ self.last_action = 'init' self.create_key() - self.tmp_file_id = base64.urlsafe_b64encode(os.urandom(32)).strip('=') + self.tmp_file_id = base64.urlsafe_b64encode(os.urandom(32)).decode('utf-8').strip('=') self.filepath = os.path.join(store_dir, '{}.aes'.format(self.tmp_file_id)) self.file = io.open(self.filepath, 'w+b') @@ -61,8 +61,8 @@ def create_key(self): grsecurity-patched kernel it uses (for further details consult https://github.com/freedomofpress/securedrop/pull/477#issuecomment-168445450). """ - self.key = os.urandom(self.AES_key_size / 8) - self.iv = os.urandom(self.AES_block_size / 8) + self.key = os.urandom(int(self.AES_key_size / 8)) + self.iv = os.urandom(int(self.AES_block_size / 8)) self.initialize_cipher() def initialize_cipher(self): @@ -83,7 +83,7 @@ def write(self, data): raise AssertionError('You cannot write after reading!') self.last_action = 'write' - if isinstance(data, unicode): # noqa + if isinstance(data, str): # noqa data = data.encode('utf-8') self.file.write(self.encryptor.update(data)) diff --git a/securedrop/source_app/info.py b/securedrop/source_app/info.py index 5d6627c06e..4c27ba219d 100644 --- a/securedrop/source_app/info.py +++ b/securedrop/source_app/info.py @@ -1,6 +1,6 @@ # -*- coding: utf-8 -*- -from cStringIO import StringIO +from io import StringIO from flask import Blueprint, render_template, send_file, current_app diff --git a/securedrop/template_filters.py b/securedrop/template_filters.py index ac79ff83b2..d73561c1bc 100644 --- a/securedrop/template_filters.py +++ b/securedrop/template_filters.py @@ -18,7 +18,7 @@ def rel_datetime_format(dt, fmt=None, relative=False): def nl2br(context, value): - formatted = u'
\n'.join(escape(value).split('\n')) + formatted = '
\n'.join(escape(value).split('\n')) if context.autoescape: formatted = Markup(formatted) return formatted From 5a2e66fd4416cb36286e14cd29671db4d0d52c38 Mon Sep 17 00:00:00 2001 From: Kushal Das Date: Wed, 6 Mar 2019 19:19:12 +0530 Subject: [PATCH 08/47] Mass update of tests using 2to3 tool --- securedrop/tests/conftest.py | 6 ++--- .../tests/functional/functional_test.py | 4 ++-- .../functional/journalist_navigation_steps.py | 24 +++++++------------ .../tests/functional/test_admin_interface.py | 4 ++-- .../tests/functional/test_journalist.py | 6 ++--- .../functional/test_make_account_changes.py | 4 ++-- securedrop/tests/functional/test_source.py | 4 ++-- .../tests/functional/test_source_notfound.py | 4 ++-- .../functional/test_source_session_timeout.py | 4 ++-- .../tests/functional/test_source_warnings.py | 4 ++-- .../test_submit_and_retrieve_file.py | 6 ++--- .../test_submit_and_retrieve_message.py | 6 ++--- securedrop/tests/i18n/code.py | 2 +- .../tests/pageslayout/test_journalist.py | 2 +- securedrop/tests/pageslayout/test_source.py | 2 +- securedrop/tests/test_2fa.py | 8 +++---- securedrop/tests/test_alembic.py | 10 ++++---- securedrop/tests/test_db.py | 2 +- securedrop/tests/test_i18n.py | 2 +- securedrop/tests/test_i18n_tool.py | 14 +++++------ securedrop/tests/test_integration.py | 8 +++---- securedrop/tests/test_journalist.py | 8 +++---- securedrop/tests/test_journalist_api.py | 14 +++++------ securedrop/tests/test_manage.py | 4 ++-- securedrop/tests/test_secure_tempfile.py | 2 +- securedrop/tests/test_source.py | 8 +++---- securedrop/tests/test_store.py | 2 +- securedrop/tests/test_template_filters.py | 2 +- securedrop/tests/utils/__init__.py | 6 ++--- securedrop/tests/utils/instrument.py | 4 ++-- 30 files changed, 85 insertions(+), 91 deletions(-) diff --git a/securedrop/tests/conftest.py b/securedrop/tests/conftest.py index d79aaad50d..c7574d0490 100644 --- a/securedrop/tests/conftest.py +++ b/securedrop/tests/conftest.py @@ -11,7 +11,7 @@ import signal import subprocess -from ConfigParser import SafeConfigParser +from configparser import SafeConfigParser from flask import url_for from pyotp import TOTP @@ -24,7 +24,7 @@ from journalist_app import create_app as create_journalist_app import models from source_app import create_app as create_source_app -import utils +from . import utils # The PID file for the redis worker is hard-coded below. # Ideally this constant would be provided by a test harness. @@ -252,7 +252,7 @@ def _get_pid_from_file(pid_file_name): try: return int(io.open(pid_file_name).read()) except IOError: - return None + return -1 def _cleanup_test_securedrop_dataroot(config): diff --git a/securedrop/tests/functional/functional_test.py b/securedrop/tests/functional/functional_test.py index c5f48580fe..23ec5a3501 100644 --- a/securedrop/tests/functional/functional_test.py +++ b/securedrop/tests/functional/functional_test.py @@ -64,8 +64,8 @@ def _create_webdriver(self, firefox, profile=None): firefox_profile=profile) if i > 0: # i==0 is normal behavior without connection refused. - print('NOTE: Retried {} time(s) due to ' - 'connection refused.'.format(i)) + print(('NOTE: Retried {} time(s) due to ' + 'connection refused.'.format(i))) return driver except socket.error as socket_error: if (socket_error.errno == errno.ECONNREFUSED diff --git a/securedrop/tests/functional/journalist_navigation_steps.py b/securedrop/tests/functional/journalist_navigation_steps.py index 676844dd04..c512bb17e2 100644 --- a/securedrop/tests/functional/journalist_navigation_steps.py +++ b/securedrop/tests/functional/journalist_navigation_steps.py @@ -1,5 +1,5 @@ import pytest -import urllib2 +import urllib.request, urllib.error, urllib.parse import re import tempfile import gzip @@ -227,7 +227,7 @@ def _add_user(self, username, is_admin=False, hotp=None): if hotp: hotp_checkbox = self.driver.find_element_by_css_selector( 'input[name="is_hotp"]') - print(str(hotp_checkbox.__dict__)) + print((str(hotp_checkbox.__dict__))) hotp_checkbox.click() hotp_secret = self.driver.find_element_by_css_selector( 'input[name="otp_secret"]') @@ -394,9 +394,7 @@ def _edit_account(self): def _edit_user(self, username): user = Journalist.query.filter_by(username=username).one() - new_user_edit_links = filter( - lambda el: el.get_attribute('data-username') == username, - self.driver.find_elements_by_tag_name('a')) + new_user_edit_links = [el for el in self.driver.find_elements_by_tag_name('a') if el.get_attribute('data-username') == username] assert 1 == len(new_user_edit_links) new_user_edit_links[0].click() # The header says "Edit user "username"". @@ -447,10 +445,8 @@ def _admin_can_edit_new_user(self): # Click the "edit user" link for the new user # self._edit_user(self.new_user['username']) - new_user_edit_links = filter( - lambda el: (el.get_attribute('data-username') == - self.new_user['username']), - self.driver.find_elements_by_tag_name('a')) + new_user_edit_links = [el for el in self.driver.find_elements_by_tag_name('a') if (el.get_attribute('data-username') == + self.new_user['username'])] assert len(new_user_edit_links) == 1 new_user_edit_links[0].click() @@ -587,12 +583,12 @@ def cookie_string_from_selenium_cookies(cookies): cookie_strs.append(cookie_str) return ' '.join(cookie_strs) - submission_req = urllib2.Request(file_url) + submission_req = urllib.request.Request(file_url) submission_req.add_header( 'Cookie', cookie_string_from_selenium_cookies( self.driver.get_cookies())) - raw_content = urllib2.urlopen(submission_req).read() + raw_content = urllib.request.urlopen(submission_req).read() decrypted_submission = self.gpg.decrypt(raw_content) submission = self._get_submission_content(file_url, @@ -652,10 +648,8 @@ def _admin_visits_add_user(self): add_user_btn.click() def _admin_visits_edit_user(self): - new_user_edit_links = filter( - lambda el: (el.get_attribute('data-username') == - self.new_user['username']), - self.driver.find_elements_by_tag_name('a')) + new_user_edit_links = [el for el in self.driver.find_elements_by_tag_name('a') if (el.get_attribute('data-username') == + self.new_user['username'])] assert len(new_user_edit_links) == 1 new_user_edit_links[0].click() diff --git a/securedrop/tests/functional/test_admin_interface.py b/securedrop/tests/functional/test_admin_interface.py index 79b402449f..d96c4123dc 100644 --- a/securedrop/tests/functional/test_admin_interface.py +++ b/securedrop/tests/functional/test_admin_interface.py @@ -1,5 +1,5 @@ -import functional_test -import journalist_navigation_steps +from . import functional_test +from . import journalist_navigation_steps class TestAdminInterface( diff --git a/securedrop/tests/functional/test_journalist.py b/securedrop/tests/functional/test_journalist.py index 127af1e34c..98235c38af 100644 --- a/securedrop/tests/functional/test_journalist.py +++ b/securedrop/tests/functional/test_journalist.py @@ -15,9 +15,9 @@ # You should have received a copy of the GNU Affero General Public License # along with this program. If not, see . # -import source_navigation_steps -import journalist_navigation_steps -import functional_test +from . import source_navigation_steps +from . import journalist_navigation_steps +from . import functional_test class TestJournalist( diff --git a/securedrop/tests/functional/test_make_account_changes.py b/securedrop/tests/functional/test_make_account_changes.py index c6f8e99a96..3606fdc76d 100644 --- a/securedrop/tests/functional/test_make_account_changes.py +++ b/securedrop/tests/functional/test_make_account_changes.py @@ -1,6 +1,6 @@ # -*- coding: utf-8 -*- -import journalist_navigation_steps -import functional_test +from . import journalist_navigation_steps +from . import functional_test class TestMakeAccountChanges( diff --git a/securedrop/tests/functional/test_source.py b/securedrop/tests/functional/test_source.py index 7d558235fc..55721d71be 100644 --- a/securedrop/tests/functional/test_source.py +++ b/securedrop/tests/functional/test_source.py @@ -1,5 +1,5 @@ -import source_navigation_steps -import functional_test +from . import source_navigation_steps +from . import functional_test class TestSourceInterface( diff --git a/securedrop/tests/functional/test_source_notfound.py b/securedrop/tests/functional/test_source_notfound.py index e037833d52..2da3889259 100644 --- a/securedrop/tests/functional/test_source_notfound.py +++ b/securedrop/tests/functional/test_source_notfound.py @@ -1,5 +1,5 @@ -import source_navigation_steps -import functional_test +from . import source_navigation_steps +from . import functional_test class TestSourceInterfaceNotFound( diff --git a/securedrop/tests/functional/test_source_session_timeout.py b/securedrop/tests/functional/test_source_session_timeout.py index a791d18f1e..aaf1c15e93 100644 --- a/securedrop/tests/functional/test_source_session_timeout.py +++ b/securedrop/tests/functional/test_source_session_timeout.py @@ -1,5 +1,5 @@ -import source_navigation_steps -import functional_test +from . import source_navigation_steps +from . import functional_test class TestSourceSessions( diff --git a/securedrop/tests/functional/test_source_warnings.py b/securedrop/tests/functional/test_source_warnings.py index eeec05a72d..74a3cc9cfb 100644 --- a/securedrop/tests/functional/test_source_warnings.py +++ b/securedrop/tests/functional/test_source_warnings.py @@ -1,5 +1,5 @@ -import source_navigation_steps -import functional_test +from . import source_navigation_steps +from . import functional_test from selenium import webdriver diff --git a/securedrop/tests/functional/test_submit_and_retrieve_file.py b/securedrop/tests/functional/test_submit_and_retrieve_file.py index c8668f25b4..299a6ae9e7 100644 --- a/securedrop/tests/functional/test_submit_and_retrieve_file.py +++ b/securedrop/tests/functional/test_submit_and_retrieve_file.py @@ -1,6 +1,6 @@ -import source_navigation_steps -import journalist_navigation_steps -import functional_test +from . import source_navigation_steps +from . import journalist_navigation_steps +from . import functional_test class TestSubmitAndRetrieveFile( diff --git a/securedrop/tests/functional/test_submit_and_retrieve_message.py b/securedrop/tests/functional/test_submit_and_retrieve_message.py index 7378b0ae81..eef263a47c 100644 --- a/securedrop/tests/functional/test_submit_and_retrieve_message.py +++ b/securedrop/tests/functional/test_submit_and_retrieve_message.py @@ -1,6 +1,6 @@ -import functional_test -import source_navigation_steps -import journalist_navigation_steps +from . import functional_test +from . import source_navigation_steps +from . import journalist_navigation_steps class TestSubmitAndRetrieveMessage( diff --git a/securedrop/tests/i18n/code.py b/securedrop/tests/i18n/code.py index 90b456adc6..446c48a932 100644 --- a/securedrop/tests/i18n/code.py +++ b/securedrop/tests/i18n/code.py @@ -1,4 +1,4 @@ # -*- coding: utf-8 -*- from flask_babel import gettext -print(gettext('code hello i18n')) +print((gettext('code hello i18n'))) diff --git a/securedrop/tests/pageslayout/test_journalist.py b/securedrop/tests/pageslayout/test_journalist.py index 53234a4932..1c50f229ba 100644 --- a/securedrop/tests/pageslayout/test_journalist.py +++ b/securedrop/tests/pageslayout/test_journalist.py @@ -17,7 +17,7 @@ # from tests.functional import journalist_navigation_steps from tests.functional import source_navigation_steps -import functional_test +from . import functional_test import pytest import time diff --git a/securedrop/tests/pageslayout/test_source.py b/securedrop/tests/pageslayout/test_source.py index cb1d9b57f9..2219356791 100644 --- a/securedrop/tests/pageslayout/test_source.py +++ b/securedrop/tests/pageslayout/test_source.py @@ -17,7 +17,7 @@ # from tests.functional import journalist_navigation_steps from tests.functional import source_navigation_steps -import functional_test +from . import functional_test import pytest diff --git a/securedrop/tests/test_2fa.py b/securedrop/tests/test_2fa.py index d3368cc5c8..67283613b3 100644 --- a/securedrop/tests/test_2fa.py +++ b/securedrop/tests/test_2fa.py @@ -10,8 +10,8 @@ os.environ['SECUREDROP_ENV'] = 'test' # noqa from models import Journalist, BadTokenException -from utils import login_user -from utils.instrument import InstrumentedApp +from .utils import login_user +from .utils.instrument import InstrumentedApp @contextmanager @@ -89,7 +89,7 @@ def test_bad_token_fails_to_verify_on_admin_new_user_two_factor_page( https://github.com/freedomofpress/securedrop/pull/1692 ''' - invalid_token = u'000000' + invalid_token = '000000' with totp_window(): with journalist_app.test_client() as app: @@ -127,7 +127,7 @@ def test_bad_token_fails_to_verify_on_new_user_two_factor_page( '''Regression test for https://github.com/freedomofpress/securedrop/pull/1692 ''' - invalid_token = u'000000' + invalid_token = '000000' with totp_window(): with journalist_app.test_client() as app: diff --git a/securedrop/tests/test_alembic.py b/securedrop/tests/test_alembic.py index b4bc1fa4fb..33ba88a3fa 100644 --- a/securedrop/tests/test_alembic.py +++ b/securedrop/tests/test_alembic.py @@ -10,7 +10,7 @@ from os import path from sqlalchemy import text -import conftest +from . import conftest from db import db from journalist_app import create_app @@ -55,7 +55,7 @@ def get_schema(app): def assert_schemas_equal(left, right): - for (k, v) in left.items(): + for (k, v) in list(left.items()): if k not in right: raise AssertionError( 'Left contained {} but right did not'.format(k)) @@ -67,7 +67,7 @@ def assert_schemas_equal(left, right): if right: raise AssertionError( - 'Right had additional tables: {}'.format(right.keys())) + 'Right had additional tables: {}'.format(list(right.keys()))) def ddl_equal(left, right): @@ -118,7 +118,7 @@ def test_alembic_head_matches_db_models(journalist_app, # The initial migration creates the table 'alembic_version', but this is # not present in the schema created by `db.create_all()`. - alembic_schema = {k: v for k, v in alembic_schema.items() + alembic_schema = {k: v for k, v in list(alembic_schema.items()) if k[2] != 'alembic_version'} assert_schemas_equal(alembic_schema, models_schema) @@ -171,7 +171,7 @@ def test_schema_unchanged_after_up_then_downgrade(alembic_config, # The initial migration is a degenerate case because it creates the table # 'alembic_version', but rolling back the migration doesn't clear it. if len(migrations) == 1: - reverted_schema = {k: v for k, v in reverted_schema.items() + reverted_schema = {k: v for k, v in list(reverted_schema.items()) if k[2] != 'alembic_version'} assert_schemas_equal(reverted_schema, original_schema) diff --git a/securedrop/tests/test_db.py b/securedrop/tests/test_db.py index 8f8af2bcd3..c0462c1d94 100644 --- a/securedrop/tests/test_db.py +++ b/securedrop/tests/test_db.py @@ -3,7 +3,7 @@ from mock import MagicMock -from utils import db_helper +from .utils import db_helper from models import (Journalist, Submission, Reply, Source, get_one_or_else, LoginThrottledException) diff --git a/securedrop/tests/test_i18n.py b/securedrop/tests/test_i18n.py index 8f0a633d79..2a315bdd91 100644 --- a/securedrop/tests/test_i18n.py +++ b/securedrop/tests/test_i18n.py @@ -25,7 +25,7 @@ os.environ['SECUREDROP_ENV'] = 'test' # noqa from sdconfig import SDConfig -import i18n +from . import i18n import i18n_tool import journalist_app as journalist_app_module import pytest diff --git a/securedrop/tests/test_i18n_tool.py b/securedrop/tests/test_i18n_tool.py index ad66ca2e41..ea22e47185 100644 --- a/securedrop/tests/test_i18n_tool.py +++ b/securedrop/tests/test_i18n_tool.py @@ -65,7 +65,7 @@ def test_translate_desktop_l10n(self, tmpdir): '--verbose', 'translate-desktop', '--translations-dir', str(tmpdir), - '--sources', ",".join(in_files.values()), + '--sources', ",".join(list(in_files.values())), '--extract-update', ]) assert not exists(i18n_file) @@ -108,7 +108,7 @@ def test_translate_desktop_l10n(self, tmpdir): '--verbose', 'translate-desktop', '--translations-dir', str(tmpdir), - '--sources', ",".join(in_files.values() + ['BOOM']), + '--sources', ",".join(list(in_files.values()) + ['BOOM']), '--compile', ]) assert old_messages_mtime == getmtime(messages_file) @@ -253,7 +253,7 @@ def test_update_from_weblate(self, tmpdir, caplog): k = {'_cwd': join(d, repo)} git.init(**k) git.config('user.email', 'you@example.com', **k) - git.config('user.name', u'Loïc Nordhøy', **k) + git.config('user.name', 'Loïc Nordhøy', **k) touch('README.md', **k) git.add('README.md', **k) git.commit('-m', 'README', 'README.md', **k) @@ -314,9 +314,9 @@ def r(): ]) assert 'l10n: updated nl' not in r() assert 'l10n: updated de_DE' not in r() - message = unicode(git('--no-pager', '-C', 'securedrop', 'show', + message = str(git('--no-pager', '-C', 'securedrop', 'show', _cwd=d, _encoding='utf-8')) - assert u"Loïc" in message + assert "Loïc" in message # # an update is done to nl in weblate @@ -347,7 +347,7 @@ def r(): ]) assert 'l10n: updated nl' in r() assert 'l10n: updated de_DE' not in r() - message = unicode(git('--no-pager', '-C', 'securedrop', 'show', + message = str(git('--no-pager', '-C', 'securedrop', 'show', _cwd=d)) assert "Someone Else" in message - assert u"Loïc" not in message + assert "Loïc" not in message diff --git a/securedrop/tests/test_integration.py b/securedrop/tests/test_integration.py index 05b7399974..9ad7eb660b 100644 --- a/securedrop/tests/test_integration.py +++ b/securedrop/tests/test_integration.py @@ -10,14 +10,14 @@ from base64 import b32encode from binascii import unhexlify from bs4 import BeautifulSoup -from cStringIO import StringIO +from io import StringIO from flask import session, g, escape, current_app from pyotp import TOTP, HOTP os.environ['SECUREDROP_ENV'] = 'test' # noqa -import utils +from . import utils -from utils.instrument import InstrumentedApp +from .utils.instrument import InstrumentedApp # Seed the RNG for deterministic testing random.seed('ಠ_ಠ') @@ -448,7 +448,7 @@ def test_unicode_reply_with_ansi_env(journalist_app, journalist_app.crypto_util.gpg._encoding = "ansi_x3.4_1968" source_app.crypto_util.gpg._encoding = "ansi_x3.4_1968" _helper_test_reply(journalist_app, source_app, config, test_journo, - u"ᚠᛇᚻ᛫ᛒᛦᚦ᛫ᚠᚱᚩᚠᚢᚱ᛫ᚠᛁᚱᚪ᛫ᚷᛖᚻᚹᛦᛚᚳᚢᛗ", True) + "ᚠᛇᚻ᛫ᛒᛦᚦ᛫ᚠᚱᚩᚠᚢᚱ᛫ᚠᛁᚱᚪ᛫ᚷᛖᚻᚹᛦᛚᚳᚢᛗ", True) def test_delete_collection(mocker, source_app, journalist_app, test_journo): diff --git a/securedrop/tests/test_journalist.py b/securedrop/tests/test_journalist.py index dd81df6e95..8bb6618492 100644 --- a/securedrop/tests/test_journalist.py +++ b/securedrop/tests/test_journalist.py @@ -8,7 +8,7 @@ import datetime from base64 import b64decode -from cStringIO import StringIO +from io import StringIO from io import BytesIO from flask import url_for, escape, session, current_app, g from mock import patch @@ -20,7 +20,7 @@ import crypto_util import models import journalist_app as journalist_app_module -import utils +from . import utils os.environ['SECUREDROP_ENV'] = 'test' # noqa from sdconfig import SDConfig, config @@ -28,7 +28,7 @@ from db import db from models import (InvalidPasswordLength, Journalist, Reply, Source, Submission) -from utils.instrument import InstrumentedApp +from .utils.instrument import InstrumentedApp # Smugly seed the RNG for deterministic testing random.seed('¯\_(ツ)_/¯') @@ -2022,7 +2022,7 @@ def test_does_set_cookie_headers(journalist_app, test_journo): response = app.get(url_for('main.login')) observed_headers = response.headers - assert 'Set-Cookie' in observed_headers.keys() + assert 'Set-Cookie' in list(observed_headers.keys()) assert 'Cookie' in observed_headers['Vary'] diff --git a/securedrop/tests/test_journalist_api.py b/securedrop/tests/test_journalist_api.py index f99a277ea2..f8222528a8 100644 --- a/securedrop/tests/test_journalist_api.py +++ b/securedrop/tests/test_journalist_api.py @@ -14,7 +14,7 @@ from models import Journalist, Reply, Source, SourceStar, Submission os.environ['SECUREDROP_ENV'] = 'test' # noqa -from utils.api_helper import get_api_headers +from .utils.api_helper import get_api_headers random.seed('◔ ⌣ ◔') @@ -24,10 +24,10 @@ def test_unauthenticated_user_gets_all_endpoints(journalist_app): response = app.get(url_for('api.get_endpoints')) observed_endpoints = json.loads(response.data) - expected_endpoints = [u'current_user_url', u'submissions_url', - u'sources_url', u'auth_token_url', - u'replies_url'] - assert expected_endpoints == observed_endpoints.keys() + expected_endpoints = ['current_user_url', 'submissions_url', + 'sources_url', 'auth_token_url', + 'replies_url'] + assert expected_endpoints == list(observed_endpoints.keys()) def test_valid_user_can_get_an_api_token(journalist_app, test_journo): @@ -871,8 +871,8 @@ def test_api_does_not_set_cookie_headers(journalist_app, test_journo): response = app.get(url_for('api.get_endpoints')) observed_headers = response.headers - assert 'Set-Cookie' not in observed_headers.keys() - if 'Vary' in observed_headers.keys(): + assert 'Set-Cookie' not in list(observed_headers.keys()) + if 'Vary' in list(observed_headers.keys()): assert 'Cookie' not in observed_headers['Vary'] diff --git a/securedrop/tests/test_manage.py b/securedrop/tests/test_manage.py index 57366fda8c..ccbfb3b1a9 100644 --- a/securedrop/tests/test_manage.py +++ b/securedrop/tests/test_manage.py @@ -10,12 +10,12 @@ import sys import time -from StringIO import StringIO +from io import StringIO os.environ['SECUREDROP_ENV'] = 'test' # noqa from models import Journalist, db -from utils import db_helper +from .utils import db_helper YUBIKEY_HOTP = ['cb a0 5f ad 41 a2 ff 4e eb 53 56 3a 1b f7 23 2e ce fc dc', diff --git a/securedrop/tests/test_secure_tempfile.py b/securedrop/tests/test_secure_tempfile.py index 094a64dbb0..f5c2077021 100644 --- a/securedrop/tests/test_secure_tempfile.py +++ b/securedrop/tests/test_secure_tempfile.py @@ -50,7 +50,7 @@ def test_write_then_read_then_write(): def test_read_write_unicode(): f = SecureTemporaryFile('/tmp') - unicode_msg = u'鬼神 Kill Em All 1989' + unicode_msg = '鬼神 Kill Em All 1989' f.write(unicode_msg) assert f.read().decode('utf-8') == unicode_msg diff --git a/securedrop/tests/test_source.py b/securedrop/tests/test_source.py index 68b1fcf689..26448b647d 100644 --- a/securedrop/tests/test_source.py +++ b/securedrop/tests/test_source.py @@ -5,13 +5,13 @@ import re import subprocess -from cStringIO import StringIO +from io import StringIO from flask import session, escape, current_app, url_for, g from mock import patch, ANY import crypto_util import source -import utils +from . import utils import version from datetime import date @@ -20,8 +20,8 @@ from source_app import main as source_app_main from source_app import api as source_app_api from source_app import disable as source_app_disable -from utils.db_helper import new_codename -from utils.instrument import InstrumentedApp +from .utils.db_helper import new_codename +from .utils.instrument import InstrumentedApp overly_long_codename = 'a' * (Source.MAX_CODENAME_LEN + 1) TRUSTY_DISABLED_ENDPOINTS = ['main.index', 'main.lookup', 'main.generate', 'main.login', diff --git a/securedrop/tests/test_store.py b/securedrop/tests/test_store.py index 2e2dddd97f..c278f94492 100644 --- a/securedrop/tests/test_store.py +++ b/securedrop/tests/test_store.py @@ -7,7 +7,7 @@ import zipfile os.environ['SECUREDROP_ENV'] = 'test' # noqa -import utils +from . import utils from store import Storage diff --git a/securedrop/tests/test_template_filters.py b/securedrop/tests/test_template_filters.py index 69e7d07ba9..dcb1706ff1 100644 --- a/securedrop/tests/test_template_filters.py +++ b/securedrop/tests/test_template_filters.py @@ -5,7 +5,7 @@ from flask import session os.environ['SECUREDROP_ENV'] = 'test' # noqa -import i18n +from . import i18n import i18n_tool import journalist_app import source_app diff --git a/securedrop/tests/utils/__init__.py b/securedrop/tests/utils/__init__.py index 922b55abe5..bd0eb75cc8 100644 --- a/securedrop/tests/utils/__init__.py +++ b/securedrop/tests/utils/__init__.py @@ -3,9 +3,9 @@ from flask import g from pyotp import TOTP -import async # noqa -import db_helper # noqa -import env # noqa +from . import async # noqa +from . import db_helper # noqa +from . import env # noqa def login_user(app, test_user): diff --git a/securedrop/tests/utils/instrument.py b/securedrop/tests/utils/instrument.py index aab250085f..fd4507f684 100644 --- a/securedrop/tests/utils/instrument.py +++ b/securedrop/tests/utils/instrument.py @@ -7,13 +7,13 @@ :copyright: (c) 2010 by Dan Jacob. :license: BSD, see LICENSE for more details. """ -from __future__ import absolute_import, with_statement + try: from urllib.parse import urlparse, urljoin except ImportError: # Python 2 urlparse fallback - from urlparse import urlparse, urljoin + from urllib.parse import urlparse, urljoin import pytest From 14bebc390860581cf8ad044bfbd8803d5bba5a80 Mon Sep 17 00:00:00 2001 From: Kushal Das Date: Wed, 6 Mar 2019 20:05:43 +0530 Subject: [PATCH 09/47] Fixes crypto_util tests for python3 --- securedrop/crypto_util.py | 2 +- securedrop/tests/test_crypto_util.py | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/securedrop/crypto_util.py b/securedrop/crypto_util.py index a284151aa7..cb31fdac54 100644 --- a/securedrop/crypto_util.py +++ b/securedrop/crypto_util.py @@ -259,7 +259,7 @@ def decrypt(self, secret, ciphertext): """ hashed_codename = self.hash_codename(secret, salt=self.scrypt_gpg_pepper) - return self.gpg.decrypt(ciphertext, passphrase=hashed_codename).data + return self.gpg.decrypt(ciphertext, passphrase=hashed_codename).data.decode('utf-8') def clean(s, also=''): diff --git a/securedrop/tests/test_crypto_util.py b/securedrop/tests/test_crypto_util.py index 47a35de36a..c5e2b20e1e 100644 --- a/securedrop/tests/test_crypto_util.py +++ b/securedrop/tests/test_crypto_util.py @@ -43,8 +43,8 @@ def test_encrypt_success(source_app, config, test_source): source_app.storage.path(test_source['filesystem_id'], 'somefile.gpg')) - assert isinstance(ciphertext, str) - assert ciphertext != message + assert isinstance(ciphertext, bytes) + assert ciphertext.decode('utf-8') != message assert len(ciphertext) > 0 @@ -145,7 +145,7 @@ def test_basic_encrypt_then_decrypt_multiple_recipients(source_app, # ensure we can decrypt with the `config.JOURNALIST_KEY`. source_app.crypto_util.delete_reply_keypair( test_source['filesystem_id']) - plaintext = source_app.crypto_util.gpg.decrypt(ciphertext).data + plaintext = source_app.crypto_util.gpg.decrypt(ciphertext).data.decode('utf-8') assert plaintext == message From d4840a78888e71b7a28b5146d1f0c3e6f117339f Mon Sep 17 00:00:00 2001 From: Kushal Das Date: Thu, 21 Mar 2019 15:47:20 +0530 Subject: [PATCH 10/47] Fixes bytes vs strings for Python3 Also fixes the import statement for Python3 --- securedrop/tests/test_i18n.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/securedrop/tests/test_i18n.py b/securedrop/tests/test_i18n.py index 2a315bdd91..1e6dbd9f71 100644 --- a/securedrop/tests/test_i18n.py +++ b/securedrop/tests/test_i18n.py @@ -25,7 +25,7 @@ os.environ['SECUREDROP_ENV'] = 'test' # noqa from sdconfig import SDConfig -from . import i18n +import i18n import i18n_tool import journalist_app as journalist_app_module import pytest @@ -93,15 +93,15 @@ def verify_i18n(app): page = c.get('/login') assert session.get('locale') is None assert not_translated == gettext(not_translated) - assert '?l=fr_FR' in page.data - assert '?l=en_US' not in page.data + assert b'?l=fr_FR' in page.data + assert b'?l=en_US' not in page.data page = c.get('/login?l=fr_FR', headers=Headers([('Accept-Language', 'en_US')])) assert session.get('locale') == 'fr_FR' assert translated_fr == gettext(not_translated) - assert '?l=fr_FR' not in page.data - assert '?l=en_US' in page.data + assert b'?l=fr_FR' not in page.data + assert b'?l=en_US' in page.data c.get('/', headers=Headers([('Accept-Language', 'en_US')])) assert session.get('locale') == 'fr_FR' From e31e2127d0319bfe96581245d7af48801136a1a4 Mon Sep 17 00:00:00 2001 From: Kushal Das Date: Thu, 21 Mar 2019 18:34:07 +0530 Subject: [PATCH 11/47] Fixes bytes vs strings for the i18n tool --- securedrop/i18n_tool.py | 6 +++--- securedrop/tests/test_i18n_tool.py | 16 ++++++++-------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/securedrop/i18n_tool.py b/securedrop/i18n_tool.py index 3272b66beb..116d75d18b 100755 --- a/securedrop/i18n_tool.py +++ b/securedrop/i18n_tool.py @@ -58,7 +58,7 @@ def file_is_modified(self, path): def ensure_i18n_remote(self, args): k = {'_cwd': args.root} - if 'i18n' not in git.remote(**k).stdout: + if b'i18n' not in git.remote(**k).stdout: git.remote.add('i18n', args.url, **k) git.fetch('i18n', **k) @@ -284,8 +284,8 @@ def add(p): def upstream_commit(self, args, code): self.require_git_email_name(args.root) authors = set() - diffs = git('--no-pager', '-C', args.root, - 'diff', '--name-only', '--cached').stdout + diffs = str(git('--no-pager', '-C', args.root, + 'diff', '--name-only', '--cached').stdout) for path in diffs.strip().split('\n'): previous_message = str(git( '--no-pager', '-C', args.root, 'log', '-n', '1', path, diff --git a/securedrop/tests/test_i18n_tool.py b/securedrop/tests/test_i18n_tool.py index ea22e47185..ac793b0cee 100644 --- a/securedrop/tests/test_i18n_tool.py +++ b/securedrop/tests/test_i18n_tool.py @@ -137,10 +137,10 @@ def test_translate_messages_l10n(self, tmpdir): i18n_tool.I18NTool().main(args) messages_file = join(str(tmpdir), 'messages.pot') assert exists(messages_file) - with io.open(messages_file) as fobj: + with io.open(messages_file, 'rb') as fobj: pot = fobj.read() - assert 'code hello i18n' in pot - assert 'template hello i18n' in pot + assert b'code hello i18n' in pot + assert b'template hello i18n' in pot locale = 'en_US' locale_dir = join(str(tmpdir), locale) @@ -151,8 +151,8 @@ def test_translate_messages_l10n(self, tmpdir): assert exists(mo_file) with io.open(mo_file, mode='rb') as fobj: mo = fobj.read() - assert 'code hello i18n' in mo - assert 'template hello i18n' in mo + assert b'code hello i18n' in mo + assert b'template hello i18n' in mo def test_translate_messages_compile_arg(self, tmpdir): args = [ @@ -210,15 +210,15 @@ def test_translate_messages_compile_arg(self, tmpdir): assert old_po_mtime == getmtime(po_file) with io.open(mo_file, mode='rb') as fobj: mo = fobj.read() - assert 'code hello i18n' in mo - assert 'template hello i18n' not in mo + assert b'code hello i18n' in mo + assert b'template hello i18n' not in mo def test_require_git_email_name(self, tmpdir): k = {'_cwd': str(tmpdir)} git('init', **k) with pytest.raises(Exception) as excinfo: i18n_tool.I18NTool.require_git_email_name(str(tmpdir)) - assert 'please set name' in excinfo.value.message + assert 'please set name' in str(excinfo.value) git.config('user.email', "you@example.com", **k) git.config('user.name', "Your Name", **k) From bfed0d1398678846f4f2cc32bbedfac150c7f872 Mon Sep 17 00:00:00 2001 From: Kushal Das Date: Thu, 21 Mar 2019 19:06:44 +0530 Subject: [PATCH 12/47] Fixes rq on Python3 Requires any locale other than posix for click module. --- securedrop/bin/dev-shell | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/securedrop/bin/dev-shell b/securedrop/bin/dev-shell index 36f74848d2..8e61f2dde9 100755 --- a/securedrop/bin/dev-shell +++ b/securedrop/bin/dev-shell @@ -41,13 +41,15 @@ function docker_run() { find . \( -name '*.pyc' -o -name __pycache__ \) -delete docker run \ -p 127.0.0.1:5901:5901 \ - --rm \ + --rm \ + --user "${USER:-root}" \ + --volume "${TOPLEVEL}:${TOPLEVEL}" \ + --workdir "${TOPLEVEL}/securedrop" \ -e NUM_SOURCES \ - --user "${USER:-root}" \ - --volume "${TOPLEVEL}:${TOPLEVEL}" \ - --workdir "${TOPLEVEL}/securedrop" \ + -e LC_ALL=C.UTF-8 \ + -e LANG=C.UTF-8 \ --name securedrop-dev \ - -ti ${DOCKER_RUN_ARGUMENTS:-} "securedrop-test-${1}" "${@:2}" + -ti ${DOCKER_RUN_ARGUMENTS:-} "securedrop-test-${1}" "${@:2}" } if test -n "${CIRCLE_SHA1:-}" ; then From c0a60a965bae4095cd79609c70b94fa4141d31d9 Mon Sep 17 00:00:00 2001 From: Kushal Das Date: Thu, 21 Mar 2019 19:09:03 +0530 Subject: [PATCH 13/47] Fixes for bytes vs strings in Python3 --- securedrop/source_app/main.py | 2 +- securedrop/tests/test_integration.py | 26 +++++++++++++------------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/securedrop/source_app/main.py b/securedrop/source_app/main.py index 720cf72667..429ac4b8e0 100644 --- a/securedrop/source_app/main.py +++ b/securedrop/source_app/main.py @@ -87,7 +87,7 @@ def lookup(): with io.open(reply_path, "rb") as f: contents = f.read() reply_obj = current_app.crypto_util.decrypt(g.codename, contents) - reply.decrypted = reply_obj.decode('utf-8') + reply.decrypted = reply_obj except UnicodeDecodeError: current_app.logger.error("Could not decode reply %s" % reply.filename) diff --git a/securedrop/tests/test_integration.py b/securedrop/tests/test_integration.py index 9ad7eb660b..cb722b5019 100644 --- a/securedrop/tests/test_integration.py +++ b/securedrop/tests/test_integration.py @@ -10,7 +10,7 @@ from base64 import b32encode from binascii import unhexlify from bs4 import BeautifulSoup -from io import StringIO +from io import BytesIO from flask import session, g, escape, current_app from pyotp import TOTP, HOTP @@ -45,7 +45,7 @@ def test_submit_message(source_app, journalist_app, test_journo): # redirected to submission form resp = app.post('/submit', data=dict( msg=test_msg, - fh=(StringIO(''), ''), + fh=(BytesIO(b''), ''), ), follow_redirects=True) assert resp.status_code == 200 app.get('/logout') @@ -79,7 +79,7 @@ def test_submit_message(source_app, journalist_app, test_journo): assert resp.status_code == 200 decrypted_data = journalist_app.crypto_util.gpg.decrypt(resp.data) assert decrypted_data.ok - assert decrypted_data.data == test_msg + assert decrypted_data.data.decode('utf-8') == test_msg # delete submission resp = app.get(col_url) @@ -132,7 +132,7 @@ def assertion(): def test_submit_file(source_app, journalist_app, test_journo): """When a source creates an account, test that a new entry appears in the journalist interface""" - test_file_contents = "This is a test file." + test_file_contents = b"This is a test file." test_filename = "test.txt" with source_app.test_client() as app: @@ -142,7 +142,7 @@ def test_submit_file(source_app, journalist_app, test_journo): # redirected to submission form resp = app.post('/submit', data=dict( msg="", - fh=(StringIO(test_file_contents), test_filename), + fh=(BytesIO(test_file_contents), test_filename), ), follow_redirects=True) assert resp.status_code == 200 app.get('/logout') @@ -176,7 +176,7 @@ def test_submit_file(source_app, journalist_app, test_journo): decrypted_data = journalist_app.crypto_util.gpg.decrypt(resp.data) assert decrypted_data.ok - sio = StringIO(decrypted_data.data) + sio = BytesIO(decrypted_data.data) with gzip.GzipFile(mode='rb', fileobj=sio) as gzip_file: unzipped_decrypted_data = gzip_file.read() mtime = gzip_file.mtime @@ -244,7 +244,7 @@ def _helper_test_reply(journalist_app, source_app, config, test_journo, # redirected to submission form resp = app.post('/submit', data=dict( msg=test_msg, - fh=(StringIO(''), ''), + fh=(BytesIO(b''), ''), ), follow_redirects=True) assert resp.status_code == 200 assert not g.source.flagged @@ -322,7 +322,7 @@ def assertion(): ), follow_redirects=True) assert resp.status_code == 200 - zf = zipfile.ZipFile(StringIO(resp.data), 'r') + zf = zipfile.ZipFile(BytesIO(resp.data), 'r') data = zf.read(zf.namelist()[0]) _can_decrypt_with_key(journalist_app, data) _can_decrypt_with_key( @@ -461,7 +461,7 @@ def test_delete_collection(mocker, source_app, journalist_app, test_journo): app.post('/create') resp = app.post('/submit', data=dict( msg="This is a test.", - fh=(StringIO(''), ''), + fh=(BytesIO(b''), ''), ), follow_redirects=True) assert resp.status_code == 200 @@ -510,7 +510,7 @@ def test_delete_collections(mocker, journalist_app, source_app, test_journo): app.post('/create') app.post('/submit', data=dict( msg="This is a test " + str(i) + ".", - fh=(StringIO(''), ''), + fh=(BytesIO(b''), ''), ), follow_redirects=True) app.get('/logout') @@ -543,15 +543,15 @@ def assertion(): def _helper_filenames_submit(app): app.post('/submit', data=dict( msg="This is a test.", - fh=(StringIO(''), ''), + fh=(BytesIO(b''), ''), ), follow_redirects=True) app.post('/submit', data=dict( msg="This is a test.", - fh=(StringIO('This is a test'), 'test.txt'), + fh=(BytesIO(b'This is a test'), 'test.txt'), ), follow_redirects=True) app.post('/submit', data=dict( msg="", - fh=(StringIO('This is a test'), 'test.txt'), + fh=(BytesIO(b'This is a test'), 'test.txt'), ), follow_redirects=True) From a7847d3caab1002694da2819268dd511993e50fc Mon Sep 17 00:00:00 2001 From: Kushal Das Date: Thu, 21 Mar 2019 21:29:10 +0530 Subject: [PATCH 14/47] Uses binascii.Error as the actual error in exception --- securedrop/journalist_app/utils.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/securedrop/journalist_app/utils.py b/securedrop/journalist_app/utils.py index 2024526861..fd46ef27bb 100644 --- a/securedrop/journalist_app/utils.py +++ b/securedrop/journalist_app/utils.py @@ -1,5 +1,6 @@ # -*- coding: utf-8 -*- +import binascii from datetime import datetime from flask import (g, flash, current_app, abort, send_file, redirect, url_for, render_template, Markup, sessions, request) @@ -121,7 +122,7 @@ def validate_hotp_secret(user, otp_secret): """ try: user.set_hotp_secret(otp_secret) - except TypeError as e: + except binascii.Error as e: if "Non-hexadecimal digit found" in str(e): flash(gettext( "Invalid secret format: " From 7fbefcb6813c3f96cbcb0297a8bd1b870ec3f891 Mon Sep 17 00:00:00 2001 From: Kushal Das Date: Thu, 21 Mar 2019 21:30:22 +0530 Subject: [PATCH 15/47] Fixes bytes vs strings for Python3 --- securedrop/tests/test_journalist.py | 38 ++++++++++++++--------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/securedrop/tests/test_journalist.py b/securedrop/tests/test_journalist.py index 8bb6618492..88dc28862c 100644 --- a/securedrop/tests/test_journalist.py +++ b/securedrop/tests/test_journalist.py @@ -6,9 +6,9 @@ import zipfile import base64 import datetime +import binascii from base64 import b64decode -from io import StringIO from io import BytesIO from flask import url_for, escape, session, current_app, g from mock import patch @@ -767,7 +767,7 @@ def test_admin_resets_user_hotp_error(mocker, test_admin['otp_secret']) mocker.patch('models.Journalist.set_hotp_secret', - side_effect=TypeError(error_message)) + side_effect=binascii.Error(error_message)) with InstrumentedApp(journalist_app) as ins: app.post(url_for('admin.reset_two_factor_hotp'), @@ -866,7 +866,7 @@ def test_user_resets_user_hotp_error(mocker, test_journo['otp_secret']) mocker.patch('models.Journalist.set_hotp_secret', - side_effect=TypeError(error_message)) + side_effect=binascii.Error(error_message)) with InstrumentedApp(journalist_app) as ins: app.post(url_for('account.reset_two_factor_hotp'), @@ -1145,7 +1145,7 @@ def test_admin_add_user_integrity_error(journalist_app, test_admin, mocker): "error") log_event = mocked_error_logger.call_args[0][0] - assert ("Adding user 'username' failed: (__builtin__.NoneType) " + assert ("Adding user 'username' failed: (builtins.NoneType) " "None [SQL: 'STATEMENT'] [parameters: 'PARAMETERS']") in log_event @@ -1163,8 +1163,8 @@ def test_logo_upload_with_valid_image_succeeds(journalist_app, test_admin): # Create 1px * 1px 'white' PNG file from its base64 string form = journalist_app_module.forms.LogoForm( logo=(BytesIO(base64.decodestring - ("iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQ" - "VR42mP8/x8AAwMCAO+ip1sAAAAASUVORK5CYII=")), 'test.png') + (b"iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQ" + b"VR42mP8/x8AAwMCAO+ip1sAAAAASUVORK5CYII=")), 'test.png') ) with InstrumentedApp(journalist_app) as ins: app.post(url_for('admin.manage_config'), @@ -1184,7 +1184,7 @@ def test_logo_upload_with_invalid_filetype_fails(journalist_app, test_admin): test_admin['otp_secret']) form = journalist_app_module.forms.LogoForm( - logo=(StringIO('filedata'), 'bad.exe') + logo=(BytesIO(b'filedata'), 'bad.exe') ) with InstrumentedApp(journalist_app) as ins: resp = app.post(url_for('admin.manage_config'), @@ -1214,7 +1214,7 @@ def test_logo_upload_with_empty_input_field_fails(journalist_app, test_admin): test_admin['otp_secret']) form = journalist_app_module.forms.LogoForm( - logo=(StringIO(''), '') + logo=(BytesIO(b''), '') ) with InstrumentedApp(journalist_app) as ins: @@ -1662,12 +1662,12 @@ def test_download_selected_submissions_from_source(journalist_app, # The download request was succesful, and the app returned a zipfile assert resp.status_code == 200 assert resp.content_type == 'application/zip' - assert zipfile.is_zipfile(StringIO(resp.data)) + assert zipfile.is_zipfile(BytesIO(resp.data)) # The submissions selected are in the zipfile for filename in selected_fnames: # Check that the expected filename is in the zip file - zipinfo = zipfile.ZipFile(StringIO(resp.data)).getinfo( + zipinfo = zipfile.ZipFile(BytesIO(resp.data)).getinfo( os.path.join( source.journalist_filename, "%s_%s" % (filename.split('-')[0], @@ -1684,7 +1684,7 @@ def test_download_selected_submissions_from_source(journalist_app, for filename in not_selected_fnames: with pytest.raises(KeyError): - zipfile.ZipFile(StringIO(resp.data)).getinfo( + zipfile.ZipFile(BytesIO(resp.data)).getinfo( os.path.join( source.journalist_filename, source.journalist_designation, @@ -1741,11 +1741,11 @@ def test_download_unread_all_sources(journalist_app, test_journo): # The download request was succesful, and the app returned a zipfile assert resp.status_code == 200 assert resp.content_type == 'application/zip' - assert zipfile.is_zipfile(StringIO(resp.data)) + assert zipfile.is_zipfile(BytesIO(resp.data)) # All the not dowloaded submissions are in the zipfile for submission in bulk['not_downloaded0']: - zipinfo = zipfile.ZipFile(StringIO(resp.data)).getinfo( + zipinfo = zipfile.ZipFile(BytesIO(resp.data)).getinfo( os.path.join( "unread", bulk['source0'].journalist_designation, @@ -1756,7 +1756,7 @@ def test_download_unread_all_sources(journalist_app, test_journo): assert zipinfo for submission in bulk['not_downloaded1']: - zipinfo = zipfile.ZipFile(StringIO(resp.data)).getinfo( + zipinfo = zipfile.ZipFile(BytesIO(resp.data)).getinfo( os.path.join( "unread", bulk['source1'].journalist_designation, @@ -1769,7 +1769,7 @@ def test_download_unread_all_sources(journalist_app, test_journo): # All the downloaded submissions are absent from the zipfile for submission in bulk['downloaded0']: with pytest.raises(KeyError): - zipfile.ZipFile(StringIO(resp.data)).getinfo( + zipfile.ZipFile(BytesIO(resp.data)).getinfo( os.path.join( "unread", bulk['source0'].journalist_designation, @@ -1780,7 +1780,7 @@ def test_download_unread_all_sources(journalist_app, test_journo): for submission in bulk['downloaded1']: with pytest.raises(KeyError): - zipfile.ZipFile(StringIO(resp.data)).getinfo( + zipfile.ZipFile(BytesIO(resp.data)).getinfo( os.path.join( "unread", bulk['source1'].journalist_designation, @@ -1811,11 +1811,11 @@ def test_download_all_selected_sources(journalist_app, test_journo): # The download request was succesful, and the app returned a zipfile assert resp.status_code == 200 assert resp.content_type == 'application/zip' - assert zipfile.is_zipfile(StringIO(resp.data)) + assert zipfile.is_zipfile(BytesIO(resp.data)) # All messages from source1 are in the zipfile for submission in bulk['submissions1']: - zipinfo = zipfile.ZipFile(StringIO(resp.data)).getinfo( + zipinfo = zipfile.ZipFile(BytesIO(resp.data)).getinfo( os.path.join( "all", bulk['source1'].journalist_designation, @@ -1828,7 +1828,7 @@ def test_download_all_selected_sources(journalist_app, test_journo): # All messages from source0 are absent from the zipfile for submission in bulk['submissions0']: with pytest.raises(KeyError): - zipfile.ZipFile(StringIO(resp.data)).getinfo( + zipfile.ZipFile(BytesIO(resp.data)).getinfo( os.path.join( "all", bulk['source0'].journalist_designation, From 053901798082d4e510c200d785d063a070e514b9 Mon Sep 17 00:00:00 2001 From: Kushal Das Date: Thu, 21 Mar 2019 22:45:27 +0530 Subject: [PATCH 16/47] Fixes encrypted reply file opening as text file The encrypted data is already encoded to ascii, so we can just save it with `open(filename,'w')` file. --- securedrop/store.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/securedrop/store.py b/securedrop/store.py index 924f8d5ed3..7acdbbe745 100644 --- a/securedrop/store.py +++ b/securedrop/store.py @@ -162,7 +162,7 @@ def save_pre_encrypted_reply(self, filesystem_id, count, journalist_filename) encrypted_file_path = self.path(filesystem_id, encrypted_file_name) - with open(encrypted_file_path, 'wb') as fh: + with open(encrypted_file_path, 'w') as fh: fh.write(content) return encrypted_file_path From 64308a2f4806c7016720827d00de0ec69eef3b3f Mon Sep 17 00:00:00 2001 From: Kushal Das Date: Thu, 21 Mar 2019 22:47:02 +0530 Subject: [PATCH 17/47] Fixes bytes vs strings for Python3 --- securedrop/journalist_app/api.py | 6 +-- securedrop/tests/conftest.py | 2 +- securedrop/tests/test_journalist_api.py | 57 +++++++++++++------------ 3 files changed, 34 insertions(+), 31 deletions(-) diff --git a/securedrop/journalist_app/api.py b/securedrop/journalist_app/api.py index 0d2de87604..8ffef0c704 100644 --- a/securedrop/journalist_app/api.py +++ b/securedrop/journalist_app/api.py @@ -82,13 +82,13 @@ def validate_data(): # other requests must have valid JSON payload else: try: - json.loads(request.data) + json.loads(request.data.decode('utf-8')) except (ValueError): return abort(400, 'malformed request') @api.route('/token', methods=['POST']) def get_token(): - creds = json.loads(request.data) + creds = json.loads(request.data.decode('utf-8')) username = creds.get('username', None) passphrase = creds.get('passphrase', None) @@ -232,7 +232,7 @@ def all_source_replies(source_uuid): user = get_user_object(request) - data = json.loads(request.data) + data = json.loads(request.data.decode('utf-8')) if not data['reply']: abort(400, 'reply should not be empty') diff --git a/securedrop/tests/conftest.py b/securedrop/tests/conftest.py index c7574d0490..d1b6a70dfe 100644 --- a/securedrop/tests/conftest.py +++ b/securedrop/tests/conftest.py @@ -224,7 +224,7 @@ def journalist_api_token(journalist_app, test_journo): 'passphrase': test_journo['password'], 'one_time_code': valid_token}), headers=utils.api_helper.get_api_headers()) - observed_response = json.loads(response.data) + observed_response = json.loads(response.data.decode('utf-8')) return observed_response['token'] diff --git a/securedrop/tests/test_journalist_api.py b/securedrop/tests/test_journalist_api.py index f8222528a8..0527b05770 100644 --- a/securedrop/tests/test_journalist_api.py +++ b/securedrop/tests/test_journalist_api.py @@ -23,11 +23,14 @@ def test_unauthenticated_user_gets_all_endpoints(journalist_app): with journalist_app.test_client() as app: response = app.get(url_for('api.get_endpoints')) - observed_endpoints = json.loads(response.data) + observed_endpoints = json.loads(response.data.decode('utf-8')) expected_endpoints = ['current_user_url', 'submissions_url', 'sources_url', 'auth_token_url', 'replies_url'] - assert expected_endpoints == list(observed_endpoints.keys()) + expected_endpoints.sort() + sorted_observed_endpoints = list(observed_endpoints.keys()) + sorted_observed_endpoints.sort() + assert expected_endpoints == sorted_observed_endpoints def test_valid_user_can_get_an_api_token(journalist_app, test_journo): @@ -39,7 +42,7 @@ def test_valid_user_can_get_an_api_token(journalist_app, test_journo): 'passphrase': test_journo['password'], 'one_time_code': valid_token}), headers=get_api_headers()) - observed_response = json.loads(response.data) + observed_response = json.loads(response.data.decode('utf-8')) assert observed_response['journalist_uuid'] == test_journo['uuid'] assert isinstance(Journalist.validate_api_token_and_get_user( @@ -57,7 +60,7 @@ def test_user_cannot_get_an_api_token_with_wrong_password(journalist_app, 'passphrase': 'wrong password', 'one_time_code': valid_token}), headers=get_api_headers()) - observed_response = json.loads(response.data) + observed_response = json.loads(response.data.decode('utf-8')) assert response.status_code == 403 assert observed_response['error'] == 'Forbidden' @@ -73,7 +76,7 @@ def test_user_cannot_get_an_api_token_with_wrong_2fa_token(journalist_app, 'passphrase': test_journo['password'], 'one_time_code': '123456'}), headers=get_api_headers()) - observed_response = json.loads(response.data) + observed_response = json.loads(response.data.decode('utf-8')) assert response.status_code == 403 assert observed_response['error'] == 'Forbidden' @@ -88,7 +91,7 @@ def test_user_cannot_get_an_api_token_with_no_passphase_field(journalist_app, {'username': test_journo['username'], 'one_time_code': valid_token}), headers=get_api_headers()) - observed_response = json.loads(response.data) + observed_response = json.loads(response.data.decode('utf-8')) assert response.status_code == 400 assert observed_response['error'] == 'Bad Request' @@ -104,7 +107,7 @@ def test_user_cannot_get_an_api_token_with_no_username_field(journalist_app, {'passphrase': test_journo['password'], 'one_time_code': valid_token}), headers=get_api_headers()) - observed_response = json.loads(response.data) + observed_response = json.loads(response.data.decode('utf-8')) assert response.status_code == 400 assert observed_response['error'] == 'Bad Request' @@ -119,7 +122,7 @@ def test_user_cannot_get_an_api_token_with_no_otp_field(journalist_app, {'username': test_journo['username'], 'passphrase': test_journo['password']}), headers=get_api_headers()) - observed_response = json.loads(response.data) + observed_response = json.loads(response.data.decode('utf-8')) assert response.status_code == 400 assert observed_response['error'] == 'Bad Request' @@ -132,7 +135,7 @@ def test_authorized_user_gets_all_sources(journalist_app, test_submissions, response = app.get(url_for('api.get_all_sources'), headers=get_api_headers(journalist_api_token)) - data = json.loads(response.data) + data = json.loads(response.data.decode('utf-8')) assert response.status_code == 200 @@ -257,7 +260,7 @@ def test_api_error_handler_404(journalist_app, journalist_api_token): with journalist_app.test_client() as app: response = app.get('/api/v1/invalidendpoint', headers=get_api_headers(journalist_api_token)) - json_response = json.loads(response.data) + json_response = json.loads(response.data.decode('utf-8')) assert response.status_code == 404 assert json_response['error'] == 'Not Found' @@ -270,7 +273,7 @@ def test_trailing_slash_cleanly_404s(journalist_app, test_source, response = app.get(url_for('api.single_source', source_uuid=uuid) + '/', headers=get_api_headers(journalist_api_token)) - json_response = json.loads(response.data) + json_response = json.loads(response.data.decode('utf-8')) assert response.status_code == 404 assert json_response['error'] == 'Not Found' @@ -285,7 +288,7 @@ def test_authorized_user_gets_single_source(journalist_app, test_source, assert response.status_code == 200 - data = json.loads(response.data) + data = json.loads(response.data.decode('utf-8')) assert data['uuid'] == test_source['source'].uuid assert 'BEGIN PGP PUBLIC KEY' in data['key']['public'] @@ -329,7 +332,7 @@ def test_authorized_user_can_star_a_source(journalist_app, test_source, # API should also report is_starred is true response = app.get(url_for('api.single_source', source_uuid=uuid), headers=get_api_headers(journalist_api_token)) - json_response = json.loads(response.data) + json_response = json.loads(response.data.decode('utf-8')) assert json_response['is_starred'] is True @@ -353,7 +356,7 @@ def test_authorized_user_can_unstar_a_source(journalist_app, test_source, # API should also report is_starred is false response = app.get(url_for('api.single_source', source_uuid=uuid), headers=get_api_headers(journalist_api_token)) - json_response = json.loads(response.data) + json_response = json.loads(response.data.decode('utf-8')) assert json_response['is_starred'] is False @@ -363,7 +366,7 @@ def test_disallowed_methods_produces_405(journalist_app, test_source, uuid = test_source['source'].uuid response = app.delete(url_for('api.add_star', source_uuid=uuid), headers=get_api_headers(journalist_api_token)) - json_response = json.loads(response.data) + json_response = json.loads(response.data.decode('utf-8')) assert response.status_code == 405 assert json_response['error'] == 'Method Not Allowed' @@ -377,7 +380,7 @@ def test_authorized_user_can_get_all_submissions(journalist_app, headers=get_api_headers(journalist_api_token)) assert response.status_code == 200 - json_response = json.loads(response.data) + json_response = json.loads(response.data.decode('utf-8')) observed_submissions = [submission['filename'] for submission in json_response['submissions']] @@ -397,7 +400,7 @@ def test_authorized_user_get_source_submissions(journalist_app, headers=get_api_headers(journalist_api_token)) assert response.status_code == 200 - json_response = json.loads(response.data) + json_response = json.loads(response.data.decode('utf-8')) observed_submissions = [submission['filename'] for submission in json_response['submissions']] @@ -420,7 +423,7 @@ def test_authorized_user_can_get_single_submission(journalist_app, assert response.status_code == 200 - json_response = json.loads(response.data) + json_response = json.loads(response.data.decode('utf-8')) assert json_response['uuid'] == submission_uuid assert json_response['is_read'] is False @@ -437,7 +440,7 @@ def test_authorized_user_can_get_all_replies(journalist_app, test_files, headers=get_api_headers(journalist_api_token)) assert response.status_code == 200 - json_response = json.loads(response.data) + json_response = json.loads(response.data.decode('utf-8')) observed_replies = [reply['filename'] for reply in json_response['replies']] @@ -456,7 +459,7 @@ def test_authorized_user_get_source_replies(journalist_app, test_files, headers=get_api_headers(journalist_api_token)) assert response.status_code == 200 - json_response = json.loads(response.data) + json_response = json.loads(response.data.decode('utf-8')) observed_replies = [reply['filename'] for reply in json_response['replies']] @@ -478,7 +481,7 @@ def test_authorized_user_can_get_single_reply(journalist_app, test_files, assert response.status_code == 200 - json_response = json.loads(response.data) + json_response = json.loads(response.data.decode('utf-8')) reply = Reply.query.filter(Reply.uuid == reply_uuid).one() @@ -598,7 +601,7 @@ def test_authorized_user_can_get_current_user_endpoint(journalist_app, headers=get_api_headers(journalist_api_token)) assert response.status_code == 200 - json_response = json.loads(response.data) + json_response = json.loads(response.data.decode('utf-8')) assert json_response['is_admin'] is False assert json_response['username'] == test_journo['username'] assert json_response['uuid'] == test_journo['journalist'].uuid @@ -652,7 +655,7 @@ def test_authorized_user_can_add_reply(journalist_app, journalist_api_token, response = app.post(url_for('api.all_source_replies', source_uuid=uuid), - data=json.dumps({'reply': reply_content}), + data=json.dumps({'reply': reply_content.decode('utf-8')}), headers=get_api_headers(journalist_api_token)) assert response.status_code == 201 @@ -730,7 +733,7 @@ def test_reply_with_valid_curly_json_400(journalist_app, journalist_api_token, headers=get_api_headers(journalist_api_token)) assert response.status_code == 400 - json_response = json.loads(response.data) + json_response = json.loads(response.data.decode('utf-8')) assert json_response['message'] == 'reply not found in request body' @@ -744,7 +747,7 @@ def test_reply_with_valid_square_json_400(journalist_app, journalist_api_token, headers=get_api_headers(journalist_api_token)) assert response.status_code == 400 - json_response = json.loads(response.data) + json_response = json.loads(response.data.decode('utf-8')) assert json_response['message'] == 'reply not found in request body' @@ -765,7 +768,7 @@ def test_malformed_json_400(journalist_app, journalist_api_token, test_journo, response = app.post(protected_route, data="{this is invalid {json!", headers=get_api_headers(journalist_api_token)) - observed_response = json.loads(response.data) + observed_response = json.loads(response.data.decode('utf-8')) assert response.status_code == 400 assert observed_response['error'] == 'Bad Request' @@ -786,7 +789,7 @@ def test_empty_json_400(journalist_app, journalist_api_token, test_journo, response = app.post(protected_route, data="", headers=get_api_headers(journalist_api_token)) - observed_response = json.loads(response.data) + observed_response = json.loads(response.data.decode('utf-8')) assert response.status_code == 400 assert observed_response['error'] == 'Bad Request' From 3f0750064cbcc5d3fc3806ede46ccf20b52c562a Mon Sep 17 00:00:00 2001 From: Kushal Das Date: Thu, 21 Mar 2019 23:06:59 +0530 Subject: [PATCH 18/47] Python3 stdout takes care of the QR code --- securedrop/manage.py | 1 - 1 file changed, 1 deletion(-) diff --git a/securedrop/manage.py b/securedrop/manage.py index 448653772a..0f166233e1 100755 --- a/securedrop/manage.py +++ b/securedrop/manage.py @@ -192,7 +192,6 @@ def _add_user(is_admin=False): issuer_name='SecureDrop') qr = qrcode.QRCode() qr.add_data(uri) - sys.stdout = codecs.getwriter("utf-8")(sys.stdout) qr.print_ascii(tty=sys.stdout.isatty()) print(('\nIf the barcode does not render correctly, try ' "changing your terminal's font (Monospace for Linux, " From fefd614db5e68ea6bab706c2c2789ae3f64de53f Mon Sep 17 00:00:00 2001 From: Kushal Das Date: Thu, 21 Mar 2019 23:07:13 +0530 Subject: [PATCH 19/47] Fixes tests by mocking builtins.input function This is the correct input method name. --- securedrop/tests/test_manage.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/securedrop/tests/test_manage.py b/securedrop/tests/test_manage.py index ccbfb3b1a9..54e4937472 100644 --- a/securedrop/tests/test_manage.py +++ b/securedrop/tests/test_manage.py @@ -42,24 +42,24 @@ def test_verbose(caplog): def test_get_username_success(): - with mock.patch("__builtin__.raw_input", return_value='jen'): + with mock.patch("builtins.input", return_value='jen'): assert manage._get_username() == 'jen' def test_get_username_fail(): bad_username = 'a' * (Journalist.MIN_USERNAME_LEN - 1) - with mock.patch("__builtin__.raw_input", + with mock.patch("builtins.input", side_effect=[bad_username, 'jen']): assert manage._get_username() == 'jen' def test_get_yubikey_usage_yes(): - with mock.patch("__builtin__.raw_input", return_value='y'): + with mock.patch("builtins.input", return_value='y'): assert manage._get_yubikey_usage() def test_get_yubikey_usage_no(): - with mock.patch("__builtin__.raw_input", return_value='n'): + with mock.patch("builtins.input", return_value='n'): assert not manage._get_yubikey_usage() @@ -69,7 +69,7 @@ def test_handle_invalid_secret(journalist_app, config, mocker): mocker.patch("manage._get_username", return_value='ntoll'), mocker.patch("manage._get_yubikey_usage", return_value=True), - mocker.patch("__builtin__.raw_input", side_effect=YUBIKEY_HOTP), + mocker.patch("builtins.input", side_effect=YUBIKEY_HOTP), mocker.patch("sys.stdout", new_callable=StringIO), original_config = manage.config @@ -161,7 +161,7 @@ def test_delete_non_existent_user(journalist_app, config, mocker): def test_get_username_to_delete(mocker): - mocker.patch("__builtin__.raw_input", return_value='test-user-12345') + mocker.patch("builtins.input", return_value='test-user-12345') return_value = manage._get_username_to_delete() assert return_value == 'test-user-12345' @@ -192,7 +192,7 @@ def test_reset(journalist_app, test_journo, alembic_config, config): def test_get_username(mocker): - mocker.patch("__builtin__.raw_input", return_value='foo-bar-baz') + mocker.patch("builtins.input", return_value='foo-bar-baz') assert manage._get_username() == 'foo-bar-baz' From 8c91695f32331339bbf9996973e5158035183152 Mon Sep 17 00:00:00 2001 From: Kushal Das Date: Thu, 21 Mar 2019 23:18:28 +0530 Subject: [PATCH 20/47] Uses bytes on Python3 --- securedrop/qa_loader.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/securedrop/qa_loader.py b/securedrop/qa_loader.py index 0567ac1efa..88a8fb1ea7 100755 --- a/securedrop/qa_loader.py +++ b/securedrop/qa_loader.py @@ -1,4 +1,4 @@ -#!/usr/bin/env python +#!/usr/bin/env python3 # -*- coding: utf-8 -*- import math @@ -86,8 +86,8 @@ def new_journalist(self): if random_bool(): # to add legacy passwords back in journalist.passphrase_hash = None - journalist.pw_salt = random_chars(32, nullable=False) - journalist.pw_hash = random_chars(64, nullable=False) + journalist.pw_salt = random_chars(32, nullable=False).encode('utf-8') + journalist.pw_hash = random_chars(64, nullable=False).encode('utf-8') journalist.is_admin = bool_or_none() From d152ea7480422208e072839b9ab0348f9f653e54 Mon Sep 17 00:00:00 2001 From: Kushal Das Date: Thu, 21 Mar 2019 23:24:49 +0530 Subject: [PATCH 21/47] Fixes bytes vs strings for Python3 --- securedrop/tests/test_secure_tempfile.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/securedrop/tests/test_secure_tempfile.py b/securedrop/tests/test_secure_tempfile.py index f5c2077021..50df0956e1 100644 --- a/securedrop/tests/test_secure_tempfile.py +++ b/securedrop/tests/test_secure_tempfile.py @@ -21,21 +21,21 @@ def test_read_before_writing(): def test_write_then_read_once(): f = SecureTemporaryFile('/tmp') f.write(MESSAGE) - assert f.read() == MESSAGE + assert f.read().decode('utf-8') == MESSAGE def test_write_twice_then_read_once(): f = SecureTemporaryFile('/tmp') f.write(MESSAGE) f.write(MESSAGE) - assert f.read() == MESSAGE * 2 + assert f.read().decode('utf-8') == MESSAGE * 2 def test_write_then_read_twice(): f = SecureTemporaryFile('/tmp') f.write(MESSAGE) - assert f.read() == MESSAGE - assert f.read() == '' + assert f.read().decode('utf-8') == MESSAGE + assert f.read() == b'' def test_write_then_read_then_write(): @@ -90,7 +90,7 @@ def test_buffered_read(): f = SecureTemporaryFile('/tmp') msg = MESSAGE * 1000 f.write(msg) - out = '' + out = b'' while True: chars = f.read(1024) if chars: @@ -98,7 +98,7 @@ def test_buffered_read(): else: break - assert out == msg + assert out.decode('utf-8') == msg def test_tmp_file_id_omits_invalid_chars(): From 15e443f4e064b8a20af212f4047d23aa22e1753b Mon Sep 17 00:00:00 2001 From: Kushal Das Date: Thu, 21 Mar 2019 23:27:13 +0530 Subject: [PATCH 22/47] Fixes import path for Python3 --- securedrop/tests/test_template_filters.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/securedrop/tests/test_template_filters.py b/securedrop/tests/test_template_filters.py index dcb1706ff1..69e7d07ba9 100644 --- a/securedrop/tests/test_template_filters.py +++ b/securedrop/tests/test_template_filters.py @@ -5,7 +5,7 @@ from flask import session os.environ['SECUREDROP_ENV'] = 'test' # noqa -from . import i18n +import i18n import i18n_tool import journalist_app import source_app From 2b50b0b481cdb43989c4e2d77b3ce8051cde2ff8 Mon Sep 17 00:00:00 2001 From: Kushal Das Date: Thu, 21 Mar 2019 23:44:50 +0530 Subject: [PATCH 23/47] Fixes bytes vs strings for Python3 --- securedrop/tests/test_source.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/securedrop/tests/test_source.py b/securedrop/tests/test_source.py index 26448b647d..bc004997b4 100644 --- a/securedrop/tests/test_source.py +++ b/securedrop/tests/test_source.py @@ -5,7 +5,7 @@ import re import subprocess -from io import StringIO +from io import StringIO, BytesIO from flask import session, escape, current_app, url_for, g from mock import patch, ANY @@ -96,7 +96,7 @@ def test_generate(source_app): text = resp.data.decode('utf-8') assert "This codename is what you will use in future visits" in text - codename = _find_codename(resp.data) + codename = _find_codename(resp.data.decode('utf-8')) assert len(codename.split()) == Source.NUM_WORDS # codename is also stored in the session - make sure it matches the # codename displayed to the source @@ -275,7 +275,7 @@ def _dummy_submission(app): return app.post( url_for('main.submit'), data=dict(msg="Pay no attention to the man behind the curtain.", - fh=(StringIO(''), '')), + fh=(BytesIO(b''), '')), follow_redirects=True) @@ -343,7 +343,7 @@ def test_submit_file(source_app): _dummy_submission(app) resp = app.post( url_for('main.submit'), - data=dict(msg="", fh=(StringIO('This is a test'), 'test.txt')), + data=dict(msg="", fh=(BytesIO(b'This is a test'), 'test.txt')), follow_redirects=True) assert resp.status_code == 200 text = resp.data.decode('utf-8') @@ -358,7 +358,7 @@ def test_submit_both(source_app): url_for('main.submit'), data=dict( msg="This is a test", - fh=(StringIO('This is a test'), 'test.txt')), + fh=(BytesIO(b'This is a test'), 'test.txt')), follow_redirects=True) assert resp.status_code == 200 text = resp.data.decode('utf-8') @@ -482,7 +482,7 @@ def test_submit_sanitizes_filename(source_app): url_for('main.submit'), data=dict( msg="", - fh=(StringIO('This is a test'), insecure_filename)), + fh=(BytesIO(b'This is a test'), insecure_filename)), follow_redirects=True) assert resp.status_code == 200 gzipfile.assert_called_with(filename=sanitized_filename, From 7eb87be08ed0d9e701ceb39f4678fc825a0df721 Mon Sep 17 00:00:00 2001 From: Kushal Das Date: Fri, 22 Mar 2019 00:09:10 +0530 Subject: [PATCH 24/47] Fixes bytes vs strings for Python3 --- securedrop/tests/functional/journalist_navigation_steps.py | 3 +++ securedrop/tests/functional/source_navigation_steps.py | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/securedrop/tests/functional/journalist_navigation_steps.py b/securedrop/tests/functional/journalist_navigation_steps.py index c512bb17e2..c594a8d3f8 100644 --- a/securedrop/tests/functional/journalist_navigation_steps.py +++ b/securedrop/tests/functional/journalist_navigation_steps.py @@ -593,6 +593,9 @@ def cookie_string_from_selenium_cookies(cookies): decrypted_submission = self.gpg.decrypt(raw_content) submission = self._get_submission_content(file_url, decrypted_submission) + if type(submission) == bytes: + submission = submission.decode('utf-8') + assert self.secret_message == submission def _journalist_composes_reply(self): diff --git a/securedrop/tests/functional/source_navigation_steps.py b/securedrop/tests/functional/source_navigation_steps.py index 78945671ed..d8a46f8738 100644 --- a/securedrop/tests/functional/source_navigation_steps.py +++ b/securedrop/tests/functional/source_navigation_steps.py @@ -139,7 +139,7 @@ def _source_continues_to_submit_page(self): def _source_submits_a_file(self): with tempfile.NamedTemporaryFile() as file: - file.write(self.secret_message) + file.write(self.secret_message.encode('utf-8')) file.seek(0) filename = file.name From fc1dd67386a066b4ba83cee8eb467c061bf0221b Mon Sep 17 00:00:00 2001 From: redshiftzero Date: Fri, 22 Mar 2019 12:42:21 -0700 Subject: [PATCH 25/47] dockerfiles: add python2 and python3 dockerfiles --- .../dockerfiles/xenial/python2/Dockerfile | 39 +++++++++++++++++++ .../xenial/{ => python3}/Dockerfile | 0 2 files changed, 39 insertions(+) create mode 100644 securedrop/dockerfiles/xenial/python2/Dockerfile rename securedrop/dockerfiles/xenial/{ => python3}/Dockerfile (100%) diff --git a/securedrop/dockerfiles/xenial/python2/Dockerfile b/securedrop/dockerfiles/xenial/python2/Dockerfile new file mode 100644 index 0000000000..56ae80a0d0 --- /dev/null +++ b/securedrop/dockerfiles/xenial/python2/Dockerfile @@ -0,0 +1,39 @@ +# ubuntu 16.04 image - 2019-01-22 +FROM ubuntu@sha256:e4a134999bea4abb4a27bc437e6118fdddfb172e1b9d683129b74d254af51675 +ARG USER_NAME +ENV USER_NAME ${USER_NAME:-root} +ARG USER_ID +ENV USER_ID ${USER_ID:-0} + +# If running grsecurity kernel on the host, Memprotect must be disabled on mono-sgen in the container +RUN apt-get update && \ + apt-get install -y paxctl && \ + { apt-get install -y libgtk2.0 || echo 'libgtk2.0 was not installed'; } && \ + paxctl -cm /usr/bin/mono-sgen && dpkg-reconfigure mono-runtime-sgen + +RUN apt-get install -y devscripts vim \ + python-pip libpython2.7-dev libssl-dev secure-delete \ + gnupg2 ruby redis-server firefox git xvfb haveged curl \ + gettext paxctl x11vnc enchant libffi-dev sqlite3 gettext sudo + +ENV FIREFOX_CHECKSUM=88d25053306d33658580973b063cd459a56e3596a3a298c1fb8ab1d52171d860 +RUN curl -LO https://launchpad.net/~ubuntu-mozilla-security/+archive/ubuntu/ppa/+build/9727836/+files/firefox_46.0.1+build1-0ubuntu0.14.04.3_amd64.deb && \ + shasum -a 256 firefox*deb && \ + echo "${FIREFOX_CHECKSUM} firefox_46.0.1+build1-0ubuntu0.14.04.3_amd64.deb" | shasum -a 256 -c - && \ + dpkg -i firefox*deb && apt-get install -f && \ + paxctl -cm /usr/lib/firefox/firefox + +RUN gem install sass -v 3.4.23 + +COPY requirements requirements +RUN pip install -r requirements/securedrop-app-code-requirements.txt && \ + pip install -r requirements/test-requirements.txt + +# Fixes #4036 pybabel requires latest version of setuptools +RUN pip install --upgrade setuptools + +RUN if test $USER_NAME != root ; then useradd --no-create-home --home-dir /tmp --uid $USER_ID $USER_NAME && echo "$USER_NAME ALL=(ALL) NOPASSWD:ALL" >> /etc/sudoers ; fi + +STOPSIGNAL SIGKILL + +EXPOSE 8080 8081 5901 diff --git a/securedrop/dockerfiles/xenial/Dockerfile b/securedrop/dockerfiles/xenial/python3/Dockerfile similarity index 100% rename from securedrop/dockerfiles/xenial/Dockerfile rename to securedrop/dockerfiles/xenial/python3/Dockerfile From ac92d2adf0e1093765522ffd45975fd8940307bf Mon Sep 17 00:00:00 2001 From: redshiftzero Date: Fri, 22 Mar 2019 12:55:02 -0700 Subject: [PATCH 26/47] ci: add application test run on python 3 (xenial only) --- .circleci/config.yml | 81 +++++++++++++++++++++++++++++++++++++++----- 1 file changed, 72 insertions(+), 9 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index b43c81cbbb..454a745a1e 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -13,7 +13,7 @@ common-steps: - &restorecache restore_cache: - key: v1-sd-layers-{{ checksum "securedrop/dockerfiles/xenial/Dockerfile" }} + key: v1-sd-layers-{{ checksum "securedrop/dockerfiles/xenial/python2/Dockerfile" }} paths: - /caches/layers.tar.gz @@ -42,7 +42,7 @@ common-steps: - &savecache save_cache: - key: v1-sd-layers-{{ checksum "securedrop/dockerfiles/xenial/Dockerfile" }} + key: v1-sd-layers-{{ checksum "securedrop/dockerfiles/xenial/python2/Dockerfile" }} paths: - /caches/layers.tar @@ -66,14 +66,10 @@ jobs: steps: - checkout - *rebaseontarget - - - run: - name: Ensure cache dir exists and permissions are good - command: | - sudo mkdir -p /caches && sudo chown circleci: -R /caches + - *createcachedir - restore_cache: - key: v1-sd-layers-{{ checksum "securedrop/dockerfiles/trusty/Dockerfile" }} + key: v1-sd-layers-{{ checksum "securedrop/dockerfiles/trusty/python2/Dockerfile" }} paths: - /caches/layers.tar.gz @@ -98,7 +94,7 @@ jobs: docker save -o /caches/layers.tar securedrop-test-trusty:latest - save_cache: - key: v1-sd-layers-{{ checksum "securedrop/dockerfiles/trusty/Dockerfile" }} + key: v1-sd-layers-{{ checksum "securedrop/dockerfiles/trusty/python2/Dockerfile" }} paths: - /caches/layers.tar @@ -155,6 +151,67 @@ jobs: - store_artifacts: path: ~/test-results + python3-app-tests: + machine: + enabled: true + environment: + DOCKER_API_VERSION: 1.23 + BASE_OS: xenial + PYTHON_VERSION: 3 + parallelism: 3 + steps: + - checkout + - *rebaseontarget + - *createcachedir + + - restore_cache: + key: v1-sd-layers-{{ checksum "securedrop/dockerfiles/xenial/python3/Dockerfile" }} + paths: + - /caches/layers.tar.gz + + - run: + name: Load image layer cache + command: | + set +o pipefail + docker load -i /caches/layers.tar |true + + - run: + name: Build Docker images + command: | + set +o pipefail + docker images + fromtag=$(docker images |grep securedrop-test-xenial-py3 |head -n1 |awk '{print $2}') + cd securedrop && DOCKER_BUILD_ARGUMENTS="--cache-from securedrop-test-xenial-py3:${fromtag:-latest}" ./bin/dev-shell true + + - run: + name: Save Docker image layer cache + command: | + docker images + docker save -o /caches/layers.tar securedrop-test-xenial-py3:latest + + - save_cache: + key: v1-sd-layers-{{ checksum "securedrop/dockerfiles/xenial/python3/Dockerfile" }} + paths: + - /caches/layers.tar + + - run: + name: Make test results directory + command: mkdir -p ~/test-results + + - run: + name: Run tests + command: | + export TESTFILES=$(cd securedrop; circleci tests glob 'tests/test*py' 'tests/**/test*py' |circleci tests split --split-by=timings |xargs echo) + docker rm -f securedrop-test-xenial-py3 || true + fromtag=$(docker images |grep securedrop-test-xenial-py3 |head -n1 |awk '{print $2}') + cd securedrop && DOCKER_BUILD_ARGUMENTS="--cache-from securedrop-test-xenial-py3:${fromtag:-latest}" make test + + - store_test_results: + path: ~/test-results + + - store_artifacts: + path: ~/test-results + translation-tests: machine: enabled: true @@ -284,6 +341,12 @@ workflows: ignore: - /docs-.*/ - /i18n-.*/ + - python3-app-tests: + filters: + branches: + ignore: + - /docs-.*/ + - /i18n-.*/ - admin-tests: filters: branches: From 5935641991ff9d0d74fc72a4fd77fa8f4e353965 Mon Sep 17 00:00:00 2001 From: redshiftzero Date: Fri, 22 Mar 2019 13:31:46 -0700 Subject: [PATCH 27/47] dockerfile: move trusty dockerfile into python2 dir --- securedrop/dockerfiles/trusty/{ => python2}/Dockerfile | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename securedrop/dockerfiles/trusty/{ => python2}/Dockerfile (100%) diff --git a/securedrop/dockerfiles/trusty/Dockerfile b/securedrop/dockerfiles/trusty/python2/Dockerfile similarity index 100% rename from securedrop/dockerfiles/trusty/Dockerfile rename to securedrop/dockerfiles/trusty/python2/Dockerfile From 945211d9f8b72257120c8510a69021cfff90cb62 Mon Sep 17 00:00:00 2001 From: redshiftzero Date: Fri, 22 Mar 2019 13:33:53 -0700 Subject: [PATCH 28/47] dev: add python 2 / 3 functionality to dev env --- securedrop/bin/dev-shell | 37 +++++++++++++++++++++++++++++-------- 1 file changed, 29 insertions(+), 8 deletions(-) diff --git a/securedrop/bin/dev-shell b/securedrop/bin/dev-shell index 8e61f2dde9..fac007b17e 100755 --- a/securedrop/bin/dev-shell +++ b/securedrop/bin/dev-shell @@ -14,6 +14,10 @@ if ! test -n "${BASE_OS:-}" ; then BASE_OS=xenial fi +if ! test -n "${PYTHON_VERSION:-}" ; then + PYTHON_VERSION=2 +fi + function exit_if_not_supported_base_image() { # Currently we only support Xenial or Trusty. if [[ "$1" != "xenial" && "$1" != "trusty" ]] @@ -23,20 +27,37 @@ function exit_if_not_supported_base_image() { fi } +function validate_python_version() { + # Trusty will be EOL April 2019. Orgs must be on Xenial to upgrade to Python 3. + if [[ "$1" != "xenial" && "$2" != "2" ]] + then + echo "For Ubuntu Trusty, PYTHON_VERSION must be 2" + exit 1 + fi + + if [[ "$2" != "2" && "$2" != "3" ]] + then + echo "PYTHON_VERSION must be 2 or 3" + exit 1 + fi +} + function docker_image() { exit_if_not_supported_base_image $1 + validate_python_version $1 $2 docker build \ ${DOCKER_BUILD_ARGUMENTS:-} \ - --build-arg=USER_ID="$(id -u)" \ - --build-arg=USER_NAME="${USER:-root}" \ - -t "securedrop-test-${1}" \ - --file "${TOPLEVEL}/securedrop/dockerfiles/${1}/Dockerfile" \ + --build-arg=USER_ID="$(id -u)" \ + --build-arg=USER_NAME="${USER:-root}" \ + -t "securedrop-test-${1}-py${2}" \ + --file "${TOPLEVEL}/securedrop/dockerfiles/${1}/python${2}/Dockerfile" \ "${TOPLEVEL}/securedrop" } function docker_run() { exit_if_not_supported_base_image $1 + validate_python_version $1 $2 find . \( -name '*.pyc' -o -name __pycache__ \) -delete docker run \ @@ -49,13 +70,13 @@ function docker_run() { -e LC_ALL=C.UTF-8 \ -e LANG=C.UTF-8 \ --name securedrop-dev \ - -ti ${DOCKER_RUN_ARGUMENTS:-} "securedrop-test-${1}" "${@:2}" + -ti ${DOCKER_RUN_ARGUMENTS:-} "securedrop-test-${1}-py${2}" "${@:3}" } if test -n "${CIRCLE_SHA1:-}" ; then - docker_image $BASE_OS + docker_image $BASE_OS $PYTHON_VERSION else - ticker docker_image $BASE_OS + ticker docker_image $BASE_OS $PYTHON_VERSION fi -docker_run $BASE_OS "$@" +docker_run $BASE_OS $PYTHON_VERSION "$@" From 8a8ca222c41f9fdd5d6d2422c6c25c03e65568b7 Mon Sep 17 00:00:00 2001 From: redshiftzero Date: Fri, 22 Mar 2019 15:34:12 -0700 Subject: [PATCH 29/47] dev: add makefile target for running test suite under python 3 --- securedrop/Makefile | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/securedrop/Makefile b/securedrop/Makefile index 27c8cada92..7c4f5d11f4 100644 --- a/securedrop/Makefile +++ b/securedrop/Makefile @@ -24,6 +24,10 @@ test: ## Run the test suite in a Ubuntu 16.04 (Xenial) dockerized environment test-trusty: ## Run the test suite in a Ubuntu 14.04 (Trusty) dockerized environment (to be removed April 30, 2019) BASE_OS=trusty ./bin/dev-shell ./bin/run-test -v $${TESTFILES:-tests} +.PHONY: test-python3 +test-python3: ## Run the Python 3 test suite in a Ubuntu 16.04 (Xenial) dockerized environment + PYTHON_VERSION=3 BASE_OS=xenial ./bin/dev-shell ./bin/run-test -v $${TESTFILES:-tests} + .PHONY: translation-test translation-test: ## Run all pages-layout tests in all supported languages ./bin/dev-shell ./bin/translation-test $${TESTFILES:-tests/pageslayout} From 80b5a93afbbf5181a41f5797b971f67c2b3ad19f Mon Sep 17 00:00:00 2001 From: redshiftzero Date: Fri, 22 Mar 2019 17:09:57 -0700 Subject: [PATCH 30/47] dev: remove ticker --- securedrop/bin/dev-shell | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/securedrop/bin/dev-shell b/securedrop/bin/dev-shell index fac007b17e..dba211a567 100755 --- a/securedrop/bin/dev-shell +++ b/securedrop/bin/dev-shell @@ -7,7 +7,6 @@ set -eu TOPLEVEL=$(git rev-parse --show-toplevel) -source "${BASH_SOURCE%/*}/../../devops/scripts/ticker" if ! test -n "${BASE_OS:-}" ; then # If no base OS was specified, then we use Xenial @@ -73,10 +72,5 @@ function docker_run() { -ti ${DOCKER_RUN_ARGUMENTS:-} "securedrop-test-${1}-py${2}" "${@:3}" } -if test -n "${CIRCLE_SHA1:-}" ; then - docker_image $BASE_OS $PYTHON_VERSION -else - ticker docker_image $BASE_OS $PYTHON_VERSION -fi - +docker_image $BASE_OS $PYTHON_VERSION docker_run $BASE_OS $PYTHON_VERSION "$@" From bf5460d1c77b7575e19d4ed35d481470e4728e29 Mon Sep 17 00:00:00 2001 From: redshiftzero Date: Mon, 8 Apr 2019 07:56:10 -0700 Subject: [PATCH 31/47] python 2/3 compatibility: revert when python 3 is ready for prod this commit reverts changes in python->python3, which can only be deployed when python3 changes are ready for prod. For now, we plan on having both python2 and python3 compatibility, and we will update the shebangs to point to /usr/bin/env python3 when we remove support for python2 and deploy python3 to prod (to be done in a single release). --- securedrop/Makefile | 2 +- securedrop/create-dev-data.py | 2 +- securedrop/dockerfiles/xenial/python3/Dockerfile | 3 +++ securedrop/i18n_tool.py | 2 +- securedrop/manage.py | 2 +- securedrop/management/run.py | 4 ++-- securedrop/qa_loader.py | 2 +- 7 files changed, 10 insertions(+), 7 deletions(-) diff --git a/securedrop/Makefile b/securedrop/Makefile index 7c4f5d11f4..2e6b80cae2 100644 --- a/securedrop/Makefile +++ b/securedrop/Makefile @@ -44,7 +44,7 @@ test-config: ## Generate the test config journalist_secret_key=$(shell head -c 32 /dev/urandom | base64) \ scrypt_id_pepper=$(shell head -c 32 /dev/urandom | base64) \ scrypt_gpg_pepper=$(shell head -c 32 /dev/urandom | base64) \ - python3 -c 'import os; from jinja2 import Environment, FileSystemLoader; \ + python -c 'import os; from jinja2 import Environment, FileSystemLoader; \ env = Environment(loader=FileSystemLoader(".")); \ ctx = {"securedrop_app_gpg_fingerprint": "65A1B5FF195B56353CC63DFFCC40EF1228271441"}; \ ctx.update(dict((k, {"stdout":v}) for k,v in os.environ.items())); \ diff --git a/securedrop/create-dev-data.py b/securedrop/create-dev-data.py index 15e5da936a..ab4e886710 100755 --- a/securedrop/create-dev-data.py +++ b/securedrop/create-dev-data.py @@ -1,4 +1,4 @@ -#!/usr/bin/env python3 +#!/usr/bin/env python # -*- coding: utf-8 -*- import datetime diff --git a/securedrop/dockerfiles/xenial/python3/Dockerfile b/securedrop/dockerfiles/xenial/python3/Dockerfile index 6ccbc54e77..72170c235c 100644 --- a/securedrop/dockerfiles/xenial/python3/Dockerfile +++ b/securedrop/dockerfiles/xenial/python3/Dockerfile @@ -33,6 +33,9 @@ RUN pip3 install -r requirements/securedrop-app-code-requirements.txt && \ # Fixes #4036 pybabel requires latest version of setuptools RUN pip3 install --upgrade setuptools +# Temporary workaround: Revert when python 3 is deployed to prod +RUN sudo rm /usr/bin/python && sudo ln -s /usr/bin/python3 /usr/bin/python + RUN if test $USER_NAME != root ; then useradd --no-create-home --home-dir /tmp --uid $USER_ID $USER_NAME && echo "$USER_NAME ALL=(ALL) NOPASSWD:ALL" >> /etc/sudoers ; fi STOPSIGNAL SIGKILL diff --git a/securedrop/i18n_tool.py b/securedrop/i18n_tool.py index 116d75d18b..043866a509 100755 --- a/securedrop/i18n_tool.py +++ b/securedrop/i18n_tool.py @@ -1,4 +1,4 @@ -#!/usr/bin/env python3 +#!/usr/bin/env python # -*- coding: utf-8 -*- import argparse diff --git a/securedrop/manage.py b/securedrop/manage.py index 0f166233e1..e34399e322 100755 --- a/securedrop/manage.py +++ b/securedrop/manage.py @@ -1,4 +1,4 @@ -#!/usr/bin/env python3 +#!/usr/bin/env python # -*- coding: utf-8 -*- import argparse diff --git a/securedrop/management/run.py b/securedrop/management/run.py index 16e4fbb018..fb28c88301 100644 --- a/securedrop/management/run.py +++ b/securedrop/management/run.py @@ -157,10 +157,10 @@ def run(args): # pragma: no cover procs = [ lambda: DevServerProcess('Source Interface', - ['python3', 'source.py'], + ['python', 'source.py'], 'blue'), lambda: DevServerProcess('Journalist Interface', - ['python3', 'journalist.py'], + ['python', 'journalist.py'], 'cyan'), lambda: DevServerProcess('SASS Compiler', ['sass', '--watch', 'sass:static/css'], diff --git a/securedrop/qa_loader.py b/securedrop/qa_loader.py index 88a8fb1ea7..b193345f12 100755 --- a/securedrop/qa_loader.py +++ b/securedrop/qa_loader.py @@ -1,4 +1,4 @@ -#!/usr/bin/env python3 +#!/usr/bin/env python # -*- coding: utf-8 -*- import math From 8e0aeb7d430a52fb708cc71e137effca3f67dc83 Mon Sep 17 00:00:00 2001 From: redshiftzero Date: Mon, 8 Apr 2019 08:01:50 -0700 Subject: [PATCH 32/47] python 2/3 compatibility: update remainder of imports --- securedrop/tests/conftest.py | 8 ++++++-- .../tests/functional/journalist_navigation_steps.py | 11 ++++++++--- securedrop/tests/utils/instrument.py | 2 +- 3 files changed, 15 insertions(+), 6 deletions(-) diff --git a/securedrop/tests/conftest.py b/securedrop/tests/conftest.py index d1b6a70dfe..f21877e78d 100644 --- a/securedrop/tests/conftest.py +++ b/securedrop/tests/conftest.py @@ -11,7 +11,11 @@ import signal import subprocess -from configparser import SafeConfigParser +try: + import configparser +except: + from six.moves import configparser # renamed in Python 3 + from flask import url_for from pyotp import TOTP @@ -117,7 +121,7 @@ def config(tmpdir): def alembic_config(config): base_dir = path.join(path.dirname(__file__), '..') migrations_dir = path.join(base_dir, 'alembic') - ini = SafeConfigParser() + ini = configparser.SafeConfigParser() ini.read(path.join(base_dir, 'alembic.ini')) ini.set('alembic', 'script_location', path.join(migrations_dir)) diff --git a/securedrop/tests/functional/journalist_navigation_steps.py b/securedrop/tests/functional/journalist_navigation_steps.py index c594a8d3f8..eb4e6d756e 100644 --- a/securedrop/tests/functional/journalist_navigation_steps.py +++ b/securedrop/tests/functional/journalist_navigation_steps.py @@ -1,5 +1,10 @@ import pytest -import urllib.request, urllib.error, urllib.parse + +try: + from urllib.request import urlopen, Request +except ImportError: # Python 2/3 compatibility + from urllib2 import urlopen, Request + import re import tempfile import gzip @@ -583,12 +588,12 @@ def cookie_string_from_selenium_cookies(cookies): cookie_strs.append(cookie_str) return ' '.join(cookie_strs) - submission_req = urllib.request.Request(file_url) + submission_req = Request(file_url) submission_req.add_header( 'Cookie', cookie_string_from_selenium_cookies( self.driver.get_cookies())) - raw_content = urllib.request.urlopen(submission_req).read() + raw_content = urlopen(submission_req).read() decrypted_submission = self.gpg.decrypt(raw_content) submission = self._get_submission_content(file_url, diff --git a/securedrop/tests/utils/instrument.py b/securedrop/tests/utils/instrument.py index fd4507f684..989f1c539a 100644 --- a/securedrop/tests/utils/instrument.py +++ b/securedrop/tests/utils/instrument.py @@ -13,7 +13,7 @@ from urllib.parse import urlparse, urljoin except ImportError: # Python 2 urlparse fallback - from urllib.parse import urlparse, urljoin + from urlparse import urlparse, urljoin import pytest From 41ef26069e0da1c0e7e24621dcc126d207f9d075 Mon Sep 17 00:00:00 2001 From: redshiftzero Date: Mon, 8 Apr 2019 09:25:31 -0700 Subject: [PATCH 33/47] python 2/3 compatibility: fix test_api_error_handlers_defined --- securedrop/tests/test_journalist_api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/securedrop/tests/test_journalist_api.py b/securedrop/tests/test_journalist_api.py index 0527b05770..ba90befbd4 100644 --- a/securedrop/tests/test_journalist_api.py +++ b/securedrop/tests/test_journalist_api.py @@ -253,7 +253,7 @@ def test_api_error_handlers_defined(journalist_app): result = journalist_app.error_handler_spec['api'][status_code] expected_error_handler = '_handle_api_http_exception' - assert result.values()[0].__name__ == expected_error_handler + assert list(result.values())[0].__name__ == expected_error_handler def test_api_error_handler_404(journalist_app, journalist_api_token): From 5335bb2e905e8cd411f0d38eb6f2e257115f94f3 Mon Sep 17 00:00:00 2001 From: redshiftzero Date: Mon, 8 Apr 2019 09:26:05 -0700 Subject: [PATCH 34/47] round of flake8 fixes --- securedrop/manage.py | 23 +++++++++---------- securedrop/tests/conftest.py | 2 +- .../functional/journalist_navigation_steps.py | 13 +++++++---- securedrop/tests/test_i18n_tool.py | 4 ++-- 4 files changed, 22 insertions(+), 20 deletions(-) diff --git a/securedrop/manage.py b/securedrop/manage.py index e34399e322..521f43978c 100755 --- a/securedrop/manage.py +++ b/securedrop/manage.py @@ -2,7 +2,6 @@ # -*- coding: utf-8 -*- import argparse -import codecs import datetime import logging import os @@ -123,7 +122,7 @@ def _get_yubikey_usage(): '''Function used to allow for test suite mocking''' while True: answer = input('Will this user be using a YubiKey [HOTP]? ' - '(y/N): ').lower().strip() + '(y/N): ').lower().strip() if answer in ('y', 'yes'): return True elif answer in ('', 'n', 'no'): @@ -161,8 +160,8 @@ def _add_user(is_admin=False): tmp_str = otp_secret.replace(" ", "") if len(tmp_str) != 40: print(("The length of the secret is not correct. " - "Expected 40 characters, but received {0}. " - "Try again.".format(len(tmp_str)))) + "Expected 40 characters, but received {0}. " + "Try again.".format(len(tmp_str)))) continue if otp_secret: break @@ -181,7 +180,7 @@ def _add_user(is_admin=False): else: exc_type, exc_value, exc_traceback = sys.exc_info() print((repr(traceback.format_exception(exc_type, exc_value, - exc_traceback)))) + exc_traceback)))) return 1 else: print(('User "{}" successfully added'.format(username))) @@ -194,12 +193,12 @@ def _add_user(is_admin=False): qr.add_data(uri) qr.print_ascii(tty=sys.stdout.isatty()) print(('\nIf the barcode does not render correctly, try ' - "changing your terminal's font (Monospace for Linux, " - 'Menlo for OS X). If you are using iTerm on Mac OS X, ' - 'you will need to change the "Non-ASCII Font", which ' - "is your profile\'s Text settings.\n\nCan't scan the " - 'barcode? Enter following shared secret manually:' - '\n{}\n'.format(user.formatted_otp_secret))) + "changing your terminal's font (Monospace for Linux, " + 'Menlo for OS X). If you are using iTerm on Mac OS X, ' + 'you will need to change the "Non-ASCII Font", which ' + "is your profile\'s Text settings.\n\nCan't scan the " + 'barcode? Enter following shared secret manually:' + '\n{}\n'.format(user.formatted_otp_secret))) return 0 @@ -209,7 +208,7 @@ def _get_username_to_delete(): def _get_delete_confirmation(user): confirmation = input('Are you sure you want to delete user ' - '"{}" (y/n)?'.format(user)) + '"{}" (y/n)?'.format(user)) if confirmation.lower() != 'y': print(('Confirmation not received: user "{}" was NOT ' 'deleted'.format(user))) diff --git a/securedrop/tests/conftest.py b/securedrop/tests/conftest.py index f21877e78d..52559b99d4 100644 --- a/securedrop/tests/conftest.py +++ b/securedrop/tests/conftest.py @@ -13,7 +13,7 @@ try: import configparser -except: +except ImportError: from six.moves import configparser # renamed in Python 3 from flask import url_for diff --git a/securedrop/tests/functional/journalist_navigation_steps.py b/securedrop/tests/functional/journalist_navigation_steps.py index eb4e6d756e..9aacf56fac 100644 --- a/securedrop/tests/functional/journalist_navigation_steps.py +++ b/securedrop/tests/functional/journalist_navigation_steps.py @@ -399,7 +399,8 @@ def _edit_account(self): def _edit_user(self, username): user = Journalist.query.filter_by(username=username).one() - new_user_edit_links = [el for el in self.driver.find_elements_by_tag_name('a') if el.get_attribute('data-username') == username] + new_user_edit_links = [el for el in self.driver.find_elements_by_tag_name('a') + if el.get_attribute('data-username') == username] assert 1 == len(new_user_edit_links) new_user_edit_links[0].click() # The header says "Edit user "username"". @@ -450,8 +451,9 @@ def _admin_can_edit_new_user(self): # Click the "edit user" link for the new user # self._edit_user(self.new_user['username']) - new_user_edit_links = [el for el in self.driver.find_elements_by_tag_name('a') if (el.get_attribute('data-username') == - self.new_user['username'])] + new_user_edit_links = [el for el in self.driver.find_elements_by_tag_name('a') + if (el.get_attribute('data-username') == + self.new_user['username'])] assert len(new_user_edit_links) == 1 new_user_edit_links[0].click() @@ -656,8 +658,9 @@ def _admin_visits_add_user(self): add_user_btn.click() def _admin_visits_edit_user(self): - new_user_edit_links = [el for el in self.driver.find_elements_by_tag_name('a') if (el.get_attribute('data-username') == - self.new_user['username'])] + new_user_edit_links = [el for el in self.driver.find_elements_by_tag_name('a') + if (el.get_attribute('data-username') == + self.new_user['username'])] assert len(new_user_edit_links) == 1 new_user_edit_links[0].click() diff --git a/securedrop/tests/test_i18n_tool.py b/securedrop/tests/test_i18n_tool.py index ac793b0cee..b18c365898 100644 --- a/securedrop/tests/test_i18n_tool.py +++ b/securedrop/tests/test_i18n_tool.py @@ -315,7 +315,7 @@ def r(): assert 'l10n: updated nl' not in r() assert 'l10n: updated de_DE' not in r() message = str(git('--no-pager', '-C', 'securedrop', 'show', - _cwd=d, _encoding='utf-8')) + _cwd=d, _encoding='utf-8')) assert "Loïc" in message # @@ -348,6 +348,6 @@ def r(): assert 'l10n: updated nl' in r() assert 'l10n: updated de_DE' not in r() message = str(git('--no-pager', '-C', 'securedrop', 'show', - _cwd=d)) + _cwd=d)) assert "Someone Else" in message assert "Loïc" not in message From 0af962c4a335d388b921b98b65887f11102f9a0a Mon Sep 17 00:00:00 2001 From: redshiftzero Date: Mon, 8 Apr 2019 12:31:54 -0700 Subject: [PATCH 35/47] python 2/3 compatibility: use input() from six (bandit will flag the use of input() in python 2 as a high severity security issue otherwise) Also adds a wrapper as a workaround for https://github.com/pytest-dev/pytest/issues/1598 --- securedrop/manage.py | 21 +++++++++++------ securedrop/tests/test_manage.py | 42 ++++++++++++++++----------------- 2 files changed, 34 insertions(+), 29 deletions(-) diff --git a/securedrop/manage.py b/securedrop/manage.py index 521f43978c..c5378fa1df 100755 --- a/securedrop/manage.py +++ b/securedrop/manage.py @@ -14,6 +14,7 @@ import time import traceback +from six.moves import input from contextlib import contextmanager from flask import current_app from sqlalchemy import create_engine @@ -32,6 +33,12 @@ log = logging.getLogger(__name__) +def obtain_input(text): + """Wrapper for testability as suggested in + https://github.com/pytest-dev/pytest/issues/1598#issuecomment-224761877""" + return input(text) + + def reset(args): """Clears the SecureDrop development applications' state, restoring them to the way they were immediately after running `setup_dev.sh`. This command: @@ -109,7 +116,7 @@ def add_journalist(args): def _get_username(): while True: - username = input('Username: ') + username = obtain_input('Username: ') try: Journalist.check_username_acceptable(username) except InvalidUsernameException as e: @@ -121,8 +128,8 @@ def _get_username(): def _get_yubikey_usage(): '''Function used to allow for test suite mocking''' while True: - answer = input('Will this user be using a YubiKey [HOTP]? ' - '(y/N): ').lower().strip() + answer = obtain_input('Will this user be using a YubiKey [HOTP]? ' + '(y/N): ').lower().strip() if answer in ('y', 'yes'): return True elif answer in ('', 'n', 'no'): @@ -153,7 +160,7 @@ def _add_user(is_admin=False): otp_secret = None if is_hotp: while True: - otp_secret = input( + otp_secret = obtain_input( "Please configure this user's YubiKey and enter the " "secret: ") if otp_secret: @@ -203,12 +210,12 @@ def _add_user(is_admin=False): def _get_username_to_delete(): - return input('Username to delete: ') + return obtain_input('Username to delete: ') def _get_delete_confirmation(user): - confirmation = input('Are you sure you want to delete user ' - '"{}" (y/n)?'.format(user)) + confirmation = obtain_input('Are you sure you want to delete user ' + '"{}" (y/n)?'.format(user)) if confirmation.lower() != 'y': print(('Confirmation not received: user "{}" was NOT ' 'deleted'.format(user))) diff --git a/securedrop/tests/test_manage.py b/securedrop/tests/test_manage.py index 54e4937472..ff1fbfb4d7 100644 --- a/securedrop/tests/test_manage.py +++ b/securedrop/tests/test_manage.py @@ -7,11 +7,8 @@ import os import manage import mock -import sys import time -from io import StringIO - os.environ['SECUREDROP_ENV'] = 'test' # noqa from models import Journalist, db @@ -42,35 +39,34 @@ def test_verbose(caplog): def test_get_username_success(): - with mock.patch("builtins.input", return_value='jen'): + with mock.patch("manage.obtain_input", return_value='jen'): assert manage._get_username() == 'jen' def test_get_username_fail(): bad_username = 'a' * (Journalist.MIN_USERNAME_LEN - 1) - with mock.patch("builtins.input", + with mock.patch("manage.obtain_input", side_effect=[bad_username, 'jen']): assert manage._get_username() == 'jen' def test_get_yubikey_usage_yes(): - with mock.patch("builtins.input", return_value='y'): + with mock.patch("manage.obtain_input", return_value='y'): assert manage._get_yubikey_usage() def test_get_yubikey_usage_no(): - with mock.patch("builtins.input", return_value='n'): + with mock.patch("manage.obtain_input", return_value='n'): assert not manage._get_yubikey_usage() # Note: we use the `journalist_app` fixture because it creates the DB -def test_handle_invalid_secret(journalist_app, config, mocker): +def test_handle_invalid_secret(journalist_app, config, mocker, capsys): """Regression test for bad secret logic in manage.py""" mocker.patch("manage._get_username", return_value='ntoll'), mocker.patch("manage._get_yubikey_usage", return_value=True), - mocker.patch("builtins.input", side_effect=YUBIKEY_HOTP), - mocker.patch("sys.stdout", new_callable=StringIO), + mocker.patch("manage.obtain_input", side_effect=YUBIKEY_HOTP), original_config = manage.config @@ -80,10 +76,11 @@ def test_handle_invalid_secret(journalist_app, config, mocker): # We will try to provide one invalid and one valid secret return_value = manage._add_user() + out, err = capsys.readouterr() assert return_value == 0 - assert 'Try again.' in sys.stdout.getvalue() - assert 'successfully added' in sys.stdout.getvalue() + assert 'Try again.' in out + assert 'successfully added' in out finally: manage.config = original_config @@ -91,12 +88,11 @@ def test_handle_invalid_secret(journalist_app, config, mocker): # Note: we use the `journalist_app` fixture because it creates the DB def test_exception_handling_when_duplicate_username(journalist_app, config, - mocker): + mocker, capsys): """Regression test for duplicate username logic in manage.py""" mocker.patch("manage._get_username", return_value='foo-bar-baz') mocker.patch("manage._get_yubikey_usage", return_value=False) - mocker.patch("sys.stdout", new_callable=StringIO) original_config = manage.config @@ -106,14 +102,16 @@ def test_exception_handling_when_duplicate_username(journalist_app, # Inserting the user for the first time should succeed return_value = manage._add_user() + out, err = capsys.readouterr() + assert return_value == 0 - assert 'successfully added' in sys.stdout.getvalue() + assert 'successfully added' in out # Inserting the user for a second time should fail return_value = manage._add_user() + out, err = capsys.readouterr() assert return_value == 1 - assert ('ERROR: That username is already taken!' in - sys.stdout.getvalue()) + assert 'ERROR: That username is already taken!' in out finally: manage.config = original_config @@ -142,11 +140,10 @@ def test_delete_user(journalist_app, config, mocker): # Note: we use the `journalist_app` fixture because it creates the DB -def test_delete_non_existent_user(journalist_app, config, mocker): +def test_delete_non_existent_user(journalist_app, config, mocker, capsys): mocker.patch("manage._get_username_to_delete", return_value='does-not-exist') mocker.patch('manage._get_delete_confirmation', return_value=True) - mocker.patch("sys.stdout", new_callable=StringIO) original_config = manage.config @@ -154,14 +151,15 @@ def test_delete_non_existent_user(journalist_app, config, mocker): # We need to override the config to point at the per-test DB manage.config = config return_value = manage.delete_user(args=None) + out, err = capsys.readouterr() assert return_value == 0 - assert 'ERROR: That user was not found!' in sys.stdout.getvalue() + assert 'ERROR: That user was not found!' in out finally: manage.config = original_config def test_get_username_to_delete(mocker): - mocker.patch("builtins.input", return_value='test-user-12345') + mocker.patch("manage.obtain_input", return_value='test-user-12345') return_value = manage._get_username_to_delete() assert return_value == 'test-user-12345' @@ -192,7 +190,7 @@ def test_reset(journalist_app, test_journo, alembic_config, config): def test_get_username(mocker): - mocker.patch("builtins.input", return_value='foo-bar-baz') + mocker.patch("manage.obtain_input", return_value='foo-bar-baz') assert manage._get_username() == 'foo-bar-baz' From f3855ae009e6763e6362356c4c0de7ac95db8c01 Mon Sep 17 00:00:00 2001 From: Kushal Das Date: Tue, 9 Apr 2019 21:36:20 +0530 Subject: [PATCH 36/47] Uses six to work on both Python2 and 3 --- securedrop/i18n_tool.py | 11 ++++++----- securedrop/tests/test_i18n_tool.py | 19 ++++++++++++++----- 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/securedrop/i18n_tool.py b/securedrop/i18n_tool.py index 043866a509..1b1bc085b0 100755 --- a/securedrop/i18n_tool.py +++ b/securedrop/i18n_tool.py @@ -3,6 +3,7 @@ import argparse import io +import six import logging import os import glob @@ -58,7 +59,7 @@ def file_is_modified(self, path): def ensure_i18n_remote(self, args): k = {'_cwd': args.root} - if b'i18n' not in git.remote(**k).stdout: + if six.b('i18n') not in git.remote(**k).stdout: git.remote.add('i18n', args.url, **k) git.fetch('i18n', **k) @@ -200,7 +201,7 @@ def require_git_email_name(git_dir): 'git -C {d} config --get user.email > /dev/null'.format( d=git_dir)) if subprocess.call(cmd, shell=True): # nosec - if 'docker' in io.open('/proc/1/cgroup').read(): + if six.u('docker') in io.open('/proc/1/cgroup').read(): log.error("remember ~/.gitconfig does not exist " "in the dev-shell Docker container, " "only .git/config does") @@ -208,7 +209,7 @@ def require_git_email_name(git_dir): return True def update_docs(self, args): - l10n_content = '.. GENERATED BY i18n_tool.py DO NOT EDIT:\n\n' + l10n_content = six.u('.. GENERATED BY i18n_tool.py DO NOT EDIT:\n\n') for (code, info) in sorted(I18NTool.SUPPORTED_LANGUAGES.items()): l10n_content += '* ' + info['name'] + ' (``' + code + '``)\n' includes = join(args.documentation_dir, 'includes') @@ -303,7 +304,7 @@ def upstream_commit(self, args, code): authors |= set(git_authors) current = git('-C', args.root, 'rev-parse', 'i18n/i18n').stdout info = I18NTool.SUPPORTED_LANGUAGES[code] - message = textwrap.dedent(""" + message = textwrap.dedent(six.u(""" l10n: updated {code} {name} localizers: {authors} @@ -314,7 +315,7 @@ def upstream_commit(self, args, code): name=info['name'], authors=", ".join(authors), code=code, - current=current)) + current=current))) git('-C', args.root, 'commit', '-m', message) def set_update_from_weblate_parser(self, subps): diff --git a/securedrop/tests/test_i18n_tool.py b/securedrop/tests/test_i18n_tool.py index b18c365898..e93f1c8fe1 100644 --- a/securedrop/tests/test_i18n_tool.py +++ b/securedrop/tests/test_i18n_tool.py @@ -2,6 +2,7 @@ import io import os +import six from os.path import abspath, dirname, exists, getmtime, join, realpath os.environ['SECUREDROP_ENV'] = 'test' # noqa import i18n_tool @@ -253,7 +254,7 @@ def test_update_from_weblate(self, tmpdir, caplog): k = {'_cwd': join(d, repo)} git.init(**k) git.config('user.email', 'you@example.com', **k) - git.config('user.name', 'Loïc Nordhøy', **k) + git.config('user.name', six.u('Loïc Nordhøy'), **k) touch('README.md', **k) git.add('README.md', **k) git.commit('-m', 'README', 'README.md', **k) @@ -314,9 +315,13 @@ def r(): ]) assert 'l10n: updated nl' not in r() assert 'l10n: updated de_DE' not in r() - message = str(git('--no-pager', '-C', 'securedrop', 'show', + if six.PY2: + message = unicode(git('--no-pager', '-C', 'securedrop', 'show', _cwd=d, _encoding='utf-8')) - assert "Loïc" in message + else: + message = str(git('--no-pager', '-C', 'securedrop', 'show', + _cwd=d, _encoding='utf-8')) + assert six.u("Loïc") in message # # an update is done to nl in weblate @@ -347,7 +352,11 @@ def r(): ]) assert 'l10n: updated nl' in r() assert 'l10n: updated de_DE' not in r() - message = str(git('--no-pager', '-C', 'securedrop', 'show', + if six.PY2: + message = unicode(git('--no-pager', '-C', 'securedrop', 'show', + _cwd=d)) + else: + message = str(git('--no-pager', '-C', 'securedrop', 'show', _cwd=d)) assert "Someone Else" in message - assert "Loïc" not in message + assert six.u("Loïc") not in message From a669aa442b415074cf0d9804c608b9d232ab5434 Mon Sep 17 00:00:00 2001 From: Kushal Das Date: Tue, 9 Apr 2019 22:26:42 +0530 Subject: [PATCH 37/47] Assert based on python2 or python3 --- securedrop/tests/test_journalist.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/securedrop/tests/test_journalist.py b/securedrop/tests/test_journalist.py index 88dc28862c..3d6d8ebb1b 100644 --- a/securedrop/tests/test_journalist.py +++ b/securedrop/tests/test_journalist.py @@ -2,6 +2,7 @@ import os import pytest import io +import six import random import zipfile import base64 @@ -1145,7 +1146,11 @@ def test_admin_add_user_integrity_error(journalist_app, test_admin, mocker): "error") log_event = mocked_error_logger.call_args[0][0] - assert ("Adding user 'username' failed: (builtins.NoneType) " + if six.PY2: + assert ("Adding user 'username' failed: (__builtin__.NoneType) " + "None [SQL: 'STATEMENT'] [parameters: 'PARAMETERS']") in log_event + else: + assert ("Adding user 'username' failed: (builtins.NoneType) " "None [SQL: 'STATEMENT'] [parameters: 'PARAMETERS']") in log_event From a9b75fdb3b9c5476cbc904b3a2fbe2f9ffe33a5d Mon Sep 17 00:00:00 2001 From: Kushal Das Date: Tue, 9 Apr 2019 22:27:34 +0530 Subject: [PATCH 38/47] Uses six for both Python2 and Python3 --- securedrop/crypto_util.py | 3 +++ securedrop/journalist_app/utils.py | 2 +- securedrop/secure_tempfile.py | 15 +++++++++++++-- securedrop/source_app/main.py | 6 +++++- securedrop/tests/test_integration.py | 20 +++++++++++++------- 5 files changed, 35 insertions(+), 11 deletions(-) diff --git a/securedrop/crypto_util.py b/securedrop/crypto_util.py index cb31fdac54..acf83fc867 100644 --- a/securedrop/crypto_util.py +++ b/securedrop/crypto_util.py @@ -5,6 +5,7 @@ import pretty_bad_protocol as gnupg import os import io +import six import scrypt import subprocess from random import SystemRandom @@ -259,6 +260,8 @@ def decrypt(self, secret, ciphertext): """ hashed_codename = self.hash_codename(secret, salt=self.scrypt_gpg_pepper) + if six.PY2: # Python2 + return self.gpg.decrypt(ciphertext, passphrase=hashed_codename).data return self.gpg.decrypt(ciphertext, passphrase=hashed_codename).data.decode('utf-8') diff --git a/securedrop/journalist_app/utils.py b/securedrop/journalist_app/utils.py index fd46ef27bb..08cdd69a97 100644 --- a/securedrop/journalist_app/utils.py +++ b/securedrop/journalist_app/utils.py @@ -122,7 +122,7 @@ def validate_hotp_secret(user, otp_secret): """ try: user.set_hotp_secret(otp_secret) - except binascii.Error as e: + except (binascii.Error,TypeError) as e: if "Non-hexadecimal digit found" in str(e): flash(gettext( "Invalid secret format: " diff --git a/securedrop/secure_tempfile.py b/securedrop/secure_tempfile.py index 93ccaf2968..b11e75ef6a 100644 --- a/securedrop/secure_tempfile.py +++ b/securedrop/secure_tempfile.py @@ -2,6 +2,7 @@ import base64 import os import io +import six from tempfile import _TemporaryFileWrapper from pretty_bad_protocol._util import _STREAMLIKE_TYPES @@ -47,7 +48,12 @@ def __init__(self, store_dir): """ self.last_action = 'init' self.create_key() - self.tmp_file_id = base64.urlsafe_b64encode(os.urandom(32)).decode('utf-8').strip('=') + self.temp_file_id = "" + if six.PY2: + self.tmp_file_id = base64.urlsafe_b64encode(os.urandom(32)).strip('=') + else: + self.tmp_file_id = base64.urlsafe_b64encode(os.urandom(32)).decode('utf-8').strip('=') + self.filepath = os.path.join(store_dir, '{}.aes'.format(self.tmp_file_id)) self.file = io.open(self.filepath, 'w+b') @@ -83,7 +89,12 @@ def write(self, data): raise AssertionError('You cannot write after reading!') self.last_action = 'write' - if isinstance(data, str): # noqa + # This is the old Python related code + if six.PY2: # noqa + if isinstance(data, unicode): + data = data.encode('utf-8') + elif isinstance(data, str): # noqa + # For Python 3 data = data.encode('utf-8') self.file.write(self.encryptor.update(data)) diff --git a/securedrop/source_app/main.py b/securedrop/source_app/main.py index 429ac4b8e0..fa125740a3 100644 --- a/securedrop/source_app/main.py +++ b/securedrop/source_app/main.py @@ -1,6 +1,7 @@ import operator import os import io +import six from datetime import datetime from flask import (Blueprint, render_template, flash, redirect, url_for, g, @@ -87,7 +88,10 @@ def lookup(): with io.open(reply_path, "rb") as f: contents = f.read() reply_obj = current_app.crypto_util.decrypt(g.codename, contents) - reply.decrypted = reply_obj + if six.PY2: # Python2 + reply.decrypted = reply_obj.decode('utf-8') + else: + reply.decrypted = reply_obj except UnicodeDecodeError: current_app.logger.error("Could not decode reply %s" % reply.filename) diff --git a/securedrop/tests/test_integration.py b/securedrop/tests/test_integration.py index cb722b5019..261f130e41 100644 --- a/securedrop/tests/test_integration.py +++ b/securedrop/tests/test_integration.py @@ -10,7 +10,10 @@ from base64 import b32encode from binascii import unhexlify from bs4 import BeautifulSoup -from io import BytesIO +import six +if six.PY2: + import cStringIO +from io import BytesIO, StringIO from flask import session, g, escape, current_app from pyotp import TOTP, HOTP @@ -132,7 +135,7 @@ def assertion(): def test_submit_file(source_app, journalist_app, test_journo): """When a source creates an account, test that a new entry appears in the journalist interface""" - test_file_contents = b"This is a test file." + test_file_contents = six.b("This is a test file.") test_filename = "test.txt" with source_app.test_client() as app: @@ -142,7 +145,7 @@ def test_submit_file(source_app, journalist_app, test_journo): # redirected to submission form resp = app.post('/submit', data=dict( msg="", - fh=(BytesIO(test_file_contents), test_filename), + fh=(six.BytesIO(test_file_contents), test_filename), ), follow_redirects=True) assert resp.status_code == 200 app.get('/logout') @@ -176,7 +179,10 @@ def test_submit_file(source_app, journalist_app, test_journo): decrypted_data = journalist_app.crypto_util.gpg.decrypt(resp.data) assert decrypted_data.ok - sio = BytesIO(decrypted_data.data) + if six.PY2: # Python2 + sio = cStringIO.StringIO(decrypted_data.data) + else: + sio = BytesIO(decrypted_data.data) with gzip.GzipFile(mode='rb', fileobj=sio) as gzip_file: unzipped_decrypted_data = gzip_file.read() mtime = gzip_file.mtime @@ -244,7 +250,7 @@ def _helper_test_reply(journalist_app, source_app, config, test_journo, # redirected to submission form resp = app.post('/submit', data=dict( msg=test_msg, - fh=(BytesIO(b''), ''), + fh=(six.BytesIO(six.b('')), ''), ), follow_redirects=True) assert resp.status_code == 200 assert not g.source.flagged @@ -322,7 +328,7 @@ def assertion(): ), follow_redirects=True) assert resp.status_code == 200 - zf = zipfile.ZipFile(BytesIO(resp.data), 'r') + zf = zipfile.ZipFile(six.BytesIO(resp.data), 'r') data = zf.read(zf.namelist()[0]) _can_decrypt_with_key(journalist_app, data) _can_decrypt_with_key( @@ -448,7 +454,7 @@ def test_unicode_reply_with_ansi_env(journalist_app, journalist_app.crypto_util.gpg._encoding = "ansi_x3.4_1968" source_app.crypto_util.gpg._encoding = "ansi_x3.4_1968" _helper_test_reply(journalist_app, source_app, config, test_journo, - "ᚠᛇᚻ᛫ᛒᛦᚦ᛫ᚠᚱᚩᚠᚢᚱ᛫ᚠᛁᚱᚪ᛫ᚷᛖᚻᚹᛦᛚᚳᚢᛗ", True) + six.u("ᚠᛇᚻ᛫ᛒᛦᚦ᛫ᚠᚱᚩᚠᚢᚱ᛫ᚠᛁᚱᚪ᛫ᚷᛖᚻᚹᛦᛚᚳᚢᛗ"), True) def test_delete_collection(mocker, source_app, journalist_app, test_journo): From 08c6301dbefe3e05346e3b99078169070c656b2d Mon Sep 17 00:00:00 2001 From: Kushal Das Date: Tue, 9 Apr 2019 22:51:47 +0530 Subject: [PATCH 39/47] Uses six for both Python2 and Python3 --- securedrop/tests/test_source.py | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/securedrop/tests/test_source.py b/securedrop/tests/test_source.py index bc004997b4..b029b673f9 100644 --- a/securedrop/tests/test_source.py +++ b/securedrop/tests/test_source.py @@ -4,8 +4,9 @@ import platform import re import subprocess +import six -from io import StringIO, BytesIO +from io import BytesIO from flask import session, escape, current_app, url_for, g from mock import patch, ANY @@ -274,7 +275,7 @@ def _dummy_submission(app): """ return app.post( url_for('main.submit'), - data=dict(msg="Pay no attention to the man behind the curtain.", + data=dict(msg=six.u("Pay no attention to the man behind the curtain."), fh=(BytesIO(b''), '')), follow_redirects=True) @@ -299,7 +300,7 @@ def test_submit_message(source_app): _dummy_submission(app) resp = app.post( url_for('main.submit'), - data=dict(msg="This is a test.", fh=(StringIO(''), '')), + data=dict(msg=six.u("This is a test."), fh=(six.StringIO(six.u('')), '')), follow_redirects=True) assert resp.status_code == 200 text = resp.data.decode('utf-8') @@ -311,7 +312,7 @@ def test_submit_empty_message(source_app): new_codename(app, session) resp = app.post( url_for('main.submit'), - data=dict(msg="", fh=(StringIO(''), '')), + data=dict(msg="", fh=(six.StringIO(six.u('')), '')), follow_redirects=True) assert resp.status_code == 200 text = resp.data.decode('utf-8') @@ -330,7 +331,7 @@ def test_submit_big_message(source_app): _dummy_submission(app) resp = app.post( url_for('main.submit'), - data=dict(msg="AA" * (1024 * 512), fh=(StringIO(''), '')), + data=dict(msg="AA" * (1024 * 512), fh=(six.StringIO(six.u('')), '')), follow_redirects=True) assert resp.status_code == 200 text = resp.data.decode('utf-8') @@ -376,7 +377,7 @@ def test_submit_message_with_low_entropy(source_app): _dummy_submission(app) resp = app.post( url_for('main.submit'), - data=dict(msg="This is a test.", fh=(StringIO(''), '')), + data=dict(msg="This is a test.", fh=(six.StringIO(six.u('')), '')), follow_redirects=True) assert resp.status_code == 200 assert not async_genkey.called @@ -393,7 +394,7 @@ def test_submit_message_with_enough_entropy(source_app): _dummy_submission(app) resp = app.post( url_for('main.submit'), - data=dict(msg="This is a test.", fh=(StringIO(''), '')), + data=dict(msg="This is a test.", fh=(six.StringIO(six.u('')), '')), follow_redirects=True) assert resp.status_code == 200 assert async_genkey.called @@ -569,7 +570,7 @@ def test_failed_normalize_timestamps_logs_warning(source_app): url_for('main.submit'), data=dict( msg="This is a test.", - fh=(StringIO(''), '')), + fh=(six.StringIO(six.u('')), '')), follow_redirects=True) assert resp.status_code == 200 text = resp.data.decode('utf-8') From 14f5cc7719189c380dfdc5c75224983b73a8db70 Mon Sep 17 00:00:00 2001 From: Kushal Das Date: Wed, 10 Apr 2019 15:02:19 +0530 Subject: [PATCH 40/47] Uses six for unicode in Python2 and Python3 --- securedrop/tests/test_secure_tempfile.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/securedrop/tests/test_secure_tempfile.py b/securedrop/tests/test_secure_tempfile.py index 50df0956e1..358aa1d1bf 100644 --- a/securedrop/tests/test_secure_tempfile.py +++ b/securedrop/tests/test_secure_tempfile.py @@ -1,6 +1,7 @@ # -*- coding: utf-8 -*- import io import os +import six import pytest from pretty_bad_protocol._util import _is_stream @@ -50,7 +51,7 @@ def test_write_then_read_then_write(): def test_read_write_unicode(): f = SecureTemporaryFile('/tmp') - unicode_msg = '鬼神 Kill Em All 1989' + unicode_msg = six.u('鬼神 Kill Em All 1989') f.write(unicode_msg) assert f.read().decode('utf-8') == unicode_msg From 45b635136b8b2e8012dba9e70063778fdb6baddb Mon Sep 17 00:00:00 2001 From: Kushal Das Date: Wed, 10 Apr 2019 15:03:07 +0530 Subject: [PATCH 41/47] Fixes code formatting for CI --- securedrop/journalist_app/utils.py | 2 +- securedrop/tests/test_i18n_tool.py | 8 ++++---- securedrop/tests/test_integration.py | 9 ++++----- securedrop/tests/test_journalist.py | 4 ++-- 4 files changed, 11 insertions(+), 12 deletions(-) diff --git a/securedrop/journalist_app/utils.py b/securedrop/journalist_app/utils.py index 08cdd69a97..f6f7e1a0ff 100644 --- a/securedrop/journalist_app/utils.py +++ b/securedrop/journalist_app/utils.py @@ -122,7 +122,7 @@ def validate_hotp_secret(user, otp_secret): """ try: user.set_hotp_secret(otp_secret) - except (binascii.Error,TypeError) as e: + except (binascii.Error, TypeError) as e: if "Non-hexadecimal digit found" in str(e): flash(gettext( "Invalid secret format: " diff --git a/securedrop/tests/test_i18n_tool.py b/securedrop/tests/test_i18n_tool.py index e93f1c8fe1..bd6dd4e549 100644 --- a/securedrop/tests/test_i18n_tool.py +++ b/securedrop/tests/test_i18n_tool.py @@ -317,10 +317,10 @@ def r(): assert 'l10n: updated de_DE' not in r() if six.PY2: message = unicode(git('--no-pager', '-C', 'securedrop', 'show', - _cwd=d, _encoding='utf-8')) + _cwd=d, _encoding='utf-8')) else: message = str(git('--no-pager', '-C', 'securedrop', 'show', - _cwd=d, _encoding='utf-8')) + _cwd=d, _encoding='utf-8')) assert six.u("Loïc") in message # @@ -354,9 +354,9 @@ def r(): assert 'l10n: updated de_DE' not in r() if six.PY2: message = unicode(git('--no-pager', '-C', 'securedrop', 'show', - _cwd=d)) + _cwd=d)) else: message = str(git('--no-pager', '-C', 'securedrop', 'show', - _cwd=d)) + _cwd=d)) assert "Someone Else" in message assert six.u("Loïc") not in message diff --git a/securedrop/tests/test_integration.py b/securedrop/tests/test_integration.py index 261f130e41..4fbc632101 100644 --- a/securedrop/tests/test_integration.py +++ b/securedrop/tests/test_integration.py @@ -6,14 +6,13 @@ import random import re import zipfile - +import six +if six.PY2: # noqa + import cStringIO from base64 import b32encode from binascii import unhexlify from bs4 import BeautifulSoup -import six -if six.PY2: - import cStringIO -from io import BytesIO, StringIO +from io import BytesIO from flask import session, g, escape, current_app from pyotp import TOTP, HOTP diff --git a/securedrop/tests/test_journalist.py b/securedrop/tests/test_journalist.py index 3d6d8ebb1b..324ada9bae 100644 --- a/securedrop/tests/test_journalist.py +++ b/securedrop/tests/test_journalist.py @@ -1148,10 +1148,10 @@ def test_admin_add_user_integrity_error(journalist_app, test_admin, mocker): log_event = mocked_error_logger.call_args[0][0] if six.PY2: assert ("Adding user 'username' failed: (__builtin__.NoneType) " - "None [SQL: 'STATEMENT'] [parameters: 'PARAMETERS']") in log_event + "None [SQL: 'STATEMENT'] [parameters: 'PARAMETERS']") in log_event else: assert ("Adding user 'username' failed: (builtins.NoneType) " - "None [SQL: 'STATEMENT'] [parameters: 'PARAMETERS']") in log_event + "None [SQL: 'STATEMENT'] [parameters: 'PARAMETERS']") in log_event def test_logo_upload_with_valid_image_succeeds(journalist_app, test_admin): From 2030779fac18ea95c806ed3a3314aa2d5914578f Mon Sep 17 00:00:00 2001 From: Kushal Das Date: Wed, 10 Apr 2019 19:53:03 +0530 Subject: [PATCH 42/47] Updates code branching for Python2 vs Python3 Based on code review and easy code reading of the branches. --- securedrop/crypto_util.py | 8 +++++--- securedrop/secure_tempfile.py | 8 +++++--- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/securedrop/crypto_util.py b/securedrop/crypto_util.py index acf83fc867..a91c385c67 100644 --- a/securedrop/crypto_util.py +++ b/securedrop/crypto_util.py @@ -260,9 +260,11 @@ def decrypt(self, secret, ciphertext): """ hashed_codename = self.hash_codename(secret, salt=self.scrypt_gpg_pepper) - if six.PY2: # Python2 - return self.gpg.decrypt(ciphertext, passphrase=hashed_codename).data - return self.gpg.decrypt(ciphertext, passphrase=hashed_codename).data.decode('utf-8') + data = self.gpg.decrypt(ciphertext, passphrase=hashed_codename).data + + if not six.PY2: # Python3 + return data.decode('utf-8') + return data def clean(s, also=''): diff --git a/securedrop/secure_tempfile.py b/securedrop/secure_tempfile.py index b11e75ef6a..959180a26d 100644 --- a/securedrop/secure_tempfile.py +++ b/securedrop/secure_tempfile.py @@ -49,10 +49,12 @@ def __init__(self, store_dir): self.last_action = 'init' self.create_key() self.temp_file_id = "" - if six.PY2: - self.tmp_file_id = base64.urlsafe_b64encode(os.urandom(32)).strip('=') + data = base64.urlsafe_b64encode(os.urandom(32)) + + if not six.PY2: # For Python3 + self.tmp_file_id = data.decode('utf-8').strip('=') else: - self.tmp_file_id = base64.urlsafe_b64encode(os.urandom(32)).decode('utf-8').strip('=') + self.tmp_file_id = data.strip('=') self.filepath = os.path.join(store_dir, '{}.aes'.format(self.tmp_file_id)) From 9e8b6b1d12763b38261e2887cb603673135670b9 Mon Sep 17 00:00:00 2001 From: redshiftzero Date: Thu, 11 Apr 2019 17:30:59 -0700 Subject: [PATCH 43/47] python 2/3 compatibility: use six.text_type where possible --- securedrop/tests/test_i18n_tool.py | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/securedrop/tests/test_i18n_tool.py b/securedrop/tests/test_i18n_tool.py index bd6dd4e549..e0245b690d 100644 --- a/securedrop/tests/test_i18n_tool.py +++ b/securedrop/tests/test_i18n_tool.py @@ -315,12 +315,8 @@ def r(): ]) assert 'l10n: updated nl' not in r() assert 'l10n: updated de_DE' not in r() - if six.PY2: - message = unicode(git('--no-pager', '-C', 'securedrop', 'show', - _cwd=d, _encoding='utf-8')) - else: - message = str(git('--no-pager', '-C', 'securedrop', 'show', - _cwd=d, _encoding='utf-8')) + message = six.text_type(git('--no-pager', '-C', 'securedrop', 'show', + _cwd=d, _encoding='utf-8')) assert six.u("Loïc") in message # @@ -352,11 +348,7 @@ def r(): ]) assert 'l10n: updated nl' in r() assert 'l10n: updated de_DE' not in r() - if six.PY2: - message = unicode(git('--no-pager', '-C', 'securedrop', 'show', - _cwd=d)) - else: - message = str(git('--no-pager', '-C', 'securedrop', 'show', - _cwd=d)) + message = six.text_type(git('--no-pager', '-C', 'securedrop', 'show', + _cwd=d)) assert "Someone Else" in message assert six.u("Loïc") not in message From f39195f2bb291667454bcc1b25a1e1fd54853ae8 Mon Sep 17 00:00:00 2001 From: redshiftzero Date: Thu, 11 Apr 2019 17:52:43 -0700 Subject: [PATCH 44/47] python 2/3 compatibility: use six.BytesIO in test_submit_file --- securedrop/tests/test_integration.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/securedrop/tests/test_integration.py b/securedrop/tests/test_integration.py index 4fbc632101..12940ebde9 100644 --- a/securedrop/tests/test_integration.py +++ b/securedrop/tests/test_integration.py @@ -7,8 +7,7 @@ import re import zipfile import six -if six.PY2: # noqa - import cStringIO + from base64 import b32encode from binascii import unhexlify from bs4 import BeautifulSoup @@ -178,10 +177,7 @@ def test_submit_file(source_app, journalist_app, test_journo): decrypted_data = journalist_app.crypto_util.gpg.decrypt(resp.data) assert decrypted_data.ok - if six.PY2: # Python2 - sio = cStringIO.StringIO(decrypted_data.data) - else: - sio = BytesIO(decrypted_data.data) + sio = six.BytesIO(decrypted_data.data) with gzip.GzipFile(mode='rb', fileobj=sio) as gzip_file: unzipped_decrypted_data = gzip_file.read() mtime = gzip_file.mtime From 3606e48fd5fe9cc4c3dacc92a9bc07187499fe3e Mon Sep 17 00:00:00 2001 From: redshiftzero Date: Mon, 15 Apr 2019 17:40:28 -0700 Subject: [PATCH 45/47] docs: explain how to use dev env and run tests on either py2 or 3 --- docs/development/setup_development.rst | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/docs/development/setup_development.rst b/docs/development/setup_development.rst index ee5754956a..636251e23c 100644 --- a/docs/development/setup_development.rst +++ b/docs/development/setup_development.rst @@ -106,6 +106,16 @@ To get started, you can try the following: bin/dev-shell bin/run-test tests/functional # functional tests only bin/dev-shell bash # shell inside the container +To specify the version of Python you want to use, set the ``PYTHON_VERSION`` +environmental variable, like so: + +.. code:: sh + + PYTHON_VERSION=3 make test # Run tests on Python 3 + PYTHON_VERSION=2 make test # Run tests on Python 2 + PYTHON_VERSION=3 make dev # Run dev container on Python 3 + PYTHON_VERSION=2 make dev # Run dev container on Python 2 + .. tip:: The interactive shell in the container does not run ``redis``, ``Xvfb`` etc. However you can import shell helper functions with ``source bin/dev-deps`` and call ``run_xvfb``, From 3d3e0d876a871d6bcf5601e2171280011486315f Mon Sep 17 00:00:00 2001 From: redshiftzero Date: Mon, 15 Apr 2019 18:20:07 -0700 Subject: [PATCH 46/47] python 2/3 compatibility: cleaning up nits before merge --- .../testinfra/staging/app/test_apparmor.py | 4 +- securedrop/create-dev-data.py | 7 +- .../dockerfiles/xenial/python2/Dockerfile | 4 +- securedrop/i18n_tool.py | 4 +- securedrop/journalist_app/api.py | 2 +- securedrop/manage.py | 36 +++--- securedrop/secure_tempfile.py | 7 +- securedrop/tests/conftest.py | 3 +- .../functional/journalist_navigation_steps.py | 2 +- securedrop/tests/i18n/code.py | 2 +- securedrop/tests/test_journalist_api.py | 112 +++++++----------- securedrop/tests/test_source.py | 7 +- 12 files changed, 76 insertions(+), 114 deletions(-) diff --git a/molecule/testinfra/staging/app/test_apparmor.py b/molecule/testinfra/staging/app/test_apparmor.py index d3e236ed5d..d6c91e65c6 100644 --- a/molecule/testinfra/staging/app/test_apparmor.py +++ b/molecule/testinfra/staging/app/test_apparmor.py @@ -105,8 +105,8 @@ def test_apparmor_total_profiles(host): """ Ensure number of total profiles is sum of enforced and complaining profiles """ with host.sudo(): - total_expected = str((len(sdvars.apparmor_enforce) - + len(sdvars.apparmor_complain))) + total_expected = str(len(sdvars.apparmor_enforce) + + len(sdvars.apparmor_complain)) # Trusty has ~10, Xenial about ~20 profiles, so let's expect # *at least* the sum. assert host.check_output("aa-status --profiled") >= total_expected diff --git a/securedrop/create-dev-data.py b/securedrop/create-dev-data.py index ab4e886710..2e4d753c63 100755 --- a/securedrop/create-dev-data.py +++ b/securedrop/create-dev-data.py @@ -102,10 +102,9 @@ def create_source_and_submissions(num_submissions=2, num_replies=2): db.session.commit() - print(("Test source (codename: '{}', journalist designation '{}') " - "added with {} submissions and {} replies") - .format(codename, journalist_designation, num_submissions, - num_replies)) + print("Test source (codename: '{}', journalist designation '{}') " + "added with {} submissions and {} replies").format( + codename, journalist_designation, num_submissions, num_replies) if __name__ == "__main__": # pragma: no cover diff --git a/securedrop/dockerfiles/xenial/python2/Dockerfile b/securedrop/dockerfiles/xenial/python2/Dockerfile index 56ae80a0d0..f17d7bab21 100644 --- a/securedrop/dockerfiles/xenial/python2/Dockerfile +++ b/securedrop/dockerfiles/xenial/python2/Dockerfile @@ -1,5 +1,5 @@ -# ubuntu 16.04 image - 2019-01-22 -FROM ubuntu@sha256:e4a134999bea4abb4a27bc437e6118fdddfb172e1b9d683129b74d254af51675 +# ubuntu 16.04 image from 2019-03-12 +FROM ubuntu@sha256:58d0da8bc2f434983c6ca4713b08be00ff5586eb5cdff47bcde4b2e88fd40f88 ARG USER_NAME ENV USER_NAME ${USER_NAME:-root} ARG USER_ID diff --git a/securedrop/i18n_tool.py b/securedrop/i18n_tool.py index 1b1bc085b0..8c982ebc3b 100755 --- a/securedrop/i18n_tool.py +++ b/securedrop/i18n_tool.py @@ -285,8 +285,8 @@ def add(p): def upstream_commit(self, args, code): self.require_git_email_name(args.root) authors = set() - diffs = str(git('--no-pager', '-C', args.root, - 'diff', '--name-only', '--cached').stdout) + diffs = six.text_type(git('--no-pager', '-C', args.root, + 'diff', '--name-only', '--cached').stdout) for path in diffs.strip().split('\n'): previous_message = str(git( '--no-pager', '-C', args.root, 'log', '-n', '1', path, diff --git a/securedrop/journalist_app/api.py b/securedrop/journalist_app/api.py index 8ffef0c704..1db02d1958 100644 --- a/securedrop/journalist_app/api.py +++ b/securedrop/journalist_app/api.py @@ -232,7 +232,7 @@ def all_source_replies(source_uuid): user = get_user_object(request) - data = json.loads(request.data.decode('utf-8')) + data = request.json if not data['reply']: abort(400, 'reply should not be empty') diff --git a/securedrop/manage.py b/securedrop/manage.py index c5378fa1df..97ccda2a70 100755 --- a/securedrop/manage.py +++ b/securedrop/manage.py @@ -120,7 +120,7 @@ def _get_username(): try: Journalist.check_username_acceptable(username) except InvalidUsernameException as e: - print(('Invalid username: ' + str(e))) + print('Invalid username: ' + str(e)) else: return username @@ -154,7 +154,7 @@ def _add_user(is_admin=False): print("Note: Passwords are now autogenerated.") password = _make_password() - print(("This user's password is: {}".format(password))) + print("This user's password is: {}".format(password)) is_hotp = _get_yubikey_usage() otp_secret = None @@ -166,9 +166,9 @@ def _add_user(is_admin=False): if otp_secret: tmp_str = otp_secret.replace(" ", "") if len(tmp_str) != 40: - print(("The length of the secret is not correct. " - "Expected 40 characters, but received {0}. " - "Try again.".format(len(tmp_str)))) + print("The length of the secret is not correct. " + "Expected 40 characters, but received {0}. " + "Try again.".format(len(tmp_str))) continue if otp_secret: break @@ -186,11 +186,11 @@ def _add_user(is_admin=False): print('ERROR: That username is already taken!') else: exc_type, exc_value, exc_traceback = sys.exc_info() - print((repr(traceback.format_exception(exc_type, exc_value, - exc_traceback)))) + print(repr(traceback.format_exception(exc_type, exc_value, + exc_traceback))) return 1 else: - print(('User "{}" successfully added'.format(username))) + print('User "{}" successfully added'.format(username)) if not otp_secret: # Print the QR code for FreeOTP print('\nScan the QR code below with FreeOTP:\n') @@ -199,13 +199,13 @@ def _add_user(is_admin=False): qr = qrcode.QRCode() qr.add_data(uri) qr.print_ascii(tty=sys.stdout.isatty()) - print(('\nIf the barcode does not render correctly, try ' - "changing your terminal's font (Monospace for Linux, " - 'Menlo for OS X). If you are using iTerm on Mac OS X, ' - 'you will need to change the "Non-ASCII Font", which ' - "is your profile\'s Text settings.\n\nCan't scan the " - 'barcode? Enter following shared secret manually:' - '\n{}\n'.format(user.formatted_otp_secret))) + print('\nIf the barcode does not render correctly, try ' + "changing your terminal's font (Monospace for Linux, " + 'Menlo for OS X). If you are using iTerm on Mac OS X, ' + 'you will need to change the "Non-ASCII Font", which ' + "is your profile\'s Text settings.\n\nCan't scan the " + 'barcode? Enter following shared secret manually:' + '\n{}\n'.format(user.formatted_otp_secret)) return 0 @@ -217,8 +217,8 @@ def _get_delete_confirmation(user): confirmation = obtain_input('Are you sure you want to delete user ' '"{}" (y/n)?'.format(user)) if confirmation.lower() != 'y': - print(('Confirmation not received: user "{}" was NOT ' - 'deleted'.format(user))) + print('Confirmation not received: user "{}" was NOT ' + 'deleted'.format(user)) return False return True @@ -253,7 +253,7 @@ def delete_user(args): else: raise e - print(('User "{}" successfully deleted'.format(username))) + print('User "{}" successfully deleted'.format(username)) return 0 diff --git a/securedrop/secure_tempfile.py b/securedrop/secure_tempfile.py index 959180a26d..89890974d6 100644 --- a/securedrop/secure_tempfile.py +++ b/securedrop/secure_tempfile.py @@ -48,9 +48,8 @@ def __init__(self, store_dir): """ self.last_action = 'init' self.create_key() - self.temp_file_id = "" - data = base64.urlsafe_b64encode(os.urandom(32)) + data = base64.urlsafe_b64encode(os.urandom(32)) if not six.PY2: # For Python3 self.tmp_file_id = data.decode('utf-8').strip('=') else: @@ -69,8 +68,8 @@ def create_key(self): grsecurity-patched kernel it uses (for further details consult https://github.com/freedomofpress/securedrop/pull/477#issuecomment-168445450). """ - self.key = os.urandom(int(self.AES_key_size / 8)) - self.iv = os.urandom(int(self.AES_block_size / 8)) + self.key = os.urandom(self.AES_key_size // 8) + self.iv = os.urandom(self.AES_block_size // 8) self.initialize_cipher() def initialize_cipher(self): diff --git a/securedrop/tests/conftest.py b/securedrop/tests/conftest.py index 52559b99d4..1cb8dd3dd0 100644 --- a/securedrop/tests/conftest.py +++ b/securedrop/tests/conftest.py @@ -228,8 +228,7 @@ def journalist_api_token(journalist_app, test_journo): 'passphrase': test_journo['password'], 'one_time_code': valid_token}), headers=utils.api_helper.get_api_headers()) - observed_response = json.loads(response.data.decode('utf-8')) - return observed_response['token'] + return response.json['token'] def _start_test_rqworker(config): diff --git a/securedrop/tests/functional/journalist_navigation_steps.py b/securedrop/tests/functional/journalist_navigation_steps.py index 9aacf56fac..f16dabb2ca 100644 --- a/securedrop/tests/functional/journalist_navigation_steps.py +++ b/securedrop/tests/functional/journalist_navigation_steps.py @@ -232,7 +232,7 @@ def _add_user(self, username, is_admin=False, hotp=None): if hotp: hotp_checkbox = self.driver.find_element_by_css_selector( 'input[name="is_hotp"]') - print((str(hotp_checkbox.__dict__))) + print(str(hotp_checkbox.__dict__)) hotp_checkbox.click() hotp_secret = self.driver.find_element_by_css_selector( 'input[name="otp_secret"]') diff --git a/securedrop/tests/i18n/code.py b/securedrop/tests/i18n/code.py index 446c48a932..90b456adc6 100644 --- a/securedrop/tests/i18n/code.py +++ b/securedrop/tests/i18n/code.py @@ -1,4 +1,4 @@ # -*- coding: utf-8 -*- from flask_babel import gettext -print((gettext('code hello i18n'))) +print(gettext('code hello i18n')) diff --git a/securedrop/tests/test_journalist_api.py b/securedrop/tests/test_journalist_api.py index ba90befbd4..e8e47330f1 100644 --- a/securedrop/tests/test_journalist_api.py +++ b/securedrop/tests/test_journalist_api.py @@ -23,12 +23,11 @@ def test_unauthenticated_user_gets_all_endpoints(journalist_app): with journalist_app.test_client() as app: response = app.get(url_for('api.get_endpoints')) - observed_endpoints = json.loads(response.data.decode('utf-8')) expected_endpoints = ['current_user_url', 'submissions_url', 'sources_url', 'auth_token_url', 'replies_url'] expected_endpoints.sort() - sorted_observed_endpoints = list(observed_endpoints.keys()) + sorted_observed_endpoints = list(response.json.keys()) sorted_observed_endpoints.sort() assert expected_endpoints == sorted_observed_endpoints @@ -42,11 +41,10 @@ def test_valid_user_can_get_an_api_token(journalist_app, test_journo): 'passphrase': test_journo['password'], 'one_time_code': valid_token}), headers=get_api_headers()) - observed_response = json.loads(response.data.decode('utf-8')) - assert observed_response['journalist_uuid'] == test_journo['uuid'] + assert response.json['journalist_uuid'] == test_journo['uuid'] assert isinstance(Journalist.validate_api_token_and_get_user( - observed_response['token']), Journalist) is True + response.json['token']), Journalist) is True assert response.status_code == 200 @@ -60,10 +58,9 @@ def test_user_cannot_get_an_api_token_with_wrong_password(journalist_app, 'passphrase': 'wrong password', 'one_time_code': valid_token}), headers=get_api_headers()) - observed_response = json.loads(response.data.decode('utf-8')) assert response.status_code == 403 - assert observed_response['error'] == 'Forbidden' + assert response.json['error'] == 'Forbidden' def test_user_cannot_get_an_api_token_with_wrong_2fa_token(journalist_app, @@ -76,10 +73,9 @@ def test_user_cannot_get_an_api_token_with_wrong_2fa_token(journalist_app, 'passphrase': test_journo['password'], 'one_time_code': '123456'}), headers=get_api_headers()) - observed_response = json.loads(response.data.decode('utf-8')) assert response.status_code == 403 - assert observed_response['error'] == 'Forbidden' + assert response.json['error'] == 'Forbidden' def test_user_cannot_get_an_api_token_with_no_passphase_field(journalist_app, @@ -91,11 +87,10 @@ def test_user_cannot_get_an_api_token_with_no_passphase_field(journalist_app, {'username': test_journo['username'], 'one_time_code': valid_token}), headers=get_api_headers()) - observed_response = json.loads(response.data.decode('utf-8')) assert response.status_code == 400 - assert observed_response['error'] == 'Bad Request' - assert observed_response['message'] == 'passphrase field is missing' + assert response.json['error'] == 'Bad Request' + assert response.json['message'] == 'passphrase field is missing' def test_user_cannot_get_an_api_token_with_no_username_field(journalist_app, @@ -107,11 +102,10 @@ def test_user_cannot_get_an_api_token_with_no_username_field(journalist_app, {'passphrase': test_journo['password'], 'one_time_code': valid_token}), headers=get_api_headers()) - observed_response = json.loads(response.data.decode('utf-8')) assert response.status_code == 400 - assert observed_response['error'] == 'Bad Request' - assert observed_response['message'] == 'username field is missing' + assert response.json['error'] == 'Bad Request' + assert response.json['message'] == 'username field is missing' def test_user_cannot_get_an_api_token_with_no_otp_field(journalist_app, @@ -122,11 +116,10 @@ def test_user_cannot_get_an_api_token_with_no_otp_field(journalist_app, {'username': test_journo['username'], 'passphrase': test_journo['password']}), headers=get_api_headers()) - observed_response = json.loads(response.data.decode('utf-8')) assert response.status_code == 400 - assert observed_response['error'] == 'Bad Request' - assert observed_response['message'] == 'one_time_code field is missing' + assert response.json['error'] == 'Bad Request' + assert response.json['message'] == 'one_time_code field is missing' def test_authorized_user_gets_all_sources(journalist_app, test_submissions, @@ -135,13 +128,11 @@ def test_authorized_user_gets_all_sources(journalist_app, test_submissions, response = app.get(url_for('api.get_all_sources'), headers=get_api_headers(journalist_api_token)) - data = json.loads(response.data.decode('utf-8')) - assert response.status_code == 200 # We expect to see our test source in the response assert test_submissions['source'].journalist_designation == \ - data['sources'][0]['journalist_designation'] + response.json['sources'][0]['journalist_designation'] def test_user_without_token_cannot_get_protected_endpoints(journalist_app, @@ -260,10 +251,9 @@ def test_api_error_handler_404(journalist_app, journalist_api_token): with journalist_app.test_client() as app: response = app.get('/api/v1/invalidendpoint', headers=get_api_headers(journalist_api_token)) - json_response = json.loads(response.data.decode('utf-8')) assert response.status_code == 404 - assert json_response['error'] == 'Not Found' + assert response.json['error'] == 'Not Found' def test_trailing_slash_cleanly_404s(journalist_app, test_source, @@ -273,10 +263,9 @@ def test_trailing_slash_cleanly_404s(journalist_app, test_source, response = app.get(url_for('api.single_source', source_uuid=uuid) + '/', headers=get_api_headers(journalist_api_token)) - json_response = json.loads(response.data.decode('utf-8')) assert response.status_code == 404 - assert json_response['error'] == 'Not Found' + assert response.json['error'] == 'Not Found' def test_authorized_user_gets_single_source(journalist_app, test_source, @@ -288,9 +277,8 @@ def test_authorized_user_gets_single_source(journalist_app, test_source, assert response.status_code == 200 - data = json.loads(response.data.decode('utf-8')) - assert data['uuid'] == test_source['source'].uuid - assert 'BEGIN PGP PUBLIC KEY' in data['key']['public'] + assert response.json['uuid'] == test_source['source'].uuid + assert 'BEGIN PGP PUBLIC KEY' in response.json['key']['public'] def test_get_non_existant_source_404s(journalist_app, journalist_api_token): @@ -332,8 +320,7 @@ def test_authorized_user_can_star_a_source(journalist_app, test_source, # API should also report is_starred is true response = app.get(url_for('api.single_source', source_uuid=uuid), headers=get_api_headers(journalist_api_token)) - json_response = json.loads(response.data.decode('utf-8')) - assert json_response['is_starred'] is True + assert response.json['is_starred'] is True def test_authorized_user_can_unstar_a_source(journalist_app, test_source, @@ -356,8 +343,7 @@ def test_authorized_user_can_unstar_a_source(journalist_app, test_source, # API should also report is_starred is false response = app.get(url_for('api.single_source', source_uuid=uuid), headers=get_api_headers(journalist_api_token)) - json_response = json.loads(response.data.decode('utf-8')) - assert json_response['is_starred'] is False + assert response.json['is_starred'] is False def test_disallowed_methods_produces_405(journalist_app, test_source, @@ -366,10 +352,9 @@ def test_disallowed_methods_produces_405(journalist_app, test_source, uuid = test_source['source'].uuid response = app.delete(url_for('api.add_star', source_uuid=uuid), headers=get_api_headers(journalist_api_token)) - json_response = json.loads(response.data.decode('utf-8')) assert response.status_code == 405 - assert json_response['error'] == 'Method Not Allowed' + assert response.json['error'] == 'Method Not Allowed' def test_authorized_user_can_get_all_submissions(journalist_app, @@ -380,10 +365,8 @@ def test_authorized_user_can_get_all_submissions(journalist_app, headers=get_api_headers(journalist_api_token)) assert response.status_code == 200 - json_response = json.loads(response.data.decode('utf-8')) - observed_submissions = [submission['filename'] for - submission in json_response['submissions']] + submission in response.json['submissions']] expected_submissions = [submission.filename for submission in Submission.query.all()] @@ -400,10 +383,8 @@ def test_authorized_user_get_source_submissions(journalist_app, headers=get_api_headers(journalist_api_token)) assert response.status_code == 200 - json_response = json.loads(response.data.decode('utf-8')) - observed_submissions = [submission['filename'] for - submission in json_response['submissions']] + submission in response.json['submissions']] expected_submissions = [submission.filename for submission in test_submissions['source'].submissions] @@ -423,13 +404,11 @@ def test_authorized_user_can_get_single_submission(journalist_app, assert response.status_code == 200 - json_response = json.loads(response.data.decode('utf-8')) - - assert json_response['uuid'] == submission_uuid - assert json_response['is_read'] is False - assert json_response['filename'] == \ + assert response.json['uuid'] == submission_uuid + assert response.json['is_read'] is False + assert response.json['filename'] == \ test_submissions['source'].submissions[0].filename - assert json_response['size'] == \ + assert response.json['size'] == \ test_submissions['source'].submissions[0].size @@ -440,10 +419,8 @@ def test_authorized_user_can_get_all_replies(journalist_app, test_files, headers=get_api_headers(journalist_api_token)) assert response.status_code == 200 - json_response = json.loads(response.data.decode('utf-8')) - observed_replies = [reply['filename'] for - reply in json_response['replies']] + reply in response.json['replies']] expected_replies = [reply.filename for reply in Reply.query.all()] @@ -459,10 +436,8 @@ def test_authorized_user_get_source_replies(journalist_app, test_files, headers=get_api_headers(journalist_api_token)) assert response.status_code == 200 - json_response = json.loads(response.data.decode('utf-8')) - observed_replies = [reply['filename'] for - reply in json_response['replies']] + reply in response.json['replies']] expected_replies = [reply.filename for reply in test_files['source'].replies] @@ -481,19 +456,17 @@ def test_authorized_user_can_get_single_reply(journalist_app, test_files, assert response.status_code == 200 - json_response = json.loads(response.data.decode('utf-8')) - reply = Reply.query.filter(Reply.uuid == reply_uuid).one() - assert json_response['uuid'] == reply_uuid - assert json_response['journalist_username'] == \ + assert response.json['uuid'] == reply_uuid + assert response.json['journalist_username'] == \ reply.journalist.username - assert json_response['journalist_uuid'] == \ + assert response.json['journalist_uuid'] == \ reply.journalist.uuid - assert json_response['is_deleted_by_source'] is False - assert json_response['filename'] == \ + assert response.json['is_deleted_by_source'] is False + assert response.json['filename'] == \ test_files['source'].replies[0].filename - assert json_response['size'] == \ + assert response.json['size'] == \ test_files['source'].replies[0].size @@ -601,10 +574,9 @@ def test_authorized_user_can_get_current_user_endpoint(journalist_app, headers=get_api_headers(journalist_api_token)) assert response.status_code == 200 - json_response = json.loads(response.data.decode('utf-8')) - assert json_response['is_admin'] is False - assert json_response['username'] == test_journo['username'] - assert json_response['uuid'] == test_journo['journalist'].uuid + assert response.json['is_admin'] is False + assert response.json['username'] == test_journo['username'] + assert response.json['uuid'] == test_journo['journalist'].uuid def test_request_with_missing_auth_header_triggers_403(journalist_app): @@ -733,8 +705,7 @@ def test_reply_with_valid_curly_json_400(journalist_app, journalist_api_token, headers=get_api_headers(journalist_api_token)) assert response.status_code == 400 - json_response = json.loads(response.data.decode('utf-8')) - assert json_response['message'] == 'reply not found in request body' + assert response.json['message'] == 'reply not found in request body' def test_reply_with_valid_square_json_400(journalist_app, journalist_api_token, @@ -747,8 +718,7 @@ def test_reply_with_valid_square_json_400(journalist_app, journalist_api_token, headers=get_api_headers(journalist_api_token)) assert response.status_code == 400 - json_response = json.loads(response.data.decode('utf-8')) - assert json_response['message'] == 'reply not found in request body' + assert response.json['message'] == 'reply not found in request body' def test_malformed_json_400(journalist_app, journalist_api_token, test_journo, @@ -768,10 +738,9 @@ def test_malformed_json_400(journalist_app, journalist_api_token, test_journo, response = app.post(protected_route, data="{this is invalid {json!", headers=get_api_headers(journalist_api_token)) - observed_response = json.loads(response.data.decode('utf-8')) assert response.status_code == 400 - assert observed_response['error'] == 'Bad Request' + assert response.json['error'] == 'Bad Request' def test_empty_json_400(journalist_app, journalist_api_token, test_journo, @@ -789,10 +758,9 @@ def test_empty_json_400(journalist_app, journalist_api_token, test_journo, response = app.post(protected_route, data="", headers=get_api_headers(journalist_api_token)) - observed_response = json.loads(response.data.decode('utf-8')) assert response.status_code == 400 - assert observed_response['error'] == 'Bad Request' + assert response.json['error'] == 'Bad Request' def test_empty_json_20X(journalist_app, journalist_api_token, test_journo, diff --git a/securedrop/tests/test_source.py b/securedrop/tests/test_source.py index b029b673f9..4499054642 100644 --- a/securedrop/tests/test_source.py +++ b/securedrop/tests/test_source.py @@ -1,6 +1,5 @@ # -*- coding: utf-8 -*- import gzip -import json import platform import re import subprocess @@ -532,10 +531,8 @@ def test_metadata_route(source_app): resp = app.get(url_for('api.metadata')) assert resp.status_code == 200 assert resp.headers.get('Content-Type') == 'application/json' - assert json.loads(resp.data.decode('utf-8')).get('sd_version') \ - == version.__version__ - assert json.loads(resp.data.decode('utf-8')).get('server_os') \ - == '16.04' + assert resp.json.get('sd_version') == version.__version__ + assert resp.json.get('server_os') == '16.04' def test_login_with_overly_long_codename(source_app): From 44e8d062b441df8cac8c956a4c72565c8df5d1f1 Mon Sep 17 00:00:00 2001 From: redshiftzero Date: Mon, 15 Apr 2019 21:49:55 -0700 Subject: [PATCH 47/47] ci: fix app-test job, DRY up docker layer cache loading --- .circleci/config.yml | 30 +++++++++++------------------- 1 file changed, 11 insertions(+), 19 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 454a745a1e..5fb921f520 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -30,15 +30,15 @@ common-steps: command: | set +o pipefail docker images - fromtag=$(docker images |grep securedrop-test-xenial |head -n1 |awk '{print $2}') - cd securedrop && DOCKER_BUILD_ARGUMENTS="--cache-from securedrop-test-xenial:${fromtag:-latest}" ./bin/dev-shell true + fromtag=$(docker images |grep securedrop-test-xenial-py2 |head -n1 |awk '{print $2}') + cd securedrop && DOCKER_BUILD_ARGUMENTS="--cache-from securedrop-test-xenial-py2:${fromtag:-latest}" ./bin/dev-shell true - &saveimagelayers run: name: Save Docker image layer cache command: | docker images - docker save -o /caches/layers.tar securedrop-test-xenial:latest + docker save -o /caches/layers.tar securedrop-test-xenial-py2:latest - &savecache save_cache: @@ -73,11 +73,7 @@ jobs: paths: - /caches/layers.tar.gz - - run: - name: Load image layer cache - command: | - set +o pipefail - docker load -i /caches/layers.tar |true + - *loadimagelayers - run: name: Build Docker image @@ -141,9 +137,9 @@ jobs: name: Run tests command: | export TESTFILES=$(cd securedrop; circleci tests glob 'tests/test*py' 'tests/**/test*py' |circleci tests split --split-by=timings |xargs echo) - docker rm -f securedrop-test-xenial || true - fromtag=$(docker images |grep securedrop-test-xenial |head -n1 |awk '{print $2}') - cd securedrop && DOCKER_RUN_ARGUMENTS=$(bash <(curl -s https://codecov.io/env)) DOCKER_BUILD_ARGUMENTS="--cache-from securedrop-test-xenial:${fromtag:-latest}" make test + docker rm -f securedrop-test-xenial-py2 || true + fromtag=$(docker images |grep securedrop-test-xenial-py2 |head -n1 |awk '{print $2}') + cd securedrop && DOCKER_RUN_ARGUMENTS=$(bash <(curl -s https://codecov.io/env)) DOCKER_BUILD_ARGUMENTS="--cache-from securedrop-test-xenial-py2:${fromtag:-latest}" make test - store_test_results: path: ~/test-results @@ -169,11 +165,7 @@ jobs: paths: - /caches/layers.tar.gz - - run: - name: Load image layer cache - command: | - set +o pipefail - docker load -i /caches/layers.tar |true + - *loadimagelayers - run: name: Build Docker images @@ -237,9 +229,9 @@ jobs: name: Run tests command: | export TESTFILES=$(cd securedrop; circleci tests glob 'tests/pageslayout/test*py' |circleci tests split --split-by=timings |xargs echo) - docker rm -f securedrop-test-xenial || true - fromtag=$(docker images |grep securedrop-test-xenial |head -n1 |awk '{print $2}') - cd securedrop && DOCKER_BUILD_ARGUMENTS="--cache-from securedrop-test-xenial:${fromtag:-latest}" make translation-test + docker rm -f securedrop-test-xenial-py2 || true + fromtag=$(docker images |grep securedrop-test-xenial-py2 |head -n1 |awk '{print $2}') + cd securedrop && DOCKER_BUILD_ARGUMENTS="--cache-from securedrop-test-xenial-py2:${fromtag:-latest}" make translation-test - store_test_results: path: ~/test-results