diff --git a/frontend/.gitignore b/frontend/.gitignore index 41be2a3..179a964 100644 --- a/frontend/.gitignore +++ b/frontend/.gitignore @@ -9,6 +9,7 @@ !.yarn/plugins !.yarn/releases !.yarn/versions +package-lock.json # testing /coverage diff --git a/frontend/lib/__tests__/url-validation.test.ts b/frontend/lib/__tests__/url-validation.test.ts new file mode 100644 index 0000000..d53e7cf --- /dev/null +++ b/frontend/lib/__tests__/url-validation.test.ts @@ -0,0 +1,80 @@ +import { describe, it, expect } from "vitest" +import { isSafeRedirectUrl } from "../url-validation" + +describe("isSafeRedirectUrl", () => { + describe("should return true for safe relative URLs", () => { + it("accepts simple paths", () => { + expect(isSafeRedirectUrl("/dashboard")).toBe(true) + expect(isSafeRedirectUrl("/configure")).toBe(true) + expect(isSafeRedirectUrl("/settings")).toBe(true) + }) + + it("accepts paths with query parameters", () => { + expect(isSafeRedirectUrl("/dashboard?tab=active")).toBe(true) + expect(isSafeRedirectUrl("/configure?step=2")).toBe(true) + }) + + it("accepts paths with hash fragments", () => { + expect(isSafeRedirectUrl("/dashboard#section")).toBe(true) + expect(isSafeRedirectUrl("/page#top")).toBe(true) + }) + + it("accepts nested paths", () => { + expect(isSafeRedirectUrl("/dashboard/deployments/123")).toBe(true) + expect(isSafeRedirectUrl("/a/b/c/d/e")).toBe(true) + }) + }) + + describe("should return false for unsafe URLs", () => { + it("rejects absolute URLs with http protocol", () => { + expect(isSafeRedirectUrl("http://evil.com")).toBe(false) + expect(isSafeRedirectUrl("http://example.com/path")).toBe(false) + }) + + it("rejects absolute URLs with https protocol", () => { + expect(isSafeRedirectUrl("https://evil.com")).toBe(false) + expect(isSafeRedirectUrl("https://example.com/path")).toBe(false) + }) + + it("rejects protocol-relative URLs", () => { + expect(isSafeRedirectUrl("//evil.com")).toBe(false) + expect(isSafeRedirectUrl("//example.com/path")).toBe(false) + }) + + it("rejects javascript: URLs", () => { + expect(isSafeRedirectUrl("javascript:alert(1)")).toBe(false) + expect(isSafeRedirectUrl("javascript:void(0)")).toBe(false) + }) + + it("rejects data: URLs", () => { + expect(isSafeRedirectUrl("data:text/html,")).toBe(false) + }) + + it("rejects URLs not starting with /", () => { + expect(isSafeRedirectUrl("dashboard")).toBe(false) + expect(isSafeRedirectUrl("./dashboard")).toBe(false) + expect(isSafeRedirectUrl("../dashboard")).toBe(false) + }) + + it("rejects other malicious protocols", () => { + expect(isSafeRedirectUrl("file:///etc/passwd")).toBe(false) + expect(isSafeRedirectUrl("ftp://server.com/file")).toBe(false) + }) + }) + + describe("edge cases", () => { + it("handles empty strings", () => { + expect(isSafeRedirectUrl("")).toBe(false) + }) + + it("handles URLs with encoded characters", () => { + expect(isSafeRedirectUrl("/path?url=http%3A%2F%2Fevil.com")).toBe(true) // encoded as query param is OK + expect(isSafeRedirectUrl("/path%2F..%2F..%2Fetc")).toBe(true) // path traversal encoded is still a relative path + }) + + it("rejects URLs trying to bypass with protocols in path", () => { + // These should still be rejected as they contain :// + expect(isSafeRedirectUrl("/redirect?url=https://evil.com")).toBe(false) + }) + }) +}) diff --git a/frontend/lib/url-validation.ts b/frontend/lib/url-validation.ts new file mode 100644 index 0000000..50ad1d1 --- /dev/null +++ b/frontend/lib/url-validation.ts @@ -0,0 +1,41 @@ +/** + * Validates if a URL is safe for redirecting within the application. + * + * A URL is considered safe if it: + * 1. Is a relative path starting with "/" + * 2. Does not contain protocol markers (like "://") which indicate absolute URLs + * 3. Does not start with "//" (protocol-relative URLs) + * + * This prevents open redirect vulnerabilities where attackers could redirect + * users to malicious external sites. + * + * @param url - The URL string to validate + * @returns true if the URL is safe for internal redirects, false otherwise + * + * @example + * ```ts + * isSafeRedirectUrl("/dashboard") // true + * isSafeRedirectUrl("/dashboard?tab=active") // true + * isSafeRedirectUrl("https://evil.com") // false + * isSafeRedirectUrl("//evil.com") // false + * isSafeRedirectUrl("javascript:alert(1)") // false + * ``` + */ +export function isSafeRedirectUrl(url: string): boolean { + // Must start with "/" to be a relative path + if (!url.startsWith("/")) { + return false + } + + // Must not start with "//" (protocol-relative URL) + if (url.startsWith("//")) { + return false + } + + // Must not contain "://" which indicates an absolute URL with protocol + if (url.includes("://")) { + return false + } + + return true +} diff --git a/frontend/middleware.ts b/frontend/middleware.ts index 59d513c..7780f8d 100644 --- a/frontend/middleware.ts +++ b/frontend/middleware.ts @@ -1,5 +1,6 @@ import { auth } from "@/auth" import { NextResponse } from "next/server" +import { isSafeRedirectUrl } from "@/lib/url-validation" /** * Middleware to protect routes requiring authentication @@ -38,6 +39,13 @@ export default auth((req) => { // Redirect to intended destination if already signed in and visiting sign-in page if (pathname === "/signin" && isAuthenticated) { const callbackUrl = req.nextUrl.searchParams.get("callbackUrl") || "/dashboard" + + // Validate callbackUrl to prevent open redirect vulnerability + if (!isSafeRedirectUrl(callbackUrl)) { + console.warn(`[Middleware] Rejected unsafe callbackUrl: ${callbackUrl}`) + return NextResponse.redirect(new URL("/dashboard", req.url)) + } + console.log(`[Middleware] Authenticated user on signin page, redirecting to ${callbackUrl}`) return NextResponse.redirect(new URL(callbackUrl, req.url)) } diff --git a/frontend/package.json b/frontend/package.json index 6aa528a..b648e1c 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -49,8 +49,9 @@ "@vitejs/plugin-react": "^5.1.2", "eslint": "^9", "eslint-config-next": "16.0.10", + "jsdom": "^27.4.0", "tailwindcss": "^4", "typescript": "^5", "vitest": "^4.0.16" } -} \ No newline at end of file +}