From da5b194c0df548ee0c46a53d90bbcefd338b70be Mon Sep 17 00:00:00 2001 From: Sven Tennie Date: Wed, 8 Jul 2026 06:07:46 +0200 Subject: [PATCH 01/22] Fix test description --- integration/test/Test/Spar/GetByEmail.hs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/integration/test/Test/Spar/GetByEmail.hs b/integration/test/Test/Spar/GetByEmail.hs index 155bee47f48..1402b5370ee 100644 --- a/integration/test/Test/Spar/GetByEmail.hs +++ b/integration/test/Test/Spar/GetByEmail.hs @@ -169,9 +169,9 @@ testGetSsoCodeByEmailRegular (TaggedBool requireExternalEmailVerification) (Tagg ssoCodeStr <- resp.json %. "sso_code" >>= asString ssoCodeStr `shouldMatch` idpId --- | Test that non-SCIM users get no SSO code -testGetSsoCodeByEmailNonScimUser :: (HasCallStack) => App () -testGetSsoCodeByEmailNonScimUser = do +-- | Test that non-SSO users get no SSO code +testGetSsoCodeByEmailNonSSOUser :: (HasCallStack) => App () +testGetSsoCodeByEmailNonSSOUser = do withModifiedBackend def {sparCfg = setField "enableIdPByEmailDiscovery" True} $ \domain -> do @@ -186,7 +186,7 @@ testGetSsoCodeByEmailNonScimUser = do usr <- randomUser domain def {activate = True} userEmail <- usr %. "email" & asString - -- Try to get SSO code for regular (non-SCIM) user - should return 404 with null + -- Try to get SSO code for regular (non-SSO) user - should return 404 with null getSsoCodeByEmail domain userEmail `bindResponse` \resp -> do resp.status `shouldMatchInt` 404 mbSsoCode <- lookupField resp.json "sso_code" From 985d5c09bb9088d9b8b8edddbd929f711fd8d825 Mon Sep 17 00:00:00 2001 From: Sven Tennie Date: Wed, 8 Jul 2026 07:21:41 +0200 Subject: [PATCH 02/22] Infra --- .../test/Test/Spar/CertFingerprintAllowlist.hs | 7 +------ integration/test/Testlib/Certs.hs | 6 ++++++ .../src/SAML2/WebSSO/Test/Util/TestSP.hs | 11 +++++++++-- 3 files changed, 16 insertions(+), 8 deletions(-) diff --git a/integration/test/Test/Spar/CertFingerprintAllowlist.hs b/integration/test/Test/Spar/CertFingerprintAllowlist.hs index a3e513900ff..b88d47b69a1 100644 --- a/integration/test/Test/Spar/CertFingerprintAllowlist.hs +++ b/integration/test/Test/Spar/CertFingerprintAllowlist.hs @@ -5,12 +5,11 @@ import API.Spar (createIdpWithZHostV2, updateIdp) import Control.Lens ((.~), (^.)) import Data.List.NonEmpty (NonEmpty ((:|))) import qualified Data.List.NonEmpty as NE -import qualified Data.Text as T import Data.X509 (SignedCertificate) -import qualified Data.X509.Extended as X509E import qualified SAML2.WebSSO.Test.Util as SAMLTest import qualified SAML2.WebSSO.Types as SAMLTypes import SetupHelpers +import Testlib.Certs (fingerprintHex) import Testlib.Prelude import qualified Text.XML.DSig as XMLDSig @@ -137,10 +136,6 @@ bogusFingerprint = "0000000000000000000000000000000000000000" firstCert :: SAMLTypes.IdPMetadata -> SignedCertificate firstCert meta = NE.head $ meta ^. SAMLTypes.edCertAuthnResponse --- | Cert's SHA-1 fingerprint in canonical @AA:BB:..@ hex form. -fingerprintHex :: SignedCertificate -> String -fingerprintHex = T.unpack . X509E.renderFingerprintHex . X509E.certSha1Fingerprint - -- | First cert's SHA-1, canonical @AA:BB:..@ form. firstCertFingerprint :: SAMLTypes.IdPMetadata -> String firstCertFingerprint = fingerprintHex . firstCert diff --git a/integration/test/Testlib/Certs.hs b/integration/test/Testlib/Certs.hs index f0ccdcdfb1a..026c45d494f 100644 --- a/integration/test/Testlib/Certs.hs +++ b/integration/test/Testlib/Certs.hs @@ -27,7 +27,9 @@ import Data.Hourglass import Data.Hourglass.Const import Data.PEM (PEM (PEM), pemWriteBS) import Data.String.Conversions (cs) +import qualified Data.Text as T import Data.X509 +import Data.X509.Extended import Testlib.Prelude type RSAKeyPair = (RSA.PublicKey, RSA.PrivateKey) @@ -139,3 +141,7 @@ mkSignedCert pubKey privKey caName ownerName = certPubKey = PubKeyRSA pubKey, certExtensions = Extensions Nothing } + +-- | Cert's SHA-1 fingerprint in canonical @AA:BB:..@ hex form. +fingerprintHex :: SignedCertificate -> String +fingerprintHex = T.unpack . renderFingerprintHex . certSha1Fingerprint diff --git a/libs/saml2-web-sso/src/SAML2/WebSSO/Test/Util/TestSP.hs b/libs/saml2-web-sso/src/SAML2/WebSSO/Test/Util/TestSP.hs index eab238e7b6d..0ac5f015d25 100644 --- a/libs/saml2-web-sso/src/SAML2/WebSSO/Test/Util/TestSP.hs +++ b/libs/saml2-web-sso/src/SAML2/WebSSO/Test/Util/TestSP.hs @@ -35,6 +35,7 @@ import Data.Time import Data.UUID qualified as UUID import Data.UUID.V4 qualified as UUID import Data.Void (Void) +import Data.X509 qualified as X509 import SAML2.WebSSO as SAML import SAML2.WebSSO.API.Example (GetAllIdPs (..), RequestStore, simpleGetIdPConfigBy, simpleGetIdpIssuer', simpleIsAliveID', simpleStoreID', simpleStoreRequest', simpleUnStoreID', simpleUnStoreRequest') import SAML2.WebSSO.Test.Util.Types @@ -212,12 +213,18 @@ makeTestIdPConfig = do pure (IdPConfig {..}, sampleIdP) makeSampleIdPMetadata :: (HasCallStack) => (MonadIO m, MonadRandom m) => m SampleIdP -makeSampleIdPMetadata = do +makeSampleIdPMetadata = SAML.mkSignCredsWithCert Nothing 96 >>= makeSampleIdPMetadataWithCert + +makeSampleIdPMetadataWithCert :: + (HasCallStack) => + (MonadIO m, MonadRandom m) => + (SignPrivCreds, SignCreds, X509.SignedCertificate) -> + m SampleIdP +makeSampleIdPMetadataWithCert (privcreds, creds, cert) = do issuer <- makeIssuer requri <- do uuid <- UUID.toASCIIBytes <$> liftIO UUID.nextRandom pure $ [uri|https://requri.net/|] & pathL .~ ("/" <> uuid) - (privcreds, creds, cert) <- SAML.mkSignCredsWithCert Nothing 96 pure $ SampleIdP (IdPMetadata issuer requri (NonEmpty.singleton cert)) privcreds creds cert makeIssuer :: (MonadIO m) => m Issuer From 96cb6aea4b72d3397ce6e06b395ace8b385c7d54 Mon Sep 17 00:00:00 2001 From: Sven Tennie Date: Wed, 8 Jul 2026 07:21:47 +0200 Subject: [PATCH 03/22] Adjust testMultiIngressSSOGeneralIdp --- integration/test/Test/Spar/MultiIngressSSO.hs | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/integration/test/Test/Spar/MultiIngressSSO.hs b/integration/test/Test/Spar/MultiIngressSSO.hs index 65bc8ebdc3b..3c9fab1ecbd 100644 --- a/integration/test/Test/Spar/MultiIngressSSO.hs +++ b/integration/test/Test/Spar/MultiIngressSSO.hs @@ -28,7 +28,9 @@ import qualified Data.Text as T import qualified Data.UUID as UUID import GHC.Stack import qualified SAML2.WebSSO as SAML +import SAML2.WebSSO.Test.Util import SetupHelpers +import Testlib.Certs (fingerprintHex) import qualified Testlib.KleisliXML as KXML import Testlib.Prelude import qualified Text.XML as XML @@ -47,6 +49,8 @@ testMultiIngressSSOGeneralIdp = do bertZHost = "nginz-https.bert.example.com" kermitZHost = "nginz-https.kermit.example.com" + ernieCredsWithCert@(_, _, signedCert) <- SAML.mkSignCredsWithCert Nothing 96 + withModifiedBackend def { sparCfg = @@ -70,13 +74,18 @@ testMultiIngressSSOGeneralIdp = do ] ] ) + -- TODO: Use withAllowlist + >=> setField "idpCertFingerprintAllowlist" [fingerprintHex signedCert] } $ \domain -> do (owner, tid, _) <- createTeam domain 1 void $ setTeamFeatureStatus owner tid "sso" "enabled" - (idp, _idpMeta) <- registerTestIdPWithMetaWithPrivateCreds owner - idpId <- asString $ idp.json %. "id" + SampleIdP idpmeta _pCreds _ _ <- makeSampleIdPMetadataWithCert ernieCredsWithCert + idpId <- + createIdpWithZHostV2 owner Nothing idpmeta `bindResponse` \resp -> do + assertStatus 201 resp + resp.json %. "id" >>= asString _ernieEmail <- ("ernie@" <>) <$> randomDomain checkSPMetadata domain ernieZHost tid From e2a80a3a8531f0023e63f7551a11bdce8f7ddabf Mon Sep 17 00:00:00 2001 From: Sven Tennie Date: Wed, 8 Jul 2026 07:33:34 +0200 Subject: [PATCH 04/22] Adjust testMultiIngressSSODomainBoundIdp --- integration/test/Test/Spar/MultiIngressSSO.hs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/integration/test/Test/Spar/MultiIngressSSO.hs b/integration/test/Test/Spar/MultiIngressSSO.hs index 3c9fab1ecbd..af629d4975b 100644 --- a/integration/test/Test/Spar/MultiIngressSSO.hs +++ b/integration/test/Test/Spar/MultiIngressSSO.hs @@ -120,6 +120,8 @@ testMultiIngressSSODomainBoundIdp = do bertZHost = "nginz-https.bert.example.com" kermitZHost = "nginz-https.kermit.example.com" + ernieCredsWithCert@(_, _, ernieCert) <- SAML.mkSignCredsWithCert Nothing 96 + withModifiedBackend def { sparCfg = @@ -143,13 +145,18 @@ testMultiIngressSSODomainBoundIdp = do ] ] ) + >=> setField "idpCertFingerprintAllowlist" [fingerprintHex ernieCert] } $ \domain -> do (owner, tid, _) <- createTeam domain 1 void $ setTeamFeatureStatus owner tid "sso" "enabled" - (idp, idpMeta) <- registerTestIdPWithMetaWithPrivateCredsForZHost owner (Just ernieZHost) - idpId <- asString $ idp.json %. "id" + SampleIdP ernieIdpmeta erniePrivCreds _ _ <- makeSampleIdPMetadataWithCert ernieCredsWithCert + idpId <- + createIdpWithZHostV2 owner (Just ernieZHost) ernieIdpmeta `bindResponse` \resp -> do + assertStatus 201 resp + resp.json %. "id" >>= asString + let idpMeta = (ernieIdpmeta, erniePrivCreds) ernieEmail <- ("ernie@" <>) <$> randomDomain checkSPMetadata domain ernieZHost tid From a2b45676d8c352664a0d524a0893555d0cad0b4a Mon Sep 17 00:00:00 2001 From: Sven Tennie Date: Wed, 8 Jul 2026 07:49:31 +0200 Subject: [PATCH 05/22] Adjust testMultiIngressIdpSimpleCase --- integration/test/Test/Spar/MultiIngressIdp.hs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/integration/test/Test/Spar/MultiIngressIdp.hs b/integration/test/Test/Spar/MultiIngressIdp.hs index 58960b5573a..4dfa08324f7 100644 --- a/integration/test/Test/Spar/MultiIngressIdp.hs +++ b/integration/test/Test/Spar/MultiIngressIdp.hs @@ -6,7 +6,9 @@ import Control.Lens ((.~), (^.)) import qualified SAML2.WebSSO.Test.Util as SAML import qualified SAML2.WebSSO.Types as SAML import SetupHelpers +import Testlib.Certs (fingerprintHex) import Testlib.Prelude +import qualified Text.XML.DSig as XMLDSig ernieZHost :: String ernieZHost = "nginz-https.ernie.example.com" @@ -28,6 +30,7 @@ makeSpDomainConfig zhost = testMultiIngressIdpSimpleCase :: (HasCallStack) => App () testMultiIngressIdpSimpleCase = do + credsWithCert@(_, _, signedCert) <- XMLDSig.mkSignCredsWithCert Nothing 96 withModifiedBackend def { sparCfg = @@ -42,13 +45,14 @@ testMultiIngressIdpSimpleCase = do kermitZHost .= makeSpDomainConfig kermitZHost ] ) + >=> setField "idpCertFingerprintAllowlist" [fingerprintHex signedCert] } $ \domain -> do (owner, tid, _) <- createTeam domain 1 void $ setTeamFeatureStatus owner tid "sso" "enabled" -- Create IdP for one domain - SAML.SampleIdP idpmeta _ _ _ <- SAML.makeSampleIdPMetadata + SAML.SampleIdP idpmeta _ _ _ <- SAML.makeSampleIdPMetadataWithCert credsWithCert idpId <- createIdpWithZHostV2 owner (Just ernieZHost) idpmeta `bindResponse` \resp -> do resp.status `shouldMatchInt` 201 From a9e3a6aa3a8cac8c587ca2ffbf3d3801d2081c5b Mon Sep 17 00:00:00 2001 From: Sven Tennie Date: Wed, 8 Jul 2026 07:54:43 +0200 Subject: [PATCH 06/22] Adjust testUnconfiguredDomain --- integration/test/Test/Spar/MultiIngressIdp.hs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/integration/test/Test/Spar/MultiIngressIdp.hs b/integration/test/Test/Spar/MultiIngressIdp.hs index 4dfa08324f7..33a25f1abfd 100644 --- a/integration/test/Test/Spar/MultiIngressIdp.hs +++ b/integration/test/Test/Spar/MultiIngressIdp.hs @@ -80,6 +80,9 @@ testMultiIngressIdpSimpleCase = do -- multi-ingress domain. testUnconfiguredDomain :: (HasCallStack) => App () testUnconfiguredDomain = forM_ [Nothing, Just kermitZHost] $ \unconfiguredZHost -> do + credsWithCert1@(_, _, signedCert1) <- XMLDSig.mkSignCredsWithCert Nothing 96 + credsWithCert2@(_, _, signedCert2) <- XMLDSig.mkSignCredsWithCert Nothing 96 + credsWithCert3@(_, _, signedCert3) <- XMLDSig.mkSignCredsWithCert Nothing 96 withModifiedBackend def { sparCfg = @@ -89,12 +92,13 @@ testUnconfiguredDomain = forM_ [Nothing, Just kermitZHost] $ \unconfiguredZHost >=> setField "saml.spDomainConfigs" (object [ernieZHost .= makeSpDomainConfig ernieZHost]) + >=> setField "idpCertFingerprintAllowlist" (fingerprintHex <$> [signedCert1, signedCert2, signedCert3]) } $ \domain -> do (owner, tid, _) <- createTeam domain 1 void $ setTeamFeatureStatus owner tid "sso" "enabled" - SAML.SampleIdP idpmeta1 _ _ _ <- SAML.makeSampleIdPMetadata + SAML.SampleIdP idpmeta1 _ _ _ <- SAML.makeSampleIdPMetadataWithCert credsWithCert1 idpId1 <- createIdpWithZHostV2 owner (Just ernieZHost) idpmeta1 `bindResponse` \resp -> do resp.status `shouldMatchInt` 201 @@ -120,7 +124,7 @@ testUnconfiguredDomain = forM_ [Nothing, Just kermitZHost] $ \unconfiguredZHost resp.json %. "extraInfo.domain" `shouldMatch` ernieZHost -- Create unconfigured -> no multi-ingress domain - SAML.SampleIdP idpmeta2 _ _ _ <- SAML.makeSampleIdPMetadata + SAML.SampleIdP idpmeta2 _ _ _ <- SAML.makeSampleIdPMetadataWithCert credsWithCert2 idpId2 <- createIdpWithZHostV2 owner (unconfiguredZHost) idpmeta2 `bindResponse` \resp -> do resp.status `shouldMatchInt` 201 @@ -132,7 +136,7 @@ testUnconfiguredDomain = forM_ [Nothing, Just kermitZHost] $ \unconfiguredZHost resp.json %. "extraInfo.domain" `shouldMatch` Null -- Create a second unconfigured -> no multi-ingress domain - SAML.SampleIdP idpmeta3 _ _ _ <- SAML.makeSampleIdPMetadata + SAML.SampleIdP idpmeta3 _ _ _ <- SAML.makeSampleIdPMetadataWithCert credsWithCert3 idpId3 <- createIdpWithZHostV2 owner (unconfiguredZHost) idpmeta3 `bindResponse` \resp -> do resp.status `shouldMatchInt` 201 From fabf91194c7e90642a53968a5ff39e9a40b615fc Mon Sep 17 00:00:00 2001 From: Sven Tennie Date: Wed, 8 Jul 2026 07:58:56 +0200 Subject: [PATCH 07/22] Adjust testMultiIngressAtMostOneIdPPerDomain --- integration/test/Test/Spar/MultiIngressIdp.hs | 23 ++++++++++++++----- 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/integration/test/Test/Spar/MultiIngressIdp.hs b/integration/test/Test/Spar/MultiIngressIdp.hs index 33a25f1abfd..3cfc08da82d 100644 --- a/integration/test/Test/Spar/MultiIngressIdp.hs +++ b/integration/test/Test/Spar/MultiIngressIdp.hs @@ -149,6 +149,12 @@ testUnconfiguredDomain = forM_ [Nothing, Just kermitZHost] $ \unconfiguredZHost testMultiIngressAtMostOneIdPPerDomain :: (HasCallStack) => App () testMultiIngressAtMostOneIdPPerDomain = do + credsWithCert1@(_, _, signedCert1) <- XMLDSig.mkSignCredsWithCert Nothing 96 + credsWithCert2@(_, _, signedCert2) <- XMLDSig.mkSignCredsWithCert Nothing 96 + credsWithCert3@(_, _, signedCert3) <- XMLDSig.mkSignCredsWithCert Nothing 96 + credsWithCert4@(_, _, signedCert4) <- XMLDSig.mkSignCredsWithCert Nothing 96 + credsWithCert5@(_, _, signedCert5) <- XMLDSig.mkSignCredsWithCert Nothing 96 + credsWithCert6@(_, _, signedCert6) <- XMLDSig.mkSignCredsWithCert Nothing 96 withModifiedBackend def { sparCfg = @@ -163,26 +169,31 @@ testMultiIngressAtMostOneIdPPerDomain = do kermitZHost .= makeSpDomainConfig kermitZHost ] ) + >=> setField + "idpCertFingerprintAllowlist" + ( fingerprintHex + <$> [signedCert1, signedCert2, signedCert3, signedCert4, signedCert5, signedCert6] + ) } $ \domain -> do (owner, tid, _) <- createTeam domain 1 void $ setTeamFeatureStatus owner tid "sso" "enabled" - SAML.SampleIdP idpmeta1 _ _ _ <- SAML.makeSampleIdPMetadata + SAML.SampleIdP idpmeta1 _ _ _ <- SAML.makeSampleIdPMetadataWithCert credsWithCert1 idpId1 <- createIdpWithZHostV2 owner (Just ernieZHost) idpmeta1 `bindResponse` \resp -> do resp.status `shouldMatchInt` 201 resp.json %. "id" >>= asString -- Creating a second IdP for the same domain -> failure - SAML.SampleIdP idpmeta2 _ _ _ <- SAML.makeSampleIdPMetadata + SAML.SampleIdP idpmeta2 _ _ _ <- SAML.makeSampleIdPMetadataWithCert credsWithCert2 _idpId2 <- createIdpWithZHostV2 owner (Just ernieZHost) idpmeta2 `bindResponse` \resp -> do resp.status `shouldMatchInt` 409 resp.json %. "label" `shouldMatch` "idp-duplicate-domain-for-team" -- Create an IdP for one domain and update it to another that already has one -> failure - SAML.SampleIdP idpmeta3 _ _ _ <- SAML.makeSampleIdPMetadata + SAML.SampleIdP idpmeta3 _ _ _ <- SAML.makeSampleIdPMetadataWithCert credsWithCert3 idpId3 <- createIdpWithZHostV2 owner (Just bertZHost) idpmeta2 `bindResponse` \resp -> do resp.status `shouldMatchInt` 201 @@ -194,7 +205,7 @@ testMultiIngressAtMostOneIdPPerDomain = do resp.json %. "label" `shouldMatch` "idp-duplicate-domain-for-team" -- Create an IdP with no domain and update it to a domain that already has one -> failure - SAML.SampleIdP idpmeta4 _ _ _ <- SAML.makeSampleIdPMetadata + SAML.SampleIdP idpmeta4 _ _ _ <- SAML.makeSampleIdPMetadataWithCert credsWithCert4 idpId4 <- createIdpWithZHostV2 owner Nothing idpmeta4 `bindResponse` \resp -> do resp.status `shouldMatchInt` 201 @@ -221,7 +232,7 @@ testMultiIngressAtMostOneIdPPerDomain = do deleteIdp owner idpId1 `bindResponse` \resp -> do resp.status `shouldMatchInt` 204 - SAML.SampleIdP idpmeta5 _ _ _ <- SAML.makeSampleIdPMetadata + SAML.SampleIdP idpmeta5 _ _ _ <- SAML.makeSampleIdPMetadataWithCert credsWithCert5 idpId5 <- createIdpWithZHostV2 owner (Just ernieZHost) idpmeta5 `bindResponse` \resp -> do resp.status `shouldMatchInt` 201 @@ -229,7 +240,7 @@ testMultiIngressAtMostOneIdPPerDomain = do resp.json %. "id" >>= asString -- After deletion of the IdP of a domain, one can be moved from another domain - SAML.SampleIdP idpmeta6 _ _ _ <- SAML.makeSampleIdPMetadata + SAML.SampleIdP idpmeta6 _ _ _ <- SAML.makeSampleIdPMetadataWithCert credsWithCert6 createIdpWithZHostV2 owner (Just bertZHost) idpmeta6 `bindResponse` \resp -> do resp.status `shouldMatchInt` 409 resp.json %. "label" `shouldMatch` "idp-duplicate-domain-for-team" From 20f604f5683e3d6f34bc348cc139106aef31b749 Mon Sep 17 00:00:00 2001 From: Sven Tennie Date: Wed, 8 Jul 2026 08:01:55 +0200 Subject: [PATCH 08/22] Adjust testMultiIngressIdPIssuerDifferentDomains --- integration/test/Test/Spar/MultiIngressIdp.hs | 25 +++++++++++++++---- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/integration/test/Test/Spar/MultiIngressIdp.hs b/integration/test/Test/Spar/MultiIngressIdp.hs index 3cfc08da82d..883ad3fdb91 100644 --- a/integration/test/Test/Spar/MultiIngressIdp.hs +++ b/integration/test/Test/Spar/MultiIngressIdp.hs @@ -335,6 +335,11 @@ testNonMultiIngressSetupsCanHaveMoreIdPsPerDomain = do -- practical benefit, this complexity is not justified for now. testMultiIngressIdPIssuerDifferentDomains :: (HasCallStack) => App () testMultiIngressIdPIssuerDifferentDomains = do + credsWithCertV1@(_, _, signedCertV1) <- XMLDSig.mkSignCredsWithCert Nothing 96 + credsWithCertV1_alt@(_, _, signedCertV1_alt) <- XMLDSig.mkSignCredsWithCert Nothing 96 + credsWithCertV1_differentIssuer@(_, _, signedCertV1_differentIssuer) <- XMLDSig.mkSignCredsWithCert Nothing 96 + credsWithCertV2@(_, _, signedCertV2) <- XMLDSig.mkSignCredsWithCert Nothing 96 + credsWithCertV2_alt@(_, _, signedCertV2_alt) <- XMLDSig.mkSignCredsWithCert Nothing 96 withModifiedBackend def { sparCfg = @@ -349,6 +354,16 @@ testMultiIngressIdPIssuerDifferentDomains = do kermitZHost .= makeSpDomainConfig kermitZHost ] ) + >=> setField + "idpCertFingerprintAllowlist" + ( fingerprintHex + <$> [ signedCertV1, + signedCertV1_alt, + signedCertV1_differentIssuer, + signedCertV2, + signedCertV2_alt + ] + ) } $ \domain -> do -- V1 API: Issuers must be unique per backend (across all teams) @@ -356,7 +371,7 @@ testMultiIngressIdPIssuerDifferentDomains = do void $ setTeamFeatureStatus owner1 tid1 "sso" "enabled" -- Create first IdP metadata for V1 - SAML.SampleIdP idpmetaV1 _ _ _ <- SAML.makeSampleIdPMetadata + SAML.SampleIdP idpmetaV1 _ _ _ <- SAML.makeSampleIdPMetadataWithCert credsWithCertV1 _idpId1 <- createIdpWithZHostV1 owner1 (Just ernieZHost) idpmetaV1 `bindResponse` \resp -> do resp.status `shouldMatchInt` 201 @@ -369,7 +384,7 @@ testMultiIngressIdPIssuerDifferentDomains = do void $ setTeamFeatureStatus owner2 tid2 "sso" "enabled" -- Try with same domain as original -> should fail (V1 global uniqueness) - SAML.SampleIdP idpmetaV1_alt _ _ _ <- SAML.makeSampleIdPMetadata + SAML.SampleIdP idpmetaV1_alt _ _ _ <- SAML.makeSampleIdPMetadataWithCert credsWithCertV1_alt let idpmetaV1_alt_sameIssuer = idpmetaV1_alt & SAML.edIssuer .~ (idpmetaV1 ^. SAML.edIssuer) createIdpWithZHostV1 owner2 (Just ernieZHost) idpmetaV1_alt_sameIssuer `bindResponse` \resp -> do @@ -387,7 +402,7 @@ testMultiIngressIdPIssuerDifferentDomains = do resp.json %. "label" `shouldMatch` "idp-already-in-use" -- Counter-example: V1 IdP with different issuer -> success - SAML.SampleIdP idpmetaV1_differentIssuer _ _ _ <- SAML.makeSampleIdPMetadata + SAML.SampleIdP idpmetaV1_differentIssuer _ _ _ <- SAML.makeSampleIdPMetadataWithCert credsWithCertV1_differentIssuer void $ createIdpWithZHostV1 owner2 (Just ernieZHost) idpmetaV1_differentIssuer `bindResponse` \resp -> do @@ -399,7 +414,7 @@ testMultiIngressIdPIssuerDifferentDomains = do void $ setTeamFeatureStatus owner3 tid3 "sso" "enabled" -- Create V2 IdP on team 3 with new issuer - SAML.SampleIdP idpmetaV2 _ _ _ <- SAML.makeSampleIdPMetadata + SAML.SampleIdP idpmetaV2 _ _ _ <- SAML.makeSampleIdPMetadataWithCert credsWithCertV2 _idpId3 <- createIdpWithZHostV2 owner3 (Just ernieZHost) idpmetaV2 `bindResponse` \resp -> do @@ -409,7 +424,7 @@ testMultiIngressIdPIssuerDifferentDomains = do -- Try to create another V2 IdP on same team with different metadata but same issuer -> failure -- First, try with the same domain -> hits domain constraint (409) - SAML.SampleIdP idpmetaV2_alt _ _ _ <- SAML.makeSampleIdPMetadata + SAML.SampleIdP idpmetaV2_alt _ _ _ <- SAML.makeSampleIdPMetadataWithCert credsWithCertV2_alt let idpmetaV2_alt_sameIssuer = idpmetaV2_alt & SAML.edIssuer .~ (idpmetaV2 ^. SAML.edIssuer) createIdpWithZHostV2 owner3 (Just ernieZHost) idpmetaV2_alt_sameIssuer `bindResponse` \resp -> do From bf969d3b9bcf25d19087c9226571a4bd3856d9b0 Mon Sep 17 00:00:00 2001 From: Sven Tennie Date: Wed, 8 Jul 2026 08:29:25 +0200 Subject: [PATCH 09/22] Adjust testGetSsoCodeByEmailWithMultiIngress --- integration/test/Test/Spar/GetByEmail.hs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/integration/test/Test/Spar/GetByEmail.hs b/integration/test/Test/Spar/GetByEmail.hs index 1402b5370ee..99f6c1d52e2 100644 --- a/integration/test/Test/Spar/GetByEmail.hs +++ b/integration/test/Test/Spar/GetByEmail.hs @@ -23,7 +23,9 @@ import API.Spar import GHC.Stack import qualified SAML2.WebSSO.Test.Util as SAML import SetupHelpers +import Testlib.Certs (fingerprintHex) import Testlib.Prelude +import qualified Text.XML.DSig as XMLDSig -- | Test the /sso/get-by-email endpoint with multi-ingress setup testGetSsoCodeByEmailWithMultiIngress :: @@ -35,6 +37,8 @@ testGetSsoCodeByEmailWithMultiIngress (TaggedBool requireExternalEmailVerificati let ernieZHost = "nginz-https.ernie.example.com" bertZHost = "nginz-https.bert.example.com" + credsWithCertErnie@(_, _, signedCertErnie) <- XMLDSig.mkSignCredsWithCert Nothing 96 + credsWithCertBert@(_, _, signedCertBert) <- XMLDSig.mkSignCredsWithCert Nothing 96 withModifiedBackend def { sparCfg = @@ -59,6 +63,9 @@ testGetSsoCodeByEmailWithMultiIngress (TaggedBool requireExternalEmailVerificati ] ] ) + >=> setField + "idpCertFingerprintAllowlist" + (fingerprintHex <$> [signedCertErnie, signedCertBert]) } $ \domain -> do (owner, tid, _) <- createTeam domain 1 @@ -69,7 +76,7 @@ testGetSsoCodeByEmailWithMultiIngress (TaggedBool requireExternalEmailVerificati assertSuccess =<< setTeamFeatureStatus owner tid "validateSAMLemails" status -- Create IdP for ernie domain - SAML.SampleIdP idpmetaErnie _ _ _ <- SAML.makeSampleIdPMetadata + SAML.SampleIdP idpmetaErnie _ _ _ <- SAML.makeSampleIdPMetadataWithCert credsWithCertErnie idpIdErnie <- createIdpWithZHostV2 owner (Just ernieZHost) idpmetaErnie `bindResponse` \resp -> do resp.status `shouldMatchInt` 201 @@ -77,7 +84,7 @@ testGetSsoCodeByEmailWithMultiIngress (TaggedBool requireExternalEmailVerificati resp.json %. "id" >>= asString -- Create IdP for bert domain - SAML.SampleIdP idpmetaBert _ _ _ <- SAML.makeSampleIdPMetadata + SAML.SampleIdP idpmetaBert _ _ _ <- SAML.makeSampleIdPMetadataWithCert credsWithCertBert idpIdBert <- createIdpWithZHostV2 owner (Just bertZHost) idpmetaBert `bindResponse` \resp -> do resp.status `shouldMatchInt` 201 From d5ebdd1350c3780d8b6da86998b924dc935001bb Mon Sep 17 00:00:00 2001 From: Sven Tennie Date: Wed, 8 Jul 2026 09:20:24 +0200 Subject: [PATCH 10/22] Adjust testGetSsoCodeByEmailDisabledMultiIngress --- integration/test/Test/Spar/GetByEmail.hs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/integration/test/Test/Spar/GetByEmail.hs b/integration/test/Test/Spar/GetByEmail.hs index 99f6c1d52e2..b5c59aab7e5 100644 --- a/integration/test/Test/Spar/GetByEmail.hs +++ b/integration/test/Test/Spar/GetByEmail.hs @@ -242,6 +242,7 @@ testGetSsoCodeByEmailDisabledMultiIngress = do let ernieZHost = "nginz-https.ernie.example.com" bertZHost = "nginz-https.bert.example.com" + credsWithCertErnie@(_, _, signedCertErnie) <- XMLDSig.mkSignCredsWithCert Nothing 96 withModifiedBackend def { sparCfg = @@ -266,13 +267,14 @@ testGetSsoCodeByEmailDisabledMultiIngress = do ] ] ) + >=> setField "idpCertFingerprintAllowlist" [fingerprintHex signedCertErnie] } $ \domain -> do (owner, tid, _) <- createTeam domain 1 void $ setTeamFeatureStatus owner tid "sso" "enabled" -- Create IdP for ernie domain - SAML.SampleIdP idpmetaErnie _ _ _ <- SAML.makeSampleIdPMetadata + SAML.SampleIdP idpmetaErnie _ _ _ <- SAML.makeSampleIdPMetadataWithCert credsWithCertErnie idpIdErnie <- createIdpWithZHostV2 owner (Just ernieZHost) idpmetaErnie `bindResponse` \resp -> do resp.status `shouldMatchInt` 201 From 02946aa457a11ef544f2f00f4384f6ed21203f7e Mon Sep 17 00:00:00 2001 From: Sven Tennie Date: Wed, 8 Jul 2026 09:20:48 +0200 Subject: [PATCH 11/22] Overhaul assertCertsallowlisted --- services/spar/src/Spar/API.hs | 44 ++++++++++++++++++++++++++--------- 1 file changed, 33 insertions(+), 11 deletions(-) diff --git a/services/spar/src/Spar/API.hs b/services/spar/src/Spar/API.hs index 655bdd6dc45..357c46979f9 100644 --- a/services/spar/src/Spar/API.hs +++ b/services/spar/src/Spar/API.hs @@ -813,8 +813,9 @@ idpCreateV7 samlConfig tid zUser idpmeta mReplaces mApiversion mHandle = do -- | Reject IdPs whose cert SHA-1 is not in the configured allowlist. -- --- Empty/absent allowlist is a no-op. On miss: warn-log fingerprint + --- issuer, then throw 'SparIdPCertNotAllowed' (HTTP 403). +-- Empty/absent allowlist is a no-op in the regular case, it short-circuits to +-- error for mulit-ingress setups. I.e. the allowlist is required for +-- multi-ingress setups. assertCertsAllowlisted :: ( Member (Input Opts) r, Member (Logger (Msg -> Msg)) r, @@ -824,24 +825,45 @@ assertCertsAllowlisted :: Sem r () assertCertsAllowlisted idpmeta = do mAllow <- inputs idpCertFingerprintAllowlist + samlConfig <- inputs saml + let certs = idpmeta ^. SAML.edCertAuthnResponse + issuerTxt = + TE.decodeUtf8 $ + URI.serializeURIRef' (idpmeta ^. SAML.edIssuer . SAML.fromIssuer) + when (isEmptyAllowList mAllow && SAML.isMultiIngressConfig samlConfig) $ do + forM_ certs $ \c -> do + let fingerprint = certSha1Fingerprint c + fingerprintHex = renderFingerprintHex fingerprint + logMultiIngressEmptyAllowlist fingerprintHex issuerTxt + throwSparSem (SparIdPCertNotAllowed (T.fromStrict fingerprintHex)) case mAllow of Nothing -> pure () Just (CertFingerprintAllowlist allowed) | Set.null allowed -> pure () | otherwise -> do - let certs = idpmeta ^. SAML.edCertAuthnResponse - issuerTxt = - TE.decodeUtf8 $ - URI.serializeURIRef' (idpmeta ^. SAML.edIssuer . SAML.fromIssuer) forM_ certs $ \c -> do let fingerprint = certSha1Fingerprint c + fingerprintHex = renderFingerprintHex fingerprint unless (Set.member fingerprint allowed) $ do - let fingerprintHex = renderFingerprintHex fingerprint - Logger.warn $ - Log.msg ("Refusing IdP request: cert fingerprint not in allowlist" :: ByteString) - . Log.field "fingerprint" fingerprintHex - . Log.field "issuer" issuerTxt + logCertNotInAllowlist fingerprintHex issuerTxt throwSparSem (SparIdPCertNotAllowed (T.fromStrict fingerprintHex)) + where + logMultiIngressEmptyAllowlist fingerprintHex issuerTxt = + Logger.warn $ + Log.msg ("Refusing IdP request: multi-ingress enabled and allowlist empty" :: ByteString) + . Log.field "fingerprint" fingerprintHex + . Log.field "issuer" issuerTxt + + logCertNotInAllowlist fingerprintHex issuerTxt = + Logger.warn $ + Log.msg ("Refusing IdP request: cert fingerprint not in allowlist" :: ByteString) + . Log.field "fingerprint" fingerprintHex + . Log.field "issuer" issuerTxt + + isEmptyAllowList :: Maybe CertFingerprintAllowlist -> Bool + isEmptyAllowList Nothing = True + isEmptyAllowList (Just (CertFingerprintAllowlist allowed)) | Set.null allowed = True + isEmptyAllowList (Just _) = False -- | Check that issuer is not used anywhere in the system ('WireIdPAPIV1', here it is a -- database key for finding IdPs), or anywhere in this team ('WireIdPAPIV2'), that request From 410b578fde16809427c5c0e2ec1972f8d009d25b Mon Sep 17 00:00:00 2001 From: Sven Tennie Date: Wed, 8 Jul 2026 09:32:25 +0200 Subject: [PATCH 12/22] WIP: unit tests --- services/spar/test/Test/Spar/Saml/IdPSpec.hs | 168 +++++++++++++++++-- 1 file changed, 151 insertions(+), 17 deletions(-) diff --git a/services/spar/test/Test/Spar/Saml/IdPSpec.hs b/services/spar/test/Test/Spar/Saml/IdPSpec.hs index 601a1b381ef..8eedc433883 100644 --- a/services/spar/test/Test/Spar/Saml/IdPSpec.hs +++ b/services/spar/test/Test/Spar/Saml/IdPSpec.hs @@ -526,8 +526,8 @@ spec = notifs `shouldBe` mempty describe "IdP cert fingerprint allowlist" $ do - let withAllow :: Maybe Spar.Options.CertFingerprintAllowlist -> Spar.Options.Opts - withAllow allow = defaultTestOpts {Spar.Options.idpCertFingerprintAllowlist = allow} + let withAllow :: Spar.Options.Opts -> Maybe Spar.Options.CertFingerprintAllowlist -> Spar.Options.Opts + withAllow opts allow = opts {Spar.Options.idpCertFingerprintAllowlist = allow} generateArbitraryIdPInfo :: IO IdPMetadataInfo generateArbitraryIdPInfo = do @@ -587,32 +587,32 @@ spec = describe "create" $ do it "accepts any cert when allowlist is Nothing" $ do idpInfo <- generateArbitraryIdPInfo - (_logs, res) <- runCreate (withAllow Nothing) idpInfo + (_logs, res) <- runCreate (defaultTestOpts `withAllow` Nothing) idpInfo res `shouldSatisfy` isRight it "accepts any cert when allowlist is empty" $ do idpInfo <- generateArbitraryIdPInfo let empty = Spar.Options.CertFingerprintAllowlist Set.empty - (_logs, res) <- runCreate (withAllow (Just empty)) idpInfo + (_logs, res) <- runCreate (defaultTestOpts `withAllow` (Just empty)) idpInfo res `shouldSatisfy` isRight it "accepts when all fingerprints are allowlisted" $ do idpInfo <- generateArbitraryIdPInfo let allow = allCertsAllowlist idpInfo - (_logs, res) <- runCreate (withAllow (Just allow)) idpInfo + (_logs, res) <- runCreate (defaultTestOpts `withAllow` (Just allow)) idpInfo res `shouldSatisfy` isRight it "accepts when all multi-cert fingerprints are allowlisted" $ do idpInfo <- generateTwoCertIdPInfo let allow = allCertsAllowlist idpInfo - (_logs, res) <- runCreate (withAllow (Just allow)) idpInfo + (_logs, res) <- runCreate (defaultTestOpts `withAllow` (Just allow)) idpInfo res `shouldSatisfy` isRight it "rejects when no fingerprint matches and logs the refusal" $ do idpInfo@(IdPMetadataValue _ m) <- generateArbitraryIdPInfo let allow = singletonAllowlist bogusFingerprint fingerprint = certToFingerprint . NonEmptyL.head $ m._edCertAuthnResponse - (logs, res) <- runCreate (withAllow (Just allow)) idpInfo + (logs, res) <- runCreate (defaultTestOpts `withAllow` (Just allow)) idpInfo res `shouldBe` Left (SAML.CustomError (SparIdPCertNotAllowed fingerprint)) let logged = TL.decodeUtf8 $ LBS.concat (map snd logs) logged `shouldSatisfy` (("cert fingerprint not in allowlist, fingerprint=" <> fingerprint) `TL.isInfixOf`) @@ -621,50 +621,85 @@ spec = idpInfo@(IdPMetadataValue _ m) <- generateTwoCertIdPInfo let allow = singletonAllowlist (firstCertFingerprint idpInfo) secondCertFingerprint = certToFingerprint . head . NonEmptyL.tail $ m._edCertAuthnResponse - (logs, res) <- runCreate (withAllow (Just allow)) idpInfo + (logs, res) <- runCreate (defaultTestOpts `withAllow` (Just allow)) idpInfo res `shouldBe` Left (SAML.CustomError (SparIdPCertNotAllowed secondCertFingerprint)) let logged = TL.decodeUtf8 $ LBS.concat (map snd logs) logged `shouldSatisfy` (("cert fingerprint not in allowlist, fingerprint=" <> secondCertFingerprint) `TL.isInfixOf`) + describe "multi-ingress" $ do + it "accepts when all fingerprints are allowlisted" $ do + idpInfo <- generateArbitraryIdPInfo + let allow = allCertsAllowlist idpInfo + optsWithMultiIngress = + defaultTestOpts + { Spar.Options.saml = multiIngressSamlConfig, + Spar.Options.idpCertFingerprintAllowlist = Just allow + } + (_logs, _notifs, res) <- + interpretWithLoggingMockOptsE optsWithMultiIngress Nothing $ + idpCreate multiIngressSamlConfig tid zUser miHost1 idpInfo Nothing apiVersionV2 idpHandle + res `shouldSatisfy` isRight + + let testRejectEmptyAllowlist testName allowlist = + it testName $ do + idpInfo@(IdPMetadataValue _ m) <- generateArbitraryIdPInfo + let optsWithMultiIngress = + defaultTestOpts + { Spar.Options.saml = multiIngressSamlConfig, + Spar.Options.idpCertFingerprintAllowlist = allowlist + } + fingerprint = certToFingerprint . NonEmptyL.head $ m._edCertAuthnResponse + (logs, _notifs, res) <- + interpretWithLoggingMockOptsE optsWithMultiIngress Nothing $ + idpCreate multiIngressSamlConfig tid zUser miHost1 idpInfo Nothing apiVersionV2 idpHandle + res `shouldBe` Left (SAML.CustomError (SparIdPCertNotAllowed fingerprint)) + let logged = TL.decodeUtf8 $ LBS.concat (map snd logs) + logged `shouldSatisfy` (("Refusing IdP request: multi-ingress enabled and allowlist empty" :: TL.Text) `TL.isInfixOf`) + + testRejectEmptyAllowlist "rejects when allowlist is Nothing" Nothing + testRejectEmptyAllowlist + "rejects when allowlist is empty" + (Just (Spar.Options.CertFingerprintAllowlist Set.empty)) + describe "update" $ do it "accepts any cert when allowlist is Nothing" $ do idpInfoCrt <- generateArbitraryIdPInfo idpInfoUpd <- generateArbitraryIdPInfo - (_logs, res) <- runCreateUpdate (withAllow Nothing) idpInfoCrt idpInfoUpd + (_logs, res) <- runCreateUpdate (defaultTestOpts `withAllow` Nothing) idpInfoCrt idpInfoUpd res `shouldSatisfy` isRight it "accepts any cert when allowlist is empty" $ do idpInfoCrt <- generateArbitraryIdPInfo idpInfoUpd <- generateArbitraryIdPInfo - (_logs, res) <- runCreateUpdate (withAllow (Just mempty)) idpInfoCrt idpInfoUpd + (_logs, res) <- runCreateUpdate (defaultTestOpts `withAllow` (Just mempty)) idpInfoCrt idpInfoUpd res `shouldSatisfy` isRight it "accepts when all fingerprints are allowlisted" $ do idpInfoCrt <- generateArbitraryIdPInfo idpInfoUpd <- generateArbitraryIdPInfo let allow = allCertsAllowlist idpInfoCrt <> allCertsAllowlist idpInfoUpd - (_logs, res) <- runCreateUpdate (withAllow (Just allow)) idpInfoCrt idpInfoUpd + (_logs, res) <- runCreateUpdate (defaultTestOpts `withAllow` (Just allow)) idpInfoCrt idpInfoUpd res `shouldSatisfy` isRight it "accepts when all multi-cert fingerprints are allowlisted" $ do idpInfoCrt <- generateTwoCertIdPInfo idpInfoUpd <- generateTwoCertIdPInfo let allow = allCertsAllowlist idpInfoCrt <> allCertsAllowlist idpInfoUpd - (_logs, res) <- runCreateUpdate (withAllow (Just allow)) idpInfoCrt idpInfoUpd + (_logs, res) <- runCreateUpdate (defaultTestOpts `withAllow` (Just allow)) idpInfoCrt idpInfoUpd res `shouldSatisfy` isRight it "rejects when no fingerprint matches" $ do idpInfo@(IdPMetadataValue _ m) <- generateArbitraryIdPInfo let fingerprint = certToFingerprint . NonEmptyL.head $ m._edCertAuthnResponse (_logs1, _notifs1, createdE) <- - interpretWithLoggingMockOptsE (withAllow Nothing) Nothing $ + interpretWithLoggingMockOptsE (defaultTestOpts `withAllow` Nothing) Nothing $ idpCreate singleIngressSamlConfig tid zUser host idpInfo Nothing apiVersionV2 idpHandle case createdE of Left e -> expectationFailure ("unexpected create failure: " <> show e) Right idp -> do let allow = singletonAllowlist bogusFingerprint (_logs2, _notifs2, res) <- - interpretWithLoggingMockOptsE (withAllow (Just allow)) Nothing $ do + interpretWithLoggingMockOptsE (defaultTestOpts `withAllow` (Just allow)) Nothing $ do insertConfig idp idpUpdate singleIngressSamlConfig zUser host idpInfo (idp._idpId) Nothing res `shouldBe` Left (SAML.CustomError (SparIdPCertNotAllowed fingerprint)) @@ -674,17 +709,69 @@ spec = let partialAllow = singletonAllowlist (firstCertFingerprint idpInfo) secondCertFingerprint = certToFingerprint . head . NonEmptyL.tail $ m._edCertAuthnResponse (_logs1, _notifs1, createdE) <- - interpretWithLoggingMockOptsE (withAllow Nothing) Nothing $ + interpretWithLoggingMockOptsE (defaultTestOpts `withAllow` Nothing) Nothing $ idpCreate singleIngressSamlConfig tid zUser host idpInfo Nothing apiVersionV2 idpHandle case createdE of Left e -> expectationFailure ("unexpected create failure: " <> show e) Right idp -> do (_logs2, _notifs2, res) <- - interpretWithLoggingMockOptsE (withAllow (Just partialAllow)) Nothing $ do + interpretWithLoggingMockOptsE (defaultTestOpts `withAllow` (Just partialAllow)) Nothing $ do insertConfig idp idpUpdate singleIngressSamlConfig zUser host idpInfo (idp._idpId) Nothing res `shouldBe` Left (SAML.CustomError (SparIdPCertNotAllowed secondCertFingerprint)) + describe "multi-ingress" $ do + it "accepts when all fingerprints are allowlisted" $ do + idpInfoCrt <- generateArbitraryIdPInfo + idpInfoUpd <- generateArbitraryIdPInfo + let allow = allCertsAllowlist idpInfoCrt <> allCertsAllowlist idpInfoUpd + optsWithMultiIngress = + defaultTestOpts + { Spar.Options.saml = multiIngressSamlConfig, + Spar.Options.idpCertFingerprintAllowlist = Just allow + } + (_logs, _notifs, res) <- + interpretWithLoggingMockOptsE optsWithMultiIngress Nothing $ do + idp <- idpCreate multiIngressSamlConfig tid zUser miHost1 idpInfoCrt Nothing apiVersionV2 idpHandle + idpUpdate multiIngressSamlConfig zUser miHost1 idpInfoUpd (idp._idpId) Nothing + res `shouldSatisfy` isRight + + let testRejectEmptyAllowlistOnUpdate testName updateAllowlist = + it testName $ do + idpInfoCrt <- generateArbitraryIdPInfo + idpInfoUpd@(IdPMetadataValue _ m) <- generateArbitraryIdPInfo + let allowCrt = allCertsAllowlist idpInfoCrt + fingerprint = certToFingerprint . NonEmptyL.head $ m._edCertAuthnResponse + optsForCreate = + defaultTestOpts + { Spar.Options.saml = multiIngressSamlConfig, + Spar.Options.idpCertFingerprintAllowlist = Just allowCrt + } + optsForUpdate = + defaultTestOpts + { Spar.Options.saml = multiIngressSamlConfig, + Spar.Options.idpCertFingerprintAllowlist = updateAllowlist + } + (_logsCrt, _notifsCrt, createE) <- + interpretWithLoggingMockOptsE optsForCreate Nothing $ + idpCreate multiIngressSamlConfig tid zUser miHost1 idpInfoCrt Nothing apiVersionV2 idpHandle + case createE of + Left e -> expectationFailure ("unexpected create failure: " <> show e) + Right idp -> do + (logsUpd, _notifsUpd, updateE) <- + interpretWithLoggingMockOptsE optsForUpdate Nothing $ do + insertConfig idp + idpUpdate multiIngressSamlConfig zUser miHost1 idpInfoUpd (idp._idpId) Nothing + updateE `shouldBe` Left (SAML.CustomError (SparIdPCertNotAllowed fingerprint)) + let logged = TL.decodeUtf8 $ LBS.concat (map snd logsUpd) + logged `shouldSatisfy` (("Refusing IdP request: multi-ingress enabled and allowlist empty" :: TL.Text) `TL.isInfixOf`) + + testRejectEmptyAllowlistOnUpdate "rejects when allowlist is Nothing on update" Nothing + + testRejectEmptyAllowlistOnUpdate + "rejects when allowlist is empty on update" + (Just (Spar.Options.CertFingerprintAllowlist Set.empty)) + describe "authresp" $ do let makeIdp :: IdPMetadataInfo -> IO IdP makeIdp (IdPMetadataValue _ metadata) = do @@ -720,7 +807,7 @@ spec = raw <- generate (arbitrary :: Gen (MultipartData Mem)) let dummyBody = either error id (fromMultipart raw :: Either String SAML.AuthnResponseBody) user <- generate arbitrary - let opts = withAllow allow + let opts = defaultTestOpts `withAllow` allow uref = SAML.UserRef m._edIssuer (SAML.unspecifiedNameID "test-user") user' = user {userTeam = Just authrspTestTeamId} verdict = SAML.AccessGranted uref @@ -800,6 +887,53 @@ spec = logged `shouldNotSatisfy` ("cert fingerprint not in allowlist" `TL.isInfixOf`) other -> expectationFailure $ "expected CustomServant (VerifyHandlerGranted), got: " <> show other + describe "multi-ingress" $ do + let makeAuthRespRequestMultiIngress teamIdParam idpInfo@(IdPMetadataValue _ m) allow = do + idp <- makeIdp idpInfo + ass <- makeControlledAssertion + raw <- generate (arbitrary :: Gen (MultipartData Mem)) + let dummyBody = either error id (fromMultipart raw :: Either String SAML.AuthnResponseBody) + user <- generate arbitrary + let opts = + (defaultTestOpts `withAllow` allow) + { Spar.Options.saml = multiIngressSamlConfig + } + uref = SAML.UserRef m._edIssuer (SAML.unspecifiedNameID "test-user") + user' = user {userTeam = Just authrspTestTeamId} + verdict = SAML.AccessGranted uref + interpretAuthrespE opts (Just user') (ass NonEmptyL.:| [], idp, verdict) $ do + SAMLUserStore.insert uref (userId user') + authresp teamIdParam dummyBody (Just miHost1AsText) + + it ("accepts when all fingerprints are allowlisted - teamId param " <> show requestParamTeamId) $ do + idpInfo <- generateArbitraryIdPInfo + let allow = allCertsAllowlist idpInfo + (logs, res) <- makeAuthRespRequestMultiIngress requestParamTeamId idpInfo $ Just allow + case res of + Left (SAML.CustomServant servantErr) -> do + errHTTPCode servantErr `shouldBe` 200 + errReasonPhrase servantErr `shouldBe` "success" + let logged = TL.decodeUtf8 $ LBS.concat (map snd logs) + logged `shouldNotSatisfy` ("cert fingerprint not in allowlist" `TL.isInfixOf`) + other -> expectationFailure $ "expected success, got: " <> show other + + let testRejectEmptyAllowlistAuthresp testName allowlist = + it (testName <> " - teamId param " <> show requestParamTeamId) $ do + idpInfo <- generateArbitraryIdPInfo + (logs, res) <- makeAuthRespRequestMultiIngress requestParamTeamId idpInfo allowlist + case res of + Left (SAML.CustomServant servantErr) -> do + errHTTPCode servantErr `shouldBe` 403 + errReasonPhrase servantErr `shouldBe` "idp-cert-not-allowed" + let logged = TL.decodeUtf8 $ LBS.concat (map snd logs) + logged `shouldSatisfy` (("Refusing IdP request: multi-ingress enabled and allowlist empty" :: TL.Text) `TL.isInfixOf`) + other -> expectationFailure $ "expected idp-cert-not-allowed error, got: " <> show other + + testRejectEmptyAllowlistAuthresp "rejects when allowlist is Nothing" Nothing + testRejectEmptyAllowlistAuthresp + "rejects when allowlist is empty" + (Just (Spar.Options.CertFingerprintAllowlist Set.empty)) + type LogLine = (Level, LByteString) interpretWithLoggingMock :: From 20f3c9e04d1a2edecec333d3484bd0af7f473fc9 Mon Sep 17 00:00:00 2001 From: Sven Tennie Date: Wed, 8 Jul 2026 12:28:50 +0200 Subject: [PATCH 13/22] Add changelog --- changelog.d/2-features/multi-ingress-mandatory-allowlist | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 changelog.d/2-features/multi-ingress-mandatory-allowlist diff --git a/changelog.d/2-features/multi-ingress-mandatory-allowlist b/changelog.d/2-features/multi-ingress-mandatory-allowlist new file mode 100644 index 00000000000..7920c755f91 --- /dev/null +++ b/changelog.d/2-features/multi-ingress-mandatory-allowlist @@ -0,0 +1,3 @@ +To prevent security-relevant configuration mistakes, make the configuration of +allowed IdP's fingerprints (`idpCertFingerprintAllowlist`) mandatory for +multi-ingress SSO. From f21e522284056331625531cd6c8edd637fe6ced8 Mon Sep 17 00:00:00 2001 From: Sven Tennie Date: Wed, 8 Jul 2026 12:36:50 +0200 Subject: [PATCH 14/22] Update docs --- docs/src/developer/reference/config-options.md | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/docs/src/developer/reference/config-options.md b/docs/src/developer/reference/config-options.md index ab4bbf32689..27f8ce6db60 100644 --- a/docs/src/developer/reference/config-options.md +++ b/docs/src/developer/reference/config-options.md @@ -1347,16 +1347,19 @@ configured in `nginz`'s Helm chart in the #### IdP certificate fingerprint allowlist -This optional feature restricts which X.509 certificates can be used in IdP -metadata. When configured, all certificates in IdP descriptors must have a -SHA-1 fingerprint present in the allowlist, or IdP creation/update and SAML +This feature restricts which X.509 certificates can be used in IdP metadata. +When configured, all certificates in IdP descriptors must have a SHA-1 +fingerprint present in the allowlist, or IdP creation/update and SAML AuthnResponse (`/sso/finalize-login`) requests will be rejected. This limits team admins in their choice of IdPs. E.g. a malicious team admin couldn't provision bad IdPs, as possible IdP certificates are restricted by the allowlist. -The feature is disabled by default in Helm (the attribute can be left out as well): +**For multi-ingress setups, this feature is mandatory** to prevent +security-relevant configuration mistakes. For regular (non-multi-ingress) +setups, it is optional and disabled by default in Helm (the attribute can be +left out as well): ```yaml config: @@ -1610,6 +1613,9 @@ error. Though, IdPs can be reconfigured as long as this invariant holds. Putting it differently: We require an unambiguous mapping `(team, domain) -> IdP`. +For multi-ingress setups, the [`idpCertFingerprintAllowlist`](#idp-certificate-fingerprint-allowlist) +must be configured to restrict which X.509 certificates can be used in IdP metadata. + ### Webapp The webapp runs its own web server (a NodeJS server) to serve static files and the webapp config (based on environment variables). From 287399e7da95d20cd1605065cf214b3cb6160f90 Mon Sep 17 00:00:00 2001 From: Sven Tennie Date: Wed, 8 Jul 2026 12:47:03 +0200 Subject: [PATCH 15/22] Fix typo --- services/spar/src/Spar/API.hs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/spar/src/Spar/API.hs b/services/spar/src/Spar/API.hs index 357c46979f9..17bb2fa8b82 100644 --- a/services/spar/src/Spar/API.hs +++ b/services/spar/src/Spar/API.hs @@ -814,7 +814,7 @@ idpCreateV7 samlConfig tid zUser idpmeta mReplaces mApiversion mHandle = do -- | Reject IdPs whose cert SHA-1 is not in the configured allowlist. -- -- Empty/absent allowlist is a no-op in the regular case, it short-circuits to --- error for mulit-ingress setups. I.e. the allowlist is required for +-- error for multi-ingress setups. I.e. the allowlist is required for -- multi-ingress setups. assertCertsAllowlisted :: ( Member (Input Opts) r, From 99f4a50660dab09439086835f041bfb9e083b6c7 Mon Sep 17 00:00:00 2001 From: Sven Tennie Date: Wed, 8 Jul 2026 12:48:22 +0200 Subject: [PATCH 16/22] Remove wont-do TODO --- integration/test/Test/Spar/MultiIngressSSO.hs | 1 - 1 file changed, 1 deletion(-) diff --git a/integration/test/Test/Spar/MultiIngressSSO.hs b/integration/test/Test/Spar/MultiIngressSSO.hs index af629d4975b..0d7a0180216 100644 --- a/integration/test/Test/Spar/MultiIngressSSO.hs +++ b/integration/test/Test/Spar/MultiIngressSSO.hs @@ -74,7 +74,6 @@ testMultiIngressSSOGeneralIdp = do ] ] ) - -- TODO: Use withAllowlist >=> setField "idpCertFingerprintAllowlist" [fingerprintHex signedCert] } $ \domain -> do From 1b362f1a5420acf24cf5e4a40405075929950b4e Mon Sep 17 00:00:00 2001 From: Sven Tennie Date: Wed, 8 Jul 2026 13:02:06 +0200 Subject: [PATCH 17/22] Better changelog --- .../1-api-changes/multi-ingress-mandatory-allowlist | 7 +++++++ changelog.d/2-features/multi-ingress-mandatory-allowlist | 3 --- 2 files changed, 7 insertions(+), 3 deletions(-) create mode 100644 changelog.d/1-api-changes/multi-ingress-mandatory-allowlist delete mode 100644 changelog.d/2-features/multi-ingress-mandatory-allowlist diff --git a/changelog.d/1-api-changes/multi-ingress-mandatory-allowlist b/changelog.d/1-api-changes/multi-ingress-mandatory-allowlist new file mode 100644 index 00000000000..708b4fe75e8 --- /dev/null +++ b/changelog.d/1-api-changes/multi-ingress-mandatory-allowlist @@ -0,0 +1,7 @@ +To prevent security-relevant configuration mistakes, make the configuration of +allowed IdP's fingerprints (`idpCertFingerprintAllowlist`) mandatory for +multi-ingress SSO. **This will break existing multi-ingress SSO flows until +`idpCertFingerprintAllowlist` is configured!** This breakage is unfortunately +necessary, because we're getting more lenient regarding the IdPs a user can use +to login ("auto IdP migration"). Regular (non-multi-ingress) usecases are +unaffected. diff --git a/changelog.d/2-features/multi-ingress-mandatory-allowlist b/changelog.d/2-features/multi-ingress-mandatory-allowlist deleted file mode 100644 index 7920c755f91..00000000000 --- a/changelog.d/2-features/multi-ingress-mandatory-allowlist +++ /dev/null @@ -1,3 +0,0 @@ -To prevent security-relevant configuration mistakes, make the configuration of -allowed IdP's fingerprints (`idpCertFingerprintAllowlist`) mandatory for -multi-ingress SSO. From 89118e813429e34225323e474155c0974a914557 Mon Sep 17 00:00:00 2001 From: Sven Tennie Date: Wed, 8 Jul 2026 15:08:18 +0200 Subject: [PATCH 18/22] Fix copy & paste mistake --- integration/test/Test/Spar/MultiIngressIdp.hs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integration/test/Test/Spar/MultiIngressIdp.hs b/integration/test/Test/Spar/MultiIngressIdp.hs index 883ad3fdb91..88c1a8853c8 100644 --- a/integration/test/Test/Spar/MultiIngressIdp.hs +++ b/integration/test/Test/Spar/MultiIngressIdp.hs @@ -195,7 +195,7 @@ testMultiIngressAtMostOneIdPPerDomain = do -- Create an IdP for one domain and update it to another that already has one -> failure SAML.SampleIdP idpmeta3 _ _ _ <- SAML.makeSampleIdPMetadataWithCert credsWithCert3 idpId3 <- - createIdpWithZHostV2 owner (Just bertZHost) idpmeta2 `bindResponse` \resp -> do + createIdpWithZHostV2 owner (Just bertZHost) idpmeta3 `bindResponse` \resp -> do resp.status `shouldMatchInt` 201 resp.json %. "id" >>= asString From 93b44bbfcc685bfaca725f975861c5f73f5c554d Mon Sep 17 00:00:00 2001 From: Sven Tennie Date: Wed, 8 Jul 2026 15:14:11 +0200 Subject: [PATCH 19/22] Fix rebase --- services/spar/test/Test/Spar/Saml/IdPSpec.hs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/spar/test/Test/Spar/Saml/IdPSpec.hs b/services/spar/test/Test/Spar/Saml/IdPSpec.hs index 8eedc433883..96fd6a5c625 100644 --- a/services/spar/test/Test/Spar/Saml/IdPSpec.hs +++ b/services/spar/test/Test/Spar/Saml/IdPSpec.hs @@ -903,7 +903,7 @@ spec = verdict = SAML.AccessGranted uref interpretAuthrespE opts (Just user') (ass NonEmptyL.:| [], idp, verdict) $ do SAMLUserStore.insert uref (userId user') - authresp teamIdParam dummyBody (Just miHost1AsText) + authresp teamIdParam dummyBody (Just miDomain1) it ("accepts when all fingerprints are allowlisted - teamId param " <> show requestParamTeamId) $ do idpInfo <- generateArbitraryIdPInfo From 286bec3eaf529e03049afbfb04cc9dbde6ac7aaa Mon Sep 17 00:00:00 2001 From: Sven Tennie Date: Wed, 8 Jul 2026 15:26:03 +0200 Subject: [PATCH 20/22] Grammar & spelling Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- changelog.d/1-api-changes/multi-ingress-mandatory-allowlist | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/changelog.d/1-api-changes/multi-ingress-mandatory-allowlist b/changelog.d/1-api-changes/multi-ingress-mandatory-allowlist index 708b4fe75e8..444a67a31e5 100644 --- a/changelog.d/1-api-changes/multi-ingress-mandatory-allowlist +++ b/changelog.d/1-api-changes/multi-ingress-mandatory-allowlist @@ -1,7 +1,7 @@ -To prevent security-relevant configuration mistakes, make the configuration of -allowed IdP's fingerprints (`idpCertFingerprintAllowlist`) mandatory for +To prevent security-relevant configuration mistakes, make configuration of +allowed IdP certificate fingerprints (`idpCertFingerprintAllowlist`) mandatory for multi-ingress SSO. **This will break existing multi-ingress SSO flows until `idpCertFingerprintAllowlist` is configured!** This breakage is unfortunately necessary, because we're getting more lenient regarding the IdPs a user can use -to login ("auto IdP migration"). Regular (non-multi-ingress) usecases are +to log in ("auto IdP migration"). Regular (non-multi-ingress) use cases are unaffected. From 1a1e43be9ba17307a2f54582025851f35a749582 Mon Sep 17 00:00:00 2001 From: Sven Tennie Date: Wed, 8 Jul 2026 15:57:00 +0200 Subject: [PATCH 21/22] Improve error case --- services/spar/src/Spar/API.hs | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/services/spar/src/Spar/API.hs b/services/spar/src/Spar/API.hs index 17bb2fa8b82..9c04f8e7b81 100644 --- a/services/spar/src/Spar/API.hs +++ b/services/spar/src/Spar/API.hs @@ -64,7 +64,7 @@ import Data.ByteString.Conversion import Data.Domain (Domain, domainText) import Data.HavePendingInvitations import Data.Id -import Data.List.NonEmpty (NonEmpty) +import qualified Data.List.NonEmpty as NE import qualified Data.Map as Map import Data.Proxy import Data.Range @@ -450,7 +450,7 @@ authresp mbtid arbody mbHost = do logErrors $ SAML2.authResp mbtid iss rsp go arbody where - go :: NonEmpty SAML.Assertion -> IdP -> SAML.AccessVerdict -> Sem r Void + go :: NE.NonEmpty SAML.Assertion -> IdP -> SAML.AccessVerdict -> Sem r Void go assertions idp verdict = do assertCertsAllowlisted (idp ^. SAML.idpMetadata) case verdict of @@ -831,11 +831,9 @@ assertCertsAllowlisted idpmeta = do TE.decodeUtf8 $ URI.serializeURIRef' (idpmeta ^. SAML.edIssuer . SAML.fromIssuer) when (isEmptyAllowList mAllow && SAML.isMultiIngressConfig samlConfig) $ do - forM_ certs $ \c -> do - let fingerprint = certSha1Fingerprint c - fingerprintHex = renderFingerprintHex fingerprint - logMultiIngressEmptyAllowlist fingerprintHex issuerTxt - throwSparSem (SparIdPCertNotAllowed (T.fromStrict fingerprintHex)) + let fingerprintHex = renderFingerprintHex . certSha1Fingerprint . NE.head $ certs + logMultiIngressEmptyAllowlist fingerprintHex issuerTxt + throwSparSem (SparIdPCertNotAllowed (T.fromStrict fingerprintHex)) case mAllow of Nothing -> pure () Just (CertFingerprintAllowlist allowed) @@ -1061,7 +1059,7 @@ idpUpdateXML samlConfig mbZUsr mDomain raw idpmeta idpid mHandle = withDebugLog Log.field fieldName ((intercalate ";; " . map certToString) certs) logCertField _ _ = id - compareNonEmpty :: (Eq a) => NonEmpty a -> NonEmpty a -> ([a], [a]) + compareNonEmpty :: (Eq a) => NE.NonEmpty a -> NE.NonEmpty a -> ([a], [a]) compareNonEmpty xs ys = let l = nub . toList $ xs r = nub . toList $ ys From 1fcf6fed794f5b18d1bee2016f77ef11f864e14f Mon Sep 17 00:00:00 2001 From: Sven Tennie Date: Wed, 8 Jul 2026 15:59:51 +0200 Subject: [PATCH 22/22] Briefer test name --- services/spar/test/Test/Spar/Saml/IdPSpec.hs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/services/spar/test/Test/Spar/Saml/IdPSpec.hs b/services/spar/test/Test/Spar/Saml/IdPSpec.hs index 96fd6a5c625..04f221d7f37 100644 --- a/services/spar/test/Test/Spar/Saml/IdPSpec.hs +++ b/services/spar/test/Test/Spar/Saml/IdPSpec.hs @@ -766,10 +766,10 @@ spec = let logged = TL.decodeUtf8 $ LBS.concat (map snd logsUpd) logged `shouldSatisfy` (("Refusing IdP request: multi-ingress enabled and allowlist empty" :: TL.Text) `TL.isInfixOf`) - testRejectEmptyAllowlistOnUpdate "rejects when allowlist is Nothing on update" Nothing + testRejectEmptyAllowlistOnUpdate "rejects when allowlist is Nothing" Nothing testRejectEmptyAllowlistOnUpdate - "rejects when allowlist is empty on update" + "rejects when allowlist is empty" (Just (Spar.Options.CertFingerprintAllowlist Set.empty)) describe "authresp" $ do