From 331d88bcabd4a66b0efc89fa28b90d26e88f4637 Mon Sep 17 00:00:00 2001 From: Christoph Wurst Date: Mon, 13 Jun 2016 15:38:34 +0200 Subject: create session token on all APIs --- apps/dav/lib/Connector/Sabre/Auth.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'apps/dav') diff --git a/apps/dav/lib/Connector/Sabre/Auth.php b/apps/dav/lib/Connector/Sabre/Auth.php index 653da10bc3c..51f0acbe2ee 100644 --- a/apps/dav/lib/Connector/Sabre/Auth.php +++ b/apps/dav/lib/Connector/Sabre/Auth.php @@ -115,8 +115,7 @@ 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->userSession->createSessionToken($this->request, $this->userSession->getUser()->getUID(), $username, $password); + 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(); -- cgit v1.2.3 From 465807490d7648e5675f1cdbc5b1d232cda4feee Mon Sep 17 00:00:00 2001 From: Christoph Wurst Date: Mon, 13 Jun 2016 16:00:49 +0200 Subject: create session token only for clients that support cookies --- apps/dav/tests/unit/Connector/Sabre/AuthTest.php | 17 +++------ lib/private/User/Session.php | 13 ++++++- tests/lib/User/SessionTest.php | 48 ++++++++++++++++++++++-- 3 files changed, 61 insertions(+), 17 deletions(-) (limited to 'apps/dav') diff --git a/apps/dav/tests/unit/Connector/Sabre/AuthTest.php b/apps/dav/tests/unit/Connector/Sabre/AuthTest.php index b3ab49a027e..147a0c2b8c5 100644 --- a/apps/dav/tests/unit/Connector/Sabre/AuthTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/AuthTest.php @@ -159,7 +159,7 @@ class AuthTest extends TestCase { $user = $this->getMockBuilder('\OCP\IUser') ->disableOriginalConstructor() ->getMock(); - $user->expects($this->exactly(4)) + $user->expects($this->exactly(3)) ->method('getUID') ->will($this->returnValue('MyTestUser')); $this->userSession @@ -167,7 +167,7 @@ class AuthTest extends TestCase { ->method('isLoggedIn') ->will($this->returnValue(true)); $this->userSession - ->expects($this->exactly(4)) + ->expects($this->exactly(3)) ->method('getUser') ->will($this->returnValue($user)); $this->session @@ -178,12 +178,8 @@ class AuthTest extends TestCase { $this->userSession ->expects($this->once()) ->method('logClientIn') - ->with('MyTestUser', 'MyTestPassword') + ->with('MyTestUser', 'MyTestPassword', $this->request) ->will($this->returnValue(true)); - $this->userSession - ->expects($this->once()) - ->method('createSessionToken') - ->with($this->request, 'MyTestUser', 'MyTestUser', 'MyTestPassword'); $this->session ->expects($this->once()) ->method('set') @@ -626,17 +622,14 @@ class AuthTest extends TestCase { ->method('logClientIn') ->with('username', 'password') ->will($this->returnValue(true)); - $this->userSession - ->expects($this->once()) - ->method('createSessionToken'); $user = $this->getMockBuilder('\OCP\IUser') ->disableOriginalConstructor() ->getMock(); - $user->expects($this->exactly(4)) + $user->expects($this->exactly(3)) ->method('getUID') ->will($this->returnValue('MyTestUser')); $this->userSession - ->expects($this->exactly(4)) + ->expects($this->exactly(3)) ->method('getUser') ->will($this->returnValue($user)); $response = $this->auth->check($server->httpRequest, $server->httpResponse); diff --git a/lib/private/User/Session.php b/lib/private/User/Session.php index 0376e81b6dc..0cebb3e0613 100644 --- a/lib/private/User/Session.php +++ b/lib/private/User/Session.php @@ -370,11 +370,21 @@ class Session implements IUserSession, Emitter { return false; } - $this->createSessionToken($request, $this->getUser()->getUID(), $user, $password); + if ($this->supportsCookies($request)) { + $this->createSessionToken($request, $this->getUser()->getUID(), $user, $password); + } return true; } + protected function supportsCookies(IRequest $request) { + if (!is_null($request->getCookie('cookie_test'))) { + return true; + } + setcookie('cookie_test', 'test', $this->timeFacory->getTime() + 3600); + return false; + } + private function isTokenAuthEnforced() { return $this->config->getSystemValue('token_auth_enforced', false); } @@ -432,7 +442,6 @@ class Session implements IUserSession, Emitter { */ public function tryBasicAuthLogin(IRequest $request) { if (!empty($request->server['PHP_AUTH_USER']) && !empty($request->server['PHP_AUTH_PW'])) { - $request = \OC::$server->getRequest(); $result = $this->logClientIn($request->server['PHP_AUTH_USER'], $request->server['PHP_AUTH_PW'], $request); if ($result === true) { /** diff --git a/tests/lib/User/SessionTest.php b/tests/lib/User/SessionTest.php index eac38ebba16..28f6b6a5377 100644 --- a/tests/lib/User/SessionTest.php +++ b/tests/lib/User/SessionTest.php @@ -311,11 +311,13 @@ class SessionTest extends \Test\TestCase { ->disableOriginalConstructor() ->getMock(); $session = $this->getMock('\OCP\ISession'); + $request = $this->getMock('\OCP\IRequest'); + $user = $this->getMock('\OCP\IUser'); /** @var \OC\User\Session $userSession */ $userSession = $this->getMockBuilder('\OC\User\Session') ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config]) - ->setMethods(['login']) + ->setMethods(['login', 'supportsCookies', 'createSessionToken', 'getUser']) ->getMock(); $this->tokenProvider->expects($this->once()) @@ -327,7 +329,46 @@ class SessionTest extends \Test\TestCase { ->with('token_auth_enforced', false) ->will($this->returnValue(true)); - $this->assertFalse($userSession->logClientIn('john', 'doe')); + $this->assertFalse($userSession->logClientIn('john', 'doe', $request)); + } + + public function testLogClientInWithTokenPassword() { + $manager = $this->getMockBuilder('\OC\User\Manager') + ->disableOriginalConstructor() + ->getMock(); + $session = $this->getMock('\OCP\ISession'); + $request = $this->getMock('\OCP\IRequest'); + $user = $this->getMock('\OCP\IUser'); + + /** @var \OC\User\Session $userSession */ + $userSession = $this->getMockBuilder('\OC\User\Session') + ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config]) + ->setMethods(['isTokenPassword', 'login', 'supportsCookies', 'createSessionToken', 'getUser']) + ->getMock(); + + $userSession->expects($this->once()) + ->method('isTokenPassword') + ->will($this->returnValue(true)); + $userSession->expects($this->once()) + ->method('login') + ->with('john', 'doe') + ->will($this->returnValue(true)); + + $userSession->expects($this->once()) + ->method('supportsCookies') + ->with($request) + ->will($this->returnValue(true)); + $userSession->expects($this->once()) + ->method('getUser') + ->will($this->returnValue($user)); + $user->expects($this->once()) + ->method('getUID') + ->will($this->returnValue('user123')); + $userSession->expects($this->once()) + ->method('createSessionToken') + ->with($request, 'user123', 'john', 'doe'); + + $this->assertTrue($userSession->logClientIn('john', 'doe', $request)); } public function testLogClientInNoTokenPasswordNo2fa() { @@ -336,6 +377,7 @@ class SessionTest extends \Test\TestCase { ->getMock(); $session = $this->getMock('\OCP\ISession'); $user = $this->getMock('\OCP\IUser'); + $request = $this->getMock('\OCP\IRequest'); /** @var \OC\User\Session $userSession */ $userSession = $this->getMockBuilder('\OC\User\Session') @@ -357,7 +399,7 @@ class SessionTest extends \Test\TestCase { ->with('john') ->will($this->returnValue(true)); - $this->assertFalse($userSession->logClientIn('john', 'doe')); + $this->assertFalse($userSession->logClientIn('john', 'doe', $request)); } public function testRememberLoginValidToken() { -- cgit v1.2.3 From 0b7685d326279fd822a161fb29c2256e2ad01c50 Mon Sep 17 00:00:00 2001 From: Thomas Müller Date: Thu, 16 Jun 2016 16:14:28 +0200 Subject: Move birthday calendar generation to a live migration job (#25135) --- apps/dav/appinfo/info.xml | 3 ++ apps/dav/appinfo/install.php | 26 ----------- apps/dav/appinfo/update.php | 26 ----------- apps/dav/lib/AppInfo/Application.php | 25 +++++----- apps/dav/lib/Migration/GenerateBirthdays.php | 70 ++++++++++++++++++++++++++++ 5 files changed, 84 insertions(+), 66 deletions(-) delete mode 100644 apps/dav/appinfo/install.php delete mode 100644 apps/dav/appinfo/update.php create mode 100644 apps/dav/lib/Migration/GenerateBirthdays.php (limited to 'apps/dav') diff --git a/apps/dav/appinfo/info.xml b/apps/dav/appinfo/info.xml index 26e37e6bb86..df147a032fe 100644 --- a/apps/dav/appinfo/info.xml +++ b/apps/dav/appinfo/info.xml @@ -24,5 +24,8 @@ OCA\DAV\Migration\Classification + + OCA\DAV\Migration\GenerateBirthdays + diff --git a/apps/dav/appinfo/install.php b/apps/dav/appinfo/install.php deleted file mode 100644 index d2ee06cc9fe..00000000000 --- a/apps/dav/appinfo/install.php +++ /dev/null @@ -1,26 +0,0 @@ - - * @author Thomas Müller - * - * @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 - * - */ - -use OCA\DAV\AppInfo\Application; - -$app = new Application(); -$app->generateBirthdays(); diff --git a/apps/dav/appinfo/update.php b/apps/dav/appinfo/update.php deleted file mode 100644 index d2ee06cc9fe..00000000000 --- a/apps/dav/appinfo/update.php +++ /dev/null @@ -1,26 +0,0 @@ - - * @author Thomas Müller - * - * @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 - * - */ - -use OCA\DAV\AppInfo\Application; - -$app = new Application(); -$app->generateBirthdays(); diff --git a/apps/dav/lib/AppInfo/Application.php b/apps/dav/lib/AppInfo/Application.php index 9e0d2da4e17..de2056ebc35 100644 --- a/apps/dav/lib/AppInfo/Application.php +++ b/apps/dav/lib/AppInfo/Application.php @@ -32,6 +32,7 @@ use OCA\DAV\Connector\Sabre\Principal; use OCA\DAV\DAV\GroupPrincipalBackend; use OCA\DAV\HookManager; use OCA\DAV\Migration\Classification; +use OCA\DAV\Migration\GenerateBirthdays; use \OCP\AppFramework\App; use OCP\AppFramework\IAppContainer; use OCP\Contacts\IManager; @@ -116,6 +117,16 @@ class Application extends App { $c->getServer()->getUserManager() ); }); + + $container->registerService('OCA\DAV\Migration\GenerateBirthdays', function ($c) { + /** @var IAppContainer $c */ + /** @var BirthdayService $b */ + $b = $c->query('BirthdayService'); + return new GenerateBirthdays( + $b, + $c->getServer()->getUserManager() + ); + }); } /** @@ -164,18 +175,4 @@ class Application extends App { return $this->getContainer()->query('SyncService'); } - public function generateBirthdays() { - try { - /** @var BirthdayService $migration */ - $migration = $this->getContainer()->query('BirthdayService'); - $userManager = $this->getContainer()->getServer()->getUserManager(); - - $userManager->callForAllUsers(function($user) use($migration) { - /** @var IUser $user */ - $migration->syncUser($user->getUID()); - }); - } catch (\Exception $ex) { - $this->getContainer()->getServer()->getLogger()->logException($ex); - } - } } diff --git a/apps/dav/lib/Migration/GenerateBirthdays.php b/apps/dav/lib/Migration/GenerateBirthdays.php new file mode 100644 index 00000000000..dfc8838bcbb --- /dev/null +++ b/apps/dav/lib/Migration/GenerateBirthdays.php @@ -0,0 +1,70 @@ + + * + * @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 OCA\DAV\Migration; + +use OCA\DAV\CalDAV\BirthdayService; +use OCP\IUser; +use OCP\IUserManager; +use OCP\Migration\IOutput; +use OCP\Migration\IRepairStep; + +class GenerateBirthdays implements IRepairStep { + + /** @var BirthdayService */ + private $birthdayService; + + /** @var IUserManager */ + private $userManager; + + /** + * GenerateBirthdays constructor. + * + * @param BirthdayService $birthdayService + * @param IUserManager $userManager + */ + public function __construct(BirthdayService $birthdayService, IUserManager $userManager) { + $this->birthdayService = $birthdayService; + $this->userManager = $userManager; + } + + /** + * @inheritdoc + */ + public function getName() { + return 'Regenerate birthday calendar for all users'; + } + + /** + * @inheritdoc + */ + public function run(IOutput $output) { + + $output->startProgress(); + $this->userManager->callForAllUsers(function($user) use ($output) { + /** @var IUser $user */ + $output->advance(1, $user->getDisplayName()); + $this->birthdayService->syncUser($user->getUID()); + }); + $output->finishProgress(); + } +} -- cgit v1.2.3 From 82b50d126c5ea05db3e1cc846db2b410b54fd8fe Mon Sep 17 00:00:00 2001 From: Christoph Wurst Date: Fri, 17 Jun 2016 11:01:35 +0200 Subject: add PasswordLoginForbiddenException --- apps/dav/lib/Connector/Sabre/Auth.php | 19 +++++++---- apps/dav/tests/unit/Connector/Sabre/AuthTest.php | 16 ++++++++++ .../Middleware/Security/CORSMiddleware.php | 9 ++++-- .../Exceptions/PasswordLoginForbiddenException.php | 29 +++++++++++++++++ lib/private/User/Session.php | 37 ++++++++++++---------- .../Middleware/Security/CORSMiddlewareTest.php | 25 +++++++++++++++ tests/lib/User/SessionTest.php | 10 ++++-- 7 files changed, 118 insertions(+), 27 deletions(-) create mode 100644 lib/private/Authentication/Exceptions/PasswordLoginForbiddenException.php (limited to 'apps/dav') diff --git a/apps/dav/lib/Connector/Sabre/Auth.php b/apps/dav/lib/Connector/Sabre/Auth.php index 51f0acbe2ee..a37ef787bab 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 147a0c2b8c5..60dbe6e7223 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 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 @@ + + * + * @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 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 @@ -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 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() { -- cgit v1.2.3 From 5a8cfab68f147aa094a0e96ac03c6b41937c40e0 Mon Sep 17 00:00:00 2001 From: Christoph Wurst Date: Fri, 17 Jun 2016 11:18:27 +0200 Subject: throw PasswordLoginForbidden on DAV --- apps/dav/lib/Connector/Sabre/Auth.php | 4 +- .../Sabre/Exception/PasswordLoginForbidden.php | 54 ++++++++++++++++++++++ apps/dav/tests/unit/Connector/Sabre/AuthTest.php | 5 +- 3 files changed, 60 insertions(+), 3 deletions(-) create mode 100644 apps/dav/lib/Connector/Sabre/Exception/PasswordLoginForbidden.php (limited to 'apps/dav') diff --git a/apps/dav/lib/Connector/Sabre/Auth.php b/apps/dav/lib/Connector/Sabre/Auth.php index a37ef787bab..82c2711b560 100644 --- a/apps/dav/lib/Connector/Sabre/Auth.php +++ b/apps/dav/lib/Connector/Sabre/Auth.php @@ -34,6 +34,7 @@ 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; @@ -127,9 +128,8 @@ class Auth extends AbstractBasic { return false; } } catch (PasswordLoginForbiddenException $ex) { - // TODO: throw sabre exception $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..78b27d0544c --- /dev/null +++ b/apps/dav/lib/Connector/Sabre/Exception/PasswordLoginForbidden.php @@ -0,0 +1,54 @@ + + * + * @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 OCA\DAV\Connector\Sabre\Exception; + +use DOMElement; +use Sabre\DAV\Exception; +use Sabre\DAV\Server; + +class PasswordLoginForbidden extends Exception { + + 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 60dbe6e7223..9564298f23a 100644 --- a/apps/dav/tests/unit/Connector/Sabre/AuthTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/AuthTest.php @@ -208,6 +208,9 @@ 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()) @@ -222,7 +225,7 @@ class AuthTest extends TestCase { ->expects($this->once()) ->method('close'); - $this->assertFalse($this->invokePrivate($this->auth, 'validateUserPass', ['MyTestUser', 'MyTestPassword'])); + $this->invokePrivate($this->auth, 'validateUserPass', ['MyTestUser', 'MyTestPassword']); } public function testAuthenticateAlreadyLoggedInWithoutCsrfTokenForNonGet() { -- cgit v1.2.3 From 2340660a5b93402946a36645ca05836c06f5f816 Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Fri, 17 Jun 2016 15:50:24 +0200 Subject: PasswordLoginForbidden must extend NotAuthenticated The auth code from Sabre will forward NotAuthenticated exceptions but in the case of a generic exception, it is packaged as "service not available". --- apps/dav/lib/Connector/Sabre/Exception/PasswordLoginForbidden.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'apps/dav') diff --git a/apps/dav/lib/Connector/Sabre/Exception/PasswordLoginForbidden.php b/apps/dav/lib/Connector/Sabre/Exception/PasswordLoginForbidden.php index 78b27d0544c..6537da3d56d 100644 --- a/apps/dav/lib/Connector/Sabre/Exception/PasswordLoginForbidden.php +++ b/apps/dav/lib/Connector/Sabre/Exception/PasswordLoginForbidden.php @@ -23,10 +23,10 @@ namespace OCA\DAV\Connector\Sabre\Exception; use DOMElement; -use Sabre\DAV\Exception; use Sabre\DAV\Server; +use Sabre\DAV\Exception\NotAuthenticated; -class PasswordLoginForbidden extends Exception { +class PasswordLoginForbidden extends NotAuthenticated { const NS_OWNCLOUD = 'http://owncloud.org/ns'; -- cgit v1.2.3