feat: Admin section redesign — Concept C (Master-Detail Command Center) #156

Closed
opened 2026-03-29 15:06:55 +02:00 by marcel · 3 comments
Owner

Overview

The current admin section (/admin) is a tab-based layout that has significant UX and accessibility problems at mobile and tablet widths. This issue tracks a full redesign to a Master-Detail Command Center layout (Concept C) covering all four entity types: Users, Groups, Tags, and System.

A complete wireframe spec has been committed to the repo:
📄 docs/specs/admin-redesign-concept-c.html
Open it locally in a browser for the full interactive wireframes.


Problems with the current design (UX audit findings)

Severity Issue
🔴 Critical Tag action buttons are opacity-0 group-hover:opacity-100 — completely broken on mobile/touch
🔴 Critical Users table has no horizontal scroll — truncates on 375px
🔴 Critical Sticky save bar occludes the password field on small screens
🟠 High ADMIN badge is red color only — WCAG 1.4.1 failure (no non-color indicator)
🟠 High Inline group create form is chaotic on 375px (checkboxes overflow)
🟠 High No visual hierarchy between entity types (flat tab row)
🟠 High Tab layout wastes the full width — no persistent context
🟡 Medium No empty-state messaging
🟡 Medium No unsaved-changes guard
🟡 Medium No inline error feedback on form save failure

Proposed layout: Three-panel Master-Detail

┌──────────┬──────────────────┬────────────────────────────────┐
│ Entity   │  List Panel      │  Detail Panel                  │
│ Nav      │  (searchable)    │  (form / view)                 │
│ 120px    │  240px           │  flex: 1                       │
└──────────┴──────────────────┴────────────────────────────────┘

URL scheme

State is encoded in query params so deep links and browser back work:

  • /admin?v=users — Users list, no selection
  • /admin?v=users&id=<uuid> — Users list, user selected
  • /admin?v=users&id=new — Create new user form open
  • /admin?v=system — System tab (list panel hidden)

Breakpoints

Viewport Entity Nav List Panel Detail Panel
< 768px Mobile Full-screen step 1 Full-screen step 2 Full-screen step 3
768–1023px Tablet 48px icon strip + 160px flyout on tap 200px (default) / 32px collapsed 520px (default) / 688px collapsed
≥ 1024px Desktop 120px with text labels 240px, always visible All remaining width

Tablet: Collapseable panels (addendum)

At 768px the original three-panel spec leaves only 408px for the detail — too cramped for a form. The tablet layout introduces three states:

State Triggered by Detail width
A · Default Page load 520px
B · List collapsed ‹ button in list header 688px
C · Entity flyout Tap any icon in the 48px strip Dimmed (flyout overlays)
  • Entity nav shrinks from 120px text → 48px icon-only strip (count badge + SVG icon)
  • List panel has a ‹ collapse button in its header; collapses to a 32px handle with rotated label
  • Clicking + New auto-collapses the list to give the create form maximum space
  • Collapse state persisted to localStorage key admin_list_collapsed
  • Entity flyout uses CSS transform: translateX animation (180ms ease-out), role="dialog", focus-trapped

Per-entity spec summary

Users

  • List: login + full name + group chips + chevron
  • Detail states: landing (select prompt), edit (Personal Info / Groups / Change Password sections), create (green tinted), delete confirmation (danger zone with type-to-confirm)
  • Mobile: 3-step push flow (entity picker → user list → edit form)

Groups

  • List: group name + member count + permission summary
  • Detail: group name field + permissions matrix split into Standard and ⚙ Administrative sections (admin perms in red-bordered box with checkbox + text label — fixes WCAG color-only issue)
  • Inline member management (add/remove users from the detail panel)

Tags

  • List: tag chip + document count
  • Detail: rename field + "Renaming affects N documents" warning + delete with count in button label ("Delete tag (12 documents)")

System

  • No list panel — detail fills full remaining width
  • Cards per maintenance operation: idle → inline confirm expand → spinner → success banner with affected count
  • Operations: Backfill document versions, Backfill file hashes

Edge cases (all entities)

  • Empty list: illustration + "No [entity] yet" + primary CTA button
  • Unsaved changes guard: inline warning in detail panel when navigating away (not a modal)
  • API error: error banner at top of form + field-level error message + "Retry save" button; changes are NOT lost

Interaction specification (key rules)

Trigger Behaviour
Click list item (dirty form) Inline warning: "Save or discard first" — navigation blocked
Submit create form (success) List refreshes, new item highlighted, detail switches to edit mode
Confirm delete (success) Item removed from list, detail → empty-selection, nav count decrements
Click System in entity nav List panel hidden; detail fills width with maintenance cards
Direct URL load with params Server pre-fetches entity + item; correct panel state on first paint
Browser back (mobile) Pops one step: detail → list → entity picker

Acceptance criteria

  • Three-panel shell renders correctly at all three breakpoints
  • URL params (?v=, &id=) reflect panel state; direct URL loads work
  • Users: create, edit, delete, group assignment all functional
  • Groups: create, edit, delete, permission checkboxes, member add/remove
  • Tags: rename, delete (with document count warning)
  • System: both maintenance operations with confirm → progress → success flow
  • Mobile: push-route navigation with browser back support
  • Tablet: entity icon strip (48px), list collapse toggle (‹/›), entity flyout overlay
  • Tablet: collapse state persisted to localStorage
  • Tablet: auto-collapse on "+ New"
  • Empty-state screens for all entities
  • Unsaved-changes guard (inline, no modal)
  • API error feedback (banner + field errors + "Retry save")
  • WCAG: ADMIN permission uses checkbox + label, not color alone
  • WCAG: all tag actions always visible (no hover-only)
  • WCAG: flyout has role="dialog", aria-modal, focus trap, aria-label on all icon buttons

Implementation notes

  • The current /admin route uses let activeTab = $state('users') with conditional rendering — this needs to be replaced by the new URL-param-driven panel system
  • The current /admin/users/[id] and /admin/users/new routes may be absorbed into the single /admin route via the detail panel, or kept as separate routes that the detail panel renders into
  • The Svelte store for breakpoint detection: isTablet = $derived(windowWidth >= 768 && windowWidth < 1024)
  • Panel collapse state: let listCollapsed = $state(false) in the admin layout, synced to localStorage

Spec authored by Leonie Voss (UI/UX persona) · v1.1 · 2026-03
Full wireframe spec: docs/specs/admin-redesign-concept-c.html

## Overview The current admin section (`/admin`) is a tab-based layout that has significant UX and accessibility problems at mobile and tablet widths. This issue tracks a full redesign to a **Master-Detail Command Center** layout (Concept C) covering all four entity types: Users, Groups, Tags, and System. A complete wireframe spec has been committed to the repo: 📄 **[docs/specs/admin-redesign-concept-c.html](http://heim-nas:3005/marcel/familienarchiv/src/branch/main/docs/specs/admin-redesign-concept-c.html)** Open it locally in a browser for the full interactive wireframes. --- ## Problems with the current design (UX audit findings) | Severity | Issue | |---|---| | 🔴 Critical | Tag action buttons are `opacity-0 group-hover:opacity-100` — completely broken on mobile/touch | | 🔴 Critical | Users table has no horizontal scroll — truncates on 375px | | 🔴 Critical | Sticky save bar occludes the password field on small screens | | 🟠 High | ADMIN badge is red color only — WCAG 1.4.1 failure (no non-color indicator) | | 🟠 High | Inline group create form is chaotic on 375px (checkboxes overflow) | | 🟠 High | No visual hierarchy between entity types (flat tab row) | | 🟠 High | Tab layout wastes the full width — no persistent context | | 🟡 Medium | No empty-state messaging | | 🟡 Medium | No unsaved-changes guard | | 🟡 Medium | No inline error feedback on form save failure | --- ## Proposed layout: Three-panel Master-Detail ``` ┌──────────┬──────────────────┬────────────────────────────────┐ │ Entity │ List Panel │ Detail Panel │ │ Nav │ (searchable) │ (form / view) │ │ 120px │ 240px │ flex: 1 │ └──────────┴──────────────────┴────────────────────────────────┘ ``` ### URL scheme State is encoded in query params so deep links and browser back work: - `/admin?v=users` — Users list, no selection - `/admin?v=users&id=<uuid>` — Users list, user selected - `/admin?v=users&id=new` — Create new user form open - `/admin?v=system` — System tab (list panel hidden) ### Breakpoints | Viewport | Entity Nav | List Panel | Detail Panel | |---|---|---|---| | `< 768px` Mobile | Full-screen step 1 | Full-screen step 2 | Full-screen step 3 | | `768–1023px` Tablet | **48px icon strip** + 160px flyout on tap | 200px (default) / 32px collapsed | 520px (default) / 688px collapsed | | `≥ 1024px` Desktop | 120px with text labels | 240px, always visible | All remaining width | --- ## Tablet: Collapseable panels (addendum) At 768px the original three-panel spec leaves only 408px for the detail — too cramped for a form. The tablet layout introduces three states: | State | Triggered by | Detail width | |---|---|---| | **A · Default** | Page load | 520px | | **B · List collapsed** | ‹ button in list header | 688px | | **C · Entity flyout** | Tap any icon in the 48px strip | Dimmed (flyout overlays) | - Entity nav shrinks from 120px text → **48px icon-only strip** (count badge + SVG icon) - List panel has a **‹ collapse button** in its header; collapses to a 32px handle with rotated label - Clicking **+ New** auto-collapses the list to give the create form maximum space - Collapse state persisted to `localStorage` key `admin_list_collapsed` - Entity flyout uses CSS `transform: translateX` animation (180ms ease-out), role="dialog", focus-trapped --- ## Per-entity spec summary ### Users - List: login + full name + group chips + chevron - Detail states: landing (select prompt), edit (Personal Info / Groups / Change Password sections), create (green tinted), delete confirmation (danger zone with type-to-confirm) - Mobile: 3-step push flow (entity picker → user list → edit form) ### Groups - List: group name + member count + permission summary - Detail: group name field + permissions matrix split into **Standard** and **⚙ Administrative** sections (admin perms in red-bordered box with checkbox + text label — fixes WCAG color-only issue) - Inline member management (add/remove users from the detail panel) ### Tags - List: tag chip + document count - Detail: rename field + "Renaming affects N documents" warning + delete with count in button label ("Delete tag (12 documents)") ### System - No list panel — detail fills full remaining width - Cards per maintenance operation: idle → inline confirm expand → spinner → success banner with affected count - Operations: Backfill document versions, Backfill file hashes ### Edge cases (all entities) - **Empty list**: illustration + "No [entity] yet" + primary CTA button - **Unsaved changes guard**: inline warning in detail panel when navigating away (not a modal) - **API error**: error banner at top of form + field-level error message + "Retry save" button; changes are NOT lost --- ## Interaction specification (key rules) | Trigger | Behaviour | |---|---| | Click list item (dirty form) | Inline warning: "Save or discard first" — navigation blocked | | Submit create form (success) | List refreshes, new item highlighted, detail switches to edit mode | | Confirm delete (success) | Item removed from list, detail → empty-selection, nav count decrements | | Click System in entity nav | List panel hidden; detail fills width with maintenance cards | | Direct URL load with params | Server pre-fetches entity + item; correct panel state on first paint | | Browser back (mobile) | Pops one step: detail → list → entity picker | --- ## Acceptance criteria - [ ] Three-panel shell renders correctly at all three breakpoints - [ ] URL params (`?v=`, `&id=`) reflect panel state; direct URL loads work - [ ] Users: create, edit, delete, group assignment all functional - [ ] Groups: create, edit, delete, permission checkboxes, member add/remove - [ ] Tags: rename, delete (with document count warning) - [ ] System: both maintenance operations with confirm → progress → success flow - [ ] Mobile: push-route navigation with browser back support - [ ] Tablet: entity icon strip (48px), list collapse toggle (‹/›), entity flyout overlay - [ ] Tablet: collapse state persisted to localStorage - [ ] Tablet: auto-collapse on "+ New" - [ ] Empty-state screens for all entities - [ ] Unsaved-changes guard (inline, no modal) - [ ] API error feedback (banner + field errors + "Retry save") - [ ] WCAG: ADMIN permission uses checkbox + label, not color alone - [ ] WCAG: all tag actions always visible (no hover-only) - [ ] WCAG: flyout has role="dialog", aria-modal, focus trap, aria-label on all icon buttons --- ## Implementation notes - The current `/admin` route uses `let activeTab = $state('users')` with conditional rendering — this needs to be replaced by the new URL-param-driven panel system - The current `/admin/users/[id]` and `/admin/users/new` routes may be absorbed into the single `/admin` route via the detail panel, or kept as separate routes that the detail panel renders into - The Svelte store for breakpoint detection: `isTablet = $derived(windowWidth >= 768 && windowWidth < 1024)` - Panel collapse state: `let listCollapsed = $state(false)` in the admin layout, synced to localStorage --- *Spec authored by Leonie Voss (UI/UX persona) · v1.1 · 2026-03* *Full wireframe spec: [docs/specs/admin-redesign-concept-c.html](http://heim-nas:3005/marcel/familienarchiv/src/branch/main/docs/specs/admin-redesign-concept-c.html)*
marcel added the featureui labels 2026-03-29 15:07:04 +02:00
Author
Owner

Architectural Review — Concept C Implementation

@mkeller · 2026-03-29

I have read the spec and the current code. The UX problems are real and worth fixing. Before implementation starts, four structural decisions need to be locked down. I am recording them here so they are not re-litigated during the PR.


Decision 1: Sub-routes with a shared layout shell

Use Option B: /admin/+layout.svelte as the three-panel shell, sub-routes render into the detail slot.

The spec leaves this open ("may be absorbed… or kept as separate routes"). It should not be left open — this is the load-bearing decision for everything else.

Proposed route tree:

frontend/src/routes/admin/
├── +layout.svelte          ← three-panel shell (entity nav + list slot + detail slot)
├── +layout.server.ts       ← auth guard (once, here — not per-page)
├── +page.svelte            ← redirects to /admin/users
├── users/
│   ├── +page.svelte        ← list panel content
│   ├── +page.server.ts     ← loads users list only
│   ├── new/                ← already exists, keep it
│   └── [id]/               ← already exists, keep it
├── groups/
│   ├── +page.svelte
│   ├── +page.server.ts     ← loads groups list only
│   └── [id]/
│       ├── +page.svelte
│       └── +page.server.ts ← loads one group + members
├── tags/
│   ├── +page.svelte
│   └── +page.server.ts     ← loads tags list only
└── system/
    └── +page.svelte        ← no server load needed

Why this is correct:

  • SvelteKit's router handles deep links, browser back, and mobile push-flow navigation (spec ACs) for free. No custom navigation wrapper needed.
  • beforeNavigate (see Decision 4 below) works cleanly on route changes.
  • Panel transitions use SvelteKit's page transition hooks — no manual history management.
  • The sub-routes for users already exist (/admin/users/[id], /admin/users/new). This is an evolution of what is there, not a rewrite.

Decision 2: Move the auth guard to +layout.server.ts

Currently the hasAdmin check is duplicated in every load function. Both routes/admin/+page.server.ts:17 and routes/admin/users/[id]/+page.server.ts:8 contain identical copies of:

const hasAdmin = user?.groups?.some((g: { permissions: string[] }) =>
    g.permissions.includes('ADMIN')
);
if (!hasAdmin) throw error(403, getErrorMessage('FORBIDDEN'));

With a shared layout, this belongs in +layout.server.ts once:

// admin/+layout.server.ts
export async function load({ locals }) {
    const user = locals.user;
    const hasAdmin = user?.groups?.some((g: { permissions: string[] }) =>
        g.permissions.includes('ADMIN')
    );
    if (!hasAdmin) throw error(403, getErrorMessage('FORBIDDEN'));
    return {};
}

All child load functions inherit this guard. Remove the duplicates.


Decision 3: Fetch only what each route needs — no upfront triple-fetch

The current routes/admin/+page.server.ts:24 fetches users + groups + tags in parallel on every page load, regardless of which tab was active. With sub-routes, this is gone entirely:

Route What to fetch
+layout.server.ts Nothing (auth guard only)
admin/users/+page.server.ts /api/users
admin/users/[id]/+page.server.ts /api/users/{id} + /api/groups (for group assignment — already correct)
admin/groups/+page.server.ts /api/groups
admin/groups/[id]/+page.server.ts /api/groups/{id} + /api/users (for member management)
admin/tags/+page.server.ts /api/tags
admin/system/+page.svelte Nothing at load time — maintenance ops are user-triggered

This gives you genuinely lazy loading: opening the tags panel does not fetch the users list.


Decision 4: Unsaved-changes guard via beforeNavigate

The spec says "inline warning in detail panel when navigating away (not a modal)." With sub-routes, beforeNavigate from $app/navigation fires on every route change — exactly what you need.

Pattern to use in detail panel components:

import { beforeNavigate, goto } from '$app/navigation';

let isDirty = $state(false);
let showUnsavedWarning = $state(false);
let blockedDestination: URL | null = null;

beforeNavigate(({ cancel, to }) => {
    if (isDirty && to) {
        cancel();
        blockedDestination = to.url;
        showUnsavedWarning = true;
    }
});

function discardAndNavigate() {
    isDirty = false;
    showUnsavedWarning = false;
    goto(blockedDestination!);
}

The isDirty flag is set to true on any oninput event in the form, and reset to false after a successful save or explicit discard. The warning renders inline in the detail panel — no modal, as the spec requires.

Important: isDirty must be reset to false after enhance completes successfully. Use the update callback:

use:enhance={() => async ({ result, update }) => {
    await update();
    if (result.type === 'success') isDirty = false;
}}

Decision 5: No persistence for collapse state

The spec calls for localStorage persistence of the list panel collapse state. Drop this.

Initialising from localStorage server-side is impossible; reading it in onMount causes a visible layout shift on every page load (panel renders collapsed → expands, or vice versa). The admin panel is used infrequently enough that re-collapsing on each visit is not a meaningful UX cost.

Implementation: let listCollapsed = $state(false) in the layout. Resets on navigation. Done.


What to fix before starting the layout redesign

Three of the critical defects in the UX audit are one-line fixes that should not wait for the full redesign milestone. They are production accessibility defects:

1. Tag hover buttons — routes/admin/TagsTab.svelte:79

<!-- current — broken on touch -->
<div class="flex items-center gap-2 opacity-0 transition-opacity group-hover:opacity-100">

<!-- fix -->
<div class="flex items-center gap-2">

2. Users table — no horizontal scroll, routes/admin/UsersTab.svelte:32

<!-- wrap the table -->
<div class="overflow-x-auto">
    <table class="min-w-full divide-y divide-line">

3. ADMIN badge — color-only indicator, routes/admin/GroupsTab.svelte:124

<!-- current — WCAG 1.4.1 failure, red color only -->
{perm === 'ADMIN' ? 'border-red-100 bg-red-50 text-red-700' : '...'}

<!-- fix — add a non-color indicator, e.g. a shield icon or ★ prefix -->

These three can be committed to main independently today. Do not hold them for the layout redesign milestone.


Form action re-homing

The current routes/admin/+page.server.ts owns all mutations: deleteUser, updateTag, deleteTag, createGroup, updateGroup, deleteGroup. With sub-routes, these need to move to their natural homes:

Action Move to
deleteUser admin/users/[id]/+page.server.ts (spec puts delete in the detail danger zone)
updateTag, deleteTag admin/tags/+page.server.ts
createGroup admin/groups/+page.server.ts
updateGroup, deleteGroup admin/groups/[id]/+page.server.ts

The current routes/admin/+page.server.ts becomes +layout.server.ts (auth guard only) plus +page.server.ts (redirect to /admin/users). Its actions are distributed to the routes above and can be deleted from the current file.


Summary of implementation order

  1. Hotfix now on main: tag hover visibility, users table scroll, ADMIN badge non-color indicator.
  2. Layout shell first: admin/+layout.svelte + admin/+layout.server.ts (auth guard). Get the three-panel chrome rendering at all three breakpoints with static placeholder content before touching any entity logic.
  3. Migrate one entity at a time: Users first (routes already exist), then Groups, Tags, System. Each entity is a self-contained PR.
  4. Unsaved-changes guard last: add beforeNavigate pattern to each detail panel once the routing is stable.

— @mkeller

## Architectural Review — Concept C Implementation **@mkeller · 2026-03-29** I have read the spec and the current code. The UX problems are real and worth fixing. Before implementation starts, four structural decisions need to be locked down. I am recording them here so they are not re-litigated during the PR. --- ### Decision 1: Sub-routes with a shared layout shell ✅ **Use Option B: `/admin/+layout.svelte` as the three-panel shell, sub-routes render into the detail slot.** The spec leaves this open ("may be absorbed… or kept as separate routes"). It should not be left open — this is the load-bearing decision for everything else. **Proposed route tree:** ``` frontend/src/routes/admin/ ├── +layout.svelte ← three-panel shell (entity nav + list slot + detail slot) ├── +layout.server.ts ← auth guard (once, here — not per-page) ├── +page.svelte ← redirects to /admin/users ├── users/ │ ├── +page.svelte ← list panel content │ ├── +page.server.ts ← loads users list only │ ├── new/ ← already exists, keep it │ └── [id]/ ← already exists, keep it ├── groups/ │ ├── +page.svelte │ ├── +page.server.ts ← loads groups list only │ └── [id]/ │ ├── +page.svelte │ └── +page.server.ts ← loads one group + members ├── tags/ │ ├── +page.svelte │ └── +page.server.ts ← loads tags list only └── system/ └── +page.svelte ← no server load needed ``` **Why this is correct:** - SvelteKit's router handles deep links, browser back, and mobile push-flow navigation (spec ACs) for free. No custom navigation wrapper needed. - `beforeNavigate` (see Decision 4 below) works cleanly on route changes. - Panel transitions use SvelteKit's page transition hooks — no manual history management. - The sub-routes for users already exist (`/admin/users/[id]`, `/admin/users/new`). This is an evolution of what is there, not a rewrite. --- ### Decision 2: Move the auth guard to `+layout.server.ts` Currently the `hasAdmin` check is duplicated in every load function. Both `routes/admin/+page.server.ts:17` and `routes/admin/users/[id]/+page.server.ts:8` contain identical copies of: ```typescript const hasAdmin = user?.groups?.some((g: { permissions: string[] }) => g.permissions.includes('ADMIN') ); if (!hasAdmin) throw error(403, getErrorMessage('FORBIDDEN')); ``` With a shared layout, this belongs in `+layout.server.ts` once: ```typescript // admin/+layout.server.ts export async function load({ locals }) { const user = locals.user; const hasAdmin = user?.groups?.some((g: { permissions: string[] }) => g.permissions.includes('ADMIN') ); if (!hasAdmin) throw error(403, getErrorMessage('FORBIDDEN')); return {}; } ``` All child `load` functions inherit this guard. Remove the duplicates. --- ### Decision 3: Fetch only what each route needs — no upfront triple-fetch The current `routes/admin/+page.server.ts:24` fetches users + groups + tags in parallel on every page load, regardless of which tab was active. With sub-routes, this is gone entirely: | Route | What to fetch | |---|---| | `+layout.server.ts` | Nothing (auth guard only) | | `admin/users/+page.server.ts` | `/api/users` | | `admin/users/[id]/+page.server.ts` | `/api/users/{id}` + `/api/groups` (for group assignment — already correct) | | `admin/groups/+page.server.ts` | `/api/groups` | | `admin/groups/[id]/+page.server.ts` | `/api/groups/{id}` + `/api/users` (for member management) | | `admin/tags/+page.server.ts` | `/api/tags` | | `admin/system/+page.svelte` | Nothing at load time — maintenance ops are user-triggered | This gives you genuinely lazy loading: opening the tags panel does not fetch the users list. --- ### Decision 4: Unsaved-changes guard via `beforeNavigate` The spec says "inline warning in detail panel when navigating away (not a modal)." With sub-routes, `beforeNavigate` from `$app/navigation` fires on every route change — exactly what you need. **Pattern to use in detail panel components:** ```typescript import { beforeNavigate, goto } from '$app/navigation'; let isDirty = $state(false); let showUnsavedWarning = $state(false); let blockedDestination: URL | null = null; beforeNavigate(({ cancel, to }) => { if (isDirty && to) { cancel(); blockedDestination = to.url; showUnsavedWarning = true; } }); function discardAndNavigate() { isDirty = false; showUnsavedWarning = false; goto(blockedDestination!); } ``` The `isDirty` flag is set to `true` on any `oninput` event in the form, and reset to `false` after a successful save or explicit discard. The warning renders inline in the detail panel — no modal, as the spec requires. **Important:** `isDirty` must be reset to `false` after `enhance` completes successfully. Use the `update` callback: ```typescript use:enhance={() => async ({ result, update }) => { await update(); if (result.type === 'success') isDirty = false; }} ``` --- ### Decision 5: No persistence for collapse state The spec calls for `localStorage` persistence of the list panel collapse state. **Drop this.** Initialising from `localStorage` server-side is impossible; reading it in `onMount` causes a visible layout shift on every page load (panel renders collapsed → expands, or vice versa). The admin panel is used infrequently enough that re-collapsing on each visit is not a meaningful UX cost. Implementation: `let listCollapsed = $state(false)` in the layout. Resets on navigation. Done. --- ### What to fix before starting the layout redesign Three of the critical defects in the UX audit are one-line fixes that should not wait for the full redesign milestone. They are production accessibility defects: **1. Tag hover buttons — `routes/admin/TagsTab.svelte:79`** ```svelte <!-- current — broken on touch --> <div class="flex items-center gap-2 opacity-0 transition-opacity group-hover:opacity-100"> <!-- fix --> <div class="flex items-center gap-2"> ``` **2. Users table — no horizontal scroll, `routes/admin/UsersTab.svelte:32`** ```svelte <!-- wrap the table --> <div class="overflow-x-auto"> <table class="min-w-full divide-y divide-line"> ``` **3. ADMIN badge — color-only indicator, `routes/admin/GroupsTab.svelte:124`** ```svelte <!-- current — WCAG 1.4.1 failure, red color only --> {perm === 'ADMIN' ? 'border-red-100 bg-red-50 text-red-700' : '...'} <!-- fix — add a non-color indicator, e.g. a shield icon or ★ prefix --> ``` These three can be committed to `main` independently today. Do not hold them for the layout redesign milestone. --- ### Form action re-homing The current `routes/admin/+page.server.ts` owns all mutations: `deleteUser`, `updateTag`, `deleteTag`, `createGroup`, `updateGroup`, `deleteGroup`. With sub-routes, these need to move to their natural homes: | Action | Move to | |---|---| | `deleteUser` | `admin/users/[id]/+page.server.ts` (spec puts delete in the detail danger zone) | | `updateTag`, `deleteTag` | `admin/tags/+page.server.ts` | | `createGroup` | `admin/groups/+page.server.ts` | | `updateGroup`, `deleteGroup` | `admin/groups/[id]/+page.server.ts` | The current `routes/admin/+page.server.ts` becomes `+layout.server.ts` (auth guard only) plus `+page.server.ts` (redirect to `/admin/users`). Its actions are distributed to the routes above and can be deleted from the current file. --- ### Summary of implementation order 1. **Hotfix now on `main`**: tag hover visibility, users table scroll, ADMIN badge non-color indicator. 2. **Layout shell first**: `admin/+layout.svelte` + `admin/+layout.server.ts` (auth guard). Get the three-panel chrome rendering at all three breakpoints with static placeholder content before touching any entity logic. 3. **Migrate one entity at a time**: Users first (routes already exist), then Groups, Tags, System. Each entity is a self-contained PR. 4. **Unsaved-changes guard last**: add `beforeNavigate` pattern to each detail panel once the routing is stable. — @mkeller
Author
Owner

Test Plan — Admin Redesign (Concept C)

@saraholt · 2026-03-29

Read the spec, read Markus's architectural review, read the existing test files. Here is what the suite needs to look like after this feature lands.


What changes in the existing suite

Existing file Status Action
admin/page.server.spec.ts Tests the monolithic load that fetches users+groups+tags at once — that function is being deleted Delete, split into per-route specs below
admin/page.svelte.spec.ts Tests the tab-based +page.svelte — the tab layout is gone Delete, replace with layout shell + per-entity list specs
admin/users/new/page.svelte.spec.ts Survives, but cancel/back link hrefs point to /admin — that changes to /admin/users Update href assertions
admin/users/[id]/page.svelte.spec.ts Cancel link href needs updating; unsaved-changes tests are missing Extend

Layer 2 — Load Function Tests (Vitest)

admin/+layout.server.spec.ts (new — absorbs auth tests from page.server.spec.ts)

admin layout load — permission check
  ✦ throws 403 when user has no ADMIN permission
  ✦ throws 403 when user is undefined
  ✦ throws 403 when user has no groups
  ✦ returns empty object for a valid admin user

admin/users/+page.server.spec.ts (new)

admin users load
  ✦ fetches only /api/users (not groups, not tags)
  ✦ returns users array on success
  ✦ returns empty array when API returns no data
  ✦ propagates error when API call fails

admin/groups/+page.server.spec.ts (new)

  ✦ fetches only /api/groups
  ✦ returns groups array on success
  ✦ returns empty array when API returns no data

admin/groups/[id]/+page.server.spec.ts (new)

  ✦ fetches /api/groups/{id} and /api/users in parallel (member management needs user list)
  ✦ throws 404 when group does not exist

admin/tags/+page.server.spec.ts (new)

  ✦ fetches only /api/tags
  ✦ returns tags array on success
  ✦ returns empty array when API returns no data

Layer 2 — Component Tests (Vitest + vitest-browser-svelte)

admin/+layout.svelte.spec.ts (new — the three-panel shell)

admin layout shell — entity navigation
  ✦ renders all four nav items (Users, Groups, Tags, System)
  ✦ each nav item links to its route
  ✦ icon-only buttons have aria-label (WCAG — required by spec)

admin layout shell — list panel visibility
  ✦ list slot rendered for non-system routes
  ✦ list panel hidden and detail fills width on /admin/system

admin layout shell — collapse toggle
  ✦ collapse button is present
  ✦ clicking it toggles listCollapsed
  ✦ when listCollapsed=true the list renders as 32px handle with rotated label
  ✦ collapse state is NOT read from localStorage on mount (Decision 5)

admin layout shell — auto-collapse on new
  ✦ navigating to a /new sub-route sets listCollapsed=true

admin/users/+page.svelte.spec.ts (new — replaces users section of current page.svelte.spec.ts)

admin users list — rendering
  ✦ shows username in list row
  ✦ shows full name (first + last)
  ✦ shows dash when user has no name
  ✦ shows group chips for each group
  ✦ shows "Keine Gruppen" when user has no groups
  ✦ row links to /admin/users/[id]
  ✦ "Neuer Benutzer" button links to /admin/users/new

admin users list — empty state
  ✦ shows illustration and CTA when users=[]
  ✦ CTA links to /admin/users/new

admin/groups/+page.svelte.spec.ts (new)

  ✦ shows group name, member count, permission summary chips
  ✦ row links to /admin/groups/[id]
  ✦ empty state: illustration + CTA when groups=[]

admin/groups/[id]/+page.svelte.spec.ts (new)

admin group detail — rendering
  ✦ group name input pre-filled with group.name
  ✦ permissions split into Standard section and Administrative section
  ✦ ADMIN permission renders in a red-bordered box with checkbox + text label — not color alone (WCAG 1.4.1)
  ✦ non-admin permissions rendered outside the danger box
  ✦ member list shows current members; add-member UI present

admin group detail — error feedback
  ✦ error banner shown when form.error is set (changes NOT lost)
  ✦ success feedback shown when form.success is true

admin group detail — unsaved changes guard
  ✦ isDirty is false on initial render
  ✦ isDirty becomes true after any oninput event
  ✦ isDirty resets to false after successful enhance callback
  ✦ inline warning shown when isDirty=true and navigation attempted
  ✦ warning is NOT a <dialog> element (spec is explicit on this)
  ✦ "Discard" clears the warning and resets isDirty

admin/tags/+page.svelte.spec.ts (new)

admin tags list — rendering
  ✦ shows tag name chip and document count
  ✦ action buttons always visible — NOT conditional on hover/opacity-0 (regression guard for the hotfix)
  ✦ delete button label includes document count: "Tag löschen (12 Dokumente)"
  ✦ "Renaming affects N documents" warning shown on rename action

admin tags list — empty state
  ✦ illustration + CTA when tags=[]

admin/users/[id]/+page.svelte.spec.ts (extend existing)

[all existing tests kept as-is]

admin edit user — link targets  [UPDATE]
  ✦ cancel link points to /admin/users (currently asserts /admin — update this)
  ✦ back link points to /admin/users (currently asserts /admin — update this)

admin edit user — unsaved changes guard  [NEW]
  ✦ isDirty is false on initial render
  ✦ isDirty becomes true after oninput on any field
  ✦ isDirty resets to false after successful enhance callback
  ✦ inline unsaved warning rendered when isDirty=true (not a <dialog>)
  ✦ discard action clears the warning

admin edit user — delete danger zone  [NEW]
  ✦ type-to-confirm input is present
  ✦ delete submit button disabled until username typed correctly

admin/system/+page.svelte.spec.ts (new)

  ✦ renders a card for each maintenance operation
  ✦ idle state: CTA visible, spinner and success banner absent
  ✦ clicking CTA expands inline confirm (no page navigation)
  ✦ confirming shows spinner and disables the button
  ✦ success state: success banner with affected count visible
  ✦ error state: error banner with retry button visible

Layer 4 — E2E (Playwright) — frontend/e2e/admin.spec.ts

checkA11y runs on every page visit. Critical journeys only — permutations belong in unit/component layer.

✦ create user end-to-end: fill form → submit → user appears in list
✦ edit user and change group assignment: toggle checkbox → save → persists
✦ rename tag
✦ delete tag: confirmation button label contains document count
✦ create group with ADMIN permission: verify non-color indicator present (axe catches color-only)
✦ unsaved changes guard: edit → navigate away → inline warning visible → discard navigates
✦ direct URL load: /admin/users?id=[uuid] pre-selects user on first paint (SSR smoke)
✦ /admin/system hides the list panel
✦ system maintenance: idle → confirm expand → spinner → success banner with count

mobile (375px viewport)
  ✦ entity picker visible, list hidden on load
  ✦ tap entity → list visible, picker gone
  ✦ tap list item → form visible, list gone
  ✦ browser back: form → list → entity picker

tablet (768px viewport)
  ✦ default: list panel at ~200px, detail at ~520px
  ✦ collapse ‹ button: list shrinks to 32px handle, detail widens
  ✦ + New: list auto-collapses
  ✦ entity icon tap: flyout overlay has role="dialog", aria-modal, focus trapped
  ✦ checkA11y with flyout open

visual regression snapshots (light + dark)
  ✦ desktop 1440px: three-panel shell, user list, user detail
  ✦ tablet 768px: default state + list collapsed
  ✦ mobile 375px: entity picker, list, detail (separate shots)

CI impact estimate

Layer New tests Added time
Unit / load ~30 cases +3–5 s
Component ~65 cases +15–20 s
E2E ~13 journeys +90–120 s

All within pyramid targets.


Pre-condition before any of the above

The three hotfixes Markus flagged (tag opacity-0 removal, users table overflow-x-auto, ADMIN badge non-color indicator) ship to main first as independent commits. The tag button visibility test gets written red first before the fix lands — that is the one that has been failing in production silently.

— @saraholt

## Test Plan — Admin Redesign (Concept C) **@saraholt · 2026-03-29** Read the spec, read Markus's architectural review, read the existing test files. Here is what the suite needs to look like after this feature lands. --- ### What changes in the existing suite | Existing file | Status | Action | |---|---|---| | `admin/page.server.spec.ts` | Tests the monolithic load that fetches users+groups+tags at once — that function is being deleted | **Delete**, split into per-route specs below | | `admin/page.svelte.spec.ts` | Tests the tab-based `+page.svelte` — the tab layout is gone | **Delete**, replace with layout shell + per-entity list specs | | `admin/users/new/page.svelte.spec.ts` | Survives, but cancel/back link hrefs point to `/admin` — that changes to `/admin/users` | **Update** href assertions | | `admin/users/[id]/page.svelte.spec.ts` | Cancel link href needs updating; unsaved-changes tests are missing | **Extend** | --- ### Layer 2 — Load Function Tests (Vitest) #### `admin/+layout.server.spec.ts` *(new — absorbs auth tests from `page.server.spec.ts`)* ``` admin layout load — permission check ✦ throws 403 when user has no ADMIN permission ✦ throws 403 when user is undefined ✦ throws 403 when user has no groups ✦ returns empty object for a valid admin user ``` #### `admin/users/+page.server.spec.ts` *(new)* ``` admin users load ✦ fetches only /api/users (not groups, not tags) ✦ returns users array on success ✦ returns empty array when API returns no data ✦ propagates error when API call fails ``` #### `admin/groups/+page.server.spec.ts` *(new)* ``` ✦ fetches only /api/groups ✦ returns groups array on success ✦ returns empty array when API returns no data ``` #### `admin/groups/[id]/+page.server.spec.ts` *(new)* ``` ✦ fetches /api/groups/{id} and /api/users in parallel (member management needs user list) ✦ throws 404 when group does not exist ``` #### `admin/tags/+page.server.spec.ts` *(new)* ``` ✦ fetches only /api/tags ✦ returns tags array on success ✦ returns empty array when API returns no data ``` --- ### Layer 2 — Component Tests (Vitest + vitest-browser-svelte) #### `admin/+layout.svelte.spec.ts` *(new — the three-panel shell)* ``` admin layout shell — entity navigation ✦ renders all four nav items (Users, Groups, Tags, System) ✦ each nav item links to its route ✦ icon-only buttons have aria-label (WCAG — required by spec) admin layout shell — list panel visibility ✦ list slot rendered for non-system routes ✦ list panel hidden and detail fills width on /admin/system admin layout shell — collapse toggle ✦ collapse button is present ✦ clicking it toggles listCollapsed ✦ when listCollapsed=true the list renders as 32px handle with rotated label ✦ collapse state is NOT read from localStorage on mount (Decision 5) admin layout shell — auto-collapse on new ✦ navigating to a /new sub-route sets listCollapsed=true ``` #### `admin/users/+page.svelte.spec.ts` *(new — replaces users section of current page.svelte.spec.ts)* ``` admin users list — rendering ✦ shows username in list row ✦ shows full name (first + last) ✦ shows dash when user has no name ✦ shows group chips for each group ✦ shows "Keine Gruppen" when user has no groups ✦ row links to /admin/users/[id] ✦ "Neuer Benutzer" button links to /admin/users/new admin users list — empty state ✦ shows illustration and CTA when users=[] ✦ CTA links to /admin/users/new ``` #### `admin/groups/+page.svelte.spec.ts` *(new)* ``` ✦ shows group name, member count, permission summary chips ✦ row links to /admin/groups/[id] ✦ empty state: illustration + CTA when groups=[] ``` #### `admin/groups/[id]/+page.svelte.spec.ts` *(new)* ``` admin group detail — rendering ✦ group name input pre-filled with group.name ✦ permissions split into Standard section and Administrative section ✦ ADMIN permission renders in a red-bordered box with checkbox + text label — not color alone (WCAG 1.4.1) ✦ non-admin permissions rendered outside the danger box ✦ member list shows current members; add-member UI present admin group detail — error feedback ✦ error banner shown when form.error is set (changes NOT lost) ✦ success feedback shown when form.success is true admin group detail — unsaved changes guard ✦ isDirty is false on initial render ✦ isDirty becomes true after any oninput event ✦ isDirty resets to false after successful enhance callback ✦ inline warning shown when isDirty=true and navigation attempted ✦ warning is NOT a <dialog> element (spec is explicit on this) ✦ "Discard" clears the warning and resets isDirty ``` #### `admin/tags/+page.svelte.spec.ts` *(new)* ``` admin tags list — rendering ✦ shows tag name chip and document count ✦ action buttons always visible — NOT conditional on hover/opacity-0 (regression guard for the hotfix) ✦ delete button label includes document count: "Tag löschen (12 Dokumente)" ✦ "Renaming affects N documents" warning shown on rename action admin tags list — empty state ✦ illustration + CTA when tags=[] ``` #### `admin/users/[id]/+page.svelte.spec.ts` *(extend existing)* ``` [all existing tests kept as-is] admin edit user — link targets [UPDATE] ✦ cancel link points to /admin/users (currently asserts /admin — update this) ✦ back link points to /admin/users (currently asserts /admin — update this) admin edit user — unsaved changes guard [NEW] ✦ isDirty is false on initial render ✦ isDirty becomes true after oninput on any field ✦ isDirty resets to false after successful enhance callback ✦ inline unsaved warning rendered when isDirty=true (not a <dialog>) ✦ discard action clears the warning admin edit user — delete danger zone [NEW] ✦ type-to-confirm input is present ✦ delete submit button disabled until username typed correctly ``` #### `admin/system/+page.svelte.spec.ts` *(new)* ``` ✦ renders a card for each maintenance operation ✦ idle state: CTA visible, spinner and success banner absent ✦ clicking CTA expands inline confirm (no page navigation) ✦ confirming shows spinner and disables the button ✦ success state: success banner with affected count visible ✦ error state: error banner with retry button visible ``` --- ### Layer 4 — E2E (Playwright) — `frontend/e2e/admin.spec.ts` `checkA11y` runs on every page visit. Critical journeys only — permutations belong in unit/component layer. ``` ✦ create user end-to-end: fill form → submit → user appears in list ✦ edit user and change group assignment: toggle checkbox → save → persists ✦ rename tag ✦ delete tag: confirmation button label contains document count ✦ create group with ADMIN permission: verify non-color indicator present (axe catches color-only) ✦ unsaved changes guard: edit → navigate away → inline warning visible → discard navigates ✦ direct URL load: /admin/users?id=[uuid] pre-selects user on first paint (SSR smoke) ✦ /admin/system hides the list panel ✦ system maintenance: idle → confirm expand → spinner → success banner with count mobile (375px viewport) ✦ entity picker visible, list hidden on load ✦ tap entity → list visible, picker gone ✦ tap list item → form visible, list gone ✦ browser back: form → list → entity picker tablet (768px viewport) ✦ default: list panel at ~200px, detail at ~520px ✦ collapse ‹ button: list shrinks to 32px handle, detail widens ✦ + New: list auto-collapses ✦ entity icon tap: flyout overlay has role="dialog", aria-modal, focus trapped ✦ checkA11y with flyout open visual regression snapshots (light + dark) ✦ desktop 1440px: three-panel shell, user list, user detail ✦ tablet 768px: default state + list collapsed ✦ mobile 375px: entity picker, list, detail (separate shots) ``` --- ### CI impact estimate | Layer | New tests | Added time | |---|---|---| | Unit / load | ~30 cases | +3–5 s | | Component | ~65 cases | +15–20 s | | E2E | ~13 journeys | +90–120 s | All within pyramid targets. --- ### Pre-condition before any of the above The three hotfixes Markus flagged (tag `opacity-0` removal, users table `overflow-x-auto`, ADMIN badge non-color indicator) ship to `main` first as independent commits. The tag button visibility test gets written **red first** before the fix lands — that is the one that has been failing in production silently. — @saraholt
Author
Owner

Security Review — Concept C Admin Redesign

Reviewer: Nora "NullX" Steiner · OSWE · BSCP

The feature spec is well-structured and the WCAG fixes are welcome. The backend permission enforcement via @RequirePermission / PermissionAspect is a solid centralized control point. However, I found one pre-existing IDOR that this feature will amplify, and one authorization model mismatch that the new multi-panel UI will expose more sharply than the current tab layout does.


🔴 CRITICAL — C1: IDOR on GET /api/users/{id} (CWE-862)

Location: UserController.java:64-68

// ❌ MISSING @RequirePermission — any authenticated user can hit this
@GetMapping("users/{id}")
public ResponseEntity<AppUser> getUser(@PathVariable UUID id) {
    AppUser user = userService.getById(id);
    user.setPassword(null);  // hash is nulled, but groups/email/name are not
    return ResponseEntity.ok(user);
}

Why this matters now: The new spec encodes user UUIDs in the URL (/admin?v=users&id=<uuid>). Once any authenticated user sees that URL scheme (shared link, browser history, dev tools), they can enumerate any user's profile directly:

GET /api/users/550e8400-e29b-41d4-a716-446655440000
Authorization: Basic <any valid credentials>
→ 200: { id, login, firstName, lastName, email, groups: [{ permissions: [...] }] }

The response reveals group membership and permission sets for every user in the system. This was already exploitable, but it becomes significantly more discoverable with UUIDs in the address bar.

Fix:

@GetMapping("users/{id}")
@RequirePermission(Permission.ADMIN_USER)  // ← add this
public ResponseEntity<AppUser> getUser(@PathVariable UUID id) {
    AppUser user = userService.getById(id);
    user.setPassword(null);
    return ResponseEntity.ok(user);
}

⚠️ This should be fixed before the new URL-param scheme ships.


🟠 HIGH — H1: Frontend gate checks ADMIN, backend uses granular permissions — the new UI exposes the mismatch

Location: +page.server.ts:17-20 vs. GroupController.java:25, TagController.java:33

The backend has four distinct admin permissions: ADMIN, ADMIN_USER, ADMIN_TAG, ADMIN_PERMISSION. The current tab layout blurs this because everything is on one page. The new three-panel layout makes each entity first-class — a user with only ADMIN_TAG is supposed to manage tags but currently can't even reach /admin.

Two options — pick one and document it explicitly:

Option A — flatten to ADMIN only (current behaviour, explicit):
Document that ADMIN_USER/TAG/PERMISSION are back-channel API-layer protections; the /admin frontend is gated on the omnibus ADMIN flag. Add a comment in the load function so future developers don't expect partial-admin access to work end-to-end.

Option B — honour granularity in the new UI:

// +page.server.ts load()
const canManageUsers    = hasPerm(user, 'ADMIN_USER');
const canManageTags     = hasPerm(user, 'ADMIN_TAG');
const canManageGroups   = hasPerm(user, 'ADMIN_PERMISSION');
const canRunMaintenance = hasPerm(user, 'ADMIN');

if (!canManageUsers && !canManageTags && !canManageGroups && !canRunMaintenance) {
    throw error(403, ...);
}
// Pass flags to conditionally render entity-nav items
return { canManageUsers, canManageTags, canManageGroups, canRunMaintenance };

🟡 MEDIUM — M1: ?v= parameter needs an allowlist

The ?v= param drives panel rendering. Without a validation step, unexpected values reach conditional rendering logic (JS errors, blank panels).

const VALID_VIEWS = ['users', 'groups', 'tags', 'system'] as const;
type AdminView = typeof VALID_VIEWS[number];

const rawView = url.searchParams.get('v') ?? 'users';
const view: AdminView = VALID_VIEWS.includes(rawView as AdminView)
    ? (rawView as AdminView)
    : 'users';

SecurityConfig disables CSRF with the justification that Basic Auth requires a custom Authorization header (which browsers block cross-origin). This reasoning is sound as long as the auth model stays Basic Auth.

Spring Session JDBC is in the stack. If session cookies are ever used for auth propagation, the CSRF justification breaks silently — and the backfill endpoints (POST /api/admin/backfill-*) are high-impact targets.

Two concrete checks for the System tab implementation:

  1. Confirm server.servlet.session.cookie.same-site=Lax (or Strict) in application.properties.
  2. Route the System tab confirm-POSTs through SvelteKit server actions (which inject the Authorization header), not bare client-side fetch().

🔵 LOW — L1: localStorage — scope explicitly to UI preferences

admin_list_collapsed is fine. The risk is future developers adding convenient caching (last-selected user ID, API response caches) to the same storage area.

// ⚠️ localStorage in this component is ONLY for UI layout preferences.
// Never cache entity IDs, usernames, permissions, or API responses here.
localStorage.setItem('admin_list_collapsed', String(listCollapsed));

Positive findings (keep these)

  • PermissionAspect with class-level @RequirePermission on AdminController and GroupController is correct — not missing.
  • user.setPassword(null) is consistently applied before returning AppUser — the new detail panel won't accidentally leak password hashes.
  • The unsaved-changes guard as inline warning (not a modal) is the right call — a modal would be bypassable via browser back.
  • The WCAG fix for ADMIN permission (checkbox + label, not color alone) is correctly scoped.
  • Type-to-confirm delete adds meaningful friction against accidental destructive operations.

Action Items

Priority Item Location
🔴 Critical Add @RequirePermission(Permission.ADMIN_USER) to GET /api/users/{id} UserController.java:64
🟠 High Decide and document ADMIN vs. granular-perm policy; update frontend gate to match +page.server.ts
🟡 Medium Validate ?v= against allowlist in load() new admin +page.server.ts
🟡 Medium Confirm SameSite on session cookie; route System tab POSTs through server actions application.properties + System tab component
🔵 Low Add localStorage scope comment admin layout component
## Security Review — Concept C Admin Redesign **Reviewer:** Nora "NullX" Steiner · OSWE · BSCP The feature spec is well-structured and the WCAG fixes are welcome. The backend permission enforcement via `@RequirePermission` / `PermissionAspect` is a solid centralized control point. However, I found **one pre-existing IDOR that this feature will amplify**, and **one authorization model mismatch** that the new multi-panel UI will expose more sharply than the current tab layout does. --- ### 🔴 CRITICAL — C1: IDOR on `GET /api/users/{id}` (CWE-862) **Location:** `UserController.java:64-68` ```java // ❌ MISSING @RequirePermission — any authenticated user can hit this @GetMapping("users/{id}") public ResponseEntity<AppUser> getUser(@PathVariable UUID id) { AppUser user = userService.getById(id); user.setPassword(null); // hash is nulled, but groups/email/name are not return ResponseEntity.ok(user); } ``` **Why this matters now:** The new spec encodes user UUIDs in the URL (`/admin?v=users&id=<uuid>`). Once any authenticated user sees that URL scheme (shared link, browser history, dev tools), they can enumerate any user's profile directly: ``` GET /api/users/550e8400-e29b-41d4-a716-446655440000 Authorization: Basic <any valid credentials> → 200: { id, login, firstName, lastName, email, groups: [{ permissions: [...] }] } ``` The response reveals group membership and permission sets for every user in the system. This was already exploitable, but it becomes significantly more discoverable with UUIDs in the address bar. **Fix:** ```java @GetMapping("users/{id}") @RequirePermission(Permission.ADMIN_USER) // ← add this public ResponseEntity<AppUser> getUser(@PathVariable UUID id) { AppUser user = userService.getById(id); user.setPassword(null); return ResponseEntity.ok(user); } ``` > ⚠️ This should be fixed before the new URL-param scheme ships. --- ### 🟠 HIGH — H1: Frontend gate checks `ADMIN`, backend uses granular permissions — the new UI exposes the mismatch **Location:** `+page.server.ts:17-20` vs. `GroupController.java:25`, `TagController.java:33` The backend has four distinct admin permissions: `ADMIN`, `ADMIN_USER`, `ADMIN_TAG`, `ADMIN_PERMISSION`. The current tab layout blurs this because everything is on one page. The new three-panel layout makes each entity first-class — a user with only `ADMIN_TAG` is supposed to manage tags but currently can't even reach `/admin`. Two options — pick one and document it explicitly: **Option A — flatten to `ADMIN` only (current behaviour, explicit):** Document that `ADMIN_USER/TAG/PERMISSION` are back-channel API-layer protections; the `/admin` frontend is gated on the omnibus `ADMIN` flag. Add a comment in the load function so future developers don't expect partial-admin access to work end-to-end. **Option B — honour granularity in the new UI:** ```typescript // +page.server.ts load() const canManageUsers = hasPerm(user, 'ADMIN_USER'); const canManageTags = hasPerm(user, 'ADMIN_TAG'); const canManageGroups = hasPerm(user, 'ADMIN_PERMISSION'); const canRunMaintenance = hasPerm(user, 'ADMIN'); if (!canManageUsers && !canManageTags && !canManageGroups && !canRunMaintenance) { throw error(403, ...); } // Pass flags to conditionally render entity-nav items return { canManageUsers, canManageTags, canManageGroups, canRunMaintenance }; ``` --- ### 🟡 MEDIUM — M1: `?v=` parameter needs an allowlist The `?v=` param drives panel rendering. Without a validation step, unexpected values reach conditional rendering logic (JS errors, blank panels). ```typescript const VALID_VIEWS = ['users', 'groups', 'tags', 'system'] as const; type AdminView = typeof VALID_VIEWS[number]; const rawView = url.searchParams.get('v') ?? 'users'; const view: AdminView = VALID_VIEWS.includes(rawView as AdminView) ? (rawView as AdminView) : 'users'; ``` --- ### 🟡 MEDIUM — M2: Verify `SameSite` on session cookie for maintenance operations `SecurityConfig` disables CSRF with the justification that Basic Auth requires a custom `Authorization` header (which browsers block cross-origin). This reasoning is sound **as long as the auth model stays Basic Auth**. `Spring Session JDBC` is in the stack. If session cookies are ever used for auth propagation, the CSRF justification breaks silently — and the backfill endpoints (`POST /api/admin/backfill-*`) are high-impact targets. Two concrete checks for the System tab implementation: 1. Confirm `server.servlet.session.cookie.same-site=Lax` (or `Strict`) in `application.properties`. 2. Route the System tab confirm-POSTs through SvelteKit server actions (which inject the `Authorization` header), not bare client-side `fetch()`. --- ### 🔵 LOW — L1: `localStorage` — scope explicitly to UI preferences `admin_list_collapsed` is fine. The risk is future developers adding convenient caching (last-selected user ID, API response caches) to the same storage area. ```typescript // ⚠️ localStorage in this component is ONLY for UI layout preferences. // Never cache entity IDs, usernames, permissions, or API responses here. localStorage.setItem('admin_list_collapsed', String(listCollapsed)); ``` --- ### ✅ Positive findings (keep these) - `PermissionAspect` with class-level `@RequirePermission` on `AdminController` and `GroupController` is correct — not missing. - `user.setPassword(null)` is consistently applied before returning `AppUser` — the new detail panel won't accidentally leak password hashes. - The unsaved-changes guard as **inline warning** (not a modal) is the right call — a modal would be bypassable via browser back. - The WCAG fix for ADMIN permission (checkbox + label, not color alone) is correctly scoped. - Type-to-confirm delete adds meaningful friction against accidental destructive operations. --- ### Action Items | Priority | Item | Location | |---|---|---| | 🔴 Critical | Add `@RequirePermission(Permission.ADMIN_USER)` to `GET /api/users/{id}` | `UserController.java:64` | | 🟠 High | Decide and document `ADMIN` vs. granular-perm policy; update frontend gate to match | `+page.server.ts` | | 🟡 Medium | Validate `?v=` against allowlist in `load()` | new admin `+page.server.ts` | | 🟡 Medium | Confirm `SameSite` on session cookie; route System tab POSTs through server actions | `application.properties` + System tab component | | 🔵 Low | Add localStorage scope comment | admin layout component |
Sign in to join this conversation.
No Label feature ui
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: marcel/familienarchiv#156