Skip to content

Commit

Permalink
fix(oauth): support response_mode=form_post (#1669)
Browse files Browse the repository at this point in the history
* chore: alias dev script to next

* feat(core): fallback to body when reading state

* refactor: set csrfToken on req.options implicitly

Ensures we do this similarly than
in other handlers like pkce, state, extendRes, callbackUrlHandler etc.

* chore: add code comment for debugging
  • Loading branch information
balazsorban44 committed Apr 11, 2021
1 parent 3dedf6c commit 968903d
Show file tree
Hide file tree
Showing 7 changed files with 61 additions and 34 deletions.
2 changes: 1 addition & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ npm i
> NOTE: You can add any environment variables to .env.local that you would like to use in your dev app.
> You can find the next-auth config under`pages/api/auth/[...nextauth].js`.
1. Start the dev application/server and CSS watching:
1. Start the dev application/server:
```sh
npm run dev
```
Expand Down
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@
"build": "npm run build:js && npm run build:css",
"build:js": "babel --config-file ./config/babel.config.json src --out-dir dist",
"build:css": "postcss --config config/postcss.config.js src/**/*.css --base src --dir dist && node config/wrap-css.js",
"dev": "next | npm run watch:css",
"dev:with-css": "next | npm run watch:css",
"dev": "next",
"watch": "npm run watch:js | npm run watch:css",
"watch:js": "babel --config-file ./config/babel.config.json --watch src --out-dir dist",
"watch:css": "postcss --config config/postcss.config.js --watch src/**/*.css --base src --dir dist",
Expand Down
26 changes: 26 additions & 0 deletions pages/api/auth/[...nextauth].js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,27 @@ import Providers from 'next-auth/providers'
// const prisma = new PrismaClient()

export default NextAuth({
// Used to debug https://github.com/nextauthjs/next-auth/issues/1664
// cookies: {
// csrfToken: {
// name: 'next-auth.csrf-token',
// options: {
// httpOnly: true,
// sameSite: 'none',
// path: '/',
// secure: true
// }
// },
// pkceCodeVerifier: {
// name: 'next-auth.pkce.code_verifier',
// options: {
// httpOnly: true,
// sameSite: 'none',
// path: '/',
// secure: true
// }
// }
// },
providers: [
Providers.Email({
server: process.env.EMAIL_SERVER,
Expand All @@ -19,6 +40,11 @@ export default NextAuth({
clientId: process.env.AUTH0_ID,
clientSecret: process.env.AUTH0_SECRET,
domain: process.env.AUTH0_DOMAIN,
// Used to debug https://github.com/nextauthjs/next-auth/issues/1664
// protection: ["pkce", "state"],
// authorizationParams: {
// response_mode: 'form_post'
// }
protection: 'pkce'
}),
Providers.Twitter({
Expand Down
1 change: 1 addition & 0 deletions src/server/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ export interface NextAuthInternalOptions extends Pick<NextAuthOptions, NextAuthS
basePath?: string
action?: string
csrfToken?: string
csrfTokenVerified?: boolean
}

export interface NextAuthRequest extends NextApiRequest {
Expand Down
18 changes: 8 additions & 10 deletions src/server/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,12 @@ import * as cookie from './lib/cookie'
import * as defaultEvents from './lib/default-events'
import * as defaultCallbacks from './lib/default-callbacks'
import parseProviders from './lib/providers'
import callbackUrlHandler from './lib/callback-url-handler'
import extendRes from './lib/extend-res'
import * as routes from './routes'
import renderPage from './pages'
import csrfTokenHandler from './lib/csrf-token-handler'
import createSecret from './lib/create-secret'
import callbackUrlHandler from './lib/callback-url-handler'
import extendRes from './lib/extend-res'
import csrfTokenHandler from './lib/csrf-token-handler'
import * as pkce from './lib/oauth/pkce-handler'
import * as state from './lib/oauth/state-handler'

Expand Down Expand Up @@ -67,8 +67,6 @@ async function NextAuthHandler (req, res, userOptions) {

const secret = createSecret({ userOptions, basePath, baseUrl })

const { csrfToken, csrfTokenVerified } = csrfTokenHandler(req, res, cookies, secret)

const providers = parseProviders({ providers: userOptions.providers, baseUrl, basePath })
const provider = providers.find(({ id }) => id === providerId)

Expand Down Expand Up @@ -107,7 +105,6 @@ async function NextAuthHandler (req, res, userOptions) {
provider,
cookies,
secret,
csrfToken,
providers,
// Session options
session: {
Expand Down Expand Up @@ -138,6 +135,7 @@ async function NextAuthHandler (req, res, userOptions) {
logger
}

csrfTokenHandler(req, res)
await callbackUrlHandler(req, res)

const render = renderPage(req, res)
Expand All @@ -150,7 +148,7 @@ async function NextAuthHandler (req, res, userOptions) {
case 'session':
return routes.session(req, res)
case 'csrf':
return res.json({ csrfToken })
return res.json({ csrfToken: req.options.csrfToken })
case 'signin':
if (pages.signIn) {
let signinUrl = `${pages.signIn}${pages.signIn.includes('?') ? '&' : '?'}callbackUrl=${req.options.callbackUrl}`
Expand Down Expand Up @@ -203,7 +201,7 @@ async function NextAuthHandler (req, res, userOptions) {
switch (action) {
case 'signin':
// Verified CSRF Token required for all sign in routes
if (csrfTokenVerified && provider) {
if (req.options.csrfTokenVerified && provider) {
if (await pkce.handleSignin(req, res)) return
if (await state.handleSignin(req, res)) return
return routes.signin(req, res)
Expand All @@ -212,14 +210,14 @@ async function NextAuthHandler (req, res, userOptions) {
return res.redirect(`${baseUrl}${basePath}/signin?csrf=true`)
case 'signout':
// Verified CSRF Token required for signout
if (csrfTokenVerified) {
if (req.options.csrfTokenVerified) {
return routes.signout(req, res)
}
return res.redirect(`${baseUrl}${basePath}/signout?csrf=true`)
case 'callback':
if (provider) {
// Verified CSRF Token required for credentials providers only
if (provider.type === 'credentials' && !csrfTokenVerified) {
if (provider.type === 'credentials' && !req.options.csrfTokenVerified) {
return res.redirect(`${baseUrl}${basePath}/signin?csrf=true`)
}

Expand Down
43 changes: 22 additions & 21 deletions src/server/lib/csrf-token-handler.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,29 +14,30 @@ import * as cookie from './cookie'
* For more details, see the following OWASP links:
* https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Request_Forgery_Prevention_Cheat_Sheet.html#double-submit-cookie
* https://owasp.org/www-chapter-london/assets/slides/David_Johansson-Double_Defeat_of_Double-Submit_Cookie.pdf
* @param {import("..").NextAuthRequest} req
* @param {import("..").NextAuthResponse} res
*/
export default function csrfTokenHandler (req, res, cookies, secret) {
const { csrfToken: csrfTokenFromRequest } = req.body

let csrfTokenFromCookie
let csrfTokenVerified = false
if (req.cookies[cookies.csrfToken.name]) {
const [csrfTokenValue, csrfTokenHash] = req.cookies[cookies.csrfToken.name].split('|')
if (csrfTokenHash === createHash('sha256').update(`${csrfTokenValue}${secret}`).digest('hex')) {
export default function csrfTokenHandler (req, res) {
const { cookies, secret } = req.options
if (cookies.csrfToken.name in req.cookies) {
const [csrfToken, csrfTokenHash] = req.cookies[cookies.csrfToken.name].split('|')
const expectedCsrfTokenHash = createHash('sha256').update(`${csrfToken}${secret}`).digest('hex')
if (csrfTokenHash === expectedCsrfTokenHash) {
// If hash matches then we trust the CSRF token value
csrfTokenFromCookie = csrfTokenValue

// If this is a POST request and the CSRF Token in the Post request matches
// the cookie we have already verified is one we have set, then token is verified!
if (req.method === 'POST' && csrfTokenFromCookie === csrfTokenFromRequest) { csrfTokenVerified = true }
// If this is a POST request and the CSRF Token in the POST request matches
// the cookie we have already verified is the one we have set, then the token is verified!
const csrfTokenVerified = req.method === 'POST' && csrfToken === req.body.csrfToken
req.options.csrfToken = csrfToken
req.options.csrfTokenVerified = csrfTokenVerified
return
}
}
if (!csrfTokenFromCookie) {
// If no csrfToken - because it's not been set yet, or because the hash doesn't match
// (e.g. because it's been modifed or because the secret has changed) create a new token.
csrfTokenFromCookie = randomBytes(32).toString('hex')
const newCsrfTokenCookie = `${csrfTokenFromCookie}|${createHash('sha256').update(`${csrfTokenFromCookie}${secret}`).digest('hex')}`
cookie.set(res, cookies.csrfToken.name, newCsrfTokenCookie, cookies.csrfToken.options)
}
return { csrfToken: csrfTokenFromCookie, csrfTokenVerified }
// If no csrfToken from cookie - because it's not been set yet,
// or because the hash doesn't match (e.g. because it's been modifed or because the secret has changed)
// create a new token.
const csrfToken = randomBytes(32).toString('hex')
const csrfTokenHash = createHash('sha256').update(`${csrfToken}${secret}`).digest('hex')
const csrfTokenCookie = `${csrfToken}|${csrfTokenHash}`
cookie.set(res, cookies.csrfToken.name, csrfTokenCookie, cookies.csrfToken.options)
req.options.csrfToken = csrfToken
}
2 changes: 1 addition & 1 deletion src/server/lib/oauth/state-handler.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ export async function handleCallback (req, res) {
return
}

const { state } = req.query
const state = req.query.state || req.body.state
const expectedState = createHash('sha256').update(csrfToken).digest('hex')

logger.debug(
Expand Down

1 comment on commit 968903d

@vercel
Copy link

@vercel vercel bot commented on 968903d Apr 11, 2021

Choose a reason for hiding this comment

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

Please sign in to comment.