Skip to content

Commit 559a318

Browse files
supersvensupersven
committed
Ensure only one Idp can be created for multi-ingress
1 parent 6cb7f1a commit 559a318

File tree

4 files changed

+96
-0
lines changed

4 files changed

+96
-0
lines changed

‎integration/test/API/Spar.hs‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,11 @@ getIdp user idpId = do
128128
req <- baseRequest user Spar Versioned $ joinHttpPath ["identity-providers", idpId]
129129
submit "GET" req
130130

131+
deleteIdp :: (HasCallStack, MakesValue user) => user -> String -> App Response
132+
deleteIdp user idpId = do
133+
req <- baseRequest user Spar Versioned $ joinHttpPath ["identity-providers", idpId]
134+
submit "DELETE" req
135+
131136
-- | https://staging-nginz-https.zinfra.io/v7/api/swagger-ui/#/default/sso-team-metadata
132137
getSPMetadata :: (HasCallStack, MakesValue domain) => domain -> String -> App Response
133138
getSPMetadata = (flip getSPMetadataWithZHost) Nothing

‎integration/test/Test/Spar/MultiIngressIdp.hs‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,13 @@ testMultiIngressIdp = do
7878
resp.jsonBody %. "extraInfo.domain" `shouldMatch` Null
7979

8080
-- From unconfigured to configured domain
81+
updateIdpWithZHost owner (Just bertZHost) idpId2 idpmeta2 `bindResponse` \resp -> do
82+
resp.status `shouldMatchInt` 409
83+
resp.jsonBody %. "label" `shouldMatch` "idp-duplicate-domain-for-team"
84+
85+
deleteIdp owner idpId `bindResponse` \resp -> do
86+
resp.status `shouldMatchInt` 204
87+
8188
updateIdpWithZHost owner (Just bertZHost) idpId2 idpmeta2 `bindResponse` \resp -> do
8289
resp.status `shouldMatchInt` 200
8390
resp.jsonBody %. "extraInfo.domain" `shouldMatch` bertZHost
@@ -96,4 +103,49 @@ testMultiIngressIdp = do
96103
resp.jsonBody %. "extraInfo.domain" `shouldMatch` Null
97104

98105
-- TODO: Test creation of two IDPs for the same domain -> should fail
106+
testMultiIngressAtMostIdpPerDomain :: (HasCallStack) => App ()
107+
testMultiIngressAtMostIdpPerDomain = do
108+
withModifiedBackend
109+
def
110+
{ sparCfg =
111+
removeField "saml.spSsoUri"
112+
>=> removeField "saml.spAppUri"
113+
>=> removeField "saml.contacts"
114+
>=> setField
115+
"saml.spDomainConfigs"
116+
( object
117+
[ ernieZHost
118+
.= object
119+
[ "spAppUri" .= "https://webapp.ernie.example.com",
120+
"spSsoUri" .= "https://nginz-https.ernie.example.com/sso",
121+
"contacts" .= [object ["type" .= "ContactTechnical"]]
122+
],
123+
bertZHost
124+
.= object
125+
[ "spAppUri" .= "https://webapp.bert.example.com",
126+
"spSsoUri" .= "https://nginz-https.bert.example.com/sso",
127+
"contacts" .= [object ["type" .= "ContactTechnical"]]
128+
]
129+
]
130+
)
131+
}
132+
$ \domain -> do
133+
(owner, tid, _) <- createTeam domain 1
134+
void $ setTeamFeatureStatus owner tid "sso" "enabled"
135+
136+
SampleIdP idpmeta1 _pCreds _ _ <- makeSampleIdPMetadata
137+
idpId1 <-
138+
createIdpWithZHost owner (Just ernieZHost) idpmeta1 `bindResponse` \resp -> do
139+
resp.status `shouldMatchInt` 201
140+
resp.jsonBody %. "id" >>= asString
141+
142+
SampleIdP idpmeta2 _pCreds _ _ <- makeSampleIdPMetadata
143+
void $ createIdpWithZHost owner (Just ernieZHost) idpmeta2 `bindResponse` \resp -> do
144+
resp.status `shouldMatchInt` 409
145+
resp.jsonBody %. "label" `shouldMatch` "idp-duplicate-domain-for-team"
146+
147+
updateIdpWithZHost owner (Just ernieZHost) idpId1 idpmeta2 `bindResponse` \resp -> do
148+
resp.status `shouldMatchInt` 200
149+
resp.jsonBody %. "extraInfo.domain" `shouldMatch` ernieZHost
150+
99151
-- TODO: Test updating existing IDP such that two with the same domain would exist -> should fail

‎services/spar/src/Spar/API.hs‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -621,6 +621,7 @@ idpCreate samlConfig zusr uncheckedMbHost (IdPMetadataValue rawIdpMetadata idpme
621621
let mbHost = filterMultiIngressZHost (samlConfig._cfgDomainConfigs) uncheckedMbHost
622622
teamid <- Brig.getZUsrCheckPerm zusr CreateUpdateDeleteIdp
623623
GalleyAccess.assertSSOEnabled teamid
624+
guardMultiIngressDuplicateDomain teamid mbHost
624625
idp <-
625626
maybe (IdPConfigStore.newHandle teamid) (pure . IdPHandle . fromRange) mHandle
626627
>>= validateNewIdP apiversion idpmeta teamid mReplaces mbHost
@@ -629,6 +630,21 @@ idpCreate samlConfig zusr uncheckedMbHost (IdPMetadataValue rawIdpMetadata idpme
629630
forM_ mReplaces $ \replaces ->
630631
IdPConfigStore.setReplacedBy (Replaced replaces) (Replacing (idp ^. SAML.idpId))
631632
pure idp
633+
where
634+
-- Ensure that the domain is not in use by an existing IDP
635+
guardMultiIngressDuplicateDomain ::
636+
( Member (Error SparError) r,
637+
Member IdPConfigStore r
638+
) =>
639+
TeamId ->
640+
Maybe ZHostValue ->
641+
Sem r ()
642+
guardMultiIngressDuplicateDomain _teamId Nothing = pure ()
643+
guardMultiIngressDuplicateDomain teamId (Just zHost) = do
644+
idps <- IdPConfigStore.getConfigsByTeam teamId
645+
let domains = idps ^.. traverse . SAML.idpExtraInfo . domain . _Just
646+
when (zHost `elem` domains) $
647+
throwSparSem SparIdPDomainInUse
632648

633649
-- | Only return a ZHost when multi-ingress is configured and the host value is a configured domain
634650
filterMultiIngressZHost :: Either SAML.MultiIngressDomainConfig (Map Domain SAML.MultiIngressDomainConfig) -> Maybe ZHostValue -> Maybe ZHostValue
@@ -779,6 +795,8 @@ idpUpdateXML ::
779795
idpUpdateXML zusr mDomain raw idpmeta idpid mHandle = withDebugLog "idpUpdateXML" (Just . show . (^. SAML.idpId)) $ do
780796
(teamid, idp) <- validateIdPUpdate zusr idpmeta idpid
781797
GalleyAccess.assertSSOEnabled teamid
798+
-- TODO: Move this into validateIdPUpdate?
799+
guardMultiIngressDuplicateDomain teamid mDomain idpid
782800
IdPRawMetadataStore.store (idp ^. SAML.idpId) raw
783801
let idp' :: IdP = case mHandle of
784802
Just idpHandle -> idp & (SAML.idpExtraInfo . handle) .~ IdPHandle (fromRange idpHandle)
@@ -795,6 +813,25 @@ idpUpdateXML zusr mDomain raw idpmeta idpid mHandle = withDebugLog "idpUpdateXML
795813
WireIdPAPIV2 -> Just teamid
796814
forM_ (idp'' ^. SAML.idpExtraInfo . oldIssuers) (flip IdPConfigStore.deleteIssuer mbteamid)
797815
pure idp''
816+
where
817+
-- Ensure that the domain is not in use by an existing IDP
818+
guardMultiIngressDuplicateDomain ::
819+
( Member (Error SparError) r,
820+
Member IdPConfigStore r
821+
) =>
822+
TeamId ->
823+
Maybe ZHostValue ->
824+
SAML.IdPId ->
825+
Sem r ()
826+
guardMultiIngressDuplicateDomain _teamId Nothing _ = pure ()
827+
guardMultiIngressDuplicateDomain teamId (Just zHost) idpId = do
828+
idps <- IdPConfigStore.getConfigsByTeam teamId
829+
let otherIdpsOnSameDomain =
830+
any
831+
(\idp -> (idp ^. SAML.idpExtraInfo . domain) == (Just zHost) && (idp ^. SAML.idpId) /= idpId)
832+
idps
833+
when otherIdpsOnSameDomain $
834+
throwSparSem SparIdPDomainInUse
798835

799836
-- | Check that: idp id is valid; calling user is admin in that idp's home team; team id in
800837
-- new metainfo doesn't change; new issuer (if changed) is not in use anywhere else (except as

‎services/spar/src/Spar/Error.hs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,7 @@ data SparCustomError
115115
SparInternalError LText
116116
| -- | All errors returned from SCIM handlers are wrapped into 'SparScimError'
117117
SparScimError Scim.ScimError
118+
| SparIdPDomainInUse
118119
deriving (Eq, Show)
119120

120121
data SparProvisioningMoreThanOneIdP
@@ -234,6 +235,7 @@ renderSparError (SAML.CustomError (IdpDbError AttemptToGetV2IssuerViaV1API)) = R
234235
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."
235236
renderSparError (SAML.CustomError (IdpDbError IdpWrongTeam)) = Right $ Wai.mkError status409 "idp-wrong-team" "The IdP is not part of this team."
236237
renderSparError (SAML.CustomError (IdpDbError IdpNotFound)) = renderSparError (SAML.CustomError (SparIdPNotFound ""))
238+
renderSparError (SAML.CustomError SparIdPDomainInUse) = Right $ Wai.mkError status409 "idp-duplicate-domain-for-team" "This team already has an IdP configured for this domain."
237239
-- Errors related to provisioning
238240
renderSparError (SAML.CustomError (SparProvisioningMoreThanOneIdP msg)) = Right $
239241
Wai.mkError status400 "more-than-one-idp" do

0 commit comments

Comments
 (0)