Three complaints, one cause. Logging out could not finish because the session belonged to another server, whose logout is POST-only and unreachable from here. "Ese usuario ya está en uso" for somebody absent from Miembros, because erase_member deleted our row and left the git account standing — two stores of users, one of them showing. And the hand-off to a differently-designed domain, with a Forgot password that could never work. All three followed from delegating identity, so it is no longer delegated. Members now sign in at /comunidad/login against a scrypt hash in our own database, via werkzeug.security, which arrives with Flask. They have no account on the git server at all, which makes the collision impossible rather than fixed. Logout is one click. This removes more than it adds: the OAuth round trip, the client registration, tokens.py with its refresh-before-expiry logic, the gitea_tokens table, and the logged-out page that existed to apologise for a logout that did not log you out. The editor keeps per-writer attribution without per-writer tokens: one CONTENT_TOKEN commits, and each commit names its author, which Gitea's contents API supports and a test now asserts. CONTENT_TOKEN falls back to GITEA_ADMIN_TOKEN so nothing breaks on deploy, but it only needs write access to one repository, while the admin token can modify every account on the instance — and now has no remaining job. The refusals are the interesting part. Unknown name, wrong password, suspended member and invited-but-never-arrived all answer identically, asserted by comparing the rendered bytes. The hash check runs against a decoy even when there is no such member, so an unknown name does not answer faster. Attempts are rate limited, counted inside SQLite for the reason invites.py documents. A NULL hash never matches anything. Migration 2 adds password_hash and drops the dead OAuth tokens. Everyone starts NULL, including the owner, so scripts/set-password.sh exists and was tested before this could ship: it prompts without echo, never takes the password as an argument where ps would show it, and refuses a short one or an unknown member. Rehearsed against a rebuilt copy of the server's database: starts, keeps the photo, drops the tokens table, serves a login form with no redirect, signs in, signs out, stays out. 223 tests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NizVpJ2dwzCbjCrTLCjeHn
293 lines
11 KiB
Python
293 lines
11 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",
|
|
})
|
|
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",
|
|
})
|
|
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_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
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
# --- 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,
|
|
REMOVED_ON_ERASE)
|
|
handled = REASSIGNED_ON_ERASE | CLEARED_ON_ERASE | REMOVED_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}"
|
|
)
|
|
|
|
|
|
# --- onboarding, now that there is only one account to make --------------
|
|
|
|
def test_a_new_member_has_no_password_until_they_choose_one(
|
|
client, db, post, owner_id, sign_in, monkeypatch):
|
|
"""No password is generated and none is shown. Until the invitation is
|
|
used, password_hash is NULL — and a NULL hash cannot be signed in with."""
|
|
monkeypatch.setattr("apps.board.mail.send", lambda to, subject, body: None)
|
|
sign_in(owner_id)
|
|
|
|
post("/comunidad/miembros/nuevo", {
|
|
"login": "maria", "display_name": "María", "email": "m@example.com",
|
|
"role": "user",
|
|
})
|
|
|
|
row = db.execute("SELECT password_hash FROM members WHERE gitea_login = 'maria'"
|
|
).fetchone()
|
|
assert row is not None
|
|
assert row["password_hash"] is None
|
|
|
|
|
|
def test_an_address_is_required(client, db, post, owner_id, sign_in):
|
|
"""It is the only way the invitation reaches anybody, so a member without
|
|
one is a row that can never be used."""
|
|
sign_in(owner_id)
|
|
post("/comunidad/miembros/nuevo", {
|
|
"login": "maria", "display_name": "María", "email": "", "role": "user",
|
|
})
|
|
assert db.execute("SELECT 1 FROM members WHERE gitea_login = 'maria'").fetchone() is None
|
|
|
|
|
|
def test_a_duplicate_is_refused_before_anything_is_written(
|
|
client, db, post, owner_id, make_member, sign_in, monkeypatch):
|
|
"""The error that started this: "ya están en uso" for somebody who was not
|
|
on the list. There is one list now, so the message and the screen agree."""
|
|
monkeypatch.setattr("apps.board.mail.send", lambda to, subject, body: None)
|
|
make_member("maria")
|
|
sign_in(owner_id)
|
|
|
|
response = post("/comunidad/miembros/nuevo", {
|
|
"login": "maria", "display_name": "Otra", "email": "otra@example.com",
|
|
"role": "user",
|
|
}, follow_redirects=True)
|
|
|
|
assert "ya están en uso" in response.get_data(as_text=True)
|
|
assert db.execute(
|
|
"SELECT COUNT(*) AS n FROM members WHERE gitea_login = 'maria'"
|
|
).fetchone()["n"] == 1
|
|
|
|
|
|
def test_erasing_a_member_lets_the_name_be_used_again(
|
|
client, db, post, owner_id, make_member, sign_in, monkeypatch):
|
|
"""Deleting used to leave an account behind on the other server, so the
|
|
name stayed taken somewhere invisible. With one store, gone means gone."""
|
|
monkeypatch.setattr("apps.board.mail.send", lambda to, subject, body: None)
|
|
sign_in(owner_id)
|
|
post(f"/comunidad/miembros/{make_member('salvador')}/eliminar")
|
|
|
|
post("/comunidad/miembros/nuevo", {
|
|
"login": "salvador", "display_name": "Salvador", "email": "s@example.com",
|
|
"role": "user",
|
|
})
|
|
|
|
assert db.execute(
|
|
"SELECT 1 FROM members WHERE gitea_login = 'salvador'").fetchone() is not None
|