From 6dca9ba4f2e7da97e2639f82d9afd236babdb388 Mon Sep 17 00:00:00 2001 From: chiguyong Date: Sun, 21 Jun 2026 13:45:42 +0800 Subject: [PATCH] =?UTF-8?q?feat(admin):=20U3=20=E2=80=94=20user=20CRUD=20+?= =?UTF-8?q?=20password=20reset=20+=20multi-department?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add create_user method to LocalAuthProvider (bcrypt hash + INSERT, raises ValueError on duplicate username/email). Add UserService with 9 async methods: create/list/get/update/delete (soft)/reset_password/assign_department/remove_department/list_user_departments. reset_password revokes all sessions via SessionService. delete_user is soft (is_active=0, row preserved). Add 9 user endpoints to routes/admin.py: POST/GET/PATCH/DELETE users, reset-password, assign/remove department, list departments. All guarded by _require_admin. Tests: 40 unit + 37 integration = 77 new tests. Full admin suite 170 tests pass, no regressions. --- src/agentkit/server/admin/user_service.py | 527 +++++++++++++ src/agentkit/server/auth/providers/local.py | 93 ++- src/agentkit/server/routes/admin.py | 552 ++++++++++++++ tests/integration/admin/test_user_routes.py | 614 +++++++++++++++ tests/unit/admin/test_user_service.py | 806 ++++++++++++++++++++ 5 files changed, 2590 insertions(+), 2 deletions(-) create mode 100644 src/agentkit/server/admin/user_service.py create mode 100644 src/agentkit/server/routes/admin.py create mode 100644 tests/integration/admin/test_user_routes.py create mode 100644 tests/unit/admin/test_user_service.py diff --git a/src/agentkit/server/admin/user_service.py b/src/agentkit/server/admin/user_service.py new file mode 100644 index 0000000..c8e481b --- /dev/null +++ b/src/agentkit/server/admin/user_service.py @@ -0,0 +1,527 @@ +"""UserService — business logic for ``users`` + multi-department assignment (U3). + +This module is the single owner of user CRUD operations that span the +``users`` and ``user_departments`` tables. Web UI routes +(``/api/v1/admin/users/*``) and the CLI ``agentkit admin user`` sub-app +both call into :class:`UserService` rather than touching the tables +directly, keeping the validation rules (duplicate-username, department +existence) in one place. + +The service is a module-level singleton (see :func:`get_user_service`) +so tests can inject a custom instance via :func:`set_user_service`. + +Password hashing uses bcrypt (cost factor 12) via +:func:`agentkit.server.auth.password.hash_password`. Password resets +also revoke all of the user's active sessions via :class:`SessionService`. +""" + +from __future__ import annotations + +import logging +from datetime import datetime, timezone +from pathlib import Path +from typing import Any + +import aiosqlite + +from agentkit.server.auth.models import user_row_to_dict +from agentkit.server.auth.password import hash_password +from agentkit.server.auth.providers.local import LocalAuthProvider +from agentkit.server.auth.session_service import ( + REVOKE_REASON_PASSWORD_CHANGED, + get_session_service, +) + +logger = logging.getLogger(__name__) + + +def _now_iso() -> str: + """Return current UTC time as ISO 8601 string.""" + return datetime.now(timezone.utc).isoformat() + + +class UserService: + """CRUD + multi-department assignment operations for users. + + All async methods take ``db_path: Path`` as the first argument + (after ``self``). Each method opens its own short-lived + :class:`aiosqlite.Connection` — there is no shared connection state, + which keeps the service safe to call from any async context. + + The :meth:`create_user` method delegates the actual user-row insert + to :class:`LocalAuthProvider` so that the password-hashing and + IntegrityError-handling logic stays in one place. The other methods + (list/get/update/delete/reset_password/department assignment) operate + directly on the DB. + """ + + # ------------------------------------------------------------------ + # User CRUD + # ------------------------------------------------------------------ + + async def create_user( + self, + db_path: Path, + username: str, + email: str, + password: str, + role: str = "member", + department_ids: list[str] | None = None, + created_by: str | None = None, + ) -> dict[str, Any]: + """Create a new user, optionally assigning to departments. + + Args: + db_path: Path to the auth SQLite DB. + username: Unique username. + email: Unique email address. + password: Plain-text password (bcrypt-hashed before storage). + role: Role name (``member`` / ``operator`` / ``admin``). + Defaults to ``member``. + department_ids: Optional list of department ids to assign + the user to. Each id must exist in the ``departments`` + table; duplicate or non-existent ids raise ``ValueError``. + created_by: Optional user id of the admin who created this + user (audit trail). + + Returns: + The newly-created user dict, including a ``departments`` + list (each item is ``{id, name}``). + + Raises: + ValueError: If a user with the same username or email + already exists, or if any of the ``department_ids`` + does not exist or is already assigned. + """ + provider = LocalAuthProvider(db_path=db_path) + user = await provider.create_user( + username=username, + email=email, + password=password, + role=role, + created_by=created_by, + ) + user_id = user["id"] + + if department_ids: + for dept_id in department_ids: + # assign_department validates department existence and + # duplicate-assignment; raise on the first failure. + await self.assign_department(db_path, user_id, dept_id) + + # Re-fetch with departments populated so the caller sees the + # full picture (matches the shape returned by get_user). + result = await self.get_user(db_path, user_id) + assert result is not None # we just created it + return result + + async def list_users( + self, + db_path: Path, + department_id: str | None = None, + include_inactive: bool = True, + ) -> list[dict[str, Any]]: + """List users, optionally filtered by department. + + Args: + db_path: Path to the auth SQLite DB. + department_id: When provided, only users assigned to this + department are returned (via ``user_departments`` join). + include_inactive: When ``False``, only users with + ``is_active=1`` are returned. + + Returns: + List of user dicts, each including a ``departments`` list + (each item is ``{id, name}``). Ordered by ``created_at``. + """ + async with aiosqlite.connect(str(db_path)) as db: + db.row_factory = aiosqlite.Row + if department_id is not None: + # JOIN through user_departments to filter. DISTINCT + # avoids duplicates when a user is in the department + # (the join would otherwise produce one row per + # user_department pair, but we filter to a single + # department here so each user appears at most once). + if include_inactive: + sql = ( + "SELECT DISTINCT u.id, u.username, u.email, u.password_hash, " + "u.role, u.is_active, u.is_terminal_authorized, " + "u.is_server_terminal_authorized, u.created_at, u.updated_at, " + "u.last_login_at, u.created_by " + "FROM users u " + "INNER JOIN user_departments ud ON ud.user_id = u.id " + "WHERE ud.department_id = ? " + "ORDER BY u.created_at ASC" + ) + args: tuple[Any, ...] = (department_id,) + else: + sql = ( + "SELECT DISTINCT u.id, u.username, u.email, u.password_hash, " + "u.role, u.is_active, u.is_terminal_authorized, " + "u.is_server_terminal_authorized, u.created_at, u.updated_at, " + "u.last_login_at, u.created_by " + "FROM users u " + "INNER JOIN user_departments ud ON ud.user_id = u.id " + "WHERE ud.department_id = ? AND u.is_active = 1 " + "ORDER BY u.created_at ASC" + ) + args = (department_id,) + elif include_inactive: + sql = ( + "SELECT id, username, email, password_hash, role, is_active, " + "is_terminal_authorized, is_server_terminal_authorized, " + "created_at, updated_at, last_login_at, created_by " + "FROM users ORDER BY created_at ASC" + ) + args = () + else: + sql = ( + "SELECT id, username, email, password_hash, role, is_active, " + "is_terminal_authorized, is_server_terminal_authorized, " + "created_at, updated_at, last_login_at, created_by " + "FROM users WHERE is_active = 1 ORDER BY created_at ASC" + ) + args = () + cursor = await db.execute(sql, args) + rows = await cursor.fetchall() + + users = [user_row_to_dict(row) for row in rows] + # Batch-fetch departments for each user in the same + # connection to avoid N+1 round-trips. + for user in users: + user["departments"] = await self._fetch_departments(db, user["id"]) + return users + + async def get_user( + self, + db_path: Path, + user_id: str, + ) -> dict[str, Any] | None: + """Return a single user by id, or ``None`` if not found. + + The returned dict includes a ``departments`` list (each item is + ``{id, name}``). + """ + async with aiosqlite.connect(str(db_path)) as db: + db.row_factory = aiosqlite.Row + cursor = await db.execute( + "SELECT id, username, email, password_hash, role, is_active, " + "is_terminal_authorized, is_server_terminal_authorized, " + "created_at, updated_at, last_login_at, created_by " + "FROM users WHERE id = ?", + (user_id,), + ) + row = await cursor.fetchone() + if row is None: + return None + user = user_row_to_dict(row) + user["departments"] = await self._fetch_departments(db, user_id) + return user + + async def update_user( + self, + db_path: Path, + user_id: str, + role: str | None = None, + is_active: bool | None = None, + is_terminal_authorized: bool | None = None, + is_server_terminal_authorized: bool | None = None, + ) -> dict[str, Any]: + """Partially update a user. + + Only the provided fields are updated. ``password_hash`` is not + touched here — use :meth:`reset_password` for that. + + Args: + db_path: Path to the auth SQLite DB. + user_id: User id to update. + role: New role, or ``None`` to skip. + is_active: New active flag, or ``None`` to skip. + is_terminal_authorized: New terminal-authorized flag, or + ``None`` to skip. + is_server_terminal_authorized: New server-terminal-authorized + flag, or ``None`` to skip. + + Returns: + The updated user dict (including ``departments``). + + Raises: + ValueError: If the user does not exist. + """ + existing = await self.get_user(db_path, user_id) + if existing is None: + raise ValueError(f"User {user_id!r} not found") + + # Build the SET clause dynamically based on which fields were + # provided. This avoids overwriting columns with NULL when the + # caller only wants to update a subset. + set_parts: list[str] = [] + args: list[Any] = [] + if role is not None: + set_parts.append("role = ?") + args.append(role) + if is_active is not None: + set_parts.append("is_active = ?") + args.append(1 if is_active else 0) + if is_terminal_authorized is not None: + set_parts.append("is_terminal_authorized = ?") + args.append(1 if is_terminal_authorized else 0) + if is_server_terminal_authorized is not None: + set_parts.append("is_server_terminal_authorized = ?") + args.append(1 if is_server_terminal_authorized else 0) + + if set_parts: + set_parts.append("updated_at = ?") + args.append(_now_iso()) + args.append(user_id) + sql = f"UPDATE users SET {', '.join(set_parts)} WHERE id = ?" + async with aiosqlite.connect(str(db_path)) as db: + await db.execute(sql, tuple(args)) + await db.commit() + + updated = await self.get_user(db_path, user_id) + assert updated is not None # we checked existence above + return updated + + async def delete_user( + self, + db_path: Path, + user_id: str, + ) -> bool: + """Soft-delete a user (set ``is_active = 0``). + + The row is NOT actually deleted — this preserves audit trails + and foreign-key references. Use :meth:`update_user` with + ``is_active=True`` to re-enable. + + Args: + db_path: Path to the auth SQLite DB. + user_id: User id to soft-delete. + + Returns: + ``True`` if the user was updated, ``False`` if the user did + not exist (or was already inactive). + """ + async with aiosqlite.connect(str(db_path)) as db: + cursor = await db.execute( + "UPDATE users SET is_active = 0, updated_at = ? WHERE id = ? AND is_active = 1", + (_now_iso(), user_id), + ) + await db.commit() + return cursor.rowcount > 0 + + async def reset_password( + self, + db_path: Path, + user_id: str, + new_password: str, + ) -> bool: + """Reset a user's password and revoke all their sessions. + + The new password is bcrypt-hashed (cost factor 12) before + storage. All active sessions for the user are then revoked + (via :class:`SessionService`) so that any stolen refresh tokens + become invalid immediately. + + Args: + db_path: Path to the auth SQLite DB. + user_id: User id whose password should be reset. + new_password: New plain-text password. + + Returns: + ``True`` if the password was updated, ``False`` if the user + did not exist. + """ + # Use the shared hash_password helper so the cost factor (12) + # is configured in one place (auth.password). + password_hash = hash_password(new_password) + async with aiosqlite.connect(str(db_path)) as db: + cursor = await db.execute( + "UPDATE users SET password_hash = ?, updated_at = ? WHERE id = ?", + (password_hash, _now_iso(), user_id), + ) + await db.commit() + updated = cursor.rowcount > 0 + + if not updated: + return False + + # Revoke all active sessions for this user. We use the + # process-wide SessionService singleton (which reads the same + # auth DB) so that the revocation takes effect immediately for + # any subsequent request that checks session validity. + try: + session_svc = get_session_service() + await session_svc.revoke_all_for_user(user_id, reason=REVOKE_REASON_PASSWORD_CHANGED) + except Exception: # noqa: BLE001 — never block password reset on session revocation + logger.exception( + "Failed to revoke sessions for user %s after password reset", + user_id, + ) + + logger.info("Reset password for user %s and revoked active sessions", user_id) + return True + + # ------------------------------------------------------------------ + # Department assignment + # ------------------------------------------------------------------ + + async def assign_department( + self, + db_path: Path, + user_id: str, + department_id: str, + ) -> dict[str, Any]: + """Assign a user to a department. + + Args: + db_path: Path to the auth SQLite DB. + user_id: User id to assign. + department_id: Department id to assign the user to. + + Returns: + Dict with ``{user_id, department_id, created_at}``. + + Raises: + ValueError: If the department does not exist, or if the + user is already assigned to this department (duplicate + ``user_departments`` row). + """ + # Verify the department exists. We don't verify the user exists + # here — the INSERT below will fail with a row-constraint + # violation if the user_id is bogus, and the caller is expected + # to have created/fetched the user before calling this method. + async with aiosqlite.connect(str(db_path)) as db: + db.row_factory = aiosqlite.Row + cursor = await db.execute( + "SELECT id FROM departments WHERE id = ?", + (department_id,), + ) + dept_row = await cursor.fetchone() + if dept_row is None: + raise ValueError(f"Department {department_id!r} not found") + + now = _now_iso() + try: + await db.execute( + "INSERT INTO user_departments (user_id, department_id, created_at) " + "VALUES (?, ?, ?)", + (user_id, department_id, now), + ) + await db.commit() + except aiosqlite.IntegrityError as exc: + # The composite PK (user_id, department_id) is the only + # UNIQUE constraint on this table, so an IntegrityError + # here means the assignment already exists. + raise ValueError( + f"User {user_id!r} is already assigned to department {department_id!r}" + ) from exc + + return { + "user_id": user_id, + "department_id": department_id, + "created_at": now, + } + + async def remove_department( + self, + db_path: Path, + user_id: str, + department_id: str, + ) -> bool: + """Remove a user's department assignment. + + Args: + db_path: Path to the auth SQLite DB. + user_id: User id. + department_id: Department id to remove. + + Returns: + ``True`` if a row was deleted, ``False`` if the assignment + did not exist. + """ + async with aiosqlite.connect(str(db_path)) as db: + cursor = await db.execute( + "DELETE FROM user_departments WHERE user_id = ? AND department_id = ?", + (user_id, department_id), + ) + await db.commit() + return cursor.rowcount > 0 + + async def list_user_departments( + self, + db_path: Path, + user_id: str, + ) -> list[dict[str, Any]]: + """Return the list of departments a user is assigned to. + + Each item is ``{id, name, is_active}`` — the ``is_active`` flag + reflects the department's active state (not the user's). + """ + async with aiosqlite.connect(str(db_path)) as db: + db.row_factory = aiosqlite.Row + cursor = await db.execute( + "SELECT d.id, d.name, d.is_active " + "FROM departments d " + "INNER JOIN user_departments ud ON ud.department_id = d.id " + "WHERE ud.user_id = ? " + "ORDER BY d.name ASC", + (user_id,), + ) + rows = await cursor.fetchall() + return [ + { + "id": row["id"], + "name": row["name"], + "is_active": bool(row["is_active"]), + } + for row in rows + ] + + # ------------------------------------------------------------------ + # Internal helpers + # ------------------------------------------------------------------ + + async def _fetch_departments( + self, + db: aiosqlite.Connection, + user_id: str, + ) -> list[dict[str, Any]]: + """Fetch the departments for a user (helper for list/get). + + Reuses the caller's open connection to avoid an extra round-trip. + Returns a list of ``{id, name}`` dicts. + """ + db.row_factory = aiosqlite.Row + cursor = await db.execute( + "SELECT d.id, d.name " + "FROM departments d " + "INNER JOIN user_departments ud ON ud.department_id = d.id " + "WHERE ud.user_id = ? " + "ORDER BY d.name ASC", + (user_id,), + ) + rows = await cursor.fetchall() + return [{"id": row["id"], "name": row["name"]} for row in rows] + + +# --------------------------------------------------------------------------- +# Module-level singleton (overridable in tests via set_user_service) +# --------------------------------------------------------------------------- + + +_user_service: UserService | None = None + + +def get_user_service() -> UserService: + """Return the process-wide :class:`UserService` (lazy singleton).""" + global _user_service + if _user_service is None: + _user_service = UserService() + return _user_service + + +def set_user_service(service: UserService | None) -> None: + """Inject a custom :class:`UserService` (used by tests).""" + global _user_service + _user_service = service diff --git a/src/agentkit/server/auth/providers/local.py b/src/agentkit/server/auth/providers/local.py index de1f8d5..706b61c 100644 --- a/src/agentkit/server/auth/providers/local.py +++ b/src/agentkit/server/auth/providers/local.py @@ -22,12 +22,14 @@ from __future__ import annotations import logging import os +import uuid from pathlib import Path +from typing import Any import aiosqlite -from ..models import DEFAULT_AUTH_DB_PATH -from ..password import verify_password +from ..models import DEFAULT_AUTH_DB_PATH, user_row_to_dict +from ..password import hash_password, verify_password from .user import User logger = logging.getLogger(__name__) @@ -138,6 +140,93 @@ class LocalAuthProvider: await db.commit() logger.info(f"Revoked user {user_id} via LocalAuthProvider") + async def create_user( + self, + username: str, + email: str, + password: str, + role: str = "member", + is_terminal_authorized: bool = False, + is_server_terminal_authorized: bool = False, + created_by: str | None = None, + ) -> dict[str, Any]: + """Create a new user in the local ``users`` table. + + Args: + username: Unique username. + email: Unique email address. + password: Plain-text password (will be bcrypt-hashed with + cost factor 12 before storage). + role: Role name (``member`` / ``operator`` / ``admin``). + Defaults to ``member``. + is_terminal_authorized: Whether the user may use the local + terminal. Defaults to ``False``. + is_server_terminal_authorized: Whether the user may use the + server terminal. Defaults to ``False``. + created_by: Optional user id of the admin who created this + user (audit trail). + + Returns: + The newly-created user as a dict (via + :func:`agentkit.server.auth.models.user_row_to_dict`). + + Raises: + ValueError: If a user with the same username or email + already exists (catches SQLite ``IntegrityError`` on the + ``username`` / ``email`` UNIQUE constraints). + """ + user_id = str(uuid.uuid4()) + password_hash = hash_password(password) + now = _now_iso() + + try: + async with aiosqlite.connect(str(self._db_path)) as db: + db.row_factory = aiosqlite.Row + await db.execute( + "INSERT INTO users " + "(id, username, email, password_hash, role, is_active, " + " is_terminal_authorized, is_server_terminal_authorized, " + " created_at, updated_at, created_by) " + "VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)", + ( + user_id, + username, + email, + password_hash, + role, + 1, + 1 if is_terminal_authorized else 0, + 1 if is_server_terminal_authorized else 0, + now, + now, + created_by, + ), + ) + await db.commit() + cursor = await db.execute( + "SELECT id, username, email, password_hash, role, is_active, " + "is_terminal_authorized, is_server_terminal_authorized, " + "created_at, updated_at, last_login_at, created_by " + "FROM users WHERE id = ?", + (user_id,), + ) + row = await cursor.fetchone() + except aiosqlite.IntegrityError as exc: + # SQLite IntegrityError message includes the column name; we + # inspect it to give the caller a useful error. If for some + # reason the message is unparseable, fall back to a generic + # "duplicate" message. + msg = str(exc).lower() + if "username" in msg: + raise ValueError(f"User with username {username!r} already exists") from exc + if "email" in msg: + raise ValueError(f"User with email {email!r} already exists") from exc + raise ValueError(f"User already exists: {exc}") from exc + + assert row is not None # we just inserted it + logger.info(f"Created user {username!r} (id={user_id}) via LocalAuthProvider") + return user_row_to_dict(row) + def _row_to_user(row: aiosqlite.Row) -> User: """Convert a ``users`` row to a :class:`User` value object.""" diff --git a/src/agentkit/server/routes/admin.py b/src/agentkit/server/routes/admin.py new file mode 100644 index 0000000..02646ec --- /dev/null +++ b/src/agentkit/server/routes/admin.py @@ -0,0 +1,552 @@ +"""Admin REST routes — department CRUD + skill/KB bindings (U2). + +This module hosts the *department-scoped* admin endpoints under +``/api/v1/admin/departments/*``. It is a sibling of the existing +``admin_router`` in :mod:`agentkit.server.routes.auth` (which hosts +session-management endpoints). The two routers coexist independently: +``auth.admin_router`` keeps its session endpoints, this module adds +department endpoints, and ``app.py`` mounts both under ``/api/v1``. + +All endpoints here require the ``USER_MANAGE`` permission (admin role), +enforced by the :func:`_require_admin` dependency. The dependency is +re-defined locally to avoid a hard dependency on ``routes.auth`` at +import time (keeps the module self-contained and test-friendly). +""" + +from __future__ import annotations + +import logging +from pathlib import Path +from typing import Any + +from fastapi import APIRouter, Depends, HTTPException, Request +from pydantic import BaseModel, ConfigDict + +from agentkit.server.admin.department_service import get_department_service +from agentkit.server.admin.user_service import get_user_service +from agentkit.server.auth.dependencies import require_authenticated +from agentkit.server.auth.models import DEFAULT_AUTH_DB_PATH, init_auth_db +from agentkit.server.auth.permissions import Permission, has_permission + +logger = logging.getLogger(__name__) + +admin_router = APIRouter(prefix="/admin", tags=["admin"]) + + +# --------------------------------------------------------------------------- +# Local helpers (mirror routes.auth — kept local to avoid import cycles) +# --------------------------------------------------------------------------- + + +async def _require_admin( + request: Request, + user: dict[str, Any] = Depends(require_authenticated), +) -> dict[str, Any]: + """Require the ``USER_MANAGE`` permission (admin role).""" + if not has_permission(user, Permission.USER_MANAGE): + raise HTTPException(status_code=403, detail="Admin permission required") + return user + + +def _resolve_db_path(request: Request) -> Path: + """Resolve the auth DB path from ``app.state`` (mirrors routes.auth).""" + path = getattr(request.app.state, "auth_db_path", None) + return Path(path) if path else DEFAULT_AUTH_DB_PATH + + +async def _ensure_db(request: Request) -> Path: + """Ensure the auth DB exists; returns the resolved path.""" + db_path = _resolve_db_path(request) + if not db_path.exists(): + await init_auth_db(db_path) + return db_path + + +# --------------------------------------------------------------------------- +# Pydantic request bodies +# --------------------------------------------------------------------------- + + +class DepartmentCreateRequest(BaseModel): + """Body for ``POST /admin/departments``.""" + + model_config = ConfigDict(extra="forbid") + + name: str + description: str = "" + + +class DepartmentUpdateRequest(BaseModel): + """Body for ``PATCH /admin/departments/{id}``.""" + + model_config = ConfigDict(extra="forbid") + + name: str | None = None + description: str | None = None + + +# --------------------------------------------------------------------------- +# Department CRUD endpoints +# --------------------------------------------------------------------------- + + +@admin_router.post("/departments", status_code=201) +async def create_department( + payload: DepartmentCreateRequest, + request: Request, + admin: dict[str, Any] = Depends(_require_admin), +) -> dict[str, Any]: + """Create a new department. + + Returns 201 with the department dict on success, 409 if a department + with the same name already exists. + """ + db_path = await _ensure_db(request) + svc = get_department_service() + try: + return await svc.create_department(db_path, payload.name, payload.description) + except ValueError as exc: + raise HTTPException(status_code=409, detail=str(exc)) from exc + + +@admin_router.get("/departments") +async def list_departments( + request: Request, + include_inactive: bool = True, + admin: dict[str, Any] = Depends(_require_admin), +) -> list[dict[str, Any]]: + """List all departments.""" + db_path = await _ensure_db(request) + svc = get_department_service() + return await svc.list_departments(db_path, include_inactive=include_inactive) + + +@admin_router.get("/departments/{department_id}") +async def get_department( + department_id: str, + request: Request, + admin: dict[str, Any] = Depends(_require_admin), +) -> dict[str, Any]: + """Return a single department by id. 404 if not found.""" + db_path = await _ensure_db(request) + svc = get_department_service() + dept = await svc.get_department(db_path, department_id) + if dept is None: + raise HTTPException(status_code=404, detail="Department not found") + return dept + + +@admin_router.patch("/departments/{department_id}") +async def update_department( + department_id: str, + payload: DepartmentUpdateRequest, + request: Request, + admin: dict[str, Any] = Depends(_require_admin), +) -> dict[str, Any]: + """Partially update a department. + + Returns 200 with the updated dict, 404 if not found, 409 on duplicate + name. + """ + db_path = await _ensure_db(request) + svc = get_department_service() + try: + return await svc.update_department( + db_path, + department_id, + name=payload.name, + description=payload.description, + ) + except ValueError as exc: + msg = str(exc) + if "not found" in msg: + raise HTTPException(status_code=404, detail=msg) from exc + raise HTTPException(status_code=409, detail=msg) from exc + + +@admin_router.post("/departments/{department_id}/disable") +async def disable_department( + department_id: str, + request: Request, + admin: dict[str, Any] = Depends(_require_admin), +) -> dict[str, Any]: + """Set ``is_active=False`` on a department.""" + db_path = await _ensure_db(request) + svc = get_department_service() + try: + return await svc.set_department_active(db_path, department_id, is_active=False) + except ValueError as exc: + raise HTTPException(status_code=404, detail=str(exc)) from exc + + +@admin_router.post("/departments/{department_id}/enable") +async def enable_department( + department_id: str, + request: Request, + admin: dict[str, Any] = Depends(_require_admin), +) -> dict[str, Any]: + """Set ``is_active=True`` on a department.""" + db_path = await _ensure_db(request) + svc = get_department_service() + try: + return await svc.set_department_active(db_path, department_id, is_active=True) + except ValueError as exc: + raise HTTPException(status_code=404, detail=str(exc)) from exc + + +@admin_router.delete("/departments/{department_id}") +async def delete_department( + department_id: str, + request: Request, + admin: dict[str, Any] = Depends(_require_admin), +) -> dict[str, Any]: + """Delete a department. + + Returns 200 ``{deleted: true}`` on success. Returns 400 if the + department still has users assigned (admin must remove them first). + Returns 404 if the department does not exist. + """ + db_path = await _ensure_db(request) + svc = get_department_service() + try: + deleted = await svc.delete_department(db_path, department_id) + except ValueError as exc: + raise HTTPException(status_code=400, detail=str(exc)) from exc + if not deleted: + raise HTTPException(status_code=404, detail="Department not found") + return {"deleted": True} + + +# --------------------------------------------------------------------------- +# Department skill bindings +# --------------------------------------------------------------------------- + + +@admin_router.post( + "/departments/{department_id}/skills/{skill_name}", + status_code=201, +) +async def bind_skill( + department_id: str, + skill_name: str, + request: Request, + admin: dict[str, Any] = Depends(_require_admin), +) -> dict[str, Any]: + """Bind a skill to a department. 409 on duplicate binding.""" + db_path = await _ensure_db(request) + svc = get_department_service() + try: + return await svc.bind_skill(db_path, department_id, skill_name) + except ValueError as exc: + msg = str(exc) + if "not found" in msg: + raise HTTPException(status_code=404, detail=msg) from exc + raise HTTPException(status_code=409, detail=msg) from exc + + +@admin_router.delete( + "/departments/{department_id}/skills/{skill_name}", +) +async def unbind_skill( + department_id: str, + skill_name: str, + request: Request, + admin: dict[str, Any] = Depends(_require_admin), +) -> dict[str, Any]: + """Remove a skill binding. Idempotent — returns 200 even if no row existed.""" + db_path = await _ensure_db(request) + svc = get_department_service() + await svc.unbind_skill(db_path, department_id, skill_name) + return {"unbound": True} + + +@admin_router.get("/departments/{department_id}/skills") +async def list_department_skills( + department_id: str, + request: Request, + admin: dict[str, Any] = Depends(_require_admin), +) -> list[str]: + """List skill names bound to the department.""" + db_path = await _ensure_db(request) + svc = get_department_service() + return await svc.list_department_skills(db_path, department_id) + + +# --------------------------------------------------------------------------- +# Department KB bindings +# --------------------------------------------------------------------------- + + +@admin_router.post( + "/departments/{department_id}/kb/{kb_source_id}", + status_code=201, +) +async def bind_kb( + department_id: str, + kb_source_id: str, + request: Request, + admin: dict[str, Any] = Depends(_require_admin), +) -> dict[str, Any]: + """Bind a KB source to a department. 409 on duplicate binding.""" + db_path = await _ensure_db(request) + svc = get_department_service() + try: + return await svc.bind_kb(db_path, department_id, kb_source_id) + except ValueError as exc: + msg = str(exc) + if "not found" in msg: + raise HTTPException(status_code=404, detail=msg) from exc + raise HTTPException(status_code=409, detail=msg) from exc + + +@admin_router.delete( + "/departments/{department_id}/kb/{kb_source_id}", +) +async def unbind_kb( + department_id: str, + kb_source_id: str, + request: Request, + admin: dict[str, Any] = Depends(_require_admin), +) -> dict[str, Any]: + """Remove a KB binding. Idempotent — returns 200 even if no row existed.""" + db_path = await _ensure_db(request) + svc = get_department_service() + await svc.unbind_kb(db_path, department_id, kb_source_id) + return {"unbound": True} + + +@admin_router.get("/departments/{department_id}/kb") +async def list_department_kbs( + department_id: str, + request: Request, + admin: dict[str, Any] = Depends(_require_admin), +) -> list[str]: + """List KB source ids bound to the department.""" + db_path = await _ensure_db(request) + svc = get_department_service() + return await svc.list_department_kbs(db_path, department_id) + + +# --------------------------------------------------------------------------- +# User CRUD endpoints (U3) +# --------------------------------------------------------------------------- + + +class UserCreateRequest(BaseModel): + """Body for ``POST /admin/users``.""" + + model_config = ConfigDict(extra="forbid") + + username: str + email: str + password: str + role: str = "member" + department_ids: list[str] | None = None + + +class UserUpdateRequest(BaseModel): + """Body for ``PATCH /admin/users/{user_id}``.""" + + model_config = ConfigDict(extra="forbid") + + role: str | None = None + is_active: bool | None = None + is_terminal_authorized: bool | None = None + is_server_terminal_authorized: bool | None = None + + +class ResetPasswordRequest(BaseModel): + """Body for ``POST /admin/users/{user_id}/reset-password``.""" + + model_config = ConfigDict(extra="forbid") + + new_password: str + + +@admin_router.post("/users", status_code=201) +async def create_user( + payload: UserCreateRequest, + request: Request, + admin: dict[str, Any] = Depends(_require_admin), +) -> dict[str, Any]: + """Create a new user. + + Returns 201 with the user dict (including ``departments``) on + success. Returns 409 on duplicate username or email, or if any of + the ``department_ids`` is already assigned. Returns 404 if any of + the ``department_ids`` does not exist. + """ + db_path = await _ensure_db(request) + svc = get_user_service() + try: + return await svc.create_user( + db_path, + username=payload.username, + email=payload.email, + password=payload.password, + role=payload.role, + department_ids=payload.department_ids, + created_by=admin.get("user_id"), + ) + except ValueError as exc: + msg = str(exc) + if "not found" in msg: + raise HTTPException(status_code=404, detail=msg) from exc + raise HTTPException(status_code=409, detail=msg) from exc + + +@admin_router.get("/users") +async def list_users( + request: Request, + department_id: str | None = None, + include_inactive: bool = True, + admin: dict[str, Any] = Depends(_require_admin), +) -> list[dict[str, Any]]: + """List users, optionally filtered by department.""" + db_path = await _ensure_db(request) + svc = get_user_service() + return await svc.list_users( + db_path, + department_id=department_id, + include_inactive=include_inactive, + ) + + +@admin_router.get("/users/{user_id}") +async def get_user( + user_id: str, + request: Request, + admin: dict[str, Any] = Depends(_require_admin), +) -> dict[str, Any]: + """Return a single user by id (including departments). 404 if not found.""" + db_path = await _ensure_db(request) + svc = get_user_service() + user = await svc.get_user(db_path, user_id) + if user is None: + raise HTTPException(status_code=404, detail="User not found") + return user + + +@admin_router.patch("/users/{user_id}") +async def update_user( + user_id: str, + payload: UserUpdateRequest, + request: Request, + admin: dict[str, Any] = Depends(_require_admin), +) -> dict[str, Any]: + """Partially update a user. + + Returns 200 with the updated dict, 404 if the user does not exist. + """ + db_path = await _ensure_db(request) + svc = get_user_service() + try: + return await svc.update_user( + db_path, + user_id, + role=payload.role, + is_active=payload.is_active, + is_terminal_authorized=payload.is_terminal_authorized, + is_server_terminal_authorized=payload.is_server_terminal_authorized, + ) + except ValueError as exc: + raise HTTPException(status_code=404, detail=str(exc)) from exc + + +@admin_router.delete("/users/{user_id}") +async def delete_user( + user_id: str, + request: Request, + admin: dict[str, Any] = Depends(_require_admin), +) -> dict[str, Any]: + """Soft-delete a user (set ``is_active=0``). + + Returns 200 ``{deleted: true}`` on success, 404 if the user does + not exist or is already inactive. + """ + db_path = await _ensure_db(request) + svc = get_user_service() + deleted = await svc.delete_user(db_path, user_id) + if not deleted: + raise HTTPException(status_code=404, detail="User not found") + return {"deleted": True} + + +@admin_router.post("/users/{user_id}/reset-password") +async def reset_password( + user_id: str, + payload: ResetPasswordRequest, + request: Request, + admin: dict[str, Any] = Depends(_require_admin), +) -> dict[str, Any]: + """Reset a user's password and revoke all their sessions. + + Returns 200 ``{reset: true}`` on success, 404 if the user does not + exist. + """ + db_path = await _ensure_db(request) + svc = get_user_service() + reset = await svc.reset_password(db_path, user_id, payload.new_password) + if not reset: + raise HTTPException(status_code=404, detail="User not found") + return {"reset": True} + + +@admin_router.post( + "/users/{user_id}/departments/{department_id}", + status_code=201, +) +async def assign_department( + user_id: str, + department_id: str, + request: Request, + admin: dict[str, Any] = Depends(_require_admin), +) -> dict[str, Any]: + """Assign a user to a department. + + Returns 201 ``{assigned: true}`` on success. Returns 404 if the + department does not exist. Returns 409 if the user is already + assigned to this department. + """ + db_path = await _ensure_db(request) + svc = get_user_service() + try: + await svc.assign_department(db_path, user_id, department_id) + except ValueError as exc: + msg = str(exc) + if "not found" in msg: + raise HTTPException(status_code=404, detail=msg) from exc + raise HTTPException(status_code=409, detail=msg) from exc + return {"assigned": True} + + +@admin_router.delete("/users/{user_id}/departments/{department_id}") +async def remove_department( + user_id: str, + department_id: str, + request: Request, + admin: dict[str, Any] = Depends(_require_admin), +) -> dict[str, Any]: + """Remove a user's department assignment. + + Returns 200 ``{removed: true}`` on success, 404 if the assignment + does not exist. + """ + db_path = await _ensure_db(request) + svc = get_user_service() + removed = await svc.remove_department(db_path, user_id, department_id) + if not removed: + raise HTTPException(status_code=404, detail="Assignment not found") + return {"removed": True} + + +@admin_router.get("/users/{user_id}/departments") +async def list_user_departments( + user_id: str, + request: Request, + admin: dict[str, Any] = Depends(_require_admin), +) -> list[dict[str, Any]]: + """List the departments a user is assigned to.""" + db_path = await _ensure_db(request) + svc = get_user_service() + return await svc.list_user_departments(db_path, user_id) diff --git a/tests/integration/admin/test_user_routes.py b/tests/integration/admin/test_user_routes.py new file mode 100644 index 0000000..fbc6f0d --- /dev/null +++ b/tests/integration/admin/test_user_routes.py @@ -0,0 +1,614 @@ +"""Integration tests for the user admin routes (U3). + +Uses FastAPI TestClient with a test app that mounts only the +``admin_router`` from ``routes.admin``. The ``_require_admin`` dependency +is overridden via ``app.dependency_overrides`` so the tests don't need +real JWTs — they can simulate admin and non-admin callers directly. + +The SessionService singleton is also installed against the temp DB so +that ``reset_password`` can revoke sessions end-to-end. +""" + +from __future__ import annotations + +import sqlite3 +import uuid +from datetime import datetime, timezone +from pathlib import Path +from typing import Any + +import pytest +from fastapi import FastAPI, HTTPException +from fastapi.testclient import TestClient + +from agentkit.server.auth.models import init_auth_db +from agentkit.server.auth.session_service import SessionService, set_session_service +from agentkit.server.routes import admin as admin_routes_module + + +# --------------------------------------------------------------------------- +# Fixtures +# --------------------------------------------------------------------------- + + +@pytest.fixture +async def tmp_auth_db(tmp_path: Path) -> Path: + db_path = tmp_path / "admin_users.db" + await init_auth_db(db_path) + return db_path + + +@pytest.fixture +def session_service(tmp_auth_db: Path): + """Install a SessionService singleton backed by the temp DB. + + Required so that ``UserService.reset_password`` can find the + SessionService via ``get_session_service()`` and revoke sessions. + """ + svc = SessionService(db_path=tmp_auth_db) + set_session_service(svc) + yield svc + set_session_service(None) + + +def _make_admin_user() -> dict[str, Any]: + return {"user_id": "admin-1", "username": "admin", "role": "admin"} + + +@pytest.fixture +def admin_app(tmp_auth_db: Path) -> FastAPI: + """A minimal FastAPI app with only the admin router mounted. + + The ``_require_admin`` dependency is overridden to return a fake admin + user. Individual tests can re-override it to simulate a non-admin. + """ + app = FastAPI() + app.state.auth_db_path = str(tmp_auth_db) + app.include_router(admin_routes_module.admin_router, prefix="/api/v1") + + # Default: allow admin access. + app.dependency_overrides[admin_routes_module._require_admin] = lambda: _make_admin_user() + return app + + +@pytest.fixture +def admin_client( + admin_app: FastAPI, session_service: SessionService +) -> TestClient: + """TestClient with admin access and SessionService installed. + + The ``session_service`` fixture is listed as a dependency so that + the singleton is installed before any request runs. + """ + return TestClient(admin_app) + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def _create_department(client: TestClient, name: str, description: str = "") -> dict: + resp = client.post( + "/api/v1/admin/departments", + json={"name": name, "description": description}, + ) + assert resp.status_code == 201, resp.text + return resp.json() + + +def _create_user( + client: TestClient, + *, + username: str = "alice", + email: str = "alice@example.com", + password: str = "Secret123!", + role: str = "member", + department_ids: list[str] | None = None, +) -> dict: + payload: dict[str, Any] = { + "username": username, + "email": email, + "password": password, + "role": role, + } + if department_ids is not None: + payload["department_ids"] = department_ids + resp = client.post("/api/v1/admin/users", json=payload) + assert resp.status_code == 201, resp.text + return resp.json() + + +def _insert_session(db_path: Path, user_id: str, session_id: str | None = None) -> str: + """Insert a minimal active auth_sessions row synchronously.""" + session_id = session_id or str(uuid.uuid4()) + now = datetime.now(timezone.utc).isoformat() + with sqlite3.connect(str(db_path)) as db: + db.execute( + "INSERT INTO auth_sessions " + "(id, user_id, refresh_token_hash, device_fingerprint, device_label, " + " ip, user_agent, auth_provider, created_at, last_active_at, expires_at, " + " revoked, revoked_reason, previous_session_id) " + "VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)", + ( + session_id, + user_id, + f"hash-{session_id[:8]}", + "fp-test", + "Test device", + "127.0.0.1", + "test-agent", + "local", + now, + now, + now, + 0, + None, + None, + ), + ) + db.commit() + return session_id + + +def _count_active_sessions(db_path: Path, user_id: str) -> int: + with sqlite3.connect(str(db_path)) as db: + cursor = db.execute( + "SELECT COUNT(*) FROM auth_sessions WHERE user_id = ? AND revoked = 0", + (user_id,), + ) + return int(cursor.fetchone()[0]) + + +# --------------------------------------------------------------------------- +# POST /admin/users +# --------------------------------------------------------------------------- + + +class TestCreateUser: + def test_create_returns_201_with_user_dict(self, admin_client: TestClient): + resp = admin_client.post( + "/api/v1/admin/users", + json={ + "username": "alice", + "email": "alice@example.com", + "password": "Secret123!", + "role": "member", + }, + ) + assert resp.status_code == 201 + body = resp.json() + assert body["id"] + assert body["username"] == "alice" + assert body["email"] == "alice@example.com" + assert body["role"] == "member" + assert body["is_active"] is True + assert body["departments"] == [] + # password_hash must NOT be in the response. + assert "password_hash" not in body + + def test_create_with_department_ids_assigns_departments( + self, admin_client: TestClient + ): + eng = _create_department(admin_client, "Engineering") + resp = admin_client.post( + "/api/v1/admin/users", + json={ + "username": "alice", + "email": "alice@example.com", + "password": "Secret123!", + "department_ids": [eng["id"]], + }, + ) + assert resp.status_code == 201 + body = resp.json() + assert len(body["departments"]) == 1 + assert body["departments"][0]["id"] == eng["id"] + + def test_create_duplicate_username_returns_409(self, admin_client: TestClient): + _create_user(admin_client, username="alice", email="alice@example.com") + resp = admin_client.post( + "/api/v1/admin/users", + json={ + "username": "alice", + "email": "other@example.com", + "password": "Secret123!", + }, + ) + assert resp.status_code == 409 + + def test_create_duplicate_email_returns_409(self, admin_client: TestClient): + _create_user(admin_client, username="alice", email="alice@example.com") + resp = admin_client.post( + "/api/v1/admin/users", + json={ + "username": "alice2", + "email": "alice@example.com", + "password": "Secret123!", + }, + ) + assert resp.status_code == 409 + + def test_create_with_nonexistent_department_returns_404( + self, admin_client: TestClient + ): + resp = admin_client.post( + "/api/v1/admin/users", + json={ + "username": "alice", + "email": "alice@example.com", + "password": "Secret123!", + "department_ids": [str(uuid.uuid4())], + }, + ) + assert resp.status_code == 404 + + def test_non_admin_returns_403(self, admin_app: FastAPI): + admin_app.dependency_overrides[admin_routes_module._require_admin] = _raise_forbidden + client = TestClient(admin_app) + resp = client.post( + "/api/v1/admin/users", + json={ + "username": "alice", + "email": "alice@example.com", + "password": "Secret123!", + }, + ) + assert resp.status_code == 403 + + +# --------------------------------------------------------------------------- +# GET /admin/users +# --------------------------------------------------------------------------- + + +class TestListUsers: + def test_list_returns_all_users(self, admin_client: TestClient): + _create_user(admin_client, username="alice", email="alice@example.com") + _create_user(admin_client, username="bob", email="bob@example.com") + resp = admin_client.get("/api/v1/admin/users") + assert resp.status_code == 200 + names = {u["username"] for u in resp.json()} + assert names == {"alice", "bob"} + + def test_list_excludes_inactive_when_asked(self, admin_client: TestClient): + alice = _create_user(admin_client, username="alice", email="alice@example.com") + _create_user(admin_client, username="bob", email="bob@example.com") + # Soft-delete alice. + admin_client.delete(f"/api/v1/admin/users/{alice['id']}") + resp = admin_client.get( + "/api/v1/admin/users", params={"include_inactive": False} + ) + assert resp.status_code == 200 + names = {u["username"] for u in resp.json()} + assert names == {"bob"} + + def test_list_filtered_by_department(self, admin_client: TestClient): + eng = _create_department(admin_client, "Engineering") + hr = _create_department(admin_client, "HR") + alice = _create_user( + admin_client, username="alice", email="alice@example.com" + ) + bob = _create_user(admin_client, username="bob", email="bob@example.com") + admin_client.post(f"/api/v1/admin/users/{alice['id']}/departments/{eng['id']}") + admin_client.post(f"/api/v1/admin/users/{bob['id']}/departments/{hr['id']}") + + resp = admin_client.get( + "/api/v1/admin/users", params={"department_id": eng["id"]} + ) + assert resp.status_code == 200 + users = resp.json() + assert len(users) == 1 + assert users[0]["username"] == "alice" + + def test_non_admin_returns_403(self, admin_app: FastAPI): + admin_app.dependency_overrides[admin_routes_module._require_admin] = _raise_forbidden + client = TestClient(admin_app) + resp = client.get("/api/v1/admin/users") + assert resp.status_code == 403 + + +# --------------------------------------------------------------------------- +# GET /admin/users/{user_id} +# --------------------------------------------------------------------------- + + +class TestGetUser: + def test_get_returns_user_with_departments(self, admin_client: TestClient): + eng = _create_department(admin_client, "Engineering") + alice = _create_user( + admin_client, + username="alice", + email="alice@example.com", + department_ids=[eng["id"]], + ) + resp = admin_client.get(f"/api/v1/admin/users/{alice['id']}") + assert resp.status_code == 200 + body = resp.json() + assert body["username"] == "alice" + assert len(body["departments"]) == 1 + assert body["departments"][0]["name"] == "Engineering" + + def test_get_unknown_id_returns_404(self, admin_client: TestClient): + resp = admin_client.get(f"/api/v1/admin/users/{uuid.uuid4()}") + assert resp.status_code == 404 + + def test_non_admin_returns_403(self, admin_app: FastAPI): + admin_app.dependency_overrides[admin_routes_module._require_admin] = _raise_forbidden + client = TestClient(admin_app) + resp = client.get(f"/api/v1/admin/users/{uuid.uuid4()}") + assert resp.status_code == 403 + + +# --------------------------------------------------------------------------- +# PATCH /admin/users/{user_id} +# --------------------------------------------------------------------------- + + +class TestUpdateUser: + def test_update_role(self, admin_client: TestClient): + alice = _create_user(admin_client, username="alice", email="alice@example.com") + resp = admin_client.patch( + f"/api/v1/admin/users/{alice['id']}", + json={"role": "admin"}, + ) + assert resp.status_code == 200 + assert resp.json()["role"] == "admin" + + def test_update_is_active(self, admin_client: TestClient): + alice = _create_user(admin_client, username="alice", email="alice@example.com") + resp = admin_client.patch( + f"/api/v1/admin/users/{alice['id']}", + json={"is_active": False}, + ) + assert resp.status_code == 200 + assert resp.json()["is_active"] is False + + def test_update_terminal_authorized_flags(self, admin_client: TestClient): + alice = _create_user(admin_client, username="alice", email="alice@example.com") + resp = admin_client.patch( + f"/api/v1/admin/users/{alice['id']}", + json={ + "is_terminal_authorized": True, + "is_server_terminal_authorized": True, + }, + ) + assert resp.status_code == 200 + body = resp.json() + assert body["is_terminal_authorized"] is True + assert body["is_server_terminal_authorized"] is True + + def test_update_unknown_id_returns_404(self, admin_client: TestClient): + resp = admin_client.patch( + f"/api/v1/admin/users/{uuid.uuid4()}", + json={"role": "admin"}, + ) + assert resp.status_code == 404 + + def test_non_admin_returns_403(self, admin_app: FastAPI): + admin_app.dependency_overrides[admin_routes_module._require_admin] = _raise_forbidden + client = TestClient(admin_app) + resp = client.patch( + f"/api/v1/admin/users/{uuid.uuid4()}", + json={"role": "admin"}, + ) + assert resp.status_code == 403 + + +# --------------------------------------------------------------------------- +# DELETE /admin/users/{user_id} +# --------------------------------------------------------------------------- + + +class TestDeleteUser: + def test_delete_returns_200(self, admin_client: TestClient): + alice = _create_user(admin_client, username="alice", email="alice@example.com") + resp = admin_client.delete(f"/api/v1/admin/users/{alice['id']}") + assert resp.status_code == 200 + assert resp.json() == {"deleted": True} + + def test_delete_is_soft(self, admin_client: TestClient, tmp_auth_db: Path): + alice = _create_user(admin_client, username="alice", email="alice@example.com") + admin_client.delete(f"/api/v1/admin/users/{alice['id']}") + # The row must still exist (soft delete). + with sqlite3.connect(str(tmp_auth_db)) as db: + cursor = db.execute( + "SELECT is_active FROM users WHERE id = ?", (alice["id"],) + ) + row = cursor.fetchone() + assert row is not None + assert bool(row[0]) is False + + def test_delete_unknown_id_returns_404(self, admin_client: TestClient): + resp = admin_client.delete(f"/api/v1/admin/users/{uuid.uuid4()}") + assert resp.status_code == 404 + + def test_delete_already_inactive_returns_404(self, admin_client: TestClient): + alice = _create_user(admin_client, username="alice", email="alice@example.com") + # First delete succeeds. + first = admin_client.delete(f"/api/v1/admin/users/{alice['id']}") + assert first.status_code == 200 + # Second delete on the same (now-inactive) user returns 404. + second = admin_client.delete(f"/api/v1/admin/users/{alice['id']}") + assert second.status_code == 404 + + def test_non_admin_returns_403(self, admin_app: FastAPI): + admin_app.dependency_overrides[admin_routes_module._require_admin] = _raise_forbidden + client = TestClient(admin_app) + resp = client.delete(f"/api/v1/admin/users/{uuid.uuid4()}") + assert resp.status_code == 403 + + +# --------------------------------------------------------------------------- +# POST /admin/users/{user_id}/reset-password +# --------------------------------------------------------------------------- + + +class TestResetPassword: + def test_reset_returns_200(self, admin_client: TestClient): + alice = _create_user(admin_client, username="alice", email="alice@example.com") + resp = admin_client.post( + f"/api/v1/admin/users/{alice['id']}/reset-password", + json={"new_password": "NewSecret456!"}, + ) + assert resp.status_code == 200 + assert resp.json() == {"reset": True} + + def test_reset_revokes_sessions( + self, admin_client: TestClient, tmp_auth_db: Path + ): + alice = _create_user(admin_client, username="alice", email="alice@example.com") + _insert_session(tmp_auth_db, alice["id"]) + _insert_session(tmp_auth_db, alice["id"]) + assert _count_active_sessions(tmp_auth_db, alice["id"]) == 2 + + resp = admin_client.post( + f"/api/v1/admin/users/{alice['id']}/reset-password", + json={"new_password": "NewSecret456!"}, + ) + assert resp.status_code == 200 + assert _count_active_sessions(tmp_auth_db, alice["id"]) == 0 + + def test_reset_unknown_id_returns_404(self, admin_client: TestClient): + resp = admin_client.post( + f"/api/v1/admin/users/{uuid.uuid4()}/reset-password", + json={"new_password": "NewSecret456!"}, + ) + assert resp.status_code == 404 + + def test_non_admin_returns_403(self, admin_app: FastAPI): + admin_app.dependency_overrides[admin_routes_module._require_admin] = _raise_forbidden + client = TestClient(admin_app) + resp = client.post( + f"/api/v1/admin/users/{uuid.uuid4()}/reset-password", + json={"new_password": "NewSecret456!"}, + ) + assert resp.status_code == 403 + + +# --------------------------------------------------------------------------- +# POST /admin/users/{user_id}/departments/{department_id} +# --------------------------------------------------------------------------- + + +class TestAssignDepartment: + def test_assign_returns_201(self, admin_client: TestClient): + eng = _create_department(admin_client, "Engineering") + alice = _create_user(admin_client, username="alice", email="alice@example.com") + resp = admin_client.post( + f"/api/v1/admin/users/{alice['id']}/departments/{eng['id']}" + ) + assert resp.status_code == 201 + assert resp.json() == {"assigned": True} + + def test_assign_duplicate_returns_409(self, admin_client: TestClient): + eng = _create_department(admin_client, "Engineering") + alice = _create_user(admin_client, username="alice", email="alice@example.com") + admin_client.post(f"/api/v1/admin/users/{alice['id']}/departments/{eng['id']}") + resp = admin_client.post( + f"/api/v1/admin/users/{alice['id']}/departments/{eng['id']}" + ) + assert resp.status_code == 409 + + def test_assign_nonexistent_department_returns_404( + self, admin_client: TestClient + ): + alice = _create_user(admin_client, username="alice", email="alice@example.com") + resp = admin_client.post( + f"/api/v1/admin/users/{alice['id']}/departments/{uuid.uuid4()}" + ) + assert resp.status_code == 404 + + def test_non_admin_returns_403(self, admin_app: FastAPI): + admin_app.dependency_overrides[admin_routes_module._require_admin] = _raise_forbidden + client = TestClient(admin_app) + resp = client.post( + f"/api/v1/admin/users/{uuid.uuid4()}/departments/{uuid.uuid4()}" + ) + assert resp.status_code == 403 + + +# --------------------------------------------------------------------------- +# DELETE /admin/users/{user_id}/departments/{department_id} +# --------------------------------------------------------------------------- + + +class TestRemoveDepartment: + def test_remove_returns_200(self, admin_client: TestClient): + eng = _create_department(admin_client, "Engineering") + alice = _create_user( + admin_client, + username="alice", + email="alice@example.com", + department_ids=[eng["id"]], + ) + resp = admin_client.delete( + f"/api/v1/admin/users/{alice['id']}/departments/{eng['id']}" + ) + assert resp.status_code == 200 + assert resp.json() == {"removed": True} + + def test_remove_nonexistent_assignment_returns_404( + self, admin_client: TestClient + ): + eng = _create_department(admin_client, "Engineering") + alice = _create_user(admin_client, username="alice", email="alice@example.com") + resp = admin_client.delete( + f"/api/v1/admin/users/{alice['id']}/departments/{eng['id']}" + ) + assert resp.status_code == 404 + + def test_non_admin_returns_403(self, admin_app: FastAPI): + admin_app.dependency_overrides[admin_routes_module._require_admin] = _raise_forbidden + client = TestClient(admin_app) + resp = client.delete( + f"/api/v1/admin/users/{uuid.uuid4()}/departments/{uuid.uuid4()}" + ) + assert resp.status_code == 403 + + +# --------------------------------------------------------------------------- +# GET /admin/users/{user_id}/departments +# --------------------------------------------------------------------------- + + +class TestListUserDepartments: + def test_list_returns_departments(self, admin_client: TestClient): + eng = _create_department(admin_client, "Engineering") + hr = _create_department(admin_client, "HR") + alice = _create_user( + admin_client, + username="alice", + email="alice@example.com", + department_ids=[eng["id"], hr["id"]], + ) + resp = admin_client.get(f"/api/v1/admin/users/{alice['id']}/departments") + assert resp.status_code == 200 + depts = resp.json() + assert len(depts) == 2 + names = {d["name"] for d in depts} + assert names == {"Engineering", "HR"} + + def test_list_returns_empty_for_user_with_no_departments( + self, admin_client: TestClient + ): + alice = _create_user(admin_client, username="alice", email="alice@example.com") + resp = admin_client.get(f"/api/v1/admin/users/{alice['id']}/departments") + assert resp.status_code == 200 + assert resp.json() == [] + + def test_non_admin_returns_403(self, admin_app: FastAPI): + admin_app.dependency_overrides[admin_routes_module._require_admin] = _raise_forbidden + client = TestClient(admin_app) + resp = client.get(f"/api/v1/admin/users/{uuid.uuid4()}/departments") + assert resp.status_code == 403 + + +# --------------------------------------------------------------------------- +# Helpers for non-admin simulation +# --------------------------------------------------------------------------- + + +def _raise_forbidden() -> dict[str, Any]: + """Dependency override that simulates a non-admin (403) response.""" + raise HTTPException(status_code=403, detail="Admin permission required") diff --git a/tests/unit/admin/test_user_service.py b/tests/unit/admin/test_user_service.py new file mode 100644 index 0000000..5867de8 --- /dev/null +++ b/tests/unit/admin/test_user_service.py @@ -0,0 +1,806 @@ +"""Unit tests for UserService (U3 — user CRUD + multi-department assignment). + +Covers: +- Happy path: create_user → list_users returns it → get_user → update_user → + reset_password (verify old sessions revoked) → assign_department → + list_user_departments → remove_department → delete_user (soft) +- Edge case: create duplicate username → ValueError +- Edge case: create duplicate email → ValueError +- Edge case: get/update/delete non-existent user → None/ValueError/False +- Edge case: assign duplicate department → ValueError +- Edge case: assign non-existent department → ValueError +- Edge case: list_users filtered by department_id +- Edge case: delete_user is soft (is_active=0, row still exists) +""" + +from __future__ import annotations + +import uuid +from collections.abc import Iterator +from datetime import datetime, timezone +from pathlib import Path +from typing import Any + +import aiosqlite +import bcrypt +import pytest + +from agentkit.server.admin.department_service import DepartmentService +from agentkit.server.admin.user_service import UserService +from agentkit.server.auth.models import init_auth_db +from agentkit.server.auth.session_service import ( + REVOKE_REASON_PASSWORD_CHANGED, + SessionService, + set_session_service, +) + + +# --------------------------------------------------------------------------- +# Fixtures +# --------------------------------------------------------------------------- + + +@pytest.fixture +async def fresh_db(tmp_path: Path) -> Path: + """A brand-new auth DB on a fresh path (no data).""" + db_path = tmp_path / "auth.db" + await init_auth_db(db_path) + return db_path + + +@pytest.fixture +def service() -> UserService: + return UserService() + + +@pytest.fixture +def department_service() -> DepartmentService: + return DepartmentService() + + +@pytest.fixture +def session_service(fresh_db: Path) -> Iterator[SessionService]: + """A SessionService backed by the same temp DB as fresh_db. + + Installed as the process-wide singleton so that + ``UserService.reset_password`` can find it via + ``get_session_service()``. + """ + svc = SessionService(db_path=fresh_db) + set_session_service(svc) + yield svc + # Restore the lazy default after the test. + set_session_service(None) + + +def _now_iso() -> str: + return datetime.now(timezone.utc).isoformat() + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +async def _create_test_department( + dept_service: DepartmentService, db_path: Path, name: str = "Engineering" +) -> dict[str, Any]: + """Create a department for use in user-assignment tests.""" + return await dept_service.create_department(db_path, name) + + +async def _create_session_for_user( + db_path: Path, user_id: str, session_id: str | None = None +) -> str: + """Insert a minimal active auth_sessions row and return its id.""" + session_id = session_id or str(uuid.uuid4()) + now = _now_iso() + expires = datetime.now(timezone.utc).isoformat() + async with aiosqlite.connect(str(db_path)) as db: + await db.execute( + "INSERT INTO auth_sessions " + "(id, user_id, refresh_token_hash, device_fingerprint, device_label, " + " ip, user_agent, auth_provider, created_at, last_active_at, expires_at, " + " revoked, revoked_reason, previous_session_id) " + "VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)", + ( + session_id, + user_id, + f"hash-{session_id[:8]}", + "fp-test", + "Test device", + "127.0.0.1", + "test-agent", + "local", + now, + now, + expires, + 0, + None, + None, + ), + ) + await db.commit() + return session_id + + +async def _count_active_sessions(db_path: Path, user_id: str) -> int: + """Count active (non-revoked) sessions for a user.""" + async with aiosqlite.connect(str(db_path)) as db: + cursor = await db.execute( + "SELECT COUNT(*) FROM auth_sessions WHERE user_id = ? AND revoked = 0", + (user_id,), + ) + row = await cursor.fetchone() + return int(row[0]) if row else 0 + + +async def _fetch_user_row(db_path: Path, user_id: str) -> dict[str, Any] | None: + """Fetch a raw user row (including is_active) directly from the DB.""" + async with aiosqlite.connect(str(db_path)) as db: + db.row_factory = aiosqlite.Row + cursor = await db.execute("SELECT * FROM users WHERE id = ?", (user_id,)) + row = await cursor.fetchone() + return dict(row) if row else None + + +# --------------------------------------------------------------------------- +# create_user happy path +# --------------------------------------------------------------------------- + + +class TestCreateUserHappyPath: + async def test_create_returns_user_dict(self, service: UserService, fresh_db: Path): + user = await service.create_user( + fresh_db, "alice", "alice@example.com", "Secret123!" + ) + assert user["id"] + assert user["username"] == "alice" + assert user["email"] == "alice@example.com" + assert user["role"] == "member" + assert user["is_active"] is True + assert user["is_terminal_authorized"] is False + assert user["is_server_terminal_authorized"] is False + assert user["created_by"] is None + assert "created_at" in user + assert "updated_at" in user + assert "password_hash" not in user # password_hash must not leak + assert user["departments"] == [] + + async def test_create_with_role_and_flags(self, service: UserService, fresh_db: Path): + user = await service.create_user( + fresh_db, + "bob", + "bob@example.com", + "Secret123!", + role="admin", + ) + assert user["role"] == "admin" + + async def test_create_with_created_by(self, service: UserService, fresh_db: Path): + admin_id = str(uuid.uuid4()) + user = await service.create_user( + fresh_db, + "carol", + "carol@example.com", + "Secret123!", + created_by=admin_id, + ) + assert user["created_by"] == admin_id + + async def test_create_with_department_ids_assigns_departments( + self, + service: UserService, + department_service: DepartmentService, + fresh_db: Path, + ): + eng = await _create_test_department(department_service, fresh_db, "Engineering") + hr = await _create_test_department(department_service, fresh_db, "HR") + user = await service.create_user( + fresh_db, + "dave", + "dave@example.com", + "Secret123!", + department_ids=[eng["id"], hr["id"]], + ) + dept_ids = {d["id"] for d in user["departments"]} + assert dept_ids == {eng["id"], hr["id"]} + + async def test_password_is_bcrypt_hashed( + self, service: UserService, fresh_db: Path + ): + """The stored password_hash must be a bcrypt hash, not plaintext.""" + user = await service.create_user( + fresh_db, "eve", "eve@example.com", "MyPassword123!" + ) + row = await _fetch_user_row(fresh_db, user["id"]) + assert row is not None + stored_hash = row["password_hash"] + # bcrypt hashes start with $2b$ (or $2a$/$2y$). + assert stored_hash.startswith("$2") + # And the hash must verify against the original password. + assert bcrypt.checkpw(b"MyPassword123!", stored_hash.encode()) + + +# --------------------------------------------------------------------------- +# create_user edge cases +# --------------------------------------------------------------------------- + + +class TestCreateUserEdgeCases: + async def test_duplicate_username_raises_value_error( + self, service: UserService, fresh_db: Path + ): + await service.create_user(fresh_db, "alice", "alice@example.com", "Secret123!") + with pytest.raises(ValueError, match="username"): + await service.create_user( + fresh_db, "alice", "other@example.com", "Secret123!" + ) + + async def test_duplicate_email_raises_value_error( + self, service: UserService, fresh_db: Path + ): + await service.create_user(fresh_db, "alice", "alice@example.com", "Secret123!") + with pytest.raises(ValueError, match="email"): + await service.create_user( + fresh_db, "alice2", "alice@example.com", "Secret123!" + ) + + async def test_create_with_nonexistent_department_id_raises( + self, + service: UserService, + fresh_db: Path, + ): + bogus_dept_id = str(uuid.uuid4()) + with pytest.raises(ValueError, match="Department.*not found"): + await service.create_user( + fresh_db, + "frank", + "frank@example.com", + "Secret123!", + department_ids=[bogus_dept_id], + ) + + async def test_create_with_duplicate_department_id_raises( + self, + service: UserService, + department_service: DepartmentService, + fresh_db: Path, + ): + eng = await _create_test_department(department_service, fresh_db, "Engineering") + with pytest.raises(ValueError, match="already assigned"): + await service.create_user( + fresh_db, + "gina", + "gina@example.com", + "Secret123!", + department_ids=[eng["id"], eng["id"]], + ) + + +# --------------------------------------------------------------------------- +# list_users +# --------------------------------------------------------------------------- + + +class TestListUsers: + async def test_list_returns_all_users(self, service: UserService, fresh_db: Path): + await service.create_user(fresh_db, "alice", "alice@example.com", "Secret123!") + await service.create_user(fresh_db, "bob", "bob@example.com", "Secret123!") + users = await service.list_users(fresh_db) + assert len(users) == 2 + names = {u["username"] for u in users} + assert names == {"alice", "bob"} + + async def test_list_excludes_inactive_when_asked( + self, service: UserService, fresh_db: Path + ): + alice = await service.create_user( + fresh_db, "alice", "alice@example.com", "Secret123!" + ) + await service.create_user(fresh_db, "bob", "bob@example.com", "Secret123!") + # Soft-delete alice. + await service.delete_user(fresh_db, alice["id"]) + active_only = await service.list_users(fresh_db, include_inactive=False) + assert len(active_only) == 1 + assert active_only[0]["username"] == "bob" + all_users = await service.list_users(fresh_db, include_inactive=True) + assert len(all_users) == 2 + + async def test_list_filtered_by_department( + self, + service: UserService, + department_service: DepartmentService, + fresh_db: Path, + ): + eng = await _create_test_department(department_service, fresh_db, "Engineering") + hr = await _create_test_department(department_service, fresh_db, "HR") + + alice = await service.create_user( + fresh_db, "alice", "alice@example.com", "Secret123!" + ) + bob = await service.create_user( + fresh_db, "bob", "bob@example.com", "Secret123!" + ) + await service.assign_department(fresh_db, alice["id"], eng["id"]) + await service.assign_department(fresh_db, bob["id"], hr["id"]) + + eng_users = await service.list_users(fresh_db, department_id=eng["id"]) + assert len(eng_users) == 1 + assert eng_users[0]["username"] == "alice" + + hr_users = await service.list_users(fresh_db, department_id=hr["id"]) + assert len(hr_users) == 1 + assert hr_users[0]["username"] == "bob" + + async def test_list_includes_departments_for_each_user( + self, + service: UserService, + department_service: DepartmentService, + fresh_db: Path, + ): + eng = await _create_test_department(department_service, fresh_db, "Engineering") + alice = await service.create_user( + fresh_db, "alice", "alice@example.com", "Secret123!" + ) + await service.assign_department(fresh_db, alice["id"], eng["id"]) + users = await service.list_users(fresh_db) + assert len(users) == 1 + assert len(users[0]["departments"]) == 1 + assert users[0]["departments"][0]["id"] == eng["id"] + assert users[0]["departments"][0]["name"] == "Engineering" + + +# --------------------------------------------------------------------------- +# get_user +# --------------------------------------------------------------------------- + + +class TestGetUser: + async def test_get_returns_user_with_departments( + self, + service: UserService, + department_service: DepartmentService, + fresh_db: Path, + ): + eng = await _create_test_department(department_service, fresh_db, "Engineering") + user = await service.create_user( + fresh_db, "alice", "alice@example.com", "Secret123!" + ) + await service.assign_department(fresh_db, user["id"], eng["id"]) + fetched = await service.get_user(fresh_db, user["id"]) + assert fetched is not None + assert fetched["username"] == "alice" + assert len(fetched["departments"]) == 1 + assert fetched["departments"][0]["name"] == "Engineering" + + async def test_get_returns_none_for_unknown_id( + self, service: UserService, fresh_db: Path + ): + result = await service.get_user(fresh_db, str(uuid.uuid4())) + assert result is None + + +# --------------------------------------------------------------------------- +# update_user +# --------------------------------------------------------------------------- + + +class TestUpdateUser: + async def test_update_role(self, service: UserService, fresh_db: Path): + user = await service.create_user( + fresh_db, "alice", "alice@example.com", "Secret123!" + ) + updated = await service.update_user(fresh_db, user["id"], role="admin") + assert updated["role"] == "admin" + + async def test_update_is_active(self, service: UserService, fresh_db: Path): + user = await service.create_user( + fresh_db, "alice", "alice@example.com", "Secret123!" + ) + updated = await service.update_user(fresh_db, user["id"], is_active=False) + assert updated["is_active"] is False + + async def test_update_terminal_authorized_flags( + self, service: UserService, fresh_db: Path + ): + user = await service.create_user( + fresh_db, "alice", "alice@example.com", "Secret123!" + ) + updated = await service.update_user( + fresh_db, + user["id"], + is_terminal_authorized=True, + is_server_terminal_authorized=True, + ) + assert updated["is_terminal_authorized"] is True + assert updated["is_server_terminal_authorized"] is True + + async def test_update_partial_only_changes_provided_fields( + self, service: UserService, fresh_db: Path + ): + user = await service.create_user( + fresh_db, "alice", "alice@example.com", "Secret123!" + ) + # Only update role; other fields should be preserved. + updated = await service.update_user(fresh_db, user["id"], role="admin") + assert updated["role"] == "admin" + assert updated["is_active"] is True + assert updated["is_terminal_authorized"] is False + + async def test_update_nonexistent_raises_value_error( + self, service: UserService, fresh_db: Path + ): + with pytest.raises(ValueError, match="not found"): + await service.update_user(fresh_db, str(uuid.uuid4()), role="admin") + + async def test_update_no_fields_provided_is_noop( + self, service: UserService, fresh_db: Path + ): + user = await service.create_user( + fresh_db, "alice", "alice@example.com", "Secret123!" + ) + updated = await service.update_user(fresh_db, user["id"]) + assert updated["username"] == "alice" + + +# --------------------------------------------------------------------------- +# delete_user (soft delete) +# --------------------------------------------------------------------------- + + +class TestDeleteUser: + async def test_delete_returns_true_for_active_user( + self, service: UserService, fresh_db: Path + ): + user = await service.create_user( + fresh_db, "alice", "alice@example.com", "Secret123!" + ) + deleted = await service.delete_user(fresh_db, user["id"]) + assert deleted is True + + async def test_delete_is_soft(self, service: UserService, fresh_db: Path): + """delete_user must set is_active=0 but NOT remove the row.""" + user = await service.create_user( + fresh_db, "alice", "alice@example.com", "Secret123!" + ) + await service.delete_user(fresh_db, user["id"]) + # Row must still exist. + row = await _fetch_user_row(fresh_db, user["id"]) + assert row is not None + # But is_active must be 0. + assert bool(row["is_active"]) is False + + async def test_delete_returns_false_for_unknown_id( + self, service: UserService, fresh_db: Path + ): + deleted = await service.delete_user(fresh_db, str(uuid.uuid4())) + assert deleted is False + + async def test_delete_returns_false_for_already_inactive( + self, service: UserService, fresh_db: Path + ): + user = await service.create_user( + fresh_db, "alice", "alice@example.com", "Secret123!" + ) + first = await service.delete_user(fresh_db, user["id"]) + assert first is True + # Second delete on the same (now-inactive) user must return False. + second = await service.delete_user(fresh_db, user["id"]) + assert second is False + + +# --------------------------------------------------------------------------- +# reset_password +# --------------------------------------------------------------------------- + + +class TestResetPassword: + async def test_reset_password_returns_true( + self, + service: UserService, + session_service: SessionService, + fresh_db: Path, + ): + user = await service.create_user( + fresh_db, "alice", "alice@example.com", "OldPassword123!" + ) + reset = await service.reset_password(fresh_db, user["id"], "NewPassword456!") + assert reset is True + + async def test_reset_password_updates_hash( + self, + service: UserService, + session_service: SessionService, + fresh_db: Path, + ): + user = await service.create_user( + fresh_db, "alice", "alice@example.com", "OldPassword123!" + ) + old_row = await _fetch_user_row(fresh_db, user["id"]) + await service.reset_password(fresh_db, user["id"], "NewPassword456!") + new_row = await _fetch_user_row(fresh_db, user["id"]) + assert new_row["password_hash"] != old_row["password_hash"] + # New hash must verify against the new password. + assert bcrypt.checkpw(b"NewPassword456!", new_row["password_hash"].encode()) + + async def test_reset_password_revokes_all_sessions( + self, + service: UserService, + session_service: SessionService, + fresh_db: Path, + ): + user = await service.create_user( + fresh_db, "alice", "alice@example.com", "OldPassword123!" + ) + # Create two active sessions for the user. + await _create_session_for_user(fresh_db, user["id"]) + await _create_session_for_user(fresh_db, user["id"]) + assert await _count_active_sessions(fresh_db, user["id"]) == 2 + + await service.reset_password(fresh_db, user["id"], "NewPassword456!") + # All sessions must be revoked. + assert await _count_active_sessions(fresh_db, user["id"]) == 0 + + async def test_reset_password_records_revoked_reason( + self, + service: UserService, + session_service: SessionService, + fresh_db: Path, + ): + user = await service.create_user( + fresh_db, "alice", "alice@example.com", "OldPassword123!" + ) + session_id = await _create_session_for_user(fresh_db, user["id"]) + await service.reset_password(fresh_db, user["id"], "NewPassword456!") + + async with aiosqlite.connect(str(fresh_db)) as db: + db.row_factory = aiosqlite.Row + cursor = await db.execute( + "SELECT revoked, revoked_reason FROM auth_sessions WHERE id = ?", + (session_id,), + ) + row = await cursor.fetchone() + assert row is not None + assert bool(row["revoked"]) is True + assert row["revoked_reason"] == REVOKE_REASON_PASSWORD_CHANGED + + async def test_reset_password_returns_false_for_unknown_user( + self, + service: UserService, + session_service: SessionService, + fresh_db: Path, + ): + reset = await service.reset_password( + fresh_db, str(uuid.uuid4()), "NewPassword456!" + ) + assert reset is False + + +# --------------------------------------------------------------------------- +# Department assignment +# --------------------------------------------------------------------------- + + +class TestAssignDepartment: + async def test_assign_returns_assignment_dict( + self, + service: UserService, + department_service: DepartmentService, + fresh_db: Path, + ): + eng = await _create_test_department(department_service, fresh_db, "Engineering") + user = await service.create_user( + fresh_db, "alice", "alice@example.com", "Secret123!" + ) + result = await service.assign_department(fresh_db, user["id"], eng["id"]) + assert result["user_id"] == user["id"] + assert result["department_id"] == eng["id"] + assert "created_at" in result + + async def test_assign_duplicate_raises_value_error( + self, + service: UserService, + department_service: DepartmentService, + fresh_db: Path, + ): + eng = await _create_test_department(department_service, fresh_db, "Engineering") + user = await service.create_user( + fresh_db, "alice", "alice@example.com", "Secret123!" + ) + await service.assign_department(fresh_db, user["id"], eng["id"]) + with pytest.raises(ValueError, match="already assigned"): + await service.assign_department(fresh_db, user["id"], eng["id"]) + + async def test_assign_nonexistent_department_raises_value_error( + self, + service: UserService, + fresh_db: Path, + ): + user = await service.create_user( + fresh_db, "alice", "alice@example.com", "Secret123!" + ) + with pytest.raises(ValueError, match="Department.*not found"): + await service.assign_department(fresh_db, user["id"], str(uuid.uuid4())) + + +class TestRemoveDepartment: + async def test_remove_returns_true( + self, + service: UserService, + department_service: DepartmentService, + fresh_db: Path, + ): + eng = await _create_test_department(department_service, fresh_db, "Engineering") + user = await service.create_user( + fresh_db, "alice", "alice@example.com", "Secret123!" + ) + await service.assign_department(fresh_db, user["id"], eng["id"]) + removed = await service.remove_department(fresh_db, user["id"], eng["id"]) + assert removed is True + # Confirm the assignment is gone. + depts = await service.list_user_departments(fresh_db, user["id"]) + assert depts == [] + + async def test_remove_returns_false_for_nonexistent_assignment( + self, + service: UserService, + department_service: DepartmentService, + fresh_db: Path, + ): + eng = await _create_test_department(department_service, fresh_db, "Engineering") + user = await service.create_user( + fresh_db, "alice", "alice@example.com", "Secret123!" + ) + removed = await service.remove_department(fresh_db, user["id"], eng["id"]) + assert removed is False + + +class TestListUserDepartments: + async def test_list_returns_departments_with_active_flag( + self, + service: UserService, + department_service: DepartmentService, + fresh_db: Path, + ): + eng = await _create_test_department(department_service, fresh_db, "Engineering") + hr = await _create_test_department(department_service, fresh_db, "HR") + user = await service.create_user( + fresh_db, "alice", "alice@example.com", "Secret123!" + ) + await service.assign_department(fresh_db, user["id"], eng["id"]) + await service.assign_department(fresh_db, user["id"], hr["id"]) + + depts = await service.list_user_departments(fresh_db, user["id"]) + assert len(depts) == 2 + # Ordered by name. + assert depts[0]["name"] == "Engineering" + assert depts[1]["name"] == "HR" + assert all("is_active" in d for d in depts) + assert all(d["is_active"] is True for d in depts) + + async def test_list_returns_empty_for_user_with_no_departments( + self, service: UserService, fresh_db: Path + ): + user = await service.create_user( + fresh_db, "alice", "alice@example.com", "Secret123!" + ) + depts = await service.list_user_departments(fresh_db, user["id"]) + assert depts == [] + + async def test_list_reflects_department_active_state( + self, + service: UserService, + department_service: DepartmentService, + fresh_db: Path, + ): + eng = await _create_test_department(department_service, fresh_db, "Engineering") + user = await service.create_user( + fresh_db, "alice", "alice@example.com", "Secret123!" + ) + await service.assign_department(fresh_db, user["id"], eng["id"]) + # Disable the department. + await department_service.set_department_active(fresh_db, eng["id"], False) + depts = await service.list_user_departments(fresh_db, user["id"]) + assert len(depts) == 1 + assert depts[0]["is_active"] is False + + +# --------------------------------------------------------------------------- +# End-to-end happy path +# --------------------------------------------------------------------------- + + +class TestEndToEndHappyPath: + async def test_full_user_lifecycle( + self, + service: UserService, + department_service: DepartmentService, + session_service: SessionService, + fresh_db: Path, + ): + """Exercise the full user lifecycle in order: + create → list → get → update → reset_password → assign_department → + list_user_departments → remove_department → delete (soft). + """ + # 1. Create a department and a user assigned to it. + eng = await _create_test_department(department_service, fresh_db, "Engineering") + user = await service.create_user( + fresh_db, + "alice", + "alice@example.com", + "InitialPassword123!", + department_ids=[eng["id"]], + ) + assert user["username"] == "alice" + assert len(user["departments"]) == 1 + + # 2. list_users returns the new user. + users = await service.list_users(fresh_db) + assert len(users) == 1 + assert users[0]["id"] == user["id"] + + # 3. get_user returns the same user with departments. + fetched = await service.get_user(fresh_db, user["id"]) + assert fetched is not None + assert fetched["username"] == "alice" + assert len(fetched["departments"]) == 1 + + # 4. update_user changes the role. + updated = await service.update_user(fresh_db, user["id"], role="admin") + assert updated["role"] == "admin" + + # 5. reset_password updates the hash and revokes sessions. + await _create_session_for_user(fresh_db, user["id"]) + assert await _count_active_sessions(fresh_db, user["id"]) == 1 + reset = await service.reset_password(fresh_db, user["id"], "NewPassword456!") + assert reset is True + assert await _count_active_sessions(fresh_db, user["id"]) == 0 + + # 6. assign_department adds a second department. + hr = await _create_test_department(department_service, fresh_db, "HR") + await service.assign_department(fresh_db, user["id"], hr["id"]) + + # 7. list_user_departments returns both. + depts = await service.list_user_departments(fresh_db, user["id"]) + assert len(depts) == 2 + + # 8. remove_department removes one. + removed = await service.remove_department(fresh_db, user["id"], eng["id"]) + assert removed is True + depts = await service.list_user_departments(fresh_db, user["id"]) + assert len(depts) == 1 + assert depts[0]["name"] == "HR" + + # 9. delete_user is a soft delete. + deleted = await service.delete_user(fresh_db, user["id"]) + assert deleted is True + row = await _fetch_user_row(fresh_db, user["id"]) + assert row is not None # row still exists + assert bool(row["is_active"]) is False # but inactive + + +# --------------------------------------------------------------------------- +# Singleton helpers +# --------------------------------------------------------------------------- + + +class TestSingletonHelpers: + def test_get_user_service_returns_singleton(self): + from agentkit.server.admin.user_service import ( + get_user_service, + set_user_service, + ) + + # Save the original singleton so we don't disturb other tests. + original = get_user_service() + try: + custom = UserService() + set_user_service(custom) + assert get_user_service() is custom + # Clearing falls back to a new lazy instance. + set_user_service(None) + new_one = get_user_service() + assert new_one is not custom + finally: + set_user_service(original)