diff --git a/CHANGELOG.md b/CHANGELOG.md index 86bf673..d51d511 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,27 @@ +# 0.3.0 (TBD) + +## BREAKING CHANGES: + +### Pausable Plugin + +- Replaced the combined `manager_roles` attribute with separate `pause_roles` and `unpause_roles` attributes for the `Pausable` plugin. +- This allows for separate permissions for pausing and unpausing features, enabling more granular access control. + +To migrate from the previous version: +```rust +// Old format +#[pausable(manager_roles(Role::PauseManager))] + +// New format +#[pausable( + pause_roles(Role::PauseManager), + unpause_roles(Role::UnpauseManager) +)] +``` + +See the [migration guide](docs/migrations/pausable-separate-roles.md) for more details. + + # 0.2.0 (TBD) ## BREAKING CHANGES: diff --git a/docs/migrations/pausable-separate-roles.md b/docs/migrations/pausable-separate-roles.md new file mode 100644 index 0000000..b5315dd --- /dev/null +++ b/docs/migrations/pausable-separate-roles.md @@ -0,0 +1,114 @@ +# Migrating to Separate Pause/Unpause Roles + +This guide explains how to migrate your code to use the new `pause_roles` and `unpause_roles` attributes instead of the consolidated `manager_roles` attribute in the Pausable plugin. + +## Changes Required + +### Before + +Previously, you would define permissions for both pausing and unpausing using a single attribute: + +```rust +#[pausable(manager_roles(Role::PauseManager))] +struct Contract { + // Contract fields +} +``` + +This meant that any account with the `PauseManager` role could both pause and unpause features. + +### After + +Now, you need to specify permissions for pausing and unpausing separately: + +```rust +#[pausable( + pause_roles(Role::PauseManager), + unpause_roles(Role::UnpauseManager) +)] +struct Contract { + // Contract fields +} +``` + +With this change, you can: +- Grant an account only the ability to pause features (emergency response) +- Grant a different account only the ability to unpause features (recovery process) +- Grant some accounts both abilities + +## Step-by-Step Migration + +1. **Update your Role enum** to include separate roles for pausing and unpausing, if desired: + + ```rust + #[derive(AccessControlRole, Deserialize, Serialize, Copy, Clone)] + #[serde(crate = "near_sdk::serde")] + pub enum Role { + // Previous role that could both pause and unpause + // PauseManager, + + // New separate roles + PauseManager, // Can only pause features + UnpauseManager, // Can only unpause features + // Other roles... + } + ``` + +2. **Update the pausable attribute** to use the new format: + + ```rust + // Old format + // #[pausable(manager_roles(Role::PauseManager))] + + // New format + #[pausable( + pause_roles(Role::PauseManager), + unpause_roles(Role::UnpauseManager) + )] + ``` + +3. **Update contract initialization** to grant the appropriate roles: + + ```rust + #[init] + pub fn new(pause_manager: AccountId, unpause_manager: AccountId) -> Self { + let mut contract = Self { + // contract fields + }; + + // Make the contract itself super admin + contract.acl_init_super_admin(env::current_account_id()); + + // Grant pause role + contract.acl_grant_role(Role::PauseManager.into(), pause_manager); + + // Grant unpause role (might be the same or different account) + contract.acl_grant_role(Role::UnpauseManager.into(), unpause_manager); + + contract + } + ``` + +4. **Update tests** to test both pause and unpause permissions separately. + +## Example + +Here's a complete example of a contract using the new separated roles: + +```rust +#[access_control(role_type(Role))] +#[near(contract_state)] +#[derive(Pausable, PanicOnDefault)] +#[pausable( + pause_roles(Role::PauseManager, Role::EmergencyPauser), + unpause_roles(Role::UnpauseManager, Role::ServiceRestorer) +)] +pub struct Counter { + counter: u64, +} +``` + +In this example: +- Accounts with either `PauseManager` or `EmergencyPauser` roles can pause features +- Accounts with either `UnpauseManager` or `ServiceRestorer` roles can unpause features +- An account might have multiple roles (e.g., both pause and unpause capabilities) \ No newline at end of file diff --git a/near-plugins-derive/src/pausable.rs b/near-plugins-derive/src/pausable.rs index 3c109e4..00164ab 100644 --- a/near-plugins-derive/src/pausable.rs +++ b/near-plugins-derive/src/pausable.rs @@ -12,8 +12,10 @@ struct Opts { /// Storage key under which the set of paused features is stored. If it is /// `None` the default value will be used. paused_storage_key: Option, - /// Access control roles whose grantees may pause and unpause features. - manager_roles: PathList, + /// Access control roles whose grantees may pause features. + pause_roles: PathList, + /// Access control roles whose grantees may unpause features. + unpause_roles: PathList, } /// Generates the token stream that implements `Pausable`. @@ -27,10 +29,15 @@ pub fn derive_pausable(input: TokenStream) -> TokenStream { let paused_storage_key = opts .paused_storage_key .unwrap_or_else(|| "__PAUSE__".to_string()); - let manager_roles = opts.manager_roles; + let pause_roles = opts.pause_roles; + let unpause_roles = opts.unpause_roles; assert!( - manager_roles.len() > 0, - "Specify at least one role for manager_roles" + pause_roles.len() > 0, + "Specify at least one role for pause_roles" + ); + assert!( + unpause_roles.len() > 0, + "Specify at least one role for unpause_roles" ); let output = quote! { @@ -53,7 +60,7 @@ pub fn derive_pausable(input: TokenStream) -> TokenStream { }) } - #[#cratename::access_control_any(roles(#(#manager_roles),*))] + #[#cratename::access_control_any(roles(#(#pause_roles),*))] fn pa_pause_feature(&mut self, key: String) -> bool { let mut paused_keys = self.pa_all_paused().unwrap_or_default(); let newly_paused = paused_keys.insert(key.clone()); @@ -80,7 +87,7 @@ pub fn derive_pausable(input: TokenStream) -> TokenStream { true } - #[#cratename::access_control_any(roles(#(#manager_roles),*))] + #[#cratename::access_control_any(roles(#(#unpause_roles),*))] fn pa_unpause_feature(&mut self, key: String) -> bool { let mut paused_keys = self.pa_all_paused().unwrap_or_default(); let was_paused = paused_keys.remove(&key); diff --git a/near-plugins-derive/tests/contracts/pausable/src/lib.rs b/near-plugins-derive/tests/contracts/pausable/src/lib.rs index 8c66f4a..bd78545 100644 --- a/near-plugins-derive/tests/contracts/pausable/src/lib.rs +++ b/near-plugins-derive/tests/contracts/pausable/src/lib.rs @@ -10,8 +10,10 @@ use near_sdk::{env, near, AccountId, PanicOnDefault}; #[derive(AccessControlRole, Deserialize, Serialize, Copy, Clone)] #[serde(crate = "near_sdk::serde")] pub enum Role { - /// May pause and unpause features. + /// May pause features. PauseManager, + /// May unpause features. + UnpauseManager, /// May call `increase_4` even when it is paused. Unrestricted4Increaser, /// May call `decrease_4` even when `increase_4` is not paused. @@ -23,7 +25,7 @@ pub enum Role { #[access_control(role_type(Role))] #[near(contract_state)] #[derive(Pausable, PanicOnDefault)] -#[pausable(manager_roles(Role::PauseManager))] +#[pausable(pause_roles(Role::PauseManager), unpause_roles(Role::UnpauseManager))] pub struct Counter { counter: u64, } @@ -34,13 +36,12 @@ impl Counter { /// /// * Making the contract itself super admin. /// * Granting `Role::PauseManager` to the account id `pause_manager`. + /// * Granting `Role::UnpauseManager` to the account id `unpause_manager`. /// /// For a general overview of access control, please refer to the `AccessControllable` plugin. #[init] - pub fn new(pause_manager: AccountId) -> Self { - let mut contract = Self { - counter: 0, - }; + pub fn new(pause_manager: AccountId, unpause_manager: AccountId) -> Self { + let mut contract = Self { counter: 0 }; // Make the contract itself super admin. This allows us to grant any role in the // constructor. @@ -51,7 +52,11 @@ impl Counter { // Grant `Role::PauseManager` to the provided account. let result = contract.acl_grant_role(Role::PauseManager.into(), pause_manager); - near_sdk::require!(Some(true) == result, "Failed to grant role"); + near_sdk::require!(Some(true) == result, "Failed to grant pause role"); + + // Grant `Role::UnpauseManager` to the provided account. + let result = contract.acl_grant_role(Role::UnpauseManager.into(), unpause_manager); + near_sdk::require!(Some(true) == result, "Failed to grant unpause role"); contract } @@ -99,7 +104,10 @@ impl Counter { /// Similar to `#[if_paused]` but roles passed as argument may successfully call the method even /// when the feature is _not_ paused. - #[if_paused(name = "increase_4", except(roles(Role::Unrestricted4Decreaser)))] + #[if_paused( + name = "increase_4", + except(roles(Role::Unrestricted4Decreaser, Role::Unrestricted4Modifier)) + )] pub fn decrease_4(&mut self) { self.counter -= 4; } diff --git a/near-plugins-derive/tests/pausable.rs b/near-plugins-derive/tests/pausable.rs index d01bb71..ed579b8 100644 --- a/near-plugins-derive/tests/pausable.rs +++ b/near-plugins-derive/tests/pausable.rs @@ -30,8 +30,10 @@ struct Setup { /// Wrapper around the deployed contract that facilitates interacting with /// methods provided by the `AccessControllable` plugin. acl_contract: AccessControllableContract, - /// An account with permission to pause and unpause features. + /// An account with permission to pause features. pause_manager: Account, + /// An account with permission to unpause features. + unpause_manager: Account, /// A newly created account without any `AccessControllable` permissions. unauth_account: Account, } @@ -48,10 +50,12 @@ impl Setup { // Call the contract's constructor. let pause_manager = worker.dev_create_account().await?; + let unpause_manager = worker.dev_create_account().await?; contract .call("new") .args_json(json!({ "pause_manager": pause_manager.id(), + "unpause_manager": unpause_manager.id(), })) .max_gas() .transact() @@ -65,6 +69,7 @@ impl Setup { pausable_contract, acl_contract, pause_manager, + unpause_manager, unauth_account, }) } @@ -186,19 +191,25 @@ async fn assert_pause_feature_acl_failure(contract: &PausableContract, caller: & } #[tokio::test] -/// Only accounts that were granted a manager role may pause features. +/// Only accounts that were granted a pause role may pause features. async fn test_pause_not_allowed_from_unauthorized_account() -> anyhow::Result<()> { let Setup { pausable_contract, unauth_account, + unpause_manager, .. } = Setup::new().await?; + // Unauthorized account cannot pause assert_pause_feature_acl_failure(&pausable_contract, &unauth_account).await; + + // Unpause manager cannot pause either + assert_pause_feature_acl_failure(&pausable_contract, &unpause_manager).await; + Ok(()) } #[tokio::test] -/// If not granted a manager role, the contract itself may not pause features. +/// If not granted a pause role, the contract itself may not pause features. async fn test_pause_not_allowed_from_self() -> anyhow::Result<()> { let Setup { contract, @@ -227,7 +238,7 @@ async fn test_unpause_feature() -> anyhow::Result<()> { // Unpause a feature that is paused. The method it protected can then be called successfully. let res = setup .pausable_contract - .pa_unpause_feature(&setup.pause_manager, "increase_1") + .pa_unpause_feature(&setup.unpause_manager, "increase_1") .await?; assert_success_with(res, true); setup @@ -238,7 +249,7 @@ async fn test_unpause_feature() -> anyhow::Result<()> { // Unpause a feature that is not paused. let res = setup .pausable_contract - .pa_unpause_feature(&setup.pause_manager, "increase_1") + .pa_unpause_feature(&setup.unpause_manager, "increase_1") .await?; assert_success_with(res, false); setup @@ -258,24 +269,30 @@ async fn assert_unpause_feature_acl_failure(contract: &PausableContract, caller: assert_insufficient_acl_permissions( result, "pa_unpause_feature", - vec!["PauseManager".to_string()], + vec!["UnpauseManager".to_string()], ); } #[tokio::test] -/// Only accounts that were granted a manager role may unpause features. +/// Only accounts that were granted an unpause role may unpause features. async fn test_unpause_not_allowed_from_unauthorized_account() -> anyhow::Result<()> { let Setup { pausable_contract, unauth_account, + pause_manager, .. } = Setup::new().await?; + // Unauthorized account cannot unpause assert_unpause_feature_acl_failure(&pausable_contract, &unauth_account).await; + + // Pause manager cannot unpause either + assert_unpause_feature_acl_failure(&pausable_contract, &pause_manager).await; + Ok(()) } #[tokio::test] -/// If not granted a manager role, the contract itself may not unpause features. +/// If not granted an unpause role, the contract itself may not unpause features. async fn test_unpause_not_allowed_from_self() -> anyhow::Result<()> { let Setup { contract, @@ -322,7 +339,7 @@ async fn test_pause_with_all_allows_except() -> anyhow::Result<()> { assert_success_with_unit_return(res); let res = setup .pausable_contract - .pa_unpause_feature(&setup.pause_manager, "ALL") + .pa_unpause_feature(&setup.unpause_manager, "ALL") .await?; assert_success_with(res, true); assert_eq!(setup.get_counter().await?, 4); @@ -384,7 +401,7 @@ async fn test_work_after_unpause() -> anyhow::Result<()> { // After unpausing function call succeeds. let res = setup .pausable_contract - .pa_unpause_feature(&setup.pause_manager, "increase_1") + .pa_unpause_feature(&setup.unpause_manager, "increase_1") .await?; assert_success_with(res, true); let res = setup @@ -440,7 +457,7 @@ async fn test_paused_list() -> anyhow::Result<()> { let res = setup .pausable_contract - .pa_unpause_feature(&setup.pause_manager, "feature_a") + .pa_unpause_feature(&setup.unpause_manager, "feature_a") .await?; assert_success_with(res, true); assert_paused_list( @@ -485,7 +502,7 @@ async fn test_is_paused() -> anyhow::Result<()> { let res = setup .pausable_contract - .pa_unpause_feature(&setup.pause_manager, "feature_a") + .pa_unpause_feature(&setup.unpause_manager, "feature_a") .await?; assert_success_with(res, true); assert_is_paused( @@ -499,6 +516,37 @@ async fn test_is_paused() -> anyhow::Result<()> { Ok(()) } +/// Test combining both pause and unpause roles in one account +#[tokio::test] +async fn test_combined_pause_unpause_roles() -> anyhow::Result<()> { + let setup = Setup::new().await?; + + // Create a new account with both pause and unpause roles + let combined_account = setup.worker.dev_create_account().await?; + setup + .must_grant_acl_role("PauseManager", combined_account.id()) + .await; + setup + .must_grant_acl_role("UnpauseManager", combined_account.id()) + .await; + + // Test that the combined account can pause + let res = setup + .pausable_contract + .pa_pause_feature(&combined_account, "feature_c") + .await?; + assert_success_with(res, true); + + // Test that the combined account can unpause + let res = setup + .pausable_contract + .pa_unpause_feature(&combined_account, "feature_c") + .await?; + assert_success_with(res, true); + + Ok(()) +} + /// Pausing method name has no effect if the method has a custom feature name. #[tokio::test] async fn test_pause_custom_name_ok() -> anyhow::Result<()> { diff --git a/near-plugins/src/pausable.rs b/near-plugins/src/pausable.rs index c9af043..f12a824 100644 --- a/near-plugins/src/pausable.rs +++ b/near-plugins/src/pausable.rs @@ -49,7 +49,7 @@ pub trait Pausable { fn pa_all_paused(&self) -> Option>; /// Pauses feature `key`. This method fails if the caller has not been granted one of the access - /// control `manager_roles` passed to the `Pausable` plugin. + /// control `pause_roles` passed to the `Pausable` plugin. /// /// It returns `true` if the feature is paused as a result of this function call and `false` if /// the feature was already paused. In either case, the feature is paused after the function @@ -73,7 +73,7 @@ pub trait Pausable { fn pa_pause_feature(&mut self, key: String) -> bool; /// Unpauses feature `key`. This method fails if the caller has not been granted one of the - /// access control `manager_roles` passed to the `Pausable` plugin. + /// access control `unpause_roles` passed to the `Pausable` plugin. /// /// It returns whether the feature was paused, i.e. `true` if the feature was paused and /// otherwise `false`. In either case, the feature is unpaused after the function returns