diff --git a/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php b/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php index bb3083c835c4f..7eb730ac2a3f4 100644 --- a/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php +++ b/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php @@ -1,4 +1,5 @@ reflector->hasAnnotation('NoAdminRequired')) { - if(!$this->isAdminUser) { - throw new NotAdminException($this->l10n->t('Logged in user must be an admin')); - } + if(!$this->reflector->hasAnnotation('NoAdminRequired') && !$this->isAdminUser) { + throw new NotAdminException($this->l10n->t('Logged in user must be an admin')); } } @@ -191,15 +176,20 @@ public function beforeController($controller, $methodName) { } /** - * FIXME: Use DI once available * Checks if app is enabled (also includes a check whether user is allowed to access the resource) * The getAppPath() check is here since components such as settings also use the AppFramework and * therefore won't pass this check. + * If page is public, app does not need to be enabled for current user/visitor */ - if(\OC_App::getAppPath($this->appName) !== false && !$this->appManager->isEnabledForUser($this->appName)) { - throw new AppNotEnabledException(); + try { + $appPath = $this->appManager->getAppPath($this->appName); + } catch (AppPathNotFoundException $e) { + $appPath = false; } + if ($appPath !== false && !$isPublicPage && !$this->appManager->isEnabledForUser($this->appName)) { + throw new AppNotEnabledException(); + } } /** @@ -211,7 +201,7 @@ public function beforeController($controller, $methodName) { * @param Response $response * @return Response */ - public function afterController($controller, $methodName, Response $response) { + public function afterController($controller, $methodName, Response $response): Response { $policy = !is_null($response->getContentSecurityPolicy()) ? $response->getContentSecurityPolicy() : new ContentSecurityPolicy(); if (get_class($policy) === EmptyContentSecurityPolicy::class) { @@ -240,14 +230,14 @@ public function afterController($controller, $methodName, Response $response) { * @throws \Exception the passed in exception if it can't handle it * @return Response a Response object or null in case that the exception could not be handled */ - public function afterException($controller, $methodName, \Exception $exception) { + public function afterException($controller, $methodName, \Exception $exception): Response { if($exception instanceof SecurityException) { if($exception instanceof StrictCookieMissingException) { return new RedirectResponse(\OC::$WEBROOT); } if (stripos($this->request->getHeader('Accept'),'html') === false) { $response = new JSONResponse( - array('message' => $exception->getMessage()), + ['message' => $exception->getMessage()], $exception->getCode() ); } else { diff --git a/tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php b/tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php index a631fe59a6007..f51f7e9a1c60e 100644 --- a/tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php +++ b/tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php @@ -95,22 +95,19 @@ protected function setUp() { $this->contentSecurityPolicyManager = $this->createMock(ContentSecurityPolicyManager::class); $this->csrfTokenManager = $this->createMock(CsrfTokenManager::class); $this->cspNonceManager = $this->createMock(ContentSecurityPolicyNonceManager::class); - $this->appManager = $this->createMock(IAppManager::class); $this->l10n = $this->createMock(IL10N::class); - $this->appManager->expects($this->any()) - ->method('isEnabledForUser') - ->willReturn(true); $this->middleware = $this->getMiddleware(true, true); $this->secException = new SecurityException('hey', false); $this->secAjaxException = new SecurityException('hey', true); } - /** - * @param bool $isLoggedIn - * @param bool $isAdminUser - * @return SecurityMiddleware - */ - private function getMiddleware($isLoggedIn, $isAdminUser) { + private function getMiddleware(bool $isLoggedIn, bool $isAdminUser, bool $isAppEnabledForUser = true): SecurityMiddleware { + + $this->appManager = $this->createMock(IAppManager::class); + $this->appManager->expects($this->any()) + ->method('isEnabledForUser') + ->willReturn($isAppEnabledForUser); + return new SecurityMiddleware( $this->request, $this->reader, @@ -667,4 +664,75 @@ public function testAfterControllerWithContentSecurityPolicy3Support() { $this->assertEquals($response, $this->middleware->afterController($this->controller, 'test', $response)); } + + public function dataRestrictedApp() { + return [ + [false, false, false,], + [false, false, true,], + [false, true, false,], + [false, true, true,], + [ true, false, false,], + [ true, false, true,], + [ true, true, false,], + [ true, true, true,], + ]; + } + + /** + * @PublicPage + * @NoAdminRequired + * @NoCSRFRequired + */ + public function testRestrictedAppLoggedInPublicPage() { + $middleware = $this->getMiddleware(true, false); + $this->reader->reflect(__CLASS__,__FUNCTION__); + + $this->appManager->method('getAppPath') + ->with('files') + ->willReturn('foo'); + + $this->appManager->method('isEnabledForUser') + ->with('files') + ->willReturn(false); + + $middleware->beforeController($this->controller, __FUNCTION__); + $this->addToAssertionCount(1); + } + + /** + * @PublicPage + * @NoAdminRequired + * @NoCSRFRequired + */ + public function testRestrictedAppNotLoggedInPublicPage() { + $middleware = $this->getMiddleware(false, false); + $this->reader->reflect(__CLASS__,__FUNCTION__); + + $this->appManager->method('getAppPath') + ->with('files') + ->willReturn('foo'); + + $this->appManager->method('isEnabledForUser') + ->with('files') + ->willReturn(false); + + $middleware->beforeController($this->controller, __FUNCTION__); + $this->addToAssertionCount(1); + } + + /** + * @NoAdminRequired + * @NoCSRFRequired + */ + public function testRestrictedAppLoggedIn() { + $middleware = $this->getMiddleware(true, false, false); + $this->reader->reflect(__CLASS__,__FUNCTION__); + + $this->appManager->method('getAppPath') + ->with('files') + ->willReturn('foo'); + + $this->expectException(AppNotEnabledException::class); + $middleware->beforeController($this->controller, __FUNCTION__); + } }