diff --git a/apigw/.editorconfig b/apigw/.editorconfig new file mode 100644 index 00000000000..276d6698155 --- /dev/null +++ b/apigw/.editorconfig @@ -0,0 +1,13 @@ +# SPDX-FileCopyrightText: 2017-2021 City of Espoo +# +# SPDX-License-Identifier: LGPL-2.1-or-later + +root = true + +[*] +charset = utf-8 +indent_style = space +indent_size = 2 +end_of_line = lf +insert_final_newline = true +trim_trailing_whitespace = true diff --git a/apigw/config/test-cert/slo-test-idp-cert.pem b/apigw/config/test-cert/slo-test-idp-cert.pem new file mode 100644 index 00000000000..2c15670a90a --- /dev/null +++ b/apigw/config/test-cert/slo-test-idp-cert.pem @@ -0,0 +1,22 @@ +-----BEGIN CERTIFICATE----- +MIIDpzCCAo+gAwIBAgIUXeLWbrFuIwl9965EUMbgD/bwRXYwDQYJKoZIhvcNAQEL +BQAwYjELMAkGA1UEBhMCRkkxEDAOBgNVBAgMB1V1c2ltYWExDjAMBgNVBAcMBUVz +cG9vMRgwFgYDVQQKDA9Fc3Bvb24ga2F1cHVua2kxFzAVBgNVBAMMDmV2YWthLXNs +by10ZXN0MCAXDTIxMDUwNzA5NTAwOVoYDzIxMjEwNDEzMDk1MDA5WjBiMQswCQYD +VQQGEwJGSTEQMA4GA1UECAwHVXVzaW1hYTEOMAwGA1UEBwwFRXNwb28xGDAWBgNV +BAoMD0VzcG9vbiBrYXVwdW5raTEXMBUGA1UEAwwOZXZha2Etc2xvLXRlc3QwggEi +MA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQC8065qJdUM3YGJTUVjIXJB4CBt +/qSr28OIPAb2nOT++3nS8C4jsx+nuArx2kGlyF3DlyEQRlW44fMtnhO3uSvHQ33s +rsflw5kb0XysePoXLd8uxo2993aZGFzw/fK07PB52WuHFm+GL5EIptwQDe7M/qe9 +IbwtmFkFdMY67LkG4YwW5j7MHKD7jLPQFMqGCYG9jfIwtuPK2aEoTP1QYOrL4L6E +8bufk628u8lNQJZ6F5VvzRKO/NPIp5oC8A1vVh4OJJa2b98NY34ehv3siuqwtFcu +MTS8Ul6uEJ8+705k1c6OV6n1e/4d2NwkJWqjrRv2fLlsxSUucfQp3HGrckCFAgMB +AAGjUzBRMB0GA1UdDgQWBBSnWKQiasD8qm/VrhGA75gBeodcETAfBgNVHSMEGDAW +gBSnWKQiasD8qm/VrhGA75gBeodcETAPBgNVHRMBAf8EBTADAQH/MA0GCSqGSIb3 +DQEBCwUAA4IBAQBi2O3gfVeIIPcPMQawK2oqKth4/KireobEXiHwWrheomS6CEQG +rdzxt8r4aq25Y/Lj3vDZKo9aWxC0fM1heYxaaXn4F7GOJna/AJPMi4ccxv/OLtOA +Y9gUM58i5sLvUg+Z8UrN/HJTFpFx2yJsSE7UzAS+9Zh/iF/MCHmjTKJ4BkwFcTr3 +SG7Zo5nT+BrQlhxHSYGseouHyh8JfIjRVnLJ1C6LDuNUlTn7XY3ExtRm/hnqlUlc +XZ5WbtGWzCOodFGbRbVA6sommMeCtBoKp4o74GbqxpzkJel9q7CD0TVscmt8cvKV +jdw6/QQLeq8SQYo3UI/90HZPBW/3+UVrtcNi +-----END CERTIFICATE----- diff --git a/apigw/config/test-cert/slo-test-idp-key.pem b/apigw/config/test-cert/slo-test-idp-key.pem new file mode 100644 index 00000000000..3563e889681 --- /dev/null +++ b/apigw/config/test-cert/slo-test-idp-key.pem @@ -0,0 +1,28 @@ +-----BEGIN PRIVATE KEY----- +MIIEvgIBADANBgkqhkiG9w0BAQEFAASCBKgwggSkAgEAAoIBAQC8065qJdUM3YGJ +TUVjIXJB4CBt/qSr28OIPAb2nOT++3nS8C4jsx+nuArx2kGlyF3DlyEQRlW44fMt +nhO3uSvHQ33srsflw5kb0XysePoXLd8uxo2993aZGFzw/fK07PB52WuHFm+GL5EI +ptwQDe7M/qe9IbwtmFkFdMY67LkG4YwW5j7MHKD7jLPQFMqGCYG9jfIwtuPK2aEo +TP1QYOrL4L6E8bufk628u8lNQJZ6F5VvzRKO/NPIp5oC8A1vVh4OJJa2b98NY34e +hv3siuqwtFcuMTS8Ul6uEJ8+705k1c6OV6n1e/4d2NwkJWqjrRv2fLlsxSUucfQp +3HGrckCFAgMBAAECggEAPfgqkWOBHAvF602UrAfZ+4yWmAKuAEjLTvaEQoMTFCtr +u7JfMhAjH2PjE6RRTxsGyp3amAC9OUPODvaF+hGnMGoR9Y8Ww20B3oNNqzy4tsqz +KCK5edKw9WVtexmcgYwRD6wvAdJ3H06VBoXcStiHuncIjaV4oG4TKRs9wzDVOFBT +lN/3JPD8Q6E0hdpd9t+27s7caNOjhVo1x1xyLfX2sxC81BeXA6sUo3nmthWJyspr +l3ReMJsNSe7tpZQojxNh+4xQrVuuCRDt0ic2HLczFiHG0f5iHp4tXmRHRmfH2co+ +MwEfnFAGnUu4zUkb8y5asAYuW6IIzIpvfpgw5URTMQKBgQD7NiZS1zOlJLsnolP5 +tGBt/zgGeyNYKt53EfCle1i2mwwZLTK/+WPdtf+E/36AbcSnwkjq5IVW3SGWYIo0 +Awg9i5mie4QtT9wYAiz/gLfZ1tVlUbOvo3TY34h5Ata1IK23xXeuA0/NMPzDyRRy +UOECgmzZb3Ko4Aqe5beUP3tcrwKBgQDAbRwkdY0hHE/bCEwT4xDYiLSIEJtBDjnI +Os27c56xG447ThOAcVy0kolMQTvTrPVMZlbn20zOgxycOBAT8fJbjCH4ivL8Yl4i +UyN6HQOVPTkbbX8jJqsy6SBW3EScfCs7PT4H4QmLLa1RJIelw47yGVqwmIbaBSQD +B/ftTmJLCwKBgCQQ3SWtkdOW12vUSVwjQmjoaGG90hA5b2EG6VbIw67Lycvfila3 +dlgBZiLxD3deywoOwas/jckvzD+rsovPF6LGZRNHym069u1XeqBgGYUj69U1Cqgf +vonYZd6BwtOUUnx81DbecNmTu+Zb+xyCchuLIBeDgaGvMLcpYdbd2lcvAoGBAIeV +2fiOo6yq6FGrXP++RQZt/NbK7LpALdK6LHBinXSpt+RttSwRtIK/peKHLIKQIh99 +FMs2KL5yf9xLXHjRSDXdXaplLaVMIowJDLxkaTvk8bIzyxuXiZXL0i+h8O5aR5Ps +KSMgG7tnqfG8zZ+tVbGcz9wS/SHt8Vv5Z2ZcjsHVAoGBAIRniS4Opbh3EVcRMhsE +tFGzaErUt6ml3IIL0NljtipjMNWKzs9SduFgBMCdSjj5YH85vAq/sDyIr3imd2JB +e8icpm1kNue3BAuGjmpQFpHQR5AMchmjTezZYp8b9VdiEFHNPIhxrNL6WecFpesE +d/dGyI8q01wFiAmPn0fIPpa5 +-----END PRIVATE KEY----- diff --git a/apigw/package.json b/apigw/package.json index 4378332c246..a1d174f4c71 100644 --- a/apigw/package.json +++ b/apigw/package.json @@ -70,6 +70,9 @@ "@types/through2": "^2.0.34", "@types/tough-cookie": "^4.0.0", "@types/uuid": "^8.3.0", + "@types/xml-crypto": "^1.4.1", + "@types/xml2js": "^0.4.8", + "@types/xmldom": "^0.1.30", "@typescript-eslint/eslint-plugin": "^4.6.0", "@typescript-eslint/parser": "^4.6.0", "concurrently": "^5.3.0", @@ -86,7 +89,10 @@ "tough-cookie": "^4.0.0", "ts-jest": "^26.4.3", "ts-node": "^9.1.1", - "typescript": "^4.1.3" + "typescript": "^4.1.3", + "xml-crypto": "2.1.2", + "xml2js": "^0.4.23", + "xmldom": "^0.6.0" }, "resolutions": { "@types/node": "^14.14.6", @@ -152,7 +158,8 @@ "argsIgnorePattern": "^_", "varsIgnorePattern": "^_" } - ] + ], + "@typescript-eslint/no-floating-promises": "error" } }, "engines": { diff --git a/apigw/src/__mocks__/redis.ts b/apigw/src/__mocks__/redis.ts new file mode 100644 index 00000000000..7b1f5d05292 --- /dev/null +++ b/apigw/src/__mocks__/redis.ts @@ -0,0 +1,11 @@ +// SPDX-FileCopyrightText: 2017-2021 City of Espoo +// +// SPDX-License-Identifier: LGPL-2.1-or-later + +// Automatic mock for Jest that replaces all imports of redis with redis-mock. +// See: https://jestjs.io/docs/manual-mocks#mocking-node-modules for details + +import redis from 'redis-mock' + +export const createClient = redis.createClient +export default redis diff --git a/apigw/src/enduser/app.ts b/apigw/src/enduser/app.ts index a81f591c733..cc6d67f2811 100644 --- a/apigw/src/enduser/app.ts +++ b/apigw/src/enduser/app.ts @@ -2,16 +2,18 @@ // // SPDX-License-Identifier: LGPL-2.1-or-later -import bodyParser from 'body-parser' import cookieParser from 'cookie-parser' import express, { Router } from 'express' import helmet from 'helmet' import nocache from 'nocache' import passport from 'passport' import { requireAuthentication } from '../shared/auth' -import createEvakaCustomerSamlStrategy from '../shared/auth/customer-saml' -import createSuomiFiStrategy from '../shared/auth/suomi-fi-saml' -import { nodeEnv } from '../shared/config' +import createEvakaCustomerSamlStrategy, { + createSamlConfig as createEvakaCustomerSamlConfig +} from '../shared/auth/customer-saml' +import createSuomiFiStrategy, { + createSamlConfig as createSuomiFiSamlConfig +} from '../shared/auth/suomi-fi-saml' import setupLoggingMiddleware from '../shared/logging' import { csrf, csrfCookie } from '../shared/middleware/csrf' import { errorHandler } from '../shared/middleware/error-handler' @@ -25,8 +27,7 @@ import routes from './routes' import authStatus from './routes/auth-status' const app = express() -// TODO: How to make this more easily injectable/overridable in tests? -const redisClient = nodeEnv !== 'test' ? createRedisClient() : undefined +const redisClient = createRedisClient() trustReverseProxy(app) app.set('etag', false) app.use(nocache()) @@ -36,34 +37,38 @@ app.use( contentSecurityPolicy: false }) ) -app.get('/health', (req, res) => res.status(200).json({ status: 'UP' })) +app.get('/health', (_, res) => res.status(200).json({ status: 'UP' })) app.use(tracing) -app.use(bodyParser.json()) +app.use(express.json()) app.use(cookieParser()) app.use(session('enduser', redisClient)) 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() { const router = Router() router.use(publicRoutes) + const suomifiSamlConfig = createSuomiFiSamlConfig(redisClient) router.use( createSamlRouter({ strategyName: 'suomifi', - strategy: createSuomiFiStrategy(redisClient), + strategy: createSuomiFiStrategy(suomifiSamlConfig), + samlConfig: suomifiSamlConfig, sessionType: 'enduser', pathIdentifier: 'saml' }) ) + const evakaCustomerSamlConfig = createEvakaCustomerSamlConfig(redisClient) router.use( createSamlRouter({ strategyName: 'evaka-customer', - strategy: createEvakaCustomerSamlStrategy(redisClient), + strategy: createEvakaCustomerSamlStrategy(evakaCustomerSamlConfig), + samlConfig: evakaCustomerSamlConfig, sessionType: 'enduser', pathIdentifier: 'evaka-customer' }) @@ -79,3 +84,4 @@ app.use('/api/application', apiRouter()) app.use(errorHandler(false)) export default app +export const _TEST_ONLY_redisClient = redisClient diff --git a/apigw/src/internal/app.ts b/apigw/src/internal/app.ts index a5eaf34f287..40cce4ab4f6 100644 --- a/apigw/src/internal/app.ts +++ b/apigw/src/internal/app.ts @@ -2,16 +2,19 @@ // // SPDX-License-Identifier: LGPL-2.1-or-later -import bodyParser from 'body-parser' import cookieParser from 'cookie-parser' import express, { Router } from 'express' import helmet from 'helmet' import nocache from 'nocache' import passport from 'passport' import { requireAuthentication } from '../shared/auth' -import createAdSamlStrategy from '../shared/auth/ad-saml' -import createEvakaSamlStrategy from '../shared/auth/keycloak-saml' -import { cookieSecret, enableDevApi, nodeEnv } from '../shared/config' +import createAdSamlStrategy, { + createSamlConfig as createAdSamlConfig +} from '../shared/auth/ad-saml' +import createEvakaSamlStrategy, { + createSamlConfig as createEvakaSamlconfig +} from '../shared/auth/keycloak-saml' +import { cookieSecret, enableDevApi } from '../shared/config' import setupLoggingMiddleware from '../shared/logging' import { csrf, csrfCookie } from '../shared/middleware/csrf' import { errorHandler } from '../shared/middleware/error-handler' @@ -29,8 +32,7 @@ import mobileDeviceSession, { import authStatus from './routes/auth-status' const app = express() -// TODO: How to make this more easily injectable/overridable in tests? -const redisClient = nodeEnv !== 'test' ? createRedisClient() : undefined +const redisClient = createRedisClient() trustReverseProxy(app) app.set('etag', false) app.use(nocache()) @@ -40,51 +42,55 @@ app.use( contentSecurityPolicy: false }) ) -app.get('/health', (req, res) => res.status(200).json({ status: 'UP' })) +app.get('/health', (_, res) => res.status(200).json({ status: 'UP' })) app.use(tracing) -app.use(bodyParser.json({ limit: '8mb' })) +app.use(express.json({ limit: '8mb' })) app.use(session('employee', redisClient)) app.use(cookieParser(cookieSecret)) 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) function scheduledApiRouter() { const router = Router() - router.all('*', (req, res) => res.sendStatus(404)) + router.all('*', (_, res) => res.sendStatus(404)) return router } function internalApiRouter() { const router = Router() router.use('/scheduled', scheduledApiRouter()) - router.all('/system/*', (req, res) => res.sendStatus(404)) + router.all('/system/*', (_, 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() }) + const adSamlConfig = createAdSamlConfig(redisClient) router.use( createSamlRouter({ strategyName: 'ead', - strategy: createAdSamlStrategy(redisClient), + strategy: createAdSamlStrategy(adSamlConfig), + samlConfig: adSamlConfig, sessionType: 'employee', pathIdentifier: 'saml' }) ) + const evakaSamlConfig = createEvakaSamlconfig(redisClient) router.use( createSamlRouter({ strategyName: 'evaka', - strategy: createEvakaSamlStrategy(redisClient), + strategy: createEvakaSamlStrategy(evakaSamlConfig), + samlConfig: evakaSamlConfig, sessionType: 'employee', pathIdentifier: 'evaka' }) diff --git a/apigw/src/shared/__tests__/saml-certificates-test.ts b/apigw/src/shared/__tests__/saml-certificates-test.ts index 4ef37014ec9..896cac5eadf 100644 --- a/apigw/src/shared/__tests__/saml-certificates-test.ts +++ b/apigw/src/shared/__tests__/saml-certificates-test.ts @@ -3,16 +3,16 @@ // SPDX-License-Identifier: LGPL-2.1-or-later import { differenceInMonths } from 'date-fns' -import certificates from '../certificates' +import certificates, { TrustedCertificates } from '../certificates' import { pki } from 'node-forge' describe('SAML certificates', () => { test('at least one certificate must exist', () => { expect(Object.keys(certificates).length).toBeGreaterThan(0) }) - for (const certificateName of Object.keys(certificates) as Array< - keyof typeof certificates - >) { + for (const certificateName of Object.keys( + certificates + ) as Array) { test(`${certificateName} must decode successfully`, () => { const computeHash = false const strict = true diff --git a/apigw/src/shared/__tests__/saml-slo.ts b/apigw/src/shared/__tests__/saml-slo.ts new file mode 100644 index 00000000000..dfbbc1dcb53 --- /dev/null +++ b/apigw/src/shared/__tests__/saml-slo.ts @@ -0,0 +1,333 @@ +// SPDX-FileCopyrightText: 2017-2021 City of Espoo +// +// SPDX-License-Identifier: LGPL-2.1-or-later + +import type { AxiosResponse } from 'axios' +import fs from 'fs' +import path from 'path' +import { RedisClient } from 'redis' +import { Cookie } from 'tough-cookie' +import { SignedXml } from 'xml-crypto' +import xml2js from 'xml2js' +import xmldom from 'xmldom' +import zlib from 'zlib' +import * as config from '../config' +import { sfiConfig } from '../config' +import { fromCallback } from '../promise-utils' +import type { AuthenticatedUser } from '../service-client' +import { sessionCookie } from '../session' +import { GatewayTester } from '../test/gateway-tester' + +const mockUser: AuthenticatedUser = { + id: '942b9cab-210d-4d49-b4c9-65f26390eed3', + roles: ['ENDUSER'] +} + +// Explicitly use separate domains for the simulated SP and IdP to replicate +// 3rd party cookie and SAML message parsing issues only present in those +// conditions. SP must be in a domain that, from a browser's cookie handling +// point of view, is a third party site to the IdP managing SSO / Single Logout. +// +// See also: +// https://wiki.shibboleth.net/confluence/display/IDP30/LogoutConfiguration#LogoutConfiguration-Overview +// https://simplesamlphp.org/docs/stable/simplesamlphp-idp-more#section_1 +const SAML_SP_DOMAIN = new URL(sfiConfig.callbackUrl).origin +const IDP_ENTRY_POINT_URL = sfiConfig.entryPoint + +// Helper constants to ensure correct endpoints in all cases +const SP_LOGIN_CALLBACK_ENDPOINT = '/api/application/auth/saml/login/callback' +const SP_LOGOUT_CALLBACK_ENDPOINT = '/api/application/auth/saml/logout/callback' +const SP_LOGIN_CALLBACK_URL = `${SAML_SP_DOMAIN}${SP_LOGIN_CALLBACK_ENDPOINT}` +const SP_LOGOUT_CALLBACK_URL = `${SAML_SP_DOMAIN}${SP_LOGOUT_CALLBACK_ENDPOINT}` +const SECURED_ENDPOINT = `/api/application/auth/status` + +// Use test certificates to validate actual SAML message parsing while not using +// any real certificates/domains. +const SP_ISSUER = sfiConfig.issuer +const IDP_ISSUER = 'evaka-slo-test' +const IDP_PVK = fs + .readFileSync( + path.resolve(__dirname, '../../../config/test-cert/slo-test-idp-key.pem'), + 'utf8' + ) + .toString() + +describe('SAML Single Logout', () => { + let tester: GatewayTester + let redisClient: RedisClient + const sfiMock = config.sfiMock + + beforeEach(async () => { + // In order to enable the REAL Suomi.fi passport-saml Strategy only for + // these tests, config.sfiMock should be true in every other case but this + // test suite + as Strategy vs. DummyStrategy selection is done at app + // import-time -> override the config and re-import app for all of these + // tests to prevent affecting other test suites. Theoretically tests from + // other test suites could be run in-between, so must be done beforeEach + // instead of beforeAll. + // eslint-disable-next-line @typescript-eslint/no-explicit-any + ;(config as any).sfiMock = false + const { default: app, _TEST_ONLY_redisClient } = await import( + '../../enduser/app' + ) + redisClient = _TEST_ONLY_redisClient + tester = await GatewayTester.start(app, 'enduser') + }) + afterEach(async () => { + await tester?.afterEach() + await tester?.stop() + await fromCallback((cb) => redisClient?.flushall(cb)) + // eslint-disable-next-line @typescript-eslint/no-explicit-any + ;(config as any).sfiMock = sfiMock + }) + + test('reference case (3rd party cookies available)', async () => { + const resPreAuth = await tester.client.get(SECURED_ENDPOINT, { + validateStatus: () => true + }) + expect(resPreAuth.status).toBe(200) + expect(resPreAuth.data?.loggedIn).toBeFalsy() + + // Do an IdP-initiated login (skips calling the SP /login endpoint and jumps + // directly to the SAMLResponse phase) + const nameId = 'aaaaaaaaa@aaaaaaaa.local' + const sessionIndex = '_1111111111111111111111' + const inResponseTo = 'firstAuthnRequest' + const loginResponse = buildLoginResponse(nameId, sessionIndex, inResponseTo) + await tester.login(mockUser, { SAMLResponse: loginResponse }) + + tester.nockScope.get(`/persondetails/uuid/${mockUser.id}`).reply(200, { + id: mockUser.id + }) + // Secured endpoint should now be accessible with session cookies + const res = await tester.client.get(SECURED_ENDPOINT, { + validateStatus: () => true + }) + expect(res.status).toBe(200) + expect(res.data?.loggedIn).toBe(true) + tester.nockScope.done() + + // Next the user uses another service participating to the same IdP SSO and + // initiates the SLO process from that other service. + await callSLOEndpointAndAssertResult(tester, nameId, sessionIndex) + + // Logout propagation at the IdP indicated to the user that SLO to our + // service was succesful and enduser thinks they no longer have any open + // sessions. + // + // After few moments someone with the access to the computer writes opens + // our service which must not be available without authentication. + const resPostLogout = await tester.client.get(SECURED_ENDPOINT, { + validateStatus: () => true + }) + expect(resPostLogout.status).toBe(200) + expect(resPostLogout.data?.loggedIn).toBeFalsy() + }) + + test('IdP-initiated logout works without (3rd party) cookies', async () => { + // This is otherwise identical to the reference case BUT simulates 3rd party + // cookies being blocked (even SameSite: None) which is starting to be the + // default on many browsers (Safari; Chrome & Firefox at some point). + // + // If the service only relies on a session or logout cookie being available + // to logout, the user will not actually be logged out and in the worst case + // think they _have_ been and risk exposing their data to other entities + // that have access to the same browser/machine. + + // Baseline + const resPreAuth = await tester.client.get(SECURED_ENDPOINT, { + validateStatus: () => true + }) + expect(resPreAuth.status).toBe(200) + expect(resPreAuth.data?.loggedIn).toBeFalsy() + + // Do an IdP-initiated login (skips calling the SP /login endpoint and jumps + // directly to the SAMLResponse phase) + const nameId = 'aaaaaaaaa@aaaaaaaa.local' + const sessionIndex = '_1111111111111111111111' + const inResponseTo = 'firstAuthnRequest' + const loginResponse = buildLoginResponse(nameId, sessionIndex, inResponseTo) + await tester.login(mockUser, { SAMLResponse: loginResponse }) + + // Secured endpoint should now be accessible with session cookies + tester.nockScope.get(`/persondetails/uuid/${mockUser.id}`).reply(200, { + id: mockUser.id + }) + const res = await tester.client.get(SECURED_ENDPOINT, { + validateStatus: () => true + }) + expect(res.data?.loggedIn).toBe(true) + tester.nockScope.done() + + // Proceeding to SLO... + // + // This is similar situation with "reference case" but because third party cookies are blocked + // by end user's browser following request is executed without express-session's session cookie. + // HTTP calls from within iframe do not contain cookies when third party cookies are blocked. + // + // This situation is simulated by temporarily clearing the session cookies. + // Store current cookies so that they can be restored after SLO. + const cookie = await tester.getCookie(sessionCookie('enduser')) + expect(cookie).toBeTruthy() + await tester.expireSession() + + // Next the user uses another service participating to the same IdP SSO and + // initiates the SLO process from that other service. + await callSLOEndpointAndAssertResult(tester, nameId, sessionIndex) + + // Logout propagation at the IdP indicated to the user that SLO to our + // service was succesful and enduser thinks they no longer have any open + // sessions. + // + // After few moments someone with the access to the computer writes opens + // our service which must not be available without authentication. + // + // Restore cookies to simulate returning our service + await tester.setCookie(cookie as Cookie) + const resPostLogout = await tester.client.get(SECURED_ENDPOINT, { + validateStatus: () => true + }) + expect(resPostLogout.status).toBe(200) + expect(resPostLogout.data?.loggedIn).toBeFalsy() + }) +}) + +async function callSLOEndpointAndAssertResult( + tester: GatewayTester, + nameId: string, + sessionIndex: string +) { + const idpInitiatedLogoutRequest = buildIdPInitiatedLogoutRequest( + nameId, + sessionIndex + ) + const res = await tester.client.post( + SP_LOGOUT_CALLBACK_ENDPOINT, + { SAMLRequest: idpInitiatedLogoutRequest }, + { + maxRedirects: 0, + validateStatus: () => true + } + ) + expect(res.status).toBe(302) + expect(res.headers['location']).toMatch( + new RegExp(`^${IDP_ENTRY_POINT_URL}\\?SAMLResponse=?`) + ) + const logoutResponse = getSamlMessageFromRedirectResponse(res) + const logoutResponseJson = await xml2js.parseStringPromise(logoutResponse) + expect( + logoutResponseJson['samlp:LogoutResponse']['samlp:Status'][0][ + 'samlp:StatusCode' + ][0]['$'].Value + ).toEqual('urn:oasis:names:tc:SAML:2.0:status:Success') +} + +function getSamlMessageFromRedirectResponse(res: AxiosResponse) { + const location = new URL(res.headers['location']) + const msg = + location.searchParams.get('SAMLRequest') ?? + location.searchParams.get('SAMLResponse') + if (!msg) + throw new Error( + 'Response must have a SAMLRequest or SAMLResponse search parameter' + ) + + const decoded = Buffer.from(msg, 'base64') + const inflated = zlib.inflateRawSync(decoded) + return new xmldom.DOMParser({}).parseFromString(inflated.toString()) +} + +function buildLoginResponse( + nameId: string, + sessionIndex: string, + inResponseTo: string +) { + const notBefore = '1980-01-01T01:00:00Z' + const issueInstant = '1980-01-01T01:01:00Z' + const notOnOrAfter = '4980-01-01T01:01:00Z' + + const loginResponse = ` + ${IDP_ISSUER} + + + + + ${IDP_ISSUER} + + ${nameId} + + + + + + + ${SP_ISSUER} + + + + + urn:oasis:names:tc:SAML:2.0:ac:classes:Password + + + +` + return Buffer.from(signXml(loginResponse)).toString('base64') +} + +function buildIdPInitiatedLogoutRequest(nameId: string, sessionIndex: string) { + const idpInitiatedLogoutRequest = ` + ${IDP_ISSUER} + ${nameId} + ${sessionIndex} +` + return Buffer.from(signXml(idpInitiatedLogoutRequest)).toString('base64') +} + +function signXml(xml: string) { + const sig = new SignedXml() + sig.addReference( + '/*', + [ + 'http://www.w3.org/2000/09/xmldsig#enveloped-signature', + 'http://www.w3.org/2001/10/xml-exc-c14n#' + ], + 'http://www.w3.org/2001/04/xmlenc#sha256', + '', + '', + '', + false + ) + sig.signatureAlgorithm = 'http://www.w3.org/2001/04/xmldsig-more#rsa-sha256' + sig.signingKey = IDP_PVK + sig.computeSignature(xml) + return sig.getSignedXml() +} diff --git a/apigw/src/shared/async-redis-client.ts b/apigw/src/shared/async-redis-client.ts index 90bfbefd68b..affdf0f5103 100644 --- a/apigw/src/shared/async-redis-client.ts +++ b/apigw/src/shared/async-redis-client.ts @@ -11,8 +11,14 @@ export default class AsyncRedisClient { async get(key: string): Promise { return fromCallback((cb) => this.client.get(key, cb)) } - async del(...keys: string[]): Promise { - return fromCallback((cb) => this.client.del(...keys, cb)) + // NOTE: redis-mock currently does not support parsing multiple arguments + // (see: https://github.com/yeahoffline/redis-mock/pull/178), so AsyncRedisClient + // should support providing the keys as an array and not use the spread + // operator until this is fixed upstream. + async del(keys: string[]): Promise + async del(...keys: string[]): Promise + async del(keys: string | string[]) { + return fromCallback((cb) => this.client.del(keys, cb)) } async expire(key: string, seconds: number): Promise { return fromCallback((cb) => this.client.expire(key, seconds, cb)) diff --git a/apigw/src/shared/auth/ad-saml.ts b/apigw/src/shared/auth/ad-saml.ts index f9e41aebb0f..f634a03487f 100755 --- a/apigw/src/shared/auth/ad-saml.ts +++ b/apigw/src/shared/auth/ad-saml.ts @@ -4,6 +4,7 @@ import { Profile, + SamlConfig, Strategy as SamlStrategy, VerifiedCallback } from 'passport-saml' @@ -63,8 +64,34 @@ async function verifyProfile(profile: AdProfile): Promise { } } +export function createSamlConfig(redisClient?: RedisClient): SamlConfig { + if (devLoginEnabled) return {} + if (!adConfig) throw Error('Missing AD SAML configuration') + return { + acceptedClockSkewMs: 0, + audience: adConfig.issuer, + cacheProvider: redisClient + ? redisCacheProvider(redisClient, { keyPrefix: 'ad-saml-resp:' }) + : undefined, + callbackUrl: adConfig.callbackUrl, + cert: Array.isArray(adConfig.publicCert) + ? adConfig.publicCert.map( + (certificateName) => certificates[certificateName] + ) + : adConfig.publicCert, + disableRequestedAuthnContext: true, + entryPoint: adConfig.entryPoint, + identifierFormat: 'urn:oasis:names:tc:SAML:2.0:nameid-format:transient', + issuer: adConfig.issuer, + logoutUrl: adConfig.logoutUrl, + privateCert: readFileSync(adConfig.privateCert, { encoding: 'utf8' }), + signatureAlgorithm: 'sha256', + validateInResponseTo: true + } +} + export default function createAdStrategy( - redisClient?: RedisClient + config: SamlConfig ): SamlStrategy | DevPassportStrategy { if (devLoginEnabled) { const getter = async (userId: string) => @@ -102,29 +129,8 @@ export default function createAdStrategy( return new DevPassportStrategy(getter, upserter) } else { - if (!adConfig) throw Error('Missing AD SAML configuration') return new SamlStrategy( - { - acceptedClockSkewMs: 0, - audience: adConfig.issuer, - cacheProvider: redisClient - ? redisCacheProvider(redisClient, { - keyPrefix: 'ad-saml-resp:' - }) - : undefined, - callbackUrl: adConfig.callbackUrl, - cert: adConfig.publicCert.map( - (certificateName) => certificates[certificateName] - ), - disableRequestedAuthnContext: true, - entryPoint: adConfig.entryPointUrl, - identifierFormat: 'urn:oasis:names:tc:SAML:2.0:nameid-format:transient', - issuer: adConfig.issuer, - logoutUrl: adConfig.logoutUrl, - privateCert: readFileSync(adConfig.privateCert, { encoding: 'utf8' }), - signatureAlgorithm: 'sha256', - validateInResponseTo: true - }, + config, (profile: Profile, done: VerifiedCallback) => { // eslint-disable-next-line @typescript-eslint/no-explicit-any verifyProfile((profile as any) as AdProfile) diff --git a/apigw/src/shared/auth/customer-saml.ts b/apigw/src/shared/auth/customer-saml.ts index ab7939f1f6d..1907b99c728 100644 --- a/apigw/src/shared/auth/customer-saml.ts +++ b/apigw/src/shared/auth/customer-saml.ts @@ -2,54 +2,55 @@ // // SPDX-License-Identifier: LGPL-2.1-or-later +import fs from 'fs' import { Profile, + SamlConfig, Strategy as SamlStrategy, VerifiedCallback } from 'passport-saml' +import { RedisClient } from 'redis' +import { evakaCustomerSamlConfig } from '../config' import { SamlUser } from '../routes/auth/saml/types' import { getOrCreatePerson } from '../service-client' -import { evakaCustomerSamlConfig, EvakaSamlConfig } from '../config' -import fs from 'fs' -import { RedisClient } from 'redis' import redisCacheProvider from './passport-saml-cache-redis' -export default function createEvakaCustomerSamlStrategy( - redisClient?: RedisClient -): SamlStrategy { - return createKeycloakSamlStrategy(evakaCustomerSamlConfig, redisClient) -} - -function createKeycloakSamlStrategy( - samlConfig: EvakaSamlConfig | undefined, - redisClient?: RedisClient -): SamlStrategy { - if (!samlConfig) throw new Error('Missing Keycloak SAML configuration') - const publicCert = fs.readFileSync(samlConfig.publicCert, { +export function createSamlConfig(redisClient?: RedisClient): SamlConfig { + if (!evakaCustomerSamlConfig) + throw new Error('Missing Keycloak SAML configuration') + if (Array.isArray(evakaCustomerSamlConfig.publicCert)) + throw new Error('Expected a single string as publicCert') + const publicCert = fs.readFileSync(evakaCustomerSamlConfig.publicCert, { encoding: 'utf8' }) - const privateCert = fs.readFileSync(samlConfig.privateCert, { + const privateCert = fs.readFileSync(evakaCustomerSamlConfig.privateCert, { encoding: 'utf8' }) + return { + acceptedClockSkewMs: -1, + cacheProvider: redisClient + ? redisCacheProvider(redisClient, { + keyPrefix: 'customer-saml-resp:' + }) + : undefined, + callbackUrl: evakaCustomerSamlConfig.callbackUrl, + cert: publicCert, + decryptionPvk: privateCert, + entryPoint: evakaCustomerSamlConfig.entryPoint, + identifierFormat: 'urn:oasis:names:tc:SAML:2.0:nameid-format:transient', + issuer: evakaCustomerSamlConfig.issuer, + logoutUrl: evakaCustomerSamlConfig.entryPoint, + privateCert: privateCert, + signatureAlgorithm: 'sha256', + validateInResponseTo: true + } +} + +export default function createKeycloakSamlStrategy( + config: SamlConfig +): SamlStrategy { return new SamlStrategy( - { - acceptedClockSkewMs: -1, - cacheProvider: redisClient - ? redisCacheProvider(redisClient, { - keyPrefix: 'customer-saml-resp:' - }) - : undefined, - callbackUrl: samlConfig.callbackUrl, - cert: publicCert, - decryptionPvk: privateCert, - entryPoint: samlConfig.entryPoint, - identifierFormat: 'urn:oasis:names:tc:SAML:2.0:nameid-format:transient', - issuer: 'evaka-customer', - logoutUrl: samlConfig.entryPoint, - privateCert: privateCert, - signatureAlgorithm: 'sha256', - validateInResponseTo: true - }, + config, (profile: Profile, done: VerifiedCallback) => { // eslint-disable-next-line @typescript-eslint/no-explicit-any verifyKeycloakProfile((profile as any) as KeycloakProfile) diff --git a/apigw/src/shared/auth/index.ts b/apigw/src/shared/auth/index.ts index f35ff0056a2..078bd47ec63 100644 --- a/apigw/src/shared/auth/index.ts +++ b/apigw/src/shared/auth/index.ts @@ -8,6 +8,8 @@ import { logAuditEvent } from '../logging' import { gatewayRole } from '../config' import { createJwt } from './jwt' import { SamlUser } from '../routes/auth/saml/types' +import { Profile, SAML } from 'passport-saml' +import { fromCallback } from '../promise-utils' const auditEventGatewayId = (gatewayRole === 'enduser' && 'eugw') || @@ -68,3 +70,43 @@ function createJwtToken(user: SamlUser): string { export function createAuthHeader(user: SamlUser): string { return `Bearer ${createJwtToken(user)}` } + +export function createLogoutToken( + nameID: Required['nameID'], + sessionIndex: Profile['sessionIndex'] +) { + return `${nameID}:::${sessionIndex}` +} + +/** + * If request is a SAMLRequest, parse, validate and return the Profile from it. + * @param saml Config must match active strategy's config + */ +export async function tryParseProfile( + req: Request, + saml: SAML +): Promise { + let profile: 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 parameter + const dummyOrigin = 'http://evaka' + const originalQuery = new URL(req.url, dummyOrigin).search.replace( + /^\?/, + '' + ) + profile = 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 + profile = await fromCallback((cb) => + saml.validatePostRequest(req.body, cb) + ) + } + + return profile || undefined +} diff --git a/apigw/src/shared/auth/keycloak-saml.ts b/apigw/src/shared/auth/keycloak-saml.ts index ce2a417677b..63ad2aa86e5 100644 --- a/apigw/src/shared/auth/keycloak-saml.ts +++ b/apigw/src/shared/auth/keycloak-saml.ts @@ -4,6 +4,7 @@ import { Profile, + SamlConfig, Strategy as SamlStrategy, VerifiedCallback } from 'passport-saml' @@ -14,34 +15,40 @@ import fs from 'fs' import { RedisClient } from 'redis' import redisCacheProvider from './passport-saml-cache-redis' -export default function createKeycloakSamlStrategy( - redisClient?: RedisClient -): SamlStrategy { +export function createSamlConfig(redisClient?: RedisClient): SamlConfig { if (!evakaSamlConfig) throw new Error('Missing Keycloak SAML configuration') + if (Array.isArray(evakaSamlConfig.publicCert)) + throw new Error('Expected a single string as publicCert') const publicCert = fs.readFileSync(evakaSamlConfig.publicCert, { encoding: 'utf8' }) const privateCert = fs.readFileSync(evakaSamlConfig.privateCert, { encoding: 'utf8' }) + return { + acceptedClockSkewMs: 0, + audience: evakaSamlConfig.issuer, + cacheProvider: redisClient + ? redisCacheProvider(redisClient, { keyPrefix: 'keycloak-saml-resp:' }) + : undefined, + callbackUrl: evakaSamlConfig.callbackUrl, + cert: publicCert, + decryptionPvk: privateCert, + entryPoint: evakaSamlConfig.entryPoint, + identifierFormat: 'urn:oasis:names:tc:SAML:2.0:nameid-format:transient', + issuer: evakaSamlConfig.issuer, + logoutUrl: evakaSamlConfig.entryPoint, + privateCert: privateCert, + signatureAlgorithm: 'sha256', + validateInResponseTo: true + } +} + +export default function createKeycloakSamlStrategy( + config: SamlConfig +): SamlStrategy { return new SamlStrategy( - { - acceptedClockSkewMs: 0, - audience: evakaSamlConfig.issuer, - cacheProvider: redisClient - ? redisCacheProvider(redisClient, { keyPrefix: 'keycloak-saml-resp:' }) - : undefined, - callbackUrl: evakaSamlConfig.callbackUrl, - cert: publicCert, - decryptionPvk: privateCert, - entryPoint: evakaSamlConfig.entryPoint, - identifierFormat: 'urn:oasis:names:tc:SAML:2.0:nameid-format:transient', - issuer: evakaSamlConfig.issuer, - logoutUrl: evakaSamlConfig.entryPoint, - privateCert: privateCert, - signatureAlgorithm: 'sha256', - validateInResponseTo: true - }, + config, (profile: Profile, done: VerifiedCallback) => { // eslint-disable-next-line @typescript-eslint/no-explicit-any verifyKeycloakProfile(profile as KeycloakProfile) diff --git a/apigw/src/shared/auth/suomi-fi-saml.ts b/apigw/src/shared/auth/suomi-fi-saml.ts index c7a882326d2..ccbe295f13f 100644 --- a/apigw/src/shared/auth/suomi-fi-saml.ts +++ b/apigw/src/shared/auth/suomi-fi-saml.ts @@ -2,15 +2,15 @@ // // SPDX-License-Identifier: LGPL-2.1-or-later -import { Profile, Strategy, VerifiedCallback } from 'passport-saml' -import { SamlUser } from '../routes/auth/saml/types' +import fs from 'fs' import { Strategy as DummyStrategy } from 'passport-dummy' -import { sfiConfig, sfiMock } from '../config' +import { Profile, SamlConfig, Strategy, VerifiedCallback } from 'passport-saml' +import { RedisClient } from 'redis' import certificates from '../certificates' -import fs from 'fs' +import { nodeEnv, sfiConfig, sfiMock } from '../config' +import { SamlUser } from '../routes/auth/saml/types' import { getOrCreatePerson } from '../service-client' import redisCacheProvider from './passport-saml-cache-redis' -import { RedisClient } from 'redis' // Suomi.fi e-Identification – Attributes transmitted on an identified user: // https://esuomi.fi/suomi-fi-services/suomi-fi-e-identification/14247-2/?lang=en @@ -54,8 +54,43 @@ async function verifyProfile(profile: SuomiFiProfile): Promise { sessionIndex: profile.sessionIndex } } + +export function createSamlConfig(redisClient?: RedisClient): SamlConfig { + if (sfiMock) return {} + const publicCert = Array.isArray(sfiConfig.publicCert) + ? sfiConfig.publicCert.map( + (certificateName) => certificates[certificateName] + ) + : fs.readFileSync(sfiConfig.publicCert, { + encoding: 'utf8' + }) + const privateCert = fs.readFileSync(sfiConfig.privateCert, { + encoding: 'utf8' + }) + + return { + acceptedClockSkewMs: 0, + audience: sfiConfig.issuer, + cacheProvider: redisClient + ? redisCacheProvider(redisClient, { keyPrefix: 'suomifi-saml-resp:' }) + : undefined, + callbackUrl: sfiConfig.callbackUrl, + cert: publicCert, + decryptionPvk: privateCert, + disableRequestedAuthnContext: true, + entryPoint: sfiConfig.entryPoint, + identifierFormat: 'urn:oasis:names:tc:SAML:2.0:nameid-format:transient', + issuer: sfiConfig.issuer, + logoutUrl: sfiConfig.logoutUrl, + privateCert: privateCert, + signatureAlgorithm: 'sha256', + // InResponseTo validation unnecessarily complicates testing + validateInResponseTo: nodeEnv === 'test' ? false : true + } +} + export default function createSuomiFiStrategy( - redisClient?: RedisClient + config: SamlConfig ): Strategy | DummyStrategy { if (sfiMock) { return new DummyStrategy((done) => { @@ -64,39 +99,11 @@ export default function createSuomiFiStrategy( .catch(done) }) } else { - if (!sfiConfig) throw new Error('Missing Suomi.fi SAML configuration') - const privateCert = fs.readFileSync(sfiConfig.privateCert, { - encoding: 'utf8' + return new Strategy(config, (profile: Profile, done: VerifiedCallback) => { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + verifyProfile((profile as any) as SuomiFiProfile) + .then((user) => done(null, user)) + .catch(done) }) - return new Strategy( - { - acceptedClockSkewMs: 0, - audience: sfiConfig.issuer, - cacheProvider: redisClient - ? redisCacheProvider(redisClient, { - keyPrefix: 'suomifi-saml-resp:' - }) - : undefined, - callbackUrl: sfiConfig.callbackUrl, - cert: sfiConfig.publicCert.map( - (certificateName) => certificates[certificateName] - ), - decryptionPvk: privateCert, - disableRequestedAuthnContext: true, - entryPoint: sfiConfig.entryPoint, - identifierFormat: 'urn:oasis:names:tc:SAML:2.0:nameid-format:transient', - issuer: sfiConfig.issuer, - logoutUrl: sfiConfig.logoutUrl, - privateCert: privateCert, - signatureAlgorithm: 'sha256', - validateInResponseTo: true - }, - (profile: Profile, done: VerifiedCallback) => { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - verifyProfile((profile as any) as SuomiFiProfile) - .then((user) => done(null, user)) - .catch(done) - } - ) } } diff --git a/apigw/src/shared/certificates.ts b/apigw/src/shared/certificates.ts index 86cf872a1aa..c1af2ed1c03 100644 --- a/apigw/src/shared/certificates.ts +++ b/apigw/src/shared/certificates.ts @@ -14,8 +14,10 @@ const names = [ 'tamperead-internal-staging.pem' ] as const +export type TrustedCertificates = typeof names[number] + // eslint-disable-next-line @typescript-eslint/no-explicit-any -const certificates: Record = {} as any +const certificates: Record = {} as any for (const name of names) { certificates[name] = fs.readFileSync( path.resolve(__dirname, '../../config/certificates', name), diff --git a/apigw/src/shared/config.ts b/apigw/src/shared/config.ts index 5ec723601b6..90b4eabbdea 100644 --- a/apigw/src/shared/config.ts +++ b/apigw/src/shared/config.ts @@ -2,7 +2,16 @@ // // SPDX-License-Identifier: LGPL-2.1-or-later -import certificates from './certificates' +import certificates, { TrustedCertificates } from './certificates' + +export interface EvakaSamlConfig { + callbackUrl: string + entryPoint: string + logoutUrl: string + issuer: string + publicCert: string | TrustedCertificates[] + privateCert: string +} export const gatewayRoles = ['enduser', 'internal'] as const export type NodeEnv = typeof nodeEnvs[number] @@ -129,9 +138,9 @@ export const enableDevApi = ifNodeEnv(['local', 'test'], true) ?? false -const certificateNames = Object.keys(certificates) as ReadonlyArray< - keyof typeof certificates -> +const certificateNames = Object.keys( + certificates +) as ReadonlyArray export const devLoginEnabled = env('DEV_LOGIN', parseBoolean) ?? ifNodeEnv(['local', 'test'], true) ?? false @@ -140,11 +149,11 @@ const adCallbackUrl = process.env.AD_SAML_CALLBACK_URL const adEntryPointUrl = process.env.AD_SAML_ENTRYPOINT_URL const adLogoutUrl = process.env.AD_SAML_LOGOUT_URL -export const adConfig = +export const adConfig: EvakaSamlConfig | undefined = adCallbackUrl && !devLoginEnabled ? { callbackUrl: required(adCallbackUrl), - entryPointUrl: required(adEntryPointUrl), + entryPoint: required(adEntryPointUrl), logoutUrl: required(adLogoutUrl), issuer: required(process.env.AD_SAML_ISSUER), publicCert: required( @@ -160,21 +169,41 @@ export const adExternalIdPrefix = export const sfiMock = env('SFI_MOCK', parseBoolean) ?? ifNodeEnv(['local', 'test'], true) ?? false -const sfiCallbackUrl = process.env.SFI_SAML_CALLBACK_URL - -export const sfiConfig = - sfiCallbackUrl && !sfiMock - ? { - callbackUrl: required(sfiCallbackUrl), - entryPoint: required(process.env.SFI_SAML_ENTRYPOINT), - logoutUrl: required(process.env.SFI_SAML_LOGOUT_URL), - issuer: required(process.env.SFI_SAML_ISSUER), - publicCert: required( - envArray('SFI_SAML_PUBLIC_CERT', parseEnum(certificateNames)) - ), - privateCert: required(process.env.SFI_SAML_PRIVATE_CERT) - } - : undefined +// For local development & testing: +// Explicitly use separate domains for the simulated SP and IdP to replicate +// 3rd party cookie and SAML message parsing issues only present in those +// conditions. SP must be in a domain that, from a browser's cookie handling +// point of view, is a third party site to the IdP managing SSO / Single Logout. +// +// See also: +// https://wiki.shibboleth.net/confluence/display/IDP30/LogoutConfiguration#LogoutConfiguration-Overview +// https://simplesamlphp.org/docs/stable/simplesamlphp-idp-more#section_1 +const sfiCallbackUrl = + process.env.SFI_SAML_CALLBACK_URL ?? + ifNodeEnv( + ['local', 'test'], + 'https://saml-sp.qwerty.local/api/application/auth/saml/logout/callback' + ) +const sfiEntryPointUrl = + process.env.SFI_SAML_ENTRYPOINT ?? + ifNodeEnv(['local', 'test'], 'https://identity-provider.asdf.local/idp') +const sfiLogoutUrl = process.env.SFI_SAML_LOGOUT_URL ?? sfiEntryPointUrl +const sfiIssuer = process.env.SFI_SAML_ISSUER ?? 'evaka-local' + +export const sfiConfig: EvakaSamlConfig = { + callbackUrl: required(sfiCallbackUrl), + entryPoint: required(sfiEntryPointUrl), + logoutUrl: required(sfiLogoutUrl), + issuer: required(sfiIssuer), + publicCert: required( + envArray('SFI_SAML_PUBLIC_CERT', parseEnum(certificateNames)) ?? + ifNodeEnv(['local', 'test'], 'config/test-cert/slo-test-idp-cert.pem') + ), + privateCert: required( + process.env.SFI_SAML_PRIVATE_CERT ?? + ifNodeEnv(['local', 'test'], 'config/test-cert/saml-private.pem') + ) +} const evakaCallbackUrl = process.env.EVAKA_SAML_CALLBACK_URL ?? @@ -190,7 +219,7 @@ const evakaCustomerCallbackUrl = `http://localhost:9099/api/application/auth/evaka-customer/login/callback` ) -export const evakaSamlConfig = evakaCallbackUrl +export const evakaSamlConfig: EvakaSamlConfig | undefined = evakaCallbackUrl ? { callbackUrl: required(evakaCallbackUrl), entryPoint: required( @@ -200,6 +229,14 @@ export const evakaSamlConfig = evakaCallbackUrl 'http://localhost:8080/auth/realms/evaka/protocol/saml' ) ), + // NOTE: Same as entrypoint, on purpose + logoutUrl: required( + process.env.EVAKA_SAML_ENTRYPOINT ?? + ifNodeEnv( + ['local', 'test'], + 'http://localhost:8080/auth/realms/evaka/protocol/saml' + ) + ), issuer: required( process.env.EVAKA_SAML_ISSUER ?? ifNodeEnv( @@ -218,13 +255,6 @@ export const evakaSamlConfig = evakaCallbackUrl } : undefined -export interface EvakaSamlConfig { - callbackUrl: string - entryPoint: string - publicCert: string - privateCert: string -} - export const evakaCustomerSamlConfig: | EvakaSamlConfig | undefined = evakaCustomerCallbackUrl @@ -237,6 +267,20 @@ export const evakaCustomerSamlConfig: 'http://localhost:8080/auth/realms/evaka-customer/protocol/saml' ) ), + logoutUrl: required( + process.env.EVAKA_CUSTOMER_SAML_ENTRYPOINT ?? + ifNodeEnv( + ['local', 'test'], + 'http://localhost:8080/auth/realms/evaka-customer/protocol/saml' + ) + ), + issuer: required( + process.env.EVAKA_CUSTOMER_SAML_ISSUER ?? + ifNodeEnv( + ['local', 'test'], + 'http://localhost:8080/auth/realms/evaka-customer' + ) + ), publicCert: required( process.env.EVAKA_CUSTOMER_SAML_PUBLIC_CERT ?? ifNodeEnv(['local', 'test'], 'config/test-cert/keycloak-local.pem') 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 index b31661a1a1e..9727009486b 100644 --- a/apigw/src/shared/passport-saml.d.ts +++ b/apigw/src/shared/passport-saml.d.ts @@ -12,6 +12,7 @@ declare module 'passport-saml' { import type passport from 'passport' import type express from 'express' + import { ParsedQs } from 'qs' export interface CacheItem { createdAt: number @@ -177,4 +178,30 @@ declare module 'passport-saml' { callback: (err: Error | null, metadata?: string) => void ): string } + + // SPDX-FileCopyrightText: 2017-2021 City of Espoo + // + // SPDX-License-Identifier: LGPL-2.1-or-later + 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/redis-client.ts b/apigw/src/shared/redis-client.ts index 6dc23726525..dcd6d5585e3 100644 --- a/apigw/src/shared/redis-client.ts +++ b/apigw/src/shared/redis-client.ts @@ -30,7 +30,10 @@ export function createRedisClient() { logError('Redis error', undefined, undefined, err) ) - // don't prevent the app from exiting if a redis connection is alive - redisClient.unref() + // Don't prevent the app from exiting if a redis connection is alive. + // Also, unref is not defined when running tests (with redis-mock active). + if (typeof redisClient.unref === 'function') { + redisClient.unref() + } return redisClient } diff --git a/apigw/src/shared/routes/auth/saml/index.ts b/apigw/src/shared/routes/auth/saml/index.ts index 4bb72fc9192..125224f75ca 100755 --- a/apigw/src/shared/routes/auth/saml/index.ts +++ b/apigw/src/shared/routes/auth/saml/index.ts @@ -2,19 +2,19 @@ // // SPDX-License-Identifier: LGPL-2.1-or-later -import express, { Router } from 'express' +import express, { Router, urlencoded } from 'express' +import _ from 'lodash' import passport from 'passport' -import { urlencoded } from 'body-parser' -import { logAuditEvent, logDebug } from '../../../logging' +import { AuthenticateOptions, SAML } from 'passport-saml' +import { createLogoutToken, tryParseProfile } from '../../../auth' import { devLoginEnabled, gatewayRole, nodeEnv } from '../../../config' -import { parseDescriptionFromSamlError } from './error-utils' -import { SamlEndpointConfig, SamlUser } from './types' +import { getEmployees } from '../../../dev-api' import { toMiddleware, toRequestHandler } from '../../../express' -import { logoutExpress, saveLogoutToken } from '../../../session' +import { logAuditEvent, logDebug } from '../../../logging' import { fromCallback } from '../../../promise-utils' -import { getEmployees } from '../../../dev-api' -import _ from 'lodash' -import type { AuthenticateOptions } from 'passport-saml' +import { logoutExpress, saveLogoutToken } from '../../../session' +import { parseDescriptionFromSamlError } from './error-utils' +import { SamlEndpointConfig, SamlUser } from './types' const urlencodedParser = urlencoded({ extended: false }) @@ -54,8 +54,7 @@ function getRedirectUrl(req: express.Request): string { } function createLoginHandler({ - strategyName, - sessionType + strategyName }: SamlEndpointConfig): express.RequestHandler { return (req, res, next) => { logAuditEvent( @@ -94,7 +93,15 @@ function createLoginHandler({ req, 'User logged in successfully' ) - await saveLogoutToken(req, res, sessionType, strategyName) + + if (!user.nameID) { + throw new Error('User unexpectedly missing nameID property') + } + await saveLogoutToken( + req, + strategyName, + createLogoutToken(user.nameID, user.sessionIndex) + ) const redirectUrl = getRedirectUrl(req) logDebug(`Redirecting to ${redirectUrl}`, req, { redirectUrl }) return res.redirect(redirectUrl) @@ -135,13 +142,10 @@ function createLogoutHandler({ await logoutExpress(req, res, sessionType) return res.redirect(redirectUrl) } catch (err) { - const description = - parseDescriptionFromSamlError(err, req) || - 'Could not parse SAML error.' logAuditEvent( `evaka.saml.${strategyName}.sign_out_failed`, req, - `Log out failed. Description: ${description}. Error: ${err}.` + `Log out failed. Description: Failed before redirecting user to IdP. Error: ${err}.` ) throw err } @@ -165,7 +169,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) @@ -177,7 +183,13 @@ export default function createSamlRouter(config: SamlEndpointConfig): Router { req, 'Logout callback called' ) - await logoutExpress(req, res, config.sessionType) + const profile = await tryParseProfile(req, saml) + await logoutExpress( + req, + res, + config.sessionType, + profile?.nameID && createLogoutToken(profile.nameID, profile.sessionIndex) + ) }) const router = Router() diff --git a/apigw/src/shared/routes/auth/saml/types.ts b/apigw/src/shared/routes/auth/saml/types.ts index e01caabdd06..8ac3781b823 100644 --- a/apigw/src/shared/routes/auth/saml/types.ts +++ b/apigw/src/shared/routes/auth/saml/types.ts @@ -2,7 +2,7 @@ // // SPDX-License-Identifier: LGPL-2.1-or-later -import type { Strategy } from 'passport-saml' +import type { SamlConfig, Strategy } from 'passport-saml' import type { Strategy as DummyStrategy } from 'passport-dummy' import type { SessionType } from '../../../session' import type { UserType } from '../../../service-client' @@ -10,6 +10,7 @@ import type { UserType } from '../../../service-client' export interface SamlEndpointConfig { strategyName: string strategy: Strategy | DummyStrategy + samlConfig: SamlConfig sessionType: SessionType pathIdentifier: string } diff --git a/apigw/src/shared/routes/csp.ts b/apigw/src/shared/routes/csp.ts index 92e36c95218..b491265140a 100644 --- a/apigw/src/shared/routes/csp.ts +++ b/apigw/src/shared/routes/csp.ts @@ -2,15 +2,14 @@ // // SPDX-License-Identifier: LGPL-2.1-or-later -import { Router } from 'express' +import express, { Router } from 'express' import { logWarn } from '../logging' -import bodyParser from 'body-parser' const router = Router() router.post( '/csp-report', - bodyParser.json({ type: 'application/csp-report' }), + express.json({ type: 'application/csp-report' }), (req, res) => { logWarn('CSP report received', req, { user: req.user, diff --git a/apigw/src/shared/session.ts b/apigw/src/shared/session.ts index e2a14897b99..ca59e57a46e 100644 --- a/apigw/src/shared/session.ts +++ b/apigw/src/shared/session.ts @@ -12,10 +12,10 @@ import { import express, { CookieOptions } from 'express' import session from 'express-session' import { RedisClient } from 'redis' -import { v4 as uuidv4 } from 'uuid' import AsyncRedisClient from './async-redis-client' import { cookieSecret, sessionTimeoutMinutes, useSecureCookies } from './config' -import { toMiddleware } from './express' +import { LogoutToken, toMiddleware } from './express' +import { logDebug } from './logging' import { fromCallback } from './promise-utils' export type SessionType = 'enduser' | 'employee' @@ -31,70 +31,74 @@ 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` +function sessionKey(id: string) { + return `sess:${id}` +} + +function logoutKey(token: LogoutToken['value']) { + return `slo:${token}` } 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) } }) } +/** + * Save a logout token for a user session to be consumed during logout. + * Provide the same token during logout to logoutExpress to consume it. + * + * The token must be a value generated from values available in a logout request + * without any cookies, as many browsers will disable 3rd party cookies by + * default, breaking Single Logout. For example, SAML nameID and sessionIndex. + * + * The token is used as an effective secondary index in Redis for the session, + * which would normally be recognized from the session cookie. + * + * This token can be removed if this passport-saml issue is ever fixed and this + * logic is directly implemented in the library: + * 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 + strategyName: string | null | undefined, + logoutToken?: string ): Promise { if (!req.session) return + + const token = logoutToken || req.session.logoutToken?.value + if (!token) return + + // Persist in session to allow custom logic per strategy + req.session.idpProvider = strategyName + + if (!asyncRedisClient) return const now = new Date() const expires = addMinutes(now, sessionTimeoutMinutes + 60) - const idpProvider = strategyName - const logoutToken = { + const newToken = { expiresAt: expires.valueOf(), - value: req.session.logoutToken ? req.session.logoutToken.value : uuidv4(), - idpProvider + value: token } - req.session.logoutToken = logoutToken - res.cookie(logoutCookie(sessionType), logoutToken.value, { - ...logoutCookieOptions, - expires - }) - if (!asyncRedisClient) return - const key = `logout:${logoutToken.value}` + req.session.logoutToken = newToken + + const key = logoutKey(token) const ttlSeconds = differenceInSeconds(expires, now) // https://redis.io/commands/expire - Set a timeout on key // Return value: @@ -108,33 +112,44 @@ 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 consumeLogoutToken( req: express.Request, - res: express.Response, - sessionType: SessionType + logoutToken?: LogoutToken['value'] ): 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}`) + // Prefer an explicit token from e.g. a SAMLRequest but fall back to the + // logoutToken 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 token = logoutToken || req.session?.logoutToken?.value + if (!token) { + logDebug("Can't consume logout request without a logout token, ignoring") + return + } + + if (!asyncRedisClient) return + const key = logoutKey(token) + 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(sessionKey(sid), key) } } export async function logoutExpress( req: express.Request, res: express.Response, - sessionType: SessionType + sessionType: SessionType, + logoutToken?: LogoutToken['value'] ) { req.logout() + await consumeLogoutToken(req, logoutToken) 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, redisClient?: RedisClient) => { diff --git a/apigw/src/shared/test/gateway-tester.ts b/apigw/src/shared/test/gateway-tester.ts index 9530af84c86..686d4787e79 100644 --- a/apigw/src/shared/test/gateway-tester.ts +++ b/apigw/src/shared/test/gateway-tester.ts @@ -48,8 +48,21 @@ export class GatewayTester { return undefined } - public async getCookie(key: string): Promise { - return (await this.findCookie(key))?.value + public async setCookie(cookie: Cookie): Promise { + // Cookie domain must be cleared before setting to avoid issue with "localhost" + // being in the public suffix list (and therefore denied by setCookie()) + cookie.domain = null + const cookieString = cookie.cookieString() + return await this.cookies.setCookie(cookieString, this.baseUrl, { + http: cookie.httpOnly, + secure: cookie.secure, + now: cookie.creation ?? undefined, + sameSiteContext: cookie.sameSite + }) + } + + public async getCookie(key: string): Promise { + return await this.findCookie(key) } public async expireSession(): Promise { @@ -68,12 +81,17 @@ export class GatewayTester { ) } - public async login(user: AuthenticatedUser | EmployeeUser): Promise { + public async login( + user: AuthenticatedUser | EmployeeUser, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + postData?: any + ): Promise { + postData = postData !== undefined ? postData : { preset: 'dummy' } if (this.sessionType === 'employee') { this.nockScope.post('/system/employee-identity').reply(200, user) await this.client.post( '/api/internal/auth/saml/login/callback', - { preset: 'dummy' }, + postData, { maxRedirects: 0, validateStatus: (status) => status >= 200 && status <= 302 @@ -84,7 +102,7 @@ export class GatewayTester { this.nockScope.post('/system/person-identity').reply(200, user) await this.client.post( '/api/application/auth/saml/login/callback', - { preset: 'dummy' }, + postData, { maxRedirects: 0, validateStatus: (status) => status >= 200 && status <= 302 diff --git a/apigw/src/shared/tough-cookie.d.ts b/apigw/src/shared/tough-cookie.d.ts new file mode 100644 index 00000000000..d9bb0b54694 --- /dev/null +++ b/apigw/src/shared/tough-cookie.d.ts @@ -0,0 +1,20 @@ +// SPDX-FileCopyrightText: 2017-2021 City of Espoo +// +// SPDX-License-Identifier: LGPL-2.1-or-later + +import 'tough-cookie' + +declare module 'tough-cookie' { + export namespace CookieJar { + interface SetCookieOptions { + http?: boolean + secure?: boolean + now?: Date + ignoreError?: boolean + // Missing from @types/tough-cookie@4.0.0 + // Could be defined as: "none" | "lax" | "strict" but + // Cookie.sameSite is defined as: string + sameSiteContext?: string + } + } +} diff --git a/apigw/yarn.lock b/apigw/yarn.lock index 4898e68a8f9..f05705a386a 100644 --- a/apigw/yarn.lock +++ b/apigw/yarn.lock @@ -1097,6 +1097,32 @@ __metadata: languageName: node linkType: hard +"@types/xml-crypto@npm:^1.4.1": + version: 1.4.1 + resolution: "@types/xml-crypto@npm:1.4.1" + dependencies: + "@types/node": "*" + xpath: 0.0.27 + checksum: 5d95f16be31bc639388f9fd0d8e2a9bbf93b7149d2eaa2fd8c0867cbc5a33fd6f42f75cec28396ee1b8de7fbc359d89e58e71c6bc43d8c718ea9dcdacbc05b5a + languageName: node + linkType: hard + +"@types/xml2js@npm:^0.4.8": + version: 0.4.8 + resolution: "@types/xml2js@npm:0.4.8" + dependencies: + "@types/node": "*" + checksum: e78287171ad2c4b9e97ade498d8fad5f71a4d4286071c7b43d9fda17c4b5a14ec576f478d58bd2dcd51d5342cdff214e955bed18c4153fa53fa94131b7f0e2d4 + languageName: node + linkType: hard + +"@types/xmldom@npm:^0.1.30": + version: 0.1.30 + resolution: "@types/xmldom@npm:0.1.30" + checksum: 7b195da37ca63bcba37df495706333ec0bb7232cda5c2c146e27babce0434a0ab1684b0254c9ae6d4c571b18ef1d77b4ec8edc264fd1b7d67db9f85800bf50ba + languageName: node + linkType: hard + "@types/yargs-parser@npm:*": version: 20.2.0 resolution: "@types/yargs-parser@npm:20.2.0" @@ -2863,6 +2889,9 @@ __metadata: "@types/through2": ^2.0.34 "@types/tough-cookie": ^4.0.0 "@types/uuid": ^8.3.0 + "@types/xml-crypto": ^1.4.1 + "@types/xml2js": ^0.4.8 + "@types/xmldom": ^0.1.30 "@typescript-eslint/eslint-plugin": ^4.6.0 "@typescript-eslint/parser": ^4.6.0 axios: ^0.21.1 @@ -2908,6 +2937,9 @@ __metadata: ts-node: ^9.1.1 typescript: ^4.1.3 uuid: ^8.3.1 + xml-crypto: 2.1.2 + xml2js: ^0.4.23 + xmldom: ^0.6.0 languageName: unknown linkType: soft @@ -8063,7 +8095,7 @@ typescript@^4.1.3: languageName: node linkType: hard -"xml2js@npm:0.4.x": +"xml2js@npm:0.4.x, xml2js@npm:^0.4.23": version: 0.4.23 resolution: "xml2js@npm:0.4.23" dependencies: