Merge pull request #13255 from nextcloud/backport/13181/stable30

[stable30] fix(signaling): Avoid conflict potential of user and federated user
nextcloud · Sep 11, 2024 · c7d9191 · c7d9191
2 parents bbdea3d + 0a7a3f6
commit c7d9191
Show file tree

Hide file tree

Showing 6 changed files with 20 additions and 36 deletions.
diff --git a/lib/Config.php b/lib/Config.php
@@ -16,7 +16,6 @@
 use OCP\AppFramework\Services\IAppConfig;
 use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\EventDispatcher\IEventDispatcher;
-use OCP\Federation\ICloudIdManager;
 use OCP\IConfig;
 use OCP\IGroupManager;
 use OCP\IURLGenerator;
@@ -48,7 +47,6 @@ public function __construct(
 		private ISecureRandom $secureRandom,
 		private IGroupManager $groupManager,
 		private IUserManager $userManager,
-		private ICloudIdManager $cloudIdManager,
 		private IURLGenerator $urlGenerator,
 		protected ITimeFactory $timeFactory,
 		private IEventDispatcher $dispatcher,
@@ -477,12 +475,11 @@ public function getHideSignalingWarning(): bool {
 	 * @param string|null $userId
 	 * @return string
 	 */
-	public function getSignalingTicket(int $version, ?string $userId): string {
+	public function getSignalingTicket(int $version, ?string $userId, ?string $cloudId = null): string {
 		switch ($version) {
-			case self::SIGNALING_TICKET_V1:
-				return $this->getSignalingTicketV1($userId);
 			case self::SIGNALING_TICKET_V2:
-				return $this->getSignalingTicketV2($userId);
+				return $this->getSignalingTicketV2($userId, $cloudId);
+			case self::SIGNALING_TICKET_V1:
 			default:
 				return $this->getSignalingTicketV1($userId);
 		}
@@ -606,19 +603,19 @@ public function getSignalingFederatedUserData(): array {
 	 *                            a cloud id.
 	 * @return string
 	 */
-	private function getSignalingTicketV2(?string $userId): string {
+	private function getSignalingTicketV2(?string $userId, ?string $cloudId): string {
 		$timestamp = $this->timeFactory->getTime();
 		$data = [
 			'iss' => $this->urlGenerator->getAbsoluteURL(''),
 			'iat' => $timestamp,
 			'exp' => $timestamp + 60,  // Valid for 1 minute.
 		];
-		$user = !empty($userId) ? $this->userManager->get($userId) : null;
+		$user = $userId !== null ? $this->userManager->get($userId) : null;
 		if ($user instanceof IUser) {
 			$data['sub'] = $user->getUID();
 			$data['userdata'] = $this->getSignalingUserData($user);
-		} elseif (!empty($userId) && $this->cloudIdManager->isValidCloudId($userId)) {
-			$data['sub'] = $userId;
+		} elseif ($cloudId !== null && $cloudId !== '') {
+			$data['sub'] = $cloudId;
 			$extendedData = $this->getSignalingFederatedUserData();
 			if (!empty($extendedData)) {
 				$data['userdata'] = $extendedData;

diff --git a/lib/Controller/SignalingController.php b/lib/Controller/SignalingController.php
@@ -195,14 +195,15 @@ public function getSettings(string $token = ''): DataResponse {
 		$signalingMode = $this->talkConfig->getSignalingMode();
 		$signaling = $this->signalingManager->getSignalingServerLinkForConversation($room);
 
-		$helloAuthParams20UserId = $isTalkFederation ? $this->federationAuthenticator->getCloudId() : $this->userId;
+		$helloAuthParams20UserId = $isTalkFederation ? null : $this->userId;
+		$helloAuthParams20CloudId = $isTalkFederation ? $this->federationAuthenticator->getCloudId() : null;
 		$helloAuthParams = [
 			'1.0' => [
 				'userid' => $this->userId,
 				'ticket' => $this->talkConfig->getSignalingTicket(Config::SIGNALING_TICKET_V1, $this->userId),
 			],
 			'2.0' => [
-				'token' => $this->talkConfig->getSignalingTicket(Config::SIGNALING_TICKET_V2, $helloAuthParams20UserId),
+				'token' => $this->talkConfig->getSignalingTicket(Config::SIGNALING_TICKET_V2, $helloAuthParams20UserId, $helloAuthParams20CloudId),
 			],
 		];
 		$data = [

diff --git a/tests/php/ConfigTest.php b/tests/php/ConfigTest.php
@@ -15,7 +15,6 @@
 use OCP\AppFramework\Services\IAppConfig;
 use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\EventDispatcher\IEventDispatcher;
-use OCP\Federation\ICloudIdManager;
 use OCP\IConfig;
 use OCP\IGroupManager;
 use OCP\IURLGenerator;
@@ -37,14 +36,12 @@ private function createConfig(IConfig $config) {
 		$groupManager = $this->createMock(IGroupManager::class);
 		/** @var MockObject|IUserManager $userManager */
 		$userManager = $this->createMock(IUserManager::class);
-		/** @var MockObject|ICloudIdManager $cloudIdManager */
-		$cloudIdManager = $this->createMock(ICloudIdManager::class);
 		/** @var MockObject|IURLGenerator $urlGenerator */
 		$urlGenerator = $this->createMock(IURLGenerator::class);
 		/** @var MockObject|IEventDispatcher $dispatcher */
 		$dispatcher = $this->createMock(IEventDispatcher::class);
 
-		$helper = new Config($config, $appConfig, $secureRandom, $groupManager, $userManager, $cloudIdManager, $urlGenerator, $timeFactory, $dispatcher);
+		$helper = new Config($config, $appConfig, $secureRandom, $groupManager, $userManager, $urlGenerator, $timeFactory, $dispatcher);
 		return $helper;
 	}
 
@@ -148,8 +145,6 @@ public function testGenerateTurnSettings(): void {
 		$groupManager = $this->createMock(IGroupManager::class);
 		/** @var MockObject|IUserManager $userManager */
 		$userManager = $this->createMock(IUserManager::class);
-		/** @var MockObject|ICloudIdManager $cloudIdManager */
-		$cloudIdManager = $this->createMock(ICloudIdManager::class);
 		/** @var MockObject|IURLGenerator $urlGenerator */
 		$urlGenerator = $this->createMock(IURLGenerator::class);
 		/** @var MockObject|IEventDispatcher $dispatcher */
@@ -162,7 +157,7 @@ public function testGenerateTurnSettings(): void {
 			->method('generate')
 			->with(16)
 			->willReturn('abcdefghijklmnop');
-		$helper = new Config($config, $appConfig, $secureRandom, $groupManager, $userManager, $cloudIdManager, $urlGenerator, $timeFactory, $dispatcher);
+		$helper = new Config($config, $appConfig, $secureRandom, $groupManager, $userManager, $urlGenerator, $timeFactory, $dispatcher);
 
 		//
 		$settings = $helper->getTurnSettings();
@@ -226,9 +221,6 @@ public function testGenerateTurnSettingsEvent(): void {
 		/** @var MockObject|IUserManager $userManager */
 		$userManager = $this->createMock(IUserManager::class);
 
-		/** @var MockObject|ICloudIdManager $cloudIdManager */
-		$cloudIdManager = $this->createMock(ICloudIdManager::class);
-
 		/** @var MockObject|IURLGenerator $urlGenerator */
 		$urlGenerator = $this->createMock(IURLGenerator::class);
 
@@ -257,7 +249,7 @@ public function testGenerateTurnSettingsEvent(): void {
 
 		$dispatcher->addServiceListener(BeforeTurnServersGetEvent::class, GetTurnServerListener::class);
 
-		$helper = new Config($config, $appConfig, $secureRandom, $groupManager, $userManager, $cloudIdManager, $urlGenerator, $timeFactory, $dispatcher);
+		$helper = new Config($config, $appConfig, $secureRandom, $groupManager, $userManager, $urlGenerator, $timeFactory, $dispatcher);
 
 		$settings = $helper->getTurnSettings();
 		$this->assertSame($servers, $settings);
@@ -362,8 +354,6 @@ public function testSignalingTicketV2User(string $algo): void {
 		$groupManager = $this->createMock(IGroupManager::class);
 		/** @var MockObject|IUserManager $userManager */
 		$userManager = $this->createMock(IUserManager::class);
-		/** @var MockObject|ICloudIdManager $cloudIdManager */
-		$cloudIdManager = $this->createMock(ICloudIdManager::class);
 		/** @var MockObject|IURLGenerator $urlGenerator */
 		$urlGenerator = $this->createMock(IURLGenerator::class);
 		/** @var MockObject|IEventDispatcher $dispatcher */
@@ -395,7 +385,7 @@ public function testSignalingTicketV2User(string $algo): void {
 			->method('getDisplayName')
 			->willReturn('Jane Doe');
 
-		$helper = new Config($config, $appConfig, $secureRandom, $groupManager, $userManager, $cloudIdManager, $urlGenerator, $timeFactory, $dispatcher);
+		$helper = new Config($config, $appConfig, $secureRandom, $groupManager, $userManager, $urlGenerator, $timeFactory, $dispatcher);
 
 		$config->setAppValue('spreed', 'signaling_token_alg', $algo);
 		// Make sure new keys are generated.
@@ -429,8 +419,6 @@ public function testSignalingTicketV2Anonymous(string $algo): void {
 		$groupManager = $this->createMock(IGroupManager::class);
 		/** @var MockObject|IUserManager $userManager */
 		$userManager = $this->createMock(IUserManager::class);
-		/** @var MockObject|ICloudIdManager $cloudIdManager */
-		$cloudIdManager = $this->createMock(ICloudIdManager::class);
 		/** @var MockObject|IURLGenerator $urlGenerator */
 		$urlGenerator = $this->createMock(IURLGenerator::class);
 		/** @var MockObject|IEventDispatcher $dispatcher */
@@ -447,7 +435,7 @@ public function testSignalingTicketV2Anonymous(string $algo): void {
 			->with('')
 			->willReturn('https://domain.invalid/nextcloud');
 
-		$helper = new Config($config, $appConfig, $secureRandom, $groupManager, $userManager, $cloudIdManager, $urlGenerator, $timeFactory, $dispatcher);
+		$helper = new Config($config, $appConfig, $secureRandom, $groupManager, $userManager, $urlGenerator, $timeFactory, $dispatcher);
 
 		$config->setAppValue('spreed', 'signaling_token_alg', $algo);
 		// Make sure new keys are generated.

diff --git a/tests/php/Controller/SignalingControllerTest.php b/tests/php/Controller/SignalingControllerTest.php
@@ -30,7 +30,6 @@
 use OCP\AppFramework\Services\IAppConfig;
 use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\EventDispatcher\IEventDispatcher;
-use OCP\Federation\ICloudIdManager;
 use OCP\Http\Client\IClientService;
 use OCP\IConfig;
 use OCP\IDBConnection;
@@ -71,7 +70,6 @@ class SignalingControllerTest extends TestCase {
 	protected SessionService&MockObject $sessionService;
 	protected Messages&MockObject $messages;
 	protected IUserManager&MockObject $userManager;
-	protected ICloudIdManager&MockObject $cloudIdManager;
 	protected ITimeFactory&MockObject $timeFactory;
 	protected IClientService&MockObject $clientService;
 	protected IThrottler&MockObject $throttler;
@@ -103,10 +101,9 @@ public function setUp(): void {
 		$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->cloudIdManager = $this->createMock(ICloudIdManager::class);
 		$this->dispatcher = \OCP\Server::get(IEventDispatcher::class);
 		$urlGenerator = $this->createMock(IURLGenerator::class);
-		$this->config = new Config($this->serverConfig, $appConfig, $this->secureRandom, $groupManager, $this->userManager, $this->cloudIdManager, $urlGenerator, $timeFactory, $this->dispatcher);
+		$this->config = new Config($this->serverConfig, $appConfig, $this->secureRandom, $groupManager, $this->userManager, $urlGenerator, $timeFactory, $this->dispatcher);
 		$this->session = $this->createMock(TalkSession::class);
 		$this->dbConnection = \OCP\Server::get(IDBConnection::class);
 		$this->signalingManager = $this->createMock(\OCA\Talk\Signaling\Manager::class);

diff --git a/tests/php/Listener/RestrictStartingCallsTest.php b/tests/php/Listener/RestrictStartingCallsTest.php
@@ -18,6 +18,9 @@
 use PHPUnit\Framework\MockObject\MockObject;
 use Test\TestCase;
 
+/**
+ * @group DB
+ */
 class RestrictStartingCallsTest extends TestCase {
 	protected IConfig&MockObject $serverConfig;
 	protected ParticipantService&MockObject $participantService;

diff --git a/tests/php/Recording/BackendNotifierTest.php b/tests/php/Recording/BackendNotifierTest.php
@@ -23,7 +23,6 @@
 use OCP\AppFramework\Services\IAppConfig;
 use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\EventDispatcher\IEventDispatcher;
-use OCP\Federation\ICloudIdManager;
 use OCP\Http\Client\IClientService;
 use OCP\IConfig;
 use OCP\IDBConnection;
@@ -91,11 +90,10 @@ public function setUp(): void {
 		$appConfig = $this->createMock(IAppConfig::class);
 		$groupManager = $this->createMock(IGroupManager::class);
 		$userManager = $this->createMock(IUserManager::class);
-		$cloudIdManager = $this->createMock(ICloudIdManager::class);
 		$timeFactory = $this->createMock(ITimeFactory::class);
 		$dispatcher = \OCP\Server::get(IEventDispatcher::class);
 
-		$this->config = new Config($config, $appConfig, $this->secureRandom, $groupManager, $userManager, $cloudIdManager, $this->urlGenerator, $timeFactory, $dispatcher);
+		$this->config = new Config($config, $appConfig, $this->secureRandom, $groupManager, $userManager, $this->urlGenerator, $timeFactory, $dispatcher);
 
 		$this->recreateBackendNotifier();