Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor Scim.Scim.UserSpec tests to use composable pieces #2762

Merged
merged 3 commits into from
Oct 13, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/5-internal/cleanup-scim-tests
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Refactor some internal Scim user tests
102 changes: 34 additions & 68 deletions services/spar/test/Test/Spar/Scim/UserSpec.hs
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,15 @@ import Brig.Types.User
import Control.Monad.Except (runExceptT)
import Data.Handle (parseHandle)
import Data.Id
import qualified Data.Json.Util
import Imports
import Polysemy
import Polysemy.TinyLog
import Spar.Scim.User (deleteScimUser)
import Spar.Sem.BrigAccess
import Spar.Sem.IdPConfigStore
import Spar.Sem.IdPConfigStore.Mem (TypedState, idPToMem)
import Spar.Sem.IdPConfigStore.Mem (idPToMem)
import Spar.Sem.SAMLUserStore
import Spar.Sem.SAMLUserStore.Mem (UserRefOrd, samlUserStoreToMem)
import Spar.Sem.SAMLUserStore.Mem (samlUserStoreToMem)
import qualified Spar.Sem.ScimExternalIdStore as ScimExternalIdStore
import Spar.Sem.ScimExternalIdStore.Mem (scimExternalIdStoreToMem)
import Spar.Sem.ScimUserTimesStore
Expand All @@ -25,7 +24,6 @@ import Test.Hspec
import Test.QuickCheck
import Web.Scim.Schema.Error
import Wire.API.User
import qualified Wire.API.User.Identity
import Wire.API.User.Scim
import Wire.Sem.Logger.TinyLog (discardTinyLogs)

Expand All @@ -36,33 +34,33 @@ spec = describe "deleteScimUser" $ do
acc <- someActiveUser tokenInfo
r <-
interpretWithBrigAccessMock
(mockBrigForActiveUser acc AccountDeleted)
(mockBrig (withActiveUser acc) AccountDeleted)
(deleteUserAndAssertDeletionInSpar acc tokenInfo)
handlerResult r `shouldBe` Right ()
r `shouldBe` Right ()
it "returns an error when the account was deleted before" $ do
tokenInfo <- generate arbitrary
acc <- someActiveUser tokenInfo
r <-
interpretWithBrigAccessMock
(mockBrigForActiveUser acc AccountAlreadyDeleted)
(mockBrig (withActiveUser acc) AccountAlreadyDeleted)
(deleteUserAndAssertDeletionInSpar acc tokenInfo)
handlerResult r `shouldBe` Left (notFound "user" ((idToText . userId . accountUser) acc))
r `shouldBe` Left (notFound "user" ((idToText . userId . accountUser) acc))
it "returns an error when there never was an account" $ do
uid <- generate arbitrary
tokenInfo <- generate arbitrary
r <-
interpretWithBrigAccessMock
mockBrigForNonExistendUser
(mockBrig (const Nothing) NoUser)
(runExceptT $ deleteScimUser tokenInfo uid)
handlerResult r `shouldBe` Left (notFound "user" (idToText uid))
r `shouldBe` Left (notFound "user" (idToText uid))
it "returns no error when there was a partially deleted account" $ do
uid <- generate arbitrary
tokenInfo <- generate arbitrary
r <-
interpretWithBrigAccessMock
mockBrigForPartiallyDeletedUser
(mockBrig (const Nothing) AccountDeleted)
(runExceptT $ deleteScimUser tokenInfo uid)
handlerResult r `shouldBe` Right ()
r `shouldBe` Right ()

deleteUserAndAssertDeletionInSpar ::
forall (r :: EffectRow).
Expand Down Expand Up @@ -99,77 +97,45 @@ type EffsWithoutBrigAccess =
Final IO
]

type Effs = BrigAccess ': EffsWithoutBrigAccess

type InterpreterState =
( Map (Data.Id.TeamId, Wire.API.User.Identity.Email) Data.Id.UserId,
( Map Data.Id.UserId (Data.Json.Util.UTCTimeMillis, Data.Json.Util.UTCTimeMillis),
( Map UserRefOrd UserId,
(Spar.Sem.IdPConfigStore.Mem.TypedState, Either ScimError ())
)
)
)

handlerResult :: InterpreterState -> Either ScimError ()
handlerResult = snd . snd . snd . snd

interpretWithBrigAccessMock ::
( Sem Effs (Either ScimError ()) ->
Sem EffsWithoutBrigAccess (Either ScimError ())
( Sem (BrigAccess ': EffsWithoutBrigAccess) a ->
Sem EffsWithoutBrigAccess a
) ->
Sem Effs (Either ScimError ()) ->
IO InterpreterState
Sem (BrigAccess ': EffsWithoutBrigAccess) a ->
IO a
interpretWithBrigAccessMock mock =
runFinal
. embedToFinal @IO
. discardTinyLogs
. scimExternalIdStoreToMem
. scimUserTimesStoreToMem
. samlUserStoreToMem
. idPToMem
. ignoringState scimExternalIdStoreToMem
. ignoringState scimUserTimesStoreToMem
. ignoringState samlUserStoreToMem
. ignoringState idPToMem
mdimjasevic marked this conversation as resolved.
Show resolved Hide resolved
. mock

mockBrigForNonExistendUser ::
forall (r :: EffectRow).
Members '[Embed IO] r =>
Sem (BrigAccess ': r) (Either ScimError ()) ->
Sem r (Either ScimError ())
mockBrigForNonExistendUser = interpret $ \case
(GetAccount WithPendingInvitations _) -> pure Nothing
(Spar.Sem.BrigAccess.DeleteUser _) -> pure NoUser
_ -> do
liftIO $ expectationFailure $ "Unexpected effect (call to brig)"
error "Throw error here to avoid implementation of all cases."

mockBrigForPartiallyDeletedUser ::
forall (r :: EffectRow).
Members '[Embed IO] r =>
Sem (BrigAccess ': r) (Either ScimError ()) ->
Sem r (Either ScimError ())
mockBrigForPartiallyDeletedUser = interpret $ \case
(GetAccount WithPendingInvitations _) -> pure Nothing
(Spar.Sem.BrigAccess.DeleteUser _) -> pure AccountDeleted
_ -> do
liftIO $ expectationFailure $ "Unexpected effect (call to brig)"
error "Throw error here to avoid implementation of all cases."
ignoringState :: Functor f => (a -> f (c, b)) -> a -> f b
ignoringState f = fmap snd . f

mockBrigForActiveUser ::
forall (r :: EffectRow).
mockBrig ::
forall (r :: EffectRow) a.
Members '[Embed IO] r =>
UserAccount ->
(UserId -> Maybe UserAccount) ->
DeleteUserResult ->
Sem (BrigAccess ': r) (Either ScimError ()) ->
Sem r (Either ScimError ())
mockBrigForActiveUser acc deletionResult = interpret $ \case
(GetAccount WithPendingInvitations uid) ->
if uid == (userId . accountUser) acc
then pure $ Just acc
else pure Nothing
(Spar.Sem.BrigAccess.DeleteUser _) -> pure deletionResult
Sem (BrigAccess ': r) a ->
Sem r a
mockBrig lookup_user delete_response = interpret $ \case
(GetAccount WithPendingInvitations uid) -> pure $ lookup_user uid
(Spar.Sem.BrigAccess.DeleteUser _) -> pure delete_response
_ -> do
liftIO $ expectationFailure $ "Unexpected effect (call to brig)"
error "Throw error here to avoid implementation of all cases."

withActiveUser :: UserAccount -> UserId -> Maybe UserAccount
withActiveUser acc uid =
if uid == (userId . accountUser) acc
then Just acc
else Nothing

someActiveUser :: ScimTokenInfo -> IO UserAccount
someActiveUser tokenInfo = do
user <- generate arbitrary
Expand Down