Use special "deleted" journalist for associations with deleted users

Currently when a journalist is deleted, most referential tables are
updated with `journalist_id=NULL`, forcing all clients to accomodate
that case. Instead, we are now going to either re-associate those rows
with a special "deleted" journalist account, or delete them outright.
All journalist_id columns are now NOT NULL to enforce this.

Tables with rows migrated to "deleted":
* replies
* seen_files
* seen_messages
* seen_replies

Tables with rows that are deleted outright:
* journalist_login_attempt
* revoked_tokens

The "deleted" journalist account is a real account that exists in the
database, but cannot be logged into. It is not possible to delete it nor
is it shown in the admin listing of journalists. It is lazily created on
demand using a special DeletedJournalist subclass that bypasses username
validation. For consistency reasons, a randomly generated diceware
passphrase and TOTP secret are set in the database for this account, but
never revealed to anyone.

Because we now have a real user to reference, the api.single_reply
endpoint will return a real UUID instead of the "deleted" placeholder.

Journalist objects must now be deleted by calling the new delete()
function on them. Trying to directly `db.session.delete(journalist)`
will most likely fail with an Exception because of rows that weren't
migrated first.

The migration step looks for any existing rows in those tables with
journalist_id=NULL and either migrates them to "deleted" or deletes
them. Then all the columns are changed to be NOT NULL.

Fixes #6192.

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 DeletedJournalist.get() 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)

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'))
 

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()