From 498e45c671257d717bc6426f749034a6b5dc7c35 Mon Sep 17 00:00:00 2001 From: Leif Battermann Date: Wed, 18 May 2022 10:49:02 +0200 Subject: [PATCH] SQSERVICES-1538-be-email-cannot-be-activated-when-it-was-changed-via-scim-before-the-user-registered (#2396) --- changelog.d/5-internal/pr-2396 | 1 + .../Test/Spar/Scim/UserSpec.hs | 81 +++++++++++++------ 2 files changed, 58 insertions(+), 24 deletions(-) create mode 100644 changelog.d/5-internal/pr-2396 diff --git a/changelog.d/5-internal/pr-2396 b/changelog.d/5-internal/pr-2396 new file mode 100644 index 00000000000..1ad4b2fbe6c --- /dev/null +++ b/changelog.d/5-internal/pr-2396 @@ -0,0 +1 @@ +Integration test for edge case: change external id before account registration diff --git a/services/spar/test-integration/Test/Spar/Scim/UserSpec.hs b/services/spar/test-integration/Test/Spar/Scim/UserSpec.hs index f6b4c51b44e..7ed60c28887 100644 --- a/services/spar/test-integration/Test/Spar/Scim/UserSpec.hs +++ b/services/spar/test-integration/Test/Spar/Scim/UserSpec.hs @@ -1382,6 +1382,7 @@ specUpdateUser = describe "PUT /Users/:id" $ do it "updates the matching Brig user" $ testBrigSideIsUpdated it "cannot update user to match another user's externalId" $ testUpdateToExistingExternalIdFails it "cannot remove display name" $ testCannotRemoveDisplayName + it "updates the externalId of unregistered account" $ testUpdateExternalIdOfUnregisteredAccount context "user is from different team" $ do it "fails to update user with 404" testUserUpdateFailsWithNotFoundIfOutsideTeam context "user does not exist" $ do @@ -1566,7 +1567,7 @@ testUpdateExternalId withidp = do let userid = scimUserId storedUser if withidp then call $ activateEmail brig email - else registerUser email + else registerUser brig tid email veid :: ValidExternalId <- either (error . show) pure $ mkValidExternalId midp (Scim.User.externalId user) -- Overwrite the user with another randomly-generated user (only controlling externalId) @@ -1585,8 +1586,8 @@ testUpdateExternalId withidp = do _ <- updateUser tok userid user' when hasChanged (call $ activateEmail brig otherEmail) - muserid <- lookupByValidExternalId veid - muserid' <- lookupByValidExternalId veid' + muserid <- lookupByValidExternalId tid veid + muserid' <- lookupByValidExternalId tid veid' liftIO $ do if hasChanged then do @@ -1597,30 +1598,62 @@ testUpdateExternalId withidp = do (hasChanged, muserid') `shouldBe` (hasChanged, Just userid) eventually $ checkEmail userid (Just $ if hasChanged then otherEmail else email) - lookupByValidExternalId :: ValidExternalId -> TestSpar (Maybe UserId) - lookupByValidExternalId = - runValidExternalIdEither - (runSpar . SAMLUserStore.get) - ( \email -> do - let action = SU.scimFindUserByEmail midp tid $ fromEmail email - result <- runSpar . runExceptT . runMaybeT $ action - case result of - Right muser -> pure $ Scim.id . Scim.thing <$> muser - Left err -> error $ show err - ) - - registerUser :: Email -> TestSpar () - registerUser email = do - let r = call $ get (brig . path "/i/teams/invitations/by-email" . queryItem "email" (toByteString' email)) - inv <- responseJsonError =<< r \u -> u {Scim.User.externalId = Just $ fromEmail email} + storedUser <- createUser tok user + let userid = scimUserId storedUser + veid :: ValidExternalId <- + either (error . show) pure $ mkValidExternalId Nothing (Scim.User.externalId user) + -- Overwrite the user with another randomly-generated user (only controlling externalId) + -- And update the user before they have registered their account + otherEmail <- randomEmail + user' <- do + let upd u = u {Scim.User.externalId = Just $ fromEmail otherEmail} + randomScimUser <&> upd + let veid' = either (error . show) id $ mkValidExternalId Nothing (Scim.User.externalId user') + _ <- updateUser tok userid user' + -- Now the user registers their account (via old email) + registerUser brig tid email + -- Then the user activates their new email address + call $ activateEmail brig otherEmail + muserid <- lookupByValidExternalId tid veid + muserid' <- lookupByValidExternalId tid veid' + liftIO $ do + muserid `shouldBe` Nothing + muserid' `shouldBe` Just userid + eventually $ checkEmail userid (Just otherEmail) + +lookupByValidExternalId :: TeamId -> ValidExternalId -> TestSpar (Maybe UserId) +lookupByValidExternalId tid = + runValidExternalIdEither + (runSpar . SAMLUserStore.get) + ( \email -> do + let action = SU.scimFindUserByEmail Nothing tid $ fromEmail email + result <- runSpar . runExceptT . runMaybeT $ action + case result of + Right muser -> pure $ Scim.id . Scim.thing <$> muser + Left err -> error $ show err + ) + +registerUser :: BrigReq -> TeamId -> Email -> TestSpar () +registerUser brig tid email = do + let r = call $ get (brig . path "/i/teams/invitations/by-email" . queryItem "email" (toByteString' email)) + inv <- responseJsonError =<< r