feat(oidc-auth): restrict cookie paths (#709)

* feat(oidc-auth): restrict cookie paths Signed-off-by: Axel Meinhardt <[email protected]> * chore(oidc-auth): add documentation Signed-off-by: Axel Meinhardt <[email protected]> * fix(oidc-auth): add tests Signed-off-by: Axel Meinhardt <[email protected]> * refactor and format --------- Signed-off-by: Axel Meinhardt <[email protected]> Co-authored-by: Yusuke Wada <[email protected]>
honojs · Aug 25, 2024 · cd99b40 · cd99b40
1 parent 588b0eb
commit cd99b40
Show file tree

Hide file tree

Showing 7 changed files with 90 additions and 24 deletions.
diff --git a/.changeset/kind-dolphins-itch.md b/.changeset/kind-dolphins-itch.md
@@ -0,0 +1,5 @@
+---
+'@hono/oidc-auth': minor
+---
+
+Optionally restrict cookie path with new envvar OIDC_COOKIE_PATH
diff --git a/.changeset/odd-islands-act.md b/.changeset/odd-islands-act.md
@@ -0,0 +1,5 @@
+---
+'@hono/oidc-auth': minor
+---
+
+Restrict path of callback cookies to pathname of OIDC_REDIRECT_URI
diff --git a/packages/oidc-auth/README.md b/packages/oidc-auth/README.md
@@ -52,6 +52,7 @@ The middleware requires the following environment variables to be set:
 | OIDC_CLIENT_ID             | The OAuth 2.0 client ID assigned to your application. This ID is used to identify your application to the OIDC provider. | None, must be provided |
 | OIDC_CLIENT_SECRET         | The OAuth 2.0 client secret assigned to your application. This secret is used to authenticate your application to the OIDC provider. | None, must be provided |
 | OIDC_REDIRECT_URI          | The URL to which the OIDC provider should redirect the user after authentication. This URL must be registered as a redirect URI in the OIDC provider. | None, must be provided |
+| OIDC_COOKIE_PATH          | The path to which the `oidc-auth` cookie is set. Restrict to not send it with every request to your domain | / |
 
 ## How to Use
 

diff --git a/packages/oidc-auth/package.json b/packages/oidc-auth/package.json
@@ -47,5 +47,8 @@
   },
   "dependencies": {
     "oauth4webapi": "^2.6.0"
+  },
+  "engines": {
+    "node": ">=18.0.0"
   }
 }
diff --git a/packages/oidc-auth/src/index.ts b/packages/oidc-auth/src/index.ts
@@ -4,7 +4,7 @@
 
 import type { Context, MiddlewareHandler } from 'hono'
 import { env } from 'hono/adapter'
-import { getCookie, setCookie, deleteCookie } from 'hono/cookie'
+import { deleteCookie, getCookie, setCookie } from 'hono/cookie'
 import { createMiddleware } from 'hono/factory'
 import { HTTPException } from 'hono/http-exception'
 import { sign, verify } from 'hono/jwt'
@@ -40,6 +40,7 @@ type OidcAuthEnv = {
   OIDC_CLIENT_ID: string
   OIDC_CLIENT_SECRET: string
   OIDC_REDIRECT_URI: string
+  OIDC_COOKIE_PATH?: string
 }
 
 /**
@@ -122,7 +123,7 @@ export const getAuth = async (c: Context): Promise<OidcAuth | null> => {
     try {
       auth = await verify(session_jwt, env.OIDC_AUTH_SECRET)
     } catch (e) {
-      deleteCookie(c, oidcAuthCookieName)
+      deleteCookie(c, oidcAuthCookieName, { path: env.OIDC_COOKIE_PATH ?? '/' })
       return null
     }
     if (auth === null || auth.rtkexp === undefined || auth.ssnexp === undefined) {
@@ -137,7 +138,7 @@ export const getAuth = async (c: Context): Promise<OidcAuth | null> => {
     if (auth.rtkexp < now) {
       // Refresh the token if it has expired
       if (auth.rtk === undefined || auth.rtk === '') {
-        deleteCookie(c, oidcAuthCookieName)
+        deleteCookie(c, oidcAuthCookieName, { path: env.OIDC_COOKIE_PATH ?? '/' })
         return null
       }
       const as = await getAuthorizationServer(c)
@@ -146,7 +147,7 @@ export const getAuth = async (c: Context): Promise<OidcAuth | null> => {
       const result = await oauth2.processRefreshTokenResponse(as, client, response)
       if (oauth2.isOAuth2Error(result)) {
         // The refresh_token might be expired or revoked
-        deleteCookie(c, oidcAuthCookieName)
+        deleteCookie(c, oidcAuthCookieName, { path: env.OIDC_COOKIE_PATH ?? '/' })
         return null
       }
       auth = await updateAuth(c, auth, result)
@@ -186,7 +187,11 @@ const updateAuth = async (
     ssnexp: orig?.ssnexp || Math.floor(Date.now() / 1000) + authExpires,
   }
   const session_jwt = await sign(updated, env.OIDC_AUTH_SECRET)
-  setCookie(c, oidcAuthCookieName, session_jwt, { path: '/', httpOnly: true, secure: true })
+  setCookie(c, oidcAuthCookieName, session_jwt, {
+    path: env.OIDC_COOKIE_PATH ?? '/',
+    httpOnly: true,
+    secure: true,
+  })
   c.set('oidcAuthJwt', session_jwt)
   return updated
 }
@@ -198,8 +203,8 @@ export const revokeSession = async (c: Context): Promise<void> => {
   const session_jwt = getCookie(c, oidcAuthCookieName)
   if (session_jwt !== undefined) {
     const env = getOidcAuthEnv(c)
-    deleteCookie(c, oidcAuthCookieName)
-    const auth: OidcAuth = await verify(session_jwt, env.OIDC_AUTH_SECRET)
+    deleteCookie(c, oidcAuthCookieName, { path: env.OIDC_COOKIE_PATH ?? '/' })
+    const auth = await verify(session_jwt, env.OIDC_AUTH_SECRET)
     if (auth.rtk !== undefined && auth.rtk !== '') {
       // revoke refresh token
       const as = await getAuthorizationServer(c)
@@ -215,7 +220,7 @@ export const revokeSession = async (c: Context): Promise<void> => {
       }
     }
   }
-  c.set('oidcAuth', undefined)
+  c.set('oidcAuth', null)
 }
 
 /**
@@ -273,7 +278,8 @@ export const processOAuthCallback = async (c: Context) => {
 
   // Parses the authorization response and validates the state parameter
   const state = getCookie(c, 'state')
-  deleteCookie(c, 'state')
+  const path = new URL(env.OIDC_REDIRECT_URI).pathname
+  deleteCookie(c, 'state', { path })
   const currentUrl: URL = new URL(c.req.url)
   const params = oauth2.validateAuthResponse(as, client, currentUrl, state)
   if (oauth2.isOAuth2Error(params)) {
@@ -285,11 +291,11 @@ export const processOAuthCallback = async (c: Context) => {
   // Exchanges the authorization code for a refresh token
   const code = c.req.query('code')
   const nonce = getCookie(c, 'nonce')
-  deleteCookie(c, 'nonce')
+  deleteCookie(c, 'nonce', { path })
   const code_verifier = getCookie(c, 'code_verifier')
-  deleteCookie(c, 'code_verifier')
+  deleteCookie(c, 'code_verifier', { path })
   const continue_url = getCookie(c, 'continue')
-  deleteCookie(c, 'continue')
+  deleteCookie(c, 'continue', { path })
   if (code === undefined || nonce === undefined || code_verifier === undefined) {
     throw new HTTPException(500, { message: 'Missing required parameters / cookies' })
   }
@@ -352,28 +358,33 @@ export const oidcAuthMiddleware = (): MiddlewareHandler => {
     try {
       const auth = await getAuth(c)
       if (auth === null) {
+        const path = new URL(env.OIDC_REDIRECT_URI).pathname
         // Redirect to IdP for login
         const state = oauth2.generateRandomState()
         const nonce = oauth2.generateRandomNonce()
         const code_verifier = oauth2.generateRandomCodeVerifier()
         const code_challenge = await oauth2.calculatePKCECodeChallenge(code_verifier)
         const url = await generateAuthorizationRequestUrl(c, state, nonce, code_challenge)
-        setCookie(c, 'state', state, { path: '/', httpOnly: true, secure: true })
-        setCookie(c, 'nonce', nonce, { path: '/', httpOnly: true, secure: true })
-        setCookie(c, 'code_verifier', code_verifier, { path: '/', httpOnly: true, secure: true })
-        setCookie(c, 'continue', c.req.url, { path: '/', httpOnly: true, secure: true })
+        setCookie(c, 'state', state, { path, httpOnly: true, secure: true })
+        setCookie(c, 'nonce', nonce, { path, httpOnly: true, secure: true })
+        setCookie(c, 'code_verifier', code_verifier, { path, httpOnly: true, secure: true })
+        setCookie(c, 'continue', c.req.url, { path, httpOnly: true, secure: true })
         return c.redirect(url)
       }
     } catch (e) {
-      deleteCookie(c, oidcAuthCookieName)
+      deleteCookie(c, oidcAuthCookieName, { path: env.OIDC_COOKIE_PATH ?? '/' })
       throw new HTTPException(500, { message: 'Invalid session' })
     }
     await next()
     c.res.headers.set('Cache-Control', 'private, no-cache')
     // Workaround to set the session cookie when the response is returned by the origin server
     const session_jwt = c.get('oidcAuthJwt')
     if (session_jwt !== undefined) {
-      setCookie(c, oidcAuthCookieName, session_jwt, { path: '/', httpOnly: true, secure: true })
+      setCookie(c, oidcAuthCookieName, session_jwt, {
+        path: env.OIDC_COOKIE_PATH ?? '/',
+        httpOnly: true,
+        secure: true,
+      })
     }
   })
 }
diff --git a/packages/oidc-auth/test/index.test.ts b/packages/oidc-auth/test/index.test.ts
@@ -168,11 +168,12 @@ app.all('/*', async (c) => {
 
 beforeEach(() => {
   process.env.OIDC_ISSUER = MOCK_ISSUER
-  ;(process.env.OIDC_CLIENT_ID = MOCK_CLIENT_ID),
-    (process.env.OIDC_CLIENT_SECRET = MOCK_CLIENT_SECRET),
-    (process.env.OIDC_REDIRECT_URI = MOCK_REDIRECT_URI)
+  process.env.OIDC_CLIENT_ID = MOCK_CLIENT_ID
+  process.env.OIDC_CLIENT_SECRET = MOCK_CLIENT_SECRET
+  process.env.OIDC_REDIRECT_URI = MOCK_REDIRECT_URI
   process.env.OIDC_AUTH_SECRET = MOCK_AUTH_SECRET
   process.env.OIDC_AUTH_EXPIRES = MOCK_AUTH_EXPIRES
+  delete process.env.OIDC_COOKIE_PATH
 })
 describe('oidcAuthMiddleware()', () => {
   test('Should respond with 200 OK if session is active', async () => {
@@ -228,7 +229,7 @@ describe('oidcAuthMiddleware()', () => {
     const res = await app.request(req, {}, {})
     expect(res).not.toBeNull()
     expect(res.status).toBe(302)
-    expect(res.headers.get('set-cookie')).toMatch('oidc-auth=;')
+    expect(res.headers.get('set-cookie')).toMatch(new RegExp('oidc-auth=; Max-Age=0; Path=/($|,)'))
   })
   test('Should delete session and redirect to authorization endpoint if the algorithm of the session JWT is invalid', async () => {
     const req = new Request('http://localhost/', {
@@ -238,7 +239,7 @@ describe('oidcAuthMiddleware()', () => {
     const res = await app.request(req, {}, {})
     expect(res).not.toBeNull()
     expect(res.status).toBe(302)
-    expect(res.headers.get('set-cookie')).toMatch('oidc-auth=;')
+    expect(res.headers.get('set-cookie')).toMatch(new RegExp('oidc-auth=; Max-Age=0; Path=/($|,)'))
   })
 })
 describe('processOAuthCallback()', () => {
@@ -250,8 +251,35 @@ describe('processOAuthCallback()', () => {
       },
     })
     const res = await app.request(req, {}, {})
+    const path = new URL(MOCK_REDIRECT_URI).pathname
     expect(res).not.toBeNull()
     expect(res.status).toBe(302)
+    expect(res.headers.get('set-cookie')).toMatch(new RegExp(`state=; Max-Age=0; Path=${path}($|,)`))
+    expect(res.headers.get('set-cookie')).toMatch(new RegExp(`nonce=; Max-Age=0; Path=${path}($|,)`))
+    expect(res.headers.get('set-cookie')).toMatch(new RegExp(`code_verifier=; Max-Age=0; Path=${path}($|,)`))
+    expect(res.headers.get('set-cookie')).toMatch(new RegExp(`continue=; Max-Age=0; Path=${path}($|,)`))
+    expect(res.headers.get('set-cookie')).toMatch(new RegExp('oidc-auth=[^;]+; Path=/; HttpOnly; Secure'))
+    expect(res.headers.get('location')).toBe('http://localhost/1234')
+  })
+  test('Should restrict the auth cookie to a given path', async () => {
+    const MOCK_COOKIE_PATH = process.env.OIDC_COOKIE_PATH = '/some/subpath/for/authentication'
+    process.env.OIDC_REDIRECT_URI = `http://localhost${MOCK_COOKIE_PATH}/callback`
+    const parentApp = new Hono().route(MOCK_COOKIE_PATH, app)
+    const path = new URL(process.env.OIDC_REDIRECT_URI).pathname
+    const req = new Request(`${process.env.OIDC_REDIRECT_URI}?code=1234&state=${MOCK_STATE}`, {
+      method: 'GET',
+      headers: {
+        cookie: `state=${MOCK_STATE}; nonce=${MOCK_NONCE}; code_verifier=1234; continue=http%3A%2F%2Flocalhost%2F1234`,
+      },
+    })
+    const res = await parentApp.request(req, {}, {})
+    expect(res).not.toBeNull()
+    expect(res.status).toBe(302)
+    expect(res.headers.get('set-cookie')).toMatch(new RegExp(`state=; Max-Age=0; Path=${path}($|,)`))
+    expect(res.headers.get('set-cookie')).toMatch(new RegExp(`nonce=; Max-Age=0; Path=${path}($|,)`))
+    expect(res.headers.get('set-cookie')).toMatch(new RegExp(`code_verifier=; Max-Age=0; Path=${path}($|,)`))
+    expect(res.headers.get('set-cookie')).toMatch(new RegExp(`continue=; Max-Age=0; Path=${path}($|,)`))
+    expect(res.headers.get('set-cookie')).toMatch(new RegExp(`oidc-auth=[^;]+; Path=${process.env.OIDC_COOKIE_PATH}; HttpOnly; Secure`))
     expect(res.headers.get('location')).toBe('http://localhost/1234')
   })
   test('Should return an error if the state parameter does not match', async () => {
@@ -298,6 +326,18 @@ describe('RevokeSession()', () => {
     const res = await app.request(req, {}, {})
     expect(res).not.toBeNull()
     expect(res.status).toBe(200)
-    expect(res.headers.get('set-cookie')).toMatch('oidc-auth=;')
+    expect(res.headers.get('set-cookie')).toMatch(new RegExp('oidc-auth=; Max-Age=0; Path=/($|,)'))
+  })
+  test('Should revoke the session of the given path', async () => {
+    const MOCK_COOKIE_PATH = process.env.OIDC_COOKIE_PATH = '/some/subpath/for/authentication'
+    const parentApp = new Hono().route(MOCK_COOKIE_PATH, app)
+    const req = new Request(`http://localhost${MOCK_COOKIE_PATH}/logout`, {
+      method: 'GET',
+      headers: { cookie: `oidc-auth=${MOCK_JWT_ACTIVE_SESSION}` },
+    })
+    const res = await parentApp.request(req, {}, {})
+    expect(res).not.toBeNull()
+    expect(res.status).toBe(200)
+    expect(res.headers.get('set-cookie')).toMatch(new RegExp(`oidc-auth=; Max-Age=0; Path=${MOCK_COOKIE_PATH}($|,)`))
   })
 })
diff --git a/packages/oidc-auth/tsconfig.json b/packages/oidc-auth/tsconfig.json
@@ -1,6 +1,7 @@
 {
   "extends": "../../tsconfig.json",
   "compilerOptions": {
+    "module": "ESNext",
     "rootDir": "./src",
     "outDir": "./dist",
   },