merge: pull PR48 auth and UX into pre-release

2026-02-05 23:25:56 -08:00
parent fd3b97225f 4f53b899c9
commit 700e153740
32 changed files with 4401 additions and 536 deletions
@@ -315,10 +315,11 @@ describe("Security Sanitization - Image Data URLs", () => {
 // Database integration tests
 describe("Drawing API - Database Round-Trip", () => {
  const prisma = getTestPrisma();
+  let testUser: { id: string };

  beforeAll(async () => {
    setupTestDb();
-    await initTestDb(prisma);
+    testUser = await initTestDb(prisma);
  });

  afterAll(async () => {
@@ -343,6 +344,7 @@ describe("Drawing API - Database Round-Trip", () => {
        elements: JSON.stringify([]),
        appState: JSON.stringify({ viewBackgroundColor: "#ffffff" }),
        files: JSON.stringify(files),
+        userId: testUser.id,
      },
    });
    
@@ -381,6 +383,7 @@ describe("Drawing API - Database Round-Trip", () => {
        elements: JSON.stringify([]),
        appState: JSON.stringify({}),
        files: JSON.stringify(files),
+        userId: testUser.id,
      },
    });
    
@@ -404,6 +407,7 @@ describe("Drawing API - Database Round-Trip", () => {
        elements: JSON.stringify([]),
        appState: JSON.stringify({}),
        files: JSON.stringify({}),
+        userId: testUser.id,
      },
    });
    
@@ -54,19 +54,42 @@ export const cleanupTestDb = async (prisma: PrismaClient) => {
  });
 };

+/**
+ * Create a test user for testing
+ */
+export const createTestUser = async (prisma: PrismaClient, email: string = "test@example.com") => {
+  const bcrypt = require("bcrypt");
+  const passwordHash = await bcrypt.hash("testpassword", 10);
+  
+  return await prisma.user.upsert({
+    where: { email },
+    update: {},
+    create: {
+      email,
+      passwordHash,
+      name: "Test User",
+    },
+  });
+};
+
 /**
 * Initialize test database with required data
 */
 export const initTestDb = async (prisma: PrismaClient) => {
+  // Create a test user first
+  const testUser = await createTestUser(prisma);
+  
  // Ensure Trash collection exists
  const trash = await prisma.collection.findUnique({
    where: { id: "trash" },
  });
  if (!trash) {
    await prisma.collection.create({
-      data: { id: "trash", name: "Trash" },
+      data: { id: "trash", name: "Trash", userId: testUser.id },
    });
  }
+  
+  return testUser;
 };

 /**
@@ -0,0 +1,87 @@
+/**
+ * Configuration validation and environment variable management
+ */
+import dotenv from "dotenv";
+
+dotenv.config();
+
+interface Config {
+  port: number;
+  nodeEnv: string;
+  databaseUrl: string;
+  frontendUrl: string;
+  jwtSecret: string;
+  jwtAccessExpiresIn: string;
+  jwtRefreshExpiresIn: string;
+  rateLimitMaxRequests: number;
+  csrfMaxRequests: number;
+  csrfSecret: string | null;
+  // Feature flags - all default to false for backward compatibility
+  enablePasswordReset: boolean;
+  enableRefreshTokenRotation: boolean;
+  enableAuditLogging: boolean;
+}
+
+const getRequiredEnv = (key: string): string => {
+  const value = process.env[key];
+  if (!value || value.trim().length === 0) {
+    throw new Error(`Missing required environment variable: ${key}`);
+  }
+  return value;
+};
+
+const getOptionalEnv = (key: string, defaultValue: string): string => {
+  return process.env[key] || defaultValue;
+};
+
+const getOptionalBoolean = (key: string, defaultValue: boolean): boolean => {
+  const value = process.env[key];
+  if (!value) return defaultValue;
+  return value.toLowerCase() === "true" || value === "1";
+};
+
+const getRequiredEnvNumber = (key: string, defaultValue: number): number => {
+  const value = process.env[key];
+  if (!value) return defaultValue;
+  const parsed = Number(value);
+  if (!Number.isFinite(parsed) || parsed <= 0) {
+    throw new Error(`Invalid value for environment variable ${key}: must be a positive number`);
+  }
+  return parsed;
+};
+
+export const config: Config = {
+  port: getRequiredEnvNumber("PORT", 8000),
+  nodeEnv: getOptionalEnv("NODE_ENV", "development"),
+  databaseUrl: getRequiredEnv("DATABASE_URL"),
+  frontendUrl: getOptionalEnv("FRONTEND_URL", "http://localhost:6767"),
+  jwtSecret: getRequiredEnv("JWT_SECRET"),
+  jwtAccessExpiresIn: getOptionalEnv("JWT_ACCESS_EXPIRES_IN", "15m"),
+  jwtRefreshExpiresIn: getOptionalEnv("JWT_REFRESH_EXPIRES_IN", "7d"),
+  rateLimitMaxRequests: getRequiredEnvNumber("RATE_LIMIT_MAX_REQUESTS", 1000),
+  csrfMaxRequests: getRequiredEnvNumber("CSRF_MAX_REQUESTS", 60),
+  csrfSecret: process.env.CSRF_SECRET || null,
+  // Feature flags - disabled by default for backward compatibility
+  enablePasswordReset: getOptionalBoolean("ENABLE_PASSWORD_RESET", false),
+  enableRefreshTokenRotation: getOptionalBoolean("ENABLE_REFRESH_TOKEN_ROTATION", false),
+  enableAuditLogging: getOptionalBoolean("ENABLE_AUDIT_LOGGING", false),
+};
+
+// Validate JWT_SECRET strength in production
+if (config.nodeEnv === "production") {
+  if (config.jwtSecret.length < 32) {
+    throw new Error("JWT_SECRET must be at least 32 characters long in production");
+  }
+  if (config.jwtSecret === "your-secret-key-change-in-production") {
+    throw new Error("JWT_SECRET must be changed from default value in production");
+  }
+}
+
+// Validate frontend URL format
+try {
+  new URL(config.frontendUrl);
+} catch {
+  throw new Error(`Invalid FRONTEND_URL format: ${config.frontendUrl}`);
+}
+
+console.log("Configuration validated successfully");
@@ -0,0 +1,179 @@
+/**
+ * Authentication middleware for protecting routes
+ */
+import { Request, Response, NextFunction } from "express";
+import jwt from "jsonwebtoken";
+import { config } from "../config";
+import { PrismaClient } from "../generated/client";
+
+const prisma = new PrismaClient();
+
+// Extend Express Request type to include user
+declare global {
+  namespace Express {
+    interface Request {
+      user?: {
+        id: string;
+        email: string;
+        name: string;
+      };
+    }
+  }
+}
+
+interface JwtPayload {
+  userId: string;
+  email: string;
+  type: "access" | "refresh";
+}
+
+/**
+ * Type guard to check if decoded JWT is our expected payload structure
+ */
+const isJwtPayload = (decoded: unknown): decoded is JwtPayload => {
+  if (typeof decoded !== "object" || decoded === null) {
+    return false;
+  }
+  const payload = decoded as Record<string, unknown>;
+  return (
+    typeof payload.userId === "string" &&
+    typeof payload.email === "string" &&
+    (payload.type === "access" || payload.type === "refresh")
+  );
+};
+
+/**
+ * Extract JWT token from Authorization header
+ */
+const extractToken = (req: Request): string | null => {
+  const authHeader = req.headers.authorization;
+  if (!authHeader || typeof authHeader !== "string") return null;
+
+  const parts = authHeader.split(" ");
+  if (parts.length !== 2 || parts[0] !== "Bearer") {
+    return null;
+  }
+
+  return parts[1];
+};
+
+/**
+ * Verify and decode JWT token
+ */
+const verifyToken = (token: string): JwtPayload | null => {
+  try {
+    const decoded = jwt.verify(token, config.jwtSecret);
+    if (!isJwtPayload(decoded)) {
+      return null;
+    }
+    if (decoded.type !== "access") {
+      return null; // Only accept access tokens in middleware
+    }
+    return decoded;
+  } catch {
+    return null;
+  }
+};
+
+/**
+ * Require authentication middleware
+ * Protects routes that require a valid JWT token
+ */
+export const requireAuth = async (
+  req: Request,
+  res: Response,
+  next: NextFunction
+): Promise<void> => {
+  const token = extractToken(req);
+
+  if (!token) {
+    res.status(401).json({
+      error: "Unauthorized",
+      message: "Authentication token required",
+    });
+    return;
+  }
+
+  const payload = verifyToken(token);
+
+  if (!payload) {
+    res.status(401).json({
+      error: "Unauthorized",
+      message: "Invalid or expired token",
+    });
+    return;
+  }
+
+  // Verify user still exists and is active
+  try {
+    const user = await prisma.user.findUnique({
+      where: { id: payload.userId },
+      select: { id: true, email: true, name: true, isActive: true },
+    });
+
+    if (!user || !user.isActive) {
+      res.status(401).json({
+        error: "Unauthorized",
+        message: "User account not found or inactive",
+      });
+      return;
+    }
+
+    // Attach user to request
+    req.user = {
+      id: user.id,
+      email: user.email,
+      name: user.name,
+    };
+
+    next();
+  } catch (error) {
+    console.error("Error verifying user:", error);
+    res.status(500).json({
+      error: "Internal server error",
+      message: "Failed to verify user",
+    });
+  }
+};
+
+/**
+ * Optional authentication middleware
+ * Attaches user to request if token is present, but doesn't require it
+ */
+export const optionalAuth = async (
+  req: Request,
+  res: Response,
+  next: NextFunction
+): Promise<void> => {
+  const token = extractToken(req);
+
+  if (!token) {
+    return next();
+  }
+
+  const payload = verifyToken(token);
+
+  if (!payload) {
+    return next();
+  }
+
+  try {
+    const user = await prisma.user.findUnique({
+      where: { id: payload.userId },
+      select: { id: true, email: true, name: true, isActive: true },
+    });
+
+    if (user && user.isActive) {
+      req.user = {
+        id: user.id,
+        email: user.email,
+        name: user.name,
+      };
+    }
+  } catch (error) {
+    // Silently fail for optional auth
+    console.error("Error in optional auth:", error);
+  }
+
+  next();
+};
@@ -0,0 +1,86 @@
+/**
+ * Error handling middleware
+ * Sanitizes error messages in production to prevent information leakage
+ */
+import { Request, Response, NextFunction } from "express";
+import { config } from "../config";
+
+export interface AppError extends Error {
+  statusCode?: number;
+  isOperational?: boolean;
+}
+
+/**
+ * Error handler middleware
+ * Should be added last in the middleware chain
+ */
+export const errorHandler = (
+  err: AppError,
+  req: Request,
+  res: Response,
+  next: NextFunction
+): void => {
+  const statusCode = err.statusCode || 500;
+  const isDevelopment = config.nodeEnv === "development";
+
+  // Log full error details server-side
+  console.error("Error:", {
+    message: err.message,
+    stack: err.stack,
+    statusCode,
+    path: req.path,
+    method: req.method,
+    timestamp: new Date().toISOString(),
+  });
+
+  // In production, don't expose internal error details
+  if (!isDevelopment) {
+    // Generic error messages for clients
+    if (statusCode >= 500) {
+      res.status(statusCode).json({
+        error: "Internal server error",
+        message: "An error occurred while processing your request",
+      });
+      return;
+    }
+
+    // For client errors (4xx), provide generic message
+    res.status(statusCode).json({
+      error: "Request error",
+      message: err.isOperational ? err.message : "Invalid request",
+    });
+    return;
+  }
+
+  // In development, show full error details
+  res.status(statusCode).json({
+    error: err.message,
+    stack: err.stack,
+    statusCode,
+  });
+};
+
+/**
+ * Async error wrapper
+ * Wraps async route handlers to catch errors
+ */
+export const asyncHandler = <T = void>(
+  fn: (req: Request, res: Response, next: NextFunction) => Promise<T>
+) => {
+  return (req: Request, res: Response, next: NextFunction): void => {
+    Promise.resolve(fn(req, res, next)).catch(next);
+  };
+};
+
+/**
+ * Create an operational error (known error that can be safely shown to client)
+ */
+export const createError = (
+  message: string,
+  statusCode: number = 400
+): AppError => {
+  const error: AppError = new Error(message);
+  error.statusCode = statusCode;
+  error.isOperational = true;
+  return error;
+};
@@ -0,0 +1,92 @@
+/**
+ * Data migration script for existing drawings and collections
+ * This script assigns existing data to a default user
+ * Run this if you have existing data before the auth migration
+ */
+import { PrismaClient } from '../generated/client';
+import bcrypt from 'bcrypt';
+
+const prisma = new PrismaClient();
+
+async function migrateExistingData() {
+  try {
+    console.log('Starting data migration...');
+
+    // Check if there are any drawings or collections without userId
+    // Note: After migration, userId is required, so this query is for pre-migration data
+    // We use a raw query or check for missing userId field
+    const allDrawings = await prisma.drawing.findMany({
+      select: { id: true, userId: true },
+    });
+    const drawingsWithoutUser = allDrawings.filter((d) => !d.userId);
+
+    const allCollections = await prisma.collection.findMany({
+      select: { id: true, userId: true },
+    });
+    const collectionsWithoutUser = allCollections.filter((c) => !c.userId);
+
+    if (drawingsWithoutUser.length === 0 && collectionsWithoutUser.length === 0) {
+      console.log('No data to migrate. All records already have userId.');
+      return;
+    }
+
+    console.log(`Found ${drawingsWithoutUser.length} drawings and ${collectionsWithoutUser.length} collections without userId`);
+
+    // Create a default migration user
+    const defaultEmail = 'migration@excalidash.local';
+    const defaultPassword = await bcrypt.hash('migration-temp-password-change-me', 10);
+
+    let migrationUser = await prisma.user.findUnique({
+      where: { email: defaultEmail },
+    });
+
+    if (!migrationUser) {
+      migrationUser = await prisma.user.create({
+        data: {
+          email: defaultEmail,
+          passwordHash: defaultPassword,
+          name: 'Migration User',
+        },
+      });
+      console.log('Created migration user:', migrationUser.id);
+    }
+
+    // Update collections
+    if (collectionsWithoutUser.length > 0) {
+      const collectionIds = collectionsWithoutUser.map((c) => c.id);
+      await prisma.collection.updateMany({
+        where: {
+          id: { in: collectionIds },
+        },
+        data: {
+          userId: migrationUser.id,
+        },
+      });
+      console.log(`Assigned ${collectionsWithoutUser.length} collections to migration user`);
+    }
+
+    // Update drawings
+    if (drawingsWithoutUser.length > 0) {
+      const drawingIds = drawingsWithoutUser.map((d) => d.id);
+      await prisma.drawing.updateMany({
+        where: {
+          id: { in: drawingIds },
+        },
+        data: {
+          userId: migrationUser.id,
+        },
+      });
+      console.log(`Assigned ${drawingsWithoutUser.length} drawings to migration user`);
+    }
+
+    console.log('Migration completed successfully!');
+    console.log(`⚠️  IMPORTANT: Change the password for user ${defaultEmail} or delete this user after assigning data to real users.`);
+  } catch (error) {
+    console.error('Migration failed:', error);
+    throw error;
+  } finally {
+    await prisma.$disconnect();
+  }
+}
+
+migrateExistingData();
@@ -0,0 +1,205 @@
+/**
+ * Tests for audit logging utility
+ * 
+ * These tests verify that audit logging works correctly when enabled
+ * and gracefully degrades when disabled or when tables don't exist.
+ */
+
+import { describe, it, expect, beforeAll, afterAll, beforeEach } from "vitest";
+import { getTestPrisma, setupTestDb, initTestDb, createTestUser } from "../../__tests__/testUtils";
+import { logAuditEvent, getAuditLogs, type AuditLogData } from "../audit";
+
+describe("Audit Logging", () => {
+  const prisma = getTestPrisma();
+  let testUser: { id: string; email: string };
+
+  beforeAll(async () => {
+    setupTestDb();
+    testUser = await initTestDb(prisma);
+    // Enable audit logging for tests
+    process.env.ENABLE_AUDIT_LOGGING = "true";
+  });
+
+  afterAll(async () => {
+    await prisma.$disconnect();
+    delete process.env.ENABLE_AUDIT_LOGGING;
+  });
+
+  beforeEach(async () => {
+    // Clean up audit logs before each test
+    await prisma.auditLog.deleteMany({});
+  });
+
+  describe("logAuditEvent", () => {
+    it("should create an audit log entry when enabled", async () => {
+      const auditData: AuditLogData = {
+        userId: testUser.id,
+        action: "test_action",
+        resource: "test_resource",
+        ipAddress: "127.0.0.1",
+        userAgent: "test-agent",
+        details: { test: "value" },
+      };
+
+      await logAuditEvent(auditData);
+
+      const logs = await prisma.auditLog.findMany({
+        where: { userId: testUser.id, action: "test_action" },
+      });
+
+      expect(logs.length).toBe(1);
+      expect(logs[0].action).toBe("test_action");
+      expect(logs[0].resource).toBe("test_resource");
+      expect(logs[0].ipAddress).toBe("127.0.0.1");
+      expect(logs[0].userAgent).toBe("test-agent");
+      expect(logs[0].details).toBe(JSON.stringify({ test: "value" }));
+    });
+
+    it("should handle audit log without userId", async () => {
+      const auditData: AuditLogData = {
+        action: "anonymous_action",
+        ipAddress: "127.0.0.1",
+      };
+
+      await logAuditEvent(auditData);
+
+      const logs = await prisma.auditLog.findMany({
+        where: { action: "anonymous_action" },
+      });
+
+      expect(logs.length).toBe(1);
+      expect(logs[0].userId).toBeNull();
+    });
+
+    it("should handle audit log without optional fields", async () => {
+      const auditData: AuditLogData = {
+        action: "minimal_action",
+      };
+
+      await logAuditEvent(auditData);
+
+      const logs = await prisma.auditLog.findMany({
+        where: { action: "minimal_action" },
+      });
+
+      expect(logs.length).toBe(1);
+      expect(logs[0].resource).toBeNull();
+      expect(logs[0].ipAddress).toBeNull();
+      expect(logs[0].userAgent).toBeNull();
+      expect(logs[0].details).toBeNull();
+    });
+
+    it("should gracefully handle when feature is disabled", async () => {
+      // Note: Config is cached, so we test the graceful error handling instead
+      // by checking that errors don't propagate
+      const auditData: AuditLogData = {
+        action: "should_not_log_disabled",
+      };
+
+      // Should not throw even if feature is disabled or table missing
+      await expect(logAuditEvent(auditData)).resolves.not.toThrow();
+    });
+
+    it("should serialize details object to JSON", async () => {
+      const complexDetails = {
+        nested: { value: 123 },
+        array: [1, 2, 3],
+        string: "test",
+      };
+
+      await logAuditEvent({
+        userId: testUser.id,
+        action: "complex_details",
+        details: complexDetails,
+      });
+
+      const logs = await prisma.auditLog.findMany({
+        where: { action: "complex_details" },
+      });
+
+      expect(logs.length).toBe(1);
+      const parsed = JSON.parse(logs[0].details || "{}");
+      expect(parsed).toEqual(complexDetails);
+    });
+  });
+
+  describe("getAuditLogs", () => {
+    beforeEach(async () => {
+      // Create some test audit logs
+      await prisma.auditLog.createMany({
+        data: [
+          {
+            userId: testUser.id,
+            action: "action_1",
+            createdAt: new Date("2025-01-01T10:00:00Z"),
+          },
+          {
+            userId: testUser.id,
+            action: "action_2",
+            createdAt: new Date("2025-01-01T11:00:00Z"),
+          },
+          {
+            userId: testUser.id,
+            action: "action_3",
+            createdAt: new Date("2025-01-01T12:00:00Z"),
+          },
+        ],
+      });
+    });
+
+    it("should retrieve audit logs for a specific user", async () => {
+      const logs = await getAuditLogs(testUser.id);
+
+      expect(logs.length).toBe(3);
+      expect(logs[0].action).toBe("action_3"); // Most recent first
+      expect(logs[1].action).toBe("action_2");
+      expect(logs[2].action).toBe("action_1");
+    });
+
+    it("should retrieve all audit logs when userId is not provided", async () => {
+      // Create a log for another user
+      const otherUser = await createTestUser(prisma, "other@example.com");
+      await prisma.auditLog.create({
+        data: {
+          userId: otherUser.id,
+          action: "other_action",
+        },
+      });
+
+      const logs = await getAuditLogs();
+
+      expect(logs.length).toBeGreaterThanOrEqual(4);
+    });
+
+    it("should respect limit parameter", async () => {
+      const logs = await getAuditLogs(testUser.id, 2);
+
+      expect(logs.length).toBe(2);
+    });
+
+    it("should parse details JSON in returned logs", async () => {
+      await prisma.auditLog.create({
+        data: {
+          userId: testUser.id,
+          action: "with_details",
+          details: JSON.stringify({ key: "value" }),
+        },
+      });
+
+      const logs = await getAuditLogs(testUser.id, 1);
+
+      expect(logs.length).toBe(1);
+      expect((logs[0] as { details: unknown }).details).toEqual({ key: "value" });
+    });
+
+    it("should include user information in logs", async () => {
+      const logs = await getAuditLogs(testUser.id, 1);
+
+      expect(logs.length).toBe(1);
+      const log = logs[0] as { user: { id: string; email: string; name: string } };
+      expect(log.user).toBeDefined();
+      expect(log.user.id).toBe(testUser.id);
+      expect(log.user.email).toBe(testUser.email);
+    });
+  });
+});
@@ -0,0 +1,91 @@
+/**
+ * Audit logging utility for security events
+ */
+import { PrismaClient } from "../generated/client";
+
+const prisma = new PrismaClient();
+
+export interface AuditLogData {
+  userId?: string;
+  action: string;
+  resource?: string;
+  ipAddress?: string;
+  userAgent?: string;
+  details?: Record<string, unknown>;
+}
+
+/**
+ * Log a security event to the audit log
+ * This should be called for important security-related actions
+ * Gracefully handles missing audit log table (feature disabled)
+ */
+export const logAuditEvent = async (data: AuditLogData): Promise<void> => {
+  try {
+    // Check if audit logging is enabled via config
+    const { config } = await import("../config");
+    if (!config.enableAuditLogging) {
+      return; // Feature disabled, silently skip
+    }
+
+    await prisma.auditLog.create({
+      data: {
+        userId: data.userId || null,
+        action: data.action,
+        resource: data.resource || null,
+        ipAddress: data.ipAddress || null,
+        userAgent: data.userAgent || null,
+        details: data.details ? JSON.stringify(data.details) : null,
+      },
+    });
+  } catch (error) {
+    // Don't fail the request if audit logging fails
+    // This handles cases where the table doesn't exist (feature disabled)
+    // or other database errors
+    if (process.env.NODE_ENV === "development") {
+      console.debug("Audit logging skipped (feature disabled or table missing):", error);
+    }
+  }
+};
+
+/**
+ * Get audit logs for a user (or all users if userId is not provided)
+ * Returns empty array if audit logging is disabled or table doesn't exist
+ */
+export const getAuditLogs = async (
+  userId?: string,
+  limit: number = 100
+): Promise<unknown[]> => {
+  try {
+    // Check if audit logging is enabled via config
+    const { config } = await import("../config");
+    if (!config.enableAuditLogging) {
+      return []; // Feature disabled, return empty array
+    }
+
+    const logs = await prisma.auditLog.findMany({
+      where: userId ? { userId } : undefined,
+      orderBy: { createdAt: "desc" },
+      take: limit,
+      include: {
+        user: {
+          select: {
+            id: true,
+            email: true,
+            name: true,
+          },
+        },
+      },
+    });
+
+    return logs.map((log) => ({
+      ...log,
+      details: log.details ? JSON.parse(log.details) : null,
+    }));
+  } catch (error) {
+    // Gracefully handle missing table or other errors
+    if (process.env.NODE_ENV === "development") {
+      console.debug("Failed to retrieve audit logs (feature disabled or table missing):", error);
+    }
+    return [];
+  }
+};