Complete Phase 10: authorization matrix, stale push cleanup, and retention-default tests

- Add authorization.integration.test.ts covering unauthenticated/regular-user/admin
  access across both admin-only and merely-authenticated routes
- Add stale push subscription cleanup test (disableSubscription excludes a device
  from fan-out without deleting its row/history)
- Add retention test for a topic with no override falling back to the resolved
  Admin-configured global default
- Update README status: Phases 2-10 complete

README.md

@@ -16,11 +16,15 @@ it, and it's a general-purpose tool built for a much broader audience than
 something purpose-fit for this specific use case, entirely self-hosted,
 with no data ever leaving the LAN except the push notification itself.
 
-Status: **Phases 2–9 complete**, Phase 10 (hardening/test-gap-filling) in
+Status: **Phases 2–10 complete**. Auth, topics, notification ingestion
-progress. Auth, topics, notification ingestion (canonical + ntfy-compat +
+(canonical + ntfy-compat + webhook), rate limiting, dedup, SSE live
-webhook), rate limiting, dedup, SSE live updates, retention cleanup, the
+updates, retention cleanup, the full user-facing UI, Web Push, and the
-full user-facing UI, Web Push, and the admin UI are all built and verified
+admin UI are all built and verified against a live dev stack. Phase 10's
-against a live dev stack. See
+specific test gaps (authorization matrix across roles, stale push
+subscription cleanup, retention's global-default fallback) are covered by
+the automated suite (`pnpm test`), and the dependency/CSP/HSTS security
+pass is done — see [Backup and restore](#backup-and-restore) and the
+troubleshooting entries below for the operational side. See
 [docs/phase1/08-implementation-plan.md](docs/phase1/08-implementation-plan.md)
 for the phase breakdown and [MVP acceptance status](#mvp-acceptance-status)
 below for where things stand against the original 22 criteria.

apps/server/src/modules/auth/authorization.integration.test.ts

@@ -0,0 +1,125 @@
+import { randomUUID } from 'node:crypto'
+import type { FastifyInstance } from 'fastify'
+import { eq } from 'drizzle-orm'
+import { afterAll, beforeAll, describe, expect, it } from 'vitest'
+import { db, pool } from '../../db/client.js'
+import { getSetting } from '../../db/settings.js'
+import { runMigrations } from '../../db/migrate.js'
+import { users } from '../../db/schema.js'
+import { buildWebApp } from '../../listeners/web.js'
+import { encryptJson } from '../../lib/crypto-session.js'
+
+// Integration test — same requirements/run command as
+// modules/tokens/repository.integration.test.ts. Covers the
+// admin-vs-regular-user authorization matrix across an unauthenticated,
+// a regular-user, and an admin session — the gap Phase 10 (08-implementation-plan.md)
+// specifically calls out.
+//
+// Session cookies are hand-built the same way
+// notifications/image-proxy.integration.test.ts does it (AUTH_MODE=oidc
+// applies inside the test container too, so requireAuth/requireAdmin need
+// a real session, not the disabled-mode auto-admin).
+describe('authorization matrix', () => {
+  let app: FastifyInstance
+  let adminUserId: string
+  let regularUserId: string
+  let adminCookie: string
+  let regularCookie: string
+
+  const adminSubject = `authz-admin-${randomUUID()}`
+  const regularSubject = `authz-user-${randomUUID()}`
+
+  beforeAll(async () => {
+    await runMigrations()
+    app = await buildWebApp()
+    await app.ready()
+
+    const [admin] = await db
+      .insert(users)
+      .values({
+        email: `${adminSubject}@example.test`,
+        displayName: 'Authz Test Admin',
+        authSubject: adminSubject,
+        role: 'admin',
+      })
+      .returning()
+    adminUserId = admin!.id
+
+    const [regular] = await db
+      .insert(users)
+      .values({
+        email: `${regularSubject}@example.test`,
+        displayName: 'Authz Test User',
+        authSubject: regularSubject,
+        role: 'user',
+      })
+      .returning()
+    regularUserId = regular!.id
+
+    const secretHex = await getSetting<string>('session_secret')
+    const sessionKey = Buffer.from(secretHex!, 'hex')
+    adminCookie = encryptJson(sessionKey, { userId: adminUserId })
+    regularCookie = encryptJson(sessionKey, { userId: regularUserId })
+  })
+
+  afterAll(async () => {
+    await db.delete(users).where(eq(users.id, adminUserId))
+    await db.delete(users).where(eq(users.id, regularUserId))
+    await app.close()
+    await pool.end()
+  })
+
+  it('rejects an unauthenticated request to an admin-only route', async () => {
+    const res = await app.inject({ method: 'GET', url: '/api/v1/admin/users' })
+    expect(res.statusCode).toBe(401)
+  })
+
+  it("rejects a regular (non-admin) user's request to an admin-only route", async () => {
+    const res = await app.inject({
+      method: 'GET',
+      url: '/api/v1/admin/users',
+      headers: { cookie: `notizarr_session=${regularCookie}` },
+    })
+    expect(res.statusCode).toBe(403)
+    // Confirms this 403 is specifically the role check (modules/auth/guards.ts),
+    // not e.g. a CSRF rejection that would also happen to be a 403.
+    expect(res.json().error).toBe('forbidden')
+  })
+
+  it('allows an admin request to an admin-only route', async () => {
+    const res = await app.inject({
+      method: 'GET',
+      url: '/api/v1/admin/users',
+      headers: { cookie: `notizarr_session=${adminCookie}` },
+    })
+    expect(res.statusCode).toBe(200)
+  })
+
+  it('rejects an unauthenticated request to a merely-authenticated route', async () => {
+    const res = await app.inject({ method: 'GET', url: '/api/v1/topics' })
+    expect(res.statusCode).toBe(401)
+  })
+
+  it('allows a regular user to reach a merely-authenticated (non-admin) route', async () => {
+    const res = await app.inject({
+      method: 'GET',
+      url: '/api/v1/topics',
+      headers: { cookie: `notizarr_session=${regularCookie}` },
+    })
+    expect(res.statusCode).toBe(200)
+  })
+
+  it("prevents a regular user from removing their own admin role's protection by other means", async () => {
+    // Not applicable to a non-admin (they have no admin role to touch),
+    // but the same admin-only PATCH must still 403 for them, not just the
+    // GET above — a route-specific bypass would be a real gap.
+    const res = await app.inject({
+      method: 'PATCH',
+      url: `/api/v1/admin/users/${regularUserId}`,
+      headers: { cookie: `notizarr_session=${regularCookie}`, 'x-requested-with': 'notizarr' },
+      payload: { role: 'admin' },
+    })
+    expect(res.statusCode).toBe(403)
+    expect(res.json().error).toBe('forbidden')
+  })
+})

apps/server/src/modules/notifications/retention.integration.test.ts

@@ -4,6 +4,10 @@ import { afterAll, beforeAll, describe, expect, it } from 'vitest'
 import { db, pool } from '../../db/client.js'
 import { runMigrations } from '../../db/migrate.js'
 import { notifications, topics } from '../../db/schema.js'
+import {
+  getStoredSystemSettings,
+  setStoredSystemSettings,
+} from '../admin/system-settings.js'
 import { runRetentionCleanup } from './retention.js'
 
 // Integration tests — need a real, migrated Postgres. Run with:
@@ -12,8 +16,11 @@ import { runRetentionCleanup } from './retention.js'
 describe('retention cleanup', () => {
   const shortRetentionSlug = `retention-short-${randomUUID()}`
   const foreverSlug = `retention-forever-${randomUUID()}`
+  const defaultSlug = `retention-default-${randomUUID()}`
   let shortTopicId: string
   let foreverTopicId: string
+  let defaultTopicId: string
+  let originalSystemSettings: Awaited<ReturnType<typeof getStoredSystemSettings>>
 
   beforeAll(async () => {
     await runMigrations()
@@ -29,11 +36,25 @@ describe('retention cleanup', () => {
       .values({ slug: foreverSlug, displayName: 'Forever Retention', retentionDays: 0 })
       .returning()
     foreverTopicId = foreverTopic!.id
+
+    const [defaultTopic] = await db
+      .insert(topics)
+      .values({ slug: defaultSlug, displayName: 'Default Retention', retentionDays: null })
+      .returning()
+    defaultTopicId = defaultTopic!.id
+
+    // Pin the global default (Admin > System) to something short and known
+    // for the duration of this suite, rather than depending on whatever a
+    // real deployment happens to have it set to.
+    originalSystemSettings = await getStoredSystemSettings()
+    await setStoredSystemSettings({ ...originalSystemSettings, retentionDefaultDays: 1 })
   })
 
   afterAll(async () => {
     await db.delete(topics).where(eq(topics.id, shortTopicId))
     await db.delete(topics).where(eq(topics.id, foreverTopicId))
+    await db.delete(topics).where(eq(topics.id, defaultTopicId))
+    await setStoredSystemSettings(originalSystemSettings ?? {})
     await pool.end()
   })
 
@@ -93,4 +114,27 @@ describe('retention cleanup', () => {
       .where(eq(notifications.id, veryOldNotification!.id))
     expect(stillExists).toHaveLength(1)
   })
+
+  it('falls back to the resolved global default for a topic with no retentionDays override', async () => {
+    const twoDaysAgo = new Date(Date.now() - 2 * 24 * 60 * 60 * 1000)
+
+    const [oldNotification] = await db
+      .insert(notifications)
+      .values({
+        topicId: defaultTopicId,
+        source: 'test',
+        title: 'Old, no override',
+        message: 'Should be cleaned up via the global default (pinned to 1 day above)',
+        createdAt: twoDaysAgo,
+      })
+      .returning()
+
+    await runRetentionCleanup()
+
+    const stillExists = await db
+      .select()
+      .from(notifications)
+      .where(eq(notifications.id, oldNotification!.id))
+    expect(stillExists).toHaveLength(0)
+  })
 })

apps/server/src/modules/push/repository.integration.test.ts

@@ -4,7 +4,13 @@ import { afterAll, beforeAll, describe, expect, it } from 'vitest'
 import { db, pool } from '../../db/client.js'
 import { runMigrations } from '../../db/migrate.js'
 import { users } from '../../db/schema.js'
-import { deleteUserDevice, listUserDevices, upsertPushSubscription } from './repository.js'
+import {
+  deleteUserDevice,
+  disableSubscription,
+  getActiveSubscriptionsForUser,
+  listUserDevices,
+  upsertPushSubscription,
+} from './repository.js'
 
 // Integration tests — need a real, migrated Postgres. Run with:
 //   docker compose -f docker/docker-compose.yml exec notizarr \
@@ -85,4 +91,34 @@ describe('push subscription ownership', () => {
 
     await deleteUserDevice(ownerId, first.id)
   })
+
+  it('excludes a disabled subscription from active devices without deleting the row', async () => {
+    // Mirrors what modules/push/fanout.ts does on a 404/410 from the push
+    // service: the subscription is stale (endpoint no longer valid on the
+    // browser's end) but the row — and its delivery-attempt history — is
+    // kept, just marked disabled rather than removed.
+    const device = await upsertPushSubscription({
+      userId: ownerId,
+      endpoint: `https://push.example.test/${randomUUID()}`,
+      p256dh: 'stale-key',
+      auth: 'stale-auth',
+    })
+
+    const beforeDisable = await getActiveSubscriptionsForUser(ownerId)
+    expect(beforeDisable.some((d) => d.id === device.id)).toBe(true)
+
+    await disableSubscription(device.id)
+
+    const afterDisable = await getActiveSubscriptionsForUser(ownerId)
+    expect(afterDisable.some((d) => d.id === device.id)).toBe(false)
+
+    // Still visible in the full device list (e.g. Admin > Devices) — just
+    // not eligible for the next push fan-out.
+    const allDevices = await listUserDevices(ownerId)
+    const stillListed = allDevices.find((d) => d.id === device.id)
+    expect(stillListed).toBeDefined()
+    expect(stillListed?.disabledAt).not.toBeNull()
+
+    await deleteUserDevice(ownerId, device.id)
+  })
 })