Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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" ?
Copy link
Member Author

@balazsorban44 balazsorban44 Jan 22, 2021

Choose a reason for hiding this comment

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

@mmahalwy since you added this provider (#1027) can you answer me if Salesforce supports PKCE? https://tools.ietf.org/html/rfc7636

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?
Copy link
Member Author

Choose a reason for hiding this comment

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

@iaincollins should we default to something else than "state"?

}

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"`.