From 04b1bb816e8f3f6aa79e147179d53b0e9a8fff04 Mon Sep 17 00:00:00 2001
From: Daniel Hensby <dhensby@silverstripe.com>
Date: Wed, 13 Sep 2017 14:10:55 +0100
Subject: [PATCH 1/6] NEW RateLimiter for Security controller

---
 _config/cache.yml                             |   4 +
 _config/requestprocessors.yml                 |   6 +
 _config/routes.yml                            |   2 +-
 .../Middleware/RateLimitMiddleware.php        |  72 +++++++
 src/Core/Cache/RateLimiter.php                | 190 ++++++++++++++++++
 src/Dev/SapphireTest.php                      |   2 +-
 6 files changed, 274 insertions(+), 2 deletions(-)
 create mode 100644 src/Control/Middleware/RateLimitMiddleware.php
 create mode 100644 src/Core/Cache/RateLimiter.php

diff --git a/_config/cache.yml b/_config/cache.yml
index 20bc7c77d..4eca4560d 100644
--- a/_config/cache.yml
+++ b/_config/cache.yml
@@ -18,3 +18,7 @@ SilverStripe\Core\Injector\Injector:
       factory: SilverStripe\Core\Cache\CacheFactory
       constructor:
         namespace: "VersionProvider_composerlock"
+  Psr\SimpleCache\CacheInterface.RateLimiter:
+      factory: SilverStripe\Core\Cache\CacheFactory
+      constructor:
+        namespace: 'ratelimiter'
diff --git a/_config/requestprocessors.yml b/_config/requestprocessors.yml
index 86c2c21c3..f5f4d4df8 100644
--- a/_config/requestprocessors.yml
+++ b/_config/requestprocessors.yml
@@ -17,6 +17,12 @@ SilverStripe\Core\Injector\Injector:
   SilverStripe\Control\Middleware\TrustedProxyMiddleware:
     properties:
       TrustedProxyIPs: '`SS_TRUSTED_PROXY_IPS`'
+  RateLimitedSecurityController:
+    class: SilverStripe\Control\Middleware\RequestHandlerMiddlewareAdapter
+    properties:
+      RequestHandler: '%$SilverStripe\Security\Security'
+      Middlewares:
+        - '%$SilverStripe\Control\Middleware\RateLimitMiddleware'
 ---
 Name: errorrequestprocessors
 After:
diff --git a/_config/routes.yml b/_config/routes.yml
index 86c35b233..1d9cece41 100644
--- a/_config/routes.yml
+++ b/_config/routes.yml
@@ -11,7 +11,7 @@ After:
 ---
 SilverStripe\Control\Director:
   rules:
-    'Security//$Action/$ID/$OtherID': SilverStripe\Security\Security
+    'Security//$Action/$ID/$OtherID': '%$RateLimitedSecurityController'
     'CMSSecurity//$Action/$ID/$OtherID': SilverStripe\Security\CMSSecurity
     'dev': SilverStripe\Dev\DevelopmentAdmin
     'interactive': SilverStripe\Dev\SapphireREPL
diff --git a/src/Control/Middleware/RateLimitMiddleware.php b/src/Control/Middleware/RateLimitMiddleware.php
new file mode 100644
index 000000000..2e07c8333
--- /dev/null
+++ b/src/Control/Middleware/RateLimitMiddleware.php
@@ -0,0 +1,72 @@
+<?php
+
+namespace SilverStripe\Control\Middleware;
+
+use SilverStripe\Control\HTTPRequest;
+use SilverStripe\Control\HTTPResponse;
+use SilverStripe\Core\Cache\RateLimiter;
+use SilverStripe\ErrorPage\ErrorPage;
+use SilverStripe\ORM\FieldType\DBDatetime;
+use SilverStripe\Security\Security;
+
+class RateLimitMiddleware implements HTTPMiddleware
+{
+
+    /**
+     * @param HTTPRequest $request
+     * @param callable $delegate
+     * @return HTTPResponse
+     */
+    public function process(HTTPRequest $request, callable $delegate)
+    {
+        $limiter = new RateLimiter($this->getKeyFromRequest($request), 10, 1);
+        if ($limiter->canAccess()) {
+            $limiter->hit();
+            $response = $delegate($request);
+        } else {
+            $response = $this->getErrorHTTPResponse();
+        }
+        $this->addHeadersToResponse($response, $limiter);
+        return $response;
+    }
+
+    /**
+     * @param HTTPRequest $request
+     * @return string
+     */
+    protected function getKeyFromRequest($request)
+    {
+        $domain = parse_url($request->getURL(), PHP_URL_HOST);
+        if ($currentUser = Security::getCurrentUser()) {
+            return md5($domain . '-' . $currentUser->ID);
+        }
+        return md5($domain . '-' . $request->getIP());
+    }
+
+    /**
+     * @return HTTPResponse
+     */
+    protected function getErrorHTTPResponse()
+    {
+        $response = null;
+        if (class_exists(ErrorPage::class)) {
+            $response = ErrorPage::response_for(429);
+        }
+        return $response ?: new HTTPResponse('<h1>429 - Too many requests</h1>', 429);
+    }
+
+    /**
+     * @param HTTPResponse $response
+     * @param RateLimiter $limiter
+     */
+    protected function addHeadersToResponse($response, $limiter)
+    {
+        $response->addHeader('X-RateLimit-Limit', $limiter->getMaxAttempts());
+        $response->addHeader('X-RateLimit-Remaining', $remaining = $limiter->getNumAttemptsRemaining());
+        $ttl = $limiter->getTimeToReset();
+        $response->addHeader('X-RateLimit-Reset', DBDatetime::now()->getTimestamp() + $ttl);
+        if ($remaining <= 0) {
+            $response->addHeader('Retry-After', $ttl);
+        }
+    }
+}
diff --git a/src/Core/Cache/RateLimiter.php b/src/Core/Cache/RateLimiter.php
new file mode 100644
index 000000000..b16fc5b4c
--- /dev/null
+++ b/src/Core/Cache/RateLimiter.php
@@ -0,0 +1,190 @@
+<?php
+
+namespace SilverStripe\Core\Cache;
+
+use Psr\SimpleCache\CacheInterface;
+use SilverStripe\Core\Injector\Injector;
+use SilverStripe\ORM\FieldType\DBDatetime;
+
+class RateLimiter
+{
+    /**
+     * @var CacheInterface
+     */
+    private $cache;
+
+    /**
+     * @var string
+     */
+    private $identifier;
+
+    /**
+     * @var int Maximum number of attempts allowed
+     */
+    private $maxAttempts;
+
+    /**
+     * @var int How long the rate limit lasts for
+     */
+    private $decay;
+
+    /**
+     * RateLimiter constructor.
+     * @param string $identifier
+     * @param int $maxAttempts
+     * @param int $decay
+     */
+    public function __construct($identifier, $maxAttempts, $decay)
+    {
+        $this->setIdentifier($identifier);
+        $this->setMaxAttempts($maxAttempts);
+        $this->setDecay($decay);
+    }
+
+    /**
+     * @return CacheInterface
+     */
+    public function getCache()
+    {
+        if (!$this->cache) {
+            $this->setCache(Injector::inst()->create(CacheInterface::class . '.RateLimiter'));
+        }
+        return $this->cache;
+    }
+
+    /**
+     * @param CacheInterface $cache
+     *
+     * @return $this
+     */
+    public function setCache($cache)
+    {
+        $this->cache = $cache;
+        return $this;
+    }
+
+    /**
+     * @return string
+     */
+    public function getIdentifier()
+    {
+        return $this->identifier;
+    }
+
+    /**
+     * @param string $identifier
+     * @return $this
+     */
+    public function setIdentifier($identifier)
+    {
+        $this->identifier = $identifier;
+        return $this;
+    }
+
+    /**
+     * @return int
+     */
+    public function getMaxAttempts()
+    {
+        return $this->maxAttempts;
+    }
+
+    /**
+     * @param int $maxAttempts
+     * @return $this
+     */
+    public function setMaxAttempts($maxAttempts)
+    {
+        $this->maxAttempts = $maxAttempts;
+        return $this;
+    }
+
+    /**
+     * @return int
+     */
+    public function getDecay()
+    {
+        return $this->decay;
+    }
+
+    /**
+     * @param int $decay
+     * @return $this
+     */
+    public function setDecay($decay)
+    {
+        $this->decay = $decay;
+        return $this;
+    }
+
+    /**
+     * @return int
+     */
+    public function getNumAttempts()
+    {
+        return $this->getCache()->get($this->getIdentifier(), 0);
+    }
+
+    /**
+     * @return int
+     */
+    public function getNumAttemptsRemaining()
+    {
+        return max(0, $this->getMaxAttempts() - $this->getNumAttempts());
+    }
+
+    /**
+     * @return int
+     */
+    public function getTimeToReset()
+    {
+        if ($expiry = $this->getCache()->get($this->getIdentifier() . '-timer')) {
+            return $expiry - DBDatetime::now()->getTimestamp();
+        }
+        return  0;
+    }
+
+    /**
+     * @return $this
+     */
+    public function clearAttempts()
+    {
+        $this->getCache()->delete($this->getIdentifier());
+        return $this;
+    }
+
+    /**
+     * Store a hit in the rate limit cache
+     *
+     * @return $this
+     */
+    public function hit()
+    {
+        if (!$this->getCache()->has($this->getIdentifier())) {
+            $ttl = $this->getDecay() * 60;
+            $expiry = DBDatetime::now()->getTimestamp() + $ttl;
+            $this->getCache()->set($this->getIdentifier() . '-timer', $expiry, $ttl);
+        } else {
+            $expiry = $this->getCache()->get($this->getIdentifier() . '-timer');
+            $ttl = $expiry - DBDatetime::now()->getTimestamp();
+        }
+        $this->getCache()->set($this->getIdentifier(), $this->getNumAttempts() + 1, $ttl);
+        return $this;
+    }
+
+    /**
+     * @return bool
+     */
+    public function canAccess()
+    {
+        if ($this->getNumAttempts() >= $this->getMaxAttempts()) {
+            // if the timer cache item still exists then they are locked out
+            if ($this->getCache()->has($this->getIdentifier() . '-timer')) {
+                return false;
+            }
+            // the timer key has expired so we can clear their attempts and start again
+            $this->clearAttempts();
+        }
+        return true;
+    }
+}
diff --git a/src/Dev/SapphireTest.php b/src/Dev/SapphireTest.php
index b8ae2fb48..e052df870 100644
--- a/src/Dev/SapphireTest.php
+++ b/src/Dev/SapphireTest.php
@@ -1090,7 +1090,7 @@ class SapphireTest extends PHPUnit_Framework_TestCase implements TestOnly
      */
     public static function getExtraControllers()
     {
-        return static::$extra_controllers;
+        return array_merge([Security::class], static::$extra_controllers);
     }
 
     /**

From 51ac297c599ade6a964c395f5f631f4f15ba3e5a Mon Sep 17 00:00:00 2001
From: Daniel Hensby <dhensby@silverstripe.com>
Date: Wed, 27 Sep 2017 14:44:38 +0100
Subject: [PATCH 2/6] Fixes to ratelimiter and new features

---
 _config/requestprocessors.yml                 |  8 +-
 _config/routes.yml                            | 10 +-
 src/Control/HTTPRequest.php                   |  8 ++
 .../Middleware/RateLimitMiddleware.php        | 91 +++++++++++++++++--
 src/Core/Cache/RateLimiter.php                |  5 +-
 src/Dev/SapphireTest.php                      |  2 +-
 6 files changed, 110 insertions(+), 14 deletions(-)

diff --git a/_config/requestprocessors.yml b/_config/requestprocessors.yml
index f5f4d4df8..2f86253a1 100644
--- a/_config/requestprocessors.yml
+++ b/_config/requestprocessors.yml
@@ -17,12 +17,18 @@ SilverStripe\Core\Injector\Injector:
   SilverStripe\Control\Middleware\TrustedProxyMiddleware:
     properties:
       TrustedProxyIPs: '`SS_TRUSTED_PROXY_IPS`'
+  SecurityRateLimitMiddleware:
+    class: SilverStripe\Control\Middleware\RateLimitMiddleware
+    properties:
+      ExtraKey: 'Security'
+      MaxAttempts: 10
+      Decay: 1
   RateLimitedSecurityController:
     class: SilverStripe\Control\Middleware\RequestHandlerMiddlewareAdapter
     properties:
       RequestHandler: '%$SilverStripe\Security\Security'
       Middlewares:
-        - '%$SilverStripe\Control\Middleware\RateLimitMiddleware'
+        - '%$SecurityRateLimitMiddleware'
 ---
 Name: errorrequestprocessors
 After:
diff --git a/_config/routes.yml b/_config/routes.yml
index 1d9cece41..cd690748b 100644
--- a/_config/routes.yml
+++ b/_config/routes.yml
@@ -11,10 +11,18 @@ After:
 ---
 SilverStripe\Control\Director:
   rules:
-    'Security//$Action/$ID/$OtherID': '%$RateLimitedSecurityController'
+    'Security//$Action/$ID/$OtherID': SilverStripe\Security\Security
     'CMSSecurity//$Action/$ID/$OtherID': SilverStripe\Security\CMSSecurity
     'dev': SilverStripe\Dev\DevelopmentAdmin
     'interactive': SilverStripe\Dev\SapphireREPL
     'InstallerTest//$Action/$ID/$OtherID': SilverStripe\Dev\InstallerTest
     'SapphireInfo//$Action/$ID/$OtherID': SilverStripe\Dev\SapphireInfo
     'SapphireREPL//$Action/$ID/$OtherID': SilverStripe\Dev\SapphireREPL
+---
+Name: security-limited
+Except:
+  environment: dev
+---
+SilverStripe\Control\Director:
+  rules:
+    'Security//$Action/$ID/$OtherID': '%$RateLimitedSecurityController'
\ No newline at end of file
diff --git a/src/Control/HTTPRequest.php b/src/Control/HTTPRequest.php
index b892a81b9..b77bf0d5e 100644
--- a/src/Control/HTTPRequest.php
+++ b/src/Control/HTTPRequest.php
@@ -774,6 +774,14 @@ class HTTPRequest implements ArrayAccess
         return sizeof($this->dirParts) <= $this->unshiftedButParsedParts;
     }
 
+    /**
+     * @return string Return the host from the request
+     */
+    public function getHost()
+    {
+        return $this->getHeader('host');
+    }
+
     /**
      * Returns the client IP address which originated this request.
      *
diff --git a/src/Control/Middleware/RateLimitMiddleware.php b/src/Control/Middleware/RateLimitMiddleware.php
index 2e07c8333..c1a830ee9 100644
--- a/src/Control/Middleware/RateLimitMiddleware.php
+++ b/src/Control/Middleware/RateLimitMiddleware.php
@@ -5,13 +5,27 @@ namespace SilverStripe\Control\Middleware;
 use SilverStripe\Control\HTTPRequest;
 use SilverStripe\Control\HTTPResponse;
 use SilverStripe\Core\Cache\RateLimiter;
-use SilverStripe\ErrorPage\ErrorPage;
 use SilverStripe\ORM\FieldType\DBDatetime;
 use SilverStripe\Security\Security;
 
 class RateLimitMiddleware implements HTTPMiddleware
 {
 
+    /**
+     * @var string Optional extra data to add to request key generation
+     */
+    private $extraKey;
+
+    /**
+     * @var int Maximum number of attempts within the decay period
+     */
+    private $maxAttempts = 10;
+
+    /**
+     * @var int The decay period (in minutes)
+     */
+    private $decay = 1;
+
     /**
      * @param HTTPRequest $request
      * @param callable $delegate
@@ -19,7 +33,11 @@ class RateLimitMiddleware implements HTTPMiddleware
      */
     public function process(HTTPRequest $request, callable $delegate)
     {
-        $limiter = new RateLimiter($this->getKeyFromRequest($request), 10, 1);
+        $limiter = RateLimiter::create(
+            $this->getKeyFromRequest($request),
+            $this->getMaxAttempts(),
+            $this->getDecay()
+        );
         if ($limiter->canAccess()) {
             $limiter->hit();
             $response = $delegate($request);
@@ -36,11 +54,14 @@ class RateLimitMiddleware implements HTTPMiddleware
      */
     protected function getKeyFromRequest($request)
     {
-        $domain = parse_url($request->getURL(), PHP_URL_HOST);
+        $key = $this->getExtraKey() ? $this->getExtraKey() . '-' : '';
+        $key .= $request->getHost() . '-';
         if ($currentUser = Security::getCurrentUser()) {
-            return md5($domain . '-' . $currentUser->ID);
+            $key .= $currentUser->ID;
+        } else {
+            $key .= $request->getIP();
         }
-        return md5($domain . '-' . $request->getIP());
+        return md5($key);
     }
 
     /**
@@ -48,11 +69,7 @@ class RateLimitMiddleware implements HTTPMiddleware
      */
     protected function getErrorHTTPResponse()
     {
-        $response = null;
-        if (class_exists(ErrorPage::class)) {
-            $response = ErrorPage::response_for(429);
-        }
-        return $response ?: new HTTPResponse('<h1>429 - Too many requests</h1>', 429);
+        return HTTPResponse::create('<h1>429 - Too many requests</h1>', 429);
     }
 
     /**
@@ -69,4 +86,58 @@ class RateLimitMiddleware implements HTTPMiddleware
             $response->addHeader('Retry-After', $ttl);
         }
     }
+
+    /**
+     * @param string $key
+     * @return $this
+     */
+    public function setExtraKey($key)
+    {
+        $this->extraKey = $key;
+        return $this;
+    }
+
+    /**
+     * @return string
+     */
+    public function getExtraKey()
+    {
+        return $this->extraKey;
+    }
+
+    /**
+     * @param int $maxAttempts
+     * @return $this
+     */
+    public function setMaxAttempts($maxAttempts)
+    {
+        $this->maxAttempts = $maxAttempts;
+        return $this;
+    }
+
+    /**
+     * @return int
+     */
+    public function getMaxAttempts()
+    {
+        return $this->maxAttempts;
+    }
+
+    /**
+     * @param int $decay Time in minutes
+     * @return $this
+     */
+    public function setDecay($decay)
+    {
+        $this->decay = $decay;
+        return $this;
+    }
+
+    /**
+     * @return int
+     */
+    public function getDecay()
+    {
+        return $this->decay;
+    }
 }
diff --git a/src/Core/Cache/RateLimiter.php b/src/Core/Cache/RateLimiter.php
index b16fc5b4c..5735b81c3 100644
--- a/src/Core/Cache/RateLimiter.php
+++ b/src/Core/Cache/RateLimiter.php
@@ -3,11 +3,14 @@
 namespace SilverStripe\Core\Cache;
 
 use Psr\SimpleCache\CacheInterface;
+use SilverStripe\Core\Injector\Injectable;
 use SilverStripe\Core\Injector\Injector;
 use SilverStripe\ORM\FieldType\DBDatetime;
 
 class RateLimiter
 {
+    use Injectable;
+
     /**
      * @var CacheInterface
      */
@@ -24,7 +27,7 @@ class RateLimiter
     private $maxAttempts;
 
     /**
-     * @var int How long the rate limit lasts for
+     * @var int How long the rate limit lasts for in minutes
      */
     private $decay;
 
diff --git a/src/Dev/SapphireTest.php b/src/Dev/SapphireTest.php
index e052df870..b8ae2fb48 100644
--- a/src/Dev/SapphireTest.php
+++ b/src/Dev/SapphireTest.php
@@ -1090,7 +1090,7 @@ class SapphireTest extends PHPUnit_Framework_TestCase implements TestOnly
      */
     public static function getExtraControllers()
     {
-        return array_merge([Security::class], static::$extra_controllers);
+        return static::$extra_controllers;
     }
 
     /**

From 5f739c111edbf3e31d0c7b7b3cd1d17ab88e5d32 Mon Sep 17 00:00:00 2001
From: Daniel Hensby <dhensby@silverstripe.com>
Date: Wed, 27 Sep 2017 15:56:44 +0100
Subject: [PATCH 3/6] added ratelimiter tests

---
 .../Middleware/RateLimitMiddleware.php        |  35 ++++-
 src/ORM/FieldType/DBDatetime.php              |  13 +-
 .../Middleware/Control/TestController.php     |  18 +++
 .../Middleware/RateLimitMiddlewareTest.php    |  68 +++++++++
 tests/php/Core/Cache/RateLimiterTest.php      | 129 ++++++++++++++++++
 5 files changed, 252 insertions(+), 11 deletions(-)
 create mode 100644 tests/php/Control/Middleware/Control/TestController.php
 create mode 100644 tests/php/Control/Middleware/RateLimitMiddlewareTest.php
 create mode 100644 tests/php/Core/Cache/RateLimiterTest.php

diff --git a/src/Control/Middleware/RateLimitMiddleware.php b/src/Control/Middleware/RateLimitMiddleware.php
index c1a830ee9..a9350cad3 100644
--- a/src/Control/Middleware/RateLimitMiddleware.php
+++ b/src/Control/Middleware/RateLimitMiddleware.php
@@ -26,6 +26,11 @@ class RateLimitMiddleware implements HTTPMiddleware
      */
     private $decay = 1;
 
+    /**
+     * @var RateLimiter|null
+     */
+    private $rateLimiter;
+
     /**
      * @param HTTPRequest $request
      * @param callable $delegate
@@ -33,11 +38,13 @@ class RateLimitMiddleware implements HTTPMiddleware
      */
     public function process(HTTPRequest $request, callable $delegate)
     {
-        $limiter = RateLimiter::create(
-            $this->getKeyFromRequest($request),
-            $this->getMaxAttempts(),
-            $this->getDecay()
-        );
+        if (!$limiter = $this->getRateLimiter()) {
+            $limiter = RateLimiter::create(
+                $this->getKeyFromRequest($request),
+                $this->getMaxAttempts(),
+                $this->getDecay()
+            );
+        }
         if ($limiter->canAccess()) {
             $limiter->hit();
             $response = $delegate($request);
@@ -140,4 +147,22 @@ class RateLimitMiddleware implements HTTPMiddleware
     {
         return $this->decay;
     }
+
+    /**
+     * @param RateLimiter $rateLimiter
+     * @return $this
+     */
+    public function setRateLimiter($rateLimiter)
+    {
+        $this->rateLimiter = $rateLimiter;
+        return $this;
+    }
+
+    /**
+     * @return RateLimiter|null
+     */
+    public function getRateLimiter()
+    {
+        return $this->rateLimiter;
+    }
 }
diff --git a/src/ORM/FieldType/DBDatetime.php b/src/ORM/FieldType/DBDatetime.php
index cf6c3c3b7..78a5cff7f 100644
--- a/src/ORM/FieldType/DBDatetime.php
+++ b/src/ORM/FieldType/DBDatetime.php
@@ -190,13 +190,14 @@ class DBDatetime extends DBDate implements TemplateGlobalProvider
      */
     public static function set_mock_now($datetime)
     {
-        if ($datetime instanceof DBDatetime) {
-            self::$mock_now = $datetime;
-        } elseif (is_string($datetime)) {
-            self::$mock_now = DBField::create_field('Datetime', $datetime);
-        } else {
-            throw new InvalidArgumentException('DBDatetime::set_mock_now(): Wrong format: ' . $datetime);
+        if (!$datetime instanceof DBDatetime) {
+            $value = $datetime;
+            $datetime = DBField::create_field('Datetime', $datetime);
+            if ($datetime === false) {
+                throw new InvalidArgumentException('DBDatetime::set_mock_now(): Wrong format: ' . $value);
+            }
         }
+        self::$mock_now = $datetime;
     }
 
     /**
diff --git a/tests/php/Control/Middleware/Control/TestController.php b/tests/php/Control/Middleware/Control/TestController.php
new file mode 100644
index 000000000..88b6a04ce
--- /dev/null
+++ b/tests/php/Control/Middleware/Control/TestController.php
@@ -0,0 +1,18 @@
+<?php
+
+namespace SilverStripe\Control\Tests\Middleware\Control;
+
+use SilverStripe\Control\Controller;
+
+class TestController extends Controller
+{
+    public function index($request)
+    {
+        return "Success";
+    }
+
+    public function Link($action = null)
+    {
+        return Controller::join_links('TestController', $action);
+    }
+}
\ No newline at end of file
diff --git a/tests/php/Control/Middleware/RateLimitMiddlewareTest.php b/tests/php/Control/Middleware/RateLimitMiddlewareTest.php
new file mode 100644
index 000000000..899897586
--- /dev/null
+++ b/tests/php/Control/Middleware/RateLimitMiddlewareTest.php
@@ -0,0 +1,68 @@
+<?php
+
+namespace SilverStripe\Control\Tests\Middleware;
+
+use SilverStripe\Control\Middleware\RateLimitMiddleware;
+use SilverStripe\Control\Middleware\RequestHandlerMiddlewareAdapter;
+use SilverStripe\Control\Tests\Middleware\Control\TestController;
+use SilverStripe\Core\Config\Config;
+use SilverStripe\Core\Injector\Injector;
+use SilverStripe\Dev\FunctionalTest;
+use SilverStripe\ORM\FieldType\DBDatetime;
+
+class RateLimitMiddlewareTest extends FunctionalTest
+{
+
+    protected static $extra_controllers = [
+        TestController::class,
+    ];
+
+    protected function setUp()
+    {
+        parent::setUp();
+        DBDatetime::set_mock_now('2017-09-27 00:00:00');
+        Config::modify()->set(Injector::class, 'TestRateLimitMiddleware', [
+            'class' => RateLimitMiddleware::class,
+            'properties' => [
+                'ExtraKey' => 'test',
+                'MaxAttempts' => 2,
+                'Decay' => 1,
+            ],
+        ]);
+        Config::modify()->set(Injector::class, 'RateLimitTestController', [
+            'class' => RequestHandlerMiddlewareAdapter::class,
+            'properties' => [
+                'RequestHandler' => '%$' . TestController::class,
+                'Middlewares' => [
+                    '%$TestRateLimitMiddleware'
+                ],
+            ],
+        ]);
+    }
+
+    protected function getExtraRoutes()
+    {
+        $rules = parent::getExtraRoutes();
+        $rules['TestController//$Action/$ID/$OtherID'] = '%$RateLimitTestController';
+        return $rules;
+    }
+
+    public function testRequest()
+    {
+        $response = $this->get('TestController');
+        $this->assertFalse($response->isError());
+        $this->assertEquals(2, $response->getHeader('X-RateLimit-Limit'));
+        $this->assertEquals(1, $response->getHeader('X-RateLimit-Remaining'));
+        $this->assertEquals(DBDatetime::now()->getTimestamp() + 60, $response->getHeader('X-RateLimit-Reset'));
+        $this->assertEquals('Success', $response->getBody());
+        $response = $this->get('TestController');
+        $this->assertFalse($response->isError());
+        $this->assertEquals(0, $response->getHeader('X-RateLimit-Remaining'));
+        $response = $this->get('TestController');
+        $this->assertTrue($response->isError());
+        $this->assertEquals(429, $response->getStatusCode());
+        $this->assertEquals(60, $response->getHeader('retry-after'));
+        $this->assertNotEquals('Success', $response->getBody());
+    }
+
+}
diff --git a/tests/php/Core/Cache/RateLimiterTest.php b/tests/php/Core/Cache/RateLimiterTest.php
new file mode 100644
index 000000000..113a2e408
--- /dev/null
+++ b/tests/php/Core/Cache/RateLimiterTest.php
@@ -0,0 +1,129 @@
+<?php
+
+namespace SilverStripe\Core\Tests\Cache;
+
+use Psr\SimpleCache\CacheInterface;
+use SilverStripe\Core\Cache\RateLimiter;
+use SilverStripe\Dev\SapphireTest;
+use SilverStripe\ORM\FieldType\DBDatetime;
+use Symfony\Component\Cache\Simple\ArrayCache;
+
+class RateLimiterTest extends SapphireTest
+{
+
+    protected function setUp()
+    {
+        parent::setUp();
+        DBDatetime::set_mock_now('2017-09-27 00:00:00');
+    }
+
+    public function testConstruct()
+    {
+        $cache = new ArrayCache();
+        $rateLimiter = new RateLimiter(
+            'test',
+            5,
+            1
+        );
+        $rateLimiter->setCache($cache);
+        $this->assertEquals('test', $rateLimiter->getIdentifier());
+        $this->assertEquals(5, $rateLimiter->getMaxAttempts());
+        $this->assertEquals(1, $rateLimiter->getDecay());
+    }
+
+    public function testGetNumberOfAttempts()
+    {
+        $cache = new ArrayCache();
+        $rateLimiter = new RateLimiter(
+            'test',
+            5,
+            1
+        );
+        $rateLimiter->setCache($cache);
+        for ($i = 0; $i < 7; ++$i) {
+            $this->assertEquals($i, $rateLimiter->getNumAttempts());
+            $rateLimiter->hit();
+        }
+    }
+
+    public function testGetNumAttemptsRemaining()
+    {
+        $cache = new ArrayCache();
+        $rateLimiter = new RateLimiter(
+            'test',
+            1,
+            1
+        );
+        $rateLimiter->setCache($cache);
+        $this->assertEquals(1, $rateLimiter->getNumAttemptsRemaining());
+        $rateLimiter->hit();
+        $this->assertEquals(0, $rateLimiter->getNumAttemptsRemaining());
+        $rateLimiter->hit();
+        $this->assertEquals(0, $rateLimiter->getNumAttemptsRemaining());
+
+    }
+
+    public function testGetTimeToReset()
+    {
+        $cache = new ArrayCache();
+        $rateLimiter = new RateLimiter(
+            'test',
+            1,
+            1
+        );
+        $rateLimiter->setCache($cache);
+        $this->assertEquals(0, $rateLimiter->getTimeToReset());
+        $rateLimiter->hit();
+        $this->assertEquals(60, $rateLimiter->getTimeToReset());
+        DBDatetime::set_mock_now(DBDatetime::now()->getTimestamp() + 30);
+        $this->assertEquals(30, $rateLimiter->getTimeToReset());
+    }
+
+    public function testClearAttempts()
+    {
+        $cache = new ArrayCache();
+        $rateLimiter = new RateLimiter(
+            'test',
+            1,
+            1
+        );
+        $rateLimiter->setCache($cache);
+        for ($i = 0; $i < 5; ++$i) {
+            $rateLimiter->hit();
+        }
+        $this->assertEquals(5, $rateLimiter->getNumAttempts());
+        $rateLimiter->clearAttempts();
+        $this->assertEquals(0, $rateLimiter->getNumAttempts());
+    }
+
+    public function testHit()
+    {
+        $cache = new ArrayCache();
+        $rateLimiter = new RateLimiter(
+            'test',
+            1,
+            1
+        );
+        $rateLimiter->setCache($cache);
+        $this->assertFalse($cache->has('test'));
+        $this->assertFalse($cache->has('test-timer'));
+        $rateLimiter->hit();
+        $this->assertTrue($cache->has('test'));
+        $this->assertTrue($cache->has('test-timer'));
+    }
+
+    public function testCanAccess()
+    {
+        $cache = new ArrayCache();
+        $rateLimiter = new RateLimiter(
+            'test',
+            1,
+            1
+        );
+        $rateLimiter->setCache($cache);
+        $this->assertTrue($rateLimiter->canAccess());
+        $rateLimiter->hit();
+        $this->assertFalse($rateLimiter->canAccess());
+    }
+
+}
\ No newline at end of file

From c077abf353c3a3757966c098cef621537566d789 Mon Sep 17 00:00:00 2001
From: Daniel Hensby <dhensby@silverstripe.com>
Date: Wed, 27 Sep 2017 17:29:29 +0100
Subject: [PATCH 4/6] DOCS new rate limiting docs

---
 .../09_Security/05_Rate_Limiting.md           | 72 +++++++++++++++++++
 1 file changed, 72 insertions(+)
 create mode 100644 docs/en/02_Developer_Guides/09_Security/05_Rate_Limiting.md

diff --git a/docs/en/02_Developer_Guides/09_Security/05_Rate_Limiting.md b/docs/en/02_Developer_Guides/09_Security/05_Rate_Limiting.md
new file mode 100644
index 000000000..77d26adef
--- /dev/null
+++ b/docs/en/02_Developer_Guides/09_Security/05_Rate_Limiting.md
@@ -0,0 +1,72 @@
+title: Rate Limiting
+summary: SilverStripe's in built rate limiting features
+
+# Rate Limiting
+
+SilverStripe Framework comes with a [Middleware](developer_guides/controllers/middlewares/) that provides rate limiting
+for the Security controller. This provides added protection to a potentially vulnerable part of a SilverStripe application
+where an attacker is free to bombard your login forms or other Security endpoints.
+
+## Applying rate limiting to controllers
+
+You can apply rate limiting to other specific controllers or your entire SilverStripe application. When applying rate
+limiting to other controllers you can define custom limits for each controller.
+
+First, you need to define your rate limit middleware with the required settings:
+
+```yml
+SilverStripe\Core\Injector\Injector:
+  MyRateLimitMiddleware:
+    class: SilverStripe\Control\Middleware\RateLimitMiddleware
+    properties:
+      ExtraKey: 'mylimiter' # this isolates your rate limiter from others
+      MaxAttempts: 10 # how many attempts are allowed in a decay period
+      Decay: 1 # how long the decay period is in minutes
+```
+
+Next, you need to define your request handler which will apply the middleware to the controller:
+
+```yml
+SilverStripe\Core\Injector\Injector:
+  MyRateLimitedController:
+    class: SilverStripe\Control\Middleware\RequestHandlerMiddlewareAdapter
+    properties:
+      RequestHandler: '%$MyController' # the fully qualified class name of your controller
+      Middlewares:
+        - '%$MyRateLimitMiddleware' # the rate limiter we just defined in the last step
+```
+
+Finally, you need to define the custom routing:
+
+```yml
+Director:
+  rules:
+    'MyController//$Action/$ID/$OtherID': '%$MyRateLimitedController'
+```
+
+## Applying rate limiting across an entire application
+
+If you'd like to add rate limiting to an entire application (ie: across all routes) then you'll need to define your rate
+limit middleware much like the first step outlined in the previous section and then you'll have to apply it to the entire
+site as you would with any other middleware:
+
+```yml
+SilverStripe\Core\Injector\Injector:
+  SilverStripe\Control\Director:
+    properties:
+      Middlewares:
+        SiteWideRateLimitMiddleware: '%$SiteWideRateLimitMiddleware'
+```
+
+## Disabling the Rate Limiter
+
+You may already solve the rate limiting problem on a server level and the built in rate limiting may well be redundant.
+If this is the case you can turn off the rate limiting middleware by redefining the URL rules for the Security controller.
+
+Add the following to your config.yml:
+
+```yml
+SilverStripe\Control\Director:
+  rules:
+    'Security//$Action/$ID/$OtherID': SilverStripe\Security\Security
+```
\ No newline at end of file

From 3a7c8fd0d7e0678f9494c98fe5135769ba2f1031 Mon Sep 17 00:00:00 2001
From: Damian Mooyman <damian@silverstripe.com>
Date: Thu, 28 Sep 2017 09:15:00 +1300
Subject: [PATCH 5/6] Adjust YML conditionals

---
 _config/modules.yml           | 3 ++-
 _config/requestprocessors.yml | 2 +-
 _config/routes.yml            | 4 +++-
 _config/security.yml          | 2 +-
 4 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/_config/modules.yml b/_config/modules.yml
index e4f5264c6..46ecaf9de 100644
--- a/_config/modules.yml
+++ b/_config/modules.yml
@@ -21,7 +21,8 @@ SilverStripe\Core\Manifest\ModuleManifest:
     - $project
 ---
 Name: modules-framework
-After: modules-other
+After:
+  - '#modules-other'
 ---
 SilverStripe\Core\Manifest\ModuleManifest:
   module_priority:
diff --git a/_config/requestprocessors.yml b/_config/requestprocessors.yml
index 2f86253a1..7f11a8b7c 100644
--- a/_config/requestprocessors.yml
+++ b/_config/requestprocessors.yml
@@ -32,7 +32,7 @@ SilverStripe\Core\Injector\Injector:
 ---
 Name: errorrequestprocessors
 After:
-  - requestprocessors
+  - '#requestprocessors'
 ---
 SilverStripe\Core\Injector\Injector:
   # Note: If Director config changes, take note it will affect this config too
diff --git a/_config/routes.yml b/_config/routes.yml
index cd690748b..6d1368c95 100644
--- a/_config/routes.yml
+++ b/_config/routes.yml
@@ -20,9 +20,11 @@ SilverStripe\Control\Director:
     'SapphireREPL//$Action/$ID/$OtherID': SilverStripe\Dev\SapphireREPL
 ---
 Name: security-limited
+After:
+  - '#rootroutes'
 Except:
   environment: dev
 ---
 SilverStripe\Control\Director:
   rules:
-    'Security//$Action/$ID/$OtherID': '%$RateLimitedSecurityController'
\ No newline at end of file
+    'Security//$Action/$ID/$OtherID': '%$RateLimitedSecurityController'
diff --git a/_config/security.yml b/_config/security.yml
index 1a6b42dbd..40273a393 100644
--- a/_config/security.yml
+++ b/_config/security.yml
@@ -19,7 +19,7 @@ SilverStripe\Core\Injector\Injector:
 ---
 Name: coresecurity
 After:
-  - requestprocessors
+  - '#requestprocessors'
 ---
 SilverStripe\Core\Injector\Injector:
   SilverStripe\Control\Director:

From e4fd9b4ff7e07e90ad10be516655675ad620baec Mon Sep 17 00:00:00 2001
From: Damian Mooyman <damian@silverstripe.com>
Date: Thu, 28 Sep 2017 09:54:29 +1300
Subject: [PATCH 6/6] Code style fixes

---
 src/Control/ContentNegotiator.php                        | 2 +-
 tests/php/Control/Middleware/Control/TestController.php  | 2 +-
 tests/php/Control/Middleware/RateLimitMiddlewareTest.php | 1 -
 tests/php/Core/Cache/RateLimiterTest.php                 | 4 +---
 4 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/src/Control/ContentNegotiator.php b/src/Control/ContentNegotiator.php
index 8e5f621c3..fd99dc69c 100644
--- a/src/Control/ContentNegotiator.php
+++ b/src/Control/ContentNegotiator.php
@@ -136,7 +136,7 @@ class ContentNegotiator
         }
 
         $negotiator = new ContentNegotiator();
-        $negotiator->$chosenFormat( $response );
+        $negotiator->$chosenFormat($response);
     }
 
     /**
diff --git a/tests/php/Control/Middleware/Control/TestController.php b/tests/php/Control/Middleware/Control/TestController.php
index 88b6a04ce..d906dc0df 100644
--- a/tests/php/Control/Middleware/Control/TestController.php
+++ b/tests/php/Control/Middleware/Control/TestController.php
@@ -15,4 +15,4 @@ class TestController extends Controller
     {
         return Controller::join_links('TestController', $action);
     }
-}
\ No newline at end of file
+}
diff --git a/tests/php/Control/Middleware/RateLimitMiddlewareTest.php b/tests/php/Control/Middleware/RateLimitMiddlewareTest.php
index 899897586..23da57239 100644
--- a/tests/php/Control/Middleware/RateLimitMiddlewareTest.php
+++ b/tests/php/Control/Middleware/RateLimitMiddlewareTest.php
@@ -64,5 +64,4 @@ class RateLimitMiddlewareTest extends FunctionalTest
         $this->assertEquals(60, $response->getHeader('retry-after'));
         $this->assertNotEquals('Success', $response->getBody());
     }
-
 }
diff --git a/tests/php/Core/Cache/RateLimiterTest.php b/tests/php/Core/Cache/RateLimiterTest.php
index 113a2e408..ec2a80527 100644
--- a/tests/php/Core/Cache/RateLimiterTest.php
+++ b/tests/php/Core/Cache/RateLimiterTest.php
@@ -60,7 +60,6 @@ class RateLimiterTest extends SapphireTest
         $this->assertEquals(0, $rateLimiter->getNumAttemptsRemaining());
         $rateLimiter->hit();
         $this->assertEquals(0, $rateLimiter->getNumAttemptsRemaining());
-
     }
 
     public function testGetTimeToReset()
@@ -125,5 +124,4 @@ class RateLimiterTest extends SapphireTest
         $rateLimiter->hit();
         $this->assertFalse($rateLimiter->canAccess());
     }
-
-}
\ No newline at end of file
+}