Adds update permissions intent and validation (#873)

* adds update permissions intent and validation

* replace assert false in tests

* format bindings

* set gen protosh script back

* fix names of ffi enums

* Added bindings function for reading all configurable permissions

* no need to read non changing policies

* Make update_permission_policy generic and add it to bindings

* fmt fix

* clippy fix

* make as_str helper const

---------

Co-authored-by: cameronvoell <[email protected]>

xmtp_mls/src/groups/group_mutable_metadata.rs

@@ -45,7 +45,7 @@ pub enum MetadataField {
 }
 
 impl MetadataField {
-    fn as_str(self) -> &'static str {
+    pub const fn as_str(&self) -> &'static str {
         match self {
             MetadataField::GroupName => "group_name",
             MetadataField::Description => "description",

xmtp_mls/src/groups/group_permissions.rs

@@ -46,6 +46,8 @@ pub enum GroupMutablePermissionsError {
     MissingPolicies,
     #[error("missing extension")]
     MissingExtension,
+    #[error("invalid permission policy option")]
+    InvalidPermissionPolicyOption,
 }
 
 #[derive(Debug, Clone, PartialEq)]
@@ -128,6 +130,15 @@ impl TryFrom<&Extensions> for GroupMutablePermissions {
     }
 }
 
+impl TryFrom<&OpenMlsGroup> for GroupMutablePermissions {
+    type Error = GroupMutablePermissionsError;
+
+    fn try_from(value: &OpenMlsGroup) -> Result<Self, Self::Error> {
+        let extensions = value.export_group_context().extensions();
+        extensions.try_into()
+    }
+}
+
 pub fn extract_group_permissions(
     group: &OpenMlsGroup,
 ) -> Result<GroupMutablePermissions, GroupMutablePermissionsError> {
@@ -864,8 +875,8 @@ impl PolicySet {
         let super_admin_remove_valid = commit.metadata_changes.super_admins_removed.is_empty()
             || (commit.actor.is_super_admin && commit.metadata_changes.num_super_admins > 0);
 
-        // TODO Validate permissions updates are valid
-        // once we add the user actions for updating permissions
+        // Permissions can only be changed by the super admin
+        let permissions_changes_valid = !commit.permissions_changed || commit.actor.is_super_admin;
 
         added_inboxes_valid
             && removed_inboxes_valid
@@ -874,6 +885,7 @@ impl PolicySet {
             && removed_admins_valid
             && super_admin_add_valid
             && super_admin_remove_valid
+            && permissions_changes_valid
     }
 
     fn evaluate_policy<'a, I, P>(
@@ -1148,6 +1160,7 @@ mod tests {
         member_added: Option<bool>,
         member_removed: Option<bool>,
         metadata_fields_changed: Option<Vec<String>>,
+        permissions_changed: bool,
         actor_is_super_admin: bool,
     ) -> ValidatedCommit {
         let actor = build_actor(None, None, actor_is_super_admin, actor_is_super_admin);
@@ -1181,6 +1194,7 @@ mod tests {
                 metadata_field_changes: field_changes,
                 ..Default::default()
             },
+            permissions_changed,
         }
     }
 
@@ -1195,7 +1209,7 @@ mod tests {
             PermissionsPolicies::allow_if_actor_super_admin(),
         );
 
-        let commit = build_validated_commit(Some(true), Some(true), None, false);
+        let commit = build_validated_commit(Some(true), Some(true), None, false, false);
         assert!(permissions.evaluate_commit(&commit));
     }
 
@@ -1210,10 +1224,10 @@ mod tests {
             PermissionsPolicies::allow_if_actor_super_admin(),
         );
 
-        let member_added_commit = build_validated_commit(Some(false), None, None, false);
+        let member_added_commit = build_validated_commit(Some(false), None, None, false, false);
         assert!(!permissions.evaluate_commit(&member_added_commit));
 
-        let member_removed_commit = build_validated_commit(None, Some(false), None, false);
+        let member_removed_commit = build_validated_commit(None, Some(false), None, false, false);
         assert!(!permissions.evaluate_commit(&member_removed_commit));
     }
 
@@ -1229,13 +1243,15 @@ mod tests {
         );
 
         // Can not remove the creator if they are the only super admin
-        let commit_with_creator = build_validated_commit(Some(true), Some(true), None, true);
+        let commit_with_creator = build_validated_commit(Some(true), Some(true), None, false, true);
         assert!(!permissions.evaluate_commit(&commit_with_creator));
 
-        let commit_with_creator = build_validated_commit(Some(true), Some(false), None, true);
+        let commit_with_creator =
+            build_validated_commit(Some(true), Some(false), None, false, true);
         assert!(permissions.evaluate_commit(&commit_with_creator));
 
-        let commit_without_creator = build_validated_commit(Some(true), Some(true), None, false);
+        let commit_without_creator =
+            build_validated_commit(Some(true), Some(true), None, false, false);
         assert!(!permissions.evaluate_commit(&commit_without_creator));
     }
 
@@ -1250,10 +1266,11 @@ mod tests {
             PermissionsPolicies::allow_if_actor_super_admin(),
         );
 
-        let commit_with_same_member = build_validated_commit(Some(true), None, None, false);
+        let commit_with_same_member = build_validated_commit(Some(true), None, None, false, false);
         assert!(permissions.evaluate_commit(&commit_with_same_member));
 
-        let commit_with_different_member = build_validated_commit(Some(false), None, None, false);
+        let commit_with_different_member =
+            build_validated_commit(Some(false), None, None, false, false);
         assert!(!permissions.evaluate_commit(&commit_with_different_member));
     }
 
@@ -1271,7 +1288,7 @@ mod tests {
             PermissionsPolicies::allow_if_actor_super_admin(),
         );
 
-        let member_added_commit = build_validated_commit(Some(true), None, None, false);
+        let member_added_commit = build_validated_commit(Some(true), None, None, false, false);
         assert!(!permissions.evaluate_commit(&member_added_commit));
     }
 
@@ -1289,7 +1306,7 @@ mod tests {
             PermissionsPolicies::allow_if_actor_super_admin(),
         );
 
-        let member_added_commit = build_validated_commit(Some(true), None, None, false);
+        let member_added_commit = build_validated_commit(Some(true), None, None, false, false);
         assert!(permissions.evaluate_commit(&member_added_commit));
     }
 
@@ -1336,6 +1353,7 @@ mod tests {
             None,
             Some(vec![MetadataField::GroupName.to_string()]),
             false,
+            false,
         );
 
         assert!(allow_permissions.evaluate_commit(&member_added_commit));
@@ -1419,4 +1437,24 @@ mod tests {
 
         assert!(is_policy_admin_only(&policy_set_new_metadata_permission));
     }
+
+    #[test]
+    fn test_permission_update() {
+        let permissions = PolicySet::new(
+            MembershipPolicies::allow(),
+            MembershipPolicies::allow_if_actor_admin(),
+            MetadataPolicies::default_map(MetadataPolicies::allow()),
+            PermissionsPolicies::allow_if_actor_super_admin(),
+            PermissionsPolicies::allow_if_actor_super_admin(),
+            PermissionsPolicies::allow_if_actor_super_admin(),
+        );
+
+        // Commit should fail because actor is not superadmin
+        let commit = build_validated_commit(None, None, None, true, false);
+        assert!(!permissions.evaluate_commit(&commit));
+
+        // Commit should pass because actor is superadmin
+        let commit = build_validated_commit(None, None, None, true, true);
+        assert!(permissions.evaluate_commit(&commit));
+    }
 }

xmtp_mls/src/groups/intents.rs

@@ -19,17 +19,22 @@ use xmtp_proto::xmtp::mls::database::{
         Version as UpdateGroupMembershipVersion, V1 as UpdateGroupMembershipV1,
     },
     update_metadata_data::{Version as UpdateMetadataVersion, V1 as UpdateMetadataV1},
+    update_permission_data::{Version as UpdatePermissionVersion, V1 as UpdatePermissionV1},
     AccountAddresses, AddressesOrInstallationIds as AddressesOrInstallationIdsProtoWrapper,
     InstallationIds, PostCommitAction as PostCommitActionProto, SendMessageData,
-    UpdateAdminListsData, UpdateGroupMembershipData, UpdateMetadataData,
+    UpdateAdminListsData, UpdateGroupMembershipData, UpdateMetadataData, UpdatePermissionData,
 };
 
 use crate::{
     types::Address,
     verified_key_package_v2::{KeyPackageVerificationError, VerifiedKeyPackageV2},
 };
 
-use super::{group_membership::GroupMembership, group_mutable_metadata::MetadataField};
+use super::{
+    group_membership::GroupMembership,
+    group_mutable_metadata::MetadataField,
+    group_permissions::{MembershipPolicies, MetadataPolicies, PermissionsPolicies},
+};
 
 #[derive(Debug, Error)]
 pub enum IntentError {
@@ -373,6 +378,174 @@ impl TryFrom<Vec<u8>> for UpdateAdminListIntentData {
     }
 }
 
+#[repr(i32)]
+#[derive(Debug, Clone, PartialEq)]
+pub enum PermissionUpdateType {
+    AddMember = 1,      // Matches ADD_MEMBER in Protobuf
+    RemoveMember = 2,   // Matches REMOVE_MEMBER in Protobuf
+    AddAdmin = 3,       // Matches ADD_ADMIN in Protobuf
+    RemoveAdmin = 4,    // Matches REMOVE_ADMIN in Protobuf
+    UpdateMetadata = 5, // Matches UPDATE_METADATA in Protobuf
+}
+
+impl TryFrom<i32> for PermissionUpdateType {
+    type Error = &'static str;
+
+    fn try_from(value: i32) -> Result<Self, Self::Error> {
+        match value {
+            1 => Ok(PermissionUpdateType::AddMember),
+            2 => Ok(PermissionUpdateType::RemoveMember),
+            3 => Ok(PermissionUpdateType::AddAdmin),
+            4 => Ok(PermissionUpdateType::RemoveAdmin),
+            5 => Ok(PermissionUpdateType::UpdateMetadata),
+            _ => Err("Unknown value for PermissionUpdateType"),
+        }
+    }
+}
+
+#[repr(i32)]
+#[derive(Debug, Clone, PartialEq)]
+pub enum PermissionPolicyOption {
+    Allow = 1,          // Matches ADD_MEMBER in Protobuf
+    Deny = 2,           // Matches REMOVE_MEMBER in Protobuf
+    AdminOnly = 3,      // Matches ADD_ADMIN in Protobuf
+    SuperAdminOnly = 4, // Matches REMOVE_ADMIN in Protobuf
+}
+
+impl TryFrom<i32> for PermissionPolicyOption {
+    type Error = &'static str;
+
+    fn try_from(value: i32) -> Result<Self, Self::Error> {
+        match value {
+            1 => Ok(PermissionPolicyOption::Allow),
+            2 => Ok(PermissionPolicyOption::Deny),
+            3 => Ok(PermissionPolicyOption::AdminOnly),
+            4 => Ok(PermissionPolicyOption::SuperAdminOnly),
+            _ => Err("Unknown value for PermissionPolicyOption"),
+        }
+    }
+}
+
+impl From<PermissionPolicyOption> for MembershipPolicies {
+    fn from(value: PermissionPolicyOption) -> Self {
+        match value {
+            PermissionPolicyOption::Allow => MembershipPolicies::allow(),
+            PermissionPolicyOption::Deny => MembershipPolicies::deny(),
+            PermissionPolicyOption::AdminOnly => MembershipPolicies::allow_if_actor_admin(),
+            PermissionPolicyOption::SuperAdminOnly => {
+                MembershipPolicies::allow_if_actor_super_admin()
+            }
+        }
+    }
+}
+
+impl From<PermissionPolicyOption> for MetadataPolicies {
+    fn from(value: PermissionPolicyOption) -> Self {
+        match value {
+            PermissionPolicyOption::Allow => MetadataPolicies::allow(),
+            PermissionPolicyOption::Deny => MetadataPolicies::deny(),
+            PermissionPolicyOption::AdminOnly => MetadataPolicies::allow_if_actor_admin(),
+            PermissionPolicyOption::SuperAdminOnly => {
+                MetadataPolicies::allow_if_actor_super_admin()
+            }
+        }
+    }
+}
+
+impl From<PermissionPolicyOption> for PermissionsPolicies {
+    fn from(value: PermissionPolicyOption) -> Self {
+        match value {
+            PermissionPolicyOption::Allow => {
+                log::error!("PermissionPolicyOption::Allow is not allowed for PermissionsPolicies, set to super_admin only instead");
+                PermissionsPolicies::allow_if_actor_super_admin()
+            }
+            PermissionPolicyOption::Deny => PermissionsPolicies::deny(),
+            PermissionPolicyOption::AdminOnly => PermissionsPolicies::allow_if_actor_admin(),
+            PermissionPolicyOption::SuperAdminOnly => {
+                PermissionsPolicies::allow_if_actor_super_admin()
+            }
+        }
+    }
+}
+
+#[derive(Debug, Clone)]
+pub struct UpdatePermissionIntentData {
+    pub update_type: PermissionUpdateType,
+    pub policy_option: PermissionPolicyOption,
+    pub metadata_field_name: Option<String>,
+}
+
+impl UpdatePermissionIntentData {
+    pub fn new(
+        update_type: PermissionUpdateType,
+        policy_option: PermissionPolicyOption,
+        metadata_field_name: Option<String>,
+    ) -> Self {
+        Self {
+            update_type,
+            policy_option,
+            metadata_field_name,
+        }
+    }
+}
+
+impl From<UpdatePermissionIntentData> for Vec<u8> {
+    fn from(intent: UpdatePermissionIntentData) -> Self {
+        let mut buf = Vec::new();
+        let update_type = intent.update_type as i32;
+        let policy_option = intent.policy_option as i32;
+
+        UpdatePermissionData {
+            version: Some(UpdatePermissionVersion::V1(UpdatePermissionV1 {
+                permission_update_type: update_type,
+                permission_policy_option: policy_option,
+                metadata_field_name: intent.metadata_field_name,
+            })),
+        }
+        .encode(&mut buf)
+        .expect("encode error");
+
+        buf
+    }
+}
+
+impl TryFrom<Vec<u8>> for UpdatePermissionIntentData {
+    type Error = IntentError;
+
+    fn try_from(data: Vec<u8>) -> Result<Self, Self::Error> {
+        let msg = UpdatePermissionData::decode(Bytes::from(data))?;
+
+        let update_type: PermissionUpdateType = match msg.version {
+            Some(UpdatePermissionVersion::V1(ref v1)) => {
+                PermissionUpdateType::try_from(v1.permission_update_type)
+                    .map_err(|e| IntentError::Generic(e.to_string()))?
+            }
+            None => {
+                return Err(IntentError::Generic(
+                    "missing update permission version".to_string(),
+                ))
+            }
+        };
+        let policy_option: PermissionPolicyOption = match msg.version {
+            Some(UpdatePermissionVersion::V1(ref v1)) => {
+                PermissionPolicyOption::try_from(v1.permission_policy_option)
+                    .map_err(|e| IntentError::Generic(e.to_string()))?
+            }
+            None => {
+                return Err(IntentError::Generic(
+                    "missing update permission version".to_string(),
+                ))
+            }
+        };
+        let metadata_field_name = match msg.version {
+            Some(UpdatePermissionVersion::V1(ref v1)) => v1.metadata_field_name.clone(),
+            None => None,
+        };
+
+        Ok(Self::new(update_type, policy_option, metadata_field_name))
+    }
+}
+
 #[derive(Debug, Clone)]
 pub enum PostCommitAction {
     SendWelcomes(SendWelcomesAction),