From 4a2d51195c09e5fbc19817f70d589f192a146e6f Mon Sep 17 00:00:00 2001 From: Benjamin Wallberg Date: Thu, 14 Mar 2024 02:45:33 +0100 Subject: [PATCH] fix(core): cannot parse action at /session (#10094) * fix(core): cannot parse action at /session fix: support apps hosted on subpaths * refactor(core): use const instead of let for detected host & protocol * fix(core): warn if basePath & AUTH_URL path are both provided * refactor: update logger --------- Co-authored-by: Nico Domino Co-authored-by: Thang Vu --- packages/core/src/lib/utils/env.ts | 36 +++++++++++--- packages/core/src/lib/utils/logger.ts | 8 ++- packages/core/test/env.test.ts | 71 ++++++++++++++++++++++++--- 3 files changed, 99 insertions(+), 16 deletions(-) diff --git a/packages/core/src/lib/utils/env.ts b/packages/core/src/lib/utils/env.ts index ab41433f8e..858f75c874 100644 --- a/packages/core/src/lib/utils/env.ts +++ b/packages/core/src/lib/utils/env.ts @@ -1,4 +1,5 @@ import type { AuthAction, AuthConfig } from "../../types.js" +import { logger } from "./logger.js" /** Set default env variables on the config object */ export function setEnvDefaults(envObject: any, config: AuthConfig) { @@ -51,13 +52,34 @@ export function createActionURL( envObject: any, basePath?: string ): URL { - let url = envObject.AUTH_URL ?? envObject.NEXTAUTH_URL - if (!url) { - const host = headers.get("x-forwarded-host") ?? headers.get("host") - if (!host) throw new TypeError("Missing host") - const proto = headers.get("x-forwarded-proto") ?? protocol - url = `${proto === "http" ? "http" : "https"}://${host}${basePath}` + let envUrl = envObject.AUTH_URL ?? envObject.NEXTAUTH_URL + + let url: URL + if (envUrl) { + url = new URL(envUrl) + if (basePath && basePath !== "/" && url.pathname !== "/") { + logger.warn( + url.pathname === basePath + ? "env-url-basepath-redundant" + : "env-url-basepath-mismatch" + ) + url.pathname = "/" + } + } else { + const detectedHost = headers.get("x-forwarded-host") ?? headers.get("host") + const detectedProtocol = + headers.get("x-forwarded-proto") ?? protocol ?? "https" + + url = new URL(`${detectedProtocol}://${detectedHost}`) } - return new URL(`${url.replace(/\/$/, "")}/${action}`) + // remove trailing slash + const sanitizedUrl = url.toString().replace(/\/$/, "") + + if (basePath) { + // remove leading and trailing slash + const sanitizedBasePath = basePath?.replace(/(^\/|\/$)/g, "") ?? "" + return new URL(`${sanitizedUrl}/${sanitizedBasePath}/${action}`) + } + return new URL(`${sanitizedUrl}/${action}`) } diff --git a/packages/core/src/lib/utils/logger.ts b/packages/core/src/lib/utils/logger.ts index 9a0b10c12a..133bd27e30 100644 --- a/packages/core/src/lib/utils/logger.ts +++ b/packages/core/src/lib/utils/logger.ts @@ -1,6 +1,12 @@ import { AuthError } from "../../errors.js" -export type WarningCode = "debug-enabled" | "csrf-disabled" | "experimental-webauthn" +export type WarningCode = + | "debug-enabled" + | "csrf-disabled" + | "experimental-webauthn" + | "env-url-basepath-redundant" + | "env-url-basepath-mismatch" + /** * Override any of the methods, and the rest will use the default logger. diff --git a/packages/core/test/env.test.ts b/packages/core/test/env.test.ts index 22623fa712..0e7b980b25 100644 --- a/packages/core/test/env.test.ts +++ b/packages/core/test/env.test.ts @@ -1,4 +1,4 @@ -import { beforeEach, describe, expect, it } from "vitest" +import { afterAll, afterEach, beforeEach, describe, expect, it, vi } from "vitest" import { AuthConfig } from "../src/index.js" import { setEnvDefaults, createActionURL } from "../src/lib/utils/env.js" @@ -93,6 +93,12 @@ describe("config is inferred from environment variables", () => { }) describe("createActionURL", () => { +const consoleWarnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + + afterEach(() => { + consoleWarnSpy.mockClear() + }) + it.each([ { args: { @@ -144,17 +150,66 @@ describe("createActionURL", () => { args: { action: "signout", protocol: undefined, - headers: new Headers({ - "x-forwarded-host": "example.com", - "x-forwarded-proto": "https", - }), - env: { AUTH_URL: "https://env.com/api/auth/" }, - basePath: "/auth", + headers: new Headers({}), + env: { AUTH_URL: "http://localhost:3000" }, + basePath: "/api/auth", + }, + expected: "http://localhost:3000/api/auth/signout", + }, + { + args: { + action: "signout", + protocol: undefined, + headers: new Headers({}), + env: { AUTH_URL: "https://sub.domain.env.com" }, + basePath: "/api/auth", }, - expected: "https://env.com/api/auth/signout", + expected: "https://sub.domain.env.com/api/auth/signout", + }, + { + args: { + action: "signout", + protocol: undefined, + headers: new Headers({}), + env: { AUTH_URL: "https://sub.domain.env.com/api/auth" }, + basePath: undefined, + }, + expected: "https://sub.domain.env.com/api/auth/signout", }, ])("%j", ({ args, expected }) => { // @ts-expect-error expect(createActionURL(...Object.values(args)).toString()).toBe(expected) + expect(consoleWarnSpy).not.toHaveBeenCalled() }) + + it.each([ + { + args: { + action: "signout", + protocol: undefined, + headers: new Headers({}), + env: { AUTH_URL: "http://localhost:3000/my-app/api/auth/" }, + basePath: "/my-app/api/auth", + }, + expected: "http://localhost:3000/my-app/api/auth/signout", + }, + { + args: { + action: "signout", + protocol: undefined, + headers: new Headers({}), + env: { AUTH_URL: "https://sub.domain.env.com/my-app" }, + basePath: "/api/auth", + }, + expected: "https://sub.domain.env.com/api/auth/signout", + }, + ])("Duplicate path configurations: %j", ({ args, expected }) => { + // @ts-expect-error + expect(createActionURL(...Object.values(args)).toString()).toBe(expected) + expect(consoleWarnSpy).toHaveBeenCalled() + }) + + afterAll(() => { + consoleWarnSpy.mockRestore(); + }) })