fix(dav): Use IRequest constant to match version

The pattern matches since a 10 year old client version

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

apps/dav/lib/Connector/Sabre/BlockLegacyClientPlugin.php

@@ -26,6 +26,7 @@
 namespace OCA\DAV\Connector\Sabre;
 
 use OCP\IConfig;
+use OCP\IRequest;
 use Sabre\DAV\ServerPlugin;
 use Sabre\HTTP\RequestInterface;
 use Sabre\DAV\Server;
@@ -65,10 +66,7 @@ class BlockLegacyClientPlugin extends ServerPlugin {
 		}
 
 		$minimumSupportedDesktopVersion = $this->config->getSystemValue('minimum.supported.desktop.version', '2.3.0');
-
-		// Match on the mirall version which is in scheme "Mozilla/5.0 (%1) mirall/%2" or
-		// "mirall/%1" for older releases
-		preg_match("/(?:mirall\\/)([\d.]+)/i", $userAgent, $versionMatches);
+		preg_match(IRequest::USER_AGENT_CLIENT_DESKTOP, $userAgent, $versionMatches);
 		if (isset($versionMatches[1]) &&
 			version_compare($versionMatches[1], $minimumSupportedDesktopVersion) === -1) {
 			throw new \Sabre\DAV\Exception\Forbidden('Unsupported client version.');

apps/dav/tests/unit/Connector/Sabre/AuthTest.php

@@ -301,11 +301,6 @@ class AuthTest extends TestCase {
 		$this->request
 			->expects($this->any())
 			->method('isUserAgent')
-			->with([
-				'/^Mozilla\/5\.0 \([A-Za-z ]+\) (mirall|csyncoC)\/.*$/',
-				'/^Mozilla\/5\.0 \(Android\) (ownCloud|Nextcloud)\-android.*$/',
-				'/^Mozilla\/5\.0 \(iOS\) (ownCloud|Nextcloud)\-iOS.*$/',
-			])
 			->willReturn(false);
 		$this->session
 			->expects($this->any())
@@ -351,11 +346,6 @@ class AuthTest extends TestCase {
 		$this->request
 			->expects($this->any())
 			->method('isUserAgent')
-			->with([
-				'/^Mozilla\/5\.0 \([A-Za-z ]+\) (mirall|csyncoC)\/.*$/',
-				'/^Mozilla\/5\.0 \(Android\) ownCloud\-android.*$/',
-				'/^Mozilla\/5\.0 \(iOS\) (ownCloud|Nextcloud)\-iOS.*$/',
-			])
 			->willReturn(false);
 		$this->session
 			->expects($this->any())
@@ -405,11 +395,6 @@ class AuthTest extends TestCase {
 		$this->request
 			->expects($this->any())
 			->method('isUserAgent')
-			->with([
-				'/^Mozilla\/5\.0 \([A-Za-z ]+\) (mirall|csyncoC)\/.*$/',
-				'/^Mozilla\/5\.0 \(Android\) (ownCloud|Nextcloud)\-android.*$/',
-				'/^Mozilla\/5\.0 \(iOS\) (ownCloud|Nextcloud)\-iOS.*$/',
-			])
 			->willReturn(false);
 		$this->session
 			->expects($this->any())
@@ -451,11 +436,6 @@ class AuthTest extends TestCase {
 		$this->request
 			->expects($this->any())
 			->method('isUserAgent')
-			->with([
-				'/^Mozilla\/5\.0 \([A-Za-z ]+\) (mirall|csyncoC)\/.*$/',
-				'/^Mozilla\/5\.0 \(Android\) (ownCloud|Nextcloud)\-android.*$/',
-				'/^Mozilla\/5\.0 \(iOS\) (ownCloud|Nextcloud)\-iOS.*$/',
-			])
 			->willReturn(true);
 		$this->session
 			->expects($this->any())

apps/dav/tests/unit/Connector/Sabre/BlockLegacyClientPluginTest.php

@@ -1,4 +1,6 @@
 <?php
+
+declare(strict_types=1);
 /**
  * @copyright Copyright (c) 2016, ownCloud, Inc.
  *
@@ -28,7 +30,9 @@ namespace OCA\DAV\Tests\unit\Connector\Sabre;
 
 use OCA\DAV\Connector\Sabre\BlockLegacyClientPlugin;
 use OCP\IConfig;
+use PHPUnit\Framework\MockObject\MockObject;
 use Test\TestCase;
+use Sabre\HTTP\RequestInterface;
 
 /**
  * Class BlockLegacyClientPluginTest
@@ -36,7 +40,7 @@ use Test\TestCase;
  * @package OCA\DAV\Tests\unit\Connector\Sabre
  */
 class BlockLegacyClientPluginTest extends TestCase {
-	/** @var IConfig | \PHPUnit\Framework\MockObject\MockObject */
+	/** @var IConfig|MockObject */
 	private $config;
 	/** @var BlockLegacyClientPlugin */
 	private $blockLegacyClientVersionPlugin;
@@ -44,34 +48,25 @@ class BlockLegacyClientPluginTest extends TestCase {
 	protected function setUp(): void {
 		parent::setUp();
 
-		$this->config = $this->getMockBuilder(IConfig::class)
-			->disableOriginalConstructor()
-			->getMock();
+		$this->config = $this->createMock(IConfig::class);
 		$this->blockLegacyClientVersionPlugin = new BlockLegacyClientPlugin($this->config);
 	}
 
-	/**
-	 * @return array
-	 */
-	public function oldDesktopClientProvider() {
+	public function oldDesktopClientProvider(): array {
 		return [
-			['Mozilla/5.0 (1.5.0) mirall/1.5.0'],
-			['mirall/1.5.0'],
-			['mirall/1.5.4'],
-			['mirall/1.6.0'],
+			['Mozilla/5.0 (Windows) mirall/1.5.0'],
 			['Mozilla/5.0 (Bogus Text) mirall/1.6.9'],
 		];
 	}
 
 	/**
 	 * @dataProvider oldDesktopClientProvider
-	 * @param string $userAgent
 	 */
-	public function testBeforeHandlerException($userAgent): void {
+	public function testBeforeHandlerException(string $userAgent): void {
 		$this->expectException(\Sabre\DAV\Exception\Forbidden::class);
 		$this->expectExceptionMessage('Unsupported client version.');
 
-		/** @var \Sabre\HTTP\RequestInterface | \PHPUnit\Framework\MockObject\MockObject $request */
+		/** @var RequestInterface|MockObject $request */
 		$request = $this->createMock('\Sabre\HTTP\RequestInterface');
 		$request
 			->expects($this->once())
@@ -88,26 +83,20 @@ class BlockLegacyClientPluginTest extends TestCase {
 		$this->blockLegacyClientVersionPlugin->beforeHandler($request);
 	}
 
-	/**
-	 * @return array
-	 */
-	public function newAndAlternateDesktopClientProvider() {
+	public function newAndAlternateDesktopClientProvider(): array {
 		return [
-			['Mozilla/5.0 (1.7.0) mirall/1.7.0'],
-			['mirall/1.8.3'],
-			['mirall/1.7.2'],
-			['mirall/1.7.0'],
+			['Mozilla/5.0 (Windows) mirall/1.7.0'],
 			['Mozilla/5.0 (Bogus Text) mirall/1.9.3'],
+			['Mozilla/5.0 (Not Our Client But Old Version) LegacySync/1.1.0'],
 		];
 	}
 
 	/**
 	 * @dataProvider newAndAlternateDesktopClientProvider
-	 * @param string $userAgent
 	 */
-	public function testBeforeHandlerSuccess($userAgent): void {
-		/** @var \Sabre\HTTP\RequestInterface | \PHPUnit\Framework\MockObject\MockObject $request */
-		$request = $this->createMock('\Sabre\HTTP\RequestInterface');
+	public function testBeforeHandlerSuccess(string $userAgent): void {
+		/** @var RequestInterface|MockObject $request */
+		$request = $this->createMock(RequestInterface::class);
 		$request
 			->expects($this->once())
 			->method('getHeader')
@@ -124,8 +113,8 @@ class BlockLegacyClientPluginTest extends TestCase {
 	}
 
 	public function testBeforeHandlerNoUserAgent(): void {
-		/** @var \Sabre\HTTP\RequestInterface | \PHPUnit\Framework\MockObject\MockObject $request */
-		$request = $this->createMock('\Sabre\HTTP\RequestInterface');
+		/** @var RequestInterface|MockObject $request */
+		$request = $this->createMock(RequestInterface::class);
 		$request
 			->expects($this->once())
 			->method('getHeader')