Fix invalid policy error #867
Conversation
…tom policies don't throw error
cameronvoell
force-pushed
the
cv/invalid-permission-refactor-errors
branch
from
6705dc3 to
8d82f67
Compare
| @@ -994,7 +1054,7 @@ pub(crate) fn policy_all_members() -> PolicySet { | |||
| } | |||
| PolicySet::new( | |||
| MembershipPolicies::allow(), | |||
| MembershipPolicies::allow(), | |||
| MembershipPolicies::allow_if_actor_admin(), | |||
There was a problem hiding this comment.
Remove members should require admin /super admin, even for All Member preconfigured policy to be aligned with Permissions XIP. Fixes #857
| @@ -1034,12 +1094,12 @@ impl PreconfiguredPolicies { | |||
| } | |||
|
|
|||
| pub fn from_policy_set(policy_set: &PolicySet) -> Result<Self, PolicyError> { | |||
There was a problem hiding this comment.
This function was causing the Invalid Policy Errors on existing groups after adding group image url. The reason was that the existing policies no longer matched the "default policies" for "Add Members" or "Admin only" because those policy sets now had one new property in them.
| if let Ok(preconfigured_policy) = self.inner.preconfigured_policy() { | ||
| Ok(preconfigured_policy.into()) | ||
| } else { | ||
| Ok(GroupPermissions::CustomPolicy) |
There was a problem hiding this comment.
Moving forward, we wont return an error if our Policy Set does not match one of the preconfigured policies, we will just return that it is a custom policy. This policy type function is mainly used for informing users / UI
Fixes #851
We were throwing an error when reading group policies whenever the policy did not exactly match the libxmtp defined policy set for our preconfigured policy set of All Members, or Admin Only. When we added a new metadata option, those preconfigured policies would add a policy for the new metadata option, and the existing group policy sets would not have that, so we would not match any of the preconfigured policy sets.
Part 1 of the fix is to make it so that if all the metadata policies match the all members / admin only policy set default, that they can still match as being All Members / Admin Only, even if they are missing or have an new metadata policy.
Part 2 of the fix is to not return an error if the group policy set does not perfectly match a preconfigured policy set, and instead return
GroupPermissions::CustomPolicy. This makes the SDK more robust against updates. For example this PR updates the remove member policy to default to "admin only" for "All Members". That means existing groups who already have that policy set to "allow all" will show up in new versions as a Custom Policy instead of just erroring.Related PRs: