diff --git a/apps/dav/lib/Connector/Sabre/Auth.php b/apps/dav/lib/Connector/Sabre/Auth.php index 51f0acbe2e..a37ef787ba 100644 --- a/apps/dav/lib/Connector/Sabre/Auth.php +++ b/apps/dav/lib/Connector/Sabre/Auth.php @@ -31,6 +31,7 @@ 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 OCP\IRequest; @@ -115,12 +116,18 @@ 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()); - $this->session->close(); - return true; - } else { + 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) { + // TODO: throw sabre exception $this->session->close(); return false; } diff --git a/apps/dav/tests/unit/Connector/Sabre/AuthTest.php b/apps/dav/tests/unit/Connector/Sabre/AuthTest.php index 147a0c2b8c..60dbe6e722 100644 --- a/apps/dav/tests/unit/Connector/Sabre/AuthTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/AuthTest.php @@ -208,6 +208,22 @@ class AuthTest extends TestCase { $this->assertFalse($this->invokePrivate($this->auth, 'validateUserPass', ['MyTestUser', 'MyTestPassword'])); } + 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->assertFalse($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 69bfeb5e9b..32a507623e 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 0000000000..2e9f9534db --- /dev/null +++ b/lib/private/Authentication/Exceptions/PasswordLoginForbiddenException.php @@ -0,0 +1,29 @@ + + * + * @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 + * + */ + +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 0cebb3e061..4e9c827448 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 a398dc2320..54d2831d25 100644 --- a/tests/lib/AppFramework/Middleware/Security/CORSMiddlewareTest.php +++ b/tests/lib/AppFramework/Middleware/Security/CORSMiddlewareTest.php @@ -160,6 +160,31 @@ class CORSMiddlewareTest extends \Test\TestCase { $middleware->beforeController($this, __FUNCTION__, new Response()); } + /** + * @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 diff --git a/tests/lib/User/SessionTest.php b/tests/lib/User/SessionTest.php index 28f6b6a537..7a34d42a2b 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() {