Skip to content

Commit

Permalink
fix: improve logger (#4970)
Browse files Browse the repository at this point in the history
* fix: add debug warning, only show warnings once

* fix: prefer `debug` for details

* remove url

* test: fix tests

* Update docs/docs/errors.md

Co-authored-by: Thang Vu <31528554+ThangHuuVu@users.noreply.github.com>

* Update callback.ts

Co-authored-by: Thang Vu <31528554+ThangHuuVu@users.noreply.github.com>
  • Loading branch information
balazsorban44 and ThangHuuVu committed Jul 21, 2022
1 parent 8ec940b commit 0ea9679
Show file tree
Hide file tree
Showing 14 changed files with 130 additions and 130 deletions.
21 changes: 9 additions & 12 deletions docs/docs/errors.md
Expand Up @@ -61,17 +61,20 @@ There should also be further details logged when this occurs, such as the error

### Signin / Callback

#### GET_AUTHORIZATION_URL_ERROR
#### SIGNIN_OAUTH_ERROR

This error can occur when we cannot get the OAuth v1 request token and generate the authorization URL.
This error occurs during the redirection to the authorization URL of the OAuth provider. Possible causes:

Please double check your OAuth v1 provider settings, especially the OAuth token and OAuth token secret.
1. Cookie handling
Either PKCE code verifier or the generation of the CSRF token hash in the internal state failed.

#### SIGNIN_OAUTH_ERROR
If set, check your [`cookies` configuration](/configuration/options#cookies), and make sure the browser is not blocking/restricting cookies.

2. OAuth misconfiguration

This error can occur in one of a few places, first during the redirect to the authorization URL of the provider. Next, in the signin flow while creating the PKCE code verifier. Finally, during the generation of the CSRF Token hash in the internal state during signin.
Please check your OAuth provider and make sure your URLs and other options are correctly set.

Please check your OAuth provider settings and make sure your URLs and other options are correctly set on the provider side.
If you are using an OAuth v1 provider, check your OAuth v1 provider settings, especially the OAuth token and OAuth token secret.

#### CALLBACK_OAUTH_ERROR

Expand Down Expand Up @@ -151,12 +154,6 @@ This error occurs when there was an issue deleting the session from the database

### Other

#### SEND_VERIFICATION_EMAIL_ERROR

This error occurs when the Email Authentication Provider is unable to send an email.

Check your mail server configuration.

#### MISSING_NEXTAUTH_API_ROUTE_ERROR

This error happens when `[...nextauth].js` file is not found inside `pages/api/auth`.
Expand Down
6 changes: 6 additions & 0 deletions docs/docs/warnings.md
Expand Up @@ -37,6 +37,12 @@ Twitter OAuth 2.0 is currently in beta as certain changes might still be necessa

Some APIs are still experimental; they may be changed or removed in the future. Use at your own risk.

#### DEBUG_ENABLED

You have enabled the `debug` option. It is meant for development only, to help you catch issues in your authentication flow and you should consider removing this option when deploying to production. One way of only allowing debugging while not in production is to set `debug: process.env.NODE_ENV !== "production"`, so you can commit this without needing to change the value.

If you want to log debug messages during production anyway, we recommend setting the [`logger` option](/configuration/options#logger) with proper sanitization of potentially sensitive user information.

## Adapter

### ADAPTER_TYPEORM_UPDATING_ENTITIES
Expand Down
2 changes: 1 addition & 1 deletion packages/next-auth/src/client/__tests__/csrf.test.js
Expand Up @@ -79,7 +79,7 @@ test("when the fetch fails it'll throw a client fetch error", async () => {
await waitFor(() => {
expect(logger.error).toHaveBeenCalledTimes(1)
expect(logger.error).toBeCalledWith("CLIENT_FETCH_ERROR", {
path: "csrf",
url: "/api/auth/csrf",
error: new SyntaxError("Unexpected token s in JSON at position 0"),
})
})
Expand Down
2 changes: 1 addition & 1 deletion packages/next-auth/src/client/__tests__/providers.test.js
Expand Up @@ -57,7 +57,7 @@ test("when failing to fetch the providers, it'll log the error", async () => {
await waitFor(() => {
expect(logger.error).toHaveBeenCalledTimes(1)
expect(logger.error).toBeCalledWith("CLIENT_FETCH_ERROR", {
path: "providers",
url: "/api/auth/providers",
error: new SyntaxError("Unexpected token s in JSON at position 0"),
})
})
Expand Down
2 changes: 1 addition & 1 deletion packages/next-auth/src/client/__tests__/session.test.js
Expand Up @@ -71,7 +71,7 @@ test("if there's an error fetching the session, it should log it", async () => {
await waitFor(() => {
expect(logger.error).toHaveBeenCalledTimes(1)
expect(logger.error).toBeCalledWith("CLIENT_FETCH_ERROR", {
path: "session",
url: "/api/auth/session",
error: new SyntaxError("Unexpected token S in JSON at position 0"),
})
})
Expand Down
2 changes: 1 addition & 1 deletion packages/next-auth/src/client/__tests__/sign-in.test.js
Expand Up @@ -256,7 +256,7 @@ test("when it fails to fetch the providers, it redirected back to signin page",
expect(logger.error).toHaveBeenCalledTimes(1)
expect(logger.error).toBeCalledWith("CLIENT_FETCH_ERROR", {
error: "Error when retrieving providers",
path: "providers",
url: "/api/auth/providers",
})
})
})
Expand Down
9 changes: 3 additions & 6 deletions packages/next-auth/src/client/_utils.ts
Expand Up @@ -35,20 +35,17 @@ export async function fetchData<T = any>(
logger: LoggerInstance,
{ ctx, req = ctx?.req }: CtxOrReq = {}
): Promise<T | null> {
const url = `${apiBaseUrl(__NEXTAUTH)}/${path}`
try {
const options = req?.headers.cookie
? { headers: { cookie: req.headers.cookie } }
: {}
const res = await fetch(`${apiBaseUrl(__NEXTAUTH)}/${path}`, options)
const res = await fetch(url, options)
const data = await res.json()
if (!res.ok) throw data
return Object.keys(data).length > 0 ? data : null // Return null if data empty
} catch (error) {
logger.error("CLIENT_FETCH_ERROR", {
error: error as Error,
path,
...(req ? { header: req.headers } : {}),
})
logger.error("CLIENT_FETCH_ERROR", { error: error as Error, url })
return null
}
}
Expand Down
4 changes: 2 additions & 2 deletions packages/next-auth/src/core/index.ts
Expand Up @@ -90,8 +90,8 @@ export async function NextAuthHandler<

const assertionResult = assertConfig({ options: userOptions, req })

if (typeof assertionResult === "string") {
logger.warn(assertionResult)
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
const { pages, theme } = userOptions
Expand Down
49 changes: 28 additions & 21 deletions packages/next-auth/src/core/lib/assert.ts
Expand Up @@ -9,8 +9,9 @@ import {
import parseUrl from "../../utils/parse-url"
import { defaultCookies } from "./cookie"

import type { NextAuthHandlerParams, RequestInternal } from ".."
import type { RequestInternal } from ".."
import type { WarningCode } from "../../utils/logger"
import type { NextAuthOptions } from "../types"

type ConfigError =
| MissingAPIRoute
Expand All @@ -19,7 +20,7 @@ type ConfigError =
| MissingAuthorize
| MissingAdapter

let twitterWarned = false
let warned = false

function isValidHttpUrl(url: string, baseUrl: string) {
try {
Expand All @@ -37,28 +38,35 @@ function isValidHttpUrl(url: string, baseUrl: string) {
*
* REVIEW: Make some of these and corresponding docs less Next.js specific?
*/
export function assertConfig(
params: NextAuthHandlerParams & {
req: RequestInternal
}
): ConfigError | WarningCode | undefined {
export function assertConfig(params: {
options: NextAuthOptions
req: RequestInternal
}): ConfigError | WarningCode[] {
const { options, req } = params

const warnings: WarningCode[] = []

if (!warned) {
if (!req.host) warnings.push("NEXTAUTH_URL")

// TODO: Make this throw an error in next major. This will also get rid of `NODE_ENV`
if (!options.secret && process.env.NODE_ENV !== "production")
warnings.push("NO_SECRET")

if (options.debug) warnings.push("DEBUG_ENABLED")
}

if (!options.secret && process.env.NODE_ENV === "production") {
return new MissingSecret("Please define a `secret` in production.")
}

// req.query isn't defined when asserting `unstable_getServerSession` for example
if (!req.query?.nextauth && !req.action) {
return new MissingAPIRoute(
"Cannot find [...nextauth].{js,ts} in `/pages/api/auth`. Make sure the filename is written correctly."
)
}

if (!options.secret) {
if (process.env.NODE_ENV === "production") {
return new MissingSecret("Please define a `secret` in production.")
} else {
return "NO_SECRET"
}
}

const callbackUrlParam = req.query?.callbackUrl as string | undefined

const url = parseUrl(req.host)
Expand All @@ -69,9 +77,6 @@ export function assertConfig(
)
}

// This is below the callbackUrlParam check because it would obscure the error
if (!req.host) return "NEXTAUTH_URL"

const { callbackUrl: defaultCallbackUrl } = defaultCookies(
options.useSecureCookies ?? url.base.startsWith("https://")
)
Expand Down Expand Up @@ -119,8 +124,10 @@ export function assertConfig(
return new MissingAdapter("E-mail login requires an adapter.")
}

if (!twitterWarned && hasTwitterOAuth2) {
twitterWarned = true
return "TWITTER_OAUTH_2_BETA"
if (!warned) {
if (hasTwitterOAuth2) warnings.push("TWITTER_OAUTH_2_BETA")
warned = true
}

return warnings
}
37 changes: 16 additions & 21 deletions packages/next-auth/src/core/lib/email/signin.ts
Expand Up @@ -9,9 +9,8 @@ import type { InternalOptions } from "../../types"
export default async function email(
identifier: string,
options: InternalOptions<"email">
) {
const { url, adapter, provider, logger, callbackUrl, theme } = options

): Promise<string> {
const { url, adapter, provider, callbackUrl, theme } = options
// Generate token
const token =
(await provider.generateVerificationToken?.()) ??
Expand All @@ -34,22 +33,18 @@ export default async function email(
const params = new URLSearchParams({ callbackUrl, token, email: identifier })
const _url = `${url}/callback/${provider.id}?${params}`

try {
// Send to user
await provider.sendVerificationRequest({
identifier,
token,
expires,
url: _url,
provider,
theme,
})
} catch (error) {
logger.error("SEND_VERIFICATION_EMAIL_ERROR", {
identifier,
url,
error: error as Error,
})
throw new Error("SEND_VERIFICATION_EMAIL_ERROR")
}
// Send to user
await provider.sendVerificationRequest({
identifier,
token,
expires,
url: _url,
provider,
theme,
})

return `${url}/verify-request?${new URLSearchParams({
provider: provider.id,
type: provider.type,
})}`
}
93 changes: 45 additions & 48 deletions packages/next-auth/src/core/lib/oauth/authorization-url.ts
Expand Up @@ -14,66 +14,63 @@ import type { Cookie } from "../cookie"
*
* [OAuth 2](https://www.oauth.com/oauth2-servers/authorization/the-authorization-request/) | [OAuth 1](https://oauth.net/core/1.0a/#auth_step2)
*/
export default async function getAuthorizationUrl(params: {
export default async function getAuthorizationUrl({
options,
query,
}: {
options: InternalOptions<"oauth">
query: RequestInternal["query"]
}) {
const { options, query } = params
const { logger, provider } = options
try {
let params: any = {}
let params: any = {}

if (typeof provider.authorization === "string") {
const parsedUrl = new URL(provider.authorization)
const parsedParams = Object.fromEntries(parsedUrl.searchParams.entries())
params = { ...params, ...parsedParams }
} else {
params = { ...params, ...provider.authorization?.params }
}
if (typeof provider.authorization === "string") {
const parsedUrl = new URL(provider.authorization)
const parsedParams = Object.fromEntries(parsedUrl.searchParams.entries())
params = { ...params, ...parsedParams }
} else {
params = { ...params, ...provider.authorization?.params }
}

params = { ...params, ...query }
params = { ...params, ...query }

// Handle OAuth v1.x
if (provider.version?.startsWith("1.")) {
const client = oAuth1Client(options)
const tokens = (await client.getOAuthRequestToken(params)) as any
const url = `${
// @ts-expect-error
provider.authorization?.url ?? provider.authorization
}?${new URLSearchParams({
oauth_token: tokens.oauth_token,
oauth_token_secret: tokens.oauth_token_secret,
...tokens.params,
})}`
// Handle OAuth v1.x
if (provider.version?.startsWith("1.")) {
const client = oAuth1Client(options)
const tokens = (await client.getOAuthRequestToken(params)) as any
const url = `${
// @ts-expect-error
provider.authorization?.url ?? provider.authorization
}?${new URLSearchParams({
oauth_token: tokens.oauth_token,
oauth_token_secret: tokens.oauth_token_secret,
...tokens.params,
})}`

logger.debug("GET_AUTHORIZATION_URL", { url })
return { redirect: url }
}
logger.debug("GET_AUTHORIZATION_URL", { url, provider })
return { redirect: url }
}

const client = await openidClient(options)
const client = await openidClient(options)

const authorizationParams: AuthorizationParameters = params
const cookies: Cookie[] = []
const authorizationParams: AuthorizationParameters = params
const cookies: Cookie[] = []

const state = await createState(options)
if (state) {
authorizationParams.state = state.value
cookies.push(state.cookie)
}
const state = await createState(options)
if (state) {
authorizationParams.state = state.value
cookies.push(state.cookie)
}

const pkce = await createPKCE(options)
if (pkce) {
authorizationParams.code_challenge = pkce.code_challenge
authorizationParams.code_challenge_method = pkce.code_challenge_method
cookies.push(pkce.cookie)
}
const pkce = await createPKCE(options)
if (pkce) {
authorizationParams.code_challenge = pkce.code_challenge
authorizationParams.code_challenge_method = pkce.code_challenge_method
cookies.push(pkce.cookie)
}

const url = client.authorizationUrl(authorizationParams)
const url = client.authorizationUrl(authorizationParams)

logger.debug("GET_AUTHORIZATION_URL", { url, cookies })
return { redirect: url, cookies }
} catch (error) {
logger.error("GET_AUTHORIZATION_URL_ERROR", error as Error)
throw error
}
logger.debug("GET_AUTHORIZATION_URL", { url, cookies, provider })
return { redirect: url, cookies }
}

1 comment on commit 0ea9679

@vercel
Copy link

@vercel vercel bot commented on 0ea9679 Jul 21, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.