Lower SCIM-users-only constraint for /sso/get-by-email

Author: supersven

integration/test/Test/Spar/MultiIngressCrossIdpSso.hs

@@ -20,7 +20,7 @@ module Test.Spar.MultiIngressCrossIdpSso where
 import API.BrigInternal (getUsersId)
 import API.Common (randomEmail)
 import API.GalleyInternal (setTeamFeatureStatus)
-import API.Spar (createIdpWithZHostV2)
+import API.Spar (createIdpWithZHostV2, getSsoCodeByEmailWithZHost)
 import Data.List.NonEmpty (NonEmpty ((:|)))
 import Data.Text (pack)
 import GHC.Stack
@@ -90,6 +90,7 @@ testCrossIdpSsoCreatesDistinctUsers = do
                         ]
                   ]
               )
+            >=> setField "enableIdPByEmailDiscovery" True
       }
     $ \domain -> do
       -- Create team and enable SSO
@@ -252,6 +253,7 @@ testCrossIdpSsoEmailConflict = do
                         ]
                   ]
               )
+            >=> setField "enableIdPByEmailDiscovery" True
       }
     $ \domain -> do
       -- Create team and enable SSO
@@ -301,6 +303,12 @@ testCrossIdpSsoEmailConflict = do
         ssoIdTenant `shouldContain` ernieIssuer
         ssoIdTenant `shouldNotMatch` bertIssuer
 
+      -- Verify sso/get-by-email returns Ernie's IdP
+      getSsoCodeByEmailWithZHost domain (Just ernieZHost) biboEmail `bindResponse` \resp -> do
+        resp.status `shouldMatchInt` 200
+        ssoCodeStr <- resp.json %. "sso_code" >>= asString
+        ssoCodeStr `shouldMatch` idpId1
+
       -- Step 1.5: Bibo re-logs in on Ernie (should succeed - proves SSO works on same ingress)
       (mUserIdErnieAgain, _) <-
         loginWithSamlWithZHost
@@ -342,6 +350,12 @@ testCrossIdpSsoEmailConflict = do
         ssoIdTenant `shouldContain` bertIssuer
         ssoIdTenant `shouldNotMatch` ernieIssuer
 
+      -- Verify sso/get-by-email returns Bert's IdP after migration
+      getSsoCodeByEmailWithZHost domain (Just bertZHost) biboEmail `bindResponse` \resp -> do
+        resp.status `shouldMatchInt` 200
+        ssoCodeStr <- resp.json %. "sso_code" >>= asString
+        ssoCodeStr `shouldMatch` idpId2
+
       -- Step 3: Login on Ernie again to show back-and-forth migration works
       (mUserIdErnieFinal, _) <-
         loginWithSamlWithZHost
@@ -365,3 +379,9 @@ testCrossIdpSsoEmailConflict = do
         bertIssuer <- _idp2.json %. "metadata.issuer" >>= asString
         ssoIdTenant `shouldContain` ernieIssuer
         ssoIdTenant `shouldNotMatch` bertIssuer
+
+      -- Verify sso/get-by-email returns Ernie's IdP after migration back
+      getSsoCodeByEmailWithZHost domain (Just ernieZHost) biboEmail `bindResponse` \resp -> do
+        resp.status `shouldMatchInt` 200
+        ssoCodeStr <- resp.json %. "sso_code" >>= asString
+        ssoCodeStr `shouldMatch` idpId1

libs/wire-subsystems/src/Wire/IdPSubsystem/Interpreter.hs

@@ -123,7 +123,11 @@ getSsoCodeByEmailImpl enableIdPByEmailDiscovery mbHost email =
 
     isScimOrSsoUser :: User -> Bool
     isScimOrSsoUser user =
-      userManagedBy user == ManagedByScim && isJust (userSSOId user)
+      -- TODO: This used to check if the user is SCIM AND SSO! The RFC not
+      -- really unambiguous about this. The customer currently provisions
+      -- non-SCIM. So, that would fit. However, this change needs a sing-off by
+      -- security.
+      isJust (userSSOId user)
 
     findIdPByDomain :: (Member (Logger (Log.Msg -> Log.Msg)) r) => [IP.IdP] -> Sem r (Maybe SAML.IdPId)
     findIdPByDomain idps = do

libs/wire-subsystems/test/unit/Wire/IdPSubsystem/InterpreterSpec.hs

@@ -37,7 +37,6 @@ import System.Logger.Message qualified as Log
 import Test.Hspec
 import Test.Hspec.QuickCheck
 import Test.QuickCheck
-import Test.QuickCheck.Gen
 import Wire.API.Team.Member
 import Wire.API.User
 import Wire.API.User.IdentityProvider
@@ -313,19 +312,12 @@ spec = describe "IdPSubsystem.Interpreter" $ do
       result `shouldBe` Right Nothing
       expectedSevereLogs logs mempty
 
-    prop "returns Nothing for non SCIM/SSO user" $ \(teamMember :: TeamMember) user idp userRef email teamId -> do
-      (userIdentity, userManagedBy) <-
-        generate $
-          ( do
-              ui <- Test.QuickCheck.Gen.elements [Just (SSOIdentity (UserSSOId userRef) (Just email)), Nothing]
-              mngtBy :: ManagedBy <- arbitrary
-              pure (ui, mngtBy)
-          )
-            `suchThat` (\(ui, mngtBy) -> isNothing ui || mngtBy == ManagedByWire)
+    prop "returns Nothing for non SSO user" $ \(teamMember :: TeamMember) user idp email teamId -> do
+      userManagedBy <- generate (arbitrary :: Gen ManagedBy)
 
       let userWithEmail =
             user
-              { userIdentity = userIdentity,
+              { userIdentity = Nothing, -- No SSO identity
                 userEmailUnvalidated = Just email,
                 userTeam = Just teamId,
                 userManagedBy = userManagedBy