From 4018fa025c84765a8b28b4986f83c511386fb9a3 Mon Sep 17 00:00:00 2001 From: Cameron Voell <1103838+cameronvoell@users.noreply.github.com> Date: Thu, 18 Apr 2024 22:42:14 -0700 Subject: [PATCH] Mutable Metadata group name Part 2 (#644) * added update metadata intent and mutablemetada obj * add functions for creating updating metadata via context proposal * add ffi updates * remove async methods * adds extension constant, fixed errors from upstream * added MLS extension types table for context * keep metadata update function as group name specific, avoid future breaking changes * Add default group name to constants * Fix error after merge --------- Co-authored-by: cameronvoell Co-authored-by: Naomi Plasterer --- bindings_ffi/src/mls.rs | 24 +++ xmtp_mls/src/configuration.rs | 17 ++ xmtp_mls/src/groups/group_mutable_metadata.rs | 22 ++- xmtp_mls/src/groups/intents.rs | 4 +- xmtp_mls/src/groups/mod.rs | 151 ++++++++++++++++-- xmtp_mls/src/groups/sync.rs | 33 +++- xmtp_mls/src/groups/validated_commit.rs | 23 ++- xmtp_mls/src/identity.rs | 12 +- 8 files changed, 258 insertions(+), 28 deletions(-) diff --git a/bindings_ffi/src/mls.rs b/bindings_ffi/src/mls.rs index ee49476f7..ebf70086b 100644 --- a/bindings_ffi/src/mls.rs +++ b/bindings_ffi/src/mls.rs @@ -445,6 +445,30 @@ impl FfiGroup { Ok(()) } + pub async fn update_group_name(&self, group_name: String) -> Result<(), GenericError> { + let group = MlsGroup::new( + self.inner_client.as_ref(), + self.group_id.clone(), + self.created_at_ns, + ); + + group.update_group_name(group_name).await?; + + Ok(()) + } + + pub fn group_name(&self) -> Result { + let group = MlsGroup::new( + self.inner_client.as_ref(), + self.group_id.clone(), + self.created_at_ns, + ); + + let group_name = group.group_name()?; + + Ok(group_name) + } + pub async fn stream( &self, message_callback: Box, diff --git a/xmtp_mls/src/configuration.rs b/xmtp_mls/src/configuration.rs index 98571bbeb..58fd3a0b1 100644 --- a/xmtp_mls/src/configuration.rs +++ b/xmtp_mls/src/configuration.rs @@ -18,3 +18,20 @@ const NANOSECONDS_IN_HOUR: i64 = 3_600_000_000_000; pub const UPDATE_INSTALLATIONS_INTERVAL_NS: i64 = NANOSECONDS_IN_HOUR / 2; // 30 min pub const MAX_GROUP_SIZE: u8 = 250; + +/// MLS Extension Types +/// +/// Copied from draft-ietf-mls-protocol-16: +/// +/// | Value | Name | Message(s) | Recommended | Reference | +/// |:-----------------|:-------------------------|:-----------|:------------|:----------| +/// | 0x0000 | RESERVED | N/A | N/A | RFC XXXX | +/// | 0x0001 | application_id | LN | Y | RFC XXXX | +/// | 0x0002 | ratchet_tree | GI | Y | RFC XXXX | +/// | 0x0003 | required_capabilities | GC | Y | RFC XXXX | +/// | 0x0004 | external_pub | GI | Y | RFC XXXX | +/// | 0x0005 | external_senders | GC | Y | RFC XXXX | +/// | 0xff00 - 0xffff | Reserved for Private Use | N/A | N/A | RFC XXXX | +pub const MUTABLE_METADATA_EXTENSION_ID: u16 = 0xff00; + +pub const DEFAULT_GROUP_NAME: &str = "New Group"; diff --git a/xmtp_mls/src/groups/group_mutable_metadata.rs b/xmtp_mls/src/groups/group_mutable_metadata.rs index d2f479b3d..f723a5f66 100644 --- a/xmtp_mls/src/groups/group_mutable_metadata.rs +++ b/xmtp_mls/src/groups/group_mutable_metadata.rs @@ -7,6 +7,8 @@ use thiserror::Error; use xmtp_proto::xmtp::mls::message_contents::GroupMutableMetadataV1 as GroupMutableMetadataProto; +use crate::configuration::MUTABLE_METADATA_EXTENSION_ID; + #[derive(Debug, Error)] pub enum GroupMutableMetadataError { #[error("serialization: {0}")] @@ -24,9 +26,7 @@ pub struct GroupMutableMetadata { impl GroupMutableMetadata { pub fn new(group_name: String) -> Self { - Self { - group_name - } + Self { group_name } } } @@ -36,7 +36,7 @@ impl TryFrom for Vec { fn try_from(value: GroupMutableMetadata) -> Result { let mut buf = Vec::new(); let proto_val = GroupMutableMetadataProto { - group_name: value.group_name.clone() + group_name: value.group_name.clone(), }; proto_val.encode(&mut buf)?; @@ -49,7 +49,7 @@ impl TryFrom<&Vec> for GroupMutableMetadata { fn try_from(value: &Vec) -> Result { let proto_val = GroupMutableMetadataProto::decode(value.as_slice())?; - Self::from_proto(proto_val) + Self::try_from(proto_val) } } @@ -57,14 +57,22 @@ impl TryFrom for GroupMutableMetadata { type Error = GroupMutableMetadataError; fn try_from(value: GroupMutableMetadataProto) -> Result { - Self::new(value.group_name.clone()) + Ok(Self::new(value.group_name.clone())) } } pub fn extract_group_mutable_metadata( group: &OpenMlsGroup, ) -> Result { - todo!() + let extensions = group.export_group_context().extensions(); + for extension in extensions.iter() { + if let Extension::Unknown(MUTABLE_METADATA_EXTENSION_ID, UnknownExtension(meta_data)) = + extension + { + return GroupMutableMetadata::try_from(meta_data); + } + } + Err(GroupMutableMetadataError::MissingExtension) } #[cfg(test)] diff --git a/xmtp_mls/src/groups/intents.rs b/xmtp_mls/src/groups/intents.rs index 68fe81d02..ab7f0de70 100644 --- a/xmtp_mls/src/groups/intents.rs +++ b/xmtp_mls/src/groups/intents.rs @@ -2,7 +2,7 @@ use openmls::prelude::{ tls_codec::{Error as TlsCodecError, Serialize}, MlsMessageOut, }; -use prost::{DecodeError, Message}; +use prost::{bytes::Bytes, DecodeError, Message}; use thiserror::Error; use xmtp_proto::xmtp::mls::database::{ @@ -251,8 +251,6 @@ impl From for Vec { } } -use prost::bytes::Bytes; - impl TryFrom> for UpdateMetadataIntentData { type Error = IntentError; diff --git a/xmtp_mls/src/groups/mod.rs b/xmtp_mls/src/groups/mod.rs index 86dc6d923..e341538d6 100644 --- a/xmtp_mls/src/groups/mod.rs +++ b/xmtp_mls/src/groups/mod.rs @@ -1,5 +1,6 @@ mod group_membership; pub mod group_metadata; +pub mod group_mutable_metadata; mod group_permissions; mod intents; mod members; @@ -10,13 +11,18 @@ pub mod validated_commit; use intents::SendMessageIntentData; use openmls::{ - credentials::BasicCredential, + credentials::{BasicCredential, CredentialType}, error::LibraryError, - extensions::{Extension, Extensions, Metadata}, - group::{MlsGroupCreateConfig, MlsGroupJoinConfig}, + extensions::{ + Extension, ExtensionType, Extensions, Metadata, RequiredCapabilitiesExtension, + UnknownExtension, + }, + group::{CreateGroupContextExtProposalError, MlsGroupCreateConfig, MlsGroupJoinConfig}, + messages::proposals::ProposalType, prelude::{ - BasicCredentialError, CredentialWithKey, CryptoConfig, Error as TlsCodecError, GroupId, - MlsGroup as OpenMlsGroup, StagedWelcome, Welcome as MlsWelcome, WireFormatPolicy, + BasicCredentialError, Capabilities, CredentialWithKey, CryptoConfig, + Error as TlsCodecError, GroupId, MlsGroup as OpenMlsGroup, StagedWelcome, + Welcome as MlsWelcome, WireFormatPolicy, }, }; use openmls_traits::OpenMlsProvider; @@ -33,14 +39,20 @@ use xmtp_proto::{ }, message_contents::{ plaintext_envelope::{Content, V1}, - PlaintextEnvelope, + GroupMutableMetadataV1, PlaintextEnvelope, }, }, }; -use self::group_metadata::extract_group_metadata; pub use self::group_permissions::PreconfiguredPolicies; pub use self::intents::{AddressesOrInstallationIds, IntentError}; +use self::{ + group_metadata::extract_group_metadata, + group_mutable_metadata::{ + extract_group_mutable_metadata, GroupMutableMetadata, GroupMutableMetadataError, + }, + intents::UpdateMetadataIntentData, +}; use self::{ group_metadata::{ConversationType, GroupMetadata, GroupMetadataError}, group_permissions::PolicySet, @@ -50,7 +62,9 @@ use self::{ use crate::{ client::{deserialize_welcome, ClientError, MessageProcessingError}, - configuration::{CIPHERSUITE, MAX_GROUP_SIZE}, + configuration::{ + CIPHERSUITE, DEFAULT_GROUP_NAME, MAX_GROUP_SIZE, MUTABLE_METADATA_EXTENSION_ID, + }, hpke::{decrypt_welcome, HpkeError}, identity::{Identity, IdentityError}, retry::RetryableError, @@ -118,6 +132,8 @@ pub enum GroupError { CommitValidation(#[from] CommitValidationError), #[error("Metadata error {0}")] GroupMetadata(#[from] GroupMetadataError), + #[error("Mutable Metadata error {0}")] + GroupMutableMetadata(#[from] GroupMutableMetadataError), #[error("Errors occurred during sync {0:?}")] Sync(Vec), #[error("Hpke error: {0}")] @@ -126,6 +142,8 @@ pub enum GroupError { Identity(#[from] IdentityError), #[error("serialization error: {0}")] EncodeError(#[from] prost::EncodeError), + #[error("create group context proposal error: {0}")] + CreateGroupContextExtProposalError(#[from] CreateGroupContextExtProposalError), #[error("Credential error")] CredentialError(#[from] BasicCredentialError), #[error("LeafNode error")] @@ -199,7 +217,8 @@ where &client.identity, permissions.unwrap_or_default().to_policy_set(), )?; - let group_config = build_group_config(protected_metadata)?; + let mutable_metadata = build_mutable_metadata_extension(DEFAULT_GROUP_NAME.to_string())?; + let group_config = build_group_config(protected_metadata, mutable_metadata)?; let mut mls_group = OpenMlsGroup::new( &provider, @@ -411,6 +430,25 @@ where self.sync_until_intent_resolved(conn, intent.id).await } + pub async fn update_group_name(&self, group_name: String) -> Result<(), GroupError> { + let conn = &mut self.client.store.conn()?; + let intent_data: Vec = UpdateMetadataIntentData::new(group_name).into(); + let intent = conn.insert_group_intent(NewGroupIntent::new( + IntentKind::MetadataUpdate, + self.group_id.clone(), + intent_data, + ))?; + + self.sync_until_intent_resolved(conn, intent.id).await + } + + // Query the database for stored messages. Optionally filtered by time, kind, delivery_status + // and limit + pub fn group_name(&self) -> Result { + let mutable_metadata = self.mutable_metadata()?; + Ok(mutable_metadata.group_name) + } + // Find the wallet address of the group member who added the member to the group pub fn added_by_address(&self) -> Result { let conn = self.client.store.conn()?; @@ -465,6 +503,14 @@ where Ok(extract_group_metadata(&mls_group)?) } + + pub fn mutable_metadata(&self) -> Result { + let conn = &self.client.store.conn()?; + let provider = XmtpOpenMlsProvider::new(conn); + let mls_group = self.load_mls_group(&provider)?; + + Ok(extract_group_mutable_metadata(&mls_group)?) + } } fn extract_message_v1(message: GroupMessage) -> Result { @@ -510,13 +556,55 @@ fn build_protected_metadata_extension( Ok(Extension::ImmutableMetadata(protected_metadata)) } +pub fn build_mutable_metadata_extension(group_name: String) -> Result { + let mutable_metadata: GroupMutableMetadataV1 = GroupMutableMetadataV1 { group_name }; + + let unknown_gc_extension = UnknownExtension(mutable_metadata.encode_to_vec()); + + Ok(Extension::Unknown( + MUTABLE_METADATA_EXTENSION_ID, + unknown_gc_extension, + )) +} + fn build_group_config( protected_metadata_extension: Extension, + mutable_metadata_extension: Extension, ) -> Result { - let extensions = Extensions::single(protected_metadata_extension); + let required_extension_types = &[ + ExtensionType::Unknown(MUTABLE_METADATA_EXTENSION_ID), + ExtensionType::ImmutableMetadata, + ExtensionType::LastResort, + ExtensionType::ApplicationId, + ]; + + let required_proposal_types = &[ProposalType::GroupContextExtensions]; + + let capabilities = Capabilities::new( + None, + None, + Some(required_extension_types), + Some(required_proposal_types), + None, + ); + let credentials = &[CredentialType::Basic]; + + let required_capabilities = + Extension::RequiredCapabilities(RequiredCapabilitiesExtension::new( + required_extension_types, + required_proposal_types, + credentials, + )); + + let extensions = Extensions::from_vec(vec![ + protected_metadata_extension, + mutable_metadata_extension, + required_capabilities, + ])?; Ok(MlsGroupCreateConfig::builder() .with_group_context_extensions(extensions)? + .capabilities(capabilities) .crypto_config(CryptoConfig::with_default_version(CIPHERSUITE)) .wire_format_policy(WireFormatPolicy::default()) .max_past_epochs(3) // Trying with 3 max past epochs for now @@ -1011,6 +1099,49 @@ mod tests { .is_err(),); } + #[tokio::test] + async fn test_group_mutable_data() { + let amal = ClientBuilder::new_test_client(&generate_local_wallet()).await; + let bola = ClientBuilder::new_test_client(&generate_local_wallet()).await; + + // Create a group and verify it has the default group name + let policies = Some(PreconfiguredPolicies::GroupCreatorIsAdmin); + let amal_group: MlsGroup<_> = amal.create_group(policies).unwrap(); + amal_group.sync().await.unwrap(); + + let group_mutable_metadata = amal_group.mutable_metadata().unwrap(); + assert!(group_mutable_metadata.group_name.eq("New Group")); + + // Add bola to the group + amal_group + .add_members(vec![bola.account_address()]) + .await + .unwrap(); + bola.sync_welcomes().await.unwrap(); + let bola_groups = bola.find_groups(None, None, None, None).unwrap(); + assert_eq!(bola_groups.len(), 1); + let bola_group = bola_groups.first().unwrap(); + bola_group.sync().await.unwrap(); + let group_mutable_metadata = bola_group.mutable_metadata().unwrap(); + assert!(group_mutable_metadata.group_name.eq("New Group")); + + // Update group name + amal_group + .update_group_name("New Group Name 1".to_string()) + .await + .unwrap(); + + // Verify amal group sees update + amal_group.sync().await.unwrap(); + let amal_group_name: String = amal_group.mutable_metadata().expect("msg").group_name; + assert_eq!(amal_group_name, "New Group Name 1"); + + // Verify bola group sees update + bola_group.sync().await.unwrap(); + let bola_group_name: String = bola_group.mutable_metadata().expect("msg").group_name; + assert_eq!(bola_group_name, "New Group Name 1"); + } + #[tokio::test] async fn test_staged_welcome() { // Create Clients diff --git a/xmtp_mls/src/groups/sync.rs b/xmtp_mls/src/groups/sync.rs index da39fc252..045a86086 100644 --- a/xmtp_mls/src/groups/sync.rs +++ b/xmtp_mls/src/groups/sync.rs @@ -44,7 +44,10 @@ use crate::{ client::MessageProcessingError, codecs::{membership_change::GroupMembershipChangeCodec, ContentCodec}, configuration::{MAX_INTENT_PUBLISH_ATTEMPTS, UPDATE_INSTALLATIONS_INTERVAL_NS}, - groups::validated_commit::ValidatedCommit, + groups::{ + build_mutable_metadata_extension, intents::UpdateMetadataIntentData, + validated_commit::ValidatedCommit, + }, hpke::{encrypt_welcome, HpkeError}, identity::Identity, retry, @@ -174,7 +177,10 @@ where let conn = provider.conn(); match intent.kind { - IntentKind::AddMembers | IntentKind::RemoveMembers | IntentKind::KeyUpdate => { + IntentKind::AddMembers + | IntentKind::RemoveMembers + | IntentKind::KeyUpdate + | IntentKind::MetadataUpdate => { if !allow_epoch_increment { return Err(MessageProcessingError::EpochIncrementNotAllowed); } @@ -251,7 +257,6 @@ where None => return Err(MessageProcessingError::InvalidPayload), }; } - IntentKind::MetadataUpdate => todo!(), }; conn.set_group_intent_committed(intent.id)?; @@ -669,7 +674,27 @@ where Ok((commit.tls_serialize_detached()?, None)) } - IntentKind::MetadataUpdate => todo!(), + IntentKind::MetadataUpdate => { + let metadata_intent = UpdateMetadataIntentData::try_from(intent.data.clone())?; + let mutable_metadata = + build_mutable_metadata_extension(metadata_intent.group_name)?; + let mut extensions = openmls_group.extensions().clone(); + extensions.add_or_replace(mutable_metadata); + + let (commit, _, _) = openmls_group.update_group_context_extensions( + provider, + extensions, + &self.client.identity.installation_keys, + )?; + + if let Some(staged_commit) = openmls_group.pending_commit() { + // Validate the commit, even if it's from yourself + ValidatedCommit::from_staged_commit(staged_commit, openmls_group)?; + } + let commit_bytes = commit.tls_serialize_detached()?; + + Ok((commit_bytes, None)) + } } } diff --git a/xmtp_mls/src/groups/validated_commit.rs b/xmtp_mls/src/groups/validated_commit.rs index 7e6076934..59f03d426 100644 --- a/xmtp_mls/src/groups/validated_commit.rs +++ b/xmtp_mls/src/groups/validated_commit.rs @@ -364,7 +364,10 @@ impl From for GroupMembershipChanges { mod tests { use openmls::{ credentials::{BasicCredential, CredentialWithKey}, + extensions::ExtensionType, group::config::CryptoConfig, + messages::proposals::ProposalType, + prelude::Capabilities, prelude_test::KeyPackage, versions::ProtocolVersion, }; @@ -372,7 +375,11 @@ mod tests { use xmtp_cryptography::utils::generate_local_wallet; use super::ValidatedCommit; - use crate::{builder::ClientBuilder, configuration::CIPHERSUITE, Client}; + use crate::{ + builder::ClientBuilder, + configuration::{CIPHERSUITE, MUTABLE_METADATA_EXTENSION_ID}, + Client, + }; fn get_key_package(client: &Client) -> KeyPackage { client @@ -499,8 +506,22 @@ mod tests { let amal_group = amal.create_group(None).unwrap(); let mut amal_mls_group = amal_group.load_mls_group(&amal_provider).unwrap(); + let capabilities = Capabilities::new( + None, + Some(&[CIPHERSUITE]), + Some(&[ + ExtensionType::LastResort, + ExtensionType::ApplicationId, + ExtensionType::Unknown(MUTABLE_METADATA_EXTENSION_ID), + ExtensionType::ImmutableMetadata, + ]), + Some(&[ProposalType::GroupContextExtensions]), + None, + ); + // Create a key package with a malformed credential let bad_key_package = KeyPackage::builder() + .leaf_node_capabilities(capabilities) .build( CryptoConfig { ciphersuite: CIPHERSUITE, diff --git a/xmtp_mls/src/identity.rs b/xmtp_mls/src/identity.rs index e632a046f..54bbd8ac4 100644 --- a/xmtp_mls/src/identity.rs +++ b/xmtp_mls/src/identity.rs @@ -7,6 +7,7 @@ use openmls::{ BasicCredential, }, extensions::{errors::InvalidExtensionError, ApplicationIdExtension, LastResortExtension}, + messages::proposals::ProposalType, prelude::{ tls_codec::{Error as TlsCodecError, Serialize}, Capabilities, Credential as OpenMlsCredential, CredentialWithKey, CryptoConfig, Extension, @@ -26,7 +27,7 @@ use xmtp_proto::{ use crate::{ api::{ApiClientWrapper, IdentityUpdate}, - configuration::CIPHERSUITE, + configuration::{CIPHERSUITE, MUTABLE_METADATA_EXTENSION_ID}, credential::{AssociationError, Credential, UnsignedGrantMessagingAccessData}, storage::{identity::StoredIdentity, StorageError}, types::Address, @@ -199,8 +200,13 @@ impl Identity { let capabilities = Capabilities::new( None, Some(&[CIPHERSUITE]), - Some(&[ExtensionType::LastResort, ExtensionType::ApplicationId]), - None, + Some(&[ + ExtensionType::LastResort, + ExtensionType::ApplicationId, + ExtensionType::Unknown(MUTABLE_METADATA_EXTENSION_ID), + ExtensionType::ImmutableMetadata, + ]), + Some(&[ProposalType::GroupContextExtensions]), None, ); let kp = KeyPackage::builder()