diff --git a/lib/AppWhitelist.php b/lib/AppWhitelist.php index 470e2df21..013badd3a 100644 --- a/lib/AppWhitelist.php +++ b/lib/AppWhitelist.php @@ -28,7 +28,7 @@ class AppWhitelist { private readonly int $baseUrlLength; - public const WHITELIST_ALWAYS = ',core,theming,settings,avatar,files,heartbeat,dav,guests,impersonate,accessibility,terms_of_service,dashboard,weather_status,user_status,apporder,twofactor_totp,twofactor_webauthn,twofactor_backupcodes,twofactor_nextcloud_notification'; + public const WHITELIST_ALWAYS = 'core,theming,settings,avatar,files,heartbeat,dav,guests,impersonate,accessibility,terms_of_service,dashboard,weather_status,user_status,apporder,twofactor_totp,twofactor_webauthn,twofactor_backupcodes,twofactor_nextcloud_notification'; public const DEFAULT_WHITELIST = 'files_trashbin,files_versions,files_sharing,files_texteditor,text,activity,firstrunwizard,photos,notifications,dashboard,user_status,weather_status'; diff --git a/lib/Config.php b/lib/Config.php index 2da34ff24..0022f5daa 100644 --- a/lib/Config.php +++ b/lib/Config.php @@ -104,7 +104,7 @@ public function setUseWhitelist(bool $use): void { * @return list */ public function getAppWhitelist(): array { - return explode(',', $this->appConfig->getAppValueString(ConfigLexicon::WHITE_LIST)); + return array_values(array_filter(explode(',', $this->appConfig->getAppValueString(ConfigLexicon::WHITE_LIST)), static fn (string $app): bool => $app !== '')); } public function setAppWhitelist(array $whitelist): void { diff --git a/tests/unit/AppWhitelistTest.php b/tests/unit/AppWhitelistTest.php index 025fa4cf6..d84d03bd4 100644 --- a/tests/unit/AppWhitelistTest.php +++ b/tests/unit/AppWhitelistTest.php @@ -82,4 +82,15 @@ public function testIsUrlAllowedNoWhitelist(): void { $this->assertTrue($this->appWhitelist->isUrlAllowed($user, '/apps/news/...')); $this->assertTrue($this->appWhitelist->isUrlAllowed($user, '/apps/foo/...')); } + + /** + * getRequestedApp() returns an empty string for urls that carry no + * resolvable app id, so it must never be treated as whitelisted. + */ + public function testEmptyAppIdIsNotWhitelisted(): void { + $this->config->method('getAppWhitelist') + ->willReturn(['foo', 'bar']); + + $this->assertFalse($this->appWhitelist->isAppWhitelisted('')); + } } diff --git a/tests/unit/ConfigTest.php b/tests/unit/ConfigTest.php index b76201e36..de027cf0b 100644 --- a/tests/unit/ConfigTest.php +++ b/tests/unit/ConfigTest.php @@ -123,6 +123,17 @@ public function testGetAppWhitelist(): void { $this->assertEquals(['app1', 'app2', 'app3'], $this->guestConfig->getAppWhitelist()); } + public function testGetAppWhitelistEmpty(): void { + $this->appConfig->method('getAppValueString') + ->with('whitelist') + ->willReturn(''); + + // explode() on an empty string yields a single empty entry, which + // would both whitelist the empty app id and show up as a blank entry + // in the admin settings. + $this->assertEquals([], $this->guestConfig->getAppWhitelist()); + } + public function testSetAppWhitelistArray(): void { $this->appConfig->expects($this->once()) ->method('setAppValueString')