fix(web): grey out last-active-admin toggles in system users (#2690)
This commit is contained in:
@@ -85,10 +85,12 @@ function StatusBadge({ status }: { status: string }) {
|
||||
|
||||
function UserRow({
|
||||
user,
|
||||
isLastActiveAdmin,
|
||||
onToggleRole,
|
||||
onToggleStatus,
|
||||
}: {
|
||||
user: AuthUser;
|
||||
isLastActiveAdmin: boolean;
|
||||
onToggleRole: (user: AuthUser) => void;
|
||||
onToggleStatus: (user: AuthUser) => void;
|
||||
}) {
|
||||
@@ -116,6 +118,7 @@ function UserRow({
|
||||
variant="ghost"
|
||||
className="cursor-pointer"
|
||||
onClick={() => onToggleRole(user)}
|
||||
disabled={isLastActiveAdmin}
|
||||
data-testid="users-table-toggle-role"
|
||||
>
|
||||
{/* Two whole-word variants get their own keys rather than
|
||||
@@ -128,6 +131,7 @@ function UserRow({
|
||||
variant="ghost"
|
||||
className="cursor-pointer text-destructive"
|
||||
onClick={() => onToggleStatus(user)}
|
||||
disabled={isLastActiveAdmin}
|
||||
data-testid="users-table-toggle-status"
|
||||
>
|
||||
{isDisabled ? t("system:usersEnable") : t("system:usersDisable")}
|
||||
@@ -203,6 +207,10 @@ function UsersTableList({
|
||||
onToggleStatus: (u: AuthUser) => void;
|
||||
}) {
|
||||
const { t } = useTranslation();
|
||||
// Mirrors the backend last-admin guard (ensureAnotherAdmin): an active
|
||||
// admin cannot be demoted or disabled when no other active admin exists,
|
||||
// so those toggles are greyed out on that row.
|
||||
const activeAdminCount = users.filter((u) => u.role === "admin" && u.status === "active").length;
|
||||
return (
|
||||
<Table data-testid="users-table">
|
||||
<TableHeader>
|
||||
@@ -215,14 +223,19 @@ function UsersTableList({
|
||||
</TableRow>
|
||||
</TableHeader>
|
||||
<TableBody>
|
||||
{users.map((user) => (
|
||||
<UserRow
|
||||
key={user.id}
|
||||
user={user}
|
||||
onToggleRole={onToggleRole}
|
||||
onToggleStatus={onToggleStatus}
|
||||
/>
|
||||
))}
|
||||
{users.map((user) => {
|
||||
const isLastActiveAdmin =
|
||||
user.role === "admin" && user.status === "active" && activeAdminCount === 1;
|
||||
return (
|
||||
<UserRow
|
||||
key={user.id}
|
||||
user={user}
|
||||
isLastActiveAdmin={isLastActiveAdmin}
|
||||
onToggleRole={onToggleRole}
|
||||
onToggleStatus={onToggleStatus}
|
||||
/>
|
||||
);
|
||||
})}
|
||||
</TableBody>
|
||||
</Table>
|
||||
);
|
||||
|
||||
@@ -58,6 +58,9 @@ export default defineConfig({
|
||||
// ApiClient). Serial within the file; afterAll restarts to baseline.
|
||||
name: "auth",
|
||||
testMatch: /auth\/.*\.spec\.ts/,
|
||||
// mobile-users-* specs under tests/auth/ run in the mobile-chrome
|
||||
// project (Pixel 5) instead of here.
|
||||
testIgnore: /mobile-.*\.spec\.ts/,
|
||||
use: { ...devices["Desktop Chrome"] },
|
||||
},
|
||||
{
|
||||
|
||||
@@ -0,0 +1,131 @@
|
||||
import { devices, expect } from "@playwright/test";
|
||||
import path from "node:path";
|
||||
import { backendFixture as test } from "../../fixtures/backend";
|
||||
import { login, setupAdmin } from "../../helpers/auth";
|
||||
import { responseUserId } from "./users-auth-helpers";
|
||||
|
||||
/**
|
||||
* Mobile parity for the self-actions guard: the Users page must grey out the
|
||||
* role/status toggles on an active-admin row when no other active admin
|
||||
* exists on the Pixel 5 path too. The component and state derivation are
|
||||
* shared with desktop; this spec proves the rendered disabled state at the
|
||||
* mobile viewport.
|
||||
*
|
||||
* Runs in the `mobile-chrome` project (routed away from the desktop `auth`
|
||||
* project via its testIgnore) and restarts the worker backend with auth on
|
||||
* and its own database, mirroring users-self-actions.spec.ts. afterAll
|
||||
* restarts to the fixture baseline.
|
||||
*/
|
||||
const ADMIN = { email: "admin@demo.dev", password: "adminpass123", displayName: "Ada Admin" };
|
||||
const MEMBER = { email: "sam@demo.dev", password: "memberpass123", displayName: "Sam Member" };
|
||||
const DISABLED_ADMIN = {
|
||||
email: "carol@demo.dev",
|
||||
password: "carolpass123",
|
||||
displayName: "Carol Admin (off)",
|
||||
};
|
||||
const SECOND_ADMIN = { email: "bob@demo.dev", password: "bobpass123", displayName: "Bob Admin" };
|
||||
|
||||
test.describe.serial("users self-actions guard (mobile)", () => {
|
||||
test.beforeAll(async ({ backend }) => {
|
||||
await backend.restart({
|
||||
KANDEV_FEATURES_AUTH: "true",
|
||||
KANDEV_DATABASE_PATH: path.join(backend.tmpDir, "kandev-mobile-self-actions.db"),
|
||||
});
|
||||
});
|
||||
|
||||
test.afterAll(async ({ backend }) => {
|
||||
await backend.restart();
|
||||
});
|
||||
|
||||
test("own-row toggles track the last-admin guard on mobile", async ({ browser, backend }) => {
|
||||
// Manual contexts do not inherit the project's device options; spread
|
||||
// them explicitly so this spec actually exercises the mobile viewport.
|
||||
const ctx = await browser.newContext({
|
||||
...devices["Pixel 5"],
|
||||
baseURL: backend.frontendUrl,
|
||||
});
|
||||
await setupAdmin(ctx, backend.baseUrl, ADMIN);
|
||||
await login(ctx, backend.baseUrl, ADMIN);
|
||||
|
||||
const meRes = await ctx.request.get(`${backend.baseUrl}/api/v1/auth/me`);
|
||||
expect(meRes.ok(), await meRes.text()).toBeTruthy();
|
||||
const adminId = responseUserId(await meRes.json());
|
||||
|
||||
const memberRes = await ctx.request.post(`${backend.baseUrl}/api/v1/users`, {
|
||||
data: {
|
||||
email: MEMBER.email,
|
||||
password: MEMBER.password,
|
||||
display_name: MEMBER.displayName,
|
||||
role: "member",
|
||||
},
|
||||
});
|
||||
expect(memberRes.status(), await memberRes.text()).toBe(201);
|
||||
const memberId = responseUserId(await memberRes.json());
|
||||
|
||||
const disabledRes = await ctx.request.post(`${backend.baseUrl}/api/v1/users`, {
|
||||
data: {
|
||||
email: DISABLED_ADMIN.email,
|
||||
password: DISABLED_ADMIN.password,
|
||||
display_name: DISABLED_ADMIN.displayName,
|
||||
role: "member",
|
||||
},
|
||||
});
|
||||
expect(disabledRes.status(), await disabledRes.text()).toBe(201);
|
||||
const disabledAdminId = responseUserId(await disabledRes.json());
|
||||
expect(
|
||||
(
|
||||
await ctx.request.patch(`${backend.baseUrl}/api/v1/users/${disabledAdminId}`, {
|
||||
data: { role: "admin" },
|
||||
})
|
||||
).status(),
|
||||
).toBe(200);
|
||||
expect(
|
||||
(
|
||||
await ctx.request.patch(`${backend.baseUrl}/api/v1/users/${disabledAdminId}`, {
|
||||
data: { status: "disabled" },
|
||||
})
|
||||
).status(),
|
||||
).toBe(200);
|
||||
|
||||
const page = await ctx.newPage();
|
||||
// Guard against the manual-context device regression: the assertions
|
||||
// below must run on the Pixel 5 width (393 CSS px; headless mobile
|
||||
// emulation trims the height, so only the width is pinned).
|
||||
expect((await page.viewportSize())?.width).toBe(393);
|
||||
await page.goto("/settings/system/users");
|
||||
const ownRow = page.locator(`[data-user-id="${adminId}"]`);
|
||||
const memberRow = page.locator(`[data-user-id="${memberId}"]`);
|
||||
const disabledAdminRow = page.locator(`[data-user-id="${disabledAdminId}"]`);
|
||||
await expect(ownRow).toBeVisible({ timeout: 15_000 });
|
||||
|
||||
// Sole active admin: own row disabled; member and disabled-admin rows
|
||||
// are not gated by the last-admin guard.
|
||||
await expect(ownRow.getByTestId("users-table-toggle-role")).toBeDisabled();
|
||||
await expect(ownRow.getByTestId("users-table-toggle-status")).toBeDisabled();
|
||||
await expect(memberRow.getByTestId("users-table-toggle-role")).toBeEnabled();
|
||||
await expect(memberRow.getByTestId("users-table-toggle-status")).toBeEnabled();
|
||||
await expect(disabledAdminRow.getByTestId("users-table-toggle-role")).toBeEnabled();
|
||||
await expect(disabledAdminRow.getByTestId("users-table-toggle-status")).toBeEnabled();
|
||||
|
||||
// A second active admin lifts the guard on every active-admin row.
|
||||
const secondRes = await ctx.request.post(`${backend.baseUrl}/api/v1/users`, {
|
||||
data: {
|
||||
email: SECOND_ADMIN.email,
|
||||
password: SECOND_ADMIN.password,
|
||||
display_name: SECOND_ADMIN.displayName,
|
||||
role: "admin",
|
||||
},
|
||||
});
|
||||
expect(secondRes.status(), await secondRes.text()).toBe(201);
|
||||
const secondAdminId = responseUserId(await secondRes.json());
|
||||
|
||||
await page.reload();
|
||||
const secondAdminRow = page.locator(`[data-user-id="${secondAdminId}"]`);
|
||||
await expect(ownRow.getByTestId("users-table-toggle-role")).toBeEnabled();
|
||||
await expect(ownRow.getByTestId("users-table-toggle-status")).toBeEnabled();
|
||||
await expect(secondAdminRow.getByTestId("users-table-toggle-role")).toBeEnabled();
|
||||
await expect(secondAdminRow.getByTestId("users-table-toggle-status")).toBeEnabled();
|
||||
|
||||
await ctx.close();
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,15 @@
|
||||
/**
|
||||
* Shared helpers for the auth user-management E2E specs
|
||||
* (users-self-actions.spec.ts desktop and mobile-users-self-actions.spec.ts).
|
||||
*/
|
||||
|
||||
/** Reads `user.id` from an API response body, narrowing the unknown shape. */
|
||||
export function responseUserId(body: unknown): string {
|
||||
if (body && typeof body === "object" && "user" in body) {
|
||||
const user = body.user;
|
||||
if (user && typeof user === "object" && "id" in user) {
|
||||
return String(user.id);
|
||||
}
|
||||
}
|
||||
throw new Error("API response is missing user.id");
|
||||
}
|
||||
@@ -0,0 +1,150 @@
|
||||
import { expect } from "@playwright/test";
|
||||
import path from "node:path";
|
||||
import { backendFixture as test } from "../../fixtures/backend";
|
||||
import { login, setupAdmin } from "../../helpers/auth";
|
||||
import { responseUserId } from "./users-auth-helpers";
|
||||
|
||||
/**
|
||||
* The Users page must grey out the role/status toggles on an active-admin row
|
||||
* when no other active admin exists, mirroring the backend's existing
|
||||
* last-admin guard (ErrLastAdmin -> 409). The backend itself is unchanged:
|
||||
* self-demotion stays allowed while another active admin exists.
|
||||
*
|
||||
* Runs in the `auth` project (backend restarted with auth required). Serial:
|
||||
* shares the admin and users created inside the single test.
|
||||
*
|
||||
* Isolation: the worker-scoped backend fixture and `restart()` preserve the
|
||||
* SQLite DB, and auth setup is single-shot per database. This spec restarts
|
||||
* with its own database path so the full auth project stays deterministic in
|
||||
* either file order (auth-screenshots.spec.ts sets up the same admin email on
|
||||
* the baseline database).
|
||||
*/
|
||||
const ADMIN = { email: "admin@demo.dev", password: "adminpass123", displayName: "Ada Admin" };
|
||||
const MEMBER = { email: "sam@demo.dev", password: "memberpass123", displayName: "Sam Member" };
|
||||
const DISABLED_ADMIN = {
|
||||
email: "carol@demo.dev",
|
||||
password: "carolpass123",
|
||||
displayName: "Carol Admin (off)",
|
||||
};
|
||||
const SECOND_ADMIN = { email: "bob@demo.dev", password: "bobpass123", displayName: "Bob Admin" };
|
||||
|
||||
test.describe.serial("users self-actions guard", () => {
|
||||
test.beforeAll(async ({ backend }) => {
|
||||
// The features.auth flag turns authentication on (setup mode) and reveals
|
||||
// the admin surfaces. A per-file database keeps setup single-shot from
|
||||
// colliding with other auth specs sharing the worker.
|
||||
await backend.restart({
|
||||
KANDEV_FEATURES_AUTH: "true",
|
||||
KANDEV_DATABASE_PATH: path.join(backend.tmpDir, "kandev-auth-self-actions.db"),
|
||||
});
|
||||
});
|
||||
|
||||
test.afterAll(async ({ backend }) => {
|
||||
await backend.restart();
|
||||
});
|
||||
|
||||
test("own-row toggles track the last-admin guard; backend unchanged", async ({
|
||||
browser,
|
||||
backend,
|
||||
}) => {
|
||||
const ctx = await browser.newContext({ baseURL: backend.frontendUrl });
|
||||
await setupAdmin(ctx, backend.baseUrl, ADMIN);
|
||||
await login(ctx, backend.baseUrl, ADMIN);
|
||||
|
||||
// The setup helper does not return the user; fetch the admin id.
|
||||
const meRes = await ctx.request.get(`${backend.baseUrl}/api/v1/auth/me`);
|
||||
expect(meRes.ok(), await meRes.text()).toBeTruthy();
|
||||
const adminId = responseUserId(await meRes.json());
|
||||
|
||||
// A member row provides the enabled baseline.
|
||||
const memberRes = await ctx.request.post(`${backend.baseUrl}/api/v1/users`, {
|
||||
data: {
|
||||
email: MEMBER.email,
|
||||
password: MEMBER.password,
|
||||
display_name: MEMBER.displayName,
|
||||
role: "member",
|
||||
},
|
||||
});
|
||||
expect(memberRes.status(), await memberRes.text()).toBe(201);
|
||||
const memberId = responseUserId(await memberRes.json());
|
||||
|
||||
const page = await ctx.newPage();
|
||||
await page.goto("/settings/system/users");
|
||||
const ownRow = page.locator(`[data-user-id="${adminId}"]`);
|
||||
const memberRow = page.locator(`[data-user-id="${memberId}"]`);
|
||||
await expect(ownRow).toBeVisible({ timeout: 15_000 });
|
||||
|
||||
// Sole active admin: the own row's demote/disable toggles are disabled,
|
||||
// exactly where the backend would reject the action.
|
||||
await expect(ownRow.getByTestId("users-table-toggle-role")).toBeDisabled();
|
||||
await expect(ownRow.getByTestId("users-table-toggle-status")).toBeDisabled();
|
||||
// A member row is never gated by the last-admin guard.
|
||||
await expect(memberRow.getByTestId("users-table-toggle-role")).toBeEnabled();
|
||||
await expect(memberRow.getByTestId("users-table-toggle-status")).toBeEnabled();
|
||||
|
||||
// Existing guard still enforced at the API: the sole admin cannot
|
||||
// self-demote.
|
||||
const selfDemote = await ctx.request.patch(`${backend.baseUrl}/api/v1/users/${adminId}`, {
|
||||
data: { role: "member" },
|
||||
});
|
||||
expect(selfDemote.status(), await selfDemote.text()).toBe(409);
|
||||
|
||||
// A disabled admin is NOT an active admin: the guard does not apply to
|
||||
// its toggles, and counting it must not lift the sole active admin's
|
||||
// disablement (regression: dropping `status === "active"` in either the
|
||||
// count or the per-row condition would flip these two assertions).
|
||||
const disabledRes = await ctx.request.post(`${backend.baseUrl}/api/v1/users`, {
|
||||
data: {
|
||||
email: DISABLED_ADMIN.email,
|
||||
password: DISABLED_ADMIN.password,
|
||||
display_name: DISABLED_ADMIN.displayName,
|
||||
role: "member",
|
||||
},
|
||||
});
|
||||
expect(disabledRes.status(), await disabledRes.text()).toBe(201);
|
||||
const disabledAdminId = responseUserId(await disabledRes.json());
|
||||
const promote = await ctx.request.patch(`${backend.baseUrl}/api/v1/users/${disabledAdminId}`, {
|
||||
data: { role: "admin" },
|
||||
});
|
||||
expect(promote.status(), await promote.text()).toBe(200);
|
||||
const disable = await ctx.request.patch(`${backend.baseUrl}/api/v1/users/${disabledAdminId}`, {
|
||||
data: { status: "disabled" },
|
||||
});
|
||||
expect(disable.status(), await disable.text()).toBe(200);
|
||||
|
||||
await page.reload();
|
||||
const disabledAdminRow = page.locator(`[data-user-id="${disabledAdminId}"]`);
|
||||
await expect(ownRow.getByTestId("users-table-toggle-role")).toBeDisabled();
|
||||
await expect(ownRow.getByTestId("users-table-toggle-status")).toBeDisabled();
|
||||
await expect(disabledAdminRow.getByTestId("users-table-toggle-role")).toBeEnabled();
|
||||
await expect(disabledAdminRow.getByTestId("users-table-toggle-status")).toBeEnabled();
|
||||
|
||||
// A second active admin lifts the guard on every active-admin row.
|
||||
const secondRes = await ctx.request.post(`${backend.baseUrl}/api/v1/users`, {
|
||||
data: {
|
||||
email: SECOND_ADMIN.email,
|
||||
password: SECOND_ADMIN.password,
|
||||
display_name: SECOND_ADMIN.displayName,
|
||||
role: "admin",
|
||||
},
|
||||
});
|
||||
expect(secondRes.status(), await secondRes.text()).toBe(201);
|
||||
const secondAdminId = responseUserId(await secondRes.json());
|
||||
|
||||
await page.reload();
|
||||
const secondAdminRow = page.locator(`[data-user-id="${secondAdminId}"]`);
|
||||
await expect(ownRow.getByTestId("users-table-toggle-role")).toBeEnabled();
|
||||
await expect(ownRow.getByTestId("users-table-toggle-status")).toBeEnabled();
|
||||
await expect(secondAdminRow.getByTestId("users-table-toggle-role")).toBeEnabled();
|
||||
await expect(secondAdminRow.getByTestId("users-table-toggle-status")).toBeEnabled();
|
||||
|
||||
// No new backend behavior: self-demotion is allowed again once another
|
||||
// active admin exists.
|
||||
const selfDemoteAfter = await ctx.request.patch(`${backend.baseUrl}/api/v1/users/${adminId}`, {
|
||||
data: { role: "member" },
|
||||
});
|
||||
expect(selfDemoteAfter.status(), await selfDemoteAfter.text()).toBe(200);
|
||||
|
||||
await ctx.close();
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,165 @@
|
||||
---
|
||||
spec: docs/specs/auth/self-actions-guard.md
|
||||
created: 2026-08-15
|
||||
updated: 2026-08-15
|
||||
status: complete
|
||||
---
|
||||
|
||||
# Implementation Plan: Self-Actions Guard in System Users
|
||||
|
||||
## Overview
|
||||
|
||||
Settings > System > Users lets an admin change the role or status of any listed
|
||||
account, including their own. The backend already rejects demoting or disabling
|
||||
the last active admin (409 `ErrLastAdmin`), but the page shows the toggle
|
||||
buttons as enabled, so the rejection surfaces only as a toast after the click.
|
||||
The fix greys out exactly those buttons the backend would reject: on any row
|
||||
that is an active admin, the role toggle ("Make member") and status toggle
|
||||
("Disable") are disabled when no other active admin exists.
|
||||
|
||||
This is a UI-only change. The backend keeps its current last-admin guard
|
||||
verbatim; self-demotion and self-disable remain possible while another active
|
||||
admin exists, exactly as today.
|
||||
|
||||
## Current behavior
|
||||
|
||||
- Backend: `ensureAnotherAdmin` in
|
||||
`apps/backend/internal/auth/service_users.go` rejects a change that would
|
||||
leave zero users with `role = admin` and `status = active` (excluding the
|
||||
target). `AdminSetRoleStatus` applies it to any target, self included; the
|
||||
`updateUser` handler documents that self-demotion/disable is allowed when
|
||||
another active admin remains. This behavior is correct and stays unchanged.
|
||||
- Frontend: `UserRow` in
|
||||
`apps/web/components/settings/system/users-table.tsx` renders the role and
|
||||
status toggle buttons for every row with no guard awareness. The user list
|
||||
(`useUsersList`) is the same data source the backend guard scans
|
||||
(`ListUsers`), so the disabled state can be derived from the rendered rows.
|
||||
- E2E: auth-enabled specs live in `apps/web/e2e/tests/auth/*.spec.ts`, run in
|
||||
the `auth` Playwright project (`KANDEV_FEATURES_AUTH=true` backend restart;
|
||||
helpers in `e2e/helpers/auth.ts`). No functional spec exists for the Users
|
||||
page today (only `auth-screenshots.spec.ts`).
|
||||
|
||||
## Frontend
|
||||
|
||||
- `apps/web/components/settings/system/users-table.tsx`:
|
||||
- Derive `activeAdminCount` from the loaded users:
|
||||
`users.filter((u) => u.role === "admin" && u.status === "active").length`,
|
||||
mirroring `ensureAnotherAdmin` exactly.
|
||||
- In `UsersTableList` / `UserRow`, disable a row's role and status toggles
|
||||
when the row is an active admin and `activeAdminCount === 1`
|
||||
(`isLastActiveAdmin`). This is precisely the set of actions the backend
|
||||
rejects today (demote or disable of the last active admin); every other
|
||||
button stays enabled.
|
||||
- No current-user identity is needed: the guard is target-based, not
|
||||
caller-based, and the own row is just an active-admin row.
|
||||
- Keep existing labels and `data-testid` attributes
|
||||
(`users-table-toggle-role`, `users-table-toggle-status`). Shadcn button
|
||||
already styles the disabled state
|
||||
(`disabled:opacity-45 disabled:cursor-not-allowed`).
|
||||
- No new user-facing copy; no i18n key changes.
|
||||
|
||||
## Tests
|
||||
|
||||
- No backend tests change: the backend is untouched.
|
||||
- No frontend unit tests: the change is button state in component markup,
|
||||
which this repo does not unit-test. The behavior is covered by the E2E
|
||||
spec below plus the existing `TestAdminUserManagement` backend guard tests
|
||||
(which stay green and prove the guard still exists).
|
||||
|
||||
## E2E
|
||||
|
||||
- New `apps/web/e2e/tests/auth/users-self-actions.spec.ts` in the `auth`
|
||||
project: serial describe, `backend.restart({ KANDEV_FEATURES_AUTH: "true" })`
|
||||
in `beforeAll`, restart to baseline in `afterAll` (pattern from
|
||||
`auth-screenshots.spec.ts`).
|
||||
- Setup admin + login via `e2e/helpers/auth.ts`; create a member via
|
||||
`POST /api/v1/users` with the admin context (`context.request` shares the
|
||||
session cookie).
|
||||
- Assertions:
|
||||
- Sole admin (fresh setup): own row (`[data-user-id="{adminId}"]`) has both
|
||||
toggles `toBeDisabled()`; the member row has both `toBeEnabled()`.
|
||||
- Existing guard proof: `PATCH /api/v1/users/{self}` with
|
||||
`{ "role": "member" }` as the sole admin returns 409 `ErrLastAdmin`
|
||||
(backend unchanged).
|
||||
- Create a second active admin via the API, reload the page: the own row's
|
||||
toggles are now `toBeEnabled()` (guard passes; button state tracks it).
|
||||
- No-new-behavior proof: `PATCH /api/v1/users/{self}` with
|
||||
`{ "role": "member" }` now returns 200 (self-demotion stays allowed when
|
||||
another active admin exists).
|
||||
- Run: `cd apps/web && pnpm e2e:run --project auth
|
||||
tests/auth/users-self-actions.spec.ts`.
|
||||
|
||||
## Implementation Waves And Parallel Candidates
|
||||
|
||||
Wave 1:
|
||||
|
||||
- [x] [Task 01: Disable last-admin toggles in users table](task-01-frontend-disable-last-admin.md)
|
||||
|
||||
Wave 2:
|
||||
|
||||
- [x] [Task 02: E2E coverage for guard-matched buttons](task-02-e2e-guard-mirror.md)
|
||||
|
||||
Execution is sequential. Task 02 depends on the frontend change from Task 01.
|
||||
|
||||
## Verification
|
||||
|
||||
Focused, per task:
|
||||
|
||||
```bash
|
||||
cd "$(git rev-parse --show-toplevel)" && make typecheck
|
||||
cd "$(git rev-parse --show-toplevel)/apps/backend" && go test ./internal/auth/... -run TestAdminUserManagement -count=1
|
||||
cd apps/web && pnpm e2e:run --project auth tests/auth/users-self-actions.spec.ts
|
||||
```
|
||||
|
||||
Final gate (per AGENTS.md): `make fmt`, then `make typecheck test lint`. E2E
|
||||
is not part of `make test`; the auth-project spec runs via `make test-e2e` in
|
||||
CI.
|
||||
|
||||
## Mobile parity
|
||||
|
||||
The change adds a `disabled` attribute to two buttons in the shared
|
||||
`users-table.tsx` component; layout, scrolling, navigation, and touch targets
|
||||
are unchanged, and the state derivation is shared markup. Mobile coverage is
|
||||
still provided: `apps/web/e2e/tests/auth/mobile-users-self-actions.spec.ts`
|
||||
runs in the `mobile-chrome` project (Pixel 5) with the same per-file isolated
|
||||
auth database and asserts the sole-admin own row is disabled while member and
|
||||
disabled-admin rows stay enabled, plus re-enablement after a second active
|
||||
admin. It is routed out of the desktop `auth` project via its `testIgnore`
|
||||
(`e2e/playwright.config.ts`) so it runs exactly once, on the mobile viewport.
|
||||
|
||||
## Recorded Results
|
||||
|
||||
- E2E: `cd apps/web && pnpm e2e:run --project auth
|
||||
tests/auth/users-self-actions.spec.ts` — RED first (sole-admin toggle
|
||||
enabled, expected disabled), then 1 passed after the component change.
|
||||
- `make fmt` passed (unrelated pre-existing gofmt drift in four backend files
|
||||
was restored, not committed).
|
||||
- `make typecheck` passed.
|
||||
- `make lint` passed: golangci-lint 0 issues, eslint `--max-warnings 0`
|
||||
clean, 118 harness files passed, architecture lint passed.
|
||||
- `go test ./internal/auth/... -run TestAdminUserManagement -count=1` passed
|
||||
(backend guard untouched).
|
||||
- `make test` is not fully green in this environment for pre-existing
|
||||
reasons, all proven unrelated by stashing this change: harness-injected
|
||||
`KANDEV_VERSION=v0.88.0-dirty` breaks two launcher metadata tests; the host
|
||||
`~/.profile` references a missing `/tmp/cargo/env` (agentctl config test);
|
||||
two agentctl process-lifecycle tests are timing-sensitive under host load;
|
||||
`lib/http-git-server.test.ts` requires a Docker bridge gateway (no Docker
|
||||
daemon on this host); `test-scripts` needs the `unzip` binary (absent, no
|
||||
sudo to install). CI has none of these gaps.
|
||||
|
||||
## Risks and limits
|
||||
|
||||
- Button state is a mirror of the last loaded user list, not a live
|
||||
authorization decision. A stale list (an admin demoted or disabled in
|
||||
another session since the fetch) can show an enabled button whose request
|
||||
the backend rejects; the existing 409 `ErrLastAdmin` toast covers that path
|
||||
unchanged.
|
||||
- The frontend count must match `ensureAnotherAdmin`'s condition exactly
|
||||
(`role = admin` AND `status = active`, excluding the target row). Any drift
|
||||
between the mirror and the guard would show either a clickable-rejected
|
||||
button or a needlessly disabled one; the E2E asserts both directions
|
||||
(sole-admin disabled, two-admin enabled).
|
||||
- The `users` snapshot used for the count is the same `ListUsers` data the
|
||||
backend guard scans, so the mirror and the guard evaluate the same set
|
||||
modulo request-time staleness.
|
||||
@@ -0,0 +1,100 @@
|
||||
---
|
||||
id: "01-frontend-disable-last-admin"
|
||||
title: "Disable last-admin toggles in users table"
|
||||
status: done
|
||||
wave: 1
|
||||
depends_on: []
|
||||
plan: "plan.md"
|
||||
spec: "../../specs/auth/self-actions-guard.md"
|
||||
---
|
||||
|
||||
# Task 01: Disable last-admin toggles in users table
|
||||
|
||||
## Intent
|
||||
|
||||
Grey out the role and status toggles on any active-admin row when no other
|
||||
active admin exists, mirroring the backend's existing last-admin guard. No
|
||||
backend change.
|
||||
|
||||
## Inputs
|
||||
|
||||
- The spec `What` and `Scenarios` sections.
|
||||
- `apps/web/components/settings/system/users-table.tsx` (`UsersTable`,
|
||||
`useUsersList`, `UsersTableList`, `UserRow`).
|
||||
- The backend condition to mirror:
|
||||
`apps/backend/internal/auth/service_users.go` `ensureAnotherAdmin`: reject
|
||||
when no user other than the target has `role = "admin"` AND
|
||||
`status = "active"`.
|
||||
|
||||
## Change
|
||||
|
||||
- In `UsersTable` (or `UsersTableList`), derive
|
||||
`activeAdminCount = users.filter((u) => u.role === "admin" && u.status === "active").length`
|
||||
from the loaded list.
|
||||
- Thread the count (or a per-row `isLastActiveAdmin` boolean) into `UserRow`.
|
||||
- In `UserRow`, compute
|
||||
`isLastActiveAdmin = user.role === "admin" && user.status === "active" && activeAdminCount === 1`
|
||||
and add `disabled={isLastActiveAdmin}` to both the role toggle
|
||||
(`users-table-toggle-role`) and status toggle (`users-table-toggle-status`).
|
||||
- Do not read the current user id: the guard is target-based, not caller-based,
|
||||
and applies to every row identically.
|
||||
- Keep existing labels, `data-testid` attributes, and styling. The shadcn
|
||||
button variant already applies `disabled:opacity-45` and
|
||||
`disabled:cursor-not-allowed`.
|
||||
- No new user-facing copy; no i18n key changes. No backend files change.
|
||||
|
||||
## Acceptance
|
||||
|
||||
- When the loaded list contains exactly one active admin, that row's role and
|
||||
status toggles carry the `disabled` attribute; all other rows' toggles do
|
||||
not.
|
||||
- When the list contains two or more active admins, no row's toggles are
|
||||
disabled.
|
||||
- Member rows and disabled-admin rows are never disabled by this change.
|
||||
- `make typecheck` and web lint pass; `go test ./internal/auth/... -run
|
||||
TestAdminUserManagement` stays green untouched (the backend guard is
|
||||
unchanged).
|
||||
|
||||
## TDD sequence
|
||||
|
||||
Per project convention, React component markup is not unit-tested; the
|
||||
behavior is asserted by the Task 02 E2E spec (run after this task). Verify by
|
||||
`typecheck`/lint here and the E2E spec in Task 02.
|
||||
|
||||
## Verification
|
||||
|
||||
```bash
|
||||
cd "$(git rev-parse --show-toplevel)" && make typecheck
|
||||
cd "$(git rev-parse --show-toplevel)/apps/backend" && go test ./internal/auth/... -run TestAdminUserManagement -count=1
|
||||
```
|
||||
|
||||
(The behavioral check is `pnpm e2e:run --project auth
|
||||
tests/auth/users-self-actions.spec.ts` from Task 02.)
|
||||
|
||||
## Dependencies
|
||||
|
||||
None.
|
||||
|
||||
## Parallelism
|
||||
|
||||
`sequential`. Single component file; the E2E task depends on it.
|
||||
|
||||
## Output contract
|
||||
|
||||
Report the files changed, verification results, blockers, and risks. Update
|
||||
this task and `plan.md` in the same conversation.
|
||||
|
||||
## Results
|
||||
|
||||
- Red: the E2E spec (written before the change) failed as expected on the
|
||||
sole-admin own row: `expect(locator).toBeDisabled()` received "enabled" for
|
||||
`[data-user-id="default-user"]` `users-table-toggle-role`.
|
||||
- Green: `pnpm e2e:run --project auth tests/auth/users-self-actions.spec.ts`
|
||||
passed 1 test (7.6s) covering the sole-admin disabled state, the member-row
|
||||
enabled baseline, re-enablement after adding a second admin, and the
|
||||
unchanged 409/200 self-PATCH backend outcomes.
|
||||
- `make typecheck` passed; `make lint` passed (eslint `--max-warnings 0`,
|
||||
golangci-lint 0 issues).
|
||||
- `go test ./internal/auth/... -run TestAdminUserManagement -count=1` passed
|
||||
untouched, proving the backend guard is unchanged.
|
||||
- Only `apps/web/components/settings/system/users-table.tsx` changed.
|
||||
@@ -0,0 +1,109 @@
|
||||
---
|
||||
id: "02-e2e-guard-mirror"
|
||||
title: "E2E coverage for guard-matched buttons"
|
||||
status: done
|
||||
wave: 2
|
||||
depends_on: ["01-frontend-disable-last-admin"]
|
||||
plan: "plan.md"
|
||||
spec: "../../specs/auth/self-actions-guard.md"
|
||||
---
|
||||
|
||||
# Task 02: E2E coverage for guard-matched buttons
|
||||
|
||||
## Intent
|
||||
|
||||
Prove from the browser that the toggle buttons are disabled exactly when the
|
||||
existing backend last-admin guard would reject the action, and that the
|
||||
backend behavior itself is unchanged.
|
||||
|
||||
## Inputs
|
||||
|
||||
- The spec `Scenarios` section.
|
||||
- E2E auth patterns: `apps/web/e2e/tests/auth/auth-screenshots.spec.ts`
|
||||
(serial describe, `backend.restart({ KANDEV_FEATURES_AUTH: "true" })` in
|
||||
`beforeAll`, baseline restart in `afterAll`) and
|
||||
`apps/web/e2e/helpers/auth.ts` (`setupAdmin`, `login`).
|
||||
- Row locators: `users-table-row` rows carry `data-user-id`; the toggle
|
||||
buttons carry `users-table-toggle-role` / `users-table-toggle-status`.
|
||||
|
||||
## Change
|
||||
|
||||
New file `apps/web/e2e/tests/auth/users-self-actions.spec.ts`:
|
||||
|
||||
1. Serial describe; `beforeAll` restarts the backend with
|
||||
`KANDEV_FEATURES_AUTH: "true"`; `afterAll` restarts to baseline.
|
||||
2. Setup the admin (`setupAdmin`), login the admin context, and create a
|
||||
member via `POST /api/v1/users` on `context.request` (shares the session
|
||||
cookie). Capture the admin and member ids.
|
||||
3. Sole-admin UI: open `/settings/system/users`; own row
|
||||
`[data-user-id="{adminId}"]` has both toggles `toBeDisabled()`; the member
|
||||
row has both `toBeEnabled()`.
|
||||
4. Existing-guard proof: `PATCH /api/v1/users/{adminId}` with
|
||||
`{ "role": "member" }` as the sole admin returns 409 (backend unchanged).
|
||||
5. Create a second active admin (`POST /api/v1/users`, `role: "admin"`);
|
||||
reload the page; the own row's toggles are now `toBeEnabled()`.
|
||||
6. No-new-behavior proof: `PATCH /api/v1/users/{adminId}` with
|
||||
`{ "role": "member" }` now returns 200 (self-demotion still allowed when
|
||||
another active admin exists).
|
||||
|
||||
## Acceptance
|
||||
|
||||
- The spec passes with the production build (the managed runner builds it).
|
||||
- Test discovery: `pnpm e2e:run --project auth
|
||||
tests/auth/users-self-actions.spec.ts` runs exactly the new tests.
|
||||
|
||||
## TDD sequence
|
||||
|
||||
1. Write the spec (RED: buttons are enabled for the sole admin today).
|
||||
2. Run it via the auth project; confirm the failures are the expected
|
||||
assertions.
|
||||
3. With Task 01 landed, re-run; confirm green.
|
||||
|
||||
## Verification
|
||||
|
||||
```bash
|
||||
cd "$(git rev-parse --show-toplevel)/apps/web" && pnpm e2e:run --project auth tests/auth/users-self-actions.spec.ts
|
||||
```
|
||||
|
||||
## Dependencies
|
||||
|
||||
Task 01 (disabled-button state).
|
||||
|
||||
## Parallelism
|
||||
|
||||
`sequential`. The spec asserts the Task 01 change and the unchanged backend.
|
||||
|
||||
## Output contract
|
||||
|
||||
Report the spec path, the red and green run results (with the exact command),
|
||||
blockers, and risks. Update this task and `plan.md` in the same conversation.
|
||||
|
||||
## Results
|
||||
|
||||
- Spec: `apps/web/e2e/tests/auth/users-self-actions.spec.ts`, one serial test
|
||||
in the `auth` project (`backend.restart({ KANDEV_FEATURES_AUTH: "true" })`).
|
||||
- Red (before Task 01): failed at the sole-admin `toBeDisabled()` assertion on
|
||||
`users-table-toggle-role` — button was enabled.
|
||||
- Green (after Task 01): `cd apps/web && pnpm e2e:run --project auth
|
||||
tests/auth/users-self-actions.spec.ts` → 1 passed (7.6s).
|
||||
- The spec also proves the backend is unchanged: sole-admin self-PATCH
|
||||
`{role:"member"}` → 409; after a second active admin, same PATCH → 200.
|
||||
- Review fixup (luna round 1): `beforeAll` now restarts with a per-file
|
||||
`KANDEV_DATABASE_PATH` inside the fixture tmpDir. The worker-scoped
|
||||
fixture and `restart()` preserve the SQLite DB and auth setup is
|
||||
single-shot, so two auth specs sharing the worker collided regardless of
|
||||
admin email; the isolated DB keeps the full `--project auth` suite
|
||||
deterministic in either file order. Coverage widened: a disabled-admin row
|
||||
is created and asserted enabled while the sole active admin stays disabled
|
||||
(catches dropping `status === "active"` from the count or the per-row
|
||||
condition), and both active-admin rows are asserted enabled after the
|
||||
second admin is added.
|
||||
- Full auth project run: `pnpm e2e:run --no-build --project auth tests/auth/`
|
||||
→ exit 0, 15 passed (both spec files in one worker).
|
||||
- Review fixup (luna round 2, mobile parity): added
|
||||
`apps/web/e2e/tests/auth/mobile-users-self-actions.spec.ts` (runs in the
|
||||
`mobile-chrome` Pixel 5 project via a `testIgnore` on the desktop `auth`
|
||||
project) asserting the same disabled/enabled states at the mobile viewport,
|
||||
plus the shared `users-auth-helpers.ts` used by both specs. Mobile run:
|
||||
`pnpm e2e:run --no-build --project mobile-chrome
|
||||
tests/auth/mobile-users-self-actions.spec.ts` → 1 passed (8.1s).
|
||||
@@ -265,6 +265,7 @@ System pages (Radarr/Sonarr-style) for status, disk usage, database maintenance,
|
||||
| [mcp-session-observability](mcp-session-observability/spec.md) | approved |
|
||||
| [subagent-context-persistence](subagent-context-persistence/spec.md) | draft |
|
||||
| [auth](auth/spec.md) | building |
|
||||
| [auth — self-actions guard](auth/self-actions-guard.md) | shipped |
|
||||
| [create-local-repository](create-local-repository/spec.md) | shipped |
|
||||
| [workflow-cycle-guardrails](workflow-cycle-guardrails/spec.md) | building |
|
||||
| [improve-kandev](improve-kandev/spec.md) | building |
|
||||
|
||||
@@ -0,0 +1,76 @@
|
||||
---
|
||||
status: shipped
|
||||
created: 2026-08-15
|
||||
---
|
||||
|
||||
# Self-Actions Guard in System Users
|
||||
|
||||
## Why
|
||||
|
||||
Settings > System > Users lets an admin change the role or status of any listed
|
||||
account, including their own. The backend already enforces one guard on these
|
||||
actions: an active admin cannot be demoted or disabled when no other active
|
||||
admin exists (`ErrLastAdmin` -> 409). The page does not reflect that guard, so
|
||||
the last active admin can click "Make member" or "Disable" on a row and only
|
||||
then discover the rejection in an error toast. The buttons should show the
|
||||
guard up front: greyed out exactly when the backend would reject the action.
|
||||
|
||||
## What
|
||||
|
||||
- On any row that is an active admin, the role toggle ("Make member") and the
|
||||
status toggle ("Disable") are disabled when no other active admin exists in
|
||||
the user list. This mirrors the backend's existing `ensureAnotherAdmin`
|
||||
condition exactly: another user with `role = admin` and `status = active`.
|
||||
- All other buttons keep their current behavior. "Make admin" on member rows,
|
||||
"Enable" on disabled rows, and every action on rows where another active
|
||||
admin exists stay enabled.
|
||||
- The API behavior is unchanged. The backend keeps its current last-admin
|
||||
guard for any target, including the caller's own account; self-demotion and
|
||||
self-disable remain possible when another active admin exists.
|
||||
|
||||
## API surface
|
||||
|
||||
No change. `PATCH /api/v1/users/:id` keeps its current semantics: role/status
|
||||
normalization, the last-admin guard (409 `ErrLastAdmin`), and session/token
|
||||
revocation on disable. The guard applies to any target, self included.
|
||||
|
||||
## Permissions
|
||||
|
||||
Unchanged. The route stays admin-only (`RequireRealIdentity` +
|
||||
`RequireAdmin`).
|
||||
|
||||
## Failure modes
|
||||
|
||||
- Button state is computed from the last loaded user list. If the list is
|
||||
stale (an admin was demoted elsewhere since the fetch), the backend still
|
||||
rejects with 409 and the existing `usersLastAdminGuard` toast appears; the
|
||||
guard is authoritative at request time.
|
||||
- No new failure mode is introduced on the backend.
|
||||
|
||||
## Scenarios
|
||||
|
||||
- **GIVEN** the signed-in admin is the only user with `role = admin` and
|
||||
`status = active`, **WHEN** the Users page renders, **THEN** the own row's
|
||||
role toggle ("Make member") and status toggle ("Disable") are disabled; no
|
||||
other row is affected.
|
||||
- **GIVEN** the only active admin, **WHEN** the page shows another row (a
|
||||
member or a disabled admin), **THEN** that row's toggles remain enabled
|
||||
(the guard only fires on active admins).
|
||||
- **GIVEN** at least one other user with `role = admin` and `status = active`,
|
||||
**WHEN** the Users page renders any active-admin row (including the own
|
||||
row), **THEN** both toggles on that row are enabled.
|
||||
- **GIVEN** the last active admin calls `PATCH /api/v1/users/{self}` with
|
||||
`{ "role": "member" }` or `{ "status": "disabled" }`, **THEN** the API
|
||||
responds 409 `ErrLastAdmin` (existing behavior, unchanged).
|
||||
- **GIVEN** an active admin with another active admin present calls
|
||||
`PATCH /api/v1/users/{self}` with `{ "role": "member" }`, **THEN** the API
|
||||
applies the change (existing behavior, unchanged).
|
||||
|
||||
## Out of scope
|
||||
|
||||
- No backend change: no new self-action rejection, no signature changes, no
|
||||
new error codes or messages.
|
||||
- No change to the last-admin guard itself.
|
||||
- No change to invite creation, member restrictions, or the account page.
|
||||
- Button disablement only mirrors the existing guard; it is not a new
|
||||
authorization rule.
|
||||
Reference in New Issue
Block a user