diff --git a/testplanit/app/[locale]/admin/users/AddUser.tsx b/testplanit/app/[locale]/admin/users/AddUser.tsx index 67b064503..d81545317 100644 --- a/testplanit/app/[locale]/admin/users/AddUser.tsx +++ b/testplanit/app/[locale]/admin/users/AddUser.tsx @@ -392,7 +392,7 @@ export function AddUser({ open, onClose }: AddUserProps) { // If the admin skipped the password (allowed when a passwordless login // method is available), store an unusable random secret — same pattern as - // SAML auto-provisioning in app/api/auth/saml/callback/route.ts. User + // SAML auto-provisioning in app/api/auth/callback/saml/route.ts. User // signs in via magic link / SSO. const passwordForApi = data.password.length > 0 ? data.password : crypto.randomUUID(); diff --git a/testplanit/app/api/auth/callback/saml/route.test.ts b/testplanit/app/api/auth/callback/saml/route.test.ts new file mode 100644 index 000000000..8529e0324 --- /dev/null +++ b/testplanit/app/api/auth/callback/saml/route.test.ts @@ -0,0 +1,118 @@ +// @vitest-environment node +import jwt from "jsonwebtoken"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +vi.hoisted(() => { + process.env.NEXTAUTH_SECRET = "test-secret-key-at-least-32-chars-long"; + process.env.NEXTAUTH_URL = "https://app.example.com"; +}); + +const SECRET = "test-secret-key-at-least-32-chars-long"; + +vi.mock("~/lib/valkey", () => ({ default: null })); // RelayState via signed token + +vi.mock("~/server/db", () => ({ + db: { + samlConfiguration: { findUnique: vi.fn() }, + user: { findUnique: vi.fn(), update: vi.fn(), create: vi.fn() }, + account: { upsert: vi.fn() }, + roles: { findFirst: vi.fn() }, + }, +})); + +const { validateSAMLResponse } = vi.hoisted(() => ({ + validateSAMLResponse: vi.fn(), +})); +vi.mock("~/server/saml-provider", () => ({ + createSAMLClient: vi.fn(async () => ({})), + validateSAMLResponse, +})); +vi.mock("~/lib/services/notificationService", () => ({ + NotificationService: { createUserRegistrationNotification: vi.fn() }, +})); +vi.mock("~/lib/utils/email-domain-validation", () => ({ + isEmailDomainAllowed: vi.fn(async () => true), +})); + +import { db } from "~/server/db"; +import { POST } from "./route"; + +function makeReq( + relayState: string | null, + samlResponse: string | null = "" +) { + const fd = new FormData(); + if (samlResponse) fd.set("SAMLResponse", samlResponse); + if (relayState) fd.set("RelayState", relayState); + return { + headers: new Headers(), + formData: async () => fd, + url: "https://cint-prod-pod:3000/api/auth/callback/saml", + } as any; +} + +// A RelayState as the init route would mint it without Valkey (signed token). +function relayFor(providerId: string, callbackUrl = "/dash") { + return jwt.sign({ providerId, callbackUrl }, SECRET, { expiresIn: "15m" }); +} + +describe("POST /api/auth/callback/saml — ACS validator", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("returns 400 when RelayState is missing or invalid (Bug 4 regression)", async () => { + const res = await POST(makeReq(null)); + expect(res.status).toBe(400); + const body = await res.json(); + expect(body.error).toMatch(/invalid or expired/i); + }); + + it("recovers the provider from RelayState and hands off to /api/auth/saml/complete (Bug 3)", async () => { + (db.samlConfiguration.findUnique as any).mockResolvedValue({ + id: "cfg", + entryPoint: "e", + cert: "c", + issuer: "i", + attributeMapping: {}, + autoProvisionUsers: false, + provider: { name: "okta", enabled: true }, + }); + validateSAMLResponse.mockResolvedValue({ + email: "bob@example.com", + nameID: "bob", + }); + (db.user.findUnique as any).mockResolvedValue({ + id: "user_7", + email: "bob@example.com", + name: "Bob", + authMethod: "SSO", + externalId: "bob", + }); + (db.account.upsert as any).mockResolvedValue({}); + + const res = await POST(makeReq(relayFor("ssoprovider_9"))); + + // Bug 2: looked up by the providerId carried in RelayState. + expect(db.samlConfiguration.findUnique).toHaveBeenCalledWith({ + where: { providerId: "ssoprovider_9" }, + include: { provider: true }, + }); + + // Bug 3: hands off to the real session-minting route on the public origin, + // NOT the dead /api/auth/callback/saml?token= path that hit NextAuth. + const location = res.headers.get("location")!; + expect( + location.startsWith( + "https://app.example.com/api/auth/saml/complete?token=" + ) + ).toBe(true); + expect(location).not.toContain("/api/auth/callback/saml?token="); + }); + + it("returns 404 when the provider in RelayState is unknown", async () => { + (db.samlConfiguration.findUnique as any).mockResolvedValue(null); + const res = await POST(makeReq(relayFor("nope"))); + expect(res.status).toBe(404); + }); +}); diff --git a/testplanit/app/api/auth/saml/callback/route.ts b/testplanit/app/api/auth/callback/saml/route.ts similarity index 85% rename from testplanit/app/api/auth/saml/callback/route.ts rename to testplanit/app/api/auth/callback/saml/route.ts index 2d3f078d4..15c176668 100644 --- a/testplanit/app/api/auth/saml/callback/route.ts +++ b/testplanit/app/api/auth/callback/saml/route.ts @@ -2,18 +2,26 @@ import { randomUUID } from "crypto"; import { NextRequest, NextResponse } from "next/server"; import { checkRateLimit, + consumeSamlRelayState, createTempSessionToken, + getAppBaseUrl, getSecurityHeaders, sanitizeCallbackUrl, validateSAMLTimestamp, - verifyState, } from "~/lib/auth-security"; import { NotificationService } from "~/lib/services/notificationService"; import { isEmailDomainAllowed } from "~/lib/utils/email-domain-validation"; import { db } from "~/server/db"; import { createSAMLClient, validateSAMLResponse } from "~/server/saml-provider"; -// SAML callback handler +/** + * SAML Assertion Consumer Service (ACS). + * + * The IdP POSTs the SAMLResponse here. This path matches the ACS URL embedded + * in the AuthnRequest (createSAMLClient) and shown in the admin UI. It is a + * static route, so it takes precedence over the NextAuth [...nextauth] + * catch-all for this exact path. + */ export async function POST(request: NextRequest) { try { const clientIp = @@ -46,31 +54,27 @@ export async function POST(request: NextRequest) { ); } - // Get provider and state from cookies - const providerId = request.cookies.get("saml-provider")?.value; - const storedState = request.cookies.get("saml-state")?.value; - const callbackUrl = sanitizeCallbackUrl( - request.cookies.get("saml-callback-url")?.value + // Recover the provider and destination from RelayState (the IdP echoes it + // back here; same-site cookies are not sent on this cross-site POST). The + // token is single-use and short-lived, which is what guards this endpoint. + const relay = await consumeSamlRelayState( + typeof relayState === "string" ? relayState : null ); - if (!providerId) { + if (!relay) { return NextResponse.json( - { error: "Provider information not found" }, + { error: "Invalid or expired SAML request" }, { status: 400 } ); } - // Verify state if relay state is provided - if (relayState && !verifyState(storedState, relayState as string)) { - return NextResponse.json( - { error: "Invalid state parameter" }, - { status: 400 } - ); - } + const providerId = relay.providerId; + const callbackUrl = sanitizeCallbackUrl(relay.callbackUrl); - // Fetch SAML configuration + // Fetch SAML configuration. RelayState carries the SsoProvider id, which is + // the unique foreign key on SamlConfiguration (not its own id). const samlConfig = await db.samlConfiguration.findUnique({ - where: { id: providerId }, + where: { providerId }, include: { provider: true }, }); @@ -272,19 +276,15 @@ export async function POST(request: NextRequest) { email: user.email, }); - // Create a session for the user by redirecting to NextAuth callback + // Hand off to the completion route, which verifies the token and mints the + // NextAuth session cookie before redirecting to the final destination. const response = NextResponse.redirect( new URL( - `/api/auth/callback/saml?token=${tempToken}&callbackUrl=${encodeURIComponent(callbackUrl)}`, - request.url + `/api/auth/saml/complete?token=${tempToken}&callbackUrl=${encodeURIComponent(callbackUrl)}`, + getAppBaseUrl(request) ) ); - // Clean up cookies - response.cookies.delete("saml-state"); - response.cookies.delete("saml-provider"); - response.cookies.delete("saml-callback-url"); - // Set security headers const securityHeaders = getSecurityHeaders(); Object.entries(securityHeaders).forEach(([key, value]) => { diff --git a/testplanit/app/api/auth/logout/route.ts b/testplanit/app/api/auth/logout/route.ts index 84f7c5fd4..e088e123e 100644 --- a/testplanit/app/api/auth/logout/route.ts +++ b/testplanit/app/api/auth/logout/route.ts @@ -1,4 +1,5 @@ import { NextRequest, NextResponse } from "next/server"; +import { getAppBaseUrl } from "~/lib/auth-security"; import { withAuditContext } from "~/lib/auditContextWrappers"; import { auditAuthEvent } from "~/lib/services/auditLog"; import { getServerAuthSession } from "~/server/auth"; @@ -102,9 +103,10 @@ export const POST = withAuditContext(async (request: NextRequest) => { "urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress", issuer: samlProvider.samlConfig.issuer, }, - "https://" + - request.headers.get("host") + + new URL( "/api/auth/saml/logout-callback", + getAppBaseUrl(request) + ).toString(), {} ); @@ -186,11 +188,11 @@ export const GET = withAuditContext(async (request: NextRequest) => { } // Redirect to signin page after successful logout - return NextResponse.redirect(new URL("/signin", request.url)); + return NextResponse.redirect(new URL("/signin", getAppBaseUrl(request))); } catch (error) { console.error("SAML logout callback error:", error); return NextResponse.redirect( - new URL("/signin?error=logout-failed", request.url) + new URL("/signin?error=logout-failed", getAppBaseUrl(request)) ); } }); diff --git a/testplanit/app/api/auth/saml/complete/route.test.ts b/testplanit/app/api/auth/saml/complete/route.test.ts new file mode 100644 index 000000000..35ce20748 --- /dev/null +++ b/testplanit/app/api/auth/saml/complete/route.test.ts @@ -0,0 +1,106 @@ +// @vitest-environment node +import jwt from "jsonwebtoken"; +import { decode } from "next-auth/jwt"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +vi.hoisted(() => { + process.env.NEXTAUTH_SECRET = "test-secret-key-at-least-32-chars-long"; + process.env.NEXTAUTH_URL = "https://app.example.com"; +}); + +const SECRET = "test-secret-key-at-least-32-chars-long"; + +// Capture every cookie the route sets so we can decode the session token. +const cookieJar = new Map(); +vi.mock("next/headers", () => ({ + cookies: vi.fn(async () => ({ + set: (name: string, value: string) => cookieJar.set(name, { value }), + get: (name: string) => cookieJar.get(name), + delete: (name: string) => cookieJar.delete(name), + })), +})); + +vi.mock("~/server/db", () => ({ + db: { user: { findUnique: vi.fn() } }, +})); + +import { db } from "~/server/db"; +import { GET } from "./route"; + +const user = { + id: "user_42", + email: "alice@example.com", + name: "Alice", + access: "USER", + isApi: false, + passwordChangedAt: null, + mustChangePassword: false, +}; + +function makeReq(token: string, callbackUrl = "/dashboard") { + return { + nextUrl: { searchParams: new URLSearchParams({ token, callbackUrl }) }, + headers: new Headers(), + url: "https://cint-prod-pod-xyz:3000/api/auth/saml/complete", + } as any; +} + +function tempToken() { + return jwt.sign( + { userId: user.id, provider: "saml-okta", email: user.email }, + SECRET, + { expiresIn: "5m" } + ); +} + +describe("GET /api/auth/saml/complete — post-Okta session handoff", () => { + beforeEach(() => { + cookieJar.clear(); + vi.clearAllMocks(); + (db.user.findUnique as any).mockResolvedValue(user); + }); + + afterEach(() => { + vi.unstubAllEnvs(); + }); + + it("mints a session cookie that NextAuth can decode (sub = user id)", async () => { + const res = await GET(makeReq(tempToken())); + + // Redirects to the destination on the public origin (not the pod host). + expect(res.status).toBe(307); + expect(res.headers.get("location")).toBe( + "https://app.example.com/dashboard" + ); + + // The session cookie it set must be a valid NextAuth v4 JWT — decode it the + // exact way NextAuth would on the next request. + const sessionCookie = cookieJar.get("next-auth.session-token"); + expect(sessionCookie?.value).toBeTruthy(); + + const decoded = await decode({ + token: sessionCookie!.value, + secret: SECRET, + }); + expect(decoded?.sub).toBe(user.id); + expect(decoded?.email).toBe(user.email); + // The token carries access on the first request so middleware (which only + // decodes, never runs the jwt callback) sees it without a session refresh. + expect(decoded?.access).toBe("USER"); + }); + + it("sets the __Secure- cookie name in production (matches NextAuth on https)", async () => { + vi.stubEnv("NODE_ENV", "production"); + + await GET(makeReq(tempToken())); + + expect( + cookieJar.get("__Secure-next-auth.session-token")?.value + ).toBeTruthy(); + }); + + it("rejects an invalid/expired token with 401", async () => { + const res = await GET(makeReq("not-a-valid-jwt")); + expect(res.status).toBe(401); + }); +}); diff --git a/testplanit/app/api/auth/saml/complete/route.ts b/testplanit/app/api/auth/saml/complete/route.ts index 5fc934c2b..db4bf1369 100644 --- a/testplanit/app/api/auth/saml/complete/route.ts +++ b/testplanit/app/api/auth/saml/complete/route.ts @@ -2,6 +2,7 @@ import jwt from "jsonwebtoken"; import { encode } from "next-auth/jwt"; import { cookies } from "next/headers"; import { NextRequest, NextResponse } from "next/server"; +import { getAppBaseUrl } from "~/lib/auth-security"; import { db } from "~/server/db"; // SAML completion handler - creates NextAuth session @@ -36,6 +37,10 @@ export async function GET(request: NextRequest) { id: true, email: true, name: true, + access: true, + isApi: true, + passwordChangedAt: true, + mustChangePassword: true, }, }); @@ -43,13 +48,19 @@ export async function GET(request: NextRequest) { return NextResponse.json({ error: "User not found" }, { status: 404 }); } - // Create NextAuth JWT session token + // Create NextAuth JWT session token. Seed the same fields the NextAuth `jwt` + // callback sets at sign-in so middleware (which only decodes the token, and + // never runs the callback) sees the user's access on the very first request. const sessionToken = await encode({ token: { sub: user.id, email: user.email, name: user.name, provider: tokenData.provider, + access: user.access, + isApi: user.isApi, + passwordChangedAt: user.passwordChangedAt?.toISOString() ?? null, + mustChangePassword: user.mustChangePassword ?? false, }, secret: process.env.NEXTAUTH_SECRET || "development-secret", }); @@ -76,7 +87,7 @@ export async function GET(request: NextRequest) { } // Redirect to callback URL - return NextResponse.redirect(new URL(callbackUrl, request.url)); + return NextResponse.redirect(new URL(callbackUrl, getAppBaseUrl(request))); } catch (error) { console.error("SAML completion error:", error); return NextResponse.json( diff --git a/testplanit/app/api/auth/saml/login/[id]/route.test.ts b/testplanit/app/api/auth/saml/login/[id]/route.test.ts new file mode 100644 index 000000000..3edceaf3c --- /dev/null +++ b/testplanit/app/api/auth/saml/login/[id]/route.test.ts @@ -0,0 +1,50 @@ +// @vitest-environment node +import { describe, expect, it, vi } from "vitest"; + +vi.hoisted(() => { + process.env.NEXTAUTH_URL = "https://app.example.com"; +}); + +vi.mock("~/lib/valkey", () => ({ default: null })); + +import { GET } from "./route"; + +function makeReq(callbackUrl?: string) { + const sp = new URLSearchParams(); + if (callbackUrl) sp.set("callbackUrl", callbackUrl); + return { + nextUrl: { searchParams: sp }, + headers: new Headers(), + // In production (standalone server in k8s) request.url carries the internal + // pod host — the original bug. The handler must not propagate it. + url: "https://cint-prod-pod-xyz:3000/api/auth/saml/login/prov_123", + } as any; +} + +describe("GET /api/auth/saml/login/[id] — Bug 1: must not use the request host", () => { + it("redirects to the public NEXTAUTH_URL origin, not the pod host", async () => { + const res = await GET(makeReq("/projects"), { + params: Promise.resolve({ id: "prov_123" }), + }); + + const location = res.headers.get("location")!; + const url = new URL(location); + + expect(url.origin).toBe("https://app.example.com"); + expect(url.pathname).toBe("/api/auth/saml"); + expect(url.searchParams.get("provider")).toBe("prov_123"); + expect(url.searchParams.get("callbackUrl")).toBe("/projects"); + + // Explicit guards against the reported regression. + expect(location).not.toContain("cint-prod-pod-xyz"); + expect(location).not.toContain(":3000"); + }); + + it("defaults callbackUrl to / when absent", async () => { + const res = await GET(makeReq(), { + params: Promise.resolve({ id: "p1" }), + }); + const url = new URL(res.headers.get("location")!); + expect(url.searchParams.get("callbackUrl")).toBe("/"); + }); +}); diff --git a/testplanit/app/api/auth/saml/login/[id]/route.ts b/testplanit/app/api/auth/saml/login/[id]/route.ts index d8bdcfebb..d6671f602 100644 --- a/testplanit/app/api/auth/saml/login/[id]/route.ts +++ b/testplanit/app/api/auth/saml/login/[id]/route.ts @@ -1,4 +1,5 @@ import { NextRequest, NextResponse } from "next/server"; +import { getAppBaseUrl } from "~/lib/auth-security"; /** * GET /api/auth/saml/login/[id] @@ -16,7 +17,7 @@ export async function GET( const callbackUrl = searchParams.get("callbackUrl") || "/"; // Redirect to the SAML initiation handler with the provider ID as a query param - const samlUrl = new URL("/api/auth/saml", request.url); + const samlUrl = new URL("/api/auth/saml", getAppBaseUrl(request)); samlUrl.searchParams.set("provider", id); samlUrl.searchParams.set("callbackUrl", callbackUrl); diff --git a/testplanit/app/api/auth/saml/logout-callback/route.ts b/testplanit/app/api/auth/saml/logout-callback/route.ts index b3e83db2a..701bf0f9e 100644 --- a/testplanit/app/api/auth/saml/logout-callback/route.ts +++ b/testplanit/app/api/auth/saml/logout-callback/route.ts @@ -1,4 +1,5 @@ import { NextRequest, NextResponse } from "next/server"; +import { getAppBaseUrl } from "~/lib/auth-security"; import { withAuditContext } from "~/lib/auditContextWrappers"; import { auditAuthEvent } from "~/lib/services/auditLog"; @@ -26,7 +27,7 @@ export const GET = withAuditContext(async (request: NextRequest) => { // Clear any remaining SAML cookies const response = NextResponse.redirect( - new URL("/signin?logout=success", request.url) + new URL("/signin?logout=success", getAppBaseUrl(request)) ); const cookieOptions = { @@ -45,7 +46,7 @@ export const GET = withAuditContext(async (request: NextRequest) => { } catch (error) { console.error("SAML logout callback error:", error); return NextResponse.redirect( - new URL("/signin?error=logout-callback-failed", request.url) + new URL("/signin?error=logout-callback-failed", getAppBaseUrl(request)) ); } }); diff --git a/testplanit/app/api/auth/saml/route.test.ts b/testplanit/app/api/auth/saml/route.test.ts new file mode 100644 index 000000000..efad6ccb2 --- /dev/null +++ b/testplanit/app/api/auth/saml/route.test.ts @@ -0,0 +1,84 @@ +// @vitest-environment node +import { beforeEach, describe, expect, it, vi } from "vitest"; + +vi.hoisted(() => { + process.env.NEXTAUTH_SECRET = "test-secret-key-at-least-32-chars-long"; + process.env.NEXTAUTH_URL = "https://app.example.com"; +}); + +// No Valkey in unit tests → RelayState falls back to a signed token. +vi.mock("~/lib/valkey", () => ({ default: null })); + +vi.mock("~/server/db", () => ({ + db: { samlConfiguration: { findUnique: vi.fn() } }, +})); + +const getAuthorizeUrlAsync = vi.fn(); +vi.mock("~/server/saml-provider", () => ({ + createSAMLClient: vi.fn(async () => ({ getAuthorizeUrlAsync })), +})); + +import { db } from "~/server/db"; +import { GET } from "./route"; + +function makeReq(provider: string | null, callbackUrl = "/dash") { + const sp = new URLSearchParams(); + if (provider) sp.set("provider", provider); + sp.set("callbackUrl", callbackUrl); + return { + nextUrl: { searchParams: sp }, + headers: new Headers(), + url: "https://cint-prod-pod:3000/api/auth/saml", + } as any; +} + +const enabledConfig = { + id: "samlcfg_1", + entryPoint: "https://okta.example.com/sso", + cert: "CERT", + issuer: "issuer", + provider: { name: "okta", enabled: true }, +}; + +describe("GET /api/auth/saml — Bug 2: lookup by providerId; RelayState wiring", () => { + beforeEach(() => { + vi.clearAllMocks(); + getAuthorizeUrlAsync.mockResolvedValue( + "https://okta.example.com/sso?SAMLRequest=abc&RelayState=xyz" + ); + }); + + it("looks up SamlConfiguration by providerId (the SsoProvider id), not its own id", async () => { + (db.samlConfiguration.findUnique as any).mockResolvedValue(enabledConfig); + + await GET(makeReq("ssoprovider_123")); + + expect(db.samlConfiguration.findUnique).toHaveBeenCalledWith({ + where: { providerId: "ssoprovider_123" }, + include: { provider: true }, + }); + }); + + it("returns 404 when no config exists for the provider", async () => { + (db.samlConfiguration.findUnique as any).mockResolvedValue(null); + const res = await GET(makeReq("missing")); + expect(res.status).toBe(404); + }); + + it("passes a non-empty RelayState to the AuthnRequest and redirects to the IdP", async () => { + (db.samlConfiguration.findUnique as any).mockResolvedValue(enabledConfig); + + const res = await GET(makeReq("ssoprovider_123")); + + const [relayState, host] = getAuthorizeUrlAsync.mock.calls[0]; + expect(typeof relayState).toBe("string"); + expect(relayState.length).toBeGreaterThan(0); + // Host is derived from NEXTAUTH_URL, never the pod. + expect(host).toBe("app.example.com"); + + expect(res.status).toBe(307); + expect(res.headers.get("location")).toBe( + "https://okta.example.com/sso?SAMLRequest=abc&RelayState=xyz" + ); + }); +}); diff --git a/testplanit/app/api/auth/saml/route.ts b/testplanit/app/api/auth/saml/route.ts index 062fe574c..a53efd7f2 100644 --- a/testplanit/app/api/auth/saml/route.ts +++ b/testplanit/app/api/auth/saml/route.ts @@ -1,8 +1,8 @@ import { NextRequest, NextResponse } from "next/server"; import { checkRateLimit, - generateSecureState, - getSecureCookieOptions, + createSamlRelayState, + getAppBaseUrl, sanitizeCallbackUrl, } from "~/lib/auth-security"; import { db } from "~/server/db"; @@ -39,9 +39,10 @@ export async function GET(request: NextRequest) { ); } - // Fetch SAML configuration from database + // Fetch SAML configuration from database. The UI passes the SsoProvider id, + // which is the unique foreign key on SamlConfiguration (not its own id). const samlConfig = await db.samlConfiguration.findUnique({ - where: { id: providerId }, + where: { providerId }, include: { provider: true }, }); @@ -60,35 +61,19 @@ export async function GET(request: NextRequest) { issuer: samlConfig.issuer, }); + // Carry the provider and post-login destination in RelayState. The IdP + // echoes this back on its cross-site POST to the ACS, where same-site + // cookies are not sent — so a cookie cannot be used to recover them. + const relayState = await createSamlRelayState({ providerId, callbackUrl }); + // Generate SAML auth request const authUrl = await samlClient.getAuthorizeUrlAsync( - "", - request.headers.get("host") || "", + relayState, + new URL(getAppBaseUrl(request)).host, {} ); - // Generate secure state parameter for CSRF protection - const state = generateSecureState(); - const response = NextResponse.redirect(authUrl); - const cookieOptions = getSecureCookieOptions(); - - // Set cookies with secure options - response.cookies.set("saml-state", state, { - ...cookieOptions, - maxAge: 60 * 15, // 15 minutes - }); - - response.cookies.set("saml-provider", providerId, { - ...cookieOptions, - maxAge: 60 * 15, // 15 minutes - }); - - response.cookies.set("saml-callback-url", callbackUrl, { - ...cookieOptions, - maxAge: 60 * 15, // 15 minutes - }); - - return response; + return NextResponse.redirect(authUrl); } catch (error) { console.error("SAML login error:", error); return NextResponse.json( diff --git a/testplanit/app/api/auth/send-magic-link/route.ts b/testplanit/app/api/auth/send-magic-link/route.ts index 89640f2fb..8bde5e551 100644 --- a/testplanit/app/api/auth/send-magic-link/route.ts +++ b/testplanit/app/api/auth/send-magic-link/route.ts @@ -1,5 +1,6 @@ import crypto from "crypto"; import { NextRequest, NextResponse } from "next/server"; +import { getAppBaseUrl } from "~/lib/auth-security"; import { withAuditContext } from "~/lib/auditContextWrappers"; import { prisma } from "~/lib/prisma"; import { auditAuthEvent } from "~/lib/services/auditLog"; @@ -68,13 +69,7 @@ export const POST = withAuditContext(async (req: NextRequest) => { }); // Build the magic link URL - const protocol = process.env.NEXTAUTH_URL?.startsWith("https") - ? "https" - : "http"; - const host = - req.headers.get("host") || - process.env.NEXTAUTH_URL?.replace(/^https?:\/\//, ""); - const baseUrl = `${protocol}://${host}`; + const baseUrl = getAppBaseUrl(req); // Ensure callbackUrl has a trailing slash for proper routing let finalCallbackUrl = callbackUrl.startsWith("http") diff --git a/testplanit/lib/auth-security.test.ts b/testplanit/lib/auth-security.test.ts index 424c59eb3..84db44a99 100644 --- a/testplanit/lib/auth-security.test.ts +++ b/testplanit/lib/auth-security.test.ts @@ -13,6 +13,10 @@ import { verifyTempSessionToken, } from "./auth-security"; +// Keep the static import above from opening a real Valkey connection; the relay +// tests below inject their own client per-case via vi.doMock + resetModules. +vi.mock("./valkey", () => ({ default: null })); + // Mock environment variables const originalEnv = process.env; @@ -332,6 +336,79 @@ describe("validateSAMLTimestamp", () => { }); }); +describe("createSamlRelayState / consumeSamlRelayState", () => { + const data = { providerId: "ssoprovider_123", callbackUrl: "/dashboard" }; + const SECRET = "test-secret-key-at-least-32-chars-long"; + + it("Valkey-backed: round-trips, stays within 80 bytes, and is single-use", async () => { + const store = new Map(); + const fakeRedis = { + set: vi.fn(async (k: string, v: string) => { + store.set(k, v); + return "OK"; + }), + get: vi.fn(async (k: string) => store.get(k) ?? null), + del: vi.fn(async (k: string) => (store.delete(k) ? 1 : 0)), + }; + + vi.resetModules(); + process.env.NEXTAUTH_SECRET = SECRET; + vi.doMock("./valkey", () => ({ default: fakeRedis })); + const { createSamlRelayState, consumeSamlRelayState } = + await import("./auth-security"); + + const relay = await createSamlRelayState(data); + expect(typeof relay).toBe("string"); + // SAML 2.0 binding guidance: RelayState should not exceed 80 bytes. + expect(relay.length).toBeLessThanOrEqual(80); + expect(fakeRedis.set).toHaveBeenCalledTimes(1); + + const first = await consumeSamlRelayState(relay); + expect(first).toEqual(data); + + // Single-use: the second consume finds nothing. + const second = await consumeSamlRelayState(relay); + expect(second).toBeNull(); + + vi.doUnmock("./valkey"); + }); + + it("signed fallback when Valkey is absent", async () => { + vi.resetModules(); + process.env.NEXTAUTH_SECRET = SECRET; + vi.doMock("./valkey", () => ({ default: null })); + const { createSamlRelayState, consumeSamlRelayState } = + await import("./auth-security"); + + const relay = await createSamlRelayState(data); + expect(relay.split(".")).toHaveLength(3); // a JWT + + const decoded = await consumeSamlRelayState(relay); + expect(decoded?.providerId).toBe(data.providerId); + expect(decoded?.callbackUrl).toBe(data.callbackUrl); + + vi.doUnmock("./valkey"); + }); + + it("returns null for empty, malformed, or tampered relay state", async () => { + vi.resetModules(); + process.env.NEXTAUTH_SECRET = SECRET; + vi.doMock("./valkey", () => ({ default: null })); + const { createSamlRelayState, consumeSamlRelayState } = + await import("./auth-security"); + + expect(await consumeSamlRelayState(null)).toBeNull(); + expect(await consumeSamlRelayState(undefined)).toBeNull(); + expect(await consumeSamlRelayState("not-a-jwt")).toBeNull(); + + const relay = await createSamlRelayState(data); + const tampered = relay.slice(0, -4) + "xxxx"; + expect(await consumeSamlRelayState(tampered)).toBeNull(); + + vi.doUnmock("./valkey"); + }); +}); + describe("getSecureCookieOptions", () => { it("should return httpOnly as true", () => { const options = getSecureCookieOptions(); diff --git a/testplanit/lib/auth-security.ts b/testplanit/lib/auth-security.ts index 316725f88..75f467641 100644 --- a/testplanit/lib/auth-security.ts +++ b/testplanit/lib/auth-security.ts @@ -1,5 +1,6 @@ import { createHash, randomBytes } from "crypto"; import jwt from "jsonwebtoken"; +import valkeyConnection from "./valkey"; // Generate secure state parameter for OAuth/SAML flows export function generateSecureState(): string { @@ -49,6 +50,32 @@ export function sanitizeCallbackUrl(url: string | null | undefined): string { return "/"; } +/** + * Resolve the externally-reachable base URL for building absolute redirects. + * + * Behind a reverse proxy or k3s ingress the incoming request host is the + * internal pod name (e.g. my-pod-abc123:3000), so absolute URLs built from + * `request.url` send the browser to an unreachable address. Prefer the + * configured public origin, then proxy-forwarded headers, then the request. + */ +export function getAppBaseUrl(request?: Request): string { + if (process.env.NEXTAUTH_URL) { + return process.env.NEXTAUTH_URL.replace(/\/$/, ""); + } + + if (request) { + const forwardedHost = request.headers.get("x-forwarded-host"); + if (forwardedHost) { + const forwardedProto = + request.headers.get("x-forwarded-proto") ?? "https"; + return `${forwardedProto}://${forwardedHost}`; + } + return new URL(request.url).origin; + } + + return "http://localhost:3000"; +} + // JWT token configuration for temporary session data const JWT_SECRET = process.env.NEXTAUTH_SECRET || ""; const JWT_EXPIRY = "5m"; // 5 minutes for temporary tokens @@ -71,6 +98,78 @@ export function verifyTempSessionToken(token: string): TempSessionData | null { } } +// SAML relay state — carries the provider id and post-login destination across +// the IdP round-trip. The IdP echoes RelayState back on its cross-site POST to +// the ACS, where same-site cookies are not sent, so this (not a cookie) is how +// the callback recovers which provider and destination the login belongs to. +export interface SamlRelayData { + providerId: string; + callbackUrl: string; +} + +const SAML_RELAY_PREFIX = "saml:relay:"; +const SAML_RELAY_TTL_SECONDS = 900; // 15 minutes + +/** + * Mint a RelayState value for a SAML AuthnRequest. + * + * When Valkey is available the payload is stored server-side under a short, + * single-use id — this keeps RelayState within SAML's 80-byte guidance and + * works across pods. Otherwise it falls back to a signed, self-contained token + * so SAML still works without Valkey (e.g. local dev / minimal self-host). + */ +export async function createSamlRelayState( + data: SamlRelayData +): Promise { + if (valkeyConnection) { + const relayId = randomBytes(24).toString("hex"); + await valkeyConnection.set( + `${SAML_RELAY_PREFIX}${relayId}`, + JSON.stringify(data), + "EX", + SAML_RELAY_TTL_SECONDS + ); + return relayId; + } + return jwt.sign(data, JWT_SECRET, { expiresIn: "15m" }); +} + +/** + * Resolve and consume a RelayState value returned by the IdP. Valkey-backed + * ids are single-use (deleted on read); signed fallback tokens are validated by + * signature and expiry. Returns null if the token is unknown, expired, or + * malformed. + */ +export async function consumeSamlRelayState( + relayState: string | null | undefined +): Promise { + if (!relayState) return null; + + if (valkeyConnection) { + const key = `${SAML_RELAY_PREFIX}${relayState}`; + const raw = await valkeyConnection.get(key); + if (raw) { + await valkeyConnection.del(key); + try { + const parsed = JSON.parse(raw) as SamlRelayData; + return parsed?.providerId ? parsed : null; + } catch { + return null; + } + } + // Fall through: may be a signed fallback token issued while Valkey was down. + } + + try { + const decoded = jwt.verify(relayState, JWT_SECRET) as SamlRelayData; + return decoded?.providerId + ? { providerId: decoded.providerId, callbackUrl: decoded.callbackUrl } + : null; + } catch { + return null; + } +} + // Rate limiting configuration export interface RateLimitConfig { windowMs: number;