Deleting a member returned 500. The cause is mine and hours old: attachments.uploaded_by is NOT NULL and does not cascade, so once somebody has posted a photo SQLite refuses to delete their row. erase_member knew about threads, comments and created_by — every table that existed when it was written — and Phase B added a fourth without telling it. Reproduced before fixing: a member with a thread erases cleanly, the same member with an attachment raises FOREIGN KEY constraint failed. The picture is reassigned to the tombstone rather than deleted, the same rule the words around it already follow: the thread survives as "Miembro eliminado" and keeps its shape. Removing the picture means deleting the post it hangs off. The lists of columns to reassign and to clear are now module constants that erase_member iterates, and a test reads the live schema with PRAGMA foreign_key_list and fails if any table points at members(id), does not cascade, and is not in them. Checked against the pre-fix lists: it names attachments.uploaded_by. The next table to be added will fail a test instead of a button — and this one failed at the moment somebody exercised a right they are entitled to, which is the worst time to find out. 201 tests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NizVpJ2dwzCbjCrTLCjeHn
288 lines
12 KiB
Python
288 lines
12 KiB
Python
"""The role rules, from the predicates up to the routes that enforce them."""
|
|
|
|
from __future__ import annotations
|
|
|
|
import sqlite3
|
|
|
|
import pytest
|
|
|
|
from apps.board import gitea
|
|
from apps.board.members import may_create, may_manage
|
|
|
|
|
|
# --- the rules as plain functions ---------------------------------------
|
|
|
|
def test_only_the_owner_makes_admins():
|
|
assert may_create("owner", "admin")
|
|
assert not may_create("admin", "admin")
|
|
assert not may_create("user", "admin")
|
|
|
|
|
|
def test_admins_and_the_owner_make_users():
|
|
assert may_create("owner", "user")
|
|
assert may_create("admin", "user")
|
|
assert not may_create("user", "user")
|
|
|
|
|
|
def test_the_owner_is_beyond_everyone_including_themselves():
|
|
assert not may_manage("owner", "owner")
|
|
assert not may_manage("admin", "owner")
|
|
|
|
|
|
def test_only_the_owner_manages_admins():
|
|
assert may_manage("owner", "admin")
|
|
assert not may_manage("admin", "admin")
|
|
|
|
|
|
# --- the database holds the line ----------------------------------------
|
|
|
|
def test_a_second_owner_is_impossible(db):
|
|
"""Not a route check — a direct insert, because the point of the partial
|
|
unique index is to survive a bug in the code above it."""
|
|
with pytest.raises(sqlite3.IntegrityError):
|
|
db.execute(
|
|
"INSERT INTO members (gitea_login, role) VALUES ('usurpador', 'owner')"
|
|
)
|
|
|
|
|
|
def test_transfer_leaves_exactly_one_owner(app, db, owner_id, make_member):
|
|
from apps.board.members import transfer_ownership
|
|
admin_id = make_member("segunda", role="admin")
|
|
transfer_ownership(db, owner_id, admin_id)
|
|
|
|
owners = db.execute("SELECT id FROM members WHERE role = 'owner'").fetchall()
|
|
assert [row["id"] for row in owners] == [admin_id]
|
|
assert db.execute("SELECT role FROM members WHERE id = ?",
|
|
(owner_id,)).fetchone()["role"] == "admin"
|
|
|
|
|
|
# --- the routes ----------------------------------------------------------
|
|
|
|
def test_admin_cannot_create_an_admin(client, post, make_member, sign_in):
|
|
sign_in(make_member("admina", role="admin"))
|
|
response = post("/comunidad/miembros/nuevo", {
|
|
"login": "nueva", "display_name": "Nueva", "email": "n@example.com",
|
|
"role": "admin", "create_account": "",
|
|
})
|
|
assert response.status_code == 403
|
|
|
|
|
|
def test_owner_can_create_an_admin(client, db, post, owner_id, sign_in):
|
|
sign_in(owner_id)
|
|
post("/comunidad/miembros/nuevo", {
|
|
"login": "nueva", "display_name": "Nueva", "email": "n@example.com",
|
|
"role": "admin", "create_account": "",
|
|
})
|
|
row = db.execute("SELECT role FROM members WHERE gitea_login = 'nueva'").fetchone()
|
|
assert row["role"] == "admin"
|
|
|
|
|
|
def test_a_plain_user_cannot_reach_the_admin_screens(client, make_member, sign_in):
|
|
sign_in(make_member("cualquiera"))
|
|
assert client.get("/comunidad/miembros/nuevo").status_code == 403
|
|
|
|
|
|
def test_the_owner_cannot_be_suspended(client, post, owner_id, make_member, sign_in):
|
|
sign_in(make_member("admina", role="admin"))
|
|
response = post(f"/comunidad/miembros/{owner_id}/estado", {"active": "0"})
|
|
assert response.status_code == 403
|
|
|
|
|
|
def test_the_owner_cannot_suspend_themselves(client, post, owner_id, sign_in):
|
|
sign_in(owner_id)
|
|
response = post(f"/comunidad/miembros/{owner_id}/estado", {"active": "0"})
|
|
assert response.status_code == 403
|
|
|
|
|
|
def test_an_admin_cannot_demote_another_admin(client, post, make_member, sign_in):
|
|
other = make_member("otra", role="admin")
|
|
sign_in(make_member("admina", role="admin"))
|
|
response = post(f"/comunidad/miembros/{other}/rol", {"role": "user"})
|
|
assert response.status_code == 403
|
|
|
|
|
|
def test_the_password_is_never_shown_to_the_admin(
|
|
client, monkeypatch, post, owner_id, sign_in):
|
|
"""It used to be, printed once for the admin to pass on. Now the member is
|
|
emailed a link and chooses their own, so the generated password exists only
|
|
to keep the Gitea account from being reachable before they do — and nobody,
|
|
the admin included, ever learns it."""
|
|
created = {}
|
|
monkeypatch.setattr(gitea, "admin_create_user",
|
|
lambda login, email, name, password: created.update(
|
|
login=login, password=password))
|
|
monkeypatch.setattr("apps.board.mail.send", lambda to, subject, body: None)
|
|
sign_in(owner_id)
|
|
|
|
response = post("/comunidad/miembros/nuevo", {
|
|
"login": "maria", "display_name": "María", "email": "m@example.com",
|
|
"role": "user", "create_account": "on",
|
|
})
|
|
|
|
assert created["login"] == "maria"
|
|
assert created["password"].encode() not in response.data
|
|
|
|
|
|
def test_a_rejected_gitea_call_creates_no_member(client, monkeypatch, db, post, owner_id, sign_in):
|
|
def boom(*args, **kwargs):
|
|
raise gitea.GiteaError("Ese usuario ya existe en Gitea.")
|
|
monkeypatch.setattr(gitea, "admin_create_user", boom)
|
|
sign_in(owner_id)
|
|
post("/comunidad/miembros/nuevo", {
|
|
"login": "maria", "display_name": "María", "email": "m@example.com",
|
|
"role": "user", "create_account": "on",
|
|
})
|
|
assert db.execute("SELECT 1 FROM members WHERE gitea_login = 'maria'").fetchone() is None
|
|
|
|
|
|
def test_erasing_a_member_keeps_their_threads_readable(app, db, post, owner_id, make_member, sign_in):
|
|
author = make_member("saliente")
|
|
db.execute("INSERT INTO threads (author_id, title, body_md) VALUES (?, 'Hola', 'Texto')",
|
|
(author,))
|
|
sign_in(owner_id)
|
|
post(f"/comunidad/miembros/{author}/eliminar")
|
|
|
|
assert db.execute("SELECT 1 FROM members WHERE id = ?", (author,)).fetchone() is None
|
|
row = db.execute(
|
|
"""SELECT m.display_name FROM threads t JOIN members m ON m.id = t.author_id
|
|
WHERE t.title = 'Hola'"""
|
|
).fetchone()
|
|
assert row["display_name"] == "Miembro eliminado"
|
|
|
|
|
|
def test_the_member_list_renders_for_each_role(client, db, owner_id, make_member, sign_in):
|
|
"""Every role takes a different branch through members.html — the owner
|
|
sees transfer and erase, an admin sees suspend, a user sees neither — so
|
|
each one is rendered here rather than trusted."""
|
|
admin_id = make_member("admina", role="admin")
|
|
user_id = make_member("usuaria")
|
|
|
|
for member_id, expected in ((owner_id, "Transferir titularidad"),
|
|
(admin_id, "Suspender"),
|
|
(user_id, None)):
|
|
sign_in(member_id)
|
|
body = client.get("/comunidad/miembros").get_data(as_text=True)
|
|
assert "usuaria" in body
|
|
if expected:
|
|
assert expected in body
|
|
else:
|
|
assert "Transferir titularidad" not in body
|
|
assert "Suspender" not in body
|
|
|
|
|
|
def test_the_new_member_form_hides_the_role_choice_from_admins(
|
|
client, owner_id, make_member, sign_in):
|
|
sign_in(owner_id)
|
|
assert "Administrador" in client.get("/comunidad/miembros/nuevo").get_data(as_text=True)
|
|
|
|
sign_in(make_member("admina", role="admin"))
|
|
body = client.get("/comunidad/miembros/nuevo").get_data(as_text=True)
|
|
assert 'value="admin"' not in body
|
|
|
|
|
|
def test_the_export_is_only_your_own_writing(client, db, make_member, sign_in):
|
|
mine = make_member("mia")
|
|
theirs = make_member("suya")
|
|
db.execute("INSERT INTO threads (author_id, title, body_md) VALUES (?, 'Mío', 'A')", (mine,))
|
|
db.execute("INSERT INTO threads (author_id, title, body_md) VALUES (?, 'Suyo', 'B')", (theirs,))
|
|
sign_in(mine)
|
|
|
|
body = client.get("/comunidad/mis-datos").get_data(as_text=True)
|
|
assert "Mío" in body
|
|
assert "Suyo" not in body
|
|
|
|
|
|
def test_the_form_says_so_when_accounts_cannot_be_created(app, client, owner_id, sign_in):
|
|
"""Without a site-admin token the server cannot create Gitea accounts. That
|
|
is the documented safer setup, not a fault — but it has to be said before
|
|
someone fills the form, not after they submit it."""
|
|
app.config["ADMIN_TOKEN"] = ""
|
|
sign_in(owner_id)
|
|
|
|
body = client.get("/comunidad/miembros/nuevo").get_data(as_text=True)
|
|
assert "no puede crear cuentas" in body
|
|
assert 'name="create_account" disabled' in body
|
|
assert 'name="create_account" checked' not in body
|
|
|
|
|
|
def test_with_a_token_the_form_is_unchanged(app, client, owner_id, sign_in):
|
|
app.config["ADMIN_TOKEN"] = "admintoken"
|
|
sign_in(owner_id)
|
|
|
|
body = client.get("/comunidad/miembros/nuevo").get_data(as_text=True)
|
|
assert 'name="create_account" checked' in body
|
|
assert "no puede crear cuentas" not in body
|
|
|
|
|
|
def test_ticking_it_anyway_still_creates_nothing(app, client, db, post, owner_id, sign_in):
|
|
"""A disabled input is a courtesy, not a permission — the refusal lives in
|
|
the handler, where a hand-crafted POST also meets it."""
|
|
app.config["ADMIN_TOKEN"] = ""
|
|
sign_in(owner_id)
|
|
|
|
response = post("/comunidad/miembros/nuevo", {
|
|
"login": "maria", "display_name": "María", "email": "m@example.com",
|
|
"role": "user", "create_account": "on",
|
|
}, follow_redirects=True)
|
|
|
|
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}"
|
|
)
|