Skip to content
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/2-features/update-ug-on-deletion
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Remove user from all user groups on deletion
17 changes: 17 additions & 0 deletions integration/test/Test/UserGroup.hs
Original file line number Diff line number Diff line change
Expand Up @@ -373,3 +373,20 @@ testUserGroupMembersCount = do
resp.status `shouldMatchInt` 200
resp.json %. "page.0.membersCount" `shouldMatchInt` 2
resp.json %. "total" `shouldMatchInt` 1

testUserGroupRemovalOnDelete :: (HasCallStack) => App ()
testUserGroupRemovalOnDelete = do
(alice, tid, [bob, charlie]) <- createTeam OwnDomain 3

bobId <- bob %. "id" & asString
charlieId <- charlie %. "id" & asString

ug <-
createUserGroup alice (object ["name" .= "none", "members" .= [bobId, charlieId]])
>>= getJSON 200
gid <- ug %. "id" & asString
void $ deleteTeamMember tid alice bob >>= getBody 202

bindResponse (getUserGroup alice gid) $ \resp -> do
resp.status `shouldMatchInt` 200
resp.json %. "members" `shouldMatch` [charlieId]
4 changes: 2 additions & 2 deletions libs/wire-subsystems/src/Wire/AppStore/Postgres.hs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ createAppImpl ::
StoredApp ->
Sem r ()
createAppImpl app =
runResultlessStatement app $
runStatement app $
lmapPG
[resultlessStatement|
insert into apps (user_id, team_id, metadata)
Expand All @@ -49,7 +49,7 @@ getAppImpl ::
UserId ->
Sem r (Maybe StoredApp)
getAppImpl uid =
runMaybeStatement uid $
runStatement uid $
dimapPG
[maybeStatement| select (user_id :: uuid), (team_id :: uuid), (metadata :: json)
from apps where user_id = ($1 :: uuid) |]
20 changes: 4 additions & 16 deletions libs/wire-subsystems/src/Wire/Postgres.hs
Original file line number Diff line number Diff line change
Expand Up @@ -8,26 +8,14 @@ import Polysemy
import Polysemy.Error (Error, throw)
import Polysemy.Input

runResultlessStatement ::
runStatement ::
( Member (Input Pool) r,
Member (Embed IO) r,
Member (Error UsageError) r
) =>
a ->
Statement a () ->
Sem r ()
runResultlessStatement a stmt = do
pool <- input
liftIO (use pool (statement a stmt)) >>= either throw pure

runMaybeStatement ::
( Member (Input Pool) r,
Member (Embed IO) r,
Member (Error UsageError) r
) =>
a ->
Statement a (Maybe b) ->
Sem r (Maybe b)
runMaybeStatement a stmt = do
Statement a b ->
Sem r b
runStatement a stmt = do
pool <- input
liftIO (use pool (statement a stmt)) >>= either throw pure
3 changes: 3 additions & 0 deletions libs/wire-subsystems/src/Wire/UserGroupStore.hs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ data UserGroupPageRequest = UserGroupPageRequest

data PaginationState = PaginationSortByName (Maybe (UserGroupName, UserGroupId)) | PaginationSortByCreatedAt (Maybe (UTCTimeMillis, UserGroupId))

userGroupCreatedAtPaginationState :: UserGroup_ f -> (UTCTimeMillis, UserGroupId)
userGroupCreatedAtPaginationState ug = (ug.createdAt, ug.id_)

toSortBy :: PaginationState -> SortBy
toSortBy = \case
PaginationSortByName _ -> SortByName
Expand Down
1 change: 1 addition & 0 deletions libs/wire-subsystems/src/Wire/UserGroupSubsystem.hs
Original file line number Diff line number Diff line change
Expand Up @@ -32,5 +32,6 @@ data UserGroupSubsystem m a where
AddUsers :: UserId -> UserGroupId -> Vector UserId -> UserGroupSubsystem m ()
UpdateUsers :: UserId -> UserGroupId -> Vector UserId -> UserGroupSubsystem m ()
RemoveUser :: UserId -> UserGroupId -> UserId -> UserGroupSubsystem m ()
RemoveUserFromAllGroups :: UserId -> TeamId -> UserGroupSubsystem m ()

makeSem ''UserGroupSubsystem
36 changes: 36 additions & 0 deletions libs/wire-subsystems/src/Wire/UserGroupSubsystem/Interpreter.hs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ interpretUserGroupSubsystem = interpret $ \case
AddUsers adder groupId addeeIds -> addUsers adder groupId addeeIds
UpdateUsers updater groupId uids -> updateUsers updater groupId uids
RemoveUser remover groupId removeeId -> removeUser remover groupId removeeId
RemoveUserFromAllGroups uid tid -> removeUserFromAllGroups uid tid

data UserGroupSubsystemError
= UserGroupNotATeamAdmin
Expand Down Expand Up @@ -328,3 +329,38 @@ removeUser remover groupId removeeId = do
pushNotifications
[ mkEvent remover (UserGroupUpdated groupId) admins
]

removeUserFromAllGroups ::
( Member Store.UserGroupStore r,
Member TeamSubsystem r,
Member (Error UserGroupSubsystemError) r
) =>
UserId ->
TeamId ->
Sem r ()
removeUserFromAllGroups uid tid = do
void $ internalGetTeamMember uid tid >>= note UserGroupMemberIsNotInTheSameTeam
nextPage Nothing >>= go
where
go (ug : ugs) = do
Store.removeUser ug.id_ uid
-- when we get to the last item, get a new page
ugs' <- case ugs of
[] -> nextPage (Just ug)
_ -> pure ugs
go ugs'
-- no more items, terminate
go [] = pure ()

nextPage mug =
fmap (.page) . Store.getUserGroups $
UserGroupPageRequest
{ pageSize = def,
sortOrder = Desc,
paginationState =
PaginationSortByCreatedAt $
fmap Store.userGroupCreatedAtPaginationState mug,
team = tid,
searchString = Nothing,
includeMemberCount = False
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

module Wire.UserGroupSubsystem.InterpreterSpec (spec) where

import Control.Error.Util (hush)
import Control.Lens ((.~), (^.))
import Control.Monad
import Data.Aeson qualified as A
Expand Down Expand Up @@ -266,13 +267,13 @@ spec = timeoutHook $ describe "UserGroupSubsystem.Interpreter" do

prop "team members can only get user groups from their own team" $
\(WithMods team1 :: WithMods '[AtLeastOneNonAdmin] ArbitraryTeam)
userGroupName1
(WithMods team2 :: WithMods '[AtLeastOneNonAdmin] ArbitraryTeam)
userGroupName2 ->
(WithMods team2 :: WithMods '[AtLeastOneNonAdmin] ArbitraryTeam) ->
expectRight
. runDependencies (allUsers team1 <> allUsers team2) (galleyTeam team1 <> galleyTeam team2)
. interpretUserGroupSubsystem
$ do
let userGroupName1 = fromJust . hush $ userGroupNameFromText "first"
let userGroupName2 = fromJust . hush $ userGroupNameFromText "second"
let newUserGroup1 =
NewUserGroup
{ name = userGroupName1,
Expand Down
6 changes: 5 additions & 1 deletion services/brig/src/Brig/API/Internal.hs
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ import Wire.Sem.Random (Random)
import Wire.SparAPIAccess (SparAPIAccess)
import Wire.TeamInvitationSubsystem
import Wire.TeamSubsystem (TeamSubsystem)
import Wire.UserGroupSubsystem
import Wire.UserKeyStore
import Wire.UserStore as UserStore
import Wire.UserSubsystem
Expand All @@ -148,6 +149,7 @@ servantSitemap ::
Member GalleyAPIAccess r,
Member NotificationSubsystem r,
Member UserSubsystem r,
Member UserGroupSubsystem r,
Member TeamSubsystem r,
Member TeamInvitationSubsystem r,
Member UserStore r,
Expand Down Expand Up @@ -222,6 +224,7 @@ accountAPI ::
Member (Embed HttpClientIO) r,
Member NotificationSubsystem r,
Member UserSubsystem r,
Member UserGroupSubsystem r,
Member UserKeyStore r,
Member (Input (Local ())) r,
Member UserStore r,
Expand Down Expand Up @@ -627,7 +630,8 @@ deleteUserNoAuthH ::
Member Events r,
Member UserSubsystem r,
Member PropertySubsystem r,
Member AuthenticationSubsystem r
Member AuthenticationSubsystem r,
Member UserGroupSubsystem r
) =>
UserId ->
(Handler r) DeleteUserResponse
Expand Down
6 changes: 4 additions & 2 deletions services/brig/src/Brig/API/Public.hs
Original file line number Diff line number Diff line change
Expand Up @@ -1418,7 +1418,8 @@ deleteSelfUser ::
Member Events r,
Member HashPassword r,
Member RateLimit r,
Member AuthenticationSubsystem r
Member AuthenticationSubsystem r,
Member UserGroupSubsystem r
) =>
Local UserId ->
Public.DeleteUser ->
Expand All @@ -1436,7 +1437,8 @@ verifyDeleteUser ::
Member PropertySubsystem r,
Member UserSubsystem r,
Member Events r,
Member AuthenticationSubsystem r
Member AuthenticationSubsystem r,
Member UserGroupSubsystem r
) =>
Public.VerifyDeleteUser ->
Handler r ()
Expand Down
16 changes: 11 additions & 5 deletions services/brig/src/Brig/API/User.hs
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,7 @@ import Wire.Sem.Paging.Cassandra
import Wire.StoredUser
import Wire.TeamSubsystem (TeamSubsystem)
import Wire.TeamSubsystem qualified as TeamSubsystem
import Wire.UserGroupSubsystem
import Wire.UserKeyStore
import Wire.UserStore (UserStore)
import Wire.UserStore qualified as UserStore
Expand Down Expand Up @@ -903,7 +904,8 @@ deleteSelfUser ::
Member PropertySubsystem r,
Member HashPassword r,
Member RateLimit r,
Member AuthenticationSubsystem r
Member AuthenticationSubsystem r,
Member UserGroupSubsystem r
) =>
Local UserId ->
Maybe PlainTextPassword6 ->
Expand Down Expand Up @@ -975,7 +977,8 @@ verifyDeleteUser ::
Member Events r,
Member UserSubsystem r,
Member PropertySubsystem r,
Member AuthenticationSubsystem r
Member AuthenticationSubsystem r,
Member UserGroupSubsystem r
) =>
VerifyDeleteUser ->
ExceptT DeleteUserError (AppT r) ()
Expand Down Expand Up @@ -1003,7 +1006,8 @@ ensureAccountDeleted ::
Member Events r,
Member UserSubsystem r,
Member PropertySubsystem r,
Member AuthenticationSubsystem r
Member AuthenticationSubsystem r,
Member UserGroupSubsystem r
) =>
Local UserId ->
AppT r DeleteUserResult
Expand Down Expand Up @@ -1052,7 +1056,8 @@ deleteAccount ::
Member PropertySubsystem r,
Member UserSubsystem r,
Member Events r,
Member AuthenticationSubsystem r
Member AuthenticationSubsystem r,
Member UserGroupSubsystem r
) =>
User ->
Sem r ()
Expand All @@ -1064,9 +1069,10 @@ deleteAccount user = do
for_ (userEmail user) $ deleteKeyForUser uid . mkEmailKey

PropertySubsystem.onUserDeleted uid

UserStore.deleteUser user

traverse_ (removeUserFromAllGroups uid) user.userTeam

Intra.rmUser uid (userAssets user)
embed $ Data.lookupClients uid >>= mapM_ (Data.rmClient uid . (.clientId))
luid <- embed $ qualifyLocal uid
Expand Down
4 changes: 3 additions & 1 deletion services/brig/src/Brig/InternalEvent/Process.hs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import Wire.Events (Events)
import Wire.NotificationSubsystem
import Wire.PropertySubsystem
import Wire.Sem.Delay
import Wire.UserGroupSubsystem
import Wire.UserKeyStore
import Wire.UserStore (UserStore)
import Wire.UserSubsystem
Expand All @@ -59,7 +60,8 @@ onEvent ::
Member PropertySubsystem r,
Member UserSubsystem r,
Member Events r,
Member AuthenticationSubsystem r
Member AuthenticationSubsystem r,
Member UserGroupSubsystem r
) =>
InternalNotification ->
Sem r ()
Expand Down