From 779a8a7cff9112658d78008fc51f05e553407ce7 Mon Sep 17 00:00:00 2001 From: spaced4ndy <8711996+spaced4ndy@users.noreply.github.com> Date: Thu, 28 May 2026 19:15:31 +0400 Subject: [PATCH] wip --- bots/api/TYPES.md | 1 + .../types/typescript/src/types.ts | 1 + .../src/simplex_chat/types/_types.py | 1 + src/Simplex/Chat/Library/Internal.hs | 7 +- src/Simplex/Chat/Library/Subscriber.hs | 10 +- src/Simplex/Chat/Store/Groups.hs | 13 +- tests/ChatTests/Groups.hs | 146 ++++++++++-------- 7 files changed, 107 insertions(+), 72 deletions(-) diff --git a/bots/api/TYPES.md b/bots/api/TYPES.md index 1b843bc6e4..d3fa4aeb47 100644 --- a/bots/api/TYPES.md +++ b/bots/api/TYPES.md @@ -2257,6 +2257,7 @@ MemberSupport: - uiThemes: [UIThemeEntityOverrides](#uithemeentityoverrides)? - customData: JSONObject? - groupSummary: [GroupSummary](#groupsummary) +- rosterVersion: int? - membersRequireAttention: int - viaGroupLinkUri: string? - groupKeys: [GroupKeys](#groupkeys)? diff --git a/packages/simplex-chat-client/types/typescript/src/types.ts b/packages/simplex-chat-client/types/typescript/src/types.ts index 1b9e9f6f65..b4ee5a19d2 100644 --- a/packages/simplex-chat-client/types/typescript/src/types.ts +++ b/packages/simplex-chat-client/types/typescript/src/types.ts @@ -2564,6 +2564,7 @@ export interface GroupInfo { uiThemes?: UIThemeEntityOverrides customData?: object groupSummary: GroupSummary + rosterVersion?: number // int membersRequireAttention: number // int viaGroupLinkUri?: string groupKeys?: GroupKeys diff --git a/packages/simplex-chat-python/src/simplex_chat/types/_types.py b/packages/simplex-chat-python/src/simplex_chat/types/_types.py index c378ad56fd..55641345a6 100644 --- a/packages/simplex-chat-python/src/simplex_chat/types/_types.py +++ b/packages/simplex-chat-python/src/simplex_chat/types/_types.py @@ -1801,6 +1801,7 @@ class GroupInfo(TypedDict): uiThemes: NotRequired["UIThemeEntityOverrides"] customData: NotRequired[dict[str, object]] groupSummary: "GroupSummary" + rosterVersion: NotRequired[int] # int membersRequireAttention: int # int viaGroupLinkUri: NotRequired[str] groupKeys: NotRequired["GroupKeys"] diff --git a/src/Simplex/Chat/Library/Internal.hs b/src/Simplex/Chat/Library/Internal.hs index 0086f6f12b..550b5dfffc 100644 --- a/src/Simplex/Chat/Library/Internal.hs +++ b/src/Simplex/Chat/Library/Internal.hs @@ -1182,7 +1182,8 @@ introduceInChannel _ _ _ GroupMember {activeConn = Nothing} = throwChatError $ C introduceInChannel vr user gInfo subscriber@GroupMember {activeConn = Just conn, indexInGroup = subscriberIdx} = do -- roster first, so the joiner trusts mod/admin keys before relay intros forwardCachedRoster user gInfo subscriber - modMs <- withStore' $ \db -> getGroupModerators db vr user gInfo + -- filter to current members so left/removed mods/admins aren't re-introduced to the joiner + modMs <- filter memberCurrent <$> withStore' (\db -> getGroupModerators db vr user gInfo) void $ sendGroupMessage' user gInfo modMs $ XGrpMemNew (memberInfo gInfo subscriber) Nothing withStore' $ \db -> setMemberVectorNewRelations db subscriber [(indexInGroup m, (IDSubjectIntroduced, MRIntroduced)) | m <- modMs] @@ -2143,7 +2144,7 @@ bumpAndBroadcastRoster user gInfo let rosterVer = maybe 0 (+ 1) (rosterVersion gInfo) (relays, roster) <- withStore' $ \db -> do relays <- getGroupRelayMembers db vr user gInfo - mods <- getGroupModsNoOwners db vr user gInfo + mods <- getGroupRosterMembers db vr user gInfo setGroupRosterVersion db gInfo rosterVer pure (relays, buildGroupRoster rosterVer mods) forM_ (L.nonEmpty relays) $ \relays' -> @@ -2154,7 +2155,7 @@ sendGroupRosterToRelay :: User -> GroupInfo -> GroupMember -> CM () sendGroupRosterToRelay user gInfo relayMember = forM_ (rosterVersion gInfo) $ \rosterVer -> do vr <- chatVersionRange - mods <- withStore' $ \db -> getGroupModsNoOwners db vr user gInfo + mods <- withStore' $ \db -> getGroupRosterMembers db vr user gInfo void $ sendGroupMessage' user gInfo [relayMember] (XGrpRoster (buildGroupRoster rosterVer mods)) sendGroupMessages :: MsgEncodingI e => User -> GroupInfo -> Maybe GroupChatScope -> ShowGroupAsSender -> [GroupMember] -> NonEmpty (ChatMsgEvent e) -> CM (NonEmpty (Either ChatError SndMessage), GroupSndResult) diff --git a/src/Simplex/Chat/Library/Subscriber.hs b/src/Simplex/Chat/Library/Subscriber.hs index 2eafb972b3..bfca87f015 100644 --- a/src/Simplex/Chat/Library/Subscriber.hs +++ b/src/Simplex/Chat/Library/Subscriber.hs @@ -3129,7 +3129,10 @@ processAgentMessageConn vr user@User {userId} corrId agentConnId agentMessage = | otherwise = withStore' (\db -> runExceptT $ getGroupMemberByMemberId db vr user gInfo memId) >>= \case Right member -> changeMemberRole gInfo member $ RGEMemberRole (groupMemberId' member) (fromLocalProfile $ memberProfile member) memRole - Left _ -> messageError "x.grp.mem.role with unknown member ID" $> Nothing + -- in relay groups the roster delivers the (privileged) member separately; no-op until then + Left _ + | useRelays' gInfo -> pure Nothing + | otherwise -> messageError "x.grp.mem.role with unknown member ID" $> Nothing where GroupMember {memberId = membershipMemId} = membership changeMemberRole gInfo' member@GroupMember {memberRole = fromRole} gEvent @@ -3176,7 +3179,7 @@ processAgentMessageConn vr user@User {userId} corrId agentConnId agentMessage = warnings <- withStore' $ \db -> do warnings <- forM entries (applyRosterEntryDB db) -- absent privileged members revert to the joiner default - currentPriv <- getGroupModsNoOwners db vr user gInfo + currentPriv <- getGroupRosterMembers db vr user gInfo forM_ currentPriv $ \m -> when (memberId' m `notElem` rosterIds) $ updateGroupMemberRole db user m defaultRole @@ -3340,6 +3343,9 @@ processAgentMessageConn vr user@User {userId} corrId agentConnId agentMessage = (ci, cInfo) <- saveRcvChatItemNoParse user (CDGroupRcv gInfo''' scopeInfo m') msg brokerTs (CIRcvGroupEvent RGEMemberLeft) groupMsgToView cInfo ci toView $ CEvtLeftMember user gInfo''' m' {memberStatus = GSMemLeft} msgSigned + -- a privileged leaver drops out of the roster; bumpAndBroadcastRoster is owner-guarded + when (useRelays' gInfo'' && isRosterRole (memberRole' m)) $ + bumpAndBroadcastRoster user gInfo'' `catchAllErrors` eToView pure $ memberEventDeliveryScope m xGrpDel :: GroupInfo -> GroupMember -> RcvMessage -> UTCTime -> CM () diff --git a/src/Simplex/Chat/Store/Groups.hs b/src/Simplex/Chat/Store/Groups.hs index c9173a2437..a51d304b2c 100644 --- a/src/Simplex/Chat/Store/Groups.hs +++ b/src/Simplex/Chat/Store/Groups.hs @@ -67,7 +67,7 @@ module Simplex.Chat.Store.Groups getGroupMembersByIndexes, getSupportScopeMembersByIndexes, getGroupModerators, - getGroupModsNoOwners, + getGroupRosterMembers, getGroupRelayMembers, getGroupMembersForExpiration, getRemovedMembersToCleanup, @@ -1205,11 +1205,12 @@ getGroupModerators db vr user@User {userId, userContactId} GroupInfo {groupId} = (groupMemberQuery <> " WHERE m.user_id = ? AND m.group_id = ? AND (m.contact_id IS NULL OR m.contact_id != ?) AND m.member_role IN (?,?,?)") (userId, groupId, userContactId, GRModerator, GRAdmin, GROwner) --- Moderators and admins only, excluding owners. Use for roster-related paths --- where owners must not be touched (owners are link-anchored, not roster-managed). -getGroupModsNoOwners :: DB.Connection -> VersionRangeChat -> User -> GroupInfo -> IO [GroupMember] -getGroupModsNoOwners db vr user@User {userId, userContactId} GroupInfo {groupId} = do - map (toContactMember vr user) +-- Moderators and admins only, excluding owners and non-current members. +-- Used for roster-related paths where owners must not be touched (owners are +-- link-anchored), and left/removed members must not appear in the cached roster. +getGroupRosterMembers :: DB.Connection -> VersionRangeChat -> User -> GroupInfo -> IO [GroupMember] +getGroupRosterMembers db vr user@User {userId, userContactId} GroupInfo {groupId} = do + filter memberCurrent . map (toContactMember vr user) <$> DB.query db (groupMemberQuery <> " WHERE m.user_id = ? AND m.group_id = ? AND (m.contact_id IS NULL OR m.contact_id != ?) AND m.member_role IN (?,?)") diff --git a/tests/ChatTests/Groups.hs b/tests/ChatTests/Groups.hs index f0b98d1310..ef12b7ea31 100644 --- a/tests/ChatTests/Groups.hs +++ b/tests/ChatTests/Groups.hs @@ -9407,16 +9407,13 @@ testChannelChangeRoleSigned ps = 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)")]) - -- change role of silent member (other members don't know about member) + -- change role of silent member; cath/eve don't know dan, so the relay-group branch + -- in xGrpMemRole silently no-ops (the roster will deliver the privileged set separately) threadDelay 1000000 alice ##> "/mr #team dan admin" alice <## "#team: you changed the role of dan to admin (signed)" bob <## "#team: alice changed the role of dan from member to admin (signed)" - concurrentlyN_ - [ dan <## "#team: alice changed your role from member to admin (signed)", - cath <## "error: x.grp.mem.role with unknown member ID", - eve <## "error: x.grp.mem.role with unknown member ID" - ] + dan <## "#team: alice changed your role from member to admin (signed)" 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)")]) cath #$> ("/_get chat #1 count=1", chat, [(0, "changed your role to admin (signed)")]) -- now new chat item @@ -9493,46 +9490,59 @@ testChannelModeratorActionViaRoster ps = withNewTestChatOpts ps relayTestOpts "bob" bobProfile $ \bob -> withNewTestChat ps "cath" cathProfile $ \cath -> withNewTestChat ps "dan" danProfile $ \dan -> - withNewTestChat ps "eve" eveProfile $ \eve -> do - createChannel1Relay "team" alice bob cath dan eve + withNewTestChat ps "eve" eveProfile $ \eve -> + withNewTestChat ps "frank" frankProfile $ \frank -> do + (shortLink, fullLink) <- prepareChannel1Relay "team" alice bob + forM_ [cath, dan, eve] $ \member -> + memberJoinChannel "team" [bob] [alice] shortLink fullLink member - -- dan posts, so cath (the future moderator) and eve both discover dan - threadDelay 1000000 - dan #> "#team hello from dan" - bob <# "#team dan> hello from dan" - concurrentlyN_ - [ alice <# "#team dan> hello from dan [>>]", - do - cath <## "#team: bob introduced dan (Daniel) in the channel" - cath <# "#team dan> hello from dan [>>]", - do - eve <## "#team: bob introduced dan (Daniel) in the channel" - eve <# "#team dan> hello from dan [>>]" - ] + -- dan posts so cath and eve discover dan + threadDelay 1000000 + dan #> "#team hello from dan" + bob <# "#team dan> hello from dan" + concurrentlyN_ + [ alice <# "#team dan> hello from dan [>>]", + do + cath <## "#team: bob introduced dan (Daniel) in the channel" + cath <# "#team dan> hello from dan [>>]", + do + eve <## "#team: bob introduced dan (Daniel) in the channel" + eve <# "#team dan> hello from dan [>>]" + ] - -- alice promotes cath to moderator. dan and eve don't know cath yet, so - -- XGrpMemRole errors for them; the owner-signed roster (broadcast on the - -- set change) silently gives them cath's key, name and moderator role. - threadDelay 1000000 - alice ##> "/mr #team cath moderator" - alice <## "#team: you changed the role of cath to moderator (signed)" - bob <## "#team: alice changed the role of cath from member to moderator (signed)" - cath <## "#team: alice changed your role from member to moderator (signed)" - dan <##. "error: x.grp.mem.role" - eve <##. "error: x.grp.mem.role" - -- cath (moderator) blocks dan; eve verifies the signed block against the - -- roster-established key ("(signed)" => verified). cath's profile arrives - -- prepended to the forwarded block. - threadDelay 1000000 - cath ##> "/block for all #team dan" - cath <## "#team: you blocked dan (signed)" - bob <## "#team: cath blocked dan (signed)" - alice <## "#team: cath blocked dan (signed)" - eve <##. "#team: unknown member cath" - eve <##. "#team: bob introduced cath" - eve <## "#team: cath blocked dan (signed)" - dan <##. "#team: unknown member cath" - dan <##. "#team: bob introduced cath" + -- alice promotes cath. dan/eve don't know cath yet, so their XGrpMemRole is silently + -- skipped (channel branch in xGrpMemRole returns Nothing) until the roster arrives + -- and creates cath's record with the new role. + threadDelay 1000000 + alice ##> "/mr #team cath moderator" + alice <## "#team: you changed the role of cath to moderator (signed)" + bob <## "#team: alice changed the role of cath from member to moderator (signed)" + cath <## "#team: alice changed your role from member to moderator (signed)" + + -- cath (moderator) blocks dan; profile prepend carries cath's full profile to dan/eve + threadDelay 1000000 + cath ##> "/block for all #team dan" + cath <## "#team: you blocked dan (signed)" + bob <## "#team: cath blocked dan (signed)" + alice <## "#team: cath blocked dan (signed)" + eve <##. "#team: unknown member cath" + eve <##. "#team: bob introduced cath" + eve <## "#team: cath blocked dan (signed)" + dan <##. "#team: unknown member cath" + dan <##. "#team: bob introduced cath" + + -- frank joins after the roster update; cached roster gives him cath as moderator. + -- both alice (owner) and cath (mod) receive XGrpMemNew(frank) via introduceInChannel + threadDelay 1000000 + memberJoinChannel "team" [bob] [alice, cath] shortLink fullLink frank + threadDelay 500000 + checkMemberRole frank "cath" "moderator" + where + checkMemberRole :: HasCallStack => TestCC -> T.Text -> T.Text -> IO () + checkMemberRole cc name expectedRole = do + roles <- withCCTransaction cc $ \db -> + DB.query db "SELECT member_role FROM group_members WHERE local_display_name = ?" (Only name) :: IO [Only T.Text] + map (\(Only r) -> r) roles `shouldBe` [expectedRole] testChannelRemovedModeratorRefreshesRoster :: HasCallStack => TestParams -> IO () testChannelRemovedModeratorRefreshesRoster ps = @@ -9540,23 +9550,37 @@ testChannelRemovedModeratorRefreshesRoster ps = withNewTestChatOpts ps relayTestOpts "bob" bobProfile $ \bob -> withNewTestChat ps "cath" cathProfile $ \cath -> withNewTestChat ps "dan" danProfile $ \dan -> - withNewTestChat ps "eve" eveProfile $ \eve -> do - createChannel1Relay "team" alice bob cath dan eve - threadDelay 1000000 - alice ##> "/mr #team cath moderator" - alice <## "#team: you changed the role of cath to moderator (signed)" - bob <## "#team: alice changed the role of cath from member to moderator (signed)" - cath <## "#team: alice changed your role from member to moderator (signed)" - dan <##. "error: x.grp.mem.role" - eve <##. "error: x.grp.mem.role" - threadDelay 1000000 - alice ##> "/rm #team cath" - alice <## "#team: you removed cath from the group (signed)" - bob <## "#team: alice removed cath from the group (signed)" - cath <## "#team: alice removed you from the group (signed)" - cath <## "use /d #team to delete the group" - dan .<##. ("#team: alice removed cath", "(signed)") - eve .<##. ("#team: alice removed cath", "(signed)") + withNewTestChat ps "eve" eveProfile $ \eve -> + withNewTestChat ps "frank" frankProfile $ \frank -> do + (shortLink, fullLink) <- prepareChannel1Relay "team" alice bob + forM_ [cath, dan, eve] $ \member -> + memberJoinChannel "team" [bob] [alice] shortLink fullLink member + -- dan/eve don't know cath; XGrpMemRole is silently skipped (relay-group branch) + threadDelay 1000000 + alice ##> "/mr #team cath moderator" + alice <## "#team: you changed the role of cath to moderator (signed)" + bob <## "#team: alice changed the role of cath from member to moderator (signed)" + cath <## "#team: alice changed your role from member to moderator (signed)" + threadDelay 1000000 + alice ##> "/rm #team cath" + alice <## "#team: you removed cath from the group (signed)" + bob <## "#team: alice removed cath from the group (signed)" + cath <## "#team: alice removed you from the group (signed)" + cath <## "use /d #team to delete the group" + dan .<##. ("#team: alice removed cath", "(signed)") + eve .<##. ("#team: alice removed cath", "(signed)") + + -- frank joins after the removal; cached roster has dropped cath + threadDelay 1000000 + memberJoinChannel "team" [bob] [alice] shortLink fullLink frank + threadDelay 500000 + checkMemberRow frank "cath" Nothing + where + checkMemberRow :: HasCallStack => TestCC -> T.Text -> Maybe T.Text -> IO () + checkMemberRow cc name expectedRole = do + roles <- withCCTransaction cc $ \db -> + DB.query db "SELECT member_role FROM group_members WHERE local_display_name = ?" (Only name) :: IO [Only T.Text] + map (\(Only r) -> r) roles `shouldBe` maybeToList expectedRole testChannelRemoveMemberSigned :: HasCallStack => TestParams -> IO () testChannelRemoveMemberSigned ps =