mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-11-05 01:50:36 +00:00
* feat(authz): account disable/enable across every authentication surface (#4063 gap 3) Vertical slice 1 of the user-lifecycle work: an operator can suspend an account and every authentication surface rejects it, while the row — credentials, role, OAuth linkage — is retained so re-enabling restores exactly what was suspended. - users.disabled (NOT NULL, server default 0) via migration 0036_user_disabled; User/UserResponse carry and expose it. - Enforcement at all four surfaces: password login (no credential comparison happens), JWT session resolution (401 ACCOUNT_DISABLED — sessions die at the next request), PAT resolution (the generic 401, no oracle), OAuth provisioning (403 with a contact-admin message; the link is kept so re-enable restores access). - SQLiteUserRepository.set_disabled: single-column, serialized write (same idiom as update_system_role) with the last-ACTIVE-admin guard — disabled admins cannot authenticate, so they cannot restore management access; re-enabling is always allowed. Field-scoping completed both ways: the credential writer mirrors and never writes disabled, so a stale password-change snapshot cannot re-enable a suspended account. - PATCH /api/v1/admin/users/{id} becomes a partial account update (system_role and/or disabled), each field routed to its field-scoped writer; empty bodies 422. Tests 11->18: route (disable/re-enable, last-active-admin 409, empty 422, combined update), real-repo (field isolation both directions, disabled-admin-not-active, guard inside the serialized write, re-enable always), login rejection with the CORRECT password (only the disabled gate can reject) plus resolver-visible flag. Mutations verified red: guard removed (M19), login gate removed (M20 — after hardening the test with a real hash so the gate is load-bearing). * fix(authz): portable boolean default, alternate-authenticator suspension, active-admin role guard Review round 1 on #6566 — three P1s: - [PG default] sa.text("0") is a raw integer expression PostgreSQL rejects as a BOOLEAN default, breaking startup migration and fresh create_all bootstraps alike. Both the migration and the ORM model now use sa.false()/false() — dialect-rendered, portable. - [alternate authenticators] WebSockets bypass AuthMiddleware, so a disabled user's still-valid cookie could open a NEW browser/stream connection and drive the retained session; the standalone LangGraph authenticate callback accepted the same suspended JWT. Both now route through a shared post-lookup validator (validate_resolved_session_user in deps.py — token_version + disabled in one place) that the HTTP dependency also uses, so a lifecycle change lands everywhere at once. - [active-admin role guard] update_system_role counted every admin row, so 'disable B, then demote self' — or a combined {system_role, disabled} self-update — could strand the deployment with zero administrators able to authenticate or re-enable anyone. The demotion guard now counts ACTIVE admins only (disabled admins cannot authenticate), under the same serialization boundary. Tests 18->22: shared-validator verdicts, WS admission after suspension (real token, real repo), LangGraph-callback admission after suspension, disable-then-demote-self + combined self-update both rejected (fake-repo route level and real-repo update_system_role level). Mutations verified red: helper's disabled check removed (M21 — three admission tests fail), active-admin predicate reverted to all-admins (M22 — repo-level strand scenario fails). 366 tests green on the affected surface. * fix(migrations): chain 0036_user_disabled after upstream 0037 The maintainer main-merge brought 0037_project_document_summaries, which branched from 0035 independently of this PR's 0036 — alembic now sees two heads and every engine bootstrap fails. Chain 0036 after 0037 (single head; revision numbers are labels, the graph is what matters). * fix(migrations): renumber user_disabled to 0038 and follow the chain-test convention Upstream landed its own 0036 (run_idempotency_request) with 0037 chained after it while this PR was in review — my 0036_user_disabled collided on the number and my interim chain-fix (0036 revises 0037) left the graph in a state upstream's chain tests reject (heads == [0037] hardcoded). - 0036_user_disabled -> 0038_user_disabled, revises 0037_project_document_summaries: single head 0038. - Chain tests follow the repo convention (each migration PR appends the new head): 0032/0035 gain the DOC_SUMMARIES link and CURRENT_HEAD = 0038; 0037's single-head assertion moves to CURRENT_HEAD with its own-revision link check unchanged. * fix(migrations): pin the remaining chain tests to the 0038_user_disabled head The 0027/0028, 0029 and 0034 chain tests still pinned the single head to 0037_project_document_summaries (and 0034 asserted the head's parent was 0036_run_idempotency_request), so the renumbered lifecycle migration broke two unit-test shards. Follow the chain-test convention: every pinned head moves to 0038_user_disabled and 0034 walks the chain through 0037. * fix(authz): enforce account suspension on the internal-auth dispatch surface Round-3 review finding: the internal-auth principal is synthesized from trusted headers without a users-row lookup, so a suspended user with an active channel binding kept driving the agent, and scheduler-owned runs (their fabricated in-process requests skip AuthMiddleware entirely) kept launching. Two gates now enforce the lifecycle there: - AuthMiddleware rejects owner-bound internal calls whose users row is disabled (401 account_disabled) at the single gate every internal HTTP call passes through. Auth-disabled mode is exempt (no suspendable accounts, and its owner header carries the synthetic identity), and owner ids without a users row stay allowed — a nonexistent account cannot be suspended. - start_run re-asserts it at run admission after the owner is resolved for attribution: the scheduler records the rejection as the occurrence's launch error and no run record is created. Also addresses the remaining round-3 nits: drop the inline token_version check duplicated by validate_resolved_session_user, correct the local_provider comment (login never surfaces ACCOUNT_DISABLED), refresh the stale 0038 docstring header, and restore the deps module caches in the two suspension tests instead of leaking a closed engine. * fix(auth): carry the disabled field on the frontend synthetic users UserResponse now always includes disabled, so the auth-disabled contract e2e (gateway /auth/me deep-equals the frontend synthetic user) failed without the field on AUTH_DISABLED_USER. Add disabled to both synthetic users and declare it optional on the zod schema — absent still parses as not disabled for pre-gap-3 backends. * fix(authz): log when the internal-owner suspension check is skipped The middleware gate's users-lookup failure path silently degraded to allow; keep the fail-open behavior but leave a WARNING with the exception, so a store that cannot be read is observable in production logs instead of invisible. --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
185 lines
6.3 KiB
Python
185 lines
6.3 KiB
Python
"""User repository interface for abstracting database operations."""
|
|
|
|
from abc import ABC, abstractmethod
|
|
|
|
from app.gateway.auth.models import User
|
|
|
|
|
|
class LastAdminRemainsError(RuntimeError):
|
|
"""Raised when a role write would demote the last remaining admin.
|
|
|
|
Bootstrap lockout protection: raised inside the same serialized
|
|
transaction that would perform the write, so the check-then-act window
|
|
that two concurrent demotions could race through is closed.
|
|
"""
|
|
|
|
|
|
class LastActiveAdminError(RuntimeError):
|
|
"""Raised when an account write would disable the last active admin.
|
|
|
|
"Active" = an enabled admin row: disabled admins cannot authenticate, so
|
|
they cannot restore the deployment's management access. Raised inside the
|
|
same serialized transaction that would perform the write (see
|
|
``LastAdminRemainsError`` for the race this closes).
|
|
"""
|
|
|
|
|
|
class UserNotFoundError(LookupError):
|
|
"""Raised when a user repository operation targets a non-existent row.
|
|
|
|
Subclass of :class:`LookupError` so callers that already catch
|
|
``LookupError`` for "missing entity" can keep working unchanged,
|
|
while specific call sites can pin to this class to distinguish
|
|
"concurrent delete during update" from other lookups.
|
|
"""
|
|
|
|
|
|
class UserRepository(ABC):
|
|
"""Abstract interface for user data storage.
|
|
|
|
Implement this interface to support different storage backends
|
|
(SQLite)
|
|
"""
|
|
|
|
@abstractmethod
|
|
async def create_user(self, user: User) -> User:
|
|
"""Create a new user.
|
|
|
|
Args:
|
|
user: User object to create
|
|
|
|
Returns:
|
|
Created User with ID assigned. ``email`` is the canonical
|
|
(lowercase) stored form, which may differ in case from what was
|
|
passed in -- implementations mutate the input ``user`` in place
|
|
to reflect this rather than returning a fresh object.
|
|
|
|
Raises:
|
|
ValueError: If email already exists
|
|
"""
|
|
raise NotImplementedError
|
|
|
|
@abstractmethod
|
|
async def create_first_admin(self, user: User) -> User | None:
|
|
"""Create *user* as the first admin, atomically.
|
|
|
|
Implementations must read the admin count and insert in one
|
|
serialized transaction: a count-then-create pair lets two concurrent
|
|
first-boot requests both find an empty system.
|
|
|
|
Returns:
|
|
The created User, or None if an admin already exists.
|
|
|
|
Raises:
|
|
ValueError: If email already exists
|
|
"""
|
|
raise NotImplementedError
|
|
|
|
@abstractmethod
|
|
async def get_user_by_id(self, user_id: str) -> User | None:
|
|
"""Get user by ID.
|
|
|
|
Args:
|
|
user_id: User UUID as string
|
|
|
|
Returns:
|
|
User if found, None otherwise
|
|
"""
|
|
raise NotImplementedError
|
|
|
|
@abstractmethod
|
|
async def get_user_by_email(self, email: str) -> User | None:
|
|
"""Get user by email.
|
|
|
|
Args:
|
|
email: User email address
|
|
|
|
Returns:
|
|
User if found, None otherwise
|
|
"""
|
|
raise NotImplementedError
|
|
|
|
@abstractmethod
|
|
async def set_disabled(self, user_id: str, disabled: bool) -> User:
|
|
"""Enable/disable an account as a single-column, serialized write.
|
|
|
|
Origin: admin user-management surface (RFC #4063 / #3462 gap 3).
|
|
Field-scoped like :meth:`update_system_role` — account-lifecycle
|
|
state has exactly one writer, and credential/role writers never
|
|
touch it. Disabling the last remaining ACTIVE admin raises
|
|
:class:`LastActiveAdminError` (re-enabling is always allowed).
|
|
"""
|
|
raise NotImplementedError
|
|
|
|
async def update_system_role(self, user_id: str, system_role: str) -> User:
|
|
"""Assign a user's system role as a single-column, serialized write.
|
|
|
|
Origin: admin role-assignment API (RFC #4063 / #3462 gap 2). Distinct
|
|
from :meth:`update_user` on purpose — role mutations and credential
|
|
writes are field-scoped against each other, so a password change
|
|
holding a stale account snapshot can never restore a revoked role,
|
|
and a role change can never overwrite credentials.
|
|
|
|
Raises:
|
|
UserNotFoundError: the target row does not exist.
|
|
LastAdminRemainsError: the write would demote the only remaining
|
|
admin (the count and the write share one serialized
|
|
transaction — two concurrent demotions cannot both pass).
|
|
"""
|
|
raise NotImplementedError
|
|
|
|
async def update_user(self, user: User) -> User:
|
|
"""Update an existing user.
|
|
|
|
Args:
|
|
user: User object with updated fields
|
|
|
|
Returns:
|
|
Updated User. ``email`` is the canonical (lowercase) stored
|
|
form -- implementations mutate the input ``user`` in place to
|
|
reflect this rather than returning a fresh object.
|
|
|
|
Raises:
|
|
UserNotFoundError: If no row exists for ``user.id``. This is
|
|
a hard failure (not a no-op) so callers cannot mistake a
|
|
concurrent-delete race for a successful update.
|
|
"""
|
|
raise NotImplementedError
|
|
|
|
@abstractmethod
|
|
async def count_users(self) -> int:
|
|
"""Return total number of registered users."""
|
|
raise NotImplementedError
|
|
|
|
@abstractmethod
|
|
async def list_user_ids(self) -> list[str]:
|
|
"""Return every registered user ID in deterministic creation order."""
|
|
raise NotImplementedError
|
|
|
|
@abstractmethod
|
|
async def list_users(self) -> list[User]:
|
|
"""Return all users ordered by creation (oldest first).
|
|
|
|
Origin: admin user-list surface (RFC #4063 / #3462 gap 2). Volume is
|
|
bounded by the deployment's user base; pagination can be added when a
|
|
real deployment needs it.
|
|
"""
|
|
raise NotImplementedError
|
|
|
|
async def count_admin_users(self) -> int:
|
|
"""Return number of users with system_role == 'admin'."""
|
|
raise NotImplementedError
|
|
|
|
@abstractmethod
|
|
async def get_user_by_oauth(self, provider: str, oauth_id: str) -> User | None:
|
|
"""Get user by OAuth provider and ID.
|
|
|
|
Args:
|
|
provider: OAuth provider name (e.g. 'github', 'google')
|
|
oauth_id: User ID from the OAuth provider
|
|
|
|
Returns:
|
|
User if found, None otherwise
|
|
"""
|
|
raise NotImplementedError
|