Merge pull request #6921 from nextcloud/appmanager-securitymiddleware

Use proper DI for security middleware for app enabled check
This commit is contained in:
Roeland Jago Douma 2017-10-24 19:58:24 +02:00 committed by GitHub
commit b88db3a389
3 changed files with 19 additions and 4 deletions

View File

@ -230,7 +230,8 @@ class DIContainer extends SimpleContainer implements IAppContainer {
$app->isAdminUser(), $app->isAdminUser(),
$server->getContentSecurityPolicyManager(), $server->getContentSecurityPolicyManager(),
$server->getCsrfTokenManager(), $server->getCsrfTokenManager(),
$server->getContentSecurityPolicyNonceManager() $server->getContentSecurityPolicyNonceManager(),
$server->getAppManager()
); );
}); });

View File

@ -39,6 +39,7 @@ use OC\AppFramework\Utility\ControllerMethodReflector;
use OC\Security\CSP\ContentSecurityPolicyManager; use OC\Security\CSP\ContentSecurityPolicyManager;
use OC\Security\CSP\ContentSecurityPolicyNonceManager; use OC\Security\CSP\ContentSecurityPolicyNonceManager;
use OC\Security\CSRF\CsrfTokenManager; use OC\Security\CSRF\CsrfTokenManager;
use OCP\App\IAppManager;
use OCP\AppFramework\Http\ContentSecurityPolicy; use OCP\AppFramework\Http\ContentSecurityPolicy;
use OCP\AppFramework\Http\EmptyContentSecurityPolicy; use OCP\AppFramework\Http\EmptyContentSecurityPolicy;
use OCP\AppFramework\Http\RedirectResponse; use OCP\AppFramework\Http\RedirectResponse;
@ -87,6 +88,8 @@ class SecurityMiddleware extends Middleware {
private $csrfTokenManager; private $csrfTokenManager;
/** @var ContentSecurityPolicyNonceManager */ /** @var ContentSecurityPolicyNonceManager */
private $cspNonceManager; private $cspNonceManager;
/** @var IAppManager */
private $appManager;
/** /**
* @param IRequest $request * @param IRequest $request
@ -101,6 +104,7 @@ class SecurityMiddleware extends Middleware {
* @param ContentSecurityPolicyManager $contentSecurityPolicyManager * @param ContentSecurityPolicyManager $contentSecurityPolicyManager
* @param CSRFTokenManager $csrfTokenManager * @param CSRFTokenManager $csrfTokenManager
* @param ContentSecurityPolicyNonceManager $cspNonceManager * @param ContentSecurityPolicyNonceManager $cspNonceManager
* @param IAppManager $appManager
*/ */
public function __construct(IRequest $request, public function __construct(IRequest $request,
ControllerMethodReflector $reflector, ControllerMethodReflector $reflector,
@ -113,7 +117,8 @@ class SecurityMiddleware extends Middleware {
$isAdminUser, $isAdminUser,
ContentSecurityPolicyManager $contentSecurityPolicyManager, ContentSecurityPolicyManager $contentSecurityPolicyManager,
CsrfTokenManager $csrfTokenManager, CsrfTokenManager $csrfTokenManager,
ContentSecurityPolicyNonceManager $cspNonceManager) { ContentSecurityPolicyNonceManager $cspNonceManager,
IAppManager $appManager) {
$this->navigationManager = $navigationManager; $this->navigationManager = $navigationManager;
$this->request = $request; $this->request = $request;
$this->reflector = $reflector; $this->reflector = $reflector;
@ -126,6 +131,7 @@ class SecurityMiddleware extends Middleware {
$this->contentSecurityPolicyManager = $contentSecurityPolicyManager; $this->contentSecurityPolicyManager = $contentSecurityPolicyManager;
$this->csrfTokenManager = $csrfTokenManager; $this->csrfTokenManager = $csrfTokenManager;
$this->cspNonceManager = $cspNonceManager; $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 * The getAppPath() check is here since components such as settings also use the AppFramework and
* therefore won't pass this check. * 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(); throw new AppNotEnabledException();
} }

View File

@ -37,6 +37,7 @@ use OC\Security\CSP\ContentSecurityPolicyManager;
use OC\Security\CSP\ContentSecurityPolicyNonceManager; use OC\Security\CSP\ContentSecurityPolicyNonceManager;
use OC\Security\CSRF\CsrfToken; use OC\Security\CSRF\CsrfToken;
use OC\Security\CSRF\CsrfTokenManager; use OC\Security\CSRF\CsrfTokenManager;
use OCP\App\IAppManager;
use OCP\AppFramework\Controller; use OCP\AppFramework\Controller;
use OCP\AppFramework\Http\EmptyContentSecurityPolicy; use OCP\AppFramework\Http\EmptyContentSecurityPolicy;
use OCP\AppFramework\Http\RedirectResponse; use OCP\AppFramework\Http\RedirectResponse;
@ -79,6 +80,8 @@ class SecurityMiddlewareTest extends \Test\TestCase {
private $csrfTokenManager; private $csrfTokenManager;
/** @var ContentSecurityPolicyNonceManager|\PHPUnit_Framework_MockObject_MockObject */ /** @var ContentSecurityPolicyNonceManager|\PHPUnit_Framework_MockObject_MockObject */
private $cspNonceManager; private $cspNonceManager;
/** @var IAppManager|\PHPUnit_Framework_MockObject_MockObject */
private $appManager;
protected function setUp() { protected function setUp() {
parent::setUp(); parent::setUp();
@ -93,6 +96,10 @@ class SecurityMiddlewareTest extends \Test\TestCase {
$this->contentSecurityPolicyManager = $this->createMock(ContentSecurityPolicyManager::class); $this->contentSecurityPolicyManager = $this->createMock(ContentSecurityPolicyManager::class);
$this->csrfTokenManager = $this->createMock(CsrfTokenManager::class); $this->csrfTokenManager = $this->createMock(CsrfTokenManager::class);
$this->cspNonceManager = $this->createMock(ContentSecurityPolicyNonceManager::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->middleware = $this->getMiddleware(true, true);
$this->secException = new SecurityException('hey', false); $this->secException = new SecurityException('hey', false);
$this->secAjaxException = new SecurityException('hey', true); $this->secAjaxException = new SecurityException('hey', true);
@ -116,7 +123,8 @@ class SecurityMiddlewareTest extends \Test\TestCase {
$isAdminUser, $isAdminUser,
$this->contentSecurityPolicyManager, $this->contentSecurityPolicyManager,
$this->csrfTokenManager, $this->csrfTokenManager,
$this->cspNonceManager $this->cspNonceManager,
$this->appManager
); );
} }