mbk-lab
/
nextcloud
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Merge pull request #4346 from nextcloud/properly-do-bruteforce-protection-via-annotation 

Make BruteForceProtection annotation more clever
This commit is contained in:
Roeland Jago Douma 2017-04-14 08:15:55 +02:00 committed by GitHub
parent 0f96d5a641 8149945a91
commit 6b79bf0960
11 changed files with 329 additions and 247 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

36
core/Controller/LoginController.php
View File

@ -29,7 +29,6 @@
namespace OC\Core\Controller;
use OC\Authentication\TwoFactorAuth\Manager;
use OC\Security\Bruteforce\Throttler;
use OC\User\Session;
use OC_App;
use OC_Util;
@ -64,8 +63,6 @@ class LoginController extends Controller {
private $logger;
/** @var Manager */
private $twoFactorManager;
/** @var Throttler */
private $throttler;
/**
* @param string $appName
@ -77,7 +74,6 @@ class LoginController extends Controller {
* @param IURLGenerator $urlGenerator
* @param ILogger $logger
* @param Manager $twoFactorManager
* @param Throttler $throttler
*/
public function __construct($appName,
IRequest $request,
@ -87,8 +83,7 @@ class LoginController extends Controller {
IUserSession $userSession,
IURLGenerator $urlGenerator,
ILogger $logger,
Manager $twoFactorManager,
Throttler $throttler) {
Manager $twoFactorManager) {
parent::__construct($appName, $request);
$this->userManager = $userManager;
$this->config = $config;
@ -97,7 +92,6 @@ class LoginController extends Controller {
$this->urlGenerator = $urlGenerator;
$this->logger = $logger;
$this->twoFactorManager = $twoFactorManager;
$this->throttler = $throttler;
}
/**
@ -203,6 +197,7 @@ class LoginController extends Controller {
* @PublicPage
* @UseSession
* @NoCSRFRequired
* @BruteForceProtection(action=login)
*
* @param string $user
* @param string $password
@ -216,8 +211,6 @@ class LoginController extends Controller {
if(!is_string($user)) {
throw new \InvalidArgumentException('Username must be string');
}
$currentDelay = $this->throttler->getDelay($this->request->getRemoteAddress(), 'login');
$this->throttler->sleepDelay($this->request->getRemoteAddress(), 'login');
// If the user is already logged in and the CSRF check does not pass then
// simply redirect the user to the correct page as required. This is the
@ -245,16 +238,14 @@ class LoginController extends Controller {
}
}
if ($loginResult === false) {
$this->throttler->registerAttempt('login', $this->request->getRemoteAddress(), ['user' => $originalUser]);
if($currentDelay === 0) {
$this->throttler->sleepDelay($this->request->getRemoteAddress(), 'login');
}
// Read current user and append if possible - we need to return the unmodified user otherwise we will leak the login name
$args = !is_null($user) ? ['user' => $originalUser] : [];
$response = new RedirectResponse($this->urlGenerator->linkToRoute('core.login.showLoginForm', $args));
$response->throttle();
$this->session->set('loginMessages', [
['invalidpassword'], []
]);
// Read current user and append if possible - we need to return the unmodified user otherwise we will leak the login name
$args = !is_null($user) ? ['user' => $originalUser] : [];
return new RedirectResponse($this->urlGenerator->linkToRoute('core.login.showLoginForm', $args));
return $response;
}
// TODO: remove password checks from above and let the user session handle failures
// requires https://github.com/owncloud/core/pull/24616
@ -305,6 +296,7 @@ class LoginController extends Controller {
/**
* @NoAdminRequired
* @UseSession
* @BruteForceProtection(action=sudo)
*
* @license GNU AGPL version 3 or any later version
*
@ -312,18 +304,12 @@ class LoginController extends Controller {
* @return DataResponse
*/
public function confirmPassword($password) {
$currentDelay = $this->throttler->getDelay($this->request->getRemoteAddress(), 'sudo');
$this->throttler->sleepDelay($this->request->getRemoteAddress(), 'sudo');
$loginName = $this->userSession->getLoginName();
$loginResult = $this->userManager->checkPassword($loginName, $password);
if ($loginResult === false) {
$this->throttler->registerAttempt('sudo', $this->request->getRemoteAddress(), ['user' => $loginName]);
if ($currentDelay === 0) {
$this->throttler->sleepDelay($this->request->getRemoteAddress(), 'sudo');
}
return new DataResponse([], Http::STATUS_FORBIDDEN);
$response = new DataResponse([], Http::STATUS_FORBIDDEN);
$response->throttle();
return $response;
}
$confirmTimestamp = time();

1
lib/composer/composer/autoload_classmap.php
View File

@ -290,6 +290,7 @@ return array(
'OC\\AppFramework\\Http\\Request' => $baseDir . '/lib/private/AppFramework/Http/Request.php',
'OC\\AppFramework\\Middleware\\MiddlewareDispatcher' => $baseDir . '/lib/private/AppFramework/Middleware/MiddlewareDispatcher.php',
'OC\\AppFramework\\Middleware\\OCSMiddleware' => $baseDir . '/lib/private/AppFramework/Middleware/OCSMiddleware.php',
'OC\\AppFramework\\Middleware\\Security\\BruteForceMiddleware' => $baseDir . '/lib/private/AppFramework/Middleware/Security/BruteForceMiddleware.php',
'OC\\AppFramework\\Middleware\\Security\\CORSMiddleware' => $baseDir . '/lib/private/AppFramework/Middleware/Security/CORSMiddleware.php',
'OC\\AppFramework\\Middleware\\Security\\Exceptions\\AppNotEnabledException' => $baseDir . '/lib/private/AppFramework/Middleware/Security/Exceptions/AppNotEnabledException.php',
'OC\\AppFramework\\Middleware\\Security\\Exceptions\\CrossSiteRequestForgeryException' => $baseDir . '/lib/private/AppFramework/Middleware/Security/Exceptions/CrossSiteRequestForgeryException.php',

1
lib/composer/composer/autoload_static.php
View File

@ -320,6 +320,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c
'OC\\AppFramework\\Http\\Request' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Http/Request.php',
'OC\\AppFramework\\Middleware\\MiddlewareDispatcher' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/MiddlewareDispatcher.php',
'OC\\AppFramework\\Middleware\\OCSMiddleware' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/OCSMiddleware.php',
'OC\\AppFramework\\Middleware\\Security\\BruteForceMiddleware' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/Security/BruteForceMiddleware.php',
'OC\\AppFramework\\Middleware\\Security\\CORSMiddleware' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/Security/CORSMiddleware.php',
'OC\\AppFramework\\Middleware\\Security\\Exceptions\\AppNotEnabledException' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/Security/Exceptions/AppNotEnabledException.php',
'OC\\AppFramework\\Middleware\\Security\\Exceptions\\CrossSiteRequestForgeryException' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/Security/Exceptions/CrossSiteRequestForgeryException.php',

17
lib/private/AppFramework/DependencyInjection/DIContainer.php
View File

@ -224,12 +224,22 @@ class DIContainer extends SimpleContainer implements IAppContainer {
$app->isAdminUser(),
$server->getContentSecurityPolicyManager(),
$server->getCsrfTokenManager(),
$server->getContentSecurityPolicyNonceManager(),
$server->getBruteForceThrottler()
$server->getContentSecurityPolicyNonceManager()
);
});
$this->registerService('BruteForceMiddleware', function($c) use ($app) {
/** @var \OC\Server $server */
$server = $app->getServer();
return new OC\AppFramework\Middleware\Security\BruteForceMiddleware(
$c['ControllerMethodReflector'],
$server->getBruteForceThrottler(),
$server->getRequest()
);
});
$this->registerService('RateLimitingMiddleware', function($c) use ($app) {
/** @var \OC\Server $server */
$server = $app->getServer();
@ -281,7 +291,8 @@ class DIContainer extends SimpleContainer implements IAppContainer {
$dispatcher->registerMiddleware($c['CORSMiddleware']);
$dispatcher->registerMiddleware($c['OCSMiddleware']);
$dispatcher->registerMiddleware($c['SecurityMiddleware']);
$dispatcher->registerMiddleWare($c['TwoFactorMiddleware']);
$dispatcher->registerMiddleware($c['TwoFactorMiddleware']);
$dispatcher->registerMiddleware($c['BruteForceMiddleware']);
$dispatcher->registerMiddleware($c['RateLimitingMiddleware']);
foreach($middleWares as $middleWare) {

83
lib/private/AppFramework/Middleware/Security/BruteForceMiddleware.php Normal file
View File

@ -0,0 +1,83 @@
<?php
/**
* @copyright Copyright (c) 2017 Lukas Reschke <lukas@statuscode.ch>
*
* @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 <http://www.gnu.org/licenses/>.
*
*/
namespace OC\AppFramework\Middleware\Security;
use OC\AppFramework\Utility\ControllerMethodReflector;
use OC\Security\Bruteforce\Throttler;
use OCP\AppFramework\Http\Response;
use OCP\AppFramework\Middleware;
use OCP\IRequest;
/**
* Class BruteForceMiddleware performs the bruteforce protection for controllers
* that are annotated with @BruteForceProtection(action=$action) whereas $action
* is the action that should be logged within the database.
*
* @package OC\AppFramework\Middleware\Security
*/
class BruteForceMiddleware extends Middleware {
/** @var ControllerMethodReflector */
private $reflector;
/** @var Throttler */
private $throttler;
/** @var IRequest */
private $request;
/**
* @param ControllerMethodReflector $controllerMethodReflector
* @param Throttler $throttler
* @param IRequest $request
*/
public function __construct(ControllerMethodReflector $controllerMethodReflector,
Throttler $throttler,
IRequest $request) {
$this->reflector = $controllerMethodReflector;
$this->throttler = $throttler;
$this->request = $request;
}
/**
* {@inheritDoc}
*/
public function beforeController($controller, $methodName) {
parent::beforeController($controller, $methodName);
if($this->reflector->hasAnnotation('BruteForceProtection')) {
$action = $this->reflector->getAnnotationParameter('BruteForceProtection', 'action');
$this->throttler->sleepDelay($this->request->getRemoteAddress(), $action);
}
}
/**
* {@inheritDoc}
*/
public function afterController($controller, $methodName, Response $response) {
if($this->reflector->hasAnnotation('BruteForceProtection') && $response->isThrottled()) {
$action = $this->reflector->getAnnotationParameter('BruteForceProtection', 'action');
$ip = $this->request->getRemoteAddress();
$this->throttler->sleepDelay($ip, $action);
$this->throttler->registerAttempt($action, $ip);
}
return parent::afterController($controller, $methodName, $response);
}
}

15
lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php
View File

@ -36,7 +36,6 @@ 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;
use OC\Security\Bruteforce\Throttler;
use OC\Security\CSP\ContentSecurityPolicyManager;
use OC\Security\CSP\ContentSecurityPolicyNonceManager;
use OC\Security\CSRF\CsrfTokenManager;
@ -88,8 +87,6 @@ class SecurityMiddleware extends Middleware {
private $csrfTokenManager;
/** @var ContentSecurityPolicyNonceManager */
private $cspNonceManager;
/** @var Throttler */
private $throttler;
/**
* @param IRequest $request
@ -104,7 +101,6 @@ class SecurityMiddleware extends Middleware {
* @param ContentSecurityPolicyManager $contentSecurityPolicyManager
* @param CSRFTokenManager $csrfTokenManager
* @param ContentSecurityPolicyNonceManager $cspNonceManager
* @param Throttler $throttler
*/
public function __construct(IRequest $request,
ControllerMethodReflector $reflector,
@ -117,8 +113,7 @@ class SecurityMiddleware extends Middleware {
$isAdminUser,
ContentSecurityPolicyManager $contentSecurityPolicyManager,
CsrfTokenManager $csrfTokenManager,
ContentSecurityPolicyNonceManager $cspNonceManager,
Throttler $throttler) {
ContentSecurityPolicyNonceManager $cspNonceManager) {
$this->navigationManager = $navigationManager;
$this->request = $request;
$this->reflector = $reflector;
@ -131,10 +126,8 @@ class SecurityMiddleware extends Middleware {
$this->contentSecurityPolicyManager = $contentSecurityPolicyManager;
$this->csrfTokenManager = $csrfTokenManager;
$this->cspNonceManager = $cspNonceManager;
$this->throttler = $throttler;
}
/**
* This runs all the security checks before a method call. The
* security checks are determined by inspecting the controller method
@ -191,12 +184,6 @@ class SecurityMiddleware extends Middleware {
}
}
if($this->reflector->hasAnnotation('BruteForceProtection')) {
$action = $this->reflector->getAnnotationParameter('BruteForceProtection', 'action');
$this->throttler->sleepDelay($this->request->getRemoteAddress(), $action);
$this->throttler->registerAttempt($action, $this->request->getRemoteAddress());
}
/**
* FIXME: Use DI once available
* Checks if app is enabled (also includes a check whether user is allowed to access the resource)

19
lib/public/AppFramework/Http/Response.php
View File

@ -81,6 +81,8 @@ class Response {
/** @var ContentSecurityPolicy|null Used Content-Security-Policy */
private $contentSecurityPolicy = null;
/** @var bool */
private $throttled = false;
/**
* Caches the response
@ -322,5 +324,22 @@ class Response {
return $this;
}
/**
* Marks the response as to throttle. Will be throttled when the
* @BruteForceProtection annotation is added.
*
* @since 12.0.0
*/
public function throttle() {
$this->throttled = true;
}
/**
* Whether the current response is throttled.
*
* @since 12.0.0
*/
public function isThrottled() {
return $this->throttled;
}
}

133
tests/Core/Controller/LoginControllerTest.php
View File

@ -55,8 +55,6 @@ class LoginControllerTest extends TestCase {
private $logger;
/** @var Manager|\PHPUnit_Framework_MockObject_MockObject */
private $twoFactorManager;
/** @var Throttler|\PHPUnit_Framework_MockObject_MockObject */
private $throttler;
public function setUp() {
parent::setUp();
@ -68,7 +66,6 @@ class LoginControllerTest extends TestCase {
$this->urlGenerator = $this->createMock(IURLGenerator::class);
$this->logger = $this->createMock(ILogger::class);
$this->twoFactorManager = $this->createMock(Manager::class);
$this->throttler = $this->createMock(Throttler::class);
$this->loginController = new LoginController(
'core',
@ -79,8 +76,7 @@ class LoginControllerTest extends TestCase {
$this->userSession,
$this->urlGenerator,
$this->logger,
$this->twoFactorManager,
$this->throttler
$this->twoFactorManager
);
}
@ -287,27 +283,10 @@ class LoginControllerTest extends TestCase {
$password = 'secret';
$loginPageUrl = 'some url';
$this->request
->expects($this->exactly(5))
->method('getRemoteAddress')
->willReturn('192.168.0.1');
$this->request
->expects($this->once())
->method('passesCSRFCheck')
->willReturn(true);
$this->throttler
->expects($this->exactly(2))
->method('sleepDelay')
->with('192.168.0.1');
$this->throttler
->expects($this->once())
->method('getDelay')
->with('192.168.0.1')
->willReturn(0);
$this->throttler
->expects($this->once())
->method('registerAttempt')
->with('login', '192.168.0.1', ['user' => 'MyUserName']);
$this->userManager->expects($this->once())
->method('checkPasswordNoLogging')
->will($this->returnValue(false));
@ -324,6 +303,7 @@ class LoginControllerTest extends TestCase {
->method('deleteUserValue');
$expected = new \OCP\AppFramework\Http\RedirectResponse($loginPageUrl);
$expected->throttle();
$this->assertEquals($expected, $this->loginController->tryLogin($user, $password, ''));
}
@ -340,23 +320,10 @@ class LoginControllerTest extends TestCase {
$password = 'secret';
$indexPageUrl = \OC_Util::getDefaultPageUrl();
$this->request
->expects($this->exactly(2))
->method('getRemoteAddress')
->willReturn('192.168.0.1');
$this->request
->expects($this->once())
->method('passesCSRFCheck')
->willReturn(true);
$this->throttler
->expects($this->once())
->method('sleepDelay')
->with('192.168.0.1');
$this->throttler
->expects($this->once())
->method('getDelay')
->with('192.168.0.1')
->willReturn(200);
$this->userManager->expects($this->once())
->method('checkPasswordNoLogging')
->will($this->returnValue($user));
@ -400,23 +367,10 @@ class LoginControllerTest extends TestCase {
$password = 'secret';
$indexPageUrl = \OC_Util::getDefaultPageUrl();
$this->request
->expects($this->exactly(2))
->method('getRemoteAddress')
->willReturn('192.168.0.1');
$this->request
->expects($this->once())
->method('passesCSRFCheck')
->willReturn(true);
$this->throttler
->expects($this->once())
->method('sleepDelay')
->with('192.168.0.1');
$this->throttler
->expects($this->once())
->method('getDelay')
->with('192.168.0.1')
->willReturn(200);
$this->userManager->expects($this->once())
->method('checkPasswordNoLogging')
->will($this->returnValue($user));
@ -450,23 +404,10 @@ class LoginControllerTest extends TestCase {
$password = 'secret';
$originalUrl = 'another%20url';
$this->request
->expects($this->exactly(2))
->method('getRemoteAddress')
->willReturn('192.168.0.1');
$this->request
->expects($this->once())
->method('passesCSRFCheck')
->willReturn(false);
$this->throttler
->expects($this->once())
->method('sleepDelay')
->with('192.168.0.1');
$this->throttler
->expects($this->once())
->method('getDelay')
->with('192.168.0.1')
->willReturn(200);
$this->userSession->expects($this->once())
->method('isLoggedIn')
->with()
@ -490,23 +431,10 @@ class LoginControllerTest extends TestCase {
$originalUrl = 'another%20url';
$redirectUrl = 'http://localhost/another url';
$this->request
->expects($this->exactly(2))
->method('getRemoteAddress')
->willReturn('192.168.0.1');
$this->request
->expects($this->once())
->method('passesCSRFCheck')
->willReturn(false);
$this->throttler
->expects($this->once())
->method('sleepDelay')
->with('192.168.0.1');
$this->throttler
->expects($this->once())
->method('getDelay')
->with('192.168.0.1')
->willReturn(200);
$this->userSession->expects($this->once())
->method('isLoggedIn')
->with()
@ -534,23 +462,10 @@ class LoginControllerTest extends TestCase {
$originalUrl = 'another%20url';
$redirectUrl = 'http://localhost/another url';
$this->request
->expects($this->exactly(2))
->method('getRemoteAddress')
->willReturn('192.168.0.1');
$this->request
->expects($this->once())
->method('passesCSRFCheck')
->willReturn(true);
$this->throttler
->expects($this->once())
->method('sleepDelay')
->with('192.168.0.1');
$this->throttler
->expects($this->once())
->method('getDelay')
->with('192.168.0.1')
->willReturn(200);
$this->userManager->expects($this->once())
->method('checkPasswordNoLogging')
->with('Jane', $password)
@ -584,23 +499,10 @@ class LoginControllerTest extends TestCase {
$challengeUrl = 'challenge/url';
$provider = $this->getMockBuilder('\OCP\Authentication\TwoFactorAuth\IProvider')->getMock();
$this->request
->expects($this->exactly(2))
->method('getRemoteAddress')
->willReturn('192.168.0.1');
$this->request
->expects($this->once())
->method('passesCSRFCheck')
->willReturn(true);
$this->throttler
->expects($this->once())
->method('sleepDelay')
->with('192.168.0.1');
$this->throttler
->expects($this->once())
->method('getDelay')
->with('192.168.0.1')
->willReturn(200);
$this->userManager->expects($this->once())
->method('checkPasswordNoLogging')
->will($this->returnValue($user));
@ -651,23 +553,10 @@ class LoginControllerTest extends TestCase {
$provider1 = $this->getMockBuilder('\OCP\Authentication\TwoFactorAuth\IProvider')->getMock();
$provider2 = $this->getMockBuilder('\OCP\Authentication\TwoFactorAuth\IProvider')->getMock();
$this->request
->expects($this->exactly(2))
->method('getRemoteAddress')
->willReturn('192.168.0.1');
$this->request
->expects($this->once())
->method('passesCSRFCheck')
->willReturn(true);
$this->throttler
->expects($this->once())
->method('sleepDelay')
->with('192.168.0.1');
$this->throttler
->expects($this->once())
->method('getDelay')
->with('192.168.0.1')
->willReturn(200);
$this->userManager->expects($this->once())
->method('checkPasswordNoLogging')
->will($this->returnValue($user));
@ -731,33 +620,17 @@ class LoginControllerTest extends TestCase {
->method('linkToRoute')
->with('core.login.showLoginForm', ['user' => 'john@doe.com'])
->will($this->returnValue(''));
$this->request
->expects($this->exactly(3))
->method('getRemoteAddress')
->willReturn('192.168.0.1');
$this->request
->expects($this->once())
->method('passesCSRFCheck')
->willReturn(true);
$this->throttler
->expects($this->once())
->method('getDelay')
->with('192.168.0.1')
->willReturn(200);
$this->throttler
->expects($this->once())
->method('sleepDelay')
->with('192.168.0.1');
$this->throttler
->expects($this->once())
->method('registerAttempt')
->with('login', '192.168.0.1', ['user' => 'john@doe.com']);
$this->config->expects($this->never())
->method('deleteUserValue');
$this->userSession->expects($this->never())
->method('createRememberMeToken');
$expected = new RedirectResponse('');
$expected->throttle();
$this->assertEquals($expected, $this->loginController->tryLogin('john@doe.com', 'just wrong', null));
}
}

5
tests/lib/AppFramework/Http/ResponseTest.php
View File

@ -264,4 +264,9 @@ class ResponseTest extends \Test\TestCase {
}
public function testThrottle() {
$this->assertFalse($this->childResponse->isThrottled());
$this->childResponse->throttle();
$this->assertTrue($this->childResponse->isThrottled());
}
}

190
tests/lib/AppFramework/Middleware/Security/BruteForceMiddlewareTest.php Normal file
View File

@ -0,0 +1,190 @@
<?php
/**
* @copyright Copyright (c) 2017 Lukas Reschke <lukas@statuscode.ch>
*
* @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 <http://www.gnu.org/licenses/>.
*
*/
namespace Test\AppFramework\Middleware\Security;
use OC\AppFramework\Middleware\Security\BruteForceMiddleware;
use OC\AppFramework\Utility\ControllerMethodReflector;
use OC\Security\Bruteforce\Throttler;
use OCP\AppFramework\Controller;
use OCP\AppFramework\Http\Response;
use OCP\Http\Client\IResponse;
use OCP\IRequest;
use Test\TestCase;
class BruteForceMiddlewareTest extends TestCase {
/** @var ControllerMethodReflector|\PHPUnit_Framework_MockObject_MockObject */
private $reflector;
/** @var Throttler|\PHPUnit_Framework_MockObject_MockObject */
private $throttler;
/** @var IRequest|\PHPUnit_Framework_MockObject_MockObject */
private $request;
/** @var BruteForceMiddleware */
private $bruteForceMiddleware;
public function setUp() {
parent::setUp();
$this->reflector = $this->createMock(ControllerMethodReflector::class);
$this->throttler = $this->createMock(Throttler::class);
$this->request = $this->createMock(IRequest::class);
$this->bruteForceMiddleware = new BruteForceMiddleware(
$this->reflector,
$this->throttler,
$this->request
);
}
public function testBeforeControllerWithAnnotation() {
$this->reflector
->expects($this->once())
->method('hasAnnotation')
->with('BruteForceProtection')
->willReturn(true);
$this->reflector
->expects($this->once())
->method('getAnnotationParameter')
->with('BruteForceProtection', 'action')
->willReturn('login');
$this->request
->expects($this->once())
->method('getRemoteAddress')
->willReturn('127.0.0.1');
$this->throttler
->expects($this->once())
->method('sleepDelay')
->with('127.0.0.1', 'login');
/** @var Controller|\PHPUnit_Framework_MockObject_MockObject $controller */
$controller = $this->createMock(Controller::class);
$this->bruteForceMiddleware->beforeController($controller, 'testMethod');
}
public function testBeforeControllerWithoutAnnotation() {
$this->reflector
->expects($this->once())
->method('hasAnnotation')
->with('BruteForceProtection')
->willReturn(false);
$this->reflector
->expects($this->never())
->method('getAnnotationParameter');
$this->request
->expects($this->never())
->method('getRemoteAddress');
$this->throttler
->expects($this->never())
->method('sleepDelay');
/** @var Controller|\PHPUnit_Framework_MockObject_MockObject $controller */
$controller = $this->createMock(Controller::class);
$this->bruteForceMiddleware->beforeController($controller, 'testMethod');
}
public function testAfterControllerWithAnnotationAndThrottledRequest() {
/** @var Response|\PHPUnit_Framework_MockObject_MockObject $response */
$response = $this->createMock(Response::class);
$this->reflector
->expects($this->once())
->method('hasAnnotation')
->with('BruteForceProtection')
->willReturn(true);
$response
->expects($this->once())
->method('isThrottled')
->willReturn(true);
$this->reflector
->expects($this->once())
->method('getAnnotationParameter')
->with('BruteForceProtection', 'action')
->willReturn('login');
$this->request
->expects($this->once())
->method('getRemoteAddress')
->willReturn('127.0.0.1');
$this->throttler
->expects($this->once())
->method('sleepDelay')
->with('127.0.0.1', 'login');
$this->throttler
->expects($this->once())
->method('registerAttempt')
->with('login', '127.0.0.1');
/** @var Controller|\PHPUnit_Framework_MockObject_MockObject $controller */
$controller = $this->createMock(Controller::class);
$this->bruteForceMiddleware->afterController($controller, 'testMethod' ,$response);
}
public function testAfterControllerWithAnnotationAndNotThrottledRequest() {
/** @var Response|\PHPUnit_Framework_MockObject_MockObject $response */
$response = $this->createMock(Response::class);
$this->reflector
->expects($this->once())
->method('hasAnnotation')
->with('BruteForceProtection')
->willReturn(true);
$response
->expects($this->once())
->method('isThrottled')
->willReturn(false);
$this->reflector
->expects($this->never())
->method('getAnnotationParameter');
$this->request
->expects($this->never())
->method('getRemoteAddress');
$this->throttler
->expects($this->never())
->method('sleepDelay');
$this->throttler
->expects($this->never())
->method('registerAttempt');
/** @var Controller|\PHPUnit_Framework_MockObject_MockObject $controller */
$controller = $this->createMock(Controller::class);
$this->bruteForceMiddleware->afterController($controller, 'testMethod' ,$response);
}
public function testAfterControllerWithoutAnnotation() {
$this->reflector
->expects($this->once())
->method('hasAnnotation')
->with('BruteForceProtection')
->willReturn(false);
$this->reflector
->expects($this->never())
->method('getAnnotationParameter');
$this->request
->expects($this->never())
->method('getRemoteAddress');
$this->throttler
->expects($this->never())
->method('sleepDelay');
/** @var Controller|\PHPUnit_Framework_MockObject_MockObject $controller */
$controller = $this->createMock(Controller::class);
/** @var Response|\PHPUnit_Framework_MockObject_MockObject $response */
$response = $this->createMock(Response::class);
$this->bruteForceMiddleware->afterController($controller, 'testMethod' ,$response);
}
}

76
tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php
View File

@ -20,8 +20,6 @@
*
*/
namespace Test\AppFramework\Middleware\Security;
use OC\AppFramework\Http;
@ -34,7 +32,6 @@ use OC\AppFramework\Middleware\Security\Exceptions\SecurityException;
use OC\Appframework\Middleware\Security\Exceptions\StrictCookieMissingException;
use OC\AppFramework\Middleware\Security\SecurityMiddleware;
use OC\AppFramework\Utility\ControllerMethodReflector;
use OC\Security\Bruteforce\Throttler;
use OC\Security\CSP\ContentSecurityPolicy;
use OC\Security\CSP\ContentSecurityPolicyManager;
use OC\Security\CSP\ContentSecurityPolicyNonceManager;
@ -54,7 +51,6 @@ use OCP\ISession;
use OCP\IURLGenerator;
use OCP\Security\ISecureRandom;
class SecurityMiddlewareTest extends \Test\TestCase {
/** @var SecurityMiddleware|\PHPUnit_Framework_MockObject_MockObject */
@ -83,8 +79,6 @@ class SecurityMiddlewareTest extends \Test\TestCase {
private $csrfTokenManager;
/** @var ContentSecurityPolicyNonceManager|\PHPUnit_Framework_MockObject_MockObject */
private $cspNonceManager;
/** @var Throttler|\PHPUnit_Framework_MockObject_MockObject */
private $bruteForceThrottler;
protected function setUp() {
parent::setUp();
@ -99,7 +93,6 @@ class SecurityMiddlewareTest extends \Test\TestCase {
$this->contentSecurityPolicyManager = $this->createMock(ContentSecurityPolicyManager::class);
$this->csrfTokenManager = $this->createMock(CsrfTokenManager::class);
$this->cspNonceManager = $this->createMock(ContentSecurityPolicyNonceManager::class);
$this->bruteForceThrottler = $this->getMockBuilder(Throttler::class)->disableOriginalConstructor()->getMock();
$this->middleware = $this->getMiddleware(true, true);
$this->secException = new SecurityException('hey', false);
$this->secAjaxException = new SecurityException('hey', true);
@ -123,8 +116,7 @@ class SecurityMiddlewareTest extends \Test\TestCase {
$isAdminUser,
$this->contentSecurityPolicyManager,
$this->csrfTokenManager,
$this->cspNonceManager,
$this->bruteForceThrottler
$this->cspNonceManager
);
}
@ -657,70 +649,4 @@ class SecurityMiddlewareTest extends \Test\TestCase {
$this->assertEquals($response, $this->middleware->afterController($this->controller, 'test', $response));
}
/**
* @dataProvider dataTestBeforeControllerBruteForce
*/
public function testBeforeControllerBruteForce($bruteForceProtectionEnabled) {
/** @var ControllerMethodReflector|\PHPUnit_Framework_MockObject_MockObject $reader */
$reader = $this->getMockBuilder(ControllerMethodReflector::class)->disableOriginalConstructor()->getMock();
$middleware = new SecurityMiddleware(
$this->request,
$reader,
$this->navigationManager,
$this->urlGenerator,
$this->logger,
$this->session,
'files',
false,
false,
$this->contentSecurityPolicyManager,
$this->csrfTokenManager,
$this->cspNonceManager,
$this->bruteForceThrottler
);
$reader->expects($this->any())->method('hasAnnotation')
->willReturnCallback(
function($annotation) use ($bruteForceProtectionEnabled) {
switch ($annotation) {
case 'BruteForceProtection':
return $bruteForceProtectionEnabled;
case 'PasswordConfirmationRequired':
case 'StrictCookieRequired':
return false;
case 'PublicPage':
case 'NoCSRFRequired':
return true;
}
return true;
}
);
$reader->expects($this->any())->method('getAnnotationParameter')->willReturn('action');
$this->request->expects($this->any())->method('getRemoteAddress')->willReturn('remoteAddress');
if ($bruteForceProtectionEnabled) {
$this->bruteForceThrottler->expects($this->once())->method('sleepDelay')
->with('remoteAddress', 'action');
$this->bruteForceThrottler->expects($this->once())->method('registerAttempt')
->with('action', 'remoteAddress');
} else {
$this->bruteForceThrottler->expects($this->never())->method('sleepDelay');
$this->bruteForceThrottler->expects($this->never())->method('registerAttempt');
}
$middleware->beforeController($this->controller, 'test');
}
public function dataTestBeforeControllerBruteForce() {
return [
[true],
[false]
];
}
}