From 8d478b716752a1303fd88809ce8fea832774f65a Mon Sep 17 00:00:00 2001 From: spaced4ndy <8711996+spaced4ndy@users.noreply.github.com> Date: Wed, 24 Jun 2026 14:33:12 +0000 Subject: [PATCH] core: don't create member role change chat item in channels (#7124) --- src/Simplex/Chat/Library/Subscriber.hs | 19 ++++++++++++------- tests/ChatTests/Groups.hs | 14 +++++++------- 2 files changed, 19 insertions(+), 14 deletions(-) diff --git a/src/Simplex/Chat/Library/Subscriber.hs b/src/Simplex/Chat/Library/Subscriber.hs index c3f61f83b0..829319635f 100644 --- a/src/Simplex/Chat/Library/Subscriber.hs +++ b/src/Simplex/Chat/Library/Subscriber.hs @@ -3266,7 +3266,7 @@ processAgentMessageConn cxt user@User {userId} corrId agentConnId agentMessage = | membershipMemId == memId = applyAtRosterVersion gInfo m rosterVer_ $ let gInfo' = gInfo {membership = membership {memberRole = memRole}} - in changeMemberRole gInfo' membership False (\db -> updateGroupMemberRole db user membership memRole) $ RGEUserRole memRole + in changeMemberRole gInfo' membership False (\db -> updateGroupMemberRole db user membership memRole) (RGEUserRole memRole) True | otherwise = applyAtRosterVersion gInfo m rosterVer_ $ do defaultRole <- unknownMemberRole gInfo -- an owner-signed event with a key TOFU-creates an unknown member only for a roster role; else a plain lookup @@ -3276,11 +3276,11 @@ processAgentMessageConn cxt user@User {userId} corrId agentConnId agentMessage = -- just created (keyless, and allowCreate ensured the event carries its key): pin key + role | created, Just (MemberKey pubKey) <- memberKey_ -> let gEvent = RGEMemberRole (groupMemberId' member) (fromLocalProfile $ memberProfile member) memRole - in changeMemberRole gInfo member created (\db -> void $ applyMemberKeyRole db member pubKey memRole) gEvent + in changeMemberRole gInfo member created (\db -> void $ applyMemberKeyRole db member pubKey memRole) gEvent (not $ useRelays' gInfo) -- known member: apply the role (its key is established via roster/intro; the event's key is ignored) | otherwise -> let gEvent = RGEMemberRole (groupMemberId' member) (fromLocalProfile $ memberProfile member) memRole - in changeMemberRole gInfo member created (\db -> updateGroupMemberRole db user member memRole) gEvent + in changeMemberRole gInfo member created (\db -> updateGroupMemberRole db user member memRole) gEvent (not $ useRelays' gInfo) -- in relay groups the roster may deliver role update for previously-unknown privileged members _ | useRelays' gInfo -> pure Nothing | otherwise -> messageError "x.grp.mem.role with unknown member ID" $> Nothing @@ -3288,7 +3288,7 @@ processAgentMessageConn cxt user@User {userId} corrId agentConnId agentMessage = GroupMember {memberId = membershipMemId} = membership -- applyMember writes the change (role, or role + pinned key for a freshly TOFU-created member); -- the delivery scope (relay forwarding) is computed on the pre-change role - changeMemberRole gInfo' member@GroupMember {memberRole = fromRole} created applyMember gEvent + changeMemberRole gInfo' member@GroupMember {memberRole = fromRole} created applyMember gEvent createItem | senderRole < maximum ([GRAdmin, fromRole, memRole] :: [GroupMemberRole]) = messageError "x.grp.mem.role with insufficient member permissions" $> Nothing | useRelays' gInfo && (isRosterRole memRole || isRosterRole fromRole) && senderRole /= GROwner = @@ -3298,9 +3298,14 @@ processAgentMessageConn cxt user@User {userId} corrId agentConnId agentMessage = | useRelays' gInfo && not created && fromRole == memRole = pure $ memberEventDeliveryScope member | otherwise = do withStore' applyMember - (gInfo'', m', scopeInfo) <- mkGroupChatScope gInfo' m - (ci, cInfo) <- saveRcvChatItemNoParse user (CDGroupRcv gInfo'' scopeInfo m') msg brokerTs (CIRcvGroupEvent gEvent) - groupMsgToView cInfo ci + (gInfo'', m') <- + if createItem + then do + (gInfo'', m', scopeInfo) <- mkGroupChatScope gInfo' m + (ci, cInfo) <- saveRcvChatItemNoParse user (CDGroupRcv gInfo'' scopeInfo m') msg brokerTs (CIRcvGroupEvent gEvent) + groupMsgToView cInfo ci + pure (gInfo'', m') + else pure (gInfo', m) toView CEvtMemberRole {user, groupInfo = gInfo'', byMember = m', member = member {memberRole = memRole}, fromRole, toRole = memRole, msgSigned} pure $ memberEventDeliveryScope member diff --git a/tests/ChatTests/Groups.hs b/tests/ChatTests/Groups.hs index a32aa2d07c..d637328925 100644 --- a/tests/ChatTests/Groups.hs +++ b/tests/ChatTests/Groups.hs @@ -9515,6 +9515,8 @@ testChannelChangeRoleSigned ps = -- promote cath to member (observer default) so it can post promoteChannelMember "team" alice bob cath [dan, eve] + threadDelay 1000000 + -- other members discover cath cath #> "#team hello from cath" bob <# "#team cath> hello from cath" @@ -9540,14 +9542,14 @@ testChannelChangeRoleSigned ps = dan <## "#team: alice changed the role of cath from member to admin (signed)", eve <## "#team: alice changed the role of cath from member to admin (signed)" ] + -- chat item is not created for other members alice #$> ("/_get chat #1 count=1", chat, [(1, "changed role of cath to admin (signed)")]) - bob #$> ("/_get chat #1 count=1", chat, [(0, "changed role of cath to admin (signed)")]) + bob #$> ("/_get chat #1 count=1", chat, [(0, "hello from cath")]) cath #$> ("/_get chat #1 count=1", chat, [(0, "changed your role to admin (signed)")]) - dan #$> ("/_get chat #1 count=1", chat, [(0, "changed role of cath to admin (signed)")]) - eve #$> ("/_get chat #1 count=1", chat, [(0, "changed role of cath to admin (signed)")]) + dan #$> ("/_get chat #1 count=1", chat, [(0, "hello from cath")]) + eve #$> ("/_get chat #1 count=1", chat, [(0, "hello from cath")]) - -- change role of silent member; cath/eve don't know dan via xGrpMemRole, but the - -- subsequent roster apply emits the chat item with dan TOFU-created at the new role + -- change role of silent member threadDelay 1000000 alice ##> "/mr #team dan admin" alice <## "#team: you changed the role of dan to admin (signed)" @@ -9557,9 +9559,7 @@ testChannelChangeRoleSigned ps = cath .<##. ("#team: alice changed the role of ", " from observer to admin (signed)"), eve .<##. ("#team: alice changed the role of ", " from observer to admin (signed)") ] - -- cath/eve render dan by id hash (unknown to them, roster-TOFU); arrival verified above alice #$> ("/_get chat #1 count=1", chat, [(1, "changed role of dan to admin (signed)")]) - bob #$> ("/_get chat #1 count=1", chat, [(0, "changed role of dan to admin (signed)")]) dan #$> ("/_get chat #1 count=1", chat, [(0, "changed your role to admin (signed)")]) testChannelBlockMemberSigned :: HasCallStack => TestParams -> IO ()