WPB-21744: fix email verification resend for SCIM users

changelog.d/3-bug-fixes/WPB-21744

@@ -0,0 +1 @@
+Fix email verification resend for SCIM users.

integration/test/Test/User.hs

@@ -418,3 +418,94 @@ testEphemeralUserCreation (TaggedBool enabled) = do
   where
     registerEphemeralUser domain = addUser domain def
     registerUserWithEmail domain = addUser domain def {email = Just ("user@" <> domain)}
+
+testBrigSCIMResendVerificationEmail :: (HasCallStack) => App ()
+testBrigSCIMResendVerificationEmail = do
+  -- 1. Create team (owner)
+  (owner, _tid, _) <- createTeam OwnDomain 1
+
+  -- 2. Create SCIM token
+  tok <- bindResponse (Spar.createScimTokenV6 owner def) $ \resp -> do
+    resp.status `shouldMatchInt` 200
+    resp.json %. "token" >>= asString
+
+  -- 3. Create SCIM User
+  scimEmail <- randomEmail
+  scimExtId <- randomEmail
+  scimUser <- randomScimUserWithEmail scimExtId scimEmail
+
+  userId <- bindResponse (Spar.createScimUser OwnDomain tok scimUser) $ \resp -> do
+    resp.status `shouldMatchInt` 201
+    resp.json %. "id" >>= asString
+
+  -- 4. Re-send verification email
+  putUserEmail owner (object ["id" .= userId, "domain" .= ("example.com" :: String)]) scimEmail `bindResponse` \resp -> do
+    resp.status `shouldMatchInt` 200
+
+testBrigSCIMPromotedResendVerificationEmail :: (HasCallStack) => App ()
+testBrigSCIMPromotedResendVerificationEmail = do
+  -- 1. Create team (owner) and a normal user (alice)
+  (owner, tid, _) <- createTeam OwnDomain 1
+
+  aliceEmail <- randomEmail
+  alice <- createUser OwnDomain def {email = Just aliceEmail, password = Just defPassword, activate = True} >>= getJSON 201
+  aliceId <- asString $ alice %. "id"
+
+  -- Add alice to team
+  bindResponse (postInvitation owner def {email = Just aliceEmail}) $ \resp -> do
+    resp.status `shouldMatchInt` 201
+
+  inv <- getInvitationByEmail OwnDomain aliceEmail >>= getJSON 200
+  invCodeResp <- I.getInvitationCodeForTeam OwnDomain tid inv >>= getJSON 200
+  code <- invCodeResp %. "code" & asString
+
+  bindResponse (acceptTeamInvitation alice code (Just defPassword)) $ \resp -> do
+    resp.status `shouldMatchInt` 200
+
+  -- 2. Login as alice
+  (cookie, token) <- bindResponse (login OwnDomain aliceEmail defPassword) $ \resp -> do
+    resp.status `shouldMatchInt` 200
+    token <- resp.json %. "access_token" & asString
+    let cookie = fromJust $ getCookie "zuid" resp
+    pure ("zuid=" <> cookie, token)
+
+  -- 3. Alice requests email change to B (pending state)
+  emailDomain <- randomDomain
+  newEmail <- randomName <&> (<> "@" <> emailDomain)
+  updateEmail alice newEmail cookie token >>= assertSuccess
+
+  -- 4. Make alice managed by SCIM (Adopt via PUT)
+  scimToken <- bindResponse (Spar.createScimToken owner def) $ \resp -> do
+    resp.status `shouldMatchInt` 200
+    resp.json %. "token" & asString
+
+  let aliceUserName = map (\c -> if c == '@' then '_' else c) aliceEmail
+  let scimUser =
+        object
+          [ "schemas" .= ["urn:ietf:params:scim:schemas:core:2.0:User"],
+            "userName" .= aliceUserName,
+            "emails" .= [object ["value" .= aliceEmail, "type" .= "work", "primary" .= True]],
+            "externalId" .= aliceId,
+            "name" .= object ["familyName" .= ("User" :: String), "givenName" .= ("Alice" :: String)]
+          ]
+
+  -- Try to update user via SCIM. This should set managed_by=scim.  (`GET .../Users` would have the same effect.)
+  bindResponse (Spar.updateScimUser owner scimToken aliceId scimUser) $ \resp -> do
+    resp.status `shouldMatchOneOf` [Number 200, Number 201, Number 204]
+
+  -- 7. Alice tries to resend verification for B (client side)
+  -- This should now succeed (202) despite being managed-by-scim, because it is a resend of pending email
+  updateEmail alice newEmail cookie token `bindResponse` \resp -> do
+    resp.status `shouldMatchInt` 403 -- or something; also check error label.
+  updateEmail alice aliceEmail cookie token `bindResponse` \resp -> do
+    resp.status `shouldMatchInt` 204
+
+-- (TODO: there may be a refresh problem in team-management here: if
+-- scim updates the user, team-management may not notice.)
+
+-- relevant end-points that we may want to test:
+-- https://staging-nginz-https.zinfra.io/v14/api/swagger-ui/#/default/accept-team-invitation  (should not work any more, or be a noop, not sure)
+-- https://staging-nginz-https.zinfra.io/v14/api/swagger-ui/#/default/head-team-invitations
+-- https://staging-nginz-https.zinfra.io/v14/api/swagger-ui/#/default/get-team-invitation-info
+-- https://staging-nginz-https.zinfra.io/v14/api/swagger-ui/#/default/browse-team
+-- https://staging-nginz-https.zinfra.io/v14/api/swagger-ui/#/default/get-team-member

libs/wire-api/src/Wire/API/User/Identity.hs

@@ -135,7 +135,7 @@ ssoIdentity _ = Nothing
 --------------------------------------------------------------------------------
 -- UserSSOId
 
--- | User's external identity.
+-- | User's external identity (both SAML idpid, nameid, and SCIM externalId)
 --
 -- NB: this type is serialized to the full xml encoding of the `SAML.UserRef` components, but
 -- deserialiation is more lenient: it also allows for the `Issuer` to be a plain URL (without

services/brig/src/Brig/Data/User.hs

@@ -185,7 +185,7 @@ updateSSOId u ssoid = do
   mteamid <- lookupUserTeam u
   case mteamid of
     Just _ -> do
-      retry x5 $ write userSSOIdUpdate (params LocalQuorum (ssoid, u))
+      retry x5 $ write userSSOIdUpdate (params LocalQuorum (ssoid, u)) -- TODO: why is this not asking team member for address verification, even with feature flag enabled?
       pure True
     Nothing -> pure False
 

services/spar/src/Spar/Scim/User.hs

@@ -735,7 +735,7 @@ updateVsuUref team uid old new = do
   ScimExternalIdStore.insert team new.validScimIdExternal uid
   for_ (justThere new.validScimIdAuthInfo) (`SAMLUserStore.insert` uid)
 
-  BrigAccess.setSSOId uid $ veidToUserSSOId new
+  BrigAccess.setSSOId uid $ veidToUserSSOId new -- TODO: theory: this doesn't remove the old email update verification token.
 
 toScimStoredUser ::
   (HasCallStack) =>
@@ -1186,11 +1186,13 @@ scimFindUserByExternalId mIdpConfig stiTeam eid = do
         -- there are a few ways to find a user. this should all be redundant, especially the where
         -- we lookup a user from brig by email, throw it away and only keep the uid, and then use
         -- the uid to lookup the account again. but cassandra, and also reasons.
+        -- TODO: BUG! this is handled by `listActivatedAccountsH` in brig, but we also want *inactive* accounts here!
         mViaEmail :: Maybe UserId <- join <$> (for (justHere veid.validScimIdAuthInfo) ((userId <$$>) . BrigAccess.getByEmail))
         mViaUref :: Maybe UserId <- join <$> (for (justThere veid.validScimIdAuthInfo) SAMLUserStore.get)
         pure $ mViaEmail <|> mViaUref
     Just uid -> pure uid
   acc <- MaybeT . lift . BrigAccess.getAccount Brig.WithPendingInvitations $ uid
+  -- TODO: how can userId acc /= uid here?  do we even need to call getAccount?
   getUserById mIdpConfig stiTeam (userId acc)
 
 logFilter :: Filter -> (Msg -> Msg)