[Web] Mail policies, groups: check domain access first

data/web/inc/lib/mailcow/MailPolicy/Service/PolicyService.php

@@ -168,13 +168,14 @@ class PolicyService
     /** @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;
     }
 

data/web/inc/lib/mailcow/MailboxGroup/Service/GroupService.php

@@ -195,12 +195,12 @@ class GroupService
     {
         $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->lookup->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;
     }
 
@@ -304,6 +304,9 @@ class GroupService
         $new = array_values(array_diff(array_unique($add), array_map('strval', array_keys($members))));
         $regular = $this->lookup->regularMailboxes($new);
         foreach ($new as $username) {
+            if ($this->foreignDomain($username)) {
+                throw new AccessDenied();
+            }
             if (!in_array($username, $regular, true)) {
                 throw new UserError('group_member_invalid', [htmlspecialchars($username)]);
             }
@@ -337,12 +340,12 @@ class GroupService
                 // The group's own address is covered by sender_allowed
                 $valid = ($address !== null && $address !== $group->address && $this->lookup->aliasExists($address));
             }
+            if ($this->foreignDomain($address)) {
+                throw new AccessDenied();
+            }
             if (!$valid) {
                 throw new UserError('group_send_as_invalid', [htmlspecialchars($value)]);
             }
-            if (!$this->context->hasAdminRights() && !$this->context->canAccessDomain(self::domainOf((string)$address))) {
-                throw new AccessDenied();
-            }
             $sendAs[] = (string)$address;
         }
         $sendAs = array_values(array_unique($sendAs));
@@ -505,4 +508,11 @@ class GroupService
     {
         return substr($address, strrpos($address, '@') + 1);
     }
+
+    /** A domain admin names an address or @domain outside their domains: denied before anything is looked up */
+    private function foreignDomain(?string $address): bool
+    {
+        $domain = ($address !== null && str_contains($address, '@')) ? self::domainOf($address) : '';
+        return $domain !== '' && !$this->context->hasAdminRights() && !$this->context->canAccessDomain($domain);
+    }
 }

tests/php/MailPolicy/Service/PolicyServiceTest.php

@@ -28,7 +28,7 @@ class PolicyServiceTest extends TestCase
     protected function setUp(): void
     {
         $this->repo = $this->createMock(PolicyRepository::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(function (int $id): ?string {
@@ -91,6 +91,7 @@ class PolicyServiceTest extends TestCase
         ], array_intersect_key($policy->toArray(), array_flip(['domain', 'name', 'description', 'send_mode', 'send_allow', 'send_block',
             'receive_mode', 'receive_allow', 'receive_block', 'reject_text', 'active', 'mailboxes', 'groups'])));
         $this->assertCount(1, $this->saved);
+        $this->assertSame('xn--mller-kva.de', $this->service()->create(['domain' => 'Müller.de', 'name' => 'Test'])->domain);
     }
 
     public function testUpdateKeepsMissingValuesAndReplacesListGroups(): void
@@ -153,6 +154,7 @@ class PolicyServiceTest extends TestCase
         $this->assertNotNull($this->service($da)->get(3));
         // other domains, no ACL, users and read-only keys
         $this->expectUserError('access_denied', fn() => $this->service($da)->create(['domain' => 'tochter.de', 'name' => 'Test']));
+        $this->expectUserError('access_denied', fn() => $this->service($da)->create(['domain' => 'unbekannt.de', 'name' => 'Test']));
         $this->expectUserError(['mail_policy_entry_invalid', 'group:9'],
             fn() => $this->service($da)->create(['domain' => 'firma.de', 'name' => 'Test', 'send_block_groups' => ['9']]));
         $this->assertNull($this->service(FakeContext::domainAdmin([], ['tochter.de'], ['mail_policies' => '1']))->get(3));

tests/php/MailboxGroup/Service/GroupServiceTest.php

@@ -184,6 +184,15 @@ class GroupServiceTest extends TestCase
         $this->expectUserError('access_denied', fn() => $service->create(['domain' => 'other.org', 'name' => 'X']));
         $this->expectUserError('access_denied', fn() => $service->create(['domain' => 'example.org', 'name' => 'X', 'members' => ['c@other.org']]));
         $this->expectUserError('access_denied', fn() => $service->create(['domain' => 'example.org', 'name' => 'X', 'send_as' => '@other.org']));
+        // unknown names outside their domains look the same as existing ones
+        $this->expectUserError('access_denied', fn() => $service->create(['domain' => 'nope.org', 'name' => 'X']));
+        $this->expectUserError('access_denied', fn() => $service->create(['domain' => 'example.org', 'name' => 'X', 'members' => ['room1@other.org']]));
+        $this->expectUserError('access_denied', fn() => $service->create(['domain' => 'example.org', 'name' => 'X', 'send_as' => '@nope.org']));
+        $this->expectUserError('access_denied', fn() => $service->create(['domain' => 'example.org', 'name' => 'X', 'send_as' => 'nobody@other.org']));
+        $this->expectUserError(['group_member_invalid', 'room1@example.org'],
+            fn() => $service->create(['domain' => 'example.org', 'name' => 'X', 'members' => ['room1@example.org']]));
+        $this->expectUserError(['group_send_as_invalid', 'nobody@example.org'],
+            fn() => $service->create(['domain' => 'example.org', 'name' => 'X', 'send_as' => 'nobody@example.org']));
     }
 
     public function testAdminFieldsAreIgnoredForDomainAdmins(): void