Merge pull request #1502 from nextcloud/bugfix/noid/remove-room-information-unless-authenticated

Remove room information for users which are not a participant

lib/Controller/CallController.php

@@ -34,7 +34,6 @@ use OCP\AppFramework\Http;
 use OCP\AppFramework\Http\DataResponse;
 use OCP\AppFramework\OCSController;
 use OCP\IRequest;
-use OCP\IUserManager;
 
 class CallController extends OCSController {
 	/** @var string */
@@ -48,14 +47,12 @@ class CallController extends OCSController {
 	 * @param string $appName
 	 * @param string $UserId
 	 * @param IRequest $request
-	 * @param IUserManager $userManager
 	 * @param TalkSession $session
 	 * @param Manager $manager
 	 */
 	public function __construct($appName,
 								$UserId,
 								IRequest $request,
-								IUserManager $userManager,
 								TalkSession $session,
 								Manager $manager) {
 		parent::__construct($appName, $request);

lib/Controller/RoomController.php

@@ -69,26 +69,9 @@ class RoomController extends OCSController {
 	private $chatManager;
 	/** @var MessageParser */
 	private $messageParser;
-	/** @var IMailer */
-	private $mailer;
 	/** @var IL10N */
 	private $l10n;
 
-	/**
-	 * @param string $appName
-	 * @param string $UserId
-	 * @param IRequest $request
-	 * @param TalkSession $session
-	 * @param IUserManager $userManager
-	 * @param IGroupManager $groupManager
-	 * @param ILogger $logger
-	 * @param Manager $manager
-	 * @param GuestManager $guestManager
-	 * @param ChatManager $chatManager
-	 * @param MessageParser $messageParser
-	 * @param IMailer $mailer
-	 * @param IL10N $l10n
-	 */
 	public function __construct($appName,
 								$UserId,
 								IRequest $request,
@@ -100,7 +83,6 @@ class RoomController extends OCSController {
 								GuestManager $guestManager,
 								ChatManager $chatManager,
 								MessageParser $messageParser,
-								IMailer $mailer,
 								IL10N $l10n) {
 		parent::__construct($appName, $request);
 		$this->session = $session;
@@ -112,7 +94,6 @@ class RoomController extends OCSController {
 		$this->guestManager = $guestManager;
 		$this->chatManager = $chatManager;
 		$this->messageParser = $messageParser;
-		$this->mailer = $mailer;
 		$this->l10n = $l10n;
 	}
 
@@ -171,45 +152,26 @@ class RoomController extends OCSController {
 	 * @throws RoomNotFoundException
 	 */
 	protected function formatRoom(Room $room, Participant $currentParticipant = null): array {
-
-		if ($currentParticipant instanceof Participant) {
-			$participantType = $currentParticipant->getParticipantType();
-			$participantFlags = $currentParticipant->getInCallFlags();
-			$favorite = $currentParticipant->isFavorite();
-		} else {
-			$participantType = Participant::GUEST;
-			$participantFlags = Participant::FLAG_DISCONNECTED;
-			$favorite = false;
-		}
-		$participantInCall = ($participantFlags & Participant::FLAG_IN_CALL) !== 0;
-
-		$lastActivity = $room->getLastActivity();
-		if ($lastActivity instanceof \DateTimeInterface) {
-			$lastActivity = $lastActivity->getTimestamp();
-		} else {
-			$lastActivity = 0;
-		}
-
 		$roomData = [
 			'id' => $room->getId(),
 			'token' => $room->getToken(),
 			'type' => $room->getType(),
-			'name' => $room->getName(),
-			'displayName' => $room->getName(),
-			'objectType' => $room->getObjectType(),
-			'objectId' => $room->getObjectId(),
-			'participantType' => $participantType,
+			'name' => '',
+			'displayName' => '',
+			'objectType' => '',
+			'objectId' => '',
+			'participantType' => Participant::GUEST,
 			// Deprecated, use participantFlags instead.
-			'participantInCall' => $participantInCall,
-			'participantFlags' => $participantFlags,
-			'count' => $room->getNumberOfParticipants(false, time() - 30),
+			'participantInCall' => false,
+			'participantFlags' => Participant::FLAG_DISCONNECTED,
+			'count' => 0,
 			'hasPassword' => $room->hasPassword(),
-			'hasCall' => $room->getActiveSince() instanceof \DateTimeInterface,
-			'lastActivity' => $lastActivity,
+			'hasCall' => false,
+			'lastActivity' => 0,
 			'unreadMessages' => 0,
 			'unreadMention' => false,
-			'isFavorite' => $favorite,
-			'notificationLevel' => $room->getType() === Room::ONE_TO_ONE_CALL ? Participant::NOTIFY_ALWAYS : Participant::NOTIFY_MENTION,
+			'isFavorite' => false,
+			'notificationLevel' => Participant::NOTIFY_NEVER,
 			'lastPing' => 0,
 			'sessionId' => '0',
 			'participants' => [],
@@ -222,8 +184,35 @@ class RoomController extends OCSController {
 			return $roomData;
 		}
 
-		if ($currentParticipant->getNotificationLevel() !== Participant::NOTIFY_DEFAULT) {
-			$roomData['notificationLevel'] = $currentParticipant->getNotificationLevel();
+		$lastActivity = $room->getLastActivity();
+		if ($lastActivity instanceof \DateTimeInterface) {
+			$lastActivity = $lastActivity->getTimestamp();
+		} else {
+			$lastActivity = 0;
+		}
+
+		$roomData = array_merge($roomData, [
+			'name' => $room->getName(),
+			'displayName' => $room->getName(),
+			'objectType' => $room->getObjectType(),
+			'objectId' => $room->getObjectId(),
+			'participantType' => $currentParticipant->getParticipantType(),
+			// Deprecated, use participantFlags instead.
+			'participantInCall' => ($currentParticipant->getInCallFlags() & Participant::FLAG_IN_CALL) !== 0,
+			'participantFlags' => $currentParticipant->getInCallFlags(),
+			'count' => $room->getNumberOfParticipants(false, time() - 30),
+			'hasCall' => $room->getActiveSince() instanceof \DateTimeInterface,
+			'lastActivity' => $lastActivity,
+			'isFavorite' => $currentParticipant->isFavorite(),
+			'notificationLevel' => $currentParticipant->getNotificationLevel(),
+		]);
+
+		if ($roomData['notificationLevel'] === Participant::NOTIFY_DEFAULT) {
+			if ($currentParticipant->isGuest()) {
+				$roomData['notificationLevel'] = Participant::NOTIFY_NEVER;
+			} else {
+				$roomData['notificationLevel'] = $room->getType() === Room::ONE_TO_ONE_CALL ? Participant::NOTIFY_ALWAYS : Participant::NOTIFY_MENTION;
+			}
 		}
 
 		if ($room->getObjectType() === 'share:password') {
@@ -231,7 +220,7 @@ class RoomController extends OCSController {
 			$roomData['displayName'] = $this->l10n->t('Password request: %s', [$room->getName()]);
 		}
 
-		$currentUser = $this->userManager->get($this->userId);
+		$currentUser = $this->userManager->get($currentParticipant->getUser());
 		if ($currentUser instanceof IUser) {
 			$unreadSince = $this->chatManager->getUnreadMarker($room, $currentUser);
 			if ($currentParticipant instanceof Participant) {
@@ -292,8 +281,8 @@ class RoomController extends OCSController {
 			'lastMessage' => $lastMessage,
 		]);
 
-		if ($this->userId !== null) {
-			unset($participantList[$this->userId]);
+		if (!$currentParticipant->isGuest()) {
+			unset($participantList[$currentParticipant->getUser()]);
 			$numOtherParticipants = \count($participantList);
 			$numGuestParticipants = $numActiveGuests;
 		} else {
@@ -321,7 +310,7 @@ class RoomController extends OCSController {
 
 			/** @noinspection PhpMissingBreakStatementInspection */
 			case Room::PUBLIC_CALL:
-				if ($this->userId === null && $numGuestParticipants) {
+				if ($currentParticipant->isGuest() && $numGuestParticipants) {
 					$guestString = $this->l10n->n('%n other guest', '%n other guests', $numGuestParticipants);
 				} else if ($numGuestParticipants) {
 					$guestString = $this->l10n->n('%n guest', '%n guests', $numGuestParticipants);
@@ -335,7 +324,7 @@ class RoomController extends OCSController {
 					$participantList = array_map(function($participant) {
 						return $participant['name'];
 					}, $participantList);
-					if ($this->userId === null) {
+					if ($currentParticipant->isGuest()) {
 						$participantList[] = $this->l10n->t('You');
 					} else if ($numOtherParticipants === 0) {
 						$participantList = [$this->l10n->t('You')];

tests/php/Controller/RoomControllerTest.php

@@ -40,7 +40,6 @@ use OCP\IL10N;
 use OCP\ILogger;
 use OCP\IRequest;
 use OCP\IUserManager;
-use OCP\Mail\IMailer;
 
 class RoomControllerTest extends \Test\TestCase {
 
@@ -62,8 +61,6 @@ class RoomControllerTest extends \Test\TestCase {
 	protected $guestManager;
 	/** @var MessageParser|\PHPUnit_Framework_MockObject_MockObject */
 	protected $messageParser;
-	/** @var IMailer|\PHPUnit_Framework_MockObject_MockObject */
-	protected $mailer;
 	/** @var IL10N|\PHPUnit_Framework_MockObject_MockObject */
 	private $l;
 
@@ -80,7 +77,6 @@ class RoomControllerTest extends \Test\TestCase {
 		$this->guestManager = $this->createMock(GuestManager::class);
 		$this->chatManager = $this->createMock(ChatManager::class);
 		$this->messageParser = $this->createMock(MessageParser::class);
-		$this->mailer = $this->createMock(IMailer::class);
 		$this->l = $this->createMock(IL10N::class);
 	}
 
@@ -97,7 +93,6 @@ class RoomControllerTest extends \Test\TestCase {
 			$this->guestManager,
 			$this->chatManager,
 			$this->messageParser,
-			$this->mailer,
 			$this->l
 		);
 	}

tests/php/Signaling/BackendNotifierTest.php

@@ -48,7 +48,7 @@ class CustomBackendNotifier extends BackendNotifier {
 		$this->requests = [];
 	}
 
-	protected function doRequest($url, $params) {
+	protected function doRequest(string $url, array $params) {
 		$this->requests[] = [
 			'url' => $url,
 			'params' => $params,