diff --git a/core/js/js.js b/core/js/js.js index a918066340..0b28310c72 100644 --- a/core/js/js.js +++ b/core/js/js.js @@ -1682,7 +1682,8 @@ OC.PasswordConfirmation = { requiresPasswordConfirmation: function() { var timeSinceLogin = moment.now() - (nc_lastLogin * 1000); - return timeSinceLogin > 30 * 60 * 1000; // 30 minutes + // if timeSinceLogin > 30 minutes and user backend allows password confirmation + return (backendAllowsPasswordConfirmation && timeSinceLogin > 30 * 60 * 1000); }, /** diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 738054cd37..974a634386 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -335,6 +335,7 @@ return array( 'OC\\AppFramework\\Middleware\\Security\\Exceptions\\NotLoggedInException' => $baseDir . '/lib/private/AppFramework/Middleware/Security/Exceptions/NotLoggedInException.php', 'OC\\AppFramework\\Middleware\\Security\\Exceptions\\SecurityException' => $baseDir . '/lib/private/AppFramework/Middleware/Security/Exceptions/SecurityException.php', 'OC\\AppFramework\\Middleware\\Security\\Exceptions\\StrictCookieMissingException' => $baseDir . '/lib/private/AppFramework/Middleware/Security/Exceptions/StrictCookieMissingException.php', + 'OC\\AppFramework\\Middleware\\Security\\PasswordConfirmationMiddleware' => $baseDir . '/lib/private/AppFramework/Middleware/Security/PasswordConfirmationMiddleware.php', 'OC\\AppFramework\\Middleware\\Security\\RateLimitingMiddleware' => $baseDir . '/lib/private/AppFramework/Middleware/Security/RateLimitingMiddleware.php', 'OC\\AppFramework\\Middleware\\Security\\SameSiteCookieMiddleware' => $baseDir . '/lib/private/AppFramework/Middleware/Security/SameSiteCookieMiddleware.php', 'OC\\AppFramework\\Middleware\\Security\\SecurityMiddleware' => $baseDir . '/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index 7ffbd4c788..fc71d085f0 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -365,6 +365,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c 'OC\\AppFramework\\Middleware\\Security\\Exceptions\\NotLoggedInException' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/Security/Exceptions/NotLoggedInException.php', 'OC\\AppFramework\\Middleware\\Security\\Exceptions\\SecurityException' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/Security/Exceptions/SecurityException.php', 'OC\\AppFramework\\Middleware\\Security\\Exceptions\\StrictCookieMissingException' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/Security/Exceptions/StrictCookieMissingException.php', + 'OC\\AppFramework\\Middleware\\Security\\PasswordConfirmationMiddleware' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/Security/PasswordConfirmationMiddleware.php', 'OC\\AppFramework\\Middleware\\Security\\RateLimitingMiddleware' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/Security/RateLimitingMiddleware.php', 'OC\\AppFramework\\Middleware\\Security\\SameSiteCookieMiddleware' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/Security/SameSiteCookieMiddleware.php', 'OC\\AppFramework\\Middleware\\Security\\SecurityMiddleware' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php', diff --git a/lib/private/AppFramework/DependencyInjection/DIContainer.php b/lib/private/AppFramework/DependencyInjection/DIContainer.php index 0b6291d46d..612728d135 100644 --- a/lib/private/AppFramework/DependencyInjection/DIContainer.php +++ b/lib/private/AppFramework/DependencyInjection/DIContainer.php @@ -54,6 +54,7 @@ use OCP\AppFramework\Http\IOutput; use OCP\AppFramework\IApi; use OCP\AppFramework\IAppContainer; use OCP\AppFramework\QueryException; +use OCP\AppFramework\Utility\ITimeFactory; use OCP\Files\Folder; use OCP\Files\IAppData; use OCP\GlobalScale\IConfig; @@ -227,7 +228,6 @@ class DIContainer extends SimpleContainer implements IAppContainer { $server->getNavigationManager(), $server->getURLGenerator(), $server->getLogger(), - $server->getSession(), $c['AppName'], $app->isLoggedIn(), $app->isAdminUser(), @@ -236,7 +236,18 @@ class DIContainer extends SimpleContainer implements IAppContainer { $server->getContentSecurityPolicyNonceManager(), $server->getAppManager() ); + }); + $this->registerService(OC\AppFramework\Middleware\Security\PasswordConfirmationMiddleware::class, function ($c) use ($app) { + /** @var \OC\Server $server */ + $server = $app->getServer(); + + return new OC\AppFramework\Middleware\Security\PasswordConfirmationMiddleware( + $c['ControllerMethodReflector'], + $server->getSession(), + $server->getUserSession(), + $server->query(ITimeFactory::class) + ); }); $this->registerService('BruteForceMiddleware', function($c) use ($app) { @@ -309,6 +320,7 @@ class DIContainer extends SimpleContainer implements IAppContainer { $dispatcher->registerMiddleware($c['CORSMiddleware']); $dispatcher->registerMiddleware($c['OCSMiddleware']); $dispatcher->registerMiddleware($c['SecurityMiddleware']); + $dispatcher->registerMiddleware($c[OC\AppFramework\Middleware\Security\PasswordConfirmationMiddleware::class]); $dispatcher->registerMiddleware($c['TwoFactorMiddleware']); $dispatcher->registerMiddleware($c['BruteForceMiddleware']); $dispatcher->registerMiddleware($c['RateLimitingMiddleware']); diff --git a/lib/private/AppFramework/Middleware/Security/PasswordConfirmationMiddleware.php b/lib/private/AppFramework/Middleware/Security/PasswordConfirmationMiddleware.php new file mode 100644 index 0000000000..463e7cd93c --- /dev/null +++ b/lib/private/AppFramework/Middleware/Security/PasswordConfirmationMiddleware.php @@ -0,0 +1,81 @@ + + * + * @author Roeland Jago Douma + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * 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 + * along with this program. If not, see . + * + */ +namespace OC\AppFramework\Middleware\Security; + +use OC\AppFramework\Middleware\Security\Exceptions\NotConfirmedException; +use OC\AppFramework\Utility\ControllerMethodReflector; +use OCP\AppFramework\Controller; +use OCP\AppFramework\Middleware; +use OCP\AppFramework\Utility\ITimeFactory; +use OCP\ISession; +use OCP\IUserSession; + +class PasswordConfirmationMiddleware extends Middleware { + /** @var ControllerMethodReflector */ + private $reflector; + /** @var ISession */ + private $session; + /** @var IUserSession */ + private $userSession; + /** @var ITimeFactory */ + private $timeFactory; + + /** + * PasswordConfirmationMiddleware constructor. + * + * @param ControllerMethodReflector $reflector + * @param ISession $session + * @param IUserSession $userSession + * @param ITimeFactory $timeFactory + */ + public function __construct(ControllerMethodReflector $reflector, + ISession $session, + IUserSession $userSession, + ITimeFactory $timeFactory) { + $this->reflector = $reflector; + $this->session = $session; + $this->userSession = $userSession; + $this->timeFactory = $timeFactory; + } + + /** + * @param Controller $controller + * @param string $methodName + * @throws NotConfirmedException + */ + public function beforeController($controller, $methodName) { + if ($this->reflector->hasAnnotation('PasswordConfirmationRequired')) { + $user = $this->userSession->getUser(); + $backendClassName = ''; + if ($user !== null) { + $backendClassName = $user->getBackendClassName(); + } + + $lastConfirm = (int) $this->session->get('last-password-confirm'); + // we can't check the password against a SAML backend, so skip password confirmation in this case + if ($backendClassName !== 'user_saml' && $lastConfirm < ($this->timeFactory->getTime() - (30 * 60 + 15))) { // allow 15 seconds delay + throw new NotConfirmedException(); + } + } + } +} diff --git a/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php b/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php index ecd7b1bad5..c147b5b247 100644 --- a/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php +++ b/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php @@ -33,7 +33,6 @@ namespace OC\AppFramework\Middleware\Security; use OC\AppFramework\Middleware\Security\Exceptions\AppNotEnabledException; use OC\AppFramework\Middleware\Security\Exceptions\CrossSiteRequestForgeryException; use OC\AppFramework\Middleware\Security\Exceptions\NotAdminException; -use OC\AppFramework\Middleware\Security\Exceptions\NotConfirmedException; use OC\AppFramework\Middleware\Security\Exceptions\NotLoggedInException; use OC\AppFramework\Middleware\Security\Exceptions\StrictCookieMissingException; use OC\AppFramework\Utility\ControllerMethodReflector; @@ -50,7 +49,6 @@ use OCP\AppFramework\Http\Response; use OCP\AppFramework\Http\JSONResponse; use OCP\AppFramework\OCSController; use OCP\INavigationManager; -use OCP\ISession; use OCP\IURLGenerator; use OCP\IRequest; use OCP\ILogger; @@ -77,8 +75,6 @@ class SecurityMiddleware extends Middleware { private $urlGenerator; /** @var ILogger */ private $logger; - /** @var ISession */ - private $session; /** @var bool */ private $isLoggedIn; /** @var bool */ @@ -98,7 +94,6 @@ class SecurityMiddleware extends Middleware { * @param INavigationManager $navigationManager * @param IURLGenerator $urlGenerator * @param ILogger $logger - * @param ISession $session * @param string $appName * @param bool $isLoggedIn * @param bool $isAdminUser @@ -112,21 +107,20 @@ class SecurityMiddleware extends Middleware { INavigationManager $navigationManager, IURLGenerator $urlGenerator, ILogger $logger, - ISession $session, $appName, $isLoggedIn, $isAdminUser, ContentSecurityPolicyManager $contentSecurityPolicyManager, CsrfTokenManager $csrfTokenManager, ContentSecurityPolicyNonceManager $cspNonceManager, - IAppManager $appManager) { + IAppManager $appManager + ) { $this->navigationManager = $navigationManager; $this->request = $request; $this->reflector = $reflector; $this->appName = $appName; $this->urlGenerator = $urlGenerator; $this->logger = $logger; - $this->session = $session; $this->isLoggedIn = $isLoggedIn; $this->isAdminUser = $isAdminUser; $this->contentSecurityPolicyManager = $contentSecurityPolicyManager; @@ -163,13 +157,6 @@ class SecurityMiddleware extends Middleware { } } - if ($this->reflector->hasAnnotation('PasswordConfirmationRequired')) { - $lastConfirm = (int) $this->session->get('last-password-confirm'); - if ($lastConfirm < (time() - (30 * 60 + 15))) { // allow 15 seconds delay - throw new NotConfirmedException(); - } - } - // Check for strict cookie requirement if($this->reflector->hasAnnotation('StrictCookieRequired') || !$this->reflector->hasAnnotation('NoCSRFRequired')) { if(!$this->request->passesStrictCookieCheck()) { diff --git a/lib/private/Template/JSConfigHelper.php b/lib/private/Template/JSConfigHelper.php index 551fc3b9b0..bdb747e1c9 100644 --- a/lib/private/Template/JSConfigHelper.php +++ b/lib/private/Template/JSConfigHelper.php @@ -101,8 +101,10 @@ class JSConfigHelper { if ($this->currentUser !== null) { $uid = $this->currentUser->getUID(); + $userBackend = $this->currentUser->getBackendClassName(); } else { $uid = null; + $userBackend = ''; } // Get the config @@ -147,6 +149,7 @@ class JSConfigHelper { $array = [ "oc_debug" => $this->config->getSystemValue('debug', false) ? 'true' : 'false', "oc_isadmin" => $this->groupManager->isAdmin($uid) ? 'true' : 'false', + "backendAllowsPasswordConfirmation" => $userBackend === 'user_saml'? 'false' : 'true', "oc_dataURL" => is_string($dataLocation) ? "\"".$dataLocation."\"" : 'false', "oc_webroot" => "\"".\OC::$WEBROOT."\"", "oc_appswebroots" => str_replace('\\/', '/', json_encode($apps_paths)), // Ugly unescape slashes waiting for better solution diff --git a/tests/lib/AppFramework/Middleware/Security/PasswordConfirmationMiddlewareTest.php b/tests/lib/AppFramework/Middleware/Security/PasswordConfirmationMiddlewareTest.php new file mode 100644 index 0000000000..2c610736f4 --- /dev/null +++ b/tests/lib/AppFramework/Middleware/Security/PasswordConfirmationMiddlewareTest.php @@ -0,0 +1,129 @@ + + * + * @author Roeland Jago Douma + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * 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 + * along with this program. If not, see . + * + */ +namespace Test\AppFramework\Middleware\Security; + +use OC\AppFramework\Middleware\Security\Exceptions\NotConfirmedException; +use OC\AppFramework\Middleware\Security\PasswordConfirmationMiddleware; +use OC\AppFramework\Utility\ControllerMethodReflector; +use OCP\AppFramework\Controller; +use OCP\AppFramework\Utility\ITimeFactory; +use OCP\ISession; +use OCP\IUser; +use OCP\IUserSession; +use Test\TestCase; + +class PasswordConfirmationMiddlewareTest extends TestCase { + /** @var ControllerMethodReflector */ + private $reflector; + /** @var ISession|\PHPUnit_Framework_MockObject_MockObject */ + private $session; + /** @var IUserSession|\PHPUnit_Framework_MockObject_MockObject */ + private $userSession; + /** @var IUser|\PHPUnit_Framework_MockObject_MockObject */ + private $user; + /** @var PasswordConfirmationMiddleware */ + private $middleware; + /** @var Controller */ + private $contoller; + /** @var ITimeFactory|\PHPUnit_Framework_MockObject_MockObject */ + private $timeFactory; + + protected function setUp() { + $this->reflector = new ControllerMethodReflector(); + $this->session = $this->createMock(ISession::class); + $this->userSession = $this->createMock(IUserSession::class); + $this->user = $this->createMock(IUser::class); + $this->contoller = $this->createMock(Controller::class); + $this->timeFactory = $this->createMock(ITimeFactory::class); + + $this->middleware = new PasswordConfirmationMiddleware( + $this->reflector, + $this->session, + $this->userSession, + $this->timeFactory + ); + } + + public function testNoAnnotation() { + $this->reflector->reflect(__CLASS__, __FUNCTION__); + $this->session->expects($this->never()) + ->method($this->anything()); + $this->userSession->expects($this->never()) + ->method($this->anything()); + + $this->middleware->beforeController($this->contoller, __FUNCTION__); + } + + /** + * @TestAnnotation + */ + public function testDifferentAnnotation() { + $this->reflector->reflect(__CLASS__, __FUNCTION__); + $this->session->expects($this->never()) + ->method($this->anything()); + $this->userSession->expects($this->never()) + ->method($this->anything()); + + $this->middleware->beforeController($this->contoller, __FUNCTION__); + } + + /** + * @PasswordConfirmationRequired + * @dataProvider testProvider + */ + public function testAnnotation($backend, $lastConfirm, $currentTime, $exception) { + $this->reflector->reflect(__CLASS__, __FUNCTION__); + + $this->user->method('getBackendClassName') + ->willReturn($backend); + $this->userSession->method('getUser') + ->willReturn($this->user); + + $this->session->method('get') + ->with('last-password-confirm') + ->willReturn($lastConfirm); + + $this->timeFactory->method('getTime') + ->willReturn($currentTime); + + $thrown = false; + try { + $this->middleware->beforeController($this->contoller, __FUNCTION__); + } catch (NotConfirmedException $e) { + $thrown = true; + } + + $this->assertSame($exception, $thrown); + } + + public function testProvider() { + return [ + ['foo', 2000, 4000, true], + ['foo', 2000, 3000, false], + ['user_saml', 2000, 4000, false], + ['user_saml', 2000, 3000, false], + ['foo', 2000, 3815, false], + ['foo', 2000, 3816, true], + ]; + } +} diff --git a/tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php b/tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php index 6b311c7ae1..151d6935e7 100644 --- a/tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php +++ b/tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php @@ -50,6 +50,8 @@ use OCP\INavigationManager; use OCP\IRequest; use OCP\ISession; use OCP\IURLGenerator; +use OCP\IUser; +use OCP\IUserSession; use OCP\Security\ISecureRandom; class SecurityMiddlewareTest extends \Test\TestCase { @@ -62,8 +64,6 @@ class SecurityMiddlewareTest extends \Test\TestCase { private $secException; /** @var SecurityException */ private $secAjaxException; - /** @var ISession|\PHPUnit_Framework_MockObject_MockObject */ - private $session; /** @var IRequest|\PHPUnit_Framework_MockObject_MockObject */ private $request; /** @var ControllerMethodReflector */ @@ -82,6 +82,8 @@ class SecurityMiddlewareTest extends \Test\TestCase { private $cspNonceManager; /** @var IAppManager|\PHPUnit_Framework_MockObject_MockObject */ private $appManager; + /** @var IUserSession|\PHPUnit_Framework_MockObject_MockObject */ + private $userSession; protected function setUp() { parent::setUp(); @@ -91,7 +93,6 @@ class SecurityMiddlewareTest extends \Test\TestCase { $this->logger = $this->createMock(ILogger::class); $this->navigationManager = $this->createMock(INavigationManager::class); $this->urlGenerator = $this->createMock(IURLGenerator::class); - $this->session = $this->createMock(ISession::class); $this->request = $this->createMock(IRequest::class); $this->contentSecurityPolicyManager = $this->createMock(ContentSecurityPolicyManager::class); $this->csrfTokenManager = $this->createMock(CsrfTokenManager::class); @@ -117,7 +118,6 @@ class SecurityMiddlewareTest extends \Test\TestCase { $this->navigationManager, $this->urlGenerator, $this->logger, - $this->session, 'files', $isLoggedIn, $isAdminUser,