Round 4's only finding: the file header and one comment still said Deny stops a refused person re-asking. It does not — upsert_pending_invite reopens a denied row as pending — and the delete_invite docstring in the same branch already says so. The real difference is that Deny records an auditable decision and mails the requester; Delete forgets the address.
171 lines
7.3 KiB
Python
171 lines
7.3 KiB
Python
"""Deleting a whitelist / access-request entry (admin).
|
|
|
|
Deny records an auditable decision and keeps the row; delete forgets the address outright, for an
|
|
approval added by mistake or one that is no longer wanted. Neither stops the person asking again —
|
|
`upsert_pending_invite` reopens a denied row as pending, and a deleted one comes back as a fresh
|
|
request. The one hard rule is that an entry backed by an account is NOT deletable here: this row is
|
|
that account's approval gate, and orphaning it would strand someone who could then neither sign in
|
|
nor be approved.
|
|
"""
|
|
|
|
from sqlalchemy import select
|
|
|
|
from app.models import Invite, User
|
|
|
|
|
|
def _invites(client):
|
|
r = client.get("/api/admin/invites")
|
|
assert r.status_code == 200
|
|
return {i["email"]: i for i in r.json()}
|
|
|
|
|
|
def test_admin_deletes_a_whitelist_entry_that_never_became_an_account(db, make_user, as_user):
|
|
admin = make_user(email="admin@example.com", role="admin")
|
|
db.add(Invite(email="mistake@example.com", status="approved", decided_by=admin.email))
|
|
db.commit()
|
|
client = as_user(admin)
|
|
listed = _invites(client)["mistake@example.com"]
|
|
assert listed["has_account"] is False # drives the delete affordance in the UI
|
|
|
|
r = client.delete(f"/api/admin/invites/{listed['id']}")
|
|
|
|
assert r.status_code == 200 and r.json()["deleted"] == listed["id"]
|
|
db.expire_all()
|
|
assert (
|
|
db.execute(select(Invite).where(Invite.email == "mistake@example.com")).scalar_one_or_none()
|
|
is None
|
|
)
|
|
|
|
|
|
def test_deleting_an_entry_that_has_an_account_is_refused(db, make_user, as_user):
|
|
# The guard that matters: this row is what is_allowed() reads, so deleting it would revoke the
|
|
# user's access without touching the account or saying so.
|
|
admin = make_user(email="admin@example.com", role="admin")
|
|
member = make_user(email="member@example.com")
|
|
db.add(Invite(email=member.email, status="approved", decided_by=admin.email))
|
|
db.commit()
|
|
client = as_user(admin)
|
|
listed = _invites(client)[member.email]
|
|
assert listed["has_account"] is True
|
|
|
|
r = client.delete(f"/api/admin/invites/{listed['id']}")
|
|
|
|
assert r.status_code == 409
|
|
db.expire_all()
|
|
assert (
|
|
db.execute(select(Invite).where(Invite.email == member.email)).scalar_one_or_none()
|
|
is not None
|
|
)
|
|
|
|
|
|
def test_a_pending_request_from_a_registration_is_not_deletable(db, make_user, as_user):
|
|
# The shape the ordinary sign-up produces: _register_account writes the User (inactive) AND the
|
|
# pending Invite together. The invite is that account's approval gate, so deleting it would
|
|
# strand the person — they could neither sign in nor be approved, and re-registering is a silent
|
|
# no-op. Managed from Users & roles instead, which is what the UI and the 409 both say.
|
|
admin = make_user(email="admin@example.com", role="admin")
|
|
make_user(email="signup@example.com", is_active=False, email_verified=False)
|
|
db.add(Invite(email="signup@example.com", status="pending"))
|
|
db.commit()
|
|
client = as_user(admin)
|
|
listed = _invites(client)["signup@example.com"]
|
|
assert listed["has_account"] is True # the UI shows a disabled control, not a missing one
|
|
|
|
r = client.delete(f"/api/admin/invites/{listed['id']}")
|
|
|
|
assert r.status_code == 409
|
|
db.expire_all()
|
|
assert (
|
|
db.execute(select(Invite).where(Invite.email == "signup@example.com")).scalar_one_or_none()
|
|
is not None
|
|
)
|
|
|
|
|
|
def test_deleting_an_env_allow_listed_address_reports_that_it_kept_access(
|
|
db, make_user, as_user, monkeypatch
|
|
):
|
|
# Removing the row is NOT always a revoke: is_allowed falls back to the env bootstrap lists once
|
|
# no invite exists. The admin has to be told, or they think they removed access and did not.
|
|
from app.config import settings
|
|
|
|
# Patch the raw env field, not the derived property: that keeps the split/strip/lower parsing
|
|
# under test, and matches how the rest of the suite stubs settings.
|
|
monkeypatch.setattr(settings, "allowed_emails", " OPS@Example.com , someone@else.test ")
|
|
admin = make_user(email="admin@example.com", role="admin")
|
|
db.add(Invite(email="ops@example.com", status="approved", decided_by=admin.email))
|
|
db.commit()
|
|
client = as_user(admin)
|
|
listed = _invites(client)["ops@example.com"]
|
|
# The LIST carries it too, so the confirm dialog can warn before the delete happens.
|
|
assert listed["still_allowed_by_env"] is True
|
|
|
|
r = client.delete(f"/api/admin/invites/{listed['id']}")
|
|
|
|
assert r.status_code == 200
|
|
assert r.json()["still_allowed_by_env"] is True
|
|
|
|
|
|
def test_an_admin_emails_address_is_also_reported(db, make_user, as_user, monkeypatch):
|
|
# ADMIN_EMAILS is the sharper case: deleting the row not only keeps access, the next Google
|
|
# sign-in re-promotes the address to admin.
|
|
from app.config import settings
|
|
|
|
monkeypatch.setattr(settings, "admin_emails", "boss@example.com")
|
|
admin = make_user(email="admin@example.com", role="admin")
|
|
db.add(Invite(email="boss@example.com", status="approved", decided_by=admin.email))
|
|
db.commit()
|
|
client = as_user(admin)
|
|
|
|
assert _invites(client)["boss@example.com"]["still_allowed_by_env"] is True
|
|
|
|
|
|
def test_deleting_a_pending_request_forgets_it(db, make_user, as_user):
|
|
# A pending row is deletable too — "drop this without recording a decision" is a different
|
|
# choice from Deny, which stores a decision (status, decided_by) and mails the requester.
|
|
admin = make_user(email="admin@example.com", role="admin")
|
|
db.add(Invite(email="asker@example.com", status="pending"))
|
|
db.commit()
|
|
client = as_user(admin)
|
|
|
|
r = client.delete(f"/api/admin/invites/{_invites(client)['asker@example.com']['id']}")
|
|
|
|
assert r.status_code == 200
|
|
assert r.json()["still_allowed_by_env"] is False # nothing in the env allow-lists
|
|
assert "asker@example.com" not in _invites(client)
|
|
|
|
|
|
def test_deleting_a_missing_invite_is_404(db, make_user, as_user):
|
|
admin = make_user(email="admin@example.com", role="admin")
|
|
assert as_user(admin).delete("/api/admin/invites/9999").status_code == 404
|
|
|
|
|
|
def test_a_regular_user_cannot_delete_an_invite(db, make_user, as_user):
|
|
make_user(email="admin@example.com", role="admin")
|
|
plain = make_user(email="plain@example.com")
|
|
db.add(Invite(email="mistake@example.com", status="approved"))
|
|
db.commit()
|
|
inv_id = db.execute(select(Invite.id).where(Invite.email == "mistake@example.com")).scalar_one()
|
|
|
|
assert as_user(plain).delete(f"/api/admin/invites/{inv_id}").status_code == 403
|
|
|
|
db.expire_all()
|
|
assert db.get(Invite, inv_id) is not None
|
|
|
|
|
|
def test_deleting_the_user_also_clears_the_way_to_re_invite(db, make_user, as_user):
|
|
# The refusal above tells the admin to delete the account instead; purge_user takes the invite
|
|
# with it, so the address really is forgotten and a fresh request starts over as pending.
|
|
admin = make_user(email="admin@example.com", role="admin")
|
|
member = make_user(email="member@example.com")
|
|
db.add(Invite(email=member.email, status="approved", decided_by=admin.email))
|
|
db.commit()
|
|
client = as_user(admin)
|
|
|
|
assert client.delete(f"/api/admin/users/{member.id}").status_code == 200
|
|
|
|
db.expire_all()
|
|
assert db.execute(select(User).where(User.email == member.email)).scalar_one_or_none() is None
|
|
assert (
|
|
db.execute(select(Invite).where(Invite.email == member.email)).scalar_one_or_none() is None
|
|
)
|