Fix redirect in proxy controller with no referrer

lib/AppInfo/Application.php

@@ -70,7 +70,6 @@ private function initializeAppContainer() {
 		});
 
 		$container->registerParameter("testSmtp", $testSmtp);
-		$container->registerParameter("referrer", isset($_SERVER['HTTP_REFERER']) ? $_SERVER['HTTP_REFERER'] : null);
 		$container->registerParameter("hostname", Util::getServerHostName());
 
 		$container->registerAlias('ErrorMiddleware', ErrorMiddleware::class);

lib/Controller/ProxyController.php

@@ -46,9 +46,6 @@ class ProxyController extends Controller {
 	/** @var IClientService */
 	private $clientService;
 
-	/** @var string */
-	private $referrer;
-
 	/** @var string */
 	private $hostname;
 
@@ -58,16 +55,19 @@ class ProxyController extends Controller {
 	 * @param IURLGenerator $urlGenerator
 	 * @param ISession $session
 	 * @param IClientService $clientService
-	 * @param string $referrer
 	 * @param string $hostname
 	 */
-	public function __construct(string $appName, IRequest $request,
-								IURLGenerator $urlGenerator, ISession $session, IClientService $clientService, $referrer, $hostname) {
+	public function __construct(string $appName,
+								IRequest $request,
+								IURLGenerator $urlGenerator,
+								ISession $session,
+								IClientService $clientService,
+								$hostname) {
 		parent::__construct($appName, $request);
+		$this->request = $request;
 		$this->urlGenerator = $urlGenerator;
 		$this->session = $session;
 		$this->clientService = $clientService;
-		$this->referrer = $referrer;
 		$this->hostname = $hostname;
 	}
 
@@ -93,7 +93,8 @@ public function redirect(string $src): TemplateResponse {
 		// this is there to prevent an open redirector.
 		// Since we can't prevent the referrer from being added with a HTTP only header we rely on an
 		// additional JS file here.
-		if (parse_url($this->referrer, PHP_URL_HOST) === $this->hostname) {
+		$referrer = $this->request->server['HTTP_REFERER'] ?? null;
+		if (is_string($referrer) && parse_url($referrer, PHP_URL_HOST) === $this->hostname) {
 			$authorizedRedirect = true;
 		}
 

tests/Controller/ProxyControllerTest.php

@@ -21,11 +21,17 @@
 
 namespace OCA\Mail\Tests\Controller;
 
+use ArrayAccess;
 use ChristophWurst\Nextcloud\Testing\TestCase;
 use Exception;
 use OCA\Mail\Controller\ProxyController;
 use OCA\Mail\Http\ProxyDownloadResponse;
 use OCP\AppFramework\Http\TemplateResponse;
+use OCP\Http\Client\IClientService;
+use OCP\Http\Client\IResponse;
+use OCP\IRequest;
+use OCP\ISession;
+use OCP\IURLGenerator;
 
 class ProxyControllerTest extends TestCase {
 
@@ -41,39 +47,43 @@ protected function setUp() {
 		parent::setUp();
 
 		$this->appName = 'mail';
-		$this->request = $this->getMockBuilder('\OCP\IRequest')
-			->disableOriginalConstructor()
-			->getMock();
-		$this->urlGenerator = $this->getMockBuilder('\OCP\IURLGenerator')
-			->disableOriginalConstructor()
-			->getMock();
-		$this->session = $this->getMockBuilder('\OCP\ISession')
-			->disableOriginalConstructor()
-			->getMock();
-		$this->clientService = $this->getMockBuilder('\OCP\Http\Client\IClientService')->getMock();
+		$this->request = $this->createMock(IRequest::class);
+		$this->urlGenerator = $this->createMock(IURLGenerator::class);
+		$this->session = $this->createMock(ISession::class);
+		$this->clientService = $this->createMock(IClientService::class);
 		$this->hostname = 'example.com';
 	}
 
 	public function redirectDataProvider() {
 		return [
 			[
 				'http://nextcloud.com',
+				'http://anotherhostname.com',
 				false
 			],
 			[
 				'https://nextcloud.com',
+				'http://anotherhostname.com',
 				false
 			],
+			[
+				'http://nextcloud.com',
+				'https://example.com',
+				true
+			],
 			[
 				'http://example.com',
+				'https://example.com',
 				true
 			],
 			[
+				'https://example.com',
 				'https://example.com',
 				true
 			],
 			[
 				'ftp://example.com',
+				'https://example.com',
 				true
 			],
 		];
@@ -82,22 +92,36 @@ public function redirectDataProvider() {
 	/**
 	 * @dataProvider redirectDataProvider
 	 */
-	public function testRedirect($url, $authorized) {
+	public function testRedirect(string $url,
+								 string $referrer,
+								 bool $authorized) {
 		$this->urlGenerator->expects($this->once())
 			->method('linkToRoute')
 			->with('mail.page.index')
 			->will($this->returnValue('mail-route'));
-		$this->controller = new ProxyController($this->appName, $this->request,
-			$this->urlGenerator, $this->session, $this->clientService, $url,
-			'example.com');
-
-		$expected = new TemplateResponse($this->appName, 'redirect',
+		$this->request->server = [
+			'HTTP_REFERER' => $referrer,
+		];
+		$this->controller = new ProxyController(
+			$this->appName,
+			$this->request,
+			$this->urlGenerator,
+			$this->session,
+			$this->clientService,
+			'example.com'
+		);
+		$expected = new TemplateResponse(
+			$this->appName,
+			'redirect',
 			[
-			'authorizedRedirect' => $authorized,
-			'url' => $url,
-			'urlHost' => parse_url($url, PHP_URL_HOST),
-			'mailURL' => 'mail-route'
-			], 'guest');
+				'authorizedRedirect' => $authorized,
+				'url' => $url,
+				'urlHost' => parse_url($url, PHP_URL_HOST),
+				'mailURL' => 'mail-route'
+			],
+			'guest'
+		);
+
 		$response = $this->controller->redirect($url);
 
 		$this->assertEquals($expected, $response);
@@ -107,14 +131,20 @@ public function testRedirect($url, $authorized) {
 	 * @expectedException Exception
 	 */
 	public function testRedirectInvalidUrl() {
-		$this->controller = new ProxyController($this->appName, $this->request,
-			$this->urlGenerator, $this->session, $this->clientService, '', '');
+		$this->controller = new ProxyController(
+			$this->appName,
+			$this->request,
+			$this->urlGenerator,
+			$this->session,
+			$this->clientService,
+			''
+		);
 		$this->controller->redirect('ftps://example.com');
 	}
 
 	public function testProxy() {
 		$src = 'http://example.com';
-		$httpResponse = $this->getMockBuilder('\OCP\Http\Client\IResponse')->getMock();
+		$httpResponse = $this->createMock(IResponse::class);
 		$content = '🐵🐵🐵';
 
 		$this->session->expects($this->once())
@@ -131,10 +161,20 @@ public function testProxy() {
 			->method('getBody')
 			->will($this->returnValue($content));
 
-		$expected = new ProxyDownloadResponse($content, $src,
-			'application/octet-stream');
-		$this->controller = new ProxyController($this->appName, $this->request,
-			$this->urlGenerator, $this->session, $this->clientService, '', '');
+		$expected = new ProxyDownloadResponse(
+			$content,
+			$src,
+			'application/octet-stream'
+		);
+		$this->controller = new ProxyController(
+			$this->appName,
+			$this->request,
+			$this->urlGenerator,
+			$this->session,
+			$this->clientService,
+			''
+		);
+
 		$response = $this->controller->proxy($src);
 
 		$this->assertEquals($expected, $response);