diff options
| -rw-r--r-- | lib/private/AppFramework/Middleware/Security/CORSMiddleware.php | 2 | ||||
| -rw-r--r-- | tests/lib/AppFramework/Middleware/Security/CORSMiddlewareTest.php | 36 |
2 files changed, 36 insertions, 2 deletions
diff --git a/lib/private/AppFramework/Middleware/Security/CORSMiddleware.php b/lib/private/AppFramework/Middleware/Security/CORSMiddleware.php index 2476f4ec9b3..30ba8d8d6e4 100644 --- a/lib/private/AppFramework/Middleware/Security/CORSMiddleware.php +++ b/lib/private/AppFramework/Middleware/Security/CORSMiddleware.php @@ -83,7 +83,7 @@ class CORSMiddleware extends Middleware { public function beforeController($controller, $methodName) { // ensure that @CORS annotated API routes are not used in conjunction // with session authentication since this enables CSRF attack vectors - if ($this->reflector->hasAnnotation('CORS') && !$this->reflector->hasAnnotation('PublicPage')) { + if ($this->reflector->hasAnnotation('CORS') && (!$this->reflector->hasAnnotation('PublicPage') || $this->session->isLoggedIn())) { $user = array_key_exists('PHP_AUTH_USER', $this->request->server) ? $this->request->server['PHP_AUTH_USER'] : null; $pass = array_key_exists('PHP_AUTH_PW', $this->request->server) ? $this->request->server['PHP_AUTH_PW'] : null; diff --git a/tests/lib/AppFramework/Middleware/Security/CORSMiddlewareTest.php b/tests/lib/AppFramework/Middleware/Security/CORSMiddlewareTest.php index f3c1f7934ef..986d0e577b7 100644 --- a/tests/lib/AppFramework/Middleware/Security/CORSMiddlewareTest.php +++ b/tests/lib/AppFramework/Middleware/Security/CORSMiddlewareTest.php @@ -123,10 +123,12 @@ class CORSMiddlewareTest extends \Test\TestCase { } /** + * CORS must not be enforced for anonymous users on public pages + * * @CORS * @PublicPage */ - public function testNoCORSShouldAllowCookieAuth() { + public function testNoCORSOnAnonymousPublicPage() { $request = new Request( [], $this->createMock(IRequestId::class), @@ -134,6 +136,9 @@ class CORSMiddlewareTest extends \Test\TestCase { ); $this->reflector->reflect($this, __FUNCTION__); $middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler); + $this->session->expects($this->once()) + ->method('isLoggedIn') + ->willReturn(false); $this->session->expects($this->never()) ->method('logout'); $this->session->expects($this->never()) @@ -146,6 +151,35 @@ class CORSMiddlewareTest extends \Test\TestCase { } /** + * Even on public pages users logged in using session cookies, + * that do not provide a valid CSRF token are disallowed + * + * @CORS + * @PublicPage + */ + public function testCORSShouldNeverAllowCookieAuth() { + $request = new Request( + [], + $this->createMock(IRequestId::class), + $this->createMock(IConfig::class) + ); + $this->reflector->reflect($this, __FUNCTION__); + $middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler); + $this->session->expects($this->once()) + ->method('isLoggedIn') + ->willReturn(true); + $this->session->expects($this->once()) + ->method('logout'); + $this->session->expects($this->never()) + ->method('logClientIn') + ->with($this->equalTo('user'), $this->equalTo('pass')) + ->willReturn(true); + + $this->expectException(SecurityException::class); + $middleware->beforeController($this->controller, __FUNCTION__); + } + + /** * @CORS */ public function testCORSShouldRelogin() { |