From 20978da82322352446086398f6260df13084462e Mon Sep 17 00:00:00 2001 From: Mojtaba Chenani Date: Tue, 7 Jan 2025 17:42:58 +0100 Subject: [PATCH 1/9] feat(group): add message expiration to group metadata --- bindings_ffi/src/mls.rs | 27 +++++++++++++++++++ bindings_node/src/conversations.rs | 2 ++ bindings_node/src/permissions.rs | 8 ++++++ bindings_wasm/src/conversations.rs | 5 ++++ bindings_wasm/src/permissions.rs | 11 ++++++++ xmtp_mls/src/configuration.rs | 1 + xmtp_mls/src/groups/group_mutable_metadata.rs | 14 +++++++++- xmtp_mls/src/groups/group_permissions.rs | 14 ++++++++-- xmtp_mls/src/groups/mod.rs | 2 ++ 9 files changed, 81 insertions(+), 3 deletions(-) diff --git a/bindings_ffi/src/mls.rs b/bindings_ffi/src/mls.rs index f4524df2d..ab9be674e 100644 --- a/bindings_ffi/src/mls.rs +++ b/bindings_ffi/src/mls.rs @@ -783,6 +783,7 @@ pub struct FfiPermissionPolicySet { pub update_group_description_policy: FfiPermissionPolicy, pub update_group_image_url_square_policy: FfiPermissionPolicy, pub update_group_pinned_frame_url_policy: FfiPermissionPolicy, + pub update_message_expiration_ms_policy: FfiPermissionPolicy, } impl From for FfiGroupPermissionsOptions { @@ -814,6 +815,10 @@ impl TryFrom for PolicySet { MetadataField::GroupPinnedFrameUrl.to_string(), policy_set.update_group_pinned_frame_url_policy.try_into()?, ); + metadata_permissions_map.insert( + MetadataField::MessageExpirationMillis.to_string(), + policy_set.update_message_expiration_ms_policy.try_into()?, + ); Ok(PolicySet { add_member_policy: policy_set.add_member_policy.try_into()?, @@ -1365,6 +1370,7 @@ pub struct FfiCreateGroupOptions { pub group_description: Option, pub group_pinned_frame_url: Option, pub custom_permission_policy_set: Option, + pub message_expiration_ms: Option, } impl FfiCreateGroupOptions { @@ -1374,6 +1380,7 @@ impl FfiCreateGroupOptions { image_url_square: self.group_image_url_square, description: self.group_description, pinned_frame_url: self.group_pinned_frame_url, + message_expiration_ms: self.message_expiration_ms, } } } @@ -1963,6 +1970,9 @@ impl FfiGroupPermissions { update_group_pinned_frame_url_policy: get_policy( MetadataField::GroupPinnedFrameUrl.as_str(), ), + update_message_expiration_ms_policy: get_policy( + MetadataField::MessageExpirationMillis.as_str(), + ), }) } } @@ -2679,6 +2689,7 @@ mod tests { group_description: Some("group description".to_string()), group_pinned_frame_url: Some("pinned frame".to_string()), custom_permission_policy_set: None, + message_expiration_ms: None, }, ) .await @@ -3918,6 +3929,7 @@ mod tests { update_group_description_policy: FfiPermissionPolicy::Admin, update_group_image_url_square_policy: FfiPermissionPolicy::Admin, update_group_pinned_frame_url_policy: FfiPermissionPolicy::Admin, + update_message_expiration_ms_policy: FfiPermissionPolicy::Admin, }; assert_eq!(alix_permission_policy_set, expected_permission_policy_set); @@ -3947,6 +3959,7 @@ mod tests { update_group_description_policy: FfiPermissionPolicy::Allow, update_group_image_url_square_policy: FfiPermissionPolicy::Allow, update_group_pinned_frame_url_policy: FfiPermissionPolicy::Allow, + update_message_expiration_ms_policy: FfiPermissionPolicy::Admin, }; assert_eq!(alix_permission_policy_set, expected_permission_policy_set); } @@ -3980,6 +3993,7 @@ mod tests { update_group_description_policy: FfiPermissionPolicy::Admin, update_group_image_url_square_policy: FfiPermissionPolicy::Admin, update_group_pinned_frame_url_policy: FfiPermissionPolicy::Admin, + update_message_expiration_ms_policy: FfiPermissionPolicy::Admin, }; assert_eq!(alix_group_permissions, expected_permission_policy_set); @@ -4007,6 +4021,7 @@ mod tests { update_group_description_policy: FfiPermissionPolicy::Admin, update_group_image_url_square_policy: FfiPermissionPolicy::Allow, update_group_pinned_frame_url_policy: FfiPermissionPolicy::Admin, + update_message_expiration_ms_policy: FfiPermissionPolicy::Admin, }; assert_eq!(alix_group_permissions, new_expected_permission_policy_set); @@ -4060,6 +4075,7 @@ mod tests { update_group_pinned_frame_url_policy: FfiPermissionPolicy::Admin, add_member_policy: FfiPermissionPolicy::Allow, remove_member_policy: FfiPermissionPolicy::Deny, + update_message_expiration_ms_policy: FfiPermissionPolicy::Admin, }; let create_group_options = FfiCreateGroupOptions { @@ -4069,6 +4085,7 @@ mod tests { group_description: Some("A test group".to_string()), group_pinned_frame_url: Some("https://example.com/frame.png".to_string()), custom_permission_policy_set: Some(custom_permissions), + message_expiration_ms: None, }; let alix_group = alix @@ -4107,6 +4124,10 @@ mod tests { group_permissions_policy_set.update_group_pinned_frame_url_policy, FfiPermissionPolicy::Admin ); + assert_eq!( + group_permissions_policy_set.update_message_expiration_ms_policy, + FfiPermissionPolicy::Admin + ); assert_eq!( group_permissions_policy_set.add_member_policy, FfiPermissionPolicy::Allow @@ -4170,6 +4191,7 @@ mod tests { update_group_pinned_frame_url_policy: FfiPermissionPolicy::Admin, add_member_policy: FfiPermissionPolicy::Allow, remove_member_policy: FfiPermissionPolicy::Deny, + update_message_expiration_ms_policy: FfiPermissionPolicy::Admin, }; let custom_permissions_valid = FfiPermissionPolicySet { @@ -4181,6 +4203,7 @@ mod tests { update_group_pinned_frame_url_policy: FfiPermissionPolicy::Admin, add_member_policy: FfiPermissionPolicy::Allow, remove_member_policy: FfiPermissionPolicy::Deny, + update_message_expiration_ms_policy: FfiPermissionPolicy::Admin, }; let create_group_options_invalid_1 = FfiCreateGroupOptions { @@ -4190,6 +4213,7 @@ mod tests { group_description: Some("A test group".to_string()), group_pinned_frame_url: Some("https://example.com/frame.png".to_string()), custom_permission_policy_set: Some(custom_permissions_invalid_1), + message_expiration_ms: None, }; let results_1 = alix @@ -4209,6 +4233,7 @@ mod tests { group_description: Some("A test group".to_string()), group_pinned_frame_url: Some("https://example.com/frame.png".to_string()), custom_permission_policy_set: Some(custom_permissions_valid.clone()), + message_expiration_ms: None, }; let results_2 = alix @@ -4228,6 +4253,7 @@ mod tests { group_description: Some("A test group".to_string()), group_pinned_frame_url: Some("https://example.com/frame.png".to_string()), custom_permission_policy_set: Some(custom_permissions_valid.clone()), + message_expiration_ms: None, }; let results_3 = alix @@ -4247,6 +4273,7 @@ mod tests { group_description: Some("A test group".to_string()), group_pinned_frame_url: Some("https://example.com/frame.png".to_string()), custom_permission_policy_set: Some(custom_permissions_valid), + message_expiration_ms: None, }; let results_4 = alix diff --git a/bindings_node/src/conversations.rs b/bindings_node/src/conversations.rs index b9eda875d..d6aded13a 100644 --- a/bindings_node/src/conversations.rs +++ b/bindings_node/src/conversations.rs @@ -122,6 +122,7 @@ pub struct CreateGroupOptions { pub group_description: Option, pub group_pinned_frame_url: Option, pub custom_permission_policy_set: Option, + pub message_expiration_ms: Option, } impl CreateGroupOptions { @@ -131,6 +132,7 @@ impl CreateGroupOptions { image_url_square: self.group_image_url_square, description: self.group_description, pinned_frame_url: self.group_pinned_frame_url, + message_expiration_ms: self.message_expiration_ms, } } } diff --git a/bindings_node/src/permissions.rs b/bindings_node/src/permissions.rs index 3008c4b4e..024edd7f7 100644 --- a/bindings_node/src/permissions.rs +++ b/bindings_node/src/permissions.rs @@ -161,6 +161,7 @@ pub struct PermissionPolicySet { pub update_group_description_policy: PermissionPolicy, pub update_group_image_url_square_policy: PermissionPolicy, pub update_group_pinned_frame_url_policy: PermissionPolicy, + pub update_message_expiration_ms_policy: PermissionPolicy, } impl From for GroupPermissionsOptions { @@ -215,6 +216,9 @@ impl GroupPermissions { update_group_pinned_frame_url_policy: get_policy( XmtpMetadataField::GroupPinnedFrameUrl.as_str(), ), + update_message_expiration_ms_policy: get_policy( + XmtpMetadataField::MessageExpirationMillis.as_str(), + ), }) } } @@ -241,6 +245,10 @@ impl TryFrom for PolicySet { XmtpMetadataField::GroupPinnedFrameUrl.to_string(), policy_set.update_group_pinned_frame_url_policy.try_into()?, ); + metadata_permissions_map.insert( + XmtpMetadataField::MessageExpirationMillis.to_string(), + policy_set.update_message_expiration_ms_policy.try_into()?, + ); Ok(PolicySet { add_member_policy: policy_set.add_member_policy.try_into()?, diff --git a/bindings_wasm/src/conversations.rs b/bindings_wasm/src/conversations.rs index 50d907666..d01db4537 100644 --- a/bindings_wasm/src/conversations.rs +++ b/bindings_wasm/src/conversations.rs @@ -129,6 +129,8 @@ pub struct CreateGroupOptions { pub group_pinned_frame_url: Option, #[wasm_bindgen(js_name = customPermissionPolicySet)] pub custom_permission_policy_set: Option, + #[wasm_bindgen(js_name = messageExpirationMillis)] + pub message_expiration_ms: Option, } #[wasm_bindgen] @@ -141,6 +143,7 @@ impl CreateGroupOptions { group_description: Option, group_pinned_frame_url: Option, custom_permission_policy_set: Option, + message_expiration_ms: Option, ) -> Self { Self { permissions, @@ -149,6 +152,7 @@ impl CreateGroupOptions { group_description, group_pinned_frame_url, custom_permission_policy_set, + message_expiration_ms, } } } @@ -160,6 +164,7 @@ impl CreateGroupOptions { image_url_square: self.group_image_url_square, description: self.group_description, pinned_frame_url: self.group_pinned_frame_url, + message_expiration_ms: self.message_expiration_ms, } } } diff --git a/bindings_wasm/src/permissions.rs b/bindings_wasm/src/permissions.rs index 78ce50221..4e22af8fe 100644 --- a/bindings_wasm/src/permissions.rs +++ b/bindings_wasm/src/permissions.rs @@ -170,6 +170,8 @@ pub struct PermissionPolicySet { pub update_group_image_url_square_policy: PermissionPolicy, #[wasm_bindgen(js_name = updateGroupPinnedFrameUrlPolicy)] pub update_group_pinned_frame_url_policy: PermissionPolicy, + #[wasm_bindgen(js_name = updateMessageExpirationPolicy)] + pub update_message_expiration_ms_policy: PermissionPolicy, } #[wasm_bindgen] @@ -185,6 +187,7 @@ impl PermissionPolicySet { update_group_description_policy: PermissionPolicy, update_group_image_url_square_policy: PermissionPolicy, update_group_pinned_frame_url_policy: PermissionPolicy, + update_message_expiration_ms_policy: PermissionPolicy, ) -> Self { Self { add_member_policy, @@ -195,6 +198,7 @@ impl PermissionPolicySet { update_group_description_policy, update_group_image_url_square_policy, update_group_pinned_frame_url_policy, + update_message_expiration_ms_policy, } } } @@ -253,6 +257,9 @@ impl GroupPermissions { update_group_pinned_frame_url_policy: get_policy( XmtpMetadataField::GroupPinnedFrameUrl.as_str(), ), + update_message_expiration_ms_policy: get_policy( + XmtpMetadataField::MessageExpirationMillis.as_str(), + ), }) } } @@ -277,6 +284,10 @@ impl TryFrom for PolicySet { XmtpMetadataField::GroupPinnedFrameUrl.to_string(), policy_set.update_group_pinned_frame_url_policy.try_into()?, ); + metadata_permissions_map.insert( + XmtpMetadataField::MessageExpirationMillis.to_string(), + policy_set.update_message_expiration_ms_policy.try_into()?, + ); Ok(PolicySet { add_member_policy: policy_set.add_member_policy.try_into()?, diff --git a/xmtp_mls/src/configuration.rs b/xmtp_mls/src/configuration.rs index 118231a57..8dc47bdcd 100644 --- a/xmtp_mls/src/configuration.rs +++ b/xmtp_mls/src/configuration.rs @@ -55,6 +55,7 @@ pub const DEFAULT_GROUP_NAME: &str = ""; pub const DEFAULT_GROUP_DESCRIPTION: &str = ""; pub const DEFAULT_GROUP_IMAGE_URL_SQUARE: &str = ""; pub const DEFAULT_GROUP_PINNED_FRAME_URL: &str = ""; +pub const DEFAULT_MESSAGE_EXPIRATION_MS: i64 = 0; // If a metadata field name starts with this character, // and it does not have a policy set, it is a super admin only field diff --git a/xmtp_mls/src/groups/group_mutable_metadata.rs b/xmtp_mls/src/groups/group_mutable_metadata.rs index 2db12769e..7bb88311b 100644 --- a/xmtp_mls/src/groups/group_mutable_metadata.rs +++ b/xmtp_mls/src/groups/group_mutable_metadata.rs @@ -13,7 +13,7 @@ use xmtp_proto::xmtp::mls::message_contents::{ use crate::configuration::{ DEFAULT_GROUP_DESCRIPTION, DEFAULT_GROUP_IMAGE_URL_SQUARE, DEFAULT_GROUP_NAME, - DEFAULT_GROUP_PINNED_FRAME_URL, MUTABLE_METADATA_EXTENSION_ID, + DEFAULT_GROUP_PINNED_FRAME_URL, DEFAULT_MESSAGE_EXPIRATION_MS, MUTABLE_METADATA_EXTENSION_ID, }; use super::GroupMetadataOptions; @@ -47,6 +47,7 @@ pub enum MetadataField { Description, GroupImageUrlSquare, GroupPinnedFrameUrl, + MessageExpirationMillis, } impl MetadataField { @@ -57,6 +58,7 @@ impl MetadataField { MetadataField::Description => "description", MetadataField::GroupImageUrlSquare => "group_image_url_square", MetadataField::GroupPinnedFrameUrl => "group_pinned_frame_url", + MetadataField::MessageExpirationMillis => "message_expiration_ms", } } } @@ -121,6 +123,12 @@ impl GroupMutableMetadata { opts.pinned_frame_url .unwrap_or_else(|| DEFAULT_GROUP_PINNED_FRAME_URL.to_string()), ); + attributes.insert( + MetadataField::MessageExpirationMillis.to_string(), + opts.message_expiration_ms + .unwrap_or_else(|| DEFAULT_MESSAGE_EXPIRATION_MS) + .to_string(), + ); let admin_list = vec![]; let super_admin_list = vec![creator_inbox_id.clone()]; Self { @@ -150,6 +158,10 @@ impl GroupMutableMetadata { MetadataField::GroupPinnedFrameUrl.to_string(), DEFAULT_GROUP_PINNED_FRAME_URL.to_string(), ); + attributes.insert( + MetadataField::MessageExpirationMillis.to_string(), + DEFAULT_MESSAGE_EXPIRATION_MS.to_string(), + ); let admin_list = vec![]; let super_admin_list = vec![]; Self { diff --git a/xmtp_mls/src/groups/group_permissions.rs b/xmtp_mls/src/groups/group_permissions.rs index 1a1b04594..771d04d09 100644 --- a/xmtp_mls/src/groups/group_permissions.rs +++ b/xmtp_mls/src/groups/group_permissions.rs @@ -25,12 +25,12 @@ use xmtp_proto::xmtp::mls::message_contents::{ PermissionsUpdatePolicy as PermissionsPolicyProto, PolicySet as PolicySetProto, }; -use crate::configuration::{GROUP_PERMISSIONS_EXTENSION_ID, SUPER_ADMIN_METADATA_PREFIX}; - use super::{ group_mutable_metadata::GroupMutableMetadata, validated_commit::{CommitParticipant, Inbox, MetadataFieldChange, ValidatedCommit}, }; +use crate::configuration::{GROUP_PERMISSIONS_EXTENSION_ID, SUPER_ADMIN_METADATA_PREFIX}; +use crate::groups::group_mutable_metadata::MetadataField; /// Errors that can occur when working with GroupMutablePermissions. #[derive(Debug, Error)] @@ -1197,6 +1197,11 @@ pub(crate) fn policy_all_members() -> PolicySet { for field in GroupMutableMetadata::supported_fields() { metadata_policies_map.insert(field.to_string(), MetadataPolicies::allow()); } + metadata_policies_map.insert( + MetadataField::MessageExpirationMillis.to_string(), + MetadataPolicies::allow_if_actor_admin(), + ); + PolicySet::new( MembershipPolicies::allow(), MembershipPolicies::allow_if_actor_admin(), @@ -1215,6 +1220,11 @@ pub(crate) fn policy_admin_only() -> PolicySet { for field in GroupMutableMetadata::supported_fields() { metadata_policies_map.insert(field.to_string(), MetadataPolicies::allow_if_actor_admin()); } + metadata_policies_map.insert( + MetadataField::MessageExpirationMillis.to_string(), + MetadataPolicies::allow_if_actor_admin(), + ); + PolicySet::new( MembershipPolicies::allow_if_actor_admin(), MembershipPolicies::allow_if_actor_admin(), diff --git a/xmtp_mls/src/groups/mod.rs b/xmtp_mls/src/groups/mod.rs index 70f9ae97c..2a235a26f 100644 --- a/xmtp_mls/src/groups/mod.rs +++ b/xmtp_mls/src/groups/mod.rs @@ -287,6 +287,7 @@ pub struct GroupMetadataOptions { pub image_url_square: Option, pub description: Option, pub pinned_frame_url: Option, + pub message_expiration_ms: Option, } impl Clone for MlsGroup { @@ -2547,6 +2548,7 @@ pub(crate) mod tests { image_url_square: Some("url".to_string()), description: Some("group description".to_string()), pinned_frame_url: Some("pinned frame".to_string()), + message_expiration_ms: None, }, ) .unwrap(); From 8dbd6842224c38367168c7d7a63529477f145502 Mon Sep 17 00:00:00 2001 From: Mojtaba Chenani Date: Tue, 7 Jan 2025 17:52:03 +0100 Subject: [PATCH 2/9] fix missing fields --- bindings_node/src/conversations.rs | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/bindings_node/src/conversations.rs b/bindings_node/src/conversations.rs index d6aded13a..6be13f747 100644 --- a/bindings_node/src/conversations.rs +++ b/bindings_node/src/conversations.rs @@ -154,17 +154,15 @@ impl Conversations { account_addresses: Vec, options: Option, ) -> Result { - let options = match options { - Some(options) => options, - None => CreateGroupOptions { - permissions: None, - group_name: None, - group_image_url_square: None, - group_description: None, - group_pinned_frame_url: None, - custom_permission_policy_set: None, - }, - }; + let options = options.unwrap_or_else(|| CreateGroupOptions { + permissions: None, + group_name: None, + group_image_url_square: None, + group_description: None, + group_pinned_frame_url: None, + custom_permission_policy_set: None, + message_expiration_ms: None, + }); if let Some(GroupPermissionsOptions::CustomPolicy) = options.permissions { if options.custom_permission_policy_set.is_none() { From c179ee45d13e3319a3a22c008c188f0c360fce31 Mon Sep 17 00:00:00 2001 From: Mojtaba Chenani Date: Tue, 7 Jan 2025 18:08:59 +0100 Subject: [PATCH 3/9] fix clippy --- bindings_node/src/conversations.rs | 2 +- bindings_wasm/src/conversations.rs | 20 +++++++++---------- xmtp_mls/src/groups/group_mutable_metadata.rs | 2 +- 3 files changed, 11 insertions(+), 13 deletions(-) diff --git a/bindings_node/src/conversations.rs b/bindings_node/src/conversations.rs index 6be13f747..ebfc3a9ec 100644 --- a/bindings_node/src/conversations.rs +++ b/bindings_node/src/conversations.rs @@ -154,7 +154,7 @@ impl Conversations { account_addresses: Vec, options: Option, ) -> Result { - let options = options.unwrap_or_else(|| CreateGroupOptions { + let options = options.unwrap_or(CreateGroupOptions { permissions: None, group_name: None, group_image_url_square: None, diff --git a/bindings_wasm/src/conversations.rs b/bindings_wasm/src/conversations.rs index d01db4537..5421e1a0e 100644 --- a/bindings_wasm/src/conversations.rs +++ b/bindings_wasm/src/conversations.rs @@ -188,17 +188,15 @@ impl Conversations { account_addresses: Vec, options: Option, ) -> Result { - let options = match options { - Some(options) => options, - None => CreateGroupOptions { - permissions: None, - group_name: None, - group_image_url_square: None, - group_description: None, - group_pinned_frame_url: None, - custom_permission_policy_set: None, - }, - }; + let options = options.unwrap_or(CreateGroupOptions { + permissions: None, + group_name: None, + group_image_url_square: None, + group_description: None, + group_pinned_frame_url: None, + custom_permission_policy_set: None, + message_expiration_ms: None, + }); if let Some(GroupPermissionsOptions::CustomPolicy) = options.permissions { if options.custom_permission_policy_set.is_none() { diff --git a/xmtp_mls/src/groups/group_mutable_metadata.rs b/xmtp_mls/src/groups/group_mutable_metadata.rs index 7bb88311b..b382e9b98 100644 --- a/xmtp_mls/src/groups/group_mutable_metadata.rs +++ b/xmtp_mls/src/groups/group_mutable_metadata.rs @@ -126,7 +126,7 @@ impl GroupMutableMetadata { attributes.insert( MetadataField::MessageExpirationMillis.to_string(), opts.message_expiration_ms - .unwrap_or_else(|| DEFAULT_MESSAGE_EXPIRATION_MS) + .unwrap_or(DEFAULT_MESSAGE_EXPIRATION_MS) .to_string(), ); let admin_list = vec![]; From ba0a4b39553b44fb08d655ec799f45c9f2bbd552 Mon Sep 17 00:00:00 2001 From: Mojtaba Chenani Date: Thu, 9 Jan 2025 12:29:48 +0100 Subject: [PATCH 4/9] fix tests and add message expiration starts timestamp --- bindings_ffi/src/mls.rs | 28 +++++++++++++---- bindings_node/src/conversations.rs | 7 +++-- bindings_node/src/permissions.rs | 4 +-- bindings_wasm/src/conversations.rs | 11 +++++-- bindings_wasm/src/permissions.rs | 4 +-- examples/cli/cli-client.rs | 2 +- xmtp_mls/src/configuration.rs | 1 - xmtp_mls/src/groups/group_mutable_metadata.rs | 28 ++++++++++------- xmtp_mls/src/groups/group_permissions.rs | 30 +++++++++++-------- xmtp_mls/src/groups/mod.rs | 8 +++-- 10 files changed, 81 insertions(+), 42 deletions(-) diff --git a/bindings_ffi/src/mls.rs b/bindings_ffi/src/mls.rs index ab9be674e..854d157aa 100644 --- a/bindings_ffi/src/mls.rs +++ b/bindings_ffi/src/mls.rs @@ -633,7 +633,7 @@ pub struct FfiConversations { #[derive(uniffi::Enum, Clone, Debug)] pub enum FfiGroupPermissionsOptions { - AllMembers, + Default, AdminOnly, CustomPolicy, } @@ -789,7 +789,7 @@ pub struct FfiPermissionPolicySet { impl From for FfiGroupPermissionsOptions { fn from(policy: PreconfiguredPolicies) -> Self { match policy { - PreconfiguredPolicies::AllMembers => FfiGroupPermissionsOptions::AllMembers, + PreconfiguredPolicies::Default => FfiGroupPermissionsOptions::Default, PreconfiguredPolicies::AdminsOnly => FfiGroupPermissionsOptions::AdminOnly, } } @@ -815,6 +815,14 @@ impl TryFrom for PolicySet { MetadataField::GroupPinnedFrameUrl.to_string(), policy_set.update_group_pinned_frame_url_policy.try_into()?, ); + // MessageExpirationFromMillis follows the same policy as MessageExpirationMillis + metadata_permissions_map.insert( + MetadataField::MessageExpirationFromMillis.to_string(), + policy_set + .update_message_expiration_ms_policy + .clone() + .try_into()?, + ); metadata_permissions_map.insert( MetadataField::MessageExpirationMillis.to_string(), policy_set.update_message_expiration_ms_policy.try_into()?, @@ -877,8 +885,8 @@ impl FfiConversations { let metadata_options = opts.clone().into_group_metadata_options(); let group_permissions = match opts.permissions { - Some(FfiGroupPermissionsOptions::AllMembers) => { - Some(xmtp_mls::groups::PreconfiguredPolicies::AllMembers.to_policy_set()) + Some(FfiGroupPermissionsOptions::Default) => { + Some(xmtp_mls::groups::PreconfiguredPolicies::Default.to_policy_set()) } Some(FfiGroupPermissionsOptions::AdminOnly) => { Some(xmtp_mls::groups::PreconfiguredPolicies::AdminsOnly.to_policy_set()) @@ -1370,6 +1378,7 @@ pub struct FfiCreateGroupOptions { pub group_description: Option, pub group_pinned_frame_url: Option, pub custom_permission_policy_set: Option, + pub message_expiration_from_ms: Option, pub message_expiration_ms: Option, } @@ -1380,6 +1389,7 @@ impl FfiCreateGroupOptions { image_url_square: self.group_image_url_square, description: self.group_description, pinned_frame_url: self.group_pinned_frame_url, + message_expiration_from_ms: self.message_expiration_from_ms, message_expiration_ms: self.message_expiration_ms, } } @@ -2689,6 +2699,7 @@ mod tests { group_description: Some("group description".to_string()), group_pinned_frame_url: Some("pinned frame".to_string()), custom_permission_policy_set: None, + message_expiration_from_ms: None, message_expiration_ms: None, }, ) @@ -3935,7 +3946,7 @@ mod tests { // Create all_members group let all_members_options = FfiCreateGroupOptions { - permissions: Some(FfiGroupPermissionsOptions::AllMembers), + permissions: Some(FfiGroupPermissionsOptions::Default), ..Default::default() }; let alix_group_all_members = alix @@ -4085,6 +4096,7 @@ mod tests { group_description: Some("A test group".to_string()), group_pinned_frame_url: Some("https://example.com/frame.png".to_string()), custom_permission_policy_set: Some(custom_permissions), + message_expiration_from_ms: None, message_expiration_ms: None, }; @@ -4213,6 +4225,7 @@ mod tests { group_description: Some("A test group".to_string()), group_pinned_frame_url: Some("https://example.com/frame.png".to_string()), custom_permission_policy_set: Some(custom_permissions_invalid_1), + message_expiration_from_ms: None, message_expiration_ms: None, }; @@ -4227,12 +4240,13 @@ mod tests { assert!(results_1.is_err()); let create_group_options_invalid_2 = FfiCreateGroupOptions { - permissions: Some(FfiGroupPermissionsOptions::AllMembers), + permissions: Some(FfiGroupPermissionsOptions::Default), group_name: Some("Test Group".to_string()), group_image_url_square: Some("https://example.com/image.png".to_string()), group_description: Some("A test group".to_string()), group_pinned_frame_url: Some("https://example.com/frame.png".to_string()), custom_permission_policy_set: Some(custom_permissions_valid.clone()), + message_expiration_from_ms: None, message_expiration_ms: None, }; @@ -4253,6 +4267,7 @@ mod tests { group_description: Some("A test group".to_string()), group_pinned_frame_url: Some("https://example.com/frame.png".to_string()), custom_permission_policy_set: Some(custom_permissions_valid.clone()), + message_expiration_from_ms: None, message_expiration_ms: None, }; @@ -4273,6 +4288,7 @@ mod tests { group_description: Some("A test group".to_string()), group_pinned_frame_url: Some("https://example.com/frame.png".to_string()), custom_permission_policy_set: Some(custom_permissions_valid), + message_expiration_from_ms: None, message_expiration_ms: None, }; diff --git a/bindings_node/src/conversations.rs b/bindings_node/src/conversations.rs index ebfc3a9ec..b9be3580c 100644 --- a/bindings_node/src/conversations.rs +++ b/bindings_node/src/conversations.rs @@ -122,6 +122,7 @@ pub struct CreateGroupOptions { pub group_description: Option, pub group_pinned_frame_url: Option, pub custom_permission_policy_set: Option, + pub message_expiration_from_ms: Option, pub message_expiration_ms: Option, } @@ -132,6 +133,7 @@ impl CreateGroupOptions { image_url_square: self.group_image_url_square, description: self.group_description, pinned_frame_url: self.group_pinned_frame_url, + message_expiration_from_ms: self.message_expiration_from_ms, message_expiration_ms: self.message_expiration_ms, } } @@ -161,6 +163,7 @@ impl Conversations { group_description: None, group_pinned_frame_url: None, custom_permission_policy_set: None, + message_expiration_from_ms: None, message_expiration_ms: None, }); @@ -177,8 +180,8 @@ impl Conversations { let metadata_options = options.clone().into_group_metadata_options(); let group_permissions = match options.permissions { - Some(GroupPermissionsOptions::AllMembers) => { - Some(PreconfiguredPolicies::AllMembers.to_policy_set()) + Some(GroupPermissionsOptions::Default) => { + Some(PreconfiguredPolicies::Default.to_policy_set()) } Some(GroupPermissionsOptions::AdminOnly) => { Some(PreconfiguredPolicies::AdminsOnly.to_policy_set()) diff --git a/bindings_node/src/permissions.rs b/bindings_node/src/permissions.rs index 024edd7f7..1eb667f64 100644 --- a/bindings_node/src/permissions.rs +++ b/bindings_node/src/permissions.rs @@ -14,7 +14,7 @@ use xmtp_mls::groups::{ #[napi] pub enum GroupPermissionsOptions { - AllMembers, + Default, AdminOnly, CustomPolicy, } @@ -167,7 +167,7 @@ pub struct PermissionPolicySet { impl From for GroupPermissionsOptions { fn from(policy: PreconfiguredPolicies) -> Self { match policy { - PreconfiguredPolicies::AllMembers => GroupPermissionsOptions::AllMembers, + PreconfiguredPolicies::Default => GroupPermissionsOptions::Default, PreconfiguredPolicies::AdminsOnly => GroupPermissionsOptions::AdminOnly, } } diff --git a/bindings_wasm/src/conversations.rs b/bindings_wasm/src/conversations.rs index 5421e1a0e..95863e2eb 100644 --- a/bindings_wasm/src/conversations.rs +++ b/bindings_wasm/src/conversations.rs @@ -129,6 +129,8 @@ pub struct CreateGroupOptions { pub group_pinned_frame_url: Option, #[wasm_bindgen(js_name = customPermissionPolicySet)] pub custom_permission_policy_set: Option, + #[wasm_bindgen(js_name = messageExpirationFromMillis)] + pub message_expiration_from_ms: Option, #[wasm_bindgen(js_name = messageExpirationMillis)] pub message_expiration_ms: Option, } @@ -136,6 +138,7 @@ pub struct CreateGroupOptions { #[wasm_bindgen] impl CreateGroupOptions { #[wasm_bindgen(constructor)] + #[allow(clippy::too_many_arguments)] pub fn new( permissions: Option, group_name: Option, @@ -143,6 +146,7 @@ impl CreateGroupOptions { group_description: Option, group_pinned_frame_url: Option, custom_permission_policy_set: Option, + message_expiration_from_ms: Option, message_expiration_ms: Option, ) -> Self { Self { @@ -152,6 +156,7 @@ impl CreateGroupOptions { group_description, group_pinned_frame_url, custom_permission_policy_set, + message_expiration_from_ms, message_expiration_ms, } } @@ -164,6 +169,7 @@ impl CreateGroupOptions { image_url_square: self.group_image_url_square, description: self.group_description, pinned_frame_url: self.group_pinned_frame_url, + message_expiration_from_ms: self.message_expiration_from_ms, message_expiration_ms: self.message_expiration_ms, } } @@ -195,6 +201,7 @@ impl Conversations { group_description: None, group_pinned_frame_url: None, custom_permission_policy_set: None, + message_expiration_from_ms: None, message_expiration_ms: None, }); @@ -209,8 +216,8 @@ impl Conversations { let metadata_options = options.clone().into_group_metadata_options(); let group_permissions = match options.permissions { - Some(GroupPermissionsOptions::AllMembers) => { - Some(PreconfiguredPolicies::AllMembers.to_policy_set()) + Some(GroupPermissionsOptions::Default) => { + Some(PreconfiguredPolicies::Default.to_policy_set()) } Some(GroupPermissionsOptions::AdminOnly) => { Some(PreconfiguredPolicies::AdminsOnly.to_policy_set()) diff --git a/bindings_wasm/src/permissions.rs b/bindings_wasm/src/permissions.rs index 4e22af8fe..2baa7ca74 100644 --- a/bindings_wasm/src/permissions.rs +++ b/bindings_wasm/src/permissions.rs @@ -14,7 +14,7 @@ use xmtp_mls::groups::{ #[wasm_bindgen] #[derive(Clone)] pub enum GroupPermissionsOptions { - AllMembers, + Default, AdminOnly, CustomPolicy, } @@ -206,7 +206,7 @@ impl PermissionPolicySet { impl From for GroupPermissionsOptions { fn from(policy: PreconfiguredPolicies) -> Self { match policy { - PreconfiguredPolicies::AllMembers => GroupPermissionsOptions::AllMembers, + PreconfiguredPolicies::Default => GroupPermissionsOptions::Default, PreconfiguredPolicies::AdminsOnly => GroupPermissionsOptions::AdminOnly, } } diff --git a/examples/cli/cli-client.rs b/examples/cli/cli-client.rs index ee836e854..127883443 100755 --- a/examples/cli/cli-client.rs +++ b/examples/cli/cli-client.rs @@ -406,7 +406,7 @@ async fn main() -> color_eyre::eyre::Result<()> { } Commands::CreateGroup { permissions } => { let group_permissions = match permissions { - Permissions::EveryoneIsAdmin => xmtp_mls::groups::PreconfiguredPolicies::AllMembers, + Permissions::EveryoneIsAdmin => xmtp_mls::groups::PreconfiguredPolicies::Default, Permissions::GroupCreatorIsAdmin => { xmtp_mls::groups::PreconfiguredPolicies::AdminsOnly } diff --git a/xmtp_mls/src/configuration.rs b/xmtp_mls/src/configuration.rs index 8dc47bdcd..118231a57 100644 --- a/xmtp_mls/src/configuration.rs +++ b/xmtp_mls/src/configuration.rs @@ -55,7 +55,6 @@ pub const DEFAULT_GROUP_NAME: &str = ""; pub const DEFAULT_GROUP_DESCRIPTION: &str = ""; pub const DEFAULT_GROUP_IMAGE_URL_SQUARE: &str = ""; pub const DEFAULT_GROUP_PINNED_FRAME_URL: &str = ""; -pub const DEFAULT_MESSAGE_EXPIRATION_MS: i64 = 0; // If a metadata field name starts with this character, // and it does not have a policy set, it is a super admin only field diff --git a/xmtp_mls/src/groups/group_mutable_metadata.rs b/xmtp_mls/src/groups/group_mutable_metadata.rs index b382e9b98..87d6eac5e 100644 --- a/xmtp_mls/src/groups/group_mutable_metadata.rs +++ b/xmtp_mls/src/groups/group_mutable_metadata.rs @@ -13,7 +13,7 @@ use xmtp_proto::xmtp::mls::message_contents::{ use crate::configuration::{ DEFAULT_GROUP_DESCRIPTION, DEFAULT_GROUP_IMAGE_URL_SQUARE, DEFAULT_GROUP_NAME, - DEFAULT_GROUP_PINNED_FRAME_URL, DEFAULT_MESSAGE_EXPIRATION_MS, MUTABLE_METADATA_EXTENSION_ID, + DEFAULT_GROUP_PINNED_FRAME_URL, MUTABLE_METADATA_EXTENSION_ID, }; use super::GroupMetadataOptions; @@ -47,6 +47,7 @@ pub enum MetadataField { Description, GroupImageUrlSquare, GroupPinnedFrameUrl, + MessageExpirationFromMillis, MessageExpirationMillis, } @@ -58,6 +59,7 @@ impl MetadataField { MetadataField::Description => "description", MetadataField::GroupImageUrlSquare => "group_image_url_square", MetadataField::GroupPinnedFrameUrl => "group_pinned_frame_url", + MetadataField::MessageExpirationFromMillis => "message_expiration_from_ms", MetadataField::MessageExpirationMillis => "message_expiration_ms", } } @@ -123,12 +125,20 @@ impl GroupMutableMetadata { opts.pinned_frame_url .unwrap_or_else(|| DEFAULT_GROUP_PINNED_FRAME_URL.to_string()), ); - attributes.insert( - MetadataField::MessageExpirationMillis.to_string(), - opts.message_expiration_ms - .unwrap_or(DEFAULT_MESSAGE_EXPIRATION_MS) - .to_string(), - ); + + if let Some(message_expiration_from_ms) = opts.message_expiration_from_ms { + attributes.insert( + MetadataField::MessageExpirationFromMillis.to_string(), + message_expiration_from_ms.to_string(), + ); + } + if let Some(message_expiration_ms) = opts.message_expiration_ms { + attributes.insert( + MetadataField::MessageExpirationMillis.to_string(), + message_expiration_ms.to_string(), + ); + } + let admin_list = vec![]; let super_admin_list = vec![creator_inbox_id.clone()]; Self { @@ -158,10 +168,6 @@ impl GroupMutableMetadata { MetadataField::GroupPinnedFrameUrl.to_string(), DEFAULT_GROUP_PINNED_FRAME_URL.to_string(), ); - attributes.insert( - MetadataField::MessageExpirationMillis.to_string(), - DEFAULT_MESSAGE_EXPIRATION_MS.to_string(), - ); let admin_list = vec![]; let super_admin_list = vec![]; Self { diff --git a/xmtp_mls/src/groups/group_permissions.rs b/xmtp_mls/src/groups/group_permissions.rs index 771d04d09..3eae85437 100644 --- a/xmtp_mls/src/groups/group_permissions.rs +++ b/xmtp_mls/src/groups/group_permissions.rs @@ -31,6 +31,7 @@ use super::{ }; use crate::configuration::{GROUP_PERMISSIONS_EXTENSION_ID, SUPER_ADMIN_METADATA_PREFIX}; use crate::groups::group_mutable_metadata::MetadataField; +use crate::groups::group_mutable_metadata::MetadataField::MessageExpirationMillis; /// Errors that can occur when working with GroupMutablePermissions. #[derive(Debug, Error)] @@ -1145,7 +1146,7 @@ impl PolicySet { /// since the group was created, the number of metadata policies might not match /// the default All Members Policy Set. As long as all metadata policies are allow, we will /// match against All Members Preconfigured Policy -pub fn is_policy_all_members(policy: &PolicySet) -> Result { +pub fn is_policy_default(policy: &PolicySet) -> Result { let mut metadata_policies_equal = true; for field_name in policy.update_metadata_policy.keys() { let metadata_policy = policy.update_metadata_policy.get(field_name).ok_or( @@ -1153,8 +1154,13 @@ pub fn is_policy_all_members(policy: &PolicySet) -> Result { name: field_name.to_string(), }, )?; - metadata_policies_equal = - metadata_policies_equal && metadata_policy.eq(&MetadataPolicies::allow()); + if field_name == MessageExpirationMillis.as_str() { + metadata_policies_equal = metadata_policies_equal + && metadata_policy.eq(&MetadataPolicies::allow_if_actor_admin()); + } else { + metadata_policies_equal = + metadata_policies_equal && metadata_policy.eq(&MetadataPolicies::allow()); + } } Ok(metadata_policies_equal && policy.add_member_policy == MembershipPolicies::allow() @@ -1192,13 +1198,13 @@ pub fn is_policy_admin_only(policy: &PolicySet) -> Result { /// Returns the "All Members" preconfigured policy. /// /// A policy where any member can add or remove any other member -pub(crate) fn policy_all_members() -> PolicySet { +pub(crate) fn default_policy() -> PolicySet { let mut metadata_policies_map: HashMap = HashMap::new(); for field in GroupMutableMetadata::supported_fields() { metadata_policies_map.insert(field.to_string(), MetadataPolicies::allow()); } metadata_policies_map.insert( - MetadataField::MessageExpirationMillis.to_string(), + MessageExpirationMillis.to_string(), MetadataPolicies::allow_if_actor_admin(), ); @@ -1247,7 +1253,7 @@ impl Default for PolicySet { pub enum PreconfiguredPolicies { /// The "All Members" preconfigured policy. #[default] - AllMembers, + Default, /// The "Admin Only" preconfigured policy. AdminsOnly, } @@ -1256,15 +1262,15 @@ impl PreconfiguredPolicies { /// Converts the PreconfiguredPolicies to a PolicySet. pub fn to_policy_set(&self) -> PolicySet { match self { - PreconfiguredPolicies::AllMembers => policy_all_members(), + PreconfiguredPolicies::Default => default_policy(), PreconfiguredPolicies::AdminsOnly => policy_admin_only(), } } /// Creates a PreconfiguredPolicies from a PolicySet. pub fn from_policy_set(policy_set: &PolicySet) -> Result { - if is_policy_all_members(policy_set)? { - Ok(PreconfiguredPolicies::AllMembers) + if is_policy_default(policy_set)? { + Ok(PreconfiguredPolicies::Default) } else if is_policy_admin_only(policy_set)? { Ok(PreconfiguredPolicies::AdminsOnly) } else { @@ -1624,11 +1630,11 @@ pub(crate) mod tests { #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test::wasm_bindgen_test)] #[cfg_attr(not(target_arch = "wasm32"), test)] fn test_preconfigured_policy() { - let group_permissions = GroupMutablePermissions::new(policy_all_members()); + let group_permissions = GroupMutablePermissions::new(default_policy()); assert_eq!( group_permissions.preconfigured_policy().unwrap(), - PreconfiguredPolicies::AllMembers + PreconfiguredPolicies::Default ); let group_group_permissions_creator_admin = @@ -1657,7 +1663,7 @@ pub(crate) mod tests { update_permissions_policy: PermissionsPolicies::allow_if_actor_super_admin(), }; - assert!(is_policy_all_members(&policy_set_new_metadata_permission).unwrap()); + assert!(is_policy_default(&policy_set_new_metadata_permission).unwrap()); let mut metadata_policies_map = MetadataPolicies::default_map(MetadataPolicies::allow_if_actor_admin()); diff --git a/xmtp_mls/src/groups/mod.rs b/xmtp_mls/src/groups/mod.rs index 2a235a26f..966fecb00 100644 --- a/xmtp_mls/src/groups/mod.rs +++ b/xmtp_mls/src/groups/mod.rs @@ -287,6 +287,7 @@ pub struct GroupMetadataOptions { pub image_url_square: Option, pub description: Option, pub pinned_frame_url: Option, + pub message_expiration_from_ms: Option, pub message_expiration_ms: Option, } @@ -2548,6 +2549,7 @@ pub(crate) mod tests { image_url_square: Some("url".to_string()), description: Some("group description".to_string()), pinned_frame_url: Some("pinned frame".to_string()), + message_expiration_from_ms: None, message_expiration_ms: None, }, ) @@ -2779,7 +2781,7 @@ pub(crate) mod tests { let bola = ClientBuilder::new_test_client(&bola_wallet).await; // Create a group and verify it has the default group name - let policy_set = Some(PreconfiguredPolicies::AllMembers.to_policy_set()); + let policy_set = Some(PreconfiguredPolicies::Default.to_policy_set()); let amal_group = amal .create_group(policy_set, GroupMetadataOptions::default()) .unwrap(); @@ -3233,7 +3235,7 @@ pub(crate) mod tests { #[wasm_bindgen_test(unsupported = tokio::test(flavor = "current_thread"))] async fn test_can_read_group_creator_inbox_id() { let amal = ClientBuilder::new_test_client(&generate_local_wallet()).await; - let policy_set = Some(PreconfiguredPolicies::AllMembers.to_policy_set()); + let policy_set = Some(PreconfiguredPolicies::Default.to_policy_set()); let amal_group = amal .create_group(policy_set, GroupMetadataOptions::default()) .unwrap(); @@ -3261,7 +3263,7 @@ pub(crate) mod tests { async fn test_can_update_gce_after_failed_commit() { // Step 1: Amal creates a group let amal = ClientBuilder::new_test_client(&generate_local_wallet()).await; - let policy_set = Some(PreconfiguredPolicies::AllMembers.to_policy_set()); + let policy_set = Some(PreconfiguredPolicies::Default.to_policy_set()); let amal_group = amal .create_group(policy_set, GroupMetadataOptions::default()) .unwrap(); From cdef053f43f45cbc1c8006c1a330fe4013b82bee Mon Sep 17 00:00:00 2001 From: Mojtaba Chenani Date: Thu, 9 Jan 2025 12:46:15 +0100 Subject: [PATCH 5/9] fix node tests --- bindings_node/test/Conversations.test.ts | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/bindings_node/test/Conversations.test.ts b/bindings_node/test/Conversations.test.ts index f9d7beefd..74425bdf7 100644 --- a/bindings_node/test/Conversations.test.ts +++ b/bindings_node/test/Conversations.test.ts @@ -41,7 +41,7 @@ describe('Conversations', () => { expect(group.isActive()).toBe(true) expect(group.groupName()).toBe('') expect(group.groupPermissions().policyType()).toBe( - GroupPermissionsOptions.AllMembers + GroupPermissionsOptions.Default ) expect(group.groupPermissions().policySet()).toEqual({ addMemberPolicy: 0, @@ -52,6 +52,7 @@ describe('Conversations', () => { updateGroupDescriptionPolicy: 0, updateGroupImageUrlSquarePolicy: 0, updateGroupPinnedFrameUrlPolicy: 0, + updateMessageExpirationMsPolicy: 2, }) expect(group.addedByInboxId()).toBe(client1.inboxId()) expect((await group.findMessages()).length).toBe(1) @@ -103,6 +104,7 @@ describe('Conversations', () => { updateGroupDescriptionPolicy: 1, updateGroupImageUrlSquarePolicy: 0, updateGroupPinnedFrameUrlPolicy: 3, + updateMessageExpirationMsPolicy: 2 }, }) expect(group).toBeDefined() @@ -118,6 +120,7 @@ describe('Conversations', () => { updateGroupDescriptionPolicy: 1, updateGroupImageUrlSquarePolicy: 0, updateGroupPinnedFrameUrlPolicy: 3, + updateMessageExpirationMsPolicy: 2 }) }) @@ -139,6 +142,7 @@ describe('Conversations', () => { updateGroupDescriptionPolicy: 0, updateGroupImageUrlSquarePolicy: 0, updateGroupPinnedFrameUrlPolicy: 0, + updateMessageExpirationMsPolicy: 2, }) await group.updatePermissionPolicy( @@ -155,6 +159,7 @@ describe('Conversations', () => { updateGroupDescriptionPolicy: 0, updateGroupImageUrlSquarePolicy: 0, updateGroupPinnedFrameUrlPolicy: 0, + updateMessageExpirationMsPolicy: 2, }) await group.updatePermissionPolicy( @@ -172,6 +177,7 @@ describe('Conversations', () => { updateGroupDescriptionPolicy: 0, updateGroupImageUrlSquarePolicy: 0, updateGroupPinnedFrameUrlPolicy: 0, + updateMessageExpirationMsPolicy: 2, }) }) @@ -198,6 +204,7 @@ describe('Conversations', () => { updateGroupImageUrlSquarePolicy: 0, updateGroupNamePolicy: 0, updateGroupPinnedFrameUrlPolicy: 0, + updateMessageExpirationMsPolicy: 2, }) expect(group.addedByInboxId()).toBe(client1.inboxId()) expect((await group.findMessages()).length).toBe(0) @@ -335,6 +342,7 @@ describe('Conversations', () => { updateGroupDescriptionPolicy: 2, updateGroupImageUrlSquarePolicy: 2, updateGroupPinnedFrameUrlPolicy: 2, + updateMessageExpirationMsPolicy: 2, }) const groupWithDescription = await client1 From 75553e08f98b4beeb080574b4e00e022856ce958 Mon Sep 17 00:00:00 2001 From: Mojtaba Chenani Date: Thu, 9 Jan 2025 12:52:41 +0100 Subject: [PATCH 6/9] fix fmt on nodes --- bindings_node/test/Conversations.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bindings_node/test/Conversations.test.ts b/bindings_node/test/Conversations.test.ts index 74425bdf7..43d12f617 100644 --- a/bindings_node/test/Conversations.test.ts +++ b/bindings_node/test/Conversations.test.ts @@ -104,7 +104,7 @@ describe('Conversations', () => { updateGroupDescriptionPolicy: 1, updateGroupImageUrlSquarePolicy: 0, updateGroupPinnedFrameUrlPolicy: 3, - updateMessageExpirationMsPolicy: 2 + updateMessageExpirationMsPolicy: 2, }, }) expect(group).toBeDefined() @@ -120,7 +120,7 @@ describe('Conversations', () => { updateGroupDescriptionPolicy: 1, updateGroupImageUrlSquarePolicy: 0, updateGroupPinnedFrameUrlPolicy: 3, - updateMessageExpirationMsPolicy: 2 + updateMessageExpirationMsPolicy: 2, }) }) From 57345c387b69aa05013a7a0261e7d19b190fb763 Mon Sep 17 00:00:00 2001 From: Mojtaba Chenani Date: Thu, 9 Jan 2025 13:49:19 +0100 Subject: [PATCH 7/9] add dm permission tests --- bindings_ffi/src/mls.rs | 63 ++++++++++++++++++- bindings_node/test/Conversations.test.ts | 2 +- xmtp_mls/src/groups/group_mutable_metadata.rs | 1 + 3 files changed, 64 insertions(+), 2 deletions(-) diff --git a/bindings_ffi/src/mls.rs b/bindings_ffi/src/mls.rs index 854d157aa..844459298 100644 --- a/bindings_ffi/src/mls.rs +++ b/bindings_ffi/src/mls.rs @@ -3911,7 +3911,7 @@ mod tests { } #[tokio::test(flavor = "multi_thread", worker_threads = 5)] - async fn test_permissions_show_expected_values() { + async fn test_group_permissions_show_expected_values() { let alix = new_test_client().await; let bo = new_test_client().await; // Create admin_only group @@ -3975,6 +3975,67 @@ mod tests { assert_eq!(alix_permission_policy_set, expected_permission_policy_set); } + #[tokio::test(flavor = "multi_thread", worker_threads = 5)] + async fn test_dm_permissions_show_expected_values() { + let alix = new_test_client().await; + let bo = new_test_client().await; + + let alix_group_admin_only = alix + .conversations() + .create_dm(bo.account_address.clone()) + .await + .unwrap(); + + // Verify we can read the expected permissions + let alix_permission_policy_set = alix_group_admin_only + .group_permissions() + .unwrap() + .policy_set() + .unwrap(); + let expected_permission_policy_set = FfiPermissionPolicySet { + add_member_policy: FfiPermissionPolicy::Deny, + remove_member_policy: FfiPermissionPolicy::Deny, + add_admin_policy: FfiPermissionPolicy::Deny, + remove_admin_policy: FfiPermissionPolicy::Deny, + update_group_name_policy: FfiPermissionPolicy::Allow, + update_group_description_policy: FfiPermissionPolicy::Allow, + update_group_image_url_square_policy: FfiPermissionPolicy::Allow, + update_group_pinned_frame_url_policy: FfiPermissionPolicy::Allow, + update_message_expiration_ms_policy: FfiPermissionPolicy::Allow, + }; + assert_eq!(alix_permission_policy_set, expected_permission_policy_set); + + // Create all_members group + let all_members_options = FfiCreateGroupOptions { + permissions: Some(FfiGroupPermissionsOptions::Default), + ..Default::default() + }; + let alix_group_all_members = alix + .conversations() + .create_group(vec![bo.account_address.clone()], all_members_options) + .await + .unwrap(); + + // Verify we can read the expected permissions + let alix_permission_policy_set = alix_group_all_members + .group_permissions() + .unwrap() + .policy_set() + .unwrap(); + let expected_permission_policy_set = FfiPermissionPolicySet { + add_member_policy: FfiPermissionPolicy::Allow, + remove_member_policy: FfiPermissionPolicy::Admin, + add_admin_policy: FfiPermissionPolicy::SuperAdmin, + remove_admin_policy: FfiPermissionPolicy::SuperAdmin, + update_group_name_policy: FfiPermissionPolicy::Allow, + update_group_description_policy: FfiPermissionPolicy::Allow, + update_group_image_url_square_policy: FfiPermissionPolicy::Allow, + update_group_pinned_frame_url_policy: FfiPermissionPolicy::Allow, + update_message_expiration_ms_policy: FfiPermissionPolicy::Admin, + }; + assert_eq!(alix_permission_policy_set, expected_permission_policy_set); + } + #[tokio::test(flavor = "multi_thread", worker_threads = 5)] async fn test_permissions_updates() { let alix = new_test_client().await; diff --git a/bindings_node/test/Conversations.test.ts b/bindings_node/test/Conversations.test.ts index 43d12f617..eccf8b0c1 100644 --- a/bindings_node/test/Conversations.test.ts +++ b/bindings_node/test/Conversations.test.ts @@ -204,7 +204,7 @@ describe('Conversations', () => { updateGroupImageUrlSquarePolicy: 0, updateGroupNamePolicy: 0, updateGroupPinnedFrameUrlPolicy: 0, - updateMessageExpirationMsPolicy: 2, + updateMessageExpirationMsPolicy: 0, }) expect(group.addedByInboxId()).toBe(client1.inboxId()) expect((await group.findMessages()).length).toBe(0) diff --git a/xmtp_mls/src/groups/group_mutable_metadata.rs b/xmtp_mls/src/groups/group_mutable_metadata.rs index 87d6eac5e..269fdb94b 100644 --- a/xmtp_mls/src/groups/group_mutable_metadata.rs +++ b/xmtp_mls/src/groups/group_mutable_metadata.rs @@ -186,6 +186,7 @@ impl GroupMutableMetadata { MetadataField::Description, MetadataField::GroupImageUrlSquare, MetadataField::GroupPinnedFrameUrl, + MetadataField::MessageExpirationMillis ] } From 747e27b91487a67742007706e93eb62c13802d71 Mon Sep 17 00:00:00 2001 From: Mojtaba Chenani Date: Thu, 9 Jan 2025 14:05:27 +0100 Subject: [PATCH 8/9] fix fmt --- xmtp_mls/src/groups/group_mutable_metadata.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xmtp_mls/src/groups/group_mutable_metadata.rs b/xmtp_mls/src/groups/group_mutable_metadata.rs index 269fdb94b..e1285a2aa 100644 --- a/xmtp_mls/src/groups/group_mutable_metadata.rs +++ b/xmtp_mls/src/groups/group_mutable_metadata.rs @@ -186,7 +186,7 @@ impl GroupMutableMetadata { MetadataField::Description, MetadataField::GroupImageUrlSquare, MetadataField::GroupPinnedFrameUrl, - MetadataField::MessageExpirationMillis + MetadataField::MessageExpirationMillis, ] } From c2d1d1c89a861f4c531ef41e2440315b2b5c776f Mon Sep 17 00:00:00 2001 From: Mojtaba Chenani Date: Thu, 9 Jan 2025 14:26:23 +0100 Subject: [PATCH 9/9] fix tests --- xmtp_mls/src/groups/group_permissions.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/xmtp_mls/src/groups/group_permissions.rs b/xmtp_mls/src/groups/group_permissions.rs index 3eae85437..f15f4c6b6 100644 --- a/xmtp_mls/src/groups/group_permissions.rs +++ b/xmtp_mls/src/groups/group_permissions.rs @@ -228,7 +228,11 @@ impl MetadataPolicies { pub fn default_map(policies: MetadataPolicies) -> HashMap { let mut map: HashMap = HashMap::new(); for field in GroupMutableMetadata::supported_fields() { - map.insert(field.to_string(), policies.clone()); + if field == MessageExpirationMillis { + map.insert(field.to_string(), MetadataPolicies::allow_if_actor_admin()); + } else { + map.insert(field.to_string(), policies.clone()); + } } map }