From 203bd1e7f20ddbbb8f181266524818b7fd1ab5bd Mon Sep 17 00:00:00 2001 From: Jason Woltje Date: Thu, 5 Feb 2026 18:04:01 -0600 Subject: [PATCH] fix(#338): Standardize API base URL and auth mechanism across components - Create centralized config module (apps/web/src/lib/config.ts) exporting: - API_BASE_URL: Main API server URL from NEXT_PUBLIC_API_URL - ORCHESTRATOR_URL: Orchestrator service URL from NEXT_PUBLIC_ORCHESTRATOR_URL - Helper functions for building full URLs - Update client.ts to import from central config - Update LoginButton.tsx to use API_BASE_URL from config - Update useWebSocket.ts to use API_BASE_URL from config - Update AgentStatusWidget.tsx to use ORCHESTRATOR_URL from config - Update TaskProgressWidget.tsx to use ORCHESTRATOR_URL from config - Update useGraphData.ts to use API_BASE_URL from config - Fixed wrong default port (was 8000, now uses correct 3001) - Add comprehensive tests for config module - Update useWebSocket tests to properly mock config module Refs #338 Co-Authored-By: Claude Opus 4.5 --- apps/web/src/components/auth/LoginButton.tsx | 5 +- .../components/mindmap/hooks/useGraphData.ts | 3 +- .../components/widgets/AgentStatusWidget.tsx | 6 +- .../components/widgets/TaskProgressWidget.tsx | 5 +- apps/web/src/hooks/useWebSocket.test.tsx | 53 ++++++++--- apps/web/src/hooks/useWebSocket.ts | 5 +- apps/web/src/lib/api/client.ts | 2 +- apps/web/src/lib/config.test.ts | 91 +++++++++++++++++++ apps/web/src/lib/config.ts | 60 ++++++++++++ 9 files changed, 204 insertions(+), 26 deletions(-) create mode 100644 apps/web/src/lib/config.test.ts create mode 100644 apps/web/src/lib/config.ts diff --git a/apps/web/src/components/auth/LoginButton.tsx b/apps/web/src/components/auth/LoginButton.tsx index 858dd62..8c293ed 100644 --- a/apps/web/src/components/auth/LoginButton.tsx +++ b/apps/web/src/components/auth/LoginButton.tsx @@ -1,14 +1,13 @@ "use client"; import { Button } from "@mosaic/ui"; - -const API_URL = process.env.NEXT_PUBLIC_API_URL ?? "http://localhost:3001"; +import { API_BASE_URL } from "@/lib/config"; export function LoginButton(): React.JSX.Element { const handleLogin = (): void => { // Redirect to the backend OIDC authentication endpoint // BetterAuth will handle the OIDC flow and redirect back to the callback - window.location.assign(`${API_URL}/auth/signin/authentik`); + window.location.assign(`${API_BASE_URL}/auth/signin/authentik`); }; return ( diff --git a/apps/web/src/components/mindmap/hooks/useGraphData.ts b/apps/web/src/components/mindmap/hooks/useGraphData.ts index 6e8336a..5ee5d92 100644 --- a/apps/web/src/components/mindmap/hooks/useGraphData.ts +++ b/apps/web/src/components/mindmap/hooks/useGraphData.ts @@ -4,6 +4,7 @@ import { useCallback, useEffect, useState } from "react"; import { useSession } from "@/lib/auth-client"; import { handleSessionExpired, isSessionExpiring } from "@/lib/api"; +import { API_BASE_URL } from "@/lib/config"; // API Response types interface TagDto { @@ -119,7 +120,7 @@ interface UseGraphDataResult { searchNodes: (query: string) => Promise; } -const API_BASE = process.env.NEXT_PUBLIC_API_URL ?? "http://localhost:8000"; +const API_BASE = API_BASE_URL; /** * Sanitize labels for Mermaid diagrams to prevent XSS diff --git a/apps/web/src/components/widgets/AgentStatusWidget.tsx b/apps/web/src/components/widgets/AgentStatusWidget.tsx index 87c551e..3a329a5 100644 --- a/apps/web/src/components/widgets/AgentStatusWidget.tsx +++ b/apps/web/src/components/widgets/AgentStatusWidget.tsx @@ -5,6 +5,7 @@ import { useState, useEffect } from "react"; import { Bot, Activity, AlertCircle, CheckCircle, Clock } from "lucide-react"; import type { WidgetProps } from "@mosaic/shared"; +import { ORCHESTRATOR_URL } from "@/lib/config"; interface Agent { agentId: string; @@ -28,10 +29,7 @@ export function AgentStatusWidget({ id: _id, config: _config }: WidgetProps): Re setError(null); try { - // Get orchestrator URL from environment or default to localhost - const orchestratorUrl = process.env.NEXT_PUBLIC_ORCHESTRATOR_URL ?? "http://localhost:8001"; - - const response = await fetch(`${orchestratorUrl}/agents`, { + const response = await fetch(`${ORCHESTRATOR_URL}/agents`, { headers: { "Content-Type": "application/json", }, diff --git a/apps/web/src/components/widgets/TaskProgressWidget.tsx b/apps/web/src/components/widgets/TaskProgressWidget.tsx index 172f8fe..18a917e 100644 --- a/apps/web/src/components/widgets/TaskProgressWidget.tsx +++ b/apps/web/src/components/widgets/TaskProgressWidget.tsx @@ -8,6 +8,7 @@ import { useState, useEffect } from "react"; import { Activity, CheckCircle, XCircle, Clock, Loader2 } from "lucide-react"; import type { WidgetProps } from "@mosaic/shared"; +import { ORCHESTRATOR_URL } from "@/lib/config"; interface AgentTask { agentId: string; @@ -98,10 +99,8 @@ export function TaskProgressWidget({ id: _id, config: _config }: WidgetProps): R const [error, setError] = useState(null); useEffect(() => { - const orchestratorUrl = process.env.NEXT_PUBLIC_ORCHESTRATOR_URL ?? "http://localhost:3001"; - const fetchTasks = (): void => { - fetch(`${orchestratorUrl}/agents`) + fetch(`${ORCHESTRATOR_URL}/agents`) .then((res) => { if (!res.ok) throw new Error(`HTTP ${String(res.status)}`); return res.json() as Promise; diff --git a/apps/web/src/hooks/useWebSocket.test.tsx b/apps/web/src/hooks/useWebSocket.test.tsx index 5b5d2fc..4e0f46a 100644 --- a/apps/web/src/hooks/useWebSocket.test.tsx +++ b/apps/web/src/hooks/useWebSocket.test.tsx @@ -315,15 +315,23 @@ describe("useWebSocket", (): void => { describe("WSS enforcement", (): void => { afterEach((): void => { vi.unstubAllEnvs(); + vi.resetModules(); }); - it("should warn when using ws:// in production", (): void => { + it("should warn when using ws:// in production", async (): Promise => { const consoleWarnSpy = vi.spyOn(console, "warn").mockImplementation(() => undefined); vi.stubEnv("NODE_ENV", "production"); - vi.stubEnv("NEXT_PUBLIC_API_URL", "http://insecure-server.com"); - renderHook(() => useWebSocket("workspace-123", "token")); + // Mock the config module to return insecure URL + vi.doMock("@/lib/config", () => ({ + API_BASE_URL: "http://insecure-server.com", + })); + + // Re-import to get fresh module with mocked config + const { useWebSocket: useWebSocketMocked } = await import("./useWebSocket"); + + renderHook(() => useWebSocketMocked("workspace-123", "token")); expect(consoleWarnSpy).toHaveBeenCalledWith(expect.stringContaining("[Security Warning]")); expect(consoleWarnSpy).toHaveBeenCalledWith(expect.stringContaining("insecure protocol")); @@ -331,39 +339,60 @@ describe("useWebSocket", (): void => { consoleWarnSpy.mockRestore(); }); - it("should not warn when using https:// in production", (): void => { + it("should not warn when using https:// in production", async (): Promise => { const consoleWarnSpy = vi.spyOn(console, "warn").mockImplementation(() => undefined); vi.stubEnv("NODE_ENV", "production"); - vi.stubEnv("NEXT_PUBLIC_API_URL", "https://secure-server.com"); - renderHook(() => useWebSocket("workspace-123", "token")); + // Mock the config module to return secure URL + vi.doMock("@/lib/config", () => ({ + API_BASE_URL: "https://secure-server.com", + })); + + // Re-import to get fresh module with mocked config + const { useWebSocket: useWebSocketMocked } = await import("./useWebSocket"); + + renderHook(() => useWebSocketMocked("workspace-123", "token")); expect(consoleWarnSpy).not.toHaveBeenCalled(); consoleWarnSpy.mockRestore(); }); - it("should not warn when using wss:// in production", (): void => { + it("should not warn when using wss:// in production", async (): Promise => { const consoleWarnSpy = vi.spyOn(console, "warn").mockImplementation(() => undefined); vi.stubEnv("NODE_ENV", "production"); - vi.stubEnv("NEXT_PUBLIC_API_URL", "wss://secure-server.com"); - renderHook(() => useWebSocket("workspace-123", "token")); + // Mock the config module to return secure WSS URL + vi.doMock("@/lib/config", () => ({ + API_BASE_URL: "wss://secure-server.com", + })); + + // Re-import to get fresh module with mocked config + const { useWebSocket: useWebSocketMocked } = await import("./useWebSocket"); + + renderHook(() => useWebSocketMocked("workspace-123", "token")); expect(consoleWarnSpy).not.toHaveBeenCalled(); consoleWarnSpy.mockRestore(); }); - it("should not warn in development mode even with http://", (): void => { + it("should not warn in development mode even with http://", async (): Promise => { const consoleWarnSpy = vi.spyOn(console, "warn").mockImplementation(() => undefined); vi.stubEnv("NODE_ENV", "development"); - vi.stubEnv("NEXT_PUBLIC_API_URL", "http://localhost:3001"); - renderHook(() => useWebSocket("workspace-123", "token")); + // Mock the config module to return insecure URL (but we're in dev mode) + vi.doMock("@/lib/config", () => ({ + API_BASE_URL: "http://localhost:3001", + })); + + // Re-import to get fresh module with mocked config + const { useWebSocket: useWebSocketMocked } = await import("./useWebSocket"); + + renderHook(() => useWebSocketMocked("workspace-123", "token")); expect(consoleWarnSpy).not.toHaveBeenCalled(); diff --git a/apps/web/src/hooks/useWebSocket.ts b/apps/web/src/hooks/useWebSocket.ts index 4896ddb..e5cea5f 100644 --- a/apps/web/src/hooks/useWebSocket.ts +++ b/apps/web/src/hooks/useWebSocket.ts @@ -1,6 +1,7 @@ import { useEffect, useState } from "react"; import type { Socket } from "socket.io-client"; import { io } from "socket.io-client"; +import { API_BASE_URL } from "@/lib/config"; interface Task { id: string; @@ -87,8 +88,8 @@ export function useWebSocket( } = callbacks; useEffect(() => { - // Get WebSocket URL from environment or default to API URL - const wsUrl = process.env.NEXT_PUBLIC_API_URL ?? "http://localhost:3001"; + // Use WebSocket URL from central config + const wsUrl = API_BASE_URL; // Validate WebSocket security - warn if using insecure connection in production validateWebSocketSecurity(wsUrl); diff --git a/apps/web/src/lib/api/client.ts b/apps/web/src/lib/api/client.ts index 1077570..35b3d63 100644 --- a/apps/web/src/lib/api/client.ts +++ b/apps/web/src/lib/api/client.ts @@ -5,7 +5,7 @@ /* eslint-disable @typescript-eslint/no-unsafe-assignment */ -const API_BASE_URL = process.env.NEXT_PUBLIC_API_URL ?? "http://localhost:3001"; +import { API_BASE_URL } from "../config"; /** * In-memory CSRF token storage diff --git a/apps/web/src/lib/config.test.ts b/apps/web/src/lib/config.test.ts new file mode 100644 index 0000000..f4bf828 --- /dev/null +++ b/apps/web/src/lib/config.test.ts @@ -0,0 +1,91 @@ +/** + * Tests for centralized API configuration + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; + +// Store original env +const originalEnv = { ...process.env }; + +describe("API Configuration", () => { + beforeEach(() => { + // Reset modules to pick up new env values + vi.resetModules(); + }); + + afterEach(() => { + // Restore original env + process.env = originalEnv; + }); + + describe("default values", () => { + it("should use default API URL when NEXT_PUBLIC_API_URL is not set", async () => { + delete process.env.NEXT_PUBLIC_API_URL; + delete process.env.NEXT_PUBLIC_ORCHESTRATOR_URL; + + const { API_BASE_URL, ORCHESTRATOR_URL } = await import("./config"); + + expect(API_BASE_URL).toBe("http://localhost:3001"); + expect(ORCHESTRATOR_URL).toBe("http://localhost:3001"); + }); + }); + + describe("custom values", () => { + it("should use NEXT_PUBLIC_API_URL when set", async () => { + process.env.NEXT_PUBLIC_API_URL = "https://api.example.com"; + delete process.env.NEXT_PUBLIC_ORCHESTRATOR_URL; + + const { API_BASE_URL, ORCHESTRATOR_URL } = await import("./config"); + + expect(API_BASE_URL).toBe("https://api.example.com"); + // ORCHESTRATOR_URL should fall back to API_BASE_URL + expect(ORCHESTRATOR_URL).toBe("https://api.example.com"); + }); + + it("should use separate NEXT_PUBLIC_ORCHESTRATOR_URL when set", async () => { + process.env.NEXT_PUBLIC_API_URL = "https://api.example.com"; + process.env.NEXT_PUBLIC_ORCHESTRATOR_URL = "https://orchestrator.example.com"; + + const { API_BASE_URL, ORCHESTRATOR_URL } = await import("./config"); + + expect(API_BASE_URL).toBe("https://api.example.com"); + expect(ORCHESTRATOR_URL).toBe("https://orchestrator.example.com"); + }); + }); + + describe("helper functions", () => { + it("should build API URLs correctly", async () => { + process.env.NEXT_PUBLIC_API_URL = "https://api.example.com"; + delete process.env.NEXT_PUBLIC_ORCHESTRATOR_URL; + + const { buildApiUrl } = await import("./config"); + + expect(buildApiUrl("/api/v1/tasks")).toBe("https://api.example.com/api/v1/tasks"); + expect(buildApiUrl("/auth/signin")).toBe("https://api.example.com/auth/signin"); + }); + + it("should build orchestrator URLs correctly", async () => { + process.env.NEXT_PUBLIC_API_URL = "https://api.example.com"; + process.env.NEXT_PUBLIC_ORCHESTRATOR_URL = "https://orch.example.com"; + + const { buildOrchestratorUrl } = await import("./config"); + + expect(buildOrchestratorUrl("/agents")).toBe("https://orch.example.com/agents"); + expect(buildOrchestratorUrl("/tasks/status")).toBe("https://orch.example.com/tasks/status"); + }); + }); + + describe("apiConfig object", () => { + it("should expose all configuration through apiConfig", async () => { + process.env.NEXT_PUBLIC_API_URL = "https://api.example.com"; + process.env.NEXT_PUBLIC_ORCHESTRATOR_URL = "https://orch.example.com"; + + const { apiConfig } = await import("./config"); + + expect(apiConfig.baseUrl).toBe("https://api.example.com"); + expect(apiConfig.orchestratorUrl).toBe("https://orch.example.com"); + expect(apiConfig.buildUrl("/test")).toBe("https://api.example.com/test"); + expect(apiConfig.buildOrchestratorUrl("/test")).toBe("https://orch.example.com/test"); + }); + }); +}); diff --git a/apps/web/src/lib/config.ts b/apps/web/src/lib/config.ts new file mode 100644 index 0000000..cdb8bfd --- /dev/null +++ b/apps/web/src/lib/config.ts @@ -0,0 +1,60 @@ +/** + * Centralized API Configuration + * + * This module provides a single source of truth for all API endpoints and URLs. + * All components should import from here instead of reading environment variables directly. + * + * Environment Variables: + * - NEXT_PUBLIC_API_URL: The main API server URL (default: http://localhost:3001) + * - NEXT_PUBLIC_ORCHESTRATOR_URL: The orchestrator service URL (default: same as API URL) + */ + +/** + * Default API server URL for local development + */ +const DEFAULT_API_URL = "http://localhost:3001"; + +/** + * Main API server URL + * Used for authentication, tasks, events, knowledge, and all core API calls + */ +export const API_BASE_URL = process.env.NEXT_PUBLIC_API_URL ?? DEFAULT_API_URL; + +/** + * Orchestrator service URL + * Used for agent management, task progress, and orchestration features + * Falls back to main API URL if not specified (they may run on the same server) + */ +export const ORCHESTRATOR_URL = process.env.NEXT_PUBLIC_ORCHESTRATOR_URL ?? API_BASE_URL; + +/** + * Build a full API endpoint URL + * @param endpoint - The API endpoint path (should start with /) + * @returns The full URL for the endpoint + */ +export function buildApiUrl(endpoint: string): string { + return `${API_BASE_URL}${endpoint}`; +} + +/** + * Build a full orchestrator endpoint URL + * @param endpoint - The orchestrator endpoint path (should start with /) + * @returns The full URL for the endpoint + */ +export function buildOrchestratorUrl(endpoint: string): string { + return `${ORCHESTRATOR_URL}${endpoint}`; +} + +/** + * Configuration object for convenient access to all URLs + */ +export const apiConfig = { + /** Main API base URL */ + baseUrl: API_BASE_URL, + /** Orchestrator service URL */ + orchestratorUrl: ORCHESTRATOR_URL, + /** Build full API URL for an endpoint */ + buildUrl: buildApiUrl, + /** Build full orchestrator URL for an endpoint */ + buildOrchestratorUrl, +} as const;