From e6c11529e8da2181f30e757dcc29fc2b0446d9cf Mon Sep 17 00:00:00 2001 From: mdecimus Date: Fri, 20 Sep 2024 15:52:48 +0200 Subject: Keep a copy of external principal data to support roles and OAuth --- crates/directory/src/backend/internal/manage.rs | 138 +++++++++---- crates/directory/src/backend/ldap/lookup.rs | 129 +++++++------ crates/directory/src/backend/sql/lookup.rs | 223 +++++++++++---------- crates/directory/src/core/principal.rs | 61 +++++- crates/jmap/src/api/management/principal.rs | 21 +- tests/src/directory/internal.rs | 247 ++++++++++-------------- tests/src/directory/sql.rs | 4 +- 7 files changed, 457 insertions(+), 366 deletions(-) diff --git a/crates/directory/src/backend/internal/manage.rs b/crates/directory/src/backend/internal/manage.rs index de717dce..39f9ba54 100644 --- a/crates/directory/src/backend/internal/manage.rs +++ b/crates/directory/src/backend/internal/manage.rs @@ -33,12 +33,19 @@ pub struct PrincipalList { pub total: u64, } +pub struct UpdatePrincipal<'x> { + query: QueryBy<'x>, + changes: Vec, + tenant_id: Option, + validate: bool, +} + #[allow(async_fn_in_trait)] pub trait ManageDirectory: Sized { async fn get_principal_id(&self, name: &str) -> trc::Result>; async fn get_principal_info(&self, name: &str) -> trc::Result>; async fn get_or_create_principal_id(&self, name: &str, typ: Type) -> trc::Result; - async fn get_principal_name(&self, principal_id: u32) -> trc::Result>; + async fn get_principal(&self, principal_id: u32) -> trc::Result>; async fn get_member_of(&self, principal_id: u32) -> trc::Result>; async fn get_members(&self, principal_id: u32) -> trc::Result>; async fn create_principal( @@ -46,12 +53,7 @@ pub trait ManageDirectory: Sized { principal: Principal, tenant_id: Option, ) -> trc::Result; - async fn update_principal( - &self, - by: QueryBy<'_>, - changes: Vec, - tenant_id: Option, - ) -> trc::Result<()>; + async fn update_principal(&self, params: UpdatePrincipal<'_>) -> trc::Result<()>; async fn delete_principal(&self, by: QueryBy<'_>) -> trc::Result<()>; async fn list_principals( &self, @@ -76,12 +78,11 @@ pub trait ManageDirectory: Sized { } impl ManageDirectory for Store { - async fn get_principal_name(&self, principal_id: u32) -> trc::Result> { + async fn get_principal(&self, principal_id: u32) -> trc::Result> { self.get_value::(ValueKey::from(ValueClass::Directory( DirectoryClass::Principal(principal_id), ))) .await - .map(|v| v.and_then(|mut v| v.take_str(PrincipalField::Name))) .caused_by(trc::location!()) } @@ -130,6 +131,25 @@ impl ManageDirectory for Store { .with_field(PrincipalField::Name, name.to_string()), ); + // Add default user role + if typ == Type::Individual { + batch + .set( + ValueClass::Directory(DirectoryClass::MemberOf { + principal_id: MaybeDynamicId::Dynamic(0), + member_of: MaybeDynamicId::Static(ROLE_USER), + }), + vec![Type::Role as u8], + ) + .set( + ValueClass::Directory(DirectoryClass::Members { + principal_id: MaybeDynamicId::Static(ROLE_USER), + has_member: MaybeDynamicId::Dynamic(0), + }), + vec![], + ); + } + match self .write(batch.build()) .await @@ -589,13 +609,8 @@ impl ManageDirectory for Store { Ok(()) } - async fn update_principal( - &self, - by: QueryBy<'_>, - changes: Vec, - tenant_id: Option, - ) -> trc::Result<()> { - let principal_id = match by { + async fn update_principal(&self, params: UpdatePrincipal<'_>) -> trc::Result<()> { + let principal_id = match params.query { QueryBy::Name(name) => self .get_principal_id(name) .await @@ -604,6 +619,9 @@ impl ManageDirectory for Store { QueryBy::Id(principal_id) => principal_id, QueryBy::Credentials(_) => unreachable!(), }; + let changes = params.changes; + let tenant_id = params.tenant_id; + let validate = params.validate; // Fetch principal let mut principal = self @@ -911,16 +929,21 @@ impl ManageDirectory for Store { .collect::>(); for email in &emails { if !principal.inner.has_str_value(PrincipalField::Emails, email) { - if self.rcpt(email).await.caused_by(trc::location!())? { - return Err(err_exists(PrincipalField::Emails, email.to_string())); - } - if let Some(domain) = email.split('@').nth(1) { - if !self - .is_local_domain(domain) - .await - .caused_by(trc::location!())? - { - return Err(not_found(domain.to_string())); + if validate { + if self.rcpt(email).await.caused_by(trc::location!())? { + return Err(err_exists( + PrincipalField::Emails, + email.to_string(), + )); + } + if let Some(domain) = email.split('@').nth(1) { + if !self + .is_local_domain(domain) + .await + .caused_by(trc::location!())? + { + return Err(not_found(domain.to_string())); + } } } batch.set( @@ -952,16 +975,18 @@ impl ManageDirectory for Store { .inner .has_str_value(PrincipalField::Emails, &email) { - if self.rcpt(&email).await.caused_by(trc::location!())? { - return Err(err_exists(PrincipalField::Emails, email)); - } - if let Some(domain) = email.split('@').nth(1) { - if !self - .is_local_domain(domain) - .await - .caused_by(trc::location!())? - { - return Err(not_found(domain.to_string())); + if validate { + if self.rcpt(&email).await.caused_by(trc::location!())? { + return Err(err_exists(PrincipalField::Emails, email)); + } + if let Some(domain) = email.split('@').nth(1) { + if !self + .is_local_domain(domain) + .await + .caused_by(trc::location!())? + { + return Err(not_found(domain.to_string())); + } } } batch.set( @@ -1564,9 +1589,10 @@ impl ManageDirectory for Store { } principal_id => { if let Some(name) = self - .get_principal_name(principal_id) + .get_principal(principal_id) .await .caused_by(trc::location!())? + .and_then(|mut p| p.take_str(PrincipalField::Name)) { principal.append_str(field, name); } @@ -1651,9 +1677,10 @@ impl ManageDirectory for Store { if let Some(tenant_id) = principal.take_int(PrincipalField::Tenant) { if fields.is_empty() || fields.contains(&PrincipalField::Tenant) { if let Some(name) = self - .get_principal_name(tenant_id as u32) + .get_principal(tenant_id as u32) .await .caused_by(trc::location!())? + .and_then(|mut p| p.take_str(PrincipalField::Name)) { principal.set(PrincipalField::Tenant, name); } @@ -1723,6 +1750,41 @@ impl From for MaybeDynamicValue { } } +impl<'x> UpdatePrincipal<'x> { + pub fn by_id(id: u32) -> Self { + Self { + query: QueryBy::Id(id), + changes: Vec::new(), + validate: true, + tenant_id: None, + } + } + + pub fn by_name(name: &'x str) -> Self { + Self { + query: QueryBy::Name(name), + changes: Vec::new(), + validate: true, + tenant_id: None, + } + } + + pub fn with_tenant(mut self, tenant_id: Option) -> Self { + self.tenant_id = tenant_id; + self + } + + pub fn with_updates(mut self, changes: Vec) -> Self { + self.changes = changes; + self + } + + pub fn no_validate(mut self) -> Self { + self.validate = false; + self + } +} + fn validate_member_of( field: PrincipalField, typ: Type, diff --git a/crates/directory/src/backend/ldap/lookup.rs b/crates/directory/src/backend/ldap/lookup.rs index 5cc2a3d6..f238c7b2 100644 --- a/crates/directory/src/backend/ldap/lookup.rs +++ b/crates/directory/src/backend/ldap/lookup.rs @@ -9,7 +9,11 @@ use mail_send::Credentials; use trc::AddContext; use crate::{ - backend::internal::{manage::ManageDirectory, PrincipalField}, + backend::internal::{ + lookup::DirectoryStore, + manage::{self, ManageDirectory, UpdatePrincipal}, + PrincipalField, + }, IntoError, Principal, QueryBy, Type, ROLE_ADMIN, ROLE_USER, }; @@ -22,35 +26,38 @@ impl LdapDirectory { return_member_of: bool, ) -> trc::Result> { let mut conn = self.pool.get().await.map_err(|err| err.into_error())?; - let mut account_id = None; - let account_name; - let principal = match by { + let (mut external_principal, stored_principal) = match by { QueryBy::Name(username) => { - account_name = username.to_string(); - if let Some(principal) = self .find_principal(&mut conn, &self.mappings.filter_name.build(username)) .await? { - principal + ( + principal.with_field(PrincipalField::Name, username.to_string()), + None, + ) } else { return Ok(None); } } QueryBy::Id(uid) => { - if let Some(username) = self.data_store.get_principal_name(uid).await? { - account_name = username; - } else { - return Ok(None); - } - account_id = Some(uid); - - if let Some(principal) = self - .find_principal(&mut conn, &self.mappings.filter_name.build(&account_name)) + if let Some(stored_principal_) = self + .data_store + .query(QueryBy::Id(uid), return_member_of) .await? { - principal + if let Some(principal) = self + .find_principal( + &mut conn, + &self.mappings.filter_name.build(stored_principal_.name()), + ) + .await? + { + (principal, Some(stored_principal_)) + } else { + return Ok(None); + } } else { return Ok(None); } @@ -61,7 +68,6 @@ impl LdapDirectory { Credentials::OAuthBearer { token } => (token, token), Credentials::XOauth2 { username, secret } => (username, secret), }; - account_name = username.to_string(); if let Some(auth_bind) = &self.auth_bind { let (conn, mut ldap) = LdapConnAsync::with_settings( @@ -91,7 +97,10 @@ impl LdapDirectory { .find_principal(&mut ldap, &self.mappings.filter_name.build(username)) .await { - Ok(Some(principal)) => principal, + Ok(Some(principal)) => ( + principal.with_field(PrincipalField::Name, username.to_string()), + None, + ), Err(err) if err.matches(trc::EventType::Store(trc::StoreEvent::LdapError)) && err @@ -109,7 +118,10 @@ impl LdapDirectory { .await? { if principal.verify_secret(secret).await? { - principal + ( + principal.with_field(PrincipalField::Name, username.to_string()), + None, + ) } else { return Ok(None); } @@ -118,27 +130,12 @@ impl LdapDirectory { } } }; - let mut principal = principal; - // Obtain account ID if not available - if let Some(account_id) = account_id { - principal.id = account_id; - } else { - principal.id = self - .data_store - .get_or_create_principal_id(&account_name, Type::Individual) - .await?; - } - principal.append_str(PrincipalField::Name, account_name); - - if return_member_of { - // Obtain groups - if principal.has_field(PrincipalField::MemberOf) { - let mut member_of = Vec::new(); - for mut name in principal - .take_str_array(PrincipalField::MemberOf) - .unwrap_or_default() - { + // Query groups + match external_principal.take_str_array(PrincipalField::MemberOf) { + Some(names) if return_member_of => { + let mut member_of = Vec::with_capacity(names.len()); + for mut name in names { if name.contains('=') { let (rs, _res) = conn .search( @@ -174,35 +171,39 @@ impl LdapDirectory { } // Map ids - principal.set(PrincipalField::MemberOf, member_of); + external_principal.set(PrincipalField::MemberOf, member_of); } + _ => (), + } - // Obtain roles - let mut did_role_cleanup = false; - for member in self + // Obtain account ID if not available + let mut principal = if let Some(stored_principal) = stored_principal { + stored_principal + } else { + let id = self .data_store - .get_member_of(principal.id) + .get_or_create_principal_id(external_principal.name(), Type::Individual) + .await + .caused_by(trc::location!())?; + + self.data_store + .query(QueryBy::Id(id), return_member_of) .await .caused_by(trc::location!())? - { - match member.typ { - Type::List => { - principal.append_int(PrincipalField::Lists, member.principal_id); - } - Type::Role => { - if !did_role_cleanup { - principal.remove(PrincipalField::Roles); - did_role_cleanup = true; - } - principal.append_int(PrincipalField::Roles, member.principal_id); - } - _ => { - principal.append_int(PrincipalField::MemberOf, member.principal_id); - } - } - } - } else if principal.has_field(PrincipalField::MemberOf) { - principal.remove(PrincipalField::MemberOf); + .ok_or_else(|| manage::not_found(id).caused_by(trc::location!()))? + }; + + // Keep the internal store up to date with the LDAP server + let changes = principal.update_external(external_principal); + if !changes.is_empty() { + self.data_store + .update_principal( + UpdatePrincipal::by_id(principal.id) + .with_updates(changes) + .no_validate(), + ) + .await + .caused_by(trc::location!())?; } Ok(Some(principal)) diff --git a/crates/directory/src/backend/sql/lookup.rs b/crates/directory/src/backend/sql/lookup.rs index 7e2607cb..79ab78b8 100644 --- a/crates/directory/src/backend/sql/lookup.rs +++ b/crates/directory/src/backend/sql/lookup.rs @@ -9,7 +9,11 @@ use store::{NamedRows, Rows, Value}; use trc::AddContext; use crate::{ - backend::internal::{manage::ManageDirectory, PrincipalField, PrincipalValue}, + backend::internal::{ + lookup::DirectoryStore, + manage::{self, ManageDirectory, UpdatePrincipal}, + PrincipalField, PrincipalValue, + }, Principal, QueryBy, Type, ROLE_ADMIN, ROLE_USER, }; @@ -21,144 +25,117 @@ impl SqlDirectory { by: QueryBy<'_>, return_member_of: bool, ) -> trc::Result> { - let mut account_id = None; - let account_name; - let mut secret = None; - - let result = match by { - QueryBy::Name(username) => { - account_name = username.to_string(); - - self.store - .query::(&self.mappings.query_name, vec![username.into()]) - .await + let (external_principal, stored_principal) = match by { + QueryBy::Name(username) => ( + self.mappings + .row_to_principal( + self.store + .query::(&self.mappings.query_name, vec![username.into()]) + .await + .caused_by(trc::location!())?, + ) .caused_by(trc::location!())? - } + .map(|p| p.with_field(PrincipalField::Name, username.to_string())), + None, + ), QueryBy::Id(uid) => { - if let Some(username) = self + if let Some(principal) = self .data_store - .get_principal_name(uid) + .query(QueryBy::Id(uid), return_member_of) .await .caused_by(trc::location!())? { - account_name = username; + ( + self.mappings + .row_to_principal( + self.store + .query::( + &self.mappings.query_name, + vec![principal.name().into()], + ) + .await + .caused_by(trc::location!())?, + ) + .caused_by(trc::location!())?, + Some(principal), + ) } else { return Ok(None); } - account_id = Some(uid); - - self.store - .query::( - &self.mappings.query_name, - vec![account_name.clone().into()], - ) - .await - .caused_by(trc::location!())? } QueryBy::Credentials(credentials) => { - let (username, secret_) = match credentials { + let (username, secret) = match credentials { Credentials::Plain { username, secret } => (username, secret), Credentials::OAuthBearer { token } => (token, token), Credentials::XOauth2 { username, secret } => (username, secret), }; - account_name = username.to_string(); - secret = secret_.into(); - self.store - .query::(&self.mappings.query_name, vec![username.into()]) - .await + match self + .mappings + .row_to_principal( + self.store + .query::(&self.mappings.query_name, vec![username.into()]) + .await + .caused_by(trc::location!())?, + ) .caused_by(trc::location!())? + { + Some(principal) + if principal + .verify_secret(secret) + .await + .caused_by(trc::location!())? => + { + ( + Some(principal.with_field(PrincipalField::Name, username.to_string())), + None, + ) + } + _ => (None, None), + } } }; - if result.rows.is_empty() { - return Ok(None); - } - - // Map row to principal - let mut principal = self - .mappings - .row_to_principal(result) - .caused_by(trc::location!())?; - - // Validate password - if let Some(secret) = secret { - if !principal - .verify_secret(secret) - .await - .caused_by(trc::location!())? - { - return Ok(None); - } - } - - // Obtain account ID if not available - if let Some(account_id) = account_id { - principal.id = account_id; + let mut external_principal = if let Some(external_principal) = external_principal { + external_principal } else { - principal.id = self - .data_store - .get_or_create_principal_id(&account_name, Type::Individual) - .await - .caused_by(trc::location!())?; - } - principal.set(PrincipalField::Name, account_name); + return Ok(None); + }; // Obtain members - if return_member_of { - if !self.mappings.query_members.is_empty() { - for row in self - .store - .query::(&self.mappings.query_members, vec![principal.name().into()]) - .await - .caused_by(trc::location!())? - .rows - { - if let Some(Value::Text(account_id)) = row.values.first() { - principal.append_int( - PrincipalField::MemberOf, - self.data_store - .get_or_create_principal_id(account_id, Type::Group) - .await - .caused_by(trc::location!())?, - ); - } - } - } - - // Obtain roles - let mut did_role_cleanup = false; - for member in self - .data_store - .get_member_of(principal.id) + if return_member_of && !self.mappings.query_members.is_empty() { + for row in self + .store + .query::( + &self.mappings.query_members, + vec![external_principal.name().into()], + ) .await .caused_by(trc::location!())? + .rows { - match member.typ { - Type::List => { - principal.append_int(PrincipalField::Lists, member.principal_id); - } - Type::Role => { - if !did_role_cleanup { - principal.remove(PrincipalField::Roles); - did_role_cleanup = true; - } - principal.append_int(PrincipalField::Roles, member.principal_id); - } - _ => { - principal.append_int(PrincipalField::MemberOf, member.principal_id); - } + if let Some(Value::Text(account_id)) = row.values.first() { + external_principal.append_int( + PrincipalField::MemberOf, + self.data_store + .get_or_create_principal_id(account_id, Type::Group) + .await + .caused_by(trc::location!())?, + ); } } } // Obtain emails if !self.mappings.query_emails.is_empty() { - principal.set( + external_principal.set( PrincipalField::Emails, PrincipalValue::StringList( self.store - .query::(&self.mappings.query_emails, vec![principal.name().into()]) + .query::( + &self.mappings.query_emails, + vec![external_principal.name().into()], + ) .await .caused_by(trc::location!())? .into(), @@ -166,6 +143,36 @@ impl SqlDirectory { ); } + // Obtain account ID if not available + let mut principal = if let Some(stored_principal) = stored_principal { + stored_principal + } else { + let id = self + .data_store + .get_or_create_principal_id(external_principal.name(), Type::Individual) + .await + .caused_by(trc::location!())?; + + self.data_store + .query(QueryBy::Id(id), return_member_of) + .await + .caused_by(trc::location!())? + .ok_or_else(|| manage::not_found(id).caused_by(trc::location!()))? + }; + + // Keep the internal store up to date with the SQL server + let changes = principal.update_external(external_principal); + if !changes.is_empty() { + self.data_store + .update_principal( + UpdatePrincipal::by_id(principal.id) + .with_updates(changes) + .no_validate(), + ) + .await + .caused_by(trc::location!())?; + } + Ok(Some(principal)) } @@ -233,7 +240,11 @@ impl SqlDirectory { } impl SqlMappings { - pub fn row_to_principal(&self, rows: NamedRows) -> trc::Result { + pub fn row_to_principal(&self, rows: NamedRows) -> trc::Result> { + if rows.rows.is_empty() { + return Ok(None); + } + let mut principal = Principal::default(); let mut role = ROLE_USER; @@ -271,6 +282,6 @@ impl SqlMappings { } } - Ok(principal.with_field(PrincipalField::Roles, role)) + Ok(Some(principal.with_field(PrincipalField::Roles, role))) } } diff --git a/crates/directory/src/core/principal.rs b/crates/directory/src/core/principal.rs index 89d67fce..ef126329 100644 --- a/crates/directory/src/core/principal.rs +++ b/crates/directory/src/core/principal.rs @@ -14,7 +14,7 @@ use serde::{ use store::U64_LEN; use crate::{ - backend::internal::{PrincipalField, PrincipalValue}, + backend::internal::{PrincipalField, PrincipalUpdate, PrincipalValue}, Permission, Principal, Type, ROLE_ADMIN, }; @@ -367,6 +367,65 @@ impl Principal { } } + pub fn update_external(&mut self, mut external: Principal) -> Vec { + let mut updates = Vec::new(); + if let Some(name) = external.take_str(PrincipalField::Description) { + if self.get_str(PrincipalField::Description) != Some(name.as_str()) { + updates.push(PrincipalUpdate::set( + PrincipalField::Description, + PrincipalValue::String(name.clone()), + )); + self.set(PrincipalField::Description, name); + } + } + + for field in [PrincipalField::Secrets, PrincipalField::Emails] { + if let Some(secrets) = external.take_str_array(field).filter(|s| !s.is_empty()) { + if self.get_str_array(field) != Some(secrets.as_ref()) { + updates.push(PrincipalUpdate::set( + field, + PrincipalValue::StringList(secrets.clone()), + )); + self.set(field, secrets); + } + } + } + + if let Some(quota) = external.take_int(PrincipalField::Quota) { + if self.get_int(PrincipalField::Quota) != Some(quota) { + updates.push(PrincipalUpdate::set( + PrincipalField::Quota, + PrincipalValue::Integer(quota), + )); + self.set(PrincipalField::Quota, quota); + } + } + + // Add external members + if let Some(member_of) = external + .take_int_array(PrincipalField::MemberOf) + .filter(|s| !s.is_empty()) + { + self.set(PrincipalField::MemberOf, member_of); + } + + // If the principal has no roles, take the ones from the external principal + if let Some(member_of) = external + .take_int_array(PrincipalField::Roles) + .filter(|s| !s.is_empty()) + { + if self + .get_int_array(PrincipalField::Roles) + .filter(|s| !s.is_empty()) + .is_none() + { + self.set(PrincipalField::Roles, member_of); + } + } + + updates + } + pub fn fallback_admin(fallback_pass: impl Into) -> Self { Principal { id: u32::MAX, diff --git a/crates/jmap/src/api/management/principal.rs b/crates/jmap/src/api/management/principal.rs index add50bfc..51994756 100644 --- a/crates/jmap/src/api/management/principal.rs +++ b/crates/jmap/src/api/management/principal.rs @@ -10,7 +10,7 @@ use common::auth::AccessToken; use directory::{ backend::internal::{ lookup::DirectoryStore, - manage::{self, not_found, ManageDirectory}, + manage::{self, not_found, ManageDirectory, UpdatePrincipal}, PrincipalAction, PrincipalField, PrincipalUpdate, PrincipalValue, SpecialSecrets, }, DirectoryInner, Permission, Principal, QueryBy, Type, @@ -344,14 +344,13 @@ impl JMAP { for change in &changes { match change.field { - PrincipalField::Name | PrincipalField::Emails => { - needs_assert = true; - } PrincipalField::Secrets => { expire_session = true; needs_assert = true; } - PrincipalField::Quota + PrincipalField::Name + | PrincipalField::Emails + | PrincipalField::Quota | PrincipalField::UsedQuota | PrincipalField::Description | PrincipalField::Type @@ -392,9 +391,9 @@ impl JMAP { .storage .data .update_principal( - QueryBy::Id(account_id), - changes, - access_token.tenant.map(|t| t.id), + UpdatePrincipal::by_id(account_id) + .with_updates(changes) + .with_tenant(access_token.tenant.map(|t| t.id)), ) .await?; @@ -571,9 +570,9 @@ impl JMAP { .storage .data .update_principal( - QueryBy::Id(access_token.primary_id()), - actions, - access_token.tenant.map(|t| t.id), + UpdatePrincipal::by_id(access_token.primary_id()) + .with_updates(actions) + .with_tenant(access_token.tenant.map(|t| t.id)), ) .await?; diff --git a/tests/src/directory/internal.rs b/tests/src/directory/internal.rs index 3cde6c15..fed8a2cb 100644 --- a/tests/src/directory/internal.rs +++ b/tests/src/directory/internal.rs @@ -8,7 +8,7 @@ use ahash::AHashSet; use directory::{ backend::internal::{ lookup::DirectoryStore, - manage::{self, ManageDirectory}, + manage::{self, ManageDirectory, UpdatePrincipal}, PrincipalField, PrincipalUpdate, PrincipalValue, }, Principal, QueryBy, Type, @@ -102,14 +102,12 @@ async fn internal_directory() { // Add an email address assert_eq!( store - .update_principal( - QueryBy::Name("john"), - vec![PrincipalUpdate::add_item( + .update_principal(UpdatePrincipal::by_name("john").with_updates(vec![ + PrincipalUpdate::add_item( PrincipalField::Emails, PrincipalValue::String("john@example.org".to_string()), - )], - None - ) + ) + ])) .await, Ok(()) ); @@ -122,14 +120,12 @@ async fn internal_directory() { // Using non-existent domain should fail assert_eq!( store - .update_principal( - QueryBy::Name("john"), - vec![PrincipalUpdate::add_item( + .update_principal(UpdatePrincipal::by_name("john").with_updates(vec![ + PrincipalUpdate::add_item( PrincipalField::Emails, PrincipalValue::String("john@otherdomain.org".to_string()), - )], - None - ) + ) + ])) .await, Err(manage::not_found("otherdomain.org".to_string())) ); @@ -230,14 +226,12 @@ async fn internal_directory() { .unwrap(); assert_eq!( store - .update_principal( - QueryBy::Name("list"), - vec![PrincipalUpdate::set( + .update_principal(UpdatePrincipal::by_name("list").with_updates(vec![ + PrincipalUpdate::set( PrincipalField::Members, PrincipalValue::StringList(vec!["john".to_string(), "jane".to_string()]), - )], - None - ) + ) + ])) .await, Ok(()) ); @@ -310,20 +304,16 @@ async fn internal_directory() { // Add John to the Sales and Support groups assert_eq!( store - .update_principal( - QueryBy::Name("john"), - vec![ - PrincipalUpdate::add_item( - PrincipalField::MemberOf, - PrincipalValue::String("sales".to_string()), - ), - PrincipalUpdate::add_item( - PrincipalField::MemberOf, - PrincipalValue::String("support".to_string()), - ) - ], - None - ) + .update_principal(UpdatePrincipal::by_name("john").with_updates(vec![ + PrincipalUpdate::add_item( + PrincipalField::MemberOf, + PrincipalValue::String("sales".to_string()), + ), + PrincipalUpdate::add_item( + PrincipalField::MemberOf, + PrincipalValue::String("support".to_string()), + ) + ])) .await, Ok(()) ); @@ -353,14 +343,12 @@ async fn internal_directory() { // Adding a non-existent user should fail assert_eq!( store - .update_principal( - QueryBy::Name("john"), - vec![PrincipalUpdate::add_item( + .update_principal(UpdatePrincipal::by_name("john").with_updates(vec![ + PrincipalUpdate::add_item( PrincipalField::MemberOf, PrincipalValue::String("accounting".to_string()), - )], - None - ) + ) + ])) .await, Err(manage::not_found("accounting".to_string())) ); @@ -368,14 +356,12 @@ async fn internal_directory() { // Remove a member from a group assert_eq!( store - .update_principal( - QueryBy::Name("john"), - vec![PrincipalUpdate::remove_item( + .update_principal(UpdatePrincipal::by_name("john").with_updates(vec![ + PrincipalUpdate::remove_item( PrincipalField::MemberOf, PrincipalValue::String("support".to_string()), - )], - None - ) + ) + ])) .await, Ok(()) ); @@ -401,33 +387,29 @@ async fn internal_directory() { // Update multiple fields assert_eq!( store - .update_principal( - QueryBy::Name("john"), - vec![ - PrincipalUpdate::set( - PrincipalField::Name, - PrincipalValue::String("john.doe".to_string()) - ), - PrincipalUpdate::set( - PrincipalField::Description, - PrincipalValue::String("Johnny Doe".to_string()) - ), - PrincipalUpdate::set( - PrincipalField::Secrets, - PrincipalValue::StringList(vec!["12345".to_string()]) - ), - PrincipalUpdate::set(PrincipalField::Quota, PrincipalValue::Integer(1024)), - PrincipalUpdate::remove_item( - PrincipalField::Emails, - PrincipalValue::String("john@example.org".to_string()), - ), - PrincipalUpdate::add_item( - PrincipalField::Emails, - PrincipalValue::String("john.doe@example.org".to_string()), - ) - ], - None - ) + .update_principal(UpdatePrincipal::by_name("john").with_updates(vec![ + PrincipalUpdate::set( + PrincipalField::Name, + PrincipalValue::String("john.doe".to_string()) + ), + PrincipalUpdate::set( + PrincipalField::Description, + PrincipalValue::String("Johnny Doe".to_string()) + ), + PrincipalUpdate::set( + PrincipalField::Secrets, + PrincipalValue::StringList(vec!["12345".to_string()]) + ), + PrincipalUpdate::set(PrincipalField::Quota, PrincipalValue::Integer(1024)), + PrincipalUpdate::remove_item( + PrincipalField::Emails, + PrincipalValue::String("john@example.org".to_string()), + ), + PrincipalUpdate::add_item( + PrincipalField::Emails, + PrincipalValue::String("john.doe@example.org".to_string()), + ) + ])) .await, Ok(()) ); @@ -459,14 +441,12 @@ async fn internal_directory() { // Remove a member from a mailing list and then add it back assert_eq!( store - .update_principal( - QueryBy::Name("list"), - vec![PrincipalUpdate::remove_item( + .update_principal(UpdatePrincipal::by_name("list").with_updates(vec![ + PrincipalUpdate::remove_item( PrincipalField::Members, PrincipalValue::String("john.doe".to_string()), - )], - None - ) + ) + ])) .await, Ok(()) ); @@ -476,14 +456,12 @@ async fn internal_directory() { ); assert_eq!( store - .update_principal( - QueryBy::Name("list"), - vec![PrincipalUpdate::add_item( + .update_principal(UpdatePrincipal::by_name("list").with_updates(vec![ + PrincipalUpdate::add_item( PrincipalField::Members, PrincipalValue::String("john.doe".to_string()), - )], - None - ) + ) + ])) .await, Ok(()) ); @@ -500,27 +478,23 @@ async fn internal_directory() { // Field validation assert_eq!( store - .update_principal( - QueryBy::Name("john.doe"), - vec![PrincipalUpdate::set( + .update_principal(UpdatePrincipal::by_name("john.doe").with_updates(vec![ + PrincipalUpdate::set( PrincipalField::Name, PrincipalValue::String("jane".to_string()) - ),], - None - ) + ), + ])) .await, Err(manage::err_exists(PrincipalField::Name, "jane".to_string())) ); assert_eq!( store - .update_principal( - QueryBy::Name("john.doe"), - vec![PrincipalUpdate::add_item( + .update_principal(UpdatePrincipal::by_name("john.doe").with_updates(vec![ + PrincipalUpdate::add_item( PrincipalField::Emails, PrincipalValue::String("jane@example.org".to_string()) - ),], - None - ) + ), + ])) .await, Err(manage::err_exists( PrincipalField::Emails, @@ -743,28 +717,24 @@ impl TestInternalDirectory for Store { let role = if login == "admin" { "admin" } else { "user" }; self.create_test_domains(emails).await; if let Some(principal) = self.query(QueryBy::Name(login), false).await.unwrap() { - self.update_principal( - QueryBy::Id(principal.id()), - vec![ - PrincipalUpdate::set( - PrincipalField::Secrets, - PrincipalValue::StringList(vec![secret.to_string()]), - ), - PrincipalUpdate::set( - PrincipalField::Description, - PrincipalValue::String(name.to_string()), - ), - PrincipalUpdate::set( - PrincipalField::Emails, - PrincipalValue::StringList(emails.iter().map(|s| s.to_string()).collect()), - ), - PrincipalUpdate::add_item( - PrincipalField::Roles, - PrincipalValue::String(role.to_string()), - ), - ], - None, - ) + self.update_principal(UpdatePrincipal::by_id(principal.id()).with_updates(vec![ + PrincipalUpdate::set( + PrincipalField::Secrets, + PrincipalValue::StringList(vec![secret.to_string()]), + ), + PrincipalUpdate::set( + PrincipalField::Description, + PrincipalValue::String(name.to_string()), + ), + PrincipalUpdate::set( + PrincipalField::Emails, + PrincipalValue::StringList(emails.iter().map(|s| s.to_string()).collect()), + ), + PrincipalUpdate::add_item( + PrincipalField::Roles, + PrincipalValue::String(role.to_string()), + ), + ])) .await .unwrap(); principal.id() @@ -841,53 +811,42 @@ impl TestInternalDirectory for Store { } async fn set_test_quota(&self, login: &str, quota: u32) { - self.update_principal( - QueryBy::Name(login), - vec![PrincipalUpdate::set( - PrincipalField::Quota, - PrincipalValue::Integer(quota as u64), - )], - None, - ) + self.update_principal(UpdatePrincipal::by_name(login).with_updates(vec![ + PrincipalUpdate::set(PrincipalField::Quota, PrincipalValue::Integer(quota as u64)), + ])) .await .unwrap(); } async fn add_to_group(&self, login: &str, group: &str) { - self.update_principal( - QueryBy::Name(login), - vec![PrincipalUpdate::add_item( + self.update_principal(UpdatePrincipal::by_name(login).with_updates(vec![ + PrincipalUpdate::add_item( PrincipalField::MemberOf, PrincipalValue::String(group.to_string()), - )], - None, - ) + ), + ])) .await .unwrap(); } async fn remove_from_group(&self, login: &str, group: &str) { - self.update_principal( - QueryBy::Name(login), - vec![PrincipalUpdate::remove_item( + self.update_principal(UpdatePrincipal::by_name(login).with_updates(vec![ + PrincipalUpdate::remove_item( PrincipalField::MemberOf, PrincipalValue::String(group.to_string()), - )], - None, - ) + ), + ])) .await .unwrap(); } async fn remove_test_alias(&self, login: &str, alias: &str) { - self.update_principal( - QueryBy::Name(login), - vec![PrincipalUpdate::remove_item( + self.update_principal(UpdatePrincipal::by_name(login).with_updates(vec![ + PrincipalUpdate::remove_item( PrincipalField::Emails, PrincipalValue::String(alias.to_string()), - )], - None, - ) + ), + ])) .await .unwrap(); } diff --git a/tests/src/directory/sql.rs b/tests/src/directory/sql.rs index 27a86047..20f2c6eb 100644 --- a/tests/src/directory/sql.rs +++ b/tests/src/directory/sql.rs @@ -4,7 +4,7 @@ * SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-SEL */ -use directory::{backend::internal::manage::ManageDirectory, QueryBy, Type, ROLE_ADMIN, ROLE_USER}; +use directory::{backend::internal::manage::ManageDirectory, QueryBy, Type, ROLE_USER}; use mail_send::Credentials; use store::{LookupStore, Store}; @@ -181,7 +181,7 @@ async fn sql_directory() { description: "Administrator".to_string().into(), secrets: vec!["very_secret".to_string()], typ: Type::Individual, - roles: vec![ROLE_ADMIN.to_string()], + roles: vec![ROLE_USER.to_string()], ..Default::default() } ); -- cgit v1.2.3-70-g09d2