refactor: decouple CSRF-state (#3142)

* refactor: decouple csrf token from state

* refactor: simplify pkce-handler

Author: balazsorban44

src/core/index.ts

@@ -61,8 +61,6 @@ export async function NextAuthHandler<
     req.cookies?.[options.cookies.sessionToken.name] ||
     req.headers?.Authorization?.replace("Bearer ", "")
 
-  const codeVerifier = req.cookies?.[options.cookies.pkceCodeVerifier.name]
-
   if (req.method === "GET") {
     const render = renderPage({ options, query: req.query, cookies })
     const { pages } = options
@@ -98,9 +96,9 @@ export async function NextAuthHandler<
             query: req.query,
             method: req.method,
             headers: req.headers,
+            cookies: req.cookies,
             options,
             sessionToken,
-            codeVerifier,
           })
           if (callback.cookies) cookies.push(...callback.cookies)
           return { ...callback, cookies }
@@ -180,9 +178,9 @@ export async function NextAuthHandler<
             query: req.query,
             method: req.method,
             headers: req.headers,
+            cookies: req.cookies,
             options,
             sessionToken,
-            codeVerifier,
           })
           if (callback.cookies) cookies.push(...callback.cookies)
           return { ...callback, cookies }

src/core/lib/cookie.ts

@@ -205,6 +205,15 @@ export function defaultCookies(useSecureCookies: boolean): CookiesOptions {
         secure: useSecureCookies,
       },
     },
+    state: {
+      name: `${cookiePrefix}next-auth.state`,
+      options: {
+        httpOnly: true,
+        sameSite: "lax",
+        path: "/",
+        secure: useSecureCookies,
+      },
+    },
   }
 }
 

src/core/lib/oauth/authorization-url.ts

@@ -2,9 +2,11 @@ import { openidClient } from "./client"
 import { oAuth1Client } from "./client-legacy"
 import { createState } from "./state-handler"
 import { createPKCE } from "./pkce-handler"
-import { InternalOptions } from "../../../lib/types"
-import { IncomingRequest } from "../.."
-import { Cookie } from "../cookie"
+
+import type { AuthorizationParameters } from "openid-client"
+import type { InternalOptions } from "../../../lib/types"
+import type { IncomingRequest } from "../.."
+import type { Cookie } from "../cookie"
 
 /**
  *
@@ -48,24 +50,28 @@ export default async function getAuthorizationUrl(params: {
       return { redirect: url }
     }
 
-    const cookies: Cookie[] = []
     const client = await openidClient(options)
+
+    const authorizationParams: AuthorizationParameters = params
+    const cookies: Cookie[] = []
+
+    const state = await createState(options)
+    if (state) {
+      authorizationParams.state = state.value
+      cookies.push(state.cookie)
+    }
+
     const pkce = await createPKCE(options)
-    if (pkce?.cookie) {
+    if (pkce) {
+      authorizationParams.code_challenge = pkce.code_challenge
+      authorizationParams.code_challenge_method = pkce.code_challenge_method
       cookies.push(pkce.cookie)
     }
 
-    const url = client.authorizationUrl({
-      ...params,
-      ...pkce,
-      state: createState(options),
-    })
+    const url = client.authorizationUrl(authorizationParams)
 
-    logger.debug("GET_AUTHORIZATION_URL", { url })
-    return {
-      redirect: url,
-      cookies,
-    }
+    logger.debug("GET_AUTHORIZATION_URL", { url, cookies })
+    return { redirect: url, cookies }
   } catch (error) {
     logger.error("GET_AUTHORIZATION_URL_ERROR", error as Error)
     throw error

src/core/lib/oauth/callback.ts

@@ -1,22 +1,24 @@
-import { TokenSet } from "openid-client"
+import { CallbackParamsType, TokenSet } from "openid-client"
 import { openidClient } from "./client"
 import { oAuth1Client } from "./client-legacy"
-import { getState } from "./state-handler"
+import { useState } from "./state-handler"
 import { usePKCECodeVerifier } from "./pkce-handler"
 import { OAuthCallbackError } from "../../errors"
-import { Account, LoggerInstance, Profile } from "../../.."
-import { OAuthChecks, OAuthConfig } from "../../../providers"
-import { InternalOptions } from "../../../lib/types"
-import { IncomingRequest, OutgoingResponse } from "../.."
+
+import type { Account, LoggerInstance, Profile } from "../../.."
+import type { OAuthChecks, OAuthConfig } from "../../../providers"
+import type { InternalOptions } from "../../../lib/types"
+import type { IncomingRequest, OutgoingResponse } from "../.."
+import type { Cookie } from "../cookie"
 
 export default async function oAuthCallback(params: {
   options: InternalOptions<"oauth">
   query: IncomingRequest["query"]
   body: IncomingRequest["body"]
   method: IncomingRequest["method"]
-  codeVerifier?: string
+  cookies: IncomingRequest["cookies"]
 }): Promise<GetProfileResult & { cookies?: OutgoingResponse["cookies"] }> {
-  const { options, query, body, method, codeVerifier } = params
+  const { options, query, body, method, cookies } = params
   const { logger, provider } = options
 
   const errorMessage = body?.error ?? query?.error
@@ -66,15 +68,24 @@ export default async function oAuthCallback(params: {
 
     let tokens: TokenSet
 
-    const pkce = await usePKCECodeVerifier({
-      options,
-      codeVerifier,
-    })
-    const checks: OAuthChecks = {
-      code_verifier: pkce?.codeVerifier,
-      state: getState(options),
+    const checks: OAuthChecks = {}
+    const resCookies: Cookie[] = []
+
+    const state = await useState(cookies?.[options.cookies.state.name], options)
+
+    if (state) {
+      checks.state = state.value
+      resCookies.push(state.cookie)
+    }
+
+    const codeVerifier = cookies?.[options.cookies.pkceCodeVerifier.name]
+    const pkce = await usePKCECodeVerifier(codeVerifier, options)
+    if (pkce) {
+      checks.code_verifier = pkce.codeVerifier
+      resCookies.push(pkce.cookie)
     }
-    const params = {
+
+    const params: CallbackParamsType = {
       ...client.callbackParams({
         url: `http://n?${new URLSearchParams(query)}`,
         // TODO: Ask to allow object to be passed upstream:
@@ -136,12 +147,12 @@ export default async function oAuthCallback(params: {
       tokens,
       logger,
     })
-    return {
-      ...profileResult,
-      cookies: pkce?.cookie ? [pkce.cookie] : undefined,
-    }
+    return { ...profileResult, cookies: resCookies }
   } catch (error) {
-    logger.error("OAUTH_CALLBACK_ERROR", { error: error as Error, providerId: provider.id })
+    logger.error("OAUTH_CALLBACK_ERROR", {
+      error: error as Error,
+      providerId: provider.id,
+    })
     throw new OAuthCallbackError(error as Error)
   }
 }

src/core/lib/oauth/pkce-handler.ts

@@ -34,79 +34,62 @@ export async function createPKCE(options: InternalOptions<"oauth">): PKCE {
     // Provider does not support PKCE, return nothing.
     return
   }
-  const codeVerifier = generators.codeVerifier()
-  const codeChallenge = generators.codeChallenge(codeVerifier)
+  const code_verifier = generators.codeVerifier()
+  const code_challenge = generators.codeChallenge(code_verifier)
+
+  const expires = new Date()
+  expires.setTime(expires.getTime() + PKCE_MAX_AGE * 1000)
 
   // Encrypt code_verifier and save it to an encrypted cookie
-  const encryptedCodeVerifier = await jwt.encode({
-    // @ts-expect-error
-    maxAge: PKCE_MAX_AGE,
+  const encodedVerifier = await jwt.encode({
     ...options.jwt,
-    token: { code_verifier: codeVerifier },
+    maxAge: PKCE_MAX_AGE,
+    token: { code_verifier },
   })
 
-  const cookieExpires = new Date()
-  cookieExpires.setTime(cookieExpires.getTime() + PKCE_MAX_AGE * 1000)
-
   logger.debug("CREATE_PKCE_CHALLENGE_VERIFIER", {
-    pkce: {
-      code_challenge: codeChallenge,
-      code_verifier: codeVerifier,
-    },
-    method: PKCE_CODE_CHALLENGE_METHOD,
+    code_challenge,
+    code_challenge_method: PKCE_CODE_CHALLENGE_METHOD,
+    code_verifier,
+    PKCE_MAX_AGE,
   })
+
   return {
+    code_challenge,
+    code_challenge_method: PKCE_CODE_CHALLENGE_METHOD,
     cookie: {
       name: cookies.pkceCodeVerifier.name,
-      value: encryptedCodeVerifier,
-      options: {
-        expires: cookieExpires.toISOString(),
-        ...cookies.pkceCodeVerifier.options,
-      },
+      value: encodedVerifier,
+      options: { ...cookies.pkceCodeVerifier.options, expires },
     },
-    code_challenge: codeChallenge,
-    code_challenge_method: PKCE_CODE_CHALLENGE_METHOD,
   }
 }
 
 /**
  * Returns code_verifier if provider uses PKCE,
- * and clears the cookie afterwards.
+ * and clears the container cookie afterwards.
  */
-export async function usePKCECodeVerifier(params: {
+export async function usePKCECodeVerifier(
+  codeVerifier: string | undefined,
   options: InternalOptions<"oauth">
-  codeVerifier?: string
-}): Promise<
-  | {
-      codeVerifier?: string
-      cookie?: Cookie
-    }
-  | undefined
-> {
-  const { options, codeVerifier } = params
+): Promise<{ codeVerifier: string; cookie: Cookie } | undefined> {
   const { cookies, provider } = options
 
   if (!provider?.checks?.includes("pkce") || !codeVerifier) {
     return
   }
 
-  const pkce = await jwt.decode({
+  const pkce = (await jwt.decode({
     ...options.jwt,
     token: codeVerifier,
-  })
-
-  // remove PKCE cookie after it has been used up
-  const cookie: Cookie = {
-    name: cookies.pkceCodeVerifier.name,
-    value: "",
-    options: {
-      ...cookies.pkceCodeVerifier.options,
-      maxAge: 0,
-    },
-  }
+  })) as any
 
   return {
-    codeVerifier: (pkce?.code_verifier as any) ?? undefined,
-    cookie,
+    codeVerifier: pkce?.code_verifier ?? undefined,
+    cookie: {
+      name: cookies.pkceCodeVerifier.name,
+      value: "",
+      options: { ...cookies.pkceCodeVerifier.options, maxAge: 0 },
+    },
   }
 }

src/core/lib/oauth/state-handler.ts

@@ -1,33 +1,63 @@
-import { createHash } from "crypto"
-import { InternalOptions } from "src/lib/types"
+import { generators } from "openid-client"
 
-/** Returns state if provider supports it */
-export function createState(options: InternalOptions<"oauth">) {
-  const { csrfToken, logger, provider } = options
+import type { InternalOptions } from "src/lib/types"
+import type { Cookie } from "../cookie"
+
+const STATE_MAX_AGE = 60 * 15 // 15 minutes in seconds
+
+/** Returns state if the provider supports it */
+export async function createState(
+  options: InternalOptions<"oauth">
+): Promise<{ cookie: Cookie; value: string } | undefined> {
+  const { logger, provider, jwt, cookies } = options
 
   if (!provider.checks?.includes("state")) {
     // Provider does not support state, return nothing
     return
   }
 
-  if (!csrfToken) {
-    logger.warn("NO_CSRF_TOKEN")
-    return
-  }
+  const state = generators.state()
+
+  const encodedState = await jwt.encode({
+    ...jwt,
+    maxAge: STATE_MAX_AGE,
+    token: { state },
+  })
 
-  // A hash of the NextAuth.js CSRF token is used as the state
-  const state = createHash("sha256").update(csrfToken).digest("hex")
+  logger.debug("CREATE_STATE", { state, maxAge: STATE_MAX_AGE })
 
-  logger.debug("OAUTH_CALLBACK_PROTECTION", { state, csrfToken })
-  return state
+  const expires = new Date()
+  expires.setTime(expires.getTime() + STATE_MAX_AGE * 1000)
+  return {
+    value: state,
+    cookie: {
+      name: cookies.state.name,
+      value: encodedState,
+      options: { ...cookies.state.options, expires },
+    },
+  }
 }
 
 /**
- * Consistently recreate state from the csrfToken
- * if `provider.checks` supports `"state"`.
+ * Returns state from if the provider supports states,
+ * and clears the container cookie afterwards.
  */
-export function getState({ provider, csrfToken }: InternalOptions<"oauth">) {
-  if (provider?.checks?.includes("state") && csrfToken) {
-    return createHash("sha256").update(csrfToken).digest("hex")
+export async function useState(
+  state: string | undefined,
+  options: InternalOptions<"oauth">
+): Promise<{ value: string; cookie: Cookie } | undefined> {
+  const { cookies, provider, jwt } = options
+
+  if (!provider.checks?.includes("state") || !state) return
+
+  const value = (await jwt.decode({ ...options.jwt, token: state })) as any
+
+  return {
+    value: value?.state ?? undefined,
+    cookie: {
+      name: cookies.state.name,
+      value: "",
+      options: { ...cookies.pkceCodeVerifier.options, maxAge: 0 },
+    },
   }
 }