From c1c4100aeec9f1501700fa09dac9f837e0d75610 Mon Sep 17 00:00:00 2001 From: ussyalfaks Date: Tue, 29 Apr 2025 12:27:50 +0100 Subject: [PATCH 1/2] feat: implement permission system with operator management and tests --- src/base/errors.cairo | 8 +- src/base/types.cairo | 25 +++ src/chainlib/ChainLib.cairo | 175 +++++++++++++--- src/interfaces/IChainLib.cairo | 33 ++- tests/lib.cairo | 2 + tests/test_permissions.cairo | 360 +++++++++++++++++++++++++++++++++ 6 files changed, 570 insertions(+), 33 deletions(-) create mode 100644 tests/test_permissions.cairo diff --git a/src/base/errors.cairo b/src/base/errors.cairo index 8b13789..1147bc9 100644 --- a/src/base/errors.cairo +++ b/src/base/errors.cairo @@ -1 +1,7 @@ - +// Error constants for permission system +pub mod permission_errors { + pub const NO_PERMISSION: felt252 = 'You do not have permission'; + pub const NOT_ACCOUNT_OWNER: felt252 = 'Not the account owner'; + pub const PERMISSION_NOT_FOUND: felt252 = 'Permission not found'; + pub const INVALID_PERMISSION: felt252 = 'Invalid permission value'; +} diff --git a/src/base/types.cairo b/src/base/types.cairo index 163d40f..2d2274d 100644 --- a/src/base/types.cairo +++ b/src/base/types.cairo @@ -1,4 +1,5 @@ use starknet::ContractAddress; + #[derive(Drop, Serde, starknet::Store)] pub struct TokenBoundAccount { pub id: u256, @@ -8,7 +9,9 @@ pub struct TokenBoundAccount { pub init_param2: felt252, pub created_at: u64, pub updated_at: u64, + pub owner_permissions: Permissions, // Owner's permissions } + #[derive(Drop, Serde, starknet::Store)] pub struct User { pub id: u256, @@ -20,6 +23,28 @@ pub struct User { pub metadata: felt252, } +// Permission flags using bit flags for flexibility +#[derive(Drop, Copy, Serde, starknet::Store, Default, PartialEq)] +pub struct Permissions { + pub value: u64, // Using u64 to store permission bits +} + +// Permission constants +pub mod permission_flags { + // Basic permissions + pub const NONE: u64 = 0x0; + pub const FULL: u64 = 0xFFFFFFFFFFFFFFFF; + + // Specific permissions - using powers of 2 for bit flags + pub const READ: u64 = 0x1; // Can read account data + pub const WRITE: u64 = 0x2; // Can update account data + pub const TRANSFER: u64 = 0x4; // Can transfer tokens + pub const MANAGE_PERMISSIONS: u64 = 0x8; // Can update permissions + pub const EXECUTE: u64 = 0x10; // Can execute transactions + pub const MANAGE_OPERATORS: u64 = 0x20; // Can add/remove operators + pub const UPGRADE: u64 = 0x40; // Can upgrade the account + pub const DELETE: u64 = 0x80; // Can delete the account +} #[derive(Drop, Serde, starknet::Store, Clone, PartialEq)] pub enum Role { diff --git a/src/chainlib/ChainLib.cairo b/src/chainlib/ChainLib.cairo index a84186f..0eb2081 100644 --- a/src/chainlib/ChainLib.cairo +++ b/src/chainlib/ChainLib.cairo @@ -6,7 +6,7 @@ pub mod ChainLib { }; use starknet::{ContractAddress, get_block_timestamp, get_caller_address}; use crate::interfaces::IChainLib::IChainLib; - use crate::base::types::{TokenBoundAccount, User, Role, Rank}; + use crate::base::types::{TokenBoundAccount, User, Role, Rank, Permissions, permission_flags}; #[derive(Copy, Drop, Serde, starknet::Store, PartialEq, Debug)] pub enum ContentType { @@ -48,7 +48,9 @@ pub mod ChainLib { users: Map, creators_content: Map::, content: Map::, - content_tags: Map::> + content_tags: Map::>, + // Permission system storage + operator_permissions: Map::<(u256, ContractAddress), Permissions>, // Maps account_id and operator to permissions } @@ -63,6 +65,10 @@ pub mod ChainLib { pub enum Event { TokenBoundAccountCreated: TokenBoundAccountCreated, UserCreated: UserCreated, + // Permission-related events + PermissionGranted: PermissionGranted, + PermissionRevoked: PermissionRevoked, + PermissionModified: PermissionModified, } #[derive(Drop, starknet::Event)] @@ -75,15 +81,28 @@ pub mod ChainLib { pub id: u256, } + // Permission-related events + #[derive(Drop, starknet::Event)] + pub struct PermissionGranted { + pub account_id: u256, + pub operator: ContractAddress, + pub permissions: Permissions, + } + + #[derive(Drop, starknet::Event)] + pub struct PermissionRevoked { + pub account_id: u256, + pub operator: ContractAddress, + } + + #[derive(Drop, starknet::Event)] + pub struct PermissionModified { + pub account_id: u256, + pub permissions: Permissions, + } + #[abi(embed_v0)] impl ChainLibNetImpl of IChainLib { - /// @notice Creates a new token-bound account. - /// @dev This function generates a unique ID, initializes the account, and emits an event. - /// @param self The contract state reference. - /// @param user_name The unique username associated with the token-bound account. - /// @param init_param1 An initialization parameter required for the account setup. - /// @param init_param2 An additional initialization parameter. - /// @return account_id The unique identifier assigned to the token-bound account. fn create_token_account( ref self: ContractState, user_name: felt252, init_param1: felt252, init_param2: felt252 ) -> u256 { @@ -95,16 +114,21 @@ pub mod ChainLib { // Retrieve the current account ID before incrementing. let account_id = self.current_account_id.read(); + let caller = get_caller_address(); + + // Create default full permissions for the owner + let owner_permissions = Permissions { value: permission_flags::FULL }; // Create a new token-bound account with the provided parameters. let new_token_bound_account = TokenBoundAccount { id: account_id, - address: get_caller_address(), // Assign the caller's address. + address: caller, // Assign the caller's address. user_name: user_name, init_param1: init_param1, init_param2: init_param2, created_at: get_block_timestamp(), // Capture the creation timestamp. - updated_at: get_block_timestamp() // Set initial updated timestamp. + updated_at: get_block_timestamp(), // Set initial updated timestamp. + owner_permissions: owner_permissions, // Set owner permissions }; // Store the new account in the accounts mapping. @@ -133,16 +157,6 @@ pub mod ChainLib { } - /// @notice Registers a new user in the system. - /// @dev This function assigns a unique ID to the user, stores their profile, and emits an - /// event. - /// @param self The contract state reference. - /// @param username The unique username of the user. - /// @param wallet_address The blockchain address of the user. - /// @param role The role of the user (READER or WRITER). - /// @param rank The rank/level of the user. - /// @param metadata Additional metadata associated with the user. - /// @return user_id The unique identifier assigned to the user. fn register_user( ref self: ContractState, username: felt252, role: Role, rank: Rank, metadata: felt252 ) -> u256 { @@ -177,11 +191,6 @@ pub mod ChainLib { } - /// @notice Verifies a user in the system. - /// @dev Only an admin can verify a user. - /// @param self The contract state reference. - /// @param user_id The unique identifier of the user to be verified. - /// @return bool Returns true if the user is successfully verified. fn verify_user(ref self: ContractState, user_id: u256) -> bool { let caller = get_caller_address(); // Ensure that only an admin can verify users. @@ -191,11 +200,6 @@ pub mod ChainLib { self.users.write(user.id, user); true } - /// @notice Retrieves a user's profile from the system. - /// @dev This function fetches the user profile based on the provided user ID. - /// @param self The contract state reference. - /// @param user_id The unique identifier of the user whose profile is being retrieved. - /// @return User The user profile associated with the given user ID. fn retrieve_user_profile(ref self: ContractState, user_id: u256) -> User { // Read the user profile from the storage mapping. let user = self.users.read(user_id); @@ -214,5 +218,114 @@ pub mod ChainLib { let admin = self.admin.read(); admin } + + // Permission system implementation + + fn get_permissions( + self: @ContractState, account_id: u256, operator: ContractAddress + ) -> Permissions { + let account = self.accounts.read(account_id); + + // If the operator is the owner, return the owner's permissions + if operator == account.address { + return account.owner_permissions; + } + + // Otherwise, return the operator's permissions + self.operator_permissions.read((account_id, operator)) + } + + fn set_operator_permissions( + ref self: ContractState, + account_id: u256, + operator: ContractAddress, + permissions: Permissions + ) -> bool { + let caller = get_caller_address(); + let account = self.accounts.read(account_id); + + // Ensure that the caller is the account owner or has MANAGE_OPERATORS permission + let caller_permissions = self.get_permissions(account_id, caller); + assert( + account.address == caller || + (caller_permissions.value & permission_flags::MANAGE_OPERATORS) != 0, + 'No permission' + ); + + // Store the operator's permissions + self.operator_permissions.write((account_id, operator), permissions); + + // Emit the permission granted event + self.emit(PermissionGranted { + account_id, + operator, + permissions + }); + + true + } + + fn revoke_operator( + ref self: ContractState, + account_id: u256, + operator: ContractAddress + ) -> bool { + let caller = get_caller_address(); + let account = self.accounts.read(account_id); + + // Ensure that the caller is the account owner or has MANAGE_OPERATORS permission + let caller_permissions = self.get_permissions(account_id, caller); + assert( + account.address == caller || + (caller_permissions.value & permission_flags::MANAGE_OPERATORS) != 0, + 'No permission' + ); + + // Set permissions to NONE + let none_permissions = Permissions { value: permission_flags::NONE }; + self.operator_permissions.write((account_id, operator), none_permissions); + + // Emit the permission revoked event + self.emit(PermissionRevoked { + account_id, + operator + }); + + true + } + + fn has_permission( + self: @ContractState, + account_id: u256, + operator: ContractAddress, + permission: u64 + ) -> bool { + let permissions = self.get_permissions(account_id, operator); + (permissions.value & permission) != 0 + } + + fn modify_account_permissions( + ref self: ContractState, + account_id: u256, + permissions: Permissions + ) -> bool { + let caller = get_caller_address(); + let mut account = self.accounts.read(account_id); + + // Ensure that the caller is the account owner + assert(account.address == caller, 'Not owner'); + + // Update the owner's permissions + account.owner_permissions = permissions; + self.accounts.write(account_id, account); + + // Emit the permission modified event + self.emit(PermissionModified { + account_id, + permissions + }); + + true + } } } diff --git a/src/interfaces/IChainLib.cairo b/src/interfaces/IChainLib.cairo index 4f90038..fd91855 100644 --- a/src/interfaces/IChainLib.cairo +++ b/src/interfaces/IChainLib.cairo @@ -1,5 +1,5 @@ use starknet::ContractAddress; -use crate::base::types::{TokenBoundAccount, User, Role, Rank}; +use crate::base::types::{TokenBoundAccount, User, Role, Rank, Permissions}; #[starknet::interface] pub trait IChainLib { @@ -20,5 +20,36 @@ pub trait IChainLib { fn retrieve_user_profile(ref self: TContractState, user_id: u256) -> User; fn getAdmin(self: @TContractState) -> ContractAddress; fn is_verified(ref self: TContractState, user_id: u256) -> bool; + + // Permission system + fn get_permissions( + self: @TContractState, account_id: u256, operator: ContractAddress + ) -> Permissions; + + fn set_operator_permissions( + ref self: TContractState, + account_id: u256, + operator: ContractAddress, + permissions: Permissions + ) -> bool; + + fn revoke_operator( + ref self: TContractState, + account_id: u256, + operator: ContractAddress + ) -> bool; + + fn has_permission( + self: @TContractState, + account_id: u256, + operator: ContractAddress, + permission: u64 + ) -> bool; + + fn modify_account_permissions( + ref self: TContractState, + account_id: u256, + permissions: Permissions + ) -> bool; } diff --git a/tests/lib.cairo b/tests/lib.cairo index a0a3575..c5b91b0 100644 --- a/tests/lib.cairo +++ b/tests/lib.cairo @@ -1,3 +1,5 @@ #[cfg(test)] pub mod test_ChainLib; +#[cfg(test)] +pub mod test_permissions; diff --git a/tests/test_permissions.cairo b/tests/test_permissions.cairo new file mode 100644 index 0000000..b2965cd --- /dev/null +++ b/tests/test_permissions.cairo @@ -0,0 +1,360 @@ +#[cfg(test)] +mod permission_tests { + use chain_lib::chainlib::ChainLib; + use chain_lib::interfaces::IChainLib::{IChainLib, IChainLibDispatcher, IChainLibDispatcherTrait}; + use snforge_std::{CheatSpan, ContractClassTrait, DeclareResultTrait, cheat_caller_address, declare}; + use starknet::ContractAddress; + use starknet::class_hash::ClassHash; + use starknet::contract_address::contract_address_const; + use starknet::get_caller_address; + use chain_lib::base::types::{Permissions, permission_flags}; + + fn setup() -> (ContractAddress, ContractAddress) { + let declare_result = declare("ChainLib"); + assert(declare_result.is_ok(), 'declare failed'); + let admin_address: ContractAddress = contract_address_const::<'admin'>(); + + let contract_class = declare_result.unwrap().contract_class(); + let mut calldata = array![admin_address.into()]; + + let deploy_result = contract_class.deploy(@calldata); + assert(deploy_result.is_ok(), 'deploy failed'); + + let (contract_address, _) = deploy_result.unwrap(); + + (contract_address, admin_address) + } + + #[test] + fn test_token_account_owner_permissions() { + let (contract_address, _) = setup(); + let dispatcher = IChainLibDispatcher { contract_address }; + + // Test input values + let user_name: felt252 = 'Alice'; + let init_param1: felt252 = 'alice@mail.com'; + let init_param2: felt252 = 'alice profile'; + + // Create account + let account_id = dispatcher.create_token_account(user_name, init_param1, init_param2); + + // Get the token account + let token_account = dispatcher.get_token_bound_account(account_id); + + // Verify the account has full permissions for the owner + assert(token_account.owner_permissions.value == permission_flags::FULL, 'wrong perm'); + } + + #[test] + fn test_set_and_get_operator_permissions() { + let (contract_address, _) = setup(); + let dispatcher = IChainLibDispatcher { contract_address }; + + // Test input values + let user_name: felt252 = 'Bob'; + let init_param1: felt252 = 'bob@mail.com'; + let init_param2: felt252 = 'bob profile'; + + // Create account + let account_id = dispatcher.create_token_account(user_name, init_param1, init_param2); + + // Create operator address + let operator_address: ContractAddress = contract_address_const::<'operator'>(); + + // Set READ and EXECUTE permissions for the operator + let permissions = Permissions { value: permission_flags::READ | permission_flags::EXECUTE }; + + // Grant permissions to the operator + let result = dispatcher.set_operator_permissions(account_id, operator_address, permissions); + assert(result, 'set perm failed'); + + // Check operator permissions + let operator_permissions = dispatcher.get_permissions(account_id, operator_address); + assert(operator_permissions.value == (permission_flags::READ | permission_flags::EXECUTE), 'wrong perm'); + + // Verify the operator has specific permissions + let has_read = dispatcher.has_permission(account_id, operator_address, permission_flags::READ); + let has_execute = dispatcher.has_permission(account_id, operator_address, permission_flags::EXECUTE); + let has_write = dispatcher.has_permission(account_id, operator_address, permission_flags::WRITE); + + assert(has_read, 'no READ perm'); + assert(has_execute, 'no EXEC perm'); + assert(!has_write, 'has WRITE perm'); + + // Revoke operator permissions + let revoke_result = dispatcher.revoke_operator(account_id, operator_address); + assert(revoke_result, 'revoke failed'); + + // Verify permissions after revocation + let has_read_after = dispatcher.has_permission(account_id, operator_address, permission_flags::READ); + assert(!has_read_after, 'still has READ'); + + // Check that operator permissions are set to NONE + let operator_permissions_after = dispatcher.get_permissions(account_id, operator_address); + assert(operator_permissions_after.value == permission_flags::NONE, 'not NONE'); + } + + #[test] + fn test_manage_operators_permission() { + let (contract_address, _) = setup(); + let dispatcher = IChainLibDispatcher { contract_address }; + + // Create a token account + let user_name = 'Eve'; + let init_param1 = 'eve@example.com'; + let init_param2 = 'eve profile'; + + let account_id = dispatcher.create_token_account(user_name, init_param1, init_param2); + + // Create operator addresses + let operator1: ContractAddress = contract_address_const::<'operator1'>(); + let operator2: ContractAddress = contract_address_const::<'operator2'>(); + + // Give operator1 the MANAGE_OPERATORS permission + let manage_permissions = Permissions { value: permission_flags::MANAGE_OPERATORS }; + dispatcher.set_operator_permissions(account_id, operator1, manage_permissions); + + // Switch caller to operator1 + cheat_caller_address(contract_address, operator1, CheatSpan::Indefinite); + + // Have operator1 set permissions for operator2 + let read_permissions = Permissions { value: permission_flags::READ }; + let result = dispatcher.set_operator_permissions(account_id, operator2, read_permissions); + assert(result, 'set perm failed'); + + // Verify operator2 has READ permission + let has_read = dispatcher.has_permission(account_id, operator2, permission_flags::READ); + assert(has_read, 'no READ perm'); + } + + #[test] + fn test_modify_account_permissions() { + let (contract_address, _) = setup(); + let dispatcher = IChainLibDispatcher { contract_address }; + + // Create token account + let user_name: felt252 = 'Charlie'; + let init_param1: felt252 = 'charlie@mail.com'; + let init_param2: felt252 = 'charlie profile'; + + let account_id = dispatcher.create_token_account(user_name, init_param1, init_param2); + + // Get initial permissions + let token_account = dispatcher.get_token_bound_account(account_id); + assert(token_account.owner_permissions.value == permission_flags::FULL, 'wrong init perm'); + + // Modify permissions - remove WRITE permission + let modified_permissions = Permissions { + value: permission_flags::FULL & ~permission_flags::WRITE + }; + + let result = dispatcher.modify_account_permissions(account_id, modified_permissions); + assert(result, 'mod perm failed'); + + // Verify permissions were updated + let updated_account = dispatcher.get_token_bound_account(account_id); + let has_write = (updated_account.owner_permissions.value & permission_flags::WRITE) == permission_flags::WRITE; + assert(!has_write, 'still has WRITE'); + } + + #[test] + fn test_multiple_operators() { + let (contract_address, _) = setup(); + let dispatcher = IChainLibDispatcher { contract_address }; + + // Create token account + let user_name: felt252 = 'Dave'; + let init_param1: felt252 = 'dave@mail.com'; + let init_param2: felt252 = 'dave profile'; + + let account_id = dispatcher.create_token_account(user_name, init_param1, init_param2); + + // Create three operator addresses + let operator1: ContractAddress = contract_address_const::<'op1'>(); + let operator2: ContractAddress = contract_address_const::<'op2'>(); + let operator3: ContractAddress = contract_address_const::<'op3'>(); + + // Assign different permissions to each operator + dispatcher.set_operator_permissions( + account_id, + operator1, + Permissions { value: permission_flags::READ } + ); + + dispatcher.set_operator_permissions( + account_id, + operator2, + Permissions { value: permission_flags::EXECUTE } + ); + + dispatcher.set_operator_permissions( + account_id, + operator3, + Permissions { value: permission_flags::WRITE } + ); + + // Verify each operator has correct permissions + assert( + dispatcher.has_permission(account_id, operator1, permission_flags::READ), + 'op1 no READ' + ); + assert( + !dispatcher.has_permission(account_id, operator1, permission_flags::EXECUTE), + 'op1 has EXEC' + ); + + assert( + dispatcher.has_permission(account_id, operator2, permission_flags::EXECUTE), + 'op2 no EXEC' + ); + assert( + !dispatcher.has_permission(account_id, operator2, permission_flags::READ), + 'op2 has READ' + ); + + assert( + dispatcher.has_permission(account_id, operator3, permission_flags::WRITE), + 'op3 no WRITE' + ); + assert( + !dispatcher.has_permission(account_id, operator3, permission_flags::READ), + 'op3 has READ' + ); + + // Revoke one operator and check others still have permissions + dispatcher.revoke_operator(account_id, operator1); + + assert( + !dispatcher.has_permission(account_id, operator1, permission_flags::READ), + 'op1 still has READ' + ); + assert( + dispatcher.has_permission(account_id, operator2, permission_flags::EXECUTE), + 'op2 lost EXEC' + ); + assert( + dispatcher.has_permission(account_id, operator3, permission_flags::WRITE), + 'op3 lost WRITE' + ); + } + + #[test] + fn test_permission_combinations() { + let (contract_address, _) = setup(); + let dispatcher = IChainLibDispatcher { contract_address }; + + // Create token account + let user_name: felt252 = 'Frank'; + let init_param1: felt252 = 'frank@mail.com'; + let init_param2: felt252 = 'frank profile'; + + let account_id = dispatcher.create_token_account(user_name, init_param1, init_param2); + + // Create operator address + let operator: ContractAddress = contract_address_const::<'operator'>(); + + // Test various permission combinations + let test_combinations = array![ + (permission_flags::READ | permission_flags::WRITE, 'READ+WRITE'), + (permission_flags::READ | permission_flags::EXECUTE, 'READ+EXEC'), + (permission_flags::WRITE | permission_flags::TRANSFER, 'WRITE+TRANSFER'), + (permission_flags::MANAGE_PERMISSIONS | permission_flags::MANAGE_OPERATORS, 'MANAGE combo') + ]; + + let mut i: u32 = 0; + while i < test_combinations.len() { + let (perm_value, _name) = *test_combinations.at(i); + + // Set the permission combination + dispatcher.set_operator_permissions( + account_id, + operator, + Permissions { value: perm_value } + ); + + // Verify permissions + let stored_perm = dispatcher.get_permissions(account_id, operator); + assert(stored_perm.value == perm_value, 'wrong combo perm'); + + // Revoke and reset for next test + dispatcher.revoke_operator(account_id, operator); + + i += 1; + } + } + + #[test] + #[should_panic(expected: 'No permission')] + fn test_unauthorized_set_operator() { + let (contract_address, _) = setup(); + let dispatcher = IChainLibDispatcher { contract_address }; + + // Create token account + let user_name: felt252 = 'Greg'; + let init_param1: felt252 = 'greg@mail.com'; + let init_param2: felt252 = 'greg profile'; + + let account_id = dispatcher.create_token_account(user_name, init_param1, init_param2); + + // Set up unauthorized caller + let unauthorized: ContractAddress = contract_address_const::<'hacker'>(); + cheat_caller_address(contract_address, unauthorized, CheatSpan::Indefinite); + + // Attempt to set operator permissions (should fail) + let operator: ContractAddress = contract_address_const::<'operator'>(); + let permissions = Permissions { value: permission_flags::READ }; + + // This call should panic with 'No permission' + dispatcher.set_operator_permissions(account_id, operator, permissions); + } + + #[test] + #[should_panic(expected: 'No permission')] + fn test_insufficient_permissions() { + let (contract_address, _) = setup(); + let dispatcher = IChainLibDispatcher { contract_address }; + + // Create token account + let user_name: felt252 = 'Henry'; + let init_param1: felt252 = 'henry@mail.com'; + let init_param2: felt252 = 'henry profile'; + + let account_id = dispatcher.create_token_account(user_name, init_param1, init_param2); + + // Create operator with only READ permission + let operator: ContractAddress = contract_address_const::<'operator'>(); + dispatcher.set_operator_permissions( + account_id, + operator, + Permissions { value: permission_flags::READ } + ); + + // Switch to operator + cheat_caller_address(contract_address, operator, CheatSpan::Indefinite); + + // Attempt to add another operator (should fail as it requires MANAGE_OPERATORS permission) + let new_operator: ContractAddress = contract_address_const::<'new_op'>(); + let permissions = Permissions { value: permission_flags::READ }; + + // This call should panic with 'No permission' + dispatcher.set_operator_permissions(account_id, new_operator, permissions); + } + + #[test] + fn test_nonexistent_account() { + let (contract_address, _) = setup(); + let dispatcher = IChainLibDispatcher { contract_address }; + + // Attempt to get permissions for a non-existent account + let nonexistent_account_id = 9999_u256; + let operator: ContractAddress = contract_address_const::<'operator'>(); + + // Test that we get default permissions (NONE) for non-existent account + let permissions = dispatcher.get_permissions(nonexistent_account_id, operator); + assert(permissions.value == permission_flags::NONE, 'should return NONE'); + + // Check has_permission also returns false + let has_read = dispatcher.has_permission(nonexistent_account_id, operator, permission_flags::READ); + assert(!has_read, 'should not have permission'); + } +} \ No newline at end of file From 82c45183712f76a41f885b516911f19a18b39791 Mon Sep 17 00:00:00 2001 From: ussyalfaks Date: Tue, 29 Apr 2025 15:44:59 +0100 Subject: [PATCH 2/2] refactor: clean up formatting and improve readability in permission-related code --- src/base/types.cairo | 14 +- src/chainlib/ChainLib.cairo | 83 +++++------- src/interfaces/IChainLib.cairo | 27 ++-- tests/test_permissions.cairo | 226 +++++++++++++++++---------------- 4 files changed, 168 insertions(+), 182 deletions(-) diff --git a/src/base/types.cairo b/src/base/types.cairo index 2d2274d..30fdc77 100644 --- a/src/base/types.cairo +++ b/src/base/types.cairo @@ -34,16 +34,16 @@ pub mod permission_flags { // Basic permissions pub const NONE: u64 = 0x0; pub const FULL: u64 = 0xFFFFFFFFFFFFFFFF; - + // Specific permissions - using powers of 2 for bit flags - pub const READ: u64 = 0x1; // Can read account data - pub const WRITE: u64 = 0x2; // Can update account data - pub const TRANSFER: u64 = 0x4; // Can transfer tokens + pub const READ: u64 = 0x1; // Can read account data + pub const WRITE: u64 = 0x2; // Can update account data + pub const TRANSFER: u64 = 0x4; // Can transfer tokens pub const MANAGE_PERMISSIONS: u64 = 0x8; // Can update permissions - pub const EXECUTE: u64 = 0x10; // Can execute transactions + pub const EXECUTE: u64 = 0x10; // Can execute transactions pub const MANAGE_OPERATORS: u64 = 0x20; // Can add/remove operators - pub const UPGRADE: u64 = 0x40; // Can upgrade the account - pub const DELETE: u64 = 0x80; // Can delete the account + pub const UPGRADE: u64 = 0x40; // Can upgrade the account + pub const DELETE: u64 = 0x80; // Can delete the account } #[derive(Drop, Serde, starknet::Store, Clone, PartialEq)] diff --git a/src/chainlib/ChainLib.cairo b/src/chainlib/ChainLib.cairo index 0eb2081..60e6ce5 100644 --- a/src/chainlib/ChainLib.cairo +++ b/src/chainlib/ChainLib.cairo @@ -50,7 +50,9 @@ pub mod ChainLib { content: Map::, content_tags: Map::>, // Permission system storage - operator_permissions: Map::<(u256, ContractAddress), Permissions>, // Maps account_id and operator to permissions + operator_permissions: Map::< + (u256, ContractAddress), Permissions + >, // Maps account_id and operator to permissions } @@ -225,106 +227,89 @@ pub mod ChainLib { self: @ContractState, account_id: u256, operator: ContractAddress ) -> Permissions { let account = self.accounts.read(account_id); - + // If the operator is the owner, return the owner's permissions if operator == account.address { return account.owner_permissions; } - + // Otherwise, return the operator's permissions self.operator_permissions.read((account_id, operator)) } - + fn set_operator_permissions( - ref self: ContractState, - account_id: u256, - operator: ContractAddress, + ref self: ContractState, + account_id: u256, + operator: ContractAddress, permissions: Permissions ) -> bool { let caller = get_caller_address(); let account = self.accounts.read(account_id); - + // Ensure that the caller is the account owner or has MANAGE_OPERATORS permission let caller_permissions = self.get_permissions(account_id, caller); assert( - account.address == caller || - (caller_permissions.value & permission_flags::MANAGE_OPERATORS) != 0, + account.address == caller + || (caller_permissions.value & permission_flags::MANAGE_OPERATORS) != 0, 'No permission' ); - + // Store the operator's permissions self.operator_permissions.write((account_id, operator), permissions); - + // Emit the permission granted event - self.emit(PermissionGranted { - account_id, - operator, - permissions - }); - + self.emit(PermissionGranted { account_id, operator, permissions }); + true } - + fn revoke_operator( - ref self: ContractState, - account_id: u256, - operator: ContractAddress + ref self: ContractState, account_id: u256, operator: ContractAddress ) -> bool { let caller = get_caller_address(); let account = self.accounts.read(account_id); - + // Ensure that the caller is the account owner or has MANAGE_OPERATORS permission let caller_permissions = self.get_permissions(account_id, caller); assert( - account.address == caller || - (caller_permissions.value & permission_flags::MANAGE_OPERATORS) != 0, + account.address == caller + || (caller_permissions.value & permission_flags::MANAGE_OPERATORS) != 0, 'No permission' ); - + // Set permissions to NONE let none_permissions = Permissions { value: permission_flags::NONE }; self.operator_permissions.write((account_id, operator), none_permissions); - + // Emit the permission revoked event - self.emit(PermissionRevoked { - account_id, - operator - }); - + self.emit(PermissionRevoked { account_id, operator }); + true } - + fn has_permission( - self: @ContractState, - account_id: u256, - operator: ContractAddress, - permission: u64 + self: @ContractState, account_id: u256, operator: ContractAddress, permission: u64 ) -> bool { let permissions = self.get_permissions(account_id, operator); (permissions.value & permission) != 0 } - + fn modify_account_permissions( - ref self: ContractState, - account_id: u256, - permissions: Permissions + ref self: ContractState, account_id: u256, permissions: Permissions ) -> bool { let caller = get_caller_address(); let mut account = self.accounts.read(account_id); - + // Ensure that the caller is the account owner assert(account.address == caller, 'Not owner'); - + // Update the owner's permissions account.owner_permissions = permissions; self.accounts.write(account_id, account); - + // Emit the permission modified event - self.emit(PermissionModified { - account_id, - permissions - }); - + self.emit(PermissionModified { account_id, permissions }); + true } } diff --git a/src/interfaces/IChainLib.cairo b/src/interfaces/IChainLib.cairo index fd91855..f50986d 100644 --- a/src/interfaces/IChainLib.cairo +++ b/src/interfaces/IChainLib.cairo @@ -25,31 +25,24 @@ pub trait IChainLib { fn get_permissions( self: @TContractState, account_id: u256, operator: ContractAddress ) -> Permissions; - + fn set_operator_permissions( - ref self: TContractState, - account_id: u256, - operator: ContractAddress, + ref self: TContractState, + account_id: u256, + operator: ContractAddress, permissions: Permissions ) -> bool; - + fn revoke_operator( - ref self: TContractState, - account_id: u256, - operator: ContractAddress + ref self: TContractState, account_id: u256, operator: ContractAddress ) -> bool; - + fn has_permission( - self: @TContractState, - account_id: u256, - operator: ContractAddress, - permission: u64 + self: @TContractState, account_id: u256, operator: ContractAddress, permission: u64 ) -> bool; - + fn modify_account_permissions( - ref self: TContractState, - account_id: u256, - permissions: Permissions + ref self: TContractState, account_id: u256, permissions: Permissions ) -> bool; } diff --git a/tests/test_permissions.cairo b/tests/test_permissions.cairo index b2965cd..34e5507 100644 --- a/tests/test_permissions.cairo +++ b/tests/test_permissions.cairo @@ -1,8 +1,12 @@ #[cfg(test)] mod permission_tests { use chain_lib::chainlib::ChainLib; - use chain_lib::interfaces::IChainLib::{IChainLib, IChainLibDispatcher, IChainLibDispatcherTrait}; - use snforge_std::{CheatSpan, ContractClassTrait, DeclareResultTrait, cheat_caller_address, declare}; + use chain_lib::interfaces::IChainLib::{ + IChainLib, IChainLibDispatcher, IChainLibDispatcherTrait + }; + use snforge_std::{ + CheatSpan, ContractClassTrait, DeclareResultTrait, cheat_caller_address, declare + }; use starknet::ContractAddress; use starknet::class_hash::ClassHash; use starknet::contract_address::contract_address_const; @@ -34,13 +38,13 @@ mod permission_tests { let user_name: felt252 = 'Alice'; let init_param1: felt252 = 'alice@mail.com'; let init_param2: felt252 = 'alice profile'; - + // Create account let account_id = dispatcher.create_token_account(user_name, init_param1, init_param2); - + // Get the token account let token_account = dispatcher.get_token_bound_account(account_id); - + // Verify the account has full permissions for the owner assert(token_account.owner_permissions.value == permission_flags::FULL, 'wrong perm'); } @@ -49,46 +53,53 @@ mod permission_tests { fn test_set_and_get_operator_permissions() { let (contract_address, _) = setup(); let dispatcher = IChainLibDispatcher { contract_address }; - + // Test input values let user_name: felt252 = 'Bob'; let init_param1: felt252 = 'bob@mail.com'; let init_param2: felt252 = 'bob profile'; - + // Create account let account_id = dispatcher.create_token_account(user_name, init_param1, init_param2); - + // Create operator address let operator_address: ContractAddress = contract_address_const::<'operator'>(); - + // Set READ and EXECUTE permissions for the operator let permissions = Permissions { value: permission_flags::READ | permission_flags::EXECUTE }; - + // Grant permissions to the operator let result = dispatcher.set_operator_permissions(account_id, operator_address, permissions); assert(result, 'set perm failed'); - + // Check operator permissions let operator_permissions = dispatcher.get_permissions(account_id, operator_address); - assert(operator_permissions.value == (permission_flags::READ | permission_flags::EXECUTE), 'wrong perm'); - + assert( + operator_permissions.value == (permission_flags::READ | permission_flags::EXECUTE), + 'wrong perm' + ); + // Verify the operator has specific permissions - let has_read = dispatcher.has_permission(account_id, operator_address, permission_flags::READ); - let has_execute = dispatcher.has_permission(account_id, operator_address, permission_flags::EXECUTE); - let has_write = dispatcher.has_permission(account_id, operator_address, permission_flags::WRITE); - + let has_read = dispatcher + .has_permission(account_id, operator_address, permission_flags::READ); + let has_execute = dispatcher + .has_permission(account_id, operator_address, permission_flags::EXECUTE); + let has_write = dispatcher + .has_permission(account_id, operator_address, permission_flags::WRITE); + assert(has_read, 'no READ perm'); assert(has_execute, 'no EXEC perm'); assert(!has_write, 'has WRITE perm'); - + // Revoke operator permissions let revoke_result = dispatcher.revoke_operator(account_id, operator_address); assert(revoke_result, 'revoke failed'); - + // Verify permissions after revocation - let has_read_after = dispatcher.has_permission(account_id, operator_address, permission_flags::READ); + let has_read_after = dispatcher + .has_permission(account_id, operator_address, permission_flags::READ); assert(!has_read_after, 'still has READ'); - + // Check that operator permissions are set to NONE let operator_permissions_after = dispatcher.get_permissions(account_id, operator_address); assert(operator_permissions_after.value == permission_flags::NONE, 'not NONE'); @@ -98,30 +109,30 @@ mod permission_tests { fn test_manage_operators_permission() { let (contract_address, _) = setup(); let dispatcher = IChainLibDispatcher { contract_address }; - + // Create a token account let user_name = 'Eve'; let init_param1 = 'eve@example.com'; let init_param2 = 'eve profile'; - + let account_id = dispatcher.create_token_account(user_name, init_param1, init_param2); - + // Create operator addresses let operator1: ContractAddress = contract_address_const::<'operator1'>(); let operator2: ContractAddress = contract_address_const::<'operator2'>(); - + // Give operator1 the MANAGE_OPERATORS permission let manage_permissions = Permissions { value: permission_flags::MANAGE_OPERATORS }; dispatcher.set_operator_permissions(account_id, operator1, manage_permissions); - + // Switch caller to operator1 cheat_caller_address(contract_address, operator1, CheatSpan::Indefinite); - + // Have operator1 set permissions for operator2 let read_permissions = Permissions { value: permission_flags::READ }; let result = dispatcher.set_operator_permissions(account_id, operator2, read_permissions); assert(result, 'set perm failed'); - + // Verify operator2 has READ permission let has_read = dispatcher.has_permission(account_id, operator2, permission_flags::READ); assert(has_read, 'no READ perm'); @@ -131,29 +142,30 @@ mod permission_tests { fn test_modify_account_permissions() { let (contract_address, _) = setup(); let dispatcher = IChainLibDispatcher { contract_address }; - + // Create token account let user_name: felt252 = 'Charlie'; let init_param1: felt252 = 'charlie@mail.com'; let init_param2: felt252 = 'charlie profile'; - + let account_id = dispatcher.create_token_account(user_name, init_param1, init_param2); - + // Get initial permissions let token_account = dispatcher.get_token_bound_account(account_id); assert(token_account.owner_permissions.value == permission_flags::FULL, 'wrong init perm'); - + // Modify permissions - remove WRITE permission - let modified_permissions = Permissions { - value: permission_flags::FULL & ~permission_flags::WRITE + let modified_permissions = Permissions { + value: permission_flags::FULL & ~permission_flags::WRITE }; - + let result = dispatcher.modify_account_permissions(account_id, modified_permissions); assert(result, 'mod perm failed'); - + // Verify permissions were updated let updated_account = dispatcher.get_token_bound_account(account_id); - let has_write = (updated_account.owner_permissions.value & permission_flags::WRITE) == permission_flags::WRITE; + let has_write = (updated_account.owner_permissions.value + & permission_flags::WRITE) == permission_flags::WRITE; assert(!has_write, 'still has WRITE'); } @@ -161,79 +173,75 @@ mod permission_tests { fn test_multiple_operators() { let (contract_address, _) = setup(); let dispatcher = IChainLibDispatcher { contract_address }; - + // Create token account let user_name: felt252 = 'Dave'; let init_param1: felt252 = 'dave@mail.com'; let init_param2: felt252 = 'dave profile'; - + let account_id = dispatcher.create_token_account(user_name, init_param1, init_param2); - + // Create three operator addresses let operator1: ContractAddress = contract_address_const::<'op1'>(); let operator2: ContractAddress = contract_address_const::<'op2'>(); let operator3: ContractAddress = contract_address_const::<'op3'>(); - + // Assign different permissions to each operator - dispatcher.set_operator_permissions( - account_id, - operator1, - Permissions { value: permission_flags::READ } - ); - - dispatcher.set_operator_permissions( - account_id, - operator2, - Permissions { value: permission_flags::EXECUTE } - ); - - dispatcher.set_operator_permissions( - account_id, - operator3, - Permissions { value: permission_flags::WRITE } - ); - + dispatcher + .set_operator_permissions( + account_id, operator1, Permissions { value: permission_flags::READ } + ); + + dispatcher + .set_operator_permissions( + account_id, operator2, Permissions { value: permission_flags::EXECUTE } + ); + + dispatcher + .set_operator_permissions( + account_id, operator3, Permissions { value: permission_flags::WRITE } + ); + // Verify each operator has correct permissions assert( - dispatcher.has_permission(account_id, operator1, permission_flags::READ), - 'op1 no READ' + dispatcher.has_permission(account_id, operator1, permission_flags::READ), 'op1 no READ' ); assert( - !dispatcher.has_permission(account_id, operator1, permission_flags::EXECUTE), + !dispatcher.has_permission(account_id, operator1, permission_flags::EXECUTE), 'op1 has EXEC' ); - + assert( - dispatcher.has_permission(account_id, operator2, permission_flags::EXECUTE), + dispatcher.has_permission(account_id, operator2, permission_flags::EXECUTE), 'op2 no EXEC' ); assert( - !dispatcher.has_permission(account_id, operator2, permission_flags::READ), + !dispatcher.has_permission(account_id, operator2, permission_flags::READ), 'op2 has READ' ); - + assert( - dispatcher.has_permission(account_id, operator3, permission_flags::WRITE), + dispatcher.has_permission(account_id, operator3, permission_flags::WRITE), 'op3 no WRITE' ); assert( - !dispatcher.has_permission(account_id, operator3, permission_flags::READ), + !dispatcher.has_permission(account_id, operator3, permission_flags::READ), 'op3 has READ' ); - + // Revoke one operator and check others still have permissions dispatcher.revoke_operator(account_id, operator1); - + assert( - !dispatcher.has_permission(account_id, operator1, permission_flags::READ), + !dispatcher.has_permission(account_id, operator1, permission_flags::READ), 'op1 still has READ' ); assert( - dispatcher.has_permission(account_id, operator2, permission_flags::EXECUTE), + dispatcher.has_permission(account_id, operator2, permission_flags::EXECUTE), 'op2 lost EXEC' ); assert( - dispatcher.has_permission(account_id, operator3, permission_flags::WRITE), + dispatcher.has_permission(account_id, operator3, permission_flags::WRITE), 'op3 lost WRITE' ); } @@ -242,43 +250,43 @@ mod permission_tests { fn test_permission_combinations() { let (contract_address, _) = setup(); let dispatcher = IChainLibDispatcher { contract_address }; - + // Create token account let user_name: felt252 = 'Frank'; let init_param1: felt252 = 'frank@mail.com'; let init_param2: felt252 = 'frank profile'; - + let account_id = dispatcher.create_token_account(user_name, init_param1, init_param2); - + // Create operator address let operator: ContractAddress = contract_address_const::<'operator'>(); - + // Test various permission combinations let test_combinations = array![ (permission_flags::READ | permission_flags::WRITE, 'READ+WRITE'), (permission_flags::READ | permission_flags::EXECUTE, 'READ+EXEC'), (permission_flags::WRITE | permission_flags::TRANSFER, 'WRITE+TRANSFER'), - (permission_flags::MANAGE_PERMISSIONS | permission_flags::MANAGE_OPERATORS, 'MANAGE combo') + ( + permission_flags::MANAGE_PERMISSIONS | permission_flags::MANAGE_OPERATORS, + 'MANAGE combo' + ) ]; - + let mut i: u32 = 0; while i < test_combinations.len() { let (perm_value, _name) = *test_combinations.at(i); - + // Set the permission combination - dispatcher.set_operator_permissions( - account_id, - operator, - Permissions { value: perm_value } - ); - + dispatcher + .set_operator_permissions(account_id, operator, Permissions { value: perm_value }); + // Verify permissions let stored_perm = dispatcher.get_permissions(account_id, operator); assert(stored_perm.value == perm_value, 'wrong combo perm'); - + // Revoke and reset for next test dispatcher.revoke_operator(account_id, operator); - + i += 1; } } @@ -288,22 +296,22 @@ mod permission_tests { fn test_unauthorized_set_operator() { let (contract_address, _) = setup(); let dispatcher = IChainLibDispatcher { contract_address }; - + // Create token account let user_name: felt252 = 'Greg'; let init_param1: felt252 = 'greg@mail.com'; let init_param2: felt252 = 'greg profile'; - + let account_id = dispatcher.create_token_account(user_name, init_param1, init_param2); - + // Set up unauthorized caller let unauthorized: ContractAddress = contract_address_const::<'hacker'>(); cheat_caller_address(contract_address, unauthorized, CheatSpan::Indefinite); - + // Attempt to set operator permissions (should fail) let operator: ContractAddress = contract_address_const::<'operator'>(); let permissions = Permissions { value: permission_flags::READ }; - + // This call should panic with 'No permission' dispatcher.set_operator_permissions(account_id, operator, permissions); } @@ -313,29 +321,28 @@ mod permission_tests { fn test_insufficient_permissions() { let (contract_address, _) = setup(); let dispatcher = IChainLibDispatcher { contract_address }; - + // Create token account let user_name: felt252 = 'Henry'; let init_param1: felt252 = 'henry@mail.com'; let init_param2: felt252 = 'henry profile'; - + let account_id = dispatcher.create_token_account(user_name, init_param1, init_param2); - + // Create operator with only READ permission let operator: ContractAddress = contract_address_const::<'operator'>(); - dispatcher.set_operator_permissions( - account_id, - operator, - Permissions { value: permission_flags::READ } - ); - + dispatcher + .set_operator_permissions( + account_id, operator, Permissions { value: permission_flags::READ } + ); + // Switch to operator cheat_caller_address(contract_address, operator, CheatSpan::Indefinite); - + // Attempt to add another operator (should fail as it requires MANAGE_OPERATORS permission) let new_operator: ContractAddress = contract_address_const::<'new_op'>(); let permissions = Permissions { value: permission_flags::READ }; - + // This call should panic with 'No permission' dispatcher.set_operator_permissions(account_id, new_operator, permissions); } @@ -344,17 +351,18 @@ mod permission_tests { fn test_nonexistent_account() { let (contract_address, _) = setup(); let dispatcher = IChainLibDispatcher { contract_address }; - + // Attempt to get permissions for a non-existent account let nonexistent_account_id = 9999_u256; let operator: ContractAddress = contract_address_const::<'operator'>(); - + // Test that we get default permissions (NONE) for non-existent account let permissions = dispatcher.get_permissions(nonexistent_account_id, operator); assert(permissions.value == permission_flags::NONE, 'should return NONE'); - + // Check has_permission also returns false - let has_read = dispatcher.has_permission(nonexistent_account_id, operator, permission_flags::READ); + let has_read = dispatcher + .has_permission(nonexistent_account_id, operator, permission_flags::READ); assert(!has_read, 'should not have permission'); } -} \ No newline at end of file +}