diff --git a/apigw/src/enduser/app.ts b/apigw/src/enduser/app.ts index 893389ae692..2578006a322 100644 --- a/apigw/src/enduser/app.ts +++ b/apigw/src/enduser/app.ts @@ -38,7 +38,7 @@ app.use(passport.initialize()) app.use(passport.session()) passport.serializeUser((user, done) => done(null, user)) passport.deserializeUser((user, done) => done(null, user)) -app.use(refreshLogoutToken('enduser')) +app.use(refreshLogoutToken()) setupLoggingMiddleware(app) function apiRouter() { diff --git a/apigw/src/internal/app.ts b/apigw/src/internal/app.ts index e63fcafe506..e6f37147847 100644 --- a/apigw/src/internal/app.ts +++ b/apigw/src/internal/app.ts @@ -50,7 +50,7 @@ app.use(passport.initialize()) app.use(passport.session()) passport.serializeUser((user, done) => done(null, user)) passport.deserializeUser((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() diff --git a/apigw/src/internal/routes/auth-status.ts b/apigw/src/internal/routes/auth-status.ts index e9a60a63558..f2f452acf24 100644 --- a/apigw/src/internal/routes/auth-status.ts +++ b/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 { diff --git a/apigw/src/shared/auth/ad-saml.ts b/apigw/src/shared/auth/ad-saml.ts index 1dcbb788ed1..493055eea27 100755 --- a/apigw/src/shared/auth/ad-saml.ts +++ b/apigw/src/shared/auth/ad-saml.ts @@ -63,6 +63,7 @@ async function verifyProfile(profile: AdProfile): Promise { } export function createSamlConfig(): SamlConfig { + if (devLoginEnabled) return {} if (!adConfig) throw Error('Missing AD SAML configuration') return { callbackUrl: adConfig.callbackUrl, diff --git a/apigw/src/shared/express.ts b/apigw/src/shared/express.ts index bf2df0d45e6..c27b8d083ad 100644 --- a/apigw/src/shared/express.ts +++ b/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 } } diff --git a/apigw/src/shared/passport-saml.d.ts b/apigw/src/shared/passport-saml.d.ts new file mode 100755 index 00000000000..aa15c4e8fc5 --- /dev/null +++ b/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, + callback: ( + err: Error | null, + profile?: Profile | null, + loggedOut?: boolean + ) => void + ): void + } +} diff --git a/apigw/src/shared/routes/auth/saml/index.ts b/apigw/src/shared/routes/auth/saml/index.ts index d6f35be21b8..221236ad5e0 100755 --- a/apigw/src/shared/routes/auth/saml/index.ts +++ b/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() diff --git a/apigw/src/shared/session.ts b/apigw/src/shared/session.ts index 139e5b70f30..691f5937857 100644 --- a/apigw/src/shared/session.ts +++ b/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 { + // 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((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((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 { - 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,41 @@ 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 { - 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}`) + await asyncRedisClient.del(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) =>