From 3c27f8c1ae095641a1fd991c079bfaa7285c2b57 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Thu, 3 Sep 2026 09:08:52 +0200 Subject: [PATCH 01/20] Changelog. --- .../WPB-27169-include-collaborators-in-contact-search | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/0-release-notes/WPB-27169-include-collaborators-in-contact-search diff --git a/changelog.d/0-release-notes/WPB-27169-include-collaborators-in-contact-search b/changelog.d/0-release-notes/WPB-27169-include-collaborators-in-contact-search new file mode 100644 index 0000000000..af10c6dd3c --- /dev/null +++ b/changelog.d/0-release-notes/WPB-27169-include-collaborators-in-contact-search @@ -0,0 +1 @@ +Include collaborators in contact search. From ea08d192dee4fbfbd30d3be7db6551cd5c4f36b0 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Sat, 29 Aug 2026 09:50:41 +0200 Subject: [PATCH 02/20] [WPB-28089] Treat team collaborators like team members in contact search (second attempt). (#5488) * Include team collaborators in contact search. * Implement TeamCollaboratorsSubsystem interpreter with BrigAPIAccess. Was previously UserSubsystem, but since it TeamCollaboratorsSubsystem is also used outside of Brig, that is not always available. Further changes: - Support BrigAPIAccess locally in Brig. - Change collaborator field type in UserDoc to collapse `Nothing` and `Just []` (remove the Maybe). * Fix brig-index: do not bump index version in failure case. * Add release notes on required postgres setup steps. --------- Co-authored-by: Gautier DI FOLCO --- ...rators-like-team-members-in-contact-search | 29 +++ ...rators-like-team-members-in-contact-search | 1 + integration/test/Test/TeamCollaborators.hs | 106 ++++++++ .../src/Wire/BrigAPIAccess/Local.hs | 65 +++++ .../src/Wire/BrigAPIAccess/Rpc.hs | 234 ++++++++++-------- .../IndexedUserStore/Bulk/ElasticSearch.hs | 61 +++-- .../Wire/IndexedUserStore/ElasticSearch.hs | 19 +- .../src/Wire/TeamCollaboratorsStore.hs | 2 + .../Wire/TeamCollaboratorsStore/Postgres.hs | 17 ++ .../TeamCollaboratorsSubsystem/Interpreter.hs | 36 ++- .../src/Wire/UserSearch/Migration.hs | 1 + .../src/Wire/UserSearch/Types.hs | 9 +- .../src/Wire/UserStore/IndexUser.hs | 8 +- .../src/Wire/UserSubsystem/Interpreter.hs | 20 +- .../test/unit/Wire/MiniBackend.hs | 9 +- .../test/unit/Wire/MockInterpreters.hs | 1 + .../Wire/MockInterpreters/BrigAPIAccess.hs | 81 ++++++ .../TeamCollaboratorsStore.hs | 2 + .../Wire/MockInterpreters/UserSubsystem.hs | 2 +- .../Wire/ScimSubsystem/InterpreterSpec.hs | 2 +- .../test/unit/Wire/UserSearch/TypesSpec.hs | 4 +- .../Wire/UserSubsystem/InterpreterSpec.hs | 2 +- libs/wire-subsystems/wire-subsystems.cabal | 2 + .../background-worker/src/Wire/Effects.hs | 2 +- services/brig/src/Brig/App.hs | 6 + .../brig/src/Brig/CanonicalInterpreter.hs | 28 ++- services/brig/src/Brig/Index/Eval.hs | 20 +- services/brig/src/Brig/User/Search/Index.hs | 9 + services/galley/src/Galley/App.hs | 2 +- 29 files changed, 610 insertions(+), 170 deletions(-) create mode 100644 changelog.d/0-release-notes/WPB-28089-treat-team-collaborators-like-team-members-in-contact-search create mode 100644 changelog.d/1-api-changes/WPB-28089-treat-team-collaborators-like-team-members-in-contact-search create mode 100644 libs/wire-subsystems/src/Wire/BrigAPIAccess/Local.hs create mode 100644 libs/wire-subsystems/test/unit/Wire/MockInterpreters/BrigAPIAccess.hs diff --git a/changelog.d/0-release-notes/WPB-28089-treat-team-collaborators-like-team-members-in-contact-search b/changelog.d/0-release-notes/WPB-28089-treat-team-collaborators-like-team-members-in-contact-search new file mode 100644 index 0000000000..3caefeb19a --- /dev/null +++ b/changelog.d/0-release-notes/WPB-28089-treat-team-collaborators-like-team-members-in-contact-search @@ -0,0 +1,29 @@ +`GET /contacts/search` returns apps (and regular users) that collaborate with the searcher's team. + +This means that `brig-index-migrate-data` now requires you to +configure the `elasticsearch-index` chart's postgres setup, and have a +postgres instance reachable with that setup, eg., like this: + +``` +# in (charts/elasticsearch-index/)values.yaml +postgresql: + host: postgresql # DNS name without protocol + port: "5432" + user: wire-server + dbname: wire-server +postgresqlPool: + size: 100 + acquisitionTimeout: 10s + idlenessTimeout: 10m + +postgresMigration: + user: cassandra # (or postgresql, migration-to-postgresql, ...) +``` + +Notes: +- If you have experienced any elasticsearch index update issues since 2026-03-24 (Chart Release 5.29.0), this might be related. If you have not resolved them, consider updating your `values.yaml` now and running a full re-index. +- Note: `brig-index` understands `--user-storage-location`, but that is not relevant here, because collaborators are not technically user accounts, but pointers to user accounts, and they are stored in a different table. + +More info: +- [configuring postgres](https://docs.wire.com/latest/developer/reference/config-options.html?h=config#configure-postgresql) +- [maintaining elastic search](https://docs.wire.com/latest/developer/reference/elastic-search.html?h=elasticsearch) diff --git a/changelog.d/1-api-changes/WPB-28089-treat-team-collaborators-like-team-members-in-contact-search b/changelog.d/1-api-changes/WPB-28089-treat-team-collaborators-like-team-members-in-contact-search new file mode 100644 index 0000000000..d66924aaee --- /dev/null +++ b/changelog.d/1-api-changes/WPB-28089-treat-team-collaborators-like-team-members-in-contact-search @@ -0,0 +1 @@ +Treat team collaborators like team members in contact search. diff --git a/integration/test/Test/TeamCollaborators.hs b/integration/test/Test/TeamCollaborators.hs index cf55c3a558..642dad537e 100644 --- a/integration/test/Test/TeamCollaborators.hs +++ b/integration/test/Test/TeamCollaborators.hs @@ -1,3 +1,5 @@ +{-# OPTIONS_GHC -Wno-ambiguous-fields #-} + -- This file is part of the Wire Server implementation. -- -- Copyright (C) 2025 Wire Swiss GmbH @@ -17,6 +19,9 @@ module Test.TeamCollaborators where +import qualified API.Brig as BrigP +import qualified API.BrigInternal as BrigI +import API.Common (randomName) import API.Galley import qualified API.GalleyInternal as Internal import Data.Tuple.Extra @@ -317,3 +322,104 @@ testUpdateCollaborator = do [] >>= assertSuccess postOne2OneConversation bob alice team "chit-chat" >>= assertLabel 403 "operation-denied" + +-- | Collaborators are part of the search space of the team they +-- collaborate with: `GET /search/contacts` returns them to members of +-- that team, just like it returns the team's own members. We test +-- collaborators from other teams, personal user accounts that +-- collaborate, and app. +testSearchFindsCollaborator :: (HasCallStack) => App () +testSearchFindsCollaborator = do + (owner, team, [alice]) <- createTeam OwnDomain 2 + (otherOwner, otherTeam, [bob, collab1]) <- createTeam OwnDomain 3 + collab2 :: Value <- randomUser OwnDomain def + collab3 :: Value <- + BrigP.createApp otherOwner otherTeam def + `bindResponse` \resp -> resp.json %. "user" + + collab1Name <- collab1 %. "name" & asString + collab2Name <- collab2 %. "name" & asString + collab3Name <- collab3 %. "name" & asString + + collab1Name' <- randomName + collab2Name' <- randomName + collab3Name' <- randomName + + -- Find before any collaborations have been established. + let assertFinds :: + (HasCallStack, MakesValue expectFound, MakesValue searcher) => + String -> + expectFound -> + searcher -> + App () + assertFinds searchTerm expectFound searcher = do + BrigI.refreshIndex OwnDomain + BrigP.searchContacts searcher searchTerm OwnDomain `bindResponse` \resp -> do + resp.status `shouldMatchInt` 200 + foundIds :: [String] <- resp.json %. "documents" >>= asList >>= mapM objId + expectedIds :: [String] <- (make >=> asList >=> mapM objId) expectFound + assertBool + ("found: " <> show foundIds <> "; expected: " <> show expectedIds) + (sort foundIds == sort expectedIds) + + for_ [owner, alice] $ assertFinds collab1Name ([] @Value) + for_ [otherOwner, bob] $ assertFinds collab1Name [collab1] + + for_ [owner, alice] $ assertFinds collab2Name [collab2] + for_ [otherOwner, bob] $ assertFinds collab2Name [collab2] + + for_ [owner, alice] $ assertFinds collab3Name ([] @Value) + for_ [otherOwner, bob] $ assertFinds collab3Name [collab3] + + -- Add collaborators to team + for_ [collab1, collab2, collab3] + $ \collab -> + addTeamCollaborator owner team collab ["implicit_connection"] >>= assertSuccess + + for_ [owner, alice] $ assertFinds collab1Name [collab1] + for_ [otherOwner, bob] $ assertFinds collab1Name [collab1] + + for_ [owner, alice] $ assertFinds collab2Name [collab2] + for_ [otherOwner, bob] $ assertFinds collab2Name [collab2] + + for_ [owner, alice] $ assertFinds collab3Name [collab3] + for_ [otherOwner, bob] $ assertFinds collab3Name [collab3] + + -- Check that updating name does not erase collaborating teams in index. + for_ [(collab1, collab1Name'), (collab2, collab2Name'), (collab3, collab3Name')] + $ \(collab, newName) -> do + let updateBody = (def :: BrigP.PutSelf) {BrigP.name = Just newName} + in BrigP.putSelf collab updateBody >>= assertSuccess + + for_ [owner, alice] $ assertFinds collab1Name' [collab1] + for_ [otherOwner, bob] $ assertFinds collab1Name' [collab1] + + for_ [owner, alice] $ assertFinds collab2Name' [collab2] + for_ [otherOwner, bob] $ assertFinds collab2Name' [collab2] + + for_ [owner, alice] $ assertFinds collab3Name' [collab3] + for_ [otherOwner, bob] $ assertFinds collab3Name' [collab3] + + -- Check that updating collaborating teams does not erase name in index. + for_ [collab1, collab2, collab3] + $ \collab -> do + removeTeamCollaborator owner team collab >>= assertSuccess + + for_ [owner, alice] $ assertFinds collab1Name' ([] @Value) + for_ [otherOwner, bob] $ assertFinds collab1Name' [collab1] + + for_ [owner, alice] $ assertFinds collab2Name' [collab2] + for_ [otherOwner, bob] $ assertFinds collab2Name' [collab2] + + for_ [owner, alice] $ assertFinds collab3Name' ([] @Value) + for_ [otherOwner, bob] $ assertFinds collab3Name' [collab3] + + -- Can one user collaborate in multiple teams without breaking search? + (_thirdOwner, _thirdTeam, [multiCollab]) <- createTeam OwnDomain 2 + multiCollabName <- multiCollab %. "name" & asString + + addTeamCollaborator owner team multiCollab ["implicit_connection"] >>= assertSuccess + addTeamCollaborator otherOwner otherTeam multiCollab ["implicit_connection"] >>= assertSuccess + + for_ [owner, alice] $ assertFinds multiCollabName [multiCollab] + for_ [otherOwner, bob] $ assertFinds multiCollabName [multiCollab] diff --git a/libs/wire-subsystems/src/Wire/BrigAPIAccess/Local.hs b/libs/wire-subsystems/src/Wire/BrigAPIAccess/Local.hs new file mode 100644 index 0000000000..d89904b0a8 --- /dev/null +++ b/libs/wire-subsystems/src/Wire/BrigAPIAccess/Local.hs @@ -0,0 +1,65 @@ +-- This file is part of the Wire Server implementation. +-- +-- Copyright (C) 2026 Wire Swiss GmbH +-- +-- This program is free software: you can redistribute it and/or modify it under +-- the terms of the GNU Affero General Public License as published by the Free +-- Software Foundation, either version 3 of the License, or (at your option) any +-- later version. +-- +-- This program is distributed in the hope that it will be useful, but WITHOUT +-- ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS +-- FOR A PARTICULAR PURPOSE. See the GNU Affero General Public License for more +-- details. +-- +-- You should have received a copy of the GNU Affero General Public License along +-- with this program. If not, see . + +-- | Interprets 'BrigAPIAccess' from within brig itself, by calling into the local +-- subsystems directly instead of round-tripping over HTTP to itself (as +-- 'Wire.BrigAPIAccess.Rpc.interpretBrigAccess' does for every other service). +-- +-- Only the operations needed by code shared with other services (e.g. +-- 'Wire.TeamCollaboratorsSubsystem') are implemented locally. Everything else +-- falls back to the RPC handler, pointed at brig itself: correct, but a wasteful +-- round-trip through our own listen socket, so it logs a warning and should be +-- given a local implementation once something actually relies on it. +module Wire.BrigAPIAccess.Local where + +import Imports +import Polysemy +import Polysemy.Error (Error) +import Polysemy.Input (runInputConst) +import Polysemy.TinyLog (TinyLog) +import Polysemy.TinyLog qualified as Log +import System.Logger.Message qualified as Log +import Util.Options (Endpoint) +import Wire.BrigAPIAccess +import Wire.BrigAPIAccess.Rpc (brigAccessRpcHandler) +import Wire.ParseException (ParseException) +import Wire.Rpc (Rpc) +import Wire.RpcException (RpcException) +import Wire.UserSubsystem (UserSubsystem) +import Wire.UserSubsystem qualified as UserSubsystem + +-- | The 'Endpoint' is brig's own; it is only used for the operations that have +-- no local implementation yet. +interpretBrigAPIAccessLocally :: + forall r. + ( Member TinyLog r, + Member Rpc r, + Member (Error ParseException) r, + Member (Error RpcException) r + ) => + Endpoint -> + InterpreterFor UserSubsystem r -> + InterpreterFor BrigAPIAccess r +interpretBrigAPIAccessLocally selfEndpoint runUser = interpret $ \case + UpdateSearchIndex uid -> runUser (UserSubsystem.internalUpdateSearchIndex uid) + other -> selfRpc other + where + selfRpc :: forall m x. BrigAPIAccess m x -> Sem r x + selfRpc action = do + Log.warn $ + Log.msg (Log.val "BrigAPIAccess.Local: no local implementation, calling brig over HTTP") + runInputConst selfEndpoint (brigAccessRpcHandler action) diff --git a/libs/wire-subsystems/src/Wire/BrigAPIAccess/Rpc.hs b/libs/wire-subsystems/src/Wire/BrigAPIAccess/Rpc.hs index e42a479139..b96d0abeea 100644 --- a/libs/wire-subsystems/src/Wire/BrigAPIAccess/Rpc.hs +++ b/libs/wire-subsystems/src/Wire/BrigAPIAccess/Rpc.hs @@ -82,115 +82,131 @@ interpretBrigAccess :: Sem (BrigAPIAccess ': r) a -> Sem r a interpretBrigAccess brigEndpoint = - interpret $ - runInputConst brigEndpoint . \case - GetConnectionsUnqualified uids muids mrel -> do - getConnectionsUnqualified uids muids mrel - GetConnections uids mquids mrel -> do - getConnections uids mquids mrel - PutConnectionInternal uc -> do - putConnectionInternal uc - ReauthUser uid reauth -> do - reAuthUser uid reauth - LookupActivatedUsers uids -> do - lookupActivatedUsers uids - GetUsers uids -> do - getUsers uids - DeleteUser uid -> do - deleteUser uid - GetContactList uid -> do - getContactList uid - GetUserExportData uid -> do - getUserExportData uid - GetSize tid -> do - getSize tid - LookupClients uids -> do - lookupClients uids - LookupClientsFull uids -> do - lookupClientsFull uids - NotifyClientsAboutLegalHoldRequest self other pk -> do - notifyClientsAboutLegalHoldRequest self other pk - GetLegalHoldAuthToken uid mpwd -> do - getLegalHoldAuthToken uid mpwd - AddLegalHoldClientToUserEither uid conn pks lpk -> do - addLegalHoldClientToUser uid conn pks lpk - RemoveLegalHoldClientFromUser uid -> do - removeLegalHoldClientFromUser uid - GetAccountConferenceCallingConfigClient uid -> do - getAccountConferenceCallingConfigClient uid - GetLocalMLSClients qusr ss -> do - getLocalMLSClients qusr ss - GetLocalMLSClient qusr cid ss -> do - getLocalMLSClient qusr cid ss - UpdateSearchVisibilityInbound status -> do - updateSearchVisibilityInbound status - DeleteBot convId botId -> - deleteBot convId botId - UpdateSearchIndex uid -> updateSearchIndex uid - GetAccountsBy localGetBy -> - getAccountsBy localGetBy - GetUsersByVariousKeys uids handles emails includePendingInvitations -> - getUsersByVariousKeys uids handles emails includePendingInvitations - CreateGroupInternal managedBy teamId creatorUserId newGroup -> - createGroupInternal managedBy teamId creatorUserId newGroup - GetGroupsInternal tid mbFilter mbManagedBy startIndex mbCount -> - getGroupsInternal tid mbFilter mbManagedBy startIndex mbCount - GetGroupInternal tid gid includeChannels -> - getGroupInternal tid gid includeChannels - UpdateGroup req -> - updateGroup req - DeleteGroupInternal managedBy teamId groupId -> - deleteGroupInternal managedBy teamId groupId - GetAppIdsForTeam teamId -> - getAppIdsForTeam teamId - SetAccountStatus uid status -> - setAccountStatus uid status - DeleteApp teamId uid -> - deleteApp teamId uid - CreateSAML uref buid teamid name managedBy handle richInfo mLocale role -> - createSAML uref buid teamid name managedBy handle richInfo mLocale role - CreateNoSAML extId email uid teamid uname locale role -> - createNoSAML extId email uid teamid uname locale role - UpdateEmail uid email activation -> - updateEmail uid email activation - GetAccount havePending uid -> - getAccount havePending uid - GetAccountByHandle handle -> - getByHandle handle - GetByEmail email -> - getByEmail email - SetName uid name -> - setName uid name - SetHandle uid handle -> - setHandle uid handle - SetManagedBy uid managedBy -> - setManagedBy uid managedBy - DeletePendingEmailUpdate uid -> - deletePendingEmailUpdate uid - SetSSOId uid ssoId -> - setSSOId uid ssoId - SetRichInfo uid richInfo -> - setRichInfo uid richInfo - SetLocale uid mLocale -> - setLocale uid mLocale - GetRichInfo uid -> - getRichInfo uid - CheckHandleAvailable handle -> - checkHandleAvailable handle - SsoLogin uid mLabel -> - ssoLogin uid mLabel - GetStatus uid -> - getStatus uid - GetStatusMaybe uid -> - getStatusMaybe uid - SetStatus uid status -> - setStatus uid status - GetDefaultUserLocale -> - getDefaultUserLocale - CheckAdminGetTeamId uid -> - checkAdminGetTeamId uid - SendSAMLIdPChangedEmail notif -> - sendSAMLIdPChangedEmail notif + interpret $ runInputConst brigEndpoint . brigAccessRpcHandler + +-- | Handles a single 'BrigAPIAccess' action by calling brig over HTTP. +-- +-- Exposed separately from 'interpretBrigAccess' so that +-- 'Wire.BrigAPIAccess.Local.interpretBrigAPIAccessLocally' can delegate the +-- actions it does not implement itself. 'BrigAPIAccess' is a first-order +-- effect, so @m@ is unconstrained and any handler's action can be passed here. +brigAccessRpcHandler :: + ( Member TinyLog r, + Member Rpc r, + Member (Error ParseException) r, + Member (Error RpcException) r, + Member (Input Endpoint) r + ) => + BrigAPIAccess m a -> + Sem r a +brigAccessRpcHandler = \case + GetConnectionsUnqualified uids muids mrel -> do + getConnectionsUnqualified uids muids mrel + GetConnections uids mquids mrel -> do + getConnections uids mquids mrel + PutConnectionInternal uc -> do + putConnectionInternal uc + ReauthUser uid reauth -> do + reAuthUser uid reauth + LookupActivatedUsers uids -> do + lookupActivatedUsers uids + GetUsers uids -> do + getUsers uids + DeleteUser uid -> do + deleteUser uid + GetContactList uid -> do + getContactList uid + GetUserExportData uid -> do + getUserExportData uid + GetSize tid -> do + getSize tid + LookupClients uids -> do + lookupClients uids + LookupClientsFull uids -> do + lookupClientsFull uids + NotifyClientsAboutLegalHoldRequest self other pk -> do + notifyClientsAboutLegalHoldRequest self other pk + GetLegalHoldAuthToken uid mpwd -> do + getLegalHoldAuthToken uid mpwd + AddLegalHoldClientToUserEither uid conn pks lpk -> do + addLegalHoldClientToUser uid conn pks lpk + RemoveLegalHoldClientFromUser uid -> do + removeLegalHoldClientFromUser uid + GetAccountConferenceCallingConfigClient uid -> do + getAccountConferenceCallingConfigClient uid + GetLocalMLSClients qusr ss -> do + getLocalMLSClients qusr ss + GetLocalMLSClient qusr cid ss -> do + getLocalMLSClient qusr cid ss + UpdateSearchVisibilityInbound status -> do + updateSearchVisibilityInbound status + DeleteBot convId botId -> + deleteBot convId botId + UpdateSearchIndex uid -> updateSearchIndex uid + GetAccountsBy localGetBy -> + getAccountsBy localGetBy + GetUsersByVariousKeys uids handles emails includePendingInvitations -> + getUsersByVariousKeys uids handles emails includePendingInvitations + CreateGroupInternal managedBy teamId creatorUserId newGroup -> + createGroupInternal managedBy teamId creatorUserId newGroup + GetGroupsInternal tid mbFilter mbManagedBy startIndex mbCount -> + getGroupsInternal tid mbFilter mbManagedBy startIndex mbCount + GetGroupInternal tid gid includeChannels -> + getGroupInternal tid gid includeChannels + UpdateGroup req -> + updateGroup req + DeleteGroupInternal managedBy teamId groupId -> + deleteGroupInternal managedBy teamId groupId + GetAppIdsForTeam teamId -> + getAppIdsForTeam teamId + SetAccountStatus uid status -> + setAccountStatus uid status + DeleteApp teamId uid -> + deleteApp teamId uid + CreateSAML uref buid teamid name managedBy handle richInfo mLocale role -> + createSAML uref buid teamid name managedBy handle richInfo mLocale role + CreateNoSAML extId email uid teamid uname locale role -> + createNoSAML extId email uid teamid uname locale role + UpdateEmail uid email activation -> + updateEmail uid email activation + GetAccount havePending uid -> + getAccount havePending uid + GetAccountByHandle handle -> + getByHandle handle + GetByEmail email -> + getByEmail email + SetName uid name -> + setName uid name + SetHandle uid handle -> + setHandle uid handle + SetManagedBy uid managedBy -> + setManagedBy uid managedBy + DeletePendingEmailUpdate uid -> + deletePendingEmailUpdate uid + SetSSOId uid ssoId -> + setSSOId uid ssoId + SetRichInfo uid richInfo -> + setRichInfo uid richInfo + SetLocale uid mLocale -> + setLocale uid mLocale + GetRichInfo uid -> + getRichInfo uid + CheckHandleAvailable handle -> + checkHandleAvailable handle + SsoLogin uid mLabel -> + ssoLogin uid mLabel + GetStatus uid -> + getStatus uid + GetStatusMaybe uid -> + getStatusMaybe uid + SetStatus uid status -> + setStatus uid status + GetDefaultUserLocale -> + getDefaultUserLocale + CheckAdminGetTeamId uid -> + checkAdminGetTeamId uid + SendSAMLIdPChangedEmail notif -> + sendSAMLIdPChangedEmail notif brigRequest :: (Member Rpc r, Member (Input Endpoint) r) => (Request -> Request) -> Sem r (Response (Maybe LByteString)) brigRequest req = do diff --git a/libs/wire-subsystems/src/Wire/IndexedUserStore/Bulk/ElasticSearch.hs b/libs/wire-subsystems/src/Wire/IndexedUserStore/Bulk/ElasticSearch.hs index 6317ed7ba2..31d7777901 100644 --- a/libs/wire-subsystems/src/Wire/IndexedUserStore/Bulk/ElasticSearch.hs +++ b/libs/wire-subsystems/src/Wire/IndexedUserStore/Bulk/ElasticSearch.hs @@ -29,6 +29,7 @@ import Data.Conduit.List qualified as CL import Data.Id import Data.Json.Util (UTCTimeMillis (fromUTCTimeMillis)) import Data.Map qualified as Map +import Data.Set qualified as Set import Database.Bloodhound qualified as ES import Imports import Polysemy @@ -37,6 +38,7 @@ import Polysemy.TinyLog import Polysemy.TinyLog qualified as Log import System.Logger.Message qualified as Log import UnliftIO (pooledForConcurrentlyN) +import Wire.API.Team.Collaborator (gTeam, gUser) import Wire.API.Team.Feature import Wire.API.Team.Member.Info import Wire.API.Team.Role @@ -45,6 +47,7 @@ import Wire.IndexedUserStore (IndexedUserStore) import Wire.IndexedUserStore qualified as IndexedUserStore import Wire.IndexedUserStore.MigrationStore import Wire.IndexedUserStore.MigrationStore qualified as MigrationStore +import Wire.TeamCollaboratorsStore (TeamCollaboratorsStore, getTeamCollaborationsForUsers) import Wire.UserSearch.Migration import Wire.UserSearch.Types import Wire.UserStore @@ -54,22 +57,28 @@ type IOInterpreter r = forall a. Sem r a -> IO a -- | Increase this number any time you want to force reindexing. expectedMigrationVersion :: MigrationVersion -expectedMigrationVersion = MigrationVersion 6 +expectedMigrationVersion = MigrationVersion 7 -syncAllUsers :: (Member UserStore r, Member IndexedUserStore r, Member TinyLog r, Member GalleyAPIAccess r) => IOInterpreter r -> Int32 -> IO () +syncAllUsers :: (Member UserStore r, Member IndexedUserStore r, Member TinyLog r, Member GalleyAPIAccess r, Member TeamCollaboratorsStore r) => IOInterpreter r -> Int32 -> IO Int syncAllUsers interpreter pageSize = syncAllUsersWithVersion interpreter pageSize ES.ExternalGT -forceSyncAllUsers :: (Member UserStore r, Member IndexedUserStore r, Member TinyLog r, Member GalleyAPIAccess r) => IOInterpreter r -> Int32 -> IO () +forceSyncAllUsers :: (Member UserStore r, Member IndexedUserStore r, Member TinyLog r, Member GalleyAPIAccess r, Member TeamCollaboratorsStore r) => IOInterpreter r -> Int32 -> IO Int forceSyncAllUsers interpreter pageSize = syncAllUsersWithVersion interpreter pageSize ES.ExternalGTE -syncAllUsersWithVersion :: (Member UserStore r, Member IndexedUserStore r, Member TinyLog r, Member GalleyAPIAccess r) => IOInterpreter r -> Int32 -> (ES.ExternalDocVersion -> ES.VersionControl) -> IO () +-- | Returns the number of users that could not be indexed because some of the +-- data needed to build their document was unavailable. Those users have been +-- logged individually by 'logAndHush'. +syncAllUsersWithVersion :: (Member UserStore r, Member IndexedUserStore r, Member TinyLog r, Member GalleyAPIAccess r, Member TeamCollaboratorsStore r) => IOInterpreter r -> Int32 -> (ES.ExternalDocVersion -> ES.VersionControl) -> IO Int syncAllUsersWithVersion interpreter pageSize mkVersion = - runConduit $ + fmap getSum . runConduit $ zipSources (CL.sourceList [1 ..]) (paginateWithStateC (interpreter . getIndexUsersPaginated pageSize)) .| logPage .| mkUserDocs - .| Conduit.mapM_ (interpreter . IndexedUserStore.bulkUpsert) + .| Conduit.foldMapM upsertPage where + upsertPage :: (Int, [(ES.DocId, UserDoc, ES.VersionControl)]) -> IO (Sum Int) + upsertPage (skipped, docs) = Sum skipped <$ interpreter (IndexedUserStore.bulkUpsert docs) + logPage :: ConduitT (Int32, [IndexUser]) [IndexUser] IO () logPage = Conduit.mapM $ \(pageNumber, page) -> do interpreter $ @@ -79,7 +88,9 @@ syncAllUsersWithVersion interpreter pageSize mkVersion = . Log.field "firstUser" (maybe "N/A" (idToText . (.userId)) (headMay page)) pure page - mkUserDocs :: ConduitT [IndexUser] [(ES.DocId, UserDoc, ES.VersionControl)] IO () + -- Emits the documents to be indexed together with the number of users of + -- this page that had to be skipped. + mkUserDocs :: ConduitT [IndexUser] (Int, [(ES.DocId, UserDoc, ES.VersionControl)]) IO () mkUserDocs = Conduit.mapM $ \page -> do -- FUTUREWORK: extract team visibilities, roles and user type -- more efficiently sending one query per page @@ -88,10 +99,10 @@ syncAllUsersWithVersion interpreter pageSize mkVersion = -- contains User, Maybe Role, UserType, ..., and pass around -- ExtendedUser. this should make the code less convoluted. - let teams :: Map TeamId [IndexUser] = Map.fromListWith (<>) $ mapMaybe (\u -> (,[u]) <$> u.teamId) page - teamIds = Map.keys teams + let teams :: Map TeamId [IndexUser] + teams = Map.fromListWith (<>) $ mapMaybe (\u -> (,[u]) <$> u.teamId) page - visMap <- fmap Map.fromList . pooledForConcurrentlyN 16 teamIds $ \t -> do + visMap <- fmap Map.fromList . pooledForConcurrentlyN 16 (Map.keys teams) $ \t -> do x <- try $ interpreter $ teamSearchVisibilityInbound t pure (t, x) @@ -114,6 +125,12 @@ syncAllUsersWithVersion interpreter pageSize mkVersion = fmap Map.unions . pooledForConcurrentlyN 16 (Map.toList teams) $ \(t, us) -> getRoles t (fmap (.userId) us) + -- One query for the whole page. A failure here fails every document of the + -- page, which 'logAndHush' then logs and skips. + eithCollabTeams :: Either SomeException (Map UserId [TeamId]) <- + try . fmap (Map.fromListWith (<>) . map (\tc -> (gUser tc, [gTeam tc]))) . interpreter $ + getTeamCollaborationsForUsers (Set.fromList (map (.userId) page)) + let vis :: IndexUser -> Either SomeException SearchVisibilityInbound vis indexUser = fromMaybe (Right defaultSearchVisibilityInbound) $ flip Map.lookup visMap =<< indexUser.teamId @@ -122,15 +139,20 @@ syncAllUsersWithVersion interpreter pageSize mkVersion = mkUserDoc indexUser = do currentVis <- vis indexUser currentRole <- sequence $ Map.lookup indexUser.userId roles - pure $ indexUserToDoc currentVis ((.value) <$> currentRole) indexUser + currentCollabTeams <- Map.findWithDefault [] indexUser.userId <$> eithCollabTeams + pure $ indexUserToDoc currentVis ((.value) <$> currentRole) currentCollabTeams indexUser mkDocVersion :: IndexUser -> Either SomeException ES.VersionControl mkDocVersion u = do roleWithTime <- sequence (Map.lookup u.userId roles) pure . mkVersion . ES.ExternalDocVersion . docVersion $ indexUserToVersion roleWithTime u - let docsWithErrors = map (\u -> (userIdToDocId u.userId, mkUserDoc u, mkDocVersion u)) page - interpreter . flip mapMaybeM docsWithErrors $ logAndHush + docsWithErrors :: (e ~ Either SomeException) => [(ES.DocId, e UserDoc, e ES.VersionControl)] + docsWithErrors = map (\u -> (userIdToDocId u.userId, mkUserDoc u, mkDocVersion u)) page + + docs <- interpreter . flip mapMaybeM docsWithErrors $ logAndHush + let skipped = length docsWithErrors - length docs + pure (skipped, docs) rightSecond :: (a, b) -> (a, Either c b) rightSecond (a, b) = (a, Right b) @@ -159,7 +181,7 @@ syncAllUsersWithVersion interpreter pageSize mkVersion = <$> permissionsToRole tmi.permissions migrateData :: - (Member (Embed IO) r, Member IndexedUserStore r, Member (Error MigrationException) r, Member IndexedUserMigrationStore r, Member TinyLog r, Member UserStore r, Member GalleyAPIAccess r) => + (Member (Embed IO) r, Member IndexedUserStore r, Member (Error MigrationException) r, Member IndexedUserMigrationStore r, Member TinyLog r, Member UserStore r, Member GalleyAPIAccess r, Member TeamCollaboratorsStore r) => IOInterpreter r -> Int32 -> IO () @@ -174,8 +196,15 @@ migrateData interpreter pageSize = interpreter $ do Log.msg (Log.val "Migration necessary.") . Log.field "expectedVersion" expectedMigrationVersion . Log.field "foundVersion" foundVersion - embed $ forceSyncAllUsers interpreter pageSize - MigrationStore.persistMigrationVersion expectedMigrationVersion + skipped <- embed $ forceSyncAllUsers interpreter pageSize + if skipped == 0 + then MigrationStore.persistMigrationVersion expectedMigrationVersion + else do + Log.err $ + Log.msg (Log.val "Migration incomplete, not persisting migration version.") + . Log.field "expectedVersion" expectedMigrationVersion + . Log.field "skippedUsers" skipped + throw $ SyncIncomplete else do Log.info $ Log.msg (Log.val "No migration necessary.") diff --git a/libs/wire-subsystems/src/Wire/IndexedUserStore/ElasticSearch.hs b/libs/wire-subsystems/src/Wire/IndexedUserStore/ElasticSearch.hs index 156f8f6e47..07572a2985 100644 --- a/libs/wire-subsystems/src/Wire/IndexedUserStore/ElasticSearch.hs +++ b/libs/wire-subsystems/src/Wire/IndexedUserStore/ElasticSearch.hs @@ -526,13 +526,17 @@ matchSelf :: UserId -> Maybe ES.Query matchSelf searcher = Just (termQ "_id" (idToText searcher)) -- | Exclude apps from other teams. --- Apps should only be searchable within their own team. +-- Apps should only be searchable within their own team, or within a team they +-- collaborate with. matchAppsFromOtherTeams :: Maybe TeamId -> Maybe ES.Query matchAppsFromOtherTeams mSearcherTeamId = Just $ ES.QueryBoolQuery boolQuery - { ES.boolQueryMustMatch = + { -- Apps collaborating with the searcher's team are not excluded. + ES.boolQueryMustNotMatch = + maybeToList (termQ "collaborating_teams" . idToText <$> mSearcherTeamId), + ES.boolQueryMustMatch = [ -- Match apps (type = "app") termQ "type" "app", -- That are from a different team than the searcher @@ -640,7 +644,16 @@ restrictSearchSpaceByUserType = \case else ES.TermsQuery "type" (userTypeFilterToText <$> (utsH :| utsT)) matchTeamMembersOf :: TeamId -> ES.Query -matchTeamMembersOf team = ES.TermQuery (ES.Term "team" $ idToText team) Nothing +matchTeamMembersOf team = + ES.QueryBoolQuery + boolQuery + { ES.boolQueryShouldMatch = + [ -- Match users who are members of the team + ES.TermQuery (ES.Term "team" $ idToText team) Nothing, + -- Match users who are collaborators in the team + ES.TermQuery (ES.Term "collaborating_teams" $ idToText team) Nothing + ] + } matchTeamMembersSearchableByAllTeams :: ES.Query matchTeamMembersSearchableByAllTeams = diff --git a/libs/wire-subsystems/src/Wire/TeamCollaboratorsStore.hs b/libs/wire-subsystems/src/Wire/TeamCollaboratorsStore.hs index fcdf0731b0..ebf79c9672 100644 --- a/libs/wire-subsystems/src/Wire/TeamCollaboratorsStore.hs +++ b/libs/wire-subsystems/src/Wire/TeamCollaboratorsStore.hs @@ -29,6 +29,8 @@ data TeamCollaboratorsStore m a where GetAllTeamCollaborators :: TeamId -> TeamCollaboratorsStore m [TeamCollaborator] GetTeamCollaborator :: TeamId -> UserId -> TeamCollaboratorsStore m (Maybe TeamCollaborator) GetTeamCollaborations :: UserId -> TeamCollaboratorsStore m ([TeamCollaborator]) + -- | Batched 'GetTeamCollaborations', for callers that process users in pages. + GetTeamCollaborationsForUsers :: Set UserId -> TeamCollaboratorsStore m [TeamCollaborator] GetTeamCollaboratorsWithIds :: Set TeamId -> Set UserId -> TeamCollaboratorsStore m [TeamCollaborator] UpdateTeamCollaborator :: UserId -> TeamId -> Set CollaboratorPermission -> TeamCollaboratorsStore m () RemoveTeamCollaborator :: UserId -> TeamId -> TeamCollaboratorsStore m () diff --git a/libs/wire-subsystems/src/Wire/TeamCollaboratorsStore/Postgres.hs b/libs/wire-subsystems/src/Wire/TeamCollaboratorsStore/Postgres.hs index a6a1e968a7..b898ae69b5 100644 --- a/libs/wire-subsystems/src/Wire/TeamCollaboratorsStore/Postgres.hs +++ b/libs/wire-subsystems/src/Wire/TeamCollaboratorsStore/Postgres.hs @@ -49,6 +49,7 @@ interpretTeamCollaboratorsStoreToPostgres = GetAllTeamCollaborators teamId -> getAllTeamCollaboratorsImpl teamId GetTeamCollaborator teamId userId -> getTeamCollaboratorImpl teamId userId GetTeamCollaborations userId -> getTeamCollaborationsImpl userId + GetTeamCollaborationsForUsers userIds -> getTeamCollaborationsForUsersImpl userIds GetTeamCollaboratorsWithIds teamIds userIds -> getTeamCollaboratorsWithIdsImpl teamIds userIds UpdateTeamCollaborator userId teamId permissions -> updateTeamCollaboratorImpl userId teamId permissions RemoveTeamCollaborator userId teamId -> removeTeamCollaboratorImpl userId teamId @@ -181,6 +182,22 @@ getTeamCollaborationsImpl teamId = do select user_id :: uuid, team_id :: uuid, permissions :: int2[] from collaborators where user_id = ($1 :: uuid) |] +getTeamCollaborationsForUsersImpl :: + (PGConstraints r) => + Set UserId -> + Sem r [TeamCollaborator] +getTeamCollaborationsForUsersImpl userIds = do + runStatement (Data.Set.toList userIds) getAllCollaborationsByUsersStatement + where + getAllCollaborationsByUsersStatement :: Statement [UserId] [TeamCollaborator] + getAllCollaborationsByUsersStatement = + dimap + (Data.Vector.fromList . Imports.map toUUID) + (Data.Vector.toList . (toTeamCollaborator <$>)) + $ [vectorStatement| + select user_id :: uuid, team_id :: uuid, permissions :: int2[] from collaborators where user_id = ANY($1 :: uuid[]) + |] + getTeamCollaboratorsWithIdsImpl :: (PGConstraints r) => Set TeamId -> diff --git a/libs/wire-subsystems/src/Wire/TeamCollaboratorsSubsystem/Interpreter.hs b/libs/wire-subsystems/src/Wire/TeamCollaboratorsSubsystem/Interpreter.hs index 30a970706e..bb0541636b 100644 --- a/libs/wire-subsystems/src/Wire/TeamCollaboratorsSubsystem/Interpreter.hs +++ b/libs/wire-subsystems/src/Wire/TeamCollaboratorsSubsystem/Interpreter.hs @@ -29,6 +29,8 @@ import Wire.API.Error.Brig qualified as E import Wire.API.Event.Team import Wire.API.Team.Collaborator import Wire.API.Team.Member qualified as TeamMember +import Wire.BrigAPIAccess (BrigAPIAccess) +import Wire.BrigAPIAccess qualified as BrigAPIAccess import Wire.Error import Wire.NotificationSubsystem import Wire.Sem.Now @@ -44,15 +46,18 @@ interpretTeamCollaboratorsSubsystem :: Member Now r, Member NotificationSubsystem r ) => + InterpreterFor BrigAPIAccess r -> InterpreterFor TeamCollaboratorsSubsystem r -interpretTeamCollaboratorsSubsystem = interpret $ \case - CreateTeamCollaborator zUser user team perms -> createTeamCollaboratorImpl zUser user team perms - GetAllTeamCollaborators zUser team -> getAllTeamCollaboratorsImpl zUser team - InternalGetTeamCollaborator team user -> internalGetTeamCollaboratorImpl team user - InternalGetTeamCollaborations userId -> internalGetTeamCollaborationsImpl userId - InternalGetTeamCollaboratorsWithIds teams userIds -> internalGetTeamCollaboratorsWithIdsImpl teams userIds - InternalUpdateTeamCollaborator user team perms -> internalUpdateTeamCollaboratorImpl user team perms - InternalRemoveTeamCollaborator user team -> internalRemoveTeamCollaboratorImpl user team +interpretTeamCollaboratorsSubsystem brigAPIAccess = + interpret $ + brigAPIAccess . \case + CreateTeamCollaborator zUser user team perms -> createTeamCollaboratorImpl zUser user team perms + GetAllTeamCollaborators zUser team -> getAllTeamCollaboratorsImpl zUser team + InternalGetTeamCollaborator team user -> internalGetTeamCollaboratorImpl team user + InternalGetTeamCollaborations userId -> internalGetTeamCollaborationsImpl userId + InternalGetTeamCollaboratorsWithIds teams userIds -> internalGetTeamCollaboratorsWithIdsImpl teams userIds + InternalUpdateTeamCollaborator user team perms -> internalUpdateTeamCollaboratorImpl user team perms + InternalRemoveTeamCollaborator user team -> internalRemoveTeamCollaboratorImpl user team internalGetTeamCollaboratorImpl :: (Member Store.TeamCollaboratorsStore r) => @@ -74,7 +79,8 @@ createTeamCollaboratorImpl :: Member (Error TeamCollaboratorsError) r, Member Store.TeamCollaboratorsStore r, Member Now r, - Member NotificationSubsystem r + Member NotificationSubsystem r, + Member BrigAPIAccess r ) => Local UserId -> UserId -> @@ -85,9 +91,11 @@ createTeamCollaboratorImpl zUser user team perms = do guardPermission (tUnqualified zUser) team TeamMember.GetTeamCollaborators InsufficientRights Store.createTeamCollaborator user team perms - -- TODO: Review the event's values generateTeamEvents (tUnqualified zUser) team [EdCollaboratorAdd user (Set.toList perms)] + -- Reindex the collaborator with their new collaboration team + BrigAPIAccess.updateSearchIndex user + getAllTeamCollaboratorsImpl :: ( Member TeamSubsystem r, Member (Error TeamCollaboratorsError) r, @@ -109,21 +117,25 @@ internalGetTeamCollaboratorsWithIdsImpl = do Store.getTeamCollaboratorsWithIds internalUpdateTeamCollaboratorImpl :: - (Member Store.TeamCollaboratorsStore r) => + (Member Store.TeamCollaboratorsStore r, Member BrigAPIAccess r) => UserId -> TeamId -> Set CollaboratorPermission -> Sem r () internalUpdateTeamCollaboratorImpl user team perms = do Store.updateTeamCollaborator user team perms + -- Reindex collaborator when permissions change + BrigAPIAccess.updateSearchIndex user internalRemoveTeamCollaboratorImpl :: - (Member Store.TeamCollaboratorsStore r) => + (Member Store.TeamCollaboratorsStore r, Member BrigAPIAccess r) => UserId -> TeamId -> Sem r () internalRemoveTeamCollaboratorImpl user team = do Store.removeTeamCollaborator user team + -- Reindex collaborator when removed + BrigAPIAccess.updateSearchIndex user -- This is of general usefulness. However, we cannot move this to wire-api as -- this would lead to a cyclic dependency. diff --git a/libs/wire-subsystems/src/Wire/UserSearch/Migration.hs b/libs/wire-subsystems/src/Wire/UserSearch/Migration.hs index 817d10370e..7cbbbcd734 100644 --- a/libs/wire-subsystems/src/Wire/UserSearch/Migration.hs +++ b/libs/wire-subsystems/src/Wire/UserSearch/Migration.hs @@ -42,6 +42,7 @@ data MigrationException | PutMappingFailed String | TargetIndexAbsent | VersionSourceMissing (ES.SearchResult MigrationVersion) + | SyncIncomplete deriving (Show) instance Exception MigrationException diff --git a/libs/wire-subsystems/src/Wire/UserSearch/Types.hs b/libs/wire-subsystems/src/Wire/UserSearch/Types.hs index 5e8dcac765..5464dae2a8 100644 --- a/libs/wire-subsystems/src/Wire/UserSearch/Types.hs +++ b/libs/wire-subsystems/src/Wire/UserSearch/Types.hs @@ -79,7 +79,10 @@ data UserDoc = UserDoc udScimExternalId :: Maybe Text, udSso :: Maybe Sso, udEmailUnvalidated :: Maybe EmailAddress, - udSearchable :: Maybe Bool + udSearchable :: Maybe Bool, + -- | Teams that have added this user as a collaborator. + -- Updated separately via 'syncUserIndexCollaborations' when collaborator relationships change. + udCollaboratingTeams :: [TeamId] } deriving (Eq, Show, Generic) deriving (Arbitrary) via (GenericUniform UserDoc) @@ -104,7 +107,8 @@ instance ToJSON UserDoc where "scim_external_id" .= udScimExternalId ud, "sso" .= udSso ud, "email_unvalidated" .= udEmailUnvalidated ud, - "searchable" .= udSearchable ud + "searchable" .= udSearchable ud, + "collaborating_teams" .= udCollaboratingTeams ud ] instance FromJSON UserDoc where @@ -128,6 +132,7 @@ instance FromJSON UserDoc where <*> o .:? "sso" <*> o .:? "email_unvalidated" <*> o .:? "searchable" + <*> o .:? "collaborating_teams" .!= [] searchVisibilityInboundFieldName :: Key searchVisibilityInboundFieldName = "search_visibility_inbound" diff --git a/libs/wire-subsystems/src/Wire/UserStore/IndexUser.hs b/libs/wire-subsystems/src/Wire/UserStore/IndexUser.hs index 09ac630d19..b051132f1e 100644 --- a/libs/wire-subsystems/src/Wire/UserStore/IndexUser.hs +++ b/libs/wire-subsystems/src/Wire/UserStore/IndexUser.hs @@ -126,8 +126,8 @@ indexUserToVersion :: Maybe (WithWritetime Role) -> IndexUser -> IndexVersion indexUserToVersion role iu = mkIndexVersion [Just $ Writetime iu.updatedAt, const () <$$> fmap writetime role] -indexUserToDoc :: SearchVisibilityInbound -> Maybe Role -> IndexUser -> UserDoc -indexUserToDoc searchVisInbound mRole IndexUser {..} = +indexUserToDoc :: SearchVisibilityInbound -> Maybe Role -> [TeamId] -> IndexUser -> UserDoc +indexUserToDoc searchVisInbound mRole collaboratingTeams IndexUser {..} = if shouldIndex then UserDoc @@ -148,7 +148,8 @@ indexUserToDoc searchVisInbound mRole IndexUser {..} = udHandle = handle, udNormalized = Just $ normalized name.fromName, udName = Just name, - udTeam = teamId + udTeam = teamId, + udCollaboratingTeams = collaboratingTeams } else -- We insert a tombstone-style user here, as it's easier than -- deleting the old one. It's mostly empty, but having the status here @@ -209,5 +210,6 @@ emptyUserDoc uid = udNormalized = Nothing, udName = Nothing, udTeam = Nothing, + udCollaboratingTeams = [], udId = uid } diff --git a/libs/wire-subsystems/src/Wire/UserSubsystem/Interpreter.hs b/libs/wire-subsystems/src/Wire/UserSubsystem/Interpreter.hs index d5cb2dfee6..f78029cebd 100644 --- a/libs/wire-subsystems/src/Wire/UserSubsystem/Interpreter.hs +++ b/libs/wire-subsystems/src/Wire/UserSubsystem/Interpreter.hs @@ -60,6 +60,7 @@ import Wire.API.Federation.Error import Wire.API.MLS.CipherSuite (CipherSuiteTag, csSignatureScheme) import Wire.API.Routes.FederationDomainConfig import Wire.API.Routes.Internal.Galley.TeamFeatureNoConfigMulti (TeamStatus (..)) +import Wire.API.Team.Collaborator (gTeam) import Wire.API.Team.Export import Wire.API.Team.Feature import Wire.API.Team.Member @@ -102,6 +103,8 @@ import Wire.Sem.Metrics qualified as Metrics import Wire.Sem.Now (Now) import Wire.Sem.Now qualified as Now import Wire.StoredUser +import Wire.TeamCollaboratorsStore (TeamCollaboratorsStore) +import Wire.TeamCollaboratorsStore qualified as TeamCollaboratorsStore import Wire.TeamSubsystem import Wire.UserGroupStore (UserGroupStore, getUserGroupIdsForUsers) import Wire.UserKeyStore @@ -141,6 +144,7 @@ runUserSubsystem :: Member TinyLog r, Member (Input UserSubsystemConfig) r, Member TeamSubsystem r, + Member TeamCollaboratorsStore r, Member UserGroupStore r, Member (Input (Local any)) r ) => @@ -711,6 +715,7 @@ updateUserProfileImpl :: Member Events r, Member GalleyAPIAccess r, Member IndexedUserStore r, + Member TeamCollaboratorsStore r, Member Metrics r ) => Local UserId -> @@ -772,6 +777,7 @@ updateHandleImpl :: Member Events r, Member UserStore r, Member IndexedUserStore r, + Member TeamCollaboratorsStore r, Member Metrics r ) => Local UserId -> @@ -839,7 +845,8 @@ syncUserIndex :: ( Member UserStore r, Member GalleyAPIAccess r, Member IndexedUserStore r, - Member Metrics r + Member Metrics r, + Member TeamCollaboratorsStore r ) => UserId -> Sem r () @@ -860,9 +867,13 @@ syncUserIndex uid = teamSearchVisibilityInbound indexUser.teamId tm <- maybe (pure Nothing) selectTeamMember indexUser.teamId + collabTeams <- map gTeam <$> TeamCollaboratorsStore.getTeamCollaborations uid let mRole = tm >>= mkRoleWithWriteTime - userDoc = indexUserToDoc vis (value <$> mRole) indexUser - version = ES.ExternalGT . ES.ExternalDocVersion . docVersion $ indexUserToVersion mRole indexUser + userDoc = indexUserToDoc vis (value <$> mRole) collabTeams indexUser + -- GTE, not GT: the version comes from the user row alone, but the document also + -- holds data that changes without touching that row (collaborations), and under + -- GT those updates would be dropped as version conflicts. Older writes still lose. + version = ES.ExternalGTE . ES.ExternalDocVersion . docVersion $ indexUserToVersion mRole indexUser Metrics.incCounter indexUpdateCounter IndexedUserStore.upsert (userIdToDocId uid) userDoc version @@ -1180,6 +1191,7 @@ acceptTeamInvitationImpl :: Member (Error UserSubsystemError) r, Member InvitationStore r, Member IndexedUserStore r, + Member TeamCollaboratorsStore r, Member Metrics r, Member Events r, Member AuthenticationSubsystem r, @@ -1244,6 +1256,7 @@ removeEmailEitherImpl :: Member UserStore r, Member Events r, Member IndexedUserStore r, + Member TeamCollaboratorsStore r, Member (Input UserSubsystemConfig) r, Member GalleyAPIAccess r, Member Metrics r @@ -1280,6 +1293,7 @@ setUserSearchableImpl :: Member TeamSubsystem r, Member GalleyAPIAccess r, Member IndexedUserStore r, + Member TeamCollaboratorsStore r, Member Metrics r ) => Local UserId -> diff --git a/libs/wire-subsystems/test/unit/Wire/MiniBackend.hs b/libs/wire-subsystems/test/unit/Wire/MiniBackend.hs index 1324d919db..ff27a27fa7 100644 --- a/libs/wire-subsystems/test/unit/Wire/MiniBackend.hs +++ b/libs/wire-subsystems/test/unit/Wire/MiniBackend.hs @@ -365,22 +365,21 @@ miniBackendLowerEffectsInterpreters mb@(MiniBackendParams {..}) = . runInputConst conversationCfg . runClientSubsystem undefined undefined where - -- Mock BrigAPIAccess interpreter for tests - mockBrigAPIAccess :: forall r'. InterpreterFor BrigAPIAccess r' - mockBrigAPIAccess = interpret $ \case - _ -> error "Unimplemented BrigAPIAccess operation in mock" -- Mock UserClientIndexStore interpreter for tests mockUserClientIndexStore :: forall r'. InterpreterFor UserClientIndexStore r' mockUserClientIndexStore = interpret $ \case _ -> error "Unimplemented UserClientIndexStore operation in mock" + -- Mock BackendNotificationQueueAccess interpreter for tests mockBackendNotificationQueueAccess :: forall r'. InterpreterFor BackendNotificationQueueAccess r' mockBackendNotificationQueueAccess = interpret $ \case _ -> error "Unimplemented BackendNotificationQueueAccess operation in mock" + -- Mock ConversationSubsystem interpreter for tests mockConversationSubsystem :: forall r'. InterpreterFor ConversationSubsystem r' mockConversationSubsystem = interpretH $ \case _ -> error "Unimplemented ConversationSubsystem operation in mock" + mockMlsKeyPackageSubsystem :: forall r'. InterpreterFor MlsKeyPackageSubsystem r' mockMlsKeyPackageSubsystem = interpret $ \case HasMlsKeyPackages {} -> pure False @@ -786,7 +785,7 @@ interpretMaybeFederationStackState :: Sem (MiniBackendEffects `Append` r) a -> Sem r (MiniBackend, a) interpretMaybeFederationStackState mb = - miniBackendLowerEffectsInterpreters mb . interpretTeamCollaboratorsSubsystem . runRecursiveAuthUserApp + miniBackendLowerEffectsInterpreters mb . interpretTeamCollaboratorsSubsystem subsume . runRecursiveAuthUserApp -- FUTUREWORK(fisx): it would be nice to have a definition of an -- interpreter of all the subsystems combined, but since the diff --git a/libs/wire-subsystems/test/unit/Wire/MockInterpreters.hs b/libs/wire-subsystems/test/unit/Wire/MockInterpreters.hs index 4630c0c7f7..ea57140aad 100644 --- a/libs/wire-subsystems/test/unit/Wire/MockInterpreters.hs +++ b/libs/wire-subsystems/test/unit/Wire/MockInterpreters.hs @@ -25,6 +25,7 @@ import Wire.MockInterpreters.AppStore as MockInterpreters import Wire.MockInterpreters.AuthenticationSubsystem as MockInterpreters import Wire.MockInterpreters.BackgroundJobPublisher as MockInterpreters import Wire.MockInterpreters.BlockListStore as MockInterpreters +import Wire.MockInterpreters.BrigAPIAccess as MockInterpreters import Wire.MockInterpreters.ClientStore as MockInterpreters import Wire.MockInterpreters.ConversationStore as MockInterpreters import Wire.MockInterpreters.ConversationSubsystem as MockInterpreters diff --git a/libs/wire-subsystems/test/unit/Wire/MockInterpreters/BrigAPIAccess.hs b/libs/wire-subsystems/test/unit/Wire/MockInterpreters/BrigAPIAccess.hs new file mode 100644 index 0000000000..aeb7629909 --- /dev/null +++ b/libs/wire-subsystems/test/unit/Wire/MockInterpreters/BrigAPIAccess.hs @@ -0,0 +1,81 @@ +-- This file is part of the Wire Server implementation. +-- +-- Copyright (C) 2026 Wire Swiss GmbH +-- +-- This program is free software: you can redistribute it and/or modify it under +-- the terms of the GNU Affero General Public License as published by the Free +-- Software Foundation, either version 3 of the License, or (at your option) any +-- later version. +-- +-- This program is distributed in the hope that it will be useful, but WITHOUT +-- ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS +-- FOR A PARTICULAR PURPOSE. See the GNU Affero General Public License for more +-- details. +-- +-- You should have received a copy of the GNU Affero General Public License along +-- with this program. If not, see . + +module Wire.MockInterpreters.BrigAPIAccess where + +import Imports +import Polysemy +import Wire.BrigAPIAccess + +-- | Errors out on everything except 'UpdateSearchIndex', which is a no-op. +mockBrigAPIAccess :: InterpreterFor BrigAPIAccess r +mockBrigAPIAccess = interpret $ \case + UpdateSearchIndex _ -> pure () + -- everything else is not implemented + GetConnectionsUnqualified {} -> error "GetConnectionsUnqualified: implement on demand (mockBrigAPIAccess)" + GetConnections {} -> error "GetConnections: implement on demand (mockBrigAPIAccess)" + PutConnectionInternal {} -> error "PutConnectionInternal: implement on demand (mockBrigAPIAccess)" + ReauthUser {} -> error "ReauthUser: implement on demand (mockBrigAPIAccess)" + LookupActivatedUsers {} -> error "LookupActivatedUsers: implement on demand (mockBrigAPIAccess)" + GetUsers {} -> error "GetUsers: implement on demand (mockBrigAPIAccess)" + DeleteUser {} -> error "DeleteUser: implement on demand (mockBrigAPIAccess)" + GetContactList {} -> error "GetContactList: implement on demand (mockBrigAPIAccess)" + GetSize {} -> error "GetSize: implement on demand (mockBrigAPIAccess)" + LookupClients {} -> error "LookupClients: implement on demand (mockBrigAPIAccess)" + LookupClientsFull {} -> error "LookupClientsFull: implement on demand (mockBrigAPIAccess)" + NotifyClientsAboutLegalHoldRequest {} -> error "NotifyClientsAboutLegalHoldRequest: implement on demand (mockBrigAPIAccess)" + GetLegalHoldAuthToken {} -> error "GetLegalHoldAuthToken: implement on demand (mockBrigAPIAccess)" + AddLegalHoldClientToUserEither {} -> error "AddLegalHoldClientToUserEither: implement on demand (mockBrigAPIAccess)" + RemoveLegalHoldClientFromUser {} -> error "RemoveLegalHoldClientFromUser: implement on demand (mockBrigAPIAccess)" + GetAccountConferenceCallingConfigClient {} -> error "GetAccountConferenceCallingConfigClient: implement on demand (mockBrigAPIAccess)" + GetLocalMLSClients {} -> error "GetLocalMLSClients: implement on demand (mockBrigAPIAccess)" + GetLocalMLSClient {} -> error "GetLocalMLSClient: implement on demand (mockBrigAPIAccess)" + UpdateSearchVisibilityInbound {} -> error "UpdateSearchVisibilityInbound: implement on demand (mockBrigAPIAccess)" + GetUserExportData {} -> error "GetUserExportData: implement on demand (mockBrigAPIAccess)" + DeleteBot {} -> error "DeleteBot: implement on demand (mockBrigAPIAccess)" + GetAccountsBy {} -> error "GetAccountsBy: implement on demand (mockBrigAPIAccess)" + GetUsersByVariousKeys {} -> error "GetUsersByVariousKeys: implement on demand (mockBrigAPIAccess)" + CreateGroupInternal {} -> error "CreateGroupInternal: implement on demand (mockBrigAPIAccess)" + GetGroupInternal {} -> error "GetGroupInternal: implement on demand (mockBrigAPIAccess)" + GetGroupsInternal {} -> error "GetGroupsInternal: implement on demand (mockBrigAPIAccess)" + UpdateGroup {} -> error "UpdateGroup: implement on demand (mockBrigAPIAccess)" + DeleteGroupInternal {} -> error "DeleteGroupInternal: implement on demand (mockBrigAPIAccess)" + DeleteApp {} -> error "DeleteApp: implement on demand (mockBrigAPIAccess)" + GetAppIdsForTeam {} -> error "GetAppIdsForTeam: implement on demand (mockBrigAPIAccess)" + SetAccountStatus {} -> error "SetAccountStatus: implement on demand (mockBrigAPIAccess)" + CreateSAML {} -> error "CreateSAML: implement on demand (mockBrigAPIAccess)" + CreateNoSAML {} -> error "CreateNoSAML: implement on demand (mockBrigAPIAccess)" + UpdateEmail {} -> error "UpdateEmail: implement on demand (mockBrigAPIAccess)" + GetAccount {} -> error "GetAccount: implement on demand (mockBrigAPIAccess)" + GetAccountByHandle {} -> error "GetAccountByHandle: implement on demand (mockBrigAPIAccess)" + GetByEmail {} -> error "GetByEmail: implement on demand (mockBrigAPIAccess)" + SetName {} -> error "SetName: implement on demand (mockBrigAPIAccess)" + SetHandle {} -> error "SetHandle: implement on demand (mockBrigAPIAccess)" + SetManagedBy {} -> error "SetManagedBy: implement on demand (mockBrigAPIAccess)" + DeletePendingEmailUpdate {} -> error "DeletePendingEmailUpdate: implement on demand (mockBrigAPIAccess)" + SetSSOId {} -> error "SetSSOId: implement on demand (mockBrigAPIAccess)" + SetRichInfo {} -> error "SetRichInfo: implement on demand (mockBrigAPIAccess)" + SetLocale {} -> error "SetLocale: implement on demand (mockBrigAPIAccess)" + GetRichInfo {} -> error "GetRichInfo: implement on demand (mockBrigAPIAccess)" + CheckHandleAvailable {} -> error "CheckHandleAvailable: implement on demand (mockBrigAPIAccess)" + SsoLogin {} -> error "SsoLogin: implement on demand (mockBrigAPIAccess)" + GetStatus {} -> error "GetStatus: implement on demand (mockBrigAPIAccess)" + GetStatusMaybe {} -> error "GetStatusMaybe: implement on demand (mockBrigAPIAccess)" + SetStatus {} -> error "SetStatus: implement on demand (mockBrigAPIAccess)" + GetDefaultUserLocale {} -> error "GetDefaultUserLocale: implement on demand (mockBrigAPIAccess)" + CheckAdminGetTeamId {} -> error "CheckAdminGetTeamId: implement on demand (mockBrigAPIAccess)" + SendSAMLIdPChangedEmail {} -> error "SendSAMLIdPChangedEmail: implement on demand (mockBrigAPIAccess)" diff --git a/libs/wire-subsystems/test/unit/Wire/MockInterpreters/TeamCollaboratorsStore.hs b/libs/wire-subsystems/test/unit/Wire/MockInterpreters/TeamCollaboratorsStore.hs index 4def51eeef..63a334527a 100644 --- a/libs/wire-subsystems/test/unit/Wire/MockInterpreters/TeamCollaboratorsStore.hs +++ b/libs/wire-subsystems/test/unit/Wire/MockInterpreters/TeamCollaboratorsStore.hs @@ -41,6 +41,8 @@ inMemoryTeamCollaboratorsStoreInterpreter = gets $ \(s :: Map TeamId [TeamCollaborator]) -> find (\tc -> tc.gUser == userId) =<< Map.lookup teamId s GetTeamCollaborations userId -> gets $ \(s :: Map TeamId [TeamCollaborator]) -> concatMap (filter (\tc -> tc.gUser == userId)) (Map.elems s) + GetTeamCollaborationsForUsers userIds -> + gets $ \(s :: Map TeamId [TeamCollaborator]) -> concatMap (filter (\tc -> tc.gUser `elem` userIds)) (Map.elems s) GetTeamCollaboratorsWithIds teamIds userIds -> gets $ \(s :: Map TeamId [TeamCollaborator]) -> concatMap (concatMap (filter (\tc -> tc.gUser `elem` userIds)) . (\(tid :: TeamId) -> Map.lookup tid s)) teamIds diff --git a/libs/wire-subsystems/test/unit/Wire/MockInterpreters/UserSubsystem.hs b/libs/wire-subsystems/test/unit/Wire/MockInterpreters/UserSubsystem.hs index 786f733240..043f3a834d 100644 --- a/libs/wire-subsystems/test/unit/Wire/MockInterpreters/UserSubsystem.hs +++ b/libs/wire-subsystems/test/unit/Wire/MockInterpreters/UserSubsystem.hs @@ -87,7 +87,7 @@ inMemoryUserSubsystemInterpreter = BlockListInsert _ -> error "BlockListInsert: implement on demand (userSubsystemInterpreter)" UpdateTeamSearchVisibilityInbound _ -> error "UpdateTeamSearchVisibilityInbound: implement on demand (userSubsystemInterpreter)" AcceptTeamInvitation {} -> error "AcceptTeamInvitation: implement on demand (userSubsystemInterpreter)" - InternalUpdateSearchIndex _ -> error "InternalUpdateSearchIndex: implement on demand (userSubsystemInterpreter)" + InternalUpdateSearchIndex {} -> error "InternalUpdateSearchIndex: implement on demand (userSubsystemInterpreter)" InternalFindTeamInvitation {} -> error "InternalFindTeamInvitation: implement on demand (userSubsystemInterpreter)" GetUserExportData _ -> error "GetUserExportData: implement on demand (userSubsystemInterpreter)" RemoveEmailEither _ -> error "RemoveEmailEither: implement on demand (userSubsystemInterpreter)" diff --git a/libs/wire-subsystems/test/unit/Wire/ScimSubsystem/InterpreterSpec.hs b/libs/wire-subsystems/test/unit/Wire/ScimSubsystem/InterpreterSpec.hs index 6861f09779..f623e0012d 100644 --- a/libs/wire-subsystems/test/unit/Wire/ScimSubsystem/InterpreterSpec.hs +++ b/libs/wire-subsystems/test/unit/Wire/ScimSubsystem/InterpreterSpec.hs @@ -42,7 +42,7 @@ import Wire.API.User as User import Wire.API.User.Scim import Wire.API.UserGroup import Wire.BrigAPIAccess (BrigAPIAccess (..)) -import Wire.MockInterpreters +import Wire.MockInterpreters hiding (mockBrigAPIAccess) import Wire.ScimSubsystem import Wire.ScimSubsystem.Interpreter import Wire.StoredUser diff --git a/libs/wire-subsystems/test/unit/Wire/UserSearch/TypesSpec.hs b/libs/wire-subsystems/test/unit/Wire/UserSearch/TypesSpec.hs index a09d56bd8f..ea72188739 100644 --- a/libs/wire-subsystems/test/unit/Wire/UserSearch/TypesSpec.hs +++ b/libs/wire-subsystems/test/unit/Wire/UserSearch/TypesSpec.hs @@ -50,6 +50,7 @@ userDoc1 = UserDoc { udId = fromJust . hush . parseIdFromText $ "0a96b396-57d6-11ea-a04b-7b93d1a5c19c", udTeam = hush . parseIdFromText $ "17c59b18-57d6-11ea-9220-8bbf5eee961a", + udCollaboratingTeams = either (error . show) (: []) . parseIdFromText $ "17c59b18-57d6-11ea-9220-8bbf5eee961a", udName = Just . Name $ "Carl Phoomp", udNormalized = Just $ "carl phoomp", udHandle = Just . fromJust . parseHandle $ "phoompy", @@ -68,6 +69,5 @@ userDoc1 = udType = Nothing } --- Dont touch this. This represents serialized legacy data. userDoc1ByteString :: LByteString -userDoc1ByteString = "{\"email\":\"phoompy@example.com\",\"account_status\":\"active\",\"handle\":\"phoompy\",\"managed_by\":\"scim\",\"role\":\"admin\",\"accent_id\":32,\"name\":\"Carl Phoomp\",\"created_at\":\"2020-08-29T21:50:00.000Z\",\"team\":\"17c59b18-57d6-11ea-9220-8bbf5eee961a\",\"id\":\"0a96b396-57d6-11ea-a04b-7b93d1a5c19c\",\"normalized\":\"carl phoomp\",\"saml_idp\":\"https://issuer.net/214234\"}" +userDoc1ByteString = "{\"collaborating_teams\":[\"17c59b18-57d6-11ea-9220-8bbf5eee961a\"],\"email\":\"phoompy@example.com\",\"account_status\":\"active\",\"handle\":\"phoompy\",\"managed_by\":\"scim\",\"role\":\"admin\",\"accent_id\":32,\"name\":\"Carl Phoomp\",\"created_at\":\"2020-08-29T21:50:00.000Z\",\"team\":\"17c59b18-57d6-11ea-9220-8bbf5eee961a\",\"id\":\"0a96b396-57d6-11ea-a04b-7b93d1a5c19c\",\"normalized\":\"carl phoomp\",\"saml_idp\":\"https://issuer.net/214234\"}" diff --git a/libs/wire-subsystems/test/unit/Wire/UserSubsystem/InterpreterSpec.hs b/libs/wire-subsystems/test/unit/Wire/UserSubsystem/InterpreterSpec.hs index 68942a77cb..9e309d50dd 100644 --- a/libs/wire-subsystems/test/unit/Wire/UserSubsystem/InterpreterSpec.hs +++ b/libs/wire-subsystems/test/unit/Wire/UserSubsystem/InterpreterSpec.hs @@ -1110,7 +1110,7 @@ spec = describe "UserSubsystem.Interpreter" do searchee = searcheeNoHandle {handle = Just searcheeHandle} :: StoredUser storedUserToDoc :: StoredUser -> UserDoc - storedUserToDoc user = indexUserToDoc defaultSearchVisibilityInbound Nothing (storedUserToIndexUser user) + storedUserToDoc user = indexUserToDoc defaultSearchVisibilityInbound Nothing [] (storedUserToIndexUser user) indexFromStoredUsers :: [StoredUser] -> UserIndex indexFromStoredUsers storedUsers = do diff --git a/libs/wire-subsystems/wire-subsystems.cabal b/libs/wire-subsystems/wire-subsystems.cabal index 66662926e3..3d1c32c7af 100644 --- a/libs/wire-subsystems/wire-subsystems.cabal +++ b/libs/wire-subsystems/wire-subsystems.cabal @@ -244,6 +244,7 @@ library Wire.BoundedQueue Wire.BoundedQueue.STM Wire.BrigAPIAccess + Wire.BrigAPIAccess.Local Wire.BrigAPIAccess.Rpc Wire.BudgetStore Wire.BudgetStore.Cassandra @@ -653,6 +654,7 @@ test-suite wire-subsystems-tests Wire.MockInterpreters.AuthenticationSubsystem Wire.MockInterpreters.BackgroundJobPublisher Wire.MockInterpreters.BlockListStore + Wire.MockInterpreters.BrigAPIAccess Wire.MockInterpreters.ClientStore Wire.MockInterpreters.ConversationStore Wire.MockInterpreters.ConversationSubsystem diff --git a/services/background-worker/src/Wire/Effects.hs b/services/background-worker/src/Wire/Effects.hs index 0d367f19ee..66dbee09c5 100644 --- a/services/background-worker/src/Wire/Effects.hs +++ b/services/background-worker/src/Wire/Effects.hs @@ -367,7 +367,7 @@ runBackgroundWorkerEffects env extEnv requestId mJobId = ) . runFeaturesConfigSubsystem . runInputSem getAllTeamFeaturesForServer - . interpretTeamCollaboratorsSubsystem + . interpretTeamCollaboratorsSubsystem (interpretBrigAccess env.brigEndpoint) . discardMeetingNotifier . interpretConversationSubsystem where diff --git a/services/brig/src/Brig/App.hs b/services/brig/src/Brig/App.hs index 6c2145ea8c..27812d06b4 100644 --- a/services/brig/src/Brig/App.hs +++ b/services/brig/src/Brig/App.hs @@ -36,6 +36,7 @@ module Brig.App cargoholdLens, galleyLens, galleyEndpointLens, + brigEndpointLens, sparEndpointLens, gundeckEndpointLens, cargoholdEndpointLens, @@ -189,6 +190,10 @@ data Env = Env { cargohold :: RPC.Request, galley :: RPC.Request, galleyEndpoint :: Endpoint, + -- | Brig's own listen address. Used only to call ourselves over HTTP for + -- 'BrigAPIAccess' operations that have no local implementation yet; see + -- 'Wire.BrigAPIAccess.Local'. + brigEndpoint :: Endpoint, sparEndpoint :: Endpoint, gundeckEndpoint :: Endpoint, cargoholdEndpoint :: Endpoint, @@ -307,6 +312,7 @@ newEnv opts = do { cargohold = mkEndpoint $ opts.cargohold, galley = mkEndpoint $ opts.galley, galleyEndpoint = opts.galley, + brigEndpoint = opts.brig, sparEndpoint = opts.spar, gundeckEndpoint = opts.gundeck, cargoholdEndpoint = opts.cargohold, diff --git a/services/brig/src/Brig/CanonicalInterpreter.hs b/services/brig/src/Brig/CanonicalInterpreter.hs index d44c91648d..e6103cb472 100644 --- a/services/brig/src/Brig/CanonicalInterpreter.hs +++ b/services/brig/src/Brig/CanonicalInterpreter.hs @@ -47,6 +47,7 @@ import Polysemy.Input (Input, runInputConst) import Polysemy.Internal.Kind import Polysemy.Resource import Polysemy.TinyLog (TinyLog) +import Util.Options (Endpoint) import Wire.API.Error (ErrorS, errorToWai) import Wire.API.Error.Galley import Wire.API.Federation.Client qualified @@ -68,6 +69,8 @@ import Wire.BackgroundJobsPublisher (BackgroundJobPublisher) import Wire.BackgroundJobsPublisher.RabbitMQ (interpretBackgroundJobPublisherRabbitMQ) import Wire.BlockListStore import Wire.BlockListStore.Cassandra +import Wire.BrigAPIAccess (BrigAPIAccess) +import Wire.BrigAPIAccess.Local (interpretBrigAPIAccessLocally) import Wire.BudgetStore import Wire.BudgetStore.Cassandra import Wire.ClientStore (ClientStore) @@ -130,6 +133,7 @@ import Wire.PropertySubsystem.Interpreter import Wire.RateLimit import Wire.RateLimit.Interpreter import Wire.Rpc +import Wire.RpcException (RpcException) import Wire.SAMLEmailSubsystem import Wire.SAMLEmailSubsystem.Interpreter import Wire.SFT (SFT, interpretSFT) @@ -189,13 +193,12 @@ type RecursiveEffects = '[ AuthenticationSubsystem, UserSubsystem, AppSubsystem, - ClientSubsystem + ClientSubsystem, + BrigAPIAccess, + TeamCollaboratorsSubsystem ] -type NonRecursiveEffects2 = - '[ TeamCollaboratorsSubsystem - ] - `Append` BrigLowerLevelEffects +type NonRecursiveEffects2 = BrigLowerLevelEffects -- | These effects have interpreters which don't depend on each other type BrigLowerLevelEffects = @@ -276,6 +279,7 @@ type BrigLowerLevelEffects = Embed Cas.Client, Error ClientError, Error ParseException, + Error RpcException, Error ErrorCall, Error SomeException, Error HttpError, @@ -297,9 +301,11 @@ type BrigLowerLevelEffects = -- Cloned from "Wire.MiniBackend". runRecursiveEffects :: (Members NonRecursiveEffects2 r) => + -- | Brig's own endpoint; see 'interpretBrigAPIAccessLocally'. + Endpoint -> Sem (RecursiveEffects `Append` r) a -> Sem r a -runRecursiveEffects = runClient . runApp . runUser . runAuth +runRecursiveEffects selfEndpoint = runTeamCollaborators . runBrigAPIAccess . runClient . runApp . runUser . runAuth where runAuth :: forall r. (Members NonRecursiveEffects2 r) => InterpreterFor AuthenticationSubsystem r runAuth = interpretAuthenticationSubsystem runUser @@ -313,6 +319,12 @@ runRecursiveEffects = runClient . runApp . runUser . runAuth runClient :: forall r. (Members NonRecursiveEffects2 r) => InterpreterFor ClientSubsystem r runClient = runClientSubsystem runAuth runUser + runBrigAPIAccess :: forall r. (Members NonRecursiveEffects2 r) => InterpreterFor BrigAPIAccess r + runBrigAPIAccess = interpretBrigAPIAccessLocally selfEndpoint runUser + + runTeamCollaborators :: forall r. (Members NonRecursiveEffects2 r) => InterpreterFor TeamCollaboratorsSubsystem r + runTeamCollaborators = interpretTeamCollaboratorsSubsystem runBrigAPIAccess + runBrigToIO :: App.Env -> AppT BrigCanonicalEffects a -> IO a runBrigToIO e (AppT ma) = do let blockedDomains = @@ -433,6 +445,7 @@ runBrigToIO e (AppT ma) = do . rethrowHttpErrorIO . runError @SomeException . mapError @ErrorCall SomeException + . mapError @RpcException SomeException . mapError @ParseException SomeException . mapError clientErrorToHttpError . interpretClientToIO e.casClient @@ -510,8 +523,7 @@ runBrigToIO e (AppT ma) = do . interpretTeamCollaboratorsStoreToPostgres . interpretTeamSubsystemToGalleyAPI . samlEmailSubsystemInterpreter - . interpretTeamCollaboratorsSubsystem - . runRecursiveEffects + . runRecursiveEffects e.brigEndpoint . interpretUserGroupSubsystem . maybe runEnterpriseLoginSubsystemNoConfig diff --git a/services/brig/src/Brig/Index/Eval.hs b/services/brig/src/Brig/Index/Eval.hs index f931a03276..827d202830 100644 --- a/services/brig/src/Brig/Index/Eval.hs +++ b/services/brig/src/Brig/Index/Eval.hs @@ -54,6 +54,7 @@ import Polysemy.TinyLog (TinyLog) import System.Logger qualified as Log import System.Logger.Class (Logger) import Util.Options +import Wire.API.Team.Collaborator (TeamCollaboratorsError) import Wire.ClientSubsystem.Error (ClientError) import Wire.GalleyAPIAccess (GalleyAPIAccess) import Wire.GalleyAPIAccess.Rpc @@ -69,6 +70,8 @@ import Wire.Rpc import Wire.Sem.Logger.TinyLog import Wire.Sem.Metrics (Metrics) import Wire.Sem.Metrics.IO +import Wire.TeamCollaboratorsStore (TeamCollaboratorsStore) +import Wire.TeamCollaboratorsStore.Postgres (interpretTeamCollaboratorsStoreToPostgres) import Wire.UserKeyStore (UserKeyStore) import Wire.UserKeyStore.Cassandra import Wire.UserSearch.Migration (MigrationException) @@ -79,6 +82,7 @@ import Wire.UserStore.Postgres (interpretUserStorePostgres) type BrigIndexEffectStack = [ UserKeyStore, UserStore, + TeamCollaboratorsStore, IndexedUserStore, Error IndexedUserStoreError, IndexedUserMigrationStore, @@ -91,6 +95,7 @@ type BrigIndexEffectStack = TinyLog, Input Hasql.Pool, Error UsageError, + Error TeamCollaboratorsError, Error ClientError, Resource, Race, @@ -143,6 +148,7 @@ runSem (mgr, casClient, pgPool, bhEnv, indexedUserStoreConfig, reqId, migrationI . interpretRace . runResource . throwErrorToIOFinal @ClientError + . throwErrorToIOFinal @TeamCollaboratorsError . throwPostgresUsageErrorToIOFinal . runInputConst pgPool . loggerToTinyLogReqId reqId logger @@ -155,6 +161,7 @@ runSem (mgr, casClient, pgPool, bhEnv, indexedUserStoreConfig, reqId, migrationI . interpretIndexedUserMigrationStoreES bhEnv migrationIndexName . throwErrorToIOFinal @IndexedUserStoreError . interpretIndexedUserStoreES indexedUserStoreConfig + . interpretTeamCollaboratorsStoreToPostgres . userStoreInterpreter . interpretUserKeyStoreCassandra casClient $ action @@ -181,10 +188,14 @@ runCommand l = \case runIndexIO e $ resetIndex (mkCreateIndexSettings es) Reindex es cas pg userStorageLocation galley pageSize -> do semDeps <- mkSemDeps (es ^. esConnection) cas pg l - IndexedUserStoreBulk.syncAllUsers (runSem semDeps userStorageLocation galley l) pageSize + skipped <- IndexedUserStoreBulk.syncAllUsers (runSem semDeps userStorageLocation galley l) pageSize + when (skipped /= 0) do + throwM . IndexMigrationError $ "Reindex: failed to sync " <> show skipped <> " documents." ReindexSameOrNewer es cas pg userStorageLocation galley pageSize -> do semDeps <- mkSemDeps (es ^. esConnection) cas pg l - IndexedUserStoreBulk.forceSyncAllUsers (runSem semDeps userStorageLocation galley l) pageSize + skipped <- IndexedUserStoreBulk.forceSyncAllUsers (runSem semDeps userStorageLocation galley l) pageSize + when (skipped /= 0) do + throwM . IndexMigrationError $ "ReindexSameOrNewer: failed to sync " <> show skipped <> " documents." UpdateMapping esConn galley -> do e <- initIndex l esConn galley runIndexIO e updateMapping @@ -265,6 +276,11 @@ waitForTaskToComplete timeoutSeconds taskNodeId = do errTaskGet :: ES.EsError -> m x errTaskGet e = throwM $ ReindexFromAnotherIndexError $ "Error response while getting task: " <> show e +newtype IndexMigrationError = IndexMigrationError String + deriving (Show) + +instance Exception IndexMigrationError + newtype ReindexFromAnotherIndexError = ReindexFromAnotherIndexError String deriving (Show) diff --git a/services/brig/src/Brig/User/Search/Index.hs b/services/brig/src/Brig/User/Search/Index.hs index 4c4919729d..68a17f07b1 100644 --- a/services/brig/src/Brig/User/Search/Index.hs +++ b/services/brig/src/Brig/User/Search/Index.hs @@ -364,6 +364,15 @@ indexMapping = mpAnalyzer = Nothing, mpFields = mempty }, + -- teams this user collaborates with (without being a member of them) + "collaborating_teams" + .= MappingProperty + { mpType = MPKeyword, + mpStore = False, + mpIndex = True, + mpAnalyzer = Nothing, + mpFields = mempty + }, "accent_id" .= MappingProperty { mpType = MPByte, diff --git a/services/galley/src/Galley/App.hs b/services/galley/src/Galley/App.hs index 4d755d8c61..f47dc7f379 100644 --- a/services/galley/src/Galley/App.hs +++ b/services/galley/src/Galley/App.hs @@ -572,7 +572,7 @@ evalGalley e = . interpretTeamSubsystem teamSubsystemConfig . runFeaturesConfigSubsystem . runInputSem getAllTeamFeaturesForServer - . interpretTeamCollaboratorsSubsystem + . interpretTeamCollaboratorsSubsystem (interpretBrigAccess (e ^. brig)) . runFederationSubsystem conversationSubsystemConfig.federationProtocols . runInputConst (e ^. reqId) . interpretJobSubsystem From 39a49acee02e904464fe597fe90051b457a42e8d Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Fri, 4 Sep 2026 11:22:27 +0200 Subject: [PATCH 03/20] Integration test: run new brig on old index. (I expected this to break, but it didn't. The search for the issue continues.) --- services/brig/test/integration/API/Search.hs | 64 +++++++++++++++++++- 1 file changed, 63 insertions(+), 1 deletion(-) diff --git a/services/brig/test/integration/API/Search.hs b/services/brig/test/integration/API/Search.hs index 7729573dc7..6cc05fb0e1 100644 --- a/services/brig/test/integration/API/Search.hs +++ b/services/brig/test/integration/API/Search.hs @@ -51,10 +51,12 @@ import Control.Monad.Catch (MonadCatch) import Data.Aeson (Value, decode) import Data.Aeson qualified as Aeson import Data.Aeson.Lens qualified as Aeson +import Data.ByteString.Conversion import Data.Domain (Domain (Domain)) import Data.Handle (fromHandle) import Data.Id import Data.Qualified (Qualified (qDomain, qUnqualified)) +import Data.Set qualified as Set import Data.String.Conversions import Data.Text qualified as Text import Data.Text.Encoding qualified as Text @@ -79,6 +81,7 @@ import UnliftIO (Concurrently (..), async, bracket, cancel, runConcurrently) import Util import Util.Options (Endpoint) import Wire.API.Federation.API.Brig (SearchResponse (SearchResponse)) +import Wire.API.Team.Collaborator (CollaboratorPermission (..), NewTeamCollaborator (..)) import Wire.API.Team.Feature import Wire.API.Team.Member qualified as Member import Wire.API.Team.Permission @@ -166,7 +169,12 @@ tests opts additionalElasticSearch mgr galley brig = do ], test mgr "user with unvalidated email" $ testSearchWithUnvalidatedEmail brig, test mgr "testSearchableMissing: searchable field missing defaults to true" $ - testSearchableMissing opts brig galley + testSearchableMissing opts brig galley, + testGroup "collaborator search" $ + [ test mgr "collaborator found on new index" $ testCollaboratorFoundNewIndex brig galley, + test mgr "collaborator not found on old index" $ testCollaboratorNotFoundOldIndex opts brig galley, + testWithBothIndices opts mgr "non-collaborator not found" $ testNonCollaboratorNotFound brig galley + ] ] where -- Since the tests are about querying only, we only need 1 creation @@ -968,6 +976,60 @@ runBH opts action = do let bEnv = mkBHEnv esURL mgr ES.runBH bEnv action +-- | Collaborator from another team is found on new index +testCollaboratorFoundNewIndex :: (TestConstraints m) => Brig -> Galley -> m () +testCollaboratorFoundNewIndex brig galley = do + (tidA, ownerA, []) <- createPopulatedBindingTeamWithNamesAndHandles brig 0 + (_, _, [memberB]) <- createPopulatedBindingTeamWithNamesAndHandles brig 1 + + let payload = NewTeamCollaborator (qUnqualified (userQualifiedId memberB)) (Set.fromList [CreateTeamConversation, ImplicitConnection]) + in post (galley . paths ["teams", toByteString' tidA, "collaborators"] . zUser (userId ownerA) . json payload) + !!! const 200 === statusCode + + refreshIndex brig + assertCanFind brig (userId ownerA) (userQualifiedId memberB) (fromName $ userDisplayName memberB) + +-- | Collaborator from another team is NOT found on old index, but search must not crash +testCollaboratorNotFoundOldIndex :: (TestConstraints m) => Opts -> Brig -> Galley -> m () +testCollaboratorNotFoundOldIndex opts brig galley = do + (tidA, ownerA, _) <- createPopulatedBindingTeamWithNamesAndHandles brig 1 + (_, _, [userQualifiedId -> memberB]) <- createPopulatedBindingTeamWithNamesAndHandles brig 1 + + let payload = NewTeamCollaborator (qUnqualified memberB) (Set.fromList [CreateTeamConversation, ImplicitConnection]) + in post (galley . paths ["teams", toByteString' tidA, "collaborators"] . zUser (userId ownerA) . json payload) + !!! const 200 === statusCode + + -- 'withOldIndex' uses 'withSettingsOverrides', which pulls the + -- affected service out of the cluster. So if we wrap this test + -- into 'withOldIndex', the rpc to galley will hit the brig API, + -- resulting in the 404. + -- + -- Solution: update the index separately, after the call to galley, + -- thus + withOldIndex opts defaultMigrationIndexName $ do + post (brig . paths ["i", "index", "update", toByteString' (qUnqualified memberB)]) !!! const 200 === statusCode + refreshIndex brig + + -- refreshIndex brig + res <- searchResults <$> executeSearch brig (userId ownerA) "" + liftIO $ + assertBool "collaborator should NOT be found on old mapping" $ + memberB `notElem` map contactQualifiedId res + +-- | Non-collaborator from another team is not found on any index +testNonCollaboratorNotFound :: (TestConstraints m) => Brig -> Galley -> m () +testNonCollaboratorNotFound brig _galley = do + (_, ownerA, _) <- createPopulatedBindingTeamWithNamesAndHandles brig 1 + (_, _, [memberB]) <- createPopulatedBindingTeamWithNamesAndHandles brig 1 + + -- Do NOT add memberB as collaborator + -- Search should not find memberB regardless of index version + refreshIndex brig + res <- searchResults <$> executeSearch brig (userId ownerA) "" + liftIO $ + assertBool "non-collaborator should not be found" $ + userQualifiedId memberB `notElem` map contactQualifiedId res + -- | This was generated from Brig.User.Search.Index.indexMapping at commit 18885bc -- how to generate: -- - run `cabal repl brig` From b342775b87e5842b82be5e2a411c779722717867 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Fri, 4 Sep 2026 12:15:16 +0200 Subject: [PATCH 04/20] [throw-away debug code] dump ES state. --- services/brig/test/integration/API/Search.hs | 76 +++++++++++++++++--- 1 file changed, 68 insertions(+), 8 deletions(-) diff --git a/services/brig/test/integration/API/Search.hs b/services/brig/test/integration/API/Search.hs index 6cc05fb0e1..54fbb46a8b 100644 --- a/services/brig/test/integration/API/Search.hs +++ b/services/brig/test/integration/API/Search.hs @@ -64,6 +64,7 @@ import Data.UUID qualified as UUID import Database.Bloodhound qualified as ES import Federation.Util import Imports +import Network.HTTP.Client qualified as HTTPClient import Network.HTTP.ReverseProxy (waiProxyTo) import Network.HTTP.ReverseProxy qualified as Wai import Network.HTTP.Types qualified as HTTP @@ -993,9 +994,12 @@ testCollaboratorFoundNewIndex brig galley = do testCollaboratorNotFoundOldIndex :: (TestConstraints m) => Opts -> Brig -> Galley -> m () testCollaboratorNotFoundOldIndex opts brig galley = do (tidA, ownerA, _) <- createPopulatedBindingTeamWithNamesAndHandles brig 1 - (_, _, [userQualifiedId -> memberB]) <- createPopulatedBindingTeamWithNamesAndHandles brig 1 + (_, _, [memberB]) <- createPopulatedBindingTeamWithNamesAndHandles brig 1 + let memberBQid = userQualifiedId memberB + memberBUid = qUnqualified memberBQid + memberBName = fromName (userDisplayName memberB) - let payload = NewTeamCollaborator (qUnqualified memberB) (Set.fromList [CreateTeamConversation, ImplicitConnection]) + let payload = NewTeamCollaborator memberBUid (Set.fromList [CreateTeamConversation, ImplicitConnection]) in post (galley . paths ["teams", toByteString' tidA, "collaborators"] . zUser (userId ownerA) . json payload) !!! const 200 === statusCode @@ -1005,16 +1009,72 @@ testCollaboratorNotFoundOldIndex opts brig galley = do -- resulting in the 404. -- -- Solution: update the index separately, after the call to galley, - -- thus - withOldIndex opts defaultMigrationIndexName $ do - post (brig . paths ["i", "index", "update", toByteString' (qUnqualified memberB)]) !!! const 200 === statusCode + -- thus. We use 'optsForOldIndex' rather than 'withOldIndex' because we need + -- the index name for the diagnostics below, and because 'withOldIndex' + -- deletes the index the moment the block ends (so nothing that runs *after* + -- the block can observe it). + (oldOpts, oldIndex) <- optsForOldIndex opts defaultMigrationIndexName + let ES.IndexName defaultIndex = opts ^. Opt.elasticsearchLens . Opt.indexLens + + dumpEsState opts "default index (new mapping), before reindex" defaultIndex tidA memberBUid + dumpEsState opts "old-mapping index, before reindex" oldIndex tidA memberBUid + + -- Search against the *old* index, i.e. from inside the session. + (resOldByName, resOldByEmptyTerm) <- withSettingsOverrides oldOpts $ do + post (brig . paths ["i", "index", "update", toByteString' memberBUid]) !!! const 200 === statusCode refreshIndex brig + dumpEsState opts "old-mapping index, after reindex" oldIndex tidA memberBUid + (,) + <$> (searchResults <$> executeSearch brig (userId ownerA) memberBName) + <*> (searchResults <$> executeSearch brig (userId ownerA) "") + + -- Search against the default index, for contrast. + refreshIndex brig + resDefaultByName <- searchResults <$> executeSearch brig (userId ownerA) memberBName + dumpEsState opts "default index (new mapping), after reindex" defaultIndex tidA memberBUid + + liftIO $ do + putStrLn $ "### searcher: " <> show (userId ownerA) <> " (team " <> show tidA <> ")" + putStrLn $ "### looking for: " <> show memberBQid <> " (name " <> show memberBName <> ")" + putStrLn $ "### brig search, old index, term = name: " <> show (map contactQualifiedId resOldByName) + putStrLn $ "### brig search, old index, term = \"\": " <> show (map contactQualifiedId resOldByEmptyTerm) + putStrLn $ "### brig search, default index, term = name: " <> show (map contactQualifiedId resDefaultByName) + + deleteIndex opts oldIndex - -- refreshIndex brig - res <- searchResults <$> executeSearch brig (userId ownerA) "" liftIO $ assertBool "collaborator should NOT be found on old mapping" $ - memberB `notElem` map contactQualifiedId res + memberBQid `notElem` map contactQualifiedId resOldByName + +-- | Throwaway diagnostics: dump the mapping of an ES index, the document we +-- care about, and what a direct query on @collaborating_teams@ turns up. +dumpEsState :: (MonadIO m) => Opt.Opts -> String -> Text -> TeamId -> UserId -> m () +dumpEsState opts label idx tid uid = do + let ES.MappingName mpp = mappingName + uid' = cs (toByteString' uid) + tid' = cs (toByteString' tid) + mapping <- esGet opts (cs idx <> "/_mapping") + doc <- esGet opts (cs idx <> "/" <> cs mpp <> "/" <> uid') + allDocs <- esGet opts (cs idx <> "/_search?size=100") + byTeam <- esGet opts (cs idx <> "/_search?q=collaborating_teams:" <> tid') + liftIO $ do + putStrLn $ "\n### === " <> label <> " (index " <> cs idx <> ") ===" + putStrLn $ "### mapping mentions collaborating_teams: " <> show ("collaborating_teams" `Text.isInfixOf` cs mapping) + putStrLn $ "### mapping: " <> cs mapping + putStrLn $ "### document " <> uid' <> ": " <> cs doc + putStrLn $ "### all documents: " <> cs allDocs + putStrLn $ "### q=collaborating_teams:" <> tid' <> " -> " <> cs byTeam + +-- | Raw GET against ES, for diagnostics. Goes through 'mkBHEnv' so we inherit +-- its credentials and TLS setup. +esGet :: (MonadIO m) => Opt.Opts -> String -> m LByteString +esGet opts urlPath = liftIO $ do + let (ES.Server esURL) = opts ^. Opt.elasticsearchLens . Opt.urlLens + mgr <- initHttpManagerWithTLSConfig opts.elasticsearch.insecureSkipVerifyTls opts.elasticsearch.caCert + let bEnv = mkBHEnv esURL mgr + baseReq <- HTTPClient.parseRequest (cs esURL <> "/" <> urlPath) + req <- ES.bhRequestHook bEnv baseReq + HTTPClient.responseBody <$> HTTPClient.httpLbs req (ES.bhManager bEnv) -- | Non-collaborator from another team is not found on any index testNonCollaboratorNotFound :: (TestConstraints m) => Brig -> Galley -> m () From 6e003cdd26de5258505ca2291e676da0c0c38af5 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Mon, 7 Sep 2026 10:42:51 +0200 Subject: [PATCH 05/20] Revert "[throw-away debug code] dump ES state." This reverts commit 7aa933b633d0bc7c809877d93f02e0a7c4898f75. --- services/brig/test/integration/API/Search.hs | 76 +++----------------- 1 file changed, 8 insertions(+), 68 deletions(-) diff --git a/services/brig/test/integration/API/Search.hs b/services/brig/test/integration/API/Search.hs index 54fbb46a8b..6cc05fb0e1 100644 --- a/services/brig/test/integration/API/Search.hs +++ b/services/brig/test/integration/API/Search.hs @@ -64,7 +64,6 @@ import Data.UUID qualified as UUID import Database.Bloodhound qualified as ES import Federation.Util import Imports -import Network.HTTP.Client qualified as HTTPClient import Network.HTTP.ReverseProxy (waiProxyTo) import Network.HTTP.ReverseProxy qualified as Wai import Network.HTTP.Types qualified as HTTP @@ -994,12 +993,9 @@ testCollaboratorFoundNewIndex brig galley = do testCollaboratorNotFoundOldIndex :: (TestConstraints m) => Opts -> Brig -> Galley -> m () testCollaboratorNotFoundOldIndex opts brig galley = do (tidA, ownerA, _) <- createPopulatedBindingTeamWithNamesAndHandles brig 1 - (_, _, [memberB]) <- createPopulatedBindingTeamWithNamesAndHandles brig 1 - let memberBQid = userQualifiedId memberB - memberBUid = qUnqualified memberBQid - memberBName = fromName (userDisplayName memberB) + (_, _, [userQualifiedId -> memberB]) <- createPopulatedBindingTeamWithNamesAndHandles brig 1 - let payload = NewTeamCollaborator memberBUid (Set.fromList [CreateTeamConversation, ImplicitConnection]) + let payload = NewTeamCollaborator (qUnqualified memberB) (Set.fromList [CreateTeamConversation, ImplicitConnection]) in post (galley . paths ["teams", toByteString' tidA, "collaborators"] . zUser (userId ownerA) . json payload) !!! const 200 === statusCode @@ -1009,72 +1005,16 @@ testCollaboratorNotFoundOldIndex opts brig galley = do -- resulting in the 404. -- -- Solution: update the index separately, after the call to galley, - -- thus. We use 'optsForOldIndex' rather than 'withOldIndex' because we need - -- the index name for the diagnostics below, and because 'withOldIndex' - -- deletes the index the moment the block ends (so nothing that runs *after* - -- the block can observe it). - (oldOpts, oldIndex) <- optsForOldIndex opts defaultMigrationIndexName - let ES.IndexName defaultIndex = opts ^. Opt.elasticsearchLens . Opt.indexLens - - dumpEsState opts "default index (new mapping), before reindex" defaultIndex tidA memberBUid - dumpEsState opts "old-mapping index, before reindex" oldIndex tidA memberBUid - - -- Search against the *old* index, i.e. from inside the session. - (resOldByName, resOldByEmptyTerm) <- withSettingsOverrides oldOpts $ do - post (brig . paths ["i", "index", "update", toByteString' memberBUid]) !!! const 200 === statusCode + -- thus + withOldIndex opts defaultMigrationIndexName $ do + post (brig . paths ["i", "index", "update", toByteString' (qUnqualified memberB)]) !!! const 200 === statusCode refreshIndex brig - dumpEsState opts "old-mapping index, after reindex" oldIndex tidA memberBUid - (,) - <$> (searchResults <$> executeSearch brig (userId ownerA) memberBName) - <*> (searchResults <$> executeSearch brig (userId ownerA) "") - - -- Search against the default index, for contrast. - refreshIndex brig - resDefaultByName <- searchResults <$> executeSearch brig (userId ownerA) memberBName - dumpEsState opts "default index (new mapping), after reindex" defaultIndex tidA memberBUid - - liftIO $ do - putStrLn $ "### searcher: " <> show (userId ownerA) <> " (team " <> show tidA <> ")" - putStrLn $ "### looking for: " <> show memberBQid <> " (name " <> show memberBName <> ")" - putStrLn $ "### brig search, old index, term = name: " <> show (map contactQualifiedId resOldByName) - putStrLn $ "### brig search, old index, term = \"\": " <> show (map contactQualifiedId resOldByEmptyTerm) - putStrLn $ "### brig search, default index, term = name: " <> show (map contactQualifiedId resDefaultByName) - - deleteIndex opts oldIndex + -- refreshIndex brig + res <- searchResults <$> executeSearch brig (userId ownerA) "" liftIO $ assertBool "collaborator should NOT be found on old mapping" $ - memberBQid `notElem` map contactQualifiedId resOldByName - --- | Throwaway diagnostics: dump the mapping of an ES index, the document we --- care about, and what a direct query on @collaborating_teams@ turns up. -dumpEsState :: (MonadIO m) => Opt.Opts -> String -> Text -> TeamId -> UserId -> m () -dumpEsState opts label idx tid uid = do - let ES.MappingName mpp = mappingName - uid' = cs (toByteString' uid) - tid' = cs (toByteString' tid) - mapping <- esGet opts (cs idx <> "/_mapping") - doc <- esGet opts (cs idx <> "/" <> cs mpp <> "/" <> uid') - allDocs <- esGet opts (cs idx <> "/_search?size=100") - byTeam <- esGet opts (cs idx <> "/_search?q=collaborating_teams:" <> tid') - liftIO $ do - putStrLn $ "\n### === " <> label <> " (index " <> cs idx <> ") ===" - putStrLn $ "### mapping mentions collaborating_teams: " <> show ("collaborating_teams" `Text.isInfixOf` cs mapping) - putStrLn $ "### mapping: " <> cs mapping - putStrLn $ "### document " <> uid' <> ": " <> cs doc - putStrLn $ "### all documents: " <> cs allDocs - putStrLn $ "### q=collaborating_teams:" <> tid' <> " -> " <> cs byTeam - --- | Raw GET against ES, for diagnostics. Goes through 'mkBHEnv' so we inherit --- its credentials and TLS setup. -esGet :: (MonadIO m) => Opt.Opts -> String -> m LByteString -esGet opts urlPath = liftIO $ do - let (ES.Server esURL) = opts ^. Opt.elasticsearchLens . Opt.urlLens - mgr <- initHttpManagerWithTLSConfig opts.elasticsearch.insecureSkipVerifyTls opts.elasticsearch.caCert - let bEnv = mkBHEnv esURL mgr - baseReq <- HTTPClient.parseRequest (cs esURL <> "/" <> urlPath) - req <- ES.bhRequestHook bEnv baseReq - HTTPClient.responseBody <$> HTTPClient.httpLbs req (ES.bhManager bEnv) + memberB `notElem` map contactQualifiedId res -- | Non-collaborator from another team is not found on any index testNonCollaboratorNotFound :: (TestConstraints m) => Brig -> Galley -> m () From a504101c5c94d0e50de7f548548a1df2b476661e Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Mon, 7 Sep 2026 12:13:20 +0200 Subject: [PATCH 06/20] Make (forceR,r}efreshIndex calls to brig fail more helpfully. --- .../IndexedUserStore/Bulk/ElasticSearch.hs | 44 ++++++++++--------- services/brig/src/Brig/Index/Eval.hs | 8 ++-- 2 files changed, 28 insertions(+), 24 deletions(-) diff --git a/libs/wire-subsystems/src/Wire/IndexedUserStore/Bulk/ElasticSearch.hs b/libs/wire-subsystems/src/Wire/IndexedUserStore/Bulk/ElasticSearch.hs index 31d7777901..829daffa12 100644 --- a/libs/wire-subsystems/src/Wire/IndexedUserStore/Bulk/ElasticSearch.hs +++ b/libs/wire-subsystems/src/Wire/IndexedUserStore/Bulk/ElasticSearch.hs @@ -22,7 +22,6 @@ import Cassandra.Util (Writetime (Writetime)) import Conduit (ConduitT, runConduit, (.|)) import Control.Error (headMay) import Control.Exception (try) -import Control.Monad.Extra (mapMaybeM) import Data.Conduit.Combinators qualified as Conduit import Data.Conduit.Internal (zipSources) import Data.Conduit.List qualified as CL @@ -59,25 +58,26 @@ type IOInterpreter r = forall a. Sem r a -> IO a expectedMigrationVersion :: MigrationVersion expectedMigrationVersion = MigrationVersion 7 -syncAllUsers :: (Member UserStore r, Member IndexedUserStore r, Member TinyLog r, Member GalleyAPIAccess r, Member TeamCollaboratorsStore r) => IOInterpreter r -> Int32 -> IO Int +syncAllUsers :: (Member UserStore r, Member IndexedUserStore r, Member TinyLog r, Member GalleyAPIAccess r, Member TeamCollaboratorsStore r) => IOInterpreter r -> Int32 -> IO (Int, [String]) syncAllUsers interpreter pageSize = syncAllUsersWithVersion interpreter pageSize ES.ExternalGT -forceSyncAllUsers :: (Member UserStore r, Member IndexedUserStore r, Member TinyLog r, Member GalleyAPIAccess r, Member TeamCollaboratorsStore r) => IOInterpreter r -> Int32 -> IO Int +forceSyncAllUsers :: (Member UserStore r, Member IndexedUserStore r, Member TinyLog r, Member GalleyAPIAccess r, Member TeamCollaboratorsStore r) => IOInterpreter r -> Int32 -> IO (Int, [String]) forceSyncAllUsers interpreter pageSize = syncAllUsersWithVersion interpreter pageSize ES.ExternalGTE -- | Returns the number of users that could not be indexed because some of the --- data needed to build their document was unavailable. Those users have been --- logged individually by 'logAndHush'. -syncAllUsersWithVersion :: (Member UserStore r, Member IndexedUserStore r, Member TinyLog r, Member GalleyAPIAccess r, Member TeamCollaboratorsStore r) => IOInterpreter r -> Int32 -> (ES.ExternalDocVersion -> ES.VersionControl) -> IO Int +-- data needed to build their document was unavailable, together with a list +-- of error messages describing the failures (one per skipped user). Those +-- users have been logged individually by 'logFailures'. +syncAllUsersWithVersion :: (Member UserStore r, Member IndexedUserStore r, Member TinyLog r, Member GalleyAPIAccess r, Member TeamCollaboratorsStore r) => IOInterpreter r -> Int32 -> (ES.ExternalDocVersion -> ES.VersionControl) -> IO (Int, [String]) syncAllUsersWithVersion interpreter pageSize mkVersion = - fmap getSum . runConduit $ + fmap (\(Sum skipped, errors) -> (skipped, errors)) . runConduit $ zipSources (CL.sourceList [1 ..]) (paginateWithStateC (interpreter . getIndexUsersPaginated pageSize)) .| logPage .| mkUserDocs .| Conduit.foldMapM upsertPage where - upsertPage :: (Int, [(ES.DocId, UserDoc, ES.VersionControl)]) -> IO (Sum Int) - upsertPage (skipped, docs) = Sum skipped <$ interpreter (IndexedUserStore.bulkUpsert docs) + upsertPage :: (Int, [String], [(ES.DocId, UserDoc, ES.VersionControl)]) -> IO (Sum Int, [String]) + upsertPage (skipped, errors, docs) = (Sum skipped, errors) <$ interpreter (IndexedUserStore.bulkUpsert docs) logPage :: ConduitT (Int32, [IndexUser]) [IndexUser] IO () logPage = Conduit.mapM $ \(pageNumber, page) -> do @@ -89,8 +89,9 @@ syncAllUsersWithVersion interpreter pageSize mkVersion = pure page -- Emits the documents to be indexed together with the number of users of - -- this page that had to be skipped. - mkUserDocs :: ConduitT [IndexUser] (Int, [(ES.DocId, UserDoc, ES.VersionControl)]) IO () + -- this page that had to be skipped and the error messages for each of + -- those skipped users. + mkUserDocs :: ConduitT [IndexUser] (Int, [String], [(ES.DocId, UserDoc, ES.VersionControl)]) IO () mkUserDocs = Conduit.mapM $ \page -> do -- FUTUREWORK: extract team visibilities, roles and user type -- more efficiently sending one query per page @@ -126,7 +127,7 @@ syncAllUsersWithVersion interpreter pageSize mkVersion = getRoles t (fmap (.userId) us) -- One query for the whole page. A failure here fails every document of the - -- page, which 'logAndHush' then logs and skips. + -- page, which 'logFailures' then logs and skips. eithCollabTeams :: Either SomeException (Map UserId [TeamId]) <- try . fmap (Map.fromListWith (<>) . map (\tc -> (gUser tc, [gTeam tc]))) . interpreter $ getTeamCollaborationsForUsers (Set.fromList (map (.userId) page)) @@ -150,23 +151,25 @@ syncAllUsersWithVersion interpreter pageSize mkVersion = docsWithErrors :: (e ~ Either SomeException) => [(ES.DocId, e UserDoc, e ES.VersionControl)] docsWithErrors = map (\u -> (userIdToDocId u.userId, mkUserDoc u, mkDocVersion u)) page - docs <- interpreter . flip mapMaybeM docsWithErrors $ logAndHush - let skipped = length docsWithErrors - length docs - pure (skipped, docs) + results <- interpreter $ traverse logFailures docsWithErrors + let (errors, docs) = partitionEithers results + skipped = length errors + cappedErrors = take 1000 errors + pure (skipped, cappedErrors, docs) rightSecond :: (a, b) -> (a, Either c b) rightSecond (a, b) = (a, Right b) - logAndHush :: (Member TinyLog r) => (ES.DocId, Either SomeException UserDoc, Either SomeException ES.VersionControl) -> Sem r (Maybe (ES.DocId, UserDoc, ES.VersionControl)) - logAndHush (docId@(ES.DocId idText), eithUserDoc, eithVersion) = + logFailures :: (Member TinyLog r) => (ES.DocId, Either SomeException UserDoc, Either SomeException ES.VersionControl) -> Sem r (Either String (ES.DocId, UserDoc, ES.VersionControl)) + logFailures (docId@(ES.DocId idText), eithUserDoc, eithVersion) = case (,) <$> eithUserDoc <*> eithVersion of Left e -> do Log.err $ Log.msg (Log.val "Error ocurred while indexing user") . Log.field "userId" idText . Log.field "error" (show e) - pure Nothing - Right (userDoc, version) -> pure $ Just (docId, userDoc, version) + pure $ Left $ show idText <> ": " <> show e + Right (userDoc, version) -> pure $ Right (docId, userDoc, version) mkRoleWithWriteTime :: TeamMemberInfo -> Maybe (UserId, WithWritetime Role) mkRoleWithWriteTime tmi = @@ -196,7 +199,7 @@ migrateData interpreter pageSize = interpreter $ do Log.msg (Log.val "Migration necessary.") . Log.field "expectedVersion" expectedMigrationVersion . Log.field "foundVersion" foundVersion - skipped <- embed $ forceSyncAllUsers interpreter pageSize + (skipped, errors) <- embed $ forceSyncAllUsers interpreter pageSize if skipped == 0 then MigrationStore.persistMigrationVersion expectedMigrationVersion else do @@ -204,6 +207,7 @@ migrateData interpreter pageSize = interpreter $ do Log.msg (Log.val "Migration incomplete, not persisting migration version.") . Log.field "expectedVersion" expectedMigrationVersion . Log.field "skippedUsers" skipped + . Log.field "errors" (show errors) throw $ SyncIncomplete else do Log.info $ diff --git a/services/brig/src/Brig/Index/Eval.hs b/services/brig/src/Brig/Index/Eval.hs index 827d202830..04d949f286 100644 --- a/services/brig/src/Brig/Index/Eval.hs +++ b/services/brig/src/Brig/Index/Eval.hs @@ -188,14 +188,14 @@ runCommand l = \case runIndexIO e $ resetIndex (mkCreateIndexSettings es) Reindex es cas pg userStorageLocation galley pageSize -> do semDeps <- mkSemDeps (es ^. esConnection) cas pg l - skipped <- IndexedUserStoreBulk.syncAllUsers (runSem semDeps userStorageLocation galley l) pageSize + (skipped, errors) <- IndexedUserStoreBulk.syncAllUsers (runSem semDeps userStorageLocation galley l) pageSize when (skipped /= 0) do - throwM . IndexMigrationError $ "Reindex: failed to sync " <> show skipped <> " documents." + throwM . IndexMigrationError $ "Reindex: failed to sync " <> show skipped <> " documents. Errors: " <> show errors ReindexSameOrNewer es cas pg userStorageLocation galley pageSize -> do semDeps <- mkSemDeps (es ^. esConnection) cas pg l - skipped <- IndexedUserStoreBulk.forceSyncAllUsers (runSem semDeps userStorageLocation galley l) pageSize + (skipped, errors) <- IndexedUserStoreBulk.forceSyncAllUsers (runSem semDeps userStorageLocation galley l) pageSize when (skipped /= 0) do - throwM . IndexMigrationError $ "ReindexSameOrNewer: failed to sync " <> show skipped <> " documents." + throwM . IndexMigrationError $ "ReindexSameOrNewer: failed to sync " <> show skipped <> " documents. Errors: " <> show errors UpdateMapping esConn galley -> do e <- initIndex l esConn galley runIndexIO e updateMapping From c6dee5bb0519f2a714f0e93147a9c559d7c9de23 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Mon, 7 Sep 2026 12:43:03 +0200 Subject: [PATCH 07/20] [drive-by] rm constraint declaration duplication. --- services/brig/test/integration/API/Search.hs | 3 ++- services/brig/test/integration/API/TeamUserSearch.hs | 7 ++----- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/services/brig/test/integration/API/Search.hs b/services/brig/test/integration/API/Search.hs index 6cc05fb0e1..1792ccc0b2 100644 --- a/services/brig/test/integration/API/Search.hs +++ b/services/brig/test/integration/API/Search.hs @@ -25,7 +25,8 @@ -- with this program. If not, see . module API.Search - ( tests, + ( TestConstraints, + tests, testWithBothIndices, ) where diff --git a/services/brig/test/integration/API/TeamUserSearch.hs b/services/brig/test/integration/API/TeamUserSearch.hs index d6ddbecfbf..01eab68b95 100644 --- a/services/brig/test/integration/API/TeamUserSearch.hs +++ b/services/brig/test/integration/API/TeamUserSearch.hs @@ -17,13 +17,12 @@ module API.TeamUserSearch (tests) where -import API.Search (testWithBothIndices) +import API.Search (TestConstraints, testWithBothIndices) import API.Search.Util (executeTeamUserSearch, executeTeamUserSearchWithMaybeState, refreshIndex) import API.Team.Util (createPopulatedBindingTeamWithNamesAndHandles) import API.User.Util (initiateEmailUpdateAutoActivate) -import Bilge (Manager, MonadHttp) +import Bilge (Manager) import Brig.Options qualified as Opt -import Control.Monad.Catch (MonadCatch) import Control.Retry () import Data.ByteString.Conversion (toByteString) import Data.Handle (fromHandle) @@ -39,8 +38,6 @@ import Wire.API.User (User (..), userEmail, userId) import Wire.API.User.Identity hiding (toByteString) import Wire.API.User.Search -type TestConstraints m = (MonadFail m, MonadCatch m, MonadIO m, MonadHttp m) - tests :: Opt.Opts -> Manager -> Galley -> Brig -> IO TestTree tests opts mgr _galley brig = do pure $ From 5ce7a6936861f64ede8c604d5adf6f7882b79261 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Wed, 9 Sep 2026 12:35:08 +0200 Subject: [PATCH 08/20] Make ES index recreation more relaxed about galley lookup failures. The Problem: User accounts can legitimately be in an inconsistent state, eg., because a team is in the process of being deleted: Team member entry is already gone, but brig account entry still exists. This causes `brig-index`'s bulk sync to fail because Galley returns 404 for team-related lookups. (The `try`-wrapped calls produce `Left SomeException`; `mkUserDoc`, `mkDocVersion` handle this by skipping the account, `ReindexSameOrNewer` throws.) Evidence from the logs coinciding with a failing index migration: ``` 2026-09-02 13:53:42.640 error { "error": "RPCException {"remote" = "galley", "path" = "i/teams/645c4b69-46f1-49a8-be57-35374d0aa947/features/searchVisibilityInbound", "headers" = [("Request-Id","brig-index")], "cause" = HttpExceptionRequest Request { host = "galley" port = 8080 secure = False requestHeaders = [("Request-Id","brig-index")] path = "i/teams/645c4b69-46f1-49a8-be57-35374d0aa947/features/searchVisibilityInbound" queryString = "" method = "GET" proxy = Nothing rawBody = False redirectCount = 10 responseTimeout = ResponseTimeoutDefault requestVersion = HTTP/1.1 proxySecureMode = ProxySecureWithConnect } (Response {responseStatus = Status {statusCode = 404, statusMessage = "Not Found"}, responseVersion = HTTP/1.1, responseHeaders = [("Transfer-Encoding","chunked"),("Date","Wed, 02 Sep 2026 11:53:35 GMT"), ("traceparent","00-f96c377d85487f15da6832eed1144233-1f618513676d5162-01"),("tracestate",""),("Content-Encoding","gzip"),("Content-Type","application/json"),("Vary","Accept-Encoding")], responseBody = (), responseCookieJar = CJ {expose = []}, responseClose' = ResponseClose, responseOriginalRequest = ... responseEarlyHints = []}) "{\"code\":404,\"label\":\"no-team\",\"message\":\"Team not found\"}")}", "userId": "19d790a4-8ac3-4fd9-81a9-ef24af8c8bf9", "request": "brig-index", "msgs": [ "E", "Error ocurred while indexing user" ] } ``` The solution is graceful degradation in Galley lookups (search visibility, roles): when Galley says "team not found" (or any Galley error), treat it as "team no longer exists" and fall back to safe defaults instead of skipping the user entirely. --- .../IndexedUserStore/Bulk/ElasticSearch.hs | 51 +++++++++---------- 1 file changed, 23 insertions(+), 28 deletions(-) diff --git a/libs/wire-subsystems/src/Wire/IndexedUserStore/Bulk/ElasticSearch.hs b/libs/wire-subsystems/src/Wire/IndexedUserStore/Bulk/ElasticSearch.hs index 829daffa12..93ba572d5a 100644 --- a/libs/wire-subsystems/src/Wire/IndexedUserStore/Bulk/ElasticSearch.hs +++ b/libs/wire-subsystems/src/Wire/IndexedUserStore/Bulk/ElasticSearch.hs @@ -20,7 +20,7 @@ module Wire.IndexedUserStore.Bulk.ElasticSearch where import Cassandra.Exec (paginateWithStateC) import Cassandra.Util (Writetime (Writetime)) import Conduit (ConduitT, runConduit, (.|)) -import Control.Error (headMay) +import Control.Error (headMay, hush) import Control.Exception (try) import Data.Conduit.Combinators qualified as Conduit import Data.Conduit.Internal (zipSources) @@ -104,25 +104,23 @@ syncAllUsersWithVersion interpreter pageSize mkVersion = teams = Map.fromListWith (<>) $ mapMaybe (\u -> (,[u]) <$> u.teamId) page visMap <- fmap Map.fromList . pooledForConcurrentlyN 16 (Map.keys teams) $ \t -> do - x <- try $ interpreter $ teamSearchVisibilityInbound t + x <- try @SomeException $ interpreter $ teamSearchVisibilityInbound t pure (t, x) - let getRoles :: TeamId -> [UserId] -> IO (Map UserId (Either SomeException (WithWritetime Role))) + let -- Accounts that have no team, can have no role. If a role + -- can't be found on brig for an account, that account does + -- not have role info in their index or document any more. + -- This is fine because it only affects accounts that are + -- already inconsistent accross cassandras (user entry with + -- team ref, but no team member entry). + getRoles :: TeamId -> [UserId] -> IO (Map UserId (WithWritetime Role)) getRoles tid uids = do - eithMembers <- try $ interpreter $ (.members) <$> selectTeamMemberInfos tid uids - case eithMembers of - Left e -> do - let lenUids = length uids - if lenUids <= 1 - then pure . Map.fromList $ map (,Left e) uids - else do - let (uids1, uids2) = splitAt (lenUids `div` 2) uids - roles1 <- getRoles tid uids1 - roles2 <- getRoles tid uids2 - pure $ Map.union roles1 roles2 - Right tms -> pure . Map.fromList $ mapMaybe (fmap rightSecond . mkRoleWithWriteTime) tms - - roles :: Map UserId (Either SomeException (WithWritetime Role)) <- + eithMembers <- try @SomeException $ interpreter $ (.members) <$> selectTeamMemberInfos tid uids + pure case eithMembers of + Left _ -> Map.empty + Right tms -> Map.fromList $ mapMaybe mkRoleWithWriteTime tms + + roles :: Map UserId (WithWritetime Role) <- fmap Map.unions . pooledForConcurrentlyN 16 (Map.toList teams) $ \(t, us) -> getRoles t (fmap (.userId) us) @@ -132,21 +130,21 @@ syncAllUsersWithVersion interpreter pageSize mkVersion = try . fmap (Map.fromListWith (<>) . map (\tc -> (gUser tc, [gTeam tc]))) . interpreter $ getTeamCollaborationsForUsers (Set.fromList (map (.userId) page)) - let vis :: IndexUser -> Either SomeException SearchVisibilityInbound + let vis :: IndexUser -> SearchVisibilityInbound vis indexUser = - fromMaybe (Right defaultSearchVisibilityInbound) $ flip Map.lookup visMap =<< indexUser.teamId + fromMaybe SearchableByOwnTeam $ hush =<< flip Map.lookup visMap =<< indexUser.teamId mkUserDoc :: IndexUser -> Either SomeException UserDoc mkUserDoc indexUser = do - currentVis <- vis indexUser - currentRole <- sequence $ Map.lookup indexUser.userId roles + let currentVis = vis indexUser + currentRole = ((.value)) <$> Map.lookup indexUser.userId roles currentCollabTeams <- Map.findWithDefault [] indexUser.userId <$> eithCollabTeams - pure $ indexUserToDoc currentVis ((.value) <$> currentRole) currentCollabTeams indexUser + pure $ indexUserToDoc currentVis currentRole currentCollabTeams indexUser mkDocVersion :: IndexUser -> Either SomeException ES.VersionControl - mkDocVersion u = do - roleWithTime <- sequence (Map.lookup u.userId roles) - pure . mkVersion . ES.ExternalDocVersion . docVersion $ indexUserToVersion roleWithTime u + mkDocVersion u = + let roleWithTime = Map.lookup u.userId roles + in pure . mkVersion . ES.ExternalDocVersion . docVersion $ indexUserToVersion roleWithTime u docsWithErrors :: (e ~ Either SomeException) => [(ES.DocId, e UserDoc, e ES.VersionControl)] docsWithErrors = map (\u -> (userIdToDocId u.userId, mkUserDoc u, mkDocVersion u)) page @@ -157,9 +155,6 @@ syncAllUsersWithVersion interpreter pageSize mkVersion = cappedErrors = take 1000 errors pure (skipped, cappedErrors, docs) - rightSecond :: (a, b) -> (a, Either c b) - rightSecond (a, b) = (a, Right b) - logFailures :: (Member TinyLog r) => (ES.DocId, Either SomeException UserDoc, Either SomeException ES.VersionControl) -> Sem r (Either String (ES.DocId, UserDoc, ES.VersionControl)) logFailures (docId@(ES.DocId idText), eithUserDoc, eithVersion) = case (,) <$> eithUserDoc <*> eithVersion of From f0cabb4f806c0a512195f20a8e6b1c137677e89b Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Thu, 10 Sep 2026 10:23:33 +0200 Subject: [PATCH 09/20] Remove stale changelog entry. --- .../WPB-27169-include-collaborators-in-contact-search | 1 - 1 file changed, 1 deletion(-) delete mode 100644 changelog.d/0-release-notes/WPB-27169-include-collaborators-in-contact-search diff --git a/changelog.d/0-release-notes/WPB-27169-include-collaborators-in-contact-search b/changelog.d/0-release-notes/WPB-27169-include-collaborators-in-contact-search deleted file mode 100644 index af10c6dd3c..0000000000 --- a/changelog.d/0-release-notes/WPB-27169-include-collaborators-in-contact-search +++ /dev/null @@ -1 +0,0 @@ -Include collaborators in contact search. From 9ace988d34d89f288a3f7495d8cc66e138dfbc39 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Thu, 10 Sep 2026 19:05:31 +0200 Subject: [PATCH 10/20] Refactor. --- .../Wire/IndexedUserStore/Bulk/ElasticSearch.hs | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/libs/wire-subsystems/src/Wire/IndexedUserStore/Bulk/ElasticSearch.hs b/libs/wire-subsystems/src/Wire/IndexedUserStore/Bulk/ElasticSearch.hs index 93ba572d5a..6e8fb9ebc5 100644 --- a/libs/wire-subsystems/src/Wire/IndexedUserStore/Bulk/ElasticSearch.hs +++ b/libs/wire-subsystems/src/Wire/IndexedUserStore/Bulk/ElasticSearch.hs @@ -103,10 +103,6 @@ syncAllUsersWithVersion interpreter pageSize mkVersion = let teams :: Map TeamId [IndexUser] teams = Map.fromListWith (<>) $ mapMaybe (\u -> (,[u]) <$> u.teamId) page - visMap <- fmap Map.fromList . pooledForConcurrentlyN 16 (Map.keys teams) $ \t -> do - x <- try @SomeException $ interpreter $ teamSearchVisibilityInbound t - pure (t, x) - let -- Accounts that have no team, can have no role. If a role -- can't be found on brig for an account, that account does -- not have role info in their index or document any more. @@ -130,11 +126,14 @@ syncAllUsersWithVersion interpreter pageSize mkVersion = try . fmap (Map.fromListWith (<>) . map (\tc -> (gUser tc, [gTeam tc]))) . interpreter $ getTeamCollaborationsForUsers (Set.fromList (map (.userId) page)) - let vis :: IndexUser -> SearchVisibilityInbound - vis indexUser = - fromMaybe SearchableByOwnTeam $ hush =<< flip Map.lookup visMap =<< indexUser.teamId + vis :: IndexUser -> SearchVisibilityInbound <- do + visMap <- fmap Map.fromList . pooledForConcurrentlyN 16 (Map.keys teams) $ \t -> do + x <- try @SomeException $ interpreter $ teamSearchVisibilityInbound t + pure (t, x) + pure $ \vis indexUser -> + fromMaybe SearchableByOwnTeam $ hush =<< flip Map.lookup visMap =<< indexUser.teamId - mkUserDoc :: IndexUser -> Either SomeException UserDoc + let mkUserDoc :: IndexUser -> Either SomeException UserDoc mkUserDoc indexUser = do let currentVis = vis indexUser currentRole = ((.value)) <$> Map.lookup indexUser.userId roles From df324ce43a4a9b8cbdcce6c08d3785c10c418a5a Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Thu, 10 Sep 2026 19:07:15 +0200 Subject: [PATCH 11/20] Refactor. --- .../IndexedUserStore/Bulk/ElasticSearch.hs | 54 +++++++++---------- 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/libs/wire-subsystems/src/Wire/IndexedUserStore/Bulk/ElasticSearch.hs b/libs/wire-subsystems/src/Wire/IndexedUserStore/Bulk/ElasticSearch.hs index 6e8fb9ebc5..08b5a56d18 100644 --- a/libs/wire-subsystems/src/Wire/IndexedUserStore/Bulk/ElasticSearch.hs +++ b/libs/wire-subsystems/src/Wire/IndexedUserStore/Bulk/ElasticSearch.hs @@ -103,29 +103,23 @@ syncAllUsersWithVersion interpreter pageSize mkVersion = let teams :: Map TeamId [IndexUser] teams = Map.fromListWith (<>) $ mapMaybe (\u -> (,[u]) <$> u.teamId) page - let -- Accounts that have no team, can have no role. If a role - -- can't be found on brig for an account, that account does - -- not have role info in their index or document any more. - -- This is fine because it only affects accounts that are - -- already inconsistent accross cassandras (user entry with - -- team ref, but no team member entry). - getRoles :: TeamId -> [UserId] -> IO (Map UserId (WithWritetime Role)) - getRoles tid uids = do - eithMembers <- try @SomeException $ interpreter $ (.members) <$> selectTeamMemberInfos tid uids - pure case eithMembers of - Left _ -> Map.empty - Right tms -> Map.fromList $ mapMaybe mkRoleWithWriteTime tms - - roles :: Map UserId (WithWritetime Role) <- + roles :: Map UserId (WithWritetime Role) <- do + let -- Accounts that have no team, can have no role. If a role + -- can't be found on brig for an account, that account does + -- not have role info in their index or document any more. + -- This is fine because it only affects accounts that are + -- already inconsistent accross cassandras (user entry with + -- team ref, but no team member entry). + getRoles :: TeamId -> [UserId] -> IO (Map UserId (WithWritetime Role)) + getRoles tid uids = do + eithMembers <- try @SomeException $ interpreter $ (.members) <$> selectTeamMemberInfos tid uids + pure case eithMembers of + Left _ -> Map.empty + Right tms -> Map.fromList $ mapMaybe mkRoleWithWriteTime tms + fmap Map.unions . pooledForConcurrentlyN 16 (Map.toList teams) $ \(t, us) -> getRoles t (fmap (.userId) us) - -- One query for the whole page. A failure here fails every document of the - -- page, which 'logFailures' then logs and skips. - eithCollabTeams :: Either SomeException (Map UserId [TeamId]) <- - try . fmap (Map.fromListWith (<>) . map (\tc -> (gUser tc, [gTeam tc]))) . interpreter $ - getTeamCollaborationsForUsers (Set.fromList (map (.userId) page)) - vis :: IndexUser -> SearchVisibilityInbound <- do visMap <- fmap Map.fromList . pooledForConcurrentlyN 16 (Map.keys teams) $ \t -> do x <- try @SomeException $ interpreter $ teamSearchVisibilityInbound t @@ -133,14 +127,20 @@ syncAllUsersWithVersion interpreter pageSize mkVersion = pure $ \vis indexUser -> fromMaybe SearchableByOwnTeam $ hush =<< flip Map.lookup visMap =<< indexUser.teamId - let mkUserDoc :: IndexUser -> Either SomeException UserDoc - mkUserDoc indexUser = do - let currentVis = vis indexUser - currentRole = ((.value)) <$> Map.lookup indexUser.userId roles - currentCollabTeams <- Map.findWithDefault [] indexUser.userId <$> eithCollabTeams - pure $ indexUserToDoc currentVis currentRole currentCollabTeams indexUser + mkUserDoc :: IndexUser -> Either SomeException UserDoc <- do + -- One query for the whole page. A failure here fails every document of the + -- page, which 'logFailures' then logs and skips. + eithCollabTeams :: Either SomeException (Map UserId [TeamId]) <- + try . fmap (Map.fromListWith (<>) . map (\tc -> (gUser tc, [gTeam tc]))) . interpreter $ + getTeamCollaborationsForUsers (Set.fromList (map (.userId) page)) + + pure \indexUser -> do + let currentVis = vis indexUser + currentRole = ((.value)) <$> Map.lookup indexUser.userId roles + currentCollabTeams <- Map.findWithDefault [] indexUser.userId <$> eithCollabTeams + pure $ indexUserToDoc currentVis currentRole currentCollabTeams indexUser - mkDocVersion :: IndexUser -> Either SomeException ES.VersionControl + let mkDocVersion :: IndexUser -> Either SomeException ES.VersionControl mkDocVersion u = let roleWithTime = Map.lookup u.userId roles in pure . mkVersion . ES.ExternalDocVersion . docVersion $ indexUserToVersion roleWithTime u From 31b6e6558622a9eb7c90c14a9535faa05145b272 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Fri, 11 Sep 2026 09:58:58 +0200 Subject: [PATCH 12/20] Refactor. --- .../IndexedUserStore/Bulk/ElasticSearch.hs | 23 ++++++++----------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/libs/wire-subsystems/src/Wire/IndexedUserStore/Bulk/ElasticSearch.hs b/libs/wire-subsystems/src/Wire/IndexedUserStore/Bulk/ElasticSearch.hs index 08b5a56d18..51de864dd9 100644 --- a/libs/wire-subsystems/src/Wire/IndexedUserStore/Bulk/ElasticSearch.hs +++ b/libs/wire-subsystems/src/Wire/IndexedUserStore/Bulk/ElasticSearch.hs @@ -93,17 +93,10 @@ syncAllUsersWithVersion interpreter pageSize mkVersion = -- those skipped users. mkUserDocs :: ConduitT [IndexUser] (Int, [String], [(ES.DocId, UserDoc, ES.VersionControl)]) IO () mkUserDocs = Conduit.mapM $ \page -> do - -- FUTUREWORK: extract team visibilities, roles and user type - -- more efficiently sending one query per page - - -- FUTUREWORK: introduce type ExtendedUser (or something), which - -- contains User, Maybe Role, UserType, ..., and pass around - -- ExtendedUser. this should make the code less convoluted. - let teams :: Map TeamId [IndexUser] teams = Map.fromListWith (<>) $ mapMaybe (\u -> (,[u]) <$> u.teamId) page - roles :: Map UserId (WithWritetime Role) <- do + lookupRole :: UserId -> Maybe (WithWritetime Role) <- do let -- Accounts that have no team, can have no role. If a role -- can't be found on brig for an account, that account does -- not have role info in their index or document any more. @@ -117,14 +110,16 @@ syncAllUsersWithVersion interpreter pageSize mkVersion = Left _ -> Map.empty Right tms -> Map.fromList $ mapMaybe mkRoleWithWriteTime tms - fmap Map.unions . pooledForConcurrentlyN 16 (Map.toList teams) $ \(t, us) -> + rolesMap <- fmap Map.unions . pooledForConcurrentlyN 16 (Map.toList teams) $ \(t, us) -> getRoles t (fmap (.userId) us) - vis :: IndexUser -> SearchVisibilityInbound <- do + pure $ \uid -> Map.lookup uid rolesMap + + lookupVisibility :: IndexUser -> SearchVisibilityInbound <- do visMap <- fmap Map.fromList . pooledForConcurrentlyN 16 (Map.keys teams) $ \t -> do x <- try @SomeException $ interpreter $ teamSearchVisibilityInbound t pure (t, x) - pure $ \vis indexUser -> + pure $ \indexUser -> fromMaybe SearchableByOwnTeam $ hush =<< flip Map.lookup visMap =<< indexUser.teamId mkUserDoc :: IndexUser -> Either SomeException UserDoc <- do @@ -135,14 +130,14 @@ syncAllUsersWithVersion interpreter pageSize mkVersion = getTeamCollaborationsForUsers (Set.fromList (map (.userId) page)) pure \indexUser -> do - let currentVis = vis indexUser - currentRole = ((.value)) <$> Map.lookup indexUser.userId roles + let currentVis = lookupVisibility indexUser + currentRole = ((.value)) <$> lookupRole indexUser.userId currentCollabTeams <- Map.findWithDefault [] indexUser.userId <$> eithCollabTeams pure $ indexUserToDoc currentVis currentRole currentCollabTeams indexUser let mkDocVersion :: IndexUser -> Either SomeException ES.VersionControl mkDocVersion u = - let roleWithTime = Map.lookup u.userId roles + let roleWithTime = lookupRole u.userId in pure . mkVersion . ES.ExternalDocVersion . docVersion $ indexUserToVersion roleWithTime u docsWithErrors :: (e ~ Either SomeException) => [(ES.DocId, e UserDoc, e ES.VersionControl)] From 71cba70f463b7249ab0bd436d4d37a729e9c940a Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Fri, 11 Sep 2026 10:44:42 +0200 Subject: [PATCH 13/20] Refactor. --- .../src/Wire/IndexedUserStore/Bulk/ElasticSearch.hs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/libs/wire-subsystems/src/Wire/IndexedUserStore/Bulk/ElasticSearch.hs b/libs/wire-subsystems/src/Wire/IndexedUserStore/Bulk/ElasticSearch.hs index 51de864dd9..a410e290c0 100644 --- a/libs/wire-subsystems/src/Wire/IndexedUserStore/Bulk/ElasticSearch.hs +++ b/libs/wire-subsystems/src/Wire/IndexedUserStore/Bulk/ElasticSearch.hs @@ -149,7 +149,10 @@ syncAllUsersWithVersion interpreter pageSize mkVersion = cappedErrors = take 1000 errors pure (skipped, cappedErrors, docs) - logFailures :: (Member TinyLog r) => (ES.DocId, Either SomeException UserDoc, Either SomeException ES.VersionControl) -> Sem r (Either String (ES.DocId, UserDoc, ES.VersionControl)) + logFailures :: + (Member TinyLog r) => + (ES.DocId, Either SomeException UserDoc, Either SomeException ES.VersionControl) -> + Sem r (Either String (ES.DocId, UserDoc, ES.VersionControl)) logFailures (docId@(ES.DocId idText), eithUserDoc, eithVersion) = case (,) <$> eithUserDoc <*> eithVersion of Left e -> do @@ -197,7 +200,7 @@ migrateData interpreter pageSize = interpreter $ do . Log.field "expectedVersion" expectedMigrationVersion . Log.field "skippedUsers" skipped . Log.field "errors" (show errors) - throw $ SyncIncomplete + throw SyncIncomplete else do Log.info $ Log.msg (Log.val "No migration necessary.") From 630ca1657721a5a5aa531496d0edc83a099e8066 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Fri, 11 Sep 2026 11:52:58 +0200 Subject: [PATCH 14/20] Fail the whole index page when galley's role lookup errors. `selectTeamMemberInfos` always returns a member list on 2xx, so a missing entry just means "no role" (an account inconsistent across cassandras). A non-2xx response, however, is a real error: swallowing it wrote documents with both a wrong role and a wrong version, making the damage permanent. The new behavior is the same as for collaborators. --- .../IndexedUserStore/Bulk/ElasticSearch.hs | 81 ++++++++++------ .../unit/Wire/IndexedUserStore/BulkSpec.hs | 94 +++++++++++++++++++ libs/wire-subsystems/wire-subsystems.cabal | 1 + 3 files changed, 146 insertions(+), 30 deletions(-) create mode 100644 libs/wire-subsystems/test/unit/Wire/IndexedUserStore/BulkSpec.hs diff --git a/libs/wire-subsystems/src/Wire/IndexedUserStore/Bulk/ElasticSearch.hs b/libs/wire-subsystems/src/Wire/IndexedUserStore/Bulk/ElasticSearch.hs index a410e290c0..9f453b1078 100644 --- a/libs/wire-subsystems/src/Wire/IndexedUserStore/Bulk/ElasticSearch.hs +++ b/libs/wire-subsystems/src/Wire/IndexedUserStore/Bulk/ElasticSearch.hs @@ -96,24 +96,30 @@ syncAllUsersWithVersion interpreter pageSize mkVersion = let teams :: Map TeamId [IndexUser] teams = Map.fromListWith (<>) $ mapMaybe (\u -> (,[u]) <$> u.teamId) page - lookupRole :: UserId -> Maybe (WithWritetime Role) <- do - let -- Accounts that have no team, can have no role. If a role - -- can't be found on brig for an account, that account does - -- not have role info in their index or document any more. - -- This is fine because it only affects accounts that are - -- already inconsistent accross cassandras (user entry with - -- team ref, but no team member entry). - getRoles :: TeamId -> [UserId] -> IO (Map UserId (WithWritetime Role)) - getRoles tid uids = do - eithMembers <- try @SomeException $ interpreter $ (.members) <$> selectTeamMemberInfos tid uids - pure case eithMembers of - Left _ -> Map.empty - Right tms -> Map.fromList $ mapMaybe mkRoleWithWriteTime tms - - rolesMap <- fmap Map.unions . pooledForConcurrentlyN 16 (Map.toList teams) $ \(t, us) -> + lookupRole :: UserId -> Either SomeException (Maybe (WithWritetime Role)) <- do + let -- NB: `selectTeamMemberInfos` conveniently always returns a + -- member list, even if some or all users or the team don't + -- exist. So a *missing* entry merely means "this account has no + -- role", which is fine: it only affects accounts that are + -- already inconsistent accross cassandras (user entry with team + -- ref, but no team member entry). A non-2xx response, on the + -- other hand, means some real error occurred, and must fail the + -- whole page rather than silently drop the role. + getRoles :: TeamId -> [UserId] -> IO (Either SomeException (Map UserId (WithWritetime Role))) + getRoles tid uids = + try @SomeException . interpreter $ + rolesFromMemberInfos . (.members) <$> selectTeamMemberInfos tid uids + + results <- pooledForConcurrentlyN 16 (Map.toList teams) $ \(t, us) -> getRoles t (fmap (.userId) us) - pure $ \uid -> Map.lookup uid rolesMap + -- log the root cause once per page, rather than once per user + for_ (lefts results) $ \e -> + interpreter . Log.err $ + Log.msg (Log.val "Failed to look up team member roles; skipping this page") + . Log.field "error" (show e) + + pure $ mkRoleLookup results lookupVisibility :: IndexUser -> SearchVisibilityInbound <- do visMap <- fmap Map.fromList . pooledForConcurrentlyN 16 (Map.keys teams) $ \t -> do @@ -131,14 +137,14 @@ syncAllUsersWithVersion interpreter pageSize mkVersion = pure \indexUser -> do let currentVis = lookupVisibility indexUser - currentRole = ((.value)) <$> lookupRole indexUser.userId + currentRole <- fmap (.value) <$> lookupRole indexUser.userId currentCollabTeams <- Map.findWithDefault [] indexUser.userId <$> eithCollabTeams pure $ indexUserToDoc currentVis currentRole currentCollabTeams indexUser let mkDocVersion :: IndexUser -> Either SomeException ES.VersionControl - mkDocVersion u = - let roleWithTime = lookupRole u.userId - in pure . mkVersion . ES.ExternalDocVersion . docVersion $ indexUserToVersion roleWithTime u + mkDocVersion u = do + roleWithTime <- lookupRole u.userId + pure . mkVersion . ES.ExternalDocVersion . docVersion $ indexUserToVersion roleWithTime u docsWithErrors :: (e ~ Either SomeException) => [(ES.DocId, e UserDoc, e ES.VersionControl)] docsWithErrors = map (\u -> (userIdToDocId u.userId, mkUserDoc u, mkDocVersion u)) page @@ -163,17 +169,32 @@ syncAllUsersWithVersion interpreter pageSize mkVersion = pure $ Left $ show idText <> ": " <> show e Right (userDoc, version) -> pure $ Right (docId, userDoc, version) - mkRoleWithWriteTime :: TeamMemberInfo -> Maybe (UserId, WithWritetime Role) - mkRoleWithWriteTime tmi = - ( \role -> - ( tmi.userId, - WithWriteTime - { value = role, - writetime = Writetime $ fromUTCTimeMillis tmi.permissionsWriteTime - } - ) +mkRoleWithWriteTime :: TeamMemberInfo -> Maybe (UserId, WithWritetime Role) +mkRoleWithWriteTime tmi = + ( \role -> + ( tmi.userId, + WithWriteTime + { value = role, + writetime = Writetime $ fromUTCTimeMillis tmi.permissionsWriteTime + } ) - <$> permissionsToRole tmi.permissions + ) + <$> permissionsToRole tmi.permissions + +-- | The roles of one team, extracted from a *successful* galley response. +-- Users galley does not know about simply do not show up in the result; that +-- is not an error. +rolesFromMemberInfos :: [TeamMemberInfo] -> Map UserId (WithWritetime Role) +rolesFromMemberInfos = Map.fromList . mapMaybe mkRoleWithWriteTime + +-- | Fold the per-team lookup results of one page into a single lookup +-- function. A galley error for any team fails the entire page: every +-- document derived from this lookup will be logged and skipped by +-- 'logFailures'. +mkRoleLookup :: + [Either SomeException (Map UserId (WithWritetime Role))] -> + (UserId -> Either SomeException (Maybe (WithWritetime Role))) +mkRoleLookup results uid = Map.lookup uid . Map.unions <$> sequence results migrateData :: (Member (Embed IO) r, Member IndexedUserStore r, Member (Error MigrationException) r, Member IndexedUserMigrationStore r, Member TinyLog r, Member UserStore r, Member GalleyAPIAccess r, Member TeamCollaboratorsStore r) => diff --git a/libs/wire-subsystems/test/unit/Wire/IndexedUserStore/BulkSpec.hs b/libs/wire-subsystems/test/unit/Wire/IndexedUserStore/BulkSpec.hs new file mode 100644 index 0000000000..82df2ea9d5 --- /dev/null +++ b/libs/wire-subsystems/test/unit/Wire/IndexedUserStore/BulkSpec.hs @@ -0,0 +1,94 @@ +-- This file is part of the Wire Server implementation. +-- +-- Copyright (C) 2025 Wire Swiss GmbH +-- +-- This program is free software: you can redistribute it and/or modify it under +-- the terms of the GNU Affero General Public License as published by the Free +-- Software Foundation, either version 3 of the License, or (at your option) any +-- later version. +-- +-- This program is distributed in the hope that it will be useful, but WITHOUT +-- ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS +-- FOR A PARTICULAR PURPOSE. See the GNU Affero General Public License for more +-- details. +-- +-- You should have received a copy of the GNU Affero General Public License along +-- with this program. If not, see . + +module Wire.IndexedUserStore.BulkSpec where + +import Cassandra.Util (Writetime (Writetime)) +import Control.Exception (ErrorCall (..)) +import Data.Id +import Data.Json.Util (toUTCTimeMillis) +import Data.Map qualified as Map +import Data.Time (UTCTime (UTCTime), fromGregorian) +import Data.UUID qualified as UUID +import Imports +import Test.Hspec +import Wire.API.Team.Member.Info (TeamMemberInfo (..)) +import Wire.API.Team.Permission (Permissions, fullPermissions, noPermissions) +import Wire.API.Team.Role (Role (..)) +import Wire.IndexedUserStore.Bulk.ElasticSearch (mkRoleLookup, rolesFromMemberInfos) +import Wire.UserStore.IndexUser (WithWritetime (..)) + +spec :: Spec +spec = do + describe "rolesFromMemberInfos" $ do + it "keeps members whose permissions map to a role, with their writetime" $ do + rolesFromMemberInfos [memberInfo uid1 fullPermissions] + `shouldBe` Map.fromList [(uid1, withWritetime RoleOwner)] + + it "drops members whose permissions map to no role" $ do + rolesFromMemberInfos [memberInfo uid1 noPermissions] `shouldBe` mempty + + describe "mkRoleLookup" $ do + it "finds roles across all the teams of a page" $ do + let lookupRole = mkRoleLookup [Right (roleMap uid1 RoleOwner), Right (roleMap uid2 RoleMember)] + simplify (lookupRole uid1) `shouldBe` Right (Just (withWritetime RoleOwner)) + simplify (lookupRole uid2) `shouldBe` Right (Just (withWritetime RoleMember)) + + -- galley answered 2xx, it just doesn't have a team member entry for this + -- account: that means "no role", not "error". + it "treats a member galley does not know about as role-less" $ do + let lookupRole = mkRoleLookup [Right (roleMap uid1 RoleOwner)] + simplify (lookupRole uid2) `shouldBe` Right Nothing + + -- galley answered non-2xx: we cannot tell role-less accounts from accounts + -- whose role we failed to fetch, so the whole page has to fail. + it "fails every member of the page if the lookup failed for any team" $ do + let lookupRole = + mkRoleLookup + [ Right (roleMap uid1 RoleOwner), + Left (toException (ErrorCall "galley is down")) + ] + lookupRole uid1 `shouldSatisfy` isLeft + lookupRole uid2 `shouldSatisfy` isLeft + + it "is role-less, not failing, on an empty page" $ do + simplify (mkRoleLookup [] uid1) `shouldBe` Right Nothing + where + uid1, uid2 :: UserId + uid1 = Id $ UUID.fromWords 1 1 1 1 + uid2 = Id $ UUID.fromWords 2 2 2 2 + + writeTime :: UTCTime + writeTime = UTCTime (fromGregorian 2026 9 11) 0 + + memberInfo :: UserId -> Permissions -> TeamMemberInfo + memberInfo uid perms = + TeamMemberInfo + { userId = uid, + permissions = perms, + permissionsWriteTime = toUTCTimeMillis writeTime + } + + withWritetime :: Role -> WithWritetime Role + withWritetime role = WithWriteTime {value = role, writetime = Writetime writeTime} + + roleMap :: UserId -> Role -> Map UserId (WithWritetime Role) + roleMap uid role = Map.fromList [(uid, withWritetime role)] + + -- 'SomeException' has no 'Eq', so render it before comparing. + simplify :: Either SomeException a -> Either String a + simplify = either (Left . displayException) Right diff --git a/libs/wire-subsystems/wire-subsystems.cabal b/libs/wire-subsystems/wire-subsystems.cabal index 3d1c32c7af..439c7322b5 100644 --- a/libs/wire-subsystems/wire-subsystems.cabal +++ b/libs/wire-subsystems/wire-subsystems.cabal @@ -645,6 +645,7 @@ test-suite wire-subsystems-tests Wire.FederationSubsystem.InternalsSpec Wire.HashPassword.InterpreterSpec Wire.IdPSubsystem.InterpreterSpec + Wire.IndexedUserStore.BulkSpec Wire.MeetingNotifierSpec Wire.MeetingsSubsystem.InterpreterSpec Wire.MiniBackend From 4cce5c225f683abbd88c2706c10fe07f89cf6208 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Fri, 11 Sep 2026 13:59:42 +0200 Subject: [PATCH 15/20] Fix brig-index: (1) role lookup; (2) collaborator update. --- ...bump-index-version-on-collaboration-change | 4 +++ ...089-handle-ongoing-deletions-in-brig-index | 3 ++ integration/test/Test/TeamCollaborators.hs | 12 ++++++++ .../API/Routes/Internal/Brig/SearchIndex.hs | 15 ++++++++++ .../wire-subsystems/src/Wire/BrigAPIAccess.hs | 2 ++ .../src/Wire/BrigAPIAccess/Local.hs | 1 + .../src/Wire/BrigAPIAccess/Rpc.hs | 11 +++++++ .../IndexedUserStore/Bulk/ElasticSearch.hs | 27 +++++++++++++++++ .../TeamCollaboratorsSubsystem/Interpreter.hs | 16 ++++++---- libs/wire-subsystems/src/Wire/UserStore.hs | 6 ++++ .../src/Wire/UserStore/Cassandra.hs | 11 +++++++ .../src/Wire/UserStore/IndexUser.hs | 11 +++++++ .../src/Wire/UserStore/Postgres.hs | 13 +++++++++ .../wire-subsystems/src/Wire/UserSubsystem.hs | 5 ++++ .../src/Wire/UserSubsystem/Interpreter.hs | 29 ++++++++++++++++--- .../Wire/MockInterpreters/BrigAPIAccess.hs | 1 + .../unit/Wire/MockInterpreters/UserStore.hs | 2 ++ .../Wire/MockInterpreters/UserSubsystem.hs | 1 + services/brig/src/Brig/API/Internal.hs | 1 + 19 files changed, 161 insertions(+), 10 deletions(-) create mode 100644 changelog.d/3-bug-fixes/WPB-28089-bump-index-version-on-collaboration-change create mode 100644 changelog.d/3-bug-fixes/WPB-28089-handle-ongoing-deletions-in-brig-index diff --git a/changelog.d/3-bug-fixes/WPB-28089-bump-index-version-on-collaboration-change b/changelog.d/3-bug-fixes/WPB-28089-bump-index-version-on-collaboration-change new file mode 100644 index 0000000000..ecdb0c38be --- /dev/null +++ b/changelog.d/3-bug-fixes/WPB-28089-bump-index-version-on-collaboration-change @@ -0,0 +1,4 @@ +Adding, changing or removing a team collaboration now bumps the affected user's +search index version. Previously the updated document had the same version as +the one already in Elasticsearch, so `brig-index reindex` dropped it as a +version conflict and the collaborating teams in the index went stale. diff --git a/changelog.d/3-bug-fixes/WPB-28089-handle-ongoing-deletions-in-brig-index b/changelog.d/3-bug-fixes/WPB-28089-handle-ongoing-deletions-in-brig-index new file mode 100644 index 0000000000..de39f8aad8 --- /dev/null +++ b/changelog.d/3-bug-fixes/WPB-28089-handle-ongoing-deletions-in-brig-index @@ -0,0 +1,3 @@ +brig-index: tolerate accounts whose team member entry is already gone +(eg. during team deletion), but keep failing the affected page if +galley itself errors out. diff --git a/integration/test/Test/TeamCollaborators.hs b/integration/test/Test/TeamCollaborators.hs index 642dad537e..c0620713a9 100644 --- a/integration/test/Test/TeamCollaborators.hs +++ b/integration/test/Test/TeamCollaborators.hs @@ -419,7 +419,19 @@ testSearchFindsCollaborator = do multiCollabName <- multiCollab %. "name" & asString addTeamCollaborator owner team multiCollab ["implicit_connection"] >>= assertSuccess + -- NB: this second change to multiCollab's collaborations, like the removals + -- above, only reaches the index because repeating a write of the value that + -- is already stored still produces a fresh cassandra writetime -- that is what + -- advances the document version (see 'Wire.UserStore.BumpWriteTime'). Without + -- it the updated document would be dropped as a version conflict, so these + -- steps are the only coverage that mechanism has: please do not collapse them. addTeamCollaborator otherOwner otherTeam multiCollab ["implicit_connection"] >>= assertSuccess for_ [owner, alice] $ assertFinds multiCollabName [multiCollab] for_ [otherOwner, bob] $ assertFinds multiCollabName [multiCollab] + + -- Dropping one of the two collaborations leaves the other one in the index. + removeTeamCollaborator owner team multiCollab >>= assertSuccess + + for_ [owner, alice] $ assertFinds multiCollabName ([] @Value) + for_ [otherOwner, bob] $ assertFinds multiCollabName [multiCollab] diff --git a/libs/wire-api/src/Wire/API/Routes/Internal/Brig/SearchIndex.hs b/libs/wire-api/src/Wire/API/Routes/Internal/Brig/SearchIndex.hs index 016c41f082..65739505c1 100644 --- a/libs/wire-api/src/Wire/API/Routes/Internal/Brig/SearchIndex.hs +++ b/libs/wire-api/src/Wire/API/Routes/Internal/Brig/SearchIndex.hs @@ -39,3 +39,18 @@ type ISearchIndexAPI = :> Capture "userId" UserId :> Post '[JSON] NoContent ) + :<|> Named + "bump-write-time-and-update-search-index" + ( Summary "updates the search index for a single user, forcing the document version to advance" + :> Description + "Use this instead of `update-search-index` when the change that needs to be \ + \indexed does not live in the user record itself (currently: team \ + \collaborations). The index version is derived from the user record, so \ + \without bumping it the updated document would be rejected as a version \ + \conflict." + :> "index" + :> "update" + :> Capture "userId" UserId + :> "bump-write-time" + :> Post '[JSON] NoContent + ) diff --git a/libs/wire-subsystems/src/Wire/BrigAPIAccess.hs b/libs/wire-subsystems/src/Wire/BrigAPIAccess.hs index c23b679ed7..3db4223d86 100644 --- a/libs/wire-subsystems/src/Wire/BrigAPIAccess.hs +++ b/libs/wire-subsystems/src/Wire/BrigAPIAccess.hs @@ -112,6 +112,8 @@ data BrigAPIAccess m a where GetUserExportData :: UserId -> BrigAPIAccess m (Maybe TeamExportUser) DeleteBot :: ConvId -> BotId -> BrigAPIAccess m () UpdateSearchIndex :: UserId -> BrigAPIAccess m () + -- | See 'Wire.UserSubsystem.InternalBumpWriteTimeAndUpdateSearchIndex'. + BumpWriteTimeAndUpdateSearchIndex :: UserId -> BrigAPIAccess m () GetAccountsBy :: GetBy -> BrigAPIAccess m [User] GetUsersByVariousKeys :: [UserId] -> [Handle] -> [EmailAddress] -> HavePendingInvitations -> BrigAPIAccess m [User] CreateGroupInternal :: ManagedBy -> TeamId -> Maybe UserId -> NewUserGroup -> BrigAPIAccess m (Either Wai.Error UserGroup) diff --git a/libs/wire-subsystems/src/Wire/BrigAPIAccess/Local.hs b/libs/wire-subsystems/src/Wire/BrigAPIAccess/Local.hs index d89904b0a8..76546fdc2b 100644 --- a/libs/wire-subsystems/src/Wire/BrigAPIAccess/Local.hs +++ b/libs/wire-subsystems/src/Wire/BrigAPIAccess/Local.hs @@ -56,6 +56,7 @@ interpretBrigAPIAccessLocally :: InterpreterFor BrigAPIAccess r interpretBrigAPIAccessLocally selfEndpoint runUser = interpret $ \case UpdateSearchIndex uid -> runUser (UserSubsystem.internalUpdateSearchIndex uid) + BumpWriteTimeAndUpdateSearchIndex uid -> runUser (UserSubsystem.internalBumpWriteTimeAndUpdateSearchIndex uid) other -> selfRpc other where selfRpc :: forall m x. BrigAPIAccess m x -> Sem r x diff --git a/libs/wire-subsystems/src/Wire/BrigAPIAccess/Rpc.hs b/libs/wire-subsystems/src/Wire/BrigAPIAccess/Rpc.hs index b96d0abeea..9ae5cc7a65 100644 --- a/libs/wire-subsystems/src/Wire/BrigAPIAccess/Rpc.hs +++ b/libs/wire-subsystems/src/Wire/BrigAPIAccess/Rpc.hs @@ -143,6 +143,7 @@ brigAccessRpcHandler = \case DeleteBot convId botId -> deleteBot convId botId UpdateSearchIndex uid -> updateSearchIndex uid + BumpWriteTimeAndUpdateSearchIndex uid -> bumpWriteTimeAndUpdateSearchIndex uid GetAccountsBy localGetBy -> getAccountsBy localGetBy GetUsersByVariousKeys uids handles emails includePendingInvitations -> @@ -607,6 +608,16 @@ updateSearchIndex uid = do . paths ["i", "index", "update", toByteString' uid] . expect2xx +bumpWriteTimeAndUpdateSearchIndex :: + (Member Rpc r, Member (Input Endpoint) r) => + UserId -> + Sem r () +bumpWriteTimeAndUpdateSearchIndex uid = do + void . brigRequest $ + method POST + . paths ["i", "index", "update", toByteString' uid, "bump-write-time"] + . expect2xx + -- | Calls 'Brig.API.Internal.getAccountsByInternalH'. getAccountsBy :: (Member Rpc r, Member (Input Endpoint) r, Member (Error ParseException) r) => diff --git a/libs/wire-subsystems/src/Wire/IndexedUserStore/Bulk/ElasticSearch.hs b/libs/wire-subsystems/src/Wire/IndexedUserStore/Bulk/ElasticSearch.hs index 9f453b1078..bcbf95e03a 100644 --- a/libs/wire-subsystems/src/Wire/IndexedUserStore/Bulk/ElasticSearch.hs +++ b/libs/wire-subsystems/src/Wire/IndexedUserStore/Bulk/ElasticSearch.hs @@ -58,9 +58,36 @@ type IOInterpreter r = forall a. Sem r a -> IO a expectedMigrationVersion :: MigrationVersion expectedMigrationVersion = MigrationVersion 7 +-- | @brig-index reindex@: no-op-if-same sync. 'ES.ExternalGT' makes ES reject +-- any document whose version did not advance, so users that have not changed +-- since the last run cost nothing beyond being read and offered. +-- +-- This is only correct as long as the version really is a function of the +-- document -- i.e. as long as everything 'indexUserToDoc' puts into the +-- document is also reflected by 'indexUserToVersion'. Data that does not live +-- in the user record has to bump the version explicitly (see +-- 'Wire.UserStore.BumpWriteTime'), and the one deliberate exception, +-- 'udSearchVisibilityInbound', is maintained out of band (see +-- 'Wire.UserSubsystem.Interpreter.updateTeamSearchVisibilityInboundImpl'). +-- +-- When a document does turn up stale, the tempting repair is to +-- switch this call from 'ES.ExternalGT' to 'ES.ExternalGTE' so that +-- the write is accepted regardless. Please don't. It hides the +-- defect here without repairing it anywhere: the online, single-user +-- path in 'Wire.UserSubsystem.Interpreter.syncUserIndex' compares +-- versions the same way, so a version that fails to advance goes on +-- dropping updates there. And it costs the property this command +-- exists for -- accepting equal versions means rewriting every +-- document on every run, which makes this the same operation as +-- 'forceSyncAllUsers'. Fix stale documents by making the version +-- advance instead. syncAllUsers :: (Member UserStore r, Member IndexedUserStore r, Member TinyLog r, Member GalleyAPIAccess r, Member TeamCollaboratorsStore r) => IOInterpreter r -> Int32 -> IO (Int, [String]) syncAllUsers interpreter pageSize = syncAllUsersWithVersion interpreter pageSize ES.ExternalGT +-- | @brig-index reindex-if-same-or-newer@ and @migrate-data@: always-resync. +-- 'ES.ExternalGTE' rewrites documents even when the version is unchanged, which +-- is what you want when the mapping or the document shape itself changed and +-- the version therefore says nothing useful. Strictly older writes still lose. forceSyncAllUsers :: (Member UserStore r, Member IndexedUserStore r, Member TinyLog r, Member GalleyAPIAccess r, Member TeamCollaboratorsStore r) => IOInterpreter r -> Int32 -> IO (Int, [String]) forceSyncAllUsers interpreter pageSize = syncAllUsersWithVersion interpreter pageSize ES.ExternalGTE diff --git a/libs/wire-subsystems/src/Wire/TeamCollaboratorsSubsystem/Interpreter.hs b/libs/wire-subsystems/src/Wire/TeamCollaboratorsSubsystem/Interpreter.hs index bb0541636b..6889023756 100644 --- a/libs/wire-subsystems/src/Wire/TeamCollaboratorsSubsystem/Interpreter.hs +++ b/libs/wire-subsystems/src/Wire/TeamCollaboratorsSubsystem/Interpreter.hs @@ -93,8 +93,10 @@ createTeamCollaboratorImpl zUser user team perms = do generateTeamEvents (tUnqualified zUser) team [EdCollaboratorAdd user (Set.toList perms)] - -- Reindex the collaborator with their new collaboration team - BrigAPIAccess.updateSearchIndex user + -- Reindex the collaborator with their new collaboration team. Collaborations + -- are not part of the user record, so the index version has to be bumped + -- explicitly; see 'Wire.UserStore.BumpWriteTime'. + BrigAPIAccess.bumpWriteTimeAndUpdateSearchIndex user getAllTeamCollaboratorsImpl :: ( Member TeamSubsystem r, @@ -124,8 +126,9 @@ internalUpdateTeamCollaboratorImpl :: Sem r () internalUpdateTeamCollaboratorImpl user team perms = do Store.updateTeamCollaborator user team perms - -- Reindex collaborator when permissions change - BrigAPIAccess.updateSearchIndex user + -- Reindex collaborator when permissions change (see 'createTeamCollaboratorImpl' + -- for why the write time has to be bumped) + BrigAPIAccess.bumpWriteTimeAndUpdateSearchIndex user internalRemoveTeamCollaboratorImpl :: (Member Store.TeamCollaboratorsStore r, Member BrigAPIAccess r) => @@ -134,8 +137,9 @@ internalRemoveTeamCollaboratorImpl :: Sem r () internalRemoveTeamCollaboratorImpl user team = do Store.removeTeamCollaborator user team - -- Reindex collaborator when removed - BrigAPIAccess.updateSearchIndex user + -- Reindex collaborator when removed (see 'createTeamCollaboratorImpl' for why + -- the write time has to be bumped) + BrigAPIAccess.bumpWriteTimeAndUpdateSearchIndex user -- This is of general usefulness. However, we cannot move this to wire-api as -- this would lead to a cyclic dependency. diff --git a/libs/wire-subsystems/src/Wire/UserStore.hs b/libs/wire-subsystems/src/Wire/UserStore.hs index 46bc9e2ff5..3c879dc170 100644 --- a/libs/wire-subsystems/src/Wire/UserStore.hs +++ b/libs/wire-subsystems/src/Wire/UserStore.hs @@ -80,6 +80,12 @@ data UserStore m a where DeleteEmail :: UserId -> UserStore m () UpdateEmailUnvalidated :: UserId -> EmailAddress -> UserStore m () DeleteEmailUnvalidated :: UserId -> UserStore m () + -- | Advance the user's ES index version without otherwise changing the + -- user. 'indexUserToVersion' derives the version from the user row alone, + -- so a change to data that lives outside that row (team collaborations) + -- has to bump the row explicitly; otherwise the new document is rejected + -- as a version conflict. See 'Wire.UserSearch.Types.WriteTimeBumper'. + BumpWriteTime :: UserId -> UserStore m () UpdateUserHandleEither :: UserId -> StoredUserHandleUpdate -> UserStore m (Either StoredUserUpdateError ()) UpdateSSOId :: UserId -> Maybe UserSSOId -> UserStore m Bool UpdateManagedBy :: UserId -> ManagedBy -> UserStore m () diff --git a/libs/wire-subsystems/src/Wire/UserStore/Cassandra.hs b/libs/wire-subsystems/src/Wire/UserStore/Cassandra.hs index 54fefc2d48..884eb990e8 100644 --- a/libs/wire-subsystems/src/Wire/UserStore/Cassandra.hs +++ b/libs/wire-subsystems/src/Wire/UserStore/Cassandra.hs @@ -66,6 +66,7 @@ interpretUserStoreCassandra casClient = UpdateEmail uid email -> updateEmailImpl uid email UpdateEmailUnvalidated uid email -> updateEmailUnvalidatedImpl uid email DeleteEmailUnvalidated uid -> deleteEmailUnvalidatedImpl uid + BumpWriteTime uid -> bumpWriteTimeImpl uid UpdateUserHandleEither uid update -> updateUserHandleEitherImpl uid update UpdateSSOId uid ssoId -> updateSSOIdImpl uid ssoId UpdateManagedBy uid managedBy -> updateManagedByImpl uid managedBy @@ -147,6 +148,8 @@ interpretUserStoreToCassandraAndPostgres casClient = runAppropriateInterpreter casClient uid $ UserStore.updateEmailUnvalidated uid email DeleteEmailUnvalidated uid -> runAppropriateInterpreter casClient uid $ UserStore.deleteEmailUnvalidated uid + BumpWriteTime uid -> + runAppropriateInterpreter casClient uid $ UserStore.bumpWriteTime uid LookupName uid -> runAppropriateInterpreter casClient uid $ UserStore.lookupName uid LookupHandle hdl -> do @@ -576,6 +579,9 @@ getRichInfoImpl uid = deleteEmailImpl :: UserId -> Client () deleteEmailImpl u = retry x5 $ write userEmailDelete (params LocalQuorum (Identity u)) +bumpWriteTimeImpl :: UserId -> Client () +bumpWriteTimeImpl u = retry x5 $ write writeTimeBump (params LocalQuorum (Identity u)) + setUserSearchableImpl :: UserId -> SetSearchable -> Client () setUserSearchableImpl uid (SetSearchable searchable) = retry x5 $ write q (params LocalQuorum (searchable, uid)) where @@ -721,3 +727,8 @@ localeSelect = "SELECT language, country FROM user WHERE id = ?" userEmailDelete :: PrepQuery W (Identity UserId) () userEmailDelete = {- `IF EXISTS`, but that requires benchmarking -} "UPDATE user SET email = null, write_time_bumper = 0 WHERE id = ?" + +-- | Only the *writetime* of `write_time_bumper` matters; the value is +-- irrelevant. See 'Wire.UserSearch.Types.WriteTimeBumper'. +writeTimeBump :: PrepQuery W (Identity UserId) () +writeTimeBump = "UPDATE user SET write_time_bumper = 0 WHERE id = ?" diff --git a/libs/wire-subsystems/src/Wire/UserStore/IndexUser.hs b/libs/wire-subsystems/src/Wire/UserStore/IndexUser.hs index b051132f1e..18917b056a 100644 --- a/libs/wire-subsystems/src/Wire/UserStore/IndexUser.hs +++ b/libs/wire-subsystems/src/Wire/UserStore/IndexUser.hs @@ -122,6 +122,17 @@ indexUserFromTuple } {- ORMOLU_ENABLE -} +-- | Invariant: this has to advance whenever 'indexUserToDoc' would produce a +-- different document, otherwise the new document is dropped as a version +-- conflict by the no-op-if-same sync (see +-- 'Wire.IndexedUserStore.Bulk.ElasticSearch.syncAllUsers'). +-- +-- The writetimes of the user record cover most of the document, and the role's +-- writetime covers 'udRole'. Document data that lives elsewhere has to keep the +-- invariant by bumping the user record; see 'Wire.UserStore.BumpWriteTime', +-- which is what team collaborations do. 'udSearchVisibilityInbound' is the one +-- documented exception, see +-- 'Wire.UserSubsystem.Interpreter.updateTeamSearchVisibilityInboundImpl'. indexUserToVersion :: Maybe (WithWritetime Role) -> IndexUser -> IndexVersion indexUserToVersion role iu = mkIndexVersion [Just $ Writetime iu.updatedAt, const () <$$> fmap writetime role] diff --git a/libs/wire-subsystems/src/Wire/UserStore/Postgres.hs b/libs/wire-subsystems/src/Wire/UserStore/Postgres.hs index ca9e002bfc..0a4ee422b5 100644 --- a/libs/wire-subsystems/src/Wire/UserStore/Postgres.hs +++ b/libs/wire-subsystems/src/Wire/UserStore/Postgres.hs @@ -74,6 +74,7 @@ interpretUserStorePostgres = DeleteEmail uid -> updateEmailImpl uid Nothing UpdateEmailUnvalidated uid email -> updateEmailUnvalidatedImpl uid (Just email) DeleteEmailUnvalidated uid -> updateEmailUnvalidatedImpl uid Nothing + BumpWriteTime uid -> bumpWriteTimeImpl uid LookupName uid -> lookupNameImpl uid LookupHandle hdl -> lookupHandleImpl hdl GlimpseHandle hdl -> lookupHandleImpl hdl @@ -542,6 +543,18 @@ updateEmailUnvalidatedImpl uid email = lmapPG [resultlessStatement|UPDATE wire_user SET email_unvalidated = ($2 :: text?) WHERE id = ($1 :: uuid)|] +-- | The `update_user_updated_at` trigger refreshes `updated_at` on any update +-- of the row, but we set it explicitly so that this statement is not a no-op. +-- 'indexUserToVersion' reads `updated_at`; see 'UserStore.BumpWriteTime'. +bumpWriteTimeImpl :: (PGConstraints r) => UserId -> Sem r () +bumpWriteTimeImpl uid = + runStatement uid update + where + update :: Hasql.Statement UserId () + update = + lmapPG + [resultlessStatement|UPDATE wire_user SET updated_at = now() WHERE id = ($1 :: uuid)|] + updateEmailImpl :: (PGConstraints r) => UserId -> Maybe EmailAddress -> Sem r () updateEmailImpl uid email = runStatement (uid, email) update diff --git a/libs/wire-subsystems/src/Wire/UserSubsystem.hs b/libs/wire-subsystems/src/Wire/UserSubsystem.hs index 0f5f428ae7..1f1d673feb 100644 --- a/libs/wire-subsystems/src/Wire/UserSubsystem.hs +++ b/libs/wire-subsystems/src/Wire/UserSubsystem.hs @@ -181,6 +181,11 @@ data UserSubsystem m a where -- | The following "internal" functions exists to support migration in this susbystem, after the -- migration this would just be an internal detail of the subsystem InternalUpdateSearchIndex :: UserId -> UserSubsystem m () + -- | Like 'InternalUpdateSearchIndex', but additionally bumps the user's + -- index version (see 'Wire.UserStore.BumpWriteTime'). Use this, and only + -- this, when what changed does not live in the user row -- currently that + -- means team collaborations. + InternalBumpWriteTimeAndUpdateSearchIndex :: UserId -> UserSubsystem m () InternalFindTeamInvitation :: Maybe EmailKey -> InvitationCode -> UserSubsystem m StoredInvitation GetUserExportData :: UserId -> UserSubsystem m (Maybe TeamExportUser) RemoveEmailEither :: Local UserId -> UserSubsystem m (Either UserSubsystemError ()) diff --git a/libs/wire-subsystems/src/Wire/UserSubsystem/Interpreter.hs b/libs/wire-subsystems/src/Wire/UserSubsystem/Interpreter.hs index f78029cebd..b4204ba6df 100644 --- a/libs/wire-subsystems/src/Wire/UserSubsystem/Interpreter.hs +++ b/libs/wire-subsystems/src/Wire/UserSubsystem/Interpreter.hs @@ -200,6 +200,9 @@ runUserSubsystem authInterpreter appInterpreter clientInterpreter = browseTeamImpl uid browseTeamFilters mMaxResults mPagingState InternalUpdateSearchIndex uid -> syncUserIndex uid + InternalBumpWriteTimeAndUpdateSearchIndex uid -> do + UserStore.bumpWriteTime uid + syncUserIndex uid AcceptTeamInvitation luid pwd code -> acceptTeamInvitationImpl luid pwd code InternalFindTeamInvitation mEmailKey code -> @@ -870,10 +873,13 @@ syncUserIndex uid = collabTeams <- map gTeam <$> TeamCollaboratorsStore.getTeamCollaborations uid let mRole = tm >>= mkRoleWithWriteTime userDoc = indexUserToDoc vis (value <$> mRole) collabTeams indexUser - -- GTE, not GT: the version comes from the user row alone, but the document also - -- holds data that changes without touching that row (collaborations), and under - -- GT those updates would be dropped as version conflicts. Older writes still lose. - version = ES.ExternalGTE . ES.ExternalDocVersion . docVersion $ indexUserToVersion mRole indexUser + -- GT, not GTE: every change this document reflects also advances the + -- version, so a write that does not advance it has nothing new to say and + -- is correctly dropped as a version conflict. Data that does not live in + -- the user record keeps that invariant by bumping the version explicitly + -- (see 'Wire.UserStore.BumpWriteTime'); the single deliberate exception is + -- 'udSearchVisibilityInbound', see 'updateTeamSearchVisibilityInboundImpl'. + version = ES.ExternalGT . ES.ExternalDocVersion . docVersion $ indexUserToVersion mRole indexUser Metrics.incCounter indexUpdateCounter IndexedUserStore.upsert (userIdToDocId uid) userDoc version @@ -891,6 +897,21 @@ syncUserIndex uid = ) <$> permissionsToRole info.permissions +-- | 'udSearchVisibilityInbound' is the one field of 'UserDoc' that the index +-- version does not cover, and that is a deliberate design choice rather than the +-- same gap that 'Wire.UserStore.BumpWriteTime' closes for collaborations: +-- +-- * It is a team-wide setting. Propagating it the way collaborations are +-- propagated would mean bumping the write time of, and re-uploading a document +-- for, every member of the team -- for a single flag. +-- +-- * It therefore never travels through 'syncUserIndex' at all. This is an +-- in-place ES update-by-query that rewrites just that one field and leaves the +-- document version untouched, so it cannot lose a race against a concurrent +-- full document write of the same user. +-- +-- * Nothing drifts as a result: the value is derived from galley, not from the +-- user record, so any later resync recomputes it from the same source of truth. updateTeamSearchVisibilityInboundImpl :: (Member IndexedUserStore r) => TeamStatus SearchVisibilityInboundConfig -> Sem r () updateTeamSearchVisibilityInboundImpl teamStatus = IndexedUserStore.updateTeamSearchVisibilityInbound teamStatus.team $ diff --git a/libs/wire-subsystems/test/unit/Wire/MockInterpreters/BrigAPIAccess.hs b/libs/wire-subsystems/test/unit/Wire/MockInterpreters/BrigAPIAccess.hs index aeb7629909..aa9f2138d8 100644 --- a/libs/wire-subsystems/test/unit/Wire/MockInterpreters/BrigAPIAccess.hs +++ b/libs/wire-subsystems/test/unit/Wire/MockInterpreters/BrigAPIAccess.hs @@ -25,6 +25,7 @@ import Wire.BrigAPIAccess mockBrigAPIAccess :: InterpreterFor BrigAPIAccess r mockBrigAPIAccess = interpret $ \case UpdateSearchIndex _ -> pure () + BumpWriteTimeAndUpdateSearchIndex _ -> pure () -- everything else is not implemented GetConnectionsUnqualified {} -> error "GetConnectionsUnqualified: implement on demand (mockBrigAPIAccess)" GetConnections {} -> error "GetConnections: implement on demand (mockBrigAPIAccess)" diff --git a/libs/wire-subsystems/test/unit/Wire/MockInterpreters/UserStore.hs b/libs/wire-subsystems/test/unit/Wire/MockInterpreters/UserStore.hs index d30c854658..c4ca5f9c0f 100644 --- a/libs/wire-subsystems/test/unit/Wire/MockInterpreters/UserStore.hs +++ b/libs/wire-subsystems/test/unit/Wire/MockInterpreters/UserStore.hs @@ -116,6 +116,8 @@ inMemoryUserStoreInterpreterWithDeleteHook onDelete = interpret $ \case pure $ storedUserToIndexUser <$> mUser GetIndexUsersPaginated _pageSize _pagingState -> error "GetIndexUsersPaginated not implemented in inMemoryUserStoreInterpreter" + -- the in-memory store has no writetimes, so there is nothing to bump + BumpWriteTime _ -> pure () UpdateUserHandleEither uid hUpdate -> runError $ modifyLocalUsers (traverse doUpdate) where doUpdate :: StoredUser -> Sem (Error StoredUserUpdateError : r) StoredUser diff --git a/libs/wire-subsystems/test/unit/Wire/MockInterpreters/UserSubsystem.hs b/libs/wire-subsystems/test/unit/Wire/MockInterpreters/UserSubsystem.hs index 043f3a834d..0b735215ba 100644 --- a/libs/wire-subsystems/test/unit/Wire/MockInterpreters/UserSubsystem.hs +++ b/libs/wire-subsystems/test/unit/Wire/MockInterpreters/UserSubsystem.hs @@ -88,6 +88,7 @@ inMemoryUserSubsystemInterpreter = UpdateTeamSearchVisibilityInbound _ -> error "UpdateTeamSearchVisibilityInbound: implement on demand (userSubsystemInterpreter)" AcceptTeamInvitation {} -> error "AcceptTeamInvitation: implement on demand (userSubsystemInterpreter)" InternalUpdateSearchIndex {} -> error "InternalUpdateSearchIndex: implement on demand (userSubsystemInterpreter)" + InternalBumpWriteTimeAndUpdateSearchIndex {} -> error "InternalBumpWriteTimeAndUpdateSearchIndex: implement on demand (userSubsystemInterpreter)" InternalFindTeamInvitation {} -> error "InternalFindTeamInvitation: implement on demand (userSubsystemInterpreter)" GetUserExportData _ -> error "GetUserExportData: implement on demand (userSubsystemInterpreter)" RemoveEmailEither _ -> error "RemoveEmailEither: implement on demand (userSubsystemInterpreter)" diff --git a/services/brig/src/Brig/API/Internal.hs b/services/brig/src/Brig/API/Internal.hs index 86f9f2e9b2..1161476fcd 100644 --- a/services/brig/src/Brig/API/Internal.hs +++ b/services/brig/src/Brig/API/Internal.hs @@ -507,6 +507,7 @@ internalSearchIndexAPI :: forall r. (Member UserSubsystem r) => ServerT BrigIRou internalSearchIndexAPI = Named @"indexRefresh" (NoContent <$ lift (wrapClient Search.refreshIndexes)) :<|> Named @"update-search-index" (\uid -> lift $ liftSem $ UserSubsystem.internalUpdateSearchIndex uid $> NoContent) + :<|> Named @"bump-write-time-and-update-search-index" (\uid -> lift $ liftSem $ UserSubsystem.internalBumpWriteTimeAndUpdateSearchIndex uid $> NoContent) enterpriseLoginApi :: ( Member EnterpriseLoginSubsystem r, From 3a0cb4332c61e2ab35aa32eec70f9eb2ca7a63cb Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Fri, 11 Sep 2026 15:05:58 +0200 Subject: [PATCH 16/20] Fix brig-index: (3) bump version when deleting sso_id. --- services/brig/src/Brig/API/Internal.hs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/services/brig/src/Brig/API/Internal.hs b/services/brig/src/Brig/API/Internal.hs index 1161476fcd..d9732f0393 100644 --- a/services/brig/src/Brig/API/Internal.hs +++ b/services/brig/src/Brig/API/Internal.hs @@ -895,7 +895,10 @@ deleteSSOIdH uid = lift $ do success <- liftSem $ UserStore.updateSSOId uid Nothing if success then liftSem $ do - UserSubsystem.internalUpdateSearchIndex uid + -- nulling `sso_id` in cassandra takes its writetime with it, so the index + -- version would not advance here (it can even go backwards) and the SSO + -- identity would stay in the index + UserSubsystem.internalBumpWriteTimeAndUpdateSearchIndex uid Events.generateUserEvent uid Nothing (UserUpdated ((emptyUserUpdatedData uid) {eupSSOIdRemoved = True})) pure UpdateSSOIdSuccess else pure UpdateSSOIdNotFound From 2f732d2adc4d0a6cd14008bed93034e4b4dfd651 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Fri, 11 Sep 2026 15:08:41 +0200 Subject: [PATCH 17/20] Edit haddocks to be more comprehensive. --- libs/wire-subsystems/src/Wire/UserStore.hs | 13 +++++++++---- libs/wire-subsystems/src/Wire/UserSubsystem.hs | 9 ++++++--- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/libs/wire-subsystems/src/Wire/UserStore.hs b/libs/wire-subsystems/src/Wire/UserStore.hs index 3c879dc170..2378c84ca7 100644 --- a/libs/wire-subsystems/src/Wire/UserStore.hs +++ b/libs/wire-subsystems/src/Wire/UserStore.hs @@ -81,10 +81,15 @@ data UserStore m a where UpdateEmailUnvalidated :: UserId -> EmailAddress -> UserStore m () DeleteEmailUnvalidated :: UserId -> UserStore m () -- | Advance the user's ES index version without otherwise changing the - -- user. 'indexUserToVersion' derives the version from the user row alone, - -- so a change to data that lives outside that row (team collaborations) - -- has to bump the row explicitly; otherwise the new document is rejected - -- as a version conflict. See 'Wire.UserSearch.Types.WriteTimeBumper'. + -- user. 'indexUserToVersion' derives the version from the writetimes of the + -- user record, so two kinds of change need this: data that lives outside the + -- record altogether (team collaborations), and data *removed* from the + -- record, because a nulled cassandra column takes its writetime with it and + -- the version can then even go backwards. Without the bump the updated + -- document is rejected as a version conflict. Postgres-resident users are + -- not affected by the second case (one `updated_at` column, refreshed by + -- trigger), but the bump is harmless there. See + -- 'Wire.UserSearch.Types.WriteTimeBumper'. BumpWriteTime :: UserId -> UserStore m () UpdateUserHandleEither :: UserId -> StoredUserHandleUpdate -> UserStore m (Either StoredUserUpdateError ()) UpdateSSOId :: UserId -> Maybe UserSSOId -> UserStore m Bool diff --git a/libs/wire-subsystems/src/Wire/UserSubsystem.hs b/libs/wire-subsystems/src/Wire/UserSubsystem.hs index 1f1d673feb..a437f94377 100644 --- a/libs/wire-subsystems/src/Wire/UserSubsystem.hs +++ b/libs/wire-subsystems/src/Wire/UserSubsystem.hs @@ -182,9 +182,12 @@ data UserSubsystem m a where -- migration this would just be an internal detail of the subsystem InternalUpdateSearchIndex :: UserId -> UserSubsystem m () -- | Like 'InternalUpdateSearchIndex', but additionally bumps the user's - -- index version (see 'Wire.UserStore.BumpWriteTime'). Use this, and only - -- this, when what changed does not live in the user row -- currently that - -- means team collaborations. + -- index version (see 'Wire.UserStore.BumpWriteTime'). Use this whenever the + -- change would otherwise leave the version where it is: either because the + -- changed data does not live in the user record at all (team + -- collaborations), or because it was *removed* from the user record -- a + -- nulled cassandra column takes its writetime with it, so the version does + -- not advance and can even go backwards. InternalBumpWriteTimeAndUpdateSearchIndex :: UserId -> UserSubsystem m () InternalFindTeamInvitation :: Maybe EmailKey -> InvitationCode -> UserSubsystem m StoredInvitation GetUserExportData :: UserId -> UserSubsystem m (Maybe TeamExportUser) From a59d2248e73052baa5f0a26b2ac7ed6ead8de558 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Fri, 11 Sep 2026 15:11:22 +0200 Subject: [PATCH 18/20] Edit changelog. --- .../WPB-28089-bump-index-version-on-collaboration-change | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/changelog.d/3-bug-fixes/WPB-28089-bump-index-version-on-collaboration-change b/changelog.d/3-bug-fixes/WPB-28089-bump-index-version-on-collaboration-change index ecdb0c38be..3f9ea538e0 100644 --- a/changelog.d/3-bug-fixes/WPB-28089-bump-index-version-on-collaboration-change +++ b/changelog.d/3-bug-fixes/WPB-28089-bump-index-version-on-collaboration-change @@ -1,4 +1,5 @@ -Adding, changing or removing a team collaboration now bumps the affected user's -search index version. Previously the updated document had the same version as -the one already in Elasticsearch, so `brig-index reindex` dropped it as a -version conflict and the collaborating teams in the index went stale. +Changes that are invisible to the user record now advance the search index +version, so the updated document is no longer dropped as a version conflict: +adding, changing or removing a team collaboration, and removing a user's SSO +identity. Previously the collaborating teams, resp. the SSO identity, could +stay in the index indefinitely. From 55f542125747885a0e53263661313dca304b0087 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Fri, 11 Sep 2026 15:22:19 +0200 Subject: [PATCH 19/20] Fix brig-index: (4) bump version on dropping unvalidated email. --- .../WPB-28089-bump-index-version-on-collaboration-change | 2 ++ services/brig/src/Brig/API/Internal.hs | 7 ++++++- services/brig/src/Brig/API/User.hs | 2 ++ 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/changelog.d/3-bug-fixes/WPB-28089-bump-index-version-on-collaboration-change b/changelog.d/3-bug-fixes/WPB-28089-bump-index-version-on-collaboration-change index 3f9ea538e0..c050f47712 100644 --- a/changelog.d/3-bug-fixes/WPB-28089-bump-index-version-on-collaboration-change +++ b/changelog.d/3-bug-fixes/WPB-28089-bump-index-version-on-collaboration-change @@ -3,3 +3,5 @@ version, so the updated document is no longer dropped as a version conflict: adding, changing or removing a team collaboration, and removing a user's SSO identity. Previously the collaborating teams, resp. the SSO identity, could stay in the index indefinitely. +Cancelling a pending email change did not trigger a re-index at all; it now +does, so the unvalidated email address no longer stays in the index either. diff --git a/services/brig/src/Brig/API/Internal.hs b/services/brig/src/Brig/API/Internal.hs index d9732f0393..13574c498b 100644 --- a/services/brig/src/Brig/API/Internal.hs +++ b/services/brig/src/Brig/API/Internal.hs @@ -909,7 +909,8 @@ updateManagedByH uid (ManagedByUpdate managedBy) = do deletePendingEmailUpdateH :: ( Member UserStore r, - Member ActivationCodeStore r + Member ActivationCodeStore r, + Member UserSubsystem r ) => UserId -> (Handler r) NoContent @@ -919,6 +920,10 @@ deletePendingEmailUpdateH uid = do lift . liftSem $ do ActivationCode.deleteActivationCode (mkEmailKey email) UserStore.deleteEmailUnvalidated uid + -- `udEmailUnvalidated` is part of the indexed document, and nulling the + -- column in cassandra takes its writetime with it, so the version has to + -- be bumped for the updated document to be accepted + UserSubsystem.internalBumpWriteTimeAndUpdateSearchIndex uid pure NoContent updateRichInfoH :: (Member UserStore r) => UserId -> RichInfoUpdate -> (Handler r) NoContent diff --git a/services/brig/src/Brig/API/User.hs b/services/brig/src/Brig/API/User.hs index d26a22d5fa..05c44064fb 100644 --- a/services/brig/src/Brig/API/User.hs +++ b/services/brig/src/Brig/API/User.hs @@ -865,6 +865,8 @@ onActivated (AccountActivated account) = liftSem $ do onActivated (EmailActivated uid email) = liftSem $ do User.internalUpdateSearchIndex uid Events.generateUserEvent uid Nothing (emailUpdated uid email) + -- FUTUREWORK: deleteEmailUnvalidated may already have happened at + -- this point, maybe remove this call? UserStore.deleteEmailUnvalidated uid pure (uid, Just (EmailIdentity email), False) From a26db192da5fdb0ce4e48e3aeaae988b3d57486e Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Mon, 14 Sep 2026 17:26:32 +0200 Subject: [PATCH 20/20] Fix search query in integration test. --- services/brig/test/integration/API/Search.hs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/brig/test/integration/API/Search.hs b/services/brig/test/integration/API/Search.hs index 1792ccc0b2..4b223ebac6 100644 --- a/services/brig/test/integration/API/Search.hs +++ b/services/brig/test/integration/API/Search.hs @@ -1026,7 +1026,7 @@ testNonCollaboratorNotFound brig _galley = do -- Do NOT add memberB as collaborator -- Search should not find memberB regardless of index version refreshIndex brig - res <- searchResults <$> executeSearch brig (userId ownerA) "" + res <- searchResults <$> executeSearch brig (userId ownerA) memberB.userDisplayName.fromName liftIO $ assertBool "non-collaborator should not be found" $ userQualifiedId memberB `notElem` map contactQualifiedId res