From 8796782d1d705fd5e1dab360673f58f89338e4b7 Mon Sep 17 00:00:00 2001 From: Hendrik Belitz Date: Mon, 1 Sep 2025 15:47:51 +0200 Subject: [PATCH 1/2] Fix session expiration and tests --- .../auth/__tests__/session-service.test.ts | 209 +++++++++++++++++ src/lib/server/auth/session-service.ts | 3 +- src/routes/api/auth/session/+server.ts | 2 +- .../api/auth/session/__tests__/server.test.ts | 212 ++++++++++++++++++ src/server-hooks/apiAuthHandle.ts | 1 + 5 files changed, 425 insertions(+), 2 deletions(-) create mode 100644 src/lib/server/auth/__tests__/session-service.test.ts create mode 100644 src/routes/api/auth/session/__tests__/server.test.ts diff --git a/src/lib/server/auth/__tests__/session-service.test.ts b/src/lib/server/auth/__tests__/session-service.test.ts new file mode 100644 index 0000000..c843f36 --- /dev/null +++ b/src/lib/server/auth/__tests__/session-service.test.ts @@ -0,0 +1,209 @@ +/* eslint-disable @typescript-eslint/no-explicit-any */ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { SessionService } from "../session-service"; +import * as jwtUtils from "../jwt-utils"; + +// Mock dependencies +vi.mock("$lib/server/db", () => ({ + db: { + select: vi.fn(), + update: vi.fn() + } +})); +vi.mock("../jwt-utils"); + +const mockUser = { + id: "test-user-id", + email: "test@example.com", + name: "Test User", + role: "STAFF" as const, + tenantId: "tenant-123", + isActive: true, + confirmed: true, + createdAt: new Date(), + updatedAt: new Date(), + lastLoginAt: null +}; + +const mockSession = { + id: "session-id", + userId: "test-user-id", + sessionToken: "session-token", + accessToken: "access-token", + refreshToken: "refresh-token", + expiresAt: new Date(Date.now() + 7 * 24 * 60 * 60 * 1000), // 7 days from now + lastUsedAt: new Date(), + ipAddress: "127.0.0.1", + userAgent: "test-agent", + createdAt: new Date(), + updatedAt: new Date() +}; + +describe("SessionService.validateTokenWithDB", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("should return user, sessionId and exp for valid token", async () => { + const { db } = await import("$lib/server/db"); + + // Mock JWT verification + const mockTokenData = { + sessionId: "session-id", + userId: "test-user-id", + exp: Math.floor(Date.now() / 1000) + 3600 + }; + vi.mocked(jwtUtils.verifyAccessToken).mockResolvedValue(mockTokenData); + + // Mock successful database query + const mockQuery = vi.fn().mockResolvedValue([ + { + user: mockUser, + user_session: mockSession + } + ]); + + vi.mocked(db.select).mockReturnValue({ + from: vi.fn().mockReturnValue({ + innerJoin: vi.fn().mockReturnValue({ + where: vi.fn().mockReturnValue({ + limit: mockQuery + }) + }) + }) + } as any); + + vi.mocked(db.update).mockReturnValue({ + set: vi.fn().mockReturnValue({ + where: vi.fn().mockResolvedValue([]) + }) + } as any); + + const result = await SessionService.validateTokenWithDB("valid-token"); + + expect(result).toEqual({ + user: mockUser, + sessionId: "session-id", + exp: mockSession.expiresAt + }); + expect(jwtUtils.verifyAccessToken).toHaveBeenCalledWith("valid-token"); + }); + + it("should return null for invalid JWT token", async () => { + vi.mocked(jwtUtils.verifyAccessToken).mockResolvedValue(null); + + const result = await SessionService.validateTokenWithDB("invalid-token"); + + expect(result).toBeNull(); + expect(jwtUtils.verifyAccessToken).toHaveBeenCalledWith("invalid-token"); + }); + + it("should return null for non-existent session", async () => { + const { db } = await import("$lib/server/db"); + + const mockTokenData = { + sessionId: "non-existent-session", + userId: "test-user-id", + exp: Math.floor(Date.now() / 1000) + 3600 + }; + vi.mocked(jwtUtils.verifyAccessToken).mockResolvedValue(mockTokenData); + + // Mock empty database result + const mockQuery = vi.fn().mockResolvedValue([]); + vi.mocked(db.select).mockReturnValue({ + from: vi.fn().mockReturnValue({ + innerJoin: vi.fn().mockReturnValue({ + where: vi.fn().mockReturnValue({ + limit: mockQuery + }) + }) + }) + } as any); + + const result = await SessionService.validateTokenWithDB("valid-token"); + + expect(result).toBeNull(); + }); + + it("should return null for inactive user", async () => { + const { db } = await import("$lib/server/db"); + + const mockTokenData = { + sessionId: "session-id", + userId: "test-user-id", + exp: Math.floor(Date.now() / 1000) + 3600 + }; + vi.mocked(jwtUtils.verifyAccessToken).mockResolvedValue(mockTokenData); + + const inactiveUser = { + ...mockUser, + isActive: false + }; + + const mockQuery = vi.fn().mockResolvedValue([ + { + user: inactiveUser, + user_session: mockSession + } + ]); + + vi.mocked(db.select).mockReturnValue({ + from: vi.fn().mockReturnValue({ + innerJoin: vi.fn().mockReturnValue({ + where: vi.fn().mockReturnValue({ + limit: mockQuery + }) + }) + }) + } as any); + + const result = await SessionService.validateTokenWithDB("valid-token"); + + expect(result).toBeNull(); + }); + + it("should return null for unconfirmed user", async () => { + const { db } = await import("$lib/server/db"); + + const mockTokenData = { + sessionId: "session-id", + userId: "test-user-id", + exp: Math.floor(Date.now() / 1000) + 3600 + }; + vi.mocked(jwtUtils.verifyAccessToken).mockResolvedValue(mockTokenData); + + const unconfirmedUser = { + ...mockUser, + confirmed: false + }; + + const mockQuery = vi.fn().mockResolvedValue([ + { + user: unconfirmedUser, + user_session: mockSession + } + ]); + + vi.mocked(db.select).mockReturnValue({ + from: vi.fn().mockReturnValue({ + innerJoin: vi.fn().mockReturnValue({ + where: vi.fn().mockReturnValue({ + limit: mockQuery + }) + }) + }) + } as any); + + const result = await SessionService.validateTokenWithDB("valid-token"); + + expect(result).toBeNull(); + }); + + it("should return null and handle errors gracefully", async () => { + vi.mocked(jwtUtils.verifyAccessToken).mockRejectedValue(new Error("JWT error")); + + const result = await SessionService.validateTokenWithDB("token"); + + expect(result).toBeNull(); + }); +}); diff --git a/src/lib/server/auth/session-service.ts b/src/lib/server/auth/session-service.ts index df94c5d..cb90a61 100644 --- a/src/lib/server/auth/session-service.ts +++ b/src/lib/server/auth/session-service.ts @@ -106,7 +106,7 @@ export class SessionService { */ static async validateTokenWithDB( accessToken: string - ): Promise<{ user: SelectUser; sessionId: string } | null> { + ): Promise<{ user: SelectUser; sessionId: string; exp: Date } | null> { logger.debug("Validating access token with database"); try { @@ -158,6 +158,7 @@ export class SessionService { return { user: session.user, + exp: session.user_session.expiresAt, sessionId: session.user_session.id }; } catch (error) { diff --git a/src/routes/api/auth/session/+server.ts b/src/routes/api/auth/session/+server.ts index 720ddbe..3bcfae0 100644 --- a/src/routes/api/auth/session/+server.ts +++ b/src/routes/api/auth/session/+server.ts @@ -84,7 +84,7 @@ export const GET: RequestHandler = async ({ locals }) => { role: locals.user.role, tenantId: locals.user.tenantId }, - expiresAt: new Date(locals.user.exp! * 1000).toISOString() + expiresAt: new Date(locals.user.exp ?? 0).toISOString() }); } catch (error) { logger.error("Session check error:", { error: String(error) }); diff --git a/src/routes/api/auth/session/__tests__/server.test.ts b/src/routes/api/auth/session/__tests__/server.test.ts new file mode 100644 index 0000000..06dfd21 --- /dev/null +++ b/src/routes/api/auth/session/__tests__/server.test.ts @@ -0,0 +1,212 @@ +/* eslint-disable @typescript-eslint/no-explicit-any */ +import { describe, it, expect, vi } from "vitest"; +import { GET } from "../+server"; +import type { RequestEvent } from "@sveltejs/kit"; + +// Mock the logger +vi.mock("$lib/logger", () => ({ + UniversalLogger: vi.fn(() => ({ + setContext: vi.fn(() => ({ + error: vi.fn(), + debug: vi.fn() + })) + })) +})); + +// Mock OpenAPI registration +vi.mock("$lib/server/openapi", () => ({ + registerOpenAPIRoute: vi.fn() +})); + +const createMockRequestEvent = (locals: any): RequestEvent => ({ + locals, + request: new Request("http://localhost/api/auth/session"), + url: new URL("http://localhost/api/auth/session"), + params: {}, + route: { id: "/api/auth/session" }, + cookies: {} as any, + fetch: fetch, + getClientAddress: () => "127.0.0.1", + isDataRequest: false, + platform: undefined, + setHeaders: vi.fn(), + depends: vi.fn(), + parent: vi.fn() +}); + +describe("/api/auth/session GET endpoint", () => { + it("should return user session data with correct exp field when user is authenticated", async () => { + const mockUser = { + userId: "user-123", + email: "test@example.com", + name: "Test User", + role: "STAFF", + tenantId: "tenant-123", + sessionId: "session-123", + exp: 1640995200000, // timestamp in milliseconds + isActive: true, + confirmed: true + }; + + const event = createMockRequestEvent({ user: mockUser }); + const response = await GET(event); + const data = await response.json(); + + expect(response.status).toBe(200); + expect(data).toEqual({ + authenticated: true, + user: { + id: "user-123", + email: "test@example.com", + name: "Test User", + role: "STAFF", + tenantId: "tenant-123" + }, + expiresAt: new Date(1640995200000).toISOString() + }); + }); + + it("should handle exp as undefined gracefully", async () => { + const mockUser = { + userId: "user-123", + email: "test@example.com", + name: "Test User", + role: "STAFF", + tenantId: "tenant-123", + sessionId: "session-123", + exp: undefined, // Test the null coalescing operator + isActive: true, + confirmed: true + }; + + const event = createMockRequestEvent({ user: mockUser }); + const response = await GET(event); + const data = await response.json(); + + expect(response.status).toBe(200); + expect(data).toEqual({ + authenticated: true, + user: { + id: "user-123", + email: "test@example.com", + name: "Test User", + role: "STAFF", + tenantId: "tenant-123" + }, + expiresAt: new Date(0).toISOString() // Should default to 0 + }); + }); + + it("should handle exp as null gracefully", async () => { + const mockUser = { + userId: "user-123", + email: "test@example.com", + name: "Test User", + role: "STAFF", + tenantId: "tenant-123", + sessionId: "session-123", + exp: null, // Test the null coalescing operator + isActive: true, + confirmed: true + }; + + const event = createMockRequestEvent({ user: mockUser }); + const response = await GET(event); + const data = await response.json(); + + expect(response.status).toBe(200); + expect(data).toEqual({ + authenticated: true, + user: { + id: "user-123", + email: "test@example.com", + name: "Test User", + role: "STAFF", + tenantId: "tenant-123" + }, + expiresAt: new Date(0).toISOString() // Should default to 0 + }); + }); + + it("should return 401 when user is not authenticated", async () => { + const event = createMockRequestEvent({ user: null }); + const response = await GET(event); + + expect(response.status).toBe(401); + const data = await response.json(); + expect(data).toEqual({ + authenticated: false, + message: "Not authenticated" + }); + }); + + it("should handle global admin without tenantId", async () => { + const mockGlobalAdmin = { + userId: "admin-123", + email: "admin@example.com", + name: "Global Admin", + role: "GLOBAL_ADMIN", + tenantId: undefined, // Global admin has no tenant + sessionId: "session-123", + exp: 1640995200000, + isActive: true, + confirmed: true + }; + + const event = createMockRequestEvent({ user: mockGlobalAdmin }); + const response = await GET(event); + const data = await response.json(); + + expect(response.status).toBe(200); + expect(data).toEqual({ + authenticated: true, + user: { + id: "admin-123", + email: "admin@example.com", + name: "Global Admin", + role: "GLOBAL_ADMIN", + tenantId: undefined + }, + expiresAt: new Date(1640995200000).toISOString() + }); + }); + + it("should handle exp as 0 (epoch time)", async () => { + const mockUser = { + userId: "user-123", + email: "test@example.com", + name: "Test User", + role: "STAFF", + tenantId: "tenant-123", + sessionId: "session-123", + exp: 0, // Epoch time + isActive: true, + confirmed: true + }; + + const event = createMockRequestEvent({ user: mockUser }); + const response = await GET(event); + const data = await response.json(); + + expect(response.status).toBe(200); + expect(data.expiresAt).toBe(new Date(0).toISOString()); + }); + + it("should return 500 when an error occurs", async () => { + // Create a mock that throws an error when accessing properties + const problematicLocals = { + get user() { + throw new Error("Database connection failed"); + } + }; + + const event = createMockRequestEvent(problematicLocals); + const response = await GET(event); + + expect(response.status).toBe(500); + const data = await response.json(); + expect(data).toEqual({ + error: "Internal server error" + }); + }); +}); diff --git a/src/server-hooks/apiAuthHandle.ts b/src/server-hooks/apiAuthHandle.ts index 3a65f9a..5a94c40 100644 --- a/src/server-hooks/apiAuthHandle.ts +++ b/src/server-hooks/apiAuthHandle.ts @@ -92,6 +92,7 @@ export const apiAuthHandle: Handle = async ({ event, resolve }) => { // Add sessionId to the user object for easy access event.locals.user = { userId: sessionData.user.id, + exp: sessionData.exp.valueOf(), ...sessionData.user, sessionId: sessionData.sessionId }; From 18df434b715ee0533474ec8911f01aff94c52a23 Mon Sep 17 00:00:00 2001 From: Hendrik Belitz Date: Mon, 1 Sep 2025 15:55:53 +0200 Subject: [PATCH 2/2] Type check fix --- .../api/auth/session/__tests__/server.test.ts | 45 ++++++++++--------- 1 file changed, 23 insertions(+), 22 deletions(-) diff --git a/src/routes/api/auth/session/__tests__/server.test.ts b/src/routes/api/auth/session/__tests__/server.test.ts index 06dfd21..f24823c 100644 --- a/src/routes/api/auth/session/__tests__/server.test.ts +++ b/src/routes/api/auth/session/__tests__/server.test.ts @@ -18,21 +18,22 @@ vi.mock("$lib/server/openapi", () => ({ registerOpenAPIRoute: vi.fn() })); -const createMockRequestEvent = (locals: any): RequestEvent => ({ - locals, - request: new Request("http://localhost/api/auth/session"), - url: new URL("http://localhost/api/auth/session"), - params: {}, - route: { id: "/api/auth/session" }, - cookies: {} as any, - fetch: fetch, - getClientAddress: () => "127.0.0.1", - isDataRequest: false, - platform: undefined, - setHeaders: vi.fn(), - depends: vi.fn(), - parent: vi.fn() -}); +const createMockRequestEvent = (locals: any): RequestEvent => + ({ + locals, + request: new Request("http://localhost/api/auth/session"), + url: new URL("http://localhost/api/auth/session"), + params: {}, + route: { id: "/api/auth/session" }, + cookies: {} as any, + fetch: fetch, + getClientAddress: () => "127.0.0.1", + isDataRequest: false, + platform: undefined, + setHeaders: vi.fn(), + depends: vi.fn(), + parent: vi.fn() + }) as any; describe("/api/auth/session GET endpoint", () => { it("should return user session data with correct exp field when user is authenticated", async () => { @@ -49,7 +50,7 @@ describe("/api/auth/session GET endpoint", () => { }; const event = createMockRequestEvent({ user: mockUser }); - const response = await GET(event); + const response = await GET(event as any); const data = await response.json(); expect(response.status).toBe(200); @@ -80,7 +81,7 @@ describe("/api/auth/session GET endpoint", () => { }; const event = createMockRequestEvent({ user: mockUser }); - const response = await GET(event); + const response = await GET(event as any); const data = await response.json(); expect(response.status).toBe(200); @@ -111,7 +112,7 @@ describe("/api/auth/session GET endpoint", () => { }; const event = createMockRequestEvent({ user: mockUser }); - const response = await GET(event); + const response = await GET(event as any); const data = await response.json(); expect(response.status).toBe(200); @@ -130,7 +131,7 @@ describe("/api/auth/session GET endpoint", () => { it("should return 401 when user is not authenticated", async () => { const event = createMockRequestEvent({ user: null }); - const response = await GET(event); + const response = await GET(event as any); expect(response.status).toBe(401); const data = await response.json(); @@ -154,7 +155,7 @@ describe("/api/auth/session GET endpoint", () => { }; const event = createMockRequestEvent({ user: mockGlobalAdmin }); - const response = await GET(event); + const response = await GET(event as any); const data = await response.json(); expect(response.status).toBe(200); @@ -185,7 +186,7 @@ describe("/api/auth/session GET endpoint", () => { }; const event = createMockRequestEvent({ user: mockUser }); - const response = await GET(event); + const response = await GET(event as any); const data = await response.json(); expect(response.status).toBe(200); @@ -201,7 +202,7 @@ describe("/api/auth/session GET endpoint", () => { }; const event = createMockRequestEvent(problematicLocals); - const response = await GET(event); + const response = await GET(event as any); expect(response.status).toBe(500); const data = await response.json();