Skip to content

Commit

Permalink
Access control changes
Browse files Browse the repository at this point in the history
  • Loading branch information
MissingNO57 committed Feb 3, 2025
1 parent 4a7697c commit 9c3e1fc
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 53 deletions.
33 changes: 17 additions & 16 deletions src/access_control/contract.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use crate::access_control::AccessControl;
use crate::events::ResponseHandler;
use cosmwasm_schema::cw_serde;
use cosmwasm_std::{Addr, Deps, DepsMut, MessageInfo, Response, StdResult};
use cosmwasm_std::{Addr, Deps, DepsMut, Env, MessageInfo, Response, StdResult};
use strum::IntoEnumIterator;

#[cw_serde]
Expand Down Expand Up @@ -39,20 +38,21 @@ pub fn query_roles<T: IntoEnumIterator + AsRef<str>>(

pub fn execute_give_role_by_admin_role<T: AsRef<str>>(
deps: DepsMut,
env: Env,
info: MessageInfo,
role: T,
addr: Addr,
required_sender_role: T,
) -> StdResult<Response> {
// Only admin can give role
AccessControl::ensure_has_role(&deps.as_ref(), &required_sender_role, &info.sender)?;

let response_handler = ResponseHandler::default();

AccessControl::ensure_has_role_or_superadmin(
&deps.as_ref(),
&env,
&required_sender_role,
&info.sender,
)?;
AccessControl::storage_set_role(deps.storage, &role, &addr)?;

Ok(response_handler
.into_response()
Ok(Response::new()
.add_attribute("action", "give_role")
.add_attribute("sender", info.sender)
.add_attribute("role", role.as_ref())
Expand All @@ -61,20 +61,21 @@ pub fn execute_give_role_by_admin_role<T: AsRef<str>>(

pub fn execute_take_role_by_admin_role<T: AsRef<str>>(
deps: DepsMut,
env: Env,
info: MessageInfo,
role: T,
addr: Addr,
required_sender_role: T,
) -> StdResult<Response> {
// Only admin can take role
AccessControl::ensure_has_role(&deps.as_ref(), &required_sender_role, &info.sender)?;

let response_handler = ResponseHandler::default();

AccessControl::ensure_has_role_or_superadmin(
&deps.as_ref(),
&env,
&required_sender_role,
&info.sender,
)?;
AccessControl::storage_remove_role(deps.storage, &role, &addr)?;

Ok(response_handler
.into_response()
Ok(Response::new()
.add_attribute("action", "take_role")
.add_attribute("sender", info.sender)
.add_attribute("role", role.as_ref())
Expand Down
69 changes: 32 additions & 37 deletions src/access_control/role.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,28 +49,18 @@ impl<T: AsRef<str>> AccessControl<T> {
HAS_ROLE.has(storage, (role.as_ref(), address))
}

pub fn ensure_role_admin(deps: &Deps, env: &Env, sender: &Addr, role: &T) -> StdResult<()> {
pub fn ensure_role_admin(deps: &Deps, sender: &Addr, role: &T) -> StdResult<()> {
if let Some(role_admin) = Self::get_role_admin(deps.storage, role)? {
if role_admin == sender {
return Ok(());
}
}

if is_super_admin(deps, env, sender)? {
return Ok(());
}

Err(sender_is_not_role_admin_error(role))
}

pub fn give_role(
deps: &mut DepsMut,
env: &Env,
sender: &Addr,
role: &T,
address: &Addr,
) -> StdResult<()> {
Self::ensure_role_admin(&deps.as_ref(), env, sender, role)?;
pub fn give_role(deps: &mut DepsMut, sender: &Addr, role: &T, address: &Addr) -> StdResult<()> {
Self::ensure_role_admin(&deps.as_ref(), sender, role)?;
Self::storage_set_role(deps.storage, role, address)?;
Ok(())
}
Expand All @@ -80,14 +70,8 @@ impl<T: AsRef<str>> AccessControl<T> {
Ok(())
}

pub fn take_role(
deps: &mut DepsMut,
env: &Env,
sender: &Addr,
role: &T,
address: &Addr,
) -> StdResult<()> {
Self::ensure_role_admin(&deps.as_ref(), env, sender, role)?;
pub fn take_role(deps: &mut DepsMut, sender: &Addr, role: &T, address: &Addr) -> StdResult<()> {
Self::ensure_role_admin(&deps.as_ref(), sender, role)?;
Self::storage_remove_role(deps.storage, role, address)?;
Ok(())
}
Expand Down Expand Up @@ -123,12 +107,11 @@ impl<T: AsRef<str>> AccessControl<T> {

pub fn change_role_admin(
deps: DepsMut,
env: &Env,
sender: &Addr,
role: &T,
new_admin: &Addr,
) -> StdResult<()> {
Self::ensure_role_admin(&deps.as_ref(), env, sender, role)?;
Self::ensure_role_admin(&deps.as_ref(), sender, role)?;
Self::_set_role_admin(deps.storage, role, new_admin)
}

Expand All @@ -152,6 +135,19 @@ impl<T: AsRef<str>> AccessControl<T> {
}
}

pub fn ensure_has_role_or_superadmin(
deps: &Deps,
env: &Env,
role: &T,
address: &Addr,
) -> StdResult<()> {
if Self::has_role(deps.storage, role, address) || is_super_admin(deps, env, address)? {
Ok(())
} else {
Err(no_role_error(address, role))
}
}

pub fn ensure_has_roles(deps: &Deps, roles: &[T], address: &Addr) -> StdResult<()> {
for role in roles {
if Self::has_role(deps.storage, role, address) {
Expand Down Expand Up @@ -264,7 +260,7 @@ mod tests {
assert!(AccessControl::create_role(deps.as_mut().storage, &role, Some(&creator)).is_ok());

// Admin should be able to give role
assert!(AccessControl::give_role(&mut deps.as_mut(), &env, &creator, &role, &user).is_ok());
assert!(AccessControl::give_role(&mut deps.as_mut(), &creator, &role, &user).is_ok());

// Ensure the user has the role
assert!(AccessControl::has_role(deps.as_mut().storage, &role, &user));
Expand All @@ -283,13 +279,13 @@ mod tests {
assert!(AccessControl::create_role(deps.as_mut().storage, &role, Some(&creator)).is_ok());

// Admin should be able to give role
assert!(AccessControl::give_role(&mut deps.as_mut(), &env, &creator, &role, &user).is_ok());
assert!(AccessControl::give_role(&mut deps.as_mut(), &creator, &role, &user).is_ok());

// Ensure the user has the role
assert!(AccessControl::has_role(deps.as_mut().storage, &role, &user));

// Admin should be able to take role
assert!(AccessControl::take_role(&mut deps.as_mut(), &env, &creator, &role, &user).is_ok());
assert!(AccessControl::take_role(&mut deps.as_mut(), &creator, &role, &user).is_ok());

// Ensure the user no longer has the role
assert!(!AccessControl::has_role(
Expand Down Expand Up @@ -321,8 +317,7 @@ mod tests {

// Change the role admin
assert!(
AccessControl::change_role_admin(deps.as_mut(), &env, &creator, &role, &new_admin)
.is_ok()
AccessControl::change_role_admin(deps.as_mut(), &creator, &role, &new_admin).is_ok()
);

// Ensure the new role admin is set correctly
Expand All @@ -347,10 +342,10 @@ mod tests {
assert!(AccessControl::create_role(deps.as_mut().storage, &role, Some(&creator)).is_ok());

// Ensure role admin passes for the correct admin
assert!(AccessControl::ensure_role_admin(&deps.as_ref(), &env, &creator, &role).is_ok());
assert!(AccessControl::ensure_role_admin(&deps.as_ref(), &creator, &role).is_ok());

// Ensure role admin fails for someone who is not the admin
assert!(AccessControl::ensure_role_admin(&deps.as_ref(), &env, &other, &role).is_err());
assert!(AccessControl::ensure_role_admin(&deps.as_ref(), &other, &role).is_err());
}

#[test]
Expand All @@ -370,13 +365,13 @@ mod tests {
);

// Ensure role admin passes for the correct admin
assert!(AccessControl::ensure_role_admin(&deps.as_ref(), &env, &role_admin, &role).is_ok());
assert!(AccessControl::ensure_role_admin(&deps.as_ref(), &role_admin, &role).is_ok());

// Ensure super-admin is also role admin
assert!(AccessControl::ensure_role_admin(&deps.as_ref(), &env, &creator, &role).is_ok());
// Super-admin is not role admin
assert!(AccessControl::ensure_role_admin(&deps.as_ref(), &creator, &role).is_err());

// Ensure role admin fails for someone who is not the admin
assert!(AccessControl::ensure_role_admin(&deps.as_ref(), &env, &other, &role).is_err());
assert!(AccessControl::ensure_role_admin(&deps.as_ref(), &other, &role).is_err());
}

#[test]
Expand All @@ -392,10 +387,10 @@ mod tests {
// Create the role and set admin
assert!(AccessControl::create_role(deps.as_mut().storage, &role, None).is_ok());

// Ensure super-admin is only role admin
assert!(AccessControl::ensure_role_admin(&deps.as_ref(), &env, &creator, &role).is_ok());
// Super-admin is not role admin
assert!(AccessControl::ensure_role_admin(&deps.as_ref(), &creator, &role).is_err());

// Ensure role admin fails for someone who is not the admin
assert!(AccessControl::ensure_role_admin(&deps.as_ref(), &env, &other, &role).is_err());
assert!(AccessControl::ensure_role_admin(&deps.as_ref(), &other, &role).is_err());
}
}

0 comments on commit 9c3e1fc

Please sign in to comment.