agnes-the-ai-analyst/tests/test_user_management.py
ZdenekSrotyr 91caefaca9
security(auth): per-IP rate limit + last-admin guard (#165)
* security(auth): per-IP rate limit on auth endpoints + generalize last-admin guard

Closes #45 and #151.

#45 — every auth endpoint was unthrottled (login, magic-link, token,
bootstrap), leaving us open to password brute-force and SMTP
email-bombing. Wires slowapi (new dep) into the middleware chain with
per-route limits: 10/min on login + token, 5/min on send-link, 3/min on
bootstrap. Returns 429 with Retry-After: 60 once exceeded. Per-IP key
respects the leftmost X-Forwarded-For hop (Caddy in front of the app
strips client-supplied XFF). Operator escape hatch:
AGNES_AUTH_RATELIMIT_ENABLED=0. Test suite disables the limiter via
autouse conftest fixture so existing auth tests that hammer endpoints
in tight loops are unaffected.

#151 — DELETE /api/admin/users/{id}/memberships/{group_id} and the
mirror DELETE /api/admin/groups/{group_id}/members/{user_id} only
guarded against self-removal as last admin. Generalizes to refuse
removing anyone from the seeded Admin group when they are the only
remaining active admin (mirrors the existing
count_admins(active_only=True) <= 1 check on delete_user / update_user).
Recovery from zero admins requires direct DB access, so this closes
a path where a scheduler/bootstrap actor that bypasses normal admin
checks could otherwise empty the group.

* security(auth): throttle remaining email-bombing + token-confirm endpoints

Address code-review gap on PR #165 — the first commit covered /send-link
but missed two endpoints with the IDENTICAL email-bombing surface:

- POST /auth/password/reset       — sends reset mail, anti-enum response
- POST /auth/password/setup/request — sends setup mail, anti-enum response

Both now share the 5/min limit with /send-link.

Also add 10/min to the token-confirm surfaces — high-entropy tokens but
partial leaks via logs / referer have surfaced before, and unbounded
guess rate would let an attacker exhaust the keyspace adjacent to a
leaked prefix:

- POST /auth/email/verify
- GET  /auth/email/verify         — closes the click-through bypass
- POST /auth/password/reset/confirm
- POST /auth/password/setup/confirm

Doc fix: rate_limit.py module docstring + CHANGELOG entry no longer
claim "disable without a redeploy" (misleading). The Limiter constructor
freezes `enabled` from env at import time, matching every other Agnes
env knob — operators set the flag and bounce the container.

Tests: 4 new cases in test_auth_rate_limit.py covering
/reset, /setup/request, /reset/confirm, GET /verify. Full suite:
2583 passed, 32 skipped, 0 failed.

* security(auth): throttle JSON /auth/password/setup — closes form-throttle bypass

Second code-review pass on PR #165 caught a fifth gap: POST /auth/password/setup
(JSON variant, kept for backward compat) consumes the same setup_token as
the web form /setup/confirm but was unthrottled — an attacker brute-forcing
the token just switches from the form path to the JSON path and resumes
at unbounded RPS. Apply the same 10/min limit and signature shape used
on /setup/confirm.

Also extend CHANGELOG note about the JSON-variant bypass for future
operators reading the security entry.

Test: 1 new case (test_password_setup_json_rate_limited_after_10_requests),
9 rate-limit tests + 28 password-flow tests + 41 auth-provider tests pass,
no regressions.

* chore(release): cut 0.30.1 — auth security hardening (rate limit + last-admin guard)
2026-05-02 21:08:33 +02:00

395 lines
15 KiB
Python

"""Tests for #11 — user management (active flag, safeguards, endpoints)."""
import os
import tempfile
import pytest
import duckdb
from src.db import _ensure_schema, get_schema_version
@pytest.fixture
def fresh_db(monkeypatch):
with tempfile.TemporaryDirectory() as tmp:
monkeypatch.setenv("DATA_DIR", tmp)
# Reset cached system DB so we open a brand-new instance in tmp
from src.db import close_system_db
close_system_db()
yield tmp
close_system_db()
def test_schema_v5_adds_active_column(fresh_db):
from src.db import get_system_db, close_system_db
conn = get_system_db()
try:
cols = conn.execute("PRAGMA table_info(users)").fetchall()
col_names = [c[1] for c in cols]
assert "active" in col_names
assert "deactivated_at" in col_names
assert "deactivated_by" in col_names
assert get_schema_version(conn) >= 5
finally:
conn.close()
close_system_db()
def test_schema_v5_backfill_keeps_existing_users_active(fresh_db):
"""Simulate upgrading from v4: insert a user pre-migration, verify active=TRUE afterwards."""
import uuid
import duckdb as _duckdb
from pathlib import Path
# 1. Create a v4-era DB by hand.
db_dir = Path(fresh_db) / "state"
db_dir.mkdir(parents=True, exist_ok=True)
db_path = db_dir / "system.duckdb"
conn = _duckdb.connect(str(db_path))
try:
conn.execute("CREATE TABLE schema_version (version INTEGER NOT NULL, applied_at TIMESTAMP DEFAULT current_timestamp)")
conn.execute("INSERT INTO schema_version (version) VALUES (4)")
conn.execute("""CREATE TABLE users (
id VARCHAR PRIMARY KEY, email VARCHAR UNIQUE NOT NULL,
name VARCHAR, role VARCHAR DEFAULT 'analyst',
password_hash VARCHAR, setup_token VARCHAR,
setup_token_created TIMESTAMP, reset_token VARCHAR,
reset_token_created TIMESTAMP,
created_at TIMESTAMP DEFAULT current_timestamp, updated_at TIMESTAMP)""")
uid = str(uuid.uuid4())
conn.execute("INSERT INTO users (id, email, name, role) VALUES (?, 'pre@v4', 'Pre', 'admin')", [uid])
finally:
conn.close()
# 2. Now let the app open it — schema should migrate to v5 and backfill active=TRUE.
from src.db import get_system_db, close_system_db, get_schema_version
close_system_db()
conn = get_system_db()
try:
assert get_schema_version(conn) >= 5
row = conn.execute("SELECT email, active FROM users WHERE email = 'pre@v4'").fetchone()
assert row is not None
assert row[1] is True
finally:
conn.close()
close_system_db()
def test_repository_update_accepts_active(fresh_db):
import uuid
from src.db import get_system_db, close_system_db
from src.repositories.users import UserRepository
conn = get_system_db()
try:
repo = UserRepository(conn)
uid = str(uuid.uuid4())
repo.create(id=uid, email="a@b.c", name="A")
repo.update(id=uid, active=False, deactivated_by="admin-uuid")
row = repo.get_by_id(uid)
assert row["active"] is False
assert row["deactivated_by"] == "admin-uuid"
finally:
conn.close()
close_system_db()
def test_repository_count_admins(fresh_db):
"""v12: count_admins counts users in the Admin system group, not users.role."""
import uuid
from src.db import SYSTEM_ADMIN_GROUP, get_system_db, close_system_db
from src.repositories.user_group_members import UserGroupMembersRepository
from src.repositories.users import UserRepository
conn = get_system_db()
try:
repo = UserRepository(conn)
assert repo.count_admins() == 0
admin_gid = conn.execute(
"SELECT id FROM user_groups WHERE name = ?", [SYSTEM_ADMIN_GROUP]
).fetchone()[0]
admin_id = str(uuid.uuid4())
repo.create(id=admin_id, email="a@b.c", name="A")
UserGroupMembersRepository(conn).add_member(admin_id, admin_gid, source="system_seed")
repo.create(id=str(uuid.uuid4()), email="b@b.c", name="B")
assert repo.count_admins() == 1
finally:
conn.close()
close_system_db()
from fastapi.testclient import TestClient
@pytest.fixture
def app_client(fresh_db, monkeypatch):
monkeypatch.setenv("TESTING", "1")
monkeypatch.setenv("JWT_SECRET_KEY", "test-jwt-secret-key-minimum-32-chars!!")
from app.main import app
return TestClient(app)
def _seed_admin(fresh_db):
"""Create an admin user (in Admin user_group) and return (id, bearer_token)."""
import uuid
from src.db import SYSTEM_ADMIN_GROUP, get_system_db
from src.repositories.user_group_members import UserGroupMembersRepository
from src.repositories.users import UserRepository
from app.auth.jwt import create_access_token
conn = get_system_db()
try:
uid = str(uuid.uuid4())
UserRepository(conn).create(id=uid, email="admin@test", name="Admin")
admin_gid = conn.execute(
"SELECT id FROM user_groups WHERE name = ?", [SYSTEM_ADMIN_GROUP]
).fetchone()[0]
UserGroupMembersRepository(conn).add_member(uid, admin_gid, source="system_seed")
token = create_access_token(user_id=uid, email="admin@test")
return uid, token
finally:
conn.close()
def test_patch_user_updates_role(app_client, fresh_db):
import uuid
from src.db import get_system_db
from src.repositories.users import UserRepository
admin_id, token = _seed_admin(fresh_db)
target_id = str(uuid.uuid4())
conn = get_system_db()
try:
UserRepository(conn).create(id=target_id, email="x@test", name="X")
finally:
conn.close()
resp = app_client.patch(
f"/api/users/{target_id}",
headers={"Authorization": f"Bearer {token}"},
json={"role": "analyst", "name": "X2"},
)
assert resp.status_code == 200
data = resp.json()
# v12: role response is admin/user based on Admin group membership.
# Patching role="analyst" is a no-op for the admin group → still "user".
assert data["role"] == "user"
assert data["name"] == "X2"
def test_cannot_self_deactivate(app_client, fresh_db):
admin_id, token = _seed_admin(fresh_db)
resp = app_client.patch(
f"/api/users/{admin_id}",
headers={"Authorization": f"Bearer {token}"},
json={"active": False},
)
assert resp.status_code == 409
assert "yourself" in resp.json()["detail"].lower()
def test_cannot_delete_last_admin(app_client, fresh_db):
"""Deleting the sole active admin must 409.
Note: the endpoint checks self-delete first, which also triggers 409 here,
so we accept either "yourself" or "last" wording — the point is the
safeguard blocks deletion of the only admin."""
admin_id, token = _seed_admin(fresh_db)
# Create a non-admin so we have ≥2 users, but admin is still the only admin.
resp = app_client.post(
"/api/users",
headers={"Authorization": f"Bearer {token}"},
json={"email": "x@test", "name": "X", "role": "viewer"},
)
x_id = resp.json()["id"]
# Try deleting the admin.
resp = app_client.delete(
f"/api/users/{admin_id}",
headers={"Authorization": f"Bearer {token}"},
)
assert resp.status_code == 409
detail = resp.json()["detail"].lower()
assert "last" in detail or "yourself" in detail
def test_deactivated_user_cannot_authenticate(app_client, fresh_db):
"""A deactivated user's old JWT must be rejected."""
import uuid
from src.db import get_system_db
from src.repositories.users import UserRepository
from app.auth.jwt import create_access_token
conn = get_system_db()
try:
uid = str(uuid.uuid4())
UserRepository(conn).create(id=uid, email="u@test", name="U")
token = create_access_token(user_id=uid, email="u@test")
UserRepository(conn).update(id=uid, active=False)
finally:
conn.close()
resp = app_client.get(
"/api/users", # any authenticated endpoint
headers={"Authorization": f"Bearer {token}", "Accept": "application/json"},
)
# Deactivated — must not succeed.
assert resp.status_code in (401, 403)
def test_admin_users_page_renders_for_admin(app_client, fresh_db):
admin_id, token = _seed_admin(fresh_db)
resp = app_client.get(
"/admin/users",
headers={"Accept": "text/html"},
cookies={"access_token": token},
)
assert resp.status_code == 200
assert 'class="users-title">Users' in resp.text
def test_admin_users_page_denies_non_admin(app_client, fresh_db):
import uuid
from src.db import get_system_db
from src.repositories.users import UserRepository
from app.auth.jwt import create_access_token
conn = get_system_db()
try:
uid = str(uuid.uuid4())
UserRepository(conn).create(id=uid, email="a@test", name="A")
token = create_access_token(user_id=uid, email="a@test")
finally:
conn.close()
resp = app_client.get(
"/admin/users",
headers={"Accept": "text/html"},
cookies={"access_token": token},
follow_redirects=False,
)
# HTML request to admin-only page → 302 (to /login) for non-admin per Phase 0, or 403.
# Phase 0 is out of scope here so we accept 403 (current behaviour) or 302.
assert resp.status_code in (302, 403)
def test_deactivated_admin_rejected_by_active_check(app_client, fresh_db):
"""Deactivating an admin must cause their token to be rejected as 401 (not succeed)."""
import uuid
from src.db import get_system_db
from src.repositories.users import UserRepository
from app.auth.jwt import create_access_token
# Seed two admins so we can deactivate one without tripping the last-admin rule.
admin_id, admin_token = _seed_admin(fresh_db)
conn = get_system_db()
try:
other_uid = str(uuid.uuid4())
UserRepository(conn).create(id=other_uid, email="other@test", name="Other")
other_token = create_access_token(user_id=other_uid, email="other@test")
# Directly deactivate the "other" admin via repository (bypass safeguard
# because we already have 2 admins; this is just a state setup).
UserRepository(conn).update(id=other_uid, active=False)
finally:
conn.close()
resp = app_client.get(
"/api/users",
headers={"Authorization": f"Bearer {other_token}", "Accept": "application/json"},
)
assert resp.status_code == 401
assert "deactivated" in resp.json().get("detail", "").lower()
def test_cannot_remove_last_admin_via_user_memberships(app_client, fresh_db):
"""v19 #151: DELETE /api/admin/users/{id}/memberships/{group_id} must
refuse to remove the only active admin from the seeded Admin group —
even when the caller is a different admin (covers the case where
a second admin was added then the first was deactivated, leaving
one active admin who could otherwise be demoted to zero)."""
from src.db import SYSTEM_ADMIN_GROUP, get_system_db
admin_id, token = _seed_admin(fresh_db)
conn = get_system_db()
try:
admin_gid = conn.execute(
"SELECT id FROM user_groups WHERE name = ?", [SYSTEM_ADMIN_GROUP]
).fetchone()[0]
finally:
conn.close()
# Sole-admin case: try to demote the only admin via the user-keyed
# memberships endpoint.
resp = app_client.delete(
f"/api/admin/users/{admin_id}/memberships/{admin_gid}",
headers={"Authorization": f"Bearer {token}"},
)
assert resp.status_code == 409
assert "last admin" in resp.json()["detail"].lower()
def test_cannot_remove_last_admin_via_group_members(app_client, fresh_db):
"""v19 #151: DELETE /api/admin/groups/{group_id}/members/{user_id} must
refuse to demote the only active admin (group-keyed mirror of the
user-keyed membership endpoint)."""
from src.db import SYSTEM_ADMIN_GROUP, get_system_db
admin_id, token = _seed_admin(fresh_db)
conn = get_system_db()
try:
admin_gid = conn.execute(
"SELECT id FROM user_groups WHERE name = ?", [SYSTEM_ADMIN_GROUP]
).fetchone()[0]
finally:
conn.close()
resp = app_client.delete(
f"/api/admin/groups/{admin_gid}/members/{admin_id}",
headers={"Authorization": f"Bearer {token}"},
)
assert resp.status_code == 409
assert "last admin" in resp.json()["detail"].lower()
def test_can_remove_admin_when_another_active_admin_exists(app_client, fresh_db):
"""Sanity: with two active admins, demoting one via the membership
endpoint must succeed — the guard fires only at count_admins <= 1."""
import uuid
from src.db import SYSTEM_ADMIN_GROUP, get_system_db
from src.repositories.user_group_members import UserGroupMembersRepository
from src.repositories.users import UserRepository
admin_id, token = _seed_admin(fresh_db)
conn = get_system_db()
try:
admin_gid = conn.execute(
"SELECT id FROM user_groups WHERE name = ?", [SYSTEM_ADMIN_GROUP]
).fetchone()[0]
other_id = str(uuid.uuid4())
UserRepository(conn).create(id=other_id, email="other@test", name="Other")
UserGroupMembersRepository(conn).add_member(
other_id, admin_gid, source="admin", added_by="admin@test",
)
finally:
conn.close()
resp = app_client.delete(
f"/api/admin/users/{other_id}/memberships/{admin_gid}",
headers={"Authorization": f"Bearer {token}"},
)
assert resp.status_code == 204
def test_cannot_deactivate_last_admin(app_client, fresh_db):
"""v19: try to deactivate the last active admin → 409.
Admin demotion is now done via group membership (DELETE /api/admin/users/{id}/memberships/{group_id}),
but the deactivate path retains its own last-admin guard.
"""
admin_id, token = _seed_admin(fresh_db)
# Create a second non-admin user.
resp = app_client.post(
"/api/users",
headers={"Authorization": f"Bearer {token}"},
json={"email": "y@test", "name": "Y"},
)
assert resp.status_code == 201
# Try to deactivate the only active admin → must fail.
resp = app_client.patch(
f"/api/users/{admin_id}",
headers={"Authorization": f"Bearer {token}"},
json={"active": False},
)
# The endpoint blocks deactivation for the last active admin BEFORE the
# self-deactivate check (the user IS themselves, but the message says "last
# active admin"). Either error is acceptable — both signal the constraint.
assert resp.status_code == 409
assert (
"admin" in resp.json()["detail"].lower()
or "yourself" in resp.json()["detail"].lower()
)