Merge pull request #30421 from owncloud/kill-base.php-handleLogin

No need to handle authentication that early aka in here at all
owncloud · Mar 19, 2018 · 580d2fd · 580d2fd
2 parents 420fd3d + ae97bff
commit 580d2fd
Show file tree

Hide file tree

Showing 6 changed files with 115 additions and 30 deletions.
diff --git a/lib/base.php b/lib/base.php
@@ -894,7 +894,6 @@ public static function handleRequest() {
 			} else {
 				// For guests: Load only filesystem and logging
 				OC_App::loadApps(['filesystem', 'logging']);
-				self::handleLogin($request);
 			}
 		}
 

diff --git a/lib/private/AppFramework/DependencyInjection/DIContainer.php b/lib/private/AppFramework/DependencyInjection/DIContainer.php
@@ -353,8 +353,8 @@ public function __construct($appName, $urlParams = []){
 				$app->getServer()->getNavigationManager(),
 				$app->getServer()->getURLGenerator(),
 				$app->getServer()->getLogger(),
+				$app->getServer()->getUserSession(),
 				$c['AppName'],
-				$app->isLoggedIn(),
 				$app->isAdminUser(),
 				$app->getServer()->getContentSecurityPolicyManager()
 			);

diff --git a/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php b/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php
@@ -46,6 +46,7 @@
 use OCP\IRequest;
 use OCP\ILogger;
 use OCP\AppFramework\Controller;
+use OCP\IUserSession;
 use OCP\Util;
 use OC\AppFramework\Middleware\Security\Exceptions\SecurityException;
 
@@ -69,20 +70,20 @@ class SecurityMiddleware extends Middleware {
 	/** @var ILogger */
 	private $logger;
 	/** @var bool */
-	private $isLoggedIn;
-	/** @var bool */
 	private $isAdminUser;
 	/** @var ContentSecurityPolicyManager */
 	private $contentSecurityPolicyManager;
+	/** @var IUserSession */
+	private $session;
 
 	/**
 	 * @param IRequest $request
 	 * @param ControllerMethodReflector $reflector
 	 * @param INavigationManager $navigationManager
 	 * @param IURLGenerator $urlGenerator
 	 * @param ILogger $logger
+	 * @param IUserSession $session
 	 * @param string $appName
-	 * @param bool $isLoggedIn
 	 * @param bool $isAdminUser
 	 * @param ContentSecurityPolicyManager $contentSecurityPolicyManager
 	 */
@@ -91,8 +92,8 @@ public function __construct(IRequest $request,
 								INavigationManager $navigationManager,
 								IURLGenerator $urlGenerator,
 								ILogger $logger,
+								IUserSession $session,
 								$appName,
-								$isLoggedIn,
 								$isAdminUser,
 								ContentSecurityPolicyManager $contentSecurityPolicyManager) {
 		$this->navigationManager = $navigationManager;
@@ -101,7 +102,7 @@ public function __construct(IRequest $request,
 		$this->appName = $appName;
 		$this->urlGenerator = $urlGenerator;
 		$this->logger = $logger;
-		$this->isLoggedIn = $isLoggedIn;
+		$this->session = $session;
 		$this->isAdminUser = $isAdminUser;
 		$this->contentSecurityPolicyManager = $contentSecurityPolicyManager;
 	}
@@ -124,7 +125,7 @@ public function beforeController($controller, $methodName) {
 		// security checks
 		$isPublicPage = $this->reflector->hasAnnotation('PublicPage');
 		if(!$isPublicPage) {
-			if(!$this->isLoggedIn) {
+			if(!$this->isLoggedIn()) {
 				throw new NotLoggedInException();
 			}
 
@@ -165,7 +166,7 @@ public function beforeController($controller, $methodName) {
 	 * @return Response
 	 */
 	public function afterController($controller, $methodName, Response $response) {
-		$policy = !is_null($response->getContentSecurityPolicy()) ? $response->getContentSecurityPolicy() : new ContentSecurityPolicy();
+		$policy = $response->getContentSecurityPolicy() !== null ? $response->getContentSecurityPolicy() : new ContentSecurityPolicy();
 
 		$defaultPolicy = $this->contentSecurityPolicyManager->getDefaultPolicy();
 		$defaultPolicy = $this->contentSecurityPolicyManager->mergePolicies($defaultPolicy, $policy);
@@ -229,4 +230,13 @@ public function afterException($controller, $methodName, \Exception $exception)
 		throw $exception;
 	}
 
+	private function isLoggedIn() {
+		static $loginCalled = false;
+		if (!$loginCalled && !$this->session->isLoggedIn()) {
+			\OC::handleLogin($this->request);
+			$loginCalled = true;
+		}
+		return $this->session->isLoggedIn();
+	}
+
 }
diff --git a/lib/private/AppFramework/Utility/ControllerMethodReflector.php b/lib/private/AppFramework/Utility/ControllerMethodReflector.php
@@ -46,8 +46,9 @@ public function __construct() {
 
 
 	/**
-	 * @param object $object an object or classname
+	 * @param object|string $object an object or classname
 	 * @param string $method the method which we want to inspect
+	 * @throws \ReflectionException
 	 */
 	public function reflect($object, $method){
 		$reflection = new \ReflectionMethod($object, $method);

diff --git a/lib/private/legacy/json.php b/lib/private/legacy/json.php
@@ -67,6 +67,12 @@ public static function checkAppEnabled($app) {
 	 * @deprecated Use annotation based ACLs from the AppFramework instead
 	 */
 	public static function checkLoggedIn() {
+		static $loginCalled = false;
+		if (!$loginCalled && !OC_User::isLoggedIn()) {
+			\OC::handleLogin(\OC::$server->getRequest());
+			$loginCalled = true;
+		}
+
 		$twoFactorAuthManger = \OC::$server->getTwoFactorAuthManager();
 		if( !OC_User::isLoggedIn()
 			|| $twoFactorAuthManger->needsSecondFactor()) {