Merge branch 'stable2.2' into backport/8383/stable2.2

nextcloud · Jun 28, 2023 · fb4abf1 · fb4abf1
2 parents 43a876a + 797b584
commit fb4abf1
Show file tree

Hide file tree

Showing 9 changed files with 78 additions and 125 deletions.
diff --git a/lib/Controller/ProxyController.php b/lib/Controller/ProxyController.php
@@ -36,6 +36,7 @@
 use OCP\IURLGenerator;
 use Psr\Http\Client\ClientExceptionInterface;
 use Psr\Log\LoggerInterface;
+use function file_get_contents;
 
 class ProxyController extends Controller {
 	private IURLGenerator $urlGenerator;
@@ -105,6 +106,12 @@ public function proxy(string $src): ProxyDownloadResponse {
 		// close the session to allow parallel downloads
 		$this->session->close();
 
+		// If strict cookies are set it means we come from the same domain so no open redirect
+		if (!$this->request->passesStrictCookieCheck()) {
+			$content = file_get_contents(__DIR__ . '/../../img/blocked-image.png');
+			return new ProxyDownloadResponse($content, $src, 'application/octet-stream');
+		}
+
 		$client = $this->clientService->newClient();
 		try {
 			$response = $client->get($src);

diff --git a/lib/Folder.php b/lib/Folder.php
@@ -44,12 +44,16 @@ class Folder {
 	/** @var string[] */
 	private $specialUse;
 
-	public function __construct(int $accountId, Horde_Imap_Client_Mailbox $mailbox, array $attributes, ?string $delimiter) {
+	public function __construct(int $accountId,
+		Horde_Imap_Client_Mailbox $mailbox,
+		array $attributes,
+		?string $delimiter,
+		array $status) {
 		$this->accountId = $accountId;
 		$this->mailbox = $mailbox;
 		$this->attributes = $attributes;
 		$this->delimiter = $delimiter;
-		$this->status = [];
+		$this->status = $status;
 		$this->specialUse = [];
 	}
 

diff --git a/lib/IMAP/FolderMapper.php b/lib/IMAP/FolderMapper.php
@@ -59,31 +59,21 @@ public function getFolders(Account $account, Horde_Imap_Client_Socket $client,
 			'delimiter' => true,
 			'attributes' => true,
 			'special_use' => true,
+			'status' => Horde_Imap_Client::STATUS_ALL,
 		]);
 
-		return array_filter(array_map(function (array $mailbox) use ($account, $client) {
+		return array_filter(array_map(static function (array $mailbox) use ($account) {
 			if (in_array($mailbox['mailbox']->utf8, self::DOVECOT_SIEVE_FOLDERS, true)) {
 				// This is a special folder that must not be shown
 				return null;
 			}
 
-			try {
-				$client->status($mailbox["mailbox"]);
-			} catch (Horde_Imap_Client_Exception $e) {
-				// ignore folders which cause errors on access
-				// (i.e. server-side system I/O errors)
-				if (in_array($e->getCode(), [
-					Horde_Imap_Client_Exception::UNSPECIFIED,
-				], true)) {
-					return null;
-				}
-			}
-
 			return new Folder(
 				$account->getId(),
 				$mailbox['mailbox'],
 				$mailbox['attributes'],
-				$mailbox['delimiter']
+				$mailbox['delimiter'],
+				$mailbox['status'],
 			);
 		}, $mailboxes));
 	}
@@ -97,39 +87,21 @@ public function createFolder(Horde_Imap_Client_Socket $client,
 			'delimiter' => true,
 			'attributes' => true,
 			'special_use' => true,
+			'status' => Horde_Imap_Client::STATUS_ALL,
 		]);
 		$mb = reset($list);
 
 		if ($mb === null) {
 			throw new ServiceException("Created mailbox does not exist");
 		}
 
-		return new Folder($account->getId(), $mb['mailbox'], $mb['attributes'], $mb['delimiter']);
-	}
-
-	/**
-	 * @param Folder[] $folders
-	 * @param Horde_Imap_Client_Socket $client
-	 *
-	 * @throws Horde_Imap_Client_Exception
-	 *
-	 * @return void
-	 */
-	public function getFoldersStatus(array $folders,
-									 Horde_Imap_Client_Socket $client): void {
-		$mailboxes = array_map(function (Folder $folder) {
-			return $folder->getMailbox();
-		}, array_filter($folders, function (Folder $folder) {
-			return !in_array('\noselect', $folder->getAttributes());
-		}));
-
-		$status = $client->status($mailboxes);
-
-		foreach ($folders as $folder) {
-			if (isset($status[$folder->getMailbox()])) {
-				$folder->setStatus($status[$folder->getMailbox()]);
-			}
-		}
+		return new Folder(
+			$account->getId(),
+			$mb['mailbox'],
+			$mb['attributes'],
+			$mb['delimiter'],
+			$mb['status'],
+		);
 	}
 
 	/**

diff --git a/lib/IMAP/MailboxSync.php b/lib/IMAP/MailboxSync.php
@@ -102,7 +102,6 @@ public function sync(Account $account,
 
 			try {
 				$folders = $this->folderMapper->getFolders($account, $client);
-				$this->folderMapper->getFoldersStatus($folders, $client);
 			} catch (Horde_Imap_Client_Exception $e) {
 				throw new ServiceException(
 					sprintf("IMAP error synchronizing account %d: %s", $account->getId(), $e->getMessage()),

diff --git a/lib/Service/MailManager.php b/lib/Service/MailManager.php
@@ -152,7 +152,6 @@ public function createMailbox(Account $account, string $name): Mailbox {
 		$client = $this->imapClientFactory->getClient($account);
 		try {
 			$folder = $this->folderMapper->createFolder($client, $account, $name);
-			$this->folderMapper->getFoldersStatus([$folder], $client);
 		} catch (Horde_Imap_Client_Exception $e) {
 			throw new ServiceException(
 				"Could not get mailbox status: " .

diff --git a/tests/FolderTest.php b/tests/FolderTest.php
@@ -40,7 +40,7 @@ private function mockFolder(array $attributes = [], $delimiter = '.') {
 		$this->accountId = 15;
 		$this->mailbox = $this->createMock(Horde_Imap_Client_Mailbox::class);
 
-		$this->folder = new Folder($this->accountId, $this->mailbox, $attributes, $delimiter);
+		$this->folder = new Folder($this->accountId, $this->mailbox, $attributes, $delimiter, []);
 	}
 
 	public function testGetMailbox() {

diff --git a/tests/Unit/Controller/ProxyControllerTest.php b/tests/Unit/Controller/ProxyControllerTest.php
@@ -1,5 +1,7 @@
 <?php
 
+declare(strict_types=1);
+
 /**
  * @author Christoph Wurst <[email protected]>
  *
@@ -156,11 +158,41 @@ public function testRedirectInvalidUrl() {
 		$this->controller->redirect('ftps://example.com');
 	}
 
-	public function testProxy() {
+	public function testProxyWithoutCookies(): void {
 		$src = 'http://example.com';
-		$httpResponse = $this->createMock(IResponse::class);
 		$content = '🐵🐵🐵';
+		$this->session->expects($this->once())
+			->method('close');
+		$client = $this->getMockBuilder(IClient::class)->getMock();
+		$this->clientService->expects(self::never())
+			->method('newClient')
+			->willReturn($client);
+		$unexpected = new ProxyDownloadResponse(
+			$content,
+			$src,
+			'application/octet-stream'
+		);
+		$this->controller = new ProxyController(
+			$this->appName,
+			$this->request,
+			$this->urlGenerator,
+			$this->session,
+			$this->clientService,
+			$this->logger
+		);
+
+		$response = $this->controller->proxy($src);
 
+		$this->assertNotEquals($unexpected, $response);
+	}
+
+	public function testProxy(): void {
+		$src = 'http://example.com';
+		$httpResponse = $this->createMock(IResponse::class);
+		$content = '🐵🐵🐵';
+		$this->request->expects(self::once())
+			->method('passesStrictCookieCheck')
+			->willReturn(true);
 		$this->session->expects($this->once())
 			->method('close');
 		$client = $this->getMockBuilder(IClient::class)->getMock();

diff --git a/tests/Unit/IMAP/FolderMapperTest.php b/tests/Unit/IMAP/FolderMapperTest.php
@@ -42,7 +42,7 @@ protected function setUp(): void {
 		$this->mapper = new FolderMapper();
 	}
 
-	public function testGetFoldersEmtpyAccount() {
+	public function testGetFoldersEmtpyAccount(): void {
 		$account = $this->createMock(Account::class);
 		$client = $this->createMock(Horde_Imap_Client_Socket::class);
 		$client->expects($this->once())
@@ -52,6 +52,7 @@ public function testGetFoldersEmtpyAccount() {
 					'delimiter' => true,
 					'attributes' => true,
 					'special_use' => true,
+					'status' => Horde_Imap_Client::STATUS_ALL,
 				]))
 			->willReturn([]);
 
@@ -60,7 +61,7 @@ public function testGetFoldersEmtpyAccount() {
 		$this->assertEquals([], $folders);
 	}
 
-	public function testGetFolders() {
+	public function testGetFolders(): void {
 		$account = $this->createMock(Account::class);
 		$account->method('getId')->willReturn(27);
 		$client = $this->createMock(Horde_Imap_Client_Socket::class);
@@ -71,32 +72,39 @@ public function testGetFolders() {
 					'delimiter' => true,
 					'attributes' => true,
 					'special_use' => true,
+					'status' => Horde_Imap_Client::STATUS_ALL,
 				]))
 			->willReturn([
 				[
 					'mailbox' => new Horde_Imap_Client_Mailbox('INBOX'),
 					'attributes' => [],
 					'delimiter' => '.',
+					'status' => [
+						'unseen' => 0,
+					],
 				],
 				[
 					'mailbox' => new Horde_Imap_Client_Mailbox('Sent'),
 					'attributes' => [
 						'\sent',
 					],
 					'delimiter' => '.',
+					'status' => [
+						'unseen' => 1,
+					],
 				],
 			]);
 		$expected = [
-			new Folder(27, new Horde_Imap_Client_Mailbox('INBOX'), [], '.'),
-			new Folder(27, new Horde_Imap_Client_Mailbox('Sent'), ['\sent'], '.'),
+			new Folder(27, new Horde_Imap_Client_Mailbox('INBOX'), [], '.', ['unseen' => 0]),
+			new Folder(27, new Horde_Imap_Client_Mailbox('Sent'), ['\sent'], '.', ['unseen' => 1]),
 		];
 
 		$folders = $this->mapper->getFolders($account, $client);
 
 		$this->assertEquals($expected, $folders);
 	}
 
-	public function testCreateFolder() {
+	public function testCreateFolder(): void {
 		$account = $this->createMock(Account::class);
 		$account->method('getId')->willReturn(42);
 		$client = $this->createMock(Horde_Imap_Client_Socket::class);
@@ -110,90 +118,25 @@ public function testCreateFolder() {
 					'delimiter' => true,
 					'attributes' => true,
 					'special_use' => true,
+					'status' => Horde_Imap_Client::STATUS_ALL,
 				]))
 			->willReturn([
 				[
 					'mailbox' => new Horde_Imap_Client_Mailbox('new'),
 					'attributes' => [],
 					'delimiter' => '.',
+					'status' => [
+						'unseen' => 0,
+					],
 				],
 			]);
 
 		$created = $this->mapper->createFolder($client, $account, 'new');
 
-		$expected = new Folder(42, new Horde_Imap_Client_Mailbox('new'), [], '.');
+		$expected = new Folder(42, new Horde_Imap_Client_Mailbox('new'), [], '.', ['unseen' => 0]);
 		$this->assertEquals($expected, $created);
 	}
 
-	public function testGetFoldersStatus() {
-		$folders = [
-			$this->createMock(Folder::class),
-		];
-		$client = $this->createMock(Horde_Imap_Client_Socket::class);
-		$folders[0]->expects($this->any())
-			->method('getMailbox')
-			->willReturn('folder1');
-		$folders[0]->expects($this->once())
-			->method('getAttributes')
-			->willReturn([]);
-		$client->expects($this->once())
-			->method('status')
-			->with($this->equalTo(['folder1']))
-			->willReturn([
-				'folder1' => [
-					'total' => 123
-				],
-			]);
-		$folders[0]->expects($this->once())
-			->method('setStatus');
-
-		$this->mapper->getFoldersStatus($folders, $client);
-	}
-
-	public function testGetFoldersStatusNoStatusReported() {
-		$folders = [
-			$this->createMock(Folder::class),
-		];
-		$client = $this->createMock(Horde_Imap_Client_Socket::class);
-		$folders[0]->expects($this->any())
-			->method('getMailbox')
-			->willReturn('folder1');
-		$folders[0]->expects($this->once())
-			->method('getAttributes')
-			->willReturn([]);
-		$client->expects($this->once())
-			->method('status')
-			->with($this->equalTo(['folder1']))
-			->willReturn([
-				// Nothing reported for this folder
-			]);
-		$folders[0]->expects($this->never())
-			->method('setStatus');
-
-		$this->mapper->getFoldersStatus($folders, $client);
-	}
-
-	public function testGetFoldersStatusNotSearchable() {
-		$folders = [
-			$this->createMock(Folder::class),
-		];
-		$client = $this->createMock(Horde_Imap_Client_Socket::class);
-		$folders[0]->expects($this->any())
-			->method('getMailbox')
-			->willReturn('folder1');
-		$folders[0]->expects($this->once())
-			->method('getAttributes')
-			->willReturn(['\\noselect']);
-		$client->expects($this->once())
-			->method('status')
-			->with($this->equalTo([]))
-			->willReturn([]);
-		$folders[0]->expects($this->never())
-			->method('setStatus');
-
-		$this->mapper->getFoldersStatus($folders, $client);
-	}
-
 	public function testGetFoldersStatusAsObject() {
 		$client = $this->createMock(Horde_Imap_Client_Socket::class);
 		$client->expects($this->once())

diff --git a/tests/Unit/Service/MailManagerTest.php b/tests/Unit/Service/MailManagerTest.php
@@ -141,9 +141,6 @@ public function testCreateFolder() {
 			->method('createFolder')
 			->with($this->equalTo($client), $this->equalTo($account), $this->equalTo('new'))
 			->willReturn($folder);
-		$this->folderMapper->expects($this->once())
-			->method('getFoldersStatus')
-			->with($this->equalTo([$folder]));
 		$this->folderMapper->expects($this->once())
 			->method('detectFolderSpecialUse')
 			->with($this->equalTo([$folder]));