Skip to content

Commit

Permalink
feat: rename protection to checks (nextauthjs#2255)
Browse files Browse the repository at this point in the history
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"]
```
  • Loading branch information
mnphpexpert committed Jul 10, 2021
1 parent cccb8cd commit cdc7d01
Show file tree
Hide file tree
Showing 15 changed files with 64 additions and 88 deletions.
2 changes: 0 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions app/pages/api/auth/[...nextauth].js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion src/providers/apple.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
}
2 changes: 1 addition & 1 deletion src/providers/dropbox.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ export default function Dropbox(options) {
email_verified: profile.email_verified,
}
},
protection: ["state", "pkce"],
checks: ["state", "pkce"],
...options,
}
}
2 changes: 1 addition & 1 deletion src/providers/naver.js
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
2 changes: 1 addition & 1 deletion src/providers/salesforce.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
26 changes: 8 additions & 18 deletions src/server/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/server/lib/oauth/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
50 changes: 27 additions & 23 deletions src/server/lib/oauth/pkce-handler.js
Original file line number Diff line number Diff line change
@@ -1,48 +1,48 @@
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

/**
* Adds `code_verifier` to `req.options.pkce`, and removes the corresponding cookie
* @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, "", {
...cookies.pkceCodeVerifier.options,
maxAge: 0
})
} catch (error) {
logger.error('CALLBACK_OAUTH_ERROR', error)
logger.error("CALLBACK_OAUTH_ERROR", error)
return res.redirect(`${baseUrl}${basePath}/error?error=OAuthCallback`)
}
}
Expand All @@ -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`)
}
}
Expand Down
40 changes: 17 additions & 23 deletions src/server/lib/oauth/state-handler.js
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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`)
}
}
Expand All @@ -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`)
}
}
Expand Down
9 changes: 2 additions & 7 deletions types/providers.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ export interface CommonProviderOptions {
* OAuth Provider
*/

type ProtectionType = "pkce" | "state" | "both" | "none"
type ChecksType = "pkce" | "state" | "both" | "none"

/**
* OAuth provider options
Expand All @@ -34,18 +34,13 @@ export interface OAuthConfig<P extends Record<string, unknown> = Profile>
authorizationUrl: string
profileUrl: string
profile(profile: P, tokens: TokenSet): Awaitable<User & { id: string }>
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
Expand Down
3 changes: 1 addition & 2 deletions www/docs/configuration/providers.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
Expand Down
2 changes: 1 addition & 1 deletion www/docs/getting-started/rest-api.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
4 changes: 0 additions & 4 deletions www/docs/getting-started/typescript.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion www/docs/providers/identity-server4.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ providers: [
domain: "demo.identityserver.io",
clientId: "interactive.confidential",
clientSecret: "secret",
protection: "pkce"
checks: ["pkce"]
})
}
...
Expand Down

0 comments on commit cdc7d01

Please sign in to comment.