Skip to content

Commit 0a89f54

Browse files
authored
get existing roles feature (#517)
* get existing roles feature * used persistent storage for existing roles
1 parent 17d26d9 commit 0a89f54

File tree

3 files changed

+275
-15
lines changed

3 files changed

+275
-15
lines changed

packages/access/src/access_control/mod.rs

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -95,11 +95,11 @@ use soroban_sdk::{contracterror, contractevent, Address, Env, Symbol};
9595

9696
pub use crate::access_control::storage::{
9797
accept_admin_transfer, add_to_role_enumeration, enforce_admin_auth,
98-
ensure_if_admin_or_admin_role, ensure_role, get_admin, get_role_admin, get_role_member,
99-
get_role_member_count, grant_role, grant_role_no_auth, has_role, remove_from_role_enumeration,
100-
remove_role_accounts_count_no_auth, remove_role_admin_no_auth, renounce_admin, renounce_role,
101-
revoke_role, revoke_role_no_auth, set_admin, set_role_admin, set_role_admin_no_auth,
102-
transfer_admin_role, AccessControlStorageKey,
98+
ensure_if_admin_or_admin_role, ensure_role, get_admin, get_existing_roles, get_role_admin,
99+
get_role_member, get_role_member_count, grant_role, grant_role_no_auth, has_role,
100+
remove_from_role_enumeration, remove_role_accounts_count_no_auth, remove_role_admin_no_auth,
101+
renounce_admin, renounce_role, revoke_role, revoke_role_no_auth, set_admin, set_role_admin,
102+
set_role_admin_no_auth, transfer_admin_role, AccessControlStorageKey,
103103
};
104104

105105
pub trait AccessControl {
@@ -175,6 +175,8 @@ pub trait AccessControl {
175175
///
176176
/// * [`AccessControlError::Unauthorized`] - If the caller does not have
177177
/// enough privileges.
178+
/// * [`AccessControlError::MaxRolesExceeded`] - If adding a new role would
179+
/// exceed the maximum allowed number of roles.
178180
///
179181
/// # Events
180182
///
@@ -346,13 +348,16 @@ pub enum AccessControlError {
346348
RoleNotHeld = 2007,
347349
RoleIsEmpty = 2008,
348350
TransferInProgress = 2009,
351+
MaxRolesExceeded = 2010,
349352
}
350353

351354
// ################## CONSTANTS ##################
352355

353356
const DAY_IN_LEDGERS: u32 = 17280;
354357
pub const ROLE_EXTEND_AMOUNT: u32 = 90 * DAY_IN_LEDGERS;
355358
pub const ROLE_TTL_THRESHOLD: u32 = ROLE_EXTEND_AMOUNT - DAY_IN_LEDGERS;
359+
/// Maximum number of roles that can exist simultaneously.
360+
pub const MAX_ROLES: u32 = 256;
356361

357362
// ################## EVENTS ##################
358363

packages/access/src/access_control/storage.rs

Lines changed: 61 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
1-
use soroban_sdk::{contracttype, panic_with_error, Address, Env, Symbol};
1+
use soroban_sdk::{contracttype, panic_with_error, Address, Env, Symbol, Vec};
22

33
use crate::{
44
access_control::{
55
emit_admin_renounced, emit_admin_transfer_completed, emit_admin_transfer_initiated,
66
emit_role_admin_changed, emit_role_granted, emit_role_revoked, AccessControlError,
7-
ROLE_EXTEND_AMOUNT, ROLE_TTL_THRESHOLD,
7+
MAX_ROLES, ROLE_EXTEND_AMOUNT, ROLE_TTL_THRESHOLD,
88
},
99
role_transfer::{accept_transfer, transfer_role},
1010
};
@@ -19,10 +19,11 @@ pub struct RoleAccountKey {
1919
/// Storage keys for the data associated with the access control
2020
#[contracttype]
2121
pub enum AccessControlStorageKey {
22+
ExistingRoles, // Vec<Symbol> of all existing roles
2223
RoleAccounts(RoleAccountKey), // (role, index) -> Address
2324
HasRole(Address, Symbol), // (account, role) -> index
24-
RoleAccountsCount(Symbol), // role -> count
25-
RoleAdmin(Symbol), // role -> the admin role
25+
RoleAccountsCount(Symbol), // role -> count of accounts with the role
26+
RoleAdmin(Symbol), // role -> the admin of the role
2627
Admin,
2728
PendingAdmin,
2829
}
@@ -42,7 +43,6 @@ pub enum AccessControlStorageKey {
4243
pub fn has_role(e: &Env, account: &Address, role: &Symbol) -> Option<u32> {
4344
let key = AccessControlStorageKey::HasRole(account.clone(), role.clone());
4445

45-
// extend ttl if `Some(index)`
4646
e.storage().persistent().get(&key).inspect(|_| {
4747
e.storage().persistent().extend_ttl(&key, ROLE_TTL_THRESHOLD, ROLE_EXTEND_AMOUNT)
4848
})
@@ -106,11 +106,29 @@ pub fn get_role_member(e: &Env, role: &Symbol, index: u32) -> Address {
106106
/// * `role` - The role to query the admin role for.
107107
pub fn get_role_admin(e: &Env, role: &Symbol) -> Option<Symbol> {
108108
let key = AccessControlStorageKey::RoleAdmin(role.clone());
109-
if let Some(admin_role) = e.storage().persistent().get(&key) {
109+
110+
e.storage().persistent().get(&key).inspect(|_| {
111+
e.storage().persistent().extend_ttl(&key, ROLE_TTL_THRESHOLD, ROLE_EXTEND_AMOUNT)
112+
})
113+
}
114+
115+
/// Returns a vector containing all existing roles.
116+
/// Defaults to empty vector if no roles exist.
117+
///
118+
/// # Arguments
119+
///
120+
/// * `e` - Access to Soroban environment.
121+
///
122+
/// # Notes
123+
///
124+
/// This function returns all roles that currently have at least one member.
125+
pub fn get_existing_roles(e: &Env) -> Vec<Symbol> {
126+
let key = AccessControlStorageKey::ExistingRoles;
127+
if let Some(existing_roles) = e.storage().persistent().get(&key) {
110128
e.storage().persistent().extend_ttl(&key, ROLE_TTL_THRESHOLD, ROLE_EXTEND_AMOUNT);
111-
Some(admin_role)
129+
existing_roles
112130
} else {
113-
None
131+
Vec::new(e)
114132
}
115133
}
116134

@@ -153,6 +171,7 @@ pub fn set_admin(e: &Env, admin: &Address) {
153171
/// # Errors
154172
///
155173
/// * refer to [`ensure_if_admin_or_admin_role`] errors.
174+
/// * refer to [`grant_role_no_auth`] errors.
156175
///
157176
/// # Events
158177
///
@@ -180,6 +199,10 @@ pub fn grant_role(e: &Env, account: &Address, role: &Symbol, caller: &Address) {
180199
/// * `role` - The role to grant.
181200
/// * `caller` - The address of the caller.
182201
///
202+
/// # Errors
203+
///
204+
/// * refer to [`add_to_role_enumeration`] errors.
205+
///
183206
/// # Events
184207
///
185208
/// * topics - `["role_granted", role: Symbol, account: Address]`
@@ -637,11 +660,29 @@ pub fn enforce_admin_auth(e: &Env) -> Address {
637660
/// * `e` - Access to Soroban environment.
638661
/// * `account` - The account to add to the role.
639662
/// * `role` - The role to add the account to.
663+
///
664+
/// # Errors
665+
///
666+
/// * [`AccessControlError::MaxRolesExceeded`] - If adding a new role would
667+
/// exceed the maximum allowed number of roles.
640668
pub fn add_to_role_enumeration(e: &Env, account: &Address, role: &Symbol) {
641669
// Get the current count of accounts with this role
642670
let count_key = AccessControlStorageKey::RoleAccountsCount(role.clone());
643671
let count = e.storage().persistent().get(&count_key).unwrap_or(0);
644672

673+
// If this is the first account with this role, add the role to ExistingRoles
674+
if count == 0 {
675+
let mut existing_roles = get_existing_roles(e);
676+
677+
// Check if we've reached the maximum number of roles
678+
if existing_roles.len() == MAX_ROLES {
679+
panic_with_error!(e, AccessControlError::MaxRolesExceeded);
680+
}
681+
682+
existing_roles.push_back(role.clone());
683+
e.storage().persistent().set(&AccessControlStorageKey::ExistingRoles, &existing_roles);
684+
}
685+
645686
// Add the account to the enumeration
646687
let new_key =
647688
AccessControlStorageKey::RoleAccounts(RoleAccountKey { role: role.clone(), index: count });
@@ -720,4 +761,16 @@ pub fn remove_from_role_enumeration(e: &Env, account: &Address, role: &Symbol) {
720761

721762
// Update the count
722763
e.storage().persistent().set(&count_key, &last_index);
764+
765+
// If this was the last account with this role, remove the role from
766+
// ExistingRoles
767+
if last_index == 0 {
768+
let mut existing_roles = get_existing_roles(e);
769+
770+
// Find and remove the role
771+
if let Some(pos) = existing_roles.iter().position(|r| r == role.clone()) {
772+
existing_roles.remove(pos as u32);
773+
e.storage().persistent().set(&AccessControlStorageKey::ExistingRoles, &existing_roles);
774+
}
775+
}
723776
}

packages/access/src/access_control/test.rs

Lines changed: 204 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@ use stellar_event_assertion::EventAssertion;
55

66
use crate::access_control::{
77
accept_admin_transfer, add_to_role_enumeration, ensure_if_admin_or_admin_role, get_admin,
8-
get_role_admin, get_role_member, get_role_member_count, grant_role, grant_role_no_auth,
9-
has_role, remove_from_role_enumeration, remove_role_accounts_count_no_auth,
8+
get_existing_roles, get_role_admin, get_role_member, get_role_member_count, grant_role,
9+
grant_role_no_auth, has_role, remove_from_role_enumeration, remove_role_accounts_count_no_auth,
1010
remove_role_admin_no_auth, renounce_admin, renounce_role, revoke_role, set_admin,
1111
set_role_admin, set_role_admin_no_auth, transfer_admin_role,
1212
};
@@ -685,3 +685,205 @@ fn renounce_admin_fails_when_transfer_in_progress() {
685685
renounce_admin(&e);
686686
});
687687
}
688+
689+
#[test]
690+
fn get_existing_roles_returns_empty_when_no_roles_exist() {
691+
let e = Env::default();
692+
e.mock_all_auths();
693+
let address = e.register(MockContract, ());
694+
695+
e.as_contract(&address, || {
696+
let roles = get_existing_roles(&e);
697+
assert_eq!(roles.len(), 0);
698+
});
699+
}
700+
701+
#[test]
702+
fn get_existing_roles_returns_roles_after_granting() {
703+
let e = Env::default();
704+
e.mock_all_auths();
705+
let address = e.register(MockContract, ());
706+
let admin = Address::generate(&e);
707+
let user = Address::generate(&e);
708+
709+
e.as_contract(&address, || {
710+
set_admin(&e, &admin);
711+
712+
// Grant first role
713+
grant_role(&e, &user, &USER_ROLE, &admin);
714+
715+
let roles = get_existing_roles(&e);
716+
assert_eq!(roles.len(), 1);
717+
assert_eq!(roles.get(0).unwrap(), USER_ROLE);
718+
});
719+
}
720+
721+
#[test]
722+
fn get_existing_roles_removes_role_when_last_account_removed() {
723+
let e = Env::default();
724+
e.mock_all_auths();
725+
let address = e.register(MockContract, ());
726+
let admin = Address::generate(&e);
727+
let user = Address::generate(&e);
728+
729+
e.as_contract(&address, || {
730+
set_admin(&e, &admin);
731+
732+
// Grant role
733+
grant_role(&e, &user, &USER_ROLE, &admin);
734+
735+
// Verify role exists
736+
let roles_before = get_existing_roles(&e);
737+
assert_eq!(roles_before.len(), 1);
738+
});
739+
740+
e.as_contract(&address, || {
741+
// Revoke role from last (and only) user
742+
revoke_role(&e, &user, &USER_ROLE, &admin);
743+
744+
// Verify role is removed from existing roles
745+
let roles_after = get_existing_roles(&e);
746+
assert_eq!(roles_after.len(), 0);
747+
});
748+
}
749+
750+
#[test]
751+
fn get_existing_roles_keeps_role_when_some_accounts_remain() {
752+
let e = Env::default();
753+
e.mock_all_auths();
754+
let address = e.register(MockContract, ());
755+
let admin = Address::generate(&e);
756+
let user1 = Address::generate(&e);
757+
let user2 = Address::generate(&e);
758+
759+
e.as_contract(&address, || {
760+
set_admin(&e, &admin);
761+
762+
// Grant role to two users
763+
grant_role(&e, &user1, &USER_ROLE, &admin);
764+
});
765+
766+
e.as_contract(&address, || {
767+
grant_role(&e, &user2, &USER_ROLE, &admin);
768+
769+
// Verify role exists
770+
let roles_before = get_existing_roles(&e);
771+
assert_eq!(roles_before.len(), 1);
772+
});
773+
774+
e.as_contract(&address, || {
775+
// Revoke role from one user (but another still has it)
776+
revoke_role(&e, &user1, &USER_ROLE, &admin);
777+
778+
// Verify role still exists
779+
let roles_after = get_existing_roles(&e);
780+
assert_eq!(roles_after.len(), 1);
781+
assert_eq!(roles_after.get(0).unwrap(), USER_ROLE);
782+
});
783+
}
784+
785+
#[test]
786+
fn get_existing_roles_does_not_create_duplicates() {
787+
let e = Env::default();
788+
e.mock_all_auths();
789+
let address = e.register(MockContract, ());
790+
let admin = Address::generate(&e);
791+
let user1 = Address::generate(&e);
792+
let user2 = Address::generate(&e);
793+
794+
e.as_contract(&address, || {
795+
set_admin(&e, &admin);
796+
797+
// Grant same role to multiple users
798+
grant_role(&e, &user1, &USER_ROLE, &admin);
799+
});
800+
801+
e.as_contract(&address, || {
802+
grant_role(&e, &user2, &USER_ROLE, &admin);
803+
804+
// Should still only have one role in the list
805+
let roles = get_existing_roles(&e);
806+
assert_eq!(roles.len(), 1);
807+
assert_eq!(roles.get(0).unwrap(), USER_ROLE);
808+
});
809+
}
810+
811+
#[test]
812+
#[should_panic(expected = "Error(Contract, #2010)")]
813+
fn grant_role_fails_when_max_roles_exceeded() {
814+
use crate::access_control::MAX_ROLES;
815+
816+
let e = Env::default();
817+
e.mock_all_auths();
818+
let address = e.register(MockContract, ());
819+
let admin = Address::generate(&e);
820+
let user = Address::generate(&e);
821+
822+
e.as_contract(&address, || {
823+
set_admin(&e, &admin);
824+
});
825+
826+
// Create MAX_ROLES roles
827+
for i in 0..MAX_ROLES {
828+
e.as_contract(&address, || {
829+
let role = Symbol::new(&e, &std::format!("role_{}", i));
830+
grant_role(&e, &user, &role, &admin);
831+
});
832+
}
833+
834+
e.as_contract(&address, || {
835+
// Verify we have MAX_ROLES
836+
let roles = get_existing_roles(&e);
837+
assert_eq!(roles.len(), MAX_ROLES);
838+
839+
// Try to create one more role - should panic
840+
let overflow_role = Symbol::new(&e, "overflow_role");
841+
grant_role(&e, &user, &overflow_role, &admin);
842+
});
843+
}
844+
845+
#[test]
846+
fn can_reuse_role_slot_after_removal() {
847+
use crate::access_control::MAX_ROLES;
848+
849+
let e = Env::default();
850+
e.mock_all_auths();
851+
let address = e.register(MockContract, ());
852+
let admin = Address::generate(&e);
853+
let user = Address::generate(&e);
854+
855+
e.as_contract(&address, || {
856+
set_admin(&e, &admin);
857+
});
858+
859+
// Create MAX_ROLES roles
860+
for i in 0..MAX_ROLES {
861+
e.as_contract(&address, || {
862+
let role = Symbol::new(&e, &std::format!("role_{}", i));
863+
grant_role(&e, &user, &role, &admin);
864+
});
865+
}
866+
e.as_contract(&address, || {
867+
// Verify we have MAX_ROLES
868+
let roles_before = get_existing_roles(&e);
869+
assert_eq!(roles_before.len(), MAX_ROLES);
870+
871+
// Remove one role
872+
let first_role = Symbol::new(&e, "role_0");
873+
revoke_role(&e, &user, &first_role, &admin);
874+
875+
// Verify we now have MAX_ROLES - 1
876+
let roles_after_removal = get_existing_roles(&e);
877+
assert_eq!(roles_after_removal.len(), MAX_ROLES - 1);
878+
});
879+
880+
e.as_contract(&address, || {
881+
// Now we should be able to add a new role
882+
let new_role = Symbol::new(&e, "new_role");
883+
grant_role(&e, &user, &new_role, &admin);
884+
885+
// Verify we're back to MAX_ROLES
886+
let roles_final = get_existing_roles(&e);
887+
assert_eq!(roles_final.len(), MAX_ROLES);
888+
});
889+
}

0 commit comments

Comments
 (0)