fix(admin): address PR #623 review feedback
All checks were successful
CI / Unit & Component Tests (pull_request) Successful in 3m15s
CI / OCR Service Tests (pull_request) Successful in 20s
CI / Backend Unit Tests (pull_request) Successful in 3m29s
CI / fail2ban Regex (pull_request) Successful in 42s
CI / Semgrep Security Scan (pull_request) Successful in 19s
CI / Compose Bucket Idempotency (pull_request) Successful in 59s
All checks were successful
CI / Unit & Component Tests (pull_request) Successful in 3m15s
CI / OCR Service Tests (pull_request) Successful in 20s
CI / Backend Unit Tests (pull_request) Successful in 3m29s
CI / fail2ban Regex (pull_request) Successful in 42s
CI / Semgrep Security Scan (pull_request) Successful in 19s
CI / Compose Bucket Idempotency (pull_request) Successful in 59s
- Add load() unit tests for admin/users/[id] (permission gate, 404, success) - Rename .test.ts → .spec.ts for consistency with rest of suite - Add @Schema(requiredMode=REQUIRED) to InviteListItem.shareableUrl - Add client-side allowlist for invite status query param Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -31,5 +31,6 @@ public class InviteListItemDTO {
|
||||
private String status;
|
||||
@Schema(requiredMode = Schema.RequiredMode.REQUIRED)
|
||||
private LocalDateTime createdAt;
|
||||
@Schema(requiredMode = Schema.RequiredMode.REQUIRED)
|
||||
private String shareableUrl;
|
||||
}
|
||||
|
||||
@@ -20,8 +20,14 @@ export interface InviteListItem {
|
||||
}
|
||||
export type UserGroup = components['schemas']['UserGroup'];
|
||||
|
||||
const VALID_STATUSES = ['ACTIVE', 'REVOKED', 'EXPIRED'] as const;
|
||||
type InviteStatus = (typeof VALID_STATUSES)[number];
|
||||
|
||||
export const load: PageServerLoad = async ({ url, fetch }) => {
|
||||
const status = url.searchParams.get('status') ?? 'active';
|
||||
const rawStatus = url.searchParams.get('status');
|
||||
const status: InviteStatus | 'active' = VALID_STATUSES.includes(rawStatus as InviteStatus)
|
||||
? (rawStatus as InviteStatus)
|
||||
: 'active';
|
||||
const api = createApiClient(fetch);
|
||||
|
||||
const [invitesResult, groupsResult] = await Promise.all([
|
||||
|
||||
198
frontend/src/routes/admin/users/[id]/page.server.spec.ts
Normal file
198
frontend/src/routes/admin/users/[id]/page.server.spec.ts
Normal file
@@ -0,0 +1,198 @@
|
||||
import { describe, it, expect, vi, beforeEach } from 'vitest';
|
||||
|
||||
vi.mock('$env/dynamic/private', () => ({
|
||||
env: { API_INTERNAL_URL: 'http://localhost:8080' }
|
||||
}));
|
||||
|
||||
vi.mock('$lib/shared/api.server', () => ({ createApiClient: vi.fn() }));
|
||||
|
||||
import { load, actions } from './+page.server';
|
||||
import { createApiClient } from '$lib/shared/api.server';
|
||||
|
||||
function mockApi(methods: Partial<Record<'GET' | 'PUT' | 'DELETE', ReturnType<typeof vi.fn>>>) {
|
||||
vi.mocked(createApiClient).mockReturnValue(methods as ReturnType<typeof createApiClient>);
|
||||
}
|
||||
|
||||
// ─── load() ──────────────────────────────────────────────────────────────────
|
||||
|
||||
describe('admin/users/[id] load()', () => {
|
||||
beforeEach(() => vi.clearAllMocks());
|
||||
|
||||
function makeEvent(permissions: string[] = ['ADMIN']) {
|
||||
return {
|
||||
params: { id: 'user-123' },
|
||||
fetch: vi.fn() as unknown as typeof fetch,
|
||||
locals: { user: { groups: [{ permissions }] } },
|
||||
request: new Request('http://localhost/admin/users/user-123'),
|
||||
url: new URL('http://localhost/admin/users/user-123')
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
} as any;
|
||||
}
|
||||
|
||||
it('throws 403 when the user lacks the ADMIN permission', async () => {
|
||||
let thrown: unknown;
|
||||
try {
|
||||
await load(makeEvent(['READ_ALL']));
|
||||
} catch (e) {
|
||||
thrown = e;
|
||||
}
|
||||
|
||||
expect((thrown as { status: number }).status).toBe(403);
|
||||
});
|
||||
|
||||
it('throws 404 when the backend returns non-ok for the user lookup', async () => {
|
||||
const mockGet = vi
|
||||
.fn()
|
||||
.mockResolvedValueOnce({ response: { ok: false, status: 404 }, data: undefined })
|
||||
.mockResolvedValueOnce({ response: { ok: true }, data: [] });
|
||||
mockApi({ GET: mockGet });
|
||||
|
||||
let thrown: unknown;
|
||||
try {
|
||||
await load(makeEvent());
|
||||
} catch (e) {
|
||||
thrown = e;
|
||||
}
|
||||
|
||||
expect((thrown as { status: number }).status).toBe(404);
|
||||
});
|
||||
|
||||
it('returns editUser and groups on success', async () => {
|
||||
const editUser = { id: 'user-123', email: 'max@example.com', firstName: 'Max' };
|
||||
const groups = [
|
||||
{ id: 'g-1', name: 'Familie', permissions: ['READ_ALL'] },
|
||||
{ id: 'g-2', name: 'Administratoren', permissions: ['ADMIN'] }
|
||||
];
|
||||
const mockGet = vi
|
||||
.fn()
|
||||
.mockResolvedValueOnce({ response: { ok: true }, data: editUser })
|
||||
.mockResolvedValueOnce({ response: { ok: true }, data: groups });
|
||||
mockApi({ GET: mockGet });
|
||||
|
||||
const result = await load(makeEvent());
|
||||
|
||||
expect(result).toMatchObject({
|
||||
editUser: expect.objectContaining({ id: 'user-123' }),
|
||||
groups: expect.arrayContaining([expect.objectContaining({ id: 'g-1' })])
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
// ─── update action ────────────────────────────────────────────────────────────
|
||||
|
||||
describe('admin/users/[id] update action', () => {
|
||||
beforeEach(() => vi.clearAllMocks());
|
||||
|
||||
function makeUpdateRequest(fields: Record<string, string | string[]> = {}) {
|
||||
const fd = new FormData();
|
||||
const defaults: Record<string, string> = {
|
||||
firstName: 'Max',
|
||||
lastName: 'Mustermann',
|
||||
email: 'max@example.com'
|
||||
};
|
||||
for (const [k, v] of Object.entries({ ...defaults, ...fields })) {
|
||||
if (Array.isArray(v)) {
|
||||
v.forEach((item) => fd.append(k, item));
|
||||
} else {
|
||||
fd.append(k, v);
|
||||
}
|
||||
}
|
||||
return new Request('http://localhost', { method: 'POST', body: fd });
|
||||
}
|
||||
|
||||
function makeEvent(request: Request) {
|
||||
return {
|
||||
params: { id: 'user-123' },
|
||||
request,
|
||||
fetch: vi.fn() as unknown as typeof fetch
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
} as any;
|
||||
}
|
||||
|
||||
it('calls PUT /api/users/{id} and returns success: true on 200', async () => {
|
||||
const mockPut = vi.fn().mockResolvedValue({ response: { ok: true, status: 200 }, data: {} });
|
||||
mockApi({ PUT: mockPut });
|
||||
|
||||
const result = await actions.update(makeEvent(makeUpdateRequest()));
|
||||
|
||||
expect(mockPut).toHaveBeenCalledWith(
|
||||
'/api/users/{id}',
|
||||
expect.objectContaining({ params: { path: { id: 'user-123' } } })
|
||||
);
|
||||
expect(result).toEqual({ success: true });
|
||||
});
|
||||
|
||||
it('returns fail with backend error code when PUT returns non-OK', async () => {
|
||||
const mockPut = vi
|
||||
.fn()
|
||||
.mockResolvedValue({ response: { ok: false, status: 403 }, error: { code: 'FORBIDDEN' } });
|
||||
mockApi({ PUT: mockPut });
|
||||
|
||||
const result = await actions.update(makeEvent(makeUpdateRequest()));
|
||||
|
||||
expect(result).toMatchObject({ status: 403 });
|
||||
});
|
||||
|
||||
it('returns fail with generic message when error body has no code field', async () => {
|
||||
const mockPut = vi
|
||||
.fn()
|
||||
.mockResolvedValue({ response: { ok: false, status: 500 }, error: null });
|
||||
mockApi({ PUT: mockPut });
|
||||
|
||||
const result = await actions.update(makeEvent(makeUpdateRequest()));
|
||||
|
||||
expect(result).toMatchObject({ status: 500 });
|
||||
});
|
||||
|
||||
it('returns fail without calling backend when passwords do not match', async () => {
|
||||
const mockPut = vi.fn();
|
||||
mockApi({ PUT: mockPut });
|
||||
|
||||
const result = await actions.update(
|
||||
makeEvent(makeUpdateRequest({ newPassword: 'abc', confirmPassword: 'xyz' }))
|
||||
);
|
||||
|
||||
expect(mockPut).not.toHaveBeenCalled();
|
||||
expect(result).toMatchObject({ status: 400 });
|
||||
});
|
||||
});
|
||||
|
||||
// ─── delete action ────────────────────────────────────────────────────────────
|
||||
|
||||
describe('admin/users/[id] delete action', () => {
|
||||
beforeEach(() => vi.clearAllMocks());
|
||||
|
||||
function makeEvent() {
|
||||
return {
|
||||
params: { id: 'user-123' },
|
||||
fetch: vi.fn() as unknown as typeof fetch
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
} as any;
|
||||
}
|
||||
|
||||
it('redirects to /admin/users on successful delete', async () => {
|
||||
const mockDelete = vi.fn().mockResolvedValue({ response: { ok: true } });
|
||||
mockApi({ DELETE: mockDelete });
|
||||
|
||||
let redirectLocation: string | null = null;
|
||||
try {
|
||||
await actions.delete(makeEvent());
|
||||
} catch (e: unknown) {
|
||||
const r = e as { location?: string };
|
||||
redirectLocation = r.location ?? null;
|
||||
}
|
||||
|
||||
expect(redirectLocation).toBe('/admin/users');
|
||||
});
|
||||
|
||||
it('returns fail when delete returns non-OK', async () => {
|
||||
const mockDelete = vi
|
||||
.fn()
|
||||
.mockResolvedValue({ response: { ok: false, status: 403 }, error: { code: 'FORBIDDEN' } });
|
||||
mockApi({ DELETE: mockDelete });
|
||||
|
||||
const result = await actions.delete(makeEvent());
|
||||
|
||||
expect(result).toMatchObject({ status: 403 });
|
||||
});
|
||||
});
|
||||
@@ -1,110 +0,0 @@
|
||||
import { describe, it, expect, vi, beforeEach } from 'vitest';
|
||||
|
||||
vi.mock('$env/dynamic/private', () => ({
|
||||
env: { API_INTERNAL_URL: 'http://localhost:8080' }
|
||||
}));
|
||||
|
||||
import { actions } from './+page.server';
|
||||
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
type AnyFetch = (...args: any[]) => any;
|
||||
|
||||
function mockResponse(ok: boolean, body: unknown, status = 200) {
|
||||
return {
|
||||
ok,
|
||||
status,
|
||||
json: async () => body,
|
||||
text: async () => JSON.stringify(body),
|
||||
headers: new Headers({ 'content-type': 'application/json' })
|
||||
} as unknown as Response;
|
||||
}
|
||||
|
||||
function makeUpdateRequest(fields: Record<string, string | string[]> = {}) {
|
||||
const fd = new FormData();
|
||||
const defaults: Record<string, string> = {
|
||||
firstName: 'Max',
|
||||
lastName: 'Mustermann',
|
||||
email: 'max@example.com'
|
||||
};
|
||||
for (const [k, v] of Object.entries({ ...defaults, ...fields })) {
|
||||
if (Array.isArray(v)) {
|
||||
v.forEach((item) => fd.append(k, item));
|
||||
} else {
|
||||
fd.append(k, v);
|
||||
}
|
||||
}
|
||||
return new Request('http://localhost', { method: 'POST', body: fd });
|
||||
}
|
||||
|
||||
describe('admin/users/[id] update action', () => {
|
||||
const mockFetch = vi.fn<AnyFetch>();
|
||||
|
||||
beforeEach(() => mockFetch.mockReset());
|
||||
|
||||
it('calls PUT /api/users/{id} via createApiClient (Request object)', async () => {
|
||||
mockFetch.mockResolvedValueOnce(mockResponse(true, {}, 200));
|
||||
|
||||
await actions.update({
|
||||
params: { id: 'user-123' },
|
||||
request: makeUpdateRequest(),
|
||||
fetch: mockFetch as unknown as typeof fetch
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
} as any);
|
||||
|
||||
const [req] = mockFetch.mock.calls[0] as [Request, unknown];
|
||||
expect(req).toBeInstanceOf(Request);
|
||||
expect(req.url).toContain('/api/users/user-123');
|
||||
expect(req.method).toBe('PUT');
|
||||
});
|
||||
|
||||
it('returns success: true when PUT responds with 200', async () => {
|
||||
mockFetch.mockResolvedValueOnce(mockResponse(true, {}, 200));
|
||||
|
||||
const result = await actions.update({
|
||||
params: { id: 'user-123' },
|
||||
request: makeUpdateRequest(),
|
||||
fetch: mockFetch as unknown as typeof fetch
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
} as any);
|
||||
|
||||
expect(result).toEqual({ success: true });
|
||||
});
|
||||
|
||||
it('returns fail with backend error code when PUT returns non-OK', async () => {
|
||||
mockFetch.mockResolvedValueOnce(mockResponse(false, { code: 'FORBIDDEN' }, 403));
|
||||
|
||||
const result = await actions.update({
|
||||
params: { id: 'user-123' },
|
||||
request: makeUpdateRequest(),
|
||||
fetch: mockFetch as unknown as typeof fetch
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
} as any);
|
||||
|
||||
expect(result).toMatchObject({ status: 403 });
|
||||
});
|
||||
|
||||
it('returns fail with generic message when error body has no code field', async () => {
|
||||
mockFetch.mockResolvedValueOnce(mockResponse(false, { message: 'internal server error' }, 500));
|
||||
|
||||
const result = await actions.update({
|
||||
params: { id: 'user-123' },
|
||||
request: makeUpdateRequest(),
|
||||
fetch: mockFetch as unknown as typeof fetch
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
} as any);
|
||||
|
||||
expect(result).toMatchObject({ status: 500 });
|
||||
});
|
||||
|
||||
it('returns fail without calling backend when passwords do not match', async () => {
|
||||
const result = await actions.update({
|
||||
params: { id: 'user-123' },
|
||||
request: makeUpdateRequest({ newPassword: 'abc', confirmPassword: 'xyz' }),
|
||||
fetch: mockFetch as unknown as typeof fetch
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
} as any);
|
||||
|
||||
expect(mockFetch).not.toHaveBeenCalled();
|
||||
expect(result).toMatchObject({ status: 400 });
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user