From cdc7d012a2bb2705d92049163ec9ec7a504ab9f8 Mon Sep 17 00:00:00 2001 From: Michael Nardolillo Date: Sat, 10 Jul 2021 23:55:20 +0200 Subject: [PATCH] feat: rename `protection` to `checks` (#2255) This change aligns the API with `openid-client`'s `checks` https://github.com/panva/node-openid-client/blob/main/docs/README.md#clientcallbackredirecturi-parameters-checks-extras, a library which we intend to migrate to in the future. Aligning our API early, so people get used to it. Also, objectively the name `protection` might not have been as clear as I first thought. `checks` better describe the intention. BREAKING CHANGE: The `state` option on OAuth providers is now deprecated. Use `checks: ["state"]` instead. `protections` is renamed to `checks`, here is an example: ```diff - protection: ["pkce"] + checks: ["pkece"] ``` Furthermore, string values are not supported anymore. This is to be able to handle fewer cases internally. ```diff - checks: "state" + checks: ["state"] ``` --- README.md | 2 -- app/pages/api/auth/[...nextauth].js | 4 +-- src/providers/apple.js | 2 +- src/providers/dropbox.js | 2 +- src/providers/naver.js | 2 +- src/providers/salesforce.js | 2 +- src/server/index.js | 26 +++++--------- src/server/lib/oauth/client.js | 2 +- src/server/lib/oauth/pkce-handler.js | 50 ++++++++++++++------------ src/server/lib/oauth/state-handler.js | 40 +++++++++------------ types/providers.d.ts | 9 ++--- www/docs/configuration/providers.md | 3 +- www/docs/getting-started/rest-api.md | 2 +- www/docs/getting-started/typescript.md | 4 --- www/docs/providers/identity-server4.md | 2 +- 15 files changed, 64 insertions(+), 88 deletions(-) diff --git a/README.md b/README.md index 552339a9d5..c4542c2575 100644 --- a/README.md +++ b/README.md @@ -83,8 +83,6 @@ Advanced options allow you to define your own routines to handle controlling wha NextAuth.js comes with built-in types. For more information and usage, check out the [TypeScript section](https://next-auth.js.org/getting-started/typescript) in the documentation. -The package at `@types/next-auth` is now deprecated. - ## Example ### Add API Route diff --git a/app/pages/api/auth/[...nextauth].js b/app/pages/api/auth/[...nextauth].js index 86d1985bdf..c163aed19d 100644 --- a/app/pages/api/auth/[...nextauth].js +++ b/app/pages/api/auth/[...nextauth].js @@ -41,11 +41,11 @@ export default NextAuth({ clientSecret: process.env.AUTH0_SECRET, domain: process.env.AUTH0_DOMAIN, // Used to debug https://github.com/nextauthjs/next-auth/issues/1664 - // protection: ["pkce", "state"], + // checks: ["pkce", "state"], // authorizationParams: { // response_mode: 'form_post' // } - protection: "pkce", + checks: ["pkce"], }), TwitterProvider({ clientId: process.env.TWITTER_ID, diff --git a/src/providers/apple.js b/src/providers/apple.js index 082080c28b..8757c14db2 100644 --- a/src/providers/apple.js +++ b/src/providers/apple.js @@ -28,7 +28,7 @@ export default function Apple(options) { privateKey: null, keyId: null, }, - protection: "none", // REVIEW: Apple does not support state, as far as I know. Can we use "pkce" then? + checks: ["none"], // REVIEW: Apple does not support state, as far as I know. Can we use "pkce" then? ...options, } } diff --git a/src/providers/dropbox.js b/src/providers/dropbox.js index 1753cc508b..f88d2cca67 100644 --- a/src/providers/dropbox.js +++ b/src/providers/dropbox.js @@ -48,7 +48,7 @@ export default function Dropbox(options) { email_verified: profile.email_verified, } }, - protection: ["state", "pkce"], + checks: ["state", "pkce"], ...options, } } diff --git a/src/providers/naver.js b/src/providers/naver.js index e266fe2510..094cb8f605 100644 --- a/src/providers/naver.js +++ b/src/providers/naver.js @@ -5,7 +5,7 @@ export default function Naver(options) { type: "oauth", version: "2.0", params: { grant_type: "authorization_code" }, - protection: ["state"], + checks: ["state"], accessTokenUrl: "https://nid.naver.com/oauth2.0/token", authorizationUrl: "https://nid.naver.com/oauth2.0/authorize?response_type=code", diff --git a/src/providers/salesforce.js b/src/providers/salesforce.js index 303d809477..c0e68b166a 100644 --- a/src/providers/salesforce.js +++ b/src/providers/salesforce.js @@ -9,7 +9,7 @@ export default function Salesforce(options) { authorizationUrl: "https://login.salesforce.com/services/oauth2/authorize?response_type=code", profileUrl: "https://login.salesforce.com/services/oauth2/userinfo", - protection: "none", + checks: ["none"], profile(profile) { return { ...profile, diff --git a/src/server/index.js b/src/server/index.js index a1cfa06a88..efb1cdc83a 100644 --- a/src/server/index.js +++ b/src/server/index.js @@ -77,26 +77,16 @@ async function NextAuthHandler(req, res, userOptions) { baseUrl, basePath, }) + const provider = providers.find(({ id }) => id === providerId) - // Protection only works on OAuth 2.x providers - // TODO: - // - rename to `checks` in 4.x, so it is similar to `openid-client` - // - stop supporting `protection` as string - // - remove `state` property - if (provider?.type === "oauth" && provider.version?.startsWith("2")) { - // Priority: (protection array > protection string) > state > default - if (provider.protection) { - provider.protection = Array.isArray(provider.protection) - ? provider.protection - : [provider.protection] - } else if (provider.state !== undefined) { - provider.protection = [provider.state ? "state" : "none"] - } else { - // Default to state, as we did in 3.1 - // REVIEW: should we use "pkce" or "none" as default? - provider.protection = ["state"] - } + // Checks only work on OAuth 2.x providers + if ( + provider?.type === "oauth" && + provider.version?.startsWith("2") && + !provider.checks + ) { + provider.checks = ["state"] } const maxAge = 30 * 24 * 60 * 60 // Sessions expire after 30 days of being idle diff --git a/src/server/lib/oauth/client.js b/src/server/lib/oauth/client.js index 2c9c2c1c5a..076bf33258 100644 --- a/src/server/lib/oauth/client.js +++ b/src/server/lib/oauth/client.js @@ -173,7 +173,7 @@ async function getOAuth2AccessToken(code, provider, codeVerifier) { headers.Authorization = `Bearer ${code}` } - if (provider.protection.includes("pkce")) { + if (provider.checks.includes("pkce")) { params.code_verifier = codeVerifier } diff --git a/src/server/lib/oauth/pkce-handler.js b/src/server/lib/oauth/pkce-handler.js index 366fd45c39..f15a2b0264 100644 --- a/src/server/lib/oauth/pkce-handler.js +++ b/src/server/lib/oauth/pkce-handler.js @@ -1,11 +1,11 @@ -import pkceChallenge from 'pkce-challenge' -import * as cookie from '../cookie' -import jwt from '../../../lib/jwt' -import logger from '../../../lib/logger' -import { OAuthCallbackError } from '../../../lib/errors' +import pkceChallenge from "pkce-challenge" +import * as cookie from "../cookie" +import jwt from "../../../lib/jwt" +import logger from "../../../lib/logger" +import { OAuthCallbackError } from "../../../lib/errors" const PKCE_LENGTH = 64 -const PKCE_CODE_CHALLENGE_METHOD = 'S256' // can be 'plain', not recommended https://tools.ietf.org/html/rfc7636#section-4.2 +const PKCE_CODE_CHALLENGE_METHOD = "S256" // can be 'plain', not recommended https://tools.ietf.org/html/rfc7636#section-4.2 const PKCE_MAX_AGE = 60 * 15 // 15 minutes in seconds /** @@ -13,28 +13,28 @@ const PKCE_MAX_AGE = 60 * 15 // 15 minutes in seconds * @param {import("types/internals").NextAuthRequest} req * @param {import("types/internals").NextAuthResponse} res */ -export async function handleCallback (req, res) { +export async function handleCallback(req, res) { const { cookies, provider, baseUrl, basePath } = req.options try { // Provider does not support PKCE, nothing to do. - if (!provider.protection?.includes('pkce')) { + if (!provider.checks?.includes("pkce")) { return } if (!(cookies.pkceCodeVerifier.name in req.cookies)) { - throw new OAuthCallbackError('The code_verifier cookie was not found.') + throw new OAuthCallbackError("The code_verifier cookie was not found.") } const pkce = await jwt.decode({ ...req.options.jwt, token: req.cookies[cookies.pkceCodeVerifier.name], maxAge: PKCE_MAX_AGE, - encryption: true + encryption: true, }) req.options.pkce = pkce - logger.debug('OAUTH_CALLBACK_PROTECTION', 'Read PKCE verifier from cookie', { + logger.debug("OAUTH_CALLBACK_CHECK", "Read PKCE verifier from cookie", { code_verifier: pkce.code_verifier, pkceLength: PKCE_LENGTH, - method: PKCE_CODE_CHALLENGE_METHOD + method: PKCE_CODE_CHALLENGE_METHOD, }) // remove PKCE after it has been used cookie.set(res, cookies.pkceCodeVerifier.name, "", { @@ -42,7 +42,7 @@ export async function handleCallback (req, res) { maxAge: 0 }) } catch (error) { - logger.error('CALLBACK_OAUTH_ERROR', error) + logger.error("CALLBACK_OAUTH_ERROR", error) return res.redirect(`${baseUrl}${basePath}/error?error=OAuthCallback`) } } @@ -52,42 +52,46 @@ export async function handleCallback (req, res) { * @param {import("types/internals").NextAuthRequest} req * @param {import("types/internals").NextAuthResponse} res */ -export async function handleSignin (req, res) { +export async function handleSignin(req, res) { const { cookies, provider, baseUrl, basePath } = req.options try { - if (!provider.protection?.includes('pkce')) { // Provider does not support PKCE, nothing to do. + if (!provider.checks?.includes("pkce")) { + // Provider does not support PKCE, nothing to do. return } // Started login flow, add generated pkce to req.options and (encrypted) code_verifier to a cookie const pkce = pkceChallenge(PKCE_LENGTH) - logger.debug('OAUTH_SIGNIN_PROTECTION', 'Created PKCE challenge/verifier', { + logger.debug("OAUTH_SIGNIN_CHECK", "Created PKCE challenge/verifier", { ...pkce, pkceLength: PKCE_LENGTH, - method: PKCE_CODE_CHALLENGE_METHOD + method: PKCE_CODE_CHALLENGE_METHOD, }) provider.authorizationParams = { ...provider.authorizationParams, code_challenge: pkce.code_challenge, - code_challenge_method: PKCE_CODE_CHALLENGE_METHOD + code_challenge_method: PKCE_CODE_CHALLENGE_METHOD, } const encryptedCodeVerifier = await jwt.encode({ ...req.options.jwt, maxAge: PKCE_MAX_AGE, token: { code_verifier: pkce.code_verifier }, - encryption: true + encryption: true, }) const cookieExpires = new Date() - cookieExpires.setTime(cookieExpires.getTime() + (PKCE_MAX_AGE * 1000)) + cookieExpires.setTime(cookieExpires.getTime() + PKCE_MAX_AGE * 1000) cookie.set(res, cookies.pkceCodeVerifier.name, encryptedCodeVerifier, { expires: cookieExpires.toISOString(), - ...cookies.pkceCodeVerifier.options + ...cookies.pkceCodeVerifier.options, }) - logger.debug('OAUTH_SIGNIN_PROTECTION', 'Created PKCE code_verifier saved in cookie') + logger.debug( + "OAUTH_SIGNIN_CHECK", + "Created PKCE code_verifier saved in cookie" + ) } catch (error) { - logger.error('SIGNIN_OAUTH_ERROR', error) + logger.error("SIGNIN_OAUTH_ERROR", error) return res.redirect(`${baseUrl}${basePath}/error?error=OAuthSignin`) } } diff --git a/src/server/lib/oauth/state-handler.js b/src/server/lib/oauth/state-handler.js index a1654644da..722af0d309 100644 --- a/src/server/lib/oauth/state-handler.js +++ b/src/server/lib/oauth/state-handler.js @@ -1,6 +1,6 @@ -import { createHash } from 'crypto' -import logger from '../../../lib/logger' -import { OAuthCallbackError } from '../../../lib/errors' +import { createHash } from "crypto" +import logger from "../../../lib/logger" +import { OAuthCallbackError } from "../../../lib/errors" /** * For OAuth 2.0 flows, if the provider supports state, @@ -9,27 +9,27 @@ import { OAuthCallbackError } from '../../../lib/errors' * @param {import("types/internals").NextAuthRequest} req * @param {import("types/internals").NextAuthResponse} res */ -export async function handleCallback (req, res) { +export async function handleCallback(req, res) { const { csrfToken, provider, baseUrl, basePath } = req.options try { // Provider does not support state, nothing to do. - if (!provider.protection?.includes('state')) { + if (!provider.checks?.includes("state")) { return } const state = req.query.state || req.body.state - const expectedState = createHash('sha256').update(csrfToken).digest('hex') + const expectedState = createHash("sha256").update(csrfToken).digest("hex") logger.debug( - 'OAUTH_CALLBACK_PROTECTION', - 'Comparing received and expected state', + "OAUTH_CALLBACK_CHECK", + "Comparing received and expected state", { state, expectedState } ) if (state !== expectedState) { - throw new OAuthCallbackError('Invalid state returned from OAuth provider') + throw new OAuthCallbackError("Invalid state returned from OAuth provider") } } catch (error) { - logger.error('STATE_ERROR', error) + logger.error("STATE_ERROR", error) return res.redirect(`${baseUrl}${basePath}/error?error=OAuthCallback`) } } @@ -39,31 +39,25 @@ export async function handleCallback (req, res) { * @param {import("types/internals").NextAuthRequest} req * @param {import("types/internals").NextAuthResponse} res */ -export async function handleSignin (req, res) { +export async function handleSignin(req, res) { const { provider, baseUrl, basePath, csrfToken } = req.options try { - if (!provider.protection?.includes('state')) { // Provider does not support state, nothing to do. + if (!provider.checks?.includes("state")) { + // Provider does not support state, nothing to do. return } - if ('state' in provider) { - logger.warn( - 'STATE_OPTION_DEPRECATION', - 'The `state` provider option is being replaced with `protection`. See the docs.' - ) - } - // A hash of the NextAuth.js CSRF token is used as the state - const state = createHash('sha256').update(csrfToken).digest('hex') + const state = createHash("sha256").update(csrfToken).digest("hex") provider.authorizationParams = { ...provider.authorizationParams, state } logger.debug( - 'OAUTH_CALLBACK_PROTECTION', - 'Added state to authorization params', + "OAUTH_CALLBACK_CHECK", + "Added state to authorization params", { state } ) } catch (error) { - logger.error('SIGNIN_OAUTH_ERROR', error) + logger.error("SIGNIN_OAUTH_ERROR", error) return res.redirect(`${baseUrl}${basePath}/error?error=OAuthSignin`) } } diff --git a/types/providers.d.ts b/types/providers.d.ts index 511cf3674e..6850f96a6b 100644 --- a/types/providers.d.ts +++ b/types/providers.d.ts @@ -14,7 +14,7 @@ export interface CommonProviderOptions { * OAuth Provider */ -type ProtectionType = "pkce" | "state" | "both" | "none" +type ChecksType = "pkce" | "state" | "both" | "none" /** * OAuth provider options @@ -34,18 +34,13 @@ export interface OAuthConfig

= Profile> authorizationUrl: string profileUrl: string profile(profile: P, tokens: TokenSet): Awaitable - protection?: ProtectionType | ProtectionType[] + checks?: ChecksType | ChecksType[] clientId: string clientSecret: | string // TODO: only allow for Apple | Record<"appleId" | "teamId" | "privateKey" | "keyId", string> idToken?: boolean - /** - * @deprecated Will be removed in an upcoming major release. Use `protection: ["state"]` instead. - */ - state?: boolean - // TODO: only allow for BattleNet region?: string // TODO: only allow for some diff --git a/www/docs/configuration/providers.md b/www/docs/configuration/providers.md index 62dddaf0b8..ca12893747 100644 --- a/www/docs/configuration/providers.md +++ b/www/docs/configuration/providers.md @@ -92,8 +92,7 @@ providers: [ | clientId | Client ID of the OAuth provider | `string` | Yes | | clientSecret | Client Secret of the OAuth provider | `string` | Yes | | profile | A callback returning an object with the user's info | `(profile, tokens) => Object` | Yes | -| protection | Additional security for OAuth login flows (defaults to `state`) | `"pkce"`,`"state"`,`"none"` | No | -| state | Same as `protection: "state"`. Being deprecated, use protection. | `boolean` | No | +| checks | Additional security checks on OAuth providers (default: [`state`]) | `("pkce"|"state"|"none")[]` | No | | headers | Any headers that should be sent to the OAuth provider | `Object` | No | | authorizationParams | Additional params to be sent to the authorization endpoint | `Object` | No | | idToken | Set to `true` for services that use ID Tokens (e.g. OpenID) | `boolean` | No | diff --git a/www/docs/getting-started/rest-api.md b/www/docs/getting-started/rest-api.md index 27e2a9076b..655bb8dc2a 100644 --- a/www/docs/getting-started/rest-api.md +++ b/www/docs/getting-started/rest-api.md @@ -19,7 +19,7 @@ The POST submission requires CSRF token from `/api/auth/csrf`. Handles returning requests from OAuth services during sign in. -For OAuth 2.0 providers that support the `state` option, the value of the `state` parameter is checked against the one that was generated when the sign in flow was started - this uses a hash of the CSRF token which MUST match for both the POST and `GET` calls during sign in. +For OAuth 2.0 providers that support the `checks: ["state"]` option, the state parameter is checked against the one that was generated when the sign in flow was started - this uses a hash of the CSRF token which MUST match for both the POST and `GET` calls during sign in. #### `GET` /api/auth/signout diff --git a/www/docs/getting-started/typescript.md b/www/docs/getting-started/typescript.md index 81741d65ec..81c9773321 100644 --- a/www/docs/getting-started/typescript.md +++ b/www/docs/getting-started/typescript.md @@ -8,10 +8,6 @@ NextAuth.js comes with its own type definitions, so you can safely use it in you Check out the example repository showcasing how to use `next-auth` on a Next.js application with TypeScript: https://github.com/nextauthjs/next-auth-typescript-example -:::warning -The types at [DefinitelyTyped](https://github.com/DefinitelyTyped/DefinitelyTyped) under the name of `@types/next-auth` are now deprecated, and not maintained anymore. -::: - --- ## Adapters diff --git a/www/docs/providers/identity-server4.md b/www/docs/providers/identity-server4.md index 9eb9211760..b0811554df 100644 --- a/www/docs/providers/identity-server4.md +++ b/www/docs/providers/identity-server4.md @@ -52,7 +52,7 @@ providers: [ domain: "demo.identityserver.io", clientId: "interactive.confidential", clientSecret: "secret", - protection: "pkce" + checks: ["pkce"] }) } ...