feat(provider): re-add state, expand protection provider options (#1184)

* refactor: move OAuthCallbackError to errors file

* refactor: improve pkce handling

* feat(provider): re-introduce state to provider options

* docs(provider): mention protection options "state" and "none"

* docs(provider): document state property deprecation

* fix: only add code_verifier param if protection is pkce

* docs: explain state deprecation better

* chore: unify string

src/lib/errors.js

@@ -1,8 +1,7 @@
-class UnknownError extends Error {
+export class UnknownError extends Error {
   constructor (message) {
     super(message)
     this.name = 'UnknownError'
-    this.message = message
   }
 
   toJSON () {
@@ -16,26 +15,25 @@ class UnknownError extends Error {
   }
 }
 
-class CreateUserError extends UnknownError {
+export class CreateUserError extends UnknownError {
   constructor (message) {
     super(message)
     this.name = 'CreateUserError'
-    this.message = message
   }
 }
 
 // Thrown when an Email address is already associated with an account
 // but the user is trying an OAuth account that is not linked to it.
-class AccountNotLinkedError extends UnknownError {
+export class AccountNotLinkedError extends UnknownError {
   constructor (message) {
     super(message)
     this.name = 'AccountNotLinkedError'
-    this.message = message
   }
 }
 
-module.exports = {
-  UnknownError,
-  CreateUserError,
-  AccountNotLinkedError
+export class OAuthCallbackError extends UnknownError {
+  constructor (message) {
+    super(message)
+    this.name = 'OAuthCallbackError'
+  }
 }

src/providers/apple.js

@@ -24,6 +24,7 @@ export default (options) => {
       privateKey: null,
       keyId: null
     },
+    protection: 'none', // REVIEW: Apple does not support state, as far as I know. Can we use "pkce" then?
     ...options
   }
 }

src/providers/salesforce.js

@@ -8,7 +8,7 @@ export default (options) => {
     accessTokenUrl: 'https://login.salesforce.com/services/oauth2/token',
     authorizationUrl: 'https://login.salesforce.com/services/oauth2/authorize?response_type=code',
     profileUrl: 'https://login.salesforce.com/services/oauth2/userinfo',
-    state: false,
+    protection: 'none', // REVIEW: Can we use "pkce" ?
     profile: (profile) => {
       return {
         ...profile,

src/server/index.js

@@ -12,7 +12,8 @@ import * as routes from './routes'
 import renderPage from './pages'
 import csrfTokenHandler from './lib/csrf-token-handler'
 import createSecret from './lib/create-secret'
-import * as pkce from './lib/pkce-handler'
+import * as pkce from './lib/oauth/pkce-handler'
+import * as state from './lib/oauth/state-handler'
 
 // To work properly in production with OAuth providers the NEXTAUTH_URL
 // environment variable must be set.
@@ -63,6 +64,13 @@ async function NextAuthHandler (req, res, userOptions) {
     const providers = parseProviders({ providers: userOptions.providers, baseUrl, basePath })
     const provider = providers.find(({ id }) => id === providerId)
 
+    if (provider &&
+      provider.type === 'oauth' && provider.version?.startsWith('2') &&
+       (!provider.protection && provider.state !== false)
+    ) {
+      provider.protection = 'state' // Default to state, as we did in 3.1 REVIEW: should we use "pkce" or "none" as default?
+    }
+
     const maxAge = 30 * 24 * 60 * 60 // Sessions expire after 30 days of being idle
 
     // Parse database / adapter
@@ -145,9 +153,8 @@ async function NextAuthHandler (req, res, userOptions) {
           return render.signout()
         case 'callback':
           if (provider) {
-            const error = await pkce.handleCallback(req, res)
-            if (error) return res.redirect(error)
-
+            if (await pkce.handleCallback(req, res)) return
+            if (await state.handleCallback(req, res)) return
             return routes.callback(req, res)
           }
           break
@@ -184,9 +191,8 @@ async function NextAuthHandler (req, res, userOptions) {
         case 'signin':
           // Verified CSRF Token required for all sign in routes
           if (csrfTokenVerified && provider) {
-            const error = await pkce.handleSignin(req, res)
-            if (error) return res.redirect(error)
-
+            if (await pkce.handleSignin(req, res)) return
+            if (await state.handleSignin(req, res)) return
             return routes.signin(req, res)
           }
 
@@ -204,9 +210,8 @@ async function NextAuthHandler (req, res, userOptions) {
               return res.redirect(`${baseUrl}${basePath}/signin?csrf=true`)
             }
 
-            const error = await pkce.handleCallback(req, res)
-            if (error) return res.redirect(error)
-
+            if (await pkce.handleCallback(req, res)) return
+            if (await state.handleCallback(req, res)) return
             return routes.callback(req, res)
           }
           break

src/server/lib/oauth/callback.js

@@ -1,33 +1,16 @@
-import { createHash } from 'crypto'
 import { decode as jwtDecode } from 'jsonwebtoken'
 import oAuthClient from './client'
 import logger from '../../../lib/logger'
-class OAuthCallbackError extends Error {
-  constructor (message) {
-    super(message)
-    this.name = 'OAuthCallbackError'
-    this.message = message
-  }
-}
+import { OAuthCallbackError } from '../../../lib/errors'
 
 export default async function oAuthCallback (req) {
-  const { provider, csrfToken, pkce } = req.options
+  const { provider, pkce } = req.options
   const client = oAuthClient(provider)
 
   if (provider.version?.startsWith('2.')) {
     // The "user" object is specific to the Apple provider and is provided on first sign in
     // e.g. {"name":{"firstName":"Johnny","lastName":"Appleseed"},"email":"johnny.appleseed@nextauth.com"}
-    let { code, user, state } = req.query // eslint-disable-line camelcase
-    // For OAuth 2.0 flows, check state returned and matches expected value
-    // (a hash of the NextAuth.js CSRF token).
-    //
-    // Apple does not support state verification.
-    if (provider.id !== 'apple') {
-      const expectedState = createHash('sha256').update(csrfToken).digest('hex')
-      if (state !== expectedState) {
-        throw new OAuthCallbackError('Invalid state returned from OAuth provider')
-      }
-    }
+    let { code, user } = req.query // eslint-disable-line camelcase
 
     if (req.method === 'POST') {
       try {

src/server/lib/pkce-handler.js → src/server/lib/oauth/pkce-handler.js

@@ -1,7 +1,8 @@
 import pkceChallenge from 'pkce-challenge'
-import jwt from '../../lib/jwt'
-import * as cookie from '../lib/cookie'
-import logger from '../../lib/logger'
+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
@@ -16,19 +17,24 @@ export async function handleCallback (req, res) {
     }
 
     if (!(cookies.pkceCodeVerifier.name in req.cookies)) {
-      throw new Error('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
     })
-    cookie.set(res, cookies.pkceCodeVerifier.name, null, { maxAge: 0 }) // remove PKCE after it has been used
     req.options.pkce = pkce
+    logger.debug('OAUTH_CALLBACK_PROTECTION', 'Read PKCE verifier from cookie', {
+      code_verifier: pkce.code_verifier,
+      pkceLength: PKCE_LENGTH,
+      method: PKCE_CODE_CHALLENGE_METHOD
+    })
+    cookie.set(res, cookies.pkceCodeVerifier.name, null, { maxAge: 0 }) // remove PKCE after it has been used
   } catch (error) {
-    logger.error('PKCE_ERROR', error)
-    return `${baseUrl}${basePath}/error?error=Configuration`
+    logger.error('CALLBACK_OAUTH_ERROR', error)
+    return res.redirect(`${baseUrl}${basePath}/error?error=OAuthCallback`)
   }
 }
 
@@ -41,10 +47,18 @@ export async function handleSignin (req, res) {
     }
     // Started login flow, add generated pkce to req.options and (encrypted) code_verifier to a cookie
     const pkce = pkceChallenge(PKCE_LENGTH)
-    req.options.pkce = {
+    logger.debug('OAUTH_SIGNIN_PROTECTION', 'Created PKCE challenge/verifier', {
+      ...pkce,
+      pkceLength: PKCE_LENGTH,
+      method: PKCE_CODE_CHALLENGE_METHOD
+    })
+
+    provider.authorizationParams = {
+      ...provider.authorizationParams,
       code_challenge: pkce.code_challenge,
       code_challenge_method: PKCE_CODE_CHALLENGE_METHOD
     }
+
     const encryptedCodeVerifier = await jwt.encode({
       ...req.options.jwt,
       maxAge: PKCE_MAX_AGE,
@@ -58,12 +72,11 @@ export async function handleSignin (req, res) {
       expires: cookieExpires.toISOString(),
       ...cookies.pkceCodeVerifier.options
     })
+    logger.debug('OAUTH_SIGNIN_PROTECTION', 'Created PKCE code_verifier saved in cookie')
   } catch (error) {
-    logger.error('PKCE_ERROR', error)
-    return `${baseUrl}${basePath}/error?error=Configuration`
+    logger.error('SIGNIN_OAUTH_ERROR', error)
+    return res.redirect(`${baseUrl}${basePath}/error?error=OAuthSignin`)
   }
 }
 
-export default {
-  handleSignin, handleCallback
-}
+export default { handleSignin, handleCallback }

src/server/lib/oauth/state-handler.js

@@ -0,0 +1,64 @@
+import { createHash } from 'crypto'
+import logger from '../../../lib/logger'
+import { OAuthCallbackError } from '../../../lib/errors'
+
+/**
+ * For OAuth 2.0 flows, if the provider supports state,
+ * check if state matches the one sent on signin
+ * (a hash of the NextAuth.js CSRF token).
+ */
+export async function handleCallback (req, res) {
+  const { csrfToken, provider, baseUrl, basePath } = req.options
+  try {
+    if (provider.protection !== 'state') { // Provider does not support state, nothing to do.
+      return
+    }
+
+    const { state } = req.query
+    const expectedState = createHash('sha256').update(csrfToken).digest('hex')
+
+    logger.debug(
+      'OAUTH_CALLBACK_PROTECTION',
+      'Comparing received and expected state',
+      { state, expectedState }
+    )
+    if (state !== expectedState) {
+      throw new OAuthCallbackError('Invalid state returned from OAuth provider')
+    }
+  } catch (error) {
+    logger.error('STATE_ERROR', error)
+    return res.redirect(`${baseUrl}${basePath}/error?error=OAuthCallback`)
+  }
+}
+
+/** Adds CSRF token to the authorizationParams. */
+export async function handleSignin (req, res) {
+  const { provider, baseUrl, basePath, csrfToken } = req.options
+  try {
+    if (provider.protection !== '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')
+
+    provider.authorizationParams = { ...provider.authorizationParams, state }
+    logger.debug(
+      'OAUTH_CALLBACK_PROTECTION',
+      'Added state to authorization params',
+      { state }
+    )
+  } catch (error) {
+    logger.error('SIGNIN_OAUTH_ERROR', error)
+    return res.redirect(`${baseUrl}${basePath}/error?error=OAuthSignin`)
+  }
+}
+
+export default { handleSignin, handleCallback }

src/server/lib/signin/oauth.js

@@ -1,21 +1,17 @@
 import oAuthClient from '../oauth/client'
-import { createHash } from 'crypto'
 import logger from '../../../lib/logger'
 
 export default async function getAuthorizationUrl (req) {
-  const { provider, csrfToken, pkce } = req.options
+  const { provider } = req.options
 
   const client = oAuthClient(provider)
   if (provider.version?.startsWith('2.')) {
     // Handle OAuth v2.x
     let url = client.getAuthorizeUrl({
       ...provider.authorizationParams,
       ...req.body.authorizationParams,
-      ...pkce,
       redirect_uri: provider.callbackUrl,
-      scope: provider.scope,
-      // A hash of the NextAuth.js CSRF token is used as the state
-      state: createHash('sha256').update(csrfToken).digest('hex')
+      scope: provider.scope
     })
 
     // If the authorizationUrl specified in the config has query parameters on it

www/docs/configuration/providers.md

@@ -114,25 +114,26 @@ providers: [
 
 ### OAuth provider options
 
-|        Name         |                         Description                         |              Type               | Required |
-| :-----------------: | :---------------------------------------------------------: | :-----------------------------: | :------: |
-|         id          |                 Unique ID for the provider                  |            `string`             |   Yes    |
-|        name         |              Descriptive name for the provider              |            `string`             |   Yes    |
-|        type         |     Type of provider, in this case it should be `oauth`     | `oauth`, `email`, `credentials` |   Yes    |
-|       version       |          OAuth version (e.g. '1.0', '1.0a', '2.0')          |            `string`             |   Yes    |
-|   accessTokenUrl    |            Endpoint to retrieve an access token             |            `string`             |   Yes    |
-|  authorizationUrl   |       Endpoint to request authorization from the user       |            `string`             |   Yes    |
-|      clientId       |               Client ID of the OAuth provider               |            `string`             |   Yes    |
-|    clientSecret     |             Client Secret of the OAuth provider             |            `string`             |    No    |
-|        scope        |        OAuth access scopes (expects array or string)        |     `string` or `string[]`      |    No    |
-|       params        |           Additional authorization URL parameters           |            `object`             |    No    |
-|   requestTokenUrl   |            Endpoint to retrieve a request token             |            `string`             |    No    |
-| authorizationParams | Additional params to be sent to the authorization endpoint  |            `object`             |    No    |
-|     profileUrl      |           Endpoint to retrieve the user's profile           |            `string`             |    No    |
-|       profile       |    An callback returning an object with the user's info     |            `object`             |    No    |
-|       idToken       | Set to `true` for services that use ID Tokens (e.g. OpenID) |            `boolean`            |    No    |
-|       headers       |    Any headers that should be sent to the OAuth provider    |            `object`             |    No    |
-|     protection      |          Additional security for OAuth login flows          |             `pkce`              |    No    |
+|        Name         |                           Description                            |              Type               | Required |
+| :-----------------: | :--------------------------------------------------------------: | :-----------------------------: | :------: |
+|         id          |                    Unique ID for the provider                    |            `string`             |   Yes    |
+|        name         |                Descriptive name for the provider                 |            `string`             |   Yes    |
+|        type         |       Type of provider, in this case it should be `oauth`        | `oauth`, `email`, `credentials` |   Yes    |
+|       version       |            OAuth version (e.g. '1.0', '1.0a', '2.0')             |            `string`             |   Yes    |
+|   accessTokenUrl    |               Endpoint to retrieve an access token               |            `string`             |   Yes    |
+|  authorizationUrl   |         Endpoint to request authorization from the user          |            `string`             |   Yes    |
+|      clientId       |                 Client ID of the OAuth provider                  |            `string`             |   Yes    |
+|    clientSecret     |               Client Secret of the OAuth provider                |            `string`             |    No    |
+|        scope        |          OAuth access scopes (expects array or string)           |     `string` or `string[]`      |    No    |
+|       params        |             Additional authorization URL parameters              |            `object`             |    No    |
+|   requestTokenUrl   |               Endpoint to retrieve a request token               |            `string`             |    No    |
+| authorizationParams |    Additional params to be sent to the authorization endpoint    |            `object`             |    No    |
+|     profileUrl      |             Endpoint to retrieve the user's profile              |            `string`             |    No    |
+|       profile       |       An callback returning an object with the user's info       |            `object`             |    No    |
+|       idToken       |   Set to `true` for services that use ID Tokens (e.g. OpenID)    |            `boolean`            |    No    |
+|       headers       |      Any headers that should be sent to the OAuth provider       |            `object`             |    No    |
+|     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    |
 
 ## Sign in with Email
 

www/docs/warnings.md

@@ -67,4 +67,11 @@ To remedy this, simply return the url instead:
 
 ```js
   return "/some/url"
-```
+```
+
+
+#### STATE_OPTION_DEPRECATION
+You provided `state: true` or `state: false` as a provider option. This is being deprecated in a later release in favour of `protection: "state"` and `protection: "none"` respectively. To remedy this warning:
+
+- If you use `state: true`, just simply remove it. The default is `protection: "state"` already..
+- If you use `state: false`, set `protection: "none"`.