diff --git a/.drone.star b/.drone.star index 62112b2..b2d141b 100644 --- a/.drone.star +++ b/.drone.star @@ -45,6 +45,9 @@ config = { 'suites': { 'webUIBruteForceProtection': 'webUIBruteForce', }, + 'servers': [ + 'daily-master-qa' + ], 'browsers': [ 'chrome', 'firefox' diff --git a/appinfo/Migrations/Version20191109111104.php b/appinfo/Migrations/Version20191109111104.php new file mode 100644 index 0000000..a4034b2 --- /dev/null +++ b/appinfo/Migrations/Version20191109111104.php @@ -0,0 +1,56 @@ + + * + * @copyright Copyright (c) 2020, ownCloud GmbH + * @license GPL-2.0 + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the Free + * Software Foundation; either version 2 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 General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + * + */ +namespace OCA\brute_force_protection\Migrations; + +use Doctrine\DBAL\Schema\Schema; +use OCP\Migration\ISchemaMigration; + +class Version20191109111104 implements ISchemaMigration { + public function changeSchema(Schema $schema, array $options) { + $prefix = $options['tablePrefix']; + if (!$schema->hasTable("{$prefix}bfp_link_accesses")) { + $table = $schema->createTable("{$prefix}bfp_link_accesses"); + $table->addColumn('id', 'integer', [ + 'autoincrement' => true, + 'unsigned' => true, + 'notnull' => true, + 'length' => 11, + ]); + $table->addColumn('ip', 'string', [ + 'notnull' => true, + 'length' => 64, + ]); + $table->addColumn('link_token', 'string', [ + 'notnull' => true, + 'length' => 64, + ]); + $table->addColumn('attempted_at', 'integer', [ + 'notnull' => true, + ]); + + $table->setPrimaryKey(['id']); + $table->addIndex(['ip'], 'bfp_link_accesses_ip'); + $table->addIndex(['attempted_at'], 'bfp_link_accesses_at'); + } + } +} diff --git a/appinfo/info.xml b/appinfo/info.xml index 433d3d1..8af02ca 100644 --- a/appinfo/info.xml +++ b/appinfo/info.xml @@ -21,11 +21,11 @@ See the [2-Factor Authentication](https://marketplace.owncloud.com/apps/twofacto Prevent attackers from guessing user passwords GPLv2 Semih Serhat Karakaya - 1.0.1 + 1.1.0 BruteForceProtection true - + diff --git a/lib/BruteForceProtectionConfig.php b/lib/BruteForceProtectionConfig.php index 890f412..ee1feaf 100644 --- a/lib/BruteForceProtectionConfig.php +++ b/lib/BruteForceProtectionConfig.php @@ -75,7 +75,7 @@ public function getBruteForceProtectionFailTolerance() { } /** - * Count failed login attempts over how many seconds + * Count failed attempts over how many seconds * * @return int */ diff --git a/lib/Db/FailedLinkAccess.php b/lib/Db/FailedLinkAccess.php new file mode 100644 index 0000000..6429cf1 --- /dev/null +++ b/lib/Db/FailedLinkAccess.php @@ -0,0 +1,48 @@ + + * + * @copyright Copyright (c) 2020, ownCloud GmbH + * @license GPL-2.0 + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the Free + * Software Foundation; either version 2 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 General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + * + */ + +namespace OCA\BruteForceProtection\Db; + +use OCP\AppFramework\Db\Entity; + +/** + * @method int getId() + * @method void setId(\int $id) + * @method string getIp() + * @method void setIp(string $ip) + * @method string getLinkToken() + * @method void setLinkToken(string $linkToken) + * @method int getAttemptedAt() + * @method void setAttemptedAt(int $attemptedAt) + */ +class FailedLinkAccess extends Entity { + + /** @var string $ip */ + protected $ip; + + /** @var string $linkToken */ + protected $linkToken; + + /** @var int $attemptedAt */ + protected $attemptedAt; +} diff --git a/lib/Db/FailedLinkAccessMapper.php b/lib/Db/FailedLinkAccessMapper.php new file mode 100644 index 0000000..a372722 --- /dev/null +++ b/lib/Db/FailedLinkAccessMapper.php @@ -0,0 +1,120 @@ + + * + * @copyright Copyright (c) 2020 ownCloud GmbH + * @license GPL-2.0 + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the Free + * Software Foundation; either version 2 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 General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + * + */ + +namespace OCA\BruteForceProtection\Db; + +use OCP\AppFramework\Db\Mapper; +use OCP\AppFramework\Utility\ITimeFactory; +use OCP\IDBConnection; + +/** + * Class FailedLinkAccessMapper + * @package OCA\BruteForceProtection\Db + */ +class FailedLinkAccessMapper extends Mapper { + + /** + * @var ITimeFactory $timeFactory + */ + protected $timeFactory; + + /** + * @var string $tableName + */ + protected $tableName = 'bfp_link_accesses'; + + /** + * FailedLinkAccessMapper constructor. + * + * @param IDBConnection $db + * @param ITimeFactory $timeFactory + */ + public function __construct( + IDBConnection $db, + ITimeFactory $timeFactory + ) { + parent::__construct($db, $this->tableName); + $this->timeFactory = $timeFactory; + } + + /** + * @param string $token + * @param string $ip + * @param int $thresholdTime the timestamp where attempts will start counting + * @return int + */ + public function getFailedAccessCountForTokenIpCombination($token, $ip, $thresholdTime) { + $builder = $this->db->getQueryBuilder(); + $attempts = $builder->selectAlias($builder->createFunction('COUNT(*)'), 'count') + ->from($this->tableName) + ->where($builder->expr()->gt('attempted_at', $builder->createNamedParameter($thresholdTime))) + ->andWhere($builder->expr()->eq('link_token', $builder->createNamedParameter($token))) + ->andWhere($builder->expr()->eq('ip', $builder->createNamedParameter($ip))) + ->execute() + ->fetch(); + return \intval($attempts['count']); + } + + /** + * @param string $token + * @param string $ip + * @return int|null unix timestamp of the last attempt or null if no prior attempt + */ + public function getLastFailedAccessTimeForTokenIpCombination($token, $ip) { + $builder = $this->db->getQueryBuilder(); + $lastAttempt = $builder->select('attempted_at') + ->from($this->tableName) + ->where($builder->expr()->eq('link_token', $builder->createNamedParameter($token))) + ->andWhere($builder->expr()->eq('ip', $builder->createNamedParameter($ip))) + ->orderBy('attempted_at', 'DESC') + ->setMaxResults(1) + ->execute() + ->fetch(); + return ($lastAttempt === false) ? null : \intval($lastAttempt['attempted_at']); + } + + /** + * @param string $token + * @param string $ip + */ + public function deleteFailedAccessForTokenIpCombination($token, $ip) { + $builder = $this->db->getQueryBuilder(); + $builder->delete($this->tableName) + ->where($builder->expr()->eq('link_token', $builder->createNamedParameter($token))) + ->andWhere($builder->expr()->eq('ip', $builder->createNamedParameter($ip))) + ->execute(); + } + + /** + * It removes entries that were created before the specified threshold seconds. + * + * @param int $threshold the amount of threshold seconds + */ + public function deleteOldFailedAccesses($threshold) { + $builder = $this->db->getQueryBuilder(); + $thresholdTime = $this->timeFactory->getTime() - $threshold; + $builder->delete($this->tableName) + ->where($builder->expr()->lt('attempted_at', $builder->createNamedParameter($thresholdTime))) + ->execute(); + } +} diff --git a/lib/Db/FailedLoginAttemptMapper.php b/lib/Db/FailedLoginAttemptMapper.php index 53962cf..ef747b6 100644 --- a/lib/Db/FailedLoginAttemptMapper.php +++ b/lib/Db/FailedLoginAttemptMapper.php @@ -25,7 +25,6 @@ namespace OCA\BruteForceProtection\Db; use OCP\AppFramework\Db\Mapper; -use OCA\BruteForceProtection\BruteForceProtectionConfig; use OCP\AppFramework\Utility\ITimeFactory; use OCP\IDBConnection; @@ -35,11 +34,6 @@ */ class FailedLoginAttemptMapper extends Mapper { - /** - * @var BruteForceProtectionConfig $config - */ - protected $config; - /** * @var ITimeFactory $timeFactory */ @@ -54,27 +48,24 @@ class FailedLoginAttemptMapper extends Mapper { * FailedLoginAttemptMapper constructor. * * @param IDBConnection $db - * @param BruteForceProtectionConfig $config * @param ITimeFactory $timeFactory */ public function __construct( IDBConnection $db, - BruteForceProtectionConfig $config, ITimeFactory $timeFactory ) { parent::__construct($db, $this->tableName); - $this->config = $config; $this->timeFactory = $timeFactory; } /** * @param string $uid * @param string $ip + * @param int $thresholdTime the timestamp where attempts will start counting * @return int */ - public function getSuspiciousActivityCountForUidIpCombination($uid, $ip) { + public function getFailedLoginCountForUidIpCombination($uid, $ip, $thresholdTime) { $builder = $this->db->getQueryBuilder(); - $thresholdTime = $this->getLastFailedLoginAttemptTimeForUidIpCombination($uid, $ip) - $this->config->getBruteForceProtectionTimeThreshold(); $attempts = $builder->selectAlias($builder->createFunction('COUNT(*)'), 'count') ->from($this->tableName) ->where($builder->expr()->gt('attempted_at', $builder->createNamedParameter($thresholdTime))) @@ -88,28 +79,26 @@ public function getSuspiciousActivityCountForUidIpCombination($uid, $ip) { /** * @param string $uid * @param string $ip - * @return int + * @return int|null unix timestamp of the last attempt or null if no prior attempt */ public function getLastFailedLoginAttemptTimeForUidIpCombination($uid, $ip) { $builder = $this->db->getQueryBuilder(); - $thresholdTime = $this->timeFactory->getTime() - $this->config->getBruteForceProtectionBanPeriod(); $lastAttempt = $builder->select('attempted_at') ->from($this->tableName) - ->where($builder->expr()->gt('attempted_at', $builder->createNamedParameter($thresholdTime))) - ->andWhere($builder->expr()->eq('uid', $builder->createNamedParameter($uid))) + ->where($builder->expr()->eq('uid', $builder->createNamedParameter($uid))) ->andWhere($builder->expr()->eq('ip', $builder->createNamedParameter($ip))) ->orderBy('attempted_at', 'DESC') ->setMaxResults(1) ->execute() ->fetch(); - return ($lastAttempt === false) ? 0 : \intval($lastAttempt['attempted_at']); + return ($lastAttempt === false) ? null : \intval($lastAttempt['attempted_at']); } /** * @param string $uid * @param string $ip */ - public function deleteSuspiciousAttemptsForUidIpCombination($uid, $ip) { + public function deleteFailedLoginAttemptsForUidIpCombination($uid, $ip) { $builder = $this->db->getQueryBuilder(); $builder->delete($this->tableName) ->where($builder->expr()->eq('uid', $builder->createNamedParameter($uid))) @@ -118,7 +107,7 @@ public function deleteSuspiciousAttemptsForUidIpCombination($uid, $ip) { } /** - * It removes entries that created before the specified threshold seconds. + * It removes entries that were created before the specified threshold seconds. * * @param int $threshold the amount of threshold seconds */ diff --git a/lib/Exceptions/LinkAuthException.php b/lib/Exceptions/LinkAuthException.php new file mode 100644 index 0000000..d24fc6f --- /dev/null +++ b/lib/Exceptions/LinkAuthException.php @@ -0,0 +1,27 @@ + + * + * @copyright Copyright (c) 2020, ownCloud GmbH + * @license GPL-2.0 + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the Free + * Software Foundation; either version 2 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 General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + * + */ + +namespace OCA\BruteForceProtection\Exceptions; + +class LinkAuthException extends \Exception { +} diff --git a/lib/Hooks.php b/lib/Hooks.php index 3027645..b4d30cc 100644 --- a/lib/Hooks.php +++ b/lib/Hooks.php @@ -25,8 +25,17 @@ namespace OCA\BruteForceProtection; use OC\User\LoginException; -use OCP\IUserManager; +use OCA\BruteForceProtection\Db\FailedLinkAccess; +use OCA\BruteForceProtection\Db\FailedLinkAccessMapper; +use OCA\BruteForceProtection\Db\FailedLoginAttempt; +use OCA\BruteForceProtection\Db\FailedLoginAttemptMapper; +use OCA\BruteForceProtection\Exceptions\LinkAuthException; +use OCP\AppFramework\Utility\ITimeFactory; use OCP\IRequest; +use OCP\IUser; +use OCP\Share\IShare; +use Symfony\Component\EventDispatcher\EventDispatcherInterface; +use Symfony\Component\EventDispatcher\GenericEvent; /** * Class Hooks @@ -34,64 +43,119 @@ */ class Hooks { - /** @var \OC\User\Manager */ - private $userManager; - - /** @var Throttle*/ + /** @var Throttle */ private $throttle; - /** @var IRequest*/ + /** @var IRequest */ private $request; + /** @var FailedLoginAttemptMapper */ + private $loginAttemptMapper; + + /** @var FailedLinkAccessMapper */ + private $linkAccessMapper; + + /** @var EventDispatcherInterface */ + private $eventDispatcher; + + /** @var ITimeFactory */ + private $timeFactory; + /** - * @param \OC\User\Manager $userManager * @param Throttle $throttle * @param IRequest $request + * @param FailedLoginAttemptMapper $loginAttemptMapper + * @param FailedLinkAccessMapper $linkAccessMapper + * @param EventDispatcherInterface $eventDispatcher + * @param ITimeFactory $timeFactory */ public function __construct( - IUserManager $userManager, Throttle $throttle, - IRequest $request + IRequest $request, + FailedLoginAttemptMapper $loginAttemptMapper, + FailedLinkAccessMapper $linkAccessMapper, + EventDispatcherInterface $eventDispatcher, + ITimeFactory $timeFactory ) { - $this->userManager = $userManager; $this->throttle = $throttle; $this->request = $request; + $this->loginAttemptMapper = $loginAttemptMapper; + $this->linkAccessMapper = $linkAccessMapper; + $this->eventDispatcher = $eventDispatcher; + $this->timeFactory = $timeFactory; } public function register() { - $this->userManager->listen('\OC\User', 'preLogin', function ($uid) { - $this->preLoginCallback($uid); - }); - - $this->userManager->listen('\OC\User', 'failedLogin', function ($uid) { - $this->failedLoginCallback($uid); - }); - - $this->userManager->listen('\OC\User', 'postLogin', function ($user) { - /** @var $user \OCP\IUser */ - $this->postLoginCallback($user->getUID()); - }); + /* Login events */ + $this->eventDispatcher->addListener('user.loginfailed', [$this, 'failedLoginCallback']); + $this->eventDispatcher->addListener('user.afterlogin', [$this, 'postLoginCallback']); + $this->eventDispatcher->addListener('user.beforelogin', [$this, 'preLoginCallback']); + + /* Public link share events */ + $this->eventDispatcher->addListener('share.failedpasswordcheck', [$this, 'failedLinkShareAuthCallback']); + $this->eventDispatcher->addListener('share.afterpasswordcheck', [$this, 'postLinkShareAuthCallback']); + $this->eventDispatcher->addListener('share.beforepasswordcheck', [$this, 'preLinkShareAuthCallback']); } /** - * @param string $uid + * @param GenericEvent $event */ - public function failedLoginCallback($uid) { - $this->throttle->addFailedLoginAttempt($uid, $this->request->getRemoteAddress()); + public function failedLoginCallback($event) { + $uid = $event->getArgument('user'); + $attempt = new FailedLoginAttempt(); + $attempt->setUid($uid); + $attempt->setIp($this->request->getRemoteAddress()); + $attempt->setAttemptedAt($this->timeFactory->getTime()); + $this->loginAttemptMapper->insert($attempt); } /** - * @param string $uid + * @param GenericEvent $event */ - public function postLoginCallback($uid) { - $this->throttle->clearSuspiciousAttemptsForUidIpCombination($uid, $this->request->getRemoteAddress()); + public function postLoginCallback($event) { + /** @var IUser $user */ + $user = $event->getArgument('user'); + $this->loginAttemptMapper->deleteFailedLoginAttemptsForUidIpCombination($user->getUID(), $this->request->getRemoteAddress()); } /** - * @param string $uid + * @param GenericEvent $event * @throws LoginException */ - public function preLoginCallback($uid) { - $this->throttle->applyBruteForcePolicy($uid, $this->request->getRemoteAddress()); + public function preLoginCallback($event) { + $uid = $event->getArgument('login'); + $this->throttle->applyBruteForcePolicyForLogin($uid, $this->request->getRemoteAddress()); + } + + /** + * @param GenericEvent $event + */ + public function failedLinkShareAuthCallback($event) { + /** @var IShare $share */ + $share = $event->getArgument('shareObject'); + $access = new FailedLinkAccess(); + $access->setLinkToken($share->getToken()); + $access->setIp($this->request->getRemoteAddress()); + $access->setAttemptedAt($this->timeFactory->getTime()); + $this->linkAccessMapper->insert($access); + } + + /** + * @param GenericEvent $event + */ + public function postLinkShareAuthCallback($event) { + /** @var IShare $share */ + $share = $event->getArgument('shareObject'); + $this->linkAccessMapper->deleteFailedAccessForTokenIpCombination($share->getToken(), $this->request->getRemoteAddress()); + } + + /** + * @param GenericEvent $event + * @throws LinkAuthException + */ + public function preLinkShareAuthCallback($event) { + /** @var IShare $share */ + $share = $event->getArgument('shareObject'); + $this->throttle->applyBruteForcePolicyForLinkShare($share->getToken(), $this->request->getRemoteAddress()); } } diff --git a/lib/Jobs/ExpireOldAttempts.php b/lib/Jobs/ExpireOldAttempts.php index 4b5d3e5..096b0f9 100644 --- a/lib/Jobs/ExpireOldAttempts.php +++ b/lib/Jobs/ExpireOldAttempts.php @@ -23,25 +23,32 @@ namespace OCA\BruteForceProtection\Jobs; use OC\BackgroundJob\TimedJob; use OCA\BruteForceProtection\BruteForceProtectionConfig; +use OCA\BruteForceProtection\Db\FailedLinkAccessMapper; use OCA\BruteForceProtection\Db\FailedLoginAttemptMapper; class ExpireOldAttempts extends TimedJob { - /** @var FailedLoginAttemptMapper $mapper */ - private $mapper; + /** @var FailedLoginAttemptMapper $failedLoginMapper */ + private $failedLoginMapper; + /** @var FailedLinkAccessMapper $linkAccessMapper */ + private $linkAccessMapper; /** @var BruteForceProtectionConfig $config */ private $config; + public function __construct( - FailedLoginAttemptMapper $mapper, + FailedLoginAttemptMapper $failedLoginMapper, + FailedLinkAccessMapper $linkAccessMapper, BruteForceProtectionConfig $config ) { // Run once a day $this->setInterval(24 * 60 * 60); - $this->mapper = $mapper; + $this->failedLoginMapper = $failedLoginMapper; + $this->linkAccessMapper = $linkAccessMapper; $this->config = $config; } public function run($argument) { $threshold = $this->config->getBruteForceProtectionTimeThreshold() + $this->config->getBruteForceProtectionBanPeriod(); - $this->mapper->deleteOldFailedLoginAttempts($threshold); + $this->failedLoginMapper->deleteOldFailedLoginAttempts($threshold); + $this->linkAccessMapper->deleteOldFailedAccesses($threshold); } } diff --git a/lib/Throttle.php b/lib/Throttle.php index 72c3fa4..e8394d3 100644 --- a/lib/Throttle.php +++ b/lib/Throttle.php @@ -25,8 +25,9 @@ namespace OCA\BruteForceProtection; use OC\User\LoginException; -use OCA\BruteForceProtection\Db\FailedLoginAttempt; +use OCA\BruteForceProtection\Db\FailedLinkAccessMapper; use OCA\BruteForceProtection\Db\FailedLoginAttemptMapper; +use OCA\BruteForceProtection\Exceptions\LinkAuthException; use OCP\AppFramework\Utility\ITimeFactory; use OCP\IL10N; @@ -36,68 +37,57 @@ */ class Throttle { - /** - * @var FailedLoginAttemptMapper $attemptMapper - */ - protected $attemptMapper; + /** @var FailedLoginAttemptMapper $loginAttemptMapper */ + protected $loginAttemptMapper; - /** - * @var BruteForceProtectionConfig $config - */ + /** @var FailedLinkAccessMapper $linkAccessMapper */ + protected $linkAccessMapper; + + /** @var BruteForceProtectionConfig $config */ protected $config; - /** - * @var IL10N $l - */ + /** @var IL10N $l */ protected $l; - /** - * @var ITimeFactory $timeFactory - */ + /** @var ITimeFactory $timeFactory */ protected $timeFactory; /** - * @param FailedLoginAttemptMapper $attemptMapper + * @param FailedLoginAttemptMapper $loginAttemptMapper + * @param FailedLinkAccessMapper $linkAccessMapper * @param BruteForceProtectionConfig $config * @param IL10N $l * @param ITimeFactory $timeFactory */ public function __construct( - FailedLoginAttemptMapper $attemptMapper, + FailedLoginAttemptMapper $loginAttemptMapper, + FailedLinkAccessMapper $linkAccessMapper, BruteForceProtectionConfig $config, IL10N $l, ITimeFactory $timeFactory ) { - $this->attemptMapper = $attemptMapper; + $this->loginAttemptMapper = $loginAttemptMapper; + $this->linkAccessMapper = $linkAccessMapper; $this->config = $config; $this->l = $l; $this->timeFactory = $timeFactory; } - /** - * @param string $uid - * @param string $ip - * @return void - */ - public function addFailedLoginAttempt($uid, $ip) { - $attempt = new FailedLoginAttempt(); - $attempt->setUid($uid); - $attempt->setIp($ip); - $attempt->setAttemptedAt($this->timeFactory->getTime()); - $this->attemptMapper->insert($attempt); - } - /** * @param string $uid * @param string $ip * @throws LoginException + * return void */ - public function applyBruteForcePolicy($uid, $ip) { + public function applyBruteForcePolicyForLogin($uid, $ip) { $banPeriod = $this->config->getBruteForceProtectionBanPeriod(); - $banUntil = $this->attemptMapper->getLastFailedLoginAttemptTimeForUidIpCombination($uid, $ip) + $banPeriod; - if ($this->attemptMapper->getSuspiciousActivityCountForUidIpCombination($uid, $ip) >= - $this->config->getBruteForceProtectionFailTolerance() && - $banUntil > $this->timeFactory->getTime()) { + $lastAttempt = $this->loginAttemptMapper->getLastFailedLoginAttemptTimeForUidIpCombination($uid, $ip); + if ($lastAttempt === null || ($lastAttempt + $banPeriod) <= $this->timeFactory->getTime()) { + return; + } + $thresholdTime = $lastAttempt - $this->config->getBruteForceProtectionTimeThreshold(); + if ($this->loginAttemptMapper->getFailedLoginCountForUidIpCombination($uid, $ip, $thresholdTime) >= + $this->config->getBruteForceProtectionFailTolerance()) { throw new LoginException($this->l->t("Too many failed login attempts. Try again in %s.", [$this->parseBanPeriodForHumans($banPeriod)]) ); @@ -105,21 +95,33 @@ public function applyBruteForcePolicy($uid, $ip) { } /** - * @param string $uid + * @param string $token * @param string $ip - * @return void + * @throws LinkAuthException + * return void */ - public function clearSuspiciousAttemptsForUidIpCombination($uid, $ip) { - $this->attemptMapper->deleteSuspiciousAttemptsForUidIpCombination($uid, $ip); + public function applyBruteForcePolicyForLinkShare($token, $ip) { + $banPeriod = $this->config->getBruteForceProtectionBanPeriod(); + $lastAttempt = $this->linkAccessMapper->getLastFailedAccessTimeForTokenIpCombination($token, $ip) ; + if ($lastAttempt === null || ($lastAttempt + $banPeriod) <= $this->timeFactory->getTime()) { + return; + } + $thresholdTime = $lastAttempt - $this->config->getBruteForceProtectionTimeThreshold(); + if ($this->linkAccessMapper->getFailedAccessCountForTokenIpCombination($token, $ip, $thresholdTime) >= + $this->config->getBruteForceProtectionFailTolerance()) { + throw new LinkAuthException($this->l->t("Too many failed attempts. Try again in %s.", + [$this->parseBanPeriodForHumans($banPeriod)]) + ); + } } /** - * @param int $banPeriod + * @param int $seconds * @return string $banPeriodForHumans */ - private function parseBanPeriodForHumans($banPeriod) { - return ($banPeriod / 60 < 60) - ? $this->l->n(' %n minute', ' %n minutes', (int)\ceil($banPeriod/60)) - : $this->l->n(' %n hour', ' %n hours', (int)\ceil(($banPeriod/60)/60)); + private function parseBanPeriodForHumans($seconds) { + return ($seconds / 60 < 60) + ? $this->l->n(' %n minute', ' %n minutes', (int)\ceil($seconds/60)) + : $this->l->n(' %n hour', ' %n hours', (int)\ceil(($seconds/60)/60)); } } diff --git a/templates/settings-admin.php b/templates/settings-admin.php index e3a6014..9235705 100644 --- a/templates/settings-admin.php +++ b/templates/settings-admin.php @@ -26,16 +26,16 @@ /** @var \OCP\IL10N $l */ script('brute_force_protection', 'settings-admin'); ?> -
-

t('Brute Force Protection')); ?>

- -
-
-
-
-
-
-
- -
-
+
+

t('Brute Force Protection')); ?>

+ +
+
+
+
+
+
+
+ +
+
diff --git a/tests/acceptance/config/behat.yml b/tests/acceptance/config/behat.yml index fae61eb..c608984 100644 --- a/tests/acceptance/config/behat.yml +++ b/tests/acceptance/config/behat.yml @@ -25,6 +25,8 @@ default: - BruteForceProtectionContext: - IpContext: - FeatureContext: *common_feature_context_params + - OccContext: + - PublicWebDavContext: extensions: jarnaiz\JUnitFormatter\JUnitFormatterExtension: diff --git a/tests/acceptance/features/apiBruteForceProtection/bruteforceprotection.feature b/tests/acceptance/features/apiBruteForceProtection/bruteforceprotection.feature index c5ae5cf..397d94a 100644 --- a/tests/acceptance/features/apiBruteForceProtection/bruteforceprotection.feature +++ b/tests/acceptance/features/apiBruteForceProtection/bruteforceprotection.feature @@ -87,3 +87,30 @@ Feature: brute force protection And user "Alice" sends HTTP method "GET" to URL "/remote.php/webdav/welcome.txt" with password "notvalid" And user "Alice" sends HTTP method "GET" to URL "/index.php/apps/files" Then the HTTP status code should be "403" + + @issue-112 + Scenario: access to public link is not blocked after too many invalid requests + Given user "Alice" has uploaded file with content "user1 file" to "/PARENT/randomfile.txt" + When user "Alice" creates a public link share using the sharing API with settings + | path | PARENT | + | password | %public% | + Then the public download of the last publicly shared file using the new public WebDAV API with password "abcdef" should fail with HTTP status code "401" + And the public download of the last publicly shared file using the new public WebDAV API with password "123abc" should fail with HTTP status code "401" + And the public download of the last publicly shared file using the new public WebDAV API with password "abc123" should fail with HTTP status code "500" + And the public download of the last publicly shared file using the new public WebDAV API with password "%public%" should fail with HTTP status code "500" + #And the public download of the last publicly shared file using the new public WebDAV API with password "abc123" should fail with HTTP status code "401" + #And the public download of the last publicly shared file using the new public WebDAV API with password "%public%" should fail with HTTP status code "401" + + @issue-112 + Scenario: access to public link is blocked after too many invalid requests + Given user "Alice" has uploaded file with content "user1 file" to "/randomfile.txt" + When user "Alice" creates a public link share using the sharing API with settings + | path | randomfile.txt | + | password | %public% | + Then the public should be able to download the last publicly shared file using the new public WebDAV API with password "%public%" and the content should be "user1 file" + And the public download of the last publicly shared file using the new public WebDAV API with password "abcdef" should fail with HTTP status code "401" + And the public download of the last publicly shared file using the new public WebDAV API with password "123abc" should fail with HTTP status code "401" + And the public download of the last publicly shared file using the new public WebDAV API with password "abc123" should fail with HTTP status code "500" + And the public download of the last publicly shared file using the new public WebDAV API with password "%public%" should fail with HTTP status code "500" + #And the public download of the last publicly shared file using the new public WebDAV API with password "abc123" should fail with HTTP status code "401" + #And the public download of the last publicly shared file using the new public WebDAV API with password "%public%" should fail with HTTP status code "401" diff --git a/tests/unit/Controller/SettingsControllerTest.php b/tests/unit/Controller/SettingsControllerTest.php index b5e8b14..56d0e87 100644 --- a/tests/unit/Controller/SettingsControllerTest.php +++ b/tests/unit/Controller/SettingsControllerTest.php @@ -39,10 +39,8 @@ class SettingsControllerTest extends TestCase { protected function setUp(): void { parent::setUp(); - $this->request = $this->getMockBuilder(IRequest::class)->getMock(); - $this->config = $this->getMockBuilder(BruteForceProtectionConfig::class) - ->disableOriginalConstructor() - ->getMock(); + $this->request = $this->createMock(IRequest::class); + $this->config = $this->createMock(BruteForceProtectionConfig::class); $this->controller = new SettingsController('brute_force_protection', $this->request, $this->config); } diff --git a/tests/unit/Db/FailedLinkAccessMapperTest.php b/tests/unit/Db/FailedLinkAccessMapperTest.php new file mode 100644 index 0000000..0dea5de --- /dev/null +++ b/tests/unit/Db/FailedLinkAccessMapperTest.php @@ -0,0 +1,149 @@ + + * + * @copyright Copyright (c) 2020, ownCloud GmbH + * @license GPL-2.0 + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the Free + * Software Foundation; either version 2 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 General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + * + */ + +namespace OCA\BruteForceProtection\Tests\Db; + +use OCA\BruteForceProtection\Db\FailedLinkAccess; +use OCA\BruteForceProtection\Db\FailedLinkAccessMapper; +use OCP\AppFramework\Utility\ITimeFactory; +use OCP\IDBConnection; +use Test\TestCase; + +/** + * @group DB + */ +class FailedLinkAccessMapperTest extends TestCase { + + /** @var FailedLinkAccess $mapper*/ + private $mapper; + + /** @var IDBConnection $connection*/ + private $connection; + + /** @var ITimeFactory | \PHPUnit\Framework\MockObject\MockObject $timeFactory */ + private $timeFactoryMock; + + /** @var int $baseTime */ + private $baseTime; + + /** @var string */ + private $dbTable = 'bfp_link_accesses'; + + /** @var int $thresholdConfigVal */ + private $thresholdConfigVal = 60; + + public function setUp(): void { + parent::setUp(); + $this->baseTime = \time(); + $this->timeFactoryMock = $this->createMock(ITimeFactory::class); + $this->connection = \OC::$server->getDatabaseConnection(); + $this->mapper = new FailedLinkAccessMapper($this->connection, $this->timeFactoryMock); + + $query = $this->connection->getQueryBuilder()->select('*')->from($this->dbTable); + $result = $query->execute()->fetchAll(); + $this->assertEmpty($result, 'we need to start with a empty bfp_link_accesses table'); + + $this->addInitialTestEntries(); + } + + public function addInitialTestEntries() { + $linkAccess = new FailedLinkAccess(); + $linkAccess->setLinkToken('token1'); + $linkAccess->setIp("192.168.1.1"); + $linkAccess->setAttemptedAt($this->baseTime+20); + $this->mapper->insert($linkAccess); + + $linkAccess = new FailedLinkAccess(); + $linkAccess->setLinkToken('token1'); + $linkAccess->setIp("192.168.1.1"); + $linkAccess->setAttemptedAt($this->baseTime+60); + $this->mapper->insert($linkAccess); + + $linkAccess = new FailedLinkAccess(); + $linkAccess->setLinkToken('token1'); + $linkAccess->setIp("192.168.1.2"); + $linkAccess->setAttemptedAt($this->baseTime+60); + $this->mapper->insert($linkAccess); + + $linkAccess = new FailedLinkAccess(); + $linkAccess->setLinkToken('token2'); + $linkAccess->setIp("192.168.1.1"); + $linkAccess->setAttemptedAt($this->baseTime+60); + $this->mapper->insert($linkAccess); + + $linkAccess = new FailedLinkAccess(); + $linkAccess->setLinkToken('token1'); + $linkAccess->setIp("192.168.1.1"); + $linkAccess->setAttemptedAt($this->baseTime+100); + $this->mapper->insert($linkAccess); + } + + public function tearDown(): void { + parent::tearDown(); + $query = $this->connection->getQueryBuilder()->delete($this->dbTable); + $query->execute(); + } + + public function testGetFailedAccessCountForTokenIpCombination() { + $this->assertEquals(3, $this->mapper->getFailedAccessCountForTokenIpCombination('token1', '192.168.1.1', $this->baseTime)); + $this->assertEquals(1, $this->mapper->getFailedAccessCountForTokenIpCombination('token1', '192.168.1.2', $this->baseTime)); + $this->assertEquals(1, $this->mapper->getFailedAccessCountForTokenIpCombination('token2', '192.168.1.1', $this->baseTime)); + } + + public function testGetLastFailedAccessTimeForTokenIpCombination() { + $lastAttemptTime = $this->baseTime+100; + $this->assertEquals($this->mapper->getLastFailedAccessTimeForTokenIpCombination('token1', '192.168.1.1'), $lastAttemptTime); + } + + public function testDeleteFailedAccessForTokenIpCombination() { + $builder = $this->connection->getQueryBuilder(); + + $query = $builder->select('*')->from($this->dbTable) + ->Where($builder->expr()->eq('ip', $builder->createNamedParameter("192.168.1.1"))) + ->andWhere($builder->expr()->eq('link_token', $builder->createNamedParameter("token1"))); + $result = $query->execute()->fetchAll(); + $this->assertCount(3, $result); + + $this->mapper->deleteFailedAccessForTokenIpCombination('token1', "192.168.1.1"); + + $query = $builder->select('*')->from($this->dbTable) + ->Where($builder->expr()->eq('ip', $builder->createNamedParameter("192.168.1.1"))) + ->andWhere($builder->expr()->eq('link_token', $builder->createNamedParameter("token1"))); + $result = $query->execute()->fetchAll(); + $this->assertCount(0, $result); + } + + public function testDeleteOldFailedLoginAttempts() { + $builder = $this->connection->getQueryBuilder(); + $functionCallTime = $this->baseTime+130; + $this->timeFactoryMock->method('getTime') + ->willReturn($functionCallTime); + $query = $builder->select('*')->from($this->dbTable); + $result = $query->execute()->fetchAll(); + $this->assertCount(5, $result); + $this->mapper->deleteOldFailedAccesses(60); + $query = $builder->select('*')->from($this->dbTable); + $result = $query->execute()->fetchAll(); + $this->assertCount(1, $result); + } +} diff --git a/tests/unit/Db/FailedLoginAttemptMapperTest.php b/tests/unit/Db/FailedLoginAttemptMapperTest.php index 434221c..3629af9 100644 --- a/tests/unit/Db/FailedLoginAttemptMapperTest.php +++ b/tests/unit/Db/FailedLoginAttemptMapperTest.php @@ -24,7 +24,6 @@ namespace OCA\BruteForceProtection\Tests\Db; -use OCA\BruteForceProtection\BruteForceProtectionConfig; use OCA\BruteForceProtection\Db\FailedLoginAttempt; use OCA\BruteForceProtection\Db\FailedLoginAttemptMapper; use OCP\AppFramework\Utility\ITimeFactory; @@ -43,9 +42,6 @@ class FailedLoginAttemptMapperTest extends TestCase { /** @var IDBConnection $connection*/ private $connection; - /** @var BruteForceProtectionConfig | MockObject $configMock */ - private $configMock; - /** @var ITimeFactory | MockObject $timeFactory */ private $timeFactoryMock; @@ -55,20 +51,12 @@ class FailedLoginAttemptMapperTest extends TestCase { /** @var string */ private $dbTable = 'bfp_failed_logins'; - /** @var int $thresholdConfigVal */ - private $thresholdConfigVal = 60; - public function setUp(): void { parent::setUp(); $this->baseTime = \time(); - $this->timeFactoryMock = $this->getMockBuilder(ITimeFactory::class) - ->disableOriginalConstructor() - ->getMock(); + $this->timeFactoryMock = $this->createMock(ITimeFactory::class); $this->connection = \OC::$server->getDatabaseConnection(); - $this->configMock = $this->getMockBuilder(BruteForceProtectionConfig::class) - ->disableOriginalConstructor() - ->getMock(); - $this->mapper = new FailedLoginAttemptMapper($this->connection, $this->configMock, $this->timeFactoryMock); + $this->mapper = new FailedLoginAttemptMapper($this->connection, $this->timeFactoryMock); $query = $this->connection->getQueryBuilder()->select('*')->from($this->dbTable); $result = $query->execute()->fetchAll(); @@ -116,31 +104,17 @@ public function tearDown(): void { } public function testGetSuspiciousActivityCountForUidIpCombination() { - $functionCallTime = $this->baseTime+110; - $this->configMock->expects($this->exactly(3)) - ->method('getBruteForceProtectionTimeThreshold') - ->willReturn($this->thresholdConfigVal); - $this->timeFactoryMock->expects($this->exactly(3)) - ->method('getTime') - ->willReturn($functionCallTime); - $this->assertEquals(3, $this->mapper->getSuspiciousActivityCountForUidIpCombination('test1', '192.168.1.1')); - $this->assertEquals(1, $this->mapper->getSuspiciousActivityCountForUidIpCombination('test1', '192.168.1.2')); - $this->assertEquals(1, $this->mapper->getSuspiciousActivityCountForUidIpCombination('test2', '192.168.1.1')); + $this->assertEquals(3, $this->mapper->getFailedLoginCountForUidIpCombination('test1', '192.168.1.1', $this->baseTime)); + $this->assertEquals(1, $this->mapper->getFailedLoginCountForUidIpCombination('test1', '192.168.1.2', $this->baseTime)); + $this->assertEquals(1, $this->mapper->getFailedLoginCountForUidIpCombination('test2', '192.168.1.1', $this->baseTime)); } public function testGetLastFailedLoginAttemptTimeForUidIpCombination() { $lastAttemptTime = $this->baseTime+100; - $this->configMock->expects($this->once()) - ->method('getBruteForceProtectionBanPeriod') - ->willReturn('250'); - $this->timeFactoryMock->expects($this->once()) - ->method('getTime') - ->willReturn($this->baseTime+300); - $this->assertEquals($this->mapper->getLastFailedLoginAttemptTimeForUidIpCombination('test1', '192.168.1.1'), $lastAttemptTime); } - public function testDeleteSuspiciousAttemptsForUidIpCombination() { + public function testDeleteFailedLoginAttemptsForUidIpCombination() { $builder = $this->connection->getQueryBuilder(); $query = $builder->select('*')->from($this->dbTable) @@ -149,7 +123,7 @@ public function testDeleteSuspiciousAttemptsForUidIpCombination() { $result = $query->execute()->fetchAll(); $this->assertCount(3, $result); - $this->mapper->deleteSuspiciousAttemptsForUidIpCombination('test1', "192.168.1.1"); + $this->mapper->deleteFailedLoginAttemptsForUidIpCombination('test1', "192.168.1.1"); $query = $builder->select('*')->from($this->dbTable) ->Where($builder->expr()->eq('ip', $builder->createNamedParameter("192.168.1.1"))) @@ -161,13 +135,12 @@ public function testDeleteSuspiciousAttemptsForUidIpCombination() { public function testDeleteOldFailedLoginAttempts() { $builder = $this->connection->getQueryBuilder(); $functionCallTime = $this->baseTime+130; - $this->timeFactoryMock->expects($this->exactly(1)) - ->method('getTime') + $this->timeFactoryMock->method('getTime') ->willReturn($functionCallTime); $query = $builder->select('*')->from($this->dbTable); $result = $query->execute()->fetchAll(); $this->assertCount(5, $result); - $this->mapper->deleteOldFailedLoginAttempts($this->thresholdConfigVal); + $this->mapper->deleteOldFailedLoginAttempts(60); $query = $builder->select('*')->from($this->dbTable); $result = $query->execute()->fetchAll(); $this->assertCount(1, $result); diff --git a/tests/unit/HooksTest.php b/tests/unit/HooksTest.php index 53a2aff..c23cfb2 100644 --- a/tests/unit/HooksTest.php +++ b/tests/unit/HooksTest.php @@ -24,76 +24,111 @@ namespace OCA\BruteForceProtection\Tests; -use OC\User\Manager; +use OCA\BruteForceProtection\Db\FailedLinkAccessMapper; +use OCA\BruteForceProtection\Db\FailedLoginAttemptMapper; use OCA\BruteForceProtection\Hooks; use OCA\BruteForceProtection\Throttle; +use OCP\AppFramework\Utility\ITimeFactory; use OCP\IRequest; +use OCP\IUser; +use OCP\Share; +use PHPUnit\Framework\MockObject\MockObject; +use Symfony\Component\EventDispatcher\EventDispatcherInterface; +use Symfony\Component\EventDispatcher\GenericEvent; use Test\TestCase; class HooksTest extends TestCase { /** @var Hooks */ private $hooks; - /** - * @var \PHPUnit\Framework\MockObject\MockObject | Manager - */ - private $userManagerMock; - /** - * @var \PHPUnit\Framework\MockObject\MockObject | Throttle - */ + + /** @var MockObject | Throttle */ private $throttleMock; - /** - * @var \PHPUnit\Framework\MockObject\MockObject | IRequest - */ + + /** @var MockObject | IRequest */ private $requestMock; - public function setUp(): void { - parent::setUp(); + /** @var MockObject | FailedLoginAttemptMapper */ + private $loginAttemptMapperMock; + + /** @var MockObject | FailedLinkAccessMapper */ + private $linkAccessMapperMock; - $this->userManagerMock = $this->getMockBuilder('\OC\User\Manager') - ->disableOriginalConstructor() - ->getMock(); + /** @var MockObject | EventDispatcherInterface */ + private $eventDispatcherMock; - $this->throttleMock = $this->getMockBuilder('OCA\BruteForceProtection\Throttle') - ->disableOriginalConstructor() - ->getMock(); - $this->requestMock = $this->getMockBuilder('OCP\IRequest') - ->disableOriginalConstructor() - ->getMock(); + /** @var MockObject | ITimeFactory */ + private $timeFactoryMock; + + public function setUp(): void { + parent::setUp(); + $this->throttleMock = $this->createMock(Throttle::class); + $this->requestMock = $this->createMock(IRequest::class); + $this->loginAttemptMapperMock = $this->createMock(FailedLoginAttemptMapper::class); + $this->linkAccessMapperMock = $this->createMock(FailedLinkAccessMapper::class); + $this->eventDispatcherMock = $this->createMock(EventDispatcherInterface::class); + $this->timeFactoryMock = $this->createMock(ITimeFactory::class); $this->hooks = new Hooks( - $this->userManagerMock, $this->throttleMock, - $this->requestMock); + $this->requestMock, + $this->loginAttemptMapperMock, + $this->linkAccessMapperMock, + $this->eventDispatcherMock, + $this->timeFactoryMock + ); } public function testRegister() { - $this->userManagerMock->expects($this->exactly(3)) - ->method('listen'); + $this->eventDispatcherMock->expects($this->exactly(6)) + ->method('addListener'); $this->hooks->register(); } public function testFailedLoginCallback() { - $this->throttleMock->expects($this->once()) - ->method('addFailedLoginAttempt'); - - $this->hooks->failedLoginCallback("test"); - $this->assertTrue(true); + $event = new GenericEvent(null, ['user' => 'test']); + $this->loginAttemptMapperMock->expects($this->once()) + ->method('insert'); + $this->hooks->failedLoginCallback($event); } public function testPostLoginCallback() { - $this->throttleMock->expects($this->once()) - ->method('clearSuspiciousAttemptsForUidIpCombination'); - - $this->hooks->postLoginCallback("test"); - $this->assertTrue(true); + $mockUser = $this->createMock(IUser::class); + $mockUser->method('getUID')->willReturn('test'); + $event = new GenericEvent(null, ['user' => $mockUser]); + $this->loginAttemptMapperMock->expects($this->once()) + ->method('deleteFailedLoginAttemptsForUidIpCombination'); + $this->hooks->postLoginCallback($event); } public function testPreLoginCallback() { + $event = new GenericEvent(null, ['login' => 'test']); $this->throttleMock->expects($this->once()) - ->method('applyBruteForcePolicy'); + ->method('applyBruteForcePolicyForLogin'); + $this->hooks->preLoginCallback($event); + } - $this->hooks->preLoginCallback('test'); - $this->assertTrue(true); + public function testFailedLinkShareAuthCallback() { + $share = $this->createMock('OCP\Share\IShare'); + $event = new GenericEvent(null, ['shareObject' => $share]); + $this->linkAccessMapperMock->expects($this->once()) + ->method('insert'); + $this->hooks->failedLinkShareAuthCallback($event); + } + + public function testPostLinkShareAuthCallback() { + $share = $this->createMock('OCP\Share\IShare'); + $event = new GenericEvent(null, ['shareObject' => $share]); + $this->linkAccessMapperMock->expects($this->once()) + ->method('deleteFailedAccessForTokenIpCombination'); + $this->hooks->postLinkShareAuthCallback($event); + } + + public function testPreLinkShareAuthCallback() { + $share = $this->createMock('OCP\Share\IShare'); + $event = new GenericEvent(null, ['shareObject' => $share]); + $this->throttleMock->expects($this->once()) + ->method('applyBruteForcePolicyForLinkShare'); + $this->hooks->preLinkShareAuthCallback($event); } } diff --git a/tests/unit/Jobs/ExpireOldAttemptsTest.php b/tests/unit/Jobs/ExpireOldAttemptsTest.php index 443b0f4..b74067c 100644 --- a/tests/unit/Jobs/ExpireOldAttemptsTest.php +++ b/tests/unit/Jobs/ExpireOldAttemptsTest.php @@ -22,6 +22,7 @@ */ namespace OCA\BruteForceProtection\Tests\Jobs; use OCA\BruteForceProtection\BruteForceProtectionConfig; +use OCA\BruteForceProtection\Db\FailedLinkAccessMapper; use OCA\BruteForceProtection\Db\FailedLoginAttemptMapper; use OCA\BruteForceProtection\Jobs\ExpireOldAttempts; use Test\TestCase; @@ -34,7 +35,9 @@ */ class ExpireOldAttemptsTest extends TestCase { /** @var FailedLoginAttemptMapper | \PHPUnit\Framework\MockObject\MockObject $mapper */ - private $mapper; + private $failedLoginMapper; + /** @var FailedLinkAccessMapper | \PHPUnit\Framework\MockObject\MockObject $mapper */ + private $linkAccessMapper; /** @var BruteForceProtectionConfig | \PHPUnit\Framework\MockObject\MockObject $config */ private $config; /** @var int $thresholdConfigVal */ @@ -43,27 +46,33 @@ class ExpireOldAttemptsTest extends TestCase { private $banPeriodConfigVal = 300; /** @var ExpireOldAttempts $expireAttempts */ private $expireAttempts; + public function setUp(): void { - $this->mapper = $this->getMockBuilder(FailedLoginAttemptMapper::class) - ->disableOriginalConstructor() - ->getMock(); - $this->config = $this->getMockBuilder(BruteForceProtectionConfig::class) - ->disableOriginalConstructor() - ->getMock(); - $this->expireAttempts = new ExpireOldAttempts($this->mapper, $this->config); + $this->failedLoginMapper = $this->createMock(FailedLoginAttemptMapper::class); + $this->linkAccessMapper = $this->createMock(FailedLinkAccessMapper::class); + $this->config = $this->createMock(BruteForceProtectionConfig::class); + $this->expireAttempts = new ExpireOldAttempts( + $this->failedLoginMapper, + $this->linkAccessMapper, + $this->config + ); } public function testExecute() { /** @var \OC\BackgroundJob\JobList $jobList */ $argument = []; + $threshold = $this->thresholdConfigVal+$this->banPeriodConfigVal; $this->config->expects($this->exactly(1)) ->method('getBruteForceProtectionTimeThreshold') ->willReturn($this->thresholdConfigVal); $this->config->expects($this->exactly(1)) ->method('getBruteForceProtectionBanPeriod') ->willReturn($this->banPeriodConfigVal); - $this->mapper->expects($this->exactly(1)) + $this->failedLoginMapper->expects($this->exactly(1)) ->method('deleteOldFailedLoginAttempts') - ->with($this->thresholdConfigVal+$this->banPeriodConfigVal); + ->with($threshold); + $this->linkAccessMapper->expects($this->exactly(1)) + ->method('deleteOldFailedAccesses') + ->with($threshold); $this->expireAttempts->run($argument); } } diff --git a/tests/unit/ThrottleTest.php b/tests/unit/ThrottleTest.php index 025af7c..b1766a0 100644 --- a/tests/unit/ThrottleTest.php +++ b/tests/unit/ThrottleTest.php @@ -24,66 +24,55 @@ namespace OCA\BruteForceProtection\Tests; +use OCA\BruteForceProtection\Db\FailedLinkAccessMapper; use OCA\BruteForceProtection\Db\FailedLoginAttemptMapper; use OCA\BruteForceProtection\Throttle; use OCA\BruteForceProtection\BruteForceProtectionConfig; use OCP\AppFramework\Utility\ITimeFactory; use OCP\IL10N; +use PHPUnit\Framework\MockObject\MockObject; use Test\TestCase; class ThrottleTest extends TestCase { /** @var Throttle */ private $throttle; - /** - * @var \PHPUnit\Framework\MockObject\MockObject | FailedLoginAttemptMapper - */ - private $attemptMapper; - /** - * @var \PHPUnit\Framework\MockObject\MockObject | BruteForceProtectionConfig - */ + + /** @var MockObject | FailedLoginAttemptMapper */ + private $loginAttemptMapper; + + /** @var MockObject | FailedLinkAccessMapper */ + private $linkAccessMapper; + + /** @var MockObject | BruteForceProtectionConfig */ private $configMock; - /** - * @var \PHPUnit\Framework\MockObject\MockObject | IL10N - */ + + /** @var MockObject | IL10N */ private $lMock; - /** - * @var \PHPUnit\Framework\MockObject\MockObject | ITimeFactory - */ + + /** @var MockObject | ITimeFactory */ private $timeFactoryMock; public function setUp(): void { parent::setUp(); - $this->attemptMapper = $this->getMockBuilder(FailedLoginAttemptMapper::class) - ->disableOriginalConstructor() - ->getMock(); - $this->lMock = $this->getMockBuilder(IL10N::class) - ->disableOriginalConstructor() - ->getMock(); - $this->timeFactoryMock = $this->getMockBuilder(ITimeFactory::class) - ->disableOriginalConstructor() - ->getMock(); - $this->configMock = $this->getMockBuilder(BruteForceProtectionConfig::class) - ->disableOriginalConstructor() - ->getMock(); + $this->loginAttemptMapper = $this->createMock(FailedLoginAttemptMapper::class); + $this->linkAccessMapper = $this->createMock(FailedLinkAccessMapper::class); + $this->lMock = $this->createMock(IL10N::class); + $this->timeFactoryMock = $this->createMock(ITimeFactory::class); + $this->configMock = $this->createMock(BruteForceProtectionConfig::class); $this->throttle = new Throttle( - $this->attemptMapper, + $this->loginAttemptMapper, + $this->linkAccessMapper, $this->configMock, $this->lMock, $this->timeFactoryMock ); } - public function testAddFailedLoginAttempt() { - $this->attemptMapper->expects($this->once())->method('insert'); - - $this->throttle->addFailedLoginAttempt('test', '192.168.1.1'); - } - /** - * @dataProvider bruteForceTestData + * @dataProvider bruteForceForLoginTestData * @param int $lastAttempt * @param int $attemptCount * @param int $banPeriod @@ -93,12 +82,49 @@ public function testAddFailedLoginAttempt() { public function testApplyBruteForcePolicy($lastAttempt, $attemptCount, $banPeriod, $failTolerance, $time) { $this->expectException(\OC\User\LoginException::class); - $this->attemptMapper->expects($this->once()) + $this->loginAttemptMapper->expects($this->once()) ->method('getLastFailedLoginAttemptTimeForUidIpCombination') ->with('test', '192.168.1.1') ->will($this->returnValue($lastAttempt)); - $this->attemptMapper->expects($this->once()) - ->method('getSuspiciousActivityCountForUidIpCombination') + $this->loginAttemptMapper->expects($this->once()) + ->method('getFailedLoginCountForUidIpCombination') + ->with('test', '192.168.1.1') + ->will($this->returnValue($attemptCount)); + $this->configMock->expects($this->once()) + ->method('getBruteForceProtectionBanPeriod') + ->will($this->returnValue($banPeriod)); + $this->configMock->expects($this->once()) + ->method('getBruteForceProtectionFailTolerance') + ->will($this->returnValue($failTolerance)); + $this->timeFactoryMock->expects($this->once()) + ->method('getTime') + ->will($this->returnValue($time)); + $this->throttle->applyBruteForcePolicyForLogin('test', '192.168.1.1'); + } + public function bruteForceForLoginTestData() { + return [ + [5, 5, 10, 4, 14], + [0, 3, 300, 2, 250] + ]; + } + + /** + * @dataProvider bruteForceForLoginTestData + * @param int $lastAttempt + * @param int $attemptCount + * @param int $banPeriod + * @param int $failTolerance + * @param int $time + */ + public function testApplyBruteForcePolicyForLinkShare($lastAttempt, $attemptCount, $banPeriod, $failTolerance, $time) { + $this->expectException(\OCA\BruteForceProtection\Exceptions\LinkAuthException::class); + + $this->linkAccessMapper->expects($this->once()) + ->method('getLastFailedAccessTimeForTokenIpCombination') + ->with('test', '192.168.1.1') + ->will($this->returnValue($lastAttempt)); + $this->linkAccessMapper->expects($this->once()) + ->method('getFailedAccessCountForTokenIpCombination') ->with('test', '192.168.1.1') ->will($this->returnValue($attemptCount)); $this->configMock->expects($this->once()) @@ -110,9 +136,9 @@ public function testApplyBruteForcePolicy($lastAttempt, $attemptCount, $banPerio $this->timeFactoryMock->expects($this->once()) ->method('getTime') ->will($this->returnValue($time)); - $this->throttle->applyBruteForcePolicy('test', '192.168.1.1'); + $this->throttle->applyBruteForcePolicyForLinkShare('test', '192.168.1.1'); } - public function bruteForceTestData() { + public function bruteForceLinkShareTestData() { return [ [5, 5, 10, 4, 14], [0, 3, 300, 2, 250]