mirror of
https://github.com/logos-messaging/libchat.git
synced 2026-08-03 13:43:18 +00:00
feat: surface pending group invites on the roster (#188)
A GroupV2 add is staged as a proposal that a later commit merges, so between the two the invited member sits in the conversation's own state and nowhere in its public roster, leaving a caller no way to observe an invite in flight. group_members now returns those invites after the committed members, flagged pending. The flag is inviter-local and transient: a conversation records only the joiners this client proposed, and drops each one as the commit admitting it lands. GroupV1 never reports a pending member, since its add merges its own commit before returning.
This commit is contained in:
parent
d2124fd07c
commit
639632b775
@ -48,6 +48,12 @@ pub(crate) trait GroupConvo<S: ExternalServices>: Convo<S> + std::fmt::Debug + S
|
||||
/// Each current member's MLS leaf-credential content (hex-encoded), self
|
||||
/// included.
|
||||
fn members(&self) -> Result<Vec<Vec<u8>>, ChatError>;
|
||||
|
||||
/// Each member this conversation invited and the group has not committed
|
||||
/// yet, in the same encoding as [`Self::members`]. Covers only invites
|
||||
/// [`Self::add_member`] made here, and is empty for a conversation kind
|
||||
/// whose add takes effect within that call.
|
||||
fn pending_members(&self) -> Result<Vec<Vec<u8>>, ChatError>;
|
||||
// All GroupConvos MUST return ConvoMetadata
|
||||
// the return type is Option<_> to support legacy ConvoTypes which
|
||||
// are being phased out.
|
||||
|
||||
@ -352,6 +352,12 @@ impl<S: ExternalServices> GroupConvo<S> for GroupV1Convo {
|
||||
.collect())
|
||||
}
|
||||
|
||||
/// Always empty: `add_member` merges its own commit, so an added member is
|
||||
/// on the roster by the time the call returns.
|
||||
fn pending_members(&self) -> Result<Vec<Vec<u8>>, ChatError> {
|
||||
Ok(Vec::new())
|
||||
}
|
||||
|
||||
fn metadata(&self) -> Option<ConvoMetadata> {
|
||||
None
|
||||
}
|
||||
|
||||
@ -386,6 +386,14 @@ where
|
||||
Ok(members)
|
||||
}
|
||||
|
||||
fn pending_members(&self) -> Result<Vec<Vec<u8>>, ChatError> {
|
||||
Ok(self
|
||||
.pending_invites
|
||||
.iter()
|
||||
.map(|(member_id, _)| member_id.clone())
|
||||
.collect())
|
||||
}
|
||||
|
||||
fn metadata(&self) -> Option<ConvoMetadata> {
|
||||
let res = self.conversation.extensions().iter().find_map(|ext| {
|
||||
if let Extension::Unknown(ext_type, UnknownExtension(bytes)) = ext
|
||||
|
||||
@ -306,6 +306,24 @@ impl<'a, S: ExternalServices + 'static> Core<S> {
|
||||
}
|
||||
}
|
||||
|
||||
/// Each member invited here and still awaiting the group's commit, in the
|
||||
/// same encoding as [`Self::group_members`]; errors if `convo_id` names a
|
||||
/// direct (non-group) conversation.
|
||||
pub fn group_pending_members(&mut self, convo_id: &str) -> Result<Vec<Vec<u8>>, ChatError> {
|
||||
let convo = self
|
||||
.cached_convos
|
||||
.get(convo_id)
|
||||
.ok_or_else(|| ChatError::NoConvo(convo_id.to_string()))?;
|
||||
|
||||
match convo {
|
||||
ConvoTypeOwned::Group(group_convo) => group_convo.pending_members(),
|
||||
ConvoTypeOwned::Direct(convo) => Err(ChatError::UnsupportedFunction(
|
||||
convo.id().into(),
|
||||
"List Pending Members".into(),
|
||||
)),
|
||||
}
|
||||
}
|
||||
|
||||
pub fn list_conversations(&self) -> Result<Vec<ConversationId>, ChatError> {
|
||||
// Check Legacy load_convo store
|
||||
let records = self.services.store.load_conversations()?;
|
||||
|
||||
@ -26,12 +26,19 @@ type LocalSignerId = IdentId;
|
||||
/// Shares [`MessageSender`]'s field semantics: `account` is set only when the
|
||||
/// member's credential claimed an account *and* the directory confirmed this
|
||||
/// device belongs to it. Unlike a message sender, an unconfirmable claim does
|
||||
/// not hide the member: it is cryptographically in the group, so it is listed
|
||||
/// by `local_identity` (its device) with `account: None`.
|
||||
/// not hide the member: a committed member is cryptographically in the group,
|
||||
/// so it is listed by `local_identity` (its device) with `account: None`.
|
||||
///
|
||||
/// `pending` marks a member whose add the group has not committed yet, so it
|
||||
/// cannot read the conversation. Only invites this client sent are reported;
|
||||
/// an add another member proposed is invisible until it commits. The flag
|
||||
/// clears when the commit admitting the member lands, and an invite the group
|
||||
/// never commits stays pending for the life of the conversation.
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub struct GroupMember {
|
||||
pub account: Option<IdentId>,
|
||||
pub local_identity: IdentId,
|
||||
pub pending: bool,
|
||||
}
|
||||
|
||||
/// Metadata a caller supplies when creating a group: its shared name and
|
||||
@ -222,17 +229,31 @@ where
|
||||
.map_err(Into::into)
|
||||
}
|
||||
|
||||
/// The group's roster, one [`GroupMember`] per account (self included). An
|
||||
/// account's several devices collapse to a single entry surfacing that
|
||||
/// account; a member whose account claim the directory can't confirm stays
|
||||
/// on the roster individually, keyed by its device. Costs one directory
|
||||
/// lookup per member that claims an account, the same per-member cost a
|
||||
/// received message's sender check pays.
|
||||
/// The group's roster, one [`GroupMember`] per account (self included),
|
||||
/// committed members first and this client's uncommitted invites after
|
||||
/// them, flagged `pending`. An account's several devices collapse to a
|
||||
/// single entry surfacing that account; a member whose account claim the
|
||||
/// directory can't confirm stays on the roster individually, keyed by its
|
||||
/// device. An account that is both committed and pending collapses to its
|
||||
/// committed entry. Costs one directory lookup per member that claims an
|
||||
/// account, the same per-member cost a received message's sender check pays.
|
||||
pub fn group_members(&mut self, convo_id: &str) -> Result<Vec<GroupMember>, ClientError> {
|
||||
let credentials = self.core.lock().group_members(convo_id)?;
|
||||
let members = credentials
|
||||
let (committed, pending) = {
|
||||
let mut core = self.core.lock();
|
||||
(
|
||||
core.group_members(convo_id)?,
|
||||
core.group_pending_members(convo_id)?,
|
||||
)
|
||||
};
|
||||
let members = committed
|
||||
.iter()
|
||||
.filter_map(|credential| roster_member(&self.directory, credential));
|
||||
.filter_map(|credential| roster_member(&self.directory, credential))
|
||||
.chain(pending.iter().filter_map(|credential| {
|
||||
roster_member(&self.directory, credential).map(|member| GroupMember {
|
||||
pending: true,
|
||||
..member
|
||||
})
|
||||
}));
|
||||
Ok(dedup_members(members))
|
||||
}
|
||||
|
||||
@ -497,6 +518,7 @@ fn roster_member(directory: &impl AccountDirectory, encoded: &[u8]) -> Option<Gr
|
||||
Some(GroupMember {
|
||||
account,
|
||||
local_identity: device,
|
||||
pending: false,
|
||||
})
|
||||
}
|
||||
|
||||
@ -763,6 +785,7 @@ mod sender_check_tests {
|
||||
Some(GroupMember {
|
||||
account: Some(local_id(&account)),
|
||||
local_identity: local_id(&device),
|
||||
pending: false,
|
||||
})
|
||||
);
|
||||
}
|
||||
@ -782,6 +805,7 @@ mod sender_check_tests {
|
||||
Some(GroupMember {
|
||||
account: None,
|
||||
local_identity: local_id(&spoofer),
|
||||
pending: false,
|
||||
})
|
||||
);
|
||||
}
|
||||
@ -797,6 +821,7 @@ mod sender_check_tests {
|
||||
Some(GroupMember {
|
||||
account: None,
|
||||
local_identity: local_id(&device),
|
||||
pending: false,
|
||||
})
|
||||
);
|
||||
}
|
||||
@ -817,6 +842,7 @@ mod sender_check_tests {
|
||||
Some(GroupMember {
|
||||
account: None,
|
||||
local_identity: local_id(&device),
|
||||
pending: false,
|
||||
})
|
||||
);
|
||||
}
|
||||
@ -833,6 +859,7 @@ mod sender_check_tests {
|
||||
Some(GroupMember {
|
||||
account: None,
|
||||
local_identity: local_id(&device),
|
||||
pending: false,
|
||||
})
|
||||
);
|
||||
}
|
||||
@ -845,10 +872,12 @@ mod sender_check_tests {
|
||||
let with_account = |account: &str, device: &str| GroupMember {
|
||||
account: Some(IdentId::new(account.to_string())),
|
||||
local_identity: IdentId::new(device.to_string()),
|
||||
pending: false,
|
||||
};
|
||||
let device_only = |device: &str| GroupMember {
|
||||
account: None,
|
||||
local_identity: IdentId::new(device.to_string()),
|
||||
pending: false,
|
||||
};
|
||||
let roster = dedup_members(vec![
|
||||
with_account("alice", "alice-dev-1"),
|
||||
@ -862,4 +891,25 @@ mod sender_check_tests {
|
||||
// Alice's collapsed entry keeps her first-seen device.
|
||||
assert_eq!(roster[0].local_identity.as_str(), "alice-dev-1");
|
||||
}
|
||||
|
||||
/// An account that is both committed and pending collapses to its committed
|
||||
/// entry: `group_members` chains committed members first, and dedup keeps
|
||||
/// the first entry per account.
|
||||
#[test]
|
||||
fn dedup_collapses_a_pending_duplicate_into_the_committed_member() {
|
||||
let committed = GroupMember {
|
||||
account: Some(IdentId::new("alice")),
|
||||
local_identity: IdentId::new("alice-dev-1"),
|
||||
pending: false,
|
||||
};
|
||||
let pending = GroupMember {
|
||||
account: Some(IdentId::new("alice")),
|
||||
local_identity: IdentId::new("alice-dev-2"),
|
||||
pending: true,
|
||||
};
|
||||
assert_eq!(
|
||||
dedup_members(vec![committed.clone(), pending]),
|
||||
vec![committed]
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@ -40,8 +40,18 @@ type TestClient = ChatClient<InProcessDelivery, EphemeralRegistry, ChatStorage>;
|
||||
/// the endorsing bundle, and builds the client on the shared bus/registry with
|
||||
/// the fast GroupV2 timers. Returns the account address peers invite by.
|
||||
fn create_test_client(
|
||||
message_bus: MessageBus,
|
||||
reg: EphemeralRegistry,
|
||||
) -> (TestClient, Receiver<Event>, String) {
|
||||
create_test_client_with(message_bus, reg, fast_group_v2_config())
|
||||
}
|
||||
|
||||
/// [`create_test_client`] with explicit GroupV2 timers, for a test that needs
|
||||
/// to observe the group between two protocol steps.
|
||||
fn create_test_client_with(
|
||||
message_bus: MessageBus,
|
||||
mut reg: EphemeralRegistry,
|
||||
config: GroupV2Config,
|
||||
) -> (TestClient, Receiver<Event>, String) {
|
||||
let account = TestLogosAccount::new();
|
||||
let delegate = DelegateSigner::random();
|
||||
@ -52,7 +62,7 @@ fn create_test_client(
|
||||
.ident(delegate)
|
||||
.transport(InProcessDelivery::new(message_bus))
|
||||
.registration(reg)
|
||||
.group_v2_config(fast_group_v2_config())
|
||||
.group_v2_config(config)
|
||||
.build()
|
||||
.expect("client create");
|
||||
let addr = client.addr().to_string();
|
||||
@ -92,10 +102,11 @@ fn wait_for_group_started(events: &Receiver<Event>, label: &str) -> String {
|
||||
})
|
||||
}
|
||||
|
||||
/// Poll a client's roster for `convo_id` until its verified accounts equal
|
||||
/// `expected` (order-independent), or panic after a timeout. The roster settles
|
||||
/// asynchronously as each member applies the add commit, so it is polled rather
|
||||
/// than snapshotted.
|
||||
/// Poll a client's roster for `convo_id` until its verified *committed*
|
||||
/// accounts equal `expected` (order-independent), or panic after a timeout. The
|
||||
/// roster settles asynchronously as each member applies the add commit, so it is
|
||||
/// polled rather than snapshotted; members still awaiting that commit are
|
||||
/// skipped so an invite alone never reads as convergence.
|
||||
fn wait_for_members(client: &mut TestClient, convo_id: &str, expected: &[&str]) {
|
||||
use std::collections::BTreeSet;
|
||||
let want: BTreeSet<&str> = expected.iter().copied().collect();
|
||||
@ -104,6 +115,7 @@ fn wait_for_members(client: &mut TestClient, convo_id: &str, expected: &[&str])
|
||||
let roster = client.group_members(convo_id).expect("group_members");
|
||||
let got: BTreeSet<&str> = roster
|
||||
.iter()
|
||||
.filter(|m| !m.pending)
|
||||
.filter_map(|m| m.account.as_ref().map(|a| a.as_str()))
|
||||
.collect();
|
||||
if got == want {
|
||||
@ -275,6 +287,77 @@ fn group_creator_is_in_own_roster() {
|
||||
assert_eq!(accounts, vec![Some(saro_addr.as_str())]);
|
||||
}
|
||||
|
||||
/// An invited member joins the roster immediately, flagged pending: the add is
|
||||
/// staged as a proposal, so the invitee is not a member until the group commits.
|
||||
#[test]
|
||||
fn invited_member_is_pending_until_the_group_commits() {
|
||||
let bus = MessageBus::default();
|
||||
let reg = EphemeralRegistry::new();
|
||||
|
||||
// A commit window far longer than the assertions below, so the add provably
|
||||
// cannot merge while they run.
|
||||
let deferred_commit = GroupV2Config {
|
||||
commit_inactivity_duration: Duration::from_secs(30),
|
||||
..fast_group_v2_config()
|
||||
};
|
||||
let (mut saro, _saro_events, saro_addr) =
|
||||
create_test_client_with(bus.clone(), reg.clone(), deferred_commit);
|
||||
let (_raya, _raya_events, raya_addr) = create_test_client(bus.clone(), reg.clone());
|
||||
|
||||
let convo_id = saro
|
||||
.create_group_conversation(&[], unnamed_group())
|
||||
.expect("empty group");
|
||||
saro.add_group_members(&convo_id, &[&raya_addr])
|
||||
.expect("saro invites raya");
|
||||
|
||||
let roster = saro.group_members(&convo_id).expect("group_members");
|
||||
let accounts = |pending: bool| -> Vec<&str> {
|
||||
roster
|
||||
.iter()
|
||||
.filter(|m| m.pending == pending)
|
||||
.filter_map(|m| m.account.as_ref().map(|a| a.as_str()))
|
||||
.collect()
|
||||
};
|
||||
assert_eq!(accounts(false), vec![saro_addr.as_str()]);
|
||||
assert_eq!(accounts(true), vec![raya_addr.as_str()]);
|
||||
}
|
||||
|
||||
/// The pending flag is transient: once the group commits the add, the invitee
|
||||
/// is an ordinary roster member and nothing is left pending. The joiner, which
|
||||
/// invited nobody, never reports a pending member at all: the flag is local to
|
||||
/// the client that sent the invite.
|
||||
#[test]
|
||||
fn pending_clears_once_the_add_commits() {
|
||||
let bus = MessageBus::default();
|
||||
let reg = EphemeralRegistry::new();
|
||||
|
||||
let (mut saro, _saro_events, saro_addr) = create_test_client(bus.clone(), reg.clone());
|
||||
let (mut raya, raya_events, raya_addr) = create_test_client(bus.clone(), reg.clone());
|
||||
|
||||
let convo_id = saro
|
||||
.create_group_conversation(&[], unnamed_group())
|
||||
.expect("empty group");
|
||||
saro.add_group_members(&convo_id, &[&raya_addr])
|
||||
.expect("saro invites raya");
|
||||
|
||||
let raya_convo_id = wait_for_group_started(&raya_events, "raya ConversationStarted");
|
||||
wait_for_members(&mut saro, &convo_id, &[&saro_addr, &raya_addr]);
|
||||
|
||||
let roster = saro.group_members(&convo_id).expect("group_members");
|
||||
assert!(
|
||||
roster.iter().all(|m| !m.pending),
|
||||
"committed roster still reports a pending member: {roster:?}"
|
||||
);
|
||||
|
||||
let joiner_roster = raya
|
||||
.group_members(&raya_convo_id)
|
||||
.expect("joiner group_members");
|
||||
assert!(
|
||||
joiner_roster.iter().all(|m| !m.pending),
|
||||
"joiner reports a pending member it never invited: {joiner_roster:?}"
|
||||
);
|
||||
}
|
||||
|
||||
/// A batch add is validated before any member is proposed: a member whose
|
||||
/// account is endorsed in the directory but whose device registered no key
|
||||
/// package fails the whole call, the resolvable member in the same batch is
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user