From 873a464629b31bbe9b009925a29ca7a6c366360b Mon Sep 17 00:00:00 2001 From: "tao.chen" <93983997+taochen-ct@users.noreply.github.com> Date: Fri, 7 Aug 2026 16:14:13 +0800 Subject: [PATCH] fix: auth error --- API.md | 11 +- backend/src/backend/platform.py | 154 ++++++++++++++---- ...0e1_seed_role_permissions_and_fix_scope.py | 30 +++- 3 files changed, 151 insertions(+), 44 deletions(-) diff --git a/API.md b/API.md index 8872341..611fe51 100644 --- a/API.md +++ b/API.md @@ -521,7 +521,7 @@ Base 前缀 `/api/v1/admin`。 > **不变量**: > - 每个 workspace 必须始终保留至少一个 `admin` 角色的活跃成员;对最后 admin 做降级 / 停用 / 删除 → 409。 -> - platform 必须始终保留至少一个 `active` 系统管理员;对最后系统管理员做降级 / 停用 / 删除 → 409。 +> - `PATCH /employees/{user_id}` 降级 admin → developer 时同样触发 workspace last-admin 守卫(因为 workspace 角色继承自 platform 角色,降级会级联到所有活跃 membership);platform 必须始终保留至少一个 `active` 系统管理员;对最后系统管理员做降级 / 停用 / 删除 → 409。 > - 系统管理员不能通过 `DELETE .../members/{self}` 把自己移除(403)。唯一退出方式是 `DELETE /workspaces/{id}` 软删整个 workspace,后者会级联软删所有成员。 > - workspace 与成员列表接口静默 `pageSize=100` 上限,无客户端分页参数(YAGNI);`GET /employees` 按契约返回全部未软删员工,不设隐藏上限。 > - 跨 workspace 操作**不**需要 `?workspace_id=` query 参数,与 `/api/v1/admin/...`(workspace 内成员管理)不要混淆。 @@ -608,7 +608,7 @@ Base 前缀 `/api/v1/admin`。 - 服务端默认 `member_status='active'`。 - 用户不存在或已软删除 → 404;用户状态不是 `active` → 409;用户**尚未分配平台角色**(`users.platform_role_id IS NULL`)→ 409 "目标用户尚未分配平台角色,无法加入 workspace"。 -- 用户已是该 workspace 成员 → 409。 +- 用户已是该 workspace 成员 → 409 "用户已是该 workspace 成员;workspace 角色继承自平台角色,要变更请 PATCH /api/v1/platform/employees/{user_id} 修改 role_code"。 - 新员工必须先通过 `POST /api/v1/platform/employees` 建立账号(可以同时传 `role_code=admin|developer`)。 ### 7.6 `PATCH /api/v1/platform/workspaces/{workspace_id}/members/{user_id}` @@ -698,11 +698,12 @@ Base 前缀 `/api/v1/admin`。 - 禁止通过该端点修改:`username`、`password`、`password_hash`、`platform_role_id`;请求体中包含这些字段 → 422。 - `role_code` 只能取 `admin` / `developer`;**不能通过本端点把 `platform_role_id` 置为 `null`**(降级为"无平台角色"须走单独的内部流程,前端不要尝试)。 -- `role_code` 改动会**立即影响**该用户在**所有** workspace 中的角色——因为 `workspace_members.role_id` 在 §7.5/§7.6 不再被端点改写,workspace 角色始终等于 `users.platform_role_id` 指向的 Roles 行。本端点是调整任何成员 workspace 角色的**唯一**入口。 +- `role_code` 改动会**立即级联同步**该用户**所有活跃 workspace 成员行**的 `workspace_members.role_id`(`PATCH /employees` 在事务内 `UPDATE workspace_members SET role_id=:new WHERE user_id=:uid AND is_deleted=0`)。否则 `/me` 返回的 `workspaces[].role_code` 与下游 `request_context` 都会读到过期角色。本端点是调整任何成员 workspace 角色的**唯一**入口。 - 自保护: - 修改自身 `status` 为非 `active` → 409 "不能停用当前登录账号"。 - 修改自身 `role_code` 为 `developer`(即降级系统管理员身份)→ 409 "不能降级自身管理员角色"。 - 最后系统管理员保护:当目标用户当前为 active 系统管理员,本次变更会让其离开"active 系统管理员"集合(降级角色 / 停用账号)时,平台必须仍保留至少一名 active 系统管理员,否则 → 409 "platform 必须保留至少一个 active 系统管理员"。**自保护在前、last-admin 计数在后**(参考 CLAUDE.md 工程笔记)。 +- **workspace last-admin 守卫**(降级路径):因为 `role_code` 改动会级联同步 `workspace_members.role_id`,把 `admin` 降级为 `developer` 时必须**额外**检查该用户在**每个**他是唯一 active admin 的 workspace 中仍有替补 admin,否则 → 409,错误信息列出将失去 admin 的 `workspace_code` 列表;提示"请先在这些 workspace 中指定其他 admin,再降级该用户"。`developer → admin` 升级路径不受此守卫约束(只增不减)。 - 目标用户不存在或已软删除 → 404 "用户不存在";`role_code` 对应的角色行不存在 → 422。 - **响应 200**:返回更新后的 `PlatformEmployeePayload`,`role_code` / `role_name` 反映最新的 `platform_role_id`。 @@ -819,8 +820,8 @@ Base 前缀 `/api/v1/admin`。 - **守卫顺序(load-bearing,不可调换)**: 1. 角色不存在 / `role_scope != 'platform'` → 404。 - 2. **`admin` 角色**:提交的 `permission_codes` 必须同时包含 `system.view` 与 `system.manage`,否则 → 409 "admin 角色必须保留 system.view 与 system.manage 权限"。这一条**先于**最后系统管理员计数,目的是让"把 admin 角色自己锁出系统管理入口"的操作在前置路径上就 409,便于前端直接报错而不必再走"我是最后一个 admin"分支。 - 3. **`admin` 角色 + 当前调用者修改的恰好是自己 `platform_role_id` 指向的角色**:若操作会让平台剩余 active 系统管理员为 0 → 409 "platform 必须保留至少一个 active 系统管理员"。**自保护检查先于计数检查**(参考 CLAUDE.md 工程笔记;放在最后会导致单测 mock 在 `is_system_admin=False` 时绕过计数 helper)。 + 2. **`admin` 角色**:提交的 `permission_codes` 必须同时包含 `system.view` 与 `system.manage`,否则 → 409 "admin 角色必须保留 system.view 与 system.manage 权限"。 + 3. **非 `admin` 角色**:`permission_codes` 中**禁止**含 `system.*` 项 → 422 "非 admin 角色不能拥有 system.* 权限: [...]"。menu permission 只控前端展示,后端 `/api/v1/platform/*` 鉴权仍按 `role_code == "admin"`;若让 developer 拿到 `system.manage`,前端会渲染"系统管理"入口但所有 platform API 调用 403,UX 割裂。 4. 任意 `permission_code` 不在 `permissions` 表活跃行中 → 422 "未知的 permission_code: [...]"。 5. 写入策略:diff-based —— 只 soft-delete `current \ new` 的关联,只 INSERT `new \ current` 的关联。重复提交同 payload 是 no-op;包含原有 codes 的 patch 不会触发 `(role_id, permission_id)` 主键冲突。**严禁**先全量 soft-delete 再全量 INSERT(会 `IntegrityError`,因为软删行仍占主键 slot)。 - 写入后,响应 `data.permission_codes` 为本次写入后的活跃集合,与再次 `GET §7.13` 完全一致。 diff --git a/backend/src/backend/platform.py b/backend/src/backend/platform.py index b4a773b..9c0e3bb 100644 --- a/backend/src/backend/platform.py +++ b/backend/src/backend/platform.py @@ -32,16 +32,30 @@ Platform employee roster:: PATCH /employees/{user_id} — update a user's profile, status or platform role DELETE /employees/{user_id} — soft delete a user (cascades to workspace memberships) +Role menu-permission management:: + + GET /roles — list platform roles with their permission_codes + GET /roles/{role_code}/permissions — one role's permission_codes + PATCH /roles/{role_code}/permissions — replace a role's permission set (diff-based) + Invariants ---------- * Every workspace must always retain at least one active ``admin`` member. + This is enforced on member PATCH/DELETE AND on + ``PATCH /employees/{user_id}`` demotions, because workspace role is + inherited from ``users.platform_role_id`` and demoting a platform + admin cascades to all of their active memberships. * A system admin cannot remove their own workspace membership via ``DELETE .../members/{self}``; the only escape is to delete the entire workspace, which cascades membership soft-deletion. * ``DELETE /workspaces/{id}`` is allowed from any non-disabled status and sets ``status='disabled'`` + ``is_deleted=1`` + ``deleted_at`` on the workspace and every one of its active memberships. +* The ``admin`` role must always keep ``system.view`` + ``system.manage`` + menu permissions; non-admin roles may never hold ``system.*`` + permissions. Menu permissions gate frontend rendering only — API + authorization always keys off ``role_code == 'admin'``. """ from __future__ import annotations @@ -423,7 +437,15 @@ async def update_platform_employee( context: SystemAdminContext = Depends(system_admin_context), session: AsyncSession = Depends(database_session), ) -> dict[str, Any]: - """Update a platform user's profile, status, or platform role.""" + """Update a platform user's profile, status, or platform role. + + Changing ``role_code`` cascades: every active ``workspace_members`` + row of the user is rewritten to the new role (workspace role is + inherited from the platform role). Demoting admin → developer is + rejected with 409 when it would leave any workspace without an + active admin member, or the platform without an active system + admin. Self-demotion is always rejected. + """ user = await session.get(Users, user_id) if user is None or user.is_deleted != 0: raise HTTPException(status.HTTP_404_NOT_FOUND, "用户不存在") @@ -469,6 +491,53 @@ async def update_platform_employee( "platform 必须保留至少一个 active 系统管理员", ) + # Workspace-level last-admin guard for the demote path. The role_code + # sync below rewrites ``workspace_members.role_id`` for every active + # membership of this user, so demoting admin → developer would + # silently strip workspace admin coverage anywhere this user is the + # sole active admin member. ``update_member`` / ``remove_member`` + # guard the same invariant via ``_count_active_admins``; this + # endpoint must too, now that it can change workspace roles. + demotes_admin = ( + is_current_system_admin + and new_role is not None + and new_role.role_code != "admin" + ) + if demotes_admin: + assert current_role is not None # implied by is_current_system_admin + admin_memberships = ( + await session.execute( + select(WorkspaceMembers.workspace_id) + .where( + WorkspaceMembers.user_id == user_id, + WorkspaceMembers.role_id == current_role.role_id, + WorkspaceMembers.member_status == "active", + WorkspaceMembers.is_deleted == 0, + ) + ) + ).all() + orphaned: list[str] = [] + for (ws_id,) in admin_memberships: + remaining_ws = await _count_active_admins( + session, ws_id, exclude_user_id=user_id, + ) + if remaining_ws == 0: + orphaned.append(ws_id) + if orphaned: + codes = ( + await session.execute( + select(Workspaces.workspace_code).where( + Workspaces.workspace_id.in_(orphaned) + ) + ) + ).all() + names = sorted(row[0] for row in codes) + raise HTTPException( + status.HTTP_409_CONFLICT, + f"以下 workspace 将失去唯一 active admin: {names};" + "请先在这些 workspace 中指定其他 admin,再降级该用户", + ) + if is_self and new_role is not None and new_role.role_code != "admin": raise HTTPException(status.HTTP_409_CONFLICT, "不能降级自身管理员角色") @@ -480,6 +549,21 @@ async def update_platform_employee( user.status = payload.status if new_role is not None: user.platform_role_id = new_role.role_id + # Workspace role is always inherited from the platform role + # (§7.5/§7.6 cannot change it). Keep workspace_members.role_id + # in sync so downstream reads — `/me` workspaces[].role_code, + # load_active_membership, §7.7 DELETE last-admin guard — + # see the up-to-date role. Without this sync, a user demoted + # from admin → developer would still appear as admin in every + # workspace they belong to until they leave and re-join. + await session.execute( + update(WorkspaceMembers) + .where( + WorkspaceMembers.user_id == user.user_id, + WorkspaceMembers.is_deleted == 0, + ) + .values(role_id=new_role.role_id) + ) await session.flush() await session.refresh(user) @@ -760,12 +844,15 @@ async def add_member( "请先 PATCH /api/v1/platform/employees/{user_id} 设置 role_code", ) role = await session.scalar( - select(Roles).where(Roles.role_id == user.platform_role_id) + select(Roles).where( + Roles.role_id == user.platform_role_id, + Roles.is_deleted == 0, + ) ) if role is None: raise HTTPException( status.HTTP_422_UNPROCESSABLE_ENTITY, - "用户的平台角色行不存在", + "用户的平台角色行不存在或已被删除", ) duplicate = await session.scalar( select(WorkspaceMembers.user_id).where( @@ -777,7 +864,8 @@ async def add_member( if duplicate is not None: raise HTTPException( status.HTTP_409_CONFLICT, - "用户已是该 workspace 成员", + "用户已是该 workspace 成员;workspace 角色继承自平台角色," + "要变更请 PATCH /api/v1/platform/employees/{user_id} 修改 role_code", ) membership = WorkspaceMembers( workspace_id=workspace_id, @@ -1006,29 +1094,29 @@ async def patch_role_permissions( ) -> dict[str, Any]: """Replace a platform role's permission set wholesale. - Guard order is load-bearing (mirrors ``update_platform_employee``): + Guard order: 1. Load the role. Reject 404 if it is missing or not platform-scoped. - 2. Self-protection: when the caller is modifying the role they - themselves hold via ``platform_role_id``, evaluate the - post-patch permission set against the last-admin rule below. - Putting this check before the last-admin count keeps the - test surface stable (see CLAUDE.md line 78). - 3. Last-admin guard (admin role only): the patched - permission_codes MUST still include both ``system.view`` and - ``system.manage``. Otherwise every active admin loses the - entry point to this very endpoint and the platform locks - itself out. Reject with 409. + 2. Admin role: the patched ``permission_codes`` MUST still include + both ``system.view`` and ``system.manage``. Otherwise every + active admin loses the menu entry to this very endpoint and + the platform locks itself out. Reject with 409. (No last-admin + count is needed here — menu permissions never gate API access; + ``system_admin_context`` keys off ``role_code == 'admin'``.) + 3. Non-admin role: ``system.*`` codes are rejected with 422 — + they would render a system-admin menu entry whose API calls + all 403. 4. Validate every code resolves to a non-deleted ``Permissions`` row; unknown codes → 422. - 5. Write: soft-delete existing ``RolePermissions`` rows for this - role, then bulk_insert the new set. Repeat-with-same-payload - is idempotent (inserts after the soft-delete). + 5. Write: diff-based. Only soft-delete codes leaving the set, + only insert codes entering it. The ``(role_id, permission_id)`` + PRIMARY KEY still occupies soft-deleted rows, so a blanket + "delete-all then insert-all" would IntegrityError. + Repeat-with-same-payload is a no-op. """ role = await _load_platform_role_by_code(session, role_code) - is_self = context.user.platform_role_id == role.role_id new_codes = list(dict.fromkeys(payload.permission_codes)) if role.role_code == "admin": @@ -1040,21 +1128,21 @@ async def patch_role_permissions( status.HTTP_409_CONFLICT, "admin 角色必须保留 system.view 与 system.manage 权限", ) - # Last-admin safety: if removing any current system.* permission - # would leave zero active admins able to reach this endpoint, - # reject. In practice the "keeps_admin_entry" check above - # already covers this for the admin role; the redundant guard - # is kept for clarity and as a safety net if a future schema - # change adds new admin-only permissions. - if is_self: - remaining = await _count_active_system_admins( - session, exclude_user_id=context.user.user_id, + else: + # Menu permissions are a frontend-display signal only — backend + # authorization keeps keying off role_code == "admin". Letting a + # non-admin role hold system.* permissions would render the + # system-admin entry in the developer's UI while every + # /api/v1/platform/* call still returns 403. Reject with 422 so + # the failure is unambiguous about *what* the input violated. + leaked_system = [ + code for code in new_codes if code.startswith("system.") + ] + if leaked_system: + raise HTTPException( + status.HTTP_422_UNPROCESSABLE_ENTITY, + f"非 admin 角色不能拥有 system.* 权限: {leaked_system}", ) - if remaining == 0: - raise HTTPException( - status.HTTP_409_CONFLICT, - "platform 必须保留至少一个 active 系统管理员", - ) # 4. Validate every requested permission_code exists and is live. if new_codes: diff --git a/migrations/versions/f6a7b8c9d0e1_seed_role_permissions_and_fix_scope.py b/migrations/versions/f6a7b8c9d0e1_seed_role_permissions_and_fix_scope.py index 36d8118..bffed63 100644 --- a/migrations/versions/f6a7b8c9d0e1_seed_role_permissions_and_fix_scope.py +++ b/migrations/versions/f6a7b8c9d0e1_seed_role_permissions_and_fix_scope.py @@ -13,12 +13,30 @@ and common/auth/membership.py::resolve_is_system_admin). This migration: 3. INSERT role_permissions join rows: admin gets all 12, developer gets the 6 `*.own` / personal-resource codes. -permission_id values are deterministic (sha256 of code) so a -downgrade → upgrade cycle is idempotent on the uk_permissions_code -unique index. Downgrade soft-deletes (is_deleted=1, deleted_at=now) -the rows this migration inserted; it does NOT revert role_scope -because that fix is not safely reversible once app code has touched -the rows. +Caveats (read before re-running): + +* ``permission_id`` is derived from a sha256 of the code with the salt + prefix ``model-platform-permission-v1:``. The legacy + ``migrations/data/migrate_system_json.py`` script uses a different + salt (``model-platform-v1:permission:``), so the same + ``permission_code`` maps to a DIFFERENT ``permission_id`` between the + two paths. The legacy script's ``existing.get(permission_code)`` check + keeps the row count correct (it reuses the live row by code), so this + is not a crash; the IDs only matter if a downstream system ever + cross-references by deterministic ID, which nothing does today. + +* ``downgrade()`` is a SOFT delete (``is_deleted=1``). Running + ``alembic downgrade`` followed by ``alembic upgrade`` will collide on + the ``permission_id`` PRIMARY KEY — downgrade is a one-way trip on + any environment that has run this migration. The role_scope fix is + not reverted on downgrade (app code already keys off `platform`). + +* If the legacy one-off ``migrations/data/migrate_system_json.py`` is + ever run AFTER this migration on the same database, its + ``existing.get(permission_code)`` check will keep counts correct but + reuses our rows; running it BEFORE this migration would cause + ``uk_permissions_code`` collisions on upgrade. Run this migration + first on a fresh database. Revision ID: f6a7b8c9d0e1 Revises: e5f6a7b8c9d0 (ensure_demo_login)