From 6df8a84ba86b7da5c7a0580517ce6e8a61b40de9 Mon Sep 17 00:00:00 2001 From: Dmitry Murzin Date: Tue, 19 Nov 2024 12:17:38 +0300 Subject: [PATCH] refactor: Simplify revoking permission in default executor (#5239) Signed-off-by: Dmitry Murzin --- crates/iroha_executor/src/default/mod.rs | 109 +++++------------------ crates/iroha_executor/src/permission.rs | 27 ++++++ 2 files changed, 47 insertions(+), 89 deletions(-) diff --git a/crates/iroha_executor/src/default/mod.rs b/crates/iroha_executor/src/default/mod.rs index 7a8479a8ee3..26d354a3f5d 100644 --- a/crates/iroha_executor/src/default/mod.rs +++ b/crates/iroha_executor/src/default/mod.rs @@ -166,7 +166,7 @@ pub mod domain { use super::*; use crate::permission::{ - account::is_account_owner, accounts_permissions, domain::is_domain_owner, roles_permissions, + account::is_account_owner, domain::is_domain_owner, revoke_permissions, }; pub fn visit_register_domain( @@ -202,30 +202,13 @@ pub mod domain { .is_owned_by(&executor.context().authority, executor.host()) } { - let mut err = None; - for (owner_id, permission) in accounts_permissions(executor.host()) { - if is_permission_domain_associated(&permission, domain_id) { - let isi = &Revoke::account_permission(permission, owner_id.clone()); - - if let Err(error) = executor.host().submit(isi) { - err = Some(error); - break; - } - } - } - if let Some(err) = err { + let err = revoke_permissions(executor, |permission| { + is_permission_domain_associated(permission, domain_id) + }); + if let Err(err) = err { deny!(executor, err); } - for (role_id, permission) in roles_permissions(executor.host()) { - if is_permission_domain_associated(&permission, domain_id) { - let isi = &Revoke::role_permission(permission, role_id.clone()); - - if let Err(err) = executor.host().submit(isi) { - deny!(executor, err); - } - } - } execute!(executor, isi); } deny!(executor, "Can't unregister domain"); @@ -389,7 +372,7 @@ pub mod account { }; use super::*; - use crate::permission::{account::is_account_owner, accounts_permissions, roles_permissions}; + use crate::permission::{account::is_account_owner, revoke_permissions}; pub fn visit_register_account( executor: &mut V, @@ -441,30 +424,13 @@ pub mod account { .is_owned_by(&executor.context().authority, executor.host()) } { - let mut err = None; - for (owner_id, permission) in accounts_permissions(executor.host()) { - if is_permission_account_associated(&permission, account_id) { - let isi = &Revoke::account_permission(permission, owner_id.clone()); - - if let Err(error) = executor.host().submit(isi) { - err = Some(error); - break; - } - } - } - if let Some(err) = err { + let err = revoke_permissions(executor, |permission| { + is_permission_account_associated(permission, account_id) + }); + if let Err(err) = err { deny!(executor, err); } - for (role_id, permission) in roles_permissions(executor.host()) { - if is_permission_account_associated(&permission, account_id) { - let isi = &Revoke::role_permission(permission, role_id.clone()); - - if let Err(err) = executor.host().submit(isi) { - deny!(executor, err); - } - } - } execute!(executor, isi); } deny!(executor, "Can't unregister another account"); @@ -580,8 +546,7 @@ pub mod asset_definition { use super::*; use crate::permission::{ - account::is_account_owner, accounts_permissions, - asset_definition::is_asset_definition_owner, roles_permissions, + account::is_account_owner, asset_definition::is_asset_definition_owner, revoke_permissions, }; pub fn visit_register_asset_definition( @@ -638,30 +603,13 @@ pub mod asset_definition { .is_owned_by(&executor.context().authority, executor.host()) } { - let mut err = None; - for (owner_id, permission) in accounts_permissions(executor.host()) { - if is_permission_asset_definition_associated(&permission, asset_definition_id) { - let isi = &Revoke::account_permission(permission, owner_id.clone()); - - if let Err(error) = executor.host().submit(isi) { - err = Some(error); - break; - } - } - } - if let Some(err) = err { + let err = revoke_permissions(executor, |permission| { + is_permission_asset_definition_associated(permission, asset_definition_id) + }); + if let Err(err) = err { deny!(executor, err); } - for (role_id, permission) in roles_permissions(executor.host()) { - if is_permission_asset_definition_associated(&permission, asset_definition_id) { - let isi = &Revoke::role_permission(permission, role_id.clone()); - - if let Err(err) = executor.host().submit(isi) { - deny!(executor, err); - } - } - } execute!(executor, isi); } deny!( @@ -1367,7 +1315,7 @@ pub mod trigger { use super::*; use crate::permission::{ - accounts_permissions, domain::is_domain_owner, roles_permissions, trigger::is_trigger_owner, + domain::is_domain_owner, revoke_permissions, trigger::is_trigger_owner, }; pub fn visit_register_trigger( @@ -1420,30 +1368,13 @@ pub mod trigger { .is_owned_by(&executor.context().authority, executor.host()) } { - let mut err = None; - for (owner_id, permission) in accounts_permissions(executor.host()) { - if is_permission_trigger_associated(&permission, trigger_id) { - let isi = &Revoke::account_permission(permission, owner_id.clone()); - - if let Err(error) = executor.host().submit(isi) { - err = Some(error); - break; - } - } - } - if let Some(err) = err { + let err = revoke_permissions(executor, |permission| { + is_permission_trigger_associated(permission, trigger_id) + }); + if let Err(err) = err { deny!(executor, err); } - for (role_id, permission) in roles_permissions(executor.host()) { - if is_permission_trigger_associated(&permission, trigger_id) { - let isi = &Revoke::role_permission(permission, role_id.clone()); - if let Err(err) = executor.host().submit(isi) { - deny!(executor, err); - } - } - } - execute!(executor, isi); } deny!( diff --git a/crates/iroha_executor/src/permission.rs b/crates/iroha_executor/src/permission.rs index 7460b5e0df4..e5ef2a51b6c 100644 --- a/crates/iroha_executor/src/permission.rs +++ b/crates/iroha_executor/src/permission.rs @@ -10,6 +10,7 @@ use crate::{ data_model::{executor::Result, permission::Permission as PermissionObject, prelude::*}, prelude::*, }, + Execute, }; /// Declare permission types of current module. Use it with a full path to the permission. @@ -1084,3 +1085,29 @@ pub(crate) fn roles_permissions(host: &Iroha) -> impl Iterator( + executor: &mut V, + condition: impl Fn(&PermissionObject) -> bool, +) -> Result<(), ValidationFail> { + for (owner_id, permission) in accounts_permissions(executor.host()) { + if condition(&permission) { + let isi = Revoke::account_permission(permission, owner_id.clone()); + + executor.host().submit(&isi)?; + } + } + + for (role_id, permission) in roles_permissions(executor.host()) { + if condition(&permission) { + let isi = Revoke::role_permission(permission, role_id.clone()); + + executor.host().submit(&isi)?; + } + } + + Ok(()) +}