Merge pull request #38274 from nextcloud/bugfix/noid/reach-max-delay-in-afterController

fix(middleware): Also abort the request when reaching max delay in af…
This commit is contained in:
Joas Schilling 2023-05-16 11:07:04 +02:00 committed by GitHub
commit 5e02def3f4
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 38 additions and 30 deletions

View file

@ -90,32 +90,40 @@ class BruteForceMiddleware extends Middleware {
*/
public function afterController($controller, $methodName, Response $response) {
if ($response->isThrottled()) {
if ($this->reflector->hasAnnotation('BruteForceProtection')) {
$action = $this->reflector->getAnnotationParameter('BruteForceProtection', 'action');
$ip = $this->request->getRemoteAddress();
$this->throttler->sleepDelay($ip, $action);
$this->throttler->registerAttempt($action, $ip, $response->getThrottleMetadata());
} else {
$reflectionMethod = new ReflectionMethod($controller, $methodName);
$attributes = $reflectionMethod->getAttributes(BruteForceProtection::class);
if (!empty($attributes)) {
try {
if ($this->reflector->hasAnnotation('BruteForceProtection')) {
$action = $this->reflector->getAnnotationParameter('BruteForceProtection', 'action');
$ip = $this->request->getRemoteAddress();
$metaData = $response->getThrottleMetadata();
foreach ($attributes as $attribute) {
/** @var BruteForceProtection $protection */
$protection = $attribute->newInstance();
$action = $protection->getAction();
if (!isset($metaData['action']) || $metaData['action'] === $action) {
$this->throttler->sleepDelay($ip, $action);
$this->throttler->registerAttempt($action, $ip, $metaData);
}
}
$this->throttler->registerAttempt($action, $ip, $response->getThrottleMetadata());
$this->throttler->sleepDelayOrThrowOnMax($ip, $action);
} else {
$this->logger->debug('Response for ' . get_class($controller) . '::' . $methodName . ' got bruteforce throttled but has no annotation nor attribute defined.');
$reflectionMethod = new ReflectionMethod($controller, $methodName);
$attributes = $reflectionMethod->getAttributes(BruteForceProtection::class);
if (!empty($attributes)) {
$ip = $this->request->getRemoteAddress();
$metaData = $response->getThrottleMetadata();
foreach ($attributes as $attribute) {
/** @var BruteForceProtection $protection */
$protection = $attribute->newInstance();
$action = $protection->getAction();
if (!isset($metaData['action']) || $metaData['action'] === $action) {
$this->throttler->registerAttempt($action, $ip, $metaData);
$this->throttler->sleepDelayOrThrowOnMax($ip, $action);
}
}
} else {
$this->logger->debug('Response for ' . get_class($controller) . '::' . $methodName . ' got bruteforce throttled but has no annotation nor attribute defined.');
}
}
} catch (MaxDelayReached $e) {
if ($controller instanceof OCSController) {
throw new OCSException($e->getMessage(), Http::STATUS_TOO_MANY_REQUESTS);
}
return new TooManyRequestsResponse();
}
}

View file

@ -157,7 +157,7 @@ class BruteForceMiddlewareTest extends TestCase {
->willReturn('127.0.0.1');
$this->throttler
->expects($this->once())
->method('sleepDelay')
->method('sleepDelayOrThrowOnMax')
->with('127.0.0.1', 'login');
$this->throttler
->expects($this->once())
@ -181,7 +181,7 @@ class BruteForceMiddlewareTest extends TestCase {
->method('getRemoteAddress');
$this->throttler
->expects($this->never())
->method('sleepDelay');
->method('sleepDelayOrThrowOnMax');
$this->throttler
->expects($this->never())
->method('registerAttempt');
@ -209,7 +209,7 @@ class BruteForceMiddlewareTest extends TestCase {
->willReturn('::1');
$this->throttler
->expects($this->once())
->method('sleepDelay')
->method('sleepDelayOrThrowOnMax')
->with('::1', 'single');
$this->throttler
->expects($this->once())
@ -239,7 +239,7 @@ class BruteForceMiddlewareTest extends TestCase {
->willReturn('::1');
$this->throttler
->expects($this->exactly(2))
->method('sleepDelay')
->method('sleepDelayOrThrowOnMax')
->withConsecutive(
['::1', 'first'],
['::1', 'second'],
@ -275,7 +275,7 @@ class BruteForceMiddlewareTest extends TestCase {
->willReturn('::1');
$this->throttler
->expects($this->once())
->method('sleepDelay')
->method('sleepDelayOrThrowOnMax')
->with('::1', 'second');
$this->throttler
->expects($this->once())
@ -293,7 +293,7 @@ class BruteForceMiddlewareTest extends TestCase {
->method('getRemoteAddress');
$this->throttler
->expects($this->never())
->method('sleepDelay');
->method('sleepDelayOrThrowOnMax');
$controller = new TestController('test', $this->request);
$this->reflector->reflect($controller, 'testMethodWithoutAnnotation');
@ -308,7 +308,7 @@ class BruteForceMiddlewareTest extends TestCase {
->method('getRemoteAddress');
$this->throttler
->expects($this->never())
->method('sleepDelay');
->method('sleepDelayOrThrowOnMax');
$controller = new TestController('test', $this->request);
$this->reflector->reflect($controller, 'testMethodWithoutAnnotation');