From da03a85c3c60adbcdd4f85d041263d4d5cee5ca5 Mon Sep 17 00:00:00 2001 From: Christoph Wurst Date: Wed, 1 Jun 2016 10:42:38 +0200 Subject: [PATCH] block DAV if 2FA challenge needs to be solved first --- apps/dav/appinfo/v1/caldav.php | 1 + apps/dav/appinfo/v1/carddav.php | 1 + apps/dav/appinfo/v1/webdav.php | 1 + apps/dav/lib/Connector/Sabre/Auth.php | 13 +++- apps/dav/lib/Server.php | 4 +- .../tests/unit/Connector/Sabre/AuthTest.php | 66 ++++++++++++++++++- 6 files changed, 80 insertions(+), 6 deletions(-) diff --git a/apps/dav/appinfo/v1/caldav.php b/apps/dav/appinfo/v1/caldav.php index d3af7144745..50348a60202 100644 --- a/apps/dav/appinfo/v1/caldav.php +++ b/apps/dav/appinfo/v1/caldav.php @@ -34,6 +34,7 @@ $authBackend = new Auth( \OC::$server->getSession(), \OC::$server->getUserSession(), \OC::$server->getRequest(), + \OC::$server->getTwoFactorAuthManager(), 'principals/' ); $principalBackend = new Principal( diff --git a/apps/dav/appinfo/v1/carddav.php b/apps/dav/appinfo/v1/carddav.php index 7ef510689c2..88582d64ee5 100644 --- a/apps/dav/appinfo/v1/carddav.php +++ b/apps/dav/appinfo/v1/carddav.php @@ -35,6 +35,7 @@ $authBackend = new Auth( \OC::$server->getSession(), \OC::$server->getUserSession(), \OC::$server->getRequest(), + \OC::$server->getTwoFactorAuthManager(), 'principals/' ); $principalBackend = new Principal( diff --git a/apps/dav/appinfo/v1/webdav.php b/apps/dav/appinfo/v1/webdav.php index 275849f618d..3b733c0fbd5 100644 --- a/apps/dav/appinfo/v1/webdav.php +++ b/apps/dav/appinfo/v1/webdav.php @@ -42,6 +42,7 @@ $authBackend = new \OCA\DAV\Connector\Sabre\Auth( \OC::$server->getSession(), \OC::$server->getUserSession(), \OC::$server->getRequest(), + \OC::$server->getTwoFactorAuthManager(), '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 8b9f86af1e7..7b959a0d899 100644 --- a/apps/dav/lib/Connector/Sabre/Auth.php +++ b/apps/dav/lib/Connector/Sabre/Auth.php @@ -31,9 +31,10 @@ namespace OCA\DAV\Connector\Sabre; use Exception; use OC\AppFramework\Http\Request; +use OC\Authentication\TwoFactorAuth\Manager; +use OC\User\Session; use OCP\IRequest; use OCP\ISession; -use OC\User\Session; use Sabre\DAV\Auth\Backend\AbstractBasic; use Sabre\DAV\Exception\NotAuthenticated; use Sabre\DAV\Exception\ServiceUnavailable; @@ -41,6 +42,8 @@ use Sabre\HTTP\RequestInterface; use Sabre\HTTP\ResponseInterface; class Auth extends AbstractBasic { + + const DAV_AUTHENTICATED = 'AUTHENTICATED_TO_DAV_BACKEND'; /** @var ISession */ @@ -51,19 +54,24 @@ class Auth extends AbstractBasic { private $request; /** @var string */ private $currentUser; + /** @var Manager */ + private $twoFactorManager; /** * @param ISession $session * @param Session $userSession * @param IRequest $request + * @param Manager $twoFactorManager * @param string $principalPrefix */ public function __construct(ISession $session, Session $userSession, IRequest $request, + Manager $twoFactorManager, $principalPrefix = 'principals/users/') { $this->session = $session; $this->userSession = $userSession; + $this->twoFactorManager = $twoFactorManager; $this->request = $request; $this->principalPrefix = $principalPrefix; } @@ -197,6 +205,9 @@ class Auth extends AbstractBasic { if($forcedLogout) { $this->userSession->logout(); } else { + if ($this->twoFactorManager->needsSecondFactor()) { + throw new \Sabre\DAV\Exception\NotAuthenticated('2FA challenge 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/Server.php b/apps/dav/lib/Server.php index 2a9c36698b1..179558e97ae 100644 --- a/apps/dav/lib/Server.php +++ b/apps/dav/lib/Server.php @@ -25,7 +25,6 @@ namespace OCA\DAV; use OCA\DAV\CalDAV\Schedule\IMipPlugin; -use OCA\DAV\Connector\FedAuth; use OCA\DAV\Connector\Sabre\Auth; use OCA\DAV\Connector\Sabre\BlockLegacyClientPlugin; use OCA\DAV\Connector\Sabre\DavAclPlugin; @@ -57,7 +56,8 @@ class Server { $authBackend = new Auth( \OC::$server->getSession(), \OC::$server->getUserSession(), - \OC::$server->getRequest() + \OC::$server->getRequest(), + \OC::$server->getTwoFactorAuthManager() ); // Set URL explicitly due to reverse-proxy situations diff --git a/apps/dav/tests/unit/Connector/Sabre/AuthTest.php b/apps/dav/tests/unit/Connector/Sabre/AuthTest.php index e5b5fe21b1f..b3ab49a027e 100644 --- a/apps/dav/tests/unit/Connector/Sabre/AuthTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/AuthTest.php @@ -27,11 +27,12 @@ namespace OCA\DAV\Tests\unit\Connector\Sabre; +use OC\Authentication\TwoFactorAuth\Manager; +use OC\User\Session; use OCP\IRequest; +use OCP\ISession; use OCP\IUser; use Test\TestCase; -use OCP\ISession; -use OC\User\Session; /** * Class AuthTest @@ -48,6 +49,8 @@ class AuthTest extends TestCase { private $userSession; /** @var IRequest */ private $request; + /** @var Manager */ + private $twoFactorManager; public function setUp() { parent::setUp(); @@ -57,10 +60,14 @@ class AuthTest extends TestCase { ->disableOriginalConstructor()->getMock(); $this->request = $this->getMockBuilder('\OCP\IRequest') ->disableOriginalConstructor()->getMock(); + $this->twoFactorManager = $this->getMockBuilder('\OC\Authentication\TwoFactorAuth\Manager') + ->disableOriginalConstructor() + ->getMock(); $this->auth = new \OCA\DAV\Connector\Sabre\Auth( $this->session, $this->userSession, - $this->request + $this->request, + $this->twoFactorManager ); } @@ -295,6 +302,59 @@ class AuthTest extends TestCase { $this->auth->check($request, $response); } + /** + * @expectedException \Sabre\DAV\Exception\NotAuthenticated + * @expectedExceptionMessage 2FA challenge not passed. + */ + public function testAuthenticateAlreadyLoggedInWithoutTwoFactorChallengePassed() { + $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(true); + $this->twoFactorManager->expects($this->once()) + ->method('needsSecondFactor') + ->will($this->returnValue(true)); + $this->auth->check($request, $response); + } + /** * @expectedException \Sabre\DAV\Exception\NotAuthenticated * @expectedExceptionMessage CSRF check not passed. -- 2.39.5