Add comprehensive backend test suite (#339)

* add test suite, mostly covers integration testing, tests are only backend side

* workflow runs the correct script

* workflow runs the correct script

* workflow runs the correct script

* unit tests incoming

* Fix multer silent rejections and error handler info leak

- Revert cb(null, false) to cb(new Error(...)) in auth.ts, collab.ts,
  and files.ts so invalid uploads return an error instead of silently
  dropping the file
- Error handler in app.ts now always returns 500 / "Internal server
  error" instead of forwarding err.message to the client

* Use statusCode consistently for multer errors and error handler

- Error handler in app.ts reads err.statusCode to forward the correct
  HTTP status while keeping the response body generic

server/tests/unit/middleware/auth.test.ts

@@ -0,0 +1,115 @@
+import { describe, it, expect, vi } from 'vitest';
+
+vi.mock('../../../src/db/database', () => ({
+  db: { prepare: () => ({ get: vi.fn(), all: vi.fn() }) },
+}));
+vi.mock('../../../src/config', () => ({ JWT_SECRET: 'test-secret' }));
+
+import { extractToken, authenticate, adminOnly } from '../../../src/middleware/auth';
+import type { Request, Response, NextFunction } from 'express';
+
+function makeReq(overrides: {
+  cookies?: Record<string, string>;
+  headers?: Record<string, string>;
+} = {}): Request {
+  return {
+    cookies: overrides.cookies || {},
+    headers: overrides.headers || {},
+  } as unknown as Request;
+}
+
+function makeRes(): { res: Response; status: ReturnType<typeof vi.fn>; json: ReturnType<typeof vi.fn> } {
+  const json = vi.fn();
+  const status = vi.fn(() => ({ json }));
+  const res = { status } as unknown as Response;
+  return { res, status, json };
+}
+
+// ── extractToken ─────────────────────────────────────────────────────────────
+
+describe('extractToken', () => {
+  it('returns cookie value when trek_session cookie is set', () => {
+    const req = makeReq({ cookies: { trek_session: 'cookie-token' } });
+    expect(extractToken(req)).toBe('cookie-token');
+  });
+
+  it('returns Bearer token from Authorization header when no cookie', () => {
+    const req = makeReq({ headers: { authorization: 'Bearer header-token' } });
+    expect(extractToken(req)).toBe('header-token');
+  });
+
+  it('prefers cookie over Authorization header when both are present', () => {
+    const req = makeReq({
+      cookies: { trek_session: 'cookie-token' },
+      headers: { authorization: 'Bearer header-token' },
+    });
+    expect(extractToken(req)).toBe('cookie-token');
+  });
+
+  it('returns null when neither cookie nor header are present', () => {
+    expect(extractToken(makeReq())).toBeNull();
+  });
+
+  it('returns null for Authorization header without a token (empty Bearer)', () => {
+    const req = makeReq({ headers: { authorization: 'Bearer ' } });
+    expect(extractToken(req)).toBeNull();
+  });
+
+  it('returns null for Authorization header without Bearer prefix', () => {
+    const req = makeReq({ headers: { authorization: 'Basic sometoken' } });
+    // split(' ')[1] returns 'sometoken' — this IS returned (not a null case)
+    // The function simply splits on space and takes index 1
+    expect(extractToken(req)).toBe('sometoken');
+  });
+});
+
+// ── authenticate ─────────────────────────────────────────────────────────────
+
+describe('authenticate', () => {
+  it('returns 401 when no token is present', () => {
+    const next = vi.fn() as unknown as NextFunction;
+    const { res, status, json } = makeRes();
+    authenticate(makeReq(), res, next);
+    expect(next).not.toHaveBeenCalled();
+    expect(status).toHaveBeenCalledWith(401);
+    expect(json).toHaveBeenCalledWith(expect.objectContaining({ code: 'AUTH_REQUIRED' }));
+  });
+
+  it('returns 401 when JWT is invalid', () => {
+    const next = vi.fn() as unknown as NextFunction;
+    const { res, status } = makeRes();
+    authenticate(makeReq({ cookies: { trek_session: 'invalid.jwt.token' } }), res, next);
+    expect(next).not.toHaveBeenCalled();
+    expect(status).toHaveBeenCalledWith(401);
+  });
+});
+
+// ── adminOnly ─────────────────────────────────────────────────────────────────
+
+describe('adminOnly', () => {
+  it('returns 403 when user role is not admin', () => {
+    const next = vi.fn() as unknown as NextFunction;
+    const { res, status, json } = makeRes();
+    const req = { ...makeReq(), user: { id: 1, role: 'user' } } as unknown as Request;
+    adminOnly(req, res, next);
+    expect(next).not.toHaveBeenCalled();
+    expect(status).toHaveBeenCalledWith(403);
+    expect(json).toHaveBeenCalledWith(expect.objectContaining({ error: expect.stringContaining('Admin') }));
+  });
+
+  it('calls next() when user role is admin', () => {
+    const next = vi.fn() as unknown as NextFunction;
+    const { res } = makeRes();
+    const req = { ...makeReq(), user: { id: 1, role: 'admin' } } as unknown as Request;
+    adminOnly(req, res, next);
+    expect(next).toHaveBeenCalled();
+  });
+
+  it('returns 403 when req.user is undefined', () => {
+    const next = vi.fn() as unknown as NextFunction;
+    const { res, status } = makeRes();
+    adminOnly(makeReq() as unknown as Request, res, next);
+    expect(next).not.toHaveBeenCalled();
+    expect(status).toHaveBeenCalledWith(403);
+  });
+});

server/tests/unit/middleware/mfaPolicy.test.ts

@@ -0,0 +1,100 @@
+import { describe, it, expect, vi } from 'vitest';
+
+vi.mock('../../../src/db/database', () => ({
+  db: { prepare: () => ({ get: vi.fn(), all: vi.fn() }) },
+}));
+vi.mock('../../../src/config', () => ({ JWT_SECRET: 'test-secret' }));
+
+import { isPublicApiPath, isMfaSetupExemptPath } from '../../../src/middleware/mfaPolicy';
+
+// ── isPublicApiPath ──────────────────────────────────────────────────────────
+
+describe('isPublicApiPath', () => {
+  // AUTH-001 — Public paths must bypass MFA
+  it('AUTH-001: GET /api/health is public', () => {
+    expect(isPublicApiPath('GET', '/api/health')).toBe(true);
+  });
+
+  it('GET /api/auth/app-config is public', () => {
+    expect(isPublicApiPath('GET', '/api/auth/app-config')).toBe(true);
+  });
+
+  it('POST /api/auth/login is public', () => {
+    expect(isPublicApiPath('POST', '/api/auth/login')).toBe(true);
+  });
+
+  it('POST /api/auth/register is public', () => {
+    expect(isPublicApiPath('POST', '/api/auth/register')).toBe(true);
+  });
+
+  it('POST /api/auth/demo-login is public', () => {
+    expect(isPublicApiPath('POST', '/api/auth/demo-login')).toBe(true);
+  });
+
+  it('GET /api/auth/invite/<token> is public', () => {
+    expect(isPublicApiPath('GET', '/api/auth/invite/abc123')).toBe(true);
+    expect(isPublicApiPath('GET', '/api/auth/invite/xyz-789')).toBe(true);
+  });
+
+  it('POST /api/auth/mfa/verify-login is public', () => {
+    expect(isPublicApiPath('POST', '/api/auth/mfa/verify-login')).toBe(true);
+  });
+
+  it('OIDC paths are public (any method)', () => {
+    expect(isPublicApiPath('GET', '/api/auth/oidc/callback')).toBe(true);
+    expect(isPublicApiPath('POST', '/api/auth/oidc/login')).toBe(true);
+    expect(isPublicApiPath('GET', '/api/auth/oidc/discovery')).toBe(true);
+  });
+
+  it('GET /api/trips is not public', () => {
+    expect(isPublicApiPath('GET', '/api/trips')).toBe(false);
+  });
+
+  it('POST /api/auth/login with wrong method (GET) is not public', () => {
+    expect(isPublicApiPath('GET', '/api/auth/login')).toBe(false);
+  });
+
+  it('GET /api/auth/me is not public', () => {
+    expect(isPublicApiPath('GET', '/api/auth/me')).toBe(false);
+  });
+
+  it('DELETE /api/auth/logout is not public', () => {
+    expect(isPublicApiPath('DELETE', '/api/auth/logout')).toBe(false);
+  });
+});
+
+// ── isMfaSetupExemptPath ─────────────────────────────────────────────────────
+
+describe('isMfaSetupExemptPath', () => {
+  it('GET /api/auth/me is MFA-setup exempt', () => {
+    expect(isMfaSetupExemptPath('GET', '/api/auth/me')).toBe(true);
+  });
+
+  it('POST /api/auth/mfa/setup is MFA-setup exempt', () => {
+    expect(isMfaSetupExemptPath('POST', '/api/auth/mfa/setup')).toBe(true);
+  });
+
+  it('POST /api/auth/mfa/enable is MFA-setup exempt', () => {
+    expect(isMfaSetupExemptPath('POST', '/api/auth/mfa/enable')).toBe(true);
+  });
+
+  it('GET /api/auth/app-settings is MFA-setup exempt', () => {
+    expect(isMfaSetupExemptPath('GET', '/api/auth/app-settings')).toBe(true);
+  });
+
+  it('PUT /api/auth/app-settings is MFA-setup exempt', () => {
+    expect(isMfaSetupExemptPath('PUT', '/api/auth/app-settings')).toBe(true);
+  });
+
+  it('POST /api/auth/app-settings is NOT exempt (wrong method)', () => {
+    expect(isMfaSetupExemptPath('POST', '/api/auth/app-settings')).toBe(false);
+  });
+
+  it('GET /api/trips is NOT exempt', () => {
+    expect(isMfaSetupExemptPath('GET', '/api/trips')).toBe(false);
+  });
+
+  it('GET /api/auth/logout is NOT exempt', () => {
+    expect(isMfaSetupExemptPath('GET', '/api/auth/logout')).toBe(false);
+  });
+});

server/tests/unit/middleware/validate.test.ts

@@ -0,0 +1,109 @@
+import { describe, it, expect, vi } from 'vitest';
+import { maxLength, validateStringLengths } from '../../../src/middleware/validate';
+import type { Request, Response, NextFunction } from 'express';
+
+function makeReq(body: Record<string, unknown> = {}): Request {
+  return { body } as Request;
+}
+
+function makeRes(): { res: Response; status: ReturnType<typeof vi.fn>; json: ReturnType<typeof vi.fn> } {
+  const json = vi.fn();
+  const status = vi.fn(() => ({ json }));
+  const res = { status } as unknown as Response;
+  return { res, status, json };
+}
+
+// ── maxLength ────────────────────────────────────────────────────────────────
+
+describe('maxLength', () => {
+  it('calls next() when field is absent from body', () => {
+    const next = vi.fn() as unknown as NextFunction;
+    const { res } = makeRes();
+    maxLength('name', 10)(makeReq({}), res, next);
+    expect(next).toHaveBeenCalled();
+  });
+
+  it('calls next() when field is not a string (number)', () => {
+    const next = vi.fn() as unknown as NextFunction;
+    const { res } = makeRes();
+    maxLength('count', 5)(makeReq({ count: 999 }), res, next);
+    expect(next).toHaveBeenCalled();
+  });
+
+  it('calls next() when string length is within limit', () => {
+    const next = vi.fn() as unknown as NextFunction;
+    const { res } = makeRes();
+    maxLength('name', 10)(makeReq({ name: 'hello' }), res, next);
+    expect(next).toHaveBeenCalled();
+  });
+
+  it('calls next() when string length equals max exactly', () => {
+    const next = vi.fn() as unknown as NextFunction;
+    const { res } = makeRes();
+    maxLength('name', 5)(makeReq({ name: 'hello' }), res, next);
+    expect(next).toHaveBeenCalled();
+  });
+
+  it('returns 400 when field exceeds max', () => {
+    const next = vi.fn() as unknown as NextFunction;
+    const { res, status, json } = makeRes();
+    maxLength('name', 4)(makeReq({ name: 'hello' }), res, next);
+    expect(next).not.toHaveBeenCalled();
+    expect(status).toHaveBeenCalledWith(400);
+    expect(json).toHaveBeenCalledWith(expect.objectContaining({ error: expect.stringContaining('name') }));
+  });
+
+  it('error message includes field name and max length', () => {
+    const next = vi.fn() as unknown as NextFunction;
+    const { res, json } = makeRes();
+    maxLength('title', 3)(makeReq({ title: 'toolong' }), res, next);
+    expect(json).toHaveBeenCalledWith(expect.objectContaining({ error: expect.stringMatching(/title.*3|3.*title/i) }));
+  });
+});
+
+// ── validateStringLengths ────────────────────────────────────────────────────
+
+describe('validateStringLengths', () => {
+  it('calls next() when all fields are within limits', () => {
+    const next = vi.fn() as unknown as NextFunction;
+    const { res } = makeRes();
+    validateStringLengths({ name: 10, bio: 100 })(makeReq({ name: 'Alice', bio: 'A short bio' }), res, next);
+    expect(next).toHaveBeenCalled();
+  });
+
+  it('returns 400 on first field that exceeds its limit', () => {
+    const next = vi.fn() as unknown as NextFunction;
+    const { res, status } = makeRes();
+    validateStringLengths({ name: 3 })(makeReq({ name: 'toolong' }), res, next);
+    expect(next).not.toHaveBeenCalled();
+    expect(status).toHaveBeenCalledWith(400);
+  });
+
+  it('skips fields not present in body', () => {
+    const next = vi.fn() as unknown as NextFunction;
+    const { res } = makeRes();
+    validateStringLengths({ name: 10, missing: 5 })(makeReq({ name: 'Alice' }), res, next);
+    expect(next).toHaveBeenCalled();
+  });
+
+  it('skips non-string fields', () => {
+    const next = vi.fn() as unknown as NextFunction;
+    const { res } = makeRes();
+    validateStringLengths({ count: 5 })(makeReq({ count: 999999 }), res, next);
+    expect(next).toHaveBeenCalled();
+  });
+
+  it('handles empty maxLengths object — calls next()', () => {
+    const next = vi.fn() as unknown as NextFunction;
+    const { res } = makeRes();
+    validateStringLengths({})(makeReq({ anything: 'value' }), res, next);
+    expect(next).toHaveBeenCalled();
+  });
+
+  it('calls next() only once even if multiple fields are valid', () => {
+    const next = vi.fn() as unknown as NextFunction;
+    const { res } = makeRes();
+    validateStringLengths({ a: 10, b: 10 })(makeReq({ a: 'ok', b: 'ok' }), res, next);
+    expect(next).toHaveBeenCalledOnce();
+  });
+});