peter
8d7aaad9a8
Round 2 on the feature diff (15 → 12): - still_allowed_by_env now rides the invite LIST as well as the delete response, so the confirmation warns while the admin can still change their mind. It mattered most for a DENIED row on an env-allow-listed address: deleting it silently converted a refusal into a grant, and the only warning arrived after the irreversible click. - Both inbox bodies claimed to name "the most recent" dropped message. The throttle records the FIRST of each hour and never updates it, so they now say "one of them", and the not-configured body finally names where to fix it. - settings.invites.emailOff told the admin they "get no notice when someone signs up" — rendered directly above the queue that shows exactly that. Only the email is lost, and it now says so. - The disabled delete buttons put their reason out of reach: `disabled` drops a control from the tab order and suppresses its title. Switched to aria-disabled + preventDefault, the pattern Playlists.tsx already established for the same reason. - deleteInvite is quiet, so the 409 the UI handles with a translated toast no longer also raises the server's English detail as a modal. - After a completed password reset, Back landed on the reset form again with a consumed token — the history change made that state reachable. The token is cleared on success so the reset view stops being the card's root. - The hu deleteHasAccount named a tab that renders under a different label. Tests: a dom test for the Access-requests panel covering both has_account variants and all three delete outcomes, /api/me flag cases, an ADMIN_EMAILS case, and the env-allowlist test now patches the raw setting so the parsing stays under test.
169 lines
7.1 KiB
Python
169 lines
7.1 KiB
Python
"""Deleting a whitelist / access-request entry (admin).
|
|
|
|
Deny keeps the row so a refused person cannot immediately re-ask; delete forgets the address
|
|
outright, for an approval added by mistake or one that is no longer wanted. The one hard rule is
|
|
that an entry backed by an account is NOT deletable here: `is_allowed` reads this row, so removing
|
|
it would silently revoke a live user's access.
|
|
"""
|
|
|
|
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 — "not now, and don't keep the record" is a different decision
|
|
# from Deny, which keeps the row precisely so the person cannot immediately re-ask.
|
|
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
|
|
)
|