From 1adc13724a6ea1e91b47a81adc2e2a3e028b6f90 Mon Sep 17 00:00:00 2001 From: Anna Larch Date: Thu, 19 Feb 2026 17:11:11 +0100 Subject: [PATCH] refactor: remove deprecated calls to IConfig::getAppValue Signed-off-by: Anna Larch --- lib/Config.php | 107 ++++---- tests/php/ConfigTest.php | 254 +++++++++--------- .../Controller/SignalingControllerTest.php | 13 +- tests/php/Recording/BackendNotifierTest.php | 20 +- tests/psalm-baseline.xml | 7 +- 5 files changed, 201 insertions(+), 200 deletions(-) diff --git a/lib/Config.php b/lib/Config.php index 73798a75d50..6d04906d07a 100644 --- a/lib/Config.php +++ b/lib/Config.php @@ -63,9 +63,11 @@ public function __construct( * @return string[] */ public function getAllowedTalkGroupIds(): array { - $groups = $this->config->getAppValue('spreed', 'allowed_groups', '[]'); - $groups = json_decode($groups, true); - return \is_array($groups) ? $groups : []; + $groups = $this->appConfig->getAppValueArray('allowed_groups'); + if (empty($groups)) { + return []; + } + return $groups; } /** @@ -98,9 +100,11 @@ public function getUserTypingPrivacy(string $userId): int { * @return string[] */ public function getSIPGroups(): array { - $groups = $this->config->getAppValue('spreed', 'sip_bridge_groups', '[]'); - $groups = json_decode($groups, true); - return \is_array($groups) ? $groups : []; + $groups = $this->appConfig->getAppValueArray('sip_bridge_groups'); + if (empty($groups)) { + return []; + } + return $groups; } public function isSIPConfigured(): bool { @@ -113,7 +117,7 @@ public function isSIPConfigured(): bool { */ public function isFederationEnabled(): bool { // TODO: Set to default true once implementation is complete - return $this->config->getAppValue('spreed', 'federation_enabled', 'no') === 'yes'; + return $this->appConfig->getAppValueBool('federation_enabled'); } public function isFederationEnabledForUserId(IUser $user): bool { @@ -127,15 +131,15 @@ public function isFederationEnabledForUserId(IUser $user): bool { } public function isBreakoutRoomsEnabled(): bool { - return $this->config->getAppValue('spreed', 'breakout_rooms', 'yes') === 'yes'; + return $this->appConfig->getAppValueBool('breakout_rooms', true); } public function getDialInInfo(): string { - return $this->config->getAppValue('spreed', 'sip_bridge_dialin_info'); + return $this->appConfig->getAppValueString('sip_bridge_dialin_info'); } public function getSIPSharedSecret(): string { - return $this->config->getAppValue('spreed', 'sip_bridge_shared_secret'); + return $this->appConfig->getAppValueString('sip_bridge_shared_secret'); } public function canUserEnableSIP(IUser $user): bool { @@ -165,14 +169,12 @@ public function canUserDialOutSIP(IUser $user): bool { } public function isSIPDialOutEnabled(): bool { - return $this->config->getAppValue('spreed', 'sip_dialout', 'no') !== 'no'; + return $this->appConfig->getAppValueBool('sip_dialout'); } public function getRecordingServers(): array { - $config = $this->config->getAppValue('spreed', 'recording_servers'); - $recording = json_decode($config, true); - - if (!is_array($recording) || !isset($recording['servers'])) { + $recording = $this->appConfig->getAppValueArray('recording_servers'); + if (empty($recording) || !isset($recording['servers'])) { return []; } @@ -180,14 +182,12 @@ public function getRecordingServers(): array { } public function getRecordingSecret(): string { - $config = $this->config->getAppValue('spreed', 'recording_servers'); - $recording = json_decode($config, true); - - if (!is_array($recording)) { + $config = $this->appConfig->getAppValueArray('recording_servers'); + if (!isset($config['secret'])) { return ''; } - return $recording['secret']; + return $config['secret']; } public function isRecordingEnabled(): bool { @@ -195,7 +195,7 @@ public function isRecordingEnabled(): bool { return false; } - if ($this->config->getAppValue('spreed', 'call_recording', 'yes') !== 'yes') { + if (!$this->appConfig->getAppValueBool('call_recording', true)) { return false; } @@ -226,7 +226,7 @@ public function recordingConsentRequired(): int { * @return RecordingService::CONSENT_REQUIRED_* */ public function getRecordingConsentConfig(): int { - return match ((int)$this->config->getAppValue('spreed', 'recording_consent', (string)RecordingService::CONSENT_REQUIRED_NO)) { + return match ($this->appConfig->getAppValueInt('recording_consent', RecordingService::CONSENT_REQUIRED_NO)) { RecordingService::CONSENT_REQUIRED_YES => RecordingService::CONSENT_REQUIRED_YES, RecordingService::CONSENT_REQUIRED_OPTIONAL => RecordingService::CONSENT_REQUIRED_OPTIONAL, default => RecordingService::CONSENT_REQUIRED_NO, @@ -265,9 +265,11 @@ public function isDisabledForUser(IUser $user): bool { * @return string[] */ public function getAllowedConversationsGroupIds(): array { - $groups = $this->config->getAppValue('spreed', 'start_conversations', '[]'); - $groups = json_decode($groups, true); - return \is_array($groups) ? $groups : []; + $groups = $this->appConfig->getAppValueArray('start_conversations'); + if (empty($groups)) { + return []; + } + return $groups; } public function isNotAllowedToCreateConversations(IUser $user): bool { @@ -286,7 +288,7 @@ public function isNotAllowedToCreateConversations(IUser $user): bool { */ public function getDefaultPermissions(): int { // Admin configured default permissions - $configurableDefault = $this->config->getAppValue('spreed', 'default_permissions'); + $configurableDefault = $this->appConfig->getAppValueString('default_permissions'); if ($configurableDefault !== '') { return min(Attendee::PERMISSIONS_MAX_CUSTOM, max(Attendee::PERMISSIONS_DEFAULT, (int)$configurableDefault)); } @@ -296,7 +298,7 @@ public function getDefaultPermissions(): int { } public function getAttachmentFolder(string $userId): string { - $defaultAttachmentFolder = $this->config->getAppValue('spreed', 'default_attachment_folder', '/Talk'); + $defaultAttachmentFolder = $this->appConfig->getAppValueString('default_attachment_folder', '/Talk'); return $this->config->getUserValue($userId, 'spreed', UserPreference::ATTACHMENT_FOLDER, $defaultAttachmentFolder); } @@ -355,7 +357,7 @@ protected function getWebSocketDomainForSignalingServer(string $url): string { * @return string[] */ public function getStunServers(): array { - $config = $this->config->getAppValue('spreed', 'stun_servers', json_encode(['stun.nextcloud.com:443'])); + $config = $this->appConfig->getAppValueString('stun_servers', json_encode(['stun.nextcloud.com:443'])); $servers = json_decode($config, true); if (!is_array($servers) || empty($servers)) { @@ -377,11 +379,9 @@ public function getStunServers(): array { * @return array */ public function getTurnServers(bool $withEvent = true): array { - $config = $this->config->getAppValue('spreed', 'turn_servers'); - $servers = json_decode($config, true); - - if ($servers === null || empty($servers) || !is_array($servers)) { - $servers = []; + $servers = $this->appConfig->getAppValueArray('turn_servers'); + if (empty($servers)) { + return []; } if ($withEvent) { @@ -428,7 +428,7 @@ public function getTurnSettings(): array { ]; } - return $turnSettings; + return $turnSettings ?? []; } public function getSignalingMode(bool $cleanExternalSignaling = true): string { @@ -438,7 +438,7 @@ public function getSignalingMode(bool $cleanExternalSignaling = true): string { self::SIGNALING_CLUSTER_CONVERSATION, ]; - $mode = $this->config->getAppValue('spreed', 'signaling_mode', null); + $mode = $this->appConfig->getAppValueString('signaling_mode'); if ($mode === self::SIGNALING_INTERNAL) { return self::SIGNALING_INTERNAL; } @@ -462,9 +462,8 @@ public function getSignalingMode(bool $cleanExternalSignaling = true): string { * @return array */ public function getSignalingServers(): array { - $config = $this->config->getAppValue('spreed', 'signaling_servers'); - $signaling = json_decode($config, true); - if (!is_array($signaling) || !isset($signaling['servers'])) { + $signaling = $this->appConfig->getAppValueArray('signaling_servers'); + if (empty($signaling) || !isset($signaling['servers'])) { return []; } @@ -475,7 +474,10 @@ public function getSignalingServers(): array { * @return string */ public function getSignalingSecret(): string { - $config = $this->config->getAppValue('spreed', 'signaling_servers'); + $config = $this->appConfig->getAppValueString('signaling_servers'); + if ($config === '') { + return ''; + } $signaling = json_decode($config, true); if (!is_array($signaling)) { @@ -486,7 +488,7 @@ public function getSignalingSecret(): string { } public function getHideSignalingWarning(): bool { - return $this->config->getAppValue('spreed', 'hide_signaling_warning', 'no') === 'yes'; + return $this->appConfig->getAppValueBool('hide_signaling_warning'); } /** @@ -510,7 +512,7 @@ public function getSignalingTicket(int $version, ?string $userId, ?string $cloud */ private function getSignalingTicketV1(?string $userId): string { if (empty($userId)) { - $secret = $this->config->getAppValue('spreed', 'signaling_ticket_secret'); + $secret = $this->appConfig->getAppValueString('signaling_ticket_secret'); } else { $secret = $this->config->getUserValue($userId, 'spreed', 'signaling_ticket_secret'); } @@ -519,7 +521,7 @@ private function getSignalingTicketV1(?string $userId): string { // TODO(fancycode): Is there a possibility for a race condition? $secret = $this->secureRandom->generate(255); if (empty($userId)) { - $this->config->setAppValue('spreed', 'signaling_ticket_secret', $secret); + $this->appConfig->setAppValueString('signaling_ticket_secret', $secret); } else { $this->config->setUserValue($userId, 'spreed', 'signaling_ticket_secret', $secret); } @@ -535,7 +537,7 @@ private function getSignalingTicketV1(?string $userId): string { } private function ensureSignalingTokenKeys(string $alg): void { - $secret = $this->config->getAppValue('spreed', 'signaling_token_privkey_' . strtolower($alg)); + $secret = $this->appConfig->getAppValueString('signaling_token_privkey_' . strtolower($alg)); if ($secret) { return; } @@ -569,12 +571,12 @@ private function ensureSignalingTokenKeys(string $alg): void { throw new \Exception('Unsupported algorithm ' . $alg); } - $this->config->setAppValue('spreed', 'signaling_token_privkey_' . strtolower($alg), $secret); - $this->config->setAppValue('spreed', 'signaling_token_pubkey_' . strtolower($alg), $public); + $this->appConfig->setAppValueString('signaling_token_privkey_' . strtolower($alg), $secret); + $this->appConfig->setAppValueString('signaling_token_pubkey_' . strtolower($alg), $public); } public function getSignalingTokenAlgorithm(): string { - return $this->config->getAppValue('spreed', 'signaling_token_alg', 'ES256'); + return $this->appConfig->getAppValueString('signaling_token_alg', 'ES256'); } public function getSignalingTokenPrivateKey(?string $alg = null): string { @@ -583,7 +585,7 @@ public function getSignalingTokenPrivateKey(?string $alg = null): string { } $this->ensureSignalingTokenKeys($alg); - return $this->config->getAppValue('spreed', 'signaling_token_privkey_' . strtolower($alg)); + return $this->appConfig->getAppValueString('signaling_token_privkey_' . strtolower($alg)); } public function getSignalingTokenPublicKey(?string $alg = null): string { @@ -592,7 +594,7 @@ public function getSignalingTokenPublicKey(?string $alg = null): string { } $this->ensureSignalingTokenKeys($alg); - return $this->config->getAppValue('spreed', 'signaling_token_pubkey_' . strtolower($alg)); + return $this->appConfig->getAppValueString('signaling_token_pubkey_' . strtolower($alg)); } public function deriveSignalingTokenPublicKey(string $privateKey, string $alg): string { @@ -678,8 +680,7 @@ private function getSignalingTicketV2(?string $userId, ?string $cloudId): string $alg = $this->getSignalingTokenAlgorithm(); $secret = $this->getSignalingTokenPrivateKey($alg); - $token = JWT::encode($data, $secret, $alg); - return $token; + return JWT::encode($data, $secret, $alg); } /** @@ -689,7 +690,7 @@ private function getSignalingTicketV2(?string $userId, ?string $cloudId): string */ public function validateSignalingTicket(?string $userId, string $ticket): bool { if (empty($userId)) { - $secret = $this->config->getAppValue('spreed', 'signaling_ticket_secret'); + $secret = $this->appConfig->getAppValueString('signaling_ticket_secret'); } else { $secret = $this->config->getUserValue($userId, 'spreed', 'signaling_ticket_secret'); } @@ -710,11 +711,11 @@ public function validateSignalingTicket(?string $userId, string $ticket): bool { } public function getGridVideosLimit(): int { - return (int)$this->config->getAppValue('spreed', 'grid_videos_limit', '19'); // 5*4 - self + return $this->appConfig->getAppValueInt('grid_videos_limit', 19); // 5*4 - self } public function getGridVideosLimitEnforced(): bool { - return $this->config->getAppValue('spreed', 'grid_videos_limit_enforced', 'no') === 'yes'; + return $this->appConfig->getAppValueBool('grid_videos_limit_enforced'); } /** diff --git a/tests/php/ConfigTest.php b/tests/php/ConfigTest.php index ae62ce77bb2..0d1add80650 100644 --- a/tests/php/ConfigTest.php +++ b/tests/php/ConfigTest.php @@ -26,24 +26,25 @@ use Test\TestCase; class ConfigTest extends TestCase { - private function createConfig(IConfig $config) { - /** @var MockObject|IAppConfig $appConfig */ - $appConfig = $this->createMock(IAppConfig::class); - /** @var MockObject|ITimeFactory $timeFactory */ + private function createConfig(IConfig $config, ?IAppConfig $appConfig = null): Config { + if ($appConfig === null) { + /** @var MockObject&IAppConfig $appConfig */ + $appConfig = $this->createMock(IAppConfig::class); + } + /** @var MockObject&ITimeFactory $timeFactory */ $timeFactory = $this->createMock(ITimeFactory::class); - /** @var MockObject|ISecureRandom $secureRandom */ + /** @var MockObject&ISecureRandom $secureRandom */ $secureRandom = $this->createMock(ISecureRandom::class); - /** @var MockObject|IGroupManager $groupManager */ + /** @var MockObject&IGroupManager $groupManager */ $groupManager = $this->createMock(IGroupManager::class); - /** @var MockObject|IUserManager $userManager */ + /** @var MockObject&IUserManager $userManager */ $userManager = $this->createMock(IUserManager::class); - /** @var MockObject|IURLGenerator $urlGenerator */ + /** @var MockObject&IURLGenerator $urlGenerator */ $urlGenerator = $this->createMock(IURLGenerator::class); - /** @var MockObject|IEventDispatcher $dispatcher */ + /** @var MockObject&IEventDispatcher $dispatcher */ $dispatcher = $this->createMock(IEventDispatcher::class); - $helper = new Config($config, $appConfig, $secureRandom, $groupManager, $userManager, $urlGenerator, $timeFactory, $dispatcher); - return $helper; + return new Config($config, $appConfig, $secureRandom, $groupManager, $userManager, $urlGenerator, $timeFactory, $dispatcher); } public function testGetStunServers(): void { @@ -52,12 +53,14 @@ public function testGetStunServers(): void { 'stun2.example.com:129', ]; - /** @var MockObject|IConfig $config */ + /** @var MockObject&IConfig $config */ $config = $this->createMock(IConfig::class); - $config + /** @var MockObject&IAppConfig $appConfig */ + $appConfig = $this->createMock(IAppConfig::class); + $appConfig ->expects($this->once()) - ->method('getAppValue') - ->with('spreed', 'stun_servers', json_encode(['stun.nextcloud.com:443'])) + ->method('getAppValueString') + ->with('stun_servers', json_encode(['stun.nextcloud.com:443'])) ->willReturn(json_encode($servers)); $config ->expects($this->once()) @@ -65,17 +68,19 @@ public function testGetStunServers(): void { ->with('has_internet_connection', true) ->willReturn(true); - $helper = $this->createConfig($config); - $this->assertSame($helper->getStunServers(), $servers); + $helper = $this->createConfig($config, $appConfig); + $this->assertSame($servers, $helper->getStunServers()); } public function testGetDefaultStunServer(): void { - /** @var MockObject|IConfig $config */ + /** @var MockObject&IConfig $config */ $config = $this->createMock(IConfig::class); - $config + /** @var MockObject&IAppConfig $appConfig */ + $appConfig = $this->createMock(IAppConfig::class); + $appConfig ->expects($this->once()) - ->method('getAppValue') - ->with('spreed', 'stun_servers', json_encode(['stun.nextcloud.com:443'])) + ->method('getAppValueString') + ->with('stun_servers', json_encode(['stun.nextcloud.com:443'])) ->willReturn(json_encode([])); $config ->expects($this->once()) @@ -83,17 +88,19 @@ public function testGetDefaultStunServer(): void { ->with('has_internet_connection', true) ->willReturn(true); - $helper = $this->createConfig($config); + $helper = $this->createConfig($config, $appConfig); $this->assertSame(['stun.nextcloud.com:443'], $helper->getStunServers()); } public function testGetDefaultStunServerNoInternet(): void { - /** @var MockObject|IConfig $config */ + /** @var MockObject&IConfig $config */ $config = $this->createMock(IConfig::class); - $config + /** @var MockObject&IAppConfig $appConfig */ + $appConfig = $this->createMock(IAppConfig::class); + $appConfig ->expects($this->once()) - ->method('getAppValue') - ->with('spreed', 'stun_servers', json_encode(['stun.nextcloud.com:443'])) + ->method('getAppValueString') + ->with('stun_servers', json_encode(['stun.nextcloud.com:443'])) ->willReturn(json_encode([])); $config ->expects($this->once()) @@ -101,18 +108,20 @@ public function testGetDefaultStunServerNoInternet(): void { ->with('has_internet_connection', true) ->willReturn(false); - $helper = $this->createConfig($config); + $helper = $this->createConfig($config, $appConfig); $this->assertSame([], $helper->getStunServers()); } public function testGenerateTurnSettings(): void { - /** @var MockObject|IConfig $config */ + /** @var MockObject&IConfig $config */ $config = $this->createMock(IConfig::class); - $config + /** @var MockObject&IAppConfig $appConfig */ + $appConfig = $this->createMock(IAppConfig::class); + $appConfig ->expects($this->once()) - ->method('getAppValue') - ->with('spreed', 'turn_servers', '') - ->willReturn(json_encode([ + ->method('getAppValueArray') + ->with('turn_servers') + ->willReturn([ [ // No scheme explicitly given 'server' => 'turn.example.org:3478', @@ -131,27 +140,25 @@ public function testGenerateTurnSettings(): void { 'secret' => 'ThisIsAlsoSuperSecret', 'protocols' => 'tcp', ], - ])); + ]); - /** @var MockObject|ITimeFactory $timeFactory */ + /** @var MockObject&ITimeFactory $timeFactory */ $timeFactory = $this->createMock(ITimeFactory::class); $timeFactory ->expects($this->once()) ->method('getTime') ->willReturn(1479743025); - /** @var MockObject|IAppConfig $appConfig */ - $appConfig = $this->createMock(IAppConfig::class); - /** @var MockObject|IGroupManager $groupManager */ + /** @var MockObject&IGroupManager $groupManager */ $groupManager = $this->createMock(IGroupManager::class); - /** @var MockObject|IUserManager $userManager */ + /** @var MockObject&IUserManager $userManager */ $userManager = $this->createMock(IUserManager::class); - /** @var MockObject|IURLGenerator $urlGenerator */ + /** @var MockObject&IURLGenerator $urlGenerator */ $urlGenerator = $this->createMock(IURLGenerator::class); - /** @var MockObject|IEventDispatcher $dispatcher */ + /** @var MockObject&IEventDispatcher $dispatcher */ $dispatcher = $this->createMock(IEventDispatcher::class); - /** @var MockObject|ISecureRandom $secureRandom */ + /** @var MockObject&ISecureRandom $secureRandom */ $secureRandom = $this->createMock(ISecureRandom::class); $secureRandom ->expects($this->once()) @@ -160,9 +167,8 @@ public function testGenerateTurnSettings(): void { ->willReturn('abcdefghijklmnop'); $helper = new Config($config, $appConfig, $secureRandom, $groupManager, $userManager, $urlGenerator, $timeFactory, $dispatcher); - // $settings = $helper->getTurnSettings(); - $this->assertEquals(3, count($settings)); + $this->assertCount(3, $settings); $this->assertSame([ 'schemes' => 'turn', 'server' => 'turn.example.org:3478', @@ -187,45 +193,41 @@ public function testGenerateTurnSettings(): void { } public function testGenerateTurnSettingsEmpty(): void { - /** @var MockObject|IConfig $config */ + /** @var MockObject&IConfig $config */ $config = $this->createMock(IConfig::class); - $config + /** @var MockObject&IAppConfig $appConfig */ + $appConfig = $this->createMock(IAppConfig::class); + $appConfig ->expects($this->once()) - ->method('getAppValue') - ->with('spreed', 'turn_servers', '') - ->willReturn(json_encode([])); + ->method('getAppValueArray') + ->with('turn_servers') + ->willReturn([]); - $helper = $this->createConfig($config); + $helper = $this->createConfig($config, $appConfig); $settings = $helper->getTurnSettings(); - $this->assertEquals(0, count($settings)); + $this->assertCount(0, $settings); } public function testGenerateTurnSettingsEvent(): void { - /** @var MockObject|IConfig $config */ + /** @var MockObject&IConfig $config */ $config = $this->createMock(IConfig::class); - $config - ->expects($this->once()) - ->method('getAppValue') - ->with('spreed', 'turn_servers', '') - ->willReturn(json_encode([])); - - /** @var MockObject|IAppConfig $appConfig */ + /** @var MockObject&IAppConfig $appConfig */ $appConfig = $this->createMock(IAppConfig::class); - /** @var MockObject|ITimeFactory $timeFactory */ + /** @var MockObject&ITimeFactory $timeFactory */ $timeFactory = $this->createMock(ITimeFactory::class); - /** @var MockObject|IGroupManager $groupManager */ + /** @var MockObject&IGroupManager $groupManager */ $groupManager = $this->createMock(IGroupManager::class); - /** @var MockObject|IUserManager $userManager */ + /** @var MockObject&IUserManager $userManager */ $userManager = $this->createMock(IUserManager::class); - /** @var MockObject|IURLGenerator $urlGenerator */ + /** @var MockObject&IURLGenerator $urlGenerator */ $urlGenerator = $this->createMock(IURLGenerator::class); - /** @var MockObject|ISecureRandom $secureRandom */ + /** @var MockObject&ISecureRandom $secureRandom */ $secureRandom = $this->createMock(ISecureRandom::class); /** @var IEventDispatcher $dispatcher */ @@ -247,7 +249,11 @@ public function testGenerateTurnSettingsEvent(): void { 'protocols' => 'tcp', ], ]; - + $appConfig + ->expects($this->once()) + ->method('getAppValueArray') + ->with('turn_servers') + ->willReturn($servers); $dispatcher->addServiceListener(BeforeTurnServersGetEvent::class, GetTurnServerListener::class); $helper = new Config($config, $appConfig, $secureRandom, $groupManager, $userManager, $urlGenerator, $timeFactory, $dispatcher); @@ -336,9 +342,8 @@ public static function dataGetWebSocketDomainForSignalingServer(): array { */ #[DataProvider('dataGetWebSocketDomainForSignalingServer')] public function testGetWebSocketDomainForSignalingServer($url, $expectedWebSocketDomain): void { - /** @var MockObject|IConfig $config */ + /** @var MockObject&IConfig $config */ $config = $this->createMock(IConfig::class); - $helper = $this->createConfig($config); $this->assertEquals( @@ -360,58 +365,51 @@ public static function dataTicketV2Algorithm(): array { #[DataProvider('dataTicketV2Algorithm')] public function testSignalingTicketV2User(string $algo): void { - $config = \OCP\Server::get(IConfig::class); - /** @var MockObject|IAppConfig $appConfig */ + /** @var MockObject&IConfig $config */ + $config = $this->createMock(IConfig::class); + /** @var MockObject&IAppConfig $appConfig */ $appConfig = $this->createMock(IAppConfig::class); - /** @var MockObject|ITimeFactory $timeFactory */ + /** @var MockObject&ITimeFactory $timeFactory */ $timeFactory = $this->createMock(ITimeFactory::class); - /** @var MockObject|ISecureRandom $secureRandom */ + /** @var MockObject&ISecureRandom $secureRandom */ $secureRandom = $this->createMock(ISecureRandom::class); - /** @var MockObject|IGroupManager $groupManager */ + /** @var MockObject&IGroupManager $groupManager */ $groupManager = $this->createMock(IGroupManager::class); - /** @var MockObject|IUserManager $userManager */ + /** @var MockObject&IUserManager $userManager */ $userManager = $this->createMock(IUserManager::class); - /** @var MockObject|IURLGenerator $urlGenerator */ + /** @var MockObject&IURLGenerator $urlGenerator */ $urlGenerator = $this->createMock(IURLGenerator::class); - /** @var MockObject|IEventDispatcher $dispatcher */ + /** @var MockObject&IEventDispatcher $dispatcher */ $dispatcher = $this->createMock(IEventDispatcher::class); - /** @var MockObject|IUser $user */ + /** @var MockObject&IUser $user */ $user = $this->createMock(IUser::class); + // Simulate IAppConfig in-memory storage, pre-seeded with the algorithm. + // No private key present initially so ensureSignalingTokenKeys() generates a fresh pair. + $storedValues = ['signaling_token_alg' => $algo]; + $appConfig->method('getAppValueString') + ->willReturnCallback(function (string $key, string $default = '') use (&$storedValues): string { + return $storedValues[$key] ?? $default; + }); + $appConfig->method('setAppValueString') + ->willReturnCallback(function (string $key, string $value) use (&$storedValues): bool { + $storedValues[$key] = $value; + return true; + }); + $now = time(); - $timeFactory - ->expects($this->once()) - ->method('getTime') - ->willReturn($now); - $urlGenerator - ->expects($this->once()) - ->method('getAbsoluteURL') - ->with('') - ->willReturn('https://domain.invalid/nextcloud'); - $userManager - ->expects($this->once()) - ->method('get') - ->with('user1') - ->willReturn($user); - $user - ->expects($this->once()) - ->method('getUID') - ->willReturn('user1'); - $user - ->expects($this->once()) - ->method('getDisplayName') - ->willReturn('Jane Doe'); + $timeFactory->expects($this->once())->method('getTime')->willReturn($now); + $urlGenerator->expects($this->once())->method('getAbsoluteURL')->with('')->willReturn('https://domain.invalid/nextcloud'); + $userManager->expects($this->once())->method('get')->with('user1')->willReturn($user); + $user->expects($this->once())->method('getUID')->willReturn('user1'); + $user->expects($this->once())->method('getDisplayName')->willReturn('Jane Doe'); $helper = new Config($config, $appConfig, $secureRandom, $groupManager, $userManager, $urlGenerator, $timeFactory, $dispatcher); - - $config->setAppValue('spreed', 'signaling_token_alg', $algo); - // Make sure new keys are generated. - $config->deleteAppValue('spreed', 'signaling_token_privkey_' . strtolower($algo)); - $config->deleteAppValue('spreed', 'signaling_token_pubkey_' . strtolower($algo)); $ticket = $helper->getSignalingTicket(Config::SIGNALING_TICKET_V2, 'user1'); $this->assertNotNull($ticket); - $key = new Key($config->getAppValue('spreed', 'signaling_token_pubkey_' . strtolower($algo)), $algo); + $pubKey = $storedValues['signaling_token_pubkey_' . strtolower($algo)]; + $key = new Key($pubKey, $algo); $decoded = JWT::decode($ticket, $key); $this->assertEquals($now, $decoded->iat); @@ -422,47 +420,49 @@ public function testSignalingTicketV2User(string $algo): void { #[DataProvider('dataTicketV2Algorithm')] public function testSignalingTicketV2Anonymous(string $algo): void { - /** @var IConfig $config */ - $config = \OCP\Server::get(IConfig::class); - /** @var MockObject|IAppConfig $appConfig */ + /** @var MockObject&IConfig $config */ + $config = $this->createMock(IConfig::class); + /** @var MockObject&IAppConfig $appConfig */ $appConfig = $this->createMock(IAppConfig::class); - /** @var MockObject|ITimeFactory $timeFactory */ + /** @var MockObject&ITimeFactory $timeFactory */ $timeFactory = $this->createMock(ITimeFactory::class); - /** @var MockObject|ISecureRandom $secureRandom */ + /** @var MockObject&ISecureRandom $secureRandom */ $secureRandom = $this->createMock(ISecureRandom::class); - /** @var MockObject|IGroupManager $groupManager */ + /** @var MockObject&IGroupManager $groupManager */ $groupManager = $this->createMock(IGroupManager::class); - /** @var MockObject|IUserManager $userManager */ + /** @var MockObject&IUserManager $userManager */ $userManager = $this->createMock(IUserManager::class); - /** @var MockObject|IURLGenerator $urlGenerator */ + /** @var MockObject&IURLGenerator $urlGenerator */ $urlGenerator = $this->createMock(IURLGenerator::class); - /** @var MockObject|IEventDispatcher $dispatcher */ + /** @var MockObject&IEventDispatcher $dispatcher */ $dispatcher = $this->createMock(IEventDispatcher::class); + // Simulate IAppConfig in-memory storage, pre-seeded with the algorithm. + $storedValues = ['signaling_token_alg' => $algo]; + $appConfig->method('getAppValueString') + ->willReturnCallback(function (string $key, string $default = '') use (&$storedValues): string { + return $storedValues[$key] ?? $default; + }); + $appConfig->method('setAppValueString') + ->willReturnCallback(function (string $key, string $value) use (&$storedValues): bool { + $storedValues[$key] = $value; + return true; + }); + $now = time(); - $timeFactory - ->expects($this->once()) - ->method('getTime') - ->willReturn($now); - $urlGenerator - ->expects($this->once()) - ->method('getAbsoluteURL') - ->with('') - ->willReturn('https://domain.invalid/nextcloud'); + $timeFactory->expects($this->once())->method('getTime')->willReturn($now); + $urlGenerator->expects($this->once())->method('getAbsoluteURL')->with('')->willReturn('https://domain.invalid/nextcloud'); $helper = new Config($config, $appConfig, $secureRandom, $groupManager, $userManager, $urlGenerator, $timeFactory, $dispatcher); - - $config->setAppValue('spreed', 'signaling_token_alg', $algo); - // Make sure new keys are generated. - $config->deleteAppValue('spreed', 'signaling_token_privkey_' . strtolower($algo)); - $config->deleteAppValue('spreed', 'signaling_token_pubkey_' . strtolower($algo)); $ticket = $helper->getSignalingTicket(Config::SIGNALING_TICKET_V2, null); $this->assertNotNull($ticket); - $key = new Key($config->getAppValue('spreed', 'signaling_token_pubkey_' . strtolower($algo)), $algo); + $pubKey = $storedValues['signaling_token_pubkey_' . strtolower($algo)]; + $key = new Key($pubKey, $algo); $decoded = JWT::decode($ticket, $key); $this->assertEquals($now, $decoded->iat); $this->assertEquals('https://domain.invalid/nextcloud', $decoded->iss); + $this->assertFalse(isset($decoded->sub)); } } diff --git a/tests/php/Controller/SignalingControllerTest.php b/tests/php/Controller/SignalingControllerTest.php index 154824cce36..2f6be6f7c10 100644 --- a/tests/php/Controller/SignalingControllerTest.php +++ b/tests/php/Controller/SignalingControllerTest.php @@ -80,6 +80,7 @@ class SignalingControllerTest extends TestCase { protected Authenticator&MockObject $authenticator; protected IDBConnection $dbConnection; protected IConfig $serverConfig; + protected IAppConfig $appConfig; protected ?Config $config = null; protected ?string $userId = null; protected ?ISecureRandom $secureRandom = null; @@ -95,13 +96,17 @@ public function setUp(): void { $this->request = $this->createMock(IRequest::class); /** @var MockObject|IAppConfig $appConfig */ $appConfig = $this->createMock(IAppConfig::class); + $appConfig->method('getAppValueString')->willReturnCallback(function (string $key): string { + return match ($key) { + 'signaling_servers' => json_encode(['secret' => 'MySecretValueMySecretValue1234567890']), + 'signaling_ticket_secret' => 'the-app-ticket-secret', + default => '', + }; + }); + $this->appConfig = $appConfig; $timeFactory = $this->createMock(ITimeFactory::class); $groupManager = $this->createMock(IGroupManager::class); $this->serverConfig = \OCP\Server::get(IConfig::class); - $this->serverConfig->setAppValue('spreed', 'signaling_servers', json_encode([ - 'secret' => 'MySecretValueMySecretValue1234567890', - ])); - $this->serverConfig->setAppValue('spreed', 'signaling_ticket_secret', 'the-app-ticket-secret'); $this->serverConfig->setUserValue($this->userId, 'spreed', 'signaling_ticket_secret', 'the-user-ticket-secret'); $this->userManager = $this->createMock(IUserManager::class); $this->dispatcher = \OCP\Server::get(IEventDispatcher::class); diff --git a/tests/php/Recording/BackendNotifierTest.php b/tests/php/Recording/BackendNotifierTest.php index e2633943378..b682c63a839 100644 --- a/tests/php/Recording/BackendNotifierTest.php +++ b/tests/php/Recording/BackendNotifierTest.php @@ -74,19 +74,21 @@ public function setUp(): void { $config = \OCP\Server::get(IConfig::class); $this->recordingSecret = 'the-recording-secret'; $this->baseUrl = 'https://localhost/recording'; - $config->setAppValue('spreed', 'recording_servers', json_encode([ - 'secret' => $this->recordingSecret, - 'servers' => [ - [ - 'server' => $this->baseUrl, - ], - ], - ])); $this->secureRandom = \OCP\Server::get(ISecureRandom::class); $this->urlGenerator = $this->createMock(IURLGenerator::class); $appConfig = $this->createMock(IAppConfig::class); + $appConfig->method('getAppValueArray') + ->with('recording_servers') + ->willReturn([ + 'secret' => $this->recordingSecret, + 'servers' => [ + [ + 'server' => $this->baseUrl, + ], + ], + ]); $groupManager = $this->createMock(IGroupManager::class); $userManager = $this->createMock(IUserManager::class); $timeFactory = $this->createMock(ITimeFactory::class); @@ -121,8 +123,6 @@ public function setUp(): void { } public function tearDown(): void { - $config = \OCP\Server::get(IConfig::class); - $config->deleteAppValue('spreed', 'recording_servers'); parent::tearDown(); } diff --git a/tests/psalm-baseline.xml b/tests/psalm-baseline.xml index fe2d7771ba7..191467716cd 100644 --- a/tests/psalm-baseline.xml +++ b/tests/psalm-baseline.xml @@ -1,5 +1,5 @@ - + @@ -55,11 +55,6 @@ - - - - -