diff --git a/client/tests/integration/smartcontracts/Cargo.toml b/client/tests/integration/smartcontracts/Cargo.toml index 663956a73a0..4739c446174 100644 --- a/client/tests/integration/smartcontracts/Cargo.toml +++ b/client/tests/integration/smartcontracts/Cargo.toml @@ -13,6 +13,7 @@ members = [ "mint_rose_trigger", "executor_with_admin", "executor_with_custom_token", + "executor_remove_token", "executor_with_migration_fail", "query_assets_and_save_cursor", ] diff --git a/client/tests/integration/smartcontracts/executor_remove_token/Cargo.toml b/client/tests/integration/smartcontracts/executor_remove_token/Cargo.toml new file mode 100644 index 00000000000..ce62e8c1150 --- /dev/null +++ b/client/tests/integration/smartcontracts/executor_remove_token/Cargo.toml @@ -0,0 +1,24 @@ +[package] +name = "executor_remove_token" + +edition.workspace = true +version.workspace = true +authors.workspace = true + +license.workspace = true + +[lib] +crate-type = ['cdylib'] + +[dependencies] +iroha_executor.workspace = true +iroha_schema.workspace = true + +parity-scale-codec.workspace = true +anyhow.workspace = true +serde_json.workspace = true +serde.workspace = true + +panic-halt.workspace = true +lol_alloc.workspace = true +getrandom.workspace = true diff --git a/client/tests/integration/smartcontracts/executor_remove_token/src/lib.rs b/client/tests/integration/smartcontracts/executor_remove_token/src/lib.rs new file mode 100644 index 00000000000..4a2c7aa16b3 --- /dev/null +++ b/client/tests/integration/smartcontracts/executor_remove_token/src/lib.rs @@ -0,0 +1,37 @@ +//! Runtime Executor which removes [`token::CanControlDomainLives`] permission token. +//! Needed for tests. + +#![no_std] + +extern crate alloc; +#[cfg(not(test))] +extern crate panic_halt; + +use iroha_executor::{default::default_permission_token_schema, prelude::*}; +use lol_alloc::{FreeListAllocator, LockedAllocator}; + +#[global_allocator] +static ALLOC: LockedAllocator = LockedAllocator::new(FreeListAllocator::new()); + +getrandom::register_custom_getrandom!(iroha_executor::stub_getrandom); + +#[derive(Constructor, ValidateEntrypoints, Validate, Visit)] +struct Executor { + verdict: Result, + block_height: u64, +} + +#[entrypoint] +pub fn migrate(_block_height: u64) -> MigrationResult { + // Note that actually migration will reset token schema to default (minus `CanUnregisterDomain`) + // So any added custom permission tokens will be also removed + let mut schema = default_permission_token_schema(); + schema.remove::(); + + let (token_ids, schema_str) = schema.serialize(); + iroha_executor::set_permission_token_schema( + &iroha_executor::data_model::permission::PermissionTokenSchema::new(token_ids, schema_str), + ); + + Ok(()) +} diff --git a/client/tests/integration/upgrade.rs b/client/tests/integration/upgrade.rs index 32e169b25a4..7c510be6e93 100644 --- a/client/tests/integration/upgrade.rs +++ b/client/tests/integration/upgrade.rs @@ -113,6 +113,76 @@ fn executor_upgrade_should_run_migration() -> Result<()> { Ok(()) } +#[test] +fn executor_upgrade_should_revoke_removed_permissions() -> Result<()> { + let (_rt, _peer, client) = ::new().with_port(10_990).start_with_runtime(); + wait_for_genesis_committed(&vec![client.clone()], 0); + + // Permission which will be removed by executor + let can_unregister_domain_token = PermissionToken::new( + "CanUnregisterDomain".parse()?, + &json!({ "domain_id": DomainId::from_str("wonderland")? }), + ); + + // Register `TEST_ROLE` with permission + let test_role_id: RoleId = "TEST_ROLE".parse()?; + let test_role = + Role::new(test_role_id.clone()).add_permission(can_unregister_domain_token.clone()); + client.submit_blocking(Register::role(test_role))?; + + // Check that permission exists + assert!(client + .request(FindPermissionTokenSchema)? + .token_ids() + .contains(&can_unregister_domain_token.definition_id)); + + // Check that `TEST_ROLE` has permission + assert!(client + .request(FindAllRoles::new())? + .collect::>>()? + .into_iter() + .find(|role| role.id == test_role_id) + .expect("Failed to find Role") + .permissions + .contains(&can_unregister_domain_token)); + + // Check that Alice has permission + let alice_id: AccountId = "alice@wonderland".parse()?; + assert!(client + .request(FindPermissionTokensByAccountId::new(alice_id.clone()))? + .collect::>>()? + .contains(&can_unregister_domain_token)); + + upgrade_executor( + &client, + "tests/integration/smartcontracts/executor_remove_token", + )?; + + // Check that permission doesn't exist + assert!(!client + .request(FindPermissionTokenSchema)? + .token_ids() + .contains(&can_unregister_domain_token.definition_id)); + + // Check that `TEST_ROLE` doesn't have permission + assert!(!client + .request(FindAllRoles::new())? + .collect::>>()? + .into_iter() + .find(|role| role.id == test_role_id) + .expect("Failed to find Role") + .permissions + .contains(&can_unregister_domain_token)); + + // Check that Alice doesn't have permission + assert!(!client + .request(FindPermissionTokensByAccountId::new(alice_id.clone()))? + .collect::>>()? + .contains(&can_unregister_domain_token)); + + Ok(()) +} + #[test] fn migration_fail_should_not_cause_any_effects() { let (_rt, _peer, client) = ::new().with_port(10_995).start_with_runtime(); diff --git a/configs/swarm/executor.wasm b/configs/swarm/executor.wasm index cc4689ba58f..55348c7b4cb 100644 Binary files a/configs/swarm/executor.wasm and b/configs/swarm/executor.wasm differ diff --git a/core/src/smartcontracts/isi/world.rs b/core/src/smartcontracts/isi/world.rs index 30af4f45561..1ba5d27425c 100644 --- a/core/src/smartcontracts/isi/world.rs +++ b/core/src/smartcontracts/isi/world.rs @@ -18,6 +18,7 @@ impl Registrable for NewRole { /// Iroha Special Instructions that have `World` as their target. pub mod isi { use eyre::Result; + use indexmap::IndexSet; use iroha_data_model::{ isi::error::{InstructionExecutionError, InvalidParameterError, RepetitionError}, prelude::*, @@ -345,6 +346,12 @@ pub mod isi { ) -> Result<(), Error> { let raw_executor = self.executor; + let permissions_before = state_transaction + .world + .permission_token_schema + .token_ids + .clone(); + // Cloning executor to avoid multiple mutable borrows of `state_transaction`. // Also it's a cheap operation. let mut upgraded_executor = state_transaction.world.executor.clone(); @@ -359,6 +366,8 @@ pub mod isi { *state_transaction.world.executor.get_mut() = upgraded_executor; + revoke_removed_permissions(authority, state_transaction, permissions_before)?; + state_transaction .world .emit_events(std::iter::once(ExecutorEvent::Upgraded)); @@ -367,6 +376,71 @@ pub mod isi { } } + fn revoke_removed_permissions( + authority: &AccountId, + state_transaction: &mut StateTransaction, + permissions_before: Vec, + ) -> Result<(), Error> { + let world = state_transaction.world(); + let permissions_after = world + .permission_token_schema() + .token_ids + .iter() + .collect::>(); + let permissions_removed = permissions_before + .into_iter() + .filter(|permission| !permissions_after.contains(permission)) + .collect::>(); + if permissions_removed.is_empty() { + return Ok(()); + } + + let to_revoke_from_accounts = find_related_accounts(world, &permissions_removed); + let to_revoke_from_roles = find_related_roles(world, &permissions_removed); + + for (account_id, permission) in to_revoke_from_accounts { + let revoke = Revoke::permission(permission, account_id); + revoke.execute(authority, state_transaction)? + } + for (role_id, permission) in to_revoke_from_roles { + let revoke = Revoke::role_permission(permission, role_id); + revoke.execute(authority, state_transaction)? + } + Ok(()) + } + + fn find_related_accounts( + world: &impl WorldReadOnly, + permissions: &IndexSet, + ) -> Vec<(AccountId, PermissionToken)> { + world + .account_permission_tokens() + .iter() + .flat_map(|(account_id, account_permissions)| { + account_permissions + .iter() + .filter(|permission| permissions.contains(&permission.definition_id)) + .map(|permission| (account_id.clone(), permission.clone())) + }) + .collect() + } + + fn find_related_roles( + world: &impl WorldReadOnly, + permissions: &IndexSet, + ) -> Vec<(RoleId, PermissionToken)> { + world + .roles() + .iter() + .flat_map(|(role_id, role)| { + role.permissions + .iter() + .filter(|permission| permissions.contains(&permission.definition_id)) + .map(|permission| (role_id.clone(), permission.clone())) + }) + .collect() + } + impl Execute for Log { fn execute( self, diff --git a/data_model/src/isi.rs b/data_model/src/isi.rs index 2e9a73504e3..31aceca6dd6 100644 --- a/data_model/src/isi.rs +++ b/data_model/src/isi.rs @@ -1276,7 +1276,7 @@ isi_box! { PermissionToken(Revoke), /// Revoke [`Role`] from [`Account`]. Role(Revoke), - /// Revoke [`PermissionToken`] from [`Account`]. + /// Revoke [`PermissionToken`] from [`Role`]. RolePermissionToken(Revoke), } }