fix(admin): protect administrator access
Reject final-admin demotion and deletion at the API boundary. Require explicit confirmation before any administrator role removal.
This commit is contained in:
@@ -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<ConflictObjectResult>(result);
|
||||
Assert.Equal("Last administrator protected", Assert.IsType<ProblemDetails>(conflict.Value).Title);
|
||||
users.Verify(x => x.RemoveFromRolesAsync(It.IsAny<ApplicationUser>(), It.IsAny<IEnumerable<string>>()), 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<IEnumerable<string>>(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<NoContentResult>(result);
|
||||
users.Verify(x => x.RemoveFromRolesAsync(admin, It.IsAny<IEnumerable<string>>()), 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<ConflictObjectResult>(result);
|
||||
users.Verify(x => x.DeleteAsync(It.IsAny<ApplicationUser>()), 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<ApplicationUser>([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<List<UsersController.UserDto>>(Assert.IsType<OkObjectResult>(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<UserManager<ApplicationUser>> users, string currentUserId)
|
||||
{
|
||||
var roleStore = new Mock<IRoleStore<IdentityRole>>();
|
||||
var roles = new Mock<RoleManager<IdentityRole>>(
|
||||
roleStore.Object,
|
||||
Array.Empty<IRoleValidator<IdentityRole>>(),
|
||||
new UpperInvariantLookupNormalizer(),
|
||||
new IdentityErrorDescriber(),
|
||||
new NullLogger<RoleManager<IdentityRole>>());
|
||||
|
||||
var controller = new UsersController(
|
||||
users.Object,
|
||||
roles.Object,
|
||||
Mock.Of<IAppEmailSender>(),
|
||||
new ConfigurationBuilder().Build(),
|
||||
new NullLogger<UsersController>(),
|
||||
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<TEntity>(IQueryProvider inner) : IAsyncQueryProvider
|
||||
{
|
||||
public IQueryable CreateQuery(Expression expression) => new TestAsyncEnumerable<TEntity>(expression);
|
||||
public IQueryable<TElement> CreateQuery<TElement>(Expression expression) => new TestAsyncEnumerable<TElement>(expression);
|
||||
public object? Execute(Expression expression) => inner.Execute(expression);
|
||||
public TResult Execute<TResult>(Expression expression) => inner.Execute<TResult>(expression);
|
||||
public TResult ExecuteAsync<TResult>(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<T> : EnumerableQuery<T>, IAsyncEnumerable<T>, IQueryable<T>
|
||||
{
|
||||
public TestAsyncEnumerable(IEnumerable<T> enumerable) : base(enumerable) { }
|
||||
public TestAsyncEnumerable(Expression expression) : base(expression) { }
|
||||
|
||||
public IAsyncEnumerator<T> GetAsyncEnumerator(CancellationToken cancellationToken = default)
|
||||
=> new TestAsyncEnumerator<T>(this.AsEnumerable().GetEnumerator());
|
||||
|
||||
IQueryProvider IQueryable.Provider => new TestAsyncQueryProvider<T>(this);
|
||||
}
|
||||
|
||||
private sealed class TestAsyncEnumerator<T>(IEnumerator<T> inner) : IAsyncEnumerator<T>
|
||||
{
|
||||
public T Current => inner.Current;
|
||||
public ValueTask DisposeAsync()
|
||||
{
|
||||
inner.Dispose();
|
||||
return ValueTask.CompletedTask;
|
||||
}
|
||||
|
||||
public ValueTask<bool> MoveNextAsync() => ValueTask.FromResult(inner.MoveNext());
|
||||
}
|
||||
}
|
||||
@@ -38,7 +38,9 @@ public sealed class UsersController : ControllerBase
|
||||
bool EmailConfirmed,
|
||||
string? GoogleEmail,
|
||||
DateTimeOffset? GoogleLinkedAt,
|
||||
List<string> Roles);
|
||||
List<string> Roles,
|
||||
bool IsCurrentUser,
|
||||
bool CanRemoveAdmin);
|
||||
|
||||
[HttpGet]
|
||||
public async Task<ActionResult<List<UserDto>>> 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<UserDto>(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<string> roles)
|
||||
private static UserDto ToDto(ApplicationUser user, List<string> 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)
|
||||
|
||||
@@ -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 |
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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<typeof api>;
|
||||
|
||||
function renderPage(users: unknown[]) {
|
||||
mockedApi.get.mockResolvedValue({ data: users } as any);
|
||||
mockedApi.put.mockResolvedValue({ data: null } as any);
|
||||
render(
|
||||
<CssVarsProvider theme={getTheme("light") as any} defaultMode="light">
|
||||
<I18nProvider>
|
||||
<ToastProvider>
|
||||
<ConfirmProvider>
|
||||
<PromptProvider>
|
||||
<AdminUsersPage />
|
||||
</PromptProvider>
|
||||
</ConfirmProvider>
|
||||
</ToastProvider>
|
||||
</I18nProvider>
|
||||
</CssVarsProvider>,
|
||||
);
|
||||
}
|
||||
|
||||
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();
|
||||
});
|
||||
@@ -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",
|
||||
|
||||
@@ -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<GridColDef[]>(() => [
|
||||
{ field: "email", headerName: t("profileEmail"), flex: 1.2, minWidth: 220 },
|
||||
{
|
||||
field: "email",
|
||||
headerName: t("profileEmail"),
|
||||
flex: 1.2,
|
||||
minWidth: 220,
|
||||
renderCell: (params) => (
|
||||
<Box sx={{ display: "flex", alignItems: "center", gap: 0.75, minWidth: 0 }}>
|
||||
<Typography variant="body2" noWrap>{params.value}</Typography>
|
||||
{(params.row.raw as UserDto).isCurrentUser ? <Chip size="small" label={t("adminUsersYou")} color="primary" variant="outlined" /> : null}
|
||||
</Box>
|
||||
),
|
||||
},
|
||||
{ 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 (
|
||||
<Box sx={{ display: "flex", gap: 1, flexWrap: "wrap", py: 0.5 }}>
|
||||
<Button size="small" variant={isAdmin ? "contained" : "outlined"} onClick={() => void setAdminRole(user, !isAdmin)}>
|
||||
{t("adminUsersAdmin")}
|
||||
</Button>
|
||||
<Tooltip title={isAdmin && !user.canRemoveAdmin ? t("adminUsersLastAdminHelp") : ""}>
|
||||
<span>
|
||||
<Button size="small" variant={isAdmin ? "contained" : "outlined"} disabled={isAdmin && !user.canRemoveAdmin} onClick={() => void setAdminRole(user, !isAdmin)}>
|
||||
{isAdmin ? t("adminUsersRemoveAdmin") : t("adminUsersMakeAdmin")}
|
||||
</Button>
|
||||
</span>
|
||||
</Tooltip>
|
||||
<Button size="small" variant="outlined" onClick={() => void sendReset(user)}>
|
||||
{t("adminUsersSendReset")}
|
||||
</Button>
|
||||
<Button size="small" color="error" variant="outlined" onClick={() => void remove(user)}>
|
||||
{t("adminUsersDelete")}
|
||||
</Button>
|
||||
<Tooltip title={isAdmin && !user.canRemoveAdmin ? t("adminUsersLastAdminHelp") : ""}>
|
||||
<span><Button size="small" color="error" variant="outlined" disabled={isAdmin && !user.canRemoveAdmin} onClick={() => void remove(user)}>{t("adminUsersDelete")}</Button></span>
|
||||
</Tooltip>
|
||||
</Box>
|
||||
);
|
||||
},
|
||||
@@ -157,6 +196,7 @@ export default function AdminUsersPage() {
|
||||
{t("adminUsersTitle")}
|
||||
</Typography>
|
||||
<Typography sx={{ color: "text.secondary", mb: 2 }}>{t("adminUsersSubtitle")}</Typography>
|
||||
<Alert severity="info" sx={{ mb: 2 }}>{t("adminUsersSafetyNotice")}</Alert>
|
||||
|
||||
<Paper sx={{ p: { xs: 1.5, sm: 2 }, mb: 2 }}>
|
||||
<Typography sx={{ fontWeight: 900, mb: 1 }}>{t("adminUsersCreateUser")}</Typography>
|
||||
@@ -201,9 +241,10 @@ export default function AdminUsersPage() {
|
||||
<Paper key={row.id} sx={{ p: 1.5, borderRadius: 3 }}>
|
||||
<Stack spacing={1.25}>
|
||||
<Box>
|
||||
<Typography sx={{ fontWeight: 900, overflowWrap: "anywhere" }}>
|
||||
{row.userName || row.email || row.id}
|
||||
</Typography>
|
||||
<Box sx={{ display: "flex", alignItems: "center", gap: 1, flexWrap: "wrap" }}>
|
||||
<Typography component="span" sx={{ fontWeight: 900, overflowWrap: "anywhere" }}>{row.userName || row.email || row.id}</Typography>
|
||||
{user.isCurrentUser ? <Chip size="small" label={t("adminUsersYou")} color="primary" variant="outlined" /> : null}
|
||||
</Box>
|
||||
<Typography variant="body2" sx={{ color: "text.secondary", overflowWrap: "anywhere" }}>
|
||||
{row.email || "—"}
|
||||
</Typography>
|
||||
@@ -217,15 +258,15 @@ export default function AdminUsersPage() {
|
||||
</Box>
|
||||
|
||||
<Stack spacing={1}>
|
||||
<Button variant={isAdmin ? "contained" : "outlined"} onClick={() => void setAdminRole(user, !isAdmin)} fullWidth>
|
||||
{t("adminUsersAdmin")}
|
||||
</Button>
|
||||
<Tooltip title={isAdmin && !user.canRemoveAdmin ? t("adminUsersLastAdminHelp") : ""}>
|
||||
<span><Button variant={isAdmin ? "contained" : "outlined"} disabled={isAdmin && !user.canRemoveAdmin} onClick={() => void setAdminRole(user, !isAdmin)} fullWidth>{isAdmin ? t("adminUsersRemoveAdmin") : t("adminUsersMakeAdmin")}</Button></span>
|
||||
</Tooltip>
|
||||
<Button variant="outlined" onClick={() => void sendReset(user)} fullWidth>
|
||||
{t("adminUsersSendReset")}
|
||||
</Button>
|
||||
<Button color="error" variant="outlined" onClick={() => void remove(user)} fullWidth>
|
||||
{t("adminUsersDelete")}
|
||||
</Button>
|
||||
<Tooltip title={isAdmin && !user.canRemoveAdmin ? t("adminUsersLastAdminHelp") : ""}>
|
||||
<span><Button color="error" variant="outlined" disabled={isAdmin && !user.canRemoveAdmin} onClick={() => void remove(user)} fullWidth>{t("adminUsersDelete")}</Button></span>
|
||||
</Tooltip>
|
||||
</Stack>
|
||||
</Stack>
|
||||
</Paper>
|
||||
|
||||
Reference in New Issue
Block a user