[BUIDL Audition Onchain] SC-009: Build Comprehensive Role-Based Access Control System

[No-bodyq]

Closes: #73

This PR introduces a comprehensive Role-Based Access Control (RBAC) system with full support for dynamic roles, delegation, expiration, auditing, and enforcement modifiers. It includes a complete test suite covering all permission logic and edge cases.

What’s Included

Contracts

• AccessControl.cairo – core logic and persistent role storage
• RBACComponent.cairo – embeddable role-checking modifiers

Role Hierarchy

OWNER > ADMIN > SESSION_CREATOR > {JUDGE, REVIEWER, ORACLE} > USER

🔧 Role Features

• ✅ grant_role / revoke_role
• ✅ grant_temporary_role (time-limited permissions)
• ✅ delegate_session_role & revoke_session_delegation
• ✅ emergency_override_role
• ✅ batch_grant_roles / batch_revoke_roles
• ✅ has_role, has_session_role, is_role_expired
• ✅ Role-based function modifiers:
  only_admin, only_owner, only_role(role), etc.

Audit Logging

Every role change creates an on-chain AuditLog entry (granted, revoked, delegated, expired).

Events

Emits events for:

• RoleGranted
• RoleRevoked
• SessionRoleDelegated
• EmergencyOverride

Test Suite Highlights

Tests cover:

Feature	Coverage ✅
Role assignment & revocation	✔️
Time-limited roles	✔️
Delegation (per-session)	✔️
Role inheritance	✔️
Unauthorized access prevention	✔️
Batch grant/revoke	✔️
Emergency override	✔️
Audit trail integrity	✔️

[No-bodyq]

kindly review @CollinsC1O

[CollinsC1O]

Hi Boss,

1. so I can see your Role type and how it has proper role hierarchy establishment. but down to the constructor. You gave the admin the highest role privilege instead of the Owner (who deployed the contract). the role hierarchy should have been as such: OWNER > ADMIN > SESSION_CREATOR > JUDGE > REVIEWER > ORACLE > USER just as in Role type. So let’s have Role::OWNER.
After effecting the change, store using new multiple roles structure e.g: 

   self.user_roles.entry((owner, Role::OWNER)).write(owner_role_data); then initialise audit log

self.audit_log_count.entry(owner).write(1);
        
self
            
.audit_entries
            
.entry((owner, 1))
           
 .write(
                
 AuditLog {
                    
account: owner,
                    
role: Role::OWNER,  // Log correct role here
                    
action: 0,                  // granted
                    
timestamp: get_block_timestamp(),
                    
actor: owner,
               
 },
           
 );

self.initialized.write(true);


this is just an example showing my idea. Implement it to flow with your current implementation incase I mixed anything.

2. Let’s change the single role per user implementation in the storage to a multiple role per user support I.e from roles: Map<ContractAddress, Role> to user_roles: Map<(ContractAddress, Role), RoleData>. 
The thing is that your current implementation only allows one role per user, which is severely limiting in real-world scenarios where users might need multiple roles simultaneously (e.g., being both an ADMIN and a SESSION_CREATOR). Considering handling a user handle more than one role
3. With the session role storage. Considering the above requested change; your current implementation can only store one session delegation per session ID right? The thing is that this will create conflicts when multiple users need different roles in the same session so lets make multiple users to have different roles in the same session, this will help with no conflicts between session role assignments and also give a proper session-based access control. So lets go from “session_roles: Map<u256, SessionDelegation>” to “session_roles: Map<(u256, Role, ContractAddress), SessionDelegation>”.
4. Considering the current changes. Can you create two function which are _can_grant_role() and _can_revoke_role() with proper authorization logic considering the owner now has the highest role and should only be authorised to grant role. This is because your current implementation has weak authorization checks, allowing unauthorized role modifications.
5. Your implementation checks for role matches, kind missing the concept of role hierarchy where higher roles should inherit lower role permissions, so with this lets make an improvement where we have two functions, lets has_any_role() to check if em got role and has_permission() functions all with role hierarchy support
6. I have a request; considering the idea of improving the hierarchy to have the owner as the highest. Now with your revoke_role() function; prevent the last owner from being removed make an assertion for this and then you can create revoke_all_roles() function prevent all roles from being removed from last owner and then you can then get all roles for the account and revoke them
7. With the grant_temporary_role() function I will love you to add an implementation where prevent temporary OWNER role. Owner role should never be temporal, lets assert this
8. With delegate_session_role() function lets add a validation that assert or validate session role in appropriate e.g
assert(role == Role::JUDGE || role == Role::REVIEWER || role == Role::ORACLE 'Invalid session role')
9. with the revoke_session_delegation() function let have a logic that properly revoke session delegation: e.g

let revoked_delegation = SessionDelegation {
                session_id,
                role,
                delegated_to: account,
                delegated_by: caller,
                granted_at: 0, // mark as revoked
                expires_at: get_block_timestamp() // mark revocation time
            };

This is just an example you could make it flow with your logic

10. 10. With the has_session_role() function at the return delegation.role. addition of an extra “&& delegation.granted_at != 0” for “not revoked”
11. With the get_active_user_roles() function we could check each role type for active roles (non-expired roles) e.g:

 let all_roles = array![
                
Role::OWNER, Role::ADMIN, Role::SESSION_CREATOR, 
                
Role::JUDGE, Role::REVIEWER, Role::ORACLE, Role::USER
           
 ];
            
            
let mut i = 0;
            
while i < all_roles.len() {
               
 let role = *all_roles[i];
                
if self.has_role(role, account) {
                    
roles.append(role);
               
 }
             
i += 1;

};

12. With the emergency_override_role() function lets have it the 
a. only OWNER can use emergency override instead of admin.
b. Prevent emergency override of OWNERS role
13. Can you add implementation to Validate appropriate session roles and improve session role storage and retrieval and also fix session role validation logic. This suggestion is because your current implementation logic has not soo robust session role validation and storage, leading to potential security gaps. This gives us an advantage where only appropriate roles e.g (JUDGE, REVIEWER, ORACLE) can be delegated to sessions, proper session role lifecycle management and much more

[CollinsC1O]

Hey man @No-bodyq Gm Gm
what's the update on this issue