mirrors/mail-server
1
0
Fork 0
mirror of https://github.com/stalwartlabs/mail-server.git synced 2024-11-24 06:19:46 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 2

Verify roles and permissions when creating or modifying accounts (closes #874)
Some checks failed
trivy / Check (push) Has been cancelled
Details

Browse source
This commit is contained in:
mdecimus 2024-10-21 19:07:29 +02:00
parent db8b53693f
commit c380ec750a
6 changed files with 153 additions and 8 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
crates/common/src/auth/roles.rs
View file

@ -167,6 +167,12 @@ impl RolePermissions {
self.enabled.difference(&self.disabled);
self.enabled
}
pub fn finalize_as_ref(&self) -> Permissions {
let mut enabled = self.enabled.clone();
enabled.difference(&self.disabled);
enabled
}
}
fn tenant_admin_permissions() -> Arc<RolePermissions> {

56
crates/directory/src/backend/internal/manage.rs
View file

@ -16,7 +16,8 @@ use store::{
use trc::AddContext;
use crate::{
Permission, Principal, QueryBy, Type, MAX_TYPE_ID, ROLE_ADMIN, ROLE_TENANT_ADMIN, ROLE_USER,
Permission, Permissions, Principal, QueryBy, Type, MAX_TYPE_ID, ROLE_ADMIN, ROLE_TENANT_ADMIN,
ROLE_USER,
};
use super::{
@ -37,6 +38,7 @@ pub struct PrincipalList {
pub struct UpdatePrincipal<'x> {
query: QueryBy<'x>,
allowed_permissions: Option<&'x Permissions>,
changes: Vec<PrincipalUpdate>,
tenant_id: Option<u32>,
create_domains: bool,
@ -54,6 +56,7 @@ pub trait ManageDirectory: Sized {
&self,
principal: Principal,
tenant_id: Option<u32>,
allowed_permissions: Option<&Permissions>,
) -> trc::Result<u32>;
async fn update_principal(&self, params: UpdatePrincipal<'_>) -> trc::Result<()>;
async fn delete_principal(&self, by: QueryBy<'_>) -> trc::Result<()>;
@ -192,6 +195,7 @@ impl ManageDirectory for Store {
&self,
mut principal: Principal,
mut tenant_id: Option<u32>,
allowed_permissions: Option<&Permissions>,
) -> trc::Result<u32> {
// Make sure the principal has a name
let name = principal.name().to_lowercase();
@ -358,7 +362,18 @@ impl ManageDirectory for Store {
.id() as u64;
if !permissions.contains(&permission) {
permissions.push(permission);
if allowed_permissions
.as_ref()
.map_or(true, |p| p.get(permission as usize))
|| field == PrincipalField::DisabledPermissions
{
permissions.push(permission);
} else {
return Err(error(
"Invalid permission",
format!("Your account cannot grant the {name:?} permission").into(),
));
}
}
}
@ -1339,7 +1354,20 @@ impl ManageDirectory for Store {
.id() as u64;
if !permissions.contains(&permission) {
permissions.push(permission);
if params
.allowed_permissions
.as_ref()
.map_or(true, |p| p.get(permission as usize))
|| change.field == PrincipalField::DisabledPermissions
{
permissions.push(permission);
} else {
return Err(error(
"Invalid permission",
format!("Your account cannot grant the {name:?} permission")
.into(),
));
}
}
}
@ -1363,7 +1391,19 @@ impl ManageDirectory for Store {
})?
.id() as u64;
principal.inner.append_int(change.field, permission);
if params
.allowed_permissions
.as_ref()
.map_or(true, |p| p.get(permission as usize))
|| change.field == PrincipalField::DisabledPermissions
{
principal.inner.append_int(change.field, permission);
} else {
return Err(error(
"Invalid permission",
format!("Your account cannot grant the {name:?} permission").into(),
));
}
}
(
PrincipalAction::RemoveItem,
@ -1810,6 +1850,7 @@ impl ValidateDirectory for Store {
.with_field(PrincipalField::Name, domain.to_string())
.with_field(PrincipalField::Description, domain.to_string()),
tenant_id,
None,
)
.await
.caused_by(trc::location!())
@ -1859,6 +1900,7 @@ impl<'x> UpdatePrincipal<'x> {
changes: Vec::new(),
create_domains: false,
tenant_id: None,
allowed_permissions: None,
}
}
@ -1868,6 +1910,7 @@ impl<'x> UpdatePrincipal<'x> {
changes: Vec::new(),
create_domains: false,
tenant_id: None,
allowed_permissions: None,
}
}
@ -1881,6 +1924,11 @@ impl<'x> UpdatePrincipal<'x> {
self
}
pub fn with_allowed_permissions(mut self, permissions: &'x Permissions) -> Self {
self.allowed_permissions = permissions.into();
self
}
pub fn create_domains(mut self) -> Self {
self.create_domains = true;
self

79
crates/jmap/src/api/management/principal.rs
View file

@ -119,12 +119,39 @@ impl PrincipalManager for Server {
self.assert_supported_directory()?;
}
// Validate roles
let tenant_id = access_token.tenant.map(|t| t.id);
for name in principal
.get_str_array(PrincipalField::Roles)
.unwrap_or_default()
{
if let Some(pinfo) = self
.store()
.get_principal_info(name)
.await
.caused_by(trc::location!())?
.filter(|v| v.typ == Type::Role && v.has_tenant_access(tenant_id))
.or_else(|| PrincipalField::Roles.map_internal_roles(name))
{
let role_permissions =
self.get_role_permissions(pinfo.id).await?.finalize_as_ref();
let mut allowed_permissions = role_permissions.clone();
allowed_permissions.intersection(&access_token.permissions);
if allowed_permissions != role_permissions {
return Err(manage::error(
"Invalid role",
format!("Your account cannot grant the {name:?} role").into(),
));
}
}
}
// Create principal
let result = self
.core
.storage
.data
.create_principal(principal, access_token.tenant.map(|t| t.id))
.create_principal(principal, tenant_id, Some(&access_token.permissions))
.await?;
Ok(JsonResponse::new(json!({
@ -416,6 +443,53 @@ impl PrincipalManager for Server {
} else {
expire_token = true;
}
if change.field == PrincipalField::Roles
&& matches!(
change.action,
PrincipalAction::AddItem | PrincipalAction::Set
)
{
let roles = match &change.value {
PrincipalValue::String(v) => std::slice::from_ref(v),
PrincipalValue::StringList(vec) => vec,
PrincipalValue::Integer(_)
| PrincipalValue::IntegerList(_) => continue,
};
// Validate roles
let tenant_id = access_token.tenant.map(|t| t.id);
for name in roles {
if let Some(pinfo) = self
.store()
.get_principal_info(name)
.await
.caused_by(trc::location!())?
.filter(|v| {
v.typ == Type::Role
&& v.has_tenant_access(tenant_id)
})
.or_else(|| {
PrincipalField::Roles.map_internal_roles(name)
})
{
let role_permissions = self
.get_role_permissions(pinfo.id)
.await?
.finalize_as_ref();
let mut allowed_permissions =
role_permissions.clone();
allowed_permissions
.intersection(&access_token.permissions);
if allowed_permissions != role_permissions {
return Err(manage::error(
"Invalid role",
format!("Your account cannot grant the {name:?} role").into(),
));
}
}
}
}
}
}
}
@ -431,7 +505,8 @@ impl PrincipalManager for Server {
.update_principal(
UpdatePrincipal::by_id(account_id)
.with_updates(changes)
.with_tenant(access_token.tenant.map(|t| t.id)),
.with_tenant(access_token.tenant.map(|t| t.id))
.with_allowed_permissions(&access_token.permissions),
)
.await?;

1
crates/jmap/src/auth/oauth/registration.rs
View file

@ -81,6 +81,7 @@ impl ClientRegistrationHandler for Server {
.with_field(PrincipalField::Emails, request.contacts.clone())
.with_opt_field(PrincipalField::Picture, request.logo_uri.clone()),
None,
None,
)
.await
.caused_by(trc::location!())?;

2
crates/trc/src/ipc/bitset.rs
View file

@ -6,7 +6,7 @@
use super::{USIZE_BITS, USIZE_BITS_MASK};
#[derive(Clone, Debug)]
#[derive(Clone, Debug, PartialEq, Eq)]
pub struct Bitset<const N: usize>(pub(crate) [usize; N]);
impl<const N: usize> Bitset<N> {

17
tests/src/directory/internal.rs
View file

@ -33,7 +33,9 @@ async fn internal_directory() {
// A principal without name should fail
assert_eq!(
store.create_principal(Principal::default(), None).await,
store
.create_principal(Principal::default(), None, None)
.await,
Err(manage::err_missing(PrincipalField::Name))
);
@ -48,6 +50,7 @@ async fn internal_directory() {
}
.into(),
None,
None,
)
.await
.unwrap();
@ -61,6 +64,7 @@ async fn internal_directory() {
..Default::default()
}
.into(),
None,
None
)
.await,
@ -77,6 +81,7 @@ async fn internal_directory() {
..Default::default()
}
.into(),
None,
None
)
.await,
@ -93,6 +98,7 @@ async fn internal_directory() {
}
.into(),
None,
None,
)
.await
.unwrap();
@ -143,6 +149,7 @@ async fn internal_directory() {
}
.into(),
None,
None,
)
.await
.unwrap();
@ -201,6 +208,7 @@ async fn internal_directory() {
..Default::default()
}
.into(),
None,
None
)
.await,
@ -221,6 +229,7 @@ async fn internal_directory() {
}
.into(),
None,
None,
)
.await
.unwrap();
@ -284,6 +293,7 @@ async fn internal_directory() {
}
.into(),
None,
None,
)
.await
.unwrap();
@ -297,6 +307,7 @@ async fn internal_directory() {
}
.into(),
None,
None,
)
.await
.unwrap();
@ -755,6 +766,7 @@ impl TestInternalDirectory for Store {
PrincipalValue::StringList(vec![role.to_string()]),
),
None,
None,
)
.await
.unwrap()
@ -779,6 +791,7 @@ impl TestInternalDirectory for Store {
PrincipalValue::StringList(vec!["user".to_string()]),
),
None,
None,
)
.await
.unwrap()
@ -803,6 +816,7 @@ impl TestInternalDirectory for Store {
PrincipalValue::StringList(vec![login.to_string()]),
),
None,
None,
)
.await
.unwrap()
@ -863,6 +877,7 @@ impl TestInternalDirectory for Store {
Principal::new(0, Type::Domain)
.with_field(PrincipalField::Name, domain.to_string()),
None,
None,
)
.await
.unwrap();