feat: Admin Area Players Management 1.00
This commit is contained in:
@@ -1,50 +0,0 @@
|
||||
# Implementation Plan: Hardened User Deletion Cleanup + Concurrency-Safe Last-Admin Lock
|
||||
|
||||
## 1. Architectural Reconnaissance
|
||||
- **Codebase style & conventions:** TypeScript NestJS 10 backend over `better-sqlite3` via TypeORM 0.3. All admin mutations live in `AdminService` (`backend/src/modules/admin/admin.service.ts:109-119`) and call a shared `UsersService.applyLastAdminSafeMutation` wrapper (`backend/src/modules/users/users.service.ts:63-95`). The wrapper currently uses `dataSource.transaction('SERIALIZABLE', ...)` which on `better-sqlite3` still emits a plain `BEGIN TRANSACTION` (deferred) and only escalates the SQLite write lock at the first write. Two concurrent transactions can therefore both pass the precondition `count > 1` before either commits, racing the invariant.
|
||||
- **Data Layer:** `user`, `solve`, and `refresh_token` are the only tables with a `user_id` column. `blog_post` has no `author_id`, and awards are persisted as rows in `solve` itself (`points_awarded`, `is_first/second/third`). The new `AddUserDeleteCascades1700000000700` migration already adds `ON DELETE CASCADE` on `solve.user_id` and `refresh_token.user_id`, so a plain `manager.delete(UserEntity, ...)` would cascade in practice — but the user request asks us to perform the dependent cleanup explicitly inside the same transaction so the operation is observable to business code, logging, and the post-condition count.
|
||||
- **Test Framework & Structure:** Jest 29 multi-project config (`tests/jest.config.js`); existing `tests/backend/last-admin-invariant.spec.ts` and `tests/backend/admin-players.spec.ts` already exercise the safe-mutation wrapper and the Players endpoints with an in-memory SQLite database. All new tests will be appended to those files (no new top-level test files required) and run via `npm run test:backend`.
|
||||
- **Required Tools & Dependencies:** No new package or global CLI is required. The plan uses the existing TypeORM `QueryRunner`, the in-repo `ApiError` / `ERROR_CODES` envelope, and SQLite's native `BEGIN IMMEDIATE` semantics. If a future deployment moves to Postgres/MySQL, the locking strategy below degrades gracefully: `setLock('pessimistic_write')` is a no-op for read paths and a real row lock on databases that support it, while `BEGIN IMMEDIATE` is harmless outside SQLite.
|
||||
|
||||
## 2. Impacted Files
|
||||
- **To Modify:**
|
||||
- `backend/src/modules/admin/admin.service.ts` — perform explicit `solve`, `refresh_token`, and any other user-owned table deletion inside the same transaction as the `user` row delete; report per-table affected counts.
|
||||
- `backend/src/modules/users/users.service.ts` — replace the read/count-only SERIALIZABLE wrapper with a `QueryRunner`-driven `BEGIN IMMEDIATE` transaction that performs row-level pessimistic locking where supported, retries on `SQLITE_BUSY`/serialization failures, and rolls back with `409 LAST_ADMIN` if the post-mutation admin count is zero.
|
||||
- `backend/src/common/errors/error-codes.ts` — add a stable `CONFLICT_RETRY`/`CONFLICT_BUSY` code (only if the retry path needs to surface a distinct code; otherwise reuse `LAST_ADMIN` and rely on logs).
|
||||
- `tests/backend/last-admin-invariant.spec.ts` — extend with explicit dependent-cleanup and `SQLITE_BUSY` retry coverage; add a concurrent `Promise.all` assertion that at most one writer can drive the admin count to zero.
|
||||
- `tests/backend/admin-players.spec.ts` — add a delete-cascade assertion (no orphan `solve`/`refresh_token` rows after deletion) and a `LAST_ADMIN` rejection when the sole remaining admin is deleted.
|
||||
- **To Create:** None. The dependent table set is already known (solve + refresh_token) and the existing migration already enforces ON DELETE CASCADE; the request explicitly asks us to delete the rows imperatively in the same transaction rather than rely solely on the cascade, so no new migration is required.
|
||||
|
||||
## 3. Proposed Changes
|
||||
1. **Backend Logic & APIs — Dependent cleanup on delete (admin.service.ts:109-119):**
|
||||
- Inside the existing `applyLastAdminSafeMutation` callback (which already supplies a TypeORM `EntityManager`), issue explicit deletions in this order:
|
||||
1. `manager.delete(SolveEntity, { userId: targetId })` (capture `affected`).
|
||||
2. `manager.delete(RefreshTokenEntity, { userId: targetId })` (capture `affected`).
|
||||
3. `manager.delete(UserEntity, { id: targetId })` — this remains the actual user delete and is the only one that returns 404 when the row is missing; dependent tables short-circuit harmlessly when there are no rows.
|
||||
- Sum the affected counts and return them from the mutate closure so the `applyLastAdminSafeMutation` can log them at `info` level (NEVER include `passwordHash`, `token_hash`, or the plaintext password). The cascade is retained as a safety net; the imperative delete is what the request mandates.
|
||||
- Treat any user-owned table that is added in the future as opt-in: introduce a small registry (`USER_OWNED_TABLES = [SolveEntity, RefreshTokenEntity]`) iterated by the delete path so we don't have to add new call sites each time a new `userId` column appears. The plan keeps this registry co-located with the service; no new file.
|
||||
2. **Backend Logic & APIs — Concurrency-safe last-admin wrapper (users.service.ts:63-95):**
|
||||
- Replace the `dataSource.transaction('SERIALIZABLE', ...)` call with a `dataSource.createQueryRunner()`-driven flow:
|
||||
1. `await queryRunner.connect()` then `await queryRunner.query('BEGIN IMMEDIATE')` so the very first statement acquires the SQLite RESERVED lock, preventing a second writer from racing through the precondition.
|
||||
2. Acquire a row-level lock on the target user via `repo.createQueryBuilder('u').setLock('pessimistic_write').where('u.id = :id', { id: targetId }).getOne()`. This is a no-op for SQLite (the database-level lock is what matters) but a real `SELECT ... FOR UPDATE` on Postgres/MySQL if the deployment is ever migrated.
|
||||
3. Acquire a second `pessimistic_write` lock on the full set of admin rows by selecting all `user.id` rows where `role = 'admin'` with `setLock('pessimistic_write')` (a no-op for SQLite; equivalent to a per-row `FOR UPDATE` on databases that support it). The aim is to serialize all concurrent admin mutations across the cluster.
|
||||
4. Count admins under the lock, fail fast with `409 LAST_ADMIN` if `count <= 1` and the operation is destructive, otherwise apply the mutation, re-count under the lock, and roll back with `409 LAST_ADMIN` if the post-condition is `count <= 0`.
|
||||
5. Commit; in the `catch`, attempt `ROLLBACK` and rethrow. Translate `SQLITE_BUSY` / serialization failures (error code `SQLITE_BUSY` from `better-sqlite3` and the TypeORM `LockNotSupportedError`/`TransactionAlreadyStartedError` chain) into a small retry loop (`maxRetries = 5`, exponential backoff 10ms / 20ms / 40ms / 80ms / 160ms). If retries are exhausted and the last failure still has the system in an indeterminate state, throw `ApiError.conflict(ERROR_CODES.LAST_ADMIN, ...)` so the caller always sees the existing 409 contract.
|
||||
- Add a database-level safety net: a forward-only migration that adds a partial unique index on the `user` table. The current `user` schema has no `is_last_admin` column to enforce a CHECK on, so the simplest correct approach is to add a server-side trigger that raises on `UPDATE`/`DELETE` that would reduce the `admin` count to zero. SQL: `CREATE TRIGGER trg_user_last_admin BEFORE UPDATE OF role ON user WHEN NOT EXISTS (SELECT 1 FROM user WHERE role = 'admin' AND id <> NEW.id) AND OLD.role = 'admin' AND NEW.role <> 'admin' BEGIN SELECT RAISE(ABORT, 'LAST_ADMIN'); END;` plus a sibling `BEFORE DELETE` trigger with the same condition. The trigger is the deployment-wide safety net so even a buggy caller that bypasses the service wrapper cannot commit a zero-admin state. The service still rolls back to 409 in normal operation; the trigger ensures the database itself rejects the offending statement if it ever does slip through (e.g. raw SQL or a future parallel module that forgets the wrapper).
|
||||
3. **Backend Logic & APIs — Error code & messaging:**
|
||||
- The existing `ApiError.conflict(ERROR_CODES.LAST_ADMIN, message)` shape is preserved for every public path. Add a one-line clarifying message in the wrapper: `'Cannot delete or demote the last admin; the system must always retain at least one enabled user with role "admin".'`. Plaintext, hashes, and tokens are never included.
|
||||
4. **Frontend Integration:**
|
||||
- No frontend code change is required; the LAST_ADMIN toast, modal handling, and row updates already map the existing 409 response. The plan only strengthens the server contract; the existing tests in `tests/frontend/admin-players.pure.spec.ts` and `tests/frontend/change-password-modal.spec.ts` already assert the LAST_ADMIN error shape, so no new frontend test is required.
|
||||
5. **Test Strategy (covered in `tests/backend/last-admin-invariant.spec.ts` and `tests/backend/admin-players.spec.ts`):**
|
||||
- Append to `applyLastAdminSafeMutation` describe block:
|
||||
- **`demote is rejected with 409 LAST_ADMIN when adminCount would reach zero after recount`** — seed two admins, then in a single transaction demote one to a player role while a second concurrent transaction simultaneously deletes the other; assert at most one of the two requests succeeds and the other receives `409 LAST_ADMIN` (use `Promise.all` with two `applyLastAdminSafeMutation` calls).
|
||||
- **`delete is rejected with 409 LAST_ADMIN when adminCount would reach zero after recount`** — same pattern but with delete instead of demote.
|
||||
- **`concurrent role-toggle + delete cannot reduce admin count below 1`** — exactly the multi-instance invariant the user requested; run two `applyLastAdminSafeMutation` calls in parallel (one role-toggle, one delete) against a two-admin seed and assert final admin count is `1` and at least one response is `409 LAST_ADMIN`.
|
||||
- Append to admin-players describe block:
|
||||
- **`deletePlayer cascades/cleans dependent solve + refresh_token rows`** — create an admin, create a player, manually insert a `solve` row tied to the player, perform `DELETE /api/v1/admin/players/:id`, then assert the `solve` and `refresh_token` tables no longer contain rows for the deleted user.
|
||||
- **`sole-admin delete returns 409 LAST_ADMIN with no row removal`** — assert the `user` row count is unchanged after the 409.
|
||||
- Use the existing in-memory better-sqlite3 + Nest `Test.createTestingModule` setup; no new infrastructure. The retry path is exercised by directly invoking the service and stubbing the inner `dataSource.createQueryRunner()` to throw `SQLITE_BUSY` once before succeeding (use a tiny `jest.spyOn` on the DataSource) — kept narrow so the test stays fast and deterministic.
|
||||
|
||||
## 4. Test Strategy
|
||||
- **Target Unit Test File:** `tests/backend/last-admin-invariant.spec.ts` (service-level) and `tests/backend/admin-players.spec.ts` (HTTP-level).
|
||||
- **Mocking Strategy:** Real in-memory SQLite + real Nest application, no DB mocks. The retry path uses a one-line `jest.spyOn(dataSource, 'createQueryRunner')` that throws a `SQLITE_BUSY`-shaped `Error` on the first call and returns the real runner on the second; this keeps the assertion deterministic without simulating the SQLite lock manager. No new test helpers and no shared state — each test clears `user`, `solve`, and `refresh_token` rows in `beforeEach` to remain isolated.
|
||||
@@ -0,0 +1,70 @@
|
||||
# Implementation Plan: Fix Job 913 Build Error (missing `auth` injection in AdminUsersComponent)
|
||||
|
||||
## 1. Root cause
|
||||
The `onToggleRole` handler added in the previous turn calls
|
||||
`this.auth.currentUser()`, `this.auth.applyPeerRoleChange(...)`, and
|
||||
`this.auth.publishRoleChange(...)`
|
||||
(`frontend/src/app/features/admin/admin-users.component.ts:244-247`), but
|
||||
`AdminUsersComponent` never declared an `auth` field. The component only injects
|
||||
`AdminService` (`admin`) and `NotificationService` (`notify`). TypeScript build
|
||||
(`ng build --configuration production`) therefore fails with `TS2339: Property
|
||||
'auth' does not exist on type 'AdminUsersComponent'` at lines 244, 246, 247.
|
||||
|
||||
The Jest tests passed because they exercise pure modules and
|
||||
`admin-players.pure`, not the component class — so the missing field was not
|
||||
caught at test time.
|
||||
|
||||
## 2. Required changes (single file)
|
||||
|
||||
**File:** `frontend/src/app/features/admin/admin-users.component.ts`
|
||||
|
||||
1. **Add the import** alongside the existing core-service imports (lines 10-11
|
||||
currently import `AdminService` and `NotificationService`):
|
||||
|
||||
```ts
|
||||
import { AuthService, AdminUser } from '...';
|
||||
```
|
||||
|
||||
The `AdminUser` type can be left on the existing `AdminService` import line. We
|
||||
must add `AuthService` and import only the value (not the type) — `AuthService`
|
||||
is the singleton provided in `'root'` by `core/services/auth.service.ts`.
|
||||
|
||||
The accurate edit is:
|
||||
|
||||
- Replace the existing `import { AdminService, AdminUser } from '...admin.service';`
|
||||
with two separate imports, or add the new import:
|
||||
|
||||
```ts
|
||||
import { AdminService, AdminUser } from '../../core/services/admin.service';
|
||||
import { AuthService } from '../../core/services/auth.service';
|
||||
```
|
||||
|
||||
2. **Inject the service** alongside the two existing injections (lines ~170-171):
|
||||
|
||||
```ts
|
||||
private readonly admin = inject(AdminService);
|
||||
private readonly notify = inject(NotificationService);
|
||||
private readonly auth = inject(AuthService);
|
||||
```
|
||||
|
||||
That is the only change required — the existing `onToggleRole` body already
|
||||
references `this.auth.currentUser`, `this.auth.applyPeerRoleChange`, and
|
||||
`this.auth.publishRoleChange`, all of which exist on `AuthService`.
|
||||
|
||||
## 3. Verification
|
||||
|
||||
- `npm --workspace frontend run build` should pass (no TypeScript errors).
|
||||
- `npm test` should still pass — the existing pure tests
|
||||
(`tests/frontend/admin-players.pure.spec.ts`,
|
||||
`tests/frontend/auth-session-events.spec.ts`,
|
||||
`tests/backend/admin-players.spec.ts`) are unaffected by adding the
|
||||
injection.
|
||||
- No public API, route, store, or service signature changes are required.
|
||||
|
||||
## 4. Out of scope
|
||||
|
||||
- No backend changes.
|
||||
- No test additions (the build error is a pure TypeScript declaration
|
||||
issue; tests already cover the behaviour).
|
||||
- No re-architecture of cross-tab role-change handling — the previous turn's
|
||||
implementation is correct, only the missing field declaration is needed.
|
||||
@@ -2,12 +2,24 @@ export const CROSS_TAB_CHANNEL_NAME = 'hipctf.auth.v1';
|
||||
export const CROSS_TAB_STORAGE_KEY = 'hipctf.auth.invalidate.v1';
|
||||
export const CROSS_TAB_EVENT_TYPE = 'hipctf.session.invalidated';
|
||||
|
||||
export const CROSS_TAB_ROLE_CHANNEL_NAME = 'hipctf.auth.role.v1';
|
||||
export const CROSS_TAB_ROLE_STORAGE_KEY = 'hipctf.role.invalidate.v1';
|
||||
export const CROSS_TAB_ROLE_EVENT_TYPE = 'hipctf.session.role-changed';
|
||||
|
||||
export interface CrossTabInvalidationMessage {
|
||||
type: 'session-invalidated';
|
||||
origin: string;
|
||||
ts: number;
|
||||
}
|
||||
|
||||
export interface CrossTabRoleChangeMessage {
|
||||
type: 'role-changed';
|
||||
origin: string;
|
||||
ts: number;
|
||||
userId: string;
|
||||
newRole: 'admin' | 'player';
|
||||
}
|
||||
|
||||
export function encodeInvalidationMessage(origin: string): CrossTabInvalidationMessage {
|
||||
return {
|
||||
type: 'session-invalidated',
|
||||
@@ -16,6 +28,20 @@ export function encodeInvalidationMessage(origin: string): CrossTabInvalidationM
|
||||
};
|
||||
}
|
||||
|
||||
export function encodeRoleChangeMessage(
|
||||
origin: string,
|
||||
userId: string,
|
||||
newRole: 'admin' | 'player',
|
||||
): CrossTabRoleChangeMessage {
|
||||
return {
|
||||
type: 'role-changed',
|
||||
origin,
|
||||
ts: Date.now(),
|
||||
userId,
|
||||
newRole,
|
||||
};
|
||||
}
|
||||
|
||||
export function isCrossTabInvalidationMessage(
|
||||
value: unknown,
|
||||
): value is CrossTabInvalidationMessage {
|
||||
@@ -28,6 +54,20 @@ export function isCrossTabInvalidationMessage(
|
||||
);
|
||||
}
|
||||
|
||||
export function isCrossTabRoleChangeMessage(
|
||||
value: unknown,
|
||||
): value is CrossTabRoleChangeMessage {
|
||||
if (!value || typeof value !== 'object') return false;
|
||||
const v = value as Record<string, unknown>;
|
||||
return (
|
||||
v['type'] === 'role-changed' &&
|
||||
typeof v['origin'] === 'string' &&
|
||||
typeof v['ts'] === 'number' &&
|
||||
typeof v['userId'] === 'string' &&
|
||||
(v['newRole'] === 'admin' || v['newRole'] === 'player')
|
||||
);
|
||||
}
|
||||
|
||||
export function isStorageInvalidationEvent(
|
||||
event: { key?: string | null; newValue?: string | null } | null | undefined,
|
||||
): boolean {
|
||||
@@ -41,3 +81,26 @@ export function isStorageInvalidationEvent(
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
export function isStorageRoleChangeEvent(
|
||||
event: { key?: string | null; newValue?: string | null } | null | undefined,
|
||||
): boolean {
|
||||
if (!event) return false;
|
||||
if (event.key !== CROSS_TAB_ROLE_STORAGE_KEY) return false;
|
||||
if (event.newValue == null) return false;
|
||||
try {
|
||||
const parsed = JSON.parse(event.newValue) as unknown;
|
||||
return isCrossTabRoleChangeMessage(parsed);
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
export function applyRoleChangeToUser<T extends { id: string; role: 'admin' | 'player' } | null>(
|
||||
user: T,
|
||||
newRole: 'admin' | 'player',
|
||||
): T {
|
||||
if (!user) return user;
|
||||
if (user.role === newRole) return user;
|
||||
return { ...user, role: newRole };
|
||||
}
|
||||
|
||||
@@ -10,10 +10,16 @@ import {
|
||||
import {
|
||||
CROSS_TAB_CHANNEL_NAME,
|
||||
CROSS_TAB_STORAGE_KEY,
|
||||
CROSS_TAB_ROLE_CHANNEL_NAME,
|
||||
CROSS_TAB_ROLE_STORAGE_KEY,
|
||||
CrossTabInvalidationMessage,
|
||||
CrossTabRoleChangeMessage,
|
||||
encodeInvalidationMessage,
|
||||
encodeRoleChangeMessage,
|
||||
isCrossTabInvalidationMessage,
|
||||
isCrossTabRoleChangeMessage,
|
||||
isStorageInvalidationEvent,
|
||||
isStorageRoleChangeEvent,
|
||||
} from './auth-session-events.pure';
|
||||
|
||||
export interface CurrentUser {
|
||||
@@ -93,10 +99,12 @@ export class AuthService {
|
||||
? crypto.randomUUID()
|
||||
: `tab-${Math.random().toString(36).slice(2)}-${Date.now()}`;
|
||||
private channel: BroadcastChannel | null = null;
|
||||
private roleChannel: BroadcastChannel | null = null;
|
||||
private storageListener: ((ev: StorageEvent) => void) | null = null;
|
||||
private suppressBroadcast = false;
|
||||
|
||||
private peerInvalidationListeners = new Set<(reason: 'peer-logout' | 'sse-unauthorized') => void>();
|
||||
private peerRoleChangeListeners = new Set<(msg: CrossTabRoleChangeMessage) => void>();
|
||||
|
||||
constructor(http?: HttpClient) {
|
||||
this.http = http ?? inject(HttpClient);
|
||||
@@ -242,6 +250,44 @@ export class AuthService {
|
||||
return () => this.peerInvalidationListeners.delete(cb);
|
||||
}
|
||||
|
||||
onPeerRoleChange(cb: (msg: CrossTabRoleChangeMessage) => void): () => void {
|
||||
this.peerRoleChangeListeners.add(cb);
|
||||
return () => this.peerRoleChangeListeners.delete(cb);
|
||||
}
|
||||
|
||||
applyPeerRoleChange(newRole: 'admin' | 'player'): void {
|
||||
const current = this.user();
|
||||
if (!current) return;
|
||||
if (current.role === newRole) return;
|
||||
const updated = { ...current, role: newRole };
|
||||
this.user.set(updated);
|
||||
if (typeof window !== 'undefined') {
|
||||
try {
|
||||
writeStoredSession(this.sessionStorage(), { token: this.accessToken() ?? '', user: updated });
|
||||
} catch {
|
||||
// ignore storage failures
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
publishRoleChange(userId: string, newRole: 'admin' | 'player'): void {
|
||||
const message = encodeRoleChangeMessage(this.crossTabOrigin, userId, newRole);
|
||||
if (this.roleChannel) {
|
||||
try {
|
||||
this.roleChannel.postMessage(message);
|
||||
} catch {
|
||||
// ignore serialization failures; storage fallback still fires
|
||||
}
|
||||
}
|
||||
if (typeof window !== 'undefined' && window.localStorage) {
|
||||
try {
|
||||
window.localStorage.setItem(CROSS_TAB_ROLE_STORAGE_KEY, JSON.stringify(message));
|
||||
} catch {
|
||||
// ignore quota / serialization errors
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
getAccessToken(): string | null {
|
||||
return this.accessToken();
|
||||
}
|
||||
@@ -272,10 +318,21 @@ export class AuthService {
|
||||
} catch {
|
||||
this.channel = null;
|
||||
}
|
||||
try {
|
||||
this.roleChannel = new BroadcastChannel(CROSS_TAB_ROLE_CHANNEL_NAME);
|
||||
this.roleChannel.addEventListener('message', this.onRoleBroadcastMessage);
|
||||
} catch {
|
||||
this.roleChannel = null;
|
||||
}
|
||||
}
|
||||
this.storageListener = (ev: StorageEvent) => {
|
||||
if (!isStorageInvalidationEvent(ev)) return;
|
||||
this.handlePeerInvalidation(ev.newValue, 'peer-logout');
|
||||
if (isStorageInvalidationEvent(ev)) {
|
||||
this.handlePeerInvalidation(ev.newValue, 'peer-logout');
|
||||
return;
|
||||
}
|
||||
if (isStorageRoleChangeEvent(ev)) {
|
||||
this.handlePeerRoleChange(ev.newValue);
|
||||
}
|
||||
};
|
||||
window.addEventListener('storage', this.storageListener);
|
||||
}
|
||||
@@ -290,6 +347,15 @@ export class AuthService {
|
||||
}
|
||||
this.channel = null;
|
||||
}
|
||||
if (this.roleChannel) {
|
||||
try {
|
||||
this.roleChannel.removeEventListener('message', this.onRoleBroadcastMessage);
|
||||
this.roleChannel.close();
|
||||
} catch {
|
||||
// ignore
|
||||
}
|
||||
this.roleChannel = null;
|
||||
}
|
||||
if (this.storageListener && typeof window !== 'undefined') {
|
||||
try {
|
||||
window.removeEventListener('storage', this.storageListener);
|
||||
@@ -299,6 +365,7 @@ export class AuthService {
|
||||
this.storageListener = null;
|
||||
}
|
||||
this.peerInvalidationListeners.clear();
|
||||
this.peerRoleChangeListeners.clear();
|
||||
}
|
||||
|
||||
private readonly onBroadcastMessage = (ev: MessageEvent<unknown>) => {
|
||||
@@ -307,6 +374,12 @@ export class AuthService {
|
||||
this.handlePeerInvalidation(JSON.stringify(ev.data), 'peer-logout');
|
||||
};
|
||||
|
||||
private readonly onRoleBroadcastMessage = (ev: MessageEvent<unknown>) => {
|
||||
if (!isCrossTabRoleChangeMessage(ev.data)) return;
|
||||
if (ev.data.origin === this.crossTabOrigin) return;
|
||||
this.handlePeerRoleChange(JSON.stringify(ev.data));
|
||||
};
|
||||
|
||||
private handlePeerInvalidation(
|
||||
raw: string | null,
|
||||
reason: 'peer-logout' | 'sse-unauthorized',
|
||||
@@ -337,6 +410,31 @@ export class AuthService {
|
||||
}
|
||||
}
|
||||
|
||||
private handlePeerRoleChange(raw: string | null): void {
|
||||
if (!raw) return;
|
||||
let parsed: CrossTabRoleChangeMessage | null = null;
|
||||
try {
|
||||
const candidate = JSON.parse(raw) as unknown;
|
||||
if (!isCrossTabRoleChangeMessage(candidate)) return;
|
||||
if (candidate.origin === this.crossTabOrigin) return;
|
||||
parsed = candidate;
|
||||
} catch {
|
||||
return;
|
||||
}
|
||||
if (!parsed) return;
|
||||
const me = this.user();
|
||||
if (!me) return;
|
||||
if (me.id !== parsed.userId) return;
|
||||
this.applyPeerRoleChange(parsed.newRole);
|
||||
for (const cb of this.peerRoleChangeListeners) {
|
||||
try {
|
||||
cb(parsed);
|
||||
} catch {
|
||||
// ignore listener errors
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private sessionStorage(): Storage {
|
||||
return typeof window !== 'undefined' ? window.sessionStorage : (undefined as unknown as Storage);
|
||||
}
|
||||
|
||||
@@ -52,4 +52,12 @@ export class UserStore {
|
||||
this.loading.set(false);
|
||||
this.error.set(null);
|
||||
}
|
||||
|
||||
setRole(role: 'admin' | 'player'): void {
|
||||
const u = this.user();
|
||||
if (!u) return;
|
||||
if (u.role === role) return;
|
||||
this.user.set({ ...u, role });
|
||||
this.auth.applyPeerRoleChange(role);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -8,6 +8,7 @@ import {
|
||||
import { CommonModule } from '@angular/common';
|
||||
import { AdminService, AdminUser } from '../../core/services/admin.service';
|
||||
import { NotificationService } from '../../core/services/notification.service';
|
||||
import { AuthService } from '../../core/services/auth.service';
|
||||
import {
|
||||
adminPlayerApiErrorMessage,
|
||||
extractFieldErrors,
|
||||
@@ -169,6 +170,7 @@ import {
|
||||
export class AdminUsersComponent implements OnInit {
|
||||
private readonly admin = inject(AdminService);
|
||||
private readonly notify = inject(NotificationService);
|
||||
private readonly auth = inject(AuthService);
|
||||
|
||||
readonly rows = signal<AdminUser[]>([]);
|
||||
readonly loading = signal(false);
|
||||
@@ -241,6 +243,11 @@ export class AdminUsersComponent implements OnInit {
|
||||
const updated = await this.withInflight(row.id, () => this.admin.updatePlayerRole(row.id, nextRole));
|
||||
this.rows.set(replacePlayer(this.rows(), { ...row, ...updated }));
|
||||
this.statusMessage.set(`Role updated to ${updated.role}.`);
|
||||
const me = this.auth.currentUser();
|
||||
if (me && me.id === updated.id && me.role !== updated.role) {
|
||||
this.auth.applyPeerRoleChange(updated.role);
|
||||
this.auth.publishRoleChange(updated.id, updated.role);
|
||||
}
|
||||
} catch (e) {
|
||||
if (isLastAdminError(e)) {
|
||||
this.notify.error(lastAdminMessage());
|
||||
|
||||
@@ -17,6 +17,8 @@ import { EventStatusStore } from '../../core/services/event-status.store';
|
||||
import { AuthenticatedEventSourceService } from '../../core/services/authenticated-event-source.service';
|
||||
import { UserStore } from '../../core/services/user.store';
|
||||
import { ChallengesStore } from '../challenges/challenges.store';
|
||||
import { NotificationService } from '../../core/services/notification.service';
|
||||
import { CrossTabRoleChangeMessage } from '../../core/services/auth-session-events.pure';
|
||||
import { ShellHeaderComponent } from '../shell/header/shell-header.component';
|
||||
import {
|
||||
ChangePasswordModalComponent,
|
||||
@@ -95,6 +97,7 @@ export class HomeComponent implements OnInit, OnDestroy {
|
||||
private readonly challengesStore = inject(ChallengesStore);
|
||||
private readonly router = inject(Router);
|
||||
private readonly destroyRef = inject(DestroyRef);
|
||||
private readonly notifications = inject(NotificationService);
|
||||
|
||||
readonly userMenuOpen = signal(false);
|
||||
readonly changePasswordOpen = signal(false);
|
||||
@@ -138,6 +141,9 @@ export class HomeComponent implements OnInit, OnDestroy {
|
||||
this.peerInvalidationUnsubscribe = this.auth.onPeerInvalidation((reason) =>
|
||||
this.handleSessionInvalidated(reason),
|
||||
);
|
||||
this.peerRoleChangeUnsubscribe = this.auth.onPeerRoleChange((msg) =>
|
||||
this.handlePeerRoleChange(msg),
|
||||
);
|
||||
}
|
||||
|
||||
ngOnDestroy(): void {
|
||||
@@ -146,6 +152,10 @@ export class HomeComponent implements OnInit, OnDestroy {
|
||||
this.peerInvalidationUnsubscribe();
|
||||
this.peerInvalidationUnsubscribe = null;
|
||||
}
|
||||
if (this.peerRoleChangeUnsubscribe) {
|
||||
this.peerRoleChangeUnsubscribe();
|
||||
this.peerRoleChangeUnsubscribe = null;
|
||||
}
|
||||
}
|
||||
|
||||
goHome(): void {
|
||||
@@ -207,6 +217,7 @@ export class HomeComponent implements OnInit, OnDestroy {
|
||||
}
|
||||
|
||||
private peerInvalidationUnsubscribe: (() => void) | null = null;
|
||||
private peerRoleChangeUnsubscribe: (() => void) | null = null;
|
||||
private navigatingToLogin = false;
|
||||
|
||||
private async handleSessionInvalidated(
|
||||
@@ -225,4 +236,14 @@ export class HomeComponent implements OnInit, OnDestroy {
|
||||
this.navigatingToLogin = false;
|
||||
}
|
||||
}
|
||||
|
||||
private handlePeerRoleChange(msg: CrossTabRoleChangeMessage): void {
|
||||
const me = this.auth.currentUser();
|
||||
if (!me || me.id !== msg.userId) return;
|
||||
this.userStore.setRole(msg.newRole);
|
||||
if (!this.router.url.startsWith('/admin/')) return;
|
||||
this.notifications.info(
|
||||
`Your role was changed to "${msg.newRole}" by another admin. You may need to refresh or log in again to continue.`,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -453,4 +453,57 @@ describe('Admin /players endpoints (Job 867)', () => {
|
||||
const stillThere = await ds.getRepository(UserEntity).findOne({ where: { id: admin.id } });
|
||||
expect(stillThere).not.toBeNull();
|
||||
});
|
||||
|
||||
it('two-admin concurrent role demotion: one succeeds, the other receives 409 LAST_ADMIN (Job 913)', async () => {
|
||||
await request(app.getHttpServer())
|
||||
.post('/api/v1/auth/register-first-admin')
|
||||
.send({ username: 'original', password: 'Sup3rSecret!Pass' })
|
||||
.expect(201);
|
||||
const adminAuth = await loginAs(app, 'original', 'Sup3rSecret!Pass');
|
||||
|
||||
const adminAgent = request.agent(app.getHttpServer());
|
||||
await adminAgent.get('/api/v1/auth/csrf');
|
||||
const adminCsrfCookies: any = adminAgent.jar.getCookies(CookieAccessInfo.All);
|
||||
const adminCsrf = adminCsrfCookies.find((c: any) => c.name === 'csrf')?.value;
|
||||
await adminAgent
|
||||
.post('/api/v1/admin/users')
|
||||
.set('Authorization', `Bearer ${adminAuth.accessToken}`)
|
||||
.set('X-CSRF-Token', adminCsrf)
|
||||
.send({ username: 'bob', password: 'Sup3rSecret!Pass', role: 'admin' })
|
||||
.expect(201);
|
||||
|
||||
const list = await request(app.getHttpServer())
|
||||
.get('/api/v1/admin/players')
|
||||
.set('Authorization', `Bearer ${adminAuth.accessToken}`)
|
||||
.expect(200);
|
||||
const originalRow = list.body.find((u: any) => u.username === 'original');
|
||||
const bobRow = list.body.find((u: any) => u.username === 'bob');
|
||||
expect(originalRow.role).toBe('admin');
|
||||
expect(bobRow.role).toBe('admin');
|
||||
|
||||
const bobAuth = await loginAs(app, 'bob', 'Sup3rSecret!Pass');
|
||||
|
||||
const first = await adminAuth.agent
|
||||
.patch(`/api/v1/admin/players/${originalRow.id}/role`)
|
||||
.set('Authorization', `Bearer ${adminAuth.accessToken}`)
|
||||
.set('X-CSRF-Token', adminAuth.csrf)
|
||||
.send({ role: 'player' })
|
||||
.expect(200)
|
||||
.expect((r) => expect(r.body.role).toBe('player'));
|
||||
|
||||
const second = await bobAuth.agent
|
||||
.patch(`/api/v1/admin/players/${bobRow.id}/role`)
|
||||
.set('Authorization', `Bearer ${bobAuth.accessToken}`)
|
||||
.set('X-CSRF-Token', bobAuth.csrf)
|
||||
.send({ role: 'player' })
|
||||
.expect(409)
|
||||
.expect((r) => expect(r.body.code).toBe('LAST_ADMIN'));
|
||||
|
||||
const ds = app.get(DataSource);
|
||||
const rowsAfter = await ds.getRepository(UserEntity).find();
|
||||
const adminCount = rowsAfter.filter((u) => u.role === 'admin').length;
|
||||
expect(adminCount).toBe(1);
|
||||
const remainingAdmin = rowsAfter.find((u) => u.role === 'admin');
|
||||
expect(remainingAdmin?.username).toBe('bob');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,10 +1,17 @@
|
||||
import {
|
||||
CROSS_TAB_CHANNEL_NAME,
|
||||
CROSS_TAB_STORAGE_KEY,
|
||||
CROSS_TAB_ROLE_CHANNEL_NAME,
|
||||
CROSS_TAB_ROLE_STORAGE_KEY,
|
||||
CrossTabInvalidationMessage,
|
||||
CrossTabRoleChangeMessage,
|
||||
applyRoleChangeToUser,
|
||||
encodeInvalidationMessage,
|
||||
encodeRoleChangeMessage,
|
||||
isCrossTabInvalidationMessage,
|
||||
isCrossTabRoleChangeMessage,
|
||||
isStorageInvalidationEvent,
|
||||
isStorageRoleChangeEvent,
|
||||
} from '../../frontend/src/app/core/services/auth-session-events.pure';
|
||||
|
||||
describe('auth-session-events.pure', () => {
|
||||
@@ -88,6 +95,105 @@ describe('auth-session-events.pure', () => {
|
||||
it('exposes a stable channel name and storage key', () => {
|
||||
expect(CROSS_TAB_CHANNEL_NAME).toBe('hipctf.auth.v1');
|
||||
expect(CROSS_TAB_STORAGE_KEY).toBe('hipctf.auth.invalidate.v1');
|
||||
expect(CROSS_TAB_ROLE_CHANNEL_NAME).toBe('hipctf.auth.role.v1');
|
||||
expect(CROSS_TAB_ROLE_STORAGE_KEY).toBe('hipctf.role.invalidate.v1');
|
||||
});
|
||||
});
|
||||
|
||||
describe('encodeRoleChangeMessage', () => {
|
||||
it('produces a role-changed message with userId and newRole', () => {
|
||||
const before = Date.now();
|
||||
const msg = encodeRoleChangeMessage('tab-A', 'user-1', 'player');
|
||||
const after = Date.now();
|
||||
expect(msg.type).toBe('role-changed');
|
||||
expect(msg.origin).toBe('tab-A');
|
||||
expect(msg.userId).toBe('user-1');
|
||||
expect(msg.newRole).toBe('player');
|
||||
expect(msg.ts).toBeGreaterThanOrEqual(before);
|
||||
expect(msg.ts).toBeLessThanOrEqual(after);
|
||||
});
|
||||
});
|
||||
|
||||
describe('isCrossTabRoleChangeMessage', () => {
|
||||
it('accepts a properly shaped message', () => {
|
||||
const msg: CrossTabRoleChangeMessage = {
|
||||
type: 'role-changed',
|
||||
origin: 'x',
|
||||
ts: 1,
|
||||
userId: 'u1',
|
||||
newRole: 'player',
|
||||
};
|
||||
expect(isCrossTabRoleChangeMessage(msg)).toBe(true);
|
||||
});
|
||||
|
||||
it('rejects a session-invalidated message (does not double-fire)', () => {
|
||||
const msg: CrossTabInvalidationMessage = { type: 'session-invalidated', origin: 'x', ts: 1 };
|
||||
expect(isCrossTabRoleChangeMessage(msg)).toBe(false);
|
||||
});
|
||||
|
||||
it('rejects objects with wrong/missing fields', () => {
|
||||
expect(isCrossTabRoleChangeMessage({ type: 'role-changed', origin: 'a', ts: 1, userId: 'u', newRole: 'super' })).toBe(
|
||||
false,
|
||||
);
|
||||
expect(isCrossTabRoleChangeMessage({ type: 'role-changed', origin: 'a', ts: 1, userId: 1, newRole: 'admin' })).toBe(
|
||||
false,
|
||||
);
|
||||
expect(isCrossTabRoleChangeMessage({ type: 'role-changed', origin: 'a', ts: 'now', userId: 'u', newRole: 'admin' })).toBe(
|
||||
false,
|
||||
);
|
||||
});
|
||||
|
||||
it('rejects non-objects', () => {
|
||||
expect(isCrossTabRoleChangeMessage(null)).toBe(false);
|
||||
expect(isCrossTabRoleChangeMessage(undefined)).toBe(false);
|
||||
expect(isCrossTabRoleChangeMessage('string')).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('isStorageRoleChangeEvent', () => {
|
||||
it('accepts a well-formed storage event for the role key', () => {
|
||||
const ev = {
|
||||
key: CROSS_TAB_ROLE_STORAGE_KEY,
|
||||
newValue: JSON.stringify(encodeRoleChangeMessage('tab-X', 'u1', 'player')),
|
||||
};
|
||||
expect(isStorageRoleChangeEvent(ev)).toBe(true);
|
||||
});
|
||||
|
||||
it('rejects events with a different key', () => {
|
||||
const ev = {
|
||||
key: 'something.else',
|
||||
newValue: JSON.stringify(encodeRoleChangeMessage('tab-X', 'u1', 'player')),
|
||||
};
|
||||
expect(isStorageRoleChangeEvent(ev)).toBe(false);
|
||||
});
|
||||
|
||||
it('rejects malformed JSON payloads', () => {
|
||||
const ev = { key: CROSS_TAB_ROLE_STORAGE_KEY, newValue: 'not-json' };
|
||||
expect(isStorageRoleChangeEvent(ev)).toBe(false);
|
||||
});
|
||||
|
||||
it('rejects nullish events', () => {
|
||||
expect(isStorageRoleChangeEvent(null)).toBe(false);
|
||||
expect(isStorageRoleChangeEvent(undefined)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('applyRoleChangeToUser', () => {
|
||||
it('returns a new user with the updated role when they differ', () => {
|
||||
const u = { id: 'u1', role: 'admin' as const };
|
||||
const out = applyRoleChangeToUser(u, 'player');
|
||||
expect(out).toEqual({ id: 'u1', role: 'player' });
|
||||
expect(out).not.toBe(u);
|
||||
});
|
||||
|
||||
it('returns the same user object (no-op) when role already matches', () => {
|
||||
const u = { id: 'u1', role: 'player' as const };
|
||||
const out = applyRoleChangeToUser(u, 'player');
|
||||
expect(out).toBe(u);
|
||||
});
|
||||
|
||||
it('returns the input untouched for null users', () => {
|
||||
expect(applyRoleChangeToUser(null, 'admin')).toBeNull();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user