Compare commits
2 Commits
313dc401ae
...
3fc022335c
| Author | SHA1 | Date | |
|---|---|---|---|
| 3fc022335c | |||
|
|
c9c549ed22 |
@ -74,6 +74,24 @@ def transfer_ownership(db: sqlite3.Connection, owner_id: int, target_id: int) ->
|
||||
raise
|
||||
|
||||
|
||||
# Every column pointing at members(id) that does not cascade has to be dealt
|
||||
# with before the row can go, or SQLite refuses the delete and the admin gets a
|
||||
# 500 with nothing to read. These two sets are the list, and
|
||||
# test_members.py checks them against the schema's actual foreign keys — so a
|
||||
# table added later fails a test here rather than a button in production. That
|
||||
# is not hypothetical: `attachments` was added and forgotten, and the first
|
||||
# person who tried to remove a member who had posted a photo got the 500.
|
||||
REASSIGNED_ON_ERASE = {
|
||||
("threads", "author_id"),
|
||||
("comments", "author_id"),
|
||||
# The picture belongs to the thread, which survives as "Miembro eliminado",
|
||||
# so it is reassigned rather than deleted, exactly like the words around it.
|
||||
# Removing the picture itself means deleting the post it is attached to.
|
||||
("attachments", "uploaded_by"),
|
||||
}
|
||||
CLEARED_ON_ERASE = {("members", "created_by")}
|
||||
|
||||
|
||||
def erase_member(db: sqlite3.Connection, member_id: int) -> None:
|
||||
"""Remove a member and their personal data, keeping the conversation intact.
|
||||
|
||||
@ -84,9 +102,15 @@ def erase_member(db: sqlite3.Connection, member_id: int) -> None:
|
||||
ghost = tombstone_id(db)
|
||||
db.execute("BEGIN IMMEDIATE")
|
||||
try:
|
||||
db.execute("UPDATE threads SET author_id = ? WHERE author_id = ?", (ghost, member_id))
|
||||
db.execute("UPDATE comments SET author_id = ? WHERE author_id = ?", (ghost, member_id))
|
||||
db.execute("UPDATE members SET created_by = NULL WHERE created_by = ?", (member_id,))
|
||||
# Table and column names come from the module constants above, never
|
||||
# from a request, so the interpolation is not a place user input can
|
||||
# reach.
|
||||
for table, column in sorted(REASSIGNED_ON_ERASE):
|
||||
db.execute(f"UPDATE {table} SET {column} = ? WHERE {column} = ?",
|
||||
(ghost, member_id))
|
||||
for table, column in sorted(CLEARED_ON_ERASE):
|
||||
db.execute(f"UPDATE {table} SET {column} = NULL WHERE {column} = ?",
|
||||
(member_id,))
|
||||
db.execute("DELETE FROM members WHERE id = ? AND role != 'owner'", (member_id,))
|
||||
db.execute("COMMIT")
|
||||
except Exception:
|
||||
|
||||
@ -227,3 +227,61 @@ def test_ticking_it_anyway_still_creates_nothing(app, client, db, post, owner_id
|
||||
|
||||
assert "GITEA_ADMIN_TOKEN" in response.get_data(as_text=True)
|
||||
assert db.execute("SELECT 1 FROM members WHERE gitea_login = 'maria'").fetchone() is None
|
||||
|
||||
|
||||
# --- erasing somebody who has left traces --------------------------------
|
||||
|
||||
def test_erasing_a_member_who_posted_a_picture(app, db, post, owner_id, make_member, sign_in):
|
||||
"""This failed in production with a 500 the first time it was tried.
|
||||
|
||||
`attachments.uploaded_by` is NOT NULL and does not cascade, so with a
|
||||
picture in the database SQLite refuses to delete the member — and the admin
|
||||
sees "Internal Server Error" with nothing to act on."""
|
||||
author = make_member("saliente")
|
||||
db.execute("INSERT INTO threads (author_id, title, body_md) VALUES (?, 'Hola', 'Texto')",
|
||||
(author,))
|
||||
db.execute(
|
||||
"""INSERT INTO attachments
|
||||
(thread_id, stored_name, original_name, content_type, bytes, uploaded_by)
|
||||
VALUES (1, 'foto-abc123abc123.jpg', 'foto.jpg', 'image/jpeg', 10, ?)""",
|
||||
(author,),
|
||||
)
|
||||
sign_in(owner_id)
|
||||
|
||||
response = post(f"/comunidad/miembros/{author}/eliminar")
|
||||
|
||||
assert response.status_code == 302
|
||||
assert db.execute("SELECT 1 FROM members WHERE id = ?", (author,)).fetchone() is None
|
||||
# The picture stays with the thread, which survives as "Miembro eliminado" —
|
||||
# the same rule the words follow. Deleting the post removes the picture.
|
||||
row = db.execute(
|
||||
"""SELECT m.display_name FROM attachments a JOIN members m ON m.id = a.uploaded_by"""
|
||||
).fetchone()
|
||||
assert row["display_name"] == "Miembro eliminado"
|
||||
|
||||
|
||||
def test_every_table_pointing_at_members_is_accounted_for(db):
|
||||
"""The guard that makes the bug above unrepeatable.
|
||||
|
||||
Read out of the live schema rather than written down twice: any future
|
||||
table with a foreign key to members(id) either cascades, or is named in
|
||||
members.py's erase lists. Otherwise erasure breaks — and it breaks at the
|
||||
moment somebody exercises a right they are entitled to, which is the worst
|
||||
possible time to find out."""
|
||||
from apps.board.members import CLEARED_ON_ERASE, REASSIGNED_ON_ERASE
|
||||
handled = REASSIGNED_ON_ERASE | CLEARED_ON_ERASE
|
||||
|
||||
tables = [row["name"] for row in db.execute(
|
||||
"SELECT name FROM sqlite_master WHERE type = 'table' AND name NOT LIKE 'sqlite_%'")]
|
||||
unhandled = [
|
||||
f"{table}.{fk['from']}"
|
||||
for table in tables
|
||||
for fk in db.execute(f"PRAGMA foreign_key_list('{table}')").fetchall()
|
||||
if fk["table"] == "members"
|
||||
and (fk["on_delete"] or "").upper() != "CASCADE"
|
||||
and (table, fk["from"]) not in handled
|
||||
]
|
||||
assert not unhandled, (
|
||||
"these point at members(id), do not cascade, and erase_member does not "
|
||||
f"touch them, so erasing a member will fail: {unhandled}"
|
||||
)
|
||||
|
||||
Loading…
Reference in New Issue
Block a user