Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement access control for AccountStore members #3204

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
8641e0b
using single storage unbounded instead of pallet::without_storage_info
silva-fj Dec 12, 2024
ce5ab39
adding integrity_test hook
silva-fj Dec 12, 2024
3758f5d
adding new types to the pallet config
silva-fj Dec 12, 2024
110c330
adding storage item for account member permissions
silva-fj Dec 12, 2024
c8adeac
adding new errors
silva-fj Dec 12, 2024
38f3a1f
storing default permission on create_account_store
silva-fj Dec 12, 2024
c06f446
adding comments
silva-fj Dec 12, 2024
8d6018b
adding default permissions
silva-fj Dec 12, 2024
0aeebe3
small refactoring
silva-fj Dec 12, 2024
b584d83
implementing ensure_permission
silva-fj Dec 12, 2024
959c83d
updating mock
silva-fj Dec 12, 2024
5cb8eed
adding optional permissions to add_account, set defaults if None
silva-fj Dec 12, 2024
7b619e2
setting permissions on update_account_store_by_one
silva-fj Dec 13, 2024
6200570
improving ensure_permission
silva-fj Dec 13, 2024
d76187c
updating mock
silva-fj Dec 13, 2024
d8cfd2c
refactoring add_account_call util
silva-fj Dec 13, 2024
c86a12f
adding tests for permissions
silva-fj Dec 13, 2024
91ae891
updating mock
silva-fj Dec 13, 2024
60d0199
adding Permission type to runtimes
silva-fj Dec 13, 2024
7ae5959
updating permissions mock and tests
silva-fj Dec 13, 2024
f729738
adding set_permissions call
silva-fj Dec 13, 2024
f34e5ba
adjusting ensure_permission
silva-fj Dec 13, 2024
8779cac
updating mock
silva-fj Dec 13, 2024
eddcf79
updating permission filters in the runtimes
silva-fj Dec 13, 2024
81b8c36
adding tests for set_permissions call
silva-fj Dec 13, 2024
e91a470
adding temporary fix for add_account
silva-fj Dec 13, 2024
f31183e
update storage on remove accounts
silva-fj Dec 14, 2024
a4876f1
setting default permission when empty, only if the sender has default
silva-fj Dec 16, 2024
8050b23
Merge remote-tracking branch 'origin/dev' into p-1240-implement-acces…
silva-fj Dec 26, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
143 changes: 139 additions & 4 deletions parachain/pallets/omni-account/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,13 @@ pub use pallet::*;
use frame_support::pallet_prelude::*;
use frame_support::{
dispatch::{GetDispatchInfo, PostDispatchInfo},
traits::{IsSubType, UnfilteredDispatchable},
traits::{InstanceFilter, IsSubType, UnfilteredDispatchable},
};
use frame_system::pallet_prelude::*;
use sp_core::H256;
use sp_runtime::traits::Dispatchable;
use sp_std::boxed::Box;
use sp_std::vec::Vec;
use sp_std::{vec, vec::Vec};

pub type MemberCount = u32;

Expand Down Expand Up @@ -68,7 +68,6 @@ pub mod pallet {

#[pallet::pallet]
#[pallet::storage_version(STORAGE_VERSION)]
#[pallet::without_storage_info]
pub struct Pallet<T>(_);

#[pallet::config]
Expand Down Expand Up @@ -106,6 +105,36 @@ pub mod pallet {

/// Convert an `Identity` to OmniAccount type
type OmniAccountConverter: OmniAccountConverter<OmniAccount = Self::AccountId>;

/// The permissions that a member account can have
/// The instance filter determines whether a given call may can be dispatched under this type.
///
/// IMPORTANT: `Default` must be provided and MUST BE the the *most permissive* value.
type Permission: Parameter
+ Member
+ Ord
+ PartialOrd
+ Default
+ InstanceFilter<<Self as Config>::RuntimeCall>
+ MaxEncodedLen;

/// The maximum number of permissions that a member account can have
#[pallet::constant]
type MaxPermissions: Get<u32>;
}

#[pallet::hooks]
impl<T: Config> Hooks<BlockNumberFor<T>> for Pallet<T> {
fn integrity_test() {
assert!(
<T as Config>::MaxAccountStoreLength::get() > 0,
"MaxAccountStoreLength must be greater than 0"
);
assert!(
<T as Config>::MaxPermissions::get() > 0,
"MaxPermissions must be greater than 0"
);
}
}

pub type MemberAccounts<T> = BoundedVec<MemberAccount, <T as Config>::MaxAccountStoreLength>;
Expand All @@ -115,6 +144,7 @@ pub mod pallet {

/// A map between OmniAccount and its MemberAccounts (a bounded vector of MemberAccount)
#[pallet::storage]
#[pallet::unbounded]
#[pallet::getter(fn account_store)]
pub type AccountStore<T: Config> =
StorageMap<Hasher = Blake2_128Concat, Key = T::AccountId, Value = MemberAccounts<T>>;
Expand All @@ -124,6 +154,21 @@ pub mod pallet {
pub type MemberAccountHash<T: Config> =
StorageMap<Hasher = Blake2_128Concat, Key = H256, Value = T::AccountId>;

#[pallet::type_value]
pub fn DefaultPermissions<T: Config>() -> BoundedVec<T::Permission, T::MaxPermissions> {
BoundedVec::try_from(vec![T::Permission::default()]).expect("default permission")
}

/// A map between hash of MemberAccount and its permissions
#[pallet::storage]
pub type MemberAccountPermissions<T: Config> = StorageMap<
Hasher = Blake2_128Concat,
Key = H256,
Value = BoundedVec<T::Permission, T::MaxPermissions>,
QueryKind = ValueQuery,
OnEmpty = DefaultPermissions<T>,
>;

#[pallet::event]
#[pallet::generate_deposit(pub(super) fn deposit_event)]
pub enum Event<T: Config> {
Expand Down Expand Up @@ -153,6 +198,8 @@ pub mod pallet {
IntentRequested { who: T::AccountId, intent: Intent },
/// Intent is executed
IntentExecuted { who: T::AccountId, intent: Intent, result: IntentExecutionResult },
/// Member permission set
AccountPermissionsSet { who: T::AccountId, member_account_hash: H256 },
/// An auth token is requested
AuthTokenRequested { who: T::AccountId, expires_at: BlockNumberFor<T> },
}
Expand All @@ -165,6 +212,8 @@ pub mod pallet {
InvalidAccount,
UnknownAccountStore,
EmptyAccount,
NoPermission,
PermissionsLenLimitReached,
}

#[pallet::call]
Expand All @@ -181,6 +230,7 @@ pub mod pallet {
let _ = T::TEECallOrigin::ensure_origin(origin)?;
let omni_account = MemberAccountHash::<T>::get(member_account_hash)
.ok_or(Error::<T>::AccountNotFound)?;
Self::ensure_permission(call.as_ref(), member_account_hash)?;
let result = call.dispatch(RawOrigin::OmniAccount(omni_account.clone()).into());
system::Pallet::<T>::inc_account_nonce(&omni_account);
Self::deposit_event(Event::DispatchedAsOmniAccount {
Expand All @@ -204,6 +254,7 @@ pub mod pallet {
let _ = T::TEECallOrigin::ensure_origin(origin)?;
let omni_account = MemberAccountHash::<T>::get(member_account_hash)
.ok_or(Error::<T>::AccountNotFound)?;
Self::ensure_permission(call.as_ref(), member_account_hash)?;
let result: Result<
PostDispatchInfo,
sp_runtime::DispatchErrorWithPostInfo<PostDispatchInfo>,
Expand Down Expand Up @@ -233,7 +284,8 @@ pub mod pallet {
#[pallet::weight((195_000_000, DispatchClass::Normal))]
pub fn add_account(
origin: OriginFor<T>,
member_account: MemberAccount, // account to be added
member_account: MemberAccount, // account to be added
permissions: Option<Vec<T::Permission>>, // permissions for the account
) -> DispatchResult {
// mutation of AccountStore requires `OmniAccountOrigin`, same as "remove" and "publicize"
let who = T::OmniAccountOrigin::ensure_origin(origin)?;
Expand All @@ -249,8 +301,16 @@ pub mod pallet {
member_accounts
.try_push(member_account)
.map_err(|_| Error::<T>::AccountStoreLenLimitReached)?;
let member_permissions: BoundedVec<T::Permission, T::MaxPermissions> = permissions
.map_or_else(
|| vec![T::Permission::default()],
|p| if p.is_empty() { vec![T::Permission::default()] } else { p },
)
.try_into()
.map_err(|_| Error::<T>::PermissionsLenLimitReached)?;

MemberAccountHash::<T>::insert(hash, who.clone());
MemberAccountPermissions::<T>::insert(hash, member_permissions);
AccountStore::<T>::insert(who.clone(), member_accounts.clone());

Self::deposit_event(Event::AccountAdded {
Expand All @@ -277,6 +337,7 @@ pub mod pallet {
member_accounts.retain(|member| {
if member_account_hashes.contains(&member.hash()) {
MemberAccountHash::<T>::remove(member.hash());
MemberAccountPermissions::<T>::remove(member.hash());
false
} else {
true
Expand Down Expand Up @@ -352,8 +413,13 @@ pub mod pallet {
.try_push(member_account.clone())
.map_err(|_| Error::<T>::AccountStoreLenLimitReached)?;
}
let mut permissions = BoundedVec::<T::Permission, T::MaxPermissions>::new();
permissions
.try_push(T::Permission::default())
.map_err(|_| Error::<T>::PermissionsLenLimitReached)?;

MemberAccountHash::<T>::insert(member_account.hash(), who_account.clone());
MemberAccountPermissions::<T>::insert(member_account.hash(), permissions);
AccountStore::<T>::insert(who_account.clone(), member_accounts.clone());
Self::deposit_event(Event::AccountStoreUpdated {
who: who_account,
Expand All @@ -378,6 +444,21 @@ pub mod pallet {

#[pallet::call_index(9)]
#[pallet::weight((195_000_000, DispatchClass::Normal))]
pub fn set_permissions(
origin: OriginFor<T>,
member_account_hash: H256,
permissions: Vec<T::Permission>,
) -> DispatchResult {
let who = T::OmniAccountOrigin::ensure_origin(origin)?;
let member_permissions: BoundedVec<T::Permission, T::MaxPermissions> =
{ permissions.try_into().map_err(|_| Error::<T>::PermissionsLenLimitReached)? };
MemberAccountPermissions::<T>::insert(member_account_hash, member_permissions);
Self::deposit_event(Event::AccountPermissionsSet { who, member_account_hash });
Ok(())
}

#[pallet::call_index(10)]
#[pallet::weight((195_000_000, DispatchClass::Normal))]
pub fn auth_token_requested(
origin: OriginFor<T>,
who: T::AccountId,
Expand Down Expand Up @@ -412,8 +493,13 @@ pub mod pallet {
member_accounts
.try_push(identity.into())
.map_err(|_| Error::<T>::AccountStoreLenLimitReached)?;
let mut permissions = BoundedVec::<T::Permission, T::MaxPermissions>::new();
permissions
.try_push(T::Permission::default())
.map_err(|_| Error::<T>::PermissionsLenLimitReached)?;

MemberAccountHash::<T>::insert(hash, omni_account.clone());
MemberAccountPermissions::<T>::insert(hash, permissions);
AccountStore::<T>::insert(omni_account.clone(), member_accounts.clone());

Self::deposit_event(Event::AccountStoreCreated { who: omni_account.clone() });
Expand All @@ -424,6 +510,55 @@ pub mod pallet {

Ok(member_accounts)
}

fn ensure_permission(
call: &<T as Config>::RuntimeCall,
member_account_hash: H256,
) -> Result<(), Error<T>> {
let member_permissions = MemberAccountPermissions::<T>::get(member_account_hash);

ensure!(
member_permissions.iter().any(|permission| permission.filter(call)),
Error::<T>::NoPermission
);

match call.is_sub_type() {
Some(Call::add_account { permissions: ref new_account_permissions, .. }) => {
// If member has default permission, they can add accounts with any permission
if member_permissions.contains(&T::Permission::default()) {
return Ok(());
}
match new_account_permissions {
Some(new_permissions) => {
// an account can only add another account with the same or less permissions
silva-fj marked this conversation as resolved.
Show resolved Hide resolved
if new_permissions.is_empty()
|| !new_permissions.iter().all(|p| member_permissions.contains(p))
{
return Err(Error::<T>::NoPermission);
}
},
None => {
// None is equivalent to default permission. It should not be allowed
// if the member_permissions have no default permission
return Err(Error::<T>::NoPermission);
},
}
},
Some(Call::set_permissions { permissions: ref new_permissions, .. }) => {
// If member has default permission, they can set permissions to any value
if member_permissions.contains(&T::Permission::default()) {
return Ok(());
}
// an account can only set permissions to the same or less permissions
if !new_permissions.iter().all(|p| member_permissions.contains(p)) {
return Err(Error::<T>::NoPermission);
}
},
_ => return Ok(()),
silva-fj marked this conversation as resolved.
Show resolved Hide resolved
}

Ok(())
}
}
}

Expand Down
95 changes: 93 additions & 2 deletions parachain/pallets/omni-account/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,18 @@
// You should have received a copy of the GNU General Public License
// along with Litentry. If not, see <https://www.gnu.org/licenses/>.

use crate::{self as pallet_omni_account, Encode, EnsureOmniAccount};
use crate::{self as pallet_omni_account, Decode, Encode, EnsureOmniAccount, MaxEncodedLen};
use core_primitives::{DefaultOmniAccountConverter, Identity, MemberAccount};
use frame_support::{
assert_ok, derive_impl,
pallet_prelude::EnsureOrigin,
parameter_types,
traits::{ConstU32, ConstU64},
traits::{ConstU32, ConstU64, InstanceFilter},
};
use frame_system::EnsureRoot;
pub use pallet_teebag::test_util::get_signer;
use pallet_teebag::test_util::{TEST8_CERT, TEST8_SIGNER_PUB, TEST8_TIMESTAMP, URL};
use sp_core::RuntimeDebug;
use sp_keyring::AccountKeyring;
use sp_runtime::{
traits::{IdentifyAccount, IdentityLookup, Verify},
Expand Down Expand Up @@ -96,6 +97,10 @@ pub fn charlie() -> Accounts {
create_accounts(AccountKeyring::Charlie)
}

pub fn dave() -> Accounts {
create_accounts(AccountKeyring::Dave)
}

pub fn public_member_account(accounts: Accounts) -> MemberAccount {
MemberAccount::Public(accounts.identity)
}
Expand Down Expand Up @@ -158,6 +163,90 @@ impl pallet_teebag::Config for Test {
type WeightInfo = ();
}

#[derive(
Copy,
Clone,
PartialEq,
Eq,
Ord,
PartialOrd,
Encode,
Decode,
RuntimeDebug,
MaxEncodedLen,
scale_info::TypeInfo,
)]
pub enum OmniAccountPermission {
All,
AccountManagement,
RequestNativeIntent,
RequestEthereumIntent,
RequestSolanaIntent,
}

impl Default for OmniAccountPermission {
fn default() -> Self {
Self::All
}
}

impl InstanceFilter<RuntimeCall> for OmniAccountPermission {
fn filter(&self, call: &RuntimeCall) -> bool {
match self {
Self::All => true,
Self::AccountManagement => {
matches!(
call,
RuntimeCall::OmniAccount(pallet_omni_account::Call::add_account { .. })
| RuntimeCall::OmniAccount(
pallet_omni_account::Call::remove_accounts { .. }
) | RuntimeCall::OmniAccount(
pallet_omni_account::Call::publicize_account { .. }
) | RuntimeCall::OmniAccount(pallet_omni_account::Call::set_permissions { .. })
)
},
Self::RequestNativeIntent => {
if let RuntimeCall::OmniAccount(pallet_omni_account::Call::request_intent {
intent,
}) = call
{
matches!(
intent,
pallet_omni_account::Intent::SystemRemark(_)
| pallet_omni_account::Intent::TransferNative(_)
)
} else {
false
}
},
Self::RequestEthereumIntent => {
if let RuntimeCall::OmniAccount(pallet_omni_account::Call::request_intent {
intent,
}) = call
{
matches!(
intent,
pallet_omni_account::Intent::TransferEthereum(_)
| pallet_omni_account::Intent::CallEthereum(_)
)
} else {
false
}
},
Self::RequestSolanaIntent => {
if let RuntimeCall::OmniAccount(pallet_omni_account::Call::request_intent {
intent,
}) = call
{
matches!(intent, pallet_omni_account::Intent::TransferSolana(_))
} else {
false
}
},
}
}
}

impl pallet_omni_account::Config for Test {
type RuntimeOrigin = RuntimeOrigin;
type RuntimeCall = RuntimeCall;
Expand All @@ -166,6 +255,8 @@ impl pallet_omni_account::Config for Test {
type MaxAccountStoreLength = ConstU32<3>;
type OmniAccountOrigin = EnsureOmniAccount<Self::AccountId>;
type OmniAccountConverter = DefaultOmniAccountConverter;
type MaxPermissions = ConstU32<4>;
type Permission = OmniAccountPermission;
}

pub fn get_tee_signer() -> SystemAccountId {
Expand Down
Loading
Loading