Merge pull request #25162 from owncloud/password-login-forbidden-hint

Password login forbidden hint

8 files changed, 176 insertions, 28 deletions

diff --git a/apps/dav/lib/Connector/Sabre/Auth.php b/apps/dav/lib/Connector/Sabre/Auth.php
index 51f0acbe2ee..82c2711b560 100644
--- a/apps/dav/lib/Connector/Sabre/Auth.php
+++ b/apps/dav/lib/Connector/Sabre/Auth.php
@@ -31,8 +31,10 @@ namespace OCA\DAV\Connector\Sabre;
 
 use Exception;
 use OC\AppFramework\Http\Request;
+use OC\Authentication\Exceptions\PasswordLoginForbiddenException;
 use OC\Authentication\TwoFactorAuth\Manager;
 use OC\User\Session;
+use OCA\DAV\Connector\Sabre\Exception\PasswordLoginForbidden;
 use OCP\IRequest;
 use OCP\ISession;
 use Sabre\DAV\Auth\Backend\AbstractBasic;
@@ -115,14 +117,19 @@ class Auth extends AbstractBasic {
 			return true;
 		} else {
 			\OC_Util::setupFS(); //login hooks may need early access to the filesystem
-			if($this->userSession->logClientIn($username, $password, $this->request)) {
-				\OC_Util::setupFS($this->userSession->getUser()->getUID());
-				$this->session->set(self::DAV_AUTHENTICATED, $this->userSession->getUser()->getUID());
+			try {
+				if ($this->userSession->logClientIn($username, $password, $this->request)) {
+					\OC_Util::setupFS($this->userSession->getUser()->getUID());
+					$this->session->set(self::DAV_AUTHENTICATED, $this->userSession->getUser()->getUID());
+					$this->session->close();
+					return true;
+				} else {
+					$this->session->close();
+					return false;
+				}
+			} catch (PasswordLoginForbiddenException $ex) {
 				$this->session->close();
-				return true;
-			} else {
-				$this->session->close();
-				return false;
+				throw new PasswordLoginForbidden();
 			}
 		}
 	}
diff --git a/apps/dav/lib/Connector/Sabre/Exception/PasswordLoginForbidden.php b/apps/dav/lib/Connector/Sabre/Exception/PasswordLoginForbidden.php
new file mode 100644
index 00000000000..6537da3d56d
--- /dev/null
+++ b/apps/dav/lib/Connector/Sabre/Exception/PasswordLoginForbidden.php
@@ -0,0 +1,54 @@
+<?php
+
+/**
+ * @author Christoph Wurst <christoph@owncloud.com>
+ *
+ * @copyright Copyright (c) 2016, ownCloud, Inc.
+ * @license AGPL-3.0
+ *
+ * This code is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License, version 3,
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License, version 3,
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>
+ *
+ */
+
+namespace OCA\DAV\Connector\Sabre\Exception;
+
+use DOMElement;
+use Sabre\DAV\Server;
+use Sabre\DAV\Exception\NotAuthenticated;
+
+class PasswordLoginForbidden extends NotAuthenticated {
+
+	const NS_OWNCLOUD = 'http://owncloud.org/ns';
+
+	public function getHTTPCode() {
+		return 401;
+	}
+
+	/**
+	 * This method allows the exception to include additional information
+	 * into the WebDAV error response
+	 *
+	 * @param Server $server
+	 * @param DOMElement $errorNode
+	 * @return void
+	 */
+	public function serialize(Server $server, DOMElement $errorNode) {
+
+		// set ownCloud namespace
+		$errorNode->setAttribute('xmlns:o', self::NS_OWNCLOUD);
+
+		$error = $errorNode->ownerDocument->createElementNS('o:', 'o:hint', 'password login forbidden');
+		$errorNode->appendChild($error);
+	}
+
+}
diff --git a/apps/dav/tests/unit/Connector/Sabre/AuthTest.php b/apps/dav/tests/unit/Connector/Sabre/AuthTest.php
index 147a0c2b8c5..9564298f23a 100644
--- a/apps/dav/tests/unit/Connector/Sabre/AuthTest.php
+++ b/apps/dav/tests/unit/Connector/Sabre/AuthTest.php
@@ -208,6 +208,25 @@ class AuthTest extends TestCase {
 		$this->assertFalse($this->invokePrivate($this->auth, 'validateUserPass', ['MyTestUser', 'MyTestPassword']));
 	}
 
+	/**
+	 * @expectedException \OCA\DAV\Connector\Sabre\Exception\PasswordLoginForbidden
+	 */
+	public function testValidateUserPassWithPasswordLoginForbidden() {
+		$this->userSession
+			->expects($this->once())
+			->method('isLoggedIn')
+			->will($this->returnValue(false));
+		$this->userSession
+			->expects($this->once())
+			->method('logClientIn')
+			->with('MyTestUser', 'MyTestPassword')
+			->will($this->throwException(new \OC\Authentication\Exceptions\PasswordLoginForbiddenException()));
+		$this->session
+			->expects($this->once())
+			->method('close');
+
+		$this->invokePrivate($this->auth, 'validateUserPass', ['MyTestUser', 'MyTestPassword']);
+	}
 
 	public function testAuthenticateAlreadyLoggedInWithoutCsrfTokenForNonGet() {
 		$request = $this->getMockBuilder('Sabre\HTTP\RequestInterface')
diff --git a/lib/private/AppFramework/Middleware/Security/CORSMiddleware.php b/lib/private/AppFramework/Middleware/Security/CORSMiddleware.php
index 69bfeb5e9bb..32a507623e3 100644
--- a/lib/private/AppFramework/Middleware/Security/CORSMiddleware.php
+++ b/lib/private/AppFramework/Middleware/Security/CORSMiddleware.php
@@ -26,6 +26,7 @@ namespace OC\AppFramework\Middleware\Security;
 
 use OC\AppFramework\Middleware\Security\Exceptions\SecurityException;
 use OC\AppFramework\Utility\ControllerMethodReflector;
+use OC\Authentication\Exceptions\PasswordLoginForbiddenException;
 use OC\User\Session;
 use OCP\AppFramework\Controller;
 use OCP\AppFramework\Http;
@@ -89,8 +90,12 @@ class CORSMiddleware extends Middleware {
 			$pass = $this->request->server['PHP_AUTH_PW'];
 
 			$this->session->logout();
-			if(!$this->session->logClientIn($user, $pass, $this->request)) {
-				throw new SecurityException('CORS requires basic auth', Http::STATUS_UNAUTHORIZED);
+			try {
+				if (!$this->session->logClientIn($user, $pass, $this->request)) {
+					throw new SecurityException('CORS requires basic auth', Http::STATUS_UNAUTHORIZED);
+				}
+			} catch (PasswordLoginForbiddenException $ex) {
+				throw new SecurityException('Password login forbidden, use token instead', Http::STATUS_UNAUTHORIZED);
 			}
 		}
 	}
diff --git a/lib/private/Authentication/Exceptions/PasswordLoginForbiddenException.php b/lib/private/Authentication/Exceptions/PasswordLoginForbiddenException.php
new file mode 100644
index 00000000000..2e9f9534dbd
--- /dev/null
+++ b/lib/private/Authentication/Exceptions/PasswordLoginForbiddenException.php
@@ -0,0 +1,29 @@
+<?php
+
+/**
+ * @author Christoph Wurst <christoph@owncloud.com>
+ *
+ * @copyright Copyright (c) 2016, ownCloud, Inc.
+ * @license AGPL-3.0
+ *
+ * This code is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License, version 3,
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License, version 3,
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>
+ *
+ */
+
+namespace OC\Authentication\Exceptions;
+
+use Exception;
+
+class PasswordLoginForbiddenException extends Exception {
+
+}
diff --git a/lib/private/User/Session.php b/lib/private/User/Session.php
index 0cebb3e0613..4e9c827448d 100644
--- a/lib/private/User/Session.php
+++ b/lib/private/User/Session.php
@@ -33,6 +33,7 @@ namespace OC\User;
 use OC;
 use OC\Authentication\Exceptions\InvalidTokenException;
 use OC\Authentication\Exceptions\PasswordlessTokenException;
+use OC\Authentication\Exceptions\PasswordLoginForbiddenException;
 use OC\Authentication\Token\IProvider;
 use OC\Authentication\Token\IToken;
 use OC\Hooks\Emitter;
@@ -350,17 +351,16 @@ class Session implements IUserSession, Emitter {
 	 * @param string $password
 	 * @param IRequest $request
 	 * @throws LoginException
+	 * @throws PasswordLoginForbiddenException
 	 * @return boolean
 	 */
 	public function logClientIn($user, $password, IRequest $request) {
 		$isTokenPassword = $this->isTokenPassword($password);
 		if (!$isTokenPassword && $this->isTokenAuthEnforced()) {
-			// TODO: throw LoginException instead (https://github.com/owncloud/core/pull/24616)
-			return false;
+			throw new PasswordLoginForbiddenException();
 		}
 		if (!$isTokenPassword && $this->isTwoFactorEnforced($user)) {
-			// TODO: throw LoginException instead (https://github.com/owncloud/core/pull/24616)
-			return false;
+			throw new PasswordLoginForbiddenException();
 		}
 		if (!$this->login($user, $password) ) {
 			$users = $this->manager->getByEmail($user);
@@ -442,19 +442,22 @@ class Session implements IUserSession, Emitter {
 	 */
 	public function tryBasicAuthLogin(IRequest $request) {
 		if (!empty($request->server['PHP_AUTH_USER']) && !empty($request->server['PHP_AUTH_PW'])) {
-			$result = $this->logClientIn($request->server['PHP_AUTH_USER'], $request->server['PHP_AUTH_PW'], $request);
-			if ($result === true) {
-				/**
-				 * Add DAV authenticated. This should in an ideal world not be
-				 * necessary but the iOS App reads cookies from anywhere instead
-				 * only the DAV endpoint.
-				 * This makes sure that the cookies will be valid for the whole scope
-				 * @see https://github.com/owncloud/core/issues/22893
-				 */
-				$this->session->set(
-					Auth::DAV_AUTHENTICATED, $this->getUser()->getUID()
-				);
-				return true;
+			try {
+				if ($this->logClientIn($request->server['PHP_AUTH_USER'], $request->server['PHP_AUTH_PW'], $request)) {
+					/**
+					 * Add DAV authenticated. This should in an ideal world not be
+					 * necessary but the iOS App reads cookies from anywhere instead
+					 * only the DAV endpoint.
+					 * This makes sure that the cookies will be valid for the whole scope
+					 * @see https://github.com/owncloud/core/issues/22893
+					 */
+					$this->session->set(
+						Auth::DAV_AUTHENTICATED, $this->getUser()->getUID()
+					);
+					return true;
+				}
+			} catch (PasswordLoginForbiddenException $ex) {
+				// Nothing to do
 			}
 		}
 		return false;
diff --git a/tests/lib/AppFramework/Middleware/Security/CORSMiddlewareTest.php b/tests/lib/AppFramework/Middleware/Security/CORSMiddlewareTest.php
index a398dc2320c..54d2831d25f 100644
--- a/tests/lib/AppFramework/Middleware/Security/CORSMiddlewareTest.php
+++ b/tests/lib/AppFramework/Middleware/Security/CORSMiddlewareTest.php
@@ -164,6 +164,31 @@ class CORSMiddlewareTest extends \Test\TestCase {
 	 * @CORS
 	 * @expectedException \OC\AppFramework\Middleware\Security\Exceptions\SecurityException
 	 */
+	public function testCORSShouldFailIfPasswordLoginIsForbidden() {
+		$request = new Request(
+			['server' => [
+				'PHP_AUTH_USER' => 'user',
+				'PHP_AUTH_PW' => 'pass'
+			]],
+			$this->getMock('\OCP\Security\ISecureRandom'),
+			$this->getMock('\OCP\IConfig')
+		);
+		$this->session->expects($this->once())
+			->method('logout');
+		$this->session->expects($this->once())
+			->method('logClientIn')
+			->with($this->equalTo('user'), $this->equalTo('pass'))
+			->will($this->throwException(new \OC\Authentication\Exceptions\PasswordLoginForbiddenException));
+		$this->reflector->reflect($this, __FUNCTION__);
+		$middleware = new CORSMiddleware($request, $this->reflector, $this->session);
+
+		$middleware->beforeController($this, __FUNCTION__, new Response());
+	}
+
+	/**
+	 * @CORS
+	 * @expectedException \OC\AppFramework\Middleware\Security\Exceptions\SecurityException
+	 */
 	public function testCORSShouldNotAllowCookieAuth() {
 		$request = new Request(
 			['server' => [
diff --git a/tests/lib/User/SessionTest.php b/tests/lib/User/SessionTest.php
index 28f6b6a5377..7a34d42a2bc 100644
--- a/tests/lib/User/SessionTest.php
+++ b/tests/lib/User/SessionTest.php
@@ -306,6 +306,9 @@ class SessionTest extends \Test\TestCase {
 		$userSession->login('foo', 'bar');
 	}
 
+	/**
+	 * @expectedException \OC\Authentication\Exceptions\PasswordLoginForbiddenException
+	 */
 	public function testLogClientInNoTokenPasswordWith2fa() {
 		$manager = $this->getMockBuilder('\OC\User\Manager')
 			->disableOriginalConstructor()
@@ -329,7 +332,7 @@ class SessionTest extends \Test\TestCase {
 			->with('token_auth_enforced', false)
 			->will($this->returnValue(true));
 
-		$this->assertFalse($userSession->logClientIn('john', 'doe', $request));
+		$userSession->logClientIn('john', 'doe', $request);
 	}
 
 	public function testLogClientInWithTokenPassword() {
@@ -371,6 +374,9 @@ class SessionTest extends \Test\TestCase {
 		$this->assertTrue($userSession->logClientIn('john', 'doe', $request));
 	}
 
+	/**
+	 * @expectedException \OC\Authentication\Exceptions\PasswordLoginForbiddenException
+	 */
 	public function testLogClientInNoTokenPasswordNo2fa() {
 		$manager = $this->getMockBuilder('\OC\User\Manager')
 			->disableOriginalConstructor()
@@ -399,7 +405,7 @@ class SessionTest extends \Test\TestCase {
 			->with('john')
 			->will($this->returnValue(true));
 
-		$this->assertFalse($userSession->logClientIn('john', 'doe', $request));
+		$userSession->logClientIn('john', 'doe', $request);
 	}
 
 	public function testRememberLoginValidToken() {