diff --git a/.planning/phases/09-admin-panel-rearchitecture/09-04-SUMMARY.md b/.planning/phases/09-admin-panel-rearchitecture/09-04-SUMMARY.md new file mode 100644 index 0000000..1ff3e46 --- /dev/null +++ b/.planning/phases/09-admin-panel-rearchitecture/09-04-SUMMARY.md @@ -0,0 +1,235 @@ +--- +phase: "09" +plan: "04" +subsystem: frontend-router-auth +tags: [admin, router, guard, tailwind, cleanup, security] +dependency_graph: + requires: [09-01, 09-02, 09-03] + provides: + - frontend/src/router/index.js (nested /admin subtree + corrected guard) + - frontend/src/views/auth/LoginView.vue (admin login redirect) + - frontend/tailwind.config.js (safelist) + affects: + - frontend/src/router/__tests__/router.guard.test.js + - frontend/src/components/admin/__tests__/ (3 test files migrated) + - frontend/src/api/admin.js (JSDoc updated) + - frontend/src/views/SettingsView.vue (stale comment removed) +tech_stack: + added: [] + patterns: + - "Vue Router 4 nested route subtree — AdminLayout as /admin component, 5 lazy-loaded children" + - "to.matched.some(r => r.meta.requiresAdmin) — only correct pattern for child-route meta inheritance in Vue Router 4" + - "D-09/D-10 strict admin role separation — two guard branches, one per direction" + - "Tailwind safelist with regex patterns — covers all dynamic color families from formatters.js + AuditLogTab.actionTypeClass()" +key_files: + created: [] + modified: + - frontend/src/router/index.js + - frontend/src/views/auth/LoginView.vue + - frontend/tailwind.config.js + - frontend/src/router/__tests__/router.guard.test.js + - frontend/src/components/admin/__tests__/AdminUsersTab.test.js + - frontend/src/components/admin/__tests__/AdminQuotasTab.test.js + - frontend/src/components/admin/__tests__/AdminAiConfigTab.test.js + - frontend/src/api/admin.js + - frontend/src/views/SettingsView.vue + deleted: + - frontend/src/views/AdminView.vue + - frontend/src/components/admin/AdminUsersTab.vue + - frontend/src/components/admin/AdminQuotasTab.vue + - frontend/src/components/admin/AdminAiConfigTab.vue + - frontend/src/components/admin/AuditLogTab.vue +decisions: + - "to.matched.some(r => r.meta.requiresAdmin) is the guard pattern — flat to.meta.requiresAdmin silently bypasses all /admin/* child routes in Vue Router 4" + - "D-09 admin-on-user-route redirect added to beforeEach — isAdminRoute + isAdmin combination covers both directions cleanly" + - "Tailwind safelist extended to include sky (OneDrive providerBg) and amber (AuditLogTab actionTypeClass admin badge) — correcting the original D-14 which omitted these families" + - "Tab tests migrated to import from views/admin/Admin*View.vue — behavior identical, no test logic changed" + - "router.guard.test.js: AdminView mock replaced with AdminLayout + 5 view mocks; D-09 redirect test added" +metrics: + duration: "18 minutes" + completed: "2026-06-12" + tasks_completed: 3 + tasks_total: 3 + files_created: 0 + files_modified: 9 + files_deleted: 5 +--- + +# Phase 09 Plan 04: Router Rewire, Login Redirect, Cleanup Summary + +**One-liner:** Nested `/admin` route subtree wired with AdminLayout as component, `to.matched.some()` guard fixed, D-08/D-09/D-10 strict admin separation enforced, Tailwind safelist installed for `sky`+`amber` color families, and five legacy files deleted with tests migrated. + +## Tasks Completed + +| Task | Name | Commit | Files | +|------|------|--------|-------| +| 1 | Rewire router/index.js + Tailwind safelist | `5dbfb6c` | frontend/src/router/index.js, frontend/tailwind.config.js | +| 2 | Wire admin login redirect in LoginView.vue | `bdd68b2` | frontend/src/views/auth/LoginView.vue | +| 3 | Delete AdminView.vue + 4 tab files; migrate tests | `e6467d1` | 5 deletions, 6 modified (tests + api comment + settings comment) | + +## What Was Built + +### Task 1 — Nested /admin Route + Guard + Tailwind Safelist + +**Router rewire:** The flat `/admin` route pointing to the deleted `AdminView.vue` is replaced with a nested route whose `component` is the lazy-loaded `AdminLayout.vue`. Five children are registered: + +``` +{ path: '', AdminOverviewView.vue } → /admin +{ path: 'users', AdminUsersView.vue } → /admin/users +{ path: 'quotas', AdminQuotasView.vue } → /admin/quotas +{ path: 'ai', AdminAiView.vue } → /admin/ai +{ path: 'audit', AdminAuditView.vue } → /admin/audit +``` + +Only the parent carries `meta: { requiresAdmin: true }`. Child routes intentionally do not repeat it — the guard uses `to.matched.some(r => r.meta.requiresAdmin)` which walks the full matched ancestors array, making the parent's meta apply to every child. + +**Guard fix:** The old `if (to.meta.requiresAdmin && ...)` check is replaced with a 5-step guard: +1. Silent refresh on non-public routes with no access token +2. Compute `isAdminRoute = to.matched.some(r => r.meta.requiresAdmin)` +3. Compute `isAdmin = authStore.user?.role === 'admin'` +4. D-10a: `isAdminRoute && !isAdmin` → redirect `{ path: '/' }` +5. D-09: `!isAdminRoute && !to.meta.public && isAdmin` → redirect `{ path: '/admin' }` + +The `!to.meta.public` clause in step 5 ensures auth routes (`/login`, `/register`, `/password-reset`, `/password-reset/confirm`) remain reachable for admin users — this is the critical escape hatch that prevents redirect loops when an admin session expires and the guard sends them to `/login`. + +**Tailwind safelist:** Two regex patterns added covering all dynamic class families: +- `bg-(blue|sky|green|purple|orange|amber|gray|indigo|red)-(50|100|500|600)` — covers `providerBg()` (OneDrive=sky, Google Drive=blue, Nextcloud=orange, WebDAV=gray) and `actionTypeClass()` (auth=blue, folder/share=purple, admin=amber, document=gray) +- `text-(blue|sky|green|purple|orange|amber|gray|indigo|red)-(400|500|600|700)` — covers `providerColor()` and `actionTypeClass()` text variants + +`sky` and `amber` were the two families missing from the original D-14 draft. RESEARCH.md Pattern 7 (confirmed in CONTEXT.md) corrects this — both are included. + +### Task 2 — Admin Login Redirect (D-08) + +The `handleLoginResult(!result)` branch in `LoginView.vue` is updated: + +```js +const defaultRedirect = authStore.user?.role === 'admin' ? '/admin' : '/' +const redirect = route.query.redirect || defaultRedirect +await router.push(redirect) +``` + +`authStore.user.role` is synchronously populated inside `authStore.login()` before the store action returns (verified by reading `auth.js` line 82: `user.value = data.user`), so the role check fires with a fully populated user object. + +The `?redirect=` query param is still honored for non-admin users. For admin users, the D-09 guard in the router would intercept any `?redirect=/` attempt and redirect back to `/admin` anyway — the D-08 check here is belt-and-suspenders. + +The `requires_totp` and `requires_password_change` branches are byte-for-byte unchanged. + +### Task 3 — Delete Legacy Files + Migrate Tests + +**Five files deleted via `git rm`:** +- `frontend/src/views/AdminView.vue` — old tab-container view, replaced by `AdminLayout` + nested routes +- `frontend/src/components/admin/AdminUsersTab.vue` — promoted to `views/admin/AdminUsersView.vue` in 09-03 +- `frontend/src/components/admin/AdminQuotasTab.vue` — promoted to `views/admin/AdminQuotasView.vue` in 09-03 +- `frontend/src/components/admin/AdminAiConfigTab.vue` — promoted to `views/admin/AdminAiView.vue` in 09-03 +- `frontend/src/components/admin/AuditLogTab.vue` — promoted to `views/admin/AdminAuditView.vue` in 09-03 + +**Test migrations** (import path rewrite only — no test logic changed): +- `__tests__/AdminUsersTab.test.js`: `import AdminUsersTab from '../AdminUsersTab.vue'` → `from '../../../views/admin/AdminUsersView.vue'` +- `__tests__/AdminQuotasTab.test.js`: `import AdminQuotasTab from '../AdminQuotasTab.vue'` → `from '../../../views/admin/AdminQuotasView.vue'` +- `__tests__/AdminAiConfigTab.test.js`: `import AdminAiConfigTab from '../AdminAiConfigTab.vue'` → `from '../../../views/admin/AdminAiView.vue'` + +**Router guard test updated:** +- Replaced `vi.mock('../../views/AdminView.vue', ...)` with mocks for `layouts/AdminLayout.vue` and all five `views/admin/Admin*View.vue` files +- Added D-09 redirect test: admin navigating to `/` is redirected to `/admin` + +**Two stale comments updated (Rule 2 — correctness):** +- `src/api/admin.js` JSDoc consumer list updated from `AdminXxxTab.vue` names to `Admin*View.vue` names +- `src/views/SettingsView.vue` stale `` comment simplified to `` + +## Acceptance Criteria Results + +| Criterion | Result | +|-----------|--------| +| `grep -c "to.matched.some(r => r.meta.requiresAdmin)" router/index.js` ≥ 1 | 1 — PASS | +| `grep -c "to.meta.requiresAdmin" router/index.js` = 0 | 0 — PASS | +| `grep -c "AdminLayout" router/index.js` ≥ 1 | 2 — PASS | +| All 5 Admin*View imports present in router | PASS | +| `grep -c "AdminView" router/index.js` = 0 | 0 — PASS | +| `grep -c "safelist" tailwind.config.js` = 1 | 1 — PASS | +| `grep -c "amber" tailwind.config.js` ≥ 1 | 2 — PASS | +| `grep -c "sky" tailwind.config.js` ≥ 1 | 2 — PASS | +| `grep -c "authStore.user?.role === 'admin'" LoginView.vue` = 1 | 1 — PASS | +| `grep -c "defaultRedirect" LoginView.vue` ≥ 2 | 2 — PASS | +| None of 5 deleted files exist | PASS | +| No stale `AdminXxxTab` import references remain | PASS | +| `npm run build` succeeds | 881 ms, 0 errors — PASS | +| `npm test` passes | 137/137 tests — PASS | + +## Smoke Check (Build Artifacts) + +`npm run build` produced separate lazy-loaded chunks for all 5 admin routes: +- `AdminLayout-BbuVmUU8.js` (4.46 kB) — AdminLayout entry chunk +- `admin-BFKH-0jn.js` (3.04 kB) — shared admin chunk +- `AdminOverviewView-DUqzmtNA.js` (3.49 kB) +- `AdminQuotasView-Cao0PSry.js` (4.53 kB) +- `AdminAuditView-DDh7qqQi.js` (9.24 kB) +- `AdminUsersView-BLLys8CV.js` (12.62 kB) +- `AdminAiView-CZ8yB8IZ.js` (16.43 kB) + +`AdminAuditView` chunk contains the full `actionTypeClass()` logic with `bg-amber-50 text-amber-700` and `bg-blue-50 text-blue-600` classes — confirmed present in build output. Tailwind safelist ensures these survive tree-shaking in production. + +## Deviations from Plan + +### Auto-fixed Issues + +**[Rule 2 - Missing Critical Functionality] Added D-09 redirect test to router guard test** + +- **Found during:** Task 3 (router guard test update) +- **Issue:** The test file verified the non-admin → `/` redirect (D-10a) but had no test for the admin → `/admin` redirect (D-09). D-09 is a security-relevant guard branch — omitting it from the test suite is a missing correctness invariant. +- **Fix:** Added `it('redirects an admin user away from / to /admin (D-09)', ...)` test with admin role mock navigating to `/` and asserting `router.currentRoute.value.path === '/admin'`. +- **Files modified:** `frontend/src/router/__tests__/router.guard.test.js` +- **Commit:** `e6467d1` + +**[Rule 1 - Bug] Stale JSDoc consumer list in api/admin.js** + +- **Found during:** Task 3 (grep scan for stale references) +- **Issue:** `src/api/admin.js` JSDoc listed the deleted `AdminXxxTab.vue` and `AuditLogTab.vue` names as consumers. Post-deletion, these filenames no longer exist — an inaccurate comment is a correctness issue. +- **Fix:** Updated JSDoc consumer list to `AdminUsersView.vue, AdminQuotasView.vue, AdminAiView.vue, AdminAuditView.vue`. +- **Files modified:** `frontend/src/api/admin.js` +- **Commit:** `e6467d1` + +**[Rule 1 - Bug] Stale AdminView reference in SettingsView.vue comment** + +- **Found during:** Task 3 (grep scan for `AdminView` references) +- **Issue:** `src/views/SettingsView.vue` contained ``. After `AdminView.vue` is deleted, this comment references a non-existent file. +- **Fix:** Simplified to ``. +- **Files modified:** `frontend/src/views/SettingsView.vue` +- **Commit:** `e6467d1` + +**[Rule 3 - Blocking] npm not installed in worktree frontend — installed deps locally** + +- **Found during:** Task 1 verification (npm run build) +- **Issue:** The worktree's `frontend/` directory had no `node_modules/`. `npm run build` failed with `ERR_MODULE_NOT_FOUND` for vite. +- **Fix:** Ran `npm install` + `npm approve-scripts --allow-scripts-pending` in the worktree's `frontend/`. Build succeeded after this. +- **Files modified:** None (dev-time infrastructure only — `node_modules/` is gitignored). + +## Known Stubs + +None — no hardcoded data or placeholder content introduced. All admin routes resolve to the real view components from 09-02 and 09-03. + +## Threat Flags + +None — no new network endpoints, API routes, or trust boundaries introduced. This plan is purely frontend routing and build configuration. + +Threat mitigations from the plan's threat register: +- **T-09-04-01 (Elevation of Privilege):** `to.matched.some(r => r.meta.requiresAdmin)` guard covers all `/admin/*` child routes — IMPLEMENTED. Backend `get_current_admin` dependency remains the authoritative second gate. +- **T-09-04-02 (Privilege Escalation via ?redirect=):** D-08 puts role check FIRST; even `?redirect=/` for an admin routes through the D-09 guard — IMPLEMENTED. +- **T-09-04-03 (CSS purge of dynamic classes):** Tailwind safelist covers `sky` (OneDrive) and `amber` (audit admin badge) — IMPLEMENTED. Build output confirms separate admin chunks exist. +- **T-09-04-04 (Redirect loop):** `isAdminRoute` for `/admin/*` short-circuits D-09 so admins on admin routes are not redirected; auth routes have `!to.meta.public` guard — IMPLEMENTED. + +## Self-Check: PASSED + +- `frontend/src/router/index.js` — MODIFIED (nested /admin + corrected guard) +- `frontend/tailwind.config.js` — MODIFIED (safelist added) +- `frontend/src/views/auth/LoginView.vue` — MODIFIED (admin login redirect) +- `frontend/src/views/AdminView.vue` — DELETED +- `frontend/src/components/admin/AdminUsersTab.vue` — DELETED +- `frontend/src/components/admin/AdminQuotasTab.vue` — DELETED +- `frontend/src/components/admin/AdminAiConfigTab.vue` — DELETED +- `frontend/src/components/admin/AuditLogTab.vue` — DELETED +- Commit `5dbfb6c` (Task 1) — FOUND +- Commit `bdd68b2` (Task 2) — FOUND +- Commit `e6467d1` (Task 3) — FOUND +- `npm run build` succeeds (0 errors, 5 admin chunks in output) — PASSED +- `npm test` — 137/137 tests passed — PASSED +- No stale `AdminView` / `AdminXxxTab` imports remain — PASSED diff --git a/frontend/src/api/admin.js b/frontend/src/api/admin.js index 028ce79..cf25da0 100644 --- a/frontend/src/api/admin.js +++ b/frontend/src/api/admin.js @@ -1,8 +1,8 @@ /** * Admin API — user management, quota, AI config, audit log, daily exports. * - * Consumers: AdminUsersTab.vue, AdminQuotasTab.vue, AdminAiConfigTab.vue, - * AuditLogTab.vue, AdminDailyExportsTab.vue + * Consumers: AdminUsersView.vue, AdminQuotasView.vue, AdminAiView.vue, + * AdminAuditView.vue */ import { request, fetchWithRetry } from './utils.js' diff --git a/frontend/src/components/admin/AdminAiConfigTab.vue b/frontend/src/components/admin/AdminAiConfigTab.vue deleted file mode 100644 index e0eed21..0000000 --- a/frontend/src/components/admin/AdminAiConfigTab.vue +++ /dev/null @@ -1,391 +0,0 @@ - - - diff --git a/frontend/src/components/admin/AdminQuotasTab.vue b/frontend/src/components/admin/AdminQuotasTab.vue deleted file mode 100644 index 7babeb4..0000000 --- a/frontend/src/components/admin/AdminQuotasTab.vue +++ /dev/null @@ -1,182 +0,0 @@ - - - diff --git a/frontend/src/components/admin/AdminUsersTab.vue b/frontend/src/components/admin/AdminUsersTab.vue deleted file mode 100644 index 289ae61..0000000 --- a/frontend/src/components/admin/AdminUsersTab.vue +++ /dev/null @@ -1,480 +0,0 @@ - - - diff --git a/frontend/src/components/admin/AuditLogTab.vue b/frontend/src/components/admin/AuditLogTab.vue deleted file mode 100644 index 6e110c8..0000000 --- a/frontend/src/components/admin/AuditLogTab.vue +++ /dev/null @@ -1,346 +0,0 @@ - - - diff --git a/frontend/src/components/admin/__tests__/AdminAiConfigTab.test.js b/frontend/src/components/admin/__tests__/AdminAiConfigTab.test.js index 8c0d0af..2e0f365 100644 --- a/frontend/src/components/admin/__tests__/AdminAiConfigTab.test.js +++ b/frontend/src/components/admin/__tests__/AdminAiConfigTab.test.js @@ -7,7 +7,7 @@ vi.mock('../../../api/client.js', () => ({ adminUpdateAiConfig: vi.fn(), })) -import AdminAiConfigTab from '../AdminAiConfigTab.vue' +import AdminAiConfigTab from '../../../views/admin/AdminAiView.vue' import * as api from '../../../api/client.js' function makeUser(overrides = {}) { diff --git a/frontend/src/components/admin/__tests__/AdminQuotasTab.test.js b/frontend/src/components/admin/__tests__/AdminQuotasTab.test.js index ed57863..fc82868 100644 --- a/frontend/src/components/admin/__tests__/AdminQuotasTab.test.js +++ b/frontend/src/components/admin/__tests__/AdminQuotasTab.test.js @@ -8,7 +8,7 @@ vi.mock('../../../api/client.js', () => ({ adminUpdateQuota: vi.fn(), })) -import AdminQuotasTab from '../AdminQuotasTab.vue' +import AdminQuotasTab from '../../../views/admin/AdminQuotasView.vue' import * as api from '../../../api/client.js' const MB = 1048576 diff --git a/frontend/src/components/admin/__tests__/AdminUsersTab.test.js b/frontend/src/components/admin/__tests__/AdminUsersTab.test.js index cfa67e9..f116353 100644 --- a/frontend/src/components/admin/__tests__/AdminUsersTab.test.js +++ b/frontend/src/components/admin/__tests__/AdminUsersTab.test.js @@ -11,7 +11,7 @@ vi.mock('../../../api/client.js', () => ({ adminCreateUser: vi.fn(), })) -import AdminUsersTab from '../AdminUsersTab.vue' +import AdminUsersTab from '../../../views/admin/AdminUsersView.vue' import * as api from '../../../api/client.js' function makeUser(overrides = {}) { diff --git a/frontend/src/router/__tests__/router.guard.test.js b/frontend/src/router/__tests__/router.guard.test.js index 99336fc..036a88c 100644 --- a/frontend/src/router/__tests__/router.guard.test.js +++ b/frontend/src/router/__tests__/router.guard.test.js @@ -11,7 +11,12 @@ vi.mock('../../views/auth/LoginView.vue', () => ({ default: { template: '
vi.mock('../../views/auth/RegisterView.vue', () => ({ default: { template: '
' } })) vi.mock('../../views/auth/PasswordResetView.vue', () => ({ default: { template: '
' } })) vi.mock('../../views/auth/NewPasswordView.vue', () => ({ default: { template: '
' } })) -vi.mock('../../views/AdminView.vue', () => ({ default: { template: '
' } })) +vi.mock('../../layouts/AdminLayout.vue', () => ({ default: { template: '
' } })) +vi.mock('../../views/admin/AdminOverviewView.vue', () => ({ default: { template: '
' } })) +vi.mock('../../views/admin/AdminUsersView.vue', () => ({ default: { template: '
' } })) +vi.mock('../../views/admin/AdminQuotasView.vue', () => ({ default: { template: '
' } })) +vi.mock('../../views/admin/AdminAiView.vue', () => ({ default: { template: '
' } })) +vi.mock('../../views/admin/AdminAuditView.vue', () => ({ default: { template: '
' } })) vi.mock('../../views/SharedView.vue', () => ({ default: { template: '
' } })) // Heavy view components imported statically — stub them too @@ -78,4 +83,18 @@ describe('router — admin guard (SEC-07)', () => { await router.push('/admin') expect(router.currentRoute.value.path).toBe('/admin') }) + + it('redirects an admin user away from / to /admin (D-09)', async () => { + // Arrange: admin trying to navigate to a regular user route + useAuthStore.mockReturnValue({ + accessToken: 'fake-token', + user: { id: '1', role: 'admin' }, + refresh: vi.fn().mockResolvedValue(undefined), + }) + + // Act: admin attempts to visit the regular user home + await router.push('/') + // The D-09 guard branch returns { path: '/admin' } + expect(router.currentRoute.value.path).toBe('/admin') + }) }) diff --git a/frontend/src/router/index.js b/frontend/src/router/index.js index 4f7226e..c9c8f9f 100644 --- a/frontend/src/router/index.js +++ b/frontend/src/router/index.js @@ -39,7 +39,22 @@ const routes = [ // Phase 2 — authenticated routes { path: '/account', redirect: '/settings' }, - { path: '/admin', component: () => import('../views/AdminView.vue'), meta: { requiresAdmin: true } }, + + // Admin panel — standalone route subtree with AdminLayout as the route component. + // Children do NOT re-declare requiresAdmin — the guard uses to.matched.some() to + // inherit from the parent (Vue Router 4 does not propagate meta to children). + { + path: '/admin', + component: () => import('../layouts/AdminLayout.vue'), + meta: { requiresAdmin: true }, + children: [ + { path: '', component: () => import('../views/admin/AdminOverviewView.vue') }, + { path: 'users', component: () => import('../views/admin/AdminUsersView.vue') }, + { path: 'quotas', component: () => import('../views/admin/AdminQuotasView.vue') }, + { path: 'ai', component: () => import('../views/admin/AdminAiView.vue') }, + { path: 'audit', component: () => import('../views/admin/AdminAuditView.vue') }, + ], + }, // Cloud storage overview and folder browser { @@ -75,11 +90,13 @@ const router = createRouter({ routes, }) -// Navigation guard (D-10): redirect unauthenticated users to /login. +// Navigation guard — enforces auth and admin role separation (D-09, D-10). // On page reload the access token is gone (memory-only per CLAUDE.md), so we attempt -// a silent refresh via the httpOnly cookie before concluding the session is gone. +// a silent refresh via the httpOnly cookie before any role checks fire. router.beforeEach(async (to) => { const authStore = useAuthStore() + + // Step 1: ensure access token is present for non-public routes. if (!to.meta.public && !authStore.accessToken) { try { await authStore.refresh() @@ -88,9 +105,18 @@ router.beforeEach(async (to) => { } } - if (to.meta.requiresAdmin && authStore.user?.role !== 'admin') { + const isAdminRoute = to.matched.some(r => r.meta.requiresAdmin) + const isAdmin = authStore.user?.role === 'admin' + + // D-10a: non-admin attempting an admin route → redirect to /. + if (isAdminRoute && !isAdmin) { return { path: '/' } } + + // D-09: admin attempting a non-admin, non-public route → redirect to /admin. + if (!isAdminRoute && !to.meta.public && isAdmin) { + return { path: '/admin' } + } }) export default router diff --git a/frontend/src/views/AdminView.vue b/frontend/src/views/AdminView.vue deleted file mode 100644 index 22dae05..0000000 --- a/frontend/src/views/AdminView.vue +++ /dev/null @@ -1,43 +0,0 @@ - - - diff --git a/frontend/src/views/SettingsView.vue b/frontend/src/views/SettingsView.vue index ec1bd19..616f5b3 100644 --- a/frontend/src/views/SettingsView.vue +++ b/frontend/src/views/SettingsView.vue @@ -27,7 +27,7 @@
- +