Skip to content

Commit

Permalink
feat(provider): re-add state, expand protection provider options (nex…
Browse files Browse the repository at this point in the history
…tauthjs#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
  • Loading branch information
mnphpexpert committed Feb 1, 2021
1 parent 4855cdd commit 659a486
Show file tree
Hide file tree
Showing 10 changed files with 148 additions and 80 deletions.
18 changes: 8 additions & 10 deletions src/lib/errors.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
class UnknownError extends Error {
export class UnknownError extends Error {
constructor (message) {
super(message)
this.name = 'UnknownError'
this.message = message
}

toJSON () {
Expand All @@ -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'
}
}
1 change: 1 addition & 0 deletions src/providers/apple.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
2 changes: 1 addition & 1 deletion src/providers/salesforce.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
25 changes: 15 additions & 10 deletions src/server/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
}

Expand All @@ -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
Expand Down
23 changes: 3 additions & 20 deletions src/server/lib/oauth/callback.js
Original file line number Diff line number Diff line change
@@ -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":"[email protected]"}
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 {
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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`)
}
}

Expand All @@ -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,
Expand All @@ -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 }
64 changes: 64 additions & 0 deletions src/server/lib/oauth/state-handler.js
Original file line number Diff line number Diff line change
@@ -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 }
8 changes: 2 additions & 6 deletions src/server/lib/signin/oauth.js
Original file line number Diff line number Diff line change
@@ -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
Expand Down
39 changes: 20 additions & 19 deletions www/docs/configuration/providers.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 8 additions & 1 deletion www/docs/warnings.md
Original file line number Diff line number Diff line change
Expand Up @@ -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"`.

0 comments on commit 659a486

Please sign in to comment.