diff --git a/README.md b/README.md index a74887933..c33ef9d1c 100644 --- a/README.md +++ b/README.md @@ -110,6 +110,16 @@ If that setting is turned on, guests can only be invited by [group admins](https Upon invitation, the group admin must select at least one of their adminstrated groups the guest shall be member of. +### Blocking email domains + +Administrators can prevent guest accounts from being created for specific email domains (_Settings > Administration > Guests > `Block guest invitations for these email domains`_). An entry such as `example.com` blocks `example.com` and all of its subdomains (e.g. `mail.example.com`), but not other domains that merely end with the same characters (e.g. `otherexample.com`). The blocklist applies to invitations from the web UI, the API and `occ guests:add`; existing guest accounts are not affected. + +The list can also be set from the command line: + +```bash +occ config:app:set guests blocked_domains --value='["example.com","example.org"]' --type=array +``` + ### Share acceptance Guest users automatically accept all files and folders that are shared with them. This is in constract to regular full users (who have to actively accept incoming shares since Nextcloud 18). diff --git a/lib/Command/AddCommand.php b/lib/Command/AddCommand.php index 5eda87ee5..c4889c6b6 100644 --- a/lib/Command/AddCommand.php +++ b/lib/Command/AddCommand.php @@ -114,6 +114,11 @@ protected function execute(InputInterface $input, OutputInterface $output): int return self::FAILURE; } + if ($this->config->isEmailDomainBlocked($email)) { + $output->writeln('Guest accounts cannot be created for the domain of "' . $email . '", it is on the blocklist.'); + return self::FAILURE; + } + $password = null; if (!$input->getOption('generate-password')) { if ($input->getOption('password-from-env')) { diff --git a/lib/Config.php b/lib/Config.php index 0022f5daa..9112442f8 100644 --- a/lib/Config.php +++ b/lib/Config.php @@ -163,4 +163,99 @@ public function getCreateRestrictedToGroup(): array { public function setCreateRestrictedToGroup(array $groups): void { $this->appConfig->setAppValueArray('create_restricted_to_group', $groups); } + + /** + * Email domains that must not be invited as guests. + * + * @return list + */ + public function getBlockedDomains(): array { + return self::normalizeDomains($this->appConfig->getAppValueArray(ConfigLexicon::BLOCKED_DOMAINS)); + } + + /** + * @param list $domains + */ + public function setBlockedDomains(array $domains): void { + $this->appConfig->setAppValueArray(ConfigLexicon::BLOCKED_DOMAINS, self::normalizeDomains($domains)); + } + + /** + * Whether the domain of the given email address is blocked for guest + * invitations. A blocked entry like "example.com" matches the domain + * itself and all of its subdomains (e.g. "mail.example.com"), but not + * unrelated domains sharing the same suffix (e.g. "otherexample.com"). + */ + public function isEmailDomainBlocked(string $email): bool { + $atPosition = strrpos($email, '@'); + if ($atPosition === false) { + return false; + } + $rawDomain = rtrim(mb_strtolower(trim(substr($email, $atPosition + 1))), '.'); + if ($rawDomain === '') { + return false; + } + $emailDomain = self::toAsciiDomain($rawDomain) ?? $rawDomain; + + foreach ($this->getBlockedDomains() as $blockedDomain) { + $blockedDomain = self::toAsciiDomain($blockedDomain); + if ($blockedDomain === null) { + continue; + } + if ($emailDomain === $blockedDomain || str_ends_with($emailDomain, '.' . $blockedDomain)) { + return true; + } + } + return false; + } + + /** + * Lower-case, trim and de-duplicate domain entries. Leading "@", "*." and + * "." as well as trailing dots are stripped, so "@Example.com" and + * "*.example.com" are both stored as "example.com". Entries that are not + * a valid domain name are dropped. Internationalized domains are kept in + * the form they were entered; "bücher.de" and "xn--bcher-kva.de" count as + * duplicates. + * + * @param array $domains + * @return list + */ + public static function normalizeDomains(array $domains): array { + $normalized = []; + foreach ($domains as $domain) { + if (!is_string($domain)) { + continue; + } + $domain = mb_strtolower(trim($domain)); + $domain = preg_replace('/^(@|\*\.|\.)+/u', '', $domain) ?? ''; + $domain = rtrim($domain, '.'); + $asciiDomain = self::toAsciiDomain($domain); + if ($asciiDomain === null || isset($normalized[$asciiDomain])) { + continue; + } + $normalized[$asciiDomain] = $domain; + } + return array_values($normalized); + } + + /** + * Convert a lower-case domain to its ASCII (punycode) form and validate it + * as a hostname: at most 253 characters, labels of 1-63 letters, digits or + * inner hyphens. + */ + private static function toAsciiDomain(string $domain): ?string { + if ($domain === '') { + return null; + } + $ascii = idn_to_ascii($domain, IDNA_NONTRANSITIONAL_TO_ASCII, INTL_IDNA_VARIANT_UTS46); + if ($ascii === false) { + return null; + } + $ascii = strtolower($ascii); + if (strlen($ascii) > 253 + || preg_match('/^[a-z0-9](?:[a-z0-9-]{0,61}[a-z0-9])?(?:\.[a-z0-9](?:[a-z0-9-]{0,61}[a-z0-9])?)*$/', $ascii) !== 1) { + return null; + } + return $ascii; + } } diff --git a/lib/ConfigLexicon.php b/lib/ConfigLexicon.php index a29a9c64f..4a7e32c23 100644 --- a/lib/ConfigLexicon.php +++ b/lib/ConfigLexicon.php @@ -28,6 +28,7 @@ class ConfigLexicon implements ILexicon { public const WHITE_LIST = 'whitelist'; public const GROUP_LIMITATION = 'create_restricted_to_group'; public const GUEST_DISK_QUOTA = 'guest_quota'; + public const BLOCKED_DOMAINS = 'blocked_domains'; public const USER_CREATED_BY = 'created_by'; #[\Override] @@ -48,6 +49,7 @@ public function getAppConfigs(): array { default => '0 B', }, definition: 'set default disk quota assigned to guest account at its creation'), new Entry(self::GROUP_LIMITATION, ValueType::ARRAY, []), + new Entry(self::BLOCKED_DOMAINS, ValueType::ARRAY, [], 'email domains that cannot be invited as guests (subdomains are included)'), ]; } diff --git a/lib/Controller/SettingsController.php b/lib/Controller/SettingsController.php index d655a7f43..656c3e5b5 100644 --- a/lib/Controller/SettingsController.php +++ b/lib/Controller/SettingsController.php @@ -58,13 +58,15 @@ public function getConfig(): DataResponse { 'createRestrictedToGroup' => $this->config->getCreateRestrictedToGroup(), 'guestQuota' => $this->config->hasGuestQuotaOverride() ? $this->config->getGuestQuota() : 'default', 'guestQuotaDefault' => $this->config->getGuestQuotaDefault(), + 'blockedDomains' => $this->config->getBlockedDomains(), ]); } /** * @param list $whitelist + * @param list|null $blockedDomains null keeps the stored list unchanged */ - public function setConfig(bool $useWhitelist, array $whitelist, bool $allowExternalStorage, bool $useHashedEmailAsUserID, bool $hideUsers, array $createRestrictedToGroup, string $guestQuota = 'default'): DataResponse { + public function setConfig(bool $useWhitelist, array $whitelist, bool $allowExternalStorage, bool $useHashedEmailAsUserID, bool $hideUsers, array $createRestrictedToGroup, string $guestQuota = 'default', ?array $blockedDomains = null): DataResponse { $newWhitelist = []; foreach ($whitelist as $app) { $newWhitelist[] = trim((string)$app); @@ -79,6 +81,9 @@ public function setConfig(bool $useWhitelist, array $whitelist, bool $allowExter if ($this->isValidQuota($guestQuota)) { $this->config->setGuestQuota($guestQuota); } + if ($blockedDomains !== null) { + $this->config->setBlockedDomains($blockedDomains); + } return new DataResponse(); } diff --git a/lib/Controller/UsersController.php b/lib/Controller/UsersController.php index ad5511425..4a003aeb9 100644 --- a/lib/Controller/UsersController.php +++ b/lib/Controller/UsersController.php @@ -115,6 +115,12 @@ public function create(string $email, string $displayName, string $language, arr ); } + if (!isset($errorMessages['email']) && $this->config->isEmailDomainBlocked($email)) { + $errorMessages['email'] = $this->l10n->t( + 'Guest accounts cannot be created for this email domain.' + ); + } + if ($this->config->useHashedEmailAsUserID()) { $email = strtolower($email); $username = hash('sha256', $email); diff --git a/package-lock.json b/package-lock.json index d832e9469..744fe1a06 100644 --- a/package-lock.json +++ b/package-lock.json @@ -6411,9 +6411,9 @@ } }, "node_modules/follow-redirects": { - "version": "1.16.1", - "resolved": "https://registry.npmjs.org/follow-redirects/-/follow-redirects-1.16.1.tgz", - "integrity": "sha512-FNvFGzoMLWmE6Yj9spb/zjd7yiNCHiAW9/Tg9CXrQ8wuu32HtlJOwWO11OJafl5FfY3DxTdQ0vj42zU1kvv5jg==", + "version": "1.16.0", + "resolved": "https://registry.npmjs.org/follow-redirects/-/follow-redirects-1.16.0.tgz", + "integrity": "sha512-y5rN/uOsadFT/JfYwhxRS5R7Qce+g3zG97+JrtFZlC9klX/W5hD7iiLzScI4nZqUS7DNUdhPgw4xI8W2LuXlUw==", "funding": [ { "type": "individual", diff --git a/src/views/GuestSettings.vue b/src/views/GuestSettings.vue index 0620157bf..68eee862f 100644 --- a/src/views/GuestSettings.vue +++ b/src/views/GuestSettings.vue @@ -76,6 +76,27 @@ @update:modelValue="onSelectGroups" />

+ +
+ + +

+ {{ t('guests', 'Subdomains are blocked as well. Existing guest accounts are not affected.') }} +

+
+ this.normalizeDomain(typeof domain === 'string' ? domain : (domain?.label ?? ''))) + const invalid = normalized.filter((domain) => domain !== '' && !this.isValidDomain(domain)) + if (invalid.length > 0) { + showError(t('guests', 'Invalid domain: {domain}', { domain: invalid[0] })) + } + this.config.blockedDomains = [...new Set(normalized.filter((domain) => this.isValidDomain(domain)))] + this.saveBlockedDomains() + }, + + async saveBlockedDomains() { + await this.saveConfig() + if (this.error) { + return + } + // The server validates and de-duplicates entries, show what it stored. + try { + const { data } = await axios.get(generateUrl('apps/guests/config')) + const rejected = this.config.blockedDomains.filter((domain) => !data.blockedDomains.includes(domain)) + if (rejected.length > 0) { + showError(t('guests', 'Invalid domain: {domain}', { domain: rejected[0] })) + } + this.config.blockedDomains = data.blockedDomains + } catch (error) { + logger.error('Error loading blocked domains', { error }) + } + }, + async loadConfig() { const { data } = await axios.get(generateUrl('apps/guests/config')) - this.config = data + this.config = { blockedDomains: [], ...data } this.loaded = true }, @@ -453,6 +517,26 @@ export default { } } +.blocked-domains { + margin-top: 1.5rem; + margin-bottom: 1rem; + max-width: 690px; + + &__label { + display: block; + margin-bottom: 4px; + } + + &__select { + width: 100%; + } + + &__hint { + margin-top: 4px; + color: var(--color-text-maxcontrast); + } +} + .guest-quota { margin-top: 1.5rem; max-width: 400px; diff --git a/tests/unit/Command/AddCommandTest.php b/tests/unit/Command/AddCommandTest.php index b8c89e730..05e817c9c 100644 --- a/tests/unit/Command/AddCommandTest.php +++ b/tests/unit/Command/AddCommandTest.php @@ -231,4 +231,35 @@ public function testCreateGuestWithCustomUid(): void { $output = $this->commandTester->getDisplay(); $this->assertStringContainsString('The guest account user "karl" was created successfully', $output); } + + public function testCreateGuestBlockedDomain(): void { + $this->userManager->expects($this->once()) + ->method('get') + ->with('creator') + ->willReturn($this->createMock(IUser::class)); + + $this->userManager->method('userExists') + ->willReturn(false); + + $this->mailer->method('validateMailAddress') + ->willReturn(true); + + $this->config->expects($this->once()) + ->method('isEmailDomainBlocked') + ->with('guest@blocked.example') + ->willReturn(true); + + $this->guestManager->expects($this->never()) + ->method('createGuest'); + + $this->commandTester->execute([ + 'created-by' => 'creator', + 'email' => 'guest@blocked.example', + '--generate-password' => true, + ]); + + $output = $this->commandTester->getDisplay(); + $this->assertStringContainsString('it is on the blocklist', $output); + $this->assertEquals(1, $this->commandTester->getStatusCode()); + } } diff --git a/tests/unit/ConfigTest.php b/tests/unit/ConfigTest.php index de027cf0b..332c96762 100644 --- a/tests/unit/ConfigTest.php +++ b/tests/unit/ConfigTest.php @@ -312,4 +312,80 @@ public function testSetGuestQuotaDefaultRemovesOverride(): void { $this->guestConfig->setGuestQuota('default'); } + + public static function normalizeDomainsProvider(): array { + return [ + 'plain' => [['example.com'], ['example.com']], + 'case and whitespace' => [[' Example.COM '], ['example.com']], + 'leading at sign' => [['@example.com'], ['example.com']], + 'wildcard' => [['*.example.com'], ['example.com']], + 'leading and trailing dot' => [['.example.com.'], ['example.com']], + 'duplicates' => [['example.com', 'EXAMPLE.com', '@example.com'], ['example.com']], + 'invalid entries dropped' => [['', 'not a domain', 'exa_mple.com', '-bad.com', 'good.org'], ['good.org']], + 'idn' => [['bücher.de'], ['bücher.de']], + 'idn upper case' => [['BÜCHER.DE'], ['bücher.de']], + 'idn duplicate in punycode' => [['bücher.de', 'xn--bcher-kva.de'], ['bücher.de']], + 'label too long' => [[str_repeat('a', 64) . '.com'], []], + 'domain too long' => [[implode('.', array_fill(0, 64, 'abcd'))], []], + ]; + } + + /** + * @dataProvider normalizeDomainsProvider + */ + public function testNormalizeDomains(array $input, array $expected): void { + $this->assertSame($expected, Config::normalizeDomains($input)); + } + + public function testSetBlockedDomainsStoresNormalizedList(): void { + $this->appConfig->expects($this->once()) + ->method('setAppValueArray') + ->with('blocked_domains', ['example.com', 'example.org']); + + $this->guestConfig->setBlockedDomains(['@Example.com', '*.example.org', 'example.com']); + } + + public static function blockedEmailProvider(): array { + return [ + 'exact domain' => ['guest@example.com', true], + 'uppercase' => ['Guest@EXAMPLE.COM', true], + 'subdomain' => ['guest@mail.example.com', true], + 'deep subdomain' => ['guest@a.b.example.com', true], + 'same suffix, other domain' => ['guest@otherexample.com', false], + 'parent domain only' => ['guest@com', false], + 'other domain' => ['guest@example.org', false], + 'second blocked entry' => ['guest@blocked.net', true], + 'no at sign' => ['example.com', false], + 'idn as punycode' => ['guest@xn--bcher-kva.de', true], + 'idn upper case' => ['guest@BÜCHER.de', true], + 'idn subdomain' => ['guest@mail.bücher.de', true], + ]; + } + + /** + * @dataProvider blockedEmailProvider + */ + public function testIsEmailDomainBlocked(string $email, bool $expected): void { + $this->appConfig->method('getAppValueArray') + ->with('blocked_domains') + ->willReturn(['example.com', 'blocked.net', 'bücher.de']); + + $this->assertSame($expected, $this->guestConfig->isEmailDomainBlocked($email)); + } + + public function testIsEmailDomainBlockedMatchesPunycodeEntry(): void { + $this->appConfig->method('getAppValueArray') + ->with('blocked_domains') + ->willReturn(['xn--bcher-kva.de']); + + $this->assertTrue($this->guestConfig->isEmailDomainBlocked('guest@bücher.de')); + } + + public function testIsEmailDomainBlockedWithEmptyBlocklist(): void { + $this->appConfig->method('getAppValueArray') + ->with('blocked_domains') + ->willReturn([]); + + $this->assertFalse($this->guestConfig->isEmailDomainBlocked('guest@example.com')); + } } diff --git a/tests/unit/Controller/SettingsControllerTest.php b/tests/unit/Controller/SettingsControllerTest.php index 34e59b824..ad9751310 100644 --- a/tests/unit/Controller/SettingsControllerTest.php +++ b/tests/unit/Controller/SettingsControllerTest.php @@ -81,4 +81,27 @@ public function testSetConfigIgnoresGarbageQuota(): void { $this->setConfigWithQuota('not a size'); } + + public function testSetConfigStoresBlockedDomains(): void { + $this->config->expects($this->once()) + ->method('setBlockedDomains') + ->with(['example.com']); + + $this->controller->setConfig(false, [], false, true, false, [], 'default', ['example.com']); + } + + public function testSetConfigKeepsBlockedDomainsWhenOmitted(): void { + $this->config->expects($this->never()) + ->method('setBlockedDomains'); + + $this->controller->setConfig(false, [], false, true, false, [], 'default'); + } + + public function testGetConfigReturnsBlockedDomains(): void { + $this->config->method('getBlockedDomains') + ->willReturn(['example.com']); + + $data = $this->controller->getConfig()->getData(); + $this->assertSame(['example.com'], $data['blockedDomains']); + } } diff --git a/tests/unit/Controller/UsersControllerTest.php b/tests/unit/Controller/UsersControllerTest.php index d382cd3c2..1fba4d2ac 100644 --- a/tests/unit/Controller/UsersControllerTest.php +++ b/tests/unit/Controller/UsersControllerTest.php @@ -159,8 +159,7 @@ public function testCreateFailsWhenUserNotInAllowedGroups(): void { // Set up group restriction check $this->appConfig->method('getAppValueArray') - ->with('create_restricted_to_group', []) - ->willReturn(['allowed_group1', 'allowed_group2']); + ->willReturnCallback(fn (string $key): array => $key === 'create_restricted_to_group' ? ['allowed_group1', 'allowed_group2'] : []); // User's groups don't intersect with allowed groups $this->groupManager->method('getUserGroupIds') @@ -190,8 +189,7 @@ public function testCreateSucceedsForAdminDespiteGroupRestrictions(): void { // Set up group restriction check $this->appConfig->method('getAppValueArray') - ->with('create_restricted_to_group', []) - ->willReturn(['allowed_group1', 'allowed_group2']); + ->willReturnCallback(fn (string $key): array => $key === 'create_restricted_to_group' ? ['allowed_group1', 'allowed_group2'] : []); // User is in the admin group $this->groupManager->method('getUserGroupIds') @@ -242,8 +240,7 @@ public function testCreateSucceedsWhenUserInAllowedGroup(): void { // Set up group restriction check $this->appConfig->method('getAppValueArray') - ->with('create_restricted_to_group', []) - ->willReturn(['allowed_group1', 'allowed_group2']); + ->willReturnCallback(fn (string $key): array => $key === 'create_restricted_to_group' ? ['allowed_group1', 'allowed_group2'] : []); // User is in one of the allowed groups $this->groupManager->method('getUserGroupIds') @@ -296,8 +293,7 @@ public function testCreateSucceedsForSubAdminWhenSharingIsRestricted(): void { // No group restrictions $this->appConfig->method('getAppValueArray') - ->with('create_restricted_to_group', []) - ->willReturn([]); + ->willReturnCallback(fn (string $key): array => $key === 'create_restricted_to_group' ? [] : []); // Sharing is restricted to group $this->globalAppConfig->method('getValueBool') @@ -353,8 +349,7 @@ public function testCreateFailsForNonSubAdminWhenSharingIsRestricted(): void { // No group restrictions $this->appConfig->method('getAppValueArray') - ->with('create_restricted_to_group', []) - ->willReturn([]); + ->willReturnCallback(fn (string $key): array => $key === 'create_restricted_to_group' ? [] : []); // Sharing is restricted to group $this->globalAppConfig->method('getValueBool') @@ -388,8 +383,7 @@ public function testCreateSucceedsWhenNoRestrictions(): void { // No group restrictions $this->appConfig->method('getAppValueArray') - ->with('create_restricted_to_group', []) - ->willReturn([]); + ->willReturnCallback(fn (string $key): array => $key === 'create_restricted_to_group' ? [] : []); // Sharing is restricted to group $this->config->method('getAppValue') @@ -433,8 +427,7 @@ public function testCreateWhenSharingRestrictedToGroupButNoGroups(): void { // Groups restriction is empty $this->appConfig->method('getAppValueArray') - ->with('create_restricted_to_group', []) - ->willReturn([]); + ->willReturnCallback(fn (string $key): array => $key === 'create_restricted_to_group' ? [] : []); // Sharing is restricted to group $this->globalAppConfig->method('getValueBool') @@ -530,8 +523,7 @@ public function testCreateWithGroupButNotSubAdmin(): void { // There is no group restriction in place $this->appConfig->method('getAppValueArray') - ->with('create_restricted_to_group', []) - ->willReturn([]); + ->willReturnCallback(fn (string $key): array => $key === 'create_restricted_to_group' ? [] : []); $this->assertTrue($this->guestsConfig->canCreateGuests()); $this->assertTrue($this->guestsConfig->isSharingRestrictedToGroup()); @@ -698,8 +690,7 @@ public function testCreateSuccessWithGroupsAsSubadmin(): void { // Enable restrictions $this->appConfig->method('getAppValueArray') - ->with('create_restricted_to_group', []) - ->willReturn(['group1', 'group2']); + ->willReturnCallback(fn (string $key): array => $key === 'create_restricted_to_group' ? ['group1', 'group2'] : []); // User's groups don't intersect with allowed groups $this->groupManager->method('getUserGroupIds') @@ -853,4 +844,34 @@ public function testConvertRejectsAccountThatLoggedIn(): void { $response = $this->controller->convert('karl'); $this->assertEquals(Http::STATUS_CONFLICT, $response->getStatus()); } + + /** + * Test that creation fails when the email domain is on the blocklist. + */ + public function testCreateFailsForBlockedDomain(): void { + $currentUser = $this->createMock(IUser::class); + $currentUser->method('getUID')->willReturn('admin'); + $this->userSession->method('getUser') + ->willReturn($currentUser); + $this->guestManager->method('isGuest') + ->willReturn(false); + $this->groupManager->method('isAdmin') + ->willReturn(true); + $this->mailer->method('validateMailAddress') + ->willReturn(true); + $this->appConfig->method('getAppValueArray') + ->willReturnCallback(fn (string $key) => $key === 'blocked_domains' ? ['example.com'] : []); + $this->userManager->method('getByEmail') + ->willReturn([]); + + $this->guestManager->expects($this->never()) + ->method('createGuest'); + + $response = $this->controller->create('guest@sub.example.com', 'Guest', 'en', []); + $this->assertEquals(Http::STATUS_UNPROCESSABLE_ENTITY, $response->getStatus()); + $this->assertEquals( + ['errorMessages' => ['email' => 'Guest accounts cannot be created for this email domain.']], + $response->getData() + ); + } }