Add tests
Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
This commit is contained in:
parent
340e8ef16c
commit
3ad7daeda5
|
@ -40,6 +40,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\AppPathNotFoundException;
|
||||||
use OCP\App\IAppManager;
|
use OCP\App\IAppManager;
|
||||||
use OCP\AppFramework\Http\ContentSecurityPolicy;
|
use OCP\AppFramework\Http\ContentSecurityPolicy;
|
||||||
use OCP\AppFramework\Http\EmptyContentSecurityPolicy;
|
use OCP\AppFramework\Http\EmptyContentSecurityPolicy;
|
||||||
|
@ -92,21 +93,6 @@ class SecurityMiddleware extends Middleware {
|
||||||
/** @var IL10N */
|
/** @var IL10N */
|
||||||
private $l10n;
|
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,
|
public function __construct(IRequest $request,
|
||||||
ControllerMethodReflector $reflector,
|
ControllerMethodReflector $reflector,
|
||||||
INavigationManager $navigationManager,
|
INavigationManager $navigationManager,
|
||||||
|
@ -190,16 +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)
|
* 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
|
* 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 page is public, app does not need to be enabled for current user/visitor
|
* If page is public, app does not need to be enabled for current user/visitor
|
||||||
*/
|
*/
|
||||||
if(\OC_App::getAppPath($this->appName) !== false && !$isPublicPage && !$this->appManager->isEnabledForUser($this->appName)) {
|
try {
|
||||||
throw new AppNotEnabledException();
|
$appPath = $this->appManager->getAppPath($this->appName);
|
||||||
|
} catch (AppPathNotFoundException $e) {
|
||||||
|
$appPath = false;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if ($appPath !== false && !$isPublicPage && !$this->appManager->isEnabledForUser($this->appName)) {
|
||||||
|
throw new AppNotEnabledException();
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|
|
@ -95,22 +95,19 @@ 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->l10n = $this->createMock(IL10N::class);
|
$this->l10n = $this->createMock(IL10N::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);
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
private function getMiddleware(bool $isLoggedIn, bool $isAdminUser, bool $isAppEnabledForUser = true): SecurityMiddleware {
|
||||||
* @param bool $isLoggedIn
|
|
||||||
* @param bool $isAdminUser
|
$this->appManager = $this->createMock(IAppManager::class);
|
||||||
* @return SecurityMiddleware
|
$this->appManager->expects($this->any())
|
||||||
*/
|
->method('isEnabledForUser')
|
||||||
private function getMiddleware($isLoggedIn, $isAdminUser) {
|
->willReturn($isAppEnabledForUser);
|
||||||
|
|
||||||
return new SecurityMiddleware(
|
return new SecurityMiddleware(
|
||||||
$this->request,
|
$this->request,
|
||||||
$this->reader,
|
$this->reader,
|
||||||
|
@ -667,4 +664,75 @@ class SecurityMiddlewareTest extends \Test\TestCase {
|
||||||
|
|
||||||
$this->assertEquals($response, $this->middleware->afterController($this->controller, 'test', $response));
|
$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__);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in New Issue