From 077d9708f5933d92a688b65ab8b1f0c5192ef1f3 Mon Sep 17 00:00:00 2001 From: ZingerLittleBee <6970999@gmail.com> Date: Tue, 7 Jul 2026 22:16:16 +0800 Subject: [PATCH 1/7] fix(server): make last-admin guard transactional in user update and delete MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The count(role='admin') check and the subsequent mutation ran outside a shared transaction, so two concurrent demotions/deletions could both pass the guard and leave the system with zero admins — a state init_admin cannot recover because it only re-bootstraps an empty users table. Wrapping each path in one transaction lets SQLite serialize the writers so the second request re-reads the count and is correctly rejected; the multi-table cleanup in delete_user becomes atomic as a bonus. --- crates/server/src/service/user.rs | 50 ++++++++++++++++++++----------- 1 file changed, 32 insertions(+), 18 deletions(-) diff --git a/crates/server/src/service/user.rs b/crates/server/src/service/user.rs index 5bd62ace..16e85e2d 100644 --- a/crates/server/src/service/user.rs +++ b/crates/server/src/service/user.rs @@ -92,13 +92,21 @@ impl UserService { } /// Update a user's role and optionally reset their password. + /// + /// Runs entirely in one transaction so the last-admin guard's count and + /// the mutation cannot interleave with a concurrent demotion/deletion — + /// two racing requests could otherwise both pass the count check and + /// leave the system with zero admins, a state `init_admin` cannot recover + /// (it only re-bootstraps an empty users table). pub async fn update_user( db: &DatabaseConnection, id: &str, input: UpdateUserInput, ) -> Result { + let txn = db.begin().await?; + let user = user::Entity::find_by_id(id) - .one(db) + .one(&txn) .await? .ok_or_else(|| AppError::NotFound("User not found".to_string()))?; @@ -110,7 +118,7 @@ impl UserService { if user.role == "admin" && role != "admin" { let admin_count = user::Entity::find() .filter(user::Column::Role.eq("admin")) - .count(db) + .count(&txn) .await?; if admin_count <= 1 { return Err(AppError::BadRequest( @@ -133,35 +141,39 @@ impl UserService { } active.updated_at = Set(now); + let updated = active.update(&txn).await?; + // If an admin reset this user's password, revoke all their existing // sessions so a previously issued (possibly stolen) session cannot // outlive the reset. This includes the mobile auth path, whose refresh // secret lives in `mobile_session` (a separate table); an admin reset - // unconditionally drops all of the target user's mobile sessions. The - // update + revocation run in one transaction so the reset can't commit + // unconditionally drops all of the target user's mobile sessions. + // Sharing the surrounding transaction means the reset can't commit // while sessions stay live. - let updated = if password_reset { - let txn = db.begin().await?; - let updated = active.update(&txn).await?; + if password_reset { session::Entity::delete_many() .filter(session::Column::UserId.eq(id)) .exec(&txn) .await?; AuthService::revoke_user_mobile_sessions(&txn, id, None).await?; - txn.commit().await?; - updated - } else { - active.update(db).await? - }; + } + + txn.commit().await?; Ok(updated) } /// Delete a user along with their sessions and API keys. /// Refuses to delete the last admin. + /// + /// Runs entirely in one transaction: the last-admin guard's count cannot + /// interleave with a concurrent demotion/deletion (see `update_user`), + /// and the multi-table cleanup is atomic. pub async fn delete_user(db: &DatabaseConnection, id: &str) -> Result<(), AppError> { + let txn = db.begin().await?; + let user = user::Entity::find_by_id(id) - .one(db) + .one(&txn) .await? .ok_or_else(|| AppError::NotFound("User not found".to_string()))?; @@ -169,7 +181,7 @@ impl UserService { if user.role == "admin" { let admin_count = user::Entity::find() .filter(user::Column::Role.eq("admin")) - .count(db) + .count(&txn) .await?; if admin_count <= 1 { return Err(AppError::BadRequest( @@ -181,23 +193,25 @@ impl UserService { // Clean up sessions session::Entity::delete_many() .filter(session::Column::UserId.eq(id)) - .exec(db) + .exec(&txn) .await?; // Clean up API keys api_key::Entity::delete_many() .filter(api_key::Column::UserId.eq(id)) - .exec(db) + .exec(&txn) .await?; // Clean up OAuth accounts oauth_account::Entity::delete_many() .filter(oauth_account::Column::UserId.eq(id)) - .exec(db) + .exec(&txn) .await?; // Delete the user - user::Entity::delete_by_id(id).exec(db).await?; + user::Entity::delete_by_id(id).exec(&txn).await?; + + txn.commit().await?; Ok(()) } From fc4a264fb7af5a6641758bf42c91797e369b6939 Mon Sep 17 00:00:00 2001 From: ZingerLittleBee <6970999@gmail.com> Date: Tue, 7 Jul 2026 22:16:33 +0800 Subject: [PATCH 2/7] feat(web): add role permission hints to create-user dialog Spell out in the dialog that a member sees all servers' monitoring data (security events and public IPs included) with no write access, and point at status pages for exposing selected servers to outsiders, so member accounts aren't mistaken for scoped/customer-facing access. --- apps/web/src/locales/en/settings.json | 2 ++ apps/web/src/locales/zh/settings.json | 2 ++ apps/web/src/routes/_authed/settings/users.tsx | 3 +++ 3 files changed, 7 insertions(+) diff --git a/apps/web/src/locales/en/settings.json b/apps/web/src/locales/en/settings.json index 7bdbdb44..f7997fc8 100644 --- a/apps/web/src/locales/en/settings.json +++ b/apps/web/src/locales/en/settings.json @@ -59,6 +59,8 @@ "users.password_hint": "Password (min 6 chars)", "users.role_member": "Member", "users.role_admin": "Admin", + "users.role_hint_member": "Members can view monitoring data for all servers — including security events and public IPs — but have no write or control access. To show only selected servers to outsiders, use a status page instead.", + "users.role_hint_admin": "Admins have full control over the entire system, including terminals, file access, and user management.", "users.no_users": "No users found", "users.two_factor": "2FA", "users.role_label": "Role:", diff --git a/apps/web/src/locales/zh/settings.json b/apps/web/src/locales/zh/settings.json index 23bff1da..f4847889 100644 --- a/apps/web/src/locales/zh/settings.json +++ b/apps/web/src/locales/zh/settings.json @@ -59,6 +59,8 @@ "users.password_hint": "密码（最少 6 位）", "users.role_member": "普通成员", "users.role_admin": "管理员", + "users.role_hint_member": "普通成员可查看所有服务器的全部监控数据（含安全事件与公网 IP），但没有任何写入或操作权限。如需仅向外部人员展示部分服务器，请使用状态页。", + "users.role_hint_admin": "管理员拥有整个系统的完全控制权，包括终端、文件访问和用户管理。", "users.no_users": "暂无用户", "users.two_factor": "2FA", "users.role_label": "角色：", diff --git a/apps/web/src/routes/_authed/settings/users.tsx b/apps/web/src/routes/_authed/settings/users.tsx index 4de10f42..cc2a5a68 100644 --- a/apps/web/src/routes/_authed/settings/users.tsx +++ b/apps/web/src/routes/_authed/settings/users.tsx @@ -202,6 +202,9 @@ function UsersPage() { {t('users.role_admin')} +

+ {state.newRole === 'admin' ? t('users.role_hint_admin') : t('users.role_hint_member')} +

{createMutation.error &&

{createMutation.error.message}

} From 728d1dc322a8491ceda2fc4a5c07b4c8bf5a173f Mon Sep 17 00:00:00 2001 From: ZingerLittleBee <6970999@gmail.com> Date: Tue, 7 Jul 2026 22:16:33 +0800 Subject: [PATCH 3/7] docs: document member visibility boundary, oauth registration risk, and admin recovery - Admin guide: state that member visibility is fleet-wide with no per-server scoping, and direct external exposure to status pages - Admin guide: add a lost-admin recovery procedure (wipe users table, restart to re-trigger the bootstrap banner) with its side effects - Security guide + ENV.md: warn that oauth allow_registration with a public provider grants fleet-wide read access to every provider user --- ENV.md | 2 +- apps/docs/content/docs/en/admin.mdx | 16 +++++++++++++++- apps/docs/content/docs/en/security.mdx | 4 ++++ apps/docs/content/docs/zh/admin.mdx | 16 +++++++++++++++- apps/docs/content/docs/zh/security.mdx | 4 ++++ 5 files changed, 39 insertions(+), 3 deletions(-) diff --git a/ENV.md b/ENV.md index 4859f906..35595fd9 100644 --- a/ENV.md +++ b/ENV.md @@ -50,7 +50,7 @@ These variables are for local repo tooling and development workflows. They are n | Environment Variable | TOML Key | Type | Default | Description | |---------------------|----------|------|---------|-------------| | `SERVERBEE_OAUTH__BASE_URL` | `oauth.base_url` | string | `""` | Public base URL for constructing OAuth callback URLs (e.g. `https://monitor.example.com`) | -| `SERVERBEE_OAUTH__ALLOW_REGISTRATION` | `oauth.allow_registration` | bool | `false` | Auto-create user accounts on first OAuth login | +| `SERVERBEE_OAUTH__ALLOW_REGISTRATION` | `oauth.allow_registration` | bool | `false` | Auto-create member accounts on first OAuth login. ⚠️ Enable only when the OAuth provider itself is access-controlled (self-hosted OIDC / org-internal IdP): members can read **all** monitoring data (security events, public IPs), so enabling this with a public provider such as GitHub grants that access to every user of the provider | | `SERVERBEE_OAUTH__GITHUB__CLIENT_ID` | `oauth.github.client_id` | string | - | GitHub OAuth App client ID | | `SERVERBEE_OAUTH__GITHUB__CLIENT_SECRET` | `oauth.github.client_secret` | string | - | GitHub OAuth App client secret | | `SERVERBEE_OAUTH__GOOGLE__CLIENT_ID` | `oauth.google.client_id` | string | - | Google OAuth client ID | diff --git a/apps/docs/content/docs/en/admin.mdx b/apps/docs/content/docs/en/admin.mdx index 4449678a..4ee00c56 100644 --- a/apps/docs/content/docs/en/admin.mdx +++ b/apps/docs/content/docs/en/admin.mdx @@ -13,7 +13,11 @@ ServerBee supports multiple users across two roles: | Role | Permissions | |------|-------------| | **Admin** | Full management: users, server config, alert rules, notifications, audit logs, etc. | -| **Member** | Read-only: view Dashboard, server details, Ping results | +| **Member** | Read-only access to **all** servers' monitoring data — dashboards, server details, Ping results, security events, public IPs — plus self-service settings (own password, 2FA, API key list, mobile devices) | + + +Member visibility is fleet-wide; there is no per-server scoping. A member account is for trusted collaborators only. To show selected servers to outsiders (e.g., customers), use a [status page](/en/docs/status-page) instead. + ### Managing Users @@ -23,6 +27,16 @@ Go to Settings → Users: - **Edit role**: Change a user's role (Admin/Member) - **Delete user**: Remove a user account (cannot delete the last Admin) +### Recovering a Lost Admin Account + +There is no password-recovery flow (self-hosted deployments have no mail channel), and passwords are argon2 hashes that cannot be hand-edited into the database. If the only admin credential (password or 2FA device) is lost: + +1. Stop the server. +2. Run `sqlite3 /path/to/serverbee.db "DELETE FROM users;"`. +3. Start the server — with an empty users table it re-creates the `admin` user and prints a fresh one-time password banner in the startup log. + +This removes **all** user accounts, and their API keys stop working. Monitoring data, servers, and configuration are untouched. + ### API Endpoints | Endpoint | Method | Description | diff --git a/apps/docs/content/docs/en/security.mdx b/apps/docs/content/docs/en/security.mdx index 3df1ca90..d83c0996 100644 --- a/apps/docs/content/docs/en/security.mdx +++ b/apps/docs/content/docs/en/security.mdx @@ -68,6 +68,10 @@ client_secret = "your-github-client-secret" - Click **Unlink** to disconnect an OAuth account - If `allow_registration = false` (default), first-time OAuth logins do not create new users — an admin must create the user first + +Enable `allow_registration = true` only when the OAuth provider itself is access-controlled (self-hosted OIDC or an org-internal IdP). Auto-created accounts get the Member role, which can read **all** monitoring data — including security events and public IPs. With a public provider such as GitHub, enabling it grants that access to every user of the provider. + + ### Login Flow 1. Click an OAuth provider button on the login page (e.g., "Login with GitHub") diff --git a/apps/docs/content/docs/zh/admin.mdx b/apps/docs/content/docs/zh/admin.mdx index d7952528..f0b162e1 100644 --- a/apps/docs/content/docs/zh/admin.mdx +++ b/apps/docs/content/docs/zh/admin.mdx @@ -13,7 +13,11 @@ ServerBee 支持多用户，分为两种角色： | 角色 | 权限 | |------|------| | **Admin** | 完全管理权限：用户管理、服务器配置、告警规则、通知渠道、审计日志等 | -| **Member** | 只读权限：查看 Dashboard、服务器详情、Ping 结果 | +| **Member** | 对**所有**服务器监控数据的只读权限——Dashboard、服务器详情、Ping 结果、安全事件、公网 IP，以及自助设置（自己的密码、2FA、API Key 列表、移动设备） | + + +Member 的可见范围是全部服务器，不支持按服务器隔离。Member 账号仅适合完全信任的协作者。如需向外部人员（如客户）展示部分服务器，请改用[状态页](/zh/docs/status-page)。 + ### 管理用户 @@ -23,6 +27,16 @@ ServerBee 支持多用户，分为两种角色： - **编辑角色**：修改用户的角色（Admin/Member） - **删除用户**：删除用户账号（禁止删除最后一个 Admin） +### 找回丢失的管理员账号 + +系统没有"忘记密码"流程（自托管部署没有邮件通道），密码以 argon2 哈希存储，无法手工改库重置。如果唯一的管理员凭证（密码或 2FA 设备）丢失： + +1. 停止服务端。 +2. 执行 `sqlite3 /path/to/serverbee.db "DELETE FROM users;"`。 +3. 重新启动服务端——users 表为空时会重新创建 `admin` 用户，并在启动日志中打印一次性的新随机密码横幅。 + +此操作会删除**所有**用户账号，其 API Key 随之失效；监控数据、服务器与配置不受影响。 + ### API 端点 | 端点 | 方法 | 说明 | diff --git a/apps/docs/content/docs/zh/security.mdx b/apps/docs/content/docs/zh/security.mdx index 54ae1045..a745977b 100644 --- a/apps/docs/content/docs/zh/security.mdx +++ b/apps/docs/content/docs/zh/security.mdx @@ -68,6 +68,10 @@ client_secret = "your-github-client-secret" - 点击 **Unlink** 可以解除 OAuth 账号关联 - 如果 `allow_registration = false`（默认），OAuth 首次登录不会自动创建新用户，需要管理员先创建用户再关联 + +仅当 OAuth 提供商本身受控（自建 OIDC 或组织内部 IdP）时才应开启 `allow_registration = true`。自动创建的账号为 Member 角色，可读取**全部**监控数据（含安全事件与公网 IP）。若配合 GitHub 等公共提供商开启，等于向该提供商的所有用户开放这些数据。 + + ### 登录流程 1. 在登录页面点击 OAuth 提供商按钮（如 "Login with GitHub"） From 0f32e3aaa9faec5fe0f94104e8ab522b25b1e0fd Mon Sep 17 00:00:00 2001 From: ZingerLittleBee <6970999@gmail.com> Date: Tue, 7 Jul 2026 22:16:33 +0800 Subject: [PATCH 4/7] docs(spec): record user system design review decisions Six decisions from a design grilling of the two-tier RBAC user system: member positioning, no forced password change, WS revalidation declined, oauth registration handled via docs, transactional last-admin guard (fixed), and documented admin lockout recovery. --- .../2026-07-07-user-system-design-review.md | 105 ++++++++++++++++++ 1 file changed, 105 insertions(+) create mode 100644 docs/superpowers/specs/2026-07-07-user-system-design-review.md diff --git a/docs/superpowers/specs/2026-07-07-user-system-design-review.md b/docs/superpowers/specs/2026-07-07-user-system-design-review.md new file mode 100644 index 00000000..c0856e08 --- /dev/null +++ b/docs/superpowers/specs/2026-07-07-user-system-design-review.md @@ -0,0 +1,105 @@ +# User System Design Review + +**Date:** 2026-07-07 +**Status:** Concluded — 6 decisions settled; follow-ups implemented alongside this document +**Scope:** The two-tier RBAC user system: `users` table, `admin`/`member` roles, `/settings/users` management UI, `require_admin` route gating, API keys, sessions. + +## Context + +ServerBee ships a deliberately simple user model: + +- `users.role` is a plain string, validated to `"admin" | "member"` (`crates/server/src/service/user.rs`). +- A default `admin` user is bootstrapped only when the users table is empty, with a random password and `must_change_password = true` (`AuthService::init_admin`). +- Authorization is enforced at the router layer: public routes → authenticated read routers → a write-router block wrapped in `require_admin` (`crates/server/src/router/api/mod.rs`). Terminal and Docker-log WebSockets check `role == "admin"` at upgrade time. +- The web UI fail-closes: every `/settings` route is admin-only except an explicit member allowlist (`MEMBER_SETTINGS_ROUTES` in `apps/web/src/routes/_authed.tsx`). + +This document records the design decisions confirmed during the review, one per section. + +## Decision 1 — Member is a trusted, fleet-wide, read-only observer + +**Question:** Should `member` get per-server / per-group visibility scoping, or stay global read-only? + +**Decision:** Keep the global two-tier model. `member` is positioned as a **trusted collaborator with full fleet visibility and zero control**. No per-server ACL will be built. + +**Rationale:** + +- ServerBee already has a three-tier disclosure spectrum; each audience maps to an existing tool: + + | Audience | Tool | Visibility | + |---|---|---| + | Strangers / customers | Status pages | Hand-picked servers, masked/aggregated status | + | Trusted collaborators | `member` account | All servers, all monitoring detail, no control | + | Operators | `admin` account | Everything | + +- "Show a customer their one server" is served by creating a status page for that server (already supports per-server selection and IP masking) — not by scoping member accounts. +- Per-server ACL would require filtering 30+ read endpoints, the browser WebSocket FullSync/Update fan-out, and every cross-server aggregate (insights, alert event streams). Any missed endpoint is a privilege leak. That complexity tax is not worth paying in a single-tenant, self-hosted product; true multi-tenant isolation should be solved by deploying separate instances. + +**Follow-ups (cheap, disclosure-only):** + +- [ ] Create-user dialog: the `member` role option must state that members can see **all** servers' monitoring data (including security events and public IPs) with no write access. +- [ ] User-management docs page: state the same boundary and point to status pages for external/partial exposure. + +## Decision 2 — No forced password change for admin-created users + +**Question:** `AuthService::create_user` hardcodes `must_change_password = false`, and the admin password-reset path (`PUT /api/users/{id}` with `password`) does not set it either — so the admin permanently knows the initial/reset password. Should both paths set `must_change_password = true` (reusing the existing onboarding machinery built for the bootstrap admin)? + +**Decision:** Keep as-is. No forced first-login password change for admin-created or admin-reset accounts. + +**Rationale (owner's call):** The admin is fully trusted and already holds maximum privilege over the whole system; knowing a member's password grants nothing the admin cannot already do. Forcing a change would add onboarding friction without a meaningful security gain in this trust model. The bootstrap admin keeps its forced change because its random password is printed to the startup log (a broader exposure channel). + +**Clarification recorded during this decision:** A bootstrap-created admin and a UI-created admin are identical after creation. There is no "super admin" concept — `role` is a plain string and every check is `role == "admin"`. Differences exist only at birth: the bootstrap admin has a fixed `admin` username, a random password shown once in a startup banner, `must_change_password = true`, and **no audit entry** (UI creation writes `user.create` to the audit log). The bootstrap admin can be deleted or demoted like any other admin as long as the count-based last-admin guard is satisfied. There is no config/env path that seeds the bootstrap admin password in production; `admin/admin123` exists only in the dev demo seed. + +## Decision 3 — Live WebSocket channels are not re-validated after role changes (known boundary) + +**Question:** Terminal (`ws/terminal.rs`) and Docker-log (`ws/docker_logs.rs`) WebSockets check `role == "admin"` only at upgrade time. A demoted or deleted admin's already-open PTY/log stream stays alive until the client disconnects, even though REST access is revoked immediately (role is re-read from the DB on every request, and deletion drops sessions). Should the WS loops periodically re-validate the session and role (e.g., every 60s) and close on failure? + +**Decision:** No — rejected as overdesign for this product's trust model. Recorded as a known boundary instead. + +**Rationale (owner's call):** Admin demotion/removal is a rare, deliberate act performed by another trusted admin in a single-operator/small-team deployment. The residual window (an already-open terminal surviving until disconnect) does not justify adding re-validation machinery to the WS loops. Operators who need immediate revocation can restart the server process, which drops all WS connections. + +## Decision 4 — OAuth auto-registration risk is handled by documentation, not an allowlist + +**Question:** With `SERVERBEE_OAUTH__ALLOW_REGISTRATION=true` (default `false`), any identity that can authenticate against the configured OAuth provider is auto-provisioned as a `member` on first login (`OAuthService::find_or_create_user`), with no domain/org/username allowlist and no approval step. Combined with Decision 1 (member = fleet-wide read visibility), enabling this against a public provider such as GitHub effectively publishes all read-only monitoring data — public IPs, process lists, security events — to every user of that provider. Should an allowlist be added? + +**Decision:** No allowlist. Documentation-only mitigation. + +**Rationale:** The flag is fail-closed by default, and the legitimate use case (a private/self-hosted IdP where the provider itself is the allowlist) doesn't need extra config. The danger is purely one of operator expectation, so it is fixed at the disclosure layer. + +**Follow-ups:** + +- [ ] ENV.md `SERVERBEE_OAUTH__ALLOW_REGISTRATION` row and the OAuth docs page must warn: enable only when the OAuth provider itself is access-controlled (self-hosted OIDC / org-internal IdP). Enabling it with a public provider (e.g., GitHub) grants every user of that provider full read access to all monitoring data, including security events and public IPs. + +## Verified clean — no credential leakage to members + +Checked during the review, no action needed: `ServerResponse` deliberately excludes `token_hash`/`token_prefix` (exposes only a `has_token` bool), and `agent::read_router` exposes only `/agent/latest-version`. Members have no read path to agent tokens or enrollment secrets, so a member account cannot escalate to agent impersonation. + +## Decision 5 — Fix the last-admin guard race (wrap check + mutation in a transaction) + +**Question:** The last-admin guard in `UserService::update_user` (demote path) and `UserService::delete_user` is check-then-act: the `count(role = 'admin')` query and the subsequent write run outside a shared transaction. Two concurrent requests (e.g., two admins demoting each other) can both observe `admin_count = 2`, both pass, and leave the system with **zero admins**. Because `init_admin` re-bootstraps only when the users table is empty, an all-admins-demoted state is unrecoverable in-product — every `require_admin` route (including user management itself) returns 403 forever; the only way out is manual SQLite surgery. + +**Decision:** Fix. Wrap the guard count and the mutation in a single transaction in both paths (`update_user` role-demotion branch, `delete_user`). SQLite serializes write transactions, so the second concurrent request re-reads `admin_count = 1` inside its transaction and is correctly rejected. No new concepts or config; `delete_user`'s multi-table cleanup should have been transactional anyway. + +**Status:** To implement (part of this review's follow-up batch). + +## Decision 6 — Admin lockout recovery is documented, not built + +**Question:** There is no forgot-password flow (reasonable — self-hosted, no mail channel), and passwords are argon2 hashes that cannot be hand-crafted via the sqlite CLI. If the only admin loses their password or 2FA device, the only real recovery is: stop the server → `sqlite3 serverbee.db "DELETE FROM users;"` → restart, which triggers `init_admin` re-bootstrap (new random password in the startup banner). This works but is written down nowhere. Should ServerBee ship a `reset-admin-password` CLI subcommand instead? + +**Decision:** No CLI. Document the wipe-and-rebootstrap recovery procedure in the docs site. + +**Rationale:** Anyone who can reach the DB file already has host-level access, so a CLI adds no new capability — only a new maintained entry point. The recovery recipe (with its side effects: all user accounts and API-key ownership are reset; monitoring data untouched) belongs in the troubleshooting docs. + +**Follow-ups:** + +- [ ] Add a "Lost admin access" recovery section to the docs (troubleshooting/FAQ area, CN+EN). + +## Summary of outcomes + +| # | Topic | Outcome | +|---|---|---| +| 1 | Member scoping | Keep global read-only member; external exposure → status pages; add UI/docs disclosure | +| 2 | Forced password change for created users | Keep as-is (admin is fully trusted) | +| 3 | Live WS revalidation after role change | Known boundary, no code change | +| 4 | OAuth auto-registration blast radius | Docs warning only, no allowlist | +| 5 | Last-admin guard race | **Fix**: wrap count+mutation in one transaction (update_user demote path, delete_user) | +| 6 | Admin lockout recovery | Document wipe-and-rebootstrap procedure, no CLI | From 4beb5e0226c9632c425b2bc94845ac3ad7202f90 Mon Sep 17 00:00:00 2001 From: ZingerLittleBee <6970999@gmail.com> Date: Tue, 7 Jul 2026 22:16:33 +0800 Subject: [PATCH 5/7] chore: sync bun.lock with web package version --- bun.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bun.lock b/bun.lock index 3ad36ea3..0b10f881 100644 --- a/bun.lock +++ b/bun.lock @@ -48,7 +48,7 @@ }, "apps/web": { "name": "@serverbee/web", - "version": "1.0.0-alpha.10", + "version": "1.0.0-alpha.11", "dependencies": { "@base-ui/react": "^1.2.0", "@fontsource-variable/inter": "^5.2.8", From 26f52476504f9e20514d051fcdf44db7abfdd4f4 Mon Sep 17 00:00:00 2001 From: ZingerLittleBee <6970999@gmail.com> Date: Tue, 7 Jul 2026 22:32:19 +0800 Subject: [PATCH 6/7] docs(spec): mark user system review follow-ups as implemented --- .../specs/2026-07-07-user-system-design-review.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/superpowers/specs/2026-07-07-user-system-design-review.md b/docs/superpowers/specs/2026-07-07-user-system-design-review.md index c0856e08..bf04e293 100644 --- a/docs/superpowers/specs/2026-07-07-user-system-design-review.md +++ b/docs/superpowers/specs/2026-07-07-user-system-design-review.md @@ -36,8 +36,8 @@ This document records the design decisions confirmed during the review, one per **Follow-ups (cheap, disclosure-only):** -- [ ] Create-user dialog: the `member` role option must state that members can see **all** servers' monitoring data (including security events and public IPs) with no write access. -- [ ] User-management docs page: state the same boundary and point to status pages for external/partial exposure. +- [x] Create-user dialog: the `member` role option must state that members can see **all** servers' monitoring data (including security events and public IPs) with no write access. +- [x] User-management docs page: state the same boundary and point to status pages for external/partial exposure. ## Decision 2 — No forced password change for admin-created users @@ -67,7 +67,7 @@ This document records the design decisions confirmed during the review, one per **Follow-ups:** -- [ ] ENV.md `SERVERBEE_OAUTH__ALLOW_REGISTRATION` row and the OAuth docs page must warn: enable only when the OAuth provider itself is access-controlled (self-hosted OIDC / org-internal IdP). Enabling it with a public provider (e.g., GitHub) grants every user of that provider full read access to all monitoring data, including security events and public IPs. +- [x] ENV.md `SERVERBEE_OAUTH__ALLOW_REGISTRATION` row and the OAuth docs page must warn: enable only when the OAuth provider itself is access-controlled (self-hosted OIDC / org-internal IdP). Enabling it with a public provider (e.g., GitHub) grants every user of that provider full read access to all monitoring data, including security events and public IPs. ## Verified clean — no credential leakage to members @@ -79,7 +79,7 @@ Checked during the review, no action needed: `ServerResponse` deliberately exclu **Decision:** Fix. Wrap the guard count and the mutation in a single transaction in both paths (`update_user` role-demotion branch, `delete_user`). SQLite serializes write transactions, so the second concurrent request re-reads `admin_count = 1` inside its transaction and is correctly rejected. No new concepts or config; `delete_user`'s multi-table cleanup should have been transactional anyway. -**Status:** To implement (part of this review's follow-up batch). +**Status:** Implemented — `update_user` and `delete_user` each run guard + mutation in one transaction. ## Decision 6 — Admin lockout recovery is documented, not built @@ -91,7 +91,7 @@ Checked during the review, no action needed: `ServerResponse` deliberately exclu **Follow-ups:** -- [ ] Add a "Lost admin access" recovery section to the docs (troubleshooting/FAQ area, CN+EN). +- [x] Add a "Lost admin access" recovery section to the docs (Admin Guide, CN+EN). ## Summary of outcomes From 4579f200a5e77b034586d4c3267addcb6c467ada Mon Sep 17 00:00:00 2001 From: ZingerLittleBee <6970999@gmail.com> Date: Tue, 7 Jul 2026 22:55:08 +0800 Subject: [PATCH 7/7] feat(ios): add role permission hints to create-user sheet Mirror the web create-user dialog: the Role section footer now states that a member sees all servers' monitoring data (security events and public IPs included) with no write access and points at status pages for exposing selected servers to outsiders, while the admin option spells out full-control scope. Three new String Catalog keys with zh-Hans translations. Also adds DEBUG verification hooks following the existing pattern: SB_UITEST_ADMIN=users pushes the Users screen and SB_UITEST_PRESENT=users-create auto-opens the create sheet, since the '+' navbar button is unreachable for the headless cliclick harness. --- apps/ios/ServerBee/Localizable.xcstrings | 51 +++++++++++++++++++ .../ios/ServerBee/Views/Admin/UsersView.swift | 22 +++++++- .../Views/Settings/SettingsView.swift | 4 +- 3 files changed, 74 insertions(+), 3 deletions(-) diff --git a/apps/ios/ServerBee/Localizable.xcstrings b/apps/ios/ServerBee/Localizable.xcstrings index 7248bac5..45b3e45a 100644 --- a/apps/ios/ServerBee/Localizable.xcstrings +++ b/apps/ios/ServerBee/Localizable.xcstrings @@ -943,6 +943,23 @@ } } }, + "Admins have full control over the entire system, including terminals, file access, and user management.": { + "extractionState": "manual", + "localizations": { + "en": { + "stringUnit": { + "state": "translated", + "value": "Admins have full control over the entire system, including terminals, file access, and user management." + } + }, + "zh-Hans": { + "stringUnit": { + "state": "translated", + "value": "管理员拥有整个系统的完全控制权，包括终端、文件访问和用户管理。" + } + } + } + }, "Advanced": { "extractionState": "manual", "localizations": { @@ -6993,6 +7010,23 @@ } } }, + "Members can view all servers' monitoring data — including security events and public IPs — with no write access.": { + "extractionState": "manual", + "localizations": { + "en": { + "stringUnit": { + "state": "translated", + "value": "Members can view all servers' monitoring data — including security events and public IPs — with no write access." + } + }, + "zh-Hans": { + "stringUnit": { + "state": "translated", + "value": "成员可查看所有服务器的全部监控数据（含安全事件与公网 IP），但没有任何写入或操作权限。" + } + } + } + }, "Memory": { "extractionState": "manual", "localizations": { @@ -12450,6 +12484,23 @@ } } }, + "To show only selected servers to outsiders, use a status page instead.": { + "extractionState": "manual", + "localizations": { + "en": { + "stringUnit": { + "state": "translated", + "value": "To show only selected servers to outsiders, use a status page instead." + } + }, + "zh-Hans": { + "stringUnit": { + "state": "translated", + "value": "如需仅向外部人员展示部分服务器，请使用状态页。" + } + } + } + }, "Too many attempts. Please try again later.": { "extractionState": "manual", "localizations": { diff --git a/apps/ios/ServerBee/Views/Admin/UsersView.swift b/apps/ios/ServerBee/Views/Admin/UsersView.swift index ef259974..bf5a0e21 100644 --- a/apps/ios/ServerBee/Views/Admin/UsersView.swift +++ b/apps/ios/ServerBee/Views/Admin/UsersView.swift @@ -32,7 +32,12 @@ struct UsersView: View { Button { showCreate = true } label: { Image(systemName: "person.badge.plus") } } } - .task { await viewModel.load(apiClient: apiClient) } + .task { + #if DEBUG + if UITestSupport.autoPresent == "users-create" { showCreate = true } + #endif + await viewModel.load(apiClient: apiClient) + } .refreshable { await viewModel.load(apiClient: apiClient) } .sheet(isPresented: $showCreate) { CreateUserSheet(viewModel: viewModel) } .sheet(item: $editing) { user in @@ -76,6 +81,15 @@ private struct CreateUserSheet: View { !username.trimmingCharacters(in: .whitespaces).isEmpty && password.count >= 8 && !working } + private var roleHint: String { + if role == "admin" { + return String(localized: "Admins have full control over the entire system, including terminals, file access, and user management.") + } + let visibility = String(localized: "Members can view all servers' monitoring data — including security events and public IPs — with no write access.") + let pointer = String(localized: "To show only selected servers to outsiders, use a status page instead.") + return "\(visibility) \(pointer)" + } + var body: some View { NavigationStack { Form { @@ -85,12 +99,16 @@ private struct CreateUserSheet: View { .autocorrectionDisabled() SecureField(String(localized: "Password (min 8)"), text: $password) } - Section(String(localized: "Role")) { + Section { Picker(String(localized: "Role"), selection: $role) { Text(String(localized: "Member")).tag("member") Text(String(localized: "Admin")).tag("admin") } .pickerStyle(.segmented) + } header: { + Text("Role") + } footer: { + Text(roleHint) } if let error { Section { Label(error, systemImage: "exclamationmark.triangle.fill").foregroundStyle(Color.serverOffline) } diff --git a/apps/ios/ServerBee/Views/Settings/SettingsView.swift b/apps/ios/ServerBee/Views/Settings/SettingsView.swift index 870855a4..e93f43f6 100644 --- a/apps/ios/ServerBee/Views/Settings/SettingsView.swift +++ b/apps/ios/ServerBee/Views/Settings/SettingsView.swift @@ -17,7 +17,7 @@ struct SettingsView: View { /// DEBUG-only value-routed admin destinations, used by the launch hook to /// push a sub-screen without the cliclick harness scrolling the list. - enum AdminRoute: Hashable { case networkProbes, ipQuality, statusPage } + enum AdminRoute: Hashable { case networkProbes, ipQuality, statusPage, users } #endif var body: some View { @@ -40,6 +40,7 @@ struct SettingsView: View { case .networkProbes: NetworkProbeConfigView(isAdmin: isAdmin) case .ipQuality: IpQualityConfigView(isAdmin: isAdmin) case .statusPage: StatusPageConfigView(isAdmin: isAdmin) + case .users: UsersView() } } .task { @@ -47,6 +48,7 @@ struct SettingsView: View { case "network-probes": debugPath.append(AdminRoute.networkProbes) case "ip-quality": debugPath.append(AdminRoute.ipQuality) case "status-page": debugPath.append(AdminRoute.statusPage) + case "users": debugPath.append(AdminRoute.users) default: break } }