Merge pull request #24935 from owncloud/2fa-block-dav

block DAV if 2FA challenge needs to be solved first

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
 		);
 	}
 
@@ -297,6 +304,59 @@ class AuthTest extends TestCase {
 
 	/**
 	 * @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.
 	 */
 	public function testAuthenticateAlreadyLoggedInWithoutCsrfTokenAndIncorrectlyDavAuthenticated() {