diff --git a/.github/workflows/close-stale-wrong-branch.yml b/.github/workflows/close-stale-wrong-branch.yml index 10b9cd1f..0e907731 100644 --- a/.github/workflows/close-stale-wrong-branch.yml +++ b/.github/workflows/close-stale-wrong-branch.yml @@ -26,6 +26,9 @@ jobs: const twentyFourHoursAgo = new Date(Date.now() - 24 * 60 * 60 * 1000); for (const pull of pulls) { + const hasBypass = pull.labels.some(l => l.name === 'bypass-branch-check'); + if (hasBypass) continue; + const hasLabel = pull.labels.some(l => l.name === 'wrong-base-branch'); if (!hasLabel) continue; diff --git a/.github/workflows/enforce-target-branch.yml b/.github/workflows/enforce-target-branch.yml index 7a326a3f..150cb646 100644 --- a/.github/workflows/enforce-target-branch.yml +++ b/.github/workflows/enforce-target-branch.yml @@ -21,6 +21,12 @@ jobs: const labels = context.payload.pull_request.labels.map(l => l.name); const prNumber = context.payload.pull_request.number; + // bypass-branch-check label skips all enforcement + if (labels.includes('bypass-branch-check')) { + console.log('bypass-branch-check label present, skipping enforcement.'); + return; + } + // If the base was fixed, remove the label and let it through if (base !== 'main') { if (labels.includes('wrong-base-branch')) { diff --git a/client/src/App.tsx b/client/src/App.tsx index 5e0f5ed2..f5d96b51 100644 --- a/client/src/App.tsx +++ b/client/src/App.tsx @@ -58,7 +58,7 @@ function ProtectedRoute({ children, adminRequired = false, addonId }: ProtectedR } if (!isAuthenticated) { - const redirectParam = encodeURIComponent(location.pathname + location.search) + const redirectParam = encodeURIComponent(location.pathname + location.search + location.hash) return } diff --git a/client/src/api/client.ts b/client/src/api/client.ts index 378ddeab..e39c6a47 100644 --- a/client/src/api/client.ts +++ b/client/src/api/client.ts @@ -75,7 +75,7 @@ apiClient.interceptors.response.use( if (error.response?.status === 401 && (error.response?.data as { code?: string } | undefined)?.code === 'AUTH_REQUIRED') { const { pathname } = window.location if (!isAuthPublicPath(pathname)) { - const currentPath = pathname + window.location.search + const currentPath = pathname + window.location.search + window.location.hash window.location.href = '/login?redirect=' + encodeURIComponent(currentPath) } } diff --git a/client/src/components/Planner/DayDetailPanel.test.tsx b/client/src/components/Planner/DayDetailPanel.test.tsx index 279fa46b..9bdc0256 100644 --- a/client/src/components/Planner/DayDetailPanel.test.tsx +++ b/client/src/components/Planner/DayDetailPanel.test.tsx @@ -892,6 +892,183 @@ describe('DayDetailPanel', () => { expect(screen.getByText(/June|15/i)).toBeInTheDocument(); }); + // ── Accommodation date-range picker — non-monotonic day IDs (issue #889) ───── + + // Builds the reporter's exact ID layout: day_number 1-9 → IDs 17-25, day_number 10-16 → IDs 1-7. + // This happens after repeated trip-length changes via generateDays (no import/migration needed). + function buildNonMonotonicDays() { + return [ + buildDay({ id: 17, trip_id: 1, date: '2026-04-30' }), + buildDay({ id: 18, trip_id: 1, date: '2026-05-01' }), + buildDay({ id: 19, trip_id: 1, date: '2026-05-02' }), + buildDay({ id: 20, trip_id: 1, date: '2026-05-03' }), + buildDay({ id: 21, trip_id: 1, date: '2026-05-04' }), + buildDay({ id: 22, trip_id: 1, date: '2026-05-05' }), + buildDay({ id: 23, trip_id: 1, date: '2026-05-06' }), + buildDay({ id: 24, trip_id: 1, date: '2026-05-07' }), + buildDay({ id: 25, trip_id: 1, date: '2026-05-08' }), + buildDay({ id: 1, trip_id: 1, date: '2026-05-09' }), + buildDay({ id: 2, trip_id: 1, date: '2026-05-10' }), + buildDay({ id: 3, trip_id: 1, date: '2026-05-11' }), + buildDay({ id: 4, trip_id: 1, date: '2026-05-12' }), + buildDay({ id: 5, trip_id: 1, date: '2026-05-13' }), + buildDay({ id: 6, trip_id: 1, date: '2026-05-14' }), + buildDay({ id: 7, trip_id: 1, date: '2026-05-15' }), + ]; + } + + // Returns the two CustomSelect trigger buttons for start/end day pickers. + // When no dropdown is open, these are the only globally-visible buttons whose textContent + // matches /Day \d+/ (the main panel title is a div, not a button). + // [0] = start trigger, [1] = end trigger (DOM source order). + function getDayPickerTriggers() { + return screen.getAllByRole('button').filter(b => /Day \d+/.test(b.textContent ?? '')); + } + + it('FE-PLANNER-DAYDETAIL-056: non-monotonic IDs — end picker does not clobber start-day', async () => { + const days = buildNonMonotonicDays(); + const place = buildPlace({ id: 50, name: 'Range Hotel' }); + let capturedBody: any; + server.use( + http.post('/api/trips/1/accommodations', async ({ request }) => { + capturedBody = await request.json(); + return HttpResponse.json({ + accommodation: { + id: 99, place_id: 50, place_name: 'Range Hotel', place_address: null, + start_day_id: capturedBody.start_day_id, end_day_id: capturedBody.end_day_id, + check_in: null, check_out: null, confirmation: null, + }, + }); + }), + ); + + render(); + await userEvent.click(await screen.findByText(/Add accommodation/i)); + await userEvent.click(await screen.findByRole('button', { name: /Range Hotel/i })); + + // Both triggers show "Day 1"; the second one is the end picker. + await userEvent.click(getDayPickerTriggers()[1]); + // Select "Day 16" (id=7) from the open dropdown — textContent starts with "Day 16". + await userEvent.click(screen.getAllByRole('button').find(b => b.textContent?.startsWith('Day 16'))!); + + await userEvent.click(screen.getByRole('button', { name: /^Save$/i })); + + await waitFor(() => { + // start must remain id 17 (day 1) — old code would clobber it to id 7 via Math.min + expect(capturedBody?.start_day_id).toBe(17); + expect(capturedBody?.end_day_id).toBe(7); + }); + }); + + it('FE-PLANNER-DAYDETAIL-057: non-monotonic IDs — start picker does not collapse end when start has high ID', async () => { + const days = buildNonMonotonicDays(); + const place = buildPlace({ id: 51, name: 'Span Hotel' }); + let capturedBody: any; + server.use( + http.post('/api/trips/1/accommodations', async ({ request }) => { + capturedBody = await request.json(); + return HttpResponse.json({ + accommodation: { + id: 100, place_id: 51, place_name: 'Span Hotel', place_address: null, + start_day_id: capturedBody.start_day_id, end_day_id: capturedBody.end_day_id, + check_in: null, check_out: null, confirmation: null, + }, + }); + }), + ); + + render(); + await userEvent.click(await screen.findByText(/Add accommodation/i)); + await userEvent.click(await screen.findByRole('button', { name: /Span Hotel/i })); + + // Set end to day 16 (id=7, low ID but last day by position). + await userEvent.click(getDayPickerTriggers()[1]); + await userEvent.click(screen.getAllByRole('button').find(b => b.textContent?.startsWith('Day 16'))!); + + // Set start to day 9 (id=25, high ID, but earlier by position than day 16). + // Old code: Math.max(25, 7) = 25 → end collapses to day 9. + // New code: position(id=25)=8 < position(id=7)=15 → end stays at 7 (day 16). + await userEvent.click(getDayPickerTriggers()[0]); + await userEvent.click(screen.getAllByRole('button').find(b => b.textContent?.startsWith('Day 9'))!); + + await userEvent.click(screen.getByRole('button', { name: /^Save$/i })); + + await waitFor(() => { + expect(capturedBody?.start_day_id).toBe(25); // day 9 + expect(capturedBody?.end_day_id).toBe(7); // day 16 — must NOT have collapsed + }); + }); + + it('FE-PLANNER-DAYDETAIL-058: non-monotonic IDs — All days button sets correct first/last IDs', async () => { + const days = buildNonMonotonicDays(); + const place = buildPlace({ id: 52, name: 'Full Trip Hotel' }); + let capturedBody: any; + server.use( + http.post('/api/trips/1/accommodations', async ({ request }) => { + capturedBody = await request.json(); + return HttpResponse.json({ + accommodation: { + id: 101, place_id: 52, place_name: 'Full Trip Hotel', place_address: null, + start_day_id: capturedBody.start_day_id, end_day_id: capturedBody.end_day_id, + check_in: null, check_out: null, confirmation: null, + }, + }); + }), + ); + + render(); + await userEvent.click(await screen.findByText(/Add accommodation/i)); + await userEvent.click(await screen.findByRole('button', { name: /Full Trip Hotel/i })); + + // "All" is the day.allDays translation (en: "All") — the Apply-to-entire-trip button. + // When categories=[] the category-filter "All" button is not rendered, so this is unique. + await userEvent.click(screen.getByRole('button', { name: /^All$/i })); + await userEvent.click(screen.getByRole('button', { name: /^Save$/i })); + + await waitFor(() => { + // days[0].id=17 (first by position), days[15].id=7 (last by position) + expect(capturedBody?.start_day_id).toBe(17); + expect(capturedBody?.end_day_id).toBe(7); + }); + }); + + it('FE-PLANNER-DAYDETAIL-059: sequential IDs — end picker clamping still works (regression guard)', async () => { + const seqDays = [ + buildDay({ id: 101, trip_id: 1, date: '2026-06-01' }), + buildDay({ id: 102, trip_id: 1, date: '2026-06-02' }), + buildDay({ id: 103, trip_id: 1, date: '2026-06-03' }), + ]; + const place = buildPlace({ id: 53, name: 'Seq Hotel' }); + let capturedBody: any; + server.use( + http.post('/api/trips/1/accommodations', async ({ request }) => { + capturedBody = await request.json(); + return HttpResponse.json({ + accommodation: { + id: 102, place_id: 53, place_name: 'Seq Hotel', place_address: null, + start_day_id: capturedBody.start_day_id, end_day_id: capturedBody.end_day_id, + check_in: null, check_out: null, confirmation: null, + }, + }); + }), + ); + + render(); + await userEvent.click(await screen.findByText(/Add accommodation/i)); + await userEvent.click(await screen.findByRole('button', { name: /Seq Hotel/i })); + + // Pick end = day 3 (id=103, position 2 > position 0 of start id=101). + await userEvent.click(getDayPickerTriggers()[1]); + await userEvent.click(screen.getAllByRole('button').find(b => b.textContent?.startsWith('Day 3'))!); + + await userEvent.click(screen.getByRole('button', { name: /^Save$/i })); + + await waitFor(() => { + expect(capturedBody?.start_day_id).toBe(101); + expect(capturedBody?.end_day_id).toBe(103); + }); + }); + it('FE-PLANNER-DAYDETAIL-040: 12h time format renders reservation time with AM/PM', async () => { seedStore(useSettingsStore, { settings: { time_format: '12h', temperature_unit: 'celsius', blur_booking_codes: false }, diff --git a/client/src/components/Planner/DayDetailPanel.tsx b/client/src/components/Planner/DayDetailPanel.tsx index 9487f402..0d53f8ff 100644 --- a/client/src/components/Planner/DayDetailPanel.tsx +++ b/client/src/components/Planner/DayDetailPanel.tsx @@ -463,7 +463,7 @@ export default function DayDetailPanel({ day, days, places, categories = [], tri
setHotelDayRange(prev => ({ start: v, end: Math.max(v, prev.end) }))} + onChange={v => setHotelDayRange(prev => ({ start: v, end: days.findIndex(d => d.id === v) > days.findIndex(d => d.id === prev.end) ? v : prev.end }))} options={days.map((d, i) => ({ value: d.id, label: d.title || t('planner.dayN', { n: i + 1 }), @@ -478,7 +478,7 @@ export default function DayDetailPanel({ day, days, places, categories = [], tri
setHotelDayRange(prev => ({ start: Math.min(prev.start, v), end: v }))} + onChange={v => setHotelDayRange(prev => ({ start: days.findIndex(d => d.id === v) < days.findIndex(d => d.id === prev.start) ? v : prev.start, end: v }))} options={days.map((d, i) => ({ value: d.id, label: d.title || t('planner.dayN', { n: i + 1 }), diff --git a/client/src/pages/LoginPage.oidc-redirect.test.tsx b/client/src/pages/LoginPage.oidc-redirect.test.tsx new file mode 100644 index 00000000..c14e0d96 --- /dev/null +++ b/client/src/pages/LoginPage.oidc-redirect.test.tsx @@ -0,0 +1,105 @@ +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { render, screen, waitFor } from '../../tests/helpers/render'; +import { http, HttpResponse } from 'msw'; +import { server } from '../../tests/helpers/msw/server'; +import { resetAllStores } from '../../tests/helpers/store'; +import LoginPage from './LoginPage'; + +const mockNavigate = vi.fn(); +vi.mock('react-router-dom', async () => { + const actual = await vi.importActual('react-router-dom'); + return { ...actual, useNavigate: () => mockNavigate }; +}); + +describe('LoginPage — OIDC redirect preservation', () => { + let savedLocation: Location; + + beforeEach(() => { + resetAllStores(); + mockNavigate.mockClear(); + sessionStorage.clear(); + savedLocation = window.location; + }); + + afterEach(() => { + Object.defineProperty(window, 'location', { + configurable: true, + writable: true, + value: savedLocation, + }); + }); + + function setSearch(search: string) { + Object.defineProperty(window, 'location', { + configurable: true, + writable: true, + value: { ...window.location, search }, + }); + } + + describe('FE-PAGE-LOGIN-022: redirect param stashed in sessionStorage on mount', () => { + it('saves decoded redirect to sessionStorage when ?redirect= is present', async () => { + setSearch('?redirect=%2Foauth%2Fauthorize%3Fclient_id%3Dfoo'); + render(); + + await waitFor(() => { + expect(sessionStorage.getItem('oidc_redirect')).toBe('/oauth/authorize?client_id=foo'); + }); + }); + + it('does not write to sessionStorage when no redirect param is present', async () => { + render(); + await waitFor(() => { + expect(screen.getByPlaceholderText('your@email.com')).toBeInTheDocument(); + }); + + expect(sessionStorage.getItem('oidc_redirect')).toBeNull(); + }); + }); + + describe('FE-PAGE-LOGIN-023: OIDC code exchange navigates to sessionStorage redirect', () => { + beforeEach(() => { + server.use( + http.get('/api/auth/oidc/exchange', () => + HttpResponse.json({ token: 'mock-oidc-token' }) + ), + ); + }); + + it('navigates to the saved sessionStorage redirect after successful OIDC exchange', async () => { + sessionStorage.setItem('oidc_redirect', '/oauth/authorize?client_id=foo&state=xyz'); + setSearch('?oidc_code=testcode123'); + render(); + + await waitFor(() => { + expect(mockNavigate).toHaveBeenCalledWith( + '/oauth/authorize?client_id=foo&state=xyz', + { replace: true }, + ); + }); + + expect(sessionStorage.getItem('oidc_redirect')).toBeNull(); + }); + + it('falls back to /dashboard when no sessionStorage redirect is set', async () => { + setSearch('?oidc_code=testcode123'); + render(); + + await waitFor(() => { + expect(mockNavigate).toHaveBeenCalledWith('/dashboard', { replace: true }); + }); + }); + }); + + describe('FE-PAGE-LOGIN-024: OIDC error clears sessionStorage redirect', () => { + it('removes oidc_redirect from sessionStorage on OIDC error', async () => { + sessionStorage.setItem('oidc_redirect', '/oauth/authorize?client_id=foo'); + setSearch('?oidc_error=token_failed'); + render(); + + await waitFor(() => { + expect(sessionStorage.getItem('oidc_redirect')).toBeNull(); + }); + }); + }); +}); diff --git a/client/src/pages/LoginPage.tsx b/client/src/pages/LoginPage.tsx index d4646635..6fa2c192 100644 --- a/client/src/pages/LoginPage.tsx +++ b/client/src/pages/LoginPage.tsx @@ -55,6 +55,12 @@ export default function LoginPage(): React.ReactElement { return '/dashboard' }, []) + useEffect(() => { + if (redirectTarget !== '/dashboard') { + sessionStorage.setItem('oidc_redirect', redirectTarget) + } + }, [redirectTarget]) + useEffect(() => { const params = new URLSearchParams(window.location.search) @@ -83,7 +89,9 @@ export default function LoginPage(): React.ReactElement { window.history.replaceState({}, '', '/login') if (data.token) { await loadUser() - navigate('/dashboard', { replace: true }) + const savedRedirect = sessionStorage.getItem('oidc_redirect') || '/dashboard' + sessionStorage.removeItem('oidc_redirect') + navigate(savedRedirect, { replace: true }) } else { setError(data.error || t('login.oidcFailed')) } @@ -104,6 +112,7 @@ export default function LoginPage(): React.ReactElement { invalid_state: t('login.oidc.invalidState'), } setError(errorMessages[oidcError] || oidcError) + sessionStorage.removeItem('oidc_redirect') window.history.replaceState({}, '', '/login') return } diff --git a/client/src/pages/OAuthAuthorizePage.tsx b/client/src/pages/OAuthAuthorizePage.tsx index 457d96ae..681326f2 100644 --- a/client/src/pages/OAuthAuthorizePage.tsx +++ b/client/src/pages/OAuthAuthorizePage.tsx @@ -124,7 +124,7 @@ export default function OAuthAuthorizePage(): React.ReactElement { } function handleLoginRedirect() { - const next = '/oauth/authorize?' + params.toString() + const next = '/oauth/authorize?' + params.toString() + window.location.hash window.location.href = '/login?redirect=' + encodeURIComponent(next) } diff --git a/server/src/app.ts b/server/src/app.ts index d21c0d3c..cc83b645 100644 --- a/server/src/app.ts +++ b/server/src/app.ts @@ -53,7 +53,7 @@ export function createApp(): express.Application { const app = express(); // Trust first proxy (nginx/Docker) for correct req.ip - if (process.env.NODE_ENV === 'production' || process.env.TRUST_PROXY) { + if (process.env.NODE_ENV?.toLowerCase() === 'production' || process.env.TRUST_PROXY) { app.set('trust proxy', Number.parseInt(process.env.TRUST_PROXY) || 1); } @@ -67,13 +67,13 @@ export function createApp(): express.Application { if (!origin || allowedOrigins.includes(origin)) callback(null, true); else callback(new Error('Not allowed by CORS')); }; - } else if (process.env.NODE_ENV === 'production') { + } else if (process.env.NODE_ENV?.toLowerCase() === 'production') { corsOrigin = false; } else { corsOrigin = true; } - const shouldForceHttps = process.env.FORCE_HTTPS === 'true'; + const shouldForceHttps = process.env.FORCE_HTTPS?.toLowerCase() === 'true'; // HSTS is worth enabling any time we're serving production traffic, // not only when FORCE_HTTPS is set. Self-hosters behind Traefik / // Caddy / Cloudflare Tunnel typically leave FORCE_HTTPS unset (the diff --git a/server/src/config.ts b/server/src/config.ts index 2941a87f..c9255cc9 100644 --- a/server/src/config.ts +++ b/server/src/config.ts @@ -105,7 +105,7 @@ export const ENCRYPTION_KEY = _encryptionKey; // Must stay in sync with client/src/i18n/supportedLanguages.ts (canonical source). // Kept duplicated here because server and client are separate npm packages. const SUPPORTED_LANG_CODES = ['de', 'en', 'es', 'fr', 'hu', 'nl', 'br', 'cs', 'pl', 'ru', 'zh', 'zh-TW', 'it', 'ar']; -const rawDefaultLang = process.env.DEFAULT_LANGUAGE || 'en'; +const rawDefaultLang = process.env.DEFAULT_LANGUAGE?.toLowerCase() || 'en'; if (!SUPPORTED_LANG_CODES.includes(rawDefaultLang)) { console.warn(`DEFAULT_LANGUAGE="${rawDefaultLang}" is not supported. Falling back to "en". Supported: ${SUPPORTED_LANG_CODES.join(', ')}`); } diff --git a/server/src/db/database.ts b/server/src/db/database.ts index 2578608b..e7057fd4 100644 --- a/server/src/db/database.ts +++ b/server/src/db/database.ts @@ -47,7 +47,7 @@ const db = new Proxy({} as Database.Database, { }, }); -if (process.env.DEMO_MODE === 'true') { +if (process.env.DEMO_MODE?.toLowerCase() === 'true') { try { const { seedDemoData } = require('../demo/demo-seed'); seedDemoData(_db); diff --git a/server/src/db/seeds.ts b/server/src/db/seeds.ts index d6e01d54..299e3f5a 100644 --- a/server/src/db/seeds.ts +++ b/server/src/db/seeds.ts @@ -6,7 +6,7 @@ import crypto from 'crypto'; // are only relevant after the first user exists; at that point seeds have already // finished and skip via the userCount > 0 guard above. function isOidcOnlyConfigured(): boolean { - if (process.env.OIDC_ONLY !== 'true') return false; + if (process.env.OIDC_ONLY?.toLowerCase() !== 'true') return false; return !!(process.env.OIDC_ISSUER && process.env.OIDC_CLIENT_ID); } diff --git a/server/src/index.ts b/server/src/index.ts index 3f1bf8dd..0e699f37 100644 --- a/server/src/index.ts +++ b/server/src/index.ts @@ -29,8 +29,9 @@ const server = app.listen(PORT, () => { const banner = [ '──────────────────────────────────────', ' TREK API started', + ` Version ${process.env.APP_VERSION}`, ` Port: ${PORT}`, - ` Environment: ${process.env.NODE_ENV || 'development'}`, + ` Environment: ${process.env.NODE_ENV?.toLowerCase() || 'development'}`, ` Timezone: ${tz}`, ` Origins: ${origins}`, ` Log level: ${LOG_LVL}`, @@ -40,8 +41,8 @@ const server = app.listen(PORT, () => { '──────────────────────────────────────', ]; banner.forEach(l => console.log(l)); - if (process.env.DEMO_MODE === 'true') sLogInfo('Demo mode: ENABLED'); - if (process.env.DEMO_MODE === 'true' && process.env.NODE_ENV === 'production') { + if (process.env.DEMO_MODE?.toLowerCase() === 'true') sLogInfo('Demo mode: ENABLED'); + if (process.env.DEMO_MODE?.toLowerCase() === 'true' && process.env.NODE_ENV?.toLowerCase() === 'production') { sLogWarn('SECURITY WARNING: DEMO_MODE is enabled in production!'); } scheduler.start(); diff --git a/server/src/middleware/auth.ts b/server/src/middleware/auth.ts index 9f254403..18be7236 100644 --- a/server/src/middleware/auth.ts +++ b/server/src/middleware/auth.ts @@ -105,7 +105,7 @@ const adminOnly = (req: Request, res: Response, next: NextFunction): void => { const demoUploadBlock = (req: Request, res: Response, next: NextFunction): void => { const authReq = req as AuthRequest; - if (process.env.DEMO_MODE === 'true' && isDemoEmail(authReq.user?.email)) { + if (process.env.DEMO_MODE?.toLowerCase() === 'true' && isDemoEmail(authReq.user?.email)) { res.status(403).json({ error: 'Uploads are disabled in demo mode. Self-host TREK for full functionality.' }); return; } diff --git a/server/src/middleware/mfaPolicy.ts b/server/src/middleware/mfaPolicy.ts index b253acc0..70b00896 100644 --- a/server/src/middleware/mfaPolicy.ts +++ b/server/src/middleware/mfaPolicy.ts @@ -68,7 +68,7 @@ export function enforceGlobalMfaPolicy(req: Request, res: Response, next: NextFu return; } - if (process.env.DEMO_MODE === 'true' && verified.email && DEMO_EMAILS.has(verified.email)) { + if (process.env.DEMO_MODE?.toLowerCase() === 'true' && verified.email && DEMO_EMAILS.has(verified.email)) { next(); return; } diff --git a/server/src/routes/admin.ts b/server/src/routes/admin.ts index 7cd32712..16a5b40b 100644 --- a/server/src/routes/admin.ts +++ b/server/src/routes/admin.ts @@ -449,7 +449,7 @@ router.put('/default-user-settings', (req: Request, res: Response) => { }); // ── Dev-only: test notification endpoints ────────────────────────────────────── -if (process.env.NODE_ENV === 'development') { +if (process.env.NODE_ENV?.toLowerCase() === 'development') { const { send } = require('../services/notificationService'); router.post('/dev/test-notification', async (req: Request, res: Response) => { diff --git a/server/src/routes/backup.ts b/server/src/routes/backup.ts index 2d9ce088..58e5995a 100644 --- a/server/src/routes/backup.ts +++ b/server/src/routes/backup.ts @@ -168,7 +168,7 @@ router.put('/auto-settings', (req: Request, res: Response) => { const msg = err instanceof Error ? err.message : String(err); res.status(500).json({ error: 'Could not save auto-backup settings', - detail: process.env.NODE_ENV !== 'production' ? msg : undefined, + detail: process.env.NODE_ENV?.toLowerCase() !== 'production' ? msg : undefined, }); } }); diff --git a/server/src/routes/oidc.ts b/server/src/routes/oidc.ts index 4a86a340..7aefe593 100644 --- a/server/src/routes/oidc.ts +++ b/server/src/routes/oidc.ts @@ -30,7 +30,7 @@ router.get('/login', async (req: Request, res: Response) => { const config = getOidcConfig(); if (!config) return res.status(400).json({ error: 'OIDC not configured' }); - if (config.issuer && !config.issuer.startsWith('https://') && process.env.NODE_ENV === 'production') { + if (config.issuer && !config.issuer.startsWith('https://') && process.env.NODE_ENV?.toLowerCase() === 'production') { return res.status(400).json({ error: 'OIDC issuer must use HTTPS in production' }); } @@ -85,7 +85,7 @@ router.get('/callback', async (req: Request, res: Response) => { const config = getOidcConfig(); if (!config) return res.redirect(frontendUrl('/login?oidc_error=not_configured')); - if (config.issuer && !config.issuer.startsWith('https://') && process.env.NODE_ENV === 'production') { + if (config.issuer && !config.issuer.startsWith('https://') && process.env.NODE_ENV?.toLowerCase() === 'production') { return res.redirect(frontendUrl('/login?oidc_error=issuer_not_https')); } diff --git a/server/src/scheduler.ts b/server/src/scheduler.ts index 87979ecc..12d81539 100644 --- a/server/src/scheduler.ts +++ b/server/src/scheduler.ts @@ -2,6 +2,7 @@ import cron, { type ScheduledTask } from 'node-cron'; import archiver from 'archiver'; import path from 'node:path'; import fs from 'node:fs'; +import { logInfo, logError } from './services/auditLog'; const dataDir = path.join(__dirname, '../data'); const backupsDir = path.join(dataDir, 'backups'); @@ -79,11 +80,9 @@ async function runBackup(): Promise { if (fs.existsSync(uploadsDir)) archive.directory(uploadsDir, 'uploads'); archive.finalize(); }); - const { logInfo: li } = require('./services/auditLog'); - li(`Auto-Backup created: ${filename}`); + logInfo(`Auto-Backup created: ${filename}`); } catch (err: unknown) { - const { logError: le } = require('./services/auditLog'); - le(`Auto-Backup: ${err instanceof Error ? err.message : err}`); + logError(`Auto-Backup: ${err instanceof Error ? err.message : err}`); if (fs.existsSync(outputPath)) fs.unlinkSync(outputPath); return; } @@ -94,23 +93,28 @@ async function runBackup(): Promise { } } -function cleanupOldBackups(keepDays: number): void { +function autoBackupTimestampMs(filename: string): number | null { + // auto-backup-2026-04-27T00-00-00.zip → 2026-04-27T00:00:00 + const stamp = filename.slice('auto-backup-'.length, -'.zip'.length); + const iso = stamp.replace(/T(\d{2})-(\d{2})-(\d{2})$/, 'T$1:$2:$3'); + const ms = Date.parse(iso); + return Number.isNaN(ms) ? null : ms; +} + +export function cleanupOldBackups(keepDays: number, now: number = Date.now()): void { try { - const MS_PER_DAY = 24 * 60 * 60 * 1000; - const cutoff = Date.now() - keepDays * MS_PER_DAY; - const files = fs.readdirSync(backupsDir).filter(f => f.endsWith('.zip')); + const cutoff = now - keepDays * 24 * 60 * 60 * 1000; + const files = fs.readdirSync(backupsDir).filter(f => f.startsWith('auto-backup-') && f.endsWith('.zip')); for (const file of files) { const filePath = path.join(backupsDir, file); - const stat = fs.statSync(filePath); - if (stat.birthtimeMs < cutoff) { + const ageMs = autoBackupTimestampMs(file) ?? fs.statSync(filePath).mtimeMs; + if (ageMs < cutoff) { fs.unlinkSync(filePath); - const { logInfo: li } = require('./services/auditLog'); - li(`Auto-Backup old backup deleted: ${file}`); + logInfo(`Auto-Backup old backup deleted: ${file}`); } } } catch (err: unknown) { - const { logError: le } = require('./services/auditLog'); - le(`Auto-Backup cleanup: ${err instanceof Error ? err.message : err}`); + logError(`Auto-Backup cleanup: ${err instanceof Error ? err.message : err}`); } } @@ -122,16 +126,14 @@ function start(): void { const settings = loadSettings(); if (!settings.enabled) { - const { logInfo: li } = require('./services/auditLog'); - li('Auto-Backup disabled'); + logInfo('Auto-Backup disabled'); return; } const expression = buildCronExpression(settings); const tz = process.env.TZ || 'UTC'; currentTask = cron.schedule(expression, runBackup, { timezone: tz }); - const { logInfo: li2 } = require('./services/auditLog'); - li2(`Auto-Backup scheduled: ${settings.interval} (${expression}), tz: ${tz}, retention: ${settings.keep_days === 0 ? 'forever' : settings.keep_days + ' days'}`); + logInfo(`Auto-Backup scheduled: ${settings.interval} (${expression}), tz: ${tz}, retention: ${settings.keep_days === 0 ? 'forever' : settings.keep_days + ' days'}`); } // Demo mode: hourly reset of demo user data @@ -139,19 +141,17 @@ let demoTask: ScheduledTask | null = null; function startDemoReset(): void { if (demoTask) { demoTask.stop(); demoTask = null; } - if (process.env.DEMO_MODE !== 'true') return; + if (process.env.DEMO_MODE?.toLowerCase() !== 'true') return; demoTask = cron.schedule('0 * * * *', () => { try { const { resetDemoUser } = require('./demo/demo-reset'); resetDemoUser(); } catch (err: unknown) { - const { logError: le } = require('./services/auditLog'); - le(`Demo reset: ${err instanceof Error ? err.message : err}`); + logError(`Demo reset: ${err instanceof Error ? err.message : err}`); } }); - const { logInfo: li3 } = require('./services/auditLog'); - li3('Demo hourly reset scheduled'); + logInfo('Demo hourly reset scheduled'); } // Trip reminders: daily check at 9 AM local time for trips starting tomorrow @@ -167,14 +167,12 @@ function startTripReminders(): void { const channelsRaw = getSetting('notification_channels') || getSetting('notification_channel') || 'none'; const activeChannels = channelsRaw === 'none' ? [] : channelsRaw.split(',').map((c: string) => c.trim()); if (!reminderEnabled) { - const { logInfo: li } = require('./services/auditLog'); - li('Trip reminders: disabled in settings'); + logInfo('Trip reminders: disabled in settings'); return; } const tripCount = (db.prepare('SELECT COUNT(*) as c FROM trips WHERE reminder_days > 0 AND start_date IS NOT NULL').get() as { c: number }).c; - const { logInfo: liSetup } = require('./services/auditLog'); - liSetup(`Trip reminders: enabled via [${activeChannels.join(',')}]${tripCount > 0 ? `, ${tripCount} trip(s) with active reminders` : ''}`); + logInfo(`Trip reminders: enabled via [${activeChannels.join(',')}]${tripCount > 0 ? `, ${tripCount} trip(s) with active reminders` : ''}`); } catch { return; } @@ -196,13 +194,11 @@ function startTripReminders(): void { await send({ event: 'trip_reminder', actorId: null, scope: 'trip', targetId: trip.id, params: { trip: trip.title, tripId: String(trip.id) } }).catch(() => {}); } - const { logInfo: li } = require('./services/auditLog'); if (trips.length > 0) { - li(`Trip reminders sent for ${trips.length} trip(s): ${trips.map(t => `"${t.title}" (${t.reminder_days}d)`).join(', ')}`); + logInfo(`Trip reminders sent for ${trips.length} trip(s): ${trips.map(t => `"${t.title}" (${t.reminder_days}d)`).join(', ')}`); } } catch (err: unknown) { - const { logError: le } = require('./services/auditLog'); - le(`Trip reminder check failed: ${err instanceof Error ? err.message : err}`); + logError(`Trip reminder check failed: ${err instanceof Error ? err.message : err}`); } }, { timezone: tz }); } @@ -222,12 +218,10 @@ function startTodoReminders(): void { const getSetting = (key: string) => (db.prepare('SELECT value FROM app_settings WHERE key = ?').get(key) as { value: string } | undefined)?.value; const enabled = getSetting('notify_todo_due') !== 'false'; if (!enabled) { - const { logInfo: li } = require('./services/auditLog'); - li('Todo due reminders: disabled in settings'); + logInfo('Todo due reminders: disabled in settings'); return; } - const { logInfo: liSetup } = require('./services/auditLog'); - liSetup(`Todo due reminders: enabled (lead ${TODO_REMINDER_LEAD_DAYS}d)`); + logInfo(`Todo due reminders: enabled (lead ${TODO_REMINDER_LEAD_DAYS}d)`); const tz = process.env.TZ || 'UTC'; todoReminderTask = cron.schedule('0 9 * * *', async () => { @@ -271,13 +265,11 @@ function startTodoReminders(): void { db.prepare('UPDATE todo_items SET reminded_at = CURRENT_TIMESTAMP WHERE id = ?').run(todo.id); } - const { logInfo: li } = require('./services/auditLog'); if (todos.length > 0) { - li(`Todo reminders sent for ${todos.length} item(s)`); + logInfo(`Todo reminders sent for ${todos.length} item(s)`); } } catch (err: unknown) { - const { logError: le } = require('./services/auditLog'); - le(`Todo reminder check failed: ${err instanceof Error ? err.message : err}`); + logError(`Todo reminder check failed: ${err instanceof Error ? err.message : err}`); } }, { timezone: tz }); } @@ -294,8 +286,7 @@ function startVersionCheck(): void { const { checkAndNotifyVersion } = require('./services/adminService'); await checkAndNotifyVersion(); } catch (err: unknown) { - const { logError: le } = require('./services/auditLog'); - le(`Version check: ${err instanceof Error ? err.message : err}`); + logError(`Version check: ${err instanceof Error ? err.message : err}`); } }, { timezone: tz }); } @@ -313,12 +304,10 @@ function startIdempotencyCleanup(): void { const cutoff = Math.floor(Date.now() / 1000) - 86400; const result = db.prepare('DELETE FROM idempotency_keys WHERE created_at < ?').run(cutoff); if (result.changes > 0) { - const { logInfo: li } = require('./services/auditLog'); - li(`Idempotency cleanup: removed ${result.changes} expired key(s)`); + logInfo(`Idempotency cleanup: removed ${result.changes} expired key(s)`); } } catch (err: unknown) { - const { logError: le } = require('./services/auditLog'); - le(`Idempotency cleanup: ${err instanceof Error ? err.message : err}`); + logError(`Idempotency cleanup: ${err instanceof Error ? err.message : err}`); } }, { timezone: tz }); } @@ -340,8 +329,7 @@ function startTrekPhotoCacheCleanup(): void { const { sweepExpired } = require('./services/memories/trekPhotoCache'); sweepExpired(); } catch (err: unknown) { - const { logError: le } = require('./services/auditLog'); - le(`Trek photo cache cleanup: ${err instanceof Error ? err.message : err}`); + logError(`Trek photo cache cleanup: ${err instanceof Error ? err.message : err}`); } }); } diff --git a/server/src/services/adminService.ts b/server/src/services/adminService.ts index 66a2d349..f0fc5420 100644 --- a/server/src/services/adminService.ts +++ b/server/src/services/adminService.ts @@ -8,6 +8,7 @@ import { updateJwtSecret } from '../config'; import { maybe_encrypt_api_key, decrypt_api_key } from './apiKeyCrypto'; import { getAllPermissions, savePermissions as savePerms, PERMISSION_ACTIONS } from './permissions'; import { revokeUserSessions, revokeUserSessionsForClient } from '../mcp'; +import { deleteUserCompletely } from './userCleanupService'; import { validatePassword } from './passwordPolicy'; import { getPhotoProviderConfig } from './memories/helpersService'; import { send as sendNotification } from './notificationService'; @@ -170,7 +171,7 @@ export function deleteUser(id: string, currentUserId: number) { const userToDel = db.prepare('SELECT id, email FROM users WHERE id = ?').get(id) as { id: number; email: string } | undefined; if (!userToDel) return { error: 'User not found', status: 404 }; - db.prepare('DELETE FROM users WHERE id = ?').run(id); + deleteUserCompletely(userToDel.id); return { email: userToDel.email }; } @@ -287,7 +288,7 @@ export function updateOidcSettings(data: { // ── Demo Baseline ────────────────────────────────────────────────────────── export function saveDemoBaseline(): { error?: string; status?: number; message?: string } { - if (process.env.DEMO_MODE !== 'true') { + if (process.env.DEMO_MODE?.toLowerCase() !== 'true') { return { error: 'Not found', status: 404 }; } try { diff --git a/server/src/services/authService.ts b/server/src/services/authService.ts index 3917e64b..ba949481 100644 --- a/server/src/services/authService.ts +++ b/server/src/services/authService.ts @@ -15,6 +15,7 @@ import { decrypt_api_key, maybe_encrypt_api_key, encrypt_api_key } from './apiKe import { createEphemeralToken } from './ephemeralTokens'; import { revokeUserSessions } from '../mcp'; import { startTripReminders } from '../scheduler'; +import { deleteUserCompletely } from './userCleanupService'; import { verifyJwtAndLoadUser } from '../middleware/auth'; import { User } from '../types'; import { DEMO_EMAIL_PRIMARY, isDemoEmail } from './demo'; @@ -130,7 +131,7 @@ export function resolveAuthToggles(): { oidc_login: get('oidc_login') !== 'false', oidc_registration: get('oidc_registration') !== 'false', }; - if (process.env.OIDC_ONLY === 'true') { + if (process.env.OIDC_ONLY?.toLowerCase() === 'true') { result.password_login = false; result.password_registration = false; } @@ -138,7 +139,7 @@ export function resolveAuthToggles(): { } // Legacy fallback - const oidcOnlyEnabled = process.env.OIDC_ONLY === 'true' || get('oidc_only') === 'true'; + const oidcOnlyEnabled = process.env.OIDC_ONLY?.toLowerCase() === 'true' || get('oidc_only') === 'true'; const oidcConfigured = !!( (process.env.OIDC_ISSUER || get('oidc_issuer')) && (process.env.OIDC_CLIENT_ID || get('oidc_client_id')) @@ -252,7 +253,7 @@ export function getPendingMfaSecret(userId: number): string | null { export function getAppConfig(authenticatedUser: { id: number } | null) { const userCount = (db.prepare('SELECT COUNT(*) as count FROM users').get() as { count: number }).count; - const isDemo = process.env.DEMO_MODE === 'true'; + const isDemo = process.env.DEMO_MODE?.toLowerCase() === 'true'; const toggles = resolveAuthToggles(); const version: string = process.env.APP_VERSION ?? require('../../package.json').version; const hasGoogleKey = !!db.prepare("SELECT maps_api_key FROM users WHERE role = 'admin' AND maps_api_key IS NOT NULL AND maps_api_key != '' LIMIT 1").get(); @@ -527,7 +528,7 @@ export function deleteAccount(userId: number, userEmail: string, userRole: strin return { error: 'Cannot delete the last admin account', status: 400 }; } } - db.prepare('DELETE FROM users WHERE id = ?').run(userId); + deleteUserCompletely(userId); return { success: true }; } diff --git a/server/src/services/cookie.ts b/server/src/services/cookie.ts index d1e88d2a..c97e187c 100644 --- a/server/src/services/cookie.ts +++ b/server/src/services/cookie.ts @@ -18,10 +18,10 @@ const COOKIE_NAME = 'trek_session'; * remains the explicit escape hatch for plain-HTTP LAN testing. */ export function cookieOptions(clear = false, req?: Request) { - if (process.env.COOKIE_SECURE === 'false') { + if (process.env.COOKIE_SECURE?.toLowerCase() === 'false') { return buildOptions(clear, false); } - const envSecure = process.env.NODE_ENV === 'production' || process.env.FORCE_HTTPS === 'true'; + const envSecure = process.env.NODE_ENV?.toLowerCase() === 'production' || process.env.FORCE_HTTPS?.toLowerCase() === 'true'; const requestSecure = req?.secure === true; return buildOptions(clear, envSecure || requestSecure); } diff --git a/server/src/services/notificationService.ts b/server/src/services/notificationService.ts index f14e48d8..67f02f2a 100644 --- a/server/src/services/notificationService.ts +++ b/server/src/services/notificationService.ts @@ -170,7 +170,7 @@ export async function send(payload: NotificationPayload): Promise { const configEntry = EVENT_NOTIFICATION_CONFIG[event]; if (!configEntry) { logDebug(`notificationService.send: unknown event type "${event}", using fallback`); - if (process.env.NODE_ENV === 'development' && actorId != null) { + if (process.env.NODE_ENV?.toLowerCase() === 'development' && actorId != null) { const devSender = (db.prepare('SELECT username, avatar FROM users WHERE id = ?').get(actorId) as { username: string; avatar: string | null } | undefined) ?? null; createNotificationForRecipient({ type: 'simple', diff --git a/server/src/services/tripService.ts b/server/src/services/tripService.ts index 1f129d2e..5b37f49f 100644 --- a/server/src/services/tripService.ts +++ b/server/src/services/tripService.ts @@ -117,10 +117,11 @@ export function generateDays(tripId: number | bigint | string, startDate: string } } - // Overflow dated days (trip shrunk): convert to dateless instead of deleting - const nullify = db.prepare('UPDATE days SET date = NULL, day_number = ? WHERE id = ?'); + // Overflow dated days (trip shrunk): delete them (issue #909). + // Cascade removes their assignments, notes, and accommodations. + const del = db.prepare('DELETE FROM days WHERE id = ?'); for (let i = targetDates.length; i < dated.length; i++) { - nullify.run(targetDates.length + (i - targetDates.length) + 1, dated[i].id); + del.run(dated[i].id); } // Any remaining unused dateless days: keep as dateless, just renumber. diff --git a/server/src/services/userCleanupService.ts b/server/src/services/userCleanupService.ts new file mode 100644 index 00000000..84239449 --- /dev/null +++ b/server/src/services/userCleanupService.ts @@ -0,0 +1,21 @@ +import { db } from '../db/database'; + +function cleanupUserReferences(userId: number): void { + db.prepare('UPDATE trip_members SET invited_by = NULL WHERE invited_by = ?').run(userId); + db.prepare('UPDATE budget_items SET paid_by_user_id = NULL WHERE paid_by_user_id = ?').run(userId); + db.prepare('DELETE FROM share_tokens WHERE created_by = ?').run(userId); + db.prepare('DELETE FROM journey_share_tokens WHERE created_by = ?').run(userId); + // Owned journeys cascade-delete their entries/contributors/share_tokens/photos via journey_id FKs + db.prepare('DELETE FROM journeys WHERE user_id = ?').run(userId); + // Entries authored on other users' journeys (not covered by the cascade above) + db.prepare('DELETE FROM journey_entries WHERE author_id = ?').run(userId); + db.prepare('DELETE FROM journey_contributors WHERE user_id = ?').run(userId); +} + +export function deleteUserCompletely(userId: number): void { + const tx = db.transaction((id: number) => { + cleanupUserReferences(id); + db.prepare('DELETE FROM users WHERE id = ?').run(id); + }); + tx(userId); +} diff --git a/server/src/utils/ssrfGuard.ts b/server/src/utils/ssrfGuard.ts index d5f2aa3f..19ed98dc 100644 --- a/server/src/utils/ssrfGuard.ts +++ b/server/src/utils/ssrfGuard.ts @@ -1,7 +1,7 @@ import dns from 'node:dns/promises'; import { Agent } from 'undici'; -const ALLOW_INTERNAL_NETWORK = process.env.ALLOW_INTERNAL_NETWORK === 'true'; +const ALLOW_INTERNAL_NETWORK = process.env.ALLOW_INTERNAL_NETWORK?.toLowerCase() === 'true'; export interface SsrfResult { allowed: boolean; diff --git a/server/tests/integration/admin.test.ts b/server/tests/integration/admin.test.ts index 0d105c61..beeaa0a1 100644 --- a/server/tests/integration/admin.test.ts +++ b/server/tests/integration/admin.test.ts @@ -41,7 +41,7 @@ import { createApp } from '../../src/app'; import { createTables } from '../../src/db/schema'; import { runMigrations } from '../../src/db/migrations'; import { resetTestDb } from '../helpers/test-db'; -import { createUser, createAdmin, createInviteToken } from '../helpers/factories'; +import { createUser, createAdmin, createInviteToken, createTrip, createBudgetItem, createJourney, createJourneyEntry, addJourneyContributor, addTripPhoto, createCategory, createTag, createTodoItem, createMcpToken, createBucketListItem, createVisitedCountry, createCollabNote, addTripMember } from '../helpers/factories'; import { authCookie } from '../helpers/auth'; import { loginAttempts, mfaAttempts } from '../../src/routes/auth'; @@ -148,6 +148,216 @@ describe('Admin user management', () => { expect(deleted).toBeUndefined(); }); + it('ADMIN-005b — DELETE /admin/users/:id succeeds when user has FK references', async () => { + const { user: admin } = createAdmin(testDb); + const { user: target } = createUser(testDb); + const { user: otherUser } = createUser(testDb); + const { user: thirdUser } = createUser(testDb); + + // trip_members.invited_by: target invited thirdUser to otherUser's trip + // (trip survives deletion; only invited_by should become NULL) + const otherTrip = createTrip(testDb, otherUser.id); + testDb.prepare('INSERT INTO trip_members (trip_id, user_id, invited_by) VALUES (?, ?, ?)').run(otherTrip.id, thirdUser.id, target.id); + + // share_tokens.created_by: target created a share token for otherUser's trip + testDb.prepare("INSERT INTO share_tokens (trip_id, token, created_by) VALUES (?, 'tok-admin-test', ?)").run(otherTrip.id, target.id); + + // budget_items.paid_by_user_id: target paid for an expense on otherUser's trip + const budgetItem = createBudgetItem(testDb, otherTrip.id); + testDb.prepare('UPDATE budget_items SET paid_by_user_id = ? WHERE id = ?').run(target.id, budgetItem.id); + + // journey_contributors: target is a contributor on otherUser's journey + const otherJourney = createJourney(testDb, otherUser.id); + addJourneyContributor(testDb, otherJourney.id, target.id); + + // journey_entries: target authored an entry on otherUser's journey + createJourneyEntry(testDb, otherJourney.id, target.id); + + // journey_share_tokens: target created a share token for otherUser's journey + testDb.prepare("INSERT INTO journey_share_tokens (journey_id, token, created_by) VALUES (?, 'jst-admin-test', ?)").run(otherJourney.id, target.id); + + // notifications.sender_id (SET NULL): target sent a notification to otherUser + const sentNotif = testDb.prepare( + "INSERT INTO notifications (type, scope, target, sender_id, recipient_id, title_key, text_key) VALUES ('simple', 'trip', ?, ?, ?, 'k', 'k')" + ).run(otherTrip.id, target.id, otherUser.id); + // notifications.recipient_id (CASCADE): otherUser sent a notification to target + testDb.prepare( + "INSERT INTO notifications (type, scope, target, sender_id, recipient_id, title_key, text_key) VALUES ('simple', 'trip', ?, ?, ?, 'k', 'k')" + ).run(otherTrip.id, otherUser.id, target.id); + + // user_notice_dismissals (CASCADE): target dismissed a notice + testDb.prepare( + "INSERT INTO user_notice_dismissals (user_id, notice_id, dismissed_at) VALUES (?, 'test-notice', ?)" + ).run(target.id, Date.now()); + + // owned journey: target owns a journey with an entry (cascade-deletes on journey deletion) + const ownedJourney = createJourney(testDb, target.id); + createJourneyEntry(testDb, ownedJourney.id, target.id); + + // trip_files.uploaded_by (SET NULL): target uploaded a file to otherUser's trip + const fileRow = testDb.prepare( + "INSERT INTO trip_files (trip_id, filename, original_name, uploaded_by) VALUES (?, 'f.pdf', 'file.pdf', ?)" + ).run(otherTrip.id, target.id); + + // trek_photos.owner_id (SET NULL): target owns a photo in the central registry + const trekPhotoRow = testDb.prepare( + "INSERT INTO trek_photos (provider, asset_id, owner_id) VALUES ('immich', 'asset-admin-test', ?)" + ).run(target.id); + + // trip_photos.user_id (CASCADE): target added a photo to otherUser's trip + addTripPhoto(testDb, otherTrip.id, target.id, 'asset-tp-admin', 'immich'); + + // trips.user_id (CASCADE): target owns a trip + const ownedTrip = createTrip(testDb, target.id); + + // trip_members.user_id (CASCADE): target is a member of otherUser's trip + addTripMember(testDb, otherTrip.id, target.id); + + // categories.user_id (SET NULL): target created a category + const userCategory = createCategory(testDb, { user_id: target.id }); + + // tags.user_id (CASCADE): target created a tag + const userTag = createTag(testDb, target.id); + + // todo_items.assigned_user_id (SET NULL): target is assigned to a todo on otherUser's trip + const todoItem = createTodoItem(testDb, otherTrip.id); + testDb.prepare('UPDATE todo_items SET assigned_user_id = ? WHERE id = ?').run(target.id, todoItem.id); + + // packing_bags.user_id (SET NULL): target owns a packing bag on otherUser's trip + const packBagRow = testDb.prepare( + "INSERT INTO packing_bags (trip_id, name, color, user_id) VALUES (?, 'Bag', '#ff0000', ?)" + ).run(otherTrip.id, target.id); + + // mcp_tokens.user_id (CASCADE): target has an MCP API token + createMcpToken(testDb, target.id); + + // oauth_tokens/consents.user_id (CASCADE): target has tokens from otherUser's OAuth client + testDb.prepare( + "INSERT INTO oauth_clients (id, user_id, name, client_id, client_secret_hash) VALUES ('cl-admin-test', ?, 'App', 'cid-admin-test', 'h')" + ).run(otherUser.id); + testDb.prepare( + "INSERT INTO oauth_tokens (client_id, user_id, access_token_hash, refresh_token_hash, access_token_expires_at, refresh_token_expires_at) VALUES ('cid-admin-test', ?, 'ath-admin', 'rth-admin', datetime('now','+1 hour'), datetime('now','+30 days'))" + ).run(target.id); + testDb.prepare( + "INSERT INTO oauth_consents (client_id, user_id) VALUES ('cid-admin-test', ?)" + ).run(target.id); + + // vacay_plans.owner_id (CASCADE): target owns a vacation plan + const vacayPlanRow = testDb.prepare("INSERT INTO vacay_plans (owner_id) VALUES (?)").run(target.id); + + // vacay_plan_members.user_id (CASCADE): target is a member of otherUser's vacay plan + const otherVacayPlanRow = testDb.prepare("INSERT INTO vacay_plans (owner_id) VALUES (?)").run(otherUser.id); + testDb.prepare("INSERT INTO vacay_plan_members (plan_id, user_id) VALUES (?, ?)").run(otherVacayPlanRow.lastInsertRowid, target.id); + + // bucket_list.user_id (CASCADE): target has a bucket list item + createBucketListItem(testDb, target.id); + + // visited_countries.user_id (CASCADE): target has visited a country + createVisitedCountry(testDb, target.id, 'JP'); + + // visited_regions.user_id (CASCADE): target has visited a region + testDb.prepare( + "INSERT INTO visited_regions (user_id, region_code, region_name, country_code) VALUES (?, 'JP-13', 'Tokyo', 'JP')" + ).run(target.id); + + // packing_templates.created_by (CASCADE): target created a packing template + const packTemplateRow = testDb.prepare( + "INSERT INTO packing_templates (name, created_by) VALUES ('My Template', ?)" + ).run(target.id); + + // invite_tokens.created_by (CASCADE): target created an invite token + createInviteToken(testDb, { created_by: target.id }); + + // collab_notes.user_id (CASCADE): target authored a collab note on otherUser's trip + createCollabNote(testDb, otherTrip.id, target.id); + + // settings.user_id (CASCADE): target has a user setting + testDb.prepare("INSERT INTO settings (user_id, key, value) VALUES (?, 'theme', 'dark')").run(target.id); + + // password_reset_tokens.user_id (CASCADE): target has a pending password reset + testDb.prepare( + "INSERT INTO password_reset_tokens (user_id, token_hash, expires_at) VALUES (?, 'prt-hash-admin', datetime('now','+1 hour'))" + ).run(target.id); + + // audit_log.user_id (SET NULL): target performed an audited action + const auditRow = testDb.prepare( + "INSERT INTO audit_log (user_id, action, ip) VALUES (?, 'test.action', '127.0.0.1')" + ).run(target.id); + + // notification_channel_preferences.user_id (CASCADE): target has notification preferences + testDb.prepare("INSERT OR IGNORE INTO notification_channel_preferences (user_id, event_type, channel) VALUES (?, 'trip_invite', 'email')").run(target.id); + + const res = await request(app) + .delete(`/api/admin/users/${target.id}`) + .set('Cookie', authCookie(admin.id)); + expect(res.status).toBe(200); + expect(res.body.success).toBe(true); + + expect(testDb.prepare('SELECT id FROM users WHERE id = ?').get(target.id)).toBeUndefined(); + // trip_members row survives but invited_by is now NULL + expect((testDb.prepare('SELECT invited_by FROM trip_members WHERE trip_id = ? AND user_id = ?').get(otherTrip.id, thirdUser.id) as any).invited_by).toBeNull(); + expect(testDb.prepare('SELECT id FROM share_tokens WHERE created_by = ?').get(target.id)).toBeUndefined(); + expect((testDb.prepare('SELECT paid_by_user_id FROM budget_items WHERE id = ?').get(budgetItem.id) as any).paid_by_user_id).toBeNull(); + expect(testDb.prepare('SELECT user_id FROM journey_contributors WHERE journey_id = ? AND user_id = ?').get(otherJourney.id, target.id)).toBeUndefined(); + expect(testDb.prepare('SELECT id FROM journey_entries WHERE author_id = ?').get(target.id)).toBeUndefined(); + expect(testDb.prepare('SELECT id FROM journey_share_tokens WHERE created_by = ?').get(target.id)).toBeUndefined(); + // sent notification survives but sender_id becomes NULL + expect((testDb.prepare('SELECT sender_id FROM notifications WHERE id = ?').get(sentNotif.lastInsertRowid) as any).sender_id).toBeNull(); + // received notification is cascade-deleted + expect(testDb.prepare('SELECT id FROM notifications WHERE recipient_id = ?').get(target.id)).toBeUndefined(); + // notice dismissals are cascade-deleted + expect(testDb.prepare("SELECT user_id FROM user_notice_dismissals WHERE user_id = ? AND notice_id = 'test-notice'").get(target.id)).toBeUndefined(); + // owned journey and its entries are cascade-deleted + expect(testDb.prepare('SELECT id FROM journeys WHERE user_id = ?').get(target.id)).toBeUndefined(); + expect(testDb.prepare('SELECT id FROM journey_entries WHERE journey_id = ?').get(ownedJourney.id)).toBeUndefined(); + // uploaded file survives but uploaded_by is now NULL + expect((testDb.prepare('SELECT uploaded_by FROM trip_files WHERE id = ?').get(fileRow.lastInsertRowid) as any).uploaded_by).toBeNull(); + // trek_photos row survives but owner_id is now NULL + expect((testDb.prepare('SELECT owner_id FROM trek_photos WHERE id = ?').get(trekPhotoRow.lastInsertRowid) as any).owner_id).toBeNull(); + // trip_photos row for target is cascade-deleted + expect(testDb.prepare("SELECT id FROM trip_photos WHERE trip_id = ? AND user_id = ?").get(otherTrip.id, target.id)).toBeUndefined(); + // owned trip is cascade-deleted + expect(testDb.prepare('SELECT id FROM trips WHERE id = ?').get(ownedTrip.id)).toBeUndefined(); + // trip membership on others' trips is removed + expect(testDb.prepare('SELECT id FROM trip_members WHERE trip_id = ? AND user_id = ?').get(otherTrip.id, target.id)).toBeUndefined(); + // category survives but user_id is NULL + expect((testDb.prepare('SELECT user_id FROM categories WHERE id = ?').get(userCategory.id) as any).user_id).toBeNull(); + // tag is deleted + expect(testDb.prepare('SELECT id FROM tags WHERE id = ?').get(userTag.id)).toBeUndefined(); + // todo assigned_user_id is NULL + expect((testDb.prepare('SELECT assigned_user_id FROM todo_items WHERE id = ?').get(todoItem.id) as any).assigned_user_id).toBeNull(); + // packing bag survives but user_id is NULL + expect((testDb.prepare('SELECT user_id FROM packing_bags WHERE id = ?').get(packBagRow.lastInsertRowid) as any).user_id).toBeNull(); + // MCP tokens are deleted + expect(testDb.prepare('SELECT id FROM mcp_tokens WHERE user_id = ?').get(target.id)).toBeUndefined(); + // OAuth tokens and consents are deleted + expect(testDb.prepare('SELECT id FROM oauth_tokens WHERE user_id = ?').get(target.id)).toBeUndefined(); + expect(testDb.prepare('SELECT id FROM oauth_consents WHERE user_id = ?').get(target.id)).toBeUndefined(); + // owned vacay plan is deleted + expect(testDb.prepare('SELECT id FROM vacay_plans WHERE id = ?').get(vacayPlanRow.lastInsertRowid)).toBeUndefined(); + // vacay plan membership on others' plans is removed + expect(testDb.prepare('SELECT id FROM vacay_plan_members WHERE plan_id = ? AND user_id = ?').get(otherVacayPlanRow.lastInsertRowid, target.id)).toBeUndefined(); + // bucket list items are deleted + expect(testDb.prepare('SELECT id FROM bucket_list WHERE user_id = ?').get(target.id)).toBeUndefined(); + // travel history is deleted + expect(testDb.prepare('SELECT user_id FROM visited_countries WHERE user_id = ? AND country_code = ?').get(target.id, 'JP')).toBeUndefined(); + expect(testDb.prepare('SELECT id FROM visited_regions WHERE user_id = ?').get(target.id)).toBeUndefined(); + // packing template is deleted + expect(testDb.prepare('SELECT id FROM packing_templates WHERE id = ?').get(packTemplateRow.lastInsertRowid)).toBeUndefined(); + // invite tokens created by target are deleted + expect(testDb.prepare('SELECT id FROM invite_tokens WHERE created_by = ?').get(target.id)).toBeUndefined(); + // collab content is deleted + expect(testDb.prepare('SELECT id FROM collab_notes WHERE user_id = ? AND trip_id = ?').get(target.id, otherTrip.id)).toBeUndefined(); + // user settings are deleted + expect(testDb.prepare("SELECT id FROM settings WHERE user_id = ?").get(target.id)).toBeUndefined(); + // password reset tokens are deleted + expect(testDb.prepare('SELECT id FROM password_reset_tokens WHERE user_id = ?').get(target.id)).toBeUndefined(); + // audit log entry survives but user_id is NULL + expect((testDb.prepare('SELECT user_id FROM audit_log WHERE id = ?').get(auditRow.lastInsertRowid) as any).user_id).toBeNull(); + // notification channel preferences are deleted + expect(testDb.prepare("SELECT user_id FROM notification_channel_preferences WHERE user_id = ? AND event_type = 'trip_invite'").get(target.id)).toBeUndefined(); + }); + it('ADMIN-006 — admin cannot delete their own account', async () => { const { user: admin } = createAdmin(testDb); diff --git a/server/tests/integration/auth.test.ts b/server/tests/integration/auth.test.ts index deb9df5a..d60dbc0e 100644 --- a/server/tests/integration/auth.test.ts +++ b/server/tests/integration/auth.test.ts @@ -52,7 +52,7 @@ import { createApp } from '../../src/app'; import { createTables } from '../../src/db/schema'; import { runMigrations } from '../../src/db/migrations'; import { resetTestDb } from '../helpers/test-db'; -import { createUser, createAdmin, createUserWithMfa, createInviteToken } from '../helpers/factories'; +import { createUser, createAdmin, createUserWithMfa, createInviteToken, createTrip, createBudgetItem, createJourney, createJourneyEntry, addJourneyContributor, addTripPhoto, createCategory, createTag, createTodoItem, createMcpToken, createBucketListItem, createVisitedCountry, createCollabNote, addTripMember } from '../helpers/factories'; import { authCookie, authHeader } from '../helpers/auth'; import { loginAttempts, mfaAttempts } from '../../src/routes/auth'; @@ -509,6 +509,225 @@ describe('Extended auth scenarios', () => { }); }); +// ───────────────────────────────────────────────────────────────────────────── +// Account deletion +// ───────────────────────────────────────────────────────────────────────────── + +describe('Account deletion', () => { + it('AUTH-040 — DELETE /auth/me succeeds when user has FK references', async () => { + const { user: admin } = createAdmin(testDb); + const { user: target } = createUser(testDb); + const { user: otherUser } = createUser(testDb); + const { user: thirdUser } = createUser(testDb); + + // trip_members.invited_by: target invited thirdUser to otherUser's trip + // (trip survives deletion; only invited_by should become NULL) + const otherTrip = createTrip(testDb, otherUser.id); + testDb.prepare('INSERT INTO trip_members (trip_id, user_id, invited_by) VALUES (?, ?, ?)').run(otherTrip.id, thirdUser.id, target.id); + + // share_tokens.created_by: target created a share token for otherUser's trip + testDb.prepare("INSERT INTO share_tokens (trip_id, token, created_by) VALUES (?, 'tok-auth-test', ?)").run(otherTrip.id, target.id); + + // budget_items.paid_by_user_id: target paid for an expense on otherUser's trip + const budgetItem = createBudgetItem(testDb, otherTrip.id); + testDb.prepare('UPDATE budget_items SET paid_by_user_id = ? WHERE id = ?').run(target.id, budgetItem.id); + + // journey_contributors: target is a contributor on otherUser's journey + const otherJourney = createJourney(testDb, otherUser.id); + addJourneyContributor(testDb, otherJourney.id, target.id); + + // journey_entries: target authored an entry on otherUser's journey + createJourneyEntry(testDb, otherJourney.id, target.id); + + // journey_share_tokens: target created a share token for otherUser's journey + testDb.prepare("INSERT INTO journey_share_tokens (journey_id, token, created_by) VALUES (?, 'jst-auth-test', ?)").run(otherJourney.id, target.id); + + // notifications.sender_id (SET NULL): target sent a notification to otherUser + const sentNotif = testDb.prepare( + "INSERT INTO notifications (type, scope, target, sender_id, recipient_id, title_key, text_key) VALUES ('simple', 'trip', ?, ?, ?, 'k', 'k')" + ).run(otherTrip.id, target.id, otherUser.id); + // notifications.recipient_id (CASCADE): otherUser sent a notification to target + testDb.prepare( + "INSERT INTO notifications (type, scope, target, sender_id, recipient_id, title_key, text_key) VALUES ('simple', 'trip', ?, ?, ?, 'k', 'k')" + ).run(otherTrip.id, otherUser.id, target.id); + + // user_notice_dismissals (CASCADE): target dismissed a notice + testDb.prepare( + "INSERT INTO user_notice_dismissals (user_id, notice_id, dismissed_at) VALUES (?, 'test-notice', ?)" + ).run(target.id, Date.now()); + + // owned journey: target owns a journey with an entry (cascade-deletes on journey deletion) + const ownedJourney = createJourney(testDb, target.id); + createJourneyEntry(testDb, ownedJourney.id, target.id); + + // trip_files.uploaded_by (SET NULL): target uploaded a file to otherUser's trip + const fileRow = testDb.prepare( + "INSERT INTO trip_files (trip_id, filename, original_name, uploaded_by) VALUES (?, 'f.pdf', 'file.pdf', ?)" + ).run(otherTrip.id, target.id); + + // trek_photos.owner_id (SET NULL): target owns a photo in the central registry + const trekPhotoRow = testDb.prepare( + "INSERT INTO trek_photos (provider, asset_id, owner_id) VALUES ('immich', 'asset-auth-test', ?)" + ).run(target.id); + + // trip_photos.user_id (CASCADE): target added a photo to otherUser's trip + addTripPhoto(testDb, otherTrip.id, target.id, 'asset-tp-auth', 'immich'); + + // trips.user_id (CASCADE): target owns a trip + const ownedTrip = createTrip(testDb, target.id); + + // trip_members.user_id (CASCADE): target is a member of otherUser's trip + addTripMember(testDb, otherTrip.id, target.id); + + // categories.user_id (SET NULL): target created a category + const userCategory = createCategory(testDb, { user_id: target.id }); + + // tags.user_id (CASCADE): target created a tag + const userTag = createTag(testDb, target.id); + + // todo_items.assigned_user_id (SET NULL): target is assigned to a todo on otherUser's trip + const todoItem = createTodoItem(testDb, otherTrip.id); + testDb.prepare('UPDATE todo_items SET assigned_user_id = ? WHERE id = ?').run(target.id, todoItem.id); + + // packing_bags.user_id (SET NULL): target owns a packing bag on otherUser's trip + const packBagRow = testDb.prepare( + "INSERT INTO packing_bags (trip_id, name, color, user_id) VALUES (?, 'Bag', '#ff0000', ?)" + ).run(otherTrip.id, target.id); + + // mcp_tokens.user_id (CASCADE): target has an MCP API token + createMcpToken(testDb, target.id); + + // oauth_tokens/consents.user_id (CASCADE): target has tokens from otherUser's OAuth client + testDb.prepare( + "INSERT INTO oauth_clients (id, user_id, name, client_id, client_secret_hash) VALUES ('cl-auth-test', ?, 'App', 'cid-auth-test', 'h')" + ).run(otherUser.id); + testDb.prepare( + "INSERT INTO oauth_tokens (client_id, user_id, access_token_hash, refresh_token_hash, access_token_expires_at, refresh_token_expires_at) VALUES ('cid-auth-test', ?, 'ath-auth', 'rth-auth', datetime('now','+1 hour'), datetime('now','+30 days'))" + ).run(target.id); + testDb.prepare( + "INSERT INTO oauth_consents (client_id, user_id) VALUES ('cid-auth-test', ?)" + ).run(target.id); + + // vacay_plans.owner_id (CASCADE): target owns a vacation plan + const vacayPlanRow = testDb.prepare("INSERT INTO vacay_plans (owner_id) VALUES (?)").run(target.id); + + // vacay_plan_members.user_id (CASCADE): target is a member of otherUser's vacay plan + const otherVacayPlanRow = testDb.prepare("INSERT INTO vacay_plans (owner_id) VALUES (?)").run(otherUser.id); + testDb.prepare("INSERT INTO vacay_plan_members (plan_id, user_id) VALUES (?, ?)").run(otherVacayPlanRow.lastInsertRowid, target.id); + + // bucket_list.user_id (CASCADE): target has a bucket list item + createBucketListItem(testDb, target.id); + + // visited_countries.user_id (CASCADE): target has visited a country + createVisitedCountry(testDb, target.id, 'JP'); + + // visited_regions.user_id (CASCADE): target has visited a region + testDb.prepare( + "INSERT INTO visited_regions (user_id, region_code, region_name, country_code) VALUES (?, 'JP-13', 'Tokyo', 'JP')" + ).run(target.id); + + // packing_templates.created_by (CASCADE): target created a packing template + const packTemplateRow = testDb.prepare( + "INSERT INTO packing_templates (name, created_by) VALUES ('My Template', ?)" + ).run(target.id); + + // invite_tokens.created_by (CASCADE): target created an invite token + createInviteToken(testDb, { created_by: target.id }); + + // collab_notes.user_id (CASCADE): target authored a collab note on otherUser's trip + createCollabNote(testDb, otherTrip.id, target.id); + + // settings.user_id (CASCADE): target has a user setting + testDb.prepare("INSERT INTO settings (user_id, key, value) VALUES (?, 'theme', 'dark')").run(target.id); + + // password_reset_tokens.user_id (CASCADE): target has a pending password reset + testDb.prepare( + "INSERT INTO password_reset_tokens (user_id, token_hash, expires_at) VALUES (?, 'prt-hash-auth', datetime('now','+1 hour'))" + ).run(target.id); + + // audit_log.user_id (SET NULL): target performed an audited action + const auditRow = testDb.prepare( + "INSERT INTO audit_log (user_id, action, ip) VALUES (?, 'test.action', '127.0.0.1')" + ).run(target.id); + + // notification_channel_preferences.user_id (CASCADE): target has notification preferences + testDb.prepare("INSERT OR IGNORE INTO notification_channel_preferences (user_id, event_type, channel) VALUES (?, 'trip_invite', 'email')").run(target.id); + + // admin exists to ensure target (non-admin user) passes the last-admin guard + void admin; + + const res = await request(app) + .delete('/api/auth/me') + .set('Cookie', authCookie(target.id)); + expect(res.status).toBe(200); + expect(res.body.success).toBe(true); + + expect(testDb.prepare('SELECT id FROM users WHERE id = ?').get(target.id)).toBeUndefined(); + // trip_members row survives but invited_by is now NULL + expect((testDb.prepare('SELECT invited_by FROM trip_members WHERE trip_id = ? AND user_id = ?').get(otherTrip.id, thirdUser.id) as any).invited_by).toBeNull(); + expect(testDb.prepare('SELECT id FROM share_tokens WHERE created_by = ?').get(target.id)).toBeUndefined(); + expect((testDb.prepare('SELECT paid_by_user_id FROM budget_items WHERE id = ?').get(budgetItem.id) as any).paid_by_user_id).toBeNull(); + expect(testDb.prepare('SELECT user_id FROM journey_contributors WHERE journey_id = ? AND user_id = ?').get(otherJourney.id, target.id)).toBeUndefined(); + expect(testDb.prepare('SELECT id FROM journey_entries WHERE author_id = ?').get(target.id)).toBeUndefined(); + expect(testDb.prepare('SELECT id FROM journey_share_tokens WHERE created_by = ?').get(target.id)).toBeUndefined(); + // sent notification survives but sender_id becomes NULL + expect((testDb.prepare('SELECT sender_id FROM notifications WHERE id = ?').get(sentNotif.lastInsertRowid) as any).sender_id).toBeNull(); + // received notification is cascade-deleted + expect(testDb.prepare('SELECT id FROM notifications WHERE recipient_id = ?').get(target.id)).toBeUndefined(); + // notice dismissals are cascade-deleted + expect(testDb.prepare("SELECT user_id FROM user_notice_dismissals WHERE user_id = ? AND notice_id = 'test-notice'").get(target.id)).toBeUndefined(); + // owned journey and its entries are cascade-deleted + expect(testDb.prepare('SELECT id FROM journeys WHERE user_id = ?').get(target.id)).toBeUndefined(); + expect(testDb.prepare('SELECT id FROM journey_entries WHERE journey_id = ?').get(ownedJourney.id)).toBeUndefined(); + // uploaded file survives but uploaded_by is now NULL + expect((testDb.prepare('SELECT uploaded_by FROM trip_files WHERE id = ?').get(fileRow.lastInsertRowid) as any).uploaded_by).toBeNull(); + // trek_photos row survives but owner_id is now NULL + expect((testDb.prepare('SELECT owner_id FROM trek_photos WHERE id = ?').get(trekPhotoRow.lastInsertRowid) as any).owner_id).toBeNull(); + // trip_photos row for target is cascade-deleted + expect(testDb.prepare("SELECT id FROM trip_photos WHERE trip_id = ? AND user_id = ?").get(otherTrip.id, target.id)).toBeUndefined(); + // owned trip is cascade-deleted + expect(testDb.prepare('SELECT id FROM trips WHERE id = ?').get(ownedTrip.id)).toBeUndefined(); + // trip membership on others' trips is removed + expect(testDb.prepare('SELECT id FROM trip_members WHERE trip_id = ? AND user_id = ?').get(otherTrip.id, target.id)).toBeUndefined(); + // category survives but user_id is NULL + expect((testDb.prepare('SELECT user_id FROM categories WHERE id = ?').get(userCategory.id) as any).user_id).toBeNull(); + // tag is deleted + expect(testDb.prepare('SELECT id FROM tags WHERE id = ?').get(userTag.id)).toBeUndefined(); + // todo assigned_user_id is NULL + expect((testDb.prepare('SELECT assigned_user_id FROM todo_items WHERE id = ?').get(todoItem.id) as any).assigned_user_id).toBeNull(); + // packing bag survives but user_id is NULL + expect((testDb.prepare('SELECT user_id FROM packing_bags WHERE id = ?').get(packBagRow.lastInsertRowid) as any).user_id).toBeNull(); + // MCP tokens are deleted + expect(testDb.prepare('SELECT id FROM mcp_tokens WHERE user_id = ?').get(target.id)).toBeUndefined(); + // OAuth tokens and consents are deleted + expect(testDb.prepare('SELECT id FROM oauth_tokens WHERE user_id = ?').get(target.id)).toBeUndefined(); + expect(testDb.prepare('SELECT id FROM oauth_consents WHERE user_id = ?').get(target.id)).toBeUndefined(); + // owned vacay plan is deleted + expect(testDb.prepare('SELECT id FROM vacay_plans WHERE id = ?').get(vacayPlanRow.lastInsertRowid)).toBeUndefined(); + // vacay plan membership on others' plans is removed + expect(testDb.prepare('SELECT id FROM vacay_plan_members WHERE plan_id = ? AND user_id = ?').get(otherVacayPlanRow.lastInsertRowid, target.id)).toBeUndefined(); + // bucket list items are deleted + expect(testDb.prepare('SELECT id FROM bucket_list WHERE user_id = ?').get(target.id)).toBeUndefined(); + // travel history is deleted + expect(testDb.prepare('SELECT user_id FROM visited_countries WHERE user_id = ? AND country_code = ?').get(target.id, 'JP')).toBeUndefined(); + expect(testDb.prepare('SELECT id FROM visited_regions WHERE user_id = ?').get(target.id)).toBeUndefined(); + // packing template is deleted + expect(testDb.prepare('SELECT id FROM packing_templates WHERE id = ?').get(packTemplateRow.lastInsertRowid)).toBeUndefined(); + // invite tokens created by target are deleted + expect(testDb.prepare('SELECT id FROM invite_tokens WHERE created_by = ?').get(target.id)).toBeUndefined(); + // collab content is deleted + expect(testDb.prepare('SELECT id FROM collab_notes WHERE user_id = ? AND trip_id = ?').get(target.id, otherTrip.id)).toBeUndefined(); + // user settings are deleted + expect(testDb.prepare("SELECT id FROM settings WHERE user_id = ?").get(target.id)).toBeUndefined(); + // password reset tokens are deleted + expect(testDb.prepare('SELECT id FROM password_reset_tokens WHERE user_id = ?').get(target.id)).toBeUndefined(); + // audit log entry survives but user_id is NULL + expect((testDb.prepare('SELECT user_id FROM audit_log WHERE id = ?').get(auditRow.lastInsertRowid) as any).user_id).toBeNull(); + // notification channel preferences are deleted + expect(testDb.prepare("SELECT user_id FROM notification_channel_preferences WHERE user_id = ? AND event_type = 'trip_invite'").get(target.id)).toBeUndefined(); + }); +}); + // ───────────────────────────────────────────────────────────────────────────── // Rate limiting (AUTH-004, AUTH-018) — placed last // ───────────────────────────────────────────────────────────────────────────── diff --git a/server/tests/integration/trips.test.ts b/server/tests/integration/trips.test.ts index f24905b1..01c718ee 100644 --- a/server/tests/integration/trips.test.ts +++ b/server/tests/integration/trips.test.ts @@ -463,7 +463,7 @@ describe('Update trip', () => { expect(notesAfter!.day_id).toBe(daysAfter[1].id); }); - it('TRIP-024 — Shrinking trip date range keeps overflow days as dateless with content intact', async () => { + it('TRIP-024 — Shrinking trip date range deletes overflow days and their content', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id, { start_date: '2026-09-01', end_date: '2026-09-05' }); @@ -481,13 +481,12 @@ describe('Update trip', () => { expect(res.status).toBe(200); const daysAfter = testDb.prepare('SELECT * FROM days WHERE trip_id = ? ORDER BY day_number').all(trip.id) as { id: number; date: string | null }[]; - expect(daysAfter).toHaveLength(5); - expect(daysAfter.filter(d => d.date !== null)).toHaveLength(3); - expect(daysAfter.filter(d => d.date === null)).toHaveLength(2); + expect(daysAfter).toHaveLength(3); + expect(daysAfter.every(d => d.date !== null)).toBe(true); - // Overflow assignments survived + // Overflow days and their assignments deleted const all = testDb.prepare('SELECT * FROM day_assignments WHERE id IN (?, ?)').all(a4.id, a5.id) as { id: number }[]; - expect(all).toHaveLength(2); + expect(all).toHaveLength(0); }); }); diff --git a/server/tests/unit/scheduler.test.ts b/server/tests/unit/scheduler.test.ts index 94447d33..69a4a2fa 100644 --- a/server/tests/unit/scheduler.test.ts +++ b/server/tests/unit/scheduler.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect, vi } from 'vitest'; +import { describe, it, expect, vi, beforeEach } from 'vitest'; // Prevent node-cron from scheduling anything at import time vi.mock('node-cron', () => ({ @@ -17,6 +17,7 @@ vi.mock('node:fs', () => ({ writeFileSync: vi.fn(), readdirSync: vi.fn(() => []), statSync: vi.fn(() => ({ mtime: new Date(), size: 0 })), + unlinkSync: vi.fn(), createWriteStream: vi.fn(() => ({ on: vi.fn(), pipe: vi.fn() })), }, existsSync: vi.fn(() => false), @@ -25,14 +26,20 @@ vi.mock('node:fs', () => ({ writeFileSync: vi.fn(), readdirSync: vi.fn(() => []), statSync: vi.fn(() => ({ mtime: new Date(), size: 0 })), + unlinkSync: vi.fn(), createWriteStream: vi.fn(() => ({ on: vi.fn(), pipe: vi.fn() })), })); vi.mock('../../../src/db/database', () => ({ db: { prepare: () => ({ all: vi.fn(() => []), get: vi.fn(), run: vi.fn() }) }, })); vi.mock('../../../src/config', () => ({ JWT_SECRET: 'test-secret', ENCRYPTION_KEY: '0'.repeat(64) })); +vi.mock('../../src/services/auditLog', () => ({ + logInfo: vi.fn(), + logError: vi.fn(), +})); -import { buildCronExpression } from '../../src/scheduler'; +import fs from 'node:fs'; +import { buildCronExpression, cleanupOldBackups } from '../../src/scheduler'; interface BackupSettings { enabled: boolean; @@ -130,3 +137,82 @@ describe('buildCronExpression', () => { }); }); }); + +describe('cleanupOldBackups', () => { + const DAY = 24 * 60 * 60 * 1000; + const NOW = new Date('2026-04-27T02:00:00Z').getTime(); + + function isoFilename(daysAgo: number, prefix: 'auto-backup' | 'backup' = 'auto-backup'): string { + const d = new Date(NOW - daysAgo * DAY); + const stamp = d.toISOString().replace(/[:.]/g, '-').slice(0, 19); + return `${prefix}-${stamp}.zip`; + } + + beforeEach(() => { + vi.mocked(fs.readdirSync).mockReset(); + vi.mocked(fs.statSync).mockReset(); + vi.mocked(fs.unlinkSync as ReturnType).mockReset(); + (vi.mocked(fs.statSync) as ReturnType).mockReturnValue({ mtime: new Date(), mtimeMs: NOW, birthtimeMs: NOW, size: 0 }); + }); + + it('never deletes manual backup-*.zip files regardless of age', () => { + const manual = isoFilename(365 * 5, 'backup'); + const auto = isoFilename(0); + vi.mocked(fs.readdirSync).mockReturnValue([manual, auto] as unknown as string[]); + cleanupOldBackups(7, NOW); + const deleted = (vi.mocked(fs.unlinkSync as ReturnType)).mock.calls.map((c: unknown[]) => c[0] as string); + expect(deleted.some((p: string) => p.includes(manual))).toBe(false); + }); + + it('keeps auto-backups newer than retention', () => { + const recent = isoFilename(3); + vi.mocked(fs.readdirSync).mockReturnValue([recent] as unknown as string[]); + cleanupOldBackups(7, NOW); + expect(vi.mocked(fs.unlinkSync as ReturnType)).not.toHaveBeenCalled(); + }); + + it('deletes auto-backups older than retention', () => { + const old = isoFilename(30); + vi.mocked(fs.readdirSync).mockReturnValue([old] as unknown as string[]); + cleanupOldBackups(7, NOW); + expect(vi.mocked(fs.unlinkSync as ReturnType)).toHaveBeenCalledOnce(); + const [calledPath] = (vi.mocked(fs.unlinkSync as ReturnType)).mock.calls[0] as string[]; + expect(calledPath).toContain(old); + }); + + it('overlayfs regression: birthtimeMs=0 does not delete a same-day backup', () => { + const fresh = isoFilename(0); + vi.mocked(fs.readdirSync).mockReturnValue([fresh] as unknown as string[]); + (vi.mocked(fs.statSync) as ReturnType).mockReturnValue({ birthtimeMs: 0, mtimeMs: NOW, mtime: new Date(NOW), size: 100 }); + cleanupOldBackups(7, NOW); + expect(vi.mocked(fs.unlinkSync as ReturnType)).not.toHaveBeenCalled(); + }); + + it('malformed filename falls back to mtimeMs: keeps recent file', () => { + vi.mocked(fs.readdirSync).mockReturnValue(['auto-backup-garbage.zip'] as unknown as string[]); + (vi.mocked(fs.statSync) as ReturnType).mockReturnValue({ birthtimeMs: 0, mtimeMs: NOW - 1 * DAY, mtime: new Date(NOW - 1 * DAY), size: 0 }); + cleanupOldBackups(7, NOW); + expect(vi.mocked(fs.unlinkSync as ReturnType)).not.toHaveBeenCalled(); + }); + + it('malformed filename falls back to mtimeMs: deletes stale file', () => { + vi.mocked(fs.readdirSync).mockReturnValue(['auto-backup-garbage.zip'] as unknown as string[]); + (vi.mocked(fs.statSync) as ReturnType).mockReturnValue({ birthtimeMs: 0, mtimeMs: NOW - 30 * DAY, mtime: new Date(NOW - 30 * DAY), size: 0 }); + cleanupOldBackups(7, NOW); + expect(vi.mocked(fs.unlinkSync as ReturnType)).toHaveBeenCalledOnce(); + }); + + it('ignores non-zip files and does not crash', () => { + const old = isoFilename(30); + vi.mocked(fs.readdirSync).mockReturnValue([old, 'notes.txt'] as unknown as string[]); + cleanupOldBackups(7, NOW); + const calls = (vi.mocked(fs.unlinkSync as ReturnType)).mock.calls as string[][]; + expect(calls.every(([p]: string[]) => !p.includes('notes.txt'))).toBe(true); + expect(calls.length).toBe(1); + }); + + it('swallows readdirSync errors without throwing', () => { + vi.mocked(fs.readdirSync).mockImplementation(() => { throw new Error('ENOENT'); }); + expect(() => cleanupOldBackups(7, NOW)).not.toThrow(); + }); +}); diff --git a/server/tests/unit/services/tripService.test.ts b/server/tests/unit/services/tripService.test.ts index 12c5a13d..795dbcb2 100644 --- a/server/tests/unit/services/tripService.test.ts +++ b/server/tests/unit/services/tripService.test.ts @@ -96,33 +96,37 @@ describe('generateDays', () => { expect(getNotes(day2.id)[0].id).toBe(note.id); }); - it('TRIP-SVC-011: shrinking range converts overflow days to dateless, preserves their assignments', () => { + it('TRIP-SVC-011: shrinking range deletes overflow days and their assignments (issue #909)', () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id, { start_date: '2025-07-01', end_date: '2025-07-05' }); const daysBefore = getDays(trip.id); expect(daysBefore).toHaveLength(5); const place = createPlace(testDb, trip.id); - // Assign places to days 4 and 5 (will become overflow) - const a4 = createDayAssignment(testDb, daysBefore[3].id, place.id); - const a5 = createDayAssignment(testDb, daysBefore[4].id, place.id); + createDayAssignment(testDb, daysBefore[3].id, place.id); + createDayAssignment(testDb, daysBefore[4].id, place.id); - // Shrink from 5 to 3 days + // Shrink from 5 to 3 days — surplus days and their content are removed generateDays(trip.id, '2025-07-01', '2025-07-03'); const daysAfter = getDays(trip.id); - expect(daysAfter).toHaveLength(5); // no rows deleted + expect(daysAfter).toHaveLength(3); + expect(daysAfter.map(d => d.date)).toEqual(['2025-07-01', '2025-07-02', '2025-07-03']); + }); - const dated = daysAfter.filter(d => d.date !== null); - const dateless = daysAfter.filter(d => d.date === null); - expect(dated).toHaveLength(3); - expect(dateless).toHaveLength(2); + it('TRIP-SVC-016: shrinking range deletes empty overflow days (issue #909)', () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id, { start_date: '2025-07-01', end_date: '2025-07-07' }); + expect(getDays(trip.id)).toHaveLength(7); - // Overflow days still have their assignments - expect(getAssignments(dateless[0].id)).toHaveLength(1); - expect(getAssignments(dateless[0].id)[0].id).toBe(a4.id); - expect(getAssignments(dateless[1].id)).toHaveLength(1); - expect(getAssignments(dateless[1].id)[0].id).toBe(a5.id); + // Shrink 7 → 5; days 6 and 7 have no content + generateDays(trip.id, '2025-07-01', '2025-07-05'); + + const daysAfter = getDays(trip.id); + expect(daysAfter).toHaveLength(5); + expect(daysAfter.map(d => d.date)).toEqual([ + '2025-07-01', '2025-07-02', '2025-07-03', '2025-07-04', '2025-07-05', + ]); }); it('TRIP-SVC-012: growing range keeps existing day content and appends new empty days', () => {