Merge pull request #32120 from owncloud/bugfix/32090

Fixes #32090 - browser extension urls in origin header do not trigger…
owncloud · Jul 30, 2018 · b8ca551 · b8ca551
2 parents 5224a2b + 9337cb5
commit b8ca551
Show file tree

Hide file tree

Showing 2 changed files with 54 additions and 3 deletions.
diff --git a/apps/dav/lib/Connector/Sabre/CorsPlugin.php b/apps/dav/lib/Connector/Sabre/CorsPlugin.php
@@ -97,8 +97,15 @@ public function initialize(\Sabre\DAV\Server $server) {
 		$this->server = $server;
 
 		$request = $this->server->httpRequest;
-		if (!$request->hasHeader('Origin') || Util::isSameDomain($request->getHeader('Origin'), $request->getAbsoluteUrl())) {
-			return false;
+		if (!$request->hasHeader('Origin')) {
+			return;
+		}
+		$originHeader = $request->getHeader('Origin');
+		if ($this->ignoreOriginHeader($originHeader)) {
+			return;
+		}
+		if (Util::isSameDomain($originHeader, $request->getAbsoluteUrl())) {
+			return;
 		}
 
 		$this->server->on('beforeMethod', [$this, 'setCorsHeaders']);
@@ -147,4 +154,21 @@ public function setOptionsRequestHeaders(RequestInterface $request, ResponseInte
 			return false;
 		}
 	}
+
+	/**
+	 * in addition to schemas used by extensions we ignore empty origin header
+	 * values as well as 'null' which is not valid by the specification but used
+	 * by some clients.
+	 * @link https://github.com/owncloud/core/pull/32120#issuecomment-407008243
+	 *
+	 * @param string $originHeader
+	 * @return bool
+	 */
+	public function ignoreOriginHeader($originHeader) {
+		if (\in_array($originHeader, ['', null, 'null'], true)) {
+			return true;
+		}
+		$schema = \parse_url($originHeader, PHP_URL_SCHEME);
+		return \in_array(\strtolower($schema), ['moz-extension', 'chrome-extension']);
+	}
 }
diff --git a/apps/dav/tests/unit/Connector/Sabre/CorsPluginTest.php b/apps/dav/tests/unit/Connector/Sabre/CorsPluginTest.php
@@ -57,7 +57,6 @@ public function setUp() {
 		$this->server->sapi = $this->getMockBuilder(\stdClass::class)
 			->setMethods(['sendResponse'])
 			->getMock();
-		$this->server->sapi->expects($this->once())->method('sendResponse')->with($this->server->httpResponse);
 
 		$this->server->httpRequest->setMethod('OPTIONS');
 		$this->server->httpRequest->setUrl('/owncloud/remote.php/dav/files/user1/target/path');
@@ -263,8 +262,15 @@ public function optionsCases() {
 
 	/**
 	 * @dataProvider optionsCases
+	 * @param $allowedDomains
+	 * @param $hasUser
+	 * @param $requestHeaders
+	 * @param $expectedStatus
+	 * @param array $expectedHeaders
+	 * @param bool $expectDavHeaders
 	 */
 	public function testOptionsHeaders($allowedDomains, $hasUser, $requestHeaders, $expectedStatus, array $expectedHeaders, $expectDavHeaders = false) {
+		$this->server->sapi->expects($this->once())->method('sendResponse')->with($this->server->httpResponse);
 		$user = $this->createMock(IUser::class);
 		$user->method('getUID')->willReturn('someuser');
 
@@ -299,4 +305,25 @@ public function testOptionsHeaders($allowedDomains, $hasUser, $requestHeaders, $
 		// if it has DAV headers, it means we did not bypass further processing
 		$this->assertEquals($expectDavHeaders, $this->server->httpResponse->hasHeader('DAV'));
 	}
+
+	/**
+	 * @dataProvider providesOriginUrls
+	 * @param $expectedValue
+	 * @param $url
+	 */
+	public function testExtensionRequests($expectedValue, $url) {
+		$plugin = new CorsPlugin($this->createMock(IUserSession::class));
+		self::assertEquals($expectedValue, $plugin->ignoreOriginHeader($url));
+	}
+
+	public function providesOriginUrls() {
+		return [
+			'Firefox extension' => [true, 'moz-extension://mgmnhfbjphngabcpbpmapnnaabhnchmi/'],
+			'Chrome extension' => [true, 'chrome-extension://mgmnhfbjphngabcpbpmapnnaabhnchmi/'],
+			'Empty Origin' => [true, ''],
+			'Null string Origin' => [true, 'null'],
+			'Null Origin' => [true, null],
+			'plain http' => [false, 'http://example.net/'],
+		];
+	}
 }