[fix] hyperledger-iroha#4242: Remove permissions from roles on entity…

… unregistration

Signed-off-by: Shanin Roman <[email protected]>

client/tests/integration/permissions.rs

@@ -7,7 +7,7 @@ use iroha_client::{
     data_model::prelude::*,
 };
 use iroha_data_model::{
-    permission::PermissionToken, transaction::error::TransactionRejectionReason,
+    permission::PermissionToken, role::RoleId, transaction::error::TransactionRejectionReason,
 };
 use iroha_genesis::GenesisNetwork;
 use serde_json::json;
@@ -422,3 +422,47 @@ fn associated_permission_tokens_removed_on_unregister() {
         .into_iter()
         .all(|token| { token != bob_to_set_kv_in_domain_token }));
 }
+
+#[test]
+fn associated_permission_tokens_removed_from_role_on_unregister() {
+    let (_rt, _peer, iroha_client) = <PeerBuilder>::new().with_port(11_255).start_with_runtime();
+    wait_for_genesis_committed(&[iroha_client.clone()], 0);
+
+    let role_id: RoleId = "role".parse().expect("Valid");
+    let kingdom_id: DomainId = "kingdom".parse().expect("Valid");
+    let kingdom = Domain::new(kingdom_id.clone());
+
+    // register kingdom and give bob permissions in this domain
+    let register_domain = Register::domain(kingdom);
+    let set_kv_in_domain_token = PermissionToken::new(
+        "CanSetKeyValueInDomain".parse().unwrap(),
+        &json!({ "domain_id": kingdom_id }),
+    );
+    let role = Role::new(role_id.clone()).add_permission(set_kv_in_domain_token.clone());
+    let register_role = Register::role(role);
+
+    iroha_client
+        .submit_all_blocking([InstructionBox::from(register_domain), register_role.into()])
+        .expect("failed to register domain and grant permission");
+
+    // check that role indeed have permission
+    assert!(iroha_client
+        .request(client::role::by_id(role_id.clone()))
+        .map(|role| role.permissions().cloned().collect::<Vec<_>>())
+        .expect("failed to get permissions for role")
+        .into_iter()
+        .any(|token| { token == set_kv_in_domain_token }));
+
+    // unregister kingdom
+    iroha_client
+        .submit_blocking(Unregister::domain(kingdom_id))
+        .expect("failed to unregister domain");
+
+    // check that permission is removed from role
+    assert!(iroha_client
+        .request(client::role::by_id(role_id.clone()))
+        .map(|role| role.permissions().cloned().collect::<Vec<_>>())
+        .expect("failed to get permissions for role")
+        .into_iter()
+        .all(|token| { token != set_kv_in_domain_token }));
+}

client/tests/integration/tx_chain_id.rs

@@ -9,7 +9,7 @@ use crate::integration::asset::asset_id_new;
 
 #[test]
 fn send_tx_with_different_chain_id() {
-    let (_rt, _peer, test_client) = <PeerBuilder>::new().with_port(11_240).start_with_runtime();
+    let (_rt, _peer, test_client) = <PeerBuilder>::new().with_port(11_250).start_with_runtime();
     wait_for_genesis_committed(&[test_client.clone()], 0);
     // Given
     let sender_account_id = AccountId::from_str("sender@wonderland").unwrap();

smart_contract/executor/src/default.rs

@@ -176,6 +176,7 @@ pub mod domain {
     use iroha_smart_contract::data_model::{domain::DomainId, permission::PermissionToken};
     use permission::{
         account::is_account_owner, accounts_permission_tokens, domain::is_domain_owner,
+        roles_permission_tokens,
     };
     use tokens::AnyPermissionToken;
 
@@ -216,6 +217,14 @@ pub mod domain {
                     }
                 }
             }
+            for (role_id, permission) in roles_permission_tokens() {
+                if is_token_domain_associated(&permission, domain_id) {
+                    let isi = Revoke::role_permission(permission, role_id.clone());
+                    if let Err(_err) = isi.execute() {
+                        deny!(executor, "Can't revoke associated permission token");
+                    }
+                }
+            }
             execute!(executor, isi);
         }
         deny!(executor, "Can't unregister domain");
@@ -416,7 +425,9 @@ pub mod domain {
 
 pub mod account {
     use iroha_smart_contract::data_model::permission::PermissionToken;
-    use permission::{account::is_account_owner, accounts_permission_tokens};
+    use permission::{
+        account::is_account_owner, accounts_permission_tokens, roles_permission_tokens,
+    };
     use tokens::AnyPermissionToken;
 
     use super::*;
@@ -474,6 +485,14 @@ pub mod account {
                     }
                 }
             }
+            for (role_id, permission) in roles_permission_tokens() {
+                if is_token_account_associated(&permission, account_id) {
+                    let isi = Revoke::role_permission(permission, role_id.clone());
+                    if let Err(_err) = isi.execute() {
+                        deny!(executor, "Can't revoke associated permission token");
+                    }
+                }
+            }
             execute!(executor, isi);
         }
         deny!(executor, "Can't unregister another account");
@@ -692,7 +711,7 @@ pub mod asset_definition {
     use iroha_smart_contract::data_model::{asset::AssetDefinitionId, permission::PermissionToken};
     use permission::{
         account::is_account_owner, accounts_permission_tokens,
-        asset_definition::is_asset_definition_owner,
+        asset_definition::is_asset_definition_owner, roles_permission_tokens,
     };
     use tokens::AnyPermissionToken;
 
@@ -753,6 +772,14 @@ pub mod asset_definition {
                     }
                 }
             }
+            for (role_id, permission) in roles_permission_tokens() {
+                if is_token_asset_definition_associated(&permission, asset_definition_id) {
+                    let isi = Revoke::role_permission(permission, role_id.clone());
+                    if let Err(_err) = isi.execute() {
+                        deny!(executor, "Can't revoke associated permission token");
+                    }
+                }
+            }
             execute!(executor, isi);
         }
         deny!(
@@ -1405,7 +1432,9 @@ pub mod role {
 
 pub mod trigger {
     use iroha_smart_contract::data_model::{permission::PermissionToken, trigger::Trigger};
-    use permission::{accounts_permission_tokens, trigger::is_trigger_owner};
+    use permission::{
+        accounts_permission_tokens, roles_permission_tokens, trigger::is_trigger_owner,
+    };
     use tokens::AnyPermissionToken;
 
     use super::*;
@@ -1445,6 +1474,14 @@ pub mod trigger {
                     }
                 }
             }
+            for (role_id, permission) in roles_permission_tokens() {
+                if is_token_trigger_associated(&permission, trigger_id) {
+                    let isi = Revoke::role_permission(permission, role_id.clone());
+                    if let Err(_err) = isi.execute() {
+                        deny!(executor, "Can't revoke associated permission token");
+                    }
+                }
+            }
             execute!(executor, isi);
         }
         deny!(

smart_contract/executor/src/permission.rs

@@ -349,3 +349,19 @@ pub(crate) fn accounts_permission_tokens() -> impl Iterator<Item = (AccountId, P
                 .map(move |token| (account.id().clone(), token))
         })
 }
+
+/// Iterator over all roles and theirs permission tokens
+pub(crate) fn roles_permission_tokens() -> impl Iterator<Item = (RoleId, PermissionToken)> {
+    FindAllRoles
+        .execute()
+        .dbg_expect("failed to query all accounts")
+        .into_iter()
+        .map(|role| role.dbg_expect("failed to retrieve account"))
+        .flat_map(|role| {
+            role.permissions()
+                .cloned()
+                .collect::<Vec<_>>()
+                .into_iter()
+                .map(move |token| (role.id().clone(), token))
+        })
+}