From 0ffaa932abb3de10009e71d14025fef44f56573e Mon Sep 17 00:00:00 2001 From: Moritz Date: Wed, 1 Feb 2023 10:21:08 +0100 Subject: [PATCH 1/2] Add `acl_get_permissoned_accounts` --- .../src/access_controllable.rs | 56 ++++++++++++++ near-plugins/src/access_controllable.rs | 76 +++++++++++++++++++ 2 files changed, 132 insertions(+) diff --git a/near-plugins-derive/src/access_controllable.rs b/near-plugins-derive/src/access_controllable.rs index 3a879c0..15adf1d 100644 --- a/near-plugins-derive/src/access_controllable.rs +++ b/near-plugins-derive/src/access_controllable.rs @@ -410,6 +410,17 @@ pub fn access_controllable(attrs: TokenStream, item: TokenStream) -> TokenStream set.iter().skip(skip).take(limit).cloned().collect() } + /// Returns _all_ bearers of `permission`. In this implementation of + /// `AccessControllable` there is no upper bound on the number of bearers per + /// permission, so gas limits should be considered when calling this function. + fn get_all_bearers(&self, permission: #bitflags_type) -> Vec<::near_sdk::AccountId> { + let set = match self.bearers.get(&permission) { + Some(set) => set, + None => return vec![], + }; + set.iter().cloned().collect() + } + /// Removes `account_id` from the set of `permission` bearers. fn remove_bearer(&mut self, permission: #bitflags_type, account_id: &::near_sdk::AccountId) { // If `permission` is invalid (more than one active bit), this @@ -420,6 +431,47 @@ pub fn access_controllable(attrs: TokenStream, item: TokenStream) -> TokenStream }; set.remove(account_id); } + + /// Provides the implementation of `AccessControllable::acl_get_permissioned_accounts`. + /// + /// Uniqueness of account ids in returned vectors is guaranteed by the ids being + /// retrieved from bearer sets. + fn get_permissioned_accounts(&self) -> #cratename::access_controllable::PermissionedAccounts { + // Get super admins. + let permission = <#bitflags_type>::from_bits( + <#role_type>::acl_super_admin_permission() + ) + .unwrap_or_else(|| ::near_sdk::env::panic_str(#ERR_PARSE_BITFLAG)); + let super_admins = self.get_all_bearers(permission); + + // Get admins and grantees per role. + let roles = <#role_type>::acl_role_variants(); + let mut map = ::std::collections::HashMap::new(); + for role in roles { + let role: #role_type = ::std::convert::TryFrom::try_from(role) + .unwrap_or_else(|_| ::near_sdk::env::panic_str(#ERR_PARSE_ROLE)); + let admin_permission = <#bitflags_type>::from_bits(role.acl_admin_permission()) + .unwrap_or_else(|| ::near_sdk::env::panic_str(#ERR_PARSE_BITFLAG)); + let admins = self.get_all_bearers(admin_permission); + + let grantee_permission = <#bitflags_type>::from_bits(role.acl_permission()) + .unwrap_or_else(|| ::near_sdk::env::panic_str(#ERR_PARSE_BITFLAG)); + let grantees = self.get_all_bearers(grantee_permission); + + map.insert( + role.into(), + #cratename::access_controllable::PermissionedAccountsPerRole { + admins, + grantees, + } + ); + } + + #cratename::access_controllable::PermissionedAccounts { + super_admins, + roles: map, + } + } } // Note that `#[near-bindgen]` exposes non-public functions in trait @@ -518,6 +570,10 @@ pub fn access_controllable(attrs: TokenStream, item: TokenStream) -> TokenStream .unwrap_or_else(|| ::near_sdk::env::panic_str(#ERR_PARSE_BITFLAG)); self.#acl_field.get_bearers(permission, skip, limit) } + + fn acl_get_permissioned_accounts(&self) -> #cratename::access_controllable::PermissionedAccounts { + self.#acl_field.get_permissioned_accounts() + } } }; diff --git a/near-plugins/src/access_controllable.rs b/near-plugins/src/access_controllable.rs index 4ab31d2..2a24433 100644 --- a/near-plugins/src/access_controllable.rs +++ b/near-plugins/src/access_controllable.rs @@ -1,4 +1,6 @@ +use near_sdk::serde::{Deserialize, Serialize}; use near_sdk::AccountId; +use std::collections::HashMap; /// # Representation of roles /// @@ -218,6 +220,80 @@ pub trait AccessControllable { /// Enables paginated retrieval of grantees of `role`. It returns up to /// `limit` grantees and skips the first `skip` grantees. fn acl_get_grantees(&self, role: String, skip: u64, limit: u64) -> Vec; + + /// Convenience method that returns all [`PermissionedAccounts`]. + /// + /// # Gas limits + /// + /// This function is eligible for view calls and while view calls are free for users, the + /// underlying transaction is still subject to a [gas limit] defined by the RPC node. + /// + /// In use cases where gas cost matters, the data returned by this function can be retrieved + /// more efficiently by a combination of the following: + /// + /// * Get roles with [`Self::acl_get_roles`]. + /// * Get (a subset) of permissioned accounts with [`Self::acl_get_super_admins`], + /// [`Self::acl_get_admins`], or [`Self::acl_get_grantees`]. + /// + /// [gas limit]: https://github.com/near/nearcore/pull/4381 + fn acl_get_permissioned_accounts(&self) -> PermissionedAccounts; +} + +/// Collects super admin accounts and accounts that have been granted permissions defined by +/// `AccessControlRole`. +/// +/// # Data structure +/// +/// Assume `AccessControlRole` is derived for the following enum, which is then passed as `role` +/// attribute to `AccessControllable`. +/// +/// ```rust +/// pub enum Role { +/// PauseManager, +/// UnpauseManager, +/// } +/// ``` +/// +/// Then the returned data has the following structure: +/// +/// ```ignore +/// PermissionedAccounts { +/// super_admins: vec!["acc1.near", "acc2.near"], +/// roles: HashMap::from([ +/// ("PauseManager", PermissionedAccountsPerRole { +/// admins: vec!["acc3.near", "acc4.near"], +/// grantees: vec!["acc5.near", "acc6.near"], +/// }), +/// ("UnpauseManager", PermissionedAccountsPerRole { +/// admins: vec!["acc7.near", "acc8.near"], +/// grantees: vec!["acc9.near", "acc10.near"], +/// }), +/// ]) +/// } +/// ``` +/// +/// # Uniqueness and ordering +/// +/// Account ids returned in vectors are unique but not ordered. +#[derive(Deserialize, Serialize, Debug)] +pub struct PermissionedAccounts { + /// The accounts that have super admin permissions. + pub super_admins: Vec, + /// The admins and grantees of all roles. + pub roles: HashMap, +} + +/// Collects all admins and grantees of a role. +/// +/// # Uniqueness and ordering +/// +/// Account ids returned in vectors are unique but not ordered. +#[derive(Deserialize, Serialize, Debug)] +pub struct PermissionedAccountsPerRole { + /// The accounts that have admin permissions for the role. + pub admins: Vec, + /// The accounts that have been granted the role. + pub grantees: Vec, } pub mod events { From aa88429d15eec571ed81f3ed7efb24da0ae10cb2 Mon Sep 17 00:00:00 2001 From: Moritz Date: Thu, 2 Feb 2023 11:04:05 +0100 Subject: [PATCH 2/2] Add test --- near-plugins/tests/access_controllable.rs | 144 +++++++++++++++++- .../common/access_controllable_contract.rs | 13 ++ near-plugins/tests/common/utils.rs | 10 ++ 3 files changed, 165 insertions(+), 2 deletions(-) diff --git a/near-plugins/tests/access_controllable.rs b/near-plugins/tests/access_controllable.rs index d6749aa..dea60a4 100644 --- a/near-plugins/tests/access_controllable.rs +++ b/near-plugins/tests/access_controllable.rs @@ -4,10 +4,12 @@ pub mod common; use common::access_controllable_contract::AccessControllableContract; use common::utils::{ - assert_insufficient_acl_permissions, assert_private_method_failure, assert_success_with, + as_sdk_account_id, assert_insufficient_acl_permissions, assert_private_method_failure, + assert_success_with, }; +use near_plugins::access_controllable::{PermissionedAccounts, PermissionedAccountsPerRole}; use near_sdk::serde_json::json; -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::convert::TryFrom; use std::path::Path; use workspaces::network::Sandbox; @@ -117,6 +119,82 @@ async fn call_increase_2( .await } +/// Returns new `PermissionedAccounts` for [`ALL_ROLES`]. +fn new_permissioned_accounts() -> PermissionedAccounts { + let mut permissioned_accounts = PermissionedAccounts { + super_admins: vec![], + roles: HashMap::new(), + }; + + for role in ALL_ROLES { + permissioned_accounts.roles.insert( + role.to_string(), + PermissionedAccountsPerRole { + admins: vec![], + grantees: vec![], + }, + ); + } + + permissioned_accounts +} + +/// Asserts both `PermissionedAcccounts` contain the same accounts with the same permissions, +/// disregarding order. +/// +/// Expects both `a` and `b` to contain every role in [`ALL_ROLES`]. +/// +/// This function is available only in tests and used for small numbers of accounts, so simplicity +/// is favored over efficiency. +fn assert_permissioned_account_equivalence(a: &PermissionedAccounts, b: &PermissionedAccounts) { + // Verify super admins. + assert_account_ids_equivalence( + a.super_admins.as_slice(), + b.super_admins.as_slice(), + "super_admins", + ); + + // Verify admins and grantees per role. + assert_eq!(a.roles.len(), b.roles.len(), "Unequal number of roles"); + assert_eq!(a.roles.len(), ALL_ROLES.len(), "More roles than expected"); + for role in ALL_ROLES { + let per_role_a = a + .roles + .get(role) + .unwrap_or_else(|| panic!("PermissionedAccounts a misses role {}", role)); + let per_role_b = b + .roles + .get(role) + .unwrap_or_else(|| panic!("PermissionedAccounts b misses role {}", role)); + + assert_account_ids_equivalence( + &per_role_a.admins, + &per_role_b.admins, + format!("admins of role {}", role).as_str(), + ); + assert_account_ids_equivalence( + &per_role_a.grantees, + &per_role_b.grantees, + format!("grantees of role {}", role).as_str(), + ); + } +} + +/// Asserts `a` and `b` contain the same `AccountId`s, disregarding order. Parameter `specifier` is +/// passed to the panic message in case of a mismatch. +/// +/// This function is available only in tests and used for small numbers of accounts, so simplicity +/// is favored over efficiency. +fn assert_account_ids_equivalence( + a: &[near_sdk::AccountId], + b: &[near_sdk::AccountId], + specifier: &str, +) { + let set_a: HashSet<_> = a.iter().cloned().collect(); + let set_b: HashSet<_> = b.iter().cloned().collect(); + assert_eq!(set_a, set_b, "Unequal sets of AccountIds for {}", specifier,); +} + /// Smoke test of contract setup and basic functionality. #[tokio::test] async fn test_increase_and_get_counter() -> anyhow::Result<()> { @@ -1125,6 +1203,68 @@ async fn test_acl_get_grantees() -> anyhow::Result<()> { Ok(()) } +#[tokio::test] +async fn test_acl_get_permissioned_accounts() -> anyhow::Result<()> { + let setup = Setup::new().await?; + + // Verify returned `PermissionedAccounts` are empty when there are no outstanding permissions. + let permissioned_accounts = setup + .contract + .acl_get_permissioned_accounts(&setup.account) + .await?; + let mut expected = new_permissioned_accounts(); + assert_permissioned_account_equivalence(&permissioned_accounts, &expected); + + // Add a super admin to the contract's Acl. + let super_admin = setup.worker.dev_create_account().await?; + let res = setup + .contract + .acl_init_super_admin(setup.contract_account(), super_admin.id()) + .await?; + assert_success_with(res, true); + + // Add admins and grantees to the contract's Acl. + let admin_0 = setup.new_account_as_admin(&[ALL_ROLES[0]]).await?; + let admin_2 = setup.new_account_as_admin(&[ALL_ROLES[2]]).await?; + let grantee_1_a = setup.new_account_with_roles(&[ALL_ROLES[1]]).await?; + let grantee_1_b = setup.new_account_with_roles(&[ALL_ROLES[1]]).await?; + + // Insert ids added to contract's Acl into `expected`. + expected + .super_admins + .push(as_sdk_account_id(super_admin.id())); + expected + .roles + .get_mut(ALL_ROLES[0]) + .unwrap() + .admins + .push(as_sdk_account_id(admin_0.id())); + expected + .roles + .get_mut(ALL_ROLES[1]) + .unwrap() + .grantees + .extend([ + as_sdk_account_id(grantee_1_a.id()), + as_sdk_account_id(grantee_1_b.id()), + ]); + expected + .roles + .get_mut(ALL_ROLES[2]) + .unwrap() + .admins + .push(as_sdk_account_id(admin_2.id())); + + // Verify returned `PermissionedAccounts` when there are outstanding permissions. + let permissioned_accounts = setup + .contract + .acl_get_permissioned_accounts(&setup.account) + .await?; + assert_permissioned_account_equivalence(&permissioned_accounts, &expected); + + Ok(()) +} + #[tokio::test] async fn test_acl_add_super_admin_unchecked_is_private() -> anyhow::Result<()> { let Setup { diff --git a/near-plugins/tests/common/access_controllable_contract.rs b/near-plugins/tests/common/access_controllable_contract.rs index 7af7599..f544440 100644 --- a/near-plugins/tests/common/access_controllable_contract.rs +++ b/near-plugins/tests/common/access_controllable_contract.rs @@ -1,3 +1,5 @@ +use near_plugins::access_controllable::PermissionedAccounts; + use near_sdk::serde_json::json; use workspaces::result::ExecutionFinalResult; use workspaces::{Account, AccountId, Contract}; @@ -384,4 +386,15 @@ impl AccessControllableContract { .json::>()?; Ok(res) } + + pub async fn acl_get_permissioned_accounts( + &self, + caller: &Account, + ) -> anyhow::Result { + let res = caller + .call(self.contract.id(), "acl_get_permissioned_accounts") + .view() + .await?; + Ok(res.json::()?) + } } diff --git a/near-plugins/tests/common/utils.rs b/near-plugins/tests/common/utils.rs index a0bc256..98dabf5 100644 --- a/near-plugins/tests/common/utils.rs +++ b/near-plugins/tests/common/utils.rs @@ -1,7 +1,17 @@ use near_sdk::serde::de::DeserializeOwned; use std::cmp::PartialEq; use std::fmt::Debug; +use std::str::FromStr; use workspaces::result::ExecutionFinalResult; +use workspaces::AccountId; + +/// Converts `account_id` to a `near_sdk::AccountId` and panics on failure. +/// +/// Only available in tests, hence favoring simplicity over efficiency. +pub fn as_sdk_account_id(account_id: &AccountId) -> near_sdk::AccountId { + near_sdk::AccountId::from_str(account_id.as_str()) + .expect("Conversion to near_sdk::AccountId should succeed") +} /// Asserts execution was successful and returned `()`. pub fn assert_success_with_unit_return(res: ExecutionFinalResult) {