From 67e019a9eee6976d6e7d849af7915e40ba430969 Mon Sep 17 00:00:00 2001 From: Leif Battermann Date: Thu, 6 Aug 2026 13:49:09 +0200 Subject: [PATCH 01/10] guard federated conversations --- .../src/Wire/ConversationSubsystem/Update.hs | 106 +++++++++++------- 1 file changed, 65 insertions(+), 41 deletions(-) diff --git a/libs/wire-subsystems/src/Wire/ConversationSubsystem/Update.hs b/libs/wire-subsystems/src/Wire/ConversationSubsystem/Update.hs index cc840b1783..421030e60c 100644 --- a/libs/wire-subsystems/src/Wire/ConversationSubsystem/Update.hs +++ b/libs/wire-subsystems/src/Wire/ConversationSubsystem/Update.hs @@ -97,6 +97,8 @@ import Polysemy import Polysemy.Error import Polysemy.Input import Polysemy.TinyLog +import Polysemy.TinyLog qualified as P +import System.Logger qualified as Log import Wire.API.Bot hiding (addBot) import Wire.API.Conversation hiding (Member) import Wire.API.Conversation.Action @@ -1195,6 +1197,11 @@ isAdminlessCheckCandidate conv = conv.metadata.cnvmType == RegularConv && maybe True (== GroupConversation) conv.metadata.cnvmGroupConvType +shouldSkipSystemAdminlessAction :: Maybe (Local UserId) -> StoredConversation -> Bool +shouldSkipSystemAdminlessAction mlusr conv = + isNothing mlusr + && not (null conv.remoteMembers) + setupAdminlessGroupsCleanup :: ( Member ConversationStore r, Member (ErrorS 'ConvNotFound) r, @@ -1207,7 +1214,8 @@ setupAdminlessGroupsCleanup :: Member BackendNotificationQueueAccess r, Member FeaturesConfigSubsystem r, Member (Input (Local ())) r, - Member JobSubsystem r + Member JobSubsystem r, + Member TinyLog r ) => Maybe (Local UserId) -> TeamId -> @@ -1216,7 +1224,14 @@ setupAdminlessGroupsCleanup mUsr tid = do teamConvIds <- E.getTeamConversations tid for_ teamConvIds $ \cnv -> do lcnv <- qualifyLocal cnv - adminlessTryAutopromote mUsr lcnv $ \_ feature _ -> scheduleDeletion lcnv mUsr tid feature + adminlessTryAutopromote mUsr lcnv $ \conv feature _ -> + if null conv.remoteMembers + then scheduleDeletion lcnv mUsr tid feature + else + P.info $ + Log.msg (Log.val "Skipping adminless cleanup for conversation with remote members") + . Log.field "conversation_id" (show conv.id_) + . Log.field "team_id" (show tid) guardPreventAdminlessGroups :: ( Member ConversationStore r, @@ -1349,7 +1364,8 @@ adminlessTryAutopromote :: Member Now r, Member E.ExternalAccess r, Member BackendNotificationQueueAccess r, - Member FeaturesConfigSubsystem r + Member FeaturesConfigSubsystem r, + Member TinyLog r ) => Maybe (Local UserId) -> Local ConvId -> @@ -1357,43 +1373,49 @@ adminlessTryAutopromote :: Sem r () adminlessTryAutopromote mlusr lcnv altAction = do conv <- getConversationWithError lcnv - when (isAdminlessCheckCandidate conv) $ for_ conv.metadata.cnvmTeam $ \tid -> do - (feature :: LockableFeature PreventAdminlessGroupsConfig) <- getFeatureForTeam tid - let adminExists = any (\member -> member.convRoleName == roleNameWireAdmin) conv.localMembers || any (\member -> member.convRoleName == roleNameWireAdmin) conv.remoteMembers - when (feature.status == FeatureStatusEnabled && not adminExists) $ do - eligibleMembers <- eligibleAdminFallbackMembers lcnv Nothing conv - case eligibleMembers of - x : xs -> do - seed <- randomWord64 - let autopromotionCandidates = selectAutopromotionCandidate seed feature.config.promotionStrategy (x :| xs) - update = OtherMemberUpdate (Just roleNameWireAdmin) - for_ autopromotionCandidates $ \candidate -> do - E.setOtherMember lcnv candidate update - case mlusr of - Just lusr -> - void $ - sendConversationActionNotifications - (sing @'ConversationMemberUpdateTag) - (tUntagged lusr) - False - Nothing - (qualifyAs lcnv conv) - (convBotsAndMembers conv) - (ConversationMemberUpdate candidate update) - def - Nothing -> do - now <- Now.get - Notify.pushSystemEvent - Nothing - ( SystemEvent - (tUntagged lcnv) + when (isAdminlessCheckCandidate conv) $ + if shouldSkipSystemAdminlessAction mlusr conv + then + P.info $ + Log.msg (Log.val "Skipping system-triggered adminless action for conversation with remote members") + . Log.field "conversation_id" (show conv.id_) + else for_ conv.metadata.cnvmTeam $ \tid -> do + (feature :: LockableFeature PreventAdminlessGroupsConfig) <- getFeatureForTeam tid + let adminExists = any (\member -> member.convRoleName == roleNameWireAdmin) conv.localMembers || any (\member -> member.convRoleName == roleNameWireAdmin) conv.remoteMembers + when (feature.status == FeatureStatusEnabled && not adminExists) $ do + eligibleMembers <- eligibleAdminFallbackMembers lcnv Nothing conv + case eligibleMembers of + x : xs -> do + seed <- randomWord64 + let autopromotionCandidates = selectAutopromotionCandidate seed feature.config.promotionStrategy (x :| xs) + update = OtherMemberUpdate (Just roleNameWireAdmin) + for_ autopromotionCandidates $ \candidate -> do + E.setOtherMember lcnv candidate update + case mlusr of + Just lusr -> + void $ + sendConversationActionNotifications + (sing @'ConversationMemberUpdateTag) + (tUntagged lusr) + False + Nothing + (qualifyAs lcnv conv) + (convBotsAndMembers conv) + (ConversationMemberUpdate candidate update) + def + Nothing -> do + now <- Now.get + Notify.pushSystemEvent Nothing - now - conv.metadata.cnvmTeam - (EdSystemMemberUpdate (memberUpdateData candidate update)) - ) - (Set.fromList (map (.id_) conv.localMembers)) - [] -> altAction conv feature eligibleMembers + ( SystemEvent + (tUntagged lcnv) + Nothing + now + conv.metadata.cnvmTeam + (EdSystemMemberUpdate (memberUpdateData candidate update)) + ) + (Set.fromList (map (.id_) conv.localMembers)) + [] -> altAction conv feature eligibleMembers where memberUpdateData candidate memberUpdate' = MemberUpdateData @@ -1419,7 +1441,8 @@ adminlessAutopromoteOrDelete :: Member BackendNotificationQueueAccess r, Member FeaturesConfigSubsystem r, Member ProposalStore r, - Member CodeStore r + Member CodeStore r, + Member TinyLog r ) => Maybe (Local UserId) -> Local ConvId -> @@ -1457,7 +1480,8 @@ adminlessAutopromoteOrSendReminder :: Member Now r, Member E.ExternalAccess r, Member BackendNotificationQueueAccess r, - Member FeaturesConfigSubsystem r + Member FeaturesConfigSubsystem r, + Member TinyLog r ) => Maybe (Local UserId) -> Local ConvId -> From 5b8fc566469611e7931689e79557f9aea33ce832 Mon Sep 17 00:00:00 2001 From: Leif Battermann Date: Thu, 6 Aug 2026 13:49:21 +0200 Subject: [PATCH 02/10] updated docs --- docs/src/developer/reference/config-options.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/docs/src/developer/reference/config-options.md b/docs/src/developer/reference/config-options.md index 066637dbb0..4769cb166f 100644 --- a/docs/src/developer/reference/config-options.md +++ b/docs/src/developer/reference/config-options.md @@ -362,6 +362,19 @@ The settings mean: - `deletionTimeoutDuration`: how long to keep an adminless conversation before it is deleted. - `reminderTimeoutDurations`: when before deletion reminder notifications should be sent. +In federated conversations, the automatic system-triggered action is skipped +when the conversation contains remote members. This applies both when the +feature is enabled and existing conversations are scanned, and when a +previously scheduled autopromotion, deletion, or reminder job runs. This +prevents the backend from changing or deleting a conversation when the +corresponding system event cannot yet be safely sent to the remote backend. +The action is logged at info level. User-triggered behavior is unchanged. + +As a result, an adminless federated conversation with remote members is not +automatically promoted, deleted, or sent deletion reminders by this feature. +It must be handled after compatible federation support or an appropriate +reconciliation mechanism is available. + Durations are strings with a number and a unit suffix. Supported units are `us`, `ms`, `s`, `m`, `h`, `d`, and `w`. It is **not** recommended or supported to set these below a day in production environments. Feature responses, including `GET /feature-configs`, `GET /teams/:tid/features`, and `GET /teams/:tid/features/preventAdminlessGroups`, include the duration fields: From 1779cbee789f8b0bb08bff3c87fff6cd6d902123 Mon Sep 17 00:00:00 2001 From: Leif Battermann Date: Thu, 6 Aug 2026 14:07:14 +0200 Subject: [PATCH 03/10] tests --- integration/test/Test/AdminlessGroups.hs | 61 ++++++++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/integration/test/Test/AdminlessGroups.hs b/integration/test/Test/AdminlessGroups.hs index 05ef119f48..44fd0caef8 100644 --- a/integration/test/Test/AdminlessGroups.hs +++ b/integration/test/Test/AdminlessGroups.hs @@ -21,6 +21,7 @@ import API.Brig import API.Galley import API.GalleyInternal hiding (getConversation) import qualified API.GalleyInternal as GalleyI +import Control.Concurrent (threadDelay) import MLS.Util import Notifications import SetupHelpers hiding (deleteUser) @@ -325,6 +326,66 @@ testAdminlessSetupMemberUpdateAfterAdminLeaves = do resp.status `shouldMatchInt` 200 resp.json %. "members.self.conversation_role" `shouldMatch` "wire_admin" +testAdminlessSetupSkipsDeletionForRemoteMembers :: (HasCallStack) => App () +testAdminlessSetupSkipsDeletionForRemoteMembers = do + -- this tests that the adminless clean up actions are skipped + -- when remote members are present + -- because remote backends do not support a system delete/member-update event, yet + -- to prevent a state drift + (alice, tid, _) <- createTeam OwnDomain 1 + remoteUser <- randomUser OtherDomain def + connectTwoUsers alice remoteUser + + configureAdminlessGroupsFeature OwnDomain tid "disabled" "1s" [] + + alice1 <- createMLSClient def alice + remoteUser1 <- createMLSClient def remoteUser + traverse_ (uploadNewKeyPackage def) [alice1, remoteUser1] + + conv <- createTeamMLSConversation alice tid alice1 [remoteUser] + + -- Create an adminless conversation while the feature is disabled. Enabling + -- the feature later exercises the system-triggered setup path. + removeMember alice conv alice >>= assertSuccess + + configureAdminlessGroupsFeature OwnDomain tid "enabled" "1s" ["1s"] + + -- The setup job must not schedule deletion or reminders for this + -- conversation because it contains a remote member. + liftIO $ threadDelay 2_000_000 + bindResponse (GalleyI.getConversation conv) $ \resp -> do + resp.status `shouldMatchInt` 200 + +testAdminlessSetupSkipsAutopromotionForRemoteMembers :: (HasCallStack) => App () +testAdminlessSetupSkipsAutopromotionForRemoteMembers = do + -- this tests that the adminless clean up actions are skipped + -- when remote members are present + -- because remote backends do not support a system delete/member-update event, yet + -- to prevent a state drift + (alice, tid, [bob]) <- createTeam OwnDomain 2 + remoteUser <- randomUser OtherDomain def + connectTwoUsers alice remoteUser + + configureAdminlessGroupsFeature OwnDomain tid "disabled" "1s" [] + + alice1 <- createMLSClient def alice + bob1 <- createMLSClient def bob + remoteUser1 <- createMLSClient def remoteUser + traverse_ (uploadNewKeyPackage def) [alice1, bob1, remoteUser1] + + conv <- createTeamMLSConversation alice tid alice1 [bob, remoteUser] + + -- Create an adminless conversation while the feature is disabled. Enabling + -- the feature later would normally promote Bob through a system action. + removeMember alice conv alice >>= assertSuccess + + configureAdminlessGroupsFeature OwnDomain tid "enabled" "1s" [] + + liftIO $ threadDelay 2_000_000 + bindResponse (getConversation bob conv) $ \resp -> do + resp.status `shouldMatchInt` 200 + resp.json %. "members.self.conversation_role" `shouldMatch` "wire_member" + testAdminlessJobsCancelledOnFeatureDisable :: (HasCallStack) => App () testAdminlessJobsCancelledOnFeatureDisable = do (alice, tid, _) <- createTeam OwnDomain 1 From f757d350d028f04098db80eb2a5c7b7a14e9ccd5 Mon Sep 17 00:00:00 2001 From: Leif Battermann Date: Thu, 6 Aug 2026 14:09:49 +0200 Subject: [PATCH 04/10] changelog --- changelog.d/2-features/WPB-26650 | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/2-features/WPB-26650 diff --git a/changelog.d/2-features/WPB-26650 b/changelog.d/2-features/WPB-26650 new file mode 100644 index 0000000000..8912bdd232 --- /dev/null +++ b/changelog.d/2-features/WPB-26650 @@ -0,0 +1 @@ +Skip system-triggered prevent-adminless cleanup for federated conversations with remote members to prevent remote state drift. From dd3507200cde36c770afcaf3bf015142a82424fe Mon Sep 17 00:00:00 2001 From: Leif Battermann Date: Thu, 6 Aug 2026 17:02:18 +0200 Subject: [PATCH 05/10] wip test to verify --- integration/test/Test/AdminlessGroups.hs | 50 ++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/integration/test/Test/AdminlessGroups.hs b/integration/test/Test/AdminlessGroups.hs index 44fd0caef8..1c7ef4f7f2 100644 --- a/integration/test/Test/AdminlessGroups.hs +++ b/integration/test/Test/AdminlessGroups.hs @@ -386,6 +386,56 @@ testAdminlessSetupSkipsAutopromotionForRemoteMembers = do resp.status `shouldMatchInt` 200 resp.json %. "members.self.conversation_role" `shouldMatch` "wire_member" +testAdminlessFederatedUserOriginDeletion :: (HasCallStack) => App () +testAdminlessFederatedUserOriginDeletion = do + (alice, tid, _) <- createTeam OwnDomain 1 + bob <- randomUser OtherDomain def + connectTwoUsers alice bob + configureAdminlessGroupsFeature OwnDomain tid "enabled" "5s" [] + + conv <- postConversation alice (defProteus {team = Just tid}) >>= getJSON 201 + bobId <- bob %. "qualified_id" + convQid <- objQidObject conv + void $ addMembers alice conv def {role = Just "wire_member", users = [bobId]} >>= getBody 200 + + eventually + ( bindResponse (listConversationIds bob def) $ \resp -> do + resp.status `shouldMatchInt` 200 + conversationIds <- resp.json %. "qualified_conversations" & asList + conversationIds `shouldContain` [convQid] + ) + + withWebSockets [alice, bob] $ \[wsAlice, wsBob] -> do + -- Bob is remote and is not an eligible replacement. The admin leave + -- therefore schedules deletion with Alice as the origin user. + bindResponse (removeMember alice conv alice) $ \resp -> do + resp.status `shouldMatchInt` 200 + + -- Consume the leave events before waiting for the delayed deletion. + void $ awaitMatchFor 20 isConvLeaveNotif wsAlice + void $ awaitMatchFor 20 isConvLeaveNotif wsBob + liftIO $ threadDelay 20_000_000 + + -- This verifies that the normal, user-originated deletion event reaches + -- the remote backend even though Alice has already left the conversation. + deleteNotif <- awaitMatchFor 20 isConvDeleteNotif wsBob + deleteNotif %. "payload.0.qualified_from" `shouldMatch` objQidObject alice + -- + -- The remote backend should still retain the remote conversation ID. + bindResponse (listConversationIds bob def) $ \resp -> do + resp.status `shouldMatchInt` 200 + conversationIds <- resp.json %. "qualified_conversations" & asList + conversationIds `shouldNotContain` [convQid] + + -- If the normal deletion event was authorized and applied remotely, this + -- should be empty. If it contains the conversation, the remote backend + -- received the event but retained stale conversation state, which is the + -- behavior this test is intended to expose. + bindResponse (listConversations bob [convQid]) $ \resp -> do + resp.status `shouldMatchInt` 200 + conversations <- resp.json %. "found" & asList + shouldBeEmpty conversations + testAdminlessJobsCancelledOnFeatureDisable :: (HasCallStack) => App () testAdminlessJobsCancelledOnFeatureDisable = do (alice, tid, _) <- createTeam OwnDomain 1 From cd0f06e7ba5f9272b46e481ecd902f67da0d96bb Mon Sep 17 00:00:00 2001 From: Leif Battermann Date: Thu, 6 Aug 2026 17:02:46 +0200 Subject: [PATCH 06/10] removed test --- integration/test/Test/AdminlessGroups.hs | 50 ------------------------ 1 file changed, 50 deletions(-) diff --git a/integration/test/Test/AdminlessGroups.hs b/integration/test/Test/AdminlessGroups.hs index 1c7ef4f7f2..44fd0caef8 100644 --- a/integration/test/Test/AdminlessGroups.hs +++ b/integration/test/Test/AdminlessGroups.hs @@ -386,56 +386,6 @@ testAdminlessSetupSkipsAutopromotionForRemoteMembers = do resp.status `shouldMatchInt` 200 resp.json %. "members.self.conversation_role" `shouldMatch` "wire_member" -testAdminlessFederatedUserOriginDeletion :: (HasCallStack) => App () -testAdminlessFederatedUserOriginDeletion = do - (alice, tid, _) <- createTeam OwnDomain 1 - bob <- randomUser OtherDomain def - connectTwoUsers alice bob - configureAdminlessGroupsFeature OwnDomain tid "enabled" "5s" [] - - conv <- postConversation alice (defProteus {team = Just tid}) >>= getJSON 201 - bobId <- bob %. "qualified_id" - convQid <- objQidObject conv - void $ addMembers alice conv def {role = Just "wire_member", users = [bobId]} >>= getBody 200 - - eventually - ( bindResponse (listConversationIds bob def) $ \resp -> do - resp.status `shouldMatchInt` 200 - conversationIds <- resp.json %. "qualified_conversations" & asList - conversationIds `shouldContain` [convQid] - ) - - withWebSockets [alice, bob] $ \[wsAlice, wsBob] -> do - -- Bob is remote and is not an eligible replacement. The admin leave - -- therefore schedules deletion with Alice as the origin user. - bindResponse (removeMember alice conv alice) $ \resp -> do - resp.status `shouldMatchInt` 200 - - -- Consume the leave events before waiting for the delayed deletion. - void $ awaitMatchFor 20 isConvLeaveNotif wsAlice - void $ awaitMatchFor 20 isConvLeaveNotif wsBob - liftIO $ threadDelay 20_000_000 - - -- This verifies that the normal, user-originated deletion event reaches - -- the remote backend even though Alice has already left the conversation. - deleteNotif <- awaitMatchFor 20 isConvDeleteNotif wsBob - deleteNotif %. "payload.0.qualified_from" `shouldMatch` objQidObject alice - -- - -- The remote backend should still retain the remote conversation ID. - bindResponse (listConversationIds bob def) $ \resp -> do - resp.status `shouldMatchInt` 200 - conversationIds <- resp.json %. "qualified_conversations" & asList - conversationIds `shouldNotContain` [convQid] - - -- If the normal deletion event was authorized and applied remotely, this - -- should be empty. If it contains the conversation, the remote backend - -- received the event but retained stale conversation state, which is the - -- behavior this test is intended to expose. - bindResponse (listConversations bob [convQid]) $ \resp -> do - resp.status `shouldMatchInt` 200 - conversations <- resp.json %. "found" & asList - shouldBeEmpty conversations - testAdminlessJobsCancelledOnFeatureDisable :: (HasCallStack) => App () testAdminlessJobsCancelledOnFeatureDisable = do (alice, tid, _) <- createTeam OwnDomain 1 From bbb1a11beae1f4540e974461c2079016b1014659 Mon Sep 17 00:00:00 2001 From: Leif Battermann Date: Thu, 6 Aug 2026 17:13:37 +0200 Subject: [PATCH 07/10] made the guard less strict after reassessment --- changelog.d/2-features/WPB-26650 | 2 +- .../src/developer/reference/config-options.md | 26 +-- integration/test/Test/AdminlessGroups.hs | 55 +++-- .../src/Wire/ConversationSubsystem/Update.hs | 193 +++++++++--------- 4 files changed, 155 insertions(+), 121 deletions(-) diff --git a/changelog.d/2-features/WPB-26650 b/changelog.d/2-features/WPB-26650 index 8912bdd232..d033ec48bd 100644 --- a/changelog.d/2-features/WPB-26650 +++ b/changelog.d/2-features/WPB-26650 @@ -1 +1 @@ -Skip system-triggered prevent-adminless cleanup for federated conversations with remote members to prevent remote state drift. +Skip senderless prevent-adminless deletion for federated conversations with remote members to prevent remote state drift. diff --git a/docs/src/developer/reference/config-options.md b/docs/src/developer/reference/config-options.md index 4769cb166f..7ae112d851 100644 --- a/docs/src/developer/reference/config-options.md +++ b/docs/src/developer/reference/config-options.md @@ -362,18 +362,20 @@ The settings mean: - `deletionTimeoutDuration`: how long to keep an adminless conversation before it is deleted. - `reminderTimeoutDurations`: when before deletion reminder notifications should be sent. -In federated conversations, the automatic system-triggered action is skipped -when the conversation contains remote members. This applies both when the -feature is enabled and existing conversations are scanned, and when a -previously scheduled autopromotion, deletion, or reminder job runs. This -prevents the backend from changing or deleting a conversation when the -corresponding system event cannot yet be safely sent to the remote backend. -The action is logged at info level. User-triggered behavior is unchanged. - -As a result, an adminless federated conversation with remote members is not -automatically promoted, deleted, or sent deletion reminders by this feature. -It must be handled after compatible federation support or an appropriate -reconciliation mechanism is available. +In federated conversations, automatic senderless deletion is skipped when the +conversation contains remote members because the corresponding system delete +event cannot yet be sent safely to the remote backend. This applies both when +the feature is enabled and existing conversations are scanned without an +origin user, and when a previously scheduled senderless deletion job runs. +Reminders for a skipped deletion are also skipped because they would be +misleading. The skipped deletion is logged at info level. + +Autopromotion still runs because the conversation-owning backend stores the +authoritative member roles. Remote clients may miss the immediate senderless +member-update notification, but a subsequent conversation fetch obtains the +current role from the owning backend. Member updates and deletions with an +origin user continue to use the existing ordinary federation events and are +not skipped. Durations are strings with a number and a unit suffix. Supported units are `us`, `ms`, `s`, `m`, `h`, `d`, and `w`. It is **not** recommended or supported to set these below a day in production environments. diff --git a/integration/test/Test/AdminlessGroups.hs b/integration/test/Test/AdminlessGroups.hs index 44fd0caef8..6541d9d823 100644 --- a/integration/test/Test/AdminlessGroups.hs +++ b/integration/test/Test/AdminlessGroups.hs @@ -326,12 +326,44 @@ testAdminlessSetupMemberUpdateAfterAdminLeaves = do resp.status `shouldMatchInt` 200 resp.json %. "members.self.conversation_role" `shouldMatch` "wire_admin" +testAdminlessSetupDeletesWithOriginAndRemoteMembers :: (HasCallStack) => App () +testAdminlessSetupDeletesWithOriginAndRemoteMembers = do + (alice, tid, _) <- createTeam OwnDomain 1 + remoteUser <- randomUser OtherDomain def + connectTwoUsers alice remoteUser + + setTeamFeatureLockStatus OwnDomain tid "preventAdminlessGroups" "unlocked" + patchTeamFeature OwnDomain tid "preventAdminlessGroups" (object ["status" .= "disabled"]) >>= assertSuccess + + conv <- + postConversation + alice + (defProteus {team = Just tid, qualifiedUsers = [remoteUser], newUsersRole = "wire_member"}) + >>= getJSON 201 + convQid <- objQidObject conv + + removeMember alice conv alice >>= assertSuccess + + eventually $ bindResponse (listConversationIds remoteUser def) $ \resp -> do + resp.status `shouldMatchInt` 200 + conversationIds <- resp.json %. "qualified_conversations" & asList + conversationIds `shouldContain` [convQid] + + withWebSockets [remoteUser] $ \[wsRemoteUser] -> do + setTeamFeatureConfigVersioned (ExplicitVersion 17) alice tid "preventAdminlessGroups" (mkAdminlessFeature "enabled" "1s" []) >>= assertSuccess + + deleteNotif <- awaitMatchFor 20 isConvDeleteNotif wsRemoteUser + deleteNotif %. "payload.0.qualified_from" `shouldMatch` objQidObject alice + + eventually $ bindResponse (listConversationIds remoteUser def) $ \resp -> do + resp.status `shouldMatchInt` 200 + conversationIds <- resp.json %. "qualified_conversations" & asList + conversationIds `shouldNotContain` [convQid] + testAdminlessSetupSkipsDeletionForRemoteMembers :: (HasCallStack) => App () testAdminlessSetupSkipsDeletionForRemoteMembers = do - -- this tests that the adminless clean up actions are skipped - -- when remote members are present - -- because remote backends do not support a system delete/member-update event, yet - -- to prevent a state drift + -- Senderless deletion is skipped when remote members are present because + -- remote backends do not support the system delete event yet. (alice, tid, _) <- createTeam OwnDomain 1 remoteUser <- randomUser OtherDomain def connectTwoUsers alice remoteUser @@ -356,12 +388,11 @@ testAdminlessSetupSkipsDeletionForRemoteMembers = do bindResponse (GalleyI.getConversation conv) $ \resp -> do resp.status `shouldMatchInt` 200 -testAdminlessSetupSkipsAutopromotionForRemoteMembers :: (HasCallStack) => App () -testAdminlessSetupSkipsAutopromotionForRemoteMembers = do - -- this tests that the adminless clean up actions are skipped - -- when remote members are present - -- because remote backends do not support a system delete/member-update event, yet - -- to prevent a state drift +testAdminlessSetupAutopromotesWithRemoteMembers :: (HasCallStack) => App () +testAdminlessSetupAutopromotesWithRemoteMembers = do + -- Autopromotion is safe with remote members because the owning backend is + -- authoritative for roles, even though remote clients do not receive the + -- senderless system member-update event yet. (alice, tid, [bob]) <- createTeam OwnDomain 2 remoteUser <- randomUser OtherDomain def connectTwoUsers alice remoteUser @@ -376,7 +407,7 @@ testAdminlessSetupSkipsAutopromotionForRemoteMembers = do conv <- createTeamMLSConversation alice tid alice1 [bob, remoteUser] -- Create an adminless conversation while the feature is disabled. Enabling - -- the feature later would normally promote Bob through a system action. + -- the feature later promotes Bob through a system action. removeMember alice conv alice >>= assertSuccess configureAdminlessGroupsFeature OwnDomain tid "enabled" "1s" [] @@ -384,7 +415,7 @@ testAdminlessSetupSkipsAutopromotionForRemoteMembers = do liftIO $ threadDelay 2_000_000 bindResponse (getConversation bob conv) $ \resp -> do resp.status `shouldMatchInt` 200 - resp.json %. "members.self.conversation_role" `shouldMatch` "wire_member" + resp.json %. "members.self.conversation_role" `shouldMatch` "wire_admin" testAdminlessJobsCancelledOnFeatureDisable :: (HasCallStack) => App () testAdminlessJobsCancelledOnFeatureDisable = do diff --git a/libs/wire-subsystems/src/Wire/ConversationSubsystem/Update.hs b/libs/wire-subsystems/src/Wire/ConversationSubsystem/Update.hs index 421030e60c..8730a58a8e 100644 --- a/libs/wire-subsystems/src/Wire/ConversationSubsystem/Update.hs +++ b/libs/wire-subsystems/src/Wire/ConversationSubsystem/Update.hs @@ -1197,11 +1197,17 @@ isAdminlessCheckCandidate conv = conv.metadata.cnvmType == RegularConv && maybe True (== GroupConversation) conv.metadata.cnvmGroupConvType -shouldSkipSystemAdminlessAction :: Maybe (Local UserId) -> StoredConversation -> Bool -shouldSkipSystemAdminlessAction mlusr conv = +shouldSkipSystemAdminlessDeletion :: Maybe (Local UserId) -> StoredConversation -> Bool +shouldSkipSystemAdminlessDeletion mlusr conv = isNothing mlusr && not (null conv.remoteMembers) +logSkippedSystemAdminlessDeletion :: (Member TinyLog r) => StoredConversation -> Sem r () +logSkippedSystemAdminlessDeletion conv = + P.info $ + Log.msg (Log.val "Skipping senderless adminless deletion for conversation with remote members") + . Log.field "conversation_id" (show conv.id_) + setupAdminlessGroupsCleanup :: ( Member ConversationStore r, Member (ErrorS 'ConvNotFound) r, @@ -1225,13 +1231,9 @@ setupAdminlessGroupsCleanup mUsr tid = do for_ teamConvIds $ \cnv -> do lcnv <- qualifyLocal cnv adminlessTryAutopromote mUsr lcnv $ \conv feature _ -> - if null conv.remoteMembers - then scheduleDeletion lcnv mUsr tid feature - else - P.info $ - Log.msg (Log.val "Skipping adminless cleanup for conversation with remote members") - . Log.field "conversation_id" (show conv.id_) - . Log.field "team_id" (show tid) + if shouldSkipSystemAdminlessDeletion mUsr conv + then logSkippedSystemAdminlessDeletion conv + else scheduleDeletion lcnv mUsr tid feature guardPreventAdminlessGroups :: ( Member ConversationStore r, @@ -1364,8 +1366,7 @@ adminlessTryAutopromote :: Member Now r, Member E.ExternalAccess r, Member BackendNotificationQueueAccess r, - Member FeaturesConfigSubsystem r, - Member TinyLog r + Member FeaturesConfigSubsystem r ) => Maybe (Local UserId) -> Local ConvId -> @@ -1373,49 +1374,43 @@ adminlessTryAutopromote :: Sem r () adminlessTryAutopromote mlusr lcnv altAction = do conv <- getConversationWithError lcnv - when (isAdminlessCheckCandidate conv) $ - if shouldSkipSystemAdminlessAction mlusr conv - then - P.info $ - Log.msg (Log.val "Skipping system-triggered adminless action for conversation with remote members") - . Log.field "conversation_id" (show conv.id_) - else for_ conv.metadata.cnvmTeam $ \tid -> do - (feature :: LockableFeature PreventAdminlessGroupsConfig) <- getFeatureForTeam tid - let adminExists = any (\member -> member.convRoleName == roleNameWireAdmin) conv.localMembers || any (\member -> member.convRoleName == roleNameWireAdmin) conv.remoteMembers - when (feature.status == FeatureStatusEnabled && not adminExists) $ do - eligibleMembers <- eligibleAdminFallbackMembers lcnv Nothing conv - case eligibleMembers of - x : xs -> do - seed <- randomWord64 - let autopromotionCandidates = selectAutopromotionCandidate seed feature.config.promotionStrategy (x :| xs) - update = OtherMemberUpdate (Just roleNameWireAdmin) - for_ autopromotionCandidates $ \candidate -> do - E.setOtherMember lcnv candidate update - case mlusr of - Just lusr -> - void $ - sendConversationActionNotifications - (sing @'ConversationMemberUpdateTag) - (tUntagged lusr) - False - Nothing - (qualifyAs lcnv conv) - (convBotsAndMembers conv) - (ConversationMemberUpdate candidate update) - def - Nothing -> do - now <- Now.get - Notify.pushSystemEvent + when (isAdminlessCheckCandidate conv) $ for_ conv.metadata.cnvmTeam $ \tid -> do + (feature :: LockableFeature PreventAdminlessGroupsConfig) <- getFeatureForTeam tid + let adminExists = any (\member -> member.convRoleName == roleNameWireAdmin) conv.localMembers || any (\member -> member.convRoleName == roleNameWireAdmin) conv.remoteMembers + when (feature.status == FeatureStatusEnabled && not adminExists) $ do + eligibleMembers <- eligibleAdminFallbackMembers lcnv Nothing conv + case eligibleMembers of + x : xs -> do + seed <- randomWord64 + let autopromotionCandidates = selectAutopromotionCandidate seed feature.config.promotionStrategy (x :| xs) + update = OtherMemberUpdate (Just roleNameWireAdmin) + for_ autopromotionCandidates $ \candidate -> do + E.setOtherMember lcnv candidate update + case mlusr of + Just lusr -> + void $ + sendConversationActionNotifications + (sing @'ConversationMemberUpdateTag) + (tUntagged lusr) + False + Nothing + (qualifyAs lcnv conv) + (convBotsAndMembers conv) + (ConversationMemberUpdate candidate update) + def + Nothing -> do + now <- Now.get + Notify.pushSystemEvent + Nothing + ( SystemEvent + (tUntagged lcnv) Nothing - ( SystemEvent - (tUntagged lcnv) - Nothing - now - conv.metadata.cnvmTeam - (EdSystemMemberUpdate (memberUpdateData candidate update)) - ) - (Set.fromList (map (.id_) conv.localMembers)) - [] -> altAction conv feature eligibleMembers + now + conv.metadata.cnvmTeam + (EdSystemMemberUpdate (memberUpdateData candidate update)) + ) + (Set.fromList (map (.id_) conv.localMembers)) + [] -> altAction conv feature eligibleMembers where memberUpdateData candidate memberUpdate' = MemberUpdateData @@ -1449,26 +1444,29 @@ adminlessAutopromoteOrDelete :: Sem r () adminlessAutopromoteOrDelete mlusr lcnv = adminlessTryAutopromote mlusr lcnv orAlternativelyDeleteConv where - orAlternativelyDeleteConv conv _ _ = do - removeConversation (qualifyAs lcnv conv) - case mlusr of - Just lusr -> - void $ - sendConversationActionNotifications - (sing @'ConversationDeleteTag) - (tUntagged lusr) - False - Nothing - (qualifyAs lcnv conv) - (convBotsAndMembers conv) - () - def - Nothing -> do - now <- Now.get - Notify.pushSystemEvent - Nothing - (SystemEvent (tUntagged lcnv) Nothing now conv.metadata.cnvmTeam EdSystemConvDelete) - (Set.fromList (map (.id_) conv.localMembers)) + orAlternativelyDeleteConv conv _ _ = + if shouldSkipSystemAdminlessDeletion mlusr conv + then logSkippedSystemAdminlessDeletion conv + else do + removeConversation (qualifyAs lcnv conv) + case mlusr of + Just lusr -> + void $ + sendConversationActionNotifications + (sing @'ConversationDeleteTag) + (tUntagged lusr) + False + Nothing + (qualifyAs lcnv conv) + (convBotsAndMembers conv) + () + def + Nothing -> do + now <- Now.get + Notify.pushSystemEvent + Nothing + (SystemEvent (tUntagged lcnv) Nothing now conv.metadata.cnvmTeam EdSystemConvDelete) + (Set.fromList (map (.id_) conv.localMembers)) adminlessAutopromoteOrSendReminder :: ( Member ConversationStore r, @@ -1489,30 +1487,33 @@ adminlessAutopromoteOrSendReminder :: Sem r () adminlessAutopromoteOrSendReminder mlusr lcnv deletionScheduledFor = adminlessTryAutopromote mlusr lcnv orAlternativelySendReminder where - orAlternativelySendReminder conv _ _ = do - now <- Now.get - case mlusr of - Just lusr -> do - let event = - Event - (tUntagged lcnv) - Nothing - (EventFromUser (tUntagged lusr)) - now - (conv.metadata.cnvmTeam) - (EdAdminlessReminder (AdminlessReminder deletionScheduledFor)) - pushConversationEvent Nothing conv event (qualifyAs lcnv (map (.id_) conv.localMembers)) [] - Nothing -> - Notify.pushSystemEvent - Nothing - ( SystemEvent - (tUntagged lcnv) + orAlternativelySendReminder conv _ _ = + if shouldSkipSystemAdminlessDeletion mlusr conv + then logSkippedSystemAdminlessDeletion conv + else do + now <- Now.get + case mlusr of + Just lusr -> do + let event = + Event + (tUntagged lcnv) + Nothing + (EventFromUser (tUntagged lusr)) + now + (conv.metadata.cnvmTeam) + (EdAdminlessReminder (AdminlessReminder deletionScheduledFor)) + pushConversationEvent Nothing conv event (qualifyAs lcnv (map (.id_) conv.localMembers)) [] + Nothing -> + Notify.pushSystemEvent Nothing - now - conv.metadata.cnvmTeam - (EdSystemAdminlessReminder (AdminlessReminder deletionScheduledFor)) - ) - (Set.fromList (map (.id_) conv.localMembers)) + ( SystemEvent + (tUntagged lcnv) + Nothing + now + conv.metadata.cnvmTeam + (EdSystemAdminlessReminder (AdminlessReminder deletionScheduledFor)) + ) + (Set.fromList (map (.id_) conv.localMembers)) -- Use eight random bytes and fold them into a big-endian Word64. This keeps -- the helper small, deterministic under tests, and free of extra Random API. From 3c2858e92c722621d4ba54584627b8934834b8aa Mon Sep 17 00:00:00 2001 From: Leif Battermann Date: Fri, 7 Aug 2026 12:38:05 +0200 Subject: [PATCH 08/10] Update libs/wire-subsystems/src/Wire/ConversationSubsystem/Update.hs Co-authored-by: Gautier DI FOLCO --- libs/wire-subsystems/src/Wire/ConversationSubsystem/Update.hs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/wire-subsystems/src/Wire/ConversationSubsystem/Update.hs b/libs/wire-subsystems/src/Wire/ConversationSubsystem/Update.hs index 8730a58a8e..c80a942b96 100644 --- a/libs/wire-subsystems/src/Wire/ConversationSubsystem/Update.hs +++ b/libs/wire-subsystems/src/Wire/ConversationSubsystem/Update.hs @@ -1206,7 +1206,7 @@ logSkippedSystemAdminlessDeletion :: (Member TinyLog r) => StoredConversation -> logSkippedSystemAdminlessDeletion conv = P.info $ Log.msg (Log.val "Skipping senderless adminless deletion for conversation with remote members") - . Log.field "conversation_id" (show conv.id_) + . Log.field "conversation_id" (idToText conv.id_) setupAdminlessGroupsCleanup :: ( Member ConversationStore r, From f7cea24df93a7dfd3c0de64ee61e33560eadfd9f Mon Sep 17 00:00:00 2001 From: Leif Battermann Date: Fri, 7 Aug 2026 12:57:48 +0200 Subject: [PATCH 09/10] fix PR findings --- integration/test/Test/AdminlessGroups.hs | 32 +++++++++++++++++++ .../src/Wire/ConversationSubsystem/Update.hs | 16 +++++----- 2 files changed, 40 insertions(+), 8 deletions(-) diff --git a/integration/test/Test/AdminlessGroups.hs b/integration/test/Test/AdminlessGroups.hs index 6541d9d823..d6c18413f3 100644 --- a/integration/test/Test/AdminlessGroups.hs +++ b/integration/test/Test/AdminlessGroups.hs @@ -388,6 +388,38 @@ testAdminlessSetupSkipsDeletionForRemoteMembers = do bindResponse (GalleyI.getConversation conv) $ \resp -> do resp.status `shouldMatchInt` 200 +testAdminlessSetupSkipsReminderForRemoteMembers :: (HasCallStack) => App () +testAdminlessSetupSkipsReminderForRemoteMembers = do + -- A remote member prevents senderless deletion. The remaining local app is + -- not eligible for promotion, but would receive a system reminder if one + -- were emitted. + (alice, tid, _) <- createTeam OwnDomain 1 + remoteUser <- randomUser OtherDomain def + connectTwoUsers alice remoteUser + + configureAdminlessGroupsFeature OwnDomain tid "disabled" "5s" ["4s"] + + alice1 <- createMLSClient def alice + remoteUser1 <- createMLSClient def remoteUser + traverse_ (uploadNewKeyPackage def) [alice1, remoteUser1] + + conv <- createTeamMLSConversation alice tid alice1 [remoteUser] + let newApp = def {name = "adminless-federated-reminder-app", description = "not eligible for promotion"} + (app, _) <- createAndAddAppMember alice tid alice1 conv newApp + + -- Create an adminless conversation while the feature is disabled. Enabling + -- it through the internal path runs senderless setup cleanup. + removeMember alice conv alice >>= assertSuccess + + withWebSockets [app] $ \[wsApp] -> do + configureAdminlessGroupsFeature OwnDomain tid "enabled" "2s" ["1s"] + + reminderResult <- awaitNMatchesResultFor 5 1 isConvSystemAdminlessReminderNotif wsApp + reminderResult.success `shouldMatch` False + + bindResponse (GalleyI.getConversation conv) $ \resp -> do + resp.status `shouldMatchInt` 200 + testAdminlessSetupAutopromotesWithRemoteMembers :: (HasCallStack) => App () testAdminlessSetupAutopromotesWithRemoteMembers = do -- Autopromotion is safe with remote members because the owning backend is diff --git a/libs/wire-subsystems/src/Wire/ConversationSubsystem/Update.hs b/libs/wire-subsystems/src/Wire/ConversationSubsystem/Update.hs index c80a942b96..b4b8f3f567 100644 --- a/libs/wire-subsystems/src/Wire/ConversationSubsystem/Update.hs +++ b/libs/wire-subsystems/src/Wire/ConversationSubsystem/Update.hs @@ -97,7 +97,6 @@ import Polysemy import Polysemy.Error import Polysemy.Input import Polysemy.TinyLog -import Polysemy.TinyLog qualified as P import System.Logger qualified as Log import Wire.API.Bot hiding (addBot) import Wire.API.Conversation hiding (Member) @@ -1202,11 +1201,12 @@ shouldSkipSystemAdminlessDeletion mlusr conv = isNothing mlusr && not (null conv.remoteMembers) -logSkippedSystemAdminlessDeletion :: (Member TinyLog r) => StoredConversation -> Sem r () -logSkippedSystemAdminlessDeletion conv = - P.info $ +logSkippedSystemAdminlessDeletion :: (Member TinyLog r) => Text -> StoredConversation -> Sem r () +logSkippedSystemAdminlessDeletion action conv = + info $ Log.msg (Log.val "Skipping senderless adminless deletion for conversation with remote members") . Log.field "conversation_id" (idToText conv.id_) + . Log.field "action" action setupAdminlessGroupsCleanup :: ( Member ConversationStore r, @@ -1232,7 +1232,7 @@ setupAdminlessGroupsCleanup mUsr tid = do lcnv <- qualifyLocal cnv adminlessTryAutopromote mUsr lcnv $ \conv feature _ -> if shouldSkipSystemAdminlessDeletion mUsr conv - then logSkippedSystemAdminlessDeletion conv + then logSkippedSystemAdminlessDeletion "schedule_for_deletion" conv else scheduleDeletion lcnv mUsr tid feature guardPreventAdminlessGroups :: @@ -1446,7 +1446,7 @@ adminlessAutopromoteOrDelete mlusr lcnv = adminlessTryAutopromote mlusr lcnv orA where orAlternativelyDeleteConv conv _ _ = if shouldSkipSystemAdminlessDeletion mlusr conv - then logSkippedSystemAdminlessDeletion conv + then logSkippedSystemAdminlessDeletion "deletion" conv else do removeConversation (qualifyAs lcnv conv) case mlusr of @@ -1485,11 +1485,11 @@ adminlessAutopromoteOrSendReminder :: Local ConvId -> UTCTimeMillis -> Sem r () -adminlessAutopromoteOrSendReminder mlusr lcnv deletionScheduledFor = adminlessTryAutopromote mlusr lcnv orAlternativelySendReminder +adminlessAutopromoteOrSendReminder mlusr lcnv deletiongheduledFor = adminlessTryAutopromote mlusr lcnv orAlternativelySendReminder where orAlternativelySendReminder conv _ _ = if shouldSkipSystemAdminlessDeletion mlusr conv - then logSkippedSystemAdminlessDeletion conv + then logSkippedSystemAdminlessDeletion "reminde" conv else do now <- Now.get case mlusr of From 434d3fc1a72913f0cafcfbcd2bced88986457704 Mon Sep 17 00:00:00 2001 From: Leif Battermann Date: Fri, 7 Aug 2026 13:19:55 +0200 Subject: [PATCH 10/10] fixed typo --- libs/wire-subsystems/src/Wire/ConversationSubsystem/Update.hs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libs/wire-subsystems/src/Wire/ConversationSubsystem/Update.hs b/libs/wire-subsystems/src/Wire/ConversationSubsystem/Update.hs index b4b8f3f567..78474964d8 100644 --- a/libs/wire-subsystems/src/Wire/ConversationSubsystem/Update.hs +++ b/libs/wire-subsystems/src/Wire/ConversationSubsystem/Update.hs @@ -1485,11 +1485,11 @@ adminlessAutopromoteOrSendReminder :: Local ConvId -> UTCTimeMillis -> Sem r () -adminlessAutopromoteOrSendReminder mlusr lcnv deletiongheduledFor = adminlessTryAutopromote mlusr lcnv orAlternativelySendReminder +adminlessAutopromoteOrSendReminder mlusr lcnv deletionScheduledFor = adminlessTryAutopromote mlusr lcnv orAlternativelySendReminder where orAlternativelySendReminder conv _ _ = if shouldSkipSystemAdminlessDeletion mlusr conv - then logSkippedSystemAdminlessDeletion "reminde" conv + then logSkippedSystemAdminlessDeletion "reminder" conv else do now <- Now.get case mlusr of