From 6f42406d34b2ab3c45b355bb67315d7a90ded868 Mon Sep 17 00:00:00 2001 From: Moritz Date: Sat, 10 Sep 2022 12:07:41 +0200 Subject: [PATCH 01/27] Add building blocks for Acl plugin --- .gitignore | 1 + .../src/access_control_role.rs | 118 ++++++++++ .../src/access_controllable.rs | 208 ++++++++++++++++++ near-plugins-derive/src/lib.rs | 12 + near-plugins/Cargo.toml | 4 + near-plugins/src/access_control_role.rs | 10 + near-plugins/src/access_controllable.rs | 33 +++ near-plugins/src/lib.rs | 7 +- near-plugins/tests/access_controllable.rs | 78 +++++++ near-plugins/tests/contracts/README.md | 3 + .../contracts/access_controllable/Cargo.toml | 22 ++ .../contracts/access_controllable/Makefile | 6 + .../contracts/access_controllable/src/lib.rs | 43 ++++ 13 files changed, 544 insertions(+), 1 deletion(-) create mode 100644 near-plugins-derive/src/access_control_role.rs create mode 100644 near-plugins-derive/src/access_controllable.rs create mode 100644 near-plugins/src/access_control_role.rs create mode 100644 near-plugins/src/access_controllable.rs create mode 100644 near-plugins/tests/access_controllable.rs create mode 100644 near-plugins/tests/contracts/README.md create mode 100644 near-plugins/tests/contracts/access_controllable/Cargo.toml create mode 100644 near-plugins/tests/contracts/access_controllable/Makefile create mode 100644 near-plugins/tests/contracts/access_controllable/src/lib.rs diff --git a/.gitignore b/.gitignore index d97337f..af0ad46 100644 --- a/.gitignore +++ b/.gitignore @@ -1,5 +1,6 @@ /target Cargo.lock +near-plugins/tests/contracts/*/target # Ignore IDE data .vscode/ diff --git a/near-plugins-derive/src/access_control_role.rs b/near-plugins-derive/src/access_control_role.rs new file mode 100644 index 0000000..81f2ce7 --- /dev/null +++ b/near-plugins-derive/src/access_control_role.rs @@ -0,0 +1,118 @@ +use proc_macro::TokenStream; +use quote::quote; +use std::convert::TryFrom; +use syn::{parse_macro_input, ItemEnum}; + +pub fn derive_access_control_role(input: TokenStream) -> TokenStream { + // This derive doesn't take attributes, so no need to use `darling`. + let input: ItemEnum = parse_macro_input!(input); + let ItemEnum { + ident, variants, .. + } = input; + + let (variant_idxs, variant_items): (Vec<_>, Vec<_>) = + variants.iter().cloned().enumerate().unzip(); + let variant_idxs = variant_idxs + .iter() + .map(|&idx| { + u8::try_from(idx).expect("The number of variants should be representable by u8") + }) + .collect::>(); + let variant_names = variants + .iter() + .map(|v| format!("{}", v.ident)) + .collect::>(); + + let output = quote! { + impl From<#ident> for u8 { + fn from(value: #ident) -> Self { + match value { + #( + #ident::#variant_items => #variant_idxs, + )* + } + } + } + + impl ::std::convert::TryFrom for #ident { + type Error = &'static str; + + fn try_from(value: u8) -> Result { + match value { + #( + #variant_idxs => Ok(#ident::#variant_items), + )* + _ => Err("Value does not correspond to a variant"), + } + } + } + + impl From<#ident> for &'static str { + fn from(value: #ident) -> Self { + match value { + #( + #ident::#variant_items => #variant_names, + )* + } + } + } + + // TODO use `TryFrom<&str>` (for consistency) + impl ::std::str::FromStr for #ident { + type Err = &'static str; + + fn from_str(value: &str) -> Result<#ident, Self::Err> { + match value { + #( + #variant_names => Ok(#ident::#variant_items), + )* + _ => Err("Value does not correspond to a variant"), + } + } + } + + /// Panics if `n` is too large. + fn safe_leftshift(value: u128, n: u8) -> u128 { + value + .checked_shl(n.into()) + .expect("Too many enum variants to be represented by bitflags") + } + + // TODO explain enum<->bitflag conversion + impl AccessControlRole for #ident { + fn acl_super_admin_permission_bitflag() -> u128 { + safe_leftshift(1, 0) + } + + fn acl_permission_bitflag(self) -> u128 { + let n = (u8::from(self) + 1) + .checked_mul(2) + .expect("Too many enum variants") - 1; + safe_leftshift(1, n) + } + + fn acl_admin_permission_bitflag(self) -> u128 { + let n = (u8::from(self) + 1) + .checked_mul(2) + .expect("Too many enum variants"); + safe_leftshift(1, n) + } + } + + ::bitflags::bitflags! { + // TODO generate dynamically + #[derive(BorshDeserialize, BorshSerialize, Default)] + struct RoleFlags: u128 { + const __SUPER_ADMIN = 1u128 << 0; + const LEVEL1 = 1u128 << 1; + const LEVEL1_ADMIN = 1u128 << 2; + const LEVEL2 = 1u128 << 3; + const LEVEL2_ADMIN = 1u128 << 4; + const LEVEL3 = 1u128 << 5; + const LEVEL3_ADMIN = 1u128 << 6; + } + } + }; + + output.into() +} diff --git a/near-plugins-derive/src/access_controllable.rs b/near-plugins-derive/src/access_controllable.rs new file mode 100644 index 0000000..682b46e --- /dev/null +++ b/near-plugins-derive/src/access_controllable.rs @@ -0,0 +1,208 @@ +use darling::FromMeta; +use proc_macro::TokenStream; +use proc_macro2::Span; +use quote::quote; +use syn::parse::Parser; +use syn::{parse_macro_input, AttributeArgs, ItemStruct}; + +#[derive(Debug, FromMeta)] +pub struct MacroArgs { + #[darling(default)] + storage_prefix: Option, + role_type: syn::Path, +} + +const DEFAULT_STORAGE_PREFIX: &str = "__acl"; + +/// TODO allow customization via macro arguments +const ACL_FIELD_NAME: &str = "__acl"; +const ACL_TYPE_NAME: &str = "__Acl"; + +const ERR_PARSE_BITFLAG: &str = "Value does not correspond to a permission"; +const ERR_PARSE_ROLE: &str = "Value does not correspond to a role"; + +pub fn access_controllable(attrs: TokenStream, item: TokenStream) -> TokenStream { + let attr_args = parse_macro_input!(attrs as AttributeArgs); + let mut input: ItemStruct = parse_macro_input!(item); + // TODO seems like this is required for ItemFn but not for ItemStruct? + /* + if is_near_bindgen_wrapped_or_marshall(&input) { + return item; + } + */ + let acl_field = syn::Ident::new(ACL_FIELD_NAME, Span::call_site()); + let acl_type = syn::Ident::new(ACL_TYPE_NAME, Span::call_site()); + // TODO determine name of ident dynamically + let permissions_ident = syn::Ident::new("RoleFlags", Span::call_site()); + if let Err(e) = inject_acl_field(&mut input, &acl_field, &acl_type) { + return TokenStream::from(e.to_compile_error()); + } + let ItemStruct { ident, .. } = input.clone(); + + // TODO verify trait bounds on `role_enum`: BorshSerialize, BorshDeserialize, ... + let macro_args = match MacroArgs::from_list(&attr_args) { + Ok(args) => args, + Err(e) => { + return TokenStream::from(e.write_errors()); + } + }; + let storage_prefix = macro_args + .storage_prefix + .unwrap_or_else(|| DEFAULT_STORAGE_PREFIX.to_string()); + let role_type = macro_args.role_type; + + let output = quote! { + #input + + #[derive(::near_sdk::borsh::BorshDeserialize, ::near_sdk::borsh::BorshSerialize)] + struct #acl_type { + /// Stores permissions per account. + permissions: ::near_sdk::collections::UnorderedMap< + ::near_sdk::AccountId, + #permissions_ident, + >, + /// Stores the set of accounts that bear a permission. + bearers: ::near_sdk::collections::UnorderedMap< + #permissions_ident, + ::near_sdk::collections::UnorderedSet<::near_sdk::AccountId>, + >, + } + + impl Default for #acl_type { + fn default() -> Self { + // TODO make AccessControllable::acl_storage_prefix static, then use here + let base_prefix = (#storage_prefix).as_bytes(); + Self { + permissions: ::near_sdk::collections::UnorderedMap::new( + __acl_storage_prefix(base_prefix, __AclStorageKey::Permissions), + ), + bearers: ::near_sdk::collections::UnorderedMap::new( + __acl_storage_prefix(base_prefix, __AclStorageKey::Bearers), + ), + } + } + } + + /// Used to make storage prefixes unique. Not to be used directly, + /// instead it should be prepended to the storage prefix specified by + /// the user. + #[derive(::near_sdk::borsh::BorshSerialize)] + enum __AclStorageKey { + Permissions, + Bearers, + BearersSet { permission: #permissions_ident }, + } + + /// Generates a prefix by concatenating the input parameters. + fn __acl_storage_prefix(base: &[u8], specifier: __AclStorageKey) -> Vec { + let specifier = specifier + .try_to_vec() + .expect("Storage key should be serializable"); + [base, specifier.as_slice()].concat() + } + + impl #acl_type { + fn new_bearers_set(permission: #permissions_ident) -> ::near_sdk::collections::UnorderedSet<::near_sdk::AccountId> { + // TODO make AccessControllable::acl_storage_prefix static, then use here + let base_prefix = (#storage_prefix).as_bytes(); + let specifier = __AclStorageKey::BearersSet { permission }; + ::near_sdk::collections::UnorderedSet::new(__acl_storage_prefix(base_prefix, specifier)) + } + + fn get_or_init_permissions(&self, account_id: &::near_sdk::AccountId) -> #permissions_ident { + match self.permissions.get(account_id) { + Some(permissions) => permissions, + None => <#permissions_ident>::empty(), + } + } + + fn grant_role_unchecked(&mut self, role: #role_type, account_id: &::near_sdk::AccountId) -> bool { + let flag = <#permissions_ident>::from_bits(role.acl_permission_bitflag()) + .expect(#ERR_PARSE_BITFLAG); + let mut permissions = self.get_or_init_permissions(account_id); + + let is_new_grantee = !permissions.contains(flag); + if is_new_grantee { + permissions.insert(flag); + self.permissions.insert(account_id, &permissions); + self.add_bearer(flag, account_id); + // TODO emit event + } + + is_new_grantee + } + + fn has_role(&self, role: #role_type, account_id: &::near_sdk::AccountId) -> bool { + match self.permissions.get(account_id) { + Some(permissions) => { + let flag = <#permissions_ident>::from_bits(role.acl_permission_bitflag()) + .expect(#ERR_PARSE_BITFLAG); + permissions.contains(flag) + } + None => false, + } + } + + /// Adds `account_id` to the set of `permission` bearers. + fn add_bearer(&mut self, permission: #permissions_ident, account_id: &::near_sdk::AccountId) { + let mut set = match self.bearers.get(&permission) { + Some(set) => set, + None => Self::new_bearers_set(permission), + }; + if let true = set.insert(account_id) { + self.bearers.insert(&permission, &set); + } + } + } + + // TODO control which functions are exposed externally + // `near_bindgen` externally exposes functions in trait implementations + // _even_ if they are not `pub`. This behavior is [documented] (but + // still surprising IMO). + // + // [documented]: https://docs.near.org/sdk/rust/contract-interface/public-methods#exposing-trait-implementations + #[near_bindgen] + impl AccessControllable for #ident { + fn acl_storage_prefix(&self) -> &[u8] { + (#storage_prefix).as_bytes() + } + + fn acl_add_admin_unchecked(&mut self, role: String, account_id: ::near_sdk::AccountId) -> bool { + false // TODO + } + + fn acl_grant_role_unchecked(&mut self, role: String, account_id: ::near_sdk::AccountId) -> bool { + let role = <#role_type>::from_str(&role).expect(#ERR_PARSE_ROLE); + self.#acl_field.grant_role_unchecked(role, &account_id) + } + + fn acl_has_role(&self, role: String, account_id: ::near_sdk::AccountId) -> bool { + let role = <#role_type>::from_str(&role).expect(#ERR_PARSE_ROLE); + self.#acl_field.has_role(role, &account_id) + } + } + }; + + output.into() +} + +fn inject_acl_field( + item: &mut ItemStruct, + field_name: &syn::Ident, + acl_type: &syn::Ident, +) -> Result<(), syn::Error> { + let fields = match item.fields { + syn::Fields::Named(ref mut fields) => fields, + _ => { + return Err(syn::Error::new( + item.ident.span(), + "Expected to have named fields", + )) + } + }; + + fields.named.push(syn::Field::parse_named.parse2(quote! { + #field_name: #acl_type + })?); + Ok(()) +} diff --git a/near-plugins-derive/src/lib.rs b/near-plugins-derive/src/lib.rs index e04b17b..6e86e0d 100644 --- a/near-plugins-derive/src/lib.rs +++ b/near-plugins-derive/src/lib.rs @@ -1,5 +1,7 @@ use proc_macro::{self, TokenStream}; +mod access_control_role; +mod access_controllable; mod full_access_key_fallback; mod ownable; mod pausable; @@ -40,3 +42,13 @@ pub fn pause(attrs: TokenStream, item: TokenStream) -> TokenStream { pub fn if_paused(attrs: TokenStream, item: TokenStream) -> TokenStream { pausable::if_paused(attrs, item) } + +#[proc_macro_derive(AccessControlRole)] +pub fn derive_access_control_role(input: TokenStream) -> TokenStream { + access_control_role::derive_access_control_role(input) +} + +#[proc_macro_attribute] +pub fn access_control(attrs: TokenStream, item: TokenStream) -> TokenStream { + access_controllable::access_controllable(attrs, item) +} diff --git a/near-plugins/Cargo.toml b/near-plugins/Cargo.toml index 1e6e99e..90e9a6c 100644 --- a/near-plugins/Cargo.toml +++ b/near-plugins/Cargo.toml @@ -22,4 +22,8 @@ near-plugins-derive = { path = "../near-plugins-derive" } serde = "1" [dev-dependencies] +anyhow = "1.0" borsh = "0.9" +tokio = { version = "1", features = ["full"] } +# Feature `unstable` is required for compiling contracts during tests. +workspaces = { version = "0.5", features = ["unstable"] } diff --git a/near-plugins/src/access_control_role.rs b/near-plugins/src/access_control_role.rs new file mode 100644 index 0000000..5699210 --- /dev/null +++ b/near-plugins/src/access_control_role.rs @@ -0,0 +1,10 @@ +pub trait AccessControlRole { + /// Returns the bitflag corresponding to the super admin permission. + fn acl_super_admin_permission_bitflag() -> u128; + + /// Returns the bitflag corresponding to the admin permission for the role. + fn acl_admin_permission_bitflag(self) -> u128; + + /// Returns the bitflag corresponding to the role's permission. + fn acl_permission_bitflag(self) -> u128; +} diff --git a/near-plugins/src/access_controllable.rs b/near-plugins/src/access_controllable.rs new file mode 100644 index 0000000..10980f1 --- /dev/null +++ b/near-plugins/src/access_controllable.rs @@ -0,0 +1,33 @@ +use near_sdk::AccountId; + +/// # Representation of roles +/// +/// This trait is unaware of the concrete type used to represent roles. It is +/// not possible to use a generic type `R` since `near-sdk` [does not support] +/// `impl` type parameters. +/// +/// ``` +/// // This is not possible: +/// impl AccessControllable for Contract {/* ... */} +/// ``` +/// +/// Instead, roles are represented by `u8`, which allows contract developers to +/// define their own enum whose variants are converted to `u8`. +/// +/// [does not support]: https://github.com/near/near-sdk-rs/blob/9d99077c6acfde68c06845f2a1eb2b5ed7983401/near-sdk/compilation_tests/impl_generic.stderr +pub trait AccessControllable { + fn acl_storage_prefix(&self) -> &[u8]; + + /// Grants admin permissions for `role` to `account_id`, __without__ + /// checking permissions of the predecessor. + /// + /// Returns whether `account_id` was newly added to the admins for `role`. + fn acl_add_admin_unchecked(&mut self, role: String, account_id: AccountId) -> bool; + + /// Grants `role` to `account_id` __without__ checking any permissions. + /// Returns whether `role` was newly granted to `account_id`. + fn acl_grant_role_unchecked(&mut self, role: String, account_id: AccountId) -> bool; + + /// Returns whether `account_id` has been granted `role`. + fn acl_has_role(&self, role: String, account_id: AccountId) -> bool; +} diff --git a/near-plugins/src/lib.rs b/near-plugins/src/lib.rs index e2d50bb..9d1cb53 100644 --- a/near-plugins/src/lib.rs +++ b/near-plugins/src/lib.rs @@ -1,3 +1,5 @@ +pub mod access_control_role; +pub mod access_controllable; pub mod events; pub mod full_access_key_fallback; pub mod ownable; @@ -6,9 +8,12 @@ pub mod pausable; mod test_utils; pub mod upgradable; +pub use access_control_role::AccessControlRole; +pub use access_controllable::AccessControllable; pub use full_access_key_fallback::FullAccessKeyFallback; pub use near_plugins_derive::{ - if_paused, only, pause, FullAccessKeyFallback, Ownable, Pausable, Upgradable, + access_control, if_paused, only, pause, AccessControlRole, FullAccessKeyFallback, Ownable, + Pausable, Upgradable, }; pub use ownable::Ownable; pub use pausable::Pausable; diff --git a/near-plugins/tests/access_controllable.rs b/near-plugins/tests/access_controllable.rs new file mode 100644 index 0000000..07fdf6e --- /dev/null +++ b/near-plugins/tests/access_controllable.rs @@ -0,0 +1,78 @@ +use near_sdk::serde_json::json; + +/// Smoke test of contract setup and basic functionality. +#[tokio::test] +async fn test_set_and_get_status() -> anyhow::Result<()> { + let worker = workspaces::sandbox().await?; + let wasm = workspaces::compile_project("./tests/contracts/access_controllable").await?; + let contract = worker.dev_deploy(&wasm).await?; + + let account = worker.dev_create_account().await?; + let message = "hello world"; + + account + .call(contract.id(), "set_status") + .args_json(json!({ + "message": message, + })) + .max_gas() + .transact() + .await? + .into_result()?; + + let res: String = account + .call(contract.id(), "get_status") + .args_json(json!({ + "account_id": account.id(), + })) + .view() + .await? + .json()?; + + assert_eq!(res, message); + Ok(()) +} + +#[tokio::test] +async fn test_acl_has_role() -> anyhow::Result<()> { + let worker = workspaces::sandbox().await?; + let wasm = workspaces::compile_project("./tests/contracts/access_controllable").await?; + let contract = worker.dev_deploy(&wasm).await?; + + let account = worker.dev_create_account().await?; + + // TODO add helper functions to execute frequent transactions + + let res = account + .call(contract.id(), "acl_has_role") + .args_json(json!({ + "role": "Level1", + "account_id": account.id(), + })) + .view() + .await?; + assert_eq!(res.json::()?, false); + + contract + .call("acl_grant_role_unchecked") + .args_json(json!({ + "role": "Level1", + "account_id": account.id(), + })) + .max_gas() + .transact() + .await? + .into_result()?; + + let res = account + .call(contract.id(), "acl_has_role") + .args_json(json!({ + "role": "Level1", + "account_id": account.id(), + })) + .view() + .await?; + assert_eq!(res.json::()?, true); + + Ok(()) +} diff --git a/near-plugins/tests/contracts/README.md b/near-plugins/tests/contracts/README.md new file mode 100644 index 0000000..dc7b888 --- /dev/null +++ b/near-plugins/tests/contracts/README.md @@ -0,0 +1,3 @@ +Contains contracts that use the plugins provided by `near-plugins`. + +These contracts are compiled during tests via Near's `workspaces-rs` and may serve as examples for smart contract developers. diff --git a/near-plugins/tests/contracts/access_controllable/Cargo.toml b/near-plugins/tests/contracts/access_controllable/Cargo.toml new file mode 100644 index 0000000..00f4856 --- /dev/null +++ b/near-plugins/tests/contracts/access_controllable/Cargo.toml @@ -0,0 +1,22 @@ +[package] +name = "access_controllable" +version = "0.0.0" +edition = "2018" + +[lib] +crate-type = ["cdylib", "rlib"] + +[dependencies] +bitflags = "1.3" +near-plugins = { path = "../../../../near-plugins" } +near-sdk = "4.0.0-pre.5" + +[profile.release] +codegen-units = 1 +opt-level = "z" +lto = true +debug = false +panic = "abort" +overflow-checks = true + +[workspace] diff --git a/near-plugins/tests/contracts/access_controllable/Makefile b/near-plugins/tests/contracts/access_controllable/Makefile new file mode 100644 index 0000000..6fb2950 --- /dev/null +++ b/near-plugins/tests/contracts/access_controllable/Makefile @@ -0,0 +1,6 @@ +build: + cargo build --target wasm32-unknown-unknown --release + +# Helpful for debugging. Requires `cargo-expand`. +expand: + cargo expand > expanded.rs diff --git a/near-plugins/tests/contracts/access_controllable/src/lib.rs b/near-plugins/tests/contracts/access_controllable/src/lib.rs new file mode 100644 index 0000000..52a1fb9 --- /dev/null +++ b/near-plugins/tests/contracts/access_controllable/src/lib.rs @@ -0,0 +1,43 @@ +use near_plugins::{access_control, AccessControlRole, AccessControllable}; +use near_sdk::borsh::{self, BorshDeserialize, BorshSerialize}; +use near_sdk::{env, log, near_bindgen, AccountId}; +use std::collections::HashMap; +use std::str::FromStr; + +#[derive(AccessControlRole, BorshSerialize, BorshDeserialize)] +pub enum Role { + Level1, + Level2, + Level3, +} + +#[access_control(storage_prefix = "__foo", role_type = "Role")] +#[near_bindgen] +#[derive(Default, BorshDeserialize, BorshSerialize)] +pub struct StatusMessage { + records: HashMap, +} + +#[near_bindgen] +impl StatusMessage { + #[payable] + pub fn set_status(&mut self, message: String) { + let account_id = env::signer_account_id(); + log!("{} set_status with message {}", account_id, message); + self.records.insert(account_id, message); + } + + pub fn get_status(&self, account_id: AccountId) -> Option { + log!("get_status for account_id {}", account_id); + self.records.get(&account_id).cloned() + } + + // The contract can interact with Acl by: + // + // a) Calling functions, e.g. + // self.acl.has_role(role, account_id) + // + // b) Using attributes (not yet implemented), e.g. + // #[acl_any(Role::Level1, Role::Level2)] + // pub fn foo(&mut self) {} +} From 40be1d304e47dbcdb95e63e5c2223347c9dcb4d2 Mon Sep 17 00:00:00 2001 From: Moritz Date: Fri, 16 Sep 2022 16:49:15 +0200 Subject: [PATCH 02/27] make trait method acl_storage_prefix static --- near-plugins-derive/src/access_controllable.rs | 8 +++----- near-plugins/src/access_controllable.rs | 3 ++- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/near-plugins-derive/src/access_controllable.rs b/near-plugins-derive/src/access_controllable.rs index 682b46e..c728bf9 100644 --- a/near-plugins-derive/src/access_controllable.rs +++ b/near-plugins-derive/src/access_controllable.rs @@ -70,8 +70,7 @@ pub fn access_controllable(attrs: TokenStream, item: TokenStream) -> TokenStream impl Default for #acl_type { fn default() -> Self { - // TODO make AccessControllable::acl_storage_prefix static, then use here - let base_prefix = (#storage_prefix).as_bytes(); + let base_prefix = <#ident as AccessControllable>::acl_storage_prefix(); Self { permissions: ::near_sdk::collections::UnorderedMap::new( __acl_storage_prefix(base_prefix, __AclStorageKey::Permissions), @@ -103,8 +102,7 @@ pub fn access_controllable(attrs: TokenStream, item: TokenStream) -> TokenStream impl #acl_type { fn new_bearers_set(permission: #permissions_ident) -> ::near_sdk::collections::UnorderedSet<::near_sdk::AccountId> { - // TODO make AccessControllable::acl_storage_prefix static, then use here - let base_prefix = (#storage_prefix).as_bytes(); + let base_prefix = <#ident as AccessControllable>::acl_storage_prefix(); let specifier = __AclStorageKey::BearersSet { permission }; ::near_sdk::collections::UnorderedSet::new(__acl_storage_prefix(base_prefix, specifier)) } @@ -163,7 +161,7 @@ pub fn access_controllable(attrs: TokenStream, item: TokenStream) -> TokenStream // [documented]: https://docs.near.org/sdk/rust/contract-interface/public-methods#exposing-trait-implementations #[near_bindgen] impl AccessControllable for #ident { - fn acl_storage_prefix(&self) -> &[u8] { + fn acl_storage_prefix() -> &'static [u8] { (#storage_prefix).as_bytes() } diff --git a/near-plugins/src/access_controllable.rs b/near-plugins/src/access_controllable.rs index 10980f1..bc0eb1c 100644 --- a/near-plugins/src/access_controllable.rs +++ b/near-plugins/src/access_controllable.rs @@ -16,7 +16,8 @@ use near_sdk::AccountId; /// /// [does not support]: https://github.com/near/near-sdk-rs/blob/9d99077c6acfde68c06845f2a1eb2b5ed7983401/near-sdk/compilation_tests/impl_generic.stderr pub trait AccessControllable { - fn acl_storage_prefix(&self) -> &[u8]; + /// Returns the storage prefix for collections related to access control. + fn acl_storage_prefix() -> &'static [u8]; /// Grants admin permissions for `role` to `account_id`, __without__ /// checking permissions of the predecessor. From 7e66d31e9663cf7c932b11127c7fc48f2658204f Mon Sep 17 00:00:00 2001 From: Moritz Date: Fri, 16 Sep 2022 17:06:31 +0200 Subject: [PATCH 03/27] implement TryFrom<&str> instead of FromStr --- near-plugins-derive/src/access_control_role.rs | 7 +++---- near-plugins-derive/src/access_controllable.rs | 4 ++-- .../tests/contracts/access_controllable/src/lib.rs | 2 +- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/near-plugins-derive/src/access_control_role.rs b/near-plugins-derive/src/access_control_role.rs index 81f2ce7..0cc25a2 100644 --- a/near-plugins-derive/src/access_control_role.rs +++ b/near-plugins-derive/src/access_control_role.rs @@ -57,11 +57,10 @@ pub fn derive_access_control_role(input: TokenStream) -> TokenStream { } } - // TODO use `TryFrom<&str>` (for consistency) - impl ::std::str::FromStr for #ident { - type Err = &'static str; + impl ::std::convert::TryFrom<&str> for #ident { + type Error = &'static str; - fn from_str(value: &str) -> Result<#ident, Self::Err> { + fn try_from(value: &str) -> Result<#ident, Self::Error> { match value { #( #variant_names => Ok(#ident::#variant_items), diff --git a/near-plugins-derive/src/access_controllable.rs b/near-plugins-derive/src/access_controllable.rs index c728bf9..3d57564 100644 --- a/near-plugins-derive/src/access_controllable.rs +++ b/near-plugins-derive/src/access_controllable.rs @@ -170,12 +170,12 @@ pub fn access_controllable(attrs: TokenStream, item: TokenStream) -> TokenStream } fn acl_grant_role_unchecked(&mut self, role: String, account_id: ::near_sdk::AccountId) -> bool { - let role = <#role_type>::from_str(&role).expect(#ERR_PARSE_ROLE); + let role = <#role_type>::try_from(role.as_str()).expect(#ERR_PARSE_ROLE); self.#acl_field.grant_role_unchecked(role, &account_id) } fn acl_has_role(&self, role: String, account_id: ::near_sdk::AccountId) -> bool { - let role = <#role_type>::from_str(&role).expect(#ERR_PARSE_ROLE); + let role = <#role_type>::try_from(role.as_str()).expect(#ERR_PARSE_ROLE); self.#acl_field.has_role(role, &account_id) } } diff --git a/near-plugins/tests/contracts/access_controllable/src/lib.rs b/near-plugins/tests/contracts/access_controllable/src/lib.rs index 52a1fb9..3e1fa04 100644 --- a/near-plugins/tests/contracts/access_controllable/src/lib.rs +++ b/near-plugins/tests/contracts/access_controllable/src/lib.rs @@ -2,7 +2,7 @@ use near_plugins::{access_control, AccessControlRole, AccessControllable}; use near_sdk::borsh::{self, BorshDeserialize, BorshSerialize}; use near_sdk::{env, log, near_bindgen, AccountId}; use std::collections::HashMap; -use std::str::FromStr; +use std::convert::TryFrom; #[derive(AccessControlRole, BorshSerialize, BorshDeserialize)] pub enum Role { From 2793779f73629bf4489c7a1134774eb906df4369 Mon Sep 17 00:00:00 2001 From: Moritz Date: Mon, 19 Sep 2022 11:31:06 +0200 Subject: [PATCH 04/27] Remove `_bitflag` suffix from trait method names --- near-plugins-derive/src/access_control_role.rs | 6 +++--- near-plugins-derive/src/access_controllable.rs | 4 ++-- near-plugins/src/access_control_role.rs | 6 +++--- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/near-plugins-derive/src/access_control_role.rs b/near-plugins-derive/src/access_control_role.rs index 0cc25a2..7a00a2c 100644 --- a/near-plugins-derive/src/access_control_role.rs +++ b/near-plugins-derive/src/access_control_role.rs @@ -79,18 +79,18 @@ pub fn derive_access_control_role(input: TokenStream) -> TokenStream { // TODO explain enum<->bitflag conversion impl AccessControlRole for #ident { - fn acl_super_admin_permission_bitflag() -> u128 { + fn acl_super_admin_permission() -> u128 { safe_leftshift(1, 0) } - fn acl_permission_bitflag(self) -> u128 { + fn acl_permission(self) -> u128 { let n = (u8::from(self) + 1) .checked_mul(2) .expect("Too many enum variants") - 1; safe_leftshift(1, n) } - fn acl_admin_permission_bitflag(self) -> u128 { + fn acl_admin_permission(self) -> u128 { let n = (u8::from(self) + 1) .checked_mul(2) .expect("Too many enum variants"); diff --git a/near-plugins-derive/src/access_controllable.rs b/near-plugins-derive/src/access_controllable.rs index 3d57564..6f6f1f2 100644 --- a/near-plugins-derive/src/access_controllable.rs +++ b/near-plugins-derive/src/access_controllable.rs @@ -115,7 +115,7 @@ pub fn access_controllable(attrs: TokenStream, item: TokenStream) -> TokenStream } fn grant_role_unchecked(&mut self, role: #role_type, account_id: &::near_sdk::AccountId) -> bool { - let flag = <#permissions_ident>::from_bits(role.acl_permission_bitflag()) + let flag = <#permissions_ident>::from_bits(role.acl_permission()) .expect(#ERR_PARSE_BITFLAG); let mut permissions = self.get_or_init_permissions(account_id); @@ -133,7 +133,7 @@ pub fn access_controllable(attrs: TokenStream, item: TokenStream) -> TokenStream fn has_role(&self, role: #role_type, account_id: &::near_sdk::AccountId) -> bool { match self.permissions.get(account_id) { Some(permissions) => { - let flag = <#permissions_ident>::from_bits(role.acl_permission_bitflag()) + let flag = <#permissions_ident>::from_bits(role.acl_permission()) .expect(#ERR_PARSE_BITFLAG); permissions.contains(flag) } diff --git a/near-plugins/src/access_control_role.rs b/near-plugins/src/access_control_role.rs index 5699210..e049c07 100644 --- a/near-plugins/src/access_control_role.rs +++ b/near-plugins/src/access_control_role.rs @@ -1,10 +1,10 @@ pub trait AccessControlRole { /// Returns the bitflag corresponding to the super admin permission. - fn acl_super_admin_permission_bitflag() -> u128; + fn acl_super_admin_permission() -> u128; /// Returns the bitflag corresponding to the admin permission for the role. - fn acl_admin_permission_bitflag(self) -> u128; + fn acl_admin_permission(self) -> u128; /// Returns the bitflag corresponding to the role's permission. - fn acl_permission_bitflag(self) -> u128; + fn acl_permission(self) -> u128; } From e8dfbac10bb451f09d02f4b6710cdd3eb1078ebe Mon Sep 17 00:00:00 2001 From: Moritz Date: Mon, 19 Sep 2022 11:41:56 +0200 Subject: [PATCH 05/27] Omit optional storage_prefix arg in test contract --- near-plugins/tests/contracts/README.md | 5 +++++ near-plugins/tests/contracts/access_controllable/src/lib.rs | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/near-plugins/tests/contracts/README.md b/near-plugins/tests/contracts/README.md index dc7b888..3721a53 100644 --- a/near-plugins/tests/contracts/README.md +++ b/near-plugins/tests/contracts/README.md @@ -1,3 +1,8 @@ Contains contracts that use the plugins provided by `near-plugins`. These contracts are compiled during tests via Near's `workspaces-rs` and may serve as examples for smart contract developers. + +# TODO: contract to test optional ACL arguments +- `#[access_control]` has optional arguments, e.g. `storage_prefix`. +- Add a contract which sets all those optional arguments. +- Purpose: docs/example + verify processing of the arguments diff --git a/near-plugins/tests/contracts/access_controllable/src/lib.rs b/near-plugins/tests/contracts/access_controllable/src/lib.rs index 3e1fa04..3053778 100644 --- a/near-plugins/tests/contracts/access_controllable/src/lib.rs +++ b/near-plugins/tests/contracts/access_controllable/src/lib.rs @@ -11,7 +11,7 @@ pub enum Role { Level3, } -#[access_control(storage_prefix = "__foo", role_type = "Role")] +#[access_control(role_type = "Role")] #[near_bindgen] #[derive(Default, BorshDeserialize, BorshSerialize)] pub struct StatusMessage { From aca932f326d6e11d480d781885bcdd149fe35cc3 Mon Sep 17 00:00:00 2001 From: Moritz Date: Mon, 19 Sep 2022 17:52:29 +0200 Subject: [PATCH 06/27] Generate bitflags dynamically --- .../src/access_control_role.rs | 43 +++++++++++++++---- 1 file changed, 34 insertions(+), 9 deletions(-) diff --git a/near-plugins-derive/src/access_control_role.rs b/near-plugins-derive/src/access_control_role.rs index 7a00a2c..820b726 100644 --- a/near-plugins-derive/src/access_control_role.rs +++ b/near-plugins-derive/src/access_control_role.rs @@ -1,8 +1,12 @@ use proc_macro::TokenStream; +use proc_macro2::{Ident, Span}; use quote::quote; use std::convert::TryFrom; use syn::{parse_macro_input, ItemEnum}; +const DEFAULT_SUPER_ADMIN_NAME: &str = "__SUPER_ADMIN"; +const DEFAULT_BITFLAGS_TYPE_NAME: &str = "RoleFlags"; + pub fn derive_access_control_role(input: TokenStream) -> TokenStream { // This derive doesn't take attributes, so no need to use `darling`. let input: ItemEnum = parse_macro_input!(input); @@ -10,6 +14,7 @@ pub fn derive_access_control_role(input: TokenStream) -> TokenStream { ident, variants, .. } = input; + // TODO cleanup by using range (see bitflags_idxs below) let (variant_idxs, variant_items): (Vec<_>, Vec<_>) = variants.iter().cloned().enumerate().unzip(); let variant_idxs = variant_idxs @@ -23,6 +28,11 @@ pub fn derive_access_control_role(input: TokenStream) -> TokenStream { .map(|v| format!("{}", v.ident)) .collect::>(); + let bitflags_type_ident = Ident::new(DEFAULT_BITFLAGS_TYPE_NAME, Span::call_site()); + let bitflags_idents = bitflags_idents(variant_names.as_ref(), bitflags_type_ident.span()); + let bitflags_idxs = 0..u8::try_from(bitflags_idents.len()) + .expect("The number of bitflags should be representable by u8"); + let output = quote! { impl From<#ident> for u8 { fn from(value: #ident) -> Self { @@ -99,19 +109,34 @@ pub fn derive_access_control_role(input: TokenStream) -> TokenStream { } ::bitflags::bitflags! { - // TODO generate dynamically #[derive(BorshDeserialize, BorshSerialize, Default)] - struct RoleFlags: u128 { - const __SUPER_ADMIN = 1u128 << 0; - const LEVEL1 = 1u128 << 1; - const LEVEL1_ADMIN = 1u128 << 2; - const LEVEL2 = 1u128 << 3; - const LEVEL2_ADMIN = 1u128 << 4; - const LEVEL3 = 1u128 << 5; - const LEVEL3_ADMIN = 1u128 << 6; + struct #bitflags_type_ident: u128 { + #( + const #bitflags_idents = 1u128 << #bitflags_idxs; + )* } } }; output.into() } + +fn bitflags_idents(names: &[String], span: Span) -> Vec { + // Assuming enum variant names are in camel case, simply converting them + // to uppercase is not ideal. However, bitflag identifiers aren't exposed, + // so let's not bother with converting camel to screaming-snake case. + let names = names + .iter() + .map(|name| name.to_uppercase()) + .collect::>(); + let admin_names = names + .iter() + .map(|name| format!("{}_ADMIN", name)) + .collect::>(); + let mut idents = vec![Ident::new(DEFAULT_SUPER_ADMIN_NAME, span.clone())]; + for (name, admin_name) in names.iter().zip(admin_names) { + idents.push(Ident::new(name.as_ref(), span.clone())); + idents.push(Ident::new(admin_name.as_ref(), span.clone())); + } + idents +} From d2352c7430465f0cc3424e53cc7c067d2f380b6a Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 20 Sep 2022 11:26:13 +0200 Subject: [PATCH 07/27] document mapping between roles and bitflags --- .../src/access_control_role.rs | 46 ++++++++++++++++++- 1 file changed, 45 insertions(+), 1 deletion(-) diff --git a/near-plugins-derive/src/access_control_role.rs b/near-plugins-derive/src/access_control_role.rs index 820b726..8756b5b 100644 --- a/near-plugins-derive/src/access_control_role.rs +++ b/near-plugins-derive/src/access_control_role.rs @@ -1,3 +1,44 @@ +//! Implements `AccessControlRole` for an enum. +//! +//! The conversion of enum variants to bitflags representable by `u128` is the +//! key part of this implementation. Assume the trait is derived on the +//! following enum: +//! +//! ``` +//! #[derive(AccessControlRole)] +//! pub enum Role { +//! LevelA, +//! LevelB, +//! LevelC, +//! } +//! ``` +//! +//! This results in the following bitflags: +//! ``` +//! bitflags! { +//! struct RoleFlags: u128 { +//! const __SUPER_ADMIN = 1u128 << 0; +//! const LEVELA = 1u128 << 1; +//! const LEVELA_ADMIN = 1u128 << 2; +//! const LEVELB = 1u128 << 3; +//! const LEVELB_ADMIN = 1u128 << 4; +//! const LEVELC = 1u128 << 5; +//! const LEVELC_ADMIN = 1u128 << 6; +//! } +//! } +//! ``` +//! +//! The mapping between enum variants and bitflag has these properties: +//! +//! - Each flag has exactly one bit with value 1. +//! - A bitflag `1u128 << x` with odd `x` represents a role permission. +//! - A bitflag `1u128 << x` with even `x` represents an admin permission. +//! - Shifting a role's 1-bit to the left by one position yields the +//! corresponding admin permission. +//! +//! The last property aims to facilitate migrations which add or remove enum +//! variants. + use proc_macro::TokenStream; use proc_macro2::{Ident, Span}; use quote::quote; @@ -87,13 +128,14 @@ pub fn derive_access_control_role(input: TokenStream) -> TokenStream { .expect("Too many enum variants to be represented by bitflags") } - // TODO explain enum<->bitflag conversion impl AccessControlRole for #ident { fn acl_super_admin_permission() -> u128 { + // See module documentation. safe_leftshift(1, 0) } fn acl_permission(self) -> u128 { + // Shift 1u128 left by an odd number, see module documentation. let n = (u8::from(self) + 1) .checked_mul(2) .expect("Too many enum variants") - 1; @@ -101,6 +143,7 @@ pub fn derive_access_control_role(input: TokenStream) -> TokenStream { } fn acl_admin_permission(self) -> u128 { + // Shift 1u128 left by an even number, see module documentation. let n = (u8::from(self) + 1) .checked_mul(2) .expect("Too many enum variants"); @@ -109,6 +152,7 @@ pub fn derive_access_control_role(input: TokenStream) -> TokenStream { } ::bitflags::bitflags! { + /// Encodes permissions for roles and admins. #[derive(BorshDeserialize, BorshSerialize, Default)] struct #bitflags_type_ident: u128 { #( From 29adf81f0f860bf147192c9e1702c0b1e273f0f5 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 20 Sep 2022 11:53:28 +0200 Subject: [PATCH 08/27] cleanup --- .../src/access_control_role.rs | 29 +++++++------------ 1 file changed, 10 insertions(+), 19 deletions(-) diff --git a/near-plugins-derive/src/access_control_role.rs b/near-plugins-derive/src/access_control_role.rs index 8756b5b..4ebead8 100644 --- a/near-plugins-derive/src/access_control_role.rs +++ b/near-plugins-derive/src/access_control_role.rs @@ -55,31 +55,22 @@ pub fn derive_access_control_role(input: TokenStream) -> TokenStream { ident, variants, .. } = input; - // TODO cleanup by using range (see bitflags_idxs below) - let (variant_idxs, variant_items): (Vec<_>, Vec<_>) = - variants.iter().cloned().enumerate().unzip(); - let variant_idxs = variant_idxs - .iter() - .map(|&idx| { - u8::try_from(idx).expect("The number of variants should be representable by u8") - }) - .collect::>(); - let variant_names = variants - .iter() - .map(|v| format!("{}", v.ident)) - .collect::>(); + let variants = variants.iter().collect::>(); + let variant_idxs: Vec<_> = + (0..u8::try_from(variants.len()).expect("Too many enum variants")).collect(); + let variant_names: Vec<_> = variants.iter().map(|v| format!("{}", v.ident)).collect(); let bitflags_type_ident = Ident::new(DEFAULT_BITFLAGS_TYPE_NAME, Span::call_site()); let bitflags_idents = bitflags_idents(variant_names.as_ref(), bitflags_type_ident.span()); - let bitflags_idxs = 0..u8::try_from(bitflags_idents.len()) - .expect("The number of bitflags should be representable by u8"); + let bitflags_idxs: Vec<_> = + (0..u8::try_from(bitflags_idents.len()).expect("Too many bitflags")).collect(); let output = quote! { impl From<#ident> for u8 { fn from(value: #ident) -> Self { match value { #( - #ident::#variant_items => #variant_idxs, + #ident::#variants => #variant_idxs, )* } } @@ -91,7 +82,7 @@ pub fn derive_access_control_role(input: TokenStream) -> TokenStream { fn try_from(value: u8) -> Result { match value { #( - #variant_idxs => Ok(#ident::#variant_items), + #variant_idxs => Ok(#ident::#variants), )* _ => Err("Value does not correspond to a variant"), } @@ -102,7 +93,7 @@ pub fn derive_access_control_role(input: TokenStream) -> TokenStream { fn from(value: #ident) -> Self { match value { #( - #ident::#variant_items => #variant_names, + #ident::#variants => #variant_names, )* } } @@ -114,7 +105,7 @@ pub fn derive_access_control_role(input: TokenStream) -> TokenStream { fn try_from(value: &str) -> Result<#ident, Self::Error> { match value { #( - #variant_names => Ok(#ident::#variant_items), + #variant_names => Ok(#ident::#variants), )* _ => Err("Value does not correspond to a variant"), } From f0032c5a012be7c8192e96b4f981d6c57e057ead Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 20 Sep 2022 12:01:20 +0200 Subject: [PATCH 09/27] Rename variants of Role enum Avoid confusion of numbers in role names with indexes of variants. --- near-plugins/tests/access_controllable.rs | 6 +++--- .../tests/contracts/access_controllable/src/lib.rs | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/near-plugins/tests/access_controllable.rs b/near-plugins/tests/access_controllable.rs index 07fdf6e..a654789 100644 --- a/near-plugins/tests/access_controllable.rs +++ b/near-plugins/tests/access_controllable.rs @@ -46,7 +46,7 @@ async fn test_acl_has_role() -> anyhow::Result<()> { let res = account .call(contract.id(), "acl_has_role") .args_json(json!({ - "role": "Level1", + "role": "LevelA", "account_id": account.id(), })) .view() @@ -56,7 +56,7 @@ async fn test_acl_has_role() -> anyhow::Result<()> { contract .call("acl_grant_role_unchecked") .args_json(json!({ - "role": "Level1", + "role": "LevelA", "account_id": account.id(), })) .max_gas() @@ -67,7 +67,7 @@ async fn test_acl_has_role() -> anyhow::Result<()> { let res = account .call(contract.id(), "acl_has_role") .args_json(json!({ - "role": "Level1", + "role": "LevelA", "account_id": account.id(), })) .view() diff --git a/near-plugins/tests/contracts/access_controllable/src/lib.rs b/near-plugins/tests/contracts/access_controllable/src/lib.rs index 3053778..821749d 100644 --- a/near-plugins/tests/contracts/access_controllable/src/lib.rs +++ b/near-plugins/tests/contracts/access_controllable/src/lib.rs @@ -6,9 +6,9 @@ use std::convert::TryFrom; #[derive(AccessControlRole, BorshSerialize, BorshDeserialize)] pub enum Role { - Level1, - Level2, - Level3, + LevelA, + LevelB, + LevelC, } #[access_control(role_type = "Role")] @@ -38,6 +38,6 @@ impl StatusMessage { // self.acl.has_role(role, account_id) // // b) Using attributes (not yet implemented), e.g. - // #[acl_any(Role::Level1, Role::Level2)] + // #[acl_any(Role::LevelA, Role::LevelB)] // pub fn foo(&mut self) {} } From 2100b14cb0b70c3c6b964e8f41c1b6c27cbf7b60 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 20 Sep 2022 12:55:17 +0200 Subject: [PATCH 10/27] cleanup --- near-plugins-derive/src/access_controllable.rs | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/near-plugins-derive/src/access_controllable.rs b/near-plugins-derive/src/access_controllable.rs index 6f6f1f2..b2bc43a 100644 --- a/near-plugins-derive/src/access_controllable.rs +++ b/near-plugins-derive/src/access_controllable.rs @@ -13,10 +13,8 @@ pub struct MacroArgs { } const DEFAULT_STORAGE_PREFIX: &str = "__acl"; - -/// TODO allow customization via macro arguments -const ACL_FIELD_NAME: &str = "__acl"; -const ACL_TYPE_NAME: &str = "__Acl"; +const DEFAULT_ACL_FIELD_NAME: &str = "__acl"; +const DEFAULT_ACL_TYPE_NAME: &str = "__Acl"; const ERR_PARSE_BITFLAG: &str = "Value does not correspond to a permission"; const ERR_PARSE_ROLE: &str = "Value does not correspond to a role"; @@ -24,14 +22,8 @@ const ERR_PARSE_ROLE: &str = "Value does not correspond to a role"; pub fn access_controllable(attrs: TokenStream, item: TokenStream) -> TokenStream { let attr_args = parse_macro_input!(attrs as AttributeArgs); let mut input: ItemStruct = parse_macro_input!(item); - // TODO seems like this is required for ItemFn but not for ItemStruct? - /* - if is_near_bindgen_wrapped_or_marshall(&input) { - return item; - } - */ - let acl_field = syn::Ident::new(ACL_FIELD_NAME, Span::call_site()); - let acl_type = syn::Ident::new(ACL_TYPE_NAME, Span::call_site()); + let acl_field = syn::Ident::new(DEFAULT_ACL_FIELD_NAME, Span::call_site()); + let acl_type = syn::Ident::new(DEFAULT_ACL_TYPE_NAME, Span::call_site()); // TODO determine name of ident dynamically let permissions_ident = syn::Ident::new("RoleFlags", Span::call_site()); if let Err(e) = inject_acl_field(&mut input, &acl_field, &acl_type) { From 41fa72578000c4f108556b80f69c3c30d9a6377e Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 20 Sep 2022 16:31:53 +0200 Subject: [PATCH 11/27] Add `fn new_bitflags_type_ident` --- .../src/access_control_role.rs | 6 ++++- .../src/access_controllable.rs | 22 +++++++++---------- 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/near-plugins-derive/src/access_control_role.rs b/near-plugins-derive/src/access_control_role.rs index 4ebead8..6e40d6b 100644 --- a/near-plugins-derive/src/access_control_role.rs +++ b/near-plugins-derive/src/access_control_role.rs @@ -60,7 +60,7 @@ pub fn derive_access_control_role(input: TokenStream) -> TokenStream { (0..u8::try_from(variants.len()).expect("Too many enum variants")).collect(); let variant_names: Vec<_> = variants.iter().map(|v| format!("{}", v.ident)).collect(); - let bitflags_type_ident = Ident::new(DEFAULT_BITFLAGS_TYPE_NAME, Span::call_site()); + let bitflags_type_ident = new_bitflags_type_ident(Span::call_site()); let bitflags_idents = bitflags_idents(variant_names.as_ref(), bitflags_type_ident.span()); let bitflags_idxs: Vec<_> = (0..u8::try_from(bitflags_idents.len()).expect("Too many bitflags")).collect(); @@ -156,6 +156,10 @@ pub fn derive_access_control_role(input: TokenStream) -> TokenStream { output.into() } +pub fn new_bitflags_type_ident(span: Span) -> Ident { + Ident::new(DEFAULT_BITFLAGS_TYPE_NAME, span) +} + fn bitflags_idents(names: &[String], span: Span) -> Vec { // Assuming enum variant names are in camel case, simply converting them // to uppercase is not ideal. However, bitflag identifiers aren't exposed, diff --git a/near-plugins-derive/src/access_controllable.rs b/near-plugins-derive/src/access_controllable.rs index b2bc43a..e1e5aa1 100644 --- a/near-plugins-derive/src/access_controllable.rs +++ b/near-plugins-derive/src/access_controllable.rs @@ -1,3 +1,4 @@ +use crate::access_control_role::new_bitflags_type_ident; use darling::FromMeta; use proc_macro::TokenStream; use proc_macro2::Span; @@ -24,8 +25,7 @@ pub fn access_controllable(attrs: TokenStream, item: TokenStream) -> TokenStream let mut input: ItemStruct = parse_macro_input!(item); let acl_field = syn::Ident::new(DEFAULT_ACL_FIELD_NAME, Span::call_site()); let acl_type = syn::Ident::new(DEFAULT_ACL_TYPE_NAME, Span::call_site()); - // TODO determine name of ident dynamically - let permissions_ident = syn::Ident::new("RoleFlags", Span::call_site()); + let bitflags_type = new_bitflags_type_ident(Span::call_site()); if let Err(e) = inject_acl_field(&mut input, &acl_field, &acl_type) { return TokenStream::from(e.to_compile_error()); } @@ -51,11 +51,11 @@ pub fn access_controllable(attrs: TokenStream, item: TokenStream) -> TokenStream /// Stores permissions per account. permissions: ::near_sdk::collections::UnorderedMap< ::near_sdk::AccountId, - #permissions_ident, + #bitflags_type, >, /// Stores the set of accounts that bear a permission. bearers: ::near_sdk::collections::UnorderedMap< - #permissions_ident, + #bitflags_type, ::near_sdk::collections::UnorderedSet<::near_sdk::AccountId>, >, } @@ -81,7 +81,7 @@ pub fn access_controllable(attrs: TokenStream, item: TokenStream) -> TokenStream enum __AclStorageKey { Permissions, Bearers, - BearersSet { permission: #permissions_ident }, + BearersSet { permission: #bitflags_type }, } /// Generates a prefix by concatenating the input parameters. @@ -93,21 +93,21 @@ pub fn access_controllable(attrs: TokenStream, item: TokenStream) -> TokenStream } impl #acl_type { - fn new_bearers_set(permission: #permissions_ident) -> ::near_sdk::collections::UnorderedSet<::near_sdk::AccountId> { + fn new_bearers_set(permission: #bitflags_type) -> ::near_sdk::collections::UnorderedSet<::near_sdk::AccountId> { let base_prefix = <#ident as AccessControllable>::acl_storage_prefix(); let specifier = __AclStorageKey::BearersSet { permission }; ::near_sdk::collections::UnorderedSet::new(__acl_storage_prefix(base_prefix, specifier)) } - fn get_or_init_permissions(&self, account_id: &::near_sdk::AccountId) -> #permissions_ident { + fn get_or_init_permissions(&self, account_id: &::near_sdk::AccountId) -> #bitflags_type { match self.permissions.get(account_id) { Some(permissions) => permissions, - None => <#permissions_ident>::empty(), + None => <#bitflags_type>::empty(), } } fn grant_role_unchecked(&mut self, role: #role_type, account_id: &::near_sdk::AccountId) -> bool { - let flag = <#permissions_ident>::from_bits(role.acl_permission()) + let flag = <#bitflags_type>::from_bits(role.acl_permission()) .expect(#ERR_PARSE_BITFLAG); let mut permissions = self.get_or_init_permissions(account_id); @@ -125,7 +125,7 @@ pub fn access_controllable(attrs: TokenStream, item: TokenStream) -> TokenStream fn has_role(&self, role: #role_type, account_id: &::near_sdk::AccountId) -> bool { match self.permissions.get(account_id) { Some(permissions) => { - let flag = <#permissions_ident>::from_bits(role.acl_permission()) + let flag = <#bitflags_type>::from_bits(role.acl_permission()) .expect(#ERR_PARSE_BITFLAG); permissions.contains(flag) } @@ -134,7 +134,7 @@ pub fn access_controllable(attrs: TokenStream, item: TokenStream) -> TokenStream } /// Adds `account_id` to the set of `permission` bearers. - fn add_bearer(&mut self, permission: #permissions_ident, account_id: &::near_sdk::AccountId) { + fn add_bearer(&mut self, permission: #bitflags_type, account_id: &::near_sdk::AccountId) { let mut set = match self.bearers.get(&permission) { Some(set) => set, None => Self::new_bearers_set(permission), From 1802bd5daf5269662674566c814b4593f54c9da0 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 20 Sep 2022 17:28:17 +0200 Subject: [PATCH 12/27] Make some methods in trait impl #[private] --- near-plugins-derive/src/access_controllable.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/near-plugins-derive/src/access_controllable.rs b/near-plugins-derive/src/access_controllable.rs index e1e5aa1..d457367 100644 --- a/near-plugins-derive/src/access_controllable.rs +++ b/near-plugins-derive/src/access_controllable.rs @@ -145,10 +145,9 @@ pub fn access_controllable(attrs: TokenStream, item: TokenStream) -> TokenStream } } - // TODO control which functions are exposed externally - // `near_bindgen` externally exposes functions in trait implementations - // _even_ if they are not `pub`. This behavior is [documented] (but - // still surprising IMO). + // Note that `#[near-bindgen]` exposes non-public functions in trait + // implementations. This is [documented] behavior. Therefore some + // functions are made `#[private]` despite _not_ being public. // // [documented]: https://docs.near.org/sdk/rust/contract-interface/public-methods#exposing-trait-implementations #[near_bindgen] @@ -157,10 +156,12 @@ pub fn access_controllable(attrs: TokenStream, item: TokenStream) -> TokenStream (#storage_prefix).as_bytes() } + #[private] fn acl_add_admin_unchecked(&mut self, role: String, account_id: ::near_sdk::AccountId) -> bool { false // TODO } + #[private] fn acl_grant_role_unchecked(&mut self, role: String, account_id: ::near_sdk::AccountId) -> bool { let role = <#role_type>::try_from(role.as_str()).expect(#ERR_PARSE_ROLE); self.#acl_field.grant_role_unchecked(role, &account_id) From bdcc45fb67e3829bb31790ad217944ccf42676c0 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 20 Sep 2022 18:42:12 +0200 Subject: [PATCH 13/27] Add event RoleGranted --- .../src/access_control_role.rs | 10 ++++++ .../src/access_controllable.rs | 10 +++++- near-plugins/src/access_controllable.rs | 32 +++++++++++++++++++ .../contracts/access_controllable/src/lib.rs | 3 +- 4 files changed, 53 insertions(+), 2 deletions(-) diff --git a/near-plugins-derive/src/access_control_role.rs b/near-plugins-derive/src/access_control_role.rs index 6e40d6b..5d5b3f9 100644 --- a/near-plugins-derive/src/access_control_role.rs +++ b/near-plugins-derive/src/access_control_role.rs @@ -99,6 +99,16 @@ pub fn derive_access_control_role(input: TokenStream) -> TokenStream { } } + impl From<#ident> for String { + fn from(value: #ident) -> Self { + match value { + #( + #ident::#variants => #variant_names.to_string(), + )* + } + } + } + impl ::std::convert::TryFrom<&str> for #ident { type Error = &'static str; diff --git a/near-plugins-derive/src/access_controllable.rs b/near-plugins-derive/src/access_controllable.rs index d457367..4f4d56a 100644 --- a/near-plugins-derive/src/access_controllable.rs +++ b/near-plugins-derive/src/access_controllable.rs @@ -1,4 +1,5 @@ use crate::access_control_role::new_bitflags_type_ident; +use crate::utils::cratename; use darling::FromMeta; use proc_macro::TokenStream; use proc_macro2::Span; @@ -21,6 +22,7 @@ const ERR_PARSE_BITFLAG: &str = "Value does not correspond to a permission"; const ERR_PARSE_ROLE: &str = "Value does not correspond to a role"; pub fn access_controllable(attrs: TokenStream, item: TokenStream) -> TokenStream { + let cratename = cratename(); let attr_args = parse_macro_input!(attrs as AttributeArgs); let mut input: ItemStruct = parse_macro_input!(item); let acl_field = syn::Ident::new(DEFAULT_ACL_FIELD_NAME, Span::call_site()); @@ -116,7 +118,13 @@ pub fn access_controllable(attrs: TokenStream, item: TokenStream) -> TokenStream permissions.insert(flag); self.permissions.insert(account_id, &permissions); self.add_bearer(flag, account_id); - // TODO emit event + + let event = ::#cratename::access_controllable::events::RoleGranted { + role: role.into(), + by: ::near_sdk::env::predecessor_account_id(), + to: account_id.clone(), + }; + event.emit(); } is_new_grantee diff --git a/near-plugins/src/access_controllable.rs b/near-plugins/src/access_controllable.rs index bc0eb1c..48a2717 100644 --- a/near-plugins/src/access_controllable.rs +++ b/near-plugins/src/access_controllable.rs @@ -32,3 +32,35 @@ pub trait AccessControllable { /// Returns whether `account_id` has been granted `role`. fn acl_has_role(&self, role: String, account_id: AccountId) -> bool; } + +pub mod events { + use crate::events::{AsEvent, EventMetadata}; + use near_sdk::serde::Serialize; + use near_sdk::AccountId; + + const STANDARD: &str = "AccessControllable"; + const VERSION: &str = "1.0.0"; + + /// Event emitted when a role is granted to an account. + #[derive(Serialize, Clone)] + #[serde(crate = "near_sdk::serde")] + pub struct RoleGranted { + /// Role that was granted. + pub role: String, + /// Account that was granted the role. + pub to: AccountId, + /// Account that granted the role. + pub by: AccountId, + } + + impl AsEvent for RoleGranted { + fn metadata(&self) -> EventMetadata { + EventMetadata { + standard: STANDARD.to_string(), + version: VERSION.to_string(), + event: "role_granted".to_string(), + data: Some(self.clone()), + } + } + } +} diff --git a/near-plugins/tests/contracts/access_controllable/src/lib.rs b/near-plugins/tests/contracts/access_controllable/src/lib.rs index 821749d..7ea2112 100644 --- a/near-plugins/tests/contracts/access_controllable/src/lib.rs +++ b/near-plugins/tests/contracts/access_controllable/src/lib.rs @@ -1,10 +1,11 @@ +use near_plugins::events::AsEvent; use near_plugins::{access_control, AccessControlRole, AccessControllable}; use near_sdk::borsh::{self, BorshDeserialize, BorshSerialize}; use near_sdk::{env, log, near_bindgen, AccountId}; use std::collections::HashMap; use std::convert::TryFrom; -#[derive(AccessControlRole, BorshSerialize, BorshDeserialize)] +#[derive(AccessControlRole, BorshSerialize, BorshDeserialize, Copy, Clone)] pub enum Role { LevelA, LevelB, From b578c25660a0ba216d440836c535790b9766f823 Mon Sep 17 00:00:00 2001 From: Moritz Date: Wed, 21 Sep 2022 12:39:08 +0200 Subject: [PATCH 14/27] test: add helpers via AccessControllableContract --- near-plugins/tests/access_controllable.rs | 38 +++------- .../common/access_controllable_contract.rs | 72 +++++++++++++++++++ near-plugins/tests/common/mod.rs | 1 + 3 files changed, 84 insertions(+), 27 deletions(-) create mode 100644 near-plugins/tests/common/access_controllable_contract.rs create mode 100644 near-plugins/tests/common/mod.rs diff --git a/near-plugins/tests/access_controllable.rs b/near-plugins/tests/access_controllable.rs index a654789..5abf412 100644 --- a/near-plugins/tests/access_controllable.rs +++ b/near-plugins/tests/access_controllable.rs @@ -1,3 +1,6 @@ +mod common; + +use common::access_controllable_contract::{AccessControllableContract, Caller}; use near_sdk::serde_json::json; /// Smoke test of contract setup and basic functionality. @@ -37,42 +40,23 @@ async fn test_set_and_get_status() -> anyhow::Result<()> { async fn test_acl_has_role() -> anyhow::Result<()> { let worker = workspaces::sandbox().await?; let wasm = workspaces::compile_project("./tests/contracts/access_controllable").await?; - let contract = worker.dev_deploy(&wasm).await?; - + let contract = AccessControllableContract::new(worker.dev_deploy(&wasm).await?); let account = worker.dev_create_account().await?; - // TODO add helper functions to execute frequent transactions - - let res = account - .call(contract.id(), "acl_has_role") - .args_json(json!({ - "role": "LevelA", - "account_id": account.id(), - })) - .view() + let has_role = contract + .acl_has_role(account.clone().into(), "LevelA", account.id()) .await?; - assert_eq!(res.json::()?, false); + assert_eq!(has_role, false); contract - .call("acl_grant_role_unchecked") - .args_json(json!({ - "role": "LevelA", - "account_id": account.id(), - })) - .max_gas() - .transact() + .acl_grant_role_unchecked(Caller::Contract, "LevelA", account.id()) .await? .into_result()?; - let res = account - .call(contract.id(), "acl_has_role") - .args_json(json!({ - "role": "LevelA", - "account_id": account.id(), - })) - .view() + let has_role = contract + .acl_has_role(account.clone().into(), "LevelA", account.id()) .await?; - assert_eq!(res.json::()?, true); + assert_eq!(has_role, true); Ok(()) } diff --git a/near-plugins/tests/common/access_controllable_contract.rs b/near-plugins/tests/common/access_controllable_contract.rs new file mode 100644 index 0000000..1440c51 --- /dev/null +++ b/near-plugins/tests/common/access_controllable_contract.rs @@ -0,0 +1,72 @@ +use near_sdk::serde_json::json; +use workspaces::result::ExecutionFinalResult; +use workspaces::{Account, AccountId, Contract}; + +/// Specifies who calls a method on the contract. +#[derive(Clone)] +pub enum Caller { + /// The contract itself. + Contract, + /// The provided account. + Account(Account), +} + +impl From for Caller { + fn from(account: Account) -> Self { + Self::Account(account) + } +} + +/// Wrapper for a contract that is `#[access_controllable]`. It allows +/// implementing helpers for calling contract methods. +pub struct AccessControllableContract { + contract: Contract, +} + +impl AccessControllableContract { + pub fn new(contract: Contract) -> Self { + Self { contract } + } + + fn account(&self, caller: Caller) -> Account { + match caller { + Caller::Contract => self.contract.as_account().clone(), + Caller::Account(account) => account, + } + } + + pub async fn acl_has_role( + &self, + caller: Caller, + role: &str, + account_id: &AccountId, + ) -> anyhow::Result { + let res = self + .account(caller) + .call(self.contract.id(), "acl_has_role") + .args_json(json!({ + "role": role, + "account_id": account_id, + })) + .view() + .await?; + Ok(res.json::()?) + } + + pub async fn acl_grant_role_unchecked( + &self, + caller: Caller, + role: &str, + account_id: &AccountId, + ) -> workspaces::Result { + self.account(caller) + .call(self.contract.id(), "acl_grant_role_unchecked") + .args_json(json!({ + "role": role, + "account_id": account_id, + })) + .max_gas() + .transact() + .await + } +} diff --git a/near-plugins/tests/common/mod.rs b/near-plugins/tests/common/mod.rs new file mode 100644 index 0000000..09d38d7 --- /dev/null +++ b/near-plugins/tests/common/mod.rs @@ -0,0 +1 @@ +pub mod access_controllable_contract; From 21570e9f3f19caa8cc72c0413ab4bc19284f2025 Mon Sep 17 00:00:00 2001 From: Moritz Date: Wed, 21 Sep 2022 15:55:29 +0200 Subject: [PATCH 15/27] test: add Setup to avoid repetition --- near-plugins/tests/access_controllable.rs | 36 ++++++++++++++----- .../common/access_controllable_contract.rs | 4 +++ 2 files changed, 31 insertions(+), 9 deletions(-) diff --git a/near-plugins/tests/access_controllable.rs b/near-plugins/tests/access_controllable.rs index 5abf412..54a767a 100644 --- a/near-plugins/tests/access_controllable.rs +++ b/near-plugins/tests/access_controllable.rs @@ -2,15 +2,34 @@ mod common; use common::access_controllable_contract::{AccessControllableContract, Caller}; use near_sdk::serde_json::json; +use workspaces::Account; + +const PROJECT_PATH: &str = "./tests/contracts/access_controllable"; + +/// Bundles resources required in tests. +struct Setup { + /// Deployed instance of the contract in [`PROJECT_PATH`]. + contract: AccessControllableContract, + /// A newly created account (which differs from the contract). + account: Account, +} + +impl Setup { + async fn new() -> anyhow::Result { + let worker = workspaces::sandbox().await?; + let wasm = workspaces::compile_project(PROJECT_PATH).await?; + let contract = AccessControllableContract::new(worker.dev_deploy(&wasm).await?); + let account = worker.dev_create_account().await?; + + Ok(Self { contract, account }) + } +} /// Smoke test of contract setup and basic functionality. #[tokio::test] async fn test_set_and_get_status() -> anyhow::Result<()> { - let worker = workspaces::sandbox().await?; - let wasm = workspaces::compile_project("./tests/contracts/access_controllable").await?; - let contract = worker.dev_deploy(&wasm).await?; - - let account = worker.dev_create_account().await?; + let Setup { contract, account } = Setup::new().await?; + let contract = contract.contract(); let message = "hello world"; account @@ -38,10 +57,7 @@ async fn test_set_and_get_status() -> anyhow::Result<()> { #[tokio::test] async fn test_acl_has_role() -> anyhow::Result<()> { - let worker = workspaces::sandbox().await?; - let wasm = workspaces::compile_project("./tests/contracts/access_controllable").await?; - let contract = AccessControllableContract::new(worker.dev_deploy(&wasm).await?); - let account = worker.dev_create_account().await?; + let Setup { contract, account } = Setup::new().await?; let has_role = contract .acl_has_role(account.clone().into(), "LevelA", account.id()) @@ -60,3 +76,5 @@ async fn test_acl_has_role() -> anyhow::Result<()> { Ok(()) } + +// TODO add test for acl_grant_role_unchecked diff --git a/near-plugins/tests/common/access_controllable_contract.rs b/near-plugins/tests/common/access_controllable_contract.rs index 1440c51..fcdd251 100644 --- a/near-plugins/tests/common/access_controllable_contract.rs +++ b/near-plugins/tests/common/access_controllable_contract.rs @@ -28,6 +28,10 @@ impl AccessControllableContract { Self { contract } } + pub fn contract(&self) -> &Contract { + &self.contract + } + fn account(&self, caller: Caller) -> Account { match caller { Caller::Contract => self.contract.as_account().clone(), From 5b6bdc4a9f3e19063d89530a0d1861d5efb37179 Mon Sep 17 00:00:00 2001 From: Moritz Date: Thu, 22 Sep 2022 11:15:03 +0200 Subject: [PATCH 16/27] test: add tests for acl_grant_role_unchecked --- near-plugins/tests/access_controllable.rs | 42 +++++++++++++++++-- .../common/access_controllable_contract.rs | 8 ++++ near-plugins/tests/common/mod.rs | 1 + near-plugins/tests/common/utils.rs | 17 ++++++++ 4 files changed, 64 insertions(+), 4 deletions(-) create mode 100644 near-plugins/tests/common/utils.rs diff --git a/near-plugins/tests/access_controllable.rs b/near-plugins/tests/access_controllable.rs index 54a767a..17565bf 100644 --- a/near-plugins/tests/access_controllable.rs +++ b/near-plugins/tests/access_controllable.rs @@ -1,6 +1,7 @@ mod common; use common::access_controllable_contract::{AccessControllableContract, Caller}; +use common::utils::assert_private_method_failure; use near_sdk::serde_json::json; use workspaces::Account; @@ -58,23 +59,56 @@ async fn test_set_and_get_status() -> anyhow::Result<()> { #[tokio::test] async fn test_acl_has_role() -> anyhow::Result<()> { let Setup { contract, account } = Setup::new().await?; + let role = "LevelA"; + // Anyone may call `acl_has_role`. let has_role = contract - .acl_has_role(account.clone().into(), "LevelA", account.id()) + .acl_has_role(account.clone().into(), role, account.id()) .await?; assert_eq!(has_role, false); contract - .acl_grant_role_unchecked(Caller::Contract, "LevelA", account.id()) + .acl_grant_role_unchecked(Caller::Contract, role, account.id()) .await? .into_result()?; let has_role = contract - .acl_has_role(account.clone().into(), "LevelA", account.id()) + .acl_has_role(account.clone().into(), role, account.id()) .await?; assert_eq!(has_role, true); Ok(()) } -// TODO add test for acl_grant_role_unchecked +#[tokio::test] +async fn test_acl_grant_role_unchecked_is_private() -> anyhow::Result<()> { + let Setup { contract, account } = Setup::new().await?; + let res = contract + .acl_grant_role_unchecked(account.clone().into(), "LevelA", account.id()) + .await?; + assert_private_method_failure(res, "acl_grant_role_unchecked"); + Ok(()) +} + +#[tokio::test] +async fn test_acl_grant_role_unchecked() -> anyhow::Result<()> { + let Setup { contract, account } = Setup::new().await?; + let role = "LevelA"; + + contract + .assert_acl_has_role(false, role, account.id()) + .await; + contract + .acl_grant_role_unchecked(Caller::Contract, role, account.id()) + .await? + .into_result()?; + contract.assert_acl_has_role(true, role, account.id()).await; + + // Granting a role again doesn't lead to failures. + contract + .acl_grant_role_unchecked(Caller::Contract, role, account.id()) + .await? + .into_result()?; + + Ok(()) +} diff --git a/near-plugins/tests/common/access_controllable_contract.rs b/near-plugins/tests/common/access_controllable_contract.rs index fcdd251..edfaf65 100644 --- a/near-plugins/tests/common/access_controllable_contract.rs +++ b/near-plugins/tests/common/access_controllable_contract.rs @@ -57,6 +57,14 @@ impl AccessControllableContract { Ok(res.json::()?) } + pub async fn assert_acl_has_role(&self, want: bool, role: &str, account_id: &AccountId) { + let has_role = self + .acl_has_role(Caller::Contract, role, account_id) + .await + .unwrap(); + assert_eq!(has_role, want); + } + pub async fn acl_grant_role_unchecked( &self, caller: Caller, diff --git a/near-plugins/tests/common/mod.rs b/near-plugins/tests/common/mod.rs index 09d38d7..528ef04 100644 --- a/near-plugins/tests/common/mod.rs +++ b/near-plugins/tests/common/mod.rs @@ -1 +1,2 @@ pub mod access_controllable_contract; +pub mod utils; diff --git a/near-plugins/tests/common/utils.rs b/near-plugins/tests/common/utils.rs new file mode 100644 index 0000000..aef11b0 --- /dev/null +++ b/near-plugins/tests/common/utils.rs @@ -0,0 +1,17 @@ +use workspaces::result::ExecutionFinalResult; + +/// Asserts transaction failure due to `method` being `#[private]`. +pub fn assert_private_method_failure(res: ExecutionFinalResult, method: &str) { + let err = res + .into_result() + .err() + .expect("Transaction should have failed"); + let err = format!("{}", err); + let must_contain = format!("Method {} is private", method); + assert!( + err.contains(&must_contain), + "'{}' is not contained in '{}'", + must_contain, + err, + ); +} From 8ab61d38e137c4b33d59f1d7c1e4eadb593e02cf Mon Sep 17 00:00:00 2001 From: Moritz Date: Thu, 22 Sep 2022 11:19:47 +0200 Subject: [PATCH 17/27] Remove method stub To be implemented in follow-up PR. --- near-plugins-derive/src/access_controllable.rs | 5 ----- near-plugins/src/access_controllable.rs | 6 ------ 2 files changed, 11 deletions(-) diff --git a/near-plugins-derive/src/access_controllable.rs b/near-plugins-derive/src/access_controllable.rs index 4f4d56a..afd4028 100644 --- a/near-plugins-derive/src/access_controllable.rs +++ b/near-plugins-derive/src/access_controllable.rs @@ -164,11 +164,6 @@ pub fn access_controllable(attrs: TokenStream, item: TokenStream) -> TokenStream (#storage_prefix).as_bytes() } - #[private] - fn acl_add_admin_unchecked(&mut self, role: String, account_id: ::near_sdk::AccountId) -> bool { - false // TODO - } - #[private] fn acl_grant_role_unchecked(&mut self, role: String, account_id: ::near_sdk::AccountId) -> bool { let role = <#role_type>::try_from(role.as_str()).expect(#ERR_PARSE_ROLE); diff --git a/near-plugins/src/access_controllable.rs b/near-plugins/src/access_controllable.rs index 48a2717..fbf1805 100644 --- a/near-plugins/src/access_controllable.rs +++ b/near-plugins/src/access_controllable.rs @@ -19,12 +19,6 @@ pub trait AccessControllable { /// Returns the storage prefix for collections related to access control. fn acl_storage_prefix() -> &'static [u8]; - /// Grants admin permissions for `role` to `account_id`, __without__ - /// checking permissions of the predecessor. - /// - /// Returns whether `account_id` was newly added to the admins for `role`. - fn acl_add_admin_unchecked(&mut self, role: String, account_id: AccountId) -> bool; - /// Grants `role` to `account_id` __without__ checking any permissions. /// Returns whether `role` was newly granted to `account_id`. fn acl_grant_role_unchecked(&mut self, role: String, account_id: AccountId) -> bool; From bf886a6e89bd19d1811c696e66d2f6a5bac409fa Mon Sep 17 00:00:00 2001 From: Moritz Date: Fri, 23 Sep 2022 18:43:50 +0200 Subject: [PATCH 18/27] Verify role type satisfies trait bounds --- .../src/access_control_role.rs | 23 +++++++++++++++++++ .../src/access_controllable.rs | 1 - .../contracts/access_controllable/src/lib.rs | 2 +- 3 files changed, 24 insertions(+), 2 deletions(-) diff --git a/near-plugins-derive/src/access_control_role.rs b/near-plugins-derive/src/access_control_role.rs index 5d5b3f9..3c714cf 100644 --- a/near-plugins-derive/src/access_control_role.rs +++ b/near-plugins-derive/src/access_control_role.rs @@ -47,6 +47,7 @@ use syn::{parse_macro_input, ItemEnum}; const DEFAULT_SUPER_ADMIN_NAME: &str = "__SUPER_ADMIN"; const DEFAULT_BITFLAGS_TYPE_NAME: &str = "RoleFlags"; +const DEFAULT_BOUNDCHECKER_TYPE_NAME: &str = "__AclBoundchecker"; pub fn derive_access_control_role(input: TokenStream) -> TokenStream { // This derive doesn't take attributes, so no need to use `darling`. @@ -60,12 +61,34 @@ pub fn derive_access_control_role(input: TokenStream) -> TokenStream { (0..u8::try_from(variants.len()).expect("Too many enum variants")).collect(); let variant_names: Vec<_> = variants.iter().map(|v| format!("{}", v.ident)).collect(); + let boundchecker_type = Ident::new(DEFAULT_BOUNDCHECKER_TYPE_NAME, ident.span()); let bitflags_type_ident = new_bitflags_type_ident(Span::call_site()); let bitflags_idents = bitflags_idents(variant_names.as_ref(), bitflags_type_ident.span()); let bitflags_idxs: Vec<_> = (0..u8::try_from(bitflags_idents.len()).expect("Too many bitflags")).collect(); let output = quote! { + // Ensure #ident satisfies bounds required for acl. This is done + // explicitly to provide a clear error message to developers whose + // enum doesn't satisfy the required bounds. + // + // Without this explicit check, compilation would still fail if a bound + // is not satisfied. Though with less a clear error message. + struct #boundchecker_type { + _marker: ::std::marker::PhantomData, + } + impl #boundchecker_type { + fn new() -> Self { + Self { _marker: Default::default() } + } + } + impl #ident { + fn check_bounds() { + // Compilation will fail if #ident doesn't satisfy above bounds. + let _x = #boundchecker_type::<#ident>::new(); + } + } + impl From<#ident> for u8 { fn from(value: #ident) -> Self { match value { diff --git a/near-plugins-derive/src/access_controllable.rs b/near-plugins-derive/src/access_controllable.rs index afd4028..191a3fe 100644 --- a/near-plugins-derive/src/access_controllable.rs +++ b/near-plugins-derive/src/access_controllable.rs @@ -33,7 +33,6 @@ pub fn access_controllable(attrs: TokenStream, item: TokenStream) -> TokenStream } let ItemStruct { ident, .. } = input.clone(); - // TODO verify trait bounds on `role_enum`: BorshSerialize, BorshDeserialize, ... let macro_args = match MacroArgs::from_list(&attr_args) { Ok(args) => args, Err(e) => { diff --git a/near-plugins/tests/contracts/access_controllable/src/lib.rs b/near-plugins/tests/contracts/access_controllable/src/lib.rs index 7ea2112..efa7bc8 100644 --- a/near-plugins/tests/contracts/access_controllable/src/lib.rs +++ b/near-plugins/tests/contracts/access_controllable/src/lib.rs @@ -5,7 +5,7 @@ use near_sdk::{env, log, near_bindgen, AccountId}; use std::collections::HashMap; use std::convert::TryFrom; -#[derive(AccessControlRole, BorshSerialize, BorshDeserialize, Copy, Clone)] +#[derive(AccessControlRole, Copy, Clone)] pub enum Role { LevelA, LevelB, From e3d88f2a6ec8d4c4bbe844872471afb5d8eb783d Mon Sep 17 00:00:00 2001 From: Moritz Date: Mon, 26 Sep 2022 20:26:46 +0200 Subject: [PATCH 19/27] Extend `is_near_bindgen_wrapped_or_marshall` --- near-plugins-derive/src/utils.rs | 56 +++++++++++++++++++++++++++----- 1 file changed, 48 insertions(+), 8 deletions(-) diff --git a/near-plugins-derive/src/utils.rs b/near-plugins-derive/src/utils.rs index 8835573..b7bc10d 100644 --- a/near-plugins-derive/src/utils.rs +++ b/near-plugins-derive/src/utils.rs @@ -1,18 +1,58 @@ use proc_macro2::Span; use proc_macro_crate::crate_name; -use syn::{Ident, ItemFn}; +use std::str::FromStr; +use syn::{FnArg, Ident, ItemFn}; -/// Determines if this block of code was generated by near_bindgen. +/// Determines if this block of code was [generated by near_bindgen]. /// Heuristic used is to check for #[no_mangle]. /// TODO: How to make this 100% safe. Discuss with near-sdk team +/// +/// [generated by near_bindgen]: https://github.com/near/near-sdk-rs/issues/722 pub(crate) fn is_near_bindgen_wrapped_or_marshall(item: &ItemFn) -> bool { - let pattern1 = "(target_arch = \"wasm32\")"; - let pattern2 = "(not(target_arch = \"wasm32\"))"; + let condition_1 = { + let pattern1 = "(target_arch = \"wasm32\")"; + let pattern2 = "(not(target_arch = \"wasm32\"))"; - item.attrs.iter().any(|attr| { - let seq = attr.tokens.to_string(); - seq == pattern1 || seq == pattern2 - }) + item.attrs.iter().any(|attr| { + let seq = attr.tokens.to_string(); + seq == pattern1 || seq == pattern2 + }) + }; + if condition_1 { + return true; + } + + // For a struct `Contract`, `#[near-bindgen]` [generates] `ContractExt`. If + // `item` is in an implementation of `ContractExt`, the span number of its + // signature differs from the span number of the self token. + // + // [generates]: https://github.com/near/near-sdk-rs/pull/742 + let condition_2 = { + let signature_span = item.sig.ident.span(); + let self_token_span = match item.sig.inputs.iter().nth(0) { + Some(FnArg::Receiver(receiver)) => receiver.self_token.span, + _ => panic!("Attribute must be used on a method with self receiver"), + }; + span_number(&signature_span) != span_number(&self_token_span) + }; + condition_2 +} + +/// Returns the number of the span. +/// +/// # Panics +/// +/// Panics if the formatted `span` does not correspond to the pattern +/// `"#42 bytes(1124..1142)"`. +fn span_number(span: &Span) -> u64 { + let formatted = format!("{:#?}", span); + let mut number_part = formatted + .split(" ") + .nth(0) + .expect("Formatting a Span yielded an unexpected pattern") + .to_string(); + number_part.remove(0); // remove the `#` + u64::from_str(&number_part).expect("Failed to extract number from formatted Span") } pub(crate) fn cratename() -> Ident { From 91d496f764b222cd71f9328b99443c6eb4e5cf05 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 27 Sep 2022 09:44:39 +0200 Subject: [PATCH 20/27] Implement `#[access_control_any(roles(...))]` --- .../src/access_control_role.rs | 4 +- .../src/access_controllable.rs | 90 ++++++++++++++++++- near-plugins-derive/src/lib.rs | 5 ++ near-plugins/src/access_controllable.rs | 5 +- near-plugins/src/lib.rs | 4 +- .../contracts/access_controllable/src/lib.rs | 23 +++-- 6 files changed, 116 insertions(+), 15 deletions(-) diff --git a/near-plugins-derive/src/access_control_role.rs b/near-plugins-derive/src/access_control_role.rs index 3c714cf..e3cdafd 100644 --- a/near-plugins-derive/src/access_control_role.rs +++ b/near-plugins-derive/src/access_control_role.rs @@ -4,7 +4,7 @@ //! key part of this implementation. Assume the trait is derived on the //! following enum: //! -//! ``` +//! ```ignore //! #[derive(AccessControlRole)] //! pub enum Role { //! LevelA, @@ -14,7 +14,7 @@ //! ``` //! //! This results in the following bitflags: -//! ``` +//! ```ignore //! bitflags! { //! struct RoleFlags: u128 { //! const __SUPER_ADMIN = 1u128 << 0; diff --git a/near-plugins-derive/src/access_controllable.rs b/near-plugins-derive/src/access_controllable.rs index 191a3fe..e96cc45 100644 --- a/near-plugins-derive/src/access_controllable.rs +++ b/near-plugins-derive/src/access_controllable.rs @@ -1,11 +1,11 @@ use crate::access_control_role::new_bitflags_type_ident; -use crate::utils::cratename; +use crate::utils::{cratename, is_near_bindgen_wrapped_or_marshall}; use darling::FromMeta; use proc_macro::TokenStream; use proc_macro2::Span; use quote::quote; use syn::parse::Parser; -use syn::{parse_macro_input, AttributeArgs, ItemStruct}; +use syn::{parse_macro_input, AttributeArgs, ItemFn, ItemStruct}; #[derive(Debug, FromMeta)] pub struct MacroArgs { @@ -140,6 +140,29 @@ pub fn access_controllable(attrs: TokenStream, item: TokenStream) -> TokenStream } } + fn has_any_role( + &self, roles: Vec<#role_type>, + account_id: &::near_sdk::AccountId + ) -> bool { + let permissions: Vec<#bitflags_type> = roles + .iter() + .map(|role| { + <#bitflags_type>::from_bits(role.acl_permission()) + .expect(#ERR_PARSE_BITFLAG) + }) + .collect(); + let target = permissions.iter().fold( + <#bitflags_type>::empty(), + |acc, &x| acc | x, + ); + self.has_any_permission(target, account_id) + } + + fn has_any_permission(&self, target: #bitflags_type, account_id: &::near_sdk::AccountId) -> bool { + let permissions = self.get_or_init_permissions(account_id); + target.intersects(permissions) + } + /// Adds `account_id` to the set of `permission` bearers. fn add_bearer(&mut self, permission: #bitflags_type, account_id: &::near_sdk::AccountId) { let mut set = match self.bearers.get(&permission) { @@ -173,6 +196,14 @@ pub fn access_controllable(attrs: TokenStream, item: TokenStream) -> TokenStream let role = <#role_type>::try_from(role.as_str()).expect(#ERR_PARSE_ROLE); self.#acl_field.has_role(role, &account_id) } + + fn acl_has_any_role(&self, roles: Vec, account_id: ::near_sdk::AccountId) -> bool { + let roles: Vec<#role_type> = roles + .iter() + .map(|role| <#role_type>::try_from(role.as_str()).expect(#ERR_PARSE_ROLE)) + .collect(); + self.#acl_field.has_any_role(roles, &account_id) + } } }; @@ -199,3 +230,58 @@ fn inject_acl_field( })?); Ok(()) } + +#[derive(Debug, FromMeta)] +pub struct MacroArgsAny { + roles: darling::util::PathList, +} + +pub fn access_control_any(attrs: TokenStream, item: TokenStream) -> TokenStream { + let attr_args = parse_macro_input!(attrs as AttributeArgs); + let cloned_item = item.clone(); + let input: ItemFn = parse_macro_input!(cloned_item); + if is_near_bindgen_wrapped_or_marshall(&input) { + return item; + } + + let ItemFn { + attrs, + vis, + sig, + block, + } = input; + let function_name = sig.ident.to_string(); + let stmts = &block.stmts; + + let macro_args = match MacroArgsAny::from_list(&attr_args) { + Ok(args) => args, + Err(e) => { + return TokenStream::from(e.write_errors()); + } + }; + let roles = macro_args.roles; + assert!(roles.len() > 0, "Specify at least one role"); + + let acl_check = quote! { + let __roles: Vec<&str> = vec![#(#roles.into()),*]; + let __roles_ser: Vec = __roles.iter().map(|&role| role.into()).collect(); + let __account_id = ::near_sdk::env::predecessor_account_id(); + if !self.acl_has_any_role(__roles_ser, __account_id) { + let message = format!( + "Insufficient permissions for method {} restricted by access control. Requires one of these roles: {:?}", + #function_name, + __roles, + ); + env::panic_str(&message); + } + }; + + // https://stackoverflow.com/a/66851407 + quote! { + #(#attrs)* #vis #sig { + #acl_check + #(#stmts)* + } + } + .into() +} diff --git a/near-plugins-derive/src/lib.rs b/near-plugins-derive/src/lib.rs index 6e86e0d..5df5cee 100644 --- a/near-plugins-derive/src/lib.rs +++ b/near-plugins-derive/src/lib.rs @@ -52,3 +52,8 @@ pub fn derive_access_control_role(input: TokenStream) -> TokenStream { pub fn access_control(attrs: TokenStream, item: TokenStream) -> TokenStream { access_controllable::access_controllable(attrs, item) } + +#[proc_macro_attribute] +pub fn access_control_any(attrs: TokenStream, item: TokenStream) -> TokenStream { + access_controllable::access_control_any(attrs, item) +} diff --git a/near-plugins/src/access_controllable.rs b/near-plugins/src/access_controllable.rs index fbf1805..ba97b90 100644 --- a/near-plugins/src/access_controllable.rs +++ b/near-plugins/src/access_controllable.rs @@ -6,7 +6,7 @@ use near_sdk::AccountId; /// not possible to use a generic type `R` since `near-sdk` [does not support] /// `impl` type parameters. /// -/// ``` +/// ```ignore /// // This is not possible: /// impl AccessControllable for Contract {/* ... */} /// ``` @@ -25,6 +25,9 @@ pub trait AccessControllable { /// Returns whether `account_id` has been granted `role`. fn acl_has_role(&self, role: String, account_id: AccountId) -> bool; + + /// Returns whether `account_id` has been granted any of the `roles`. + fn acl_has_any_role(&self, roles: Vec, account_id: AccountId) -> bool; } pub mod events { diff --git a/near-plugins/src/lib.rs b/near-plugins/src/lib.rs index 9d1cb53..46eda8d 100644 --- a/near-plugins/src/lib.rs +++ b/near-plugins/src/lib.rs @@ -12,8 +12,8 @@ pub use access_control_role::AccessControlRole; pub use access_controllable::AccessControllable; pub use full_access_key_fallback::FullAccessKeyFallback; pub use near_plugins_derive::{ - access_control, if_paused, only, pause, AccessControlRole, FullAccessKeyFallback, Ownable, - Pausable, Upgradable, + access_control, access_control_any, if_paused, only, pause, AccessControlRole, + FullAccessKeyFallback, Ownable, Pausable, Upgradable, }; pub use ownable::Ownable; pub use pausable::Pausable; diff --git a/near-plugins/tests/contracts/access_controllable/src/lib.rs b/near-plugins/tests/contracts/access_controllable/src/lib.rs index efa7bc8..f05715e 100644 --- a/near-plugins/tests/contracts/access_controllable/src/lib.rs +++ b/near-plugins/tests/contracts/access_controllable/src/lib.rs @@ -1,5 +1,5 @@ use near_plugins::events::AsEvent; -use near_plugins::{access_control, AccessControlRole, AccessControllable}; +use near_plugins::{access_control, access_control_any, AccessControlRole, AccessControllable}; use near_sdk::borsh::{self, BorshDeserialize, BorshSerialize}; use near_sdk::{env, log, near_bindgen, AccountId}; use std::collections::HashMap; @@ -33,12 +33,19 @@ impl StatusMessage { self.records.get(&account_id).cloned() } - // The contract can interact with Acl by: - // - // a) Calling functions, e.g. - // self.acl.has_role(role, account_id) + #[access_control_any(roles(Role::LevelA, Role::LevelB))] + pub fn restricted_greeting(&self) -> String { + "hello world".to_string() + } + + // In addition, `AccessControllable` trait methods can be called directly: // - // b) Using attributes (not yet implemented), e.g. - // #[acl_any(Role::LevelA, Role::LevelB)] - // pub fn foo(&mut self) {} + // ``` + // pub fn foo(&self) { + // let role = Role::LevelA; + // if self.acl_has_role(role.into(), &env::predecessor_account_id()) { + // // .. + // } + // } + // ``` } From 521e1ae6e2fa6ecf02a97060471745e155821894 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 27 Sep 2022 17:33:39 +0200 Subject: [PATCH 21/27] test: add `test_attribute_access_control_any` --- near-plugins/tests/access_controllable.rs | 154 +++++++++++++++++- near-plugins/tests/common/utils.rs | 29 ++++ .../contracts/access_controllable/src/lib.rs | 2 +- 3 files changed, 177 insertions(+), 8 deletions(-) diff --git a/near-plugins/tests/access_controllable.rs b/near-plugins/tests/access_controllable.rs index 17565bf..74539b1 100644 --- a/near-plugins/tests/access_controllable.rs +++ b/near-plugins/tests/access_controllable.rs @@ -1,14 +1,18 @@ mod common; use common::access_controllable_contract::{AccessControllableContract, Caller}; -use common::utils::assert_private_method_failure; +use common::utils::{assert_insufficient_acl_permissions, assert_private_method_failure}; use near_sdk::serde_json::json; -use workspaces::Account; +use workspaces::network::Sandbox; +use workspaces::result::ExecutionFinalResult; +use workspaces::{Account, Contract, Worker}; const PROJECT_PATH: &str = "./tests/contracts/access_controllable"; /// Bundles resources required in tests. struct Setup { + /// The worker interacting with the current sandbox. + worker: Worker, /// Deployed instance of the contract in [`PROJECT_PATH`]. contract: AccessControllableContract, /// A newly created account (which differs from the contract). @@ -22,14 +26,96 @@ impl Setup { let contract = AccessControllableContract::new(worker.dev_deploy(&wasm).await?); let account = worker.dev_create_account().await?; - Ok(Self { contract, account }) + Ok(Self { + worker, + contract, + account, + }) } + + async fn new_account_with_roles(&self, roles: &[&str]) -> anyhow::Result { + let account = self.worker.dev_create_account().await?; + for &role in roles { + self.contract + .acl_grant_role_unchecked(Caller::Contract, role, account.id()) + .await? + .into_result()?; + } + Ok(account) + } +} + +/// Represents the outcome of a transaction sent to the [`PROJECT_PATH`] +/// contract. +// TODO generic `T` instead of `String` +#[derive(Debug)] +enum TxOutcome { + Success(String), + AclFailure(AclFailure), +} + +#[derive(Debug)] +struct AclFailure { + method_name: String, + /// The roles that are allowed (specified in the contract). + allowed_roles: Vec, + /// The result of the transaction. Not allowing view calls here since + /// `ViewResultDetails` is not sufficient to verify ACL failure. + result: ExecutionFinalResult, +} + +impl TxOutcome { + fn assert_success(&self, want: String) { + let got = match self { + TxOutcome::Success(got) => got.clone(), + TxOutcome::AclFailure(failure) => panic!( + "Expected transaction success but it failed with: {:?}", + failure + ), + }; + assert_eq!(got, want); + } + + fn assert_acl_failure(&self) { + let failure = match self { + TxOutcome::Success(_) => panic!("Expected transaction failure"), + TxOutcome::AclFailure(failure) => failure, + }; + assert_insufficient_acl_permissions( + failure.result.clone(), + failure.method_name.as_str(), + failure.allowed_roles.clone(), + ); + } +} + +async fn call_restricted_greeting( + contract: &Contract, + caller: &Account, +) -> anyhow::Result { + let res = caller + .call(contract.id(), "restricted_greeting") + .args_json(()) + .max_gas() + .transact() + .await?; + let tx_outcome = match res.is_success() { + true => TxOutcome::Success(res.into_result().unwrap().json::().unwrap()), + false => TxOutcome::AclFailure(AclFailure { + method_name: "restricted_greeting".to_string(), + allowed_roles: vec!["LevelA".to_string(), "LevelC".to_string()], + result: res, + }), + }; + Ok(tx_outcome) } /// Smoke test of contract setup and basic functionality. #[tokio::test] async fn test_set_and_get_status() -> anyhow::Result<()> { - let Setup { contract, account } = Setup::new().await?; + let Setup { + contract, account, .. + } = Setup::new().await?; let contract = contract.contract(); let message = "hello world"; @@ -58,7 +144,9 @@ async fn test_set_and_get_status() -> anyhow::Result<()> { #[tokio::test] async fn test_acl_has_role() -> anyhow::Result<()> { - let Setup { contract, account } = Setup::new().await?; + let Setup { + contract, account, .. + } = Setup::new().await?; let role = "LevelA"; // Anyone may call `acl_has_role`. @@ -82,7 +170,9 @@ async fn test_acl_has_role() -> anyhow::Result<()> { #[tokio::test] async fn test_acl_grant_role_unchecked_is_private() -> anyhow::Result<()> { - let Setup { contract, account } = Setup::new().await?; + let Setup { + contract, account, .. + } = Setup::new().await?; let res = contract .acl_grant_role_unchecked(account.clone().into(), "LevelA", account.id()) .await?; @@ -92,7 +182,9 @@ async fn test_acl_grant_role_unchecked_is_private() -> anyhow::Result<()> { #[tokio::test] async fn test_acl_grant_role_unchecked() -> anyhow::Result<()> { - let Setup { contract, account } = Setup::new().await?; + let Setup { + contract, account, .. + } = Setup::new().await?; let role = "LevelA"; contract @@ -112,3 +204,51 @@ async fn test_acl_grant_role_unchecked() -> anyhow::Result<()> { Ok(()) } + +#[tokio::test] +async fn test_attribute_access_control_any() -> anyhow::Result<()> { + let setup = Setup::new().await?; + let raw_contract = setup.contract.contract(); + let want_result = "hello world".to_string(); + + // Account without any of the required permissions is restricted. + let account = setup.new_account_with_roles(&[]).await?; + call_restricted_greeting(raw_contract, &account) + .await? + .assert_acl_failure(); + let account = setup.new_account_with_roles(&["LevelB"]).await?; + call_restricted_greeting(raw_contract, &account) + .await? + .assert_acl_failure(); + + // Account with one of the required permissions succeeds. + let account = setup.new_account_with_roles(&["LevelA"]).await?; + call_restricted_greeting(raw_contract, &account) + .await? + .assert_success(want_result.clone()); + let account = setup.new_account_with_roles(&["LevelC"]).await?; + call_restricted_greeting(raw_contract, &account) + .await? + .assert_success(want_result.clone()); + let account = setup.new_account_with_roles(&["LevelA", "LevelB"]).await?; + call_restricted_greeting(raw_contract, &account) + .await? + .assert_success(want_result.clone()); + + // Account with both permissions succeeds. + let account = setup.new_account_with_roles(&["LevelA", "LevelC"]).await?; + call_restricted_greeting(raw_contract, &account) + .await? + .assert_success(want_result.clone()); + let account = setup + .new_account_with_roles(&["LevelA", "LevelB", "LevelC"]) + .await?; + call_restricted_greeting(raw_contract, &account) + .await? + .assert_success(want_result.clone()); + + // TODO once admin fns are implemented, add tests for cases mentioned in + // https://github.com/aurora-is-near/near-plugins/pull/5#discussion_r973784721 + + Ok(()) +} diff --git a/near-plugins/tests/common/utils.rs b/near-plugins/tests/common/utils.rs index aef11b0..330eda5 100644 --- a/near-plugins/tests/common/utils.rs +++ b/near-plugins/tests/common/utils.rs @@ -15,3 +15,32 @@ pub fn assert_private_method_failure(res: ExecutionFinalResult, method: &str) { err, ); } + +/// Asserts transaction failure due to insufficient `AccessControllable` (ACL) +/// permissions. +pub fn assert_insufficient_acl_permissions( + res: ExecutionFinalResult, + method: &str, + _allowed_roles: Vec, +) { + let err = res + .into_result() + .err() + .expect("Transaction should have failed"); + let err = format!("{}", err); + + // TODO fix escaping issue to also verify second sentence of the error + // Using `format!` here it'll be: Requires one of these roles: ["LevelA", "LevelB"] + // However, roles contained in `err` are escaped, i.e. [\"LevelA\", \"LevelB\"] + let must_contain = format!( + "Insufficient permissions for method {} restricted by access control.", + method, + ); + + assert!( + err.contains(&must_contain), + "'{}' is not contained in '{}'", + must_contain, + err, + ); +} diff --git a/near-plugins/tests/contracts/access_controllable/src/lib.rs b/near-plugins/tests/contracts/access_controllable/src/lib.rs index f05715e..9f829a8 100644 --- a/near-plugins/tests/contracts/access_controllable/src/lib.rs +++ b/near-plugins/tests/contracts/access_controllable/src/lib.rs @@ -33,7 +33,7 @@ impl StatusMessage { self.records.get(&account_id).cloned() } - #[access_control_any(roles(Role::LevelA, Role::LevelB))] + #[access_control_any(roles(Role::LevelA, Role::LevelC))] pub fn restricted_greeting(&self) -> String { "hello world".to_string() } From 5eca45c7e6a9f017c5b56abc0b33369d6b7825fa Mon Sep 17 00:00:00 2001 From: Moritz Date: Wed, 28 Sep 2022 12:50:23 +0200 Subject: [PATCH 22/27] chore: rename `want` -> `expected` --- near-plugins/tests/access_controllable.rs | 16 ++++++++-------- .../tests/common/access_controllable_contract.rs | 4 ++-- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/near-plugins/tests/access_controllable.rs b/near-plugins/tests/access_controllable.rs index 74539b1..e3fb6e3 100644 --- a/near-plugins/tests/access_controllable.rs +++ b/near-plugins/tests/access_controllable.rs @@ -65,7 +65,7 @@ struct AclFailure { } impl TxOutcome { - fn assert_success(&self, want: String) { + fn assert_success(&self, expected: String) { let got = match self { TxOutcome::Success(got) => got.clone(), TxOutcome::AclFailure(failure) => panic!( @@ -73,7 +73,7 @@ impl TxOutcome { failure ), }; - assert_eq!(got, want); + assert_eq!(got, expected); } fn assert_acl_failure(&self) { @@ -209,7 +209,7 @@ async fn test_acl_grant_role_unchecked() -> anyhow::Result<()> { async fn test_attribute_access_control_any() -> anyhow::Result<()> { let setup = Setup::new().await?; let raw_contract = setup.contract.contract(); - let want_result = "hello world".to_string(); + let expected_result = "hello world".to_string(); // Account without any of the required permissions is restricted. let account = setup.new_account_with_roles(&[]).await?; @@ -225,27 +225,27 @@ async fn test_attribute_access_control_any() -> anyhow::Result<()> { let account = setup.new_account_with_roles(&["LevelA"]).await?; call_restricted_greeting(raw_contract, &account) .await? - .assert_success(want_result.clone()); + .assert_success(expected_result.clone()); let account = setup.new_account_with_roles(&["LevelC"]).await?; call_restricted_greeting(raw_contract, &account) .await? - .assert_success(want_result.clone()); + .assert_success(expected_result.clone()); let account = setup.new_account_with_roles(&["LevelA", "LevelB"]).await?; call_restricted_greeting(raw_contract, &account) .await? - .assert_success(want_result.clone()); + .assert_success(expected_result.clone()); // Account with both permissions succeeds. let account = setup.new_account_with_roles(&["LevelA", "LevelC"]).await?; call_restricted_greeting(raw_contract, &account) .await? - .assert_success(want_result.clone()); + .assert_success(expected_result.clone()); let account = setup .new_account_with_roles(&["LevelA", "LevelB", "LevelC"]) .await?; call_restricted_greeting(raw_contract, &account) .await? - .assert_success(want_result.clone()); + .assert_success(expected_result.clone()); // TODO once admin fns are implemented, add tests for cases mentioned in // https://github.com/aurora-is-near/near-plugins/pull/5#discussion_r973784721 diff --git a/near-plugins/tests/common/access_controllable_contract.rs b/near-plugins/tests/common/access_controllable_contract.rs index edfaf65..d408c5d 100644 --- a/near-plugins/tests/common/access_controllable_contract.rs +++ b/near-plugins/tests/common/access_controllable_contract.rs @@ -57,12 +57,12 @@ impl AccessControllableContract { Ok(res.json::()?) } - pub async fn assert_acl_has_role(&self, want: bool, role: &str, account_id: &AccountId) { + pub async fn assert_acl_has_role(&self, expected: bool, role: &str, account_id: &AccountId) { let has_role = self .acl_has_role(Caller::Contract, role, account_id) .await .unwrap(); - assert_eq!(has_role, want); + assert_eq!(has_role, expected); } pub async fn acl_grant_role_unchecked( From ca5f01416c237c57eff023a827c32957a1dd0f03 Mon Sep 17 00:00:00 2001 From: Moritz Date: Wed, 28 Sep 2022 12:59:58 +0200 Subject: [PATCH 23/27] pr review - use into_iter() - explicit 1 --- near-plugins-derive/src/access_control_role.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/near-plugins-derive/src/access_control_role.rs b/near-plugins-derive/src/access_control_role.rs index e3cdafd..902de48 100644 --- a/near-plugins-derive/src/access_control_role.rs +++ b/near-plugins-derive/src/access_control_role.rs @@ -56,7 +56,7 @@ pub fn derive_access_control_role(input: TokenStream) -> TokenStream { ident, variants, .. } = input; - let variants = variants.iter().collect::>(); + let variants = variants.into_iter().collect::>(); let variant_idxs: Vec<_> = (0..u8::try_from(variants.len()).expect("Too many enum variants")).collect(); let variant_names: Vec<_> = variants.iter().map(|v| format!("{}", v.ident)).collect(); @@ -155,7 +155,7 @@ pub fn derive_access_control_role(input: TokenStream) -> TokenStream { impl AccessControlRole for #ident { fn acl_super_admin_permission() -> u128 { // See module documentation. - safe_leftshift(1, 0) + 1 // corresponds to safe_leftshift(1, 0) } fn acl_permission(self) -> u128 { From f60faa824d5bb134bded8d9073dd371d065aad5c Mon Sep 17 00:00:00 2001 From: Moritz Date: Wed, 28 Sep 2022 13:16:39 +0200 Subject: [PATCH 24/27] pr review - add TODO (optimization) - specialize var names in injected code --- near-plugins-derive/src/access_controllable.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/near-plugins-derive/src/access_controllable.rs b/near-plugins-derive/src/access_controllable.rs index e96cc45..b0c2893 100644 --- a/near-plugins-derive/src/access_controllable.rs +++ b/near-plugins-derive/src/access_controllable.rs @@ -262,15 +262,17 @@ pub fn access_control_any(attrs: TokenStream, item: TokenStream) -> TokenStream let roles = macro_args.roles; assert!(roles.len() > 0, "Specify at least one role"); + // TODO optimize case `roles.len() == 1` (speed up expected common case) let acl_check = quote! { - let __roles: Vec<&str> = vec![#(#roles.into()),*]; - let __roles_ser: Vec = __roles.iter().map(|&role| role.into()).collect(); - let __account_id = ::near_sdk::env::predecessor_account_id(); - if !self.acl_has_any_role(__roles_ser, __account_id) { + let __acl_any_roles: Vec<&str> = vec![#(#roles.into()),*]; + let __acl_any_roles_ser: Vec = + __acl_any_roles.iter().map(|&role| role.into()).collect(); + let __acl_any_account_id = ::near_sdk::env::predecessor_account_id(); + if !self.acl_has_any_role(__acl_any_roles_ser, __acl_any_account_id) { let message = format!( "Insufficient permissions for method {} restricted by access control. Requires one of these roles: {:?}", #function_name, - __roles, + __acl_any_roles, ); env::panic_str(&message); } From df4b6bdbaa5e97e21dd90cd869d3f8cc7612f3c8 Mon Sep 17 00:00:00 2001 From: Moritz Date: Wed, 28 Sep 2022 16:50:52 +0200 Subject: [PATCH 25/27] Validate input `permission` in `add_bearer()` --- near-plugins-derive/src/access_controllable.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/near-plugins-derive/src/access_controllable.rs b/near-plugins-derive/src/access_controllable.rs index b0c2893..72889b3 100644 --- a/near-plugins-derive/src/access_controllable.rs +++ b/near-plugins-derive/src/access_controllable.rs @@ -164,7 +164,19 @@ pub fn access_controllable(attrs: TokenStream, item: TokenStream) -> TokenStream } /// Adds `account_id` to the set of `permission` bearers. + /// + /// # Panics + /// + /// Panics if `permission` has more than one active bit. The type of + /// permission defines only flags which have one active bit. Still, + /// developers might call this function with a `permission` that has + /// multiple active bits. In that case, the panic prevents polluting + /// state. fn add_bearer(&mut self, permission: #bitflags_type, account_id: &::near_sdk::AccountId) { + assert!( + permission.bits().is_power_of_two(), + "Adding a bearer is allowed only for permissions with exactly one active bit" + ); let mut set = match self.bearers.get(&permission) { Some(set) => set, None => Self::new_bearers_set(permission), From 790824f242069d3d7735a51ef01e92137b2d6f57 Mon Sep 17 00:00:00 2001 From: Moritz Date: Wed, 28 Sep 2022 18:04:02 +0200 Subject: [PATCH 26/27] Document heuristic to detect *Ext impl --- near-plugins-derive/src/utils.rs | 39 ++++++++++++++++++++++++++++---- 1 file changed, 35 insertions(+), 4 deletions(-) diff --git a/near-plugins-derive/src/utils.rs b/near-plugins-derive/src/utils.rs index b7bc10d..574748b 100644 --- a/near-plugins-derive/src/utils.rs +++ b/near-plugins-derive/src/utils.rs @@ -22,17 +22,48 @@ pub(crate) fn is_near_bindgen_wrapped_or_marshall(item: &ItemFn) -> bool { return true; } - // For a struct `Contract`, `#[near-bindgen]` [generates] `ContractExt`. If - // `item` is in an implementation of `ContractExt`, the span number of its - // signature differs from the span number of the self token. + // Assume `#[only(self, owner)]` is used in the following way: // - // [generates]: https://github.com/near/near-sdk-rs/pull/742 + // ``` + // #[near_bindgen] + // #[derive(Ownable)] + // struct Counter { + // counter: u64, + // } + // + // #[near_bindgen] + // impl Counter { + // #[only(self, owner)] + // fn foo(&self) {} + // } + // ``` + // + // Then `near-sdk` will create a struct `CounterExt` and code like this: + // + // ``` + // impl CodeExt { + // #[only(self, owner)] // <- the attribute being forwared here is problematic + // fn foo(&self) -> near_sdk::Promise { + // // We don't want `#[only]` to inject code here. + // } + // } + // ``` + // + // The code is generated here: + // https://github.com/near/near-sdk-rs/blob/770cbce018a1b6c49d58276a075ace3da96d6dc1/near-sdk-macros/src/core_impl/code_generator/ext.rs#L79-L105 + // + // The following heuristic detects if macro expansion is invoked on an + // `ItemFn` inside an implementation of `CounterExt`. It's hacky since it + // relies on debug formatting of `proc_macro2::Span`. let condition_2 = { let signature_span = item.sig.ident.span(); let self_token_span = match item.sig.inputs.iter().nth(0) { Some(FnArg::Receiver(receiver)) => receiver.self_token.span, _ => panic!("Attribute must be used on a method with self receiver"), }; + // If `item` is in an `impl` block generated by `near_bindgen`, its + // signature should have a different span number than the self token + // referring to an *Ext struct generated by `near_bindgen`. span_number(&signature_span) != span_number(&self_token_span) }; condition_2 From 1c3a05a031432a3e147737df229dec5f58ea9b14 Mon Sep 17 00:00:00 2001 From: Moritz Date: Wed, 28 Sep 2022 18:35:22 +0200 Subject: [PATCH 27/27] refactor: avoid a Vec allocation --- near-plugins-derive/src/access_controllable.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/near-plugins-derive/src/access_controllable.rs b/near-plugins-derive/src/access_controllable.rs index 72889b3..51370a1 100644 --- a/near-plugins-derive/src/access_controllable.rs +++ b/near-plugins-derive/src/access_controllable.rs @@ -144,17 +144,17 @@ pub fn access_controllable(attrs: TokenStream, item: TokenStream) -> TokenStream &self, roles: Vec<#role_type>, account_id: &::near_sdk::AccountId ) -> bool { - let permissions: Vec<#bitflags_type> = roles + // Create a bitflags value with active bits for all `roles`. + let target = roles .iter() .map(|role| { <#bitflags_type>::from_bits(role.acl_permission()) .expect(#ERR_PARSE_BITFLAG) }) - .collect(); - let target = permissions.iter().fold( - <#bitflags_type>::empty(), - |acc, &x| acc | x, - ); + .fold( + <#bitflags_type>::empty(), + |acc, x| acc | x, + ); self.has_any_permission(target, account_id) }