Merge pull request #23530 from owncloud/fix-shibboleth
Check if request is sent from official ownCloud client
This commit is contained in:
commit
a355e3abe3
|
@ -30,6 +30,7 @@
|
|||
namespace OCA\DAV\Connector\Sabre;
|
||||
|
||||
use Exception;
|
||||
use OC\AppFramework\Http\Request;
|
||||
use OCP\IRequest;
|
||||
use OCP\ISession;
|
||||
use OCP\IUserSession;
|
||||
|
@ -48,6 +49,8 @@ class Auth extends AbstractBasic {
|
|||
private $userSession;
|
||||
/** @var IRequest */
|
||||
private $request;
|
||||
/** @var string */
|
||||
private $currentUser;
|
||||
|
||||
/**
|
||||
* @param ISession $session
|
||||
|
@ -130,7 +133,46 @@ class Auth extends AbstractBasic {
|
|||
$msg = $e->getMessage();
|
||||
throw new ServiceUnavailable("$class: $msg");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Checks whether a CSRF check is required on the request
|
||||
*
|
||||
* @return bool
|
||||
*/
|
||||
private function requiresCSRFCheck() {
|
||||
// GET requires no check at all
|
||||
if($this->request->getMethod() === 'GET') {
|
||||
return false;
|
||||
}
|
||||
|
||||
// Official ownCloud clients require no checks
|
||||
if($this->request->isUserAgent([
|
||||
Request::USER_AGENT_OWNCLOUD_DESKTOP,
|
||||
Request::USER_AGENT_OWNCLOUD_ANDROID,
|
||||
Request::USER_AGENT_OWNCLOUD_IOS,
|
||||
])) {
|
||||
return false;
|
||||
}
|
||||
|
||||
// If not logged-in no check is required
|
||||
if(!$this->userSession->isLoggedIn()) {
|
||||
return false;
|
||||
}
|
||||
|
||||
// POST always requires a check
|
||||
if($this->request->getMethod() === 'POST') {
|
||||
return true;
|
||||
}
|
||||
|
||||
// If logged-in AND DAV authenticated no check is required
|
||||
if($this->userSession->isLoggedIn() &&
|
||||
$this->isDavAuthenticated($this->userSession->getUser()->getUID())) {
|
||||
return false;
|
||||
}
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
/**
|
||||
* @param RequestInterface $request
|
||||
|
@ -139,27 +181,33 @@ class Auth extends AbstractBasic {
|
|||
* @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()) {
|
||||
$forcedLogout = false;
|
||||
if(!$this->request->passesCSRFCheck() &&
|
||||
$this->requiresCSRFCheck()) {
|
||||
// In case of a fail with POST we need to recheck the credentials
|
||||
if($this->request->getMethod() === 'POST') {
|
||||
$forcedLogout = true;
|
||||
} else {
|
||||
$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))) ||
|
||||
//Well behaved clients that only send the cookie are allowed
|
||||
($this->userSession->isLoggedIn() && $this->session->get(self::DAV_AUTHENTICATED) === $this->userSession->getUser()->getUID() && $request->getHeader('Authorization') === null)
|
||||
) {
|
||||
$user = $this->userSession->getUser()->getUID();
|
||||
\OC_Util::setupFS($user);
|
||||
$this->currentUser = $user;
|
||||
$this->session->close();
|
||||
return [true, $this->principalPrefix . $user];
|
||||
if($forcedLogout) {
|
||||
$this->userSession->logout();
|
||||
} else {
|
||||
if (\OC_User::handleApacheAuth() ||
|
||||
//Fix for broken webdav clients
|
||||
($this->userSession->isLoggedIn() && is_null($this->session->get(self::DAV_AUTHENTICATED))) ||
|
||||
//Well behaved clients that only send the cookie are allowed
|
||||
($this->userSession->isLoggedIn() && $this->session->get(self::DAV_AUTHENTICATED) === $this->userSession->getUser()->getUID() && $request->getHeader('Authorization') === null)
|
||||
) {
|
||||
$user = $this->userSession->getUser()->getUID();
|
||||
\OC_Util::setupFS($user);
|
||||
$this->currentUser = $user;
|
||||
$this->session->close();
|
||||
return [true, $this->principalPrefix . $user];
|
||||
}
|
||||
}
|
||||
|
||||
if (!$this->userSession->isLoggedIn() && in_array('XMLHttpRequest', explode(',', $request->getHeader('X-Requested-With')))) {
|
||||
|
|
|
@ -198,10 +198,7 @@ class Auth extends TestCase {
|
|||
$this->assertFalse($this->invokePrivate($this->auth, 'validateUserPass', ['MyTestUser', 'MyTestPassword']));
|
||||
}
|
||||
|
||||
/**
|
||||
* @expectedException \Sabre\DAV\Exception\NotAuthenticated
|
||||
* @expectedExceptionMessage CSRF check not passed.
|
||||
*/
|
||||
|
||||
public function testAuthenticateAlreadyLoggedInWithoutCsrfTokenForNonGet() {
|
||||
$request = $this->getMockBuilder('Sabre\HTTP\RequestInterface')
|
||||
->disableOriginalConstructor()
|
||||
|
@ -210,27 +207,182 @@ class Auth extends TestCase {
|
|||
->disableOriginalConstructor()
|
||||
->getMock();
|
||||
$this->userSession
|
||||
->expects($this->once())
|
||||
->expects($this->any())
|
||||
->method('isLoggedIn')
|
||||
->will($this->returnValue(true));
|
||||
$this->request
|
||||
->expects($this->any())
|
||||
->method('getMethod')
|
||||
->willReturn('POST');
|
||||
$this->session
|
||||
->expects($this->once())
|
||||
->expects($this->any())
|
||||
->method('get')
|
||||
->with('AUTHENTICATED_TO_DAV_BACKEND')
|
||||
->will($this->returnValue(null));
|
||||
$user = $this->getMockBuilder('\OCP\IUser')
|
||||
->disableOriginalConstructor()
|
||||
->getMock();
|
||||
$user->expects($this->once())
|
||||
$user->expects($this->any())
|
||||
->method('getUID')
|
||||
->will($this->returnValue('MyWrongDavUser'));
|
||||
$this->userSession
|
||||
->expects($this->once())
|
||||
->expects($this->any())
|
||||
->method('getUser')
|
||||
->will($this->returnValue($user));
|
||||
$this->request
|
||||
->expects($this->once())
|
||||
->method('passesCSRFCheck')
|
||||
->willReturn(false);
|
||||
|
||||
$expectedResponse = [
|
||||
false,
|
||||
"No 'Authorization: Basic' header found. Either the client didn't send one, or the server is mis-configured",
|
||||
];
|
||||
$response = $this->auth->check($request, $response);
|
||||
$this->assertEquals([true, 'principals/users/MyWrongDavUser'], $response);
|
||||
$this->assertSame($expectedResponse, $response);
|
||||
}
|
||||
|
||||
public function testAuthenticateAlreadyLoggedInWithoutCsrfTokenAndCorrectlyDavAuthenticated() {
|
||||
$request = $this->getMockBuilder('Sabre\HTTP\RequestInterface')
|
||||
->disableOriginalConstructor()
|
||||
->getMock();
|
||||
$response = $this->getMockBuilder('Sabre\HTTP\ResponseInterface')
|
||||
->disableOriginalConstructor()
|
||||
->getMock();
|
||||
$this->userSession
|
||||
->expects($this->any())
|
||||
->method('isLoggedIn')
|
||||
->willReturn(true);
|
||||
$this->request
|
||||
->expects($this->any())
|
||||
->method('getMethod')
|
||||
->willReturn('PROPFIND');
|
||||
$this->request
|
||||
->expects($this->any())
|
||||
->method('isUserAgent')
|
||||
->with([
|
||||
'/^Mozilla\/5\.0 \([A-Za-z ]+\) (mirall|csyncoC)\/.*$/',
|
||||
'/^Mozilla\/5\.0 \(Android\) ownCloud\-android.*$/',
|
||||
'/^Mozilla\/5\.0 \(iOS\) ownCloud\-iOS.*$/',
|
||||
])
|
||||
->willReturn(false);
|
||||
$this->session
|
||||
->expects($this->any())
|
||||
->method('get')
|
||||
->with('AUTHENTICATED_TO_DAV_BACKEND')
|
||||
->will($this->returnValue('LoggedInUser'));
|
||||
$user = $this->getMockBuilder('\OCP\IUser')
|
||||
->disableOriginalConstructor()
|
||||
->getMock();
|
||||
$user->expects($this->any())
|
||||
->method('getUID')
|
||||
->will($this->returnValue('LoggedInUser'));
|
||||
$this->userSession
|
||||
->expects($this->any())
|
||||
->method('getUser')
|
||||
->will($this->returnValue($user));
|
||||
$this->request
|
||||
->expects($this->once())
|
||||
->method('passesCSRFCheck')
|
||||
->willReturn(false);
|
||||
$this->auth->check($request, $response);
|
||||
}
|
||||
|
||||
/**
|
||||
* @expectedException \Sabre\DAV\Exception\NotAuthenticated
|
||||
* @expectedExceptionMessage CSRF check not passed.
|
||||
*/
|
||||
public function testAuthenticateAlreadyLoggedInWithoutCsrfTokenAndIncorrectlyDavAuthenticated() {
|
||||
$request = $this->getMockBuilder('Sabre\HTTP\RequestInterface')
|
||||
->disableOriginalConstructor()
|
||||
->getMock();
|
||||
$response = $this->getMockBuilder('Sabre\HTTP\ResponseInterface')
|
||||
->disableOriginalConstructor()
|
||||
->getMock();
|
||||
$this->userSession
|
||||
->expects($this->any())
|
||||
->method('isLoggedIn')
|
||||
->willReturn(true);
|
||||
$this->request
|
||||
->expects($this->any())
|
||||
->method('getMethod')
|
||||
->willReturn('PROPFIND');
|
||||
$this->request
|
||||
->expects($this->any())
|
||||
->method('isUserAgent')
|
||||
->with([
|
||||
'/^Mozilla\/5\.0 \([A-Za-z ]+\) (mirall|csyncoC)\/.*$/',
|
||||
'/^Mozilla\/5\.0 \(Android\) ownCloud\-android.*$/',
|
||||
'/^Mozilla\/5\.0 \(iOS\) ownCloud\-iOS.*$/',
|
||||
])
|
||||
->willReturn(false);
|
||||
$this->session
|
||||
->expects($this->any())
|
||||
->method('get')
|
||||
->with('AUTHENTICATED_TO_DAV_BACKEND')
|
||||
->will($this->returnValue('AnotherUser'));
|
||||
$user = $this->getMockBuilder('\OCP\IUser')
|
||||
->disableOriginalConstructor()
|
||||
->getMock();
|
||||
$user->expects($this->any())
|
||||
->method('getUID')
|
||||
->will($this->returnValue('LoggedInUser'));
|
||||
$this->userSession
|
||||
->expects($this->any())
|
||||
->method('getUser')
|
||||
->will($this->returnValue($user));
|
||||
$this->request
|
||||
->expects($this->once())
|
||||
->method('passesCSRFCheck')
|
||||
->willReturn(false);
|
||||
$this->auth->check($request, $response);
|
||||
}
|
||||
|
||||
public function testAuthenticateAlreadyLoggedInWithoutCsrfTokenForNonGetAndDesktopClient() {
|
||||
$request = $this->getMockBuilder('Sabre\HTTP\RequestInterface')
|
||||
->disableOriginalConstructor()
|
||||
->getMock();
|
||||
$response = $this->getMockBuilder('Sabre\HTTP\ResponseInterface')
|
||||
->disableOriginalConstructor()
|
||||
->getMock();
|
||||
$this->userSession
|
||||
->expects($this->any())
|
||||
->method('isLoggedIn')
|
||||
->will($this->returnValue(true));
|
||||
$this->request
|
||||
->expects($this->any())
|
||||
->method('getMethod')
|
||||
->willReturn('POST');
|
||||
$this->request
|
||||
->expects($this->any())
|
||||
->method('isUserAgent')
|
||||
->with([
|
||||
'/^Mozilla\/5\.0 \([A-Za-z ]+\) (mirall|csyncoC)\/.*$/',
|
||||
'/^Mozilla\/5\.0 \(Android\) ownCloud\-android.*$/',
|
||||
'/^Mozilla\/5\.0 \(iOS\) ownCloud\-iOS.*$/',
|
||||
])
|
||||
->willReturn(true);
|
||||
$this->session
|
||||
->expects($this->any())
|
||||
->method('get')
|
||||
->with('AUTHENTICATED_TO_DAV_BACKEND')
|
||||
->will($this->returnValue(null));
|
||||
$user = $this->getMockBuilder('\OCP\IUser')
|
||||
->disableOriginalConstructor()
|
||||
->getMock();
|
||||
$user->expects($this->any())
|
||||
->method('getUID')
|
||||
->will($this->returnValue('MyWrongDavUser'));
|
||||
$this->userSession
|
||||
->expects($this->any())
|
||||
->method('getUser')
|
||||
->will($this->returnValue($user));
|
||||
$this->request
|
||||
->expects($this->once())
|
||||
->method('passesCSRFCheck')
|
||||
->willReturn(false);
|
||||
|
||||
$this->auth->check($request, $response);
|
||||
}
|
||||
|
||||
public function testAuthenticateAlreadyLoggedInWithoutCsrfTokenForGet() {
|
||||
|
@ -241,26 +393,26 @@ class Auth extends TestCase {
|
|||
->disableOriginalConstructor()
|
||||
->getMock();
|
||||
$this->userSession
|
||||
->expects($this->exactly(2))
|
||||
->expects($this->any())
|
||||
->method('isLoggedIn')
|
||||
->will($this->returnValue(true));
|
||||
$this->session
|
||||
->expects($this->once())
|
||||
->expects($this->any())
|
||||
->method('get')
|
||||
->with('AUTHENTICATED_TO_DAV_BACKEND')
|
||||
->will($this->returnValue(null));
|
||||
$user = $this->getMockBuilder('\OCP\IUser')
|
||||
->disableOriginalConstructor()
|
||||
->getMock();
|
||||
$user->expects($this->once())
|
||||
$user->expects($this->any())
|
||||
->method('getUID')
|
||||
->will($this->returnValue('MyWrongDavUser'));
|
||||
$this->userSession
|
||||
->expects($this->once())
|
||||
->expects($this->any())
|
||||
->method('getUser')
|
||||
->will($this->returnValue($user));
|
||||
$this->request
|
||||
->expects($this->once())
|
||||
->expects($this->any())
|
||||
->method('getMethod')
|
||||
->willReturn('GET');
|
||||
|
||||
|
@ -268,7 +420,6 @@ class Auth extends TestCase {
|
|||
$this->assertEquals([true, 'principals/users/MyWrongDavUser'], $response);
|
||||
}
|
||||
|
||||
|
||||
public function testAuthenticateAlreadyLoggedInWithCsrfTokenForGet() {
|
||||
$request = $this->getMockBuilder('Sabre\HTTP\RequestInterface')
|
||||
->disableOriginalConstructor()
|
||||
|
@ -277,22 +428,22 @@ class Auth extends TestCase {
|
|||
->disableOriginalConstructor()
|
||||
->getMock();
|
||||
$this->userSession
|
||||
->expects($this->exactly(2))
|
||||
->expects($this->any())
|
||||
->method('isLoggedIn')
|
||||
->will($this->returnValue(true));
|
||||
$this->session
|
||||
->expects($this->exactly(2))
|
||||
->expects($this->any())
|
||||
->method('get')
|
||||
->with('AUTHENTICATED_TO_DAV_BACKEND')
|
||||
->will($this->returnValue(null));
|
||||
$user = $this->getMockBuilder('\OCP\IUser')
|
||||
->disableOriginalConstructor()
|
||||
->getMock();
|
||||
$user->expects($this->exactly(2))
|
||||
$user->expects($this->any())
|
||||
->method('getUID')
|
||||
->will($this->returnValue('MyWrongDavUser'));
|
||||
$this->userSession
|
||||
->expects($this->exactly(2))
|
||||
->expects($this->any())
|
||||
->method('getUser')
|
||||
->will($this->returnValue($user));
|
||||
$this->request
|
||||
|
@ -368,6 +519,10 @@ class Auth extends TestCase {
|
|||
->method('get')
|
||||
->with('AUTHENTICATED_TO_DAV_BACKEND')
|
||||
->will($this->returnValue('MyTestUser'));
|
||||
$this->request
|
||||
->expects($this->once())
|
||||
->method('getMethod')
|
||||
->willReturn('GET');
|
||||
$httpRequest
|
||||
->expects($this->atLeastOnce())
|
||||
->method('getHeader')
|
||||
|
|
|
@ -59,6 +59,9 @@ class Request implements \ArrayAccess, \Countable, IRequest {
|
|||
// Android Chrome user agent: https://developers.google.com/chrome/mobile/docs/user-agent
|
||||
const USER_AGENT_ANDROID_MOBILE_CHROME = '#Android.*Chrome/[.0-9]*#';
|
||||
const USER_AGENT_FREEBOX = '#^Mozilla/5\.0$#';
|
||||
const USER_AGENT_OWNCLOUD_IOS = '/^Mozilla\/5\.0 \(iOS\) ownCloud\-iOS.*$/';
|
||||
const USER_AGENT_OWNCLOUD_ANDROID = '/^Mozilla\/5\.0 \(Android\) ownCloud\-android.*$/';
|
||||
const USER_AGENT_OWNCLOUD_DESKTOP = '/^Mozilla\/5\.0 \([A-Za-z ]+\) (mirall|csyncoC)\/.*$/';
|
||||
const REGEX_LOCALHOST = '/^(127\.0\.0\.1|localhost)$/';
|
||||
|
||||
protected $inputStream;
|
||||
|
|
Loading…
Reference in New Issue