APIGW: support SLO without cookies, drop logout cookie

- Many browsers are starting to disable 3rd party cookies by default (even SameSite None) which breaks Single Logout with passport-saml that currently only supports logging out with a cookie
    - Browser details: https://webkit.org/blog/10218/full-third-party-cookie-blocking-and-more/ and https://blog.google/products/chrome/more-intuitive-privacy-and-security-controls-chrome/
    - passport-saml issues: node-saml/passport-saml#419 and node-saml/passport-saml#221
    - This is actually a real security issue as passport-saml will give a successful looking response from `passport.authenticate()` in the logout callback which results in IdPs showing "successfully logged out" messages to the user -- even though the user is still fully logged in!
    - This also causes an usability issue when the user tries to initate SLO from another application, fails to end the eVaka session, attempts a logout from eVaka and gets an ugly JSON error message as a response when APIGW attempts to make a LogoutRequest to the IdP that already ended the session -> will be fixed separately
- Other systems like Shibboleth get around the 3rd party cookie issue with Single Logout by not utilizing cookies at all for this but instead use the SAML nameID (and sessionIndex) properties presented in all SAML LogoutRequest messages
    - Source: https://wiki.shibboleth.net/confluence/display/DEV/IdP+SameSite+Testing#IdPSameSiteTesting-SameSiteandSingleLogout
- When logouts are always only done through SAML there's no need for the logout cookie itself but the idea is actually useful as an effective "secondary index" for Redis:
    - By storing a nameID + sessionIndex keyed item pointing to the session ID, we effectively create a second index that can be used with just the SAML LogoutRequest's nameID and sessionIndex properties

apigw/src/enduser/app.ts

@@ -38,7 +38,7 @@ app.use(passport.initialize())
 app.use(passport.session())
 passport.serializeUser<Express.User>((user, done) => done(null, user))
 passport.deserializeUser<Express.User>((user, done) => done(null, user))
-app.use(refreshLogoutToken('enduser'))
+app.use(refreshLogoutToken())
 setupLoggingMiddleware(app)
 
 function apiRouter() {

apigw/src/internal/app.ts

@@ -50,7 +50,7 @@ app.use(passport.initialize())
 app.use(passport.session())
 passport.serializeUser<Express.User>((user, done) => done(null, user))
 passport.deserializeUser<Express.User>((user, done) => done(null, user))
-app.use(refreshLogoutToken('employee'))
+app.use(refreshLogoutToken())
 setupLoggingMiddleware(app)
 
 app.use('/api/csp', csp)
@@ -67,7 +67,7 @@ function internalApiRouter() {
   router.all('/system/*', (req, res) => res.sendStatus(404))
 
   router.all('/auth/*', (req: express.Request, res, next) => {
-    if (req.session?.logoutToken?.idpProvider === 'evaka') {
+    if (req.session?.idpProvider === 'evaka') {
       req.url = req.url.replace('saml', 'evaka')
     }
     next()

apigw/src/internal/routes/auth-status.ts

@@ -10,6 +10,7 @@ import {
 } from '../../shared/service-client'
 import { logoutExpress } from '../../shared/session'
 import { fromCallback } from '../../shared/promise-utils'
+import { SAML } from 'passport-saml'
 
 export default toRequestHandler(async (req, res) => {
   const user = req.user
@@ -28,8 +29,8 @@ export default toRequestHandler(async (req, res) => {
           roles: [...globalRoles, ...allScopedRoles]
         })
       } else {
-        // device has been removed
-        await logoutExpress(req, res, 'employee')
+        // device has been removed (SAML config not relevant)
+        await logoutExpress(req, res, 'employee', new SAML({}))
         res.status(200).json({ loggedIn: false })
       }
     } else {

apigw/src/shared/auth/ad-saml.ts

@@ -63,6 +63,7 @@ async function verifyProfile(profile: AdProfile): Promise<SamlUser> {
 }
 
 export function createSamlConfig(): SamlConfig {
+  if (devLoginEnabled) return {}
   if (!adConfig) throw Error('Missing AD SAML configuration')
   return {
     callbackUrl: adConfig.callbackUrl,

apigw/src/shared/express.ts

@@ -11,7 +11,6 @@ export interface LogoutToken {
   // milliseconds value of a Date. Not an actual Date because it will be JSONified
   expiresAt: number
   value: string
-  idpProvider?: string | null
 }
 
 export type AsyncRequestHandler = (
@@ -52,6 +51,7 @@ export class InvalidRequest extends BaseError {}
 // Use TS interface merging to add fields to express req.session
 declare module 'express-session' {
   interface SessionData {
+    idpProvider?: string | null
     logoutToken?: LogoutToken
   }
 }

apigw/src/shared/passport-saml.d.ts

@@ -0,0 +1,32 @@
+// SPDX-FileCopyrightText: 2017-2021 City of Espoo
+//
+// SPDX-License-Identifier: LGPL-2.1-or-later
+
+import { ParsedQs } from 'qs'
+
+// TODO: Remove once using a passport-saml version with types matching the
+// actual implementation.
+declare module 'passport-saml' {
+  export class SAML {
+    constructor(options: SamlConfig)
+
+    validateRedirect(
+      container: ParsedQs,
+      originalQuery: string,
+      callback: (
+        err: Error | null,
+        profile?: Profile | null,
+        loggedOut?: boolean
+      ) => void
+    ): void
+
+    validatePostRequest(
+      container: Record<string, string>,
+      callback: (
+        err: Error | null,
+        profile?: Profile | null,
+        loggedOut?: boolean
+      ) => void
+    ): void
+  }
+}

apigw/src/shared/routes/auth/saml/index.ts

@@ -13,7 +13,7 @@ import { logoutExpress, saveLogoutToken } from '../../../session'
 import { fromCallback } from '../../../promise-utils'
 import { getEmployees } from '../../../dev-api'
 import _ from 'lodash'
-import { AuthenticateOptions } from 'passport-saml'
+import { AuthenticateOptions, SAML } from 'passport-saml'
 
 const urlencodedParser = urlencoded({ extended: false })
 
@@ -53,8 +53,7 @@ function getRedirectUrl(req: express.Request): string {
 }
 
 function createLoginHandler({
-  strategyName,
-  sessionType
+  strategyName
 }: SamlEndpointConfig): express.RequestHandler {
   return (req, res, next) => {
     logAuditEvent(
@@ -93,7 +92,7 @@ function createLoginHandler({
             req,
             'User logged in successfully'
           )
-          await saveLogoutToken(req, res, sessionType, strategyName)
+          await saveLogoutToken(req, strategyName)
           const redirectUrl = getRedirectUrl(req)
           logDebug(`Redirecting to ${redirectUrl}`, req, { redirectUrl })
           return res.redirect(redirectUrl)
@@ -115,10 +114,14 @@ function createLoginHandler({
 }
 
 function createLogoutHandler({
+  samlConfig,
   strategy,
   strategyName,
   sessionType
 }: SamlEndpointConfig): express.RequestHandler {
+  // For parsing SAML messages outside the strategy
+  const saml = new SAML(samlConfig)
+
   return toRequestHandler(async (req, res) => {
     logAuditEvent(
       `evaka.saml.${strategyName}.sign_out_requested`,
@@ -131,7 +134,7 @@ function createLogoutHandler({
           strategy.logout(req, cb)
         )
         logDebug('Logging user out from passport.', req)
-        await logoutExpress(req, res, sessionType)
+        await logoutExpress(req, res, sessionType, saml)
         return res.redirect(redirectUrl)
       } catch (err) {
         logAuditEvent(
@@ -148,7 +151,7 @@ function createLogoutHandler({
         'User signed out'
       )
       logDebug('Logging user out from passport.', req)
-      await logoutExpress(req, res, sessionType)
+      await logoutExpress(req, res, sessionType, saml)
       res.redirect(getRedirectUrl(req))
     }
   })
@@ -161,7 +164,9 @@ function createLogoutHandler({
 // * HTTP redirect: the browser makes a GET request with query parameters
 // * HTTP POST: the browser makes a POST request with URI-encoded form body
 export default function createSamlRouter(config: SamlEndpointConfig): Router {
-  const { strategyName, strategy, pathIdentifier } = config
+  const { strategyName, strategy, samlConfig, pathIdentifier } = config
+  // For parsing SAML messages outside the strategy
+  const saml = new SAML(samlConfig)
 
   passport.use(strategyName, strategy)
 
@@ -173,7 +178,7 @@ export default function createSamlRouter(config: SamlEndpointConfig): Router {
       req,
       'Logout callback called'
     )
-    await logoutExpress(req, res, config.sessionType)
+    await logoutExpress(req, res, config.sessionType, saml)
   })
 
   const router = Router()

apigw/src/shared/session.ts

@@ -2,7 +2,7 @@
 //
 // SPDX-License-Identifier: LGPL-2.1-or-later
 
-import express, { CookieOptions } from 'express'
+import express, { CookieOptions, Request } from 'express'
 import session from 'express-session'
 import connectRedis from 'connect-redis'
 import {
@@ -17,7 +17,6 @@ import {
   useSecureCookies
 } from './config'
 import redis from 'redis'
-import { v4 as uuidv4 } from 'uuid'
 import { logError } from './logging'
 import {
   addMinutes,
@@ -28,6 +27,7 @@ import {
 import { toMiddleware } from './express'
 import AsyncRedisClient from './async-redis-client'
 import { fromCallback } from './promise-utils'
+import { SAML, Profile } from 'passport-saml'
 
 export type SessionType = 'enduser' | 'employee'
 
@@ -63,70 +63,89 @@ const sessionCookieOptions: CookieOptions = {
   sameSite: 'lax'
 }
 
-const logoutCookieOptions: express.CookieOptions = {
-  path: '/',
-  httpOnly: true,
-  secure: useSecureCookies,
-  sameSite: useSecureCookies ? 'none' : undefined
-}
-
 function cookiePrefix(sessionType: SessionType) {
   return sessionType === 'enduser' ? 'evaka.eugw' : 'evaka.employee'
 }
 
-function logoutCookie(sessionType: SessionType) {
-  return `${cookiePrefix(sessionType)}.logout`
-}
-
 export function sessionCookie(sessionType: SessionType) {
   return `${cookiePrefix(sessionType)}.session`
 }
 
-export function refreshLogoutToken(sessionType: SessionType) {
-  return toMiddleware(async (req, res) => {
+export function refreshLogoutToken() {
+  return toMiddleware(async (req) => {
     if (!req.session) return
     if (!req.session.logoutToken) return
     if (!isDate(req.session.cookie.expires)) return
     const sessionExpires = req.session.cookie.expires as Date
     const logoutExpires = new Date(req.session.logoutToken.expiresAt)
-    const cookieToken = req.cookies && req.cookies[logoutCookie(sessionType)]
     // Logout token should always expire at least 30 minutes later than the session
-    if (
-      differenceInMinutes(logoutExpires, sessionExpires) < 30 ||
-      cookieToken !== req.session.logoutToken.value
-    ) {
-      await saveLogoutToken(
-        req,
-        res,
-        sessionType,
-        req.session.logoutToken.idpProvider
-      )
+    if (differenceInMinutes(logoutExpires, sessionExpires) < 30) {
+      await saveLogoutToken(req, req.session.idpProvider)
     }
   })
 }
 
+function logoutKey(nameID: string, sessionIndex?: string) {
+  return `slo:${nameID}.${sessionIndex}`
+}
+
+async function tryParseProfile(
+  req: Request,
+  saml: SAML
+): Promise<Profile | null | undefined> {
+  // NOTE: This duplicate parsing can be removed if passport-saml ever exposes
+  // an alternative for passport.authenticate() that either lets us hook into
+  // it before any redirects or separate XML parsing and authentication methods.
+  if (req.query?.SAMLRequest) {
+    // Redirects have signatures in the original query parameteru
+    const dummyOrigin = 'http://evaka'
+    const originalQuery = new URL(req.url, dummyOrigin).search.replace(
+      /^\?/,
+      ''
+    )
+    return await fromCallback<Profile | null | undefined>((cb) =>
+      saml.validateRedirect(req.query, originalQuery, cb)
+    )
+  } else if (req.body?.SAMLRequest) {
+    // POST logout callbacks have the signature in the message body directly
+    return await fromCallback<Profile | null | undefined>((cb) =>
+      saml.validatePostRequest(req.body, cb)
+    )
+  }
+}
+
+/**
+ * Save a logout token for a user session to be consumed during logout.
+ *
+ * The token is generated by creating an effective secondary
+ * index in Redis from SAML session identifiers (nameID and sessionIndex).
+ * This token can then be used with LogoutRequests without relying
+ * on 3rd party cookies which are starting to be disabled by default on many
+ * browsers, enabling Single Logout.
+ *
+ * This token can be removed if this passport-saml issue is ever fixed:
+ * https://github.com/node-saml/passport-saml/issues/419
+ */
 export async function saveLogoutToken(
   req: express.Request,
-  res: express.Response,
-  sessionType: SessionType,
   strategyName: string | null | undefined
 ): Promise<void> {
-  if (!req.session) return
+  if (!req.session || !req.user?.nameID) return
+
+  // Persist in session to allow custom logic per strategy
+  req.session.idpProvider = strategyName
+
+  if (!asyncRedisClient) return
+  const key = logoutKey(req.user.nameID, req.user.sessionIndex)
+
   const now = new Date()
   const expires = addMinutes(now, sessionTimeoutMinutes + 60)
-  const idpProvider = strategyName
   const logoutToken = {
     expiresAt: expires.valueOf(),
-    value: req.session.logoutToken ? req.session.logoutToken.value : uuidv4(),
-    idpProvider
+    value: req.session.logoutToken?.value || key
   }
   req.session.logoutToken = logoutToken
-  res.cookie(logoutCookie(sessionType), logoutToken.value, {
-    ...logoutCookieOptions,
-    expires
-  })
-  if (!asyncRedisClient) return
-  const key = `logout:${logoutToken.value}`
+
   const ttlSeconds = differenceInSeconds(expires, now)
   // https://redis.io/commands/expire - Set a timeout on key
   // Return value:
@@ -140,33 +159,43 @@ export async function saveLogoutToken(
   await asyncRedisClient.set(key, req.session.id, 'EX', ttlSeconds)
 }
 
-// If a logout token is available, delete it and delete the session it points to
-export async function consumeLogoutToken(
+async function consumeLogoutRequest(
   req: express.Request,
-  res: express.Response,
-  sessionType: SessionType
+  saml: SAML
 ): Promise<void> {
-  const token = req.cookies && req.cookies[logoutCookie(sessionType)]
-  if (!token || !asyncRedisClient) return
-  res.clearCookie(logoutCookie(sessionType), logoutCookieOptions)
-  const sid = await asyncRedisClient.get(`logout:${token}`)
+  if (!asyncRedisClient) return
+
+  const profile = await tryParseProfile(req, saml)
+  // Prefer details from the SAML message (profile) but fall back to details
+  // from the session in case a) this wasn't a SAMLRequest b) it's malformed
+  // to ensure the logout token is deleted from the store even in non-SLO cases.
+  const nameID = profile?.nameID ?? req.user?.nameID
+  const sessionIndex = profile?.sessionIndex ?? req.user?.sessionIndex
+
+  if (!nameID) return // Nothing to consume without a SAMLRequest or session
+
+  const key = logoutKey(nameID, sessionIndex)
+  const sid = await asyncRedisClient.get(key)
   if (sid) {
-    await asyncRedisClient.del(`sess:${sid}`, `logout:${token}`)
+    // Ensure both session and logout keys are cleared in case no cookies were
+    // available -> no req.session was available to be deleted.
+    await asyncRedisClient.del(`sess:${sid}`, key)
   }
 }
 
 export async function logoutExpress(
   req: express.Request,
   res: express.Response,
-  sessionType: SessionType
+  sessionType: SessionType,
+  saml: SAML
 ) {
   req.logout()
+  await consumeLogoutRequest(req, saml)
   if (req.session) {
     const session = req.session
     await fromCallback((cb) => session.destroy(cb))
   }
   res.clearCookie(sessionCookie(sessionType))
-  await consumeLogoutToken(req, res, sessionType)
 }
 
 export default (sessionType: SessionType) =>