Remove conversation names and tokens when you are not a participant

Signed-off-by: Joas Schilling <coding@schilljs.com>

6 files changed, 44 insertions, 17 deletions

diff --git a/lib/Manager.php b/lib/Manager.php
index 075e839bc..9e7e4fd03 100644
--- a/lib/Manager.php
+++ b/lib/Manager.php
@@ -583,13 +583,21 @@ class Manager {
 		if ($room->getType() === Room::CHANGELOG_CONVERSATION) {
 			return $this->l->t('Talk updates ✅');
 		}
+		if ($userId === '' && $room->getType() !== Room::PUBLIC_CALL) {
+			return $this->l->t('Private conversation');
+		}
+
 
 		if ($room->getType() !== Room::ONE_TO_ONE_CALL && $room->getName() === '') {
 			$room->setName($this->getRoomNameByParticipants($room));
 		}
 
 		// Set the room name to the other participant for one-to-one rooms
-		if ($userId !== '' && $room->getType() === Room::ONE_TO_ONE_CALL) {
+		if ($room->getType() === Room::ONE_TO_ONE_CALL) {
+			if ($userId === '') {
+				return $this->l->t('Private conversation');
+			}
+
 			$users = $room->getParticipantUserIds();
 			$otherParticipant = '';
 			$userIsParticipant = false;
@@ -616,6 +624,13 @@ class Manager {
 			return $otherParticipant;
 		}
 
+		try {
+			$room->getParticipant($userId);
+		} catch (ParticipantNotFoundException $e) {
+			// Do not leak the name of rooms the user is not a part of
+			return $this->l->t('Private conversation');
+		}
+
 		return $room->getName();
 	}
 
diff --git a/lib/Share/Helper/ShareAPIController.php b/lib/Share/Helper/ShareAPIController.php
index 64398f56a..04ecfdc4d 100644
--- a/lib/Share/Helper/ShareAPIController.php
+++ b/lib/Share/Helper/ShareAPIController.php
@@ -85,6 +85,14 @@ class ShareAPIController {
 		}
 
 		$result['share_with_displayname'] = $room->getDisplayName($this->userId);
+		try {
+			$room->getParticipant($this->userId);
+		} catch (ParticipantNotFoundException $e) {
+			// Removing the conversation token from the leaked data if not a participant.
+			// Adding some unique but reproducable part to the share_with here
+			// so the avatars for conversations are distinguishable
+			$result['share_with'] = 'private_conversation_' . substr(sha1($room->getName() . $room->getId()), 0, 6);
+		}
 		if ($room->getType() === Room::PUBLIC_CALL) {
 			$result['token'] = $share->getToken();
 		}
diff --git a/tests/integration/features/bootstrap/SharingContext.php b/tests/integration/features/bootstrap/SharingContext.php
index 0d377ec91..deabef7d0 100644
--- a/tests/integration/features/bootstrap/SharingContext.php
+++ b/tests/integration/features/bootstrap/SharingContext.php
@@ -64,7 +64,7 @@ class SharingContext implements Context {
 	 */
 	public function userCreatesFolder($user, $destination) {
 		$this->currentUser = $user;
-	
+
 		$url = "/$user/$destination/";
 
 		$this->sendingToDav('MKCOL', $url);
@@ -81,7 +81,7 @@ class SharingContext implements Context {
 	 */
 	public function userMovesFileTo(string $user, string $source, string $destination) {
 		$this->currentUser = $user;
-	
+
 		$url = "/$user/$source";
 
 		$headers = [];
@@ -111,7 +111,7 @@ class SharingContext implements Context {
 	 */
 	public function userDeletesFile($user, $file) {
 		$this->currentUser = $user;
-	
+
 		$url = "/$user/$file";
 
 		$this->sendingToDav('DELETE', $url);
@@ -587,7 +587,11 @@ class SharingContext implements Context {
 		if (array_key_exists('share_type', $expectedFields) &&
 				$expectedFields['share_type'] == 10 /* Share::SHARE_TYPE_ROOM */ &&
 				array_key_exists('share_with', $expectedFields)) {
-			$expectedFields['share_with'] = FeatureContext::getTokenForIdentifier($expectedFields['share_with']);
+			if ($expectedFields['share_with'] === 'private_conversation') {
+				$expectedFields['share_with'] = 'REGEXP /^private_conversation_[0-9a-f]{6}$/';
+			} else {
+				$expectedFields['share_with'] = FeatureContext::getTokenForIdentifier($expectedFields['share_with']);
+			}
 		}
 
 		foreach ($expectedFields as $field => $value) {
diff --git a/tests/integration/features/sharing/create.feature b/tests/integration/features/sharing/create.feature
index 5314d3144..e97ae0ffb 100644
--- a/tests/integration/features/sharing/create.feature
+++ b/tests/integration/features/sharing/create.feature
@@ -396,8 +396,8 @@ Feature: create
       | mimetype               | text/plain |
       | storage_id             | home::participant2 |
       | file_target            | /welcome (2).txt |
-      | share_with             | group room |
-      | share_with_displayname | Group room |
+      | share_with             | private_conversation |
+      | share_with_displayname | Private conversation |
     And user "participant3" gets last share
     And share is returned with
       | uid_owner              | participant1 |
diff --git a/tests/integration/features/sharing/get.feature b/tests/integration/features/sharing/get.feature
index 905e4cbbc..be1f371c5 100644
--- a/tests/integration/features/sharing/get.feature
+++ b/tests/integration/features/sharing/get.feature
@@ -330,8 +330,8 @@ Feature: get
       | mimetype               | text/plain |
       | storage_id             | home::participant1 |
       | file_target            | /welcome (2).txt |
-      | share_with             | one-to-one room not invited to |
-      | share_with_displayname | Private conversation|
+      | share_with             | private_conversation |
+      | share_with_displayname | Private conversation |
 
   Scenario: get all shares and reshares of a user who reshared a file to an owned one-to-one room
     Given user "participant2" creates room "one-to-one room not invited to"
@@ -364,7 +364,7 @@ Feature: get
       | mimetype               | text/plain |
       | storage_id             | home::participant1 |
       | file_target            | /welcome (2).txt |
-      | share_with             | one-to-one room not invited to |
+      | share_with             | private_conversation |
       | share_with_displayname | Private conversation |
 
   Scenario: get all shares and reshares of a user who reshared a file to a one-to-one room
@@ -398,7 +398,7 @@ Feature: get
       | mimetype               | text/plain |
       | storage_id             | home::participant1 |
       | file_target            | /welcome (2).txt |
-      | share_with             | one-to-one room not invited to |
+      | share_with             | private_conversation |
       | share_with_displayname | Private conversation |
 
   Scenario: get all shares of a file
@@ -543,7 +543,7 @@ Feature: get
       | mimetype               | text/plain |
       | storage_id             | home::participant1 |
       | file_target            | /welcome (2).txt |
-      | share_with             | one-to-one room not invited to |
+      | share_with             | private_conversation |
       | share_with_displayname | Private conversation |
 
   Scenario: get all shares and reshares of a file reshared to a one-to-one room by its owner
@@ -577,7 +577,7 @@ Feature: get
       | mimetype               | text/plain |
       | storage_id             | home::participant1 |
       | file_target            | /welcome (2).txt |
-      | share_with             | one-to-one room not invited to |
+      | share_with             | private_conversation |
       | share_with_displayname | Private conversation |
 
   Scenario: get all shares and reshares of a file reshared to a one-to-one room by its second participant
@@ -611,7 +611,7 @@ Feature: get
       | mimetype               | text/plain |
       | storage_id             | home::participant1 |
       | file_target            | /welcome (2).txt |
-      | share_with             | one-to-one room not invited to |
+      | share_with             | private_conversation |
       | share_with_displayname | Private conversation |
 
   Scenario: get all shares and reshares of a deleted file
@@ -729,7 +729,7 @@ Feature: get
       | mimetype               | httpd/unix-directory |
       | storage_id             | home::participant1 |
       | file_target            | /subfolder |
-      | share_with             | one-to-one room not invited to |
+      | share_with             | private_conversation |
       | share_with_displayname | Private conversation |
       | permissions            | 31 |
 
diff --git a/tests/integration/features/sharing/transfer-ownership.feature b/tests/integration/features/sharing/transfer-ownership.feature
index b7c430b44..9fcd1a842 100644
--- a/tests/integration/features/sharing/transfer-ownership.feature
+++ b/tests/integration/features/sharing/transfer-ownership.feature
@@ -128,8 +128,8 @@ Feature: transfer-ownership
       | mimetype               | text/plain |
       | storage_id             | home::participant2 |
       | file_target            | /welcome.txt |
-      | share_with             | group room |
-      | share_with_displayname | Group room |
+      | share_with             | private_conversation |
+      | share_with_displayname | Private conversation |
     And user "participant3" gets last share
     And share is returned with
       | uid_owner              | participant2 |