From 15e5464da7cd0e16d398cabf9992d9cffff92575 Mon Sep 17 00:00:00 2001 From: cesnimda Date: Sat, 15 Aug 2026 13:11:54 +0200 Subject: [PATCH] fix(admin): protect administrator access Reject final-admin demotion and deletion at the API boundary. Require explicit confirmation before any administrator role removal. --- JobTrackerApi.Tests/UsersControllerTests.cs | 159 +++++++++++++++++++ JobTrackerApi/Controllers/UsersController.cs | 59 +++++-- docs/audits/verification-log.md | 1 + docs/work-programmes/decisions.md | 20 +++ docs/work-programmes/master-progress.md | 6 +- docs/work-programmes/session-handoff.md | 16 +- job-tracker-ui/src/admin-users-page.test.tsx | 84 ++++++++++ job-tracker-ui/src/i18n/translations.ts | 20 +++ job-tracker-ui/src/views/AdminUsersPage.tsx | 91 ++++++++--- 9 files changed, 411 insertions(+), 45 deletions(-) create mode 100644 JobTrackerApi.Tests/UsersControllerTests.cs create mode 100644 job-tracker-ui/src/admin-users-page.test.tsx diff --git a/JobTrackerApi.Tests/UsersControllerTests.cs b/JobTrackerApi.Tests/UsersControllerTests.cs new file mode 100644 index 0000000..ce67c43 --- /dev/null +++ b/JobTrackerApi.Tests/UsersControllerTests.cs @@ -0,0 +1,159 @@ +using System.Security.Claims; +using System.Linq.Expressions; +using JobTrackerApi.Controllers; +using JobTrackerApi.Models; +using JobTrackerApi.Services; +using JobTrackerApi.Tests.TestSupport; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Identity; +using Microsoft.AspNetCore.Mvc; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.Extensions.Options; +using Microsoft.EntityFrameworkCore.Query; +using Moq; +using Xunit; + +namespace JobTrackerApi.Tests; + +public sealed class UsersControllerTests +{ + [Fact] + public async Task SetRoles_rejects_removing_the_final_administrator() + { + var admin = User("admin-1"); + var users = TestHostFactory.CreateUserManager(admin); + users.Setup(x => x.GetRolesAsync(admin)).ReturnsAsync(["Admin"]); + users.Setup(x => x.GetUsersInRoleAsync("Admin")).ReturnsAsync([admin]); + + var controller = CreateController(users, admin.Id); + var result = await controller.SetRoles(admin.Id, new UsersController.SetRolesRequest([]), CancellationToken.None); + + var conflict = Assert.IsType(result); + Assert.Equal("Last administrator protected", Assert.IsType(conflict.Value).Title); + users.Verify(x => x.RemoveFromRolesAsync(It.IsAny(), It.IsAny>()), Times.Never); + } + + [Fact] + public async Task SetRoles_allows_a_confirmed_self_demotion_when_another_admin_exists() + { + var admin = User("admin-1"); + var otherAdmin = User("admin-2"); + var users = TestHostFactory.CreateUserManager(admin); + users.Setup(x => x.GetRolesAsync(admin)).ReturnsAsync(["Admin"]); + users.Setup(x => x.GetUsersInRoleAsync("Admin")).ReturnsAsync([admin, otherAdmin]); + users.Setup(x => x.RemoveFromRolesAsync(admin, It.Is>(roles => roles.Contains("Admin")))) + .ReturnsAsync(IdentityResult.Success); + + var controller = CreateController(users, admin.Id); + var result = await controller.SetRoles(admin.Id, new UsersController.SetRolesRequest([]), CancellationToken.None); + + Assert.IsType(result); + users.Verify(x => x.RemoveFromRolesAsync(admin, It.IsAny>()), Times.Once); + } + + [Fact] + public async Task Delete_rejects_deleting_the_final_administrator() + { + var admin = User("admin-1"); + var users = TestHostFactory.CreateUserManager(admin); + users.Setup(x => x.IsInRoleAsync(admin, "Admin")).ReturnsAsync(true); + users.Setup(x => x.GetUsersInRoleAsync("Admin")).ReturnsAsync([admin]); + + var controller = CreateController(users, admin.Id); + var result = await controller.Delete(admin.Id, CancellationToken.None); + + Assert.IsType(result); + users.Verify(x => x.DeleteAsync(It.IsAny()), Times.Never); + } + + [Fact] + public async Task List_marks_the_current_user_and_last_admin_safety_state() + { + var admin = User("admin-1"); + var member = User("user-1"); + var users = TestHostFactory.CreateUserManager(); + users.SetupGet(x => x.Users).Returns(new TestAsyncEnumerable([admin, member])); + users.Setup(x => x.GetUsersInRoleAsync("Admin")).ReturnsAsync([admin]); + users.Setup(x => x.GetRolesAsync(admin)).ReturnsAsync(["Admin"]); + users.Setup(x => x.GetRolesAsync(member)).ReturnsAsync([]); + + var controller = CreateController(users, admin.Id); + var action = await controller.List(CancellationToken.None); + + var rows = Assert.IsType>(Assert.IsType(action.Result).Value); + var adminRow = Assert.Single(rows, row => row.Id == admin.Id); + Assert.True(adminRow.IsCurrentUser); + Assert.False(adminRow.CanRemoveAdmin); + Assert.True(Assert.Single(rows, row => row.Id == member.Id).CanRemoveAdmin); + } + + private static ApplicationUser User(string id) => new() + { + Id = id, + Email = $"{id}@example.com", + UserName = $"{id}@example.com" + }; + + private static UsersController CreateController(Mock> users, string currentUserId) + { + var roleStore = new Mock>(); + var roles = new Mock>( + roleStore.Object, + Array.Empty>(), + new UpperInvariantLookupNormalizer(), + new IdentityErrorDescriber(), + new NullLogger>()); + + var controller = new UsersController( + users.Object, + roles.Object, + Mock.Of(), + new ConfigurationBuilder().Build(), + new NullLogger(), + ExternalOrigin.Parse("http://localhost:3000", production: false)); + + var identity = new ClaimsIdentity([new Claim(ClaimTypes.NameIdentifier, currentUserId)], "test"); + controller.ControllerContext = new ControllerContext + { + HttpContext = new DefaultHttpContext { User = new ClaimsPrincipal(identity) } + }; + return controller; + } + + private sealed class TestAsyncQueryProvider(IQueryProvider inner) : IAsyncQueryProvider + { + public IQueryable CreateQuery(Expression expression) => new TestAsyncEnumerable(expression); + public IQueryable CreateQuery(Expression expression) => new TestAsyncEnumerable(expression); + public object? Execute(Expression expression) => inner.Execute(expression); + public TResult Execute(Expression expression) => inner.Execute(expression); + public TResult ExecuteAsync(Expression expression, CancellationToken cancellationToken = default) + => (TResult)typeof(Task) + .GetMethod(nameof(Task.FromResult))! + .MakeGenericMethod(typeof(TResult).GetGenericArguments()[0]) + .Invoke(null, [Execute(expression)])!; + } + + private sealed class TestAsyncEnumerable : EnumerableQuery, IAsyncEnumerable, IQueryable + { + public TestAsyncEnumerable(IEnumerable enumerable) : base(enumerable) { } + public TestAsyncEnumerable(Expression expression) : base(expression) { } + + public IAsyncEnumerator GetAsyncEnumerator(CancellationToken cancellationToken = default) + => new TestAsyncEnumerator(this.AsEnumerable().GetEnumerator()); + + IQueryProvider IQueryable.Provider => new TestAsyncQueryProvider(this); + } + + private sealed class TestAsyncEnumerator(IEnumerator inner) : IAsyncEnumerator + { + public T Current => inner.Current; + public ValueTask DisposeAsync() + { + inner.Dispose(); + return ValueTask.CompletedTask; + } + + public ValueTask MoveNextAsync() => ValueTask.FromResult(inner.MoveNext()); + } +} diff --git a/JobTrackerApi/Controllers/UsersController.cs b/JobTrackerApi/Controllers/UsersController.cs index 9ea8e4c..e829605 100644 --- a/JobTrackerApi/Controllers/UsersController.cs +++ b/JobTrackerApi/Controllers/UsersController.cs @@ -38,7 +38,9 @@ public sealed class UsersController : ControllerBase bool EmailConfirmed, string? GoogleEmail, DateTimeOffset? GoogleLinkedAt, - List Roles); + List Roles, + bool IsCurrentUser, + bool CanRemoveAdmin); [HttpGet] public async Task>> List(CancellationToken cancellationToken) @@ -46,12 +48,15 @@ public sealed class UsersController : ControllerBase var items = await _users.Users .OrderBy(u => u.Email) .ToListAsync(cancellationToken); + var currentUserId = User.FindFirstValue(ClaimTypes.NameIdentifier) ?? User.FindFirstValue("sub"); + var adminCount = (await _users.GetUsersInRoleAsync("Admin")).Count; var outList = new List(items.Count); foreach (var u in items) { var rs = await _users.GetRolesAsync(u); - outList.Add(ToDto(u, rs.ToList())); + var roles = rs.ToList(); + outList.Add(ToDto(u, roles, currentUserId, !roles.Contains("Admin", StringComparer.OrdinalIgnoreCase) || adminCount > 1)); } return Ok(outList); @@ -93,7 +98,8 @@ public sealed class UsersController : ControllerBase } var rs = await _users.GetRolesAsync(u); - return Ok(ToDto(u, rs.ToList())); + var currentUserId = User.FindFirstValue(ClaimTypes.NameIdentifier) ?? User.FindFirstValue("sub"); + return Ok(ToDto(u, rs.ToList(), currentUserId, true)); } public sealed record SetRolesRequest(string[] Roles); @@ -110,14 +116,32 @@ public sealed class UsersController : ControllerBase var toRemove = current.Where(r => !desired.Contains(r, StringComparer.OrdinalIgnoreCase)).ToList(); var toAdd = desired.Where(r => !current.Contains(r, StringComparer.OrdinalIgnoreCase)).ToList(); - if (toRemove.Count > 0) - await _users.RemoveFromRolesAsync(u, toRemove); + if (toRemove.Contains("Admin", StringComparer.OrdinalIgnoreCase) + && (await _users.GetUsersInRoleAsync("Admin")).Count <= 1) + { + return Conflict(new ProblemDetails + { + Title = "Last administrator protected", + Detail = "Assign the Admin role to another user before removing it from the final administrator." + }); + } foreach (var r in toAdd) { if (!await _roles.RoleExistsAsync(r)) - await _roles.CreateAsync(new IdentityRole(r)); - await _users.AddToRoleAsync(u, r); + { + var createRole = await _roles.CreateAsync(new IdentityRole(r)); + if (!createRole.Succeeded) return IdentityFailure(createRole); + } + + var addRole = await _users.AddToRoleAsync(u, r); + if (!addRole.Succeeded) return IdentityFailure(addRole); + } + + if (toRemove.Count > 0) + { + var removeRoles = await _users.RemoveFromRolesAsync(u, toRemove); + if (!removeRoles.Succeeded) return IdentityFailure(removeRoles); } return NoContent(); @@ -129,6 +153,16 @@ public sealed class UsersController : ControllerBase var u = await _users.FindByIdAsync(id); if (u is null) return NotFound(); + if (await _users.IsInRoleAsync(u, "Admin") + && (await _users.GetUsersInRoleAsync("Admin")).Count <= 1) + { + return Conflict(new ProblemDetails + { + Title = "Last administrator protected", + Detail = "Assign the Admin role to another user before deleting the final administrator." + }); + } + var res = await _users.DeleteAsync(u); if (!res.Succeeded) return BadRequest(string.Join("; ", res.Errors.Select(e => e.Description))); @@ -201,7 +235,7 @@ public sealed class UsersController : ControllerBase return NoContent(); } - private static UserDto ToDto(ApplicationUser user, List roles) + private static UserDto ToDto(ApplicationUser user, List roles, string? currentUserId, bool canRemoveAdmin) { return new UserDto( user.Id, @@ -213,7 +247,14 @@ public sealed class UsersController : ControllerBase user.EmailConfirmed, user.GoogleEmail, user.GoogleLinkedAt, - roles); + roles, + string.Equals(user.Id, currentUserId, StringComparison.Ordinal), + canRemoveAdmin); + } + + private BadRequestObjectResult IdentityFailure(IdentityResult result) + { + return BadRequest(string.Join("; ", result.Errors.Select(e => e.Description))); } private static string? TrimOrNull(string? value) diff --git a/docs/audits/verification-log.md b/docs/audits/verification-log.md index b20c2d7..2d48dc9 100644 --- a/docs/audits/verification-log.md +++ b/docs/audits/verification-log.md @@ -192,3 +192,4 @@ Output was reduced to filenames and commit counts. The token artifact appears un | V-158 | Phase 11 requirement, route, `JobTable`, `JobDetailsDialog`, `ApplicationWorkspacePage`, workspace architecture and test inventory | Repository root | Reproduce JOBS-002 list-context, deep-link and presentation behavior before editing | PASS — filters/page were component-local; `?open=` was consumed and removed; row Open launched the legacy quick dialog; workspace navigation left `/jobs`, and workspace Back always returned to a fresh `/jobs`. The full-page workspace already composes the owned domain sections and remains the safe reuse boundary | Source execution-path inspection; no browser behavior relabelled as tested | Confirmed product/navigation gap | | V-159 | Focused overlay Jest; full `npm test -- --runInBand --forceExit`; `npm run build`; `git diff --check` | `job-tracker-ui` / repository root | Verify first JOBS-002 route-backed embedded-workspace increment | PASS — focused 2/2, full frontend 51 suites and 206/206 tests, and optimized production build/TypeScript pass. Row Open creates `/jobs?workspace={id}`, direct URLs and section URLs render, close/Forward preserve history and in-memory search, the dialog has an accessible name/focus trap, mobile uses full screen, and the full-page route remains linked | JSDOM/mocked API only. Existing Jest force-exit notice remains. URL persistence of complete filter/page state, dirty-edit guards, table redesign and real browser/production checks remain | First cohesive increment verified | | V-160 | Focused overlay/workflow Jest; direct URL query hydration; full `npm test -- --runInBand --forceExit`; production build; diff review | `job-tracker-ui` | Verify JOBS-002 URL-owned list state without breaking workspace routes or workflow links | PASS — focused 2 suites and 6/6, full frontend 51 suites and 207/207, and optimized production build/TypeScript pass. Search survives overlay Back/Forward; direct URLs hydrate status/company/location/follow-up/readiness/deleted/sort/direction/page into the API request; company loading no longer produces a MUI out-of-range state | Existing Jest force-exit notice remains; browser refresh/history still to be exercised in Playwright | Local increment verified | +| V-161 | `UsersControllerTests`; focused theme/confirm/admin-users Jest; production frontend build; native-confirm search; `git diff --check` | Repository root / `job-tracker-ui` | Verify canonical theme persistence, semantic Alert contrast ownership, app-owned destructive dialogs and final-admin safety | PASS — backend 4/4; theme/confirm 8/8; admin UI 3/3; production build/TypeScript pass; no remaining `window.confirm` in frontend. Self-demotion cancel/confirm, other-admin warning, preserved roles and final-admin disabled/API conflict paths are covered | JSDOM/local mocks only; authenticated real-browser refresh and production remain | Repository safety increment verified | diff --git a/docs/work-programmes/decisions.md b/docs/work-programmes/decisions.md index a7723ed..4fee0a5 100644 --- a/docs/work-programmes/decisions.md +++ b/docs/work-programmes/decisions.md @@ -649,3 +649,23 @@ - **Consequences:** list state remains mounted while the overlay is open, direct workspace/section URLs and Back/Forward work, and a full-page link remains. Complete URL-backed filters and unsaved-edit guards are separate required increments before JOBS-002 can leave progress. - **User approval required:** No; this is approved repository implementation with mocked local data and no production/provider action. - **Reversible:** Revert the JOBS-002 overlay commit; no schema, dependency or stored-data change is involved. + +## DEC-066 — Make theme preference independent of authentication + +- **Date:** 2026-08-15 +- **Decision:** Supersede DEC-033's account-scoped storage portion with one canonical browser key, `jobtracker.themeMode`. Migrate the current legacy value once, apply the same resolution in the pre-paint script, and ignore auth-user changes for theme state. +- **Reason/evidence:** the user-visible preference is application chrome, while the auth-derived key is resolved asynchronously and can differ across startup paths. Tying these together retained competing sources of truth and allowed refresh to change scheme. Focused persistence/migration/provider tests and production TypeScript build pass. +- **Alternatives considered:** add route-specific theme effects; keep user/anonymous fallback ordering; let MUI own a second storage key. These preserve the race or reintroduce multiple owners. +- **Consequences:** theme remains stable through login/logout/navigation/refresh on a browser. Former account keys remain readable for one-time migration but are no longer written. +- **User approval required:** No; this corrects the requested persistence defect without schema, dependency or production change. +- **Reversible:** Remove the canonical read/write and restore auth-key subscriptions; legacy values were not deleted. + +## DEC-067 — Protect the final administrator at the API boundary + +- **Date:** 2026-08-15 +- **Decision:** Refuse demotion or deletion of the final Admin in `UsersController`; expose current-user and removal-safety state to the admin UI; require an app-owned destructive confirmation for any demotion and stronger copy for self-demotion/self-deletion. Preserve unrelated roles during an Admin toggle. +- **Reason/evidence:** confirmation alone cannot protect direct API calls or concurrent UI versions. Existing MUI confirm/prompt primitives already match the application and avoid a second dialog dependency. Four controller and three UI tests cover final-admin protection, other-admin demotion, self cancel and self confirm. +- **Alternatives considered:** SweetAlert2; silently forbid every self-demotion; UI-only warning. These duplicate the design system, prevent legitimate handover, or fail to enforce the invariant. +- **Consequences:** the final administrator cannot be removed by supported API paths. A self-demotion remains possible only when another administrator exists and the user explicitly confirms. +- **User approval required:** No; this is requested safety hardening with no production mutation. +- **Reversible:** Revert the controller/UI change; no stored data or schema changed. diff --git a/docs/work-programmes/master-progress.md b/docs/work-programmes/master-progress.md index 1bf0101..4ed6a55 100644 --- a/docs/work-programmes/master-progress.md +++ b/docs/work-programmes/master-progress.md @@ -1,9 +1,9 @@ # JobTracker master programme progress -Updated: 2026-08-10 +Updated: 2026-08-15 - **Overall programme status:** Active. Seven packages are locally verified; twenty-two packages through UX-003 are implemented with automated/runtime evidence but blocked from applicable live/provider/production gates; JOBS-002 is now in progress. Gitea run 609 passes the prior complete pull-request CI; DEP-001 awaits approved merge-to-main and production verification. -- **Current work package:** `JOBS-002` — applications table and embedded workspace (`IN PROGRESS`); route-backed overlay is pushed, and URL-owned filters/sort/page pass full regression/build. Commit/push, then dirty-edit protection are next. +- **Current work package:** `JOBS-002` — dedicated application workspace and scan-friendly applications table (`IN PROGRESS`). URL-owned list state is pushed. The requested theme/admin safety increment is locally verified; dedicated `/jobs/:id` navigation is next. - **Completed work packages:** None are `DONE`; all repository security/AI packages still have applicable browser, provider and/or production gates. - **Locally verified work:** SEC-001, SEC-002, SEC-003, SEC-005A, CORE-001, PROD-002 and DEP-001 (`VERIFIED LOCALLY`). - **Implemented, verification incomplete:** SEC-004, SEC-005B, SEC-008, CORE-002, BG-001, OPS-001A/B/C, POL-001/002, AI-001/002/003/004, UX-001/002/003, QA-001, CAREER-001/002, MAIL-001 and JOBS-001 (`IMPLEMENTED — NOT VERIFIED`). UX-003 safe local/browser scope is implemented; production/native-device gates remain. @@ -12,7 +12,7 @@ Updated: 2026-08-10 - **Deferred work:** None. Conditional multi-replica coordination, model deletion, realtime operation delivery and unrelated production changes remain outside current packages. - **Next five work packages:** JOBS-002 applications/workspace; PRODUCT-001 homepage/Pro claims; VER-001 action matrix; production-blocked SEC-006/007 when package-index permission is available; REL-001 after prerequisites. - **Status counts:** 7 `VERIFIED LOCALLY`; 22 `IMPLEMENTED — NOT VERIFIED`; 1 `IN PROGRESS`; 5 `NOT STARTED`; 5 `BLOCKED`; 0 `DONE`; 0 `DEFERRED`. -- **Test status:** backend 631/631; frontend 51/51 suites and 207/207 tests; JOBS-002 URL-state focused 2 suites and 6/6; Playwright 6/6; npm audit 0 vulnerabilities; production build passes. Historical JT-019 and Jest force-exit/open-handle behavior remain recorded. +- **Test status:** backend baseline 631/631 plus admin safety 4/4; frontend baseline 51/51 suites and 207/207 plus theme/confirm/admin focused 11/11; Playwright 6/6; npm audit 0 vulnerabilities; production build passes. Historical JT-019 and Jest force-exit/open-handle behavior remain recorded. - **Deployment status:** Gitea pull-request run 609 passed the complete CI job in 4m20s. Deploy was intentionally skipped because the workflow deploys only a `push` to `main`; live remains unchanged. No merge/deployment was performed directly, no production migrations were run and the AI operation worker remains disabled by default. - **Production status:** Unchanged and unverified. No provider/model call, model pull, external request or paid API occurred. - **Known regressions:** None found by automated/local browser checks. Jest still needs `--forceExit` and reports its existing open-handle notice. Email-provider/send tests are fake/local only; real delivery is not claimed. Current MAIL browser evidence is 1280×720 only because the browser surface could not resize or perform native Tab traversal. Interrupted attempts are aged after 15 minutes and notified without retry; the five-minute scan is unmeasured on a large ledger. Direct clean EF-only SQLite migration still hits the pre-existing historical blank-chain defect before later migrations; normal startup owns reconciliation. Cross-feature monthly AI usage accounting remains a rollout gap. diff --git a/docs/work-programmes/session-handoff.md b/docs/work-programmes/session-handoff.md index 7e5befe..996a61b 100644 --- a/docs/work-programmes/session-handoff.md +++ b/docs/work-programmes/session-handoff.md @@ -1,18 +1,18 @@ # JobTracker session handoff -Updated: 2026-08-10 +Updated: 2026-08-15 -- **Exact current task:** JOBS-002 — finish URL-owned list filters/sort/page, run full regression, commit/push, then trace and protect unsaved workspace edits. -- **Last completed step:** pushed `b67a531`; V-160 direct query hydration, full frontend 207/207 and production build pass without new console warnings. -- **Files currently modified:** `JobTable.tsx`, `application-workspace-overlay.test.tsx`, JOBS-002 verification/tracking documents. -- **Commands already run:** V-158/V-159 trace/focused/full/build; V-160 focused overlay/workflow, full frontend and production build/diff check. -- **Test results:** backend 631/631 unchanged; frontend 51 suites and 207/207; JOBS-002 URL-state focused 6/6; production build passes. Jest retains its known force-exit/open-handle notice. +- **Exact current task:** continue the 2026-08-15 application UX programme; next implement dedicated `/jobs/:id` workspace/list/sidebar/notification changes after the completed theme/admin safety increment. +- **Last completed step:** pushed `bd5362c`; then V-161 canonical theme, Alert contrast, app-owned CV confirmations and final-admin protection passed focused tests/build and awaits the next logical commit. +- **Files currently modified:** theme/bootstrap/tests, admin API/UI/tests/translations, CV dialog use, UX verification/tracking documents. +- **Commands already run:** V-160 focused/build and push; V-161 backend 4/4, theme/confirm 8/8, admin UI 3/3, production frontend build and patch/native-confirm review. +- **Test results:** backend baseline 631/631 plus focused admin 4/4; frontend baseline 207/207 plus focused V-161 11/11; production build passes. Jest retains its known force-exit/open-handle notice. - **Services currently running:** none on task-owned ports 3000/5202. Playwright stopped its disposable API/Next servers. Pre-existing Docker services were not changed. - **Temporary files or processes:** no task-owned process is running and the failed disposable migration database was removed. Existing synthetic browser evidence/account and startup-created local backup remain documented. No provider account, real email, private content, paid service or production service was accessed. - **Production changes currently active:** none. No deployment, migration, provider connection/sync/send or production payload occurred. - **Rollback status:** downgrade `20260810080858_AddEmailDraftClientRequestId`, then `20260810075206_AddEmailDrafts`, before reverting draft commits; then follow the existing MAIL rollback order (`ee5ef7e`, `449faeb`, `123fc55`/`e9937ac`, ledger downgrade before `653f011`). No production migration/deploy/provider grant occurred. -- **Uncommitted changes:** JOBS-002 URL-owned search/status/company/location/follow-up/readiness/deleted/sort/page state, tests and tracking; no dependency/schema/config change. +- **Uncommitted changes:** V-161 theme/admin/dialog safety and tracking; no dependency/schema/config change. - **Known failures:** live deployment is not verified because PR deploy is intentionally skipped and the active branch is not approved for merge. Draft export/API/UI, full thread/category actions and non-Gmail review remain; existing accounts need re-consent and IMAP stays read-only. A clean full-chain SQLite apply fails in the pre-existing JT-019 migration before the new draft migration. Browser/provider/MariaDB/production unavailable or unverified; recovery scan performance is unmeasured at large ledger scale; Jest open handles; SEC-006 parser dependency work is still separately gated; parser isolation remains SEC-007. -- **Exact next action:** review/stage/commit/push V-160; then inspect dirty state in cover letter/correspondence/AI section editors and add the smallest shared close/navigation guard. +- **Exact next action:** commit/push V-161; replace the route-backed job popup with the dedicated workspace while preserving working job actions and URL-owned list state. - **Work that can continue independently:** JOBS-002, PRODUCT-001 and VER-001. UX/JOBS production, MAIL provider mutations, SEC-006/007 and PROD packages retain their recorded external gates. - **Decisions still required from the user:** none for synthetic/code-inspected repository work. Any provider connection or send test, internet/package upgrades, private data, external/paid providers and production actions retain explicit approval/safety gates; SEC-009 retention/legal policy remains unresolved. diff --git a/job-tracker-ui/src/admin-users-page.test.tsx b/job-tracker-ui/src/admin-users-page.test.tsx new file mode 100644 index 0000000..3228c45 --- /dev/null +++ b/job-tracker-ui/src/admin-users-page.test.tsx @@ -0,0 +1,84 @@ +import React from "react"; +import "@testing-library/jest-dom"; +import { fireEvent, render, screen, waitFor, within } from "@testing-library/react"; +import { CssVarsProvider } from "@mui/material/styles"; + +import { api } from "./api"; +import { ConfirmProvider } from "./confirm"; +import { I18nProvider } from "./i18n/I18nProvider"; +import { PromptProvider } from "./prompt"; +import { getTheme } from "./theme"; +import { ToastProvider } from "./toast"; +import AdminUsersPage from "./views/AdminUsersPage"; + +jest.mock("./api", () => ({ + api: { get: jest.fn(), put: jest.fn(), post: jest.fn(), delete: jest.fn() }, + getApiErrorMessage: (_error: unknown, fallback: string) => fallback, +})); + +const mockedApi = api as jest.Mocked; + +function renderPage(users: unknown[]) { + mockedApi.get.mockResolvedValue({ data: users } as any); + mockedApi.put.mockResolvedValue({ data: null } as any); + render( + + + + + + + + + + + , + ); +} + +beforeEach(() => { + jest.clearAllMocks(); + Object.defineProperty(window, "matchMedia", { + configurable: true, + value: jest.fn().mockImplementation(() => ({ + matches: true, + addEventListener: jest.fn(), + removeEventListener: jest.fn(), + addListener: jest.fn(), + removeListener: jest.fn(), + })), + }); +}); + +test("requires explicit confirmation before removing your own admin role", async () => { + renderPage([{ id: "me", email: "me@example.com", userName: "me", roles: ["Admin"], emailConfirmed: true, isCurrentUser: true, canRemoveAdmin: true }]); + + fireEvent.click(await screen.findByRole("button", { name: "Remove admin" })); + expect(await screen.findByText(/you will immediately lose access/i)).toBeInTheDocument(); + + fireEvent.click(screen.getByRole("button", { name: "Cancel" })); + expect(mockedApi.put).not.toHaveBeenCalled(); + await waitFor(() => expect(screen.queryByRole("dialog")).not.toBeInTheDocument()); + + fireEvent.click(screen.getByRole("button", { name: "Remove admin" })); + fireEvent.click(within(await screen.findByRole("dialog")).getByRole("button", { name: "Remove admin" })); + + await waitFor(() => expect(mockedApi.put).toHaveBeenCalledWith("/users/me/roles", { roles: [] })); +}); + +test("warns before demoting another administrator", async () => { + renderPage([{ id: "other", email: "other@example.com", userName: "other", roles: ["Admin"], emailConfirmed: true, isCurrentUser: false, canRemoveAdmin: true }]); + + fireEvent.click(await screen.findByRole("button", { name: "Remove admin" })); + + expect(await screen.findByText(/they will immediately lose access/i)).toBeInTheDocument(); + fireEvent.click(screen.getByRole("button", { name: "Cancel" })); + expect(mockedApi.put).not.toHaveBeenCalled(); +}); + +test("disables demotion and deletion for the final administrator", async () => { + renderPage([{ id: "me", email: "me@example.com", userName: "me", roles: ["Admin"], emailConfirmed: true, isCurrentUser: true, canRemoveAdmin: false }]); + + expect(await screen.findByRole("button", { name: "Remove admin" })).toBeDisabled(); + expect(screen.getByRole("button", { name: "Delete" })).toBeDisabled(); +}); diff --git a/job-tracker-ui/src/i18n/translations.ts b/job-tracker-ui/src/i18n/translations.ts index f391a1d..63442c7 100644 --- a/job-tracker-ui/src/i18n/translations.ts +++ b/job-tracker-ui/src/i18n/translations.ts @@ -527,6 +527,16 @@ export const translations = { adminUsersAdminNo: "Admin: No", adminUsersDeleteConfirmBody: "Delete this user?", adminUsersDeleteConfirmNamed: "Delete user {name}?", + adminUsersYou: "You", + adminUsersMakeAdmin: "Make admin", + adminUsersRemoveAdmin: "Remove admin", + adminUsersDemoteTitle: "Remove administrator access", + adminUsersDemoteWarning: "Remove administrator access from {name}? They will immediately lose access to administration features.", + adminUsersSelfDemoteTitle: "Remove your own administrator access?", + adminUsersSelfDemoteWarning: "You will immediately lose access to user management and all other administration features. Another administrator must restore the role for you.", + adminUsersDeleteSelfWarning: "Delete your own account? You will be signed out and permanently lose access to this account and its data.", + adminUsersLastAdminHelp: "The final administrator cannot be demoted or deleted. Make another user an administrator first.", + adminUsersSafetyNotice: "The final administrator is protected. Removing your own administrator role requires explicit confirmation.", adminUsersPassword: "Password", kanbanHint: "Drag cards between columns to move a job forward. Use the card menu to set an exact stage.", kanbanDropHere: "Drop here", @@ -1658,6 +1668,16 @@ export const translations = { adminUsersAdminNo: "Admin: Nei", adminUsersDeleteConfirmBody: "Slette denne brukeren?", adminUsersDeleteConfirmNamed: "Slette bruker {name}?", + adminUsersYou: "Deg", + adminUsersMakeAdmin: "Gjør til admin", + adminUsersRemoveAdmin: "Fjern admin", + adminUsersDemoteTitle: "Fjern administratortilgang", + adminUsersDemoteWarning: "Fjerne administratortilgang fra {name}? Brukeren mister umiddelbart tilgang til administrasjonsfunksjoner.", + adminUsersSelfDemoteTitle: "Fjern din egen administratortilgang?", + adminUsersSelfDemoteWarning: "Du mister umiddelbart tilgang til brukeradministrasjon og alle andre administrasjonsfunksjoner. En annen administrator må gjenopprette rollen for deg.", + adminUsersDeleteSelfWarning: "Slette din egen konto? Du blir logget ut og mister permanent tilgang til kontoen og dataene.", + adminUsersLastAdminHelp: "Den siste administratoren kan ikke nedgraderes eller slettes. Gjør en annen bruker til administrator først.", + adminUsersSafetyNotice: "Den siste administratoren er beskyttet. Fjerning av din egen administratorrolle krever uttrykkelig bekreftelse.", adminUsersPassword: "Passord", kanbanHint: "Dra kort mellom kolonnene for å flytte en jobb videre. Bruk kortmenyen for å sette et eksakt trinn.", kanbanDropHere: "Slipp her", diff --git a/job-tracker-ui/src/views/AdminUsersPage.tsx b/job-tracker-ui/src/views/AdminUsersPage.tsx index 3a2b403..9c8f78d 100644 --- a/job-tracker-ui/src/views/AdminUsersPage.tsx +++ b/job-tracker-ui/src/views/AdminUsersPage.tsx @@ -1,6 +1,7 @@ import React, { useCallback, useEffect, useMemo, useState } from "react"; import { + Alert, Box, Button, Checkbox, @@ -9,6 +10,7 @@ import { Paper, Stack, TextField, + Tooltip, Typography, } from "@mui/material"; import useMediaQuery from "@mui/material/useMediaQuery"; @@ -25,6 +27,8 @@ type UserDto = { userName?: string | null; emailConfirmed: boolean; roles: string[]; + isCurrentUser: boolean; + canRemoveAdmin: boolean; }; export default function AdminUsersPage() { @@ -66,16 +70,33 @@ export default function AdminUsersPage() { raw: u, })), [users]); - const setAdminRole = useCallback(async (u: UserDto, isAdmin: boolean) => { + const setAdminRole = useCallback(async (u: UserDto, grantAdmin: boolean) => { + if (!grantAdmin) { + const name = u.userName || u.email || u.id; + const confirmed = await confirmAction( + u.isCurrentUser + ? t("adminUsersSelfDemoteWarning") + : t("adminUsersDemoteWarning", { name }), + { + title: u.isCurrentUser ? t("adminUsersSelfDemoteTitle") : t("adminUsersDemoteTitle"), + confirmLabel: t("adminUsersRemoveAdmin"), + destructive: true, + }, + ); + if (!confirmed) return; + } + try { - await api.put(`/users/${u.id}/roles`, { roles: isAdmin ? ["Admin"] : [] }); + const roles = grantAdmin + ? Array.from(new Set([...(u.roles || []), "Admin"])) + : (u.roles || []).filter((role) => role.toLowerCase() !== "admin"); + await api.put(`/users/${u.id}/roles`, { roles }); toast(t("adminUsersRolesUpdated"), "success"); await load(); - } catch (e: any) { - const msg = e?.response?.data || e?.message || t("adminUsersRolesUpdateFailed"); - toast(String(msg), "error"); + } catch (e) { + toast(getApiErrorMessage(e, t("adminUsersRolesUpdateFailed")), "error"); } - }, [t, toast]); + }, [confirmAction, t, toast]); const sendReset = useCallback(async (u: UserDto) => { try { @@ -88,18 +109,32 @@ export default function AdminUsersPage() { const remove = useCallback(async (u: UserDto) => { const name = u.userName || u.email || u.id; - if (!(await confirmAction(t("adminUsersDeleteConfirmNamed", { name }), { title: t("adminUsersDeleteConfirmTitle"), confirmLabel: t("adminUsersDelete"), destructive: true }))) return; + const message = u.isCurrentUser + ? t("adminUsersDeleteSelfWarning") + : t("adminUsersDeleteConfirmNamed", { name }); + if (!(await confirmAction(message, { title: t("adminUsersDeleteConfirmTitle"), confirmLabel: t("adminUsersDelete"), destructive: true }))) return; try { await api.delete(`/users/${u.id}`); toast(t("adminUsersDeleted"), "info"); await load(); - } catch { - toast(t("adminUsersDeleteFailed"), "error"); + } catch (e) { + toast(getApiErrorMessage(e, t("adminUsersDeleteFailed")), "error"); } }, [confirmAction, t, toast]); const columns = useMemo(() => [ - { field: "email", headerName: t("profileEmail"), flex: 1.2, minWidth: 220 }, + { + field: "email", + headerName: t("profileEmail"), + flex: 1.2, + minWidth: 220, + renderCell: (params) => ( + + {params.value} + {(params.row.raw as UserDto).isCurrentUser ? : null} + + ), + }, { field: "userName", headerName: t("profileUsername"), flex: 1, minWidth: 180 }, { field: "roles", @@ -136,15 +171,19 @@ export default function AdminUsersPage() { const isAdmin = (user.roles || []).includes("Admin"); return ( - + + + + + - + + + ); }, @@ -157,6 +196,7 @@ export default function AdminUsersPage() { {t("adminUsersTitle")} {t("adminUsersSubtitle")} + {t("adminUsersSafetyNotice")} {t("adminUsersCreateUser")} @@ -201,9 +241,10 @@ export default function AdminUsersPage() { - - {row.userName || row.email || row.id} - + + {row.userName || row.email || row.id} + {user.isCurrentUser ? : null} + {row.email || "—"} @@ -217,15 +258,15 @@ export default function AdminUsersPage() { - + + + - + + +