From 6e2cd7847084c145a15fb9717ad587a773cc8be0 Mon Sep 17 00:00:00 2001 From: mdecimus Date: Tue, 1 Oct 2024 16:01:51 +0200 Subject: OAuth fixes --- crates/common/src/auth/oauth/config.rs | 2 +- crates/common/src/config/network.rs | 4 +- crates/common/src/enterprise/license.rs | 26 +++++------ crates/common/src/enterprise/mod.rs | 2 +- crates/directory/src/backend/internal/manage.rs | 53 +++++++++++++++------ crates/directory/src/core/mod.rs | 61 +++++++++++++------------ crates/directory/src/core/principal.rs | 13 ++++-- crates/jmap/src/auth/oauth/auth.rs | 9 +++- tests/src/jmap/enterprise.rs | 4 +- tests/src/jmap/mod.rs | 2 +- 10 files changed, 107 insertions(+), 69 deletions(-) diff --git a/crates/common/src/auth/oauth/config.rs b/crates/common/src/auth/oauth/config.rs index 79db96f7..ab875b8a 100644 --- a/crates/common/src/auth/oauth/config.rs +++ b/crates/common/src/auth/oauth/config.rs @@ -186,7 +186,7 @@ impl OAuthConfig { .property_or_default("oauth.client-registration.anonymous", "false") .unwrap_or(false), require_client_authentication: config - .property_or_default("oauth.client-registration.required", "false") + .property_or_default("oauth.client-registration.require", "false") .unwrap_or(true), oidc_signing_secret, oidc_signature_algorithm, diff --git a/crates/common/src/config/network.rs b/crates/common/src/config/network.rs index d4c6d026..48c95809 100644 --- a/crates/common/src/config/network.rs +++ b/crates/common/src/config/network.rs @@ -91,8 +91,8 @@ impl ContactForm { .property_or_default::("form.validate-domain", "true") .unwrap_or(true), from_email: FieldOrDefault::parse(config, "form.email", "postmaster@localhost"), - from_subject: FieldOrDefault::parse(config, "form.subject", "Contact Form"), - from_name: FieldOrDefault::parse(config, "form.name", "Contact Form"), + from_subject: FieldOrDefault::parse(config, "form.subject", "Contact form submission"), + from_name: FieldOrDefault::parse(config, "form.name", "Anonymous"), field_honey_pot: config.value("form.honey-pot.field").map(|v| v.to_string()), rate: config .property_or_default::>("form.rate-limit", "5/1h") diff --git a/crates/common/src/enterprise/license.rs b/crates/common/src/enterprise/license.rs index 718af5ab..136dcfd2 100644 --- a/crates/common/src/enterprise/license.rs +++ b/crates/common/src/enterprise/license.rs @@ -40,7 +40,7 @@ pub struct LicenseGenerator { pub struct LicenseKey { pub valid_to: u64, pub valid_from: u64, - pub hostname: String, + pub domain: String, pub accounts: u32, } @@ -93,27 +93,27 @@ impl LicenseValidator { .try_into() .unwrap(), ); - let hostname_len = u32::from_le_bytes( + let domain_len = u32::from_le_bytes( key.get((U64_LEN * 2) + U32_LEN..(U64_LEN * 2) + (U32_LEN * 2)) .ok_or(LicenseError::Parse)? .try_into() .unwrap(), ) as usize; - let hostname = String::from_utf8( - key.get((U64_LEN * 2) + (U32_LEN * 2)..(U64_LEN * 2) + (U32_LEN * 2) + hostname_len) + let domain = String::from_utf8( + key.get((U64_LEN * 2) + (U32_LEN * 2)..(U64_LEN * 2) + (U32_LEN * 2) + domain_len) .ok_or(LicenseError::Parse)? .to_vec(), ) .map_err(|_| LicenseError::Parse)?; let signature = key - .get((U64_LEN * 2) + (U32_LEN * 2) + hostname_len..) + .get((U64_LEN * 2) + (U32_LEN * 2) + domain_len..) .ok_or(LicenseError::Parse)?; if valid_from == 0 || valid_to == 0 || valid_from >= valid_to || accounts == 0 - || hostname.is_empty() + || domain.is_empty() { return Err(LicenseError::InvalidParameters); } @@ -121,7 +121,7 @@ impl LicenseValidator { // Validate signature self.public_key .verify( - &key[..(U64_LEN * 2) + (U32_LEN * 2) + hostname_len], + &key[..(U64_LEN * 2) + (U32_LEN * 2) + domain_len], signature, ) .map_err(|_| LicenseError::Validation)?; @@ -129,7 +129,7 @@ impl LicenseValidator { let key = LicenseKey { valid_from, valid_to, - hostname, + domain, accounts, }; @@ -142,7 +142,7 @@ impl LicenseValidator { } impl LicenseKey { - pub fn new(hostname: String, accounts: u32, expires_in: u64) -> Self { + pub fn new(domain: String, accounts: u32, expires_in: u64) -> Self { let now = SystemTime::UNIX_EPOCH .elapsed() .unwrap_or_default() @@ -150,7 +150,7 @@ impl LicenseKey { LicenseKey { valid_from: now - 300, valid_to: now + expires_in + 300, - hostname, + domain, accounts, } } @@ -176,7 +176,7 @@ impl LicenseKey { pub fn into_validated_key(self, hostname: impl AsRef) -> Result { let local_domain = psl::domain_str(hostname.as_ref()).unwrap_or("invalid-hostname"); - let license_domain = psl::domain_str(&self.hostname).expect("Invalid license hostname"); + let license_domain = psl::domain_str(&self.domain).expect("Invalid license domain"); if local_domain != license_domain { Err(LicenseError::DomainMismatch { issued_to: license_domain.to_string(), @@ -200,8 +200,8 @@ impl LicenseGenerator { bytes.extend_from_slice(&key.valid_from.to_le_bytes()); bytes.extend_from_slice(&key.valid_to.to_le_bytes()); bytes.extend_from_slice(&key.accounts.to_le_bytes()); - bytes.extend_from_slice(&(key.hostname.len() as u32).to_le_bytes()); - bytes.extend_from_slice(key.hostname.as_bytes()); + bytes.extend_from_slice(&(key.domain.len() as u32).to_le_bytes()); + bytes.extend_from_slice(key.domain.as_bytes()); bytes.extend_from_slice(self.key_pair.sign(&bytes).as_ref()); STANDARD.encode(&bytes) } diff --git a/crates/common/src/enterprise/mod.rs b/crates/common/src/enterprise/mod.rs index 9f634564..6ffe400d 100644 --- a/crates/common/src/enterprise/mod.rs +++ b/crates/common/src/enterprise/mod.rs @@ -121,7 +121,7 @@ impl Server { trc::event!( Server(trc::ServerEvent::Licensing), Details = "Stalwart Enterprise Edition license key is valid", - Hostname = enterprise.license.hostname.clone(), + Domain = enterprise.license.domain.clone(), Total = enterprise.license.accounts, ValidFrom = DateTime::from_timestamp(enterprise.license.valid_from as i64).to_rfc3339(), diff --git a/crates/directory/src/backend/internal/manage.rs b/crates/directory/src/backend/internal/manage.rs index d672af4d..eecbef8b 100644 --- a/crates/directory/src/backend/internal/manage.rs +++ b/crates/directory/src/backend/internal/manage.rs @@ -359,18 +359,20 @@ impl ManageDirectory for Store { } // Make sure the e-mail is not taken and validate domain - for email in principal.iter_mut_str(PrincipalField::Emails) { - *email = email.to_lowercase(); - 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 valid_domains.insert(domain.to_string()) { - self.get_principal_info(domain) - .await - .caused_by(trc::location!())? - .filter(|v| v.typ == Type::Domain && v.has_tenant_access(tenant_id)) - .ok_or_else(|| not_found(domain.to_string()))?; + if principal.typ != Type::OauthClient { + for email in principal.iter_mut_str(PrincipalField::Emails) { + *email = email.to_lowercase(); + 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 valid_domains.insert(domain.to_string()) { + self.get_principal_info(domain) + .await + .caused_by(trc::location!())? + .filter(|v| v.typ == Type::Domain && v.has_tenant_access(tenant_id)) + .ok_or_else(|| not_found(domain.to_string()))?; + } } } } @@ -678,7 +680,6 @@ impl ManageDirectory for Store { }; let changes = params.changes; let tenant_id = params.tenant_id; - let validate = params.validate; // Fetch principal let mut principal = self @@ -689,6 +690,7 @@ impl ManageDirectory for Store { .caused_by(trc::location!())? .ok_or_else(|| not_found(principal_id))?; principal.inner.id = principal_id; + let validate_emails = params.validate && principal.inner.typ != Type::OauthClient; // Obtain members and memberOf let mut member_of = self @@ -986,7 +988,7 @@ impl ManageDirectory for Store { .collect::>(); for email in &emails { if !principal.inner.has_str_value(PrincipalField::Emails, email) { - if validate { + if validate_emails { if self.rcpt(email).await.caused_by(trc::location!())? { return Err(err_exists( PrincipalField::Emails, @@ -1032,7 +1034,7 @@ impl ManageDirectory for Store { .inner .has_str_value(PrincipalField::Emails, &email) { - if validate { + if validate_emails { if self.rcpt(&email).await.caused_by(trc::location!())? { return Err(err_exists(PrincipalField::Emails, email)); } @@ -1394,6 +1396,27 @@ impl ManageDirectory for Store { .inner .retain_int(change.field, |v| *v != permission); } + (PrincipalAction::Set, PrincipalField::Urls, PrincipalValue::StringList(urls)) => { + if !urls.is_empty() { + principal.inner.set(change.field, urls); + } else { + principal.inner.remove(change.field); + } + } + (PrincipalAction::AddItem, PrincipalField::Urls, PrincipalValue::String(url)) => { + if !principal.inner.has_str_value(change.field, &url) { + principal.inner.append_str(change.field, url); + } + } + ( + PrincipalAction::RemoveItem, + PrincipalField::Urls, + PrincipalValue::String(url), + ) => { + if principal.inner.has_str_value(change.field, &url) { + principal.inner.retain_str(change.field, |v| *v != url); + } + } (_, field, value) => { return Err(error( diff --git a/crates/directory/src/core/mod.rs b/crates/directory/src/core/mod.rs index a91852a2..4f420693 100644 --- a/crates/directory/src/core/mod.rs +++ b/crates/directory/src/core/mod.rs @@ -15,27 +15,29 @@ pub mod secret; impl Permission { pub fn description(&self) -> &'static str { match self { - Permission::Impersonate => "Allows acting on behalf of another user", - Permission::UnlimitedRequests => "Removes request limits or quotas", - Permission::UnlimitedUploads => "Removes upload size or frequency limits", - Permission::DeleteSystemFolders => "Allows deletion of critical system folders", + Permission::Impersonate => "Act on behalf of another user", + Permission::UnlimitedRequests => "Perform unlimited requests", + Permission::UnlimitedUploads => "Upload unlimited data", + Permission::DeleteSystemFolders => "Delete of system folders", Permission::MessageQueueList => "View message queue", Permission::MessageQueueGet => "Retrieve specific messages from the queue", Permission::MessageQueueUpdate => "Modify queued messages", Permission::MessageQueueDelete => "Remove messages from the queue", - Permission::OutgoingReportList => "View reports for outgoing emails", - Permission::OutgoingReportGet => "Retrieve specific outgoing email reports", - Permission::OutgoingReportDelete => "Remove outgoing email reports", - Permission::IncomingReportList => "View reports for incoming emails", - Permission::IncomingReportGet => "Retrieve specific incoming email reports", - Permission::IncomingReportDelete => "Remove incoming email reports", + Permission::OutgoingReportList => "View outgoing DMARC and TLS reports", + Permission::OutgoingReportGet => "Retrieve specific outgoing DMARC and TLS reports", + Permission::OutgoingReportDelete => "Remove outgoing DMARC and TLS reports", + Permission::IncomingReportList => "View incoming DMARC, TLS and ARF reports", + Permission::IncomingReportGet => { + "Retrieve specific incoming DMARC, TLS and ARF reports" + } + Permission::IncomingReportDelete => "Remove incoming DMARC, TLS and ARF reports", Permission::SettingsList => "View system settings", Permission::SettingsUpdate => "Modify system settings", Permission::SettingsDelete => "Remove system settings", Permission::SettingsReload => "Refresh system settings", - Permission::IndividualList => "View list of individual users", - Permission::IndividualGet => "Retrieve specific user information", - Permission::IndividualUpdate => "Modify user information", + Permission::IndividualList => "View list of user accounts", + Permission::IndividualGet => "Retrieve specific account information", + Permission::IndividualUpdate => "Modify user account information", Permission::IndividualDelete => "Remove user accounts", Permission::IndividualCreate => "Add new user accounts", Permission::GroupList => "View list of user groups", @@ -48,7 +50,7 @@ impl Permission { Permission::DomainCreate => "Add new email domains", Permission::DomainUpdate => "Modify domain information", Permission::DomainDelete => "Remove email domains", - Permission::TenantList => "View list of tenants (in multi-tenant setup)", + Permission::TenantList => "View list of tenants", Permission::TenantGet => "Retrieve specific tenant information", Permission::TenantCreate => "Add new tenants", Permission::TenantUpdate => "Modify tenant information", @@ -63,16 +65,16 @@ impl Permission { Permission::RoleCreate => "Create new roles", Permission::RoleUpdate => "Modify role information", Permission::RoleDelete => "Remove roles", - Permission::PrincipalList => "View list of principals (users or system entities)", + Permission::PrincipalList => "View list of principals", Permission::PrincipalGet => "Retrieve specific principal information", Permission::PrincipalCreate => "Create new principals", Permission::PrincipalUpdate => "Modify principal information", Permission::PrincipalDelete => "Remove principals", - Permission::BlobFetch => "Retrieve binary large objects", - Permission::PurgeBlobStore => "Clear the blob storage", - Permission::PurgeDataStore => "Clear the data storage", - Permission::PurgeLookupStore => "Clear the lookup storage", - Permission::PurgeAccount => "Completely remove an account and all associated data", + Permission::BlobFetch => "Retrieve arbitrary blobs", + Permission::PurgeBlobStore => "Purge the blob storage", + Permission::PurgeDataStore => "Purge the data storage", + Permission::PurgeLookupStore => "Purge the lookup storage", + Permission::PurgeAccount => "Purge user accounts", Permission::FtsReindex => "Rebuild the full-text search index", Permission::Undelete => "Restore deleted items", Permission::DkimSignatureCreate => "Create DKIM signatures for email authentication", @@ -80,19 +82,19 @@ impl Permission { Permission::UpdateSpamFilter => "Modify spam filter settings", Permission::UpdateWebadmin => "Modify web admin interface settings", Permission::LogsView => "Access system logs", - Permission::SieveRun => "Execute Sieve scripts for email filtering", + Permission::SieveRun => "Execute Sieve scripts from the REST API", Permission::Restart => "Restart the email server", - Permission::TracingList => "View list of system traces", + Permission::TracingList => "View stored traces", Permission::TracingGet => "Retrieve specific trace information", - Permission::TracingLive => "View real-time system traces", - Permission::MetricsList => "View list of system metrics", - Permission::MetricsLive => "View real-time system metrics", - Permission::Authenticate => "Perform authentication", - Permission::AuthenticateOauth => "Perform OAuth authentication", + Permission::TracingLive => "Perform real-time tracing", + Permission::MetricsList => "View stored metrics", + Permission::MetricsLive => "View real-time metrics", + Permission::Authenticate => "Authenticate", + Permission::AuthenticateOauth => "Authenticate via OAuth", Permission::EmailSend => "Send emails", Permission::EmailReceive => "Receive emails", - Permission::ManageEncryption => "Handle encryption settings and operations", - Permission::ManagePasswords => "Manage user passwords", + Permission::ManageEncryption => "Manage encryption-at-rest settings", + Permission::ManagePasswords => "Manage account passwords", Permission::JmapEmailGet => "Retrieve emails via JMAP", Permission::JmapMailboxGet => "Retrieve mailboxes via JMAP", Permission::JmapThreadGet => "Retrieve email threads via JMAP", @@ -223,6 +225,7 @@ mod test { .then_some(CHECK) .unwrap_or_default() ); + //println!("({:?},{:?}),", permission.name(), permission.description(),); } } } diff --git a/crates/directory/src/core/principal.rs b/crates/directory/src/core/principal.rs index 4eba98e9..ecd8350b 100644 --- a/crates/directory/src/core/principal.rs +++ b/crates/directory/src/core/principal.rs @@ -591,8 +591,8 @@ impl Type { Self::Tenant => "tenant", Self::Role => "role", Self::Domain => "domain", - Self::ApiKey => "api-key", - Self::OauthClient => "oauth-client", + Self::ApiKey => "apiKey", + Self::OauthClient => "oauthClient", } } @@ -623,8 +623,8 @@ impl Type { "superuser" => Some(Type::Individual), // legacy "role" => Some(Type::Role), "domain" => Some(Type::Domain), - "api-key" => Some(Type::ApiKey), - "oauth-client" => Some(Type::OauthClient), + "apiKey" => Some(Type::ApiKey), + "oauthClient" => Some(Type::OauthClient), _ => None, } } @@ -1141,6 +1141,11 @@ impl Permission { | Permission::JmapPrincipalGet | Permission::JmapPrincipalQueryChanges | Permission::JmapPrincipalQuery + | Permission::ApiKeyList + | Permission::ApiKeyGet + | Permission::ApiKeyCreate + | Permission::ApiKeyUpdate + | Permission::ApiKeyDelete ) || self.is_user_permission() } diff --git a/crates/jmap/src/auth/oauth/auth.rs b/crates/jmap/src/auth/oauth/auth.rs index a669b60b..eba15ccc 100644 --- a/crates/jmap/src/auth/oauth/auth.rs +++ b/crates/jmap/src/auth/oauth/auth.rs @@ -292,7 +292,14 @@ impl OAuthApiHandler for Server { "code token".to_string(), "id_token token".to_string(), ], - scopes_supported: vec!["openid".to_string(), "offline_access".to_string()], + scopes_supported: vec![ + "openid".to_string(), + "offline_access".to_string(), + "urn:ietf:params:jmap:core".to_string(), + "urn:ietf:params:jmap:mail".to_string(), + "urn:ietf:params:jmap:submission".to_string(), + "urn:ietf:params:jmap:vacationresponse".to_string(), + ], issuer: base_url, }) .into_http_response()) diff --git a/tests/src/jmap/enterprise.rs b/tests/src/jmap/enterprise.rs index d81eccd8..a04b73a3 100644 --- a/tests/src/jmap/enterprise.rs +++ b/tests/src/jmap/enterprise.rs @@ -86,7 +86,7 @@ pub async fn test(params: &mut JMAPTest) { license: LicenseKey { valid_to: now() + 3600, valid_from: now() - 3600, - hostname: String::new(), + domain: String::new(), accounts: 100, }, undelete: Undelete { @@ -162,7 +162,7 @@ impl EnterpriseCore for Core { license: LicenseKey { valid_to: now() + 3600, valid_from: now() - 3600, - hostname: String::new(), + domain: String::new(), accounts: 100, }, undelete: None, diff --git a/tests/src/jmap/mod.rs b/tests/src/jmap/mod.rs index fe0302ef..58f4ba52 100644 --- a/tests/src/jmap/mod.rs +++ b/tests/src/jmap/mod.rs @@ -291,7 +291,7 @@ refresh-token-renew = "2s" [oauth.client-registration] anonymous = true -required = true +require = true [oauth.oidc] signature-key = '''-----BEGIN PRIVATE KEY----- -- cgit v1.2.3-70-g09d2