Merge pull request #35652 from nextcloud/backport/35057/stable23

[stable23] Add brute force protection on all methods wrapped by PublicShareMiddleware
This commit is contained in:
Julien Veyssier 2023-03-15 12:09:32 +01:00 committed by GitHub
commit 9681be3ce6
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 26 additions and 3 deletions

View file

@ -299,7 +299,8 @@ class DIContainer extends SimpleContainer implements IAppContainer {
new OC\AppFramework\Middleware\PublicShare\PublicShareMiddleware(
$c->get(IRequest::class),
$c->get(ISession::class),
$c->get(\OCP\IConfig::class)
$c->get(\OCP\IConfig::class),
$c->get(OC\Security\Bruteforce\Throttler::class)
)
);
$dispatcher->registerMiddleware(

View file

@ -24,6 +24,7 @@
namespace OC\AppFramework\Middleware\PublicShare;
use OC\AppFramework\Middleware\PublicShare\Exceptions\NeedAuthenticationException;
use OC\Security\Bruteforce\Throttler;
use OCP\AppFramework\AuthPublicShareController;
use OCP\AppFramework\Http\NotFoundResponse;
use OCP\AppFramework\Middleware;
@ -42,11 +43,14 @@ class PublicShareMiddleware extends Middleware {
/** @var IConfig */
private $config;
/** @var Throttler */
private $throttler;
public function __construct(IRequest $request, ISession $session, IConfig $config) {
public function __construct(IRequest $request, ISession $session, IConfig $config, Throttler $throttler) {
$this->request = $request;
$this->session = $session;
$this->config = $config;
$this->throttler = $throttler;
}
public function beforeController($controller, $methodName) {
@ -54,6 +58,11 @@ class PublicShareMiddleware extends Middleware {
return;
}
$controllerClassPath = explode('\\', get_class($controller));
$controllerShortClass = end($controllerClassPath);
$bruteforceProtectionAction = $controllerShortClass . '::' . $methodName;
$this->throttler->sleepDelayOrThrowOnMax($this->request->getRemoteAddress(), $bruteforceProtectionAction);
if (!$this->isLinkSharingEnabled()) {
throw new NotFoundException('Link sharing is disabled');
}
@ -79,6 +88,7 @@ class PublicShareMiddleware extends Middleware {
// If authentication succeeds just continue
if ($controller->isAuthenticated()) {
$this->throttle($bruteforceProtectionAction, $token);
return;
}
@ -88,6 +98,7 @@ class PublicShareMiddleware extends Middleware {
throw new NeedAuthenticationException();
}
$this->throttle($bruteforceProtectionAction, $token);
throw new NotFoundException();
}
@ -128,4 +139,10 @@ class PublicShareMiddleware extends Middleware {
return true;
}
private function throttle($bruteforceProtectionAction, $token): void {
$ip = $this->request->getRemoteAddress();
$this->throttler->sleepDelay($ip, $bruteforceProtectionAction);
$this->throttler->registerAttempt($bruteforceProtectionAction, $ip, ['token' => $token]);
}
}

View file

@ -25,6 +25,7 @@ namespace Test\AppFramework\Middleware\PublicShare;
use OC\AppFramework\Middleware\PublicShare\Exceptions\NeedAuthenticationException;
use OC\AppFramework\Middleware\PublicShare\PublicShareMiddleware;
use OC\Security\Bruteforce\Throttler;
use OCP\AppFramework\AuthPublicShareController;
use OCP\AppFramework\Controller;
use OCP\AppFramework\Http\NotFoundResponse;
@ -44,6 +45,8 @@ class PublicShareMiddlewareTest extends \Test\TestCase {
private $session;
/** @var IConfig|\PHPUnit\Framework\MockObject\MockObject */
private $config;
/** @var Throttler|\PHPUnit\Framework\MockObject\MockObject */
private $throttler;
/** @var PublicShareMiddleware */
private $middleware;
@ -55,11 +58,13 @@ class PublicShareMiddlewareTest extends \Test\TestCase {
$this->request = $this->createMock(IRequest::class);
$this->session = $this->createMock(ISession::class);
$this->config = $this->createMock(IConfig::class);
$this->throttler = $this->createMock(Throttler::class);
$this->middleware = new PublicShareMiddleware(
$this->request,
$this->session,
$this->config
$this->config,
$this->throttler
);
}