diff --git a/lib/private/AppFramework/DependencyInjection/DIContainer.php b/lib/private/AppFramework/DependencyInjection/DIContainer.php index e5c84b8e1771..cd2d2c5c349b 100644 --- a/lib/private/AppFramework/DependencyInjection/DIContainer.php +++ b/lib/private/AppFramework/DependencyInjection/DIContainer.php @@ -353,8 +353,8 @@ public function __construct($appName, $urlParams = []){ $app->getServer()->getNavigationManager(), $app->getServer()->getURLGenerator(), $app->getServer()->getLogger(), + $app->getServer()->getUserSession(), $c['AppName'], - $app->isLoggedIn(), $app->isAdminUser(), $app->getServer()->getContentSecurityPolicyManager() ); diff --git a/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php b/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php index a9a3dc6a44c9..a9a25390c157 100644 --- a/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php +++ b/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php @@ -46,6 +46,7 @@ use OCP\IRequest; use OCP\ILogger; use OCP\AppFramework\Controller; +use OCP\IUserSession; use OCP\Util; use OC\AppFramework\Middleware\Security\Exceptions\SecurityException; @@ -69,11 +70,11 @@ class SecurityMiddleware extends Middleware { /** @var ILogger */ private $logger; /** @var bool */ - private $isLoggedIn; - /** @var bool */ private $isAdminUser; /** @var ContentSecurityPolicyManager */ private $contentSecurityPolicyManager; + /** @var IUserSession */ + private $session; /** * @param IRequest $request @@ -81,8 +82,8 @@ class SecurityMiddleware extends Middleware { * @param INavigationManager $navigationManager * @param IURLGenerator $urlGenerator * @param ILogger $logger + * @param IUserSession $session * @param string $appName - * @param bool $isLoggedIn * @param bool $isAdminUser * @param ContentSecurityPolicyManager $contentSecurityPolicyManager */ @@ -91,8 +92,8 @@ public function __construct(IRequest $request, INavigationManager $navigationManager, IURLGenerator $urlGenerator, ILogger $logger, + IUserSession $session, $appName, - $isLoggedIn, $isAdminUser, ContentSecurityPolicyManager $contentSecurityPolicyManager) { $this->navigationManager = $navigationManager; @@ -101,7 +102,7 @@ public function __construct(IRequest $request, $this->appName = $appName; $this->urlGenerator = $urlGenerator; $this->logger = $logger; - $this->isLoggedIn = $isLoggedIn; + $this->session = $session; $this->isAdminUser = $isAdminUser; $this->contentSecurityPolicyManager = $contentSecurityPolicyManager; } @@ -124,7 +125,7 @@ public function beforeController($controller, $methodName) { // security checks $isPublicPage = $this->reflector->hasAnnotation('PublicPage'); if(!$isPublicPage) { - if(!$this->isLoggedIn) { + if(!$this->isLoggedIn()) { throw new NotLoggedInException(); } @@ -165,7 +166,7 @@ public function beforeController($controller, $methodName) { * @return Response */ public function afterController($controller, $methodName, Response $response) { - $policy = !is_null($response->getContentSecurityPolicy()) ? $response->getContentSecurityPolicy() : new ContentSecurityPolicy(); + $policy = $response->getContentSecurityPolicy() !== null ? $response->getContentSecurityPolicy() : new ContentSecurityPolicy(); $defaultPolicy = $this->contentSecurityPolicyManager->getDefaultPolicy(); $defaultPolicy = $this->contentSecurityPolicyManager->mergePolicies($defaultPolicy, $policy); @@ -229,4 +230,9 @@ public function afterException($controller, $methodName, \Exception $exception) throw $exception; } + private function isLoggedIn() { +// \OC::handleLogin($this->request); + return $this->session->isLoggedIn(); + } + } diff --git a/tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php b/tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php index 06e0f59f145d..405e5872f170 100644 --- a/tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php +++ b/tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php @@ -39,6 +39,9 @@ use OCP\AppFramework\Http\RedirectResponse; use OCP\AppFramework\Http\TemplateResponse; use OCP\ISession; +use OCP\AppFramework\Controller; +use OCP\IUser; +use OCP\IUserSession; class SecurityMiddlewareTest extends \Test\TestCase { @@ -53,11 +56,13 @@ class SecurityMiddlewareTest extends \Test\TestCase { private $navigationManager; private $urlGenerator; private $contentSecurityPolicyManager; + /** @var IUserSession | \PHPUnit_Framework_MockObject_MockObject */ + private $session; protected function setUp() { parent::setUp(); - $this->controller = $this->getMockBuilder('OCP\AppFramework\Controller') + $this->controller = $this->getMockBuilder(Controller::class) ->disableOriginalConstructor() ->getMock(); $this->reader = new ControllerMethodReflector(); @@ -92,14 +97,16 @@ protected function setUp() { * @return SecurityMiddleware */ private function getMiddleware($isLoggedIn, $isAdminUser) { + $this->session = $this->createMock(IUserSession::class); + $this->session->expects($this->any())->method('isLoggedIn')->willReturn($isLoggedIn); return new SecurityMiddleware( $this->request, $this->reader, $this->navigationManager, $this->urlGenerator, $this->logger, + $this->session, 'files', - $isLoggedIn, $isAdminUser, $this->contentSecurityPolicyManager );