diff --git a/packages/core/src/errors.ts b/packages/core/src/errors.ts index a4a0ddadc0..216597e472 100644 --- a/packages/core/src/errors.ts +++ b/packages/core/src/errors.ts @@ -1,8 +1,8 @@ type ErrorOptions = Error | Record type ErrorType = + | "AccessDenied" | "AdapterError" - | "AuthorizedCallbackError" | "CallbackRouteError" | "ErrorPageLoop" | "EventError" @@ -102,8 +102,8 @@ export class AdapterError extends AuthError { * Thrown when the execution of the [`signIn` callback](https://authjs.dev/reference/core/types#signin) fails * or if it returns `false`. */ -export class AuthorizedCallbackError extends AuthError { - static type = "AuthorizedCallbackError" +export class AccessDenied extends AuthError { + static type = "AccessDenied" } /** @@ -188,11 +188,25 @@ export class InvalidCallbackUrl extends AuthError { } /** - * The `authorize` callback returned `null` in the [Credentials provider](https://authjs.dev/getting-started/providers/credentials-tutorial). - * We don't recommend providing information about which part of the credentials were wrong, as it might be abused by malicious hackers. + * Can be thrown from the `authorize` callback of the Credentials provider. + * When an error occurs during the `authorize` callback, two things can happen: + * 1. The user is redirected to the signin page, with `error=CredentialsSignin&code=credentials` in the URL. `code` is configurable. + * 2. If you throw this error in a framework that handles form actions server-side, this error is thrown, instead of redirecting the user, so you'll need to handle. */ export class CredentialsSignin extends SignInError { static type = "CredentialsSignin" + /** + * The error code that is set in the `code` query parameter of the redirect URL. + * + * + * ⚠ NOTE: This property is going to be included in the URL, so make sure it does not hint at sensitive errors. + * + * The full error is always logged on the server, if you need to debug. + * + * Generally, we don't recommend hinting specifically if the user had either a wrong username or password specifically, + * try rather something like "Invalid credentials". + */ + code: string = "credentials" } /** @@ -433,6 +447,26 @@ export class MissingCSRF extends SignInError { static type = "MissingCSRF" } +const clientErrors = new Set([ + "CredentialsSignin", + "OAuthAccountNotLinked", + "OAuthCallbackError", + "AccessDenied", + "Verification", + "MissingCSRF", + "AccountNotLinked", + "WebAuthnVerificationError", +]) + +/** + * Used to only allow sending a certain subset of errors to the client. + * Errors are always logged on the server, but to prevent leaking sensitive information, + * only a subset of errors are sent to the client as-is. + */ +export function isClientError(error: Error): error is AuthError { + if (error instanceof AuthError) return clientErrors.has(error.type) + return false +} /** * Thrown when multiple providers have `enableConditionalUI` set to `true`. * Only one provider can have this option enabled at a time. @@ -443,7 +477,7 @@ export class DuplicateConditionalUI extends AuthError { /** * Thrown when a WebAuthn provider has `enableConditionalUI` set to `true` but no formField has `webauthn` in its autocomplete param. - * + * * The `webauthn` autocomplete param is required for conditional UI to work. */ export class MissingWebAuthnAutocomplete extends AuthError { diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index e7541f8791..411ba37878 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -37,7 +37,12 @@ */ import { assertConfig } from "./lib/utils/assert.js" -import { AuthError, ErrorPageLoop } from "./errors.js" +import { + AuthError, + CredentialsSignin, + ErrorPageLoop, + isClientError, +} from "./errors.js" import { AuthInternal, raw, skipCSRFCheck } from "./lib/index.js" import { setEnvDefaults, createActionURL } from "./lib/utils/env.js" import renderPage from "./lib/pages/index.js" @@ -46,6 +51,7 @@ import { toInternalRequest, toResponse } from "./lib/utils/web.js" import type { Adapter } from "./adapters.js" import type { + AuthAction, CallbacksOptions, CookiesOptions, EventCallbacks, @@ -94,44 +100,42 @@ export async function Auth( setLogger(config.logger, config.debug) const internalRequest = await toInternalRequest(request, config) + // There was an error parsing the request + if (!internalRequest) return Response.json(`Bad request.`, { status: 400 }) - if (internalRequest instanceof Error) { - logger.error(internalRequest) - return Response.json( - `Error: This action with HTTP ${request.method} is not supported.`, - { status: 400 } - ) - } - - const assertionResult = assertConfig(internalRequest, config) + const warningsOrError = assertConfig(internalRequest, config) - if (Array.isArray(assertionResult)) { - assertionResult.forEach(logger.warn) - } else if (assertionResult instanceof Error) { - // Bail out early if there's an error in the user config - logger.error(assertionResult) - const htmlPages = ["signin", "signout", "error", "verify-request"] + if (Array.isArray(warningsOrError)) { + warningsOrError.forEach(logger.warn) + } else if (warningsOrError) { + // If there's an error in the user config, bail out early + logger.error(warningsOrError) + const htmlPages = new Set([ + "signin", + "signout", + "error", + "verify-request", + ]) if ( - !htmlPages.includes(internalRequest.action) || + !htmlPages.has(internalRequest.action) || internalRequest.method !== "GET" ) { - return Response.json( - { - message: - "There was a problem with the server configuration. Check the server logs for more information.", - }, - { status: 500 } - ) + const message = + "There was a problem with the server configuration. Check the server logs for more information." + return Response.json({ message }, { status: 500 }) } const { pages, theme } = config + // If this is true, the config required auth on the error page + // which could cause a redirect loop const authOnErrorPage = pages?.error && internalRequest.url.searchParams .get("callbackUrl") ?.startsWith(pages.error) + // Either there was no error page configured or the configured one contains infinite redirects if (!pages?.error || authOnErrorPage) { if (authOnErrorPage) { logger.error( @@ -140,8 +144,8 @@ export async function Auth( ) ) } - const render = renderPage({ theme }) - const page = render.error("Configuration") + + const page = renderPage({ theme }).error("Configuration") return toResponse(page) } @@ -150,11 +154,16 @@ export async function Auth( const isRedirect = request.headers?.has("X-Auth-Return-Redirect") const isRaw = config.raw === raw - let response: Response try { - const rawResponse = await AuthInternal(internalRequest, config) - if (isRaw) return rawResponse - response = await toResponse(rawResponse) + const internalResponse = await AuthInternal(internalRequest, config) + if (isRaw) return internalResponse + + const response = toResponse(internalResponse) + const url = response.headers.get("Location") + + if (!isRedirect || !url) return response + + return Response.json({ url }, { headers: response.headers }) } catch (e) { const error = e as Error logger.error(error) @@ -167,23 +176,19 @@ export async function Auth( if (request.method === "POST" && internalRequest.action === "session") return Response.json(null, { status: 400 }) - const type = isAuthError ? error.type : "Configuration" - const page = (isAuthError && error.kind) || "error" - // TODO: Filter out some error types from being sent to the client + const isClientSafeErrorType = isClientError(error) + const type = isClientSafeErrorType ? error.type : "Configuration" + const params = new URLSearchParams({ error: type }) - const path = - config.pages?.[page] ?? `${config.basePath}/${page.toLowerCase()}` + if (error instanceof CredentialsSignin) params.set("code", error.code) - const url = `${internalRequest.url.origin}${path}?${params}` + const pageKind = (isAuthError && error.kind) || "error" + const pagePath = config.pages?.[pageKind] ?? `/${pageKind.toLowerCase()}` + const url = `${internalRequest.url.origin}${config.basePath}${pagePath}?${params}` if (isRedirect) return Response.json({ url }) - return Response.redirect(url) } - - const redirect = response.headers.get("Location") - if (!isRedirect || !redirect) return response - return Response.json({ url: redirect }, { headers: response.headers }) } /** diff --git a/packages/core/src/lib/actions/callback/index.ts b/packages/core/src/lib/actions/callback/index.ts index aeb3e44c6b..73bbb3e81b 100644 --- a/packages/core/src/lib/actions/callback/index.ts +++ b/packages/core/src/lib/actions/callback/index.ts @@ -2,7 +2,7 @@ import { AuthError, - AuthorizedCallbackError, + AccessDenied, CallbackRouteError, CredentialsSignin, InvalidProvider, @@ -311,12 +311,10 @@ export async function callback( // prettier-ignore new Request(url, { headers, method, body: JSON.stringify(body) }) ) - const user = userFromAuthorize && { - ...userFromAuthorize, - id: userFromAuthorize?.id?.toString() ?? crypto.randomUUID(), - } + const user = userFromAuthorize if (!user) throw new CredentialsSignin() + else user.id = user.id?.toString() ?? crypto.randomUUID() const account = { providerAccountId: user.id, @@ -508,9 +506,9 @@ async function handleAuthorized( authorized = await signIn(params) } catch (e) { if (e instanceof AuthError) throw e - throw new AuthorizedCallbackError(e as Error) + throw new AccessDenied(e as Error) } - if (!authorized) throw new AuthorizedCallbackError("AccessDenied") + if (!authorized) throw new AccessDenied("AccessDenied") if (typeof authorized !== "string") return return await redirect({ url: authorized, baseUrl: config.url.origin }) } diff --git a/packages/core/src/lib/actions/signin/send-token.ts b/packages/core/src/lib/actions/signin/send-token.ts index 2de5c853d0..27024affaf 100644 --- a/packages/core/src/lib/actions/signin/send-token.ts +++ b/packages/core/src/lib/actions/signin/send-token.ts @@ -1,5 +1,5 @@ import { createHash, randomString, toRequest } from "../../utils/web.js" -import { AuthorizedCallbackError } from "../../../errors.js" +import { AccessDenied } from "../../../errors.js" import type { InternalOptions, RequestInternal } from "../../../types.js" import type { Account } from "../../../types.js" @@ -36,9 +36,9 @@ export async function sendToken( email: { verificationRequest: true }, }) } catch (e) { - throw new AuthorizedCallbackError(e as Error) + throw new AccessDenied(e as Error) } - if (!authorized) throw new AuthorizedCallbackError("AccessDenied") + if (!authorized) throw new AccessDenied("AccessDenied") if (typeof authorized === "string") { return { redirect: await callbacks.redirect({ diff --git a/packages/core/src/lib/utils/web.ts b/packages/core/src/lib/utils/web.ts index daae279f40..16c77116d9 100644 --- a/packages/core/src/lib/utils/web.ts +++ b/packages/core/src/lib/utils/web.ts @@ -1,5 +1,6 @@ import { parse as parseCookie, serialize } from "cookie" import { UnknownAction } from "../../errors.js" +import { logger } from "./logger.js" import type { AuthAction, @@ -24,7 +25,7 @@ async function getBody(req: Request): Promise | undefined> { export async function toInternalRequest( req: Request, config: AuthConfig -): Promise { +): Promise { try { if (req.method !== "GET" && req.method !== "POST") throw new UnknownAction("Only GET and POST requests are supported.") @@ -51,7 +52,8 @@ export async function toInternalRequest( query: Object.fromEntries(url.searchParams), } } catch (e) { - return e as Error + logger.error(e as Error) + logger.debug("request", req) } } @@ -119,8 +121,7 @@ export function parseActionAndProviderId( } { const a = pathname.match(new RegExp(`^${base}(.+)`)) - if (a === null) - throw new UnknownAction(`Cannot parse action at ${pathname}`) + if (a === null) throw new UnknownAction(`Cannot parse action at ${pathname}`) const [_, actionAndProviderId] = a diff --git a/packages/core/src/providers/credentials.ts b/packages/core/src/providers/credentials.ts index b8e18f8009..6cb1806af8 100644 --- a/packages/core/src/providers/credentials.ts +++ b/packages/core/src/providers/credentials.ts @@ -29,16 +29,31 @@ export interface CredentialsConfig< * by a popular library like [Zod](https://zod.dev) * ::: * + * This method expects a `User` object to be returned for a successful login. + * + * If an `CredentialsSignin` is thrown or `null` is returned, two things can happen: + * 1. The user is redirected to the login page, with `error=CredentialsSignin&code=credentials` in the URL. `code` is configurable, see below. + * 2. If you throw this error in a framework that handles form actions server-side, this error is thrown by the login form action, so you'll need to handle it there. + * + * In case of 1., generally, we recommend not hinting if the user for example gave a wrong username or password specifically, + * try rather something like "invalid-credentials". Try to be as generic with client-side errors as possible. + * + * To customize the error code, you can create a custom error that extends {@link CredentialsSignin} and throw it in `authorize`. + * + * @example + * ```ts + * class CustomError extends CredentialsSignin { + * code = "custom_error" + * } + * // URL will contain `error=CredentialsSignin&code=custom_error` + * ``` + * * @example * ```ts - * //... - * async authorize(credentials, request) { + * async authorize(credentials, request) { // you have access to the original request as well * if(!isValidCredentials(credentials)) return null - * const response = await fetch(request) - * if(!response.ok) return null - * return await response.json() ?? null + * return await getUser(credentials) // assuming it returns a User or null * } - * //... * ``` */ authorize: ( @@ -52,7 +67,7 @@ export interface CredentialsConfig< * or you can use a popular library like [Zod](https://zod.dev) for example. */ credentials: Partial>, - /** The original request is forward for convenience */ + /** The original request. */ request: Request ) => Awaitable } diff --git a/packages/core/src/types.ts b/packages/core/src/types.ts index ee23b1e9eb..c18c7ae83a 100644 --- a/packages/core/src/types.ts +++ b/packages/core/src/types.ts @@ -199,9 +199,9 @@ export interface CallbacksOptions

{ * Returning `false` or throwing an error will stop the sign-in flow and redirect the user to the error page. * Returning a string will redirect the user to the specified URL. * - * Unhandled errors will throw an `AuthorizedCallbackError` with the message set to the original error. + * Unhandled errors will throw an `AccessDenied` with the message set to the original error. * - * @see [`AuthorizedCallbackError`](https://authjs.dev/reference/errors#authorizedcallbackerror) + * @see [`AccessDenied`](https://authjs.dev/reference/errors#accessdenied) * * @example * ```ts diff --git a/packages/core/test/actions/callback.test.ts b/packages/core/test/actions/callback.test.ts index 14e5667950..b32c16b233 100644 --- a/packages/core/test/actions/callback.test.ts +++ b/packages/core/test/actions/callback.test.ts @@ -13,7 +13,7 @@ describe("assert GET callback action", () => { afterEach(() => { vi.restoreAllMocks() }) - it("should throw InvalidProvider error if the provider ID is not found", async () => { + it("should throw Configuration error if the provider ID is not found", async () => { const { response } = await makeAuthRequest({ action: "callback", path: "/invalid-provider", @@ -21,11 +21,11 @@ describe("assert GET callback action", () => { expect(response.status).toEqual(302) expect(response.headers.get("location")).toEqual( - `https://authjs.test/auth/error?error=InvalidProvider` + `https://authjs.test/auth/error?error=Configuration` ) }) - it("should throws InvalidCheck is missing query state and isOnRedirectProxy is true", async () => { + it("should throw Configuration error is missing query state and isOnRedirectProxy is true", async () => { const { response } = await makeAuthRequest({ action: "callback", path: "/github", @@ -38,7 +38,7 @@ describe("assert GET callback action", () => { expect(response.status).toEqual(302) expect(response.headers.get("location")).toEqual( - `https://authjs.test/auth/error?error=InvalidCheck` + `https://authjs.test/auth/error?error=Configuration` ) }) diff --git a/packages/core/test/authorize.test.ts b/packages/core/test/authorize.test.ts index b5761c4007..3524dcaa1e 100644 --- a/packages/core/test/authorize.test.ts +++ b/packages/core/test/authorize.test.ts @@ -38,7 +38,7 @@ describe("auth via callbacks.signIn", () => { it("return false", async () => { const res = await signIn({ callbacks: { signIn: () => false } }) expect(res.headers.get("Location")).toBe( - "http://a/auth/error?error=AuthorizedCallbackError" + "http://a/auth/error?error=AccessDenied" ) }) it("return redirect relative URL", async () => { @@ -64,7 +64,7 @@ describe("auth via callbacks.signIn", () => { }, }) expect(res.headers.get("Location")).toBe( - "http://a/auth/error?error=AuthorizedCallbackError" + "http://a/auth/error?error=AccessDenied" ) }) })