From e39e6d0605421faaaec57b3deba1ac2a1805d22e Mon Sep 17 00:00:00 2001
From: Lukas Reschke <lukas@statuscode.ch>
Date: Wed, 12 Apr 2017 23:00:02 +0200
Subject: [PATCH] Remove expired attempts

Signed-off-by: Lukas Reschke <lukas@statuscode.ch>
---
 .../RateLimiting/Backend/IBackend.php         | 20 +++++++++-------
 .../RateLimiting/Backend/MemoryCache.php      | 24 +++++++++++++++----
 .../RateLimiting/Backend/MemoryCacheTest.php  | 18 ++++++++++----
 3 files changed, 45 insertions(+), 17 deletions(-)

diff --git a/lib/private/Security/RateLimiting/Backend/IBackend.php b/lib/private/Security/RateLimiting/Backend/IBackend.php
index 092c0e7bb8a..9753eb4997c 100644
--- a/lib/private/Security/RateLimiting/Backend/IBackend.php
+++ b/lib/private/Security/RateLimiting/Backend/IBackend.php
@@ -32,19 +32,23 @@ interface IBackend {
 	/**
 	 * Gets the amount of attempts within the last specified seconds
 	 *
-	 * @param string $methodIdentifier
-	 * @param string $userIdentifier
-	 * @param int $seconds
+	 * @param string $methodIdentifier Identifier for the method
+	 * @param string $userIdentifier Identifier for the user
+	 * @param int $seconds Seconds to look back at
 	 * @return int
 	 */
-	public function getAttempts($methodIdentifier, $userIdentifier, $seconds);
+	public function getAttempts($methodIdentifier,
+								$userIdentifier,
+								$seconds);
 
 	/**
 	 * Registers an attempt
 	 *
-	 * @param string $methodIdentifier
-	 * @param string $userIdentifier
-	 * @param int $timestamp
+	 * @param string $methodIdentifier Identifier for the method
+	 * @param string $userIdentifier Identifier for the user
+	 * @param int $period Period in seconds how long this attempt should be stored
 	 */
-	public function registerAttempt($methodIdentifier, $userIdentifier, $timestamp);
+	public function registerAttempt($methodIdentifier,
+									$userIdentifier,
+									$period);
 }
diff --git a/lib/private/Security/RateLimiting/Backend/MemoryCache.php b/lib/private/Security/RateLimiting/Backend/MemoryCache.php
index a0c53335bcf..25595cda4a5 100644
--- a/lib/private/Security/RateLimiting/Backend/MemoryCache.php
+++ b/lib/private/Security/RateLimiting/Backend/MemoryCache.php
@@ -52,7 +52,8 @@ class MemoryCache implements IBackend {
 	 * @param string $userIdentifier
 	 * @return string
 	 */
-	private function hash($methodIdentifier, $userIdentifier) {
+	private function hash($methodIdentifier,
+						  $userIdentifier) {
 		return hash('sha512', $methodIdentifier . $userIdentifier);
 	}
 
@@ -72,7 +73,9 @@ class MemoryCache implements IBackend {
 	/**
 	 * {@inheritDoc}
 	 */
-	public function getAttempts($methodIdentifier, $userIdentifier, $seconds) {
+	public function getAttempts($methodIdentifier,
+								$userIdentifier,
+								$seconds) {
 		$identifier = $this->hash($methodIdentifier, $userIdentifier);
 		$existingAttempts = $this->getExistingAttempts($identifier);
 
@@ -91,10 +94,23 @@ class MemoryCache implements IBackend {
 	/**
 	 * {@inheritDoc}
 	 */
-	public function registerAttempt($methodIdentifier, $userIdentifier, $timestamp) {
+	public function registerAttempt($methodIdentifier,
+									$userIdentifier,
+									$period) {
 		$identifier = $this->hash($methodIdentifier, $userIdentifier);
 		$existingAttempts = $this->getExistingAttempts($identifier);
-		$existingAttempts[] = (string)$timestamp;
+		$currentTime = $this->timeFactory->getTime();
+
+		// Unset all attempts older than $period
+		foreach ($existingAttempts as $key => $attempt) {
+			if(($attempt + $period) < $currentTime) {
+				unset($existingAttempts[$key]);
+			}
+		}
+		$existingAttempts = array_values($existingAttempts);
+
+		// Store the new attempt
+		$existingAttempts[] = (string)$currentTime;
 		$this->cache->set($identifier, json_encode($existingAttempts));
 	}
 }
diff --git a/tests/lib/Security/RateLimiting/Backend/MemoryCacheTest.php b/tests/lib/Security/RateLimiting/Backend/MemoryCacheTest.php
index f00d734661d..34c326e72e1 100644
--- a/tests/lib/Security/RateLimiting/Backend/MemoryCacheTest.php
+++ b/tests/lib/Security/RateLimiting/Backend/MemoryCacheTest.php
@@ -88,6 +88,11 @@ class MemoryCacheTest extends TestCase {
 	}
 
 	public function testRegisterAttemptWithNoAttemptsBefore() {
+		$this->timeFactory
+			->expects($this->once())
+			->method('getTime')
+			->willReturn(123);
+
 		$this->cache
 			->expects($this->once())
 			->method('get')
@@ -101,10 +106,15 @@ class MemoryCacheTest extends TestCase {
 				json_encode(['123'])
 			);
 
-		$this->memoryCache->registerAttempt('Method', 'User', 123);
+		$this->memoryCache->registerAttempt('Method', 'User', 100);
 	}
 
-	public function testRegisterAttempts() {
+	public function testRegisterAttempt() {
+		$this->timeFactory
+			->expects($this->once())
+			->method('getTime')
+			->willReturn(129);
+
 		$this->cache
 			->expects($this->once())
 			->method('get')
@@ -123,8 +133,6 @@ class MemoryCacheTest extends TestCase {
 			->with(
 				'eea460b8d756885099c7f0a4c083bf6a745069ee4a301984e726df58fd4510bffa2dac4b7fd5d835726a6753ffa8343ba31c7e902bbef78fc68c2e743667cb4b',
 				json_encode([
-					'1',
-					'2',
 					'87',
 					'123',
 					'123',
@@ -133,6 +141,6 @@ class MemoryCacheTest extends TestCase {
 				])
 			);
 
-		$this->memoryCache->registerAttempt('Method', 'User', 129);
+		$this->memoryCache->registerAttempt('Method', 'User', 100);
 	}
 }
-- 
GitLab