diff --git a/cassandra-schema.cql b/cassandra-schema.cql index ec614aa685..b8b52ecf8b 100644 --- a/cassandra-schema.cql +++ b/cassandra-schema.cql @@ -2032,6 +2032,7 @@ CREATE TABLE spar_test.issuer_idp ( CREATE TABLE spar_test.idp ( idp uuid PRIMARY KEY, api_version int, + domain text, extra_public_keys list, handle text, issuer text, diff --git a/changelog.d/2-features/multi-ingress-idp-domains b/changelog.d/2-features/multi-ingress-idp-domains new file mode 100644 index 0000000000..f8944f2006 --- /dev/null +++ b/changelog.d/2-features/multi-ingress-idp-domains @@ -0,0 +1,9 @@ +When a SAML IdP is created on a multi-ingress domain (implying that +multi-ingress domains are configured in Spar) the domain is added as `domain` +field to that IdP's `extraInfo` (`WireIdP` type in Haskell.) To avoid confusion +in later lookups, at most one IdP can be configured per multi-ingress domain. +If multi-ingress is not configured or it's not configured for the specific +domain, no `domain` field gets added to the IdP. This guards against creating +multiple IdPs and then assigning them to multi-ingress domains. Thus, users who +don't use multi-ingress don't observe any change. This feature only opens the +door to later provide an IdP for a multi-ingress domain. diff --git a/docs/src/developer/reference/config-options.md b/docs/src/developer/reference/config-options.md index 1710fe7d4e..5610f8d615 100644 --- a/docs/src/developer/reference/config-options.md +++ b/docs/src/developer/reference/config-options.md @@ -1299,6 +1299,40 @@ clear as there are multiple `ssoUri`s defined. So, the SCIM base URI needs to be set explicitly in `scimBaseUri`. In spar's YAML config file `scimBaseUri` is always required. +#### SAML IdPs + +The multi-ingress configuration also affects SAML IdPs: The multi-ingress +domain (as specified by the internal `Z-Host` header; according to the domain +the `/identity-providers` endpoints are accessed on) is stored in the IdP's +configuration data. It can be observed as field `domain` in responses of +`/identity-providers`. + +For example: +```json +{ + "extraInfo": { + "apiVersion": "WireIdPAPIV2", + "domain": "nginz-https.ernie.example.com", + "handle": "IdP 1", + "oldIssuers": [ + "https://issuer.net/_c4590f08-14da-446b-89d0-fcb46ac8ccf9" + ], + "replacedBy": null, + "team": "ce2c2ade-8b93-4db3-b1d3-44ce1d987ca6" + }, + "id": "ba6afb01-3edf-416c-8561-42e7ecc9b00a", + "metadata": { + "certAuthnResponse": [ + "MIIBOTCBxKADAgECAg4TIFmNatMeqaAE8BWQBTANBgkqhkiG9w0BAQsFADAAMB4XDTI1MDkxODE2MjY1NVoXDTQ1MDkxMzE2MjY1NVowADB6MA0GCSqGSIb3DQEBAQUAA2kAMGYCYQC/KgI1kw9+dXc/XUQ8Q6no9GsT9gX1g3sekVEI7UuxrcHd+Tapzi1T99TdnBDedXCAxGTW6Rwhu3F20j0iAi0neWzi5xv+1KWxK0djzJ0Kxk5AcdDx/Tz+t1Uzd4VXkhECAREwDQYJKoZIhvcNAQELBQADYQAsFrbuDmGZphl9d9VdHyh8a9lIFh3oO5et+tPqFTTRPbbfEewqvtwFWvP9Gf1qgjk0qwKX3GDsFejQf4h94qU1Zf0IE8J/WyIiwEWRvZgAQ9UmqKljmbHKssyNwsl6tTY=" + ], + "issuer": "https://issuer.net/_2ab62f21-44c0-4c60-a115-d05b5129141d", + "requestURI": "https://requri.net/22169147-7d84-4991-9652-d7434986b7d8" + } +} +``` + +There can be at most one IdP per multi-ingress domain. Creating more returns an +error. Though, IdPs can be reconfigured as long as this invariant holds. ### Webapp diff --git a/integration/integration.cabal b/integration/integration.cabal index 6e29ba40ab..a33a8a9859 100644 --- a/integration/integration.cabal +++ b/integration/integration.cabal @@ -194,6 +194,7 @@ library Test.Search Test.Services Test.Spar + Test.Spar.MultiIngressIdp Test.Spar.MultiIngressSSO Test.Spar.STM Test.Swagger diff --git a/integration/test/API/Spar.hs b/integration/test/API/Spar.hs index f826a50803..0f74339bef 100644 --- a/integration/test/API/Spar.hs +++ b/integration/test/API/Spar.hs @@ -92,18 +92,30 @@ updateScimUser domain scimToken userId scimUser = do & addHeader "Accept" "application/scim+json" createIdp :: (HasCallStack, MakesValue user) => user -> SAML.IdPMetadata -> App Response -createIdp user metadata = do - req <- baseRequest user Spar Versioned "/identity-providers" - submit "POST" $ req - & addQueryParams [("api_version", "v2")] - & addXML (fromLT $ SAML.encode metadata) +createIdp = (flip createIdpWithZHost) Nothing + +createIdpWithZHost :: (HasCallStack, MakesValue user) => user -> Maybe String -> SAML.IdPMetadata -> App Response +createIdpWithZHost user mbZHost metadata = do + bReq <- baseRequest user Spar Versioned "/identity-providers" + let req = + bReq + & addQueryParams [("api_version", "v2")] + & addXML (fromLT $ SAML.encode metadata) + & addHeader "Content-Type" "application/xml" + submit "POST" (req & maybe id zHost mbZHost) -- | https://staging-nginz-https.zinfra.io/v7/api/swagger-ui/#/default/idp-update updateIdp :: (HasCallStack, MakesValue user) => user -> String -> SAML.IdPMetadata -> App Response -updateIdp user idpId metadata = do - req <- baseRequest user Spar Versioned $ joinHttpPath ["identity-providers", idpId] - submit "PUT" $ req - & addXML (fromLT $ SAML.encode metadata) +updateIdp = (flip updateIdpWithZHost) Nothing + +updateIdpWithZHost :: (HasCallStack, MakesValue user) => user -> Maybe String -> String -> SAML.IdPMetadata -> App Response +updateIdpWithZHost user mbZHost idpId metadata = do + bReq <- baseRequest user Spar Versioned $ joinHttpPath ["identity-providers", idpId] + let req = + bReq + & addXML (fromLT $ SAML.encode metadata) + & addHeader "Content-Type" "application/xml" + submit "PUT" (req & maybe id zHost mbZHost) -- | https://staging-nginz-https.zinfra.io/v7/api/swagger-ui/#/default/idp-get-all getIdps :: (HasCallStack, MakesValue user) => user -> App Response @@ -111,6 +123,16 @@ getIdps user = do req <- baseRequest user Spar Versioned "/identity-providers" submit "GET" req +getIdp :: (HasCallStack, MakesValue user) => user -> String -> App Response +getIdp user idpId = do + req <- baseRequest user Spar Versioned $ joinHttpPath ["identity-providers", idpId] + submit "GET" req + +deleteIdp :: (HasCallStack, MakesValue user) => user -> String -> App Response +deleteIdp user idpId = do + req <- baseRequest user Spar Versioned $ joinHttpPath ["identity-providers", idpId] + submit "DELETE" req + -- | https://staging-nginz-https.zinfra.io/v7/api/swagger-ui/#/default/sso-team-metadata getSPMetadata :: (HasCallStack, MakesValue domain) => domain -> String -> App Response getSPMetadata = (flip getSPMetadataWithZHost) Nothing diff --git a/integration/test/Test/Spar/MultiIngressIdp.hs b/integration/test/Test/Spar/MultiIngressIdp.hs new file mode 100644 index 0000000000..8294782cb4 --- /dev/null +++ b/integration/test/Test/Spar/MultiIngressIdp.hs @@ -0,0 +1,302 @@ +module Test.Spar.MultiIngressIdp where + +import API.GalleyInternal +import API.Spar +import Control.Lens ((.~), (^.)) +import qualified SAML2.WebSSO.Test.Util as SAML +import qualified SAML2.WebSSO.Types as SAML +import SetupHelpers +import Testlib.Prelude + +ernieZHost :: String +ernieZHost = "nginz-https.ernie.example.com" + +bertZHost :: String +bertZHost = "nginz-https.bert.example.com" + +kermitZHost :: String +kermitZHost = "nginz-https.kermit.example.com" + +-- | Create a `MultiIngressDomainConfig` JSON object with the given @zhost@ +makeSpDomainConfig :: String -> Value +makeSpDomainConfig zhost = + object + [ "spAppUri" .= ("https://webapp." ++ zhost), + "spSsoUri" .= ("https://nginz-https." ++ zhost ++ "/sso"), + "contacts" .= [object ["type" .= ("ContactTechnical" :: String)]] + ] + +testMultiIngressIdpSimpleCase :: (HasCallStack) => App () +testMultiIngressIdpSimpleCase = do + withModifiedBackend + def + { sparCfg = + removeField "saml.spSsoUri" + >=> removeField "saml.spAppUri" + >=> removeField "saml.contacts" + >=> setField + "saml.spDomainConfigs" + ( object + [ ernieZHost .= makeSpDomainConfig ernieZHost, + bertZHost .= makeSpDomainConfig bertZHost + ] + ) + } + $ \domain -> do + (owner, tid, _) <- createTeam domain 1 + void $ setTeamFeatureStatus owner tid "sso" "enabled" + + -- Create IdP for one domain + SAML.SampleIdP idpmeta _ _ _ <- SAML.makeSampleIdPMetadata + idpId <- + createIdpWithZHost owner (Just ernieZHost) idpmeta `bindResponse` \resp -> do + resp.status `shouldMatchInt` 201 + resp.jsonBody %. "extraInfo.domain" `shouldMatch` ernieZHost + resp.jsonBody %. "id" >>= asString + + getIdp owner idpId `bindResponse` \resp -> do + resp.status `shouldMatchInt` 200 + resp.jsonBody %. "extraInfo.domain" `shouldMatch` ernieZHost + + -- Update IdP for another domain + updateIdpWithZHost owner (Just bertZHost) idpId idpmeta `bindResponse` \resp -> do + resp.status `shouldMatchInt` 200 + resp.jsonBody %. "extraInfo.domain" `shouldMatch` bertZHost + + getIdp owner idpId `bindResponse` \resp -> do + resp.status `shouldMatchInt` 200 + resp.jsonBody %. "extraInfo.domain" `shouldMatch` bertZHost + +-- We must guard against domains being filled up with multiple IdPs and then +-- being configured as multi-ingress domains. Then, we'd have multiple IdPs for +-- a multi-ingress domain and cannot decide which one to choose. The solution +-- to this is that unconfigured domains' IdPs store no domain. I.e. the +-- assignment of domains to IdPs begins when the domain is configured as +-- multi-ingress domain. +testUnconfiguredDomain :: (HasCallStack) => App () +testUnconfiguredDomain = forM_ [Nothing, Just kermitZHost] $ \unconfiguredZHost -> do + withModifiedBackend + def + { sparCfg = + removeField "saml.spSsoUri" + >=> removeField "saml.spAppUri" + >=> removeField "saml.contacts" + >=> setField + "saml.spDomainConfigs" + (object [ernieZHost .= makeSpDomainConfig ernieZHost]) + } + $ \domain -> do + (owner, tid, _) <- createTeam domain 1 + void $ setTeamFeatureStatus owner tid "sso" "enabled" + + SAML.SampleIdP idpmeta1 _ _ _ <- SAML.makeSampleIdPMetadata + idpId1 <- + createIdpWithZHost owner (Just ernieZHost) idpmeta1 `bindResponse` \resp -> do + resp.status `shouldMatchInt` 201 + resp.jsonBody %. "extraInfo.domain" `shouldMatch` ernieZHost + resp.jsonBody %. "id" >>= asString + + -- From configured domain to unconfigured -> no multi-ingress domain + updateIdpWithZHost owner (unconfiguredZHost) idpId1 idpmeta1 `bindResponse` \resp -> do + resp.status `shouldMatchInt` 200 + resp.jsonBody %. "extraInfo.domain" `shouldMatch` Null + + getIdp owner idpId1 `bindResponse` \resp -> do + resp.status `shouldMatchInt` 200 + resp.jsonBody %. "extraInfo.domain" `shouldMatch` Null + + -- From unconfigured back to configured -> add multi-ingress domain + updateIdpWithZHost owner (Just ernieZHost) idpId1 idpmeta1 `bindResponse` \resp -> do + resp.status `shouldMatchInt` 200 + resp.jsonBody %. "extraInfo.domain" `shouldMatch` ernieZHost + + getIdp owner idpId1 `bindResponse` \resp -> do + resp.status `shouldMatchInt` 200 + resp.jsonBody %. "extraInfo.domain" `shouldMatch` ernieZHost + + -- Create unconfigured -> no multi-ingress domain + SAML.SampleIdP idpmeta2 _ _ _ <- SAML.makeSampleIdPMetadata + idpId2 <- + createIdpWithZHost owner (unconfiguredZHost) idpmeta2 `bindResponse` \resp -> do + resp.status `shouldMatchInt` 201 + resp.jsonBody %. "extraInfo.domain" `shouldMatch` Null + resp.jsonBody %. "id" >>= asString + + getIdp owner idpId2 `bindResponse` \resp -> do + resp.status `shouldMatchInt` 200 + resp.jsonBody %. "extraInfo.domain" `shouldMatch` Null + + -- Create a second unconfigured -> no multi-ingress domain + SAML.SampleIdP idpmeta3 _ _ _ <- SAML.makeSampleIdPMetadata + idpId3 <- + createIdpWithZHost owner (unconfiguredZHost) idpmeta3 `bindResponse` \resp -> do + resp.status `shouldMatchInt` 201 + resp.jsonBody %. "extraInfo.domain" `shouldMatch` Null + resp.jsonBody %. "id" >>= asString + + getIdp owner idpId3 `bindResponse` \resp -> do + resp.status `shouldMatchInt` 200 + resp.jsonBody %. "extraInfo.domain" `shouldMatch` Null + +testMultiIngressAtMostOneIdPPerDomain :: (HasCallStack) => App () +testMultiIngressAtMostOneIdPPerDomain = do + withModifiedBackend + def + { sparCfg = + removeField "saml.spSsoUri" + >=> removeField "saml.spAppUri" + >=> removeField "saml.contacts" + >=> setField + "saml.spDomainConfigs" + ( object + [ ernieZHost .= makeSpDomainConfig ernieZHost, + bertZHost .= makeSpDomainConfig bertZHost + ] + ) + } + $ \domain -> do + (owner, tid, _) <- createTeam domain 1 + void $ setTeamFeatureStatus owner tid "sso" "enabled" + + SAML.SampleIdP idpmeta1 _ _ _ <- SAML.makeSampleIdPMetadata + idpId1 <- + createIdpWithZHost owner (Just ernieZHost) idpmeta1 `bindResponse` \resp -> do + resp.status `shouldMatchInt` 201 + resp.jsonBody %. "id" >>= asString + + -- Creating a second IdP for the same domain -> failure + SAML.SampleIdP idpmeta2 _ _ _ <- SAML.makeSampleIdPMetadata + _idpId2 <- + createIdpWithZHost owner (Just ernieZHost) idpmeta2 `bindResponse` \resp -> do + resp.status `shouldMatchInt` 409 + resp.jsonBody %. "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 + idpId3 <- + createIdpWithZHost owner (Just bertZHost) idpmeta2 `bindResponse` \resp -> do + resp.status `shouldMatchInt` 201 + resp.jsonBody %. "id" >>= asString + + updateIdpWithZHost owner (Just ernieZHost) idpId3 idpmeta3 + `bindResponse` \resp -> do + resp.status `shouldMatchInt` 409 + resp.jsonBody %. "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 + idpId4 <- + createIdpWithZHost owner Nothing idpmeta4 `bindResponse` \resp -> do + resp.status `shouldMatchInt` 201 + resp.jsonBody %. "id" >>= asString + + updateIdpWithZHost owner (Just ernieZHost) idpId4 idpmeta4 + `bindResponse` \resp -> do + resp.status `shouldMatchInt` 409 + resp.jsonBody %. "label" `shouldMatch` "idp-duplicate-domain-for-team" + + -- Updating an IdP itself should still work + updateIdpWithZHost + owner + (Just ernieZHost) + idpId1 + -- The edIssuer needs to stay unchanged. Otherwise, deletion will fail + -- with a 404 (see bug https://wearezeta.atlassian.net/browse/WPB-20407) + (idpmeta2 & SAML.edIssuer .~ (idpmeta1 ^. SAML.edIssuer)) + `bindResponse` \resp -> do + resp.status `shouldMatchInt` 200 + resp.jsonBody %. "extraInfo.domain" `shouldMatch` ernieZHost + + -- After deletion of the IdP of a domain, a new one can be created + deleteIdp owner idpId1 `bindResponse` \resp -> do + resp.status `shouldMatchInt` 204 + + SAML.SampleIdP idpmeta5 _ _ _ <- SAML.makeSampleIdPMetadata + idpId5 <- + createIdpWithZHost owner (Just ernieZHost) idpmeta5 `bindResponse` \resp -> do + resp.status `shouldMatchInt` 201 + resp.jsonBody %. "extraInfo.domain" `shouldMatch` ernieZHost + resp.jsonBody %. "id" >>= asString + + -- After deletion of the IdP of a domain, one can be moved from another domain + SAML.SampleIdP idpmeta6 _ _ _ <- SAML.makeSampleIdPMetadata + createIdpWithZHost owner (Just bertZHost) idpmeta6 `bindResponse` \resp -> do + resp.status `shouldMatchInt` 409 + resp.jsonBody %. "label" `shouldMatch` "idp-duplicate-domain-for-team" + + deleteIdp owner idpId3 `bindResponse` \resp -> do + resp.status `shouldMatchInt` 204 + + idpId6 <- + createIdpWithZHost owner (Just bertZHost) idpmeta6 `bindResponse` \resp -> do + resp.status `shouldMatchInt` 201 + resp.jsonBody %. "extraInfo.domain" `shouldMatch` bertZHost + resp.jsonBody %. "id" >>= asString + + updateIdpWithZHost owner (Just ernieZHost) idpId6 idpmeta6 `bindResponse` \resp -> do + resp.status `shouldMatchInt` 409 + resp.jsonBody %. "label" `shouldMatch` "idp-duplicate-domain-for-team" + + deleteIdp owner idpId5 `bindResponse` \resp -> do + resp.status `shouldMatchInt` 204 + + updateIdpWithZHost owner (Just ernieZHost) idpId6 idpmeta6 + `bindResponse` \resp -> do + resp.status `shouldMatchInt` 200 + resp.jsonBody %. "extraInfo.domain" `shouldMatch` ernieZHost + +-- We only record the domain for multi-ingress setups. +testNonMultiIngressSetupsCanHaveMoreIdPsPerDomain :: (HasCallStack) => App () +testNonMultiIngressSetupsCanHaveMoreIdPsPerDomain = do + (owner, tid, _) <- createTeam OwnDomain 1 + void $ setTeamFeatureStatus owner tid "sso" "enabled" + + -- With Z-Host header + SAML.SampleIdP idpmeta1 _ _ _ <- SAML.makeSampleIdPMetadata + idpId1 <- + createIdpWithZHost owner (Just ernieZHost) idpmeta1 `bindResponse` \resp -> do + resp.status `shouldMatchInt` 201 + resp.jsonBody %. "extraInfo.domain" `shouldMatch` Null + resp.jsonBody %. "id" >>= asString + + SAML.SampleIdP idpmeta2 _ _ _ <- SAML.makeSampleIdPMetadata + idpId2 <- + createIdpWithZHost owner (Just ernieZHost) idpmeta2 `bindResponse` \resp -> do + resp.status `shouldMatchInt` 201 + resp.jsonBody %. "extraInfo.domain" `shouldMatch` Null + resp.jsonBody %. "id" >>= asString + + SAML.SampleIdP idpmeta3 _ _ _ <- SAML.makeSampleIdPMetadata + updateIdpWithZHost owner (Just ernieZHost) idpId1 idpmeta3 `bindResponse` \resp -> do + resp.status `shouldMatchInt` 200 + resp.jsonBody %. "extraInfo.domain" `shouldMatch` Null + + SAML.SampleIdP idpmeta4 _ _ _ <- SAML.makeSampleIdPMetadata + updateIdpWithZHost owner (Just ernieZHost) idpId2 idpmeta4 `bindResponse` \resp -> do + resp.status `shouldMatchInt` 200 + resp.jsonBody %. "extraInfo.domain" `shouldMatch` Null + + -- Without Z-Host header + SAML.SampleIdP idpmeta5 _ _ _ <- SAML.makeSampleIdPMetadata + idpId5 <- + createIdpWithZHost owner Nothing idpmeta5 `bindResponse` \resp -> do + resp.status `shouldMatchInt` 201 + resp.jsonBody %. "extraInfo.domain" `shouldMatch` Null + resp.jsonBody %. "id" >>= asString + + SAML.SampleIdP idpmeta6 _ _ _ <- SAML.makeSampleIdPMetadata + idpId6 <- + createIdpWithZHost owner Nothing idpmeta6 `bindResponse` \resp -> do + resp.status `shouldMatchInt` 201 + resp.jsonBody %. "extraInfo.domain" `shouldMatch` Null + resp.jsonBody %. "id" >>= asString + + SAML.SampleIdP idpmeta7 _ _ _ <- SAML.makeSampleIdPMetadata + updateIdpWithZHost owner Nothing idpId5 idpmeta7 `bindResponse` \resp -> do + resp.status `shouldMatchInt` 200 + resp.jsonBody %. "extraInfo.domain" `shouldMatch` Null + + SAML.SampleIdP idpmeta8 _ _ _ <- SAML.makeSampleIdPMetadata + updateIdpWithZHost owner Nothing idpId6 idpmeta8 `bindResponse` \resp -> do + resp.status `shouldMatchInt` 200 + resp.jsonBody %. "extraInfo.domain" `shouldMatch` Null diff --git a/libs/wire-api/src/Wire/API/Routes/Public/Spar.hs b/libs/wire-api/src/Wire/API/Routes/Public/Spar.hs index 474f55f109..d887c4c501 100644 --- a/libs/wire-api/src/Wire/API/Routes/Public/Spar.hs +++ b/libs/wire-api/src/Wire/API/Routes/Public/Spar.hs @@ -130,8 +130,8 @@ type APIIDP = :<|> Named "idp-get-raw" (ZOptUser :> IdpGetRaw) :<|> Named "idp-get-all" (ZOptUser :> IdpGetAll) :<|> Named "idp-create@v7" (Until 'V8 :> ZOptUser :> IdpCreate) -- (change is semantic, see handler) - :<|> Named "idp-create" (From 'V8 :> ZOptUser :> IdpCreate) - :<|> Named "idp-update" (ZOptUser :> IdpUpdate) + :<|> Named "idp-create" (From 'V8 :> ZOptUser :> ZHostOpt :> IdpCreate) + :<|> Named "idp-update" (ZOptUser :> ZHostOpt :> IdpUpdate) :<|> Named "idp-delete" (ZOptUser :> IdpDelete) type IdpGetRaw = Capture "id" SAML.IdPId :> "raw" :> Get '[RawXML] RawIdPMetadata diff --git a/libs/wire-api/src/Wire/API/User/IdentityProvider.hs b/libs/wire-api/src/Wire/API/User/IdentityProvider.hs index 8d3e2a2da8..14f2067e7a 100644 --- a/libs/wire-api/src/Wire/API/User/IdentityProvider.hs +++ b/libs/wire-api/src/Wire/API/User/IdentityProvider.hs @@ -43,6 +43,7 @@ import SAML2.WebSSO qualified as SAML import SAML2.WebSSO.Test.Arbitrary () import SAML2.WebSSO.Types.TH (deriveJSONOptions) import Servant.API as Servant hiding (MkLink, URI (..)) +import Wire.API.Routes.Public (ZHostValue) import Wire.API.User.Orphans (samlSchemaOptions) import Wire.API.Util.Aeson (defaultOptsDropChar) import Wire.Arbitrary (Arbitrary, GenericUniform (GenericUniform)) @@ -65,7 +66,8 @@ data WireIdP = WireIdP -- with the @"replaces"@ query parameter, and it is used to decide whether users not -- existing on this IdP can be auto-provisioned (if 'isJust', they can't). _replacedBy :: Maybe SAML.IdPId, - _handle :: IdPHandle + _handle :: IdPHandle, + _domain :: Maybe ZHostValue } deriving (Eq, Show, Generic) deriving (Arbitrary) via (GenericUniform WireIdP) diff --git a/libs/wire-subsystems/src/Wire/UserSubsystem.hs b/libs/wire-subsystems/src/Wire/UserSubsystem.hs index 05973184ee..accc4d1732 100644 --- a/libs/wire-subsystems/src/Wire/UserSubsystem.hs +++ b/libs/wire-subsystems/src/Wire/UserSubsystem.hs @@ -31,7 +31,7 @@ import Wire.API.Team.Feature import Wire.API.Team.Member (IsPerm (..), TeamMember) import Wire.API.User import Wire.API.User.Activation -import Wire.API.User.IdentityProvider hiding (team) +import Wire.API.User.IdentityProvider hiding (domain, team) import Wire.API.User.Search import Wire.ActivationCodeStore import Wire.Arbitrary diff --git a/services/spar/spar.cabal b/services/spar/spar.cabal index 75ff2848f6..8c55678f4d 100644 --- a/services/spar/spar.cabal +++ b/services/spar/spar.cabal @@ -43,6 +43,7 @@ library Spar.Schema.V19 Spar.Schema.V2 Spar.Schema.V20 + Spar.Schema.V21 Spar.Schema.V3 Spar.Schema.V4 Spar.Schema.V5 diff --git a/services/spar/src/Spar/API.hs b/services/spar/src/Spar/API.hs index 07c78eaee4..b900ef5c80 100644 --- a/services/spar/src/Spar/API.hs +++ b/services/spar/src/Spar/API.hs @@ -54,6 +54,7 @@ import Data.Domain import Data.HavePendingInvitations import Data.Id import Data.List.NonEmpty (NonEmpty) +import qualified Data.Map as Map import Data.Proxy import Data.Range import Data.Text.Encoding.Error @@ -106,6 +107,7 @@ import System.Logger (Msg) import qualified URI.ByteString as URI import Wire.API.Routes.Internal.Spar import Wire.API.Routes.Named +import Wire.API.Routes.Public (ZHostValue) import Wire.API.Routes.Public.Spar import Wire.API.Team.Member (HiddenPerm (CreateUpdateDeleteIdp, ReadIdp)) import Wire.API.User @@ -161,7 +163,7 @@ api :: ServerT SparAPI (Sem r) api opts = apiSSO opts - :<|> apiIDP + :<|> apiIDP opts :<|> apiScim :<|> apiINTERNAL @@ -205,14 +207,15 @@ apiIDP :: Member SAMLUserStore r, Member (Error SparError) r ) => + Opts -> ServerT APIIDP (Sem r) -apiIDP = +apiIDP opts = Named @"idp-get" idpGet -- get, json, captures idp id :<|> Named @"idp-get-raw" idpGetRaw -- get, raw xml, capture idp id :<|> Named @"idp-get-all" idpGetAll -- get, json - :<|> Named @"idp-create@v7" idpCreateV7 - :<|> Named @"idp-create" idpCreate -- post, created - :<|> Named @"idp-update" idpUpdate -- put, okay + :<|> Named @"idp-create@v7" (idpCreateV7 opts.saml) + :<|> Named @"idp-create" (idpCreate opts.saml) -- post, created + :<|> Named @"idp-update" (idpUpdate opts.saml) -- put, okay :<|> Named @"idp-delete" idpDelete -- delete, no content apiINTERNAL :: @@ -569,7 +572,7 @@ idpDelete mbzusr idpid (fromMaybe False -> purge) = withDebugLog "idpDelete" (co -- to be deleted in its old issuers list, but it's tricky to avoid race conditions, and -- there is little to be gained here: we only use old issuers to find users that have not -- been migrated yet, and if an old user points to a deleted idp, it just means that we - -- won't find any users to migrate. still, doesn't hurt mucht to look either. so we + -- won't find any users to migrate. still, doesn't hurt much to look either. so we -- leave old issuers dangling for now. updateReplacingIdP :: IdP -> Sem r () @@ -606,23 +609,47 @@ idpCreate :: Member IdPRawMetadataStore r, Member (Error SparError) r ) => + SAML.Config -> Maybe UserId -> + Maybe ZHostValue -> IdPMetadataInfo -> Maybe SAML.IdPId -> Maybe WireIdPAPIVersion -> Maybe (Range 1 32 Text) -> Sem r IdP -idpCreate zusr (IdPMetadataValue rawIdpMetadata idpmeta) mReplaces (fromMaybe defWireIdPAPIVersion -> apiversion) mHandle = withDebugLog "idpCreateXML" (Just . show . (^. SAML.idpId)) $ do +idpCreate samlConfig zusr uncheckedMbHost (IdPMetadataValue rawIdpMetadata idpmeta) mReplaces (fromMaybe defWireIdPAPIVersion -> apiversion) mHandle = withDebugLog "idpCreateXML" (Just . show . (^. SAML.idpId)) $ do + let mbHost = filterMultiIngressZHost (samlConfig._cfgDomainConfigs) uncheckedMbHost teamid <- Brig.getZUsrCheckPerm zusr CreateUpdateDeleteIdp GalleyAccess.assertSSOEnabled teamid + guardMultiIngressDuplicateDomain teamid mbHost idp <- maybe (IdPConfigStore.newHandle teamid) (pure . IdPHandle . fromRange) mHandle - >>= validateNewIdP apiversion idpmeta teamid mReplaces + >>= validateNewIdP apiversion idpmeta teamid mReplaces mbHost IdPRawMetadataStore.store (idp ^. SAML.idpId) rawIdpMetadata IdPConfigStore.insertConfig idp forM_ mReplaces $ \replaces -> IdPConfigStore.setReplacedBy (Replaced replaces) (Replacing (idp ^. SAML.idpId)) pure idp + where + -- Ensure that the domain is not in use by an existing IDP + guardMultiIngressDuplicateDomain :: + ( Member (Error SparError) r, + Member IdPConfigStore r + ) => + TeamId -> + Maybe ZHostValue -> + Sem r () + guardMultiIngressDuplicateDomain _teamId Nothing = pure () + guardMultiIngressDuplicateDomain teamId (Just zHost) = do + idps <- IdPConfigStore.getConfigsByTeam teamId + let domains = idps ^.. traverse . SAML.idpExtraInfo . domain . _Just + when (zHost `elem` domains) $ + throwSparSem SparIdPDomainInUse + +-- | Only return a ZHost when multi-ingress is configured and the host value is a configured domain +filterMultiIngressZHost :: Either SAML.MultiIngressDomainConfig (Map Domain SAML.MultiIngressDomainConfig) -> Maybe ZHostValue -> Maybe ZHostValue +filterMultiIngressZHost (Right domainMap) (Just zHost) | (Domain zHost) `Map.member` domainMap = Just zHost +filterMultiIngressZHost _ _ = Nothing idpCreateV7 :: ( Member Random r, @@ -634,16 +661,17 @@ idpCreateV7 :: Member IdPRawMetadataStore r, Member (Error SparError) r ) => + SAML.Config -> Maybe UserId -> IdPMetadataInfo -> Maybe SAML.IdPId -> Maybe WireIdPAPIVersion -> Maybe (Range 1 32 Text) -> Sem r IdP -idpCreateV7 zusr idpmeta mReplaces mApiversion mHandle = do +idpCreateV7 samlConfig zusr idpmeta mReplaces mApiversion mHandle = do teamid <- Brig.getZUsrCheckPerm zusr CreateUpdateDeleteIdp assertNoScimOrNoIdP teamid - idpCreate zusr idpmeta mReplaces mApiversion mHandle + idpCreate samlConfig zusr Nothing idpmeta mReplaces mApiversion mHandle where -- In teams with a scim access token, only one IdP is allowed. The reason is that scim user -- data contains no information about the idp issuer, only the user name, so no valid saml @@ -694,9 +722,10 @@ validateNewIdP :: SAML.IdPMetadata -> TeamId -> Maybe SAML.IdPId -> + Maybe ZHostValue -> IdPHandle -> m IdP -validateNewIdP apiversion _idpMetadata teamId mReplaces idHandle = withDebugLog "validateNewIdP" (Just . show . (^. SAML.idpId)) $ do +validateNewIdP apiversion _idpMetadata teamId mReplaces idpDomain idHandle = withDebugLog "validateNewIdP" (Just . show . (^. SAML.idpId)) $ do _idpId <- SAML.IdPId <$> Random.uuid oldIssuersList :: [SAML.Issuer] <- case mReplaces of Nothing -> pure [] @@ -704,7 +733,7 @@ validateNewIdP apiversion _idpMetadata teamId mReplaces idHandle = withDebugLog idp <- IdPConfigStore.getConfig replaces pure $ (idp ^. SAML.idpMetadata . SAML.edIssuer) : (idp ^. SAML.idpExtraInfo . oldIssuers) let requri = _idpMetadata ^. SAML.edRequestURI - _idpExtraInfo = WireIdP teamId (Just apiversion) oldIssuersList Nothing idHandle + _idpExtraInfo = WireIdP teamId (Just apiversion) oldIssuersList Nothing idHandle idpDomain enforceHttps requri mbIdp <- case apiversion of WireIdPAPIV1 -> IdPConfigStore.getIdPByIssuerV1Maybe (_idpMetadata ^. SAML.edIssuer) @@ -736,12 +765,16 @@ idpUpdate :: Member IdPRawMetadataStore r, Member (Error SparError) r ) => + SAML.Config -> Maybe UserId -> + Maybe ZHostValue -> IdPMetadataInfo -> SAML.IdPId -> Maybe (Range 1 32 Text) -> Sem r IdP -idpUpdate zusr (IdPMetadataValue raw xml) = idpUpdateXML zusr raw xml +idpUpdate samlConfig zusr uncheckedMbHost (IdPMetadataValue raw xml) = + let mbHost = filterMultiIngressZHost (samlConfig._cfgDomainConfigs) uncheckedMbHost + in idpUpdateXML zusr mbHost raw xml idpUpdateXML :: ( Member Random r, @@ -753,29 +786,51 @@ idpUpdateXML :: Member (Error SparError) r ) => Maybe UserId -> + Maybe ZHostValue -> Text -> SAML.IdPMetadata -> SAML.IdPId -> Maybe (Range 1 32 Text) -> Sem r IdP -idpUpdateXML zusr raw idpmeta idpid mHandle = withDebugLog "idpUpdateXML" (Just . show . (^. SAML.idpId)) $ do +idpUpdateXML zusr mDomain raw idpmeta idpid mHandle = withDebugLog "idpUpdateXML" (Just . show . (^. SAML.idpId)) $ do (teamid, idp) <- validateIdPUpdate zusr idpmeta idpid GalleyAccess.assertSSOEnabled teamid + guardMultiIngressDuplicateDomain teamid mDomain idpid IdPRawMetadataStore.store (idp ^. SAML.idpId) raw let idp' :: IdP = case mHandle of Just idpHandle -> idp & (SAML.idpExtraInfo . handle) .~ IdPHandle (fromRange idpHandle) Nothing -> idp + idp'' :: IdP = idp' & (SAML.idpExtraInfo . domain) .~ mDomain -- (if raw metadata is stored and then spar goes out, raw metadata won't match the -- structured idp config. since this will lead to a 5xx response, the client is expected to -- try again, which would clean up cassandra state.) - IdPConfigStore.insertConfig idp' + IdPConfigStore.insertConfig idp'' -- if the IdP issuer is updated, the old issuer must be removed explicitly. -- if this step is ommitted (due to a crash) resending the update request should fix the inconsistent state. - let mbteamid = case fromMaybe defWireIdPAPIVersion $ idp' ^. SAML.idpExtraInfo . apiVersion of + let mbteamid = case fromMaybe defWireIdPAPIVersion $ idp'' ^. SAML.idpExtraInfo . apiVersion of WireIdPAPIV1 -> Nothing WireIdPAPIV2 -> Just teamid - forM_ (idp' ^. SAML.idpExtraInfo . oldIssuers) (flip IdPConfigStore.deleteIssuer mbteamid) - pure idp' + forM_ (idp'' ^. SAML.idpExtraInfo . oldIssuers) (flip IdPConfigStore.deleteIssuer mbteamid) + pure idp'' + where + -- Ensure that the domain is not in use by an existing IDP + guardMultiIngressDuplicateDomain :: + ( Member (Error SparError) r, + Member IdPConfigStore r + ) => + TeamId -> + Maybe ZHostValue -> + SAML.IdPId -> + Sem r () + guardMultiIngressDuplicateDomain _teamId Nothing _ = pure () + guardMultiIngressDuplicateDomain teamId (Just zHost) idpId = do + idps <- IdPConfigStore.getConfigsByTeam teamId + let otherIdpsOnSameDomain = + any + (\idp -> (idp ^. SAML.idpExtraInfo . domain) == (Just zHost) && (idp ^. SAML.idpId) /= idpId) + idps + when otherIdpsOnSameDomain $ + throwSparSem SparIdPDomainInUse -- | Check that: idp id is valid; calling user is admin in that idp's home team; team id in -- new metainfo doesn't change; new issuer (if changed) is not in use anywhere else (except as diff --git a/services/spar/src/Spar/Error.hs b/services/spar/src/Spar/Error.hs index 732da191f3..7ee8018a81 100644 --- a/services/spar/src/Spar/Error.hs +++ b/services/spar/src/Spar/Error.hs @@ -115,6 +115,7 @@ data SparCustomError SparInternalError LText | -- | All errors returned from SCIM handlers are wrapped into 'SparScimError' SparScimError Scim.ScimError + | SparIdPDomainInUse deriving (Eq, Show) data SparProvisioningMoreThanOneIdP @@ -234,6 +235,7 @@ renderSparError (SAML.CustomError (IdpDbError AttemptToGetV2IssuerViaV1API)) = R renderSparError (SAML.CustomError (IdpDbError IdpNonUnique)) = Right $ Wai.mkError status409 "idp-non-unique" "We have found multiple IdPs with the same issuer. Please contact customer support." renderSparError (SAML.CustomError (IdpDbError IdpWrongTeam)) = Right $ Wai.mkError status409 "idp-wrong-team" "The IdP is not part of this team." renderSparError (SAML.CustomError (IdpDbError IdpNotFound)) = renderSparError (SAML.CustomError (SparIdPNotFound "")) +renderSparError (SAML.CustomError SparIdPDomainInUse) = Right $ Wai.mkError status409 "idp-duplicate-domain-for-team" "This team already has an IdP configured for this domain." -- Errors related to provisioning renderSparError (SAML.CustomError (SparProvisioningMoreThanOneIdP msg)) = Right $ Wai.mkError status400 "more-than-one-idp" do diff --git a/services/spar/src/Spar/Schema/Run.hs b/services/spar/src/Spar/Schema/Run.hs index 46bf1e5bd5..2ddb825c3c 100644 --- a/services/spar/src/Spar/Schema/Run.hs +++ b/services/spar/src/Spar/Schema/Run.hs @@ -35,6 +35,7 @@ import qualified Spar.Schema.V18 as V18 import qualified Spar.Schema.V19 as V19 import qualified Spar.Schema.V2 as V2 import qualified Spar.Schema.V20 as V20 +import qualified Spar.Schema.V21 as V21 import qualified Spar.Schema.V3 as V3 import qualified Spar.Schema.V4 as V4 import qualified Spar.Schema.V5 as V5 @@ -82,7 +83,8 @@ migrations = V17.migration, V18.migration, V19.migration, - V20.migration + V20.migration, + V21.migration -- TODO: Add a migration that removes unused fields -- (we don't want to risk running a migration which would -- effectively break the currently deployed spar service) diff --git a/services/spar/src/Spar/Schema/V21.hs b/services/spar/src/Spar/Schema/V21.hs new file mode 100644 index 0000000000..aaff208048 --- /dev/null +++ b/services/spar/src/Spar/Schema/V21.hs @@ -0,0 +1,32 @@ +-- This file is part of the Wire Server implementation. +-- +-- Copyright (C) 2025 Wire Swiss GmbH +-- +-- This program is free software: you can redistribute it and/or modify it under +-- the terms of the GNU Affero General Public License as published by the Free +-- Software Foundation, either version 3 of the License, or (at your option) any +-- later version. +-- +-- This program is distributed in the hope that it will be useful, but WITHOUT +-- ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS +-- FOR A PARTICULAR PURPOSE. See the GNU Affero General Public License for more +-- details. +-- +-- You should have received a copy of the GNU Affero General Public License along +-- with this program. If not, see . + +module Spar.Schema.V21 + ( migration, + ) +where + +import Cassandra.Schema +import Imports +import Text.RawString.QQ + +migration :: Migration +migration = Migration 21 "Add domain column to idp table" $ do + schema' + [r| + ALTER TABLE idp ADD (domain text); + |] diff --git a/services/spar/src/Spar/Sem/IdPConfigStore/Cassandra.hs b/services/spar/src/Spar/Sem/IdPConfigStore/Cassandra.hs index 56f60c6c4f..a755542346 100644 --- a/services/spar/src/Spar/Sem/IdPConfigStore/Cassandra.hs +++ b/services/spar/src/Spar/Sem/IdPConfigStore/Cassandra.hs @@ -38,6 +38,7 @@ import Spar.Data.Instances () import Spar.Error import Spar.Sem.IdPConfigStore (IdPConfigStore (..), Replaced (..), Replacing (..)) import URI.ByteString +import Wire.API.Routes.Public (ZHostValue) import Wire.API.User.IdentityProvider hiding (apiVersion, oldIssuers, replacedBy, team) import qualified Wire.API.User.IdentityProvider as IP @@ -66,7 +67,7 @@ idPToCassandra = ClearReplacedBy r -> embed @m $ clearReplacedBy r DeleteIssuer i t -> embed @m $ deleteIssuer i t -type IdPConfigRow = (SAML.IdPId, SAML.Issuer, URI, SignedCertificate, [SignedCertificate], TeamId, Maybe WireIdPAPIVersion, [SAML.Issuer], Maybe SAML.IdPId, Maybe Text) +type IdPConfigRow = (SAML.IdPId, SAML.Issuer, URI, SignedCertificate, [SignedCertificate], TeamId, Maybe WireIdPAPIVersion, [SAML.Issuer], Maybe SAML.IdPId, Maybe Text, Maybe ZHostValue) insertIdPConfig :: forall m. @@ -90,7 +91,8 @@ insertIdPConfig idp = do idp ^. SAML.idpExtraInfo . IP.apiVersion, idp ^. SAML.idpExtraInfo . IP.oldIssuers, idp ^. SAML.idpExtraInfo . IP.replacedBy, - Just (unIdPHandle $ idp ^. SAML.idpExtraInfo . handle) + Just (unIdPHandle $ idp ^. SAML.idpExtraInfo . handle), + idp ^. SAML.idpExtraInfo . IP.domain ) addPrepQuery byIssuer @@ -118,7 +120,7 @@ insertIdPConfig idp = do getAllIdPsByIssuerUnsafe issuer >>= mapM_ (failIfNot thisVersion) ins :: PrepQuery W IdPConfigRow () - ins = "INSERT INTO idp (idp, issuer, request_uri, public_key, extra_public_keys, team, api_version, old_issuers, replaced_by, handle) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)" + ins = "INSERT INTO idp (idp, issuer, request_uri, public_key, extra_public_keys, team, api_version, old_issuers, replaced_by, handle, domain) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)" -- FUTUREWORK: migrate `spar.issuer_idp` away, `spar.issuer_idp_v2` is enough. byIssuer :: PrepQuery W (SAML.Issuer, TeamId, SAML.IdPId) () @@ -176,18 +178,19 @@ getIdPConfig idpid = do apiVersion, oldIssuers, replacedBy, - mHandle + mHandle, + idpDomain ) = do let _edCertAuthnResponse = certsHead NL.:| certsTail _idpMetadata = SAML.IdPMetadata {..} - _idpExtraInfo = WireIdP teamId apiVersion oldIssuers replacedBy (mkHandle mHandle) + _idpExtraInfo = WireIdP teamId apiVersion oldIssuers replacedBy (mkHandle mHandle) idpDomain pure $ SAML.IdPConfig {..} mbidp <- traverse toIdp =<< retry x1 (query1 sel $ params LocalQuorum (Identity idpid)) maybe (throwError IdpNotFound) pure mbidp where sel :: PrepQuery R (Identity SAML.IdPId) IdPConfigRow - sel = "SELECT idp, issuer, request_uri, public_key, extra_public_keys, team, api_version, old_issuers, replaced_by, handle FROM idp WHERE idp = ?" + sel = "SELECT idp, issuer, request_uri, public_key, extra_public_keys, team, api_version, old_issuers, replaced_by, handle, domain FROM idp WHERE idp = ?" selTid :: PrepQuery R (Identity SAML.IdPId) (Identity (Maybe TeamId)) selTid = "SELECT team FROM idp WHERE idp = ?" diff --git a/services/spar/test-integration/Test/Spar/DataSpec.hs b/services/spar/test-integration/Test/Spar/DataSpec.hs index 49b5db4690..ec4b589145 100644 --- a/services/spar/test-integration/Test/Spar/DataSpec.hs +++ b/services/spar/test-integration/Test/Spar/DataSpec.hs @@ -201,14 +201,14 @@ spec = do it "getIdPConfigsByTeam works" $ do skipIdPAPIVersions [WireIdPAPIV1] teamid <- nextWireId - idp <- makeTestIdP <&> idpExtraInfo .~ WireIdP teamid Nothing [] Nothing (IdPHandle "IdP 1") + idp <- makeTestIdP <&> idpExtraInfo .~ WireIdP teamid Nothing [] Nothing (IdPHandle "IdP 1") Nothing () <- runSpar $ IdPEffect.insertConfig idp idps <- runSpar $ IdPEffect.getConfigsByTeam teamid liftIO $ idps `shouldBe` [idp] it "deleteIdPConfig works" $ do teamid <- nextWireId idpApiVersion <- asks (^. teWireIdPAPIVersion) - idp <- makeTestIdP <&> idpExtraInfo .~ WireIdP teamid (Just idpApiVersion) [] Nothing (IdPHandle "IdP 1") + idp <- makeTestIdP <&> idpExtraInfo .~ WireIdP teamid (Just idpApiVersion) [] Nothing (IdPHandle "IdP 1") Nothing () <- runSpar $ IdPEffect.insertConfig idp do midp <- runSpar $ IdPEffect.getConfig (idp ^. idpId) diff --git a/services/spar/test-integration/Util/Core.hs b/services/spar/test-integration/Util/Core.hs index 4262ba062c..16a13dcbd7 100644 --- a/services/spar/test-integration/Util/Core.hs +++ b/services/spar/test-integration/Util/Core.hs @@ -544,7 +544,7 @@ nextWireId :: (MonadIO m) => m (Id a) nextWireId = Id <$> liftIO UUID.nextRandom nextWireIdP :: (MonadIO m) => WireIdPAPIVersion -> m WireIdP -nextWireIdP version = WireIdP <$> iid <*> pure (Just version) <*> pure [] <*> pure Nothing <*> idpHandle +nextWireIdP version = WireIdP <$> iid <*> pure (Just version) <*> pure [] <*> pure Nothing <*> idpHandle <*> pure Nothing where iid = Id <$> liftIO UUID.nextRandom idpHandle = iid <&> IdPHandle . pack . show