Merge pull request #13969 from nextcloud/enh/additional_scripts_no_on_public_pages

No need to emit additonalscript event on public pages

core/Controller/ClientFlowLoginController.php

@@ -36,7 +36,7 @@ use OCA\OAuth2\Db\ClientMapper;
 use OCP\AppFramework\Controller;
 use OCP\AppFramework\Http;
 use OCP\AppFramework\Http\Response;
-use OCP\AppFramework\Http\TemplateResponse;
+use OCP\AppFramework\Http\StandaloneTemplateResponse;
 use OCP\Defaults;
 use OCP\IL10N;
 use OCP\IRequest;
@@ -131,10 +131,10 @@ class ClientFlowLoginController extends Controller {
 	}
 
 	/**
-	 * @return TemplateResponse
+	 * @return StandaloneTemplateResponse
 	 */
 	private function stateTokenForbiddenResponse() {
-		$response = new TemplateResponse(
+		$response = new StandaloneTemplateResponse(
 			$this->appName,
 			'403',
 			[
@@ -153,7 +153,7 @@ class ClientFlowLoginController extends Controller {
 	 *
 	 * @param string $clientIdentifier
 	 *
-	 * @return TemplateResponse
+	 * @return StandaloneTemplateResponse
 	 */
 	public function showAuthPickerPage($clientIdentifier = '') {
 		$clientName = $this->getClientName();
@@ -166,7 +166,7 @@ class ClientFlowLoginController extends Controller {
 		// No valid clientIdentifier given and no valid API Request (APIRequest header not set)
 		$clientRequest = $this->request->getHeader('OCS-APIREQUEST');
 		if ($clientRequest !== 'true' && $client === null) {
-			return new TemplateResponse(
+			return new StandaloneTemplateResponse(
 				$this->appName,
 				'error',
 				[
@@ -188,7 +188,7 @@ class ClientFlowLoginController extends Controller {
 		);
 		$this->session->set(self::stateName, $stateToken);
 
-		return new TemplateResponse(
+		return new StandaloneTemplateResponse(
 			$this->appName,
 			'loginflow/authpicker',
 			[
@@ -212,7 +212,7 @@ class ClientFlowLoginController extends Controller {
 	 *
 	 * @param string $stateToken
 	 * @param string $clientIdentifier
-	 * @return TemplateResponse
+	 * @return StandaloneTemplateResponse
 	 */
 	public function grantPage($stateToken = '',
 								 $clientIdentifier = '') {
@@ -227,7 +227,7 @@ class ClientFlowLoginController extends Controller {
 			$clientName = $client->getName();
 		}
 
-		return new TemplateResponse(
+		return new StandaloneTemplateResponse(
 			$this->appName,
 			'loginflow/grant',
 			[

core/Controller/TwoFactorChallengeController.php

@@ -31,7 +31,7 @@ use OC_User;
 use OC_Util;
 use OCP\AppFramework\Controller;
 use OCP\AppFramework\Http\RedirectResponse;
-use OCP\AppFramework\Http\TemplateResponse;
+use OCP\AppFramework\Http\StandaloneTemplateResponse;
 use OCP\Authentication\TwoFactorAuth\IProvider;
 use OCP\Authentication\TwoFactorAuth\IProvidesCustomCSP;
 use OCP\Authentication\TwoFactorAuth\TwoFactorException;
@@ -100,7 +100,7 @@ class TwoFactorChallengeController extends Controller {
 	 * @NoCSRFRequired
 	 *
 	 * @param string $redirect_url
-	 * @return TemplateResponse
+	 * @return StandaloneTemplateResponse
 	 */
 	public function selectChallenge($redirect_url) {
 		$user = $this->userSession->getUser();
@@ -115,7 +115,7 @@ class TwoFactorChallengeController extends Controller {
 			'redirect_url' => $redirect_url,
 			'logout_url' => $this->getLogoutUrl(),
 		];
-		return new TemplateResponse($this->appName, 'twofactorselectchallenge', $data, 'guest');
+		return new StandaloneTemplateResponse($this->appName, 'twofactorselectchallenge', $data, 'guest');
 	}
 
 	/**
@@ -125,7 +125,7 @@ class TwoFactorChallengeController extends Controller {
 	 *
 	 * @param string $challengeProviderId
 	 * @param string $redirect_url
-	 * @return TemplateResponse|RedirectResponse
+	 * @return StandaloneTemplateResponse|RedirectResponse
 	 */
 	public function showChallenge($challengeProviderId, $redirect_url) {
 		$user = $this->userSession->getUser();
@@ -160,7 +160,7 @@ class TwoFactorChallengeController extends Controller {
 			'redirect_url' => $redirect_url,
 			'template' => $tmpl->fetchPage(),
 		];
-		$response = new TemplateResponse($this->appName, 'twofactorshowchallenge', $data, 'guest');
+		$response = new StandaloneTemplateResponse($this->appName, 'twofactorshowchallenge', $data, 'guest');
 		if ($provider instanceof IProvidesCustomCSP) {
 			$response->setContentSecurityPolicy($provider->getCSP());
 		}

lib/composer/composer/autoload_classmap.php

@@ -45,6 +45,7 @@ return array(
     'OCP\\AppFramework\\Http\\OCSResponse' => $baseDir . '/lib/public/AppFramework/Http/OCSResponse.php',
     'OCP\\AppFramework\\Http\\RedirectResponse' => $baseDir . '/lib/public/AppFramework/Http/RedirectResponse.php',
     'OCP\\AppFramework\\Http\\Response' => $baseDir . '/lib/public/AppFramework/Http/Response.php',
+    'OCP\\AppFramework\\Http\\StandaloneTemplateResponse' => $baseDir . '/lib/public/AppFramework/Http/StandaloneTemplateResponse.php',
     'OCP\\AppFramework\\Http\\StreamResponse' => $baseDir . '/lib/public/AppFramework/Http/StreamResponse.php',
     'OCP\\AppFramework\\Http\\StrictContentSecurityPolicy' => $baseDir . '/lib/public/AppFramework/Http/StrictContentSecurityPolicy.php',
     'OCP\\AppFramework\\Http\\StrictEvalContentSecurityPolicy' => $baseDir . '/lib/public/AppFramework/Http/StrictEvalContentSecurityPolicy.php',

lib/composer/composer/autoload_static.php

@@ -75,6 +75,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c
         'OCP\\AppFramework\\Http\\OCSResponse' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Http/OCSResponse.php',
         'OCP\\AppFramework\\Http\\RedirectResponse' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Http/RedirectResponse.php',
         'OCP\\AppFramework\\Http\\Response' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Http/Response.php',
+        'OCP\\AppFramework\\Http\\StandaloneTemplateResponse' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Http/StandaloneTemplateResponse.php',
         'OCP\\AppFramework\\Http\\StreamResponse' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Http/StreamResponse.php',
         'OCP\\AppFramework\\Http\\StrictContentSecurityPolicy' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Http/StrictContentSecurityPolicy.php',
         'OCP\\AppFramework\\Http\\StrictEvalContentSecurityPolicy' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Http/StrictEvalContentSecurityPolicy.php',

lib/private/AppFramework/Middleware/AdditionalScriptsMiddleware.php

@@ -25,8 +25,10 @@ declare(strict_types=1);
 namespace OC\AppFramework\Middleware;
 
 use OCP\AppFramework\Http\Response;
+use OCP\AppFramework\Http\StandaloneTemplateResponse;
 use OCP\AppFramework\Http\TemplateResponse;
 use OCP\AppFramework\Middleware;
+use OCP\AppFramework\PublicShareController;
 use OCP\IUserSession;
 use Symfony\Component\EventDispatcher\EventDispatcherInterface;
 
@@ -42,10 +44,18 @@ class AdditionalScriptsMiddleware extends Middleware {
 	}
 
 	public function afterController($controller, $methodName, Response $response): Response {
+		/*
+		 * There is no need to emit these signals on a public share page
+		 * There is a separate event for that already
+		 */
+		if ($controller instanceof PublicShareController) {
+			return $response;
+		}
+
 		if ($response instanceof TemplateResponse) {
 			$this->dispatcher->dispatch(TemplateResponse::EVENT_LOAD_ADDITIONAL_SCRIPTS);
 
-			if ($this->userSession->isLoggedIn()) {
+			if (!($response instanceof StandaloneTemplateResponse) && $this->userSession->isLoggedIn()) {
 				$this->dispatcher->dispatch(TemplateResponse::EVENT_LOAD_ADDITIONAL_SCRIPTS_LOGGEDIN);
 			}
 		}

lib/public/AppFramework/Http/StandaloneTemplateResponse.php

@@ -0,0 +1,37 @@
+<?php
+declare(strict_types=1);
+/**
+ * @copyright Copyright (c) 2019, Roeland Jago Douma <roeland@famdouma.nl>
+ *
+ * @author Roeland Jago Douma <roeland@famdouma.nl>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+namespace OCP\AppFramework\Http;
+
+/**
+ * A template response that does not emit the loadAdditionalScripts events.
+ *
+ * This is useful for pages that are authenticated but do not yet show the
+ * full nextcloud UI. Like the 2FA page, or the grant page in the login flow.
+ *
+ * @since 16.0.0
+ */
+class StandaloneTemplateResponse extends TemplateResponse {
+
+}

tests/Core/Controller/ClientFlowLoginControllerTest.php

@@ -30,7 +30,7 @@ use OCA\OAuth2\Db\AccessTokenMapper;
 use OCA\OAuth2\Db\Client;
 use OCA\OAuth2\Db\ClientMapper;
 use OCP\AppFramework\Http;
-use OCP\AppFramework\Http\TemplateResponse;
+use OCP\AppFramework\Http\StandaloneTemplateResponse;
 use OCP\Defaults;
 use OCP\IL10N;
 use OCP\IRequest;
@@ -108,7 +108,7 @@ class ClientFlowLoginControllerTest extends TestCase {
 	}
 
 	public function testShowAuthPickerPageNoClientOrOauthRequest() {
-		$expected = new TemplateResponse(
+		$expected = new StandaloneTemplateResponse(
 			'core',
 			'error',
 			[
@@ -166,7 +166,7 @@ class ClientFlowLoginControllerTest extends TestCase {
 			->method('getServerProtocol')
 			->willReturn('https');
 
-		$expected = new TemplateResponse(
+		$expected = new StandaloneTemplateResponse(
 			'core',
 			'loginflow/authpicker',
 			[
@@ -225,7 +225,7 @@ class ClientFlowLoginControllerTest extends TestCase {
 			->method('getServerProtocol')
 			->willReturn('https');
 
-		$expected = new TemplateResponse(
+		$expected = new StandaloneTemplateResponse(
 			'core',
 			'loginflow/authpicker',
 			[
@@ -253,7 +253,7 @@ class ClientFlowLoginControllerTest extends TestCase {
 			->method('remove')
 			->with('client.flow.state.token');
 
-		$expected = new TemplateResponse(
+		$expected = new StandaloneTemplateResponse(
 			'core',
 			'403',
 			[

tests/Core/Controller/TwoFactorChallengeControllerTest.php

@@ -27,7 +27,7 @@ use OC\Authentication\TwoFactorAuth\ProviderSet;
 use OC\Core\Controller\TwoFactorChallengeController;
 use OC_Util;
 use OCP\AppFramework\Http\RedirectResponse;
-use OCP\AppFramework\Http\TemplateResponse;
+use OCP\AppFramework\Http\StandaloneTemplateResponse;
 use OCP\Authentication\TwoFactorAuth\IProvider;
 use OCP\Authentication\TwoFactorAuth\TwoFactorException;
 use OCP\IRequest;
@@ -100,7 +100,7 @@ class TwoFactorChallengeControllerTest extends TestCase {
 			->with($user)
 			->will($this->returnValue($providerSet));
 
-		$expected = new TemplateResponse('core', 'twofactorselectchallenge', [
+		$expected = new StandaloneTemplateResponse('core', 'twofactorselectchallenge', [
 			'providers' => [
 				$p1,
 			],
@@ -151,7 +151,7 @@ class TwoFactorChallengeControllerTest extends TestCase {
 			->method('fetchPage')
 			->will($this->returnValue('<html/>'));
 
-		$expected = new TemplateResponse('core', 'twofactorshowchallenge', [
+		$expected = new StandaloneTemplateResponse('core', 'twofactorshowchallenge', [
 			'error' => true,
 			'provider' => $provider,
 			'backupProvider' => $backupProvider,

tests/lib/AppFramework/Middleware/AdditionalScriptsMiddlewareTest.php

@@ -27,7 +27,9 @@ namespace Test\AppFramework\Middleware;
 use OC\AppFramework\Middleware\AdditionalScriptsMiddleware;
 use OCP\AppFramework\Controller;
 use OCP\AppFramework\Http\Response;
+use OCP\AppFramework\Http\StandaloneTemplateResponse;
 use OCP\AppFramework\Http\TemplateResponse;
+use OCP\AppFramework\PublicShareController;
 use OCP\IUserSession;
 use PHPUnit\Framework\MockObject\MockObject;
 use Symfony\Component\EventDispatcher\EventDispatcherInterface;
@@ -67,6 +69,31 @@ class AdditionalScriptsMiddlewareTest extends \Test\TestCase {
 		$this->middleWare->afterController($this->controller, 'myMethod', $this->createMock(Response::class));
 	}
 
+	public function testPublicShareController() {
+		$this->dispatcher->expects($this->never())
+			->method($this->anything());
+		$this->userSession->expects($this->never())
+			->method($this->anything());
+
+		$this->middleWare->afterController($this->createMock(PublicShareController::class), 'myMethod', $this->createMock(Response::class));
+	}
+
+	public function testStandaloneTemplateResponse() {
+		$this->dispatcher->expects($this->once())
+			->method('dispatch')
+			->willReturnCallback(function($eventName) {
+				if ($eventName === TemplateResponse::EVENT_LOAD_ADDITIONAL_SCRIPTS) {
+					return;
+				}
+
+				$this->fail('Wrong event dispatched');
+			});
+		$this->userSession->expects($this->never())
+			->method($this->anything());
+
+		$this->middleWare->afterController($this->controller, 'myMethod', $this->createMock(StandaloneTemplateResponse::class));
+	}
+
 	public function testTemplateResponseNotLoggedIn() {
 		$this->dispatcher->expects($this->once())
 			->method('dispatch')