don't rely on /logout and clear ory_session cookie on sign-out

Author: drankou

src/app/api/auth/sign-out/route.ts

@@ -2,18 +2,44 @@ import 'server-only'
 
 import { type NextRequest, NextResponse } from 'next/server'
 import { signOut } from '@/core/server/auth'
-import { orySessionCookieDeleteOptions } from '@/core/server/auth/ory/session-cookie'
+import {
+  orySessionCookieDeleteOptions,
+  resolveSessionCookieDomain,
+} from '@/core/server/auth/ory/session-cookie'
 
-// Sign-out is a plain route handler. It reads the id_token from e2b_session to
-// build Hydra's RP-logout URL, then clears the cookie on the redirect it emits
-// (before handing off to Hydra, which ends the Ory + Kratos sessions). The
-// client hard-navigates here so the logout overlay stays up until the document
-// unloads.
+// Ory's identity session cookie: `ory_kratos_session` self-hosted,
+// `ory_session_<slug>` on Ory Network. Deliberately excludes Hydra's
+// `ory_hydra_session*` cookie, which the RP-logout redirect ends, not us.
+const ORY_IDENTITY_SESSION_COOKIE = /^ory_(kratos_)?session/
+
+// Sign-out is a plain route handler the client hard-navigates to, so the logout
+// overlay stays up until the document unloads. signOut() revokes the Kratos
+// identity session server-side and the redirect ends Hydra's OAuth2 session; we
+// drop the browser cookies here — e2b_session (our token cache) and the Ory
+// identity cookie, which Hydra's logout leaves in place.
 export async function GET(request: NextRequest) {
   const { redirectTo } = await signOut({ origin: request.nextUrl.origin })
   const response = NextResponse.redirect(
     new URL(redirectTo, request.nextUrl.origin)
   )
   response.cookies.delete(orySessionCookieDeleteOptions(request.nextUrl.host))
+  clearOryIdentitySessionCookies(request, response)
   return response
 }
+
+// The cookie name and scope depend on the deployment, so clear whatever Ory
+// identity cookie the browser actually sent, scoped the same way it was issued:
+// parent-domain on Ory Network (how the @ory/nextjs proxy sets it, e.g.
+// `.e2b-staging.dev`), host-only otherwise.
+function clearOryIdentitySessionCookies(
+  request: NextRequest,
+  response: NextResponse
+): void {
+  const domain = resolveSessionCookieDomain(request.nextUrl.host)
+  for (const { name } of request.cookies.getAll()) {
+    if (!ORY_IDENTITY_SESSION_COOKIE.test(name)) continue
+    response.cookies.delete(
+      domain ? { name, path: '/', domain } : { name, path: '/' }
+    )
+  }
+}

src/core/server/auth/ory/kratos-session.ts

@@ -5,20 +5,6 @@ import { l, serializeErrorForLog } from '@/core/shared/clients/logger/logger'
 import { getOryIdentityApi } from './client'
 import { readOryError } from './ory-error'
 
-/**
- * Revokes every Kratos identity session for the given identity.
- *
- * Hydra's /oauth2/sessions/logout only ends the OAuth2 session; the Kratos
- * identity cookie on the Ory domain is independent and is what causes the
- * Account Experience to show "Reauthenticate as <last user>" on the next
- * sign-in instead of a fresh provider chooser.
- *
- * We can't surgically target a single session because the OIDC `sid` claim
- * from Hydra is Hydra's own OAuth2 session id, not a Kratos session id, and
- * we don't have access to the user's Kratos cookie from this side. Revoking
- * all identity sessions matches the expected "sign out of identity provider"
- * semantics anyway.
- */
 // Ory uses optimistic locking on identity rows; concurrent writes (e.g. our
 // admin DELETE racing with Hydra's RP-initiated logout cleanup during the
 // same signout flow) return 429 with reason "Conflicting concurrent
@@ -27,12 +13,48 @@ import { readOryError } from './ory-error'
 const REVOKE_MAX_ATTEMPTS = 3
 const REVOKE_BACKOFF_MS = 150
 
+/**
+ * Revokes a single Kratos session by its session id (admin DELETE
+ * /admin/sessions/{id}).
+ *
+ * This is the server-side equivalent of the browser self-service logout: it
+ * ends only the current session, preserving single sign-out semantics. We call
+ * it on sign-out because Hydra's /oauth2/sessions/logout skips the dashboard
+ * /logout -> Kratos bridge whenever Hydra holds no active authentication
+ * session (the production default, where the login is accepted with
+ * remember=false), which would otherwise leave the Kratos identity session
+ * alive and surface "Reauthenticate as <last user>" on the next sign-in.
+ */
+export async function revokeKratosSession(sessionId: string): Promise<void> {
+  await revokeWithRetries('revoke_kratos_session', () =>
+    getOryIdentityApi().disableSession({ id: sessionId })
+  )
+}
+
+/**
+ * Revokes every Kratos identity session for the given identity (admin DELETE
+ * /admin/identities/{id}/sessions).
+ *
+ * Used after a credential change, where signing out every device is the
+ * intended "sign out of identity provider" behavior. The OIDC `sid` claim from
+ * Hydra is Hydra's own OAuth2 session id, not a Kratos session id, so
+ * single-session targeting isn't available on that path.
+ */
 export async function revokeKratosSessionsForIdentity(
   identityId: string
+): Promise<void> {
+  await revokeWithRetries('revoke_kratos_sessions', () =>
+    getOryIdentityApi().deleteIdentitySessions({ id: identityId })
+  )
+}
+
+async function revokeWithRetries(
+  operation: string,
+  run: () => Promise<unknown>
 ): Promise<void> {
   for (let attempt = 1; attempt <= REVOKE_MAX_ATTEMPTS; attempt++) {
     try {
-      await getOryIdentityApi().deleteIdentitySessions({ id: identityId })
+      await run()
       return
     } catch (error) {
       if (error instanceof ResponseError && error.response.status === 404) {
@@ -53,11 +75,11 @@ export async function revokeKratosSessionsForIdentity(
 
       l.error(
         {
-          key: 'auth_provider:revoke_kratos_sessions:error',
+          key: `auth_provider:${operation}:error`,
           context: { ory: oryDetails, attempt },
           error: serializeErrorForLog(error),
         },
-        'failed to revoke Kratos sessions; user may see reauth UX on next sign-in'
+        'failed to revoke Kratos session(s); user may see reauth UX on next sign-in'
       )
       return
     }

src/core/server/auth/ory/session.ts

@@ -22,7 +22,10 @@ import {
 import { oryAuthFlows } from './flows'
 import { isKratosSessionFresh } from './freshness'
 import { fromKratosSessionIdentity, fromOryIdentity } from './identity'
-import { revokeKratosSessionsForIdentity } from './kratos-session'
+import {
+  revokeKratosSession,
+  revokeKratosSessionsForIdentity,
+} from './kratos-session'
 import { revokeOryOAuthSessionsForSubject } from './oauth-session'
 import {
   E2B_SESSION_COOKIE,
@@ -68,6 +71,19 @@ export async function getUserProfile(): Promise<AuthUser | null> {
 export async function signOut(
   options?: SignOutOptions
 ): Promise<SignOutResult> {
+  // Hydra's RP-initiated logout only runs the /logout -> Kratos bridge when
+  // Hydra still holds an active authentication session. In production the login
+  // is accepted with remember=false (non-persistent sessions), so Hydra
+  // short-circuits to post_logout_redirect_uri and the bridge never fires,
+  // leaving the Kratos identity session alive. Revoke just this session
+  // server-side here so sign-out is deterministic across environments without
+  // signing the user out of their other devices; the redirect below still ends
+  // Hydra's OAuth2 session.
+  const sessionId = (await readKratosSession())?.id
+  if (sessionId) {
+    await revokeKratosSession(sessionId)
+  }
+
   return {
     redirectTo: await completeOrySignOut(options?.origin),
   }

tests/integration/auth-ory-account-security.test.ts

@@ -7,6 +7,7 @@ const updateIdentityMock = vi.hoisted(() => vi.fn())
 const patchIdentityMock = vi.hoisted(() => vi.fn())
 const revokeOAuthSessionsMock = vi.hoisted(() => vi.fn())
 const revokeKratosSessionsMock = vi.hoisted(() => vi.fn())
+const revokeKratosSessionMock = vi.hoisted(() => vi.fn())
 const openOrySessionMock = vi.hoisted(() => vi.fn())
 const cookieDeleteMock = vi.hoisted(() => vi.fn())
 
@@ -50,6 +51,7 @@ vi.mock('@/core/server/auth/ory/oauth-session', () => ({
 
 vi.mock('@/core/server/auth/ory/kratos-session', () => ({
   revokeKratosSessionsForIdentity: revokeKratosSessionsMock,
+  revokeKratosSession: revokeKratosSessionMock,
 }))
 
 vi.mock('@/core/shared/clients/logger/logger', () => ({
@@ -73,11 +75,14 @@ const currentIdentity = {
 function kratosSession({
   authenticatedAt = new Date(),
   identityId = 'kratos-uuid',
+  sessionId = 'kratos-session-id',
 }: {
   authenticatedAt?: Date
   identityId?: string
+  sessionId?: string
 } = {}) {
   return {
+    id: sessionId,
     active: true,
     authenticated_at: authenticatedAt,
     identity: {
@@ -96,6 +101,7 @@ describe('Ory account security (Kratos session + e2b_session)', () => {
     patchIdentityMock.mockReset().mockResolvedValue(undefined)
     revokeOAuthSessionsMock.mockReset().mockResolvedValue(undefined)
     revokeKratosSessionsMock.mockReset().mockResolvedValue(undefined)
+    revokeKratosSessionMock.mockReset().mockResolvedValue(undefined)
     openOrySessionMock.mockReset().mockResolvedValue({
       accessToken: 'hydra-access-token',
       idToken: 'hydra-id-token',
@@ -179,16 +185,32 @@ describe('Ory account security (Kratos session + e2b_session)', () => {
     })
   })
 
-  it('signs out via Hydra RP-logout using the id_token hint', async () => {
+  it('signs out via Hydra RP-logout and revokes only the current Kratos session', async () => {
+    getServerSessionMock.mockResolvedValue(kratosSession())
+
     const result = await signOut({ origin: 'https://app.e2b.dev' })
 
     expect(result.redirectTo).toContain(
       'https://ory.example.com/oauth2/sessions/logout'
     )
     expect(result.redirectTo).toContain('id_token_hint=hydra-id-token')
     expect(result.redirectTo).toContain('post_logout_redirect_uri=')
-    // Single sign-out must not revoke every session.
+    // Revoke this session server-side so logout works even when Hydra skips the
+    // /logout -> Kratos bridge (no active authentication session in production).
+    expect(revokeKratosSessionMock).toHaveBeenCalledWith('kratos-session-id')
+    // ...but single sign-out must not revoke every device's session.
     expect(revokeKratosSessionsMock).not.toHaveBeenCalled()
     expect(revokeOAuthSessionsMock).not.toHaveBeenCalled()
   })
+
+  it('still signs out via Hydra RP-logout when no Kratos session is readable', async () => {
+    getServerSessionMock.mockResolvedValue(undefined)
+
+    const result = await signOut({ origin: 'https://app.e2b.dev' })
+
+    expect(result.redirectTo).toContain(
+      'https://ory.example.com/oauth2/sessions/logout'
+    )
+    expect(revokeKratosSessionMock).not.toHaveBeenCalled()
+  })
 })

tests/integration/auth-ory-sign-out.test.ts

@@ -0,0 +1,68 @@
+import { NextRequest } from 'next/server'
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
+
+const signOutMock = vi.hoisted(() => vi.fn())
+
+vi.mock('@/core/server/auth', () => ({
+  signOut: signOutMock,
+}))
+
+const { GET } = await import('@/app/api/auth/sign-out/route')
+
+// A cleared cookie carries an immediate expiry; Next emits Max-Age=0 and/or an
+// epoch Expires depending on version, so accept either.
+function clearedCookie(response: Response, name: string): string | undefined {
+  return response.headers
+    .getSetCookie()
+    .find(
+      (header) =>
+        header.startsWith(`${name}=`) &&
+        /(max-age=0|expires=thu, 01 jan 1970)/i.test(header)
+    )
+}
+
+describe('sign-out route cookie clearing', () => {
+  beforeEach(() => {
+    signOutMock
+      .mockReset()
+      .mockResolvedValue({ redirectTo: 'https://app.e2b.dev/' })
+  })
+
+  afterEach(() => {
+    vi.unstubAllEnvs()
+  })
+
+  it('clears the Ory Network identity cookie scoped to the parent domain', async () => {
+    vi.stubEnv('NEXT_PUBLIC_E2B_DOMAIN', 'e2b-staging.dev')
+
+    const response = await GET(
+      new NextRequest('https://e2b-staging.dev/api/auth/sign-out', {
+        headers: {
+          cookie:
+            'ory_session_abcdef=tok; e2b_session=sealed; ory_hydra_session_dev=h',
+        },
+      })
+    )
+
+    const oryClear = clearedCookie(response, 'ory_session_abcdef')
+    expect(oryClear).toBeDefined()
+    expect(oryClear).toMatch(/Domain=\.e2b-staging\.dev/i)
+
+    // Our own token cache is dropped too.
+    expect(clearedCookie(response, 'e2b_session')).toBeDefined()
+    // Hydra's session cookie is ended by the RP-logout redirect, not here.
+    expect(clearedCookie(response, 'ory_hydra_session_dev')).toBeUndefined()
+  })
+
+  it('clears the self-hosted ory_kratos_session cookie host-only', async () => {
+    const response = await GET(
+      new NextRequest('http://localhost:3000/api/auth/sign-out', {
+        headers: { cookie: 'ory_kratos_session=tok' },
+      })
+    )
+
+    const oryClear = clearedCookie(response, 'ory_kratos_session')
+    expect(oryClear).toBeDefined()
+    expect(oryClear).not.toMatch(/Domain=/i)
+  })
+})