From 09b08dab43abff1416ebce8977577b5f18968136 Mon Sep 17 00:00:00 2001
From: Mikko Piuhola <me@mikkopiuhola.fi>
Date: Thu, 15 Apr 2021 15:42:29 +0300
Subject: [PATCH] apigw: split SAML config creation from Strategies

- Allows separate instation of the `SAML` class (from `passport-saml`) used for parsing/verifying SAML messages
- Will later be used to parse logout tokens from SAML LogoutRequest messages where cookies aren't available
---
 apigw/src/internal/app.ts                  | 16 ++++--
 apigw/src/shared/auth/ad-saml.ts           | 43 ++++++++-------
 apigw/src/shared/auth/keycloak-saml.ts     | 33 +++++++-----
 apigw/src/shared/auth/suomi-fi-saml.ts     | 62 ++++++++++++----------
 apigw/src/shared/routes/auth/saml/types.ts |  3 +-
 apigw/src/shared/routes/auth/suomi-fi.ts   |  8 ++-
 6 files changed, 99 insertions(+), 66 deletions(-)

diff --git a/apigw/src/internal/app.ts b/apigw/src/internal/app.ts
index a3e522d2ece..e63fcafe506 100644
--- a/apigw/src/internal/app.ts
+++ b/apigw/src/internal/app.ts
@@ -24,8 +24,12 @@ import mobileDeviceSession, {
 } from './mobile-device-session'
 import authStatus from './routes/auth-status'
 import createSamlRouter from '../shared/routes/auth/saml'
-import createAdSamlStrategy from '../shared/auth/ad-saml'
-import createEvakaSamlStrategy from '../shared/auth/keycloak-saml'
+import createAdSamlStrategy, {
+  createSamlConfig as createAdSamlConfig
+} from '../shared/auth/ad-saml'
+import createEvakaSamlStrategy, {
+  createSamlConfig as createEvakaSalmconfig
+} from '../shared/auth/keycloak-saml'
 
 const app = express()
 trustReverseProxy(app)
@@ -69,19 +73,23 @@ function internalApiRouter() {
     next()
   })
 
+  const adSamlConfig = createAdSamlConfig()
   router.use(
     createSamlRouter({
       strategyName: 'ead',
-      strategy: createAdSamlStrategy(),
+      strategy: createAdSamlStrategy(adSamlConfig),
+      samlConfig: adSamlConfig,
       sessionType: 'employee',
       pathIdentifier: 'saml'
     })
   )
 
+  const evakaSamlConfig = createEvakaSalmconfig()
   router.use(
     createSamlRouter({
       strategyName: 'evaka',
-      strategy: createEvakaSamlStrategy(),
+      strategy: createEvakaSamlStrategy(evakaSamlConfig),
+      samlConfig: evakaSamlConfig,
       sessionType: 'employee',
       pathIdentifier: 'evaka'
     })
diff --git a/apigw/src/shared/auth/ad-saml.ts b/apigw/src/shared/auth/ad-saml.ts
index 58e176fbbfb..1dcbb788ed1 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'
@@ -61,7 +62,29 @@ async function verifyProfile(profile: AdProfile): Promise<SamlUser> {
   }
 }
 
-export default function createAdStrategy(): SamlStrategy | DevPassportStrategy {
+export function createSamlConfig(): SamlConfig {
+  if (!adConfig) throw Error('Missing AD SAML configuration')
+  return {
+    callbackUrl: adConfig.callbackUrl,
+    entryPoint: adConfig.entryPointUrl,
+    logoutUrl: adConfig.logoutUrl,
+    issuer: adConfig.issuer,
+    cert: adConfig.publicCert.map(
+      (certificateName) => certificates[certificateName]
+    ),
+    privateCert: readFileSync(adConfig.privateCert, {
+      encoding: 'utf8'
+    }),
+    identifierFormat: 'urn:oasis:names:tc:SAML:2.0:nameid-format:transient',
+    disableRequestedAuthnContext: true,
+    signatureAlgorithm: 'sha256',
+    acceptedClockSkewMs: -1
+  }
+}
+
+export default function createAdStrategy(
+  config: SamlConfig
+): SamlStrategy | DevPassportStrategy {
   if (devLoginEnabled) {
     const getter = async (userId: string) =>
       verifyProfile({
@@ -98,24 +121,8 @@ export default function createAdStrategy(): SamlStrategy | DevPassportStrategy {
 
     return new DevPassportStrategy(getter, upserter)
   } else {
-    if (!adConfig) throw Error('Missing AD SAML configuration')
     return new SamlStrategy(
-      {
-        callbackUrl: adConfig.callbackUrl,
-        entryPoint: adConfig.entryPointUrl,
-        logoutUrl: adConfig.logoutUrl,
-        issuer: adConfig.issuer,
-        cert: adConfig.publicCert.map(
-          (certificateName) => certificates[certificateName]
-        ),
-        privateCert: readFileSync(adConfig.privateCert, {
-          encoding: 'utf8'
-        }),
-        identifierFormat: 'urn:oasis:names:tc:SAML:2.0:nameid-format:transient',
-        disableRequestedAuthnContext: true,
-        signatureAlgorithm: 'sha256',
-        acceptedClockSkewMs: -1
-      },
+      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/keycloak-saml.ts b/apigw/src/shared/auth/keycloak-saml.ts
index 7370e9e9457..b8bce121b65 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'
@@ -12,7 +13,7 @@ import { getOrCreateEmployee } from '../service-client'
 import { evakaSamlConfig } from '../config'
 import fs from 'fs'
 
-export default function createKeycloakSamlStrategy(): SamlStrategy {
+export function createSamlConfig(): SamlConfig {
   if (!evakaSamlConfig) throw new Error('Missing Keycloak SAML configuration')
   const publicCert = fs.readFileSync(evakaSamlConfig.publicCert, {
     encoding: 'utf8'
@@ -20,19 +21,25 @@ export default function createKeycloakSamlStrategy(): SamlStrategy {
   const privateCert = fs.readFileSync(evakaSamlConfig.privateCert, {
     encoding: 'utf8'
   })
+  return {
+    issuer: 'evaka',
+    callbackUrl: evakaSamlConfig.callbackUrl,
+    entryPoint: evakaSamlConfig.entryPoint,
+    logoutUrl: evakaSamlConfig.entryPoint,
+    acceptedClockSkewMs: -1,
+    cert: publicCert,
+    privateCert: privateCert,
+    decryptionPvk: privateCert,
+    identifierFormat: 'urn:oasis:names:tc:SAML:2.0:nameid-format:transient',
+    signatureAlgorithm: 'sha256'
+  }
+}
+
+export default function createKeycloakSamlStrategy(
+  config: SamlConfig
+): SamlStrategy {
   return new SamlStrategy(
-    {
-      issuer: 'evaka',
-      callbackUrl: evakaSamlConfig.callbackUrl,
-      entryPoint: evakaSamlConfig.entryPoint,
-      logoutUrl: evakaSamlConfig.entryPoint,
-      acceptedClockSkewMs: -1,
-      cert: publicCert,
-      privateCert: privateCert,
-      decryptionPvk: privateCert,
-      identifierFormat: 'urn:oasis:names:tc:SAML:2.0:nameid-format:transient',
-      signatureAlgorithm: 'sha256'
-    },
+    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 08b5eed1d8c..ece2476613d 100644
--- a/apigw/src/shared/auth/suomi-fi-saml.ts
+++ b/apigw/src/shared/auth/suomi-fi-saml.ts
@@ -2,7 +2,7 @@
 //
 // SPDX-License-Identifier: LGPL-2.1-or-later
 
-import { Profile, Strategy, VerifiedCallback } from 'passport-saml'
+import { Profile, SamlConfig, Strategy, VerifiedCallback } from 'passport-saml'
 import { SamlUser } from '../routes/auth/saml/types'
 import { Strategy as DummyStrategy } from 'passport-dummy'
 import { sfiConfig, sfiMock } from '../config'
@@ -52,7 +52,34 @@ async function verifyProfile(profile: SuomiFiProfile): Promise<SamlUser> {
     sessionIndex: profile.sessionIndex
   }
 }
-export default function createSuomiFiStrategy(): Strategy | DummyStrategy {
+
+export function createSamlConfig(): SamlConfig {
+  if (sfiMock) return {}
+  if (!sfiConfig) throw new Error('Missing Suomi.fi SAML configuration')
+
+  const privateCert = fs.readFileSync(sfiConfig.privateCert, {
+    encoding: 'utf8'
+  })
+  return {
+    callbackUrl: sfiConfig.callbackUrl,
+    entryPoint: sfiConfig.entryPoint,
+    logoutUrl: sfiConfig.logoutUrl,
+    issuer: sfiConfig.issuer,
+    cert: sfiConfig.publicCert.map(
+      (certificateName) => certificates[certificateName]
+    ),
+    privateCert: privateCert,
+    decryptionPvk: privateCert,
+    identifierFormat: 'urn:oasis:names:tc:SAML:2.0:nameid-format:transient',
+    disableRequestedAuthnContext: true,
+    signatureAlgorithm: 'sha256',
+    acceptedClockSkewMs: -1
+  }
+}
+
+export default function createSuomiFiStrategy(
+  config: SamlConfig
+): Strategy | DummyStrategy {
   if (sfiMock) {
     return new DummyStrategy((done) => {
       verifyProfile(dummySuomiFiProfile)
@@ -60,32 +87,11 @@ export default function createSuomiFiStrategy(): Strategy | DummyStrategy {
         .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(
-      {
-        callbackUrl: sfiConfig.callbackUrl,
-        entryPoint: sfiConfig.entryPoint,
-        logoutUrl: sfiConfig.logoutUrl,
-        issuer: sfiConfig.issuer,
-        cert: sfiConfig.publicCert.map(
-          (certificateName) => certificates[certificateName]
-        ),
-        privateCert: privateCert,
-        decryptionPvk: privateCert,
-        identifierFormat: 'urn:oasis:names:tc:SAML:2.0:nameid-format:transient',
-        disableRequestedAuthnContext: true,
-        signatureAlgorithm: 'sha256',
-        acceptedClockSkewMs: -1
-      },
-      (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/routes/auth/saml/types.ts b/apigw/src/shared/routes/auth/saml/types.ts
index c4d7c8bd7bb..faea9d1ac1a 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 { Strategy } from 'passport-saml'
+import { SamlConfig, Strategy } from 'passport-saml'
 import { Strategy as DummyStrategy } from 'passport-dummy'
 import { SessionType } from '../../../session'
 import { UserType } from '../../../service-client'
@@ -10,6 +10,7 @@ import { 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/auth/suomi-fi.ts b/apigw/src/shared/routes/auth/suomi-fi.ts
index 56f87914fab..5537e52e594 100644
--- a/apigw/src/shared/routes/auth/suomi-fi.ts
+++ b/apigw/src/shared/routes/auth/suomi-fi.ts
@@ -3,13 +3,17 @@
 // SPDX-License-Identifier: LGPL-2.1-or-later
 
 import createSamlRouter from './saml'
-import createSuomiFiStrategy from '../../auth/suomi-fi-saml'
+import createSuomiFiStrategy, {
+  createSamlConfig as createSuomiFiSamlConfig
+} from '../../auth/suomi-fi-saml'
 import { SessionType } from '../../session'
 
 export function createAuthEndpoints(sessionType: SessionType) {
+  const samlConfig = createSuomiFiSamlConfig()
   return createSamlRouter({
     strategyName: 'suomifi',
-    strategy: createSuomiFiStrategy(),
+    strategy: createSuomiFiStrategy(samlConfig),
+    samlConfig,
     sessionType,
     pathIdentifier: 'saml'
   })