Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Avoid excessive Env URL and basePath redundant Warning #11636

Merged
merged 20 commits into from
Aug 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 23 additions & 9 deletions packages/core/src/lib/utils/env.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,27 @@ import type { AuthAction } from "../../types.js"
import type { AuthConfig } from "../../index.js"
import { setLogger } from "./logger.js"

/** Set default env variables on the config object */
export function setEnvDefaults(envObject: any, config: AuthConfig) {
/**
* Set default env variables on the config object
* @param suppressWarnings intended for framework authors.
*/
export function setEnvDefaults(
envObject: any,
config: AuthConfig,
suppressBasePathWarning = false
) {
try {
const url = envObject.AUTH_URL
if (url && !config.basePath) config.basePath = new URL(url).pathname
if (url) {
if (config.basePath) {
if (!suppressBasePathWarning) {
const logger = setLogger(config)
logger.warn("env-url-basepath-redundant")
}
} else {
config.basePath = new URL(url).pathname
}
}
} catch {
} finally {
config.basePath ??= `/auth`
Expand Down Expand Up @@ -53,19 +69,17 @@ export function createActionURL(
envObject: any,
config: Pick<AuthConfig, "basePath" | "logger">
): URL {
const logger = setLogger(config)
const basePath = config?.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"
)
if (url.pathname !== basePath) {
const logger = setLogger(config)
logger.warn("env-url-basepath-mismatch")
}
url.pathname = "/"
}
} else {
Expand Down
92 changes: 61 additions & 31 deletions packages/core/test/env.test.ts
Original file line number Diff line number Diff line change
@@ -1,29 +1,25 @@
import {
afterAll,
afterEach,
beforeEach,
describe,
expect,
it,
vi,
} from "vitest"
import { beforeEach, describe, expect, it, vi } from "vitest"

import { AuthConfig } from "../src/index.js"
import { setEnvDefaults, createActionURL } from "../src/lib/utils/env.js"
import Auth0 from "../src/providers/auth0.js"
import Resend from "../src/providers/resend.js"

const testConfig: AuthConfig = {
providers: [Auth0, Resend({})],
}
const logger = { warn: vi.fn() }

let authConfig: AuthConfig
describe("config is inferred from environment variables", () => {
const testConfig: AuthConfig = {
providers: [Auth0, Resend({})],
logger,
}

beforeEach(() => {
authConfig = { ...testConfig } // clone
})
let authConfig: AuthConfig

beforeEach(() => {
authConfig = { ...testConfig } // clone
vi.resetAllMocks()
})

describe("config is inferred from environment variables", () => {
it("providers (client id, client secret, issuer, api key)", () => {
const env = {
AUTH_AUTH0_ID: "asdf",
Expand All @@ -41,37 +37,43 @@ describe("config is inferred from environment variables", () => {
expect(p1.issuer).toBe(env.AUTH_AUTH0_ISSUER)
// @ts-expect-error
expect(p2.apiKey).toBe(env.AUTH_RESEND_KEY)
expect(logger.warn).not.toHaveBeenCalled()
})

it("AUTH_SECRET", () => {
const env = { AUTH_SECRET: "secret" }
setEnvDefaults(env, authConfig)
expect(authConfig.secret?.[0]).toBe(env.AUTH_SECRET)
expect(logger.warn).not.toHaveBeenCalled()
})

it("AUTH_SECRET, prefer config", () => {
const env = { AUTH_SECRET: "0", AUTH_SECRET_1: "1" }
setEnvDefaults(env, authConfig)
expect(authConfig.secret).toEqual(["1", "0"])
expect(logger.warn).not.toHaveBeenCalled()
})

it("AUTH_SECRET, prefer config", () => {
const env = { AUTH_SECRET: "new" }
authConfig.secret = ["old"]
setEnvDefaults(env, authConfig)
expect(authConfig.secret).toEqual(["old"])
expect(logger.warn).not.toHaveBeenCalled()
})

it("AUTH_REDIRECT_PROXY_URL", () => {
const env = { AUTH_REDIRECT_PROXY_URL: "http://example.com" }
setEnvDefaults(env, authConfig)
expect(authConfig.redirectProxyUrl).toBe(env.AUTH_REDIRECT_PROXY_URL)
expect(logger.warn).not.toHaveBeenCalled()
})

it("AUTH_URL", () => {
const env = { AUTH_URL: "http://n/api/auth" }
setEnvDefaults(env, authConfig)
expect(authConfig.basePath).toBe("/api/auth")
expect(logger.warn).not.toHaveBeenCalled()
})

it("AUTH_URL + prefer config", () => {
Expand All @@ -80,12 +82,23 @@ describe("config is inferred from environment variables", () => {
authConfig.basePath = fromConfig
setEnvDefaults(env, authConfig)
expect(authConfig.basePath).toBe(fromConfig)
expect(logger.warn).toHaveBeenCalledWith("env-url-basepath-redundant")
})

it("AUTH_URL + prefer config but suppress base path waring", () => {
const env = { AUTH_URL: "http://n/api/auth" }
const fromConfig = "/basepath-from-config"
authConfig.basePath = fromConfig
setEnvDefaults(env, authConfig, true)
expect(authConfig.basePath).toBe(fromConfig)
expect(logger.warn).not.toHaveBeenCalled()
})

it("AUTH_URL, but invalid value", () => {
const env = { AUTH_URL: "secret" }
setEnvDefaults(env, authConfig)
expect(authConfig.basePath).toBe("/auth")
expect(logger.warn).not.toHaveBeenCalled()
})

it.each([
Expand All @@ -97,14 +110,13 @@ describe("config is inferred from environment variables", () => {
])(`%j`, (env, expected) => {
setEnvDefaults(env, authConfig)
expect(authConfig).toMatchObject(expected)
expect(logger.warn).not.toHaveBeenCalled()
})
})

describe("createActionURL", () => {
const consoleWarnSpy = vi.spyOn(console, "warn").mockImplementation(() => {})

afterEach(() => {
consoleWarnSpy.mockClear()
beforeEach(() => {
vi.resetAllMocks()
})

it.each([
Expand Down Expand Up @@ -234,10 +246,23 @@ describe("createActionURL", () => {
},
expected: "https://sub.domain.env.com/api/auth/signout",
},
{
args: {
action: "signout",
protocol: undefined,
headers: new Headers({}),
env: { AUTH_URL: "http://localhost:3000/my-app/api/auth" },
config: { basePath: "/my-app/api/auth" },
},
expected: "http://localhost:3000/my-app/api/auth/signout",
},
])("%j", ({ args, expected }) => {
const argsWithLogger = { ...args, config: { ...args.config, logger } }
// @ts-expect-error
expect(createActionURL(...Object.values(args)).toString()).toBe(expected)
expect(consoleWarnSpy).not.toHaveBeenCalled()
expect(createActionURL(...Object.values(argsWithLogger)).toString()).toBe(
expected
)
expect(logger.warn).not.toHaveBeenCalled()
})

it.each([
Expand All @@ -249,7 +274,10 @@ describe("createActionURL", () => {
env: { AUTH_URL: "http://localhost:3000/my-app/api/auth/" },
config: { basePath: "/my-app/api/auth" },
},
expected: "http://localhost:3000/my-app/api/auth/signout",
expected: {
url: "http://localhost:3000/my-app/api/auth/signout",
warningMessage: "env-url-basepath-mismatch",
},
},
{
args: {
Expand All @@ -259,15 +287,17 @@ describe("createActionURL", () => {
env: { AUTH_URL: "https://sub.domain.env.com/my-app" },
config: { basePath: "/api/auth" },
},
expected: "https://sub.domain.env.com/api/auth/signout",
expected: {
url: "https://sub.domain.env.com/api/auth/signout",
warningMessage: "env-url-basepath-mismatch",
},
},
])("Duplicate path configurations: %j", ({ args, expected }) => {
const argsWithLogger = { ...args, config: { ...args.config, logger } }
// @ts-expect-error
expect(createActionURL(...Object.values(args)).toString()).toBe(expected)
expect(consoleWarnSpy).toHaveBeenCalled()
})

afterAll(() => {
consoleWarnSpy.mockRestore()
expect(createActionURL(...Object.values(argsWithLogger)).toString()).toBe(
expected.url
)
expect(logger.warn).toHaveBeenCalledWith(expected.warningMessage)
})
})
2 changes: 1 addition & 1 deletion packages/next-auth/src/lib/env.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,6 @@ export function setEnvDefaults(config: NextAuthConfig) {
} catch {
} finally {
config.basePath ||= "/api/auth"
coreSetEnvDefaults(process.env, config)
coreSetEnvDefaults(process.env, config, true)
}
}
Loading
Loading