summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php46
-rw-r--r--tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php88
2 files changed, 96 insertions, 38 deletions
diff --git a/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php b/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php
index bb3083c835c..7eb730ac2a3 100644
--- a/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php
+++ b/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php
@@ -1,4 +1,5 @@
<?php
+declare(strict_types=1);
/**
* @copyright Copyright (c) 2016, ownCloud, Inc.
*
@@ -39,6 +40,7 @@ use OC\AppFramework\Utility\ControllerMethodReflector;
use OC\Security\CSP\ContentSecurityPolicyManager;
use OC\Security\CSP\ContentSecurityPolicyNonceManager;
use OC\Security\CSRF\CsrfTokenManager;
+use OCP\App\AppPathNotFoundException;
use OCP\App\IAppManager;
use OCP\AppFramework\Http\ContentSecurityPolicy;
use OCP\AppFramework\Http\EmptyContentSecurityPolicy;
@@ -91,29 +93,14 @@ class SecurityMiddleware extends Middleware {
/** @var IL10N */
private $l10n;
- /**
- * @param IRequest $request
- * @param ControllerMethodReflector $reflector
- * @param INavigationManager $navigationManager
- * @param IURLGenerator $urlGenerator
- * @param ILogger $logger
- * @param string $appName
- * @param bool $isLoggedIn
- * @param bool $isAdminUser
- * @param ContentSecurityPolicyManager $contentSecurityPolicyManager
- * @param CSRFTokenManager $csrfTokenManager
- * @param ContentSecurityPolicyNonceManager $cspNonceManager
- * @param IAppManager $appManager
- * @param IL10N $l10n
- */
public function __construct(IRequest $request,
ControllerMethodReflector $reflector,
INavigationManager $navigationManager,
IURLGenerator $urlGenerator,
ILogger $logger,
- $appName,
- $isLoggedIn,
- $isAdminUser,
+ string $appName,
+ bool $isLoggedIn,
+ bool $isAdminUser,
ContentSecurityPolicyManager $contentSecurityPolicyManager,
CsrfTokenManager $csrfTokenManager,
ContentSecurityPolicyNonceManager $cspNonceManager,
@@ -156,10 +143,8 @@ class SecurityMiddleware extends Middleware {
throw new NotLoggedInException();
}
- if(!$this->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 @@ class SecurityMiddleware extends Middleware {
}
/**
- * 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 @@ class SecurityMiddleware extends Middleware {
* @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 @@ class SecurityMiddleware extends Middleware {
* @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 a631fe59a60..f51f7e9a1c6 100644
--- a/tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php
+++ b/tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php
@@ -95,22 +95,19 @@ class SecurityMiddlewareTest extends \Test\TestCase {
$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 @@ class SecurityMiddlewareTest extends \Test\TestCase {
$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__);
+ }
}
generated by cgit v1.2.3 (git 2.39.1) at 2025-01-24 16:30:18 +0000