diff --git a/client/.gitignore b/client/.gitignore index 51af31d..5a569ad 100644 --- a/client/.gitignore +++ b/client/.gitignore @@ -35,3 +35,6 @@ storybook-static /blob-report/ /playwright/.auth/ /playwright/.cache/ + +# Vitest browser failure screenshots +__screenshots__/ diff --git a/client/e2e/auth.spec.ts b/client/e2e/auth.spec.ts index cd2c0ab..41a9c60 100644 --- a/client/e2e/auth.spec.ts +++ b/client/e2e/auth.spec.ts @@ -2,6 +2,10 @@ import { expect, test } from '@playwright/test'; // These tests cover the unauthenticated flows; they run without a saved // session (see the 'auth-specs' project in playwright.config.ts). +// Several of them perform real logins, which overwrite the user's single +// stored refresh token server-side — so they must not overlap. +test.describe.configure({ mode: 'serial' }); + test.describe('Authentication Flow', () => { test.beforeEach(async ({ page }) => { await page.goto('/'); @@ -68,6 +72,48 @@ test.describe('Authentication Flow', () => { await expect(page.getByRole('menuitem', { name: /logout/i })).toBeVisible(); }); + test('should keep the session when logging back in and reloading', async ({ + page, + }) => { + const login = async () => { + await page.goto('/login'); + await page.getByLabel(/email/i).fill('test@example.com'); + await page.getByLabel(/password/i).fill('testpassword'); + await page.getByRole('button', { name: /login|sign in/i }).click(); + await expect(page).toHaveURL(/.*\/app/); + }; + + await login(); + + // Log out via the UI, then log back in — all within the same SPA session. + await page.getByRole('button', { name: /user menu/i }).click(); + await page.getByRole('menuitem', { name: /logout/i }).click(); + await login(); + + // A reload must restore the session from the refresh cookie. + await page.reload(); + await expect(page).toHaveURL(/.*\/app/, { timeout: 10_000 }); + await expect(page.getByRole('button', { name: /user menu/i })).toBeVisible({ + timeout: 10_000, + }); + }); + + test('should not call authenticated logout when bootstrap has no session', async ({ + page, + }) => { + const logoutCalls: string[] = []; + page.on('request', (req) => { + if (req.url().includes('/api/auth/logout')) { + logoutCalls.push(req.url()); + } + }); + + await page.goto('/login'); + await expect(page.getByLabel(/email/i)).toBeVisible(); + + expect(logoutCalls).toEqual([]); + }); + test('should register a new account and redirect to login', async ({ page, }) => { diff --git a/client/src/components/common/NewDocumentFormBody/new-document-form-body.tsx b/client/src/components/common/NewDocumentFormBody/new-document-form-body.tsx index 795622e..b46a8a2 100644 --- a/client/src/components/common/NewDocumentFormBody/new-document-form-body.tsx +++ b/client/src/components/common/NewDocumentFormBody/new-document-form-body.tsx @@ -64,6 +64,7 @@ export default function NewDocumentFormBody({ type="text" label="Document Title" placeholder="New document title" + maxLength={200} error={errors.title} registration={register('title', { required: 'Title is required', diff --git a/client/src/components/ui/Auth/login-form.tsx b/client/src/components/ui/Auth/login-form.tsx index df77622..57dd17a 100644 --- a/client/src/components/ui/Auth/login-form.tsx +++ b/client/src/components/ui/Auth/login-form.tsx @@ -49,6 +49,7 @@ export default function LoginForm({ id="email" label="Email" placeholder="Enter your email" + maxLength={254} registration={register('email')} error={errors.email} autoComplete="email" @@ -59,6 +60,7 @@ export default function LoginForm({ id="password" label="Password" placeholder="Enter your password" + maxLength={128} registration={register('password')} error={errors.password} autoComplete="current-password" diff --git a/client/src/components/ui/Auth/register-form.tsx b/client/src/components/ui/Auth/register-form.tsx index ef4b595..8975fb3 100644 --- a/client/src/components/ui/Auth/register-form.tsx +++ b/client/src/components/ui/Auth/register-form.tsx @@ -46,6 +46,7 @@ export default function RegisterForm({ id="email" label="Email" placeholder="Enter your email" + maxLength={254} registration={register('email')} error={errors.email} autoComplete="email" @@ -56,6 +57,7 @@ export default function RegisterForm({ label="Username" id="username" placeholder="Enter your username" + maxLength={50} registration={register('username')} error={errors.username} autoComplete="username" @@ -66,6 +68,7 @@ export default function RegisterForm({ label="Full Name" id="fullname" placeholder="Enter your full name" + maxLength={100} registration={register('fullName')} error={errors.fullName} autoComplete="name" @@ -76,6 +79,7 @@ export default function RegisterForm({ id="password" label="Password" placeholder="Enter your password" + maxLength={128} registration={register('password')} error={errors.password} autoComplete="new-password" diff --git a/client/src/context/auth/auth-provider.tsx b/client/src/context/auth/auth-provider.tsx index 55d618e..c09d908 100644 --- a/client/src/context/auth/auth-provider.tsx +++ b/client/src/context/auth/auth-provider.tsx @@ -1,19 +1,29 @@ -import { useState, useEffect } from 'react'; +import { useEffect, useState } from 'react'; -import { api } from '@/lib/api'; +import { api, onSessionExpired, refreshAccessToken } from '@/lib/api'; import { type User } from '@/types/api'; import { setAccessToken as storeToken, clearAccessToken } from '@/utils/token'; import { AuthContext } from './auth-context'; /** - * Session bootstrap: refreshes the token cookie on mount, exposes login/logout and mirrors the token into api defaults and module storage. + * Session bootstrap: restores the session from the refresh cookie on mount, + * exposes login/logout and mirrors the token into api defaults and module + * storage. A failed bootstrap simply means signed out — it never calls the + * authenticated logout endpoint. */ export const AuthProvider = ({ children }: { children: React.ReactNode }) => { const [user, setUser] = useState(null); const [accessToken, setAccessToken] = useState(null); const [loading, setLoading] = useState(true); + const markSignedOut = () => { + setAccessToken(null); + setUser(null); + delete api.defaults.headers.common['Authorization']; + clearAccessToken(); + }; + const login = (token: string, user: User) => { setAccessToken(token); setUser(user); @@ -22,33 +32,32 @@ export const AuthProvider = ({ children }: { children: React.ReactNode }) => { }; const logout = async () => { - await api.post('/auth/logout'); - setAccessToken(null); - setUser(null); - localStorage.setItem('wasLoggedOut', 'true'); - delete api.defaults.headers.common['Authorization']; - clearAccessToken(); + // The server may already be unreachable or the session expired; local + // cleanup happens either way. + try { + await api.post('/auth/logout'); + } catch { + // session already dead server-side + } + markSignedOut(); }; useEffect(() => { - const refresh = async () => { - if (localStorage.getItem('wasLoggedOut') === 'true') { - localStorage.removeItem('wasLoggedOut'); - setLoading(false); - return; - } + const bootstrap = async () => { try { - const res = await api.post('/auth/refresh'); - const { accessToken, user } = res.data; + // Shares the single-flight with the response interceptor, so a + // bootstrap racing an in-flight refresh triggers only one request. + const { accessToken, user } = await refreshAccessToken(); login(accessToken, user); } catch { - logout(); + markSignedOut(); } finally { setLoading(false); } }; - refresh(); + bootstrap(); + return onSessionExpired(markSignedOut); }, []); if (loading) return null; diff --git a/client/src/features/Dashboard/components/DocumentCardDropdown/rename-modal.tsx b/client/src/features/Dashboard/components/DocumentCardDropdown/rename-modal.tsx index f4f9414..89e5144 100644 --- a/client/src/features/Dashboard/components/DocumentCardDropdown/rename-modal.tsx +++ b/client/src/features/Dashboard/components/DocumentCardDropdown/rename-modal.tsx @@ -85,6 +85,7 @@ export function RenameDocumentModal({ type="text" value={newTitle} onChange={(e) => setNewTitle(e.target.value)} + maxLength={200} className="w-full px-3 py-2 border border-gray-300 rounded-md focus:outline-none focus:ring-2 focus:ring-blue-500 focus:border-transparent" placeholder="Enter document title" onKeyDown={handleKeyDown} diff --git a/client/src/features/DocumentPage/components/DocumentHeader/CollaboratorsDropdown/collaborators-dropdown.tsx b/client/src/features/DocumentPage/components/DocumentHeader/CollaboratorsDropdown/collaborators-dropdown.tsx index 66f8b3f..ffd0da6 100644 --- a/client/src/features/DocumentPage/components/DocumentHeader/CollaboratorsDropdown/collaborators-dropdown.tsx +++ b/client/src/features/DocumentPage/components/DocumentHeader/CollaboratorsDropdown/collaborators-dropdown.tsx @@ -116,6 +116,7 @@ export const CollaboratorsDropdown = ({ placeholder="Email" value={email} onChange={(e) => setEmail(e.target.value)} + maxLength={254} aria-label="Collaborator email" className="h-8 min-w-0 flex-1 rounded border border-surface-border bg-transparent px-2 text-xs outline-none focus-visible:border-ring" /> diff --git a/client/src/features/DocumentPage/components/DocumentMain/document-main.stories.tsx b/client/src/features/DocumentPage/components/DocumentMain/document-main.stories.tsx index c399e6b..62b43b7 100644 --- a/client/src/features/DocumentPage/components/DocumentMain/document-main.stories.tsx +++ b/client/src/features/DocumentPage/components/DocumentMain/document-main.stories.tsx @@ -85,3 +85,194 @@ export const ReadOnly: Story = { await expect(canvas.queryByTitle('Bold')).toBeNull(); }, }; + +/** + * Provider stub seeding enough content that both split panes overflow and + * can actually scroll (the short default seed cannot). + */ +const longProviderFactory: CollabProviderFactory = (options) => { + const ytext = options.document.getText('content'); + if (!ytext.length) { + const paragraphs = Array.from( + { length: 120 }, + (_, i) => + `\n\nParagraph ${i + 1}: lorem ipsum dolor sit amet, consectetur adipiscing elit.`, + ).join(''); + ytext.insert(0, `# Long Document${paragraphs}`); + } + return { destroy: () => {} }; +}; + +/** + * Resolves after n animation frames so effects and rAF callbacks have run. + */ +const rafFrames = (n: number) => + new Promise((resolve) => { + const step = () => (--n <= 0 ? resolve() : requestAnimationFrame(step)); + requestAnimationFrame(step); + }); + +/** + * Resolves on the element's next scroll event; rejects after timeoutMs so a + * swallowed mirror update fails fast instead of hanging the run. + */ +const nextScrollEvent = (el: HTMLElement, timeoutMs = 1000) => + new Promise((resolve, reject) => { + const timer = setTimeout(() => { + el.removeEventListener('scroll', onScroll); + reject(new Error(`no scroll event within ${timeoutMs}ms`)); + }, timeoutMs); + const onScroll = () => { + clearTimeout(timer); + el.removeEventListener('scroll', onScroll); + resolve(); + }; + el.addEventListener('scroll', onScroll); + }); + +const renderSplitWithLongDoc: Story['render'] = function RenderedStory() { + return ( +
+ +
+ ); +}; + +/** Enables synced scrolling via the handle overlay button. */ +async function enableSyncScroll(canvasElement: HTMLElement) { + const toggle = canvasElement.querySelector( + '[data-panel-resize-handle-id] div.absolute', + ) as HTMLElement | null; + if (!toggle) throw new Error('scroll-sync toggle not found'); + toggle.click(); + await rafFrames(3); +} + +export const SplitSyncMirrorsScroll: Story = { + render: renderSplitWithLongDoc, + play: async ({ canvasElement }) => { + await enableSyncScroll(canvasElement); + const editor = canvasElement + .querySelector('.cm-editor') + ?.closest('div.custom-scrollbar.overflow-y-scroll') as HTMLElement; + const preview = canvasElement.querySelector( + '.markdown-previewer', + ) as HTMLElement; + + // Editor -> preview. Expectations are computed against live geometry: + // CodeMirror's scrollHeight can still grow during deep scrolls. + const edMax = editor.scrollHeight - editor.clientHeight; + const pvMax = preview.scrollHeight - preview.clientHeight; + + editor.scrollTop = edMax * 0.4; + await nextScrollEvent(preview); + await rafFrames(2); + expect( + Math.abs( + preview.scrollTop - + (editor.scrollTop / (editor.scrollHeight - editor.clientHeight)) * + (preview.scrollHeight - preview.clientHeight), + ), + ).toBeLessThan(pvMax * 0.05); + + // Preview -> editor + const pvMax2 = preview.scrollHeight - preview.clientHeight; + preview.scrollTop = pvMax2 * 0.8; + await nextScrollEvent(editor); + await rafFrames(2); + expect( + Math.abs( + editor.scrollTop - + (preview.scrollTop / (preview.scrollHeight - preview.clientHeight)) * + (editor.scrollHeight - editor.clientHeight), + ), + ).toBeLessThan(edMax * 0.05); + }, +}; + +export const SplitSyncSurvivesLateEcho: Story = { + render: renderSplitWithLongDoc, + play: async ({ canvasElement }) => { + await enableSyncScroll(canvasElement); + const editor = canvasElement + .querySelector('.cm-editor') + ?.closest('div.custom-scrollbar.overflow-y-scroll') as HTMLElement; + const preview = canvasElement.querySelector( + '.markdown-previewer', + ) as HTMLElement; + + const edMax = editor.scrollHeight - editor.clientHeight; + const pvMax = preview.scrollHeight - preview.clientHeight; + + // A normal mirrored scroll leaves both panes aligned... + editor.scrollTop = edMax * 0.5; + await nextScrollEvent(preview); + await rafFrames(2); + + // ...then the mirrored pane's own scroll event arrives one frame LATE + // (Firefox delivers it after the syncing flag was already reset). It must + // be recognized as an echo, not consume the suppression state. + preview.dispatchEvent(new Event('scroll')); + + // And a genuine editor scroll in the SAME task must still be mirrored. + editor.scrollTop = edMax * 0.75; + await rafFrames(5); + expect( + Math.abs( + preview.scrollTop - + (editor.scrollTop / (editor.scrollHeight - editor.clientHeight)) * + (preview.scrollHeight - preview.clientHeight), + ), + ).toBeLessThan(pvMax * 0.05); + }, +}; + +export const SplitSyncMirrorsLatestUnderBurst: Story = { + render: renderSplitWithLongDoc, + play: async ({ canvasElement }) => { + await enableSyncScroll(canvasElement); + const editor = canvasElement + .querySelector('.cm-editor') + ?.closest('div.custom-scrollbar.overflow-y-scroll') as HTMLElement; + const preview = canvasElement.querySelector( + '.markdown-previewer', + ) as HTMLElement; + + const edMax = editor.scrollHeight - editor.clientHeight; + const pvMax = preview.scrollHeight - preview.clientHeight; + + // Two scroll updates land within one frame but are delivered separately + // (Firefox dispatches a scroll event per wheel-tick write instead of + // coalescing them). The second must not be swallowed by suppression + // state left behind by the first: the mirror has to end up at the + // LATEST position once the frame flushes. + editor.scrollTop = edMax * 0.2; + await nextScrollEvent(editor); + editor.scrollTop = edMax * 0.6; + await rafFrames(3); + expect( + Math.abs( + preview.scrollTop - + (editor.scrollTop / (editor.scrollHeight - editor.clientHeight)) * + (preview.scrollHeight - preview.clientHeight), + ), + ).toBeLessThan(pvMax * 0.05); + + // ...and a third update after the burst still mirrors. + editor.scrollTop = edMax * 0.85; + await rafFrames(3); + expect( + Math.abs( + preview.scrollTop - + (editor.scrollTop / (editor.scrollHeight - editor.clientHeight)) * + (preview.scrollHeight - preview.clientHeight), + ), + ).toBeLessThan(pvMax * 0.05); + }, +}; diff --git a/client/src/features/DocumentPage/components/DocumentMain/document-main.tsx b/client/src/features/DocumentPage/components/DocumentMain/document-main.tsx index ca676af..da163b3 100644 --- a/client/src/features/DocumentPage/components/DocumentMain/document-main.tsx +++ b/client/src/features/DocumentPage/components/DocumentMain/document-main.tsx @@ -10,6 +10,7 @@ import { cn } from '@/utils/cn'; import { MarkdownEditor } from './MarkdownEditor'; import { MarkdownPreview } from './MarkdownPreview'; +import { useScrollSync } from './use-scroll-sync'; /** * Wires collaboration state into either MarkdownEditor or MarkdownPreview per view mode. @@ -44,8 +45,12 @@ export function DocumentMain({ ); const editorScrollRef = useRef(null); const previewScrollRef = useRef(null); - const isSyncingRef = useRef(false); const [syncScroll, setSyncScroll] = useState(false); + const { handleEditorScroll, handlePreviewScroll } = useScrollSync({ + enabled: syncScroll, + editorRef: editorScrollRef, + previewRef: previewScrollRef, + }); useEffect(() => { if (doc && text !== doc.content) { @@ -70,60 +75,6 @@ export function DocumentMain({ percent * (previewEl.scrollHeight - previewEl.clientHeight); }, [syncScroll]); - const handleEditorScroll = () => { - if (!syncScroll) return; - if ( - !syncScroll || - isSyncingRef.current || - !editorScrollRef.current || - !previewScrollRef.current - ) { - return; - } - - isSyncingRef.current = true; - - const editor = editorScrollRef.current; - const preview = previewScrollRef.current; - - const scrollRatio = - editor.scrollTop / (editor.scrollHeight - editor.clientHeight); - preview.scrollTop = - scrollRatio * (preview.scrollHeight - preview.clientHeight); - - // Use a shorter timeout and requestAnimationFrame - requestAnimationFrame(() => { - isSyncingRef.current = false; - }); - }; - - const handlePreviewScroll = () => { - if (!syncScroll) return; - - if ( - !syncScroll || - isSyncingRef.current || - !editorScrollRef.current || - !previewScrollRef.current - ) { - return; - } - - isSyncingRef.current = true; - - const editor = editorScrollRef.current; - const preview = previewScrollRef.current; - - const scrollRatio = - preview.scrollTop / (preview.scrollHeight - preview.clientHeight); - editor.scrollTop = - scrollRatio * (editor.scrollHeight - editor.clientHeight); - - // Use a shorter timeout and requestAnimationFrame - requestAnimationFrame(() => { - isSyncingRef.current = false; - }); - }; if (!docId || !isReady || !ydoc || !ytext || !provider) { return (
diff --git a/client/src/features/DocumentPage/components/DocumentMain/use-scroll-sync.ts b/client/src/features/DocumentPage/components/DocumentMain/use-scroll-sync.ts new file mode 100644 index 0000000..4acc6b4 --- /dev/null +++ b/client/src/features/DocumentPage/components/DocumentMain/use-scroll-sync.ts @@ -0,0 +1,100 @@ +import { useCallback, useEffect, useRef } from 'react'; + +type PaneKey = 'editor' | 'preview'; + +/** + * Race-free bidirectional scroll syncing between two panes. + * + * Mirrors scroll positions once per animation frame (latest wins) and + * recognizes its own mirrored writes by position: an event whose target is + * already at the last-written offset is an echo and is ignored. This replaces + * timing-flag suppression, which browsers deliver in different orders + * (Firefox dispatches the mirrored pane's scroll event after the reset frame, + * swallowing every other genuine update). + * + * @param args - Hook arguments. + * @param args.enabled - Mirrors only run while true. + * @param args.editorRef - Scrollable editor container. + * @param args.previewRef - Scrollable preview container. + * @returns Stable scroll handlers to attach to each pane's container. + */ +export function useScrollSync(args: { + /** Mirrors only run while true. */ + enabled: boolean; + /** Scrollable editor container. */ + editorRef: React.RefObject; + /** Scrollable preview container. */ + previewRef: React.RefObject; +}) { + const { enabled, editorRef, previewRef } = args; + const enabledRef = useRef(enabled); + const lastWritten = useRef>({ + editor: Number.NaN, + preview: Number.NaN, + }); + const frameRef = useRef(null); + const pendingSource = useRef(null); + + useEffect(() => { + enabledRef.current = enabled; + if (!enabled) { + if (frameRef.current !== null) cancelAnimationFrame(frameRef.current); + frameRef.current = null; + pendingSource.current = null; + lastWritten.current = { editor: Number.NaN, preview: Number.NaN }; + } + return () => { + if (frameRef.current !== null) cancelAnimationFrame(frameRef.current); + frameRef.current = null; + }; + }, [enabled]); + + const applyMirror = useCallback(() => { + frameRef.current = null; + const source = pendingSource.current; + pendingSource.current = null; + if (!source || !enabledRef.current) return; + + const from = source === 'editor' ? editorRef.current : previewRef.current; + const to = source === 'editor' ? previewRef.current : editorRef.current; + if (!from || !to) return; + + const fromRange = from.scrollHeight - from.clientHeight; + if (fromRange <= 0) return; + + const top = + (from.scrollTop / fromRange) * (to.scrollHeight - to.clientHeight); + lastWritten.current[source === 'editor' ? 'preview' : 'editor'] = top; + to.scrollTop = top; + }, [editorRef, previewRef]); + + const queueMirror = useCallback( + (which: PaneKey) => { + if (!enabledRef.current) return; + const el = which === 'editor' ? editorRef.current : previewRef.current; + if (!el) return; + + const written = lastWritten.current[which]; + if (!Number.isNaN(written) && Math.abs(el.scrollTop - written) < 1) { + return; + } + + pendingSource.current = which; + if (frameRef.current === null) { + frameRef.current = requestAnimationFrame(applyMirror); + } + }, + [editorRef, previewRef, applyMirror], + ); + + const handleEditorScroll = useCallback( + () => queueMirror('editor'), + [queueMirror], + ); + const handlePreviewScroll = useCallback( + () => queueMirror('preview'), + [queueMirror], + ); + + return { handleEditorScroll, handlePreviewScroll }; +} diff --git a/client/src/hooks/__tests__/use-collaborators.test.tsx b/client/src/hooks/__tests__/use-collaborators.test.tsx new file mode 100644 index 0000000..de669af --- /dev/null +++ b/client/src/hooks/__tests__/use-collaborators.test.tsx @@ -0,0 +1,158 @@ +import { act } from 'react'; +import { createElement } from 'react'; +import type { ReactNode } from 'react'; +import { createRoot, type Root } from 'react-dom/client'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import { api } from '@/lib/api'; + +vi.mock('@/lib/api', () => ({ + api: { + get: vi.fn(), + post: vi.fn(), + delete: vi.fn(), + }, +})); + +import { useCollaborators } from '../use-collaborators'; + +const mockApi = vi.mocked(api, true); + +/** + * Renders a hook inside a probe component mounted on document.body and + * exposes its latest return value, since no DOM-testing library is wired + * into the client suite. + * + * @param useHook - Hook factory invoked on every render of the probe. + * @returns The latest hook result plus unmount for cleanup. + */ +function renderHook(useHook: () => T) { + let result!: T; + let root!: Root; + + const Probe = () => { + result = useHook(); + return null; + }; + + const host = document.createElement('div'); + document.body.appendChild(host); + + act(() => { + root = createRoot(host); + root.render(createElement(Probe) as ReactNode); + }); + + return { + get current() { + return result; + }, + unmount: () => { + act(() => root.unmount()); + host.remove(); + }, + }; +} + +/** + * Flushes pending React updates and microtasks. Polls until the + * predicate holds, so `await act(async () => {})` empty flushes don't + * flake when the initial fetch resolves on the next microtask. + * + * @param predicate - Condition to wait for. + * @param timeoutMs - Fail after this long. + */ +async function waitFor(predicate: () => boolean, timeoutMs = 1000) { + const start = Date.now(); + while (!predicate()) { + if (Date.now() - start > timeoutMs) { + throw new Error('waitFor timeout'); + } + await act(async () => { + await new Promise((r) => { + setTimeout(r, 0); + }); + }); + } +} + +describe('useCollaborators', () => { + beforeEach(() => { + ( + globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean } + ).IS_REACT_ACT_ENVIRONMENT = true; + mockApi.get.mockResolvedValue({ data: [] }); + }); + + afterEach(() => { + vi.clearAllMocks(); + }); + + it('clears a stale add error when a retry succeeds', async () => { + const hook = renderHook(() => useCollaborators('doc1')); + await waitFor(() => !hook.current.loading); + + mockApi.post.mockRejectedValueOnce(new Error('boom')); + await act(async () => { + await hook.current.addCollaborator('a@b.c'); + }); + expect(hook.current.error).toBe('Failed to add collaborator'); + + mockApi.post.mockResolvedValueOnce({}); + let added = false; + await act(async () => { + added = await hook.current.addCollaborator('a@b.c'); + }); + + expect(added).toBe(true); + expect(hook.current.error).toBeNull(); + hook.unmount(); + }); + + it('clears a stale remove error when a retry succeeds', async () => { + const hook = renderHook(() => useCollaborators('doc1')); + await waitFor(() => !hook.current.loading); + + mockApi.delete.mockRejectedValueOnce(new Error('boom')); + await act(async () => { + await hook.current.removeCollaborator('u1'); + }); + expect(hook.current.error).toBe('Failed to remove collaborator'); + + mockApi.delete.mockResolvedValueOnce({}); + await act(async () => { + await hook.current.removeCollaborator('u1'); + }); + + expect(hook.current.error).toBeNull(); + hook.unmount(); + }); + + it('surfaces an error when adding fails', async () => { + const hook = renderHook(() => useCollaborators('doc1')); + await waitFor(() => !hook.current.loading); + + mockApi.post.mockRejectedValueOnce(new Error('boom')); + let added = true; + await act(async () => { + added = await hook.current.addCollaborator('a@b.c'); + }); + + expect(added).toBe(false); + expect(hook.current.error).toBe('Failed to add collaborator'); + hook.unmount(); + }); + + it('surfaces an error when removing fails', async () => { + const hook = renderHook(() => useCollaborators('doc1')); + await waitFor(() => !hook.current.loading); + + mockApi.delete.mockRejectedValueOnce(new Error('boom')); + await act(async () => { + await hook.current.removeCollaborator('u1'); + }); + + expect(hook.current.error).toBe('Failed to remove collaborator'); + hook.unmount(); + }); +}); diff --git a/client/src/hooks/use-collaborators.ts b/client/src/hooks/use-collaborators.ts index 92e70e8..5b7b133 100644 --- a/client/src/hooks/use-collaborators.ts +++ b/client/src/hooks/use-collaborators.ts @@ -41,6 +41,7 @@ export const useCollaborators = (docId?: string) => { const removeCollaborator = async (userId: string) => { if (!docId) return; + setError(null); try { await api.delete(`/document/${docId}/collaborators/${userId}`); setCollaborators((prev) => prev.filter((c) => c.id !== userId)); @@ -52,6 +53,7 @@ export const useCollaborators = (docId?: string) => { const addCollaborator = async (email: string) => { if (!docId || !email) return false; + setError(null); try { await api.post(`/document/${docId}/collaborators`, { email }); const res = await api.get(`/document/${docId}/collaborators`); diff --git a/client/src/lib/__tests__/api.test.ts b/client/src/lib/__tests__/api.test.ts new file mode 100644 index 0000000..3bca57c --- /dev/null +++ b/client/src/lib/__tests__/api.test.ts @@ -0,0 +1,157 @@ +import { createServer, type Server } from 'node:http'; + +import axios from 'axios'; +import { + afterAll, + beforeAll, + beforeEach, + describe, + expect, + it, + vi, +} from 'vitest'; + +import { clearAccessToken, setAccessToken } from '@/utils/token'; + +import { api, onSessionExpired } from '../api'; + +const PORT = 4599; +const BASE = `http://localhost:${PORT}`; + +let server: Server; +let refreshCallCount = 0; +let refreshShouldFail = false; +const protectedAuthHeaders: string[] = []; + +/** + * Minimal API stub: /protected requires the *new* token, /api/auth/refresh + * issues it once. Behaves like the real server for the paths under test. + */ +async function startStub(): Promise { + server = createServer((req, res) => { + let body = ''; + req.on('data', (chunk) => (body += chunk)); + req.on('end', () => { + if (req.url === '/api/auth/refresh') { + refreshCallCount += 1; + res.setHeader('Content-Type', 'application/json'); + if (refreshShouldFail) { + res.statusCode = 401; + res.end(JSON.stringify({ error: 'Unauthorized' })); + } else { + res.end( + JSON.stringify({ accessToken: 'new-token', user: { id: 'u1' } }), + ); + } + return; + } + if (req.url === '/protected') { + protectedAuthHeaders.push(req.headers.authorization ?? ''); + if (req.headers.authorization === 'Bearer new-token') { + res.end(JSON.stringify({ ok: true })); + } else { + res.statusCode = 401; + res.end(JSON.stringify({ error: 'Invalid or expired token' })); + } + return; + } + if (req.url === '/always-401') { + res.statusCode = 401; + res.end(JSON.stringify({ error: 'Invalid or expired token' })); + return; + } + res.statusCode = 404; + res.end(); + }); + }); + await new Promise((resolve) => server.listen(PORT, resolve)); +} + +beforeAll(startStub); +afterAll(() => new Promise((resolve) => server.close(() => resolve()))); + +beforeEach(() => { + vi.restoreAllMocks(); +}); + +describe('api response interceptor', () => { + it('retries a request once after refreshing on 401', async () => { + refreshCallCount = 0; + protectedAuthHeaders.length = 0; + setAccessToken('old-token'); + + const res = await api.get(`${BASE}/protected`); + + expect(res.data).toEqual({ ok: true }); + expect(refreshCallCount).toBe(1); + // first attempt used stale token, replay used the refreshed one + expect(protectedAuthHeaders).toEqual([ + 'Bearer old-token', + 'Bearer new-token', + ]); + }); + + it('issues only one refresh for concurrent 401 responses', async () => { + refreshCallCount = 0; + protectedAuthHeaders.length = 0; + setAccessToken('old-token'); + + const [a, b, c] = await Promise.all([ + api.get(`${BASE}/protected`), + api.get(`${BASE}/protected`), + api.get(`${BASE}/protected`), + ]); + + expect(a.data).toEqual({ ok: true }); + expect(b.data).toEqual({ ok: true }); + expect(c.data).toEqual({ ok: true }); + expect(refreshCallCount).toBe(1); + }); + + it('clears the token and notifies listeners when refresh fails', async () => { + refreshShouldFail = true; + setAccessToken('expired-token'); + const expired = vi.fn(); + const unsubscribe = onSessionExpired(expired); + + await expect(api.get(`${BASE}/always-401`)).rejects.toThrow(); + + expect(expired).toHaveBeenCalledTimes(1); + unsubscribe(); + refreshShouldFail = false; + }); + + it('notifies sessionExpired listeners only once for concurrent refresh failures', async () => { + refreshShouldFail = true; + refreshCallCount = 0; + setAccessToken('expired-token'); + const expired = vi.fn(); + const unsubscribe = onSessionExpired(expired); + + await Promise.allSettled([ + api.get(`${BASE}/always-401`), + api.get(`${BASE}/always-401`), + api.get(`${BASE}/always-401`), + ]); + + expect(expired).toHaveBeenCalledTimes(1); + expect(refreshCallCount).toBe(1); + unsubscribe(); + refreshShouldFail = false; + }); + + it('does not attempt a refresh when no access token is stored', async () => { + refreshCallCount = 0; + clearAccessToken(); + + await expect(api.get(`${BASE}/always-401`)).rejects.toThrow(); + expect(refreshCallCount).toBe(0); + }); + + it('uses the shared instance against the configured base URL without manual base joining', async () => { + // sanity check that the exported api is an axios instance wired to env config + expect( + axios.isAxiosError(await api.get(`${BASE}/missing`).catch((e) => e)), + ).toBe(true); + }); +}); diff --git a/client/src/lib/api.ts b/client/src/lib/api.ts index eb50be2..ee7d82f 100644 --- a/client/src/lib/api.ts +++ b/client/src/lib/api.ts @@ -1,11 +1,26 @@ import axios from 'axios'; import { env } from '@/config/env'; -import { getAccessToken } from '@/utils/token'; +import { type User } from '@/types/api'; +import { + clearAccessToken, + getAccessToken, + setAccessToken, +} from '@/utils/token'; + +declare module 'axios' { + export interface InternalAxiosRequestConfig { + /** Access token attached when the request was originally sent. */ + _tokenUsed?: string; + /** Marks a request already replayed after a refresh. */ + _retry?: boolean; + } +} /** * Shared axios instance for all API calls. - * Attaches the stored access token to every request. + * Attaches the stored access token to every request and transparently + * refreshes an expired session once on 401 before replaying the request. */ export const api = axios.create({ baseURL: `${env.API_URL}/api`, @@ -16,6 +31,108 @@ api.interceptors.request.use((config) => { const token = getAccessToken(); if (token) { config.headers.Authorization = `Bearer ${token}`; + config._tokenUsed = token; } return config; }); + +/** + * Callback invoked when the session cannot be recovered (refresh failed). + */ +type SessionExpiredListener = () => void; + +const sessionExpiredListeners = new Set(); + +/** + * Registers a callback invoked when the session expires and cannot be + * refreshed; returns a function that unsubscribes the callback. + */ +export const onSessionExpired = ( + listener: SessionExpiredListener, +): (() => void) => { + sessionExpiredListeners.add(listener); + return () => sessionExpiredListeners.delete(listener); +}; + +// Auth endpoints manage their own credentials; refreshing from their own 401s +// would loop. +const AUTH_PATHS = [ + '/auth/login', + '/auth/register', + '/auth/refresh', + '/auth/logout', +]; + +let refreshPromise: Promise | null = null; + +/** + * Payload returned by the refresh endpoint. + */ +interface RefreshPayload { + accessToken: string; + user: User; +} + +/** + * Requests a fresh access token via the httpOnly refresh cookie; concurrent + * callers share the in-flight request so only one round-trip happens. Stores + * the new token before resolving with the full payload. On failure the + * session is cleared and listeners notified once — concurrent waiters share + * the same rejection. + */ +export const refreshAccessToken = (): Promise => { + if (!refreshPromise) { + refreshPromise = axios + .post(`${env.API_URL}/api/auth/refresh`, null, { + withCredentials: true, + }) + .then((res) => { + setAccessToken(res.data.accessToken); + return res.data; + }) + .catch((err: unknown) => { + clearAccessToken(); + sessionExpiredListeners.forEach((listener) => listener()); + throw err; + }) + .finally(() => { + refreshPromise = null; + }); + } + return refreshPromise; +}; + +api.interceptors.response.use(undefined, async (error: unknown) => { + if (!axios.isAxiosError(error) || !error.config) throw error; + const original = error.config; + const isAuthCall = AUTH_PATHS.some((path) => original.url?.includes(path)); + + if ( + error.response?.status !== 401 || + original._retry || + isAuthCall || + !original._tokenUsed + ) { + throw error; + } + + original._retry = true; + try { + // A concurrent request may have refreshed (or cleared) the token while + // this one was in flight; reuse it instead of refreshing again. If the + // token was cleared (null) the session is already expired — don't replay + // with `Bearer null` or trigger a second refresh, just fail. + const current = getAccessToken(); + let token: string; + if (current !== original._tokenUsed) { + if (!current) throw error; + token = current; + } else { + token = (await refreshAccessToken()).accessToken; + } + original.headers.Authorization = `Bearer ${token}`; + return api(original); + } catch { + throw error; + } +}); diff --git a/client/src/lib/auth.ts b/client/src/lib/auth.ts index b966778..b26d8d8 100644 --- a/client/src/lib/auth.ts +++ b/client/src/lib/auth.ts @@ -7,22 +7,32 @@ import { useAuth } from '@/context/auth'; import { api } from './api'; /** - * Zod schema validating registration input: valid email, username (min 3 - * characters), password (min 8 characters), and optional full name. + * Zod schema validating registration input: valid email (max 254 chars), + * username (3-50 chars, letters/digits/dots/dashes/underscores), password + * (8-128 characters), and optional full name (max 100 chars). Mirrors the + * server-side RegisterUserSchema bounds so users never hit a 400. */ export const RegisterSchema = z.object({ - email: z.string().email(), - username: z.string().min(3), - password: z.string().min(8), - fullName: z.string().optional(), + email: z.string().email().max(254), + username: z + .string() + .min(3) + .max(50) + .regex( + /^[a-zA-Z0-9_.-]+$/, + 'Only letters, digits, dots, dashes and underscores allowed', + ), + password: z.string().min(8).max(128), + fullName: z.string().max(100).optional(), }); /** - * zod schema validating login credentials (email format + required password). + * Zod schema validating login credentials: valid email (max 254 chars) and a + * password within the server-accepted length bounds. */ export const LoginSchema = z.object({ - email: z.string().email(), - password: z.string().min(8), + email: z.string().email().max(254), + password: z.string().min(8).max(128), }); export type RegisterSchemaType = z.infer; diff --git a/client/vite.config.ts b/client/vite.config.ts index 58a6bc2..bf0e460 100644 --- a/client/vite.config.ts +++ b/client/vite.config.ts @@ -13,7 +13,21 @@ const dirname = ? __dirname : path.dirname(fileURLToPath(import.meta.url)); -// More info at: https://storybook.js.org/docs/next/writing-tests/integrations/vitest-addon +// Fresh object per project: vitest mutates browser instance configs while +// registering nested projects, so sharing one literal collides. +const browserConfig = () => + ({ + enabled: true, + headless: true, + provider: 'playwright', + instances: [ + { + browser: 'chromium', + }, + ], + }) as const; + +// More info: https://storybook.js.org/docs/next/writing-tests/integrations/vitest-addon export default defineConfig({ plugins: [react(), tailwindcss()], resolve: { @@ -39,6 +53,19 @@ export default defineConfig({ }, test: { projects: [ + { + extends: true, + test: { + name: 'unit', + include: ['src/**/*.test.{ts,tsx}'], + exclude: ['src/hooks/__tests__/**'], + environment: 'node', + env: { + VITE_APP_API_URL: 'http://localhost:4599', + VITE_APP_SOCKET_URL: 'ws://localhost:5000/collaboration', + }, + }, + }, { extends: true, plugins: [ @@ -51,21 +78,23 @@ export default defineConfig({ test: { name: 'storybook', // One browser instance already stretches CI runners; parallel - // files alongside the server suite starves vitest's runner. + // files alongside the server suite starve vitest's runner. fileParallelism: false, - browser: { - enabled: true, - headless: true, - provider: 'playwright', - instances: [ - { - browser: 'chromium', - }, - ], - }, + browser: browserConfig(), setupFiles: ['.storybook/vitest.setup.ts'], }, }, + { + // Hook-level tests run in a real browser too — stories can't + // exercise logic that needs API interactions. + extends: true, + test: { + name: 'browser-unit', + include: ['src/hooks/__tests__/**/*.test.{ts,tsx}'], + fileParallelism: false, + browser: browserConfig(), + }, + }, ], }, }); diff --git a/server/prisma/migrations/20260828010125_add_session/migration.sql b/server/prisma/migrations/20260828010125_add_session/migration.sql new file mode 100644 index 0000000..c97cee7 --- /dev/null +++ b/server/prisma/migrations/20260828010125_add_session/migration.sql @@ -0,0 +1,23 @@ +-- CreateTable +CREATE TABLE "sessions" ( + "id" TEXT NOT NULL, + "userId" TEXT NOT NULL, + "refreshToken" TEXT NOT NULL, + "jti" TEXT NOT NULL, + "expiresAt" TIMESTAMP(3) NOT NULL, + "createdAt" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP, + + CONSTRAINT "sessions_pkey" PRIMARY KEY ("id") +); + +-- CreateIndex +CREATE UNIQUE INDEX "sessions_refreshToken_key" ON "sessions"("refreshToken"); + +-- CreateIndex +CREATE UNIQUE INDEX "sessions_jti_key" ON "sessions"("jti"); + +-- CreateIndex +CREATE INDEX "sessions_userId_idx" ON "sessions"("userId"); + +-- AddForeignKey +ALTER TABLE "sessions" ADD CONSTRAINT "sessions_userId_fkey" FOREIGN KEY ("userId") REFERENCES "users"("id") ON DELETE CASCADE ON UPDATE CASCADE; diff --git a/server/prisma/schema.prisma b/server/prisma/schema.prisma index 13a3117..e7d87de 100644 --- a/server/prisma/schema.prisma +++ b/server/prisma/schema.prisma @@ -20,10 +20,24 @@ model User { Document Document[] Collaborator Collaborator[] CollaborationRequest CollaborationRequest[] + Session Session[] @@map("users") } +model Session { + id String @id @default(uuid()) + userId String + user User @relation(fields: [userId], references: [id], onDelete: Cascade) + refreshToken String @unique + jti String @unique + expiresAt DateTime + createdAt DateTime @default(now()) + + @@index([userId]) + @@map("sessions") +} + model Document { id String @id @default(uuid()) title String diff --git a/server/src/controllers/auth.controller.ts b/server/src/controllers/auth.controller.ts index 86abcff..2441318 100644 --- a/server/src/controllers/auth.controller.ts +++ b/server/src/controllers/auth.controller.ts @@ -1,4 +1,5 @@ import bcrypt from 'bcryptjs'; +import crypto from 'crypto'; import { Request, Response } from 'express'; import asyncErrorWrapper from 'express-async-handler'; import { StatusCodes } from 'http-status-codes'; @@ -10,6 +11,12 @@ import { getClientInfo } from '@/utils/getClientInfo'; import { LoginUserSchema } from '@/validations/login.schema'; import { RegisterUserSchema } from '@/validations/register.schema'; +/** + * Fixed bcrypt hash used to equalize timing when login is attempted for an + * unknown email (#83): one bcrypt compare runs in both failure paths. + */ +const DUMMY_PASSWORD_HASH = '$2b$10$lZKU2EGQLmnz9Fi65/t3GO/coz9zBl6zMMvDyd0EOBgeU1Y28ESHG'; + export const registerUser = asyncErrorWrapper(async (req: Request, res: Response) => { const clientInfo = getClientInfo(req); @@ -48,7 +55,7 @@ export const registerUser = asyncErrorWrapper(async (req: Request, res: Response username, existingField: existing.email === email ? 'email' : 'username', }); - res.status(StatusCodes.CONFLICT).json({ error: 'Username or email already exists' }); + res.status(StatusCodes.CONFLICT).json({ error: 'Registration failed' }); return; } @@ -111,14 +118,27 @@ export const loginUser = asyncErrorWrapper(async (req: Request, res: Response) = try { const user = await prisma.user.findUnique({ where: { email } }); + if (user && user.isActive === false) { + logger.warn('Login failed - user deactivated', { + action: 'LOGIN_USER_INACTIVE', + ...clientInfo, + userId: user.id, + email, + }); + res.status(StatusCodes.UNAUTHORIZED).json({ error: 'Invalid email or password' }); + return; + } + if (!user) { + await bcrypt.compare(password, DUMMY_PASSWORD_HASH); + logger.warn('Login failed - user not found', { action: 'LOGIN_USER_NOT_FOUND', ...clientInfo, email, }); - res.status(StatusCodes.UNAUTHORIZED).json({ error: result.error }); + res.status(StatusCodes.UNAUTHORIZED).json({ error: 'Invalid email or password' }); return; } @@ -133,7 +153,7 @@ export const loginUser = asyncErrorWrapper(async (req: Request, res: Response) = username: user.username, }); - res.status(StatusCodes.UNAUTHORIZED).json({ error: result.error }); + res.status(StatusCodes.UNAUTHORIZED).json({ error: 'Invalid email or password' }); return; } @@ -149,11 +169,13 @@ export const loginUser = asyncErrorWrapper(async (req: Request, res: Response) = } ); - // Generate Access Token with a short expiration time + // Generate refresh token with jti for rotation / reuse detection + const jti = crypto.randomUUID(); const refreshToken = jwt.sign( { userId: user.id, username: user.username, + jti, }, process.env.JWT_REFRESH_SECRET, { @@ -161,7 +183,16 @@ export const loginUser = asyncErrorWrapper(async (req: Request, res: Response) = } ); - // update user with referesh token + // Create a new session for this device; do not overwrite other sessions + await prisma.session.create({ + data: { + userId: user.id, + refreshToken, + jti, + expiresAt: new Date(Date.now() + 24 * 60 * 60 * 1000), + }, + }); + // Keep legacy column in sync for any external read (not used for auth) await prisma.user.update({ where: { email }, data: { refreshToken }, @@ -176,11 +207,14 @@ export const loginUser = asyncErrorWrapper(async (req: Request, res: Response) = tokenExpiry: '15m', }); + // Production uses cross-site cookies, which require SameSite=None + Secure. + const isProduction = process.env.NODE_ENV === 'production'; + res.cookie('refreshToken', refreshToken, { httpOnly: true, maxAge: 24 * 60 * 60 * 1000, - sameSite: 'none', // ✅ allow cross-site cookies - secure: true, // ✅ must be secure for SameSite=None + sameSite: isProduction ? 'none' : 'lax', + secure: isProduction, }); res.status(StatusCodes.OK).json({ @@ -225,6 +259,22 @@ export const logoutUser = asyncErrorWrapper(async (req: AuthenticatedRequest, re } try { + const presented = req.cookies?.refreshToken as string | undefined; + if (presented) { + try { + const dec = jwt.verify(presented, process.env.JWT_REFRESH_SECRET!) as jwt.JwtPayload & { jti?: string }; + if (dec.jti) { + await prisma.session.deleteMany({ where: { jti: dec.jti, userId } }); + } else { + await prisma.session.deleteMany({ where: { refreshToken: presented, userId } }); + } + } catch { + await prisma.session.deleteMany({ where: { refreshToken: presented, userId } }); + } + } else { + // Fallback: clear all sessions for user if no token presented (e.g. legacy) + await prisma.session.deleteMany({ where: { userId } }); + } await prisma.user.update({ where: { id: userId }, data: { refreshToken: null }, @@ -236,8 +286,18 @@ export const logoutUser = asyncErrorWrapper(async (req: AuthenticatedRequest, re userId, }); - res.clearCookie('refreshToken'); - res.clearCookie('accessToken'); + // Must match the attributes used when setting the cookie, otherwise + // browsers keep the SameSite=None; Secure cookie (prod) alive and a + // subsequent refresh still succeeds after logout. + const isProduction = process.env.NODE_ENV === 'production'; + const clearOpts = { + httpOnly: true, + secure: isProduction, + sameSite: (isProduction ? 'none' : 'lax') as 'none' | 'lax', + path: '/', + }; + res.clearCookie('refreshToken', clearOpts); + res.clearCookie('accessToken', clearOpts); res.status(StatusCodes.OK).json({ message: 'Logged out successfully' }); } catch (error) { logger.error('Logout failed - database error', { @@ -286,21 +346,109 @@ export const refreshToken = asyncErrorWrapper(async (req: Request, res: Response return; } try { + const payloadWithJti = payload as jwt.JwtPayload & { jti?: string }; const user = await prisma.user.findUnique({ where: { id: payload.userId } }); - if (!user || user.refreshToken !== refreshToken) { + if (!user) { logger.warn('Token refresh failed - token mismatch or user not found', { action: 'REFRESH_TOKEN_MISMATCH', ...clientInfo, userId: payload.userId, - userExists: !!user, - tokenMatches: user?.refreshToken === refreshToken, + userExists: false, + tokenMatches: false, }); + res.status(StatusCodes.UNAUTHORIZED).json({ error: 'Invalid refresh token' }); + return; + } + // isActive enforcement — deactivated accounts cannot refresh + if (user.isActive === false) { + logger.warn('Token refresh failed - user deactivated', { + action: 'REFRESH_TOKEN_USER_INACTIVE', + ...clientInfo, + userId: user.id, + }); res.status(StatusCodes.UNAUTHORIZED).json({ error: 'Invalid refresh token' }); return; } + // Prefer Session lookup by jti (new tokens); fallback to legacy column for old tokens + let session: Awaited> | null = null; + if (payloadWithJti.jti) { + session = await prisma.session.findUnique({ where: { jti: payloadWithJti.jti } }); + + // Reuse detection: valid JWT for user but no matching session → token was already rotated/revoked + if (!session || session.refreshToken !== refreshToken || session.userId !== payload.userId) { + logger.warn('Token refresh failed - token reuse detected', { + action: 'REFRESH_TOKEN_REUSE', + ...clientInfo, + userId: payload.userId, + jti: payloadWithJti.jti, + }); + res.status(StatusCodes.UNAUTHORIZED).json({ error: 'Invalid refresh token' }); + return; + } + + if (session.expiresAt < new Date()) { + logger.warn('Token refresh failed - session expired', { + action: 'REFRESH_TOKEN_EXPIRED', + ...clientInfo, + userId: payload.userId, + }); + await prisma.session.delete({ where: { id: session.id } }); + res.status(StatusCodes.UNAUTHORIZED).json({ error: 'Invalid refresh token' }); + return; + } + } else { + // Legacy token without jti — fall back to single-column check + if (user.refreshToken !== refreshToken) { + logger.warn('Token refresh failed - token mismatch or user not found', { + action: 'REFRESH_TOKEN_MISMATCH', + ...clientInfo, + userId: payload.userId, + userExists: true, + tokenMatches: false, + }); + res.status(StatusCodes.UNAUTHORIZED).json({ error: 'Invalid refresh token' }); + return; + } + // Migrate legacy: create a session for this token so future rotates work + session = await prisma.session.create({ + data: { + userId: user.id, + refreshToken, + jti: crypto.randomUUID(), + expiresAt: new Date(Date.now() + 24 * 60 * 60 * 1000), + }, + }); + } + + // Rotate: new jti + new refresh token, update same session row + const newJti = crypto.randomUUID(); + const newRefreshToken = jwt.sign( + { + userId: user.id, + username: user.username, + jti: newJti, + }, + process.env.JWT_REFRESH_SECRET!, + { expiresIn: '24h' } + ); + + await prisma.session.update({ + where: { id: session.id }, + data: { + refreshToken: newRefreshToken, + jti: newJti, + expiresAt: new Date(Date.now() + 24 * 60 * 60 * 1000), + }, + }); + // Keep legacy column in sync (not used for auth, but for observability) + await prisma.user.update({ + where: { id: user.id }, + data: { refreshToken: newRefreshToken }, + }); + const newAccessToken = jwt.sign( { userId: user.id, @@ -317,11 +465,12 @@ export const refreshToken = asyncErrorWrapper(async (req: Request, res: Response username: user.username, }); - res.cookie('accessToken', newAccessToken, { + const isProduction = process.env.NODE_ENV === 'production'; + res.cookie('refreshToken', newRefreshToken, { httpOnly: true, - maxAge: 15 * 60 * 1000, - sameSite: 'none', // ✅ - secure: true, // ✅ + maxAge: 24 * 60 * 60 * 1000, + sameSite: isProduction ? 'none' : 'lax', + secure: isProduction, }); res.status(StatusCodes.OK).json({ diff --git a/server/src/controllers/document.controller.ts b/server/src/controllers/document.controller.ts index e958b23..b8fdea8 100644 --- a/server/src/controllers/document.controller.ts +++ b/server/src/controllers/document.controller.ts @@ -544,7 +544,8 @@ export const getShareLink = asyncErrorWrapper(async (req: AuthenticatedRequest, const clientInfo = getClientInfo(req); const userId = req.user?.userId; const { id } = req.params; - const { permission = 'view' } = req.query; + // Validated + defaulted by ShareLinkQuerySchema on the route + const { permission } = req.query as { permission: 'view' | 'edit' }; logger.debug('Share link generation attempt', { action: 'GENERATE_SHARE_LINK_ATTEMPT', @@ -554,19 +555,6 @@ export const getShareLink = asyncErrorWrapper(async (req: AuthenticatedRequest, permission, }); - if (!['view', 'edit'].includes(permission as string)) { - logger.warn('Share link generation failed - invalid permission', { - action: 'GENERATE_SHARE_LINK_INVALID_PERMISSION', - ...clientInfo, - userId, - documentId: id, - permission, - }); - - res.status(StatusCodes.BAD_REQUEST).json({ error: 'Invalid permission' }); - return; - } - try { const doc = await prisma.document.findUnique({ where: { id } }); @@ -584,7 +572,7 @@ export const getShareLink = asyncErrorWrapper(async (req: AuthenticatedRequest, return; } - const token = generateShareToken(doc.shareId, permission as 'view' | 'edit'); + const token = generateShareToken(doc.shareId, permission); // Updated URL structure - token is now in the path const url = `${process.env.CLIENT_BASE}/app/doc/share/${token}`; diff --git a/server/src/middlewares/auth.middleware.ts b/server/src/middlewares/auth.middleware.ts index a7e9409..6a57abb 100644 --- a/server/src/middlewares/auth.middleware.ts +++ b/server/src/middlewares/auth.middleware.ts @@ -3,7 +3,9 @@ import { StatusCodes } from 'http-status-codes'; import jwt from 'jsonwebtoken'; import { JwtPayload } from 'jsonwebtoken'; -export const authenticate = (req: AuthenticatedRequest, res: Response, next: NextFunction) => { +import { prisma } from '@/lib/prisma'; + +export const authenticate = async (req: AuthenticatedRequest, res: Response, next: NextFunction) => { const authHeader = req.headers.authorization; if (!authHeader || !authHeader.startsWith('Bearer ')) { @@ -13,7 +15,15 @@ export const authenticate = (req: AuthenticatedRequest, res: Response, next: Nex const token = authHeader.split(' ')[1]; try { - const decoded = jwt.verify(token, process.env.JWT_ACCESS_SECRET!) as JwtPayload; + const decoded = jwt.verify(token, process.env.JWT_ACCESS_SECRET!) as JwtPayload & { userId: string }; + // Enforce isActive so deactivated accounts lose access even with a valid JWT (15m window) + const user = await prisma.user.findUnique({ + where: { id: decoded.userId }, + select: { isActive: true }, + }); + if (user && user.isActive === false) { + return res.status(StatusCodes.UNAUTHORIZED).json({ error: 'Invalid or expired token' }); + } req.user = { userId: decoded.userId, username: decoded.username }; next(); } catch { diff --git a/server/src/routers/auth.router.ts b/server/src/routers/auth.router.ts index 40b8c5f..5043791 100644 --- a/server/src/routers/auth.router.ts +++ b/server/src/routers/auth.router.ts @@ -1,7 +1,7 @@ import express from 'express'; import { loginUser, logoutUser, refreshToken, registerUser } from '@/controllers/auth.controller'; -import { authenticate, validateRefreshToken } from '@/middlewares/auth.middleware'; +import { validateRefreshToken } from '@/middlewares/auth.middleware'; import { authLimiter } from '@/middlewares/rate-limit.middleware'; import { validate } from '@/middlewares/validation.middleware'; import { LoginUserSchema } from '@/validations/login.schema'; @@ -11,5 +11,5 @@ export const authRouter = express.Router(); authRouter.post('/register', authLimiter, validate({ body: RegisterUserSchema }), registerUser); authRouter.post('/login', authLimiter, validate({ body: LoginUserSchema }), loginUser); -authRouter.post('/logout', authenticate, logoutUser); +authRouter.post('/logout', validateRefreshToken, logoutUser); authRouter.post('/refresh', authLimiter, validateRefreshToken, refreshToken); diff --git a/server/src/routers/document.router.ts b/server/src/routers/document.router.ts index c7bdddd..8f9a32c 100644 --- a/server/src/routers/document.router.ts +++ b/server/src/routers/document.router.ts @@ -19,26 +19,43 @@ import { import { authenticate } from '@/middlewares/auth.middleware'; import { validate } from '@/middlewares/validation.middleware'; import { AddCollaboratorSchema } from '@/validations/addCollaborator.schema'; +import { CreateDocumentSchema } from '@/validations/createDocument.schema'; +import { + CollaboratorParamsSchema, + IdParamsSchema, + RequestIdParamsSchema, + ShareLinkQuerySchema, +} from '@/validations/documentParams.schema'; +import { UpdateDocSettingsSchema } from '@/validations/updateDocSettings.schema'; +import { UpdateDocumentSchema } from '@/validations/updateDocument.schema'; export const docRouter = express.Router(); docRouter.use(authenticate); docRouter.get('/share/:token', getDocByToken); -docRouter.post('/', createDoc); +docRouter.post('/', validate({ body: CreateDocumentSchema }), createDoc); docRouter.get('/', getDocs); -docRouter.get('/:id', getDoc); -docRouter.put('/:id', updateDoc); -docRouter.delete('/:id', deleteDoc); - -docRouter.patch('/:id/settings', updateDocSettings); // Used to toggle allowSelfJoin for the document // !Owner only access - -docRouter.get('/:id/share-link', getShareLink); // get the document share link with the share token - -docRouter.get('/:id/collaborators', getCollaborators); // returns list -docRouter.post('/:id/collaborators', validate({ body: AddCollaboratorSchema }), addCollaborator); // adds a new one by email //!Owner only access -docRouter.delete('/:id/collaborators/:userId', removeCollaborator); // optional - -docRouter.get('/:id/requests', getRequests); // !Owner only access -docRouter.post('/:id/requests/:requestId/approve', approveRequest); -docRouter.delete('/:id/requests/:requestId/reject', rejectRequest); +docRouter.get('/:id', validate({ params: IdParamsSchema }), getDoc); +docRouter.put('/:id', validate({ params: IdParamsSchema, body: UpdateDocumentSchema }), updateDoc); +docRouter.delete('/:id', validate({ params: IdParamsSchema }), deleteDoc); + +docRouter.patch( + '/:id/settings', + validate({ params: IdParamsSchema, body: UpdateDocSettingsSchema }), + updateDocSettings +); // Used to toggle allowSelfJoin for the document // !Owner only access + +docRouter.get('/:id/share-link', validate({ params: IdParamsSchema, query: ShareLinkQuerySchema }), getShareLink); // get the document share link with the share token + +docRouter.get('/:id/collaborators', validate({ params: IdParamsSchema }), getCollaborators); // returns list +docRouter.post( + '/:id/collaborators', + validate({ params: IdParamsSchema, body: AddCollaboratorSchema }), + addCollaborator +); // adds a new one by email //!Owner only access +docRouter.delete('/:id/collaborators/:userId', validate({ params: CollaboratorParamsSchema }), removeCollaborator); // optional + +docRouter.get('/:id/requests', validate({ params: IdParamsSchema }), getRequests); // !Owner only access +docRouter.post('/:id/requests/:requestId/approve', validate({ params: RequestIdParamsSchema }), approveRequest); +docRouter.delete('/:id/requests/:requestId/reject', validate({ params: RequestIdParamsSchema }), rejectRequest); diff --git a/server/src/validations/addCollaborator.schema.ts b/server/src/validations/addCollaborator.schema.ts index 28fdb76..4e0c7dc 100644 --- a/server/src/validations/addCollaborator.schema.ts +++ b/server/src/validations/addCollaborator.schema.ts @@ -4,6 +4,7 @@ export const AddCollaboratorSchema = z.object({ email: z .string() .email() + .max(254) .transform(email => email.trim().toLowerCase()), permission: z.enum(['edit', 'view']).default('edit'), }); diff --git a/server/src/validations/createDocument.schema.ts b/server/src/validations/createDocument.schema.ts new file mode 100644 index 0000000..00cfec3 --- /dev/null +++ b/server/src/validations/createDocument.schema.ts @@ -0,0 +1,9 @@ +import { z } from 'zod'; + +export const CreateDocumentSchema = z.object({ + title: z.string().trim().min(1).max(200), + content: z.string().max(1_000_000).optional(), + isPublic: z.boolean().default(false), +}); + +export type CreateDocumentSchema = z.infer; diff --git a/server/src/validations/documentParams.schema.ts b/server/src/validations/documentParams.schema.ts new file mode 100644 index 0000000..5e19448 --- /dev/null +++ b/server/src/validations/documentParams.schema.ts @@ -0,0 +1,27 @@ +import { z } from 'zod'; + +export const IdParamsSchema = z.object({ + id: z.string().uuid(), +}); + +export type IdParamsSchema = z.infer; + +export const RequestIdParamsSchema = z.object({ + id: z.string().uuid(), + requestId: z.string().uuid(), +}); + +export type RequestIdParamsSchema = z.infer; + +export const CollaboratorParamsSchema = z.object({ + id: z.string().uuid(), + userId: z.string().uuid(), +}); + +export type CollaboratorParamsSchema = z.infer; + +export const ShareLinkQuerySchema = z.object({ + permission: z.enum(['view', 'edit']).default('view'), +}); + +export type ShareLinkQuerySchema = z.infer; diff --git a/server/src/validations/login.schema.ts b/server/src/validations/login.schema.ts index a852fe5..f763a96 100644 --- a/server/src/validations/login.schema.ts +++ b/server/src/validations/login.schema.ts @@ -4,8 +4,9 @@ export const LoginUserSchema = z.object({ email: z .string() .email() + .max(254) .transform(email => email.trim().toLowerCase()), - password: z.string().min(6), + password: z.string().min(8).max(128), }); export type LoginUserSchema = z.infer; diff --git a/server/src/validations/register.schema.ts b/server/src/validations/register.schema.ts index 6895311..c97eac8 100644 --- a/server/src/validations/register.schema.ts +++ b/server/src/validations/register.schema.ts @@ -4,10 +4,15 @@ export const RegisterUserSchema = z.object({ email: z .string() .email() + .max(254) .transform(email => email.trim().toLowerCase()), - username: z.string().min(3), - password: z.string().min(6), - fullName: z.string().optional(), + username: z + .string() + .min(3) + .max(50) + .regex(/^[a-zA-Z0-9_.-]+$/, 'Username may only contain letters, digits, dots, dashes and underscores'), + password: z.string().min(8).max(128), + fullName: z.string().max(100).optional(), }); export type RegisterUserSchema = z.infer; diff --git a/server/src/validations/updateDocSettings.schema.ts b/server/src/validations/updateDocSettings.schema.ts new file mode 100644 index 0000000..9d877b0 --- /dev/null +++ b/server/src/validations/updateDocSettings.schema.ts @@ -0,0 +1,7 @@ +import { z } from 'zod'; + +export const UpdateDocSettingsSchema = z.object({ + allowSelfJoin: z.boolean(), +}); + +export type UpdateDocSettingsSchema = z.infer; diff --git a/server/src/validations/updateDocument.schema.ts b/server/src/validations/updateDocument.schema.ts new file mode 100644 index 0000000..72429de --- /dev/null +++ b/server/src/validations/updateDocument.schema.ts @@ -0,0 +1,9 @@ +import { z } from 'zod'; + +export const UpdateDocumentSchema = z.object({ + title: z.string().trim().min(1).max(200).optional(), + content: z.string().max(1_000_000).optional(), + isPublic: z.boolean().optional(), +}); + +export type UpdateDocumentSchema = z.infer; diff --git a/server/test/auth-isactive.test.ts b/server/test/auth-isactive.test.ts new file mode 100644 index 0000000..a3a60b8 --- /dev/null +++ b/server/test/auth-isactive.test.ts @@ -0,0 +1,82 @@ +import { StatusCodes } from 'http-status-codes'; +import request from 'supertest'; +import { describe, expect, it } from 'vitest'; + +import { prisma } from '@/lib/prisma'; +import { app } from '@/server'; + +function extractCookies(raw: string[] | string | undefined): string { + if (!raw) return ''; + const arr = Array.isArray(raw) ? raw : [raw]; + return arr.map(c => c.split(';')[0]).join('; '); +} + +describe('Session hardening — isActive enforcement (#51.2)', () => { + it('should reject login when user is deactivated', async () => { + await request(app).post('/api/auth/register').send({ + email: 'inactive-login@test.dev', + username: 'inactiveLogin', + password: 'secure123', + }); + + // deactivate + await prisma.user.update({ + where: { email: 'inactive-login@test.dev' }, + data: { isActive: false }, + }); + + const res = await request(app).post('/api/auth/login').send({ + email: 'inactive-login@test.dev', + password: 'secure123', + }); + + expect(res.status).toBe(StatusCodes.UNAUTHORIZED); + }); + + it('should reject refresh when user is deactivated', async () => { + await request(app).post('/api/auth/register').send({ + email: 'inactive-refresh@test.dev', + username: 'inactiveRefresh', + password: 'secure123', + }); + + const loginRes = await request(app).post('/api/auth/login').send({ + email: 'inactive-refresh@test.dev', + password: 'secure123', + }); + expect(loginRes.status).toBe(StatusCodes.OK); + const cookie = extractCookies(loginRes.headers['set-cookie']); + + await prisma.user.update({ + where: { email: 'inactive-refresh@test.dev' }, + data: { isActive: false }, + }); + + const refreshRes = await request(app).post('/api/auth/refresh').set('Cookie', cookie); + expect(refreshRes.status).toBe(StatusCodes.UNAUTHORIZED); + }); + + it('should reject protected route when user is deactivated (authenticate middleware)', async () => { + await request(app).post('/api/auth/register').send({ + email: 'inactive-auth@test.dev', + username: 'inactiveAuth', + password: 'secure123', + }); + + const loginRes = await request(app).post('/api/auth/login').send({ + email: 'inactive-auth@test.dev', + password: 'secure123', + }); + const token = loginRes.body.accessToken; + expect(token).toBeDefined(); + + await prisma.user.update({ + where: { email: 'inactive-auth@test.dev' }, + data: { isActive: false }, + }); + + const protectedRes = await request(app).get('/api/user').set('Authorization', `Bearer ${token}`); + + expect(protectedRes.status).toBe(StatusCodes.UNAUTHORIZED); + }); +}); diff --git a/server/test/auth-multidevice.test.ts b/server/test/auth-multidevice.test.ts new file mode 100644 index 0000000..0c07784 --- /dev/null +++ b/server/test/auth-multidevice.test.ts @@ -0,0 +1,77 @@ +import { StatusCodes } from 'http-status-codes'; +import request from 'supertest'; +import { describe, expect, it } from 'vitest'; + +import { app } from '@/server'; + +function extractCookies(raw: string[] | string | undefined): string { + if (!raw) return ''; + const arr = Array.isArray(raw) ? raw : [raw]; + return arr.map(c => c.split(';')[0]).join('; '); +} + +describe('Session hardening — multi-device (#51.1)', () => { + it('should keep first device valid after second login (no single-column overwrite)', async () => { + await request(app).post('/api/auth/register').send({ + email: 'multi@test.dev', + username: 'multiUser', + password: 'secure123', + }); + + const login1 = await request(app).post('/api/auth/login').send({ + email: 'multi@test.dev', + password: 'secure123', + }); + expect(login1.status).toBe(StatusCodes.OK); + const cookie1 = extractCookies(login1.headers['set-cookie']); + + // Second login simulates another device + const login2 = await request(app).post('/api/auth/login').send({ + email: 'multi@test.dev', + password: 'secure123', + }); + expect(login2.status).toBe(StatusCodes.OK); + const cookie2 = extractCookies(login2.headers['set-cookie']); + + expect(cookie1).not.toBe(cookie2); + + // Both cookies must still refresh independently + const refresh1 = await request(app).post('/api/auth/refresh').set('Cookie', cookie1); + expect(refresh1.status).toBe(StatusCodes.OK); + + const refresh2 = await request(app).post('/api/auth/refresh').set('Cookie', cookie2); + expect(refresh2.status).toBe(StatusCodes.OK); + }); + + it('should only revoke the presented session on logout, leaving other device', async () => { + await request(app).post('/api/auth/register').send({ + email: 'multilogout@test.dev', + username: 'multiLogout', + password: 'secure123', + }); + + const login1 = await request(app).post('/api/auth/login').send({ + email: 'multilogout@test.dev', + password: 'secure123', + }); + const cookie1 = extractCookies(login1.headers['set-cookie']); + + const login2 = await request(app).post('/api/auth/login').send({ + email: 'multilogout@test.dev', + password: 'secure123', + }); + const cookie2 = extractCookies(login2.headers['set-cookie']); + + // Logout with first device + const logout1 = await request(app).post('/api/auth/logout').set('Cookie', cookie1); + expect(logout1.status).toBe(StatusCodes.OK); + + // First device must no longer refresh + const refresh1After = await request(app).post('/api/auth/refresh').set('Cookie', cookie1); + expect(refresh1After.status).toBe(StatusCodes.UNAUTHORIZED); + + // Second device must still refresh + const refresh2After = await request(app).post('/api/auth/refresh').set('Cookie', cookie2); + expect(refresh2After.status).toBe(StatusCodes.OK); + }); +}); diff --git a/server/test/auth-session-rotation.test.ts b/server/test/auth-session-rotation.test.ts new file mode 100644 index 0000000..d4f12f7 --- /dev/null +++ b/server/test/auth-session-rotation.test.ts @@ -0,0 +1,55 @@ +import { StatusCodes } from 'http-status-codes'; +import request from 'supertest'; +import { describe, expect, it } from 'vitest'; + +import { app } from '@/server'; + +function extractCookies(raw: string[] | string | undefined): string { + if (!raw) return ''; + const arr = Array.isArray(raw) ? raw : [raw]; + return arr.map(c => c.split(';')[0]).join('; '); +} + +function getRefreshCookie(setCookie: string[] | string | undefined): string | undefined { + if (!setCookie) return undefined; + const arr = Array.isArray(setCookie) ? setCookie : [setCookie]; + return arr.find(c => c.startsWith('refreshToken=')); +} + +describe('Session hardening — refresh rotation (#51.1)', () => { + it('should rotate refresh token on refresh and invalidate the old one', async () => { + await request(app).post('/api/auth/register').send({ + email: 'rotate@test.dev', + username: 'rotater', + password: 'secure123', + }); + + const loginRes = await request(app).post('/api/auth/login').send({ + email: 'rotate@test.dev', + password: 'secure123', + }); + expect(loginRes.status).toBe(StatusCodes.OK); + const firstCookie = getRefreshCookie(loginRes.headers['set-cookie']); + expect(firstCookie).toBeDefined(); + + const firstCookieHeader = extractCookies(loginRes.headers['set-cookie']); + + // First refresh — should issue a new refreshToken cookie + const refreshRes = await request(app).post('/api/auth/refresh').set('Cookie', firstCookieHeader); + + expect(refreshRes.status).toBe(StatusCodes.OK); + const secondCookie = getRefreshCookie(refreshRes.headers['set-cookie']); + // This is the RED assertion: new implementation must set a new refresh cookie + expect(secondCookie).toBeDefined(); + expect(secondCookie).not.toBe(firstCookie); + + // Old token must no longer work + const replayOld = await request(app).post('/api/auth/refresh').set('Cookie', firstCookieHeader); + expect(replayOld.status).toBe(StatusCodes.UNAUTHORIZED); + + // New token must work + const secondCookieHeader = extractCookies(refreshRes.headers['set-cookie']); + const refreshWithNew = await request(app).post('/api/auth/refresh').set('Cookie', secondCookieHeader); + expect(refreshWithNew.status).toBe(StatusCodes.OK); + }); +}); diff --git a/server/test/auth.test.ts b/server/test/auth.test.ts index 2b1847d..feaa335 100644 --- a/server/test/auth.test.ts +++ b/server/test/auth.test.ts @@ -80,6 +80,35 @@ describe('Auth Routes', () => { }); }); + it('should not leak which field collided when registration fails (#83)', async () => { + await request(app).post('/api/auth/register').send({ + email: 'enum-email@test.dev', + username: 'enumuser', + password: 'secure123', + }); + + const dupEmail = await request(app).post('/api/auth/register').send({ + email: 'enum-email@test.dev', + username: 'unusedname', + password: 'secure123', + }); + + const dupUsername = await request(app).post('/api/auth/register').send({ + email: 'unused@test.dev', + username: 'enumuser', + password: 'secure123', + }); + + expect(dupEmail.status).toBe(StatusCodes.CONFLICT); + expect(dupUsername.status).toBe(StatusCodes.CONFLICT); + // Uniform response regardless of which field collided + expect(dupEmail.body).toEqual(dupUsername.body); + expect(typeof dupEmail.body.error).toBe('string'); + expect(dupEmail.body.error).not.toMatch(/email/i); + expect(dupEmail.body.error).not.toMatch(/username/i); + expect(dupEmail.body.error).not.toMatch(/exists/i); + }); + it('should reject login with invalid password', async () => { await request(app).post('/api/auth/register').send({ email: 'test@test.dev', @@ -104,6 +133,32 @@ describe('Auth Routes', () => { expect(res.status).toBe(StatusCodes.UNAUTHORIZED); }); + it('should not distinguish unknown user from invalid password on login (#83)', async () => { + await request(app).post('/api/auth/register').send({ + email: 'loginenum@test.dev', + username: 'loginenum', + password: 'secure123', + }); + + const badPassword = await request(app).post('/api/auth/login').send({ + email: 'loginenum@test.dev', + password: 'wrongpass', + }); + + const unknownUser = await request(app).post('/api/auth/login').send({ + email: 'ghost@test.dev', + password: 'anypassword', + }); + + expect(badPassword.status).toBe(StatusCodes.UNAUTHORIZED); + expect(unknownUser.status).toBe(StatusCodes.UNAUTHORIZED); + // Identical responses so probing cannot tell whether the account exists + expect(unknownUser.body).toEqual(badPassword.body); + // And the response carries an actual message (not the legacy empty `{}`) + expect(typeof unknownUser.body.error).toBe('string'); + expect(unknownUser.body.error.length).toBeGreaterThan(0); + }); + it('should reject registration with invalid email format', async () => { const res = await request(app).post('/api/auth/register').send({ email: 'invalid-email', @@ -124,6 +179,26 @@ describe('Auth Routes', () => { expect(res.status).toBe(StatusCodes.BAD_REQUEST); }); + it('should reject registration with a 6-character password', async () => { + const res = await request(app).post('/api/auth/register').send({ + email: 'shortpass@test.dev', + username: 'shortpass', + password: 'abcdef', + }); + + expect(res.status).toBe(StatusCodes.BAD_REQUEST); + }); + + it('should reject registration with a username containing spaces', async () => { + const res = await request(app).post('/api/auth/register').send({ + email: 'spaceuser@test.dev', + username: 'bad name', + password: 'secure123', + }); + + expect(res.status).toBe(StatusCodes.BAD_REQUEST); + }); + it('should reject registration with missing fields', async () => { const res = await request(app).post('/api/auth/register').send({ email: 'newuser@test.dev', @@ -155,6 +230,109 @@ describe('Auth Routes', () => { expect(logoutRes.status).toBe(StatusCodes.OK); }); + it('should logout with only the refresh cookie when no access token is sent', async () => { + await request(app).post('/api/auth/register').send({ + email: 'cookie-logout@test.dev', + username: 'cookieLogoutUser', + password: 'secure123', + }); + + const loginRes = await request(app).post('/api/auth/login').send({ + email: 'cookie-logout@test.dev', + password: 'secure123', + }); + const cookieHeader = extractCookies(loginRes.headers['set-cookie']); + + const logoutRes = await request(app).post('/api/auth/logout').set('Cookie', cookieHeader); + + expect(logoutRes.status).toBe(StatusCodes.OK); + + const refreshRes = await request(app).post('/api/auth/refresh').set('Cookie', cookieHeader); + expect(refreshRes.status).toBe(StatusCodes.UNAUTHORIZED); + }); + + it('should clear refreshToken cookie with matching attributes on logout', async () => { + await request(app).post('/api/auth/register').send({ + email: 'clear-attrs@test.dev', + username: 'clearAttrsUser', + password: 'secure123', + }); + + const loginRes = await request(app).post('/api/auth/login').send({ + email: 'clear-attrs@test.dev', + password: 'secure123', + }); + const cookieHeader = extractCookies(loginRes.headers['set-cookie']); + + const logoutRes = await request(app).post('/api/auth/logout').set('Cookie', cookieHeader); + + expect(logoutRes.status).toBe(StatusCodes.OK); + const setCookies = (logoutRes.headers['set-cookie'] ?? []) as string[]; + const cookiesArray = Array.isArray(setCookies) ? setCookies : [setCookies]; + // Express clearCookie sets `name=; Path=/; Expires=Thu, 01 Jan 1970 ...` + const refreshClear = cookiesArray.find(c => c.startsWith('refreshToken=;')); + expect(refreshClear).toBeDefined(); + // Must mirror login attributes or browsers (prod SameSite=None; Secure) won't clear + expect(refreshClear).toContain('Path=/'); + expect(refreshClear).toContain('HttpOnly'); + expect(refreshClear).toContain('SameSite=Lax'); + expect(refreshClear).not.toContain('Secure'); + expect(refreshClear).toMatch(/Expires=Thu, 01 Jan 1970|Max-Age=0/); + }); + + it('should clear both refreshToken and legacy accessToken cookies on logout', async () => { + await request(app).post('/api/auth/register').send({ + email: 'clear-both@test.dev', + username: 'clearBothUser', + password: 'secure123', + }); + + const loginRes = await request(app).post('/api/auth/login').send({ + email: 'clear-both@test.dev', + password: 'secure123', + }); + const cookieHeader = extractCookies(loginRes.headers['set-cookie']); + + const logoutRes = await request(app).post('/api/auth/logout').set('Cookie', cookieHeader); + + expect(logoutRes.status).toBe(StatusCodes.OK); + const setCookies = (logoutRes.headers['set-cookie'] ?? []) as string[]; + const cookiesArray = Array.isArray(setCookies) ? setCookies : [setCookies]; + const names = cookiesArray.map(c => c.split('=')[0]); + expect(names).toContain('refreshToken'); + expect(names).toContain('accessToken'); + // Both clearing cookies must carry the same path/sameSite so they actually overwrite + for (const c of cookiesArray) { + if (c.startsWith('refreshToken=;') || c.startsWith('accessToken=;')) { + expect(c).toContain('Path=/'); + expect(c).toContain('SameSite=Lax'); + } + } + }); + + it('should not allow refresh with the old cookie after logout', async () => { + await request(app).post('/api/auth/register').send({ + email: 'refresh-after-logout@test.dev', + username: 'refreshAfterLogoutUser', + password: 'secure123', + }); + + const loginRes = await request(app).post('/api/auth/login').send({ + email: 'refresh-after-logout@test.dev', + password: 'secure123', + }); + const cookieHeader = extractCookies(loginRes.headers['set-cookie']); + + const logoutRes = await request(app).post('/api/auth/logout').set('Cookie', cookieHeader); + expect(logoutRes.status).toBe(StatusCodes.OK); + + // Even though the client still holds the old cookie string, the server has + // nulled the stored refreshToken and the browser should have received a + // clearing Set-Cookie (verified above). Replaying the old cookie must fail. + const refreshRes = await request(app).post('/api/auth/refresh').set('Cookie', cookieHeader); + expect(refreshRes.status).toBe(StatusCodes.UNAUTHORIZED); + }); + it('should return 401 when accessing protected route without token', async () => { const res = await request(app).get('/api/user'); expect(res.status).toBe(StatusCodes.UNAUTHORIZED); @@ -180,9 +358,52 @@ describe('Auth Routes', () => { expect(res.body.email).toBe('me@test.dev'); }); + it('should set the refresh cookie without Secure/SameSite=None outside production', async () => { + await request(app).post('/api/auth/register').send({ + email: 'cookie-flags@test.dev', + username: 'cookieFlagsUser', + password: 'secure123', + }); + + const res = await request(app).post('/api/auth/login').send({ + email: 'cookie-flags@test.dev', + password: 'secure123', + }); + + expect(res.status).toBe(StatusCodes.OK); + const setCookies = Array.isArray(res.headers['set-cookie']) + ? res.headers['set-cookie'] + : [res.headers['set-cookie'] ?? '']; + const refreshCookie = setCookies.find(c => c.startsWith('refreshToken=')); + expect(refreshCookie).toBeDefined(); + expect(refreshCookie).not.toContain('Secure'); + expect(refreshCookie).toContain('SameSite=Lax'); + }); + it('should reject refresh with invalid refresh token', async () => { const res = await request(app).post('/api/auth/refresh').set('Cookie', 'refreshToken=invalid.token.here'); expect(res.status).toBe(StatusCodes.UNAUTHORIZED); }); + + it('should not set an accessToken cookie on refresh', async () => { + await request(app).post('/api/auth/register').send({ + email: 'refresh@test.dev', + username: 'refreshUser', + password: 'secure123', + }); + + const loginRes = await request(app).post('/api/auth/login').send({ + email: 'refresh@test.dev', + password: 'secure123', + }); + const cookieHeader = extractCookies(loginRes.headers['set-cookie']); + + const res = await request(app).post('/api/auth/refresh').set('Cookie', cookieHeader); + + expect(res.status).toBe(StatusCodes.OK); + const cookies = res.headers['set-cookie'] ?? []; + const names = (Array.isArray(cookies) ? cookies : [cookies]).map(c => c.split('=')[0]); + expect(names).not.toContain('accessToken'); + }); }); diff --git a/server/test/document.test.ts b/server/test/document.test.ts index 41e3927..af27ad0 100644 --- a/server/test/document.test.ts +++ b/server/test/document.test.ts @@ -550,6 +550,99 @@ describe('Document Routes', () => { }); }); +describe('Document request validation (#52)', () => { + it('should reject creating a document with a missing title', async () => { + const res = await request(app).post('/api/document').set('Authorization', `Bearer ${token}`).send({}); + + expect(res.status).toBe(StatusCodes.BAD_REQUEST); + }); + + it('should reject creating a document with a non-string title', async () => { + const res = await request(app).post('/api/document').set('Authorization', `Bearer ${token}`).send({ title: 123 }); + + expect(res.status).toBe(StatusCodes.BAD_REQUEST); + }); + + it('should reject creating a document with an oversized title', async () => { + const res = await request(app) + .post('/api/document') + .set('Authorization', `Bearer ${token}`) + .send({ title: 'x'.repeat(201) }); + + expect(res.status).toBe(StatusCodes.BAD_REQUEST); + }); + + it('should reject creating a document with oversized content', async () => { + const res = await request(app) + .post('/api/document') + .set('Authorization', `Bearer ${token}`) + .send({ title: 'Ok', content: 'x'.repeat(1_000_001) }); + + // A >1MB body trips express.json's 100kb payload limit first (413); + // CreateDocumentSchema's content cap remains as defense-in-depth. + expect([StatusCodes.BAD_REQUEST, StatusCodes.REQUEST_TOO_LONG]).toContain(res.status); + }); + + it('should reject updating a document with a non-boolean isPublic', async () => { + const created = await prisma.document.create({ + data: { title: 'Bool Check', authorId: userId, content: '' }, + }); + + const res = await request(app) + .put(`/api/document/${created.id}`) + .set('Authorization', `Bearer ${token}`) + .send({ title: 'Still Ok', isPublic: 'yes' }); + + expect(res.status).toBe(StatusCodes.BAD_REQUEST); + }); + + it('should reject a settings update with a non-boolean allowSelfJoin', async () => { + const created = await prisma.document.create({ + data: { title: 'Settings Validation', authorId: userId, content: '' }, + }); + + const res = await request(app) + .patch(`/api/document/${created.id}/settings`) + .set('Authorization', `Bearer ${token}`) + .send({ allowSelfJoin: 'yes' }); + + expect(res.status).toBe(StatusCodes.BAD_REQUEST); + }); + + it('should return 400 instead of 500 when getting a document with a non-uuid id', async () => { + const res = await request(app).get('/api/document/not-a-uuid').set('Authorization', `Bearer ${token}`); + + expect(res.status).toBe(StatusCodes.BAD_REQUEST); + }); + + it('should return 400 instead of 500 when updating a document with a non-uuid id', async () => { + const res = await request(app) + .put('/api/document/not-a-uuid') + .set('Authorization', `Bearer ${token}`) + .send({ title: 'Nope' }); + + expect(res.status).toBe(StatusCodes.BAD_REQUEST); + }); + + it('should return 400 instead of 500 when deleting a document with a non-uuid id', async () => { + const res = await request(app).delete('/api/document/not-a-uuid').set('Authorization', `Bearer ${token}`); + + expect(res.status).toBe(StatusCodes.BAD_REQUEST); + }); + + it('should reject share-link generation with an invalid permission', async () => { + const created = await prisma.document.create({ + data: { title: 'Share Perm', authorId: userId, content: '' }, + }); + + const res = await request(app) + .get(`/api/document/${created.id}/share-link?permission=admin`) + .set('Authorization', `Bearer ${token}`); + + expect(res.status).toBe(StatusCodes.BAD_REQUEST); + }); +}); + describe('Collaboration request decision scoping (#46)', () => { async function createOwnedDocument(title: string) { return prisma.document.create({ diff --git a/server/test/setup.ts b/server/test/setup.ts index b4d3849..94b85c3 100644 --- a/server/test/setup.ts +++ b/server/test/setup.ts @@ -20,7 +20,14 @@ beforeAll(async () => { }); afterEach(async () => { - const tableNames = ['collaboration_requests', 'collaborators', 'yjs_document_states', 'documents', 'users']; + const tableNames = [ + 'collaboration_requests', + 'collaborators', + 'yjs_document_states', + 'documents', + 'sessions', + 'users', + ]; try { await prisma.$transaction(async (tx: Prisma.TransactionClient) => {