diff --git a/securedrop/alembic/versions/2e24fc7536e8_make_journalist_id_non_nullable.py b/securedrop/alembic/versions/2e24fc7536e8_make_journalist_id_non_nullable.py new file mode 100644 index 0000000000..2af85a839b --- /dev/null +++ b/securedrop/alembic/versions/2e24fc7536e8_make_journalist_id_non_nullable.py @@ -0,0 +1,158 @@ +"""make journalist_id non-nullable + +Revision ID: 2e24fc7536e8 +Revises: de00920916bf +Create Date: 2022-01-12 19:31:06.186285 + +""" +from passlib.hash import argon2 +import os +import pyotp +import uuid + +from alembic import op +import sqlalchemy as sa + +# raise the errors if we're not in production +raise_errors = os.environ.get("SECUREDROP_ENV", "prod") != "prod" + +try: + from models import ARGON2_PARAMS + from passphrases import PassphraseGenerator +except: # noqa + if raise_errors: + raise + + +# revision identifiers, used by Alembic. +revision = '2e24fc7536e8' +down_revision = 'de00920916bf' +branch_labels = None +depends_on = None + + +def generate_passphrase_hash() -> str: + passphrase = PassphraseGenerator.get_default().generate_passphrase() + return argon2.using(**ARGON2_PARAMS).hash(passphrase) + + +def create_deleted() -> int: + """manually insert a "deleted" journalist user. + + We need to do it this way since the model will reflect the current state of + the schema, not what it is at the current migration step + + It should be basically identical to what Journalist.get_deleted() does + """ + op.execute(sa.text( + """\ + INSERT INTO journalists (uuid, username, session_nonce, passphrase_hash, otp_secret) + VALUES (:uuid, "deleted", 0, :passphrase_hash, :otp_secret); + """ + ).bindparams( + uuid=str(uuid.uuid4()), + passphrase_hash=generate_passphrase_hash(), + otp_secret=pyotp.random_base32(), + )) + # Get the autoincrement ID back + conn = op.get_bind() + result = conn.execute('SELECT id FROM journalists WHERE username="deleted";').fetchall() + return result[0][0] + + +def migrate_nulls(): + """migrate existing journalist_id=NULL over to deleted or delete them""" + op.execute("DELETE FROM journalist_login_attempt WHERE journalist_id IS NULL;") + op.execute("DELETE FROM revoked_tokens WHERE journalist_id IS NULL;") + # Look to see if we have data to migrate + tables = ('replies', 'seen_files', 'seen_messages', 'seen_replies') + needs_migration = [] + conn = op.get_bind() + for table in tables: + result = conn.execute(f'SELECT 1 FROM {table} WHERE journalist_id IS NULL;').first() # nosec + if result is not None: + needs_migration.append(table) + + if not needs_migration: + return + + deleted_id = create_deleted() + for table in needs_migration: + # The seen_ tables have UNIQUE(fk_id, journalist_id), so the deleted journalist can only have + # seen each item once. It is possible multiple NULL journalist have seen the same thing so we + # do this update in two passes. + # First we update as many rows to point to the deleted journalist as possible, ignoring any + # unique key violations. + op.execute(sa.text( + f'UPDATE OR IGNORE {table} SET journalist_id=:journalist_id WHERE journalist_id IS NULL;' + ).bindparams(journalist_id=deleted_id)) + # Then we delete any leftovers which had been ignored earlier. + op.execute(f'DELETE FROM {table} WHERE journalist_id IS NULL') # nosec + + +def upgrade(): + migrate_nulls() + + with op.batch_alter_table('journalist_login_attempt', schema=None) as batch_op: + batch_op.alter_column('journalist_id', + existing_type=sa.INTEGER(), + nullable=False) + + with op.batch_alter_table('replies', schema=None) as batch_op: + batch_op.alter_column('journalist_id', + existing_type=sa.INTEGER(), + nullable=False) + + with op.batch_alter_table('revoked_tokens', schema=None) as batch_op: + batch_op.alter_column('journalist_id', + existing_type=sa.INTEGER(), + nullable=False) + + with op.batch_alter_table('seen_files', schema=None) as batch_op: + batch_op.alter_column('journalist_id', + existing_type=sa.INTEGER(), + nullable=False) + + with op.batch_alter_table('seen_messages', schema=None) as batch_op: + batch_op.alter_column('journalist_id', + existing_type=sa.INTEGER(), + nullable=False) + + with op.batch_alter_table('seen_replies', schema=None) as batch_op: + batch_op.alter_column('journalist_id', + existing_type=sa.INTEGER(), + nullable=False) + + +def downgrade(): + # We do not un-migrate the data back to journalist_id=NULL + + with op.batch_alter_table('seen_replies', schema=None) as batch_op: + batch_op.alter_column('journalist_id', + existing_type=sa.INTEGER(), + nullable=True) + + with op.batch_alter_table('seen_messages', schema=None) as batch_op: + batch_op.alter_column('journalist_id', + existing_type=sa.INTEGER(), + nullable=True) + + with op.batch_alter_table('seen_files', schema=None) as batch_op: + batch_op.alter_column('journalist_id', + existing_type=sa.INTEGER(), + nullable=True) + + with op.batch_alter_table('revoked_tokens', schema=None) as batch_op: + batch_op.alter_column('journalist_id', + existing_type=sa.INTEGER(), + nullable=True) + + with op.batch_alter_table('replies', schema=None) as batch_op: + batch_op.alter_column('journalist_id', + existing_type=sa.INTEGER(), + nullable=True) + + with op.batch_alter_table('journalist_login_attempt', schema=None) as batch_op: + batch_op.alter_column('journalist_id', + existing_type=sa.INTEGER(), + nullable=True) diff --git a/securedrop/journalist_app/admin.py b/securedrop/journalist_app/admin.py index 57fb81c457..20541f4c4d 100644 --- a/securedrop/journalist_app/admin.py +++ b/securedrop/journalist_app/admin.py @@ -30,7 +30,7 @@ def make_blueprint(config: SDConfig) -> Blueprint: @view.route('/', methods=('GET', 'POST')) @admin_required def index() -> str: - users = Journalist.query.all() + users = Journalist.query.filter(Journalist.username != "deleted").all() return render_template("admin.html", users=users) @view.route('/config', methods=('GET', 'POST')) @@ -288,16 +288,22 @@ def delete_user(user_id: int) -> werkzeug.Response: current_app.logger.error( "Admin {} tried to delete itself".format(g.user.username)) abort(403) - elif user: - db.session.delete(user) - db.session.commit() - flash(gettext("Deleted user '{user}'.").format( - user=user.username), "notification") - else: + elif not user: current_app.logger.error( "Admin {} tried to delete nonexistent user with pk={}".format( g.user.username, user_id)) abort(404) + elif user.is_deleted_user(): + # Do not flash because the interface does not expose this. + # It can only happen by manually crafting a POST request + current_app.logger.error( + "Admin {} tried to delete \"deleted\" user".format(g.user.username)) + abort(403) + else: + user.delete() + db.session.commit() + flash(gettext("Deleted user '{user}'.").format( + user=user.username), "notification") return redirect(url_for('admin.index')) diff --git a/securedrop/loaddata.py b/securedrop/loaddata.py index a9ed87d958..3166759681 100755 --- a/securedrop/loaddata.py +++ b/securedrop/loaddata.py @@ -382,7 +382,7 @@ def load(args: argparse.Namespace) -> None: # delete one journalist _, _, journalist_to_be_deleted = journalists - db.session.delete(journalist_to_be_deleted) + journalist_to_be_deleted.delete() db.session.commit() diff --git a/securedrop/models.py b/securedrop/models.py index 9fec05e101..75845da52f 100644 --- a/securedrop/models.py +++ b/securedrop/models.py @@ -30,6 +30,7 @@ from pyotp import TOTP, HOTP from encryption import EncryptionManager, GpgKeyNotFoundError +from passphrases import PassphraseGenerator from store import Storage _default_instance_config: Optional["InstanceConfig"] = None @@ -256,7 +257,7 @@ class Reply(db.Model): id = Column(Integer, primary_key=True) uuid = Column(String(36), unique=True, nullable=False) - journalist_id = Column(Integer, ForeignKey('journalists.id')) + journalist_id = Column(Integer, ForeignKey('journalists.id'), nullable=False) journalist = relationship( "Journalist", backref=backref( @@ -296,18 +297,8 @@ def __repr__(self) -> str: return '' % (self.filename) def to_json(self) -> 'Dict[str, Any]': - journalist_username = "deleted" - journalist_first_name = "" - journalist_last_name = "" - journalist_uuid = "deleted" - if self.journalist: - journalist_username = self.journalist.username - journalist_first_name = self.journalist.first_name - journalist_last_name = self.journalist.last_name - journalist_uuid = self.journalist.uuid seen_by = [ r.journalist.uuid for r in SeenReply.query.filter(SeenReply.reply_id == self.id) - if r.journalist ] json_reply = { 'source_url': url_for('api.single_source', @@ -317,10 +308,10 @@ def to_json(self) -> 'Dict[str, Any]': reply_uuid=self.uuid) if self.source else None, 'filename': self.filename, 'size': self.size, - 'journalist_username': journalist_username, - 'journalist_first_name': journalist_first_name, - 'journalist_last_name': journalist_last_name, - 'journalist_uuid': journalist_uuid, + 'journalist_username': self.journalist.username, + 'journalist_first_name': self.journalist.first_name or '', + 'journalist_last_name': self.journalist.last_name or '', + 'journalist_uuid': self.journalist.uuid, 'uuid': self.uuid, 'is_deleted_by_source': self.deleted_by_source, 'seen_by': seen_by @@ -436,10 +427,17 @@ class Journalist(db.Model): ) # type: Column[Optional[datetime.datetime]] last_access = Column(DateTime) # type: Column[Optional[datetime.datetime]] passphrase_hash = Column(String(256)) # type: Column[Optional[str]] + login_attempts = relationship( "JournalistLoginAttempt", - backref="journalist" + backref="journalist", + cascade="all, delete" ) # type: RelationshipProperty[JournalistLoginAttempt] + revoked_tokens = relationship( + "RevokedToken", + backref="journalist", + cascade="all, delete" + ) # type: RelationshipProperty[RevokedToken] MIN_USERNAME_LEN = 3 MIN_NAME_LEN = 0 @@ -713,8 +711,7 @@ def login(cls, except NoResultFound: raise InvalidUsernameException(gettext("Invalid username")) - if user.username in Journalist.INVALID_USERNAMES and \ - user.uuid in Journalist.INVALID_USERNAMES: + if user.username in Journalist.INVALID_USERNAMES: raise InvalidUsernameException(gettext("Invalid username")) if len(user.otp_secret) < OTP_SECRET_MIN_ASCII_LENGTH: @@ -786,13 +783,84 @@ def to_json(self, all_info: bool = True) -> Dict[str, Any]: return json_user + def is_deleted_user(self) -> bool: + """Is this the special "deleted" user managed by the system?""" + return self.username == "deleted" + + @classmethod + def get_deleted(cls) -> "Journalist": + """Get a system user that represents deleted journalists for referential integrity + + Callers must commit the session themselves + """ + deleted = Journalist.query.filter_by(username='deleted').one_or_none() + if deleted is None: + # Lazily create + deleted = cls( + # Use a placeholder username to bypass validation that would reject + # "deleted" as unusable + username="placeholder", + # We store a randomly generated passphrase for this account that is + # never revealed to anyone. + password=PassphraseGenerator.get_default().generate_passphrase() + ) + deleted.username = "deleted" + db.session.add(deleted) + return deleted + + def delete(self) -> None: + """delete a journalist, migrating some data over to the "deleted" journalist + + Callers must commit the session themselves + """ + deleted = self.get_deleted() + # All replies should be reassociated with the "deleted" journalist + for reply in Reply.query.filter_by(journalist_id=self.id).all(): + reply.journalist_id = deleted.id + db.session.add(reply) + + # For seen indicators, we need to make sure one doesn't already exist + # otherwise it'll hit a unique key conflict + already_seen_files = { + file.file_id for file in + SeenFile.query.filter_by(journalist_id=deleted.id).all()} + for file in SeenFile.query.filter_by(journalist_id=self.id).all(): + if file.file_id in already_seen_files: + db.session.delete(file) + else: + file.journalist_id = deleted.id + db.session.add(file) + + already_seen_messages = { + message.message_id for message in + SeenMessage.query.filter_by(journalist_id=deleted.id).all()} + for message in SeenMessage.query.filter_by(journalist_id=self.id).all(): + if message.message_id in already_seen_messages: + db.session.delete(message) + else: + message.journalist_id = deleted.id + db.session.add(message) + + already_seen_replies = { + reply.reply_id for reply in + SeenReply.query.filter_by(journalist_id=deleted.id).all()} + for reply in SeenReply.query.filter_by(journalist_id=self.id).all(): + if reply.reply_id in already_seen_replies: + db.session.delete(reply) + else: + reply.journalist_id = deleted.id + db.session.add(reply) + + # For the rest of the associated data we rely on cascading deletions + db.session.delete(self) + class SeenFile(db.Model): __tablename__ = "seen_files" __table_args__ = (db.UniqueConstraint("file_id", "journalist_id"),) id = Column(Integer, primary_key=True) file_id = Column(Integer, ForeignKey("submissions.id"), nullable=False) - journalist_id = Column(Integer, ForeignKey("journalists.id"), nullable=True) + journalist_id = Column(Integer, ForeignKey("journalists.id"), nullable=False) file = relationship( "Submission", backref=backref("seen_files", lazy="dynamic", cascade="all,delete") ) # type: RelationshipProperty[Submission] @@ -806,7 +874,7 @@ class SeenMessage(db.Model): __table_args__ = (db.UniqueConstraint("message_id", "journalist_id"),) id = Column(Integer, primary_key=True) message_id = Column(Integer, ForeignKey("submissions.id"), nullable=False) - journalist_id = Column(Integer, ForeignKey("journalists.id"), nullable=True) + journalist_id = Column(Integer, ForeignKey("journalists.id"), nullable=False) message = relationship( "Submission", backref=backref("seen_messages", lazy="dynamic", cascade="all,delete") ) # type: RelationshipProperty[Submission] @@ -820,7 +888,7 @@ class SeenReply(db.Model): __table_args__ = (db.UniqueConstraint("reply_id", "journalist_id"),) id = Column(Integer, primary_key=True) reply_id = Column(Integer, ForeignKey("replies.id"), nullable=False) - journalist_id = Column(Integer, ForeignKey("journalists.id"), nullable=True) + journalist_id = Column(Integer, ForeignKey("journalists.id"), nullable=False) reply = relationship( "Reply", backref=backref("seen_replies", cascade="all,delete") ) # type: RelationshipProperty[Reply] @@ -840,7 +908,7 @@ class JournalistLoginAttempt(db.Model): DateTime, default=datetime.datetime.utcnow ) # type: Column[Optional[datetime.datetime]] - journalist_id = Column(Integer, ForeignKey('journalists.id')) + journalist_id = Column(Integer, ForeignKey('journalists.id'), nullable=False) def __init__(self, journalist: Journalist) -> None: self.journalist = journalist @@ -855,7 +923,7 @@ class RevokedToken(db.Model): __tablename__ = 'revoked_tokens' id = Column(Integer, primary_key=True) - journalist_id = Column(Integer, ForeignKey('journalists.id')) + journalist_id = Column(Integer, ForeignKey('journalists.id'), nullable=False) token = db.Column(db.Text, nullable=False, unique=True) diff --git a/securedrop/tests/migrations/migration_2e24fc7536e8.py b/securedrop/tests/migrations/migration_2e24fc7536e8.py new file mode 100644 index 0000000000..2bdcd0b5f2 --- /dev/null +++ b/securedrop/tests/migrations/migration_2e24fc7536e8.py @@ -0,0 +1,113 @@ +import uuid + +from sqlalchemy import text + +from db import db +from journalist_app import create_app +from .helpers import random_datetime + + +class UpgradeTester: + """Insert a Reply, SeenReply and JournalistLoginAttempt with journalist_id=NULL. + Verify that the first two are reassociated to the "Deleted" user, while the last + is deleted outright. + """ + + def __init__(self, config): + """This function MUST accept an argument named `config`. + You will likely want to save a reference to the config in your + class, so you can access the database later. + """ + self.config = config + self.app = create_app(config) + + def load_data(self): + """This function loads data into the database and filesystem. It is + executed before the upgrade. + """ + with self.app.app_context(): + params = { + 'uuid': str(uuid.uuid4()), + 'journalist_id': None, + 'source_id': 0, + 'filename': 'dummy.txt', + 'size': 1, + 'checksum': '', + 'deleted_by_source': False, + } + sql = """\ + INSERT INTO replies (uuid, journalist_id, source_id, filename, + size, checksum, deleted_by_source) + VALUES (:uuid, :journalist_id, :source_id, :filename, + :size, :checksum, :deleted_by_source);""" + db.engine.execute(text(sql), **params) + # Insert two SeenReplys corresponding to the just-inserted reply, which also + # verifies our handling of duplicate keys. + for _ in range(2): + db.engine.execute(text( + """\ + INSERT INTO seen_replies (reply_id, journalist_id) + VALUES (1, NULL); + """ + )) + # Insert a JournalistLoginAttempt + db.engine.execute(text( + """\ + INSERT INTO journalist_login_attempt (timestamp, journalist_id) + VALUES (:timestamp, NULL) + """ + ), timestamp=random_datetime(nullable=False)) + + def check_upgrade(self): + """This function is run after the upgrade and verifies the state + of the database or filesystem. It MUST raise an exception if the + check fails. + """ + with self.app.app_context(): + deleted = db.engine.execute( + 'SELECT id, passphrase_hash, otp_secret FROM journalists WHERE username="deleted"' + ).first() + # A passphrase_hash is set + assert deleted[1].startswith('$argon2') + # And a TOTP secret is set + assert len(deleted[2]) == 32 + deleted_id = deleted[0] + replies = db.engine.execute( + text('SELECT journalist_id FROM replies')).fetchall() + assert len(replies) == 1 + # And the journalist_id matches our "deleted" one + assert replies[0][0] == deleted_id + seen_replies = db.engine.execute( + text('SELECT journalist_id FROM seen_replies')).fetchall() + assert len(seen_replies) == 1 + # And the journalist_id matches our "deleted" one + assert seen_replies[0][0] == deleted_id + login_attempts = db.engine.execute( + text('SELECT * FROM journalist_login_attempt')).fetchall() + # The NULL login attempt was deleted outright + assert login_attempts == [] + + +class DowngradeTester: + """Downgrading only makes fields nullable again, which is a + non-destructive and safe operation""" + + def __init__(self, config): + """This function MUST accept an argument named `config`. + You will likely want to save a reference to the config in your + class, so you can access the database later. + """ + self.config = config + + def load_data(self): + """This function loads data into the database and filesystem. It is + executed before the downgrade. + """ + pass + + def check_downgrade(self): + """This function is run after the downgrade and verifies the state + of the database or filesystem. It MUST raise an exception if the + check fails. + """ + pass diff --git a/securedrop/tests/test_journalist.py b/securedrop/tests/test_journalist.py index 16e0d7c106..6ea13eb7a2 100644 --- a/securedrop/tests/test_journalist.py +++ b/securedrop/tests/test_journalist.py @@ -31,6 +31,7 @@ InvalidPasswordLength, InstanceConfig, Journalist, + JournalistLoginAttempt, Reply, SeenFile, SeenMessage, @@ -39,6 +40,7 @@ InvalidUsernameException, Submission ) +from passphrases import PassphraseGenerator from sdconfig import config from .utils.instrument import InstrumentedApp @@ -647,6 +649,19 @@ def test_admin_deletes_invalid_user_404(journalist_app, test_admin): assert resp.status_code == 404 +def test_admin_deletes_deleted_user_403(journalist_app, test_admin): + with journalist_app.app_context(): + deleted = Journalist.get_deleted() + db.session.commit() + deleted_id = deleted.id + + with journalist_app.test_client() as app: + _login_user(app, test_admin['username'], test_admin['password'], + test_admin['otp_secret']) + resp = app.post(url_for('admin.delete_user', user_id=deleted_id)) + assert resp.status_code == 403 + + @flaky(rerun_filter=utils.flaky_filter_xfail) @pytest.mark.parametrize("locale", get_test_locales()) def test_admin_edits_user_password_error_response( @@ -1377,17 +1392,12 @@ def test_admin_add_user_with_invalid_username(config, journalist_app, test_admin @flaky(rerun_filter=utils.flaky_filter_xfail) @pytest.mark.parametrize("locale", get_test_locales()) def test_deleted_user_cannot_login(config, journalist_app, locale): - username = 'deleted' - uuid = 'deleted' - - # Create a user with username and uuid as deleted with journalist_app.app_context(): - user, password = utils.db_helper.init_journalist(is_admin=False) - otp_secret = user.otp_secret - user.username = username - user.uuid = uuid - db.session.add(user) + user = Journalist.get_deleted() + password = PassphraseGenerator.get_default().generate_passphrase() + user.set_password(password) db.session.commit() + otp_secret = user.otp_secret with InstrumentedApp(journalist_app) as ins: # Verify that deleted user is not able to login @@ -1395,9 +1405,9 @@ def test_deleted_user_cannot_login(config, journalist_app, locale): resp = app.post( url_for('main.login', l=locale), data=dict( - username=username, + username="deleted", password=password, - token=otp_secret + token=TOTP(otp_secret).now() ), ) assert page_language(resp.data) == language_tag(locale) @@ -1421,21 +1431,16 @@ def test_deleted_user_cannot_login(config, journalist_app, locale): @flaky(rerun_filter=utils.flaky_filter_xfail) @pytest.mark.parametrize("locale", get_test_locales()) def test_deleted_user_cannot_login_exception(journalist_app, locale): - username = 'deleted' - uuid = 'deleted' - - # Create a user with username and uuid as deleted with journalist_app.app_context(): - user, password = utils.db_helper.init_journalist(is_admin=False) - otp_secret = user.otp_secret - user.username = username - user.uuid = uuid - db.session.add(user) + user = Journalist.get_deleted() + password = PassphraseGenerator.get_default().generate_passphrase() + user.set_password(password) db.session.commit() + otp_secret = user.otp_secret with journalist_app.test_request_context('/'): with pytest.raises(InvalidUsernameException): - Journalist.login(username, password, TOTP(otp_secret).now()) + Journalist.login("deleted", password, TOTP(otp_secret).now()) @flaky(rerun_filter=utils.flaky_filter_xfail) @@ -3379,3 +3384,46 @@ def test_app_error_handlers_defined(journalist_app): for status_code in [400, 401, 403, 404, 500]: # This will raise KeyError if an app-wide error handler is not defined assert journalist_app.error_handler_spec[None][status_code] + + +def test_lazy_deleted_journalist_creation(journalist_app): + """test lazy creation of "deleted" jousrnalist works""" + not_found = Journalist.query.filter_by(username='deleted').one_or_none() + assert not_found is None, "deleted journalist doesn't exist yet" + deleted = Journalist.get_deleted() + db.session.commit() + # Can be found as a normal Journalist object + found = Journalist.query.filter_by(username='deleted').one() + assert deleted.uuid == found.uuid + assert found.is_deleted_user() is True + # And get_deleted() now returns the same instance + deleted2 = Journalist.get_deleted() + assert deleted.uuid == deleted2.uuid + + +def test_journalist_deletion(journalist_app, app_storage): + """test deleting a journalist and see data reassociated to "deleted" journalist""" + # Create a journalist that's seen two replies and has a login attempt + source, _ = utils.db_helper.init_source(app_storage) + journalist, _ = utils.db_helper.init_journalist() + db.session.add(JournalistLoginAttempt(journalist)) + replies = utils.db_helper.reply(app_storage, journalist, source, 2) + # Create a second journalist that's seen those replies + journalist2, _ = utils.db_helper.init_journalist() + for reply in replies: + db.session.add(SeenReply(reply=reply, journalist=journalist2)) + db.session.commit() + # Only one login attempt in the table + assert len(JournalistLoginAttempt.query.all()) == 1 + # And four SeenReplys + assert len(SeenReply.query.all()) == 4 + # Delete the journalists + journalist.delete() + journalist2.delete() + db.session.commit() + # Verify the "deleted" journalist has 2 associated rows of both types + deleted = Journalist.get_deleted() + assert len(Reply.query.filter_by(journalist_id=deleted.id).all()) == 2 + assert len(SeenReply.query.filter_by(journalist_id=deleted.id).all()) == 2 + # And there are no login attempts + assert JournalistLoginAttempt.query.all() == [] diff --git a/securedrop/tests/test_journalist_api.py b/securedrop/tests/test_journalist_api.py index 0f0b84756b..50d53d20fc 100644 --- a/securedrop/tests/test_journalist_api.py +++ b/securedrop/tests/test_journalist_api.py @@ -488,9 +488,9 @@ def test_authorized_user_can_get_single_reply(journalist_app, test_files, assert response.json['journalist_uuid'] == \ reply.journalist.uuid assert response.json['journalist_first_name'] == \ - reply.journalist.first_name + (reply.journalist.first_name or '') assert response.json['journalist_last_name'] == \ - reply.journalist.last_name + (reply.journalist.last_name or '') assert response.json['is_deleted_by_source'] is False assert response.json['filename'] == \ test_files['source'].replies[0].filename @@ -508,12 +508,12 @@ def test_reply_of_deleted_journalist(journalist_app, source_uuid=uuid, reply_uuid=reply_uuid), headers=get_api_headers(journalist_api_token)) - + deleted_uuid = Journalist.get_deleted().uuid assert response.status_code == 200 assert response.json['uuid'] == reply_uuid assert response.json['journalist_username'] == "deleted" - assert response.json['journalist_uuid'] == "deleted" + assert response.json['journalist_uuid'] == deleted_uuid assert response.json['journalist_first_name'] == "" assert response.json['journalist_last_name'] == "" assert response.json['is_deleted_by_source'] is False diff --git a/securedrop/tests/utils/db_helper.py b/securedrop/tests/utils/db_helper.py index d203e6c141..26d7a69b3c 100644 --- a/securedrop/tests/utils/db_helper.py +++ b/securedrop/tests/utils/db_helper.py @@ -51,7 +51,7 @@ def delete_journalist(journalist): :returns: None """ - db.session.delete(journalist) + journalist.delete() db.session.commit()