crates/jmap/src/registry/mapping/domain.rs

@@ -2,6 +2,8 @@
  * SPDX-FileCopyrightText: 2020 Stalwart Labs LLC <[email protected]>
  *
  * SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-SEL
+ *
+ * Modified by Coffey Labs in 2026 for INBUXA.
  */
 
 use crate::registry::mapping::{
@@ -151,12 +153,7 @@ pub(crate) async fn validate_domain(
     // Schedule ACME renewal task if needed
     if !will_trigger_acme
         && let CertificateManagement::Automatic(details) = &domain.certificate_management
-        && old_domain.is_none_or(|old| {
-            !matches!(
-                old.certificate_management,
-                CertificateManagement::Automatic(_)
-            )
-        })
+        && needs_certificate_order(domain, old_domain)
     {
         let Some(provider) = set
             .server
@@ -179,15 +176,82 @@ pub(crate) async fn validate_domain(
                 )));
         }
 
-        tasks.push(Task::AcmeRenewal(TaskDomainManagement {
-            domain_id: Id::default(),
-            status: TaskStatus::now(),
-        }));
+        // inbuxa: CAA and the DNS-PERSIST-01 record name the provider, and the
+        // persist record says whether wildcards are allowed. Publish them
+        // before ordering, or the CA refuses the order (#290).
+        let publishes_caa = matches!(
+            &domain.dns_management,
+            DnsManagement::Automatic(dns) if dns.publish_records.contains(&DnsRecordType::Caa)
+        );
+        if matches!(domain.dns_management, DnsManagement::Automatic(_))
+            && (publishes_caa || matches!(provider.challenge_type, AcmeChallengeType::DnsPersist01))
+            && acme_records_outdated(domain, old_domain)
+        {
+            tasks.push(Task::DnsManagement(TaskDnsManagement {
+                domain_id: Id::default(),
+                update_records: Map::new(if publishes_caa {
+                    vec![DnsRecordType::Caa]
+                } else {
+                    vec![]
+                }),
+                on_success_renew_certificate: true,
+                status: TaskStatus::now(),
+            }));
+        } else {
+            tasks.push(Task::AcmeRenewal(TaskDomainManagement {
+                domain_id: Id::default(),
+                status: TaskStatus::now(),
+            }));
+        }
     }
 
     Ok(Ok(response))
 }
 
+/// inbuxa: whether a save changed what the domain's certificate is ordered
+/// for. The renewal task decides what to do about it: a certificate that
+/// already has these names just waits for its renewal date (#290).
+fn needs_certificate_order(domain: &Domain, old_domain: Option<&Domain>) -> bool {
+    let CertificateManagement::Automatic(new) = &domain.certificate_management else {
+        return false;
+    };
+    let Some(old_domain) = old_domain else {
+        return true;
+    };
+    let CertificateManagement::Automatic(old) = &old_domain.certificate_management else {
+        return true;
+    };
+    let names = |list: &'_ Map<String>| {
+        let mut names = list.as_slice().to_vec();
+        names.sort();
+        names.dedup();
+        names
+    };
+
+    old_domain.name != domain.name
+        || old.acme_provider_id != new.acme_provider_id
+        || names(&old.subject_alternative_names) != names(&new.subject_alternative_names)
+}
+
+/// inbuxa: whether a save left the published CAA and DNS-PERSIST-01 records
+/// out of date. Only an empty name list asks for a wildcard, so changing one
+/// list of names for another doesn't touch them.
+fn acme_records_outdated(domain: &Domain, old_domain: Option<&Domain>) -> bool {
+    let CertificateManagement::Automatic(new) = &domain.certificate_management else {
+        return false;
+    };
+    let Some(old_domain) = old_domain else {
+        return true;
+    };
+    let CertificateManagement::Automatic(old) = &old_domain.certificate_management else {
+        return true;
+    };
+
+    old_domain.name != domain.name
+        || old.acme_provider_id != new.acme_provider_id
+        || old.subject_alternative_names.is_empty() != new.subject_alternative_names.is_empty()
+}
+
 fn publishes_dkim(domain: &Domain) -> bool {
     matches!(
         &domain.dns_management,
@@ -226,3 +290,88 @@ pub(crate) async fn validate_dns_server(
 
     Ok(Ok(response))
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use registry::schema::structs::CertificateManagementProperties;
+
+    fn domain(name: &str, provider: u64, names: &[&str]) -> Domain {
+        Domain {
+            name: name.to_string(),
+            certificate_management: CertificateManagement::Automatic(
+                CertificateManagementProperties {
+                    acme_provider_id: Id::from(provider),
+                    subject_alternative_names: Map::new(
+                        names.iter().map(|name| name.to_string()).collect(),
+                    ),
+                },
+            ),
+            ..Default::default()
+        }
+    }
+
+    #[test]
+    fn saving_the_same_names_orders_nothing() {
+        let old = domain("example.org", 1, &["mail", "imap"]);
+        assert!(!needs_certificate_order(
+            &domain("example.org", 1, &["imap", "mail"]),
+            Some(&old)
+        ));
+        assert!(!acme_records_outdated(
+            &domain("example.org", 1, &["imap", "mail"]),
+            Some(&old)
+        ));
+    }
+
+    #[test]
+    fn changing_the_names_orders_a_certificate() {
+        let old = domain("example.org", 1, &["mail"]);
+        for new in [
+            domain("example.org", 1, &["mail", "imap"]),
+            domain("example.org", 1, &["imap"]),
+            domain("example.org", 1, &[]),
+            domain("example.org", 2, &["mail"]),
+            domain("example.net", 1, &["mail"]),
+        ] {
+            assert!(needs_certificate_order(&new, Some(&old)));
+        }
+    }
+
+    #[test]
+    fn switching_to_automatic_orders_a_certificate_and_manual_doesnt() {
+        let manual = Domain {
+            certificate_management: CertificateManagement::Manual,
+            ..domain("example.org", 1, &[])
+        };
+        let automatic = domain("example.org", 1, &[]);
+        assert!(needs_certificate_order(&automatic, Some(&manual)));
+        assert!(needs_certificate_order(&automatic, None));
+        assert!(!needs_certificate_order(&manual, Some(&automatic)));
+        assert!(!needs_certificate_order(&manual, Some(&manual)));
+    }
+
+    #[test]
+    fn records_only_go_stale_when_what_they_say_changes() {
+        let old = domain("example.org", 1, &["mail"]);
+        // Still no wildcard, same provider
+        assert!(!acme_records_outdated(
+            &domain("example.org", 1, &["imap"]),
+            Some(&old)
+        ));
+        // Wildcard allowed from here on
+        assert!(acme_records_outdated(
+            &domain("example.org", 1, &[]),
+            Some(&old)
+        ));
+        // A different provider or owner name
+        assert!(acme_records_outdated(
+            &domain("example.org", 2, &["mail"]),
+            Some(&old)
+        ));
+        assert!(acme_records_outdated(
+            &domain("example.net", 1, &["mail"]),
+            Some(&old)
+        ));
+    }
+}

crates/jmap/src/registry/set.rs

@@ -55,11 +55,12 @@ use registry::{
         },
         structs::{
             Certificate, DkimSignature, DnsServer, Domain, PublicKey, Role, SieveSystemScript,
-            SieveUserScript, Task,
+            SieveUserScript, Task, TaskDnsManagement,
         },
     },
     types::id::ObjectId,
 };
+use services::task_manager::{domain_check::clear_queued_renewals, lock::TaskLockManager};
 use std::borrow::Cow;
 use store::{
     registry::{
@@ -866,6 +867,24 @@ impl RegistrySet for Server {
                         && let Some(object_id) = object_id
                     {
                         let mut batch = BatchBuilder::new();
+
+                        // inbuxa: a new renewal replaces the domain's queued
+                        // one, or both would keep renewing from here on (#290)
+                        let replaced = if tasks.iter().any(|task| {
+                            matches!(
+                                task,
+                                Task::AcmeRenewal(_)
+                                    | Task::DnsManagement(TaskDnsManagement {
+                                        on_success_renew_certificate: true,
+                                        ..
+                                    })
+                            )
+                        }) {
+                            clear_queued_renewals(set.server, object_id, &mut batch).await?
+                        } else {
+                            Vec::new()
+                        };
+
                         for mut task in tasks.drain(..) {
                             match &mut task {
                                 Task::AcmeRenewal(task) => task.domain_id = object_id,
@@ -876,6 +895,9 @@ impl RegistrySet for Server {
                             batch.schedule_task(task);
                         }
                         set.server.store().write(batch.build_all()).await?;
+                        for id in replaced {
+                            set.server.remove_index_lock(id).await;
+                        }
                         set.server.notify_task_queue();
                     }
                 }

crates/services/src/task_manager/dns.rs

@@ -103,7 +103,9 @@ impl DnsManagementTask for Server {
 }
 
 async fn dns_management(server: &Server, task: &TaskDnsManagement) -> trc::Result<TaskResult> {
-    if task.update_records.is_empty() {
+    // inbuxa: a task that renews afterwards can have an empty list: the
+    // DNS-PERSIST-01 record is published whichever types are asked for (#290)
+    if task.update_records.is_empty() && !task.on_success_renew_certificate {
         return Ok(TaskResult::permanent(
             "No DNS records to update".to_string(),
         ));

crates/services/src/task_manager/domain_check.rs

@@ -10,6 +10,7 @@
 //! whose chain was broken) never got another, so its keys stopped rotating.
 //! This check runs on the task scheduler and queues the missing task.
 
+use crate::task_manager::lock::TaskLockManager;
 use common::Server;
 use registry::{
     schema::{
@@ -154,10 +155,45 @@ async fn has_dkim_keys(server: &Server, domain_id: Id) -> trc::Result<bool> {
         .caused_by(trc::location!())
 }
 
-async fn queued_domain_tasks(server: &Server) -> trc::Result<QueuedDomainTasks> {
-    // The due index carries each task's type, so only the few domain tasks
-    // are read in full. Failed tasks sit at the end of it (due u64::MAX).
-    let mut ids = Vec::new();
+/// Adds to `batch` the removal of a domain's queued renewal tasks, whatever
+/// their state, because a new one is about to replace them: each run queues
+/// the next, so every extra task would be another chain of renewals, for good
+/// (#290). Should the new one never run, the scheduled check queues another.
+///
+/// Returns the tasks it locked, to unlock once the batch is written. A task
+/// that's running right now can't be removed, and it stays.
+pub async fn clear_queued_renewals(
+    server: &Server,
+    domain_id: Id,
+    batch: &mut BatchBuilder,
+) -> trc::Result<Vec<u64>> {
+    let mut locked = Vec::new();
+    for (due, id) in queued_tasks(server, &[TaskType::AcmeRenewal]).await? {
+        let task = server
+            .store()
+            .get_value::<Task>(ValueKey::from(ValueClass::TaskQueue(
+                TaskQueueClass::Task { id },
+            )))
+            .await
+            .caused_by(trc::location!())?;
+        if matches!(task, Some(Task::AcmeRenewal(task)) if task.domain_id == domain_id)
+            && server.try_lock_task(id).await
+        {
+            batch
+                .clear(ValueClass::TaskQueue(TaskQueueClass::Task { id }))
+                .clear(ValueClass::TaskQueue(TaskQueueClass::Due { id, due }));
+            locked.push(id);
+        }
+    }
+
+    Ok(locked)
+}
+
+/// The due time and id of every queued task of the wanted types. The due
+/// index carries each task's type, so nothing is read in full. Failed tasks
+/// sit at the end of it (due u64::MAX).
+async fn queued_tasks(server: &Server, wanted: &[TaskType]) -> trc::Result<Vec<(u64, u64)>> {
+    let mut tasks = Vec::new();
     server
         .store()
         .iterate(
@@ -171,11 +207,8 @@ async fn queued_domain_tasks(server: &Server) -> trc::Result<QueuedDomainTasks>
             .ascending(),
             |key, value| {
                 let task_type = value.deserialize_be_u16(0).ok().and_then(TaskType::from_id);
-                if matches!(
-                    task_type,
-                    Some(TaskType::DkimManagement | TaskType::AcmeRenewal)
-                ) {
-                    ids.push(key.deserialize_be_u64(U64_LEN)?);
+                if task_type.is_some_and(|typ| wanted.contains(&typ)) {
+                    tasks.push((key.deserialize_be_u64(0)?, key.deserialize_be_u64(U64_LEN)?));
                 }
                 Ok(true)
             },
@@ -183,8 +216,14 @@ async fn queued_domain_tasks(server: &Server) -> trc::Result<QueuedDomainTasks>
         .await
         .caused_by(trc::location!())?;
 
+    Ok(tasks)
+}
+
+async fn queued_domain_tasks(server: &Server) -> trc::Result<QueuedDomainTasks> {
+    let tasks = queued_tasks(server, &[TaskType::DkimManagement, TaskType::AcmeRenewal]).await?;
+
     let mut queued = QueuedDomainTasks::default();
-    for id in ids {
+    for (_, id) in tasks {
         match server
             .store()
             .get_value::<Task>(ValueKey::from(ValueClass::TaskQueue(

tests/src/automation/acme.rs

@@ -445,6 +445,149 @@ pub async fn test(test: &TestServer) {
         vec!["*.persist.org".to_string(), "persist.org".to_string()]
     );
 
+    account.registry_destroy_all(ObjectType::Certificate).await;
+    account.registry_destroy_all(ObjectType::Task).await;
+
+    // Changing a domain's names orders a certificate for them straight away,
+    // and leaves one renewal queued (#290)
+    let names_acme_id = account
+        .registry_create_object(AcmeProvider {
+            directory: "https://localhost:14000/dir".to_string(),
+            contact: Map::new(vec!["mailto:[email protected]".to_string()]),
+            challenge_type: AcmeChallengeType::TlsAlpn01,
+            ..Default::default()
+        })
+        .await;
+    let names_domain = |names: &[&str]| {
+        CertificateManagement::Automatic(CertificateManagementProperties {
+            acme_provider_id: names_acme_id,
+            subject_alternative_names: Map::new(names.iter().map(|n| n.to_string()).collect()),
+        })
+    };
+    let names_domain_id = account
+        .registry_create_object(Domain {
+            name: "names.org".to_string(),
+            certificate_management: names_domain(&["a", "b"]),
+            dkim_management: DkimManagement::Manual,
+            dns_management: DnsManagement::Manual,
+            ..Default::default()
+        })
+        .await;
+    test.wait_for_tasks_skip_not_due().await;
+    assert_certificates(account, &[&["a.names.org", "b.names.org"]]).await;
+    assert_one_renewal(account, names_domain_id).await;
+
+    // Same names in a different order: nothing to do
+    account
+        .registry_update_object(
+            ObjectType::Domain,
+            names_domain_id,
+            json!({ Property::CertificateManagement: names_domain(&["b", "a"]) }),
+        )
+        .await;
+    test.wait_for_tasks_skip_not_due().await;
+    assert_certificates(account, &[&["a.names.org", "b.names.org"]]).await;
+    assert_one_renewal(account, names_domain_id).await;
+
+    // A new name
+    account
+        .registry_update_object(
+            ObjectType::Domain,
+            names_domain_id,
+            json!({ Property::CertificateManagement: names_domain(&["a", "b", "c"]) }),
+        )
+        .await;
+    test.wait_for_tasks_skip_not_due().await;
+    assert_certificates(
+        account,
+        &[
+            &["a.names.org", "b.names.org"],
+            &["a.names.org", "b.names.org", "c.names.org"],
+        ],
+    )
+    .await;
+    assert_one_renewal(account, names_domain_id).await;
+
+    // Off and on again, the way around this before it was fixed
+    account
+        .registry_update_object(
+            ObjectType::Domain,
+            names_domain_id,
+            json!({ Property::CertificateManagement: CertificateManagement::Manual }),
+        )
+        .await;
+    account
+        .registry_update_object(
+            ObjectType::Domain,
+            names_domain_id,
+            json!({ Property::CertificateManagement: names_domain(&["a", "b", "c"]) }),
+        )
+        .await;
+    test.wait_for_tasks_skip_not_due().await;
+    assert_one_renewal(account, names_domain_id).await;
+
+    // DNS-PERSIST-01 only allows a wildcard if its record says so. Going from
+    // named hosts to the wildcard needs that record republished first.
+    let names_persist_id = account
+        .registry_create_object(Domain {
+            name: "names-persist.org".to_string(),
+            certificate_management: CertificateManagement::Automatic(
+                CertificateManagementProperties {
+                    acme_provider_id: dns_acme_id,
+                    subject_alternative_names: Map::new(vec!["www".to_string()]),
+                },
+            ),
+            dkim_management: DkimManagement::Manual,
+            dns_management: DnsManagement::Automatic(DnsManagementProperties {
+                dns_server_id: pebble_dns_id,
+                publish_records: Map::new(vec![DnsRecordType::TlsRpt]),
+                ..Default::default()
+            }),
+            ..Default::default()
+        })
+        .await;
+    test.wait_for_tasks_skip_not_due().await;
+    account
+        .registry_update_object(
+            ObjectType::Domain,
+            names_persist_id,
+            json!({
+                Property::CertificateManagement: CertificateManagement::Automatic(
+                    CertificateManagementProperties {
+                        acme_provider_id: dns_acme_id,
+                        subject_alternative_names: Default::default(),
+                    },
+                ),
+            }),
+        )
+        .await;
+    test.wait_for_tasks_skip_not_due().await;
+    let persist_sans = account
+        .registry_get_all::<Certificate>()
+        .await
+        .into_iter()
+        .map(|(_, certificate)| {
+            let mut sans = certificate.subject_alternative_names.into_inner();
+            sans.sort();
+            sans
+        })
+        .collect::<Vec<_>>();
+    assert!(
+        persist_sans.contains(&vec![
+            "*.names-persist.org".to_string(),
+            "names-persist.org".to_string()
+        ]),
+        "no wildcard certificate: {persist_sans:?}"
+    );
+    assert_one_renewal(account, names_persist_id).await;
+
+    account
+        .registry_destroy(ObjectType::Domain, [names_domain_id, names_persist_id])
+        .await
+        .assert_destroyed(&[names_domain_id, names_persist_id]);
+    account.registry_destroy_all(ObjectType::Certificate).await;
+    account.registry_destroy_all(ObjectType::Task).await;
+
     // Test preferred chain selection against the alternate chains Pebble offers (RFC 8555 7.4.2)
     let pebble_roots = pebble_root_common_names().await;
     assert!(
@@ -688,6 +831,39 @@ pub async fn test(test: &TestServer) {
     account.registry_destroy_all(ObjectType::Task).await;
 }
 
+/// The stored certificates are exactly these sets of names.
+async fn assert_certificates(account: &crate::utils::account::Account, expected: &[&[&str]]) {
+    let mut found = account
+        .registry_get_all::<Certificate>()
+        .await
+        .into_iter()
+        .map(|(_, certificate)| {
+            let mut sans = certificate.subject_alternative_names.into_inner();
+            sans.sort();
+            sans
+        })
+        .collect::<Vec<_>>();
+    found.sort();
+    let mut expected = expected
+        .iter()
+        .map(|names| names.iter().map(|n| n.to_string()).collect::<Vec<_>>())
+        .collect::<Vec<_>>();
+    expected.sort();
+    assert_eq!(found, expected);
+}
+
+async fn assert_one_renewal(account: &crate::utils::account::Account, domain: types::id::Id) {
+    let renewals = account
+        .registry_get_all::<Task>()
+        .await
+        .into_iter()
+        .filter(|(_, task)| {
+            matches!(task, Task::AcmeRenewal(TaskDomainManagement { domain_id, .. }) if *domain_id == domain)
+        })
+        .count();
+    assert_eq!(renewals, 1, "expected one queued renewal for the domain");
+}
+
 const ISSUER_ABC_CERT: &str = r#"-----BEGIN CERTIFICATE-----
 MIIDLjCCAhagAwIBAgIURV4DMcpCSV95vODPEDWtzZ0XTjIwDQYJKoZIhvcNAQEL
 BQAwGDEWMBQGA1UEAwwNSXNzdWVyIEFCQyBDQTAeFw0yNjA0MDMxMjQ1MjVaFw0y