From ce0c45a4eabbe622500a4b621ccc4393720fc5ad Mon Sep 17 00:00:00 2001 From: Morris Jobke Date: Mon, 23 Oct 2017 23:40:17 +0200 Subject: [PATCH] Use proper DI for security middleware for app enabled check Signed-off-by: Morris Jobke --- .../AppFramework/DependencyInjection/DIContainer.php | 3 ++- .../Middleware/Security/SecurityMiddleware.php | 10 ++++++++-- .../Middleware/Security/SecurityMiddlewareTest.php | 10 +++++++++- 3 files changed, 19 insertions(+), 4 deletions(-) diff --git a/lib/private/AppFramework/DependencyInjection/DIContainer.php b/lib/private/AppFramework/DependencyInjection/DIContainer.php index 2290f0d004..0ea7eed4ae 100644 --- a/lib/private/AppFramework/DependencyInjection/DIContainer.php +++ b/lib/private/AppFramework/DependencyInjection/DIContainer.php @@ -230,7 +230,8 @@ class DIContainer extends SimpleContainer implements IAppContainer { $app->isAdminUser(), $server->getContentSecurityPolicyManager(), $server->getCsrfTokenManager(), - $server->getContentSecurityPolicyNonceManager() + $server->getContentSecurityPolicyNonceManager(), + $server->getAppManager() ); }); diff --git a/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php b/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php index 4e41c94643..5200498790 100644 --- a/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php +++ b/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php @@ -39,6 +39,7 @@ use OC\AppFramework\Utility\ControllerMethodReflector; use OC\Security\CSP\ContentSecurityPolicyManager; use OC\Security\CSP\ContentSecurityPolicyNonceManager; use OC\Security\CSRF\CsrfTokenManager; +use OCP\App\IAppManager; use OCP\AppFramework\Http\ContentSecurityPolicy; use OCP\AppFramework\Http\EmptyContentSecurityPolicy; use OCP\AppFramework\Http\RedirectResponse; @@ -87,6 +88,8 @@ class SecurityMiddleware extends Middleware { private $csrfTokenManager; /** @var ContentSecurityPolicyNonceManager */ private $cspNonceManager; + /** @var IAppManager */ + private $appManager; /** * @param IRequest $request @@ -101,6 +104,7 @@ class SecurityMiddleware extends Middleware { * @param ContentSecurityPolicyManager $contentSecurityPolicyManager * @param CSRFTokenManager $csrfTokenManager * @param ContentSecurityPolicyNonceManager $cspNonceManager + * @param IAppManager $appManager */ public function __construct(IRequest $request, ControllerMethodReflector $reflector, @@ -113,7 +117,8 @@ class SecurityMiddleware extends Middleware { $isAdminUser, ContentSecurityPolicyManager $contentSecurityPolicyManager, CsrfTokenManager $csrfTokenManager, - ContentSecurityPolicyNonceManager $cspNonceManager) { + ContentSecurityPolicyNonceManager $cspNonceManager, + IAppManager $appManager) { $this->navigationManager = $navigationManager; $this->request = $request; $this->reflector = $reflector; @@ -126,6 +131,7 @@ class SecurityMiddleware extends Middleware { $this->contentSecurityPolicyManager = $contentSecurityPolicyManager; $this->csrfTokenManager = $csrfTokenManager; $this->cspNonceManager = $cspNonceManager; + $this->appManager = $appManager; } /** @@ -190,7 +196,7 @@ class SecurityMiddleware extends Middleware { * The getAppPath() check is here since components such as settings also use the AppFramework and * therefore won't pass this check. */ - if(\OC_App::getAppPath($this->appName) !== false && !\OC_App::isEnabled($this->appName)) { + if(\OC_App::getAppPath($this->appName) !== false && !$this->appManager->isEnabledForUser($this->appName)) { throw new AppNotEnabledException(); } diff --git a/tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php b/tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php index 773cb2b196..6b311c7ae1 100644 --- a/tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php +++ b/tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php @@ -37,6 +37,7 @@ use OC\Security\CSP\ContentSecurityPolicyManager; use OC\Security\CSP\ContentSecurityPolicyNonceManager; use OC\Security\CSRF\CsrfToken; use OC\Security\CSRF\CsrfTokenManager; +use OCP\App\IAppManager; use OCP\AppFramework\Controller; use OCP\AppFramework\Http\EmptyContentSecurityPolicy; use OCP\AppFramework\Http\RedirectResponse; @@ -79,6 +80,8 @@ class SecurityMiddlewareTest extends \Test\TestCase { private $csrfTokenManager; /** @var ContentSecurityPolicyNonceManager|\PHPUnit_Framework_MockObject_MockObject */ private $cspNonceManager; + /** @var IAppManager|\PHPUnit_Framework_MockObject_MockObject */ + private $appManager; protected function setUp() { parent::setUp(); @@ -93,6 +96,10 @@ 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->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); @@ -116,7 +123,8 @@ class SecurityMiddlewareTest extends \Test\TestCase { $isAdminUser, $this->contentSecurityPolicyManager, $this->csrfTokenManager, - $this->cspNonceManager + $this->cspNonceManager, + $this->appManager ); }