Use a special deleted journalist user for associations with deleted users

[eloquence]

As discussed in #5467 and #5503, we currently delete rows for deleted journalist users. After deletion, journalist_id will be NULL in other tables that reference the deleted user (e.g., replies and the seen tables; see the database diagram). The API will return the username deleted and the UUID deleted wherever these users are referenced.

There are a few issues with this:

• It creates an avoidable inconsistency between the data's internal representation and its API representation, which makes it harder to reason about where the data returned by the API originates
• We cannot rely on the /users endpoint of the API to get a complete list of users that an API consumer may need to associate data with
• User management in the SecureDrop Client does not map directly to user management on the server

In #5467, we considered preserving a record for each user, while removing all data about the user. This may be desirable in the long run (alongside features such as user locking: #3926), but even if we do this, we will need a migration for historical data. The solution we've settled on for now is to create a global deleted user. A database migration will need to a) create this user, b) associate records currently associated with NULL users with this new user.

This puts us in a good position to make other changes like #5467 in future, and unblocks further work on SecureDrop Client features that rely on journalist attribution.

[sssoleileraaa]

I think there are two approaches, but first here are some notes (recap):

Notes

• The endpoint /get_all_replies tells us which replies exist on the server (so that we can add, update, or delete reply records accordingly to match the server), and which journalists have seen each reply (so that we can add seen records accordingly- note that we decided early on that we only delete seen records when the source or conversation are deleted).

• The only reason a reply should be updated is because the reply filename, size, or journalist_id has changed. (We can remove the update logic around journalist_id if we go with Option B take below.)

• The endpoint only sends UUIDs (IDs are partial PII), which is why "deleted" is a hard-coded string for journalist_uuid in the API endpoint

• [ISSUE] The endpoint does not hard-code a "deleted" string when returning the list of journalist UUIDs in the seen_by field, which means that a SeenReply record with a NULL journalist_id would exist in the seen_replies table (this is also true for SeenMessage and SeenFile), but the endpoint would never report it. Downstream, the client has to ignore this (usually we we rely on remote data from the API to ensure or confirm that we match the server). Overall, the API is inconsistent with how it treats data associated with deleted users.

Option A

1. Create a deleted user account.
2. When a user account is deleted, reassociate that user's replies and seen* records to point to the deleted user.
3. Do the data migration where journalist_id is NULL to point to the new deleted user account.
4. This automatically fixes the issue described above and the API will work without change. (It should be cleaned up so that we no longer hard-code the "deleted" string.)

Option B

1. On the client, rely on the new /users endpoint to delete user accounts (we need to do this anyway), which will update the replies and seen* tables to match the server with NULL journalist_ids (this is default behaviour for foreign keys).

2. No longer create user accounts when we see new journalist_uuids for replies (again, we were going to do this anyway).

   No client data migration is needed because steps 1 and 2 will ensure that we delete the local "deleted" user account, and journalist_ids will be updated to NULL by default.

3. Fix the issue above by updating the Journalist API so that it returns the same "deleted" string for journalist_uuid in the seen_by field.

[legoktm]

I looked into this a bit, my personal preference would be to have the placeholder "deleted" user (Option A), just so we can rely on journalist_id being type uuid rather than Optional[uuid] (sidenote, MediaWiki takes a similar approach, "deleting" users by merging them into one named "Anonymous").

A bit more concrete proposal: Rather than calling db.session.delete(journalist), we could add a delete(session) method on Journalist, that would scan through whatever tables we want to keep data for, updating the journalist_id to point to the "deleted" user instead. All other models would set cascade="delete" since we want to drop the data on journalist deletion (e.g. journalist_login_attempt, revoked_tokens, etc.). The main downside of this is if someone does use db.session.delete(journalist) we're going to have inconsistent state again.

The migration step would basically do the same thing, look for NULL journalist_ids, either assigning them to the "deleted" user or deleting them.

Another option is to set an event to listen for deletion attempts (https://docs.sqlalchemy.org/en/13/orm/session_events.html), and intercept them to update the seen/reply models to point to "deleted". This seems more much more complex. Also the system was changed in 1.4, so we might have to update/migrate it when upgrading sqlalchemy.

[sssoleileraaa]

Thanks for joining the conversation @legoktm! Option A is part of what I've been advocating for for at least a year to resolve downstream issues (the other part being: #5467). The reason I mention Option B is that it allows us to avoid the data migration and to continue relying on the DB to null out the journalist IDs upon account deletion. It is worth considering in case more issues come up with Option A. Either way, Read Receipts is partially unblocked, so I'll head over that way while you work on this Github Issue and iron out even more details with @zenmonkeykstop.

[legoktm]

I pushed my current WIP to the "delete-journalist" branch, so far it lazily creates a "deleted" journalist and reassigns the seen rows to it upon deletion (or so I think, I'll do more testing tomorrow).