From 0b35400597b40dcfa31db1c7f438d3234b0be3fc Mon Sep 17 00:00:00 2001 From: Mohiiit <48082542+Mohiiit@users.noreply.github.com> Date: Fri, 29 Mar 2024 00:59:50 +0530 Subject: [PATCH] [sozo][auth]: revoke logic added (#1712) * [sozo][auth]: revoke logic added * test added for auth revoke * fix: remove world reader as argument WorldContractReader can be initialized from the inside of the ops. Hence, we avoid passing it as an argument and it is initialized inside the ops function. tests added for the auth revoke * test added for auth revoke * fix: refacto testing functions * fix: add missing spaces --------- Co-authored-by: glihm --- bin/sozo/src/commands/auth.rs | 23 ++++- crates/sozo/ops/src/auth.rs | 97 +++++++++++++++++-- crates/sozo/ops/src/execute.rs | 14 ++- crates/sozo/ops/src/lib.rs | 23 ----- crates/sozo/ops/src/tests/auth.rs | 145 ++++++++++++++++++++--------- crates/sozo/ops/src/tests/mod.rs | 2 + crates/sozo/ops/src/tests/setup.rs | 46 +++++++++ crates/sozo/ops/src/tests/utils.rs | 36 +++++++ crates/sozo/ops/src/utils.rs | 43 ++++++++- 9 files changed, 351 insertions(+), 78 deletions(-) create mode 100644 crates/sozo/ops/src/tests/setup.rs create mode 100644 crates/sozo/ops/src/tests/utils.rs diff --git a/bin/sozo/src/commands/auth.rs b/bin/sozo/src/commands/auth.rs index 32e78d5cdc..4dfffe17c8 100644 --- a/bin/sozo/src/commands/auth.rs +++ b/bin/sozo/src/commands/auth.rs @@ -62,6 +62,25 @@ pub async fn grant( } } +pub async fn revoke( + world: WorldOptions, + account: AccountOptions, + starknet: StarknetOptions, + env_metadata: Option, + kind: AuthKind, + transaction: TransactionOptions, +) -> Result<()> { + let world = + utils::world_from_env_metadata(world, account, starknet, &env_metadata).await.unwrap(); + match kind { + AuthKind::Writer { models_contracts } => { + auth::revoke_writer(&world, models_contracts, transaction.into()).await + } + AuthKind::Owner { owners_resources } => { + auth::revoke_owner(&world, owners_resources, transaction.into()).await + } + } +} #[derive(Debug, Subcommand)] pub enum AuthCommand { #[command(about = "Grant an auth role.")] @@ -108,7 +127,9 @@ impl AuthArgs { AuthCommand::Grant { kind, world, starknet, account, transaction } => config .tokio_handle() .block_on(grant(world, account, starknet, env_metadata, kind, transaction)), - _ => todo!(), + AuthCommand::Revoke { kind, world, starknet, account, transaction } => config + .tokio_handle() + .block_on(revoke(world, account, starknet, env_metadata, kind, transaction)), } } } diff --git a/crates/sozo/ops/src/auth.rs b/crates/sozo/ops/src/auth.rs index a02afd541f..a852e84174 100644 --- a/crates/sozo/ops/src/auth.rs +++ b/crates/sozo/ops/src/auth.rs @@ -10,8 +10,7 @@ use starknet::core::types::{BlockId, BlockTag}; use starknet::core::utils::parse_cairo_short_string; use starknet_crypto::FieldElement; -use super::get_contract_address; -use crate::utils::handle_transaction_result; +use crate::utils; #[derive(Debug, Clone, PartialEq)] pub enum ResourceType { @@ -104,7 +103,7 @@ where let model_name = parse_cairo_short_string(&mc.model)?; match world_reader.model_reader(&model_name).await { Ok(_) => { - let contract = get_contract_address(world, mc.contract).await?; + let contract = utils::get_contract_address(world, mc.contract).await?; calls.push(world.grant_writer_getcall(&mc.model, &contract.into())); } @@ -126,7 +125,7 @@ where .await .with_context(|| "Failed to send transaction")?; - handle_transaction_result( + utils::handle_transaction_result( &world.account.provider(), res, transaction.wait, @@ -152,7 +151,7 @@ where let resource = match &or.resource { ResourceType::Model(name) => *name, ResourceType::Contract(name_or_address) => { - get_contract_address(world, name_or_address.clone()).await? + utils::get_contract_address(world, name_or_address.clone()).await? } }; @@ -162,7 +161,93 @@ where let res = world.account.execute(calls).send().await.with_context(|| "Failed to send transaction")?; - handle_transaction_result( + utils::handle_transaction_result( + &world.account.provider(), + res, + transaction.wait, + transaction.receipt, + ) + .await?; + + Ok(()) +} + +pub async fn revoke_writer( + world: &WorldContract, + models_contracts: Vec, + transaction: TxConfig, +) -> Result<()> +where + A: ConnectedAccount + Sync + Send + 'static, +{ + let mut calls = Vec::new(); + + let world_reader = WorldContractReader::new(world.address, world.account.provider()) + .with_block(BlockId::Tag(BlockTag::Pending)); + + for mc in models_contracts { + let model_name = parse_cairo_short_string(&mc.model)?; + match world_reader.model_reader(&model_name).await { + Ok(_) => { + let contract = utils::get_contract_address(world, mc.contract).await?; + calls.push(world.revoke_writer_getcall(&mc.model, &contract.into())); + } + + Err(ModelError::ModelNotFound) => { + println!("Unknown model '{}' => IGNORED", model_name); + } + + Err(err) => { + return Err(err.into()); + } + } + } + + if !calls.is_empty() { + let res = world + .account + .execute(calls) + .send() + .await + .with_context(|| "Failed to send transaction")?; + + utils::handle_transaction_result( + &world.account.provider(), + res, + transaction.wait, + transaction.receipt, + ) + .await?; + } + + Ok(()) +} + +pub async fn revoke_owner( + world: &WorldContract, + owners_resources: Vec, + transaction: TxConfig, +) -> Result<()> +where + A: ConnectedAccount + Sync + Send + 'static, +{ + let mut calls = Vec::new(); + + for or in owners_resources { + let resource = match &or.resource { + ResourceType::Model(name) => *name, + ResourceType::Contract(name_or_address) => { + utils::get_contract_address(world, name_or_address.clone()).await? + } + }; + + calls.push(world.revoke_owner_getcall(&or.owner.into(), &resource)); + } + + let res = + world.account.execute(calls).send().await.with_context(|| "Failed to send transaction")?; + + utils::handle_transaction_result( &world.account.provider(), res, transaction.wait, diff --git a/crates/sozo/ops/src/execute.rs b/crates/sozo/ops/src/execute.rs index 467056274f..8530dc50c9 100644 --- a/crates/sozo/ops/src/execute.rs +++ b/crates/sozo/ops/src/execute.rs @@ -5,8 +5,7 @@ use starknet::accounts::{Call, ConnectedAccount}; use starknet::core::types::FieldElement; use starknet::core::utils::get_selector_from_name; -use super::get_contract_address; -use crate::utils::handle_transaction_result; +use crate::utils; pub async fn execute( contract: String, @@ -18,7 +17,7 @@ pub async fn execute( where A: ConnectedAccount + Sync + Send + 'static, { - let contract_address = get_contract_address(world, contract).await?; + let contract_address = utils::get_contract_address(world, contract).await?; let res = world .account .execute(vec![Call { @@ -30,6 +29,11 @@ where .await .with_context(|| "Failed to send transaction")?; - handle_transaction_result(&world.account.provider(), res, transaction.wait, transaction.receipt) - .await + utils::handle_transaction_result( + &world.account.provider(), + res, + transaction.wait, + transaction.receipt, + ) + .await } diff --git a/crates/sozo/ops/src/lib.rs b/crates/sozo/ops/src/lib.rs index 94217a7b2a..2ba2bf238e 100644 --- a/crates/sozo/ops/src/lib.rs +++ b/crates/sozo/ops/src/lib.rs @@ -1,9 +1,3 @@ -use anyhow::Result; -use dojo_world::contracts::world::WorldContract; -use dojo_world::migration::strategy::generate_salt; -use starknet::accounts::ConnectedAccount; -use starknet::core::types::FieldElement; - pub mod auth; pub mod events; pub mod execute; @@ -14,20 +8,3 @@ pub mod utils; #[cfg(test)] pub mod tests; - -pub async fn get_contract_address( - world: &WorldContract, - name_or_address: String, -) -> Result { - if name_or_address.starts_with("0x") { - FieldElement::from_hex_be(&name_or_address).map_err(anyhow::Error::from) - } else { - let contract_class_hash = world.base().call().await?; - Ok(starknet::core::utils::get_contract_address( - generate_salt(&name_or_address), - contract_class_hash.into(), - &[], - world.address, - )) - } -} diff --git a/crates/sozo/ops/src/tests/auth.rs b/crates/sozo/ops/src/tests/auth.rs index 3c686865ac..ae5a97355a 100644 --- a/crates/sozo/ops/src/tests/auth.rs +++ b/crates/sozo/ops/src/tests/auth.rs @@ -1,62 +1,62 @@ -use anyhow::Result; -use dojo_test_utils::compiler::build_test_config; -use dojo_test_utils::migration::prepare_migration; use dojo_test_utils::sequencer::{ get_default_test_starknet_config, SequencerConfig, TestSequencer, }; use dojo_world::contracts::world::WorldContract; use dojo_world::migration::TxConfig; -use scarb::ops; -use starknet::accounts::{Account, ConnectedAccount, SingleOwnerAccount}; -use starknet::core::types::{BlockId, BlockTag}; +use starknet::accounts::{Account, ConnectedAccount}; use starknet::core::utils::cairo_short_string_to_felt; -use starknet::providers::jsonrpc::HttpTransport; -use starknet::providers::JsonRpcClient; -use starknet::signers::LocalWallet; +use super::setup; use crate::auth::{self, ModelContract, OwnerResource, ResourceType}; -use crate::{execute, migration}; +use crate::execute; const ACTION_CONTRACT_NAME: &str = "dojo_examples::actions::actions"; -/// Setups the project by migrating the spawn-and-moves project. -/// -/// # Returns -/// -/// A [`WorldContract`] initialized with the migrator account, -/// the account 0 of the sequencer. -async fn setup( - sequencer: &TestSequencer, -) -> Result, LocalWallet>>> { - let config = build_test_config("../../../examples/spawn-and-move/Scarb.toml")?; - let ws = ops::read_workspace(config.manifest_path(), &config) - .unwrap_or_else(|op| panic!("Error building workspace: {op:?}")); - let base_dir = "../../../examples/spawn-and-move"; - let target_dir = format!("{}/target/dev", base_dir); - - let migration = prepare_migration(base_dir.into(), target_dir.into())?; - - let mut account = sequencer.account(); - account.set_block_id(BlockId::Tag(BlockTag::Pending)); - - let output = migration::execute_strategy( - &ws, - &migration, - &account, - Some(TxConfig { wait: true, ..Default::default() }), +#[tokio::test(flavor = "multi_thread")] +async fn auth_grant_writer_ok() { + let sequencer = + TestSequencer::start(SequencerConfig::default(), get_default_test_starknet_config()).await; + + let world = setup::setup(&sequencer).await.unwrap(); + + // Shouldn't have any permission at this point. + let account2 = sequencer.account_at_index(2); + + // Setup new world contract handler with account 2. + let world_2 = WorldContract::new(world.address, account2); + + assert!(!execute_spawn(&world_2).await); + + // Account2 does not have the permission to write, but granting + // writer to the actions contract allows the execution of it's systems by + // any account. + let moves_mc = ModelContract { + model: cairo_short_string_to_felt("Moves").unwrap(), + contract: ACTION_CONTRACT_NAME.to_string(), + }; + + let position_mc = ModelContract { + model: cairo_short_string_to_felt("Position").unwrap(), + contract: ACTION_CONTRACT_NAME.to_string(), + }; + + auth::grant_writer( + &world, + vec![moves_mc, position_mc], + TxConfig { wait: true, ..Default::default() }, ) - .await?; - let world = WorldContract::new(output.world_address, account); + .await + .unwrap(); - Ok(world) + assert!(execute_spawn(&world_2).await); } #[tokio::test(flavor = "multi_thread")] -async fn auth_grant_writer_ok() { +async fn auth_revoke_writer_ok() { let sequencer = TestSequencer::start(SequencerConfig::default(), get_default_test_starknet_config()).await; - let world = setup(&sequencer).await.unwrap(); + let world = setup::setup(&sequencer).await.unwrap(); // Shouldn't have any permission at this point. let account2 = sequencer.account_at_index(2); @@ -79,15 +79,29 @@ async fn auth_grant_writer_ok() { contract: ACTION_CONTRACT_NAME.to_string(), }; + // Here we are granting the permission to write auth::grant_writer( &world, - vec![moves_mc, position_mc], + vec![moves_mc.clone(), position_mc.clone()], TxConfig { wait: true, ..Default::default() }, ) .await .unwrap(); + // This should be executable now assert!(execute_spawn(&world_2).await); + + // Here we are revoking the access again. + auth::revoke_writer( + &world, + vec![moves_mc, position_mc], + TxConfig { wait: true, ..Default::default() }, + ) + .await + .unwrap(); + + // Here it shouldn't be executable. + assert!(!execute_spawn(&world_2).await); } #[tokio::test(flavor = "multi_thread")] @@ -95,7 +109,7 @@ async fn auth_grant_owner_ok() { let sequencer = TestSequencer::start(SequencerConfig::default(), get_default_test_starknet_config()).await; - let world = setup(&sequencer).await.unwrap(); + let world = setup::setup(&sequencer).await.unwrap(); // Shouldn't have any permission at this point. let account_2 = sequencer.account_at_index(2); @@ -125,6 +139,53 @@ async fn auth_grant_owner_ok() { assert!(execute_spawn(&world_2).await); } +#[tokio::test(flavor = "multi_thread")] +async fn auth_revoke_owner_ok() { + let sequencer = + TestSequencer::start(SequencerConfig::default(), get_default_test_starknet_config()).await; + + let world = setup::setup(&sequencer).await.unwrap(); + + // Shouldn't have any permission at this point. + let account_2 = sequencer.account_at_index(2); + let account_2_addr = account_2.address(); + + // Setup new world contract handler with account 2. + let world_2 = WorldContract::new(world.address, account_2); + + assert!(!execute_spawn(&world_2).await); + + // Account2 does not have the permission to write, let's give this account + // ownership of both models. + let moves = OwnerResource { + resource: ResourceType::Model(cairo_short_string_to_felt("Moves").unwrap()), + owner: account_2_addr, + }; + + let position = OwnerResource { + resource: ResourceType::Model(cairo_short_string_to_felt("Position").unwrap()), + owner: account_2_addr, + }; + + auth::grant_owner( + &world, + vec![moves.clone(), position.clone()], + TxConfig { wait: true, ..Default::default() }, + ) + .await + .unwrap(); + + assert!(execute_spawn(&world_2).await); + auth::revoke_owner( + &world, + vec![moves, position], + TxConfig { wait: true, ..Default::default() }, + ) + .await + .unwrap(); + + assert!(!execute_spawn(&world_2).await); +} /// Executes the `spawn` system on `actions` contract. /// /// # Returns diff --git a/crates/sozo/ops/src/tests/mod.rs b/crates/sozo/ops/src/tests/mod.rs index 0e4a05d597..1cc5eab8d1 100644 --- a/crates/sozo/ops/src/tests/mod.rs +++ b/crates/sozo/ops/src/tests/mod.rs @@ -1 +1,3 @@ pub mod auth; +pub mod setup; +pub mod utils; diff --git a/crates/sozo/ops/src/tests/setup.rs b/crates/sozo/ops/src/tests/setup.rs new file mode 100644 index 0000000000..1d5457e368 --- /dev/null +++ b/crates/sozo/ops/src/tests/setup.rs @@ -0,0 +1,46 @@ +use anyhow::Result; +use dojo_test_utils::compiler::build_test_config; +use dojo_test_utils::migration::prepare_migration; +use dojo_test_utils::sequencer::TestSequencer; +use dojo_world::contracts::world::WorldContract; +use dojo_world::migration::TxConfig; +use scarb::ops; +use starknet::accounts::SingleOwnerAccount; +use starknet::core::types::{BlockId, BlockTag}; +use starknet::providers::jsonrpc::HttpTransport; +use starknet::providers::JsonRpcClient; +use starknet::signers::LocalWallet; + +use crate::migration; + +/// Setups the project by migrating the full spawn-and-moves project. +/// +/// # Returns +/// +/// A [`WorldContract`] initialized with the migrator account, +/// the account 0 of the sequencer. +pub async fn setup( + sequencer: &TestSequencer, +) -> Result, LocalWallet>>> { + let config = build_test_config("../../../examples/spawn-and-move/Scarb.toml")?; + let ws = ops::read_workspace(config.manifest_path(), &config) + .unwrap_or_else(|op| panic!("Error building workspace: {op:?}")); + let base_dir = "../../../examples/spawn-and-move"; + let target_dir = format!("{}/target/dev", base_dir); + + let migration = prepare_migration(base_dir.into(), target_dir.into())?; + + let mut account = sequencer.account(); + account.set_block_id(BlockId::Tag(BlockTag::Pending)); + + let output = migration::execute_strategy( + &ws, + &migration, + &account, + Some(TxConfig { wait: true, ..Default::default() }), + ) + .await?; + let world = WorldContract::new(output.world_address, account); + + Ok(world) +} diff --git a/crates/sozo/ops/src/tests/utils.rs b/crates/sozo/ops/src/tests/utils.rs new file mode 100644 index 0000000000..92d22fb0b6 --- /dev/null +++ b/crates/sozo/ops/src/tests/utils.rs @@ -0,0 +1,36 @@ +use dojo_test_utils::sequencer::{ + get_default_test_starknet_config, SequencerConfig, TestSequencer, +}; +use dojo_world::contracts::world::WorldContract; +use starknet::core::types::FieldElement; + +use super::setup; +use crate::utils; + +const ACTION_CONTRACT_NAME: &str = "dojo_examples::actions::actions"; + +#[tokio::test(flavor = "multi_thread")] +async fn get_contract_address_from_world() { + let sequencer = + TestSequencer::start(SequencerConfig::default(), get_default_test_starknet_config()).await; + + let world = setup::setup(&sequencer).await.unwrap(); + + let contract_address = + utils::get_contract_address(&world, ACTION_CONTRACT_NAME.to_string()).await.unwrap(); + + assert!(contract_address != FieldElement::ZERO); +} + +#[tokio::test(flavor = "multi_thread")] +async fn get_contract_address_from_string() { + let sequencer = + TestSequencer::start(SequencerConfig::default(), get_default_test_starknet_config()).await; + + let account = sequencer.account(); + let world = WorldContract::new(FieldElement::ZERO, account); + + let contract_address = utils::get_contract_address(&world, "0x1234".to_string()).await.unwrap(); + + assert_eq!(contract_address, FieldElement::from_hex_be("0x1234").unwrap()); +} diff --git a/crates/sozo/ops/src/utils.rs b/crates/sozo/ops/src/utils.rs index a2aaf99f96..673a9a9a29 100644 --- a/crates/sozo/ops/src/utils.rs +++ b/crates/sozo/ops/src/utils.rs @@ -1,8 +1,49 @@ use anyhow::Result; +use dojo_world::contracts::world::WorldContract; +use dojo_world::migration::strategy::generate_salt; use dojo_world::utils::{execution_status_from_maybe_pending_receipt, TransactionWaiter}; -use starknet::core::types::{ExecutionResult, InvokeTransactionResult}; +use starknet::accounts::ConnectedAccount; +use starknet::core::types::{ExecutionResult, FieldElement, InvokeTransactionResult}; use starknet::providers::Provider; +/// Retrieves a contract address from it's name +/// using the world's data, or parses a hex string into +/// a [`FieldElement`]. +/// +/// # Arguments +/// +/// * `world` - The world's contract connector. +/// * `name_or_address` - A string with a contract name or a hexadecimal address. +/// +/// # Returns +/// +/// A [`FieldElement`] with the address of the contract on success. +pub async fn get_contract_address( + world: &WorldContract, + name_or_address: String, +) -> Result { + if name_or_address.starts_with("0x") { + FieldElement::from_hex_be(&name_or_address).map_err(anyhow::Error::from) + } else { + let contract_class_hash = world.base().call().await?; + Ok(starknet::core::utils::get_contract_address( + generate_salt(&name_or_address), + contract_class_hash.into(), + &[], + world.address, + )) + } +} + +/// Handles a transaction result configuring a +/// [`TransactionWaiter`] if required. +/// +/// # Arguments +/// +/// * `provider` - Starknet provider to fetch transaction status. +/// * `transaction_result` - Result of the transaction to handle. +/// * `wait_for_tx` - Wait for the transaction to be mined. +/// * `show_receipt` - If the receipt of the transaction should be displayed on stdout. pub async fn handle_transaction_result

( provider: P, transaction_result: InvokeTransactionResult,