From 6796db6b8f1f32f467ebde4ac535413c91ecd29b Mon Sep 17 00:00:00 2001
From: FreddleSpl0it <75116288+FreddleSpl0it@users.noreply.github.com>
Date: Mon, 9 Nov 2026 12:09:57 +0100
Subject: [PATCH] [Web] Retention: last run for single rules, mode only when
 editing, check domain access first

---
 .../Controller/RetentionApiController.php     |  2 +-
 .../mailcow/Retention/Service/RuleService.php |  9 +++---
 .../templates/edit/retention-rule-fields.twig |  2 ++
 .../Controller/RetentionApiControllerTest.php | 32 +++++++++++++++++++
 .../php/Retention/Service/RuleServiceTest.php |  5 ++-
 5 files changed, 44 insertions(+), 6 deletions(-)
 create mode 100644 tests/php/Retention/Controller/RetentionApiControllerTest.php

diff --git a/data/web/inc/lib/mailcow/Retention/Controller/RetentionApiController.php b/data/web/inc/lib/mailcow/Retention/Controller/RetentionApiController.php
index 3c55862b8..321f937b5 100644
--- a/data/web/inc/lib/mailcow/Retention/Controller/RetentionApiController.php
+++ b/data/web/inc/lib/mailcow/Retention/Controller/RetentionApiController.php
@@ -71,7 +71,7 @@ class RetentionApiController
     {
         $id = $this->toId($id);
         $rule = ($id === null) ? null : $this->rules->get($id);
-        return ($rule === null) ? [] : $this->rules->toArray($rule);
+        return ($rule === null) ? [] : $this->rules->toArray($rule, $this->rules->lastRuns([$id])[$id] ?? null);
     }
 
     /**
diff --git a/data/web/inc/lib/mailcow/Retention/Service/RuleService.php b/data/web/inc/lib/mailcow/Retention/Service/RuleService.php
index 06acbe2f0..3596b9543 100644
--- a/data/web/inc/lib/mailcow/Retention/Service/RuleService.php
+++ b/data/web/inc/lib/mailcow/Retention/Service/RuleService.php
@@ -196,13 +196,14 @@ class RuleService
     /** @throws UserError */
     private function domainFor(mixed $value): string
     {
-        $domain = strtolower((string)Input::text($value));
+        $domain = Input::text($value);
+        $domain = ($domain === null || $domain === '') ? '' : (string)idn_to_ascii(strtolower($domain), 0, INTL_IDNA_VARIANT_UTS46);
+        if ($domain !== '' && !$this->context->hasAdminRights() && !$this->context->canAccessDomain($domain)) {
+            throw new AccessDenied();
+        }
         if ($domain === '' || !$this->repo->isDomain($domain)) {
             throw new UserError('domain_not_found', [htmlspecialchars((string)Input::text($value))]);
         }
-        if (!$this->context->hasAdminRights() && !$this->context->canAccessDomain($domain)) {
-            throw new AccessDenied();
-        }
         return $domain;
     }
 
diff --git a/data/web/templates/edit/retention-rule-fields.twig b/data/web/templates/edit/retention-rule-fields.twig
index 2827a30b8..d739af52a 100644
--- a/data/web/templates/edit/retention-rule-fields.twig
+++ b/data/web/templates/edit/retention-rule-fields.twig
@@ -43,6 +43,7 @@
     </div>
   </div>
 </div>
+{% if rule.id is defined %}
 <div class="row mb-2">
   <label class="control-label col-sm-2{{ label_class }}" for="{{ id }}_mode">{{ lang.retention.mode }}</label>
   <div class="col-sm-10">
@@ -54,6 +55,7 @@
     <small class="text-muted d-block">{{ lang.retention.mode_info }}</small>
   </div>
 </div>
+{% endif %}
 <legend class="fs-5 mt-4"><i class="bi bi-people"></i> {{ lang.retention.assign }}</legend><hr>
 <p class="text-muted small">{{ lang.retention.assign_info }}</p>
 <div class="row mb-2">
diff --git a/tests/php/Retention/Controller/RetentionApiControllerTest.php b/tests/php/Retention/Controller/RetentionApiControllerTest.php
new file mode 100644
index 000000000..6ad6b4d52
--- /dev/null
+++ b/tests/php/Retention/Controller/RetentionApiControllerTest.php
@@ -0,0 +1,32 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Mailcow\Tests\Retention\Controller;
+
+use Mailcow\Core\ApiResponse;
+use Mailcow\Retention\Controller\RetentionApiController;
+use Mailcow\Retention\Model\Rule;
+use Mailcow\Retention\Repository\RuleRepository;
+use Mailcow\Retention\Service\RuleService;
+use Mailcow\Tests\Support\FakeContext;
+use PHPUnit\Framework\TestCase;
+
+class RetentionApiControllerTest extends TestCase
+{
+    public function testASingleRuleCarriesItsLastRun(): void
+    {
+        $rule = new Rule();
+        $rule->id = 3;
+        $rule->domain = 'firma.de';
+        $rule->name = 'Papierkorb';
+        $run = ['rule_id' => 3, 'created' => '2026-11-09 03:30:00', 'mailboxes' => 2, 'matched' => 5, 'deleted' => 0, 'errors' => 0, 'simulated' => 1];
+        $repo = $this->createMock(RuleRepository::class);
+        $repo->method('find')->willReturn($rule);
+        $repo->method('lastRuns')->with([3])->willReturn([3 => $run]);
+        $api = new RetentionApiController(new RuleService($repo, FakeContext::admin()), new ApiResponse());
+
+        $this->assertSame($run, $api->getRule('3')['last_run']);
+        $this->assertSame([], $api->getRule('x'));
+    }
+}
diff --git a/tests/php/Retention/Service/RuleServiceTest.php b/tests/php/Retention/Service/RuleServiceTest.php
index 4e293b32d..18b2af15b 100644
--- a/tests/php/Retention/Service/RuleServiceTest.php
+++ b/tests/php/Retention/Service/RuleServiceTest.php
@@ -24,7 +24,7 @@ class RuleServiceTest extends TestCase
     protected function setUp(): void
     {
         $this->repo = $this->createMock(RuleRepository::class);
-        $this->repo->method('isDomain')->willReturnCallback(fn(string $domain): bool => in_array($domain, ['firma.de', 'tochter.de'], true));
+        $this->repo->method('isDomain')->willReturnCallback(fn(string $domain): bool => in_array($domain, ['firma.de', 'tochter.de', 'xn--mller-kva.de'], true));
         $this->repo->method('mailboxDomain')->willReturnCallback(fn(string $username): ?string =>
             ['azubi@firma.de' => 'firma.de', 'chef@firma.de' => 'firma.de', 'andere@tochter.de' => 'tochter.de'][$username] ?? null);
         $this->repo->method('groupDomain')->willReturnCallback(fn(int $id): ?string => [7 => 'firma.de', 9 => 'tochter.de'][$id] ?? null);
@@ -74,6 +74,7 @@ class RuleServiceTest extends TestCase
         $this->assertSame(['domain' => 'firma.de', 'name' => 'Aufräumen', 'folders' => ['trash', 'junk', 'folder:Projekte/*', 'folder:Newsletter'],
             'days' => 30, 'keep_flagged' => 1, 'mode' => 'simulate', 'all_mailboxes' => 0, 'active' => 1, 'mailboxes' => ['azubi@firma.de'], 'groups' => [7]],
             array_intersect_key($rule->toArray(), array_flip(['domain', 'name', 'folders', 'days', 'keep_flagged', 'mode', 'all_mailboxes', 'active', 'mailboxes', 'groups'])));
+        $this->assertSame('xn--mller-kva.de', $this->service()->create(['domain' => 'Müller.de', 'name' => 'Test', 'folders' => ['trash'], 'days' => 30])->domain);
     }
 
     public function testUpdateKeepsMissingValuesAndReplacesOwnFolders(): void
@@ -122,6 +123,8 @@ class RuleServiceTest extends TestCase
         $this->assertSame('Test', $this->service($da)->create($create('firma.de'))->name);
         $this->assertNotNull($this->service($da)->get(3));
         $this->expectUserError('access_denied', fn() => $this->service($da)->create($create('tochter.de')));
+        $this->expectUserError('access_denied', fn() => $this->service($da)->create($create('unbekannt.de')));
+        $this->expectUserError(['domain_not_found', 'unbekannt.de'], fn() => $this->service()->create($create('unbekannt.de')));
         $this->assertNull($this->service(FakeContext::domainAdmin([], ['tochter.de'], ['retention' => '1']))->get(3));
         $this->expectUserError('access_denied', fn() => $this->service(FakeContext::domainAdmin([], ['firma.de'], ['mail_policies' => '1']))->create($create('firma.de')));
         $this->expectUserError('access_denied', fn() => $this->service(FakeContext::user('azubi@firma.de'))->update(3, []));