Require CSRF token for non WebDAV authenticated requests
This commit is contained in:
parent
3a97a0ad7f
commit
9b3c4e8dc4
|
@ -30,6 +30,7 @@ use Sabre\CalDAV\CalendarRoot;
|
||||||
$authBackend = new Auth(
|
$authBackend = new Auth(
|
||||||
\OC::$server->getSession(),
|
\OC::$server->getSession(),
|
||||||
\OC::$server->getUserSession(),
|
\OC::$server->getUserSession(),
|
||||||
|
\OC::$server->getRequest(),
|
||||||
'principals/'
|
'principals/'
|
||||||
);
|
);
|
||||||
$principalBackend = new Principal(
|
$principalBackend = new Principal(
|
||||||
|
|
|
@ -32,6 +32,7 @@ use Sabre\CardDAV\Plugin;
|
||||||
$authBackend = new Auth(
|
$authBackend = new Auth(
|
||||||
\OC::$server->getSession(),
|
\OC::$server->getSession(),
|
||||||
\OC::$server->getUserSession(),
|
\OC::$server->getUserSession(),
|
||||||
|
\OC::$server->getRequest(),
|
||||||
'principals/'
|
'principals/'
|
||||||
);
|
);
|
||||||
$principalBackend = new Principal(
|
$principalBackend = new Principal(
|
||||||
|
|
|
@ -41,6 +41,7 @@ $serverFactory = new \OCA\DAV\Connector\Sabre\ServerFactory(
|
||||||
$authBackend = new \OCA\DAV\Connector\Sabre\Auth(
|
$authBackend = new \OCA\DAV\Connector\Sabre\Auth(
|
||||||
\OC::$server->getSession(),
|
\OC::$server->getSession(),
|
||||||
\OC::$server->getUserSession(),
|
\OC::$server->getUserSession(),
|
||||||
|
\OC::$server->getRequest(),
|
||||||
'principals/'
|
'principals/'
|
||||||
);
|
);
|
||||||
$requestUri = \OC::$server->getRequest()->getRequestUri();
|
$requestUri = \OC::$server->getRequest()->getRequestUri();
|
||||||
|
|
|
@ -30,6 +30,7 @@
|
||||||
namespace OCA\DAV\Connector\Sabre;
|
namespace OCA\DAV\Connector\Sabre;
|
||||||
|
|
||||||
use Exception;
|
use Exception;
|
||||||
|
use OCP\IRequest;
|
||||||
use OCP\ISession;
|
use OCP\ISession;
|
||||||
use OCP\IUserSession;
|
use OCP\IUserSession;
|
||||||
use Sabre\DAV\Auth\Backend\AbstractBasic;
|
use Sabre\DAV\Auth\Backend\AbstractBasic;
|
||||||
|
@ -45,17 +46,22 @@ class Auth extends AbstractBasic {
|
||||||
private $session;
|
private $session;
|
||||||
/** @var IUserSession */
|
/** @var IUserSession */
|
||||||
private $userSession;
|
private $userSession;
|
||||||
|
/** @var IRequest */
|
||||||
|
private $request;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* @param ISession $session
|
* @param ISession $session
|
||||||
* @param IUserSession $userSession
|
* @param IUserSession $userSession
|
||||||
|
* @param IRequest $request
|
||||||
* @param string $principalPrefix
|
* @param string $principalPrefix
|
||||||
*/
|
*/
|
||||||
public function __construct(ISession $session,
|
public function __construct(ISession $session,
|
||||||
IUserSession $userSession,
|
IUserSession $userSession,
|
||||||
|
IRequest $request,
|
||||||
$principalPrefix = 'principals/users/') {
|
$principalPrefix = 'principals/users/') {
|
||||||
$this->session = $session;
|
$this->session = $session;
|
||||||
$this->userSession = $userSession;
|
$this->userSession = $userSession;
|
||||||
|
$this->request = $request;
|
||||||
$this->principalPrefix = $principalPrefix;
|
$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 RequestInterface $request
|
||||||
* @param ResponseInterface $response
|
* @param ResponseInterface $response
|
||||||
|
@ -150,8 +136,19 @@ class Auth extends AbstractBasic {
|
||||||
* @param RequestInterface $request
|
* @param RequestInterface $request
|
||||||
* @param ResponseInterface $response
|
* @param ResponseInterface $response
|
||||||
* @return array
|
* @return array
|
||||||
|
* @throws NotAuthenticated
|
||||||
*/
|
*/
|
||||||
private function auth(RequestInterface $request, ResponseInterface $response) {
|
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() ||
|
if (\OC_User::handleApacheAuth() ||
|
||||||
//Fix for broken webdav clients
|
//Fix for broken webdav clients
|
||||||
($this->userSession->isLoggedIn() && is_null($this->session->get(self::DAV_AUTHENTICATED))) ||
|
($this->userSession->isLoggedIn() && is_null($this->session->get(self::DAV_AUTHENTICATED))) ||
|
||||||
|
|
|
@ -129,9 +129,6 @@ class Plugin extends ServerPlugin {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
// CSRF protection
|
|
||||||
$this->protectAgainstCSRF();
|
|
||||||
|
|
||||||
$requestBody = $request->getBodyAsString();
|
$requestBody = $request->getBodyAsString();
|
||||||
|
|
||||||
// If this request handler could not deal with this POST request, it
|
// 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();
|
|
||||||
}
|
|
||||||
|
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
|
@ -53,7 +53,8 @@ class Server {
|
||||||
// Backends
|
// Backends
|
||||||
$authBackend = new Auth(
|
$authBackend = new Auth(
|
||||||
\OC::$server->getSession(),
|
\OC::$server->getSession(),
|
||||||
\OC::$server->getUserSession()
|
\OC::$server->getUserSession(),
|
||||||
|
\OC::$server->getRequest()
|
||||||
);
|
);
|
||||||
|
|
||||||
// Set URL explicitly due to reverse-proxy situations
|
// Set URL explicitly due to reverse-proxy situations
|
||||||
|
|
|
@ -24,6 +24,7 @@
|
||||||
|
|
||||||
namespace OCA\DAV\Tests\Unit\Connector\Sabre;
|
namespace OCA\DAV\Tests\Unit\Connector\Sabre;
|
||||||
|
|
||||||
|
use OCP\IRequest;
|
||||||
use OCP\IUser;
|
use OCP\IUser;
|
||||||
use Test\TestCase;
|
use Test\TestCase;
|
||||||
use OCP\ISession;
|
use OCP\ISession;
|
||||||
|
@ -42,6 +43,8 @@ class Auth extends TestCase {
|
||||||
private $auth;
|
private $auth;
|
||||||
/** @var IUserSession */
|
/** @var IUserSession */
|
||||||
private $userSession;
|
private $userSession;
|
||||||
|
/** @var IRequest */
|
||||||
|
private $request;
|
||||||
|
|
||||||
public function setUp() {
|
public function setUp() {
|
||||||
parent::setUp();
|
parent::setUp();
|
||||||
|
@ -49,7 +52,13 @@ class Auth extends TestCase {
|
||||||
->disableOriginalConstructor()->getMock();
|
->disableOriginalConstructor()->getMock();
|
||||||
$this->userSession = $this->getMockBuilder('\OCP\IUserSession')
|
$this->userSession = $this->getMockBuilder('\OCP\IUserSession')
|
||||||
->disableOriginalConstructor()->getMock();
|
->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() {
|
public function testIsDavAuthenticatedWithoutDavSession() {
|
||||||
|
@ -189,71 +198,11 @@ class Auth extends TestCase {
|
||||||
$this->assertFalse($this->invokePrivate($this->auth, 'validateUserPass', ['MyTestUser', 'MyTestPassword']));
|
$this->assertFalse($this->invokePrivate($this->auth, 'validateUserPass', ['MyTestUser', 'MyTestPassword']));
|
||||||
}
|
}
|
||||||
|
|
||||||
public function testGetCurrentUserWithoutBeingLoggedIn() {
|
/**
|
||||||
$this->assertSame(null, $this->auth->getCurrentUser());
|
* @expectedException \Sabre\DAV\Exception\NotAuthenticated
|
||||||
}
|
* @expectedExceptionMessage CSRF check not passed.
|
||||||
|
*/
|
||||||
public function testGetCurrentUserWithValidDAVLogin() {
|
public function testAuthenticateAlreadyLoggedInWithoutCsrfTokenForNonGet() {
|
||||||
$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() {
|
|
||||||
$request = $this->getMockBuilder('Sabre\HTTP\RequestInterface')
|
$request = $this->getMockBuilder('Sabre\HTTP\RequestInterface')
|
||||||
->disableOriginalConstructor()
|
->disableOriginalConstructor()
|
||||||
->getMock();
|
->getMock();
|
||||||
|
@ -279,9 +228,77 @@ class Auth extends TestCase {
|
||||||
->expects($this->once())
|
->expects($this->once())
|
||||||
->method('getUser')
|
->method('getUser')
|
||||||
->will($this->returnValue($user));
|
->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
|
$this->session
|
||||||
->expects($this->once())
|
->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);
|
$response = $this->auth->check($request, $response);
|
||||||
$this->assertEquals([true, 'principals/users/MyWrongDavUser'], $response);
|
$this->assertEquals([true, 'principals/users/MyWrongDavUser'], $response);
|
||||||
|
|
|
@ -37,7 +37,10 @@
|
||||||
}
|
}
|
||||||
|
|
||||||
url += options.host + this._root;
|
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;
|
this._baseUrl = url;
|
||||||
|
|
||||||
var clientOptions = {
|
var clientOptions = {
|
||||||
|
|
|
@ -240,7 +240,8 @@
|
||||||
return options.url;
|
return options.url;
|
||||||
};
|
};
|
||||||
var headers = _.extend({
|
var headers = _.extend({
|
||||||
'X-Requested-With': 'XMLHttpRequest'
|
'X-Requested-With': 'XMLHttpRequest',
|
||||||
|
'requesttoken': OC.requestToken
|
||||||
}, options.headers);
|
}, options.headers);
|
||||||
if (options.type === 'PROPFIND') {
|
if (options.type === 'PROPFIND') {
|
||||||
return callPropFind(client, options, model, headers);
|
return callPropFind(client, options, model, headers);
|
||||||
|
|
Loading…
Reference in New Issue