diff --git a/apps/dav/appinfo/v1/caldav.php b/apps/dav/appinfo/v1/caldav.php index f860ced387..333e8bbb3c 100644 --- a/apps/dav/appinfo/v1/caldav.php +++ b/apps/dav/appinfo/v1/caldav.php @@ -30,6 +30,7 @@ use Sabre\CalDAV\CalendarRoot; $authBackend = new Auth( \OC::$server->getSession(), \OC::$server->getUserSession(), + \OC::$server->getRequest(), 'principals/' ); $principalBackend = new Principal( diff --git a/apps/dav/appinfo/v1/carddav.php b/apps/dav/appinfo/v1/carddav.php index e0c79c75b7..54f0d259bb 100644 --- a/apps/dav/appinfo/v1/carddav.php +++ b/apps/dav/appinfo/v1/carddav.php @@ -32,6 +32,7 @@ use Sabre\CardDAV\Plugin; $authBackend = new Auth( \OC::$server->getSession(), \OC::$server->getUserSession(), + \OC::$server->getRequest(), 'principals/' ); $principalBackend = new Principal( diff --git a/apps/dav/appinfo/v1/webdav.php b/apps/dav/appinfo/v1/webdav.php index 3d3e51e84b..211ffabef8 100644 --- a/apps/dav/appinfo/v1/webdav.php +++ b/apps/dav/appinfo/v1/webdav.php @@ -41,6 +41,7 @@ $serverFactory = new \OCA\DAV\Connector\Sabre\ServerFactory( $authBackend = new \OCA\DAV\Connector\Sabre\Auth( \OC::$server->getSession(), \OC::$server->getUserSession(), + \OC::$server->getRequest(), 'principals/' ); $requestUri = \OC::$server->getRequest()->getRequestUri(); diff --git a/apps/dav/lib/connector/sabre/auth.php b/apps/dav/lib/connector/sabre/auth.php index a046e07848..4bb07c5f0e 100644 --- a/apps/dav/lib/connector/sabre/auth.php +++ b/apps/dav/lib/connector/sabre/auth.php @@ -30,6 +30,7 @@ namespace OCA\DAV\Connector\Sabre; use Exception; +use OCP\IRequest; use OCP\ISession; use OCP\IUserSession; use Sabre\DAV\Auth\Backend\AbstractBasic; @@ -45,17 +46,22 @@ class Auth extends AbstractBasic { private $session; /** @var IUserSession */ private $userSession; + /** @var IRequest */ + private $request; /** * @param ISession $session * @param IUserSession $userSession + * @param IRequest $request * @param string $principalPrefix */ public function __construct(ISession $session, IUserSession $userSession, + IRequest $request, $principalPrefix = 'principals/users/') { $this->session = $session; $this->userSession = $userSession; + $this->request = $request; $this->principalPrefix = $principalPrefix; } @@ -106,26 +112,6 @@ class Auth extends AbstractBasic { } } - /** - * Returns information about the currently logged in username. - * - * If nobody is currently logged in, this method should return null. - * - * @return string|null - */ - public function getCurrentUser() { - $user = $this->userSession->getUser() ? $this->userSession->getUser()->getUID() : null; - if($user !== null && $this->isDavAuthenticated($user)) { - return $user; - } - - if($user !== null && is_null($this->session->get(self::DAV_AUTHENTICATED))) { - return $user; - } - - return null; - } - /** * @param RequestInterface $request * @param ResponseInterface $response @@ -150,8 +136,19 @@ class Auth extends AbstractBasic { * @param RequestInterface $request * @param ResponseInterface $response * @return array + * @throws NotAuthenticated */ private function auth(RequestInterface $request, ResponseInterface $response) { + // If request is not GET and not authenticated via WebDAV a requesttoken is required + if($this->userSession->isLoggedIn() && + $this->request->getMethod() !== 'GET' && + !$this->isDavAuthenticated($this->userSession->getUser()->getUID())) { + if(!$this->request->passesCSRFCheck()) { + $response->setStatus(401); + throw new \Sabre\DAV\Exception\NotAuthenticated('CSRF check not passed.'); + } + } + if (\OC_User::handleApacheAuth() || //Fix for broken webdav clients ($this->userSession->isLoggedIn() && is_null($this->session->get(self::DAV_AUTHENTICATED))) || diff --git a/apps/dav/lib/dav/sharing/plugin.php b/apps/dav/lib/dav/sharing/plugin.php index f6e2cceebd..e6eab3539b 100644 --- a/apps/dav/lib/dav/sharing/plugin.php +++ b/apps/dav/lib/dav/sharing/plugin.php @@ -129,9 +129,6 @@ class Plugin extends ServerPlugin { return; } - // CSRF protection - $this->protectAgainstCSRF(); - $requestBody = $request->getBodyAsString(); // If this request handler could not deal with this POST request, it @@ -201,18 +198,4 @@ class Plugin extends ServerPlugin { } } - private function protectAgainstCSRF() { - $user = $this->auth->getCurrentUser(); - if ($this->auth->isDavAuthenticated($user)) { - return true; - } - - if ($this->request->passesCSRFCheck()) { - return true; - } - - throw new BadRequest(); - } - - } diff --git a/apps/dav/lib/server.php b/apps/dav/lib/server.php index f5f1875a48..ed1ee68404 100644 --- a/apps/dav/lib/server.php +++ b/apps/dav/lib/server.php @@ -53,7 +53,8 @@ class Server { // Backends $authBackend = new Auth( \OC::$server->getSession(), - \OC::$server->getUserSession() + \OC::$server->getUserSession(), + \OC::$server->getRequest() ); // Set URL explicitly due to reverse-proxy situations diff --git a/apps/dav/tests/unit/connector/sabre/auth.php b/apps/dav/tests/unit/connector/sabre/auth.php index 5ff736664f..57ed44f01c 100644 --- a/apps/dav/tests/unit/connector/sabre/auth.php +++ b/apps/dav/tests/unit/connector/sabre/auth.php @@ -24,6 +24,7 @@ namespace OCA\DAV\Tests\Unit\Connector\Sabre; +use OCP\IRequest; use OCP\IUser; use Test\TestCase; use OCP\ISession; @@ -42,6 +43,8 @@ class Auth extends TestCase { private $auth; /** @var IUserSession */ private $userSession; + /** @var IRequest */ + private $request; public function setUp() { parent::setUp(); @@ -49,7 +52,13 @@ class Auth extends TestCase { ->disableOriginalConstructor()->getMock(); $this->userSession = $this->getMockBuilder('\OCP\IUserSession') ->disableOriginalConstructor()->getMock(); - $this->auth = new \OCA\DAV\Connector\Sabre\Auth($this->session, $this->userSession); + $this->request = $this->getMockBuilder('\OCP\IRequest') + ->disableOriginalConstructor()->getMock(); + $this->auth = new \OCA\DAV\Connector\Sabre\Auth( + $this->session, + $this->userSession, + $this->request + ); } public function testIsDavAuthenticatedWithoutDavSession() { @@ -189,71 +198,11 @@ class Auth extends TestCase { $this->assertFalse($this->invokePrivate($this->auth, 'validateUserPass', ['MyTestUser', 'MyTestPassword'])); } - public function testGetCurrentUserWithoutBeingLoggedIn() { - $this->assertSame(null, $this->auth->getCurrentUser()); - } - - public function testGetCurrentUserWithValidDAVLogin() { - $user = $this->getMockBuilder('\OCP\IUser') - ->disableOriginalConstructor() - ->getMock(); - $user->expects($this->once()) - ->method('getUID') - ->will($this->returnValue('MyTestUser')); - $this->userSession - ->expects($this->exactly(2)) - ->method('getUser') - ->will($this->returnValue($user)); - $this->session - ->expects($this->exactly(2)) - ->method('get') - ->with('AUTHENTICATED_TO_DAV_BACKEND') - ->will($this->returnValue('MyTestUser')); - - $this->assertSame('MyTestUser', $this->auth->getCurrentUser()); - } - - public function testGetCurrentUserWithoutAnyDAVLogin() { - $user = $this->getMockBuilder('\OCP\IUser') - ->disableOriginalConstructor() - ->getMock(); - $user->expects($this->once()) - ->method('getUID') - ->will($this->returnValue('MyTestUser')); - $this->userSession - ->expects($this->exactly(2)) - ->method('getUser') - ->will($this->returnValue($user)); - $this->session - ->expects($this->exactly(2)) - ->method('get') - ->with('AUTHENTICATED_TO_DAV_BACKEND') - ->will($this->returnValue(null)); - - $this->assertSame('MyTestUser', $this->auth->getCurrentUser()); - } - - public function testGetCurrentUserWithWrongDAVUser() { - $user = $this->getMockBuilder('\OCP\IUser') - ->disableOriginalConstructor() - ->getMock(); - $user->expects($this->once()) - ->method('getUID') - ->will($this->returnValue('MyWrongDavUser')); - $this->userSession - ->expects($this->exactly(2)) - ->method('getUser') - ->will($this->returnValue($user)); - $this->session - ->expects($this->exactly(3)) - ->method('get') - ->with('AUTHENTICATED_TO_DAV_BACKEND') - ->will($this->returnValue('AnotherUser')); - - $this->assertSame(null, $this->auth->getCurrentUser()); - } - - public function testAuthenticateAlreadyLoggedIn() { + /** + * @expectedException \Sabre\DAV\Exception\NotAuthenticated + * @expectedExceptionMessage CSRF check not passed. + */ + public function testAuthenticateAlreadyLoggedInWithoutCsrfTokenForNonGet() { $request = $this->getMockBuilder('Sabre\HTTP\RequestInterface') ->disableOriginalConstructor() ->getMock(); @@ -279,9 +228,77 @@ class Auth extends TestCase { ->expects($this->once()) ->method('getUser') ->will($this->returnValue($user)); + + $response = $this->auth->check($request, $response); + $this->assertEquals([true, 'principals/users/MyWrongDavUser'], $response); + } + + public function testAuthenticateAlreadyLoggedInWithoutCsrfTokenForGet() { + $request = $this->getMockBuilder('Sabre\HTTP\RequestInterface') + ->disableOriginalConstructor() + ->getMock(); + $response = $this->getMockBuilder('Sabre\HTTP\ResponseInterface') + ->disableOriginalConstructor() + ->getMock(); + $this->userSession + ->expects($this->exactly(2)) + ->method('isLoggedIn') + ->will($this->returnValue(true)); $this->session ->expects($this->once()) - ->method('close'); + ->method('get') + ->with('AUTHENTICATED_TO_DAV_BACKEND') + ->will($this->returnValue(null)); + $user = $this->getMockBuilder('\OCP\IUser') + ->disableOriginalConstructor() + ->getMock(); + $user->expects($this->once()) + ->method('getUID') + ->will($this->returnValue('MyWrongDavUser')); + $this->userSession + ->expects($this->once()) + ->method('getUser') + ->will($this->returnValue($user)); + $this->request + ->expects($this->once()) + ->method('getMethod') + ->willReturn('GET'); + + $response = $this->auth->check($request, $response); + $this->assertEquals([true, 'principals/users/MyWrongDavUser'], $response); + } + + + public function testAuthenticateAlreadyLoggedInWithCsrfTokenForGet() { + $request = $this->getMockBuilder('Sabre\HTTP\RequestInterface') + ->disableOriginalConstructor() + ->getMock(); + $response = $this->getMockBuilder('Sabre\HTTP\ResponseInterface') + ->disableOriginalConstructor() + ->getMock(); + $this->userSession + ->expects($this->exactly(2)) + ->method('isLoggedIn') + ->will($this->returnValue(true)); + $this->session + ->expects($this->exactly(2)) + ->method('get') + ->with('AUTHENTICATED_TO_DAV_BACKEND') + ->will($this->returnValue(null)); + $user = $this->getMockBuilder('\OCP\IUser') + ->disableOriginalConstructor() + ->getMock(); + $user->expects($this->exactly(2)) + ->method('getUID') + ->will($this->returnValue('MyWrongDavUser')); + $this->userSession + ->expects($this->exactly(2)) + ->method('getUser') + ->will($this->returnValue($user)); + $this->request + ->expects($this->once()) + ->method('passesCSRFCheck') + ->willReturn(true); $response = $this->auth->check($request, $response); $this->assertEquals([true, 'principals/users/MyWrongDavUser'], $response); diff --git a/core/js/files/client.js b/core/js/files/client.js index a7f393d325..0bf5a69e19 100644 --- a/core/js/files/client.js +++ b/core/js/files/client.js @@ -37,7 +37,10 @@ } url += options.host + this._root; - this._defaultHeaders = options.defaultHeaders || {'X-Requested-With': 'XMLHttpRequest'}; + this._defaultHeaders = options.defaultHeaders || { + 'X-Requested-With': 'XMLHttpRequest', + 'requesttoken': OC.requestToken + }; this._baseUrl = url; var clientOptions = { diff --git a/core/js/oc-backbone-webdav.js b/core/js/oc-backbone-webdav.js index ba678a32fc..1c1b5c71d8 100644 --- a/core/js/oc-backbone-webdav.js +++ b/core/js/oc-backbone-webdav.js @@ -240,7 +240,8 @@ return options.url; }; var headers = _.extend({ - 'X-Requested-With': 'XMLHttpRequest' + 'X-Requested-With': 'XMLHttpRequest', + 'requesttoken': OC.requestToken }, options.headers); if (options.type === 'PROPFIND') { return callPropFind(client, options, model, headers);