From 4fb965f8c007fa24849a749cda00d709e1fe9ee3 Mon Sep 17 00:00:00 2001 From: John Hensley Date: Fri, 8 May 2020 13:02:40 -0400 Subject: [PATCH] Make deletion of multiple sources asynchronous Add Source.deleted_at timestamp. Start generally filtering sources that have it set. When deleting multiple sources with journalist_app.utils.col_delete, just mark them deleted with this and defer the removal of their submissions, keypair and database record to asynchronous processing. Add a new systemd service to periodically purge sources marked for deletion. Assign explicit names to source GPG keys. Instead of using the python-gnupg default of "Autogenerated Key", label source keys with "Source Key". When asked to delete a reply keypair, verify that its UID contains one of those labels. --- .../defaults/main.yml | 1 + .../securedrop_source_deleter.service | 19 +++++ .../debian/securedrop-app-code.install | 1 + ...securedrop_source_deleter_configuration.py | 48 +++++++++++ .../35513370ba0d_add_source_deleted_at.py | 32 +++++++ securedrop/bin/run | 1 + securedrop/crypto_util.py | 38 ++++++++- securedrop/journalist.py | 2 +- securedrop/journalist_app/api.py | 2 +- securedrop/journalist_app/col.py | 7 +- securedrop/journalist_app/forms.py | 10 +-- securedrop/journalist_app/main.py | 4 +- securedrop/journalist_app/utils.py | 53 +++++++++--- securedrop/models.py | 3 + securedrop/scripts/shredder | 20 +++-- securedrop/scripts/source_deleter | 59 +++++++++++++ securedrop/source_app/__init__.py | 1 + securedrop/tests/conftest.py | 2 +- .../migrations/migration_35513370ba0d.py | 83 +++++++++++++++++++ securedrop/tests/test_crypto_util.py | 11 +++ securedrop/tests/test_integration.py | 5 +- securedrop/tests/test_journalist.py | 12 ++- securedrop/tests/test_source.py | 11 +-- 23 files changed, 379 insertions(+), 46 deletions(-) create mode 100644 install_files/ansible-base/roles/build-securedrop-app-code-deb-pkg/templates/securedrop_source_deleter.service create mode 100644 molecule/testinfra/staging/app-code/test_securedrop_source_deleter_configuration.py create mode 100644 securedrop/alembic/versions/35513370ba0d_add_source_deleted_at.py create mode 100755 securedrop/scripts/source_deleter create mode 100644 securedrop/tests/migrations/migration_35513370ba0d.py diff --git a/install_files/ansible-base/roles/build-securedrop-app-code-deb-pkg/defaults/main.yml b/install_files/ansible-base/roles/build-securedrop-app-code-deb-pkg/defaults/main.yml index 6202d43f53c..80643216ff3 100644 --- a/install_files/ansible-base/roles/build-securedrop-app-code-deb-pkg/defaults/main.yml +++ b/install_files/ansible-base/roles/build-securedrop-app-code-deb-pkg/defaults/main.yml @@ -24,6 +24,7 @@ systemd_services: - securedrop_rqrequeue.service - securedrop_rqworker.service - securedrop_shredder.service + - securedrop_source_deleter.service # SecureDrop rq worker log directory securedrop_worker_log_dir: /var/log/securedrop_worker diff --git a/install_files/ansible-base/roles/build-securedrop-app-code-deb-pkg/templates/securedrop_source_deleter.service b/install_files/ansible-base/roles/build-securedrop-app-code-deb-pkg/templates/securedrop_source_deleter.service new file mode 100644 index 00000000000..17437993684 --- /dev/null +++ b/install_files/ansible-base/roles/build-securedrop-app-code-deb-pkg/templates/securedrop_source_deleter.service @@ -0,0 +1,19 @@ +[Unit] +Description=SecureDrop Source deleter + +[Service] +Environment=PYTHONPATH="{{ securedrop_code }}:{{ securedrop_venv_site_packages }}" +ExecStart={{ securedrop_venv_bin }}/python {{ securedrop_code }}/scripts/source_deleter --interval 10 +PrivateDevices=yes +PrivateTmp=yes +ProtectSystem=full +ReadOnlyDirectories=/ +ReadWriteDirectories={{ securedrop_data }} +Restart=always +RestartSec=10s +UMask=077 +User=www-data +WorkingDirectory={{ securedrop_code }} + +[Install] +WantedBy=multi-user.target diff --git a/install_files/securedrop-app-code/debian/securedrop-app-code.install b/install_files/securedrop-app-code/debian/securedrop-app-code.install index 9e1f866b207..d36242a2174 100644 --- a/install_files/securedrop-app-code/debian/securedrop-app-code.install +++ b/install_files/securedrop-app-code/debian/securedrop-app-code.install @@ -4,3 +4,4 @@ etc/apparmor.d/usr.sbin.tor /etc/apparmor.d lib/systemd/system/securedrop_rqrequeue.service /lib/systemd/system lib/systemd/system/securedrop_rqworker.service /lib/systemd/system lib/systemd/system/securedrop_shredder.service /lib/systemd/system +lib/systemd/system/securedrop_source_deleter.service /lib/systemd/system diff --git a/molecule/testinfra/staging/app-code/test_securedrop_source_deleter_configuration.py b/molecule/testinfra/staging/app-code/test_securedrop_source_deleter_configuration.py new file mode 100644 index 00000000000..3dd6dce217d --- /dev/null +++ b/molecule/testinfra/staging/app-code/test_securedrop_source_deleter_configuration.py @@ -0,0 +1,48 @@ +import pytest + + +testinfra_hosts = ["app-staging"] + + +def test_securedrop_source_deleter_service(host): + """ + Verify configuration of securedrop_source_deleter systemd service. + """ + securedrop_test_vars = pytest.securedrop_test_vars + service_file = "/lib/systemd/system/securedrop_source_deleter.service" + expected_content = "\n".join([ + "[Unit]", + "Description=SecureDrop Source deleter", + "", + "[Service]", + 'Environment=PYTHONPATH="{}:{}"'.format( + securedrop_test_vars.securedrop_code, securedrop_test_vars.securedrop_venv_site_packages + ), + "ExecStart={}/python /var/www/securedrop/scripts/source_deleter --interval 10".format( + securedrop_test_vars.securedrop_venv_bin + ), + "PrivateDevices=yes", + "PrivateTmp=yes", + "ProtectSystem=full", + "ReadOnlyDirectories=/", + "ReadWriteDirectories={}".format(securedrop_test_vars.securedrop_data), + "Restart=always", + "RestartSec=10s", + "UMask=077", + "User={}".format(securedrop_test_vars.securedrop_user), + "WorkingDirectory={}".format(securedrop_test_vars.securedrop_code), + "", + "[Install]", + "WantedBy=multi-user.target\n", + ]) + + f = host.file(service_file) + assert f.is_file + assert f.mode == 0o644 + assert f.user == "root" + assert f.group == "root" + assert f.content_string == expected_content + + s = host.service("securedrop_source_deleter") + assert s.is_enabled + assert s.is_running diff --git a/securedrop/alembic/versions/35513370ba0d_add_source_deleted_at.py b/securedrop/alembic/versions/35513370ba0d_add_source_deleted_at.py new file mode 100644 index 00000000000..2c73e971eec --- /dev/null +++ b/securedrop/alembic/versions/35513370ba0d_add_source_deleted_at.py @@ -0,0 +1,32 @@ +"""add Source.deleted_at + +Revision ID: 35513370ba0d +Revises: 523fff3f969c +Create Date: 2020-05-06 22:28:01.214359 + +""" +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = '35513370ba0d' +down_revision = '523fff3f969c' +branch_labels = None +depends_on = None + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + with op.batch_alter_table('sources', schema=None) as batch_op: + batch_op.add_column(sa.Column('deleted_at', sa.DateTime(), nullable=True)) + + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + with op.batch_alter_table('sources', schema=None) as batch_op: + batch_op.drop_column('deleted_at') + + # ### end Alembic commands ### diff --git a/securedrop/bin/run b/securedrop/bin/run index 7ce6479b63f..e82cc63200d 100755 --- a/securedrop/bin/run +++ b/securedrop/bin/run @@ -18,5 +18,6 @@ reset_demo /opt/venvs/securedrop-app-code/bin/rqworker & PYTHONPATH="${REPOROOT}/securedrop" /opt/venvs/securedrop-app-code/bin/python "${REPOROOT}/securedrop/scripts/rqrequeue" --interval 60 & /opt/venvs/securedrop-app-code/bin/python "${REPOROOT}/securedrop/scripts/shredder" --interval 60 & +/opt/venvs/securedrop-app-code/bin/python "${REPOROOT}/securedrop/scripts/source_deleter" --interval 10 & ./manage.py run diff --git a/securedrop/crypto_util.py b/securedrop/crypto_util.py index 392e22f2728..5d7a05008e7 100644 --- a/securedrop/crypto_util.py +++ b/securedrop/crypto_util.py @@ -4,6 +4,7 @@ import pretty_bad_protocol as gnupg import os import io +import re import scrypt from random import SystemRandom @@ -76,6 +77,8 @@ class CryptoUtil: REDIS_FINGERPRINT_HASH = "sd/crypto-util/fingerprints" REDIS_KEY_HASH = "sd/crypto-util/keys" + SOURCE_KEY_UID_RE = re.compile(r"(Source|Autogenerated) Key <[-A-Za-z0-9+/=_]+>") + def __init__(self, scrypt_params, scrypt_id_pepper, @@ -213,23 +216,52 @@ def genkeypair(self, name, secret): key_length=self.__gpg_key_length, passphrase=secret, name_email=name, + name_real="Source Key", creation_date=self.DEFAULT_KEY_CREATION_DATE.isoformat(), expire_date=self.DEFAULT_KEY_EXPIRATION_DATE )) return genkey_obj + def find_source_key(self, fingerprint: str) -> typing.Optional[typing.Dict]: + """ + Searches the GPG keyring for a source key. + + A source key has the given fingerprint and is labeled either + "Source Key" or "Autogenerated Key". + + Returns the key or None. + """ + keys = self.gpg.list_keys() + for key in keys: + if fingerprint != key["fingerprint"]: + continue + + for uid in key["uids"]: + if self.SOURCE_KEY_UID_RE.match(uid): + return key + else: + return None + return None + def delete_reply_keypair(self, source_filesystem_id): - key = self.get_fingerprint(source_filesystem_id) + fingerprint = self.get_fingerprint(source_filesystem_id) + # If this source was never flagged for review, they won't have a reply # keypair - if not key: + if not fingerprint: return + # verify that the key with the given fingerprint belongs to a source + key = self.find_source_key(fingerprint) + if not key: + raise ValueError("source key not found") + # Always delete keys without invoking pinentry-mode = loopback # see: https://lists.gnupg.org/pipermail/gnupg-users/2016-May/055965.html temp_gpg = gnupg.GPG(binary='gpg2', homedir=self.gpg_key_dir) + # The subkeys keyword argument deletes both secret and public keys. - temp_gpg.delete_keys(key, secret=True, subkeys=True) + temp_gpg.delete_keys(fingerprint, secret=True, subkeys=True) self.redis.hdel(self.REDIS_KEY_HASH, self.get_fingerprint(source_filesystem_id)) self.redis.hdel(self.REDIS_FINGERPRINT_HASH, source_filesystem_id) diff --git a/securedrop/journalist.py b/securedrop/journalist.py index ea8ca9573d5..348d2ae878c 100644 --- a/securedrop/journalist.py +++ b/securedrop/journalist.py @@ -15,7 +15,7 @@ def prime_keycache(): Preloads CryptoUtil.keycache. """ with app.app_context(): - for source in Source.query.filter_by(pending=False).all(): + for source in Source.query.filter_by(pending=False, deleted_at=None).all(): app.crypto_util.get_pubkey(source.filesystem_id) diff --git a/securedrop/journalist_app/api.py b/securedrop/journalist_app/api.py index cf6d76585ad..f1a6e5df55f 100644 --- a/securedrop/journalist_app/api.py +++ b/securedrop/journalist_app/api.py @@ -135,7 +135,7 @@ def get_token(): @api.route('/sources', methods=['GET']) @token_required def get_all_sources(): - sources = Source.query.filter_by(pending=False).all() + sources = Source.query.filter_by(pending=False, deleted_at=None).all() return jsonify( {'sources': [source.to_json() for source in sources]}), 200 diff --git a/securedrop/journalist_app/col.py b/securedrop/journalist_app/col.py index 0f92b04e39c..31476b16724 100644 --- a/securedrop/journalist_app/col.py +++ b/securedrop/journalist_app/col.py @@ -41,7 +41,12 @@ def col(filesystem_id): def delete_single(filesystem_id): """deleting a single collection from its /col page""" source = get_source(filesystem_id) - delete_collection(filesystem_id) + try: + delete_collection(filesystem_id) + except ValueError as e: + current_app.logger.error("error deleting collection: %s", e) + abort(500) + flash(gettext("{source_name}'s collection deleted") .format(source_name=source.journalist_designation), "notification") diff --git a/securedrop/journalist_app/forms.py b/securedrop/journalist_app/forms.py index 5e196a59e3f..21e0dbdd7a2 100644 --- a/securedrop/journalist_app/forms.py +++ b/securedrop/journalist_app/forms.py @@ -3,7 +3,7 @@ from flask_babel import lazy_gettext as gettext from flask_wtf import FlaskForm from flask_wtf.file import FileField, FileAllowed, FileRequired -from wtforms import (TextAreaField, TextField, BooleanField, HiddenField, +from wtforms import (TextAreaField, StringField, BooleanField, HiddenField, ValidationError) from wtforms.validators import InputRequired, Optional @@ -38,16 +38,16 @@ def name_length_validation(form, field): class NewUserForm(FlaskForm): - username = TextField('username', validators=[ + username = StringField('username', validators=[ InputRequired(message=gettext('This field is required.')), minimum_length_validation ]) - first_name = TextField('first_name', validators=[name_length_validation, Optional()]) - last_name = TextField('last_name', validators=[name_length_validation, Optional()]) + first_name = StringField('first_name', validators=[name_length_validation, Optional()]) + last_name = StringField('last_name', validators=[name_length_validation, Optional()]) password = HiddenField('password') is_admin = BooleanField('is_admin') is_hotp = BooleanField('is_hotp') - otp_secret = TextField('otp_secret', validators=[ + otp_secret = StringField('otp_secret', validators=[ otp_secret_validation, Optional() ]) diff --git a/securedrop/journalist_app/main.py b/securedrop/journalist_app/main.py index a8658fd4726..593b9e0a28f 100644 --- a/securedrop/journalist_app/main.py +++ b/securedrop/journalist_app/main.py @@ -64,7 +64,7 @@ def index(): # Long SQLAlchemy statements look best when formatted according to # the Pocoo style guide, IMHO: # http://www.pocoo.org/internal/styleguide/ - sources = Source.query.filter_by(pending=False) \ + sources = Source.query.filter_by(pending=False, deleted_at=None) \ .filter(Source.last_updated.isnot(None)) \ .order_by(Source.last_updated.desc()) \ .all() @@ -171,7 +171,7 @@ def bulk(): @view.route('/download_unread/') def download_unread_filesystem_id(filesystem_id): id = Source.query.filter(Source.filesystem_id == filesystem_id) \ - .one().id + .filter_by(deleted_at=None).one().id submissions = Submission.query.filter( Submission.source_id == id, Submission.downloaded == false()).all() diff --git a/securedrop/journalist_app/utils.py b/securedrop/journalist_app/utils.py index a6aa5adac55..feb0a042ecd 100644 --- a/securedrop/journalist_app/utils.py +++ b/securedrop/journalist_app/utils.py @@ -1,7 +1,8 @@ # -*- coding: utf-8 -*- import binascii +import datetime +import os -from datetime import datetime from flask import (g, flash, current_app, abort, send_file, redirect, url_for, render_template, Markup, sessions, request) from flask_babel import gettext, ngettext @@ -53,11 +54,17 @@ def commit_account_changes(user): flash(gettext("Account updated."), "success") -def get_source(filesystem_id): - """Return a Source object, representing the database row, for the source - with the `filesystem_id`""" +def get_source(filesystem_id, include_deleted=False): + """ + Return the Source object with `filesystem_id` + + If `include_deleted` is False, only sources with a null `deleted_at` will + be returned. + """ source = None query = Source.query.filter(Source.filesystem_id == filesystem_id) + if not include_deleted: + query = query.filter_by(deleted_at=None) source = get_one_or_else(query, current_app.logger, abort) return source @@ -157,7 +164,7 @@ def download(zip_basename, submissions): zf = current_app.storage.get_bulk_archive(submissions, zip_directory=zip_basename) attachment_filename = "{}--{}.zip".format( - zip_basename, datetime.utcnow().strftime("%Y-%m-%d--%H-%M-%S")) + zip_basename, datetime.datetime.utcnow().strftime("%Y-%m-%d--%H-%M-%S")) # Mark the submissions that have been downloaded as such for submission in submissions: @@ -233,8 +240,11 @@ def col_delete(cols_selected): if len(cols_selected) < 1: flash(gettext("No collections selected for deletion."), "error") else: - for filesystem_id in cols_selected: - delete_collection(filesystem_id) + now = datetime.datetime.utcnow() + sources = Source.query.filter(Source.filesystem_id.in_(cols_selected)) + sources.update({Source.deleted_at: now}, synchronize_session="fetch") + db.session.commit() + num = len(cols_selected) flash(ngettext('{num} collection deleted', '{num} collections deleted', num).format(num=num), @@ -259,17 +269,36 @@ def make_password(config): def delete_collection(filesystem_id): # Delete the source's collection of submissions path = current_app.storage.path(filesystem_id) - current_app.storage.move_to_shredder(path) + if os.path.exists(path): + current_app.storage.move_to_shredder(path) # Delete the source's reply keypair - current_app.crypto_util.delete_reply_keypair(filesystem_id) + try: + current_app.crypto_util.delete_reply_keypair(filesystem_id) + except ValueError as e: + current_app.logger.error("could not delete reply keypair: %s", e) + raise # Delete their entry in the db - source = get_source(filesystem_id) + source = get_source(filesystem_id, include_deleted=True) db.session.delete(source) db.session.commit() +def purge_deleted_sources(): + """ + Deletes all Sources with a non-null `deleted_at` attribute. + """ + sources = Source.query.filter(Source.deleted_at.isnot(None)).order_by(Source.deleted_at).all() + if sources: + current_app.logger.info("Purging deleted sources (%s)", len(sources)) + for source in sources: + try: + delete_collection(source.filesystem_id) + except Exception as e: + current_app.logger.error("Error deleting source %s: %s", source.uuid, e) + + def set_name(user, first_name, last_name): try: user.set_name(first_name, last_name) @@ -312,7 +341,7 @@ def col_download_unread(cols_selected): submissions = [] for filesystem_id in cols_selected: id = Source.query.filter(Source.filesystem_id == filesystem_id) \ - .one().id + .filter_by(deleted_at=None).one().id submissions += Submission.query.filter( Submission.downloaded == false(), Submission.source_id == id).all() @@ -328,7 +357,7 @@ def col_download_all(cols_selected): submissions = [] for filesystem_id in cols_selected: id = Source.query.filter(Source.filesystem_id == filesystem_id) \ - .one().id + .filter_by(deleted_at=None).one().id submissions += Submission.query.filter( Submission.source_id == id).all() return download("all", submissions) diff --git a/securedrop/models.py b/securedrop/models.py index 38e07a9e471..68e02ca16fe 100644 --- a/securedrop/models.py +++ b/securedrop/models.py @@ -73,6 +73,9 @@ class Source(db.Model): # keep track of how many interactions have happened, for filenames interaction_count = Column(Integer, default=0, nullable=False) + # when deletion of the source was requested + deleted_at = Column(DateTime) + # Don't create or bother checking excessively long codenames to prevent DoS NUM_WORDS = 7 MAX_CODENAME_LEN = 128 diff --git a/securedrop/scripts/shredder b/securedrop/scripts/shredder index f4188d8cab1..9371cb96230 100755 --- a/securedrop/scripts/shredder +++ b/securedrop/scripts/shredder @@ -10,7 +10,9 @@ import sys import time sys.path.insert(0, ".") # noqa: E402 -sys.path.insert(0, "/opt/venvs/securedrop-app-code/lib/python3.5/site-packages") # noqa: E402 +sys.path.insert( + 0, "/opt/venvs/securedrop-app-code/lib/python3.5/site-packages" +) # noqa: E402 sys.path.insert(0, "/var/www/securedrop") # noqa: E402 import journalist_app @@ -19,13 +21,11 @@ from sdconfig import config def parse_args(): parser = argparse.ArgumentParser( - prog=__file__, description="Utility for clearing deleted content in the SecureDrop store." + prog=__file__, + description="Utility for clearing deleted content in the SecureDrop store.", ) parser.add_argument( - "-i", - "--interval", - type=int, - help="Keep running every 'interval' seconds.", + "-i", "--interval", type=int, help="Keep running every 'interval' seconds.", ) return parser.parse_args() @@ -40,14 +40,16 @@ def clear_shredder(): def main(): args = parse_args() - logging.basicConfig(format="%(asctime)s %(levelname)s %(message)s", level=logging.INFO) + logging.basicConfig( + format="%(asctime)s %(levelname)s %(message)s", level=logging.INFO + ) if args.interval: - logging.info("Running every {} seconds.".format(args.interval)) + logging.info("Clearing shredder every {} seconds.".format(args.interval)) while 1: clear_shredder() time.sleep(args.interval) else: - logging.info("Running once.") + logging.info("Clearing shredder once.") clear_shredder() diff --git a/securedrop/scripts/source_deleter b/securedrop/scripts/source_deleter new file mode 100755 index 00000000000..043ea13fbb9 --- /dev/null +++ b/securedrop/scripts/source_deleter @@ -0,0 +1,59 @@ +#!/opt/venvs/securedrop-app-code/bin/python + +# +# Purges deleted sources and their data. +# + +import argparse +import logging +import sys +import time + +sys.path.insert(0, ".") # noqa: E402 +sys.path.insert( + 0, "/opt/venvs/securedrop-app-code/lib/python3.5/site-packages" +) # noqa: E402 +sys.path.insert(0, "/var/www/securedrop") # noqa: E402 + +import journalist_app +from sdconfig import config + + +def parse_args(): + parser = argparse.ArgumentParser( + prog=__file__, + description="Utility for asynchronous deletion of SecureDrop sources and their data.", + ) + parser.add_argument( + "-i", "--interval", type=int, help="Keep running every 'interval' seconds.", + ) + + return parser.parse_args() + + +def purge_deleted_sources(): + try: + journalist_app.utils.purge_deleted_sources() + except Exception as e: + logging.info("Error purging deleted sources: {}".format(e)) + + +def main(): + args = parse_args() + logging.basicConfig( + format="%(asctime)s %(levelname)s %(message)s", level=logging.INFO + ) + if args.interval: + logging.info("Purging deleted sources every {} seconds.".format(args.interval)) + while 1: + purge_deleted_sources() + time.sleep(args.interval) + else: + logging.info("Purging deleted sources once.") + purge_deleted_sources() + + +if __name__ == "__main__": + app = journalist_app.create_app(config) + with app.app_context(): + main() diff --git a/securedrop/source_app/__init__.py b/securedrop/source_app/__init__.py index b52d76849a5..9ae573294cd 100644 --- a/securedrop/source_app/__init__.py +++ b/securedrop/source_app/__init__.py @@ -165,6 +165,7 @@ def setup_g(): try: g.source = Source.query \ .filter(Source.filesystem_id == g.filesystem_id) \ + .filter_by(deleted_at=None) \ .one() except NoResultFound as e: app.logger.error( diff --git a/securedrop/tests/conftest.py b/securedrop/tests/conftest.py index 8cc584d18d0..9347a397eaf 100644 --- a/securedrop/tests/conftest.py +++ b/securedrop/tests/conftest.py @@ -119,7 +119,7 @@ def config(tmpdir): def alembic_config(config): base_dir = path.join(path.dirname(__file__), '..') migrations_dir = path.join(base_dir, 'alembic') - ini = configparser.SafeConfigParser() + ini = configparser.ConfigParser() ini.read(path.join(base_dir, 'alembic.ini')) ini.set('alembic', 'script_location', path.join(migrations_dir)) diff --git a/securedrop/tests/migrations/migration_35513370ba0d.py b/securedrop/tests/migrations/migration_35513370ba0d.py new file mode 100644 index 00000000000..0c0257cf2ee --- /dev/null +++ b/securedrop/tests/migrations/migration_35513370ba0d.py @@ -0,0 +1,83 @@ +# -*- coding: utf-8 -*- + +import random +from uuid import uuid4 + +from db import db +from journalist_app import create_app +import sqlalchemy +import pytest + +from .helpers import bool_or_none, random_bool, random_chars, random_datetime + + +class UpgradeTester: + def __init__(self, config): + self.config = config + self.app = create_app(config) + + def load_data(self): + with self.app.app_context(): + self.add_source() + self.valid_source_id = 1 + + db.session.commit() + + @staticmethod + def add_source(): + filesystem_id = random_chars(96) if random_bool() else None + params = { + "uuid": str(uuid4()), + "filesystem_id": filesystem_id, + "journalist_designation": random_chars(50), + "flagged": bool_or_none(), + "last_updated": random_datetime(nullable=True), + "pending": bool_or_none(), + "interaction_count": random.randint(0, 1000), + } + sql = """ + INSERT INTO sources ( + uuid, filesystem_id, journalist_designation, flagged, last_updated, + pending, interaction_count + ) VALUES ( + :uuid, :filesystem_id, :journalist_designation, :flagged, :last_updated, + :pending, :interaction_count + ) + """ + + db.engine.execute(sqlalchemy.text(sql), **params) + + def check_upgrade(self): + """ + Check the new `deleted_at` column + + Querying `deleted_at` shouldn't cause an error, and no source + should already have it set. + """ + with self.app.app_context(): + sources = db.engine.execute( + sqlalchemy.text("SELECT * FROM sources WHERE deleted_at IS NOT NULL") + ).fetchall() + assert len(sources) == 0 + + +class DowngradeTester: + def __init__(self, config): + self.config = config + self.app = create_app(config) + + def load_data(self): + pass + + def check_downgrade(self): + """ + After downgrade, using `deleted_at` in a query should raise an exception + """ + with self.app.app_context(): + with pytest.raises(sqlalchemy.exc.OperationalError): + sources = db.engine.execute( + sqlalchemy.text( + "SELECT * FROM sources WHERE deleted_at IS NOT NULL" + ) + ).fetchall() + assert len(sources) == 0 diff --git a/securedrop/tests/test_crypto_util.py b/securedrop/tests/test_crypto_util.py index 2b77cc624a4..07ad9c9c706 100644 --- a/securedrop/tests/test_crypto_util.py +++ b/securedrop/tests/test_crypto_util.py @@ -295,6 +295,17 @@ def test_delete_reply_keypair_no_key(source_app): source_app.crypto_util.delete_reply_keypair('Reality Winner') +def test_delete_reply_keypair_non_source(source_app): + """ + Checks that a non-source key is not deleted by delete_reply_keypair. + """ + name = "SecureDrop Test/Development (DO NOT USE IN PRODUCTION)" + with pytest.raises(ValueError) as excinfo: + source_app.crypto_util.delete_reply_keypair(name) + assert "source key not found" in str(excinfo.value) + assert source_app.crypto_util.get_fingerprint(name) + + def test_get_fingerprint(source_app, test_source): assert (source_app.crypto_util.get_fingerprint(test_source['filesystem_id']) is not None) diff --git a/securedrop/tests/test_integration.py b/securedrop/tests/test_integration.py index 345ad0ee102..cff50a293c2 100644 --- a/securedrop/tests/test_integration.py +++ b/securedrop/tests/test_integration.py @@ -549,9 +549,12 @@ def test_delete_collections(mocker, journalist_app, source_app, test_journo): ), follow_redirects=True) assert resp.status_code == 200 text = resp.data.decode('utf-8') - assert "{} collections deleted".format(num_sources) in text + assert "started secure deletion of {} collections".format(num_sources) in text assert async_genkey.called + # simulate the source_deleter's work + journalist_app_module.utils.purge_deleted_sources() + # Make sure the collections are deleted from the filesystem def assertion(): assert not ( diff --git a/securedrop/tests/test_journalist.py b/securedrop/tests/test_journalist.py index 87c699d74ea..5f9d188b64a 100644 --- a/securedrop/tests/test_journalist.py +++ b/securedrop/tests/test_journalist.py @@ -1330,7 +1330,7 @@ def test_logo_upload_with_valid_image_succeeds(journalist_app, test_admin): test_admin['otp_secret']) # Create 1px * 1px 'white' PNG file from its base64 string form = journalist_app_module.forms.LogoForm( - logo=(BytesIO(base64.decodestring + logo=(BytesIO(base64.decodebytes (b"iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQ" b"VR42mP8/x8AAwMCAO+ip1sAAAAASUVORK5CYII=")), 'test.png') ) @@ -2067,6 +2067,8 @@ def test_col_process_successfully_deletes_multiple_sources(journalist_app, utils.db_helper.submit(source_1, 1) source_2, _ = utils.db_helper.init_source() utils.db_helper.submit(source_2, 1) + source_3, _ = utils.db_helper.init_source() + utils.db_helper.submit(source_3, 1) with journalist_app.test_client() as app: _login_user(app, test_journo['username'], test_journo['password'], @@ -2081,9 +2083,13 @@ def test_col_process_successfully_deletes_multiple_sources(journalist_app, assert resp.status_code == 200 - # Verify there are no remaining sources + # simulate the source_deleter's work + journalist_app_module.utils.purge_deleted_sources() + + # Verify that all of the specified sources were deleted, but no others remaining_sources = Source.query.all() - assert not remaining_sources + assert len(remaining_sources) == 1 + assert remaining_sources[0].uuid == source_3.uuid def test_col_process_successfully_stars_sources(journalist_app, diff --git a/securedrop/tests/test_source.py b/securedrop/tests/test_source.py index 0313a6790f1..c28b9a8ebf6 100644 --- a/securedrop/tests/test_source.py +++ b/securedrop/tests/test_source.py @@ -14,6 +14,7 @@ import version from db import db +from journalist_app.utils import delete_collection from models import InstanceConfig, Source, Reply from source_app import main as source_app_main from source_app import api as source_app_api @@ -653,11 +654,7 @@ def test_source_is_deleted_while_logged_in(source_app): # Now the journalist deletes the source filesystem_id = source_app.crypto_util.hash_codename(codename) - source_app.crypto_util.delete_reply_keypair(filesystem_id) - source = Source.query.filter_by( - filesystem_id=filesystem_id).one() - db.session.delete(source) - db.session.commit() + delete_collection(filesystem_id) # Source attempts to continue to navigate resp = app.post(url_for('main.lookup'), follow_redirects=True) @@ -668,8 +665,8 @@ def test_source_is_deleted_while_logged_in(source_app): assert 'codename' not in session logger.assert_called_once_with( - "Found no Sources when one was expected: " - "No row was found for one()") + "Found no Sources when one was expected: No row was found for one()" + ) def test_login_with_invalid_codename(source_app):