fix(OIDC): typo + make PKCE and nonce mandatory as per specs (#491)

Co-authored-by: Sébastien Chopin <seb@nuxt.com>

Author: sandros94

src/runtime/server/lib/oauth/oidc.ts

@@ -6,7 +6,7 @@ import { createError, eventHandler, getQuery, sendRedirect } from 'h3'
 import type { QueryObject } from 'ufo'
 import { withQuery } from 'ufo'
 import type { RequestAccessTokenBody } from '../utils'
-import { getOAuthRedirectURL, handleAccessTokenErrorResponse, handleInvalidState, handleMissingConfiguration, handlePkceVerifier, handleState, requestAccessToken } from '../utils'
+import { getOAuthRedirectURL, handleAccessTokenErrorResponse, handleInvalidState, handleMissingConfiguration, handlePkceVerifier, handleState, requestAccessToken, handleNonce, parseJwt } from '../utils'
 
 export interface OAuthOidcConfig {
   /**
@@ -17,7 +17,6 @@ export interface OAuthOidcConfig {
   clientId?: string
   /**
    * OAuth Client secret.
-   * If unset, PKCE will be used where no client secret is needed.
    *
    * @default process.env.NUXT_OAUTH_OIDC_CLIENT_SECRET
    */
@@ -269,8 +268,9 @@ export function defineOAuthOidcEventHandler<TUser = OidcUser>({ config, onSucces
     const redirectURL = config.redirectURL || getOAuthRedirectURL(event)
     const state = await handleState(event)
 
-    // if no client secret is provided, we will use PKCE so no client secret is needed
-    const verifier = !config.clientSecret ? await handlePkceVerifier(event) : undefined
+    // We always use PKCE to mitigate man-in-the-middle attacks
+    const verifier = await handlePkceVerifier(event)
+    const nonce = handleNonce(event)
 
     if (!query.code) {
       config.scope = config.scope || []
@@ -280,17 +280,13 @@ export function defineOAuthOidcEventHandler<TUser = OidcUser>({ config, onSucces
         redirect_uri: redirectURL,
         scope: config.scope.join(' '),
         state,
+        nonce,
         response_type: 'code',
         ...config.params?.authorization_endpoint,
       }
 
-      // when using PKCE, we need to set the code_challenge in the request
-      // since some OIDC providers fail with an error if those params are set with "undefined" value
-      // we make sure to only include them at all if they are set
-      if (verifier) {
-        authQuery.code_challenge = verifier.code_challenge
-        authQuery.code_challenge_method = verifier.code_challenge_method
-      }
+      authQuery.code_challenge = verifier.code_challenge
+      authQuery.code_challenge_method = verifier.code_challenge_method
 
       return sendRedirect(event, withQuery(oidcConfig.authorization_endpoint, authQuery),
       )
@@ -306,16 +302,10 @@ export function defineOAuthOidcEventHandler<TUser = OidcUser>({ config, onSucces
       client_secret: config.clientSecret,
       redirect_uri: redirectURL,
       code: query.code,
+      code_verifier: verifier.code_verifier,
       ...config.params?.token_endpoint,
     }
 
-    // when using PKCE, we need to set the code_challenge in the request
-    // since some OIDC providers fail with an error if those params are set with "undefined" value
-    // we make sure to only include them at all if they are set
-    if (verifier) {
-      tokenQuery.code_verifier = verifier.code_verifier
-    }
-
     const tokens = await requestAccessToken<OidcTokens & { error?: unknown }>(oidcConfig.token_endpoint, {
       body: tokenQuery,
     })
@@ -324,6 +314,18 @@ export function defineOAuthOidcEventHandler<TUser = OidcUser>({ config, onSucces
       return handleAccessTokenErrorResponse(event, 'oidc', tokens, onError)
     }
 
+    if (tokens.id_token) {
+      const claims = parseJwt(tokens.id_token)
+      if (claims.nonce !== nonce) {
+        const error = createError({
+          statusCode: 401,
+          message: 'OIDC login failed: nonce mismatch',
+        })
+        if (!onError) throw error
+        return onError(event, error)
+      }
+    }
+
     let user = {} as TUser
 
     // some OIDC providers do not support a userinfo endpoint so we only call it when its defined inside the OIDC config

src/runtime/server/lib/utils.ts

@@ -239,3 +239,28 @@ export async function handleState(event: H3Event) {
   })
   return state
 }
+
+export function parseJwt(token: string) {
+  return jose.decodeJwt(token)
+}
+
+export function handleNonce(event: H3Event) {
+  const query = getQuery<{ code?: string }>(event)
+  // If the code is in the query, get the nonce from the cookie and delete the cookie
+  if (query.code) {
+    const nonce = getCookie(event, 'nuxt-auth-nonce')
+    deleteCookie(event, 'nuxt-auth-nonce')
+    return nonce
+  }
+
+  // If the code is not in the query, generate a new nonce and set it in the cookie
+  const nonce = encodeBase64Url(getRandomBytes(8))
+  setCookie(event, 'nuxt-auth-nonce', nonce, {
+    httpOnly: true,
+    secure: !isDevelopment,
+    sameSite: 'lax',
+    maxAge: OAUTH_COOKIE_MAX_AGE,
+    path: '/',
+  })
+  return nonce
+}