BUG Fix flushing on live mode (#7241)

* BUG Fix flushing on live mode
Fixes #7217

* Clarify injector service documentation

_config/requestprocessors.yml

@@ -3,6 +3,7 @@ Name: requestprocessors
 ---
 SilverStripe\Core\Injector\Injector:
   SilverStripe\Control\Director:
+    # Note: Don't add 'class' config here, as it will affect ErrorDirector as well
     properties:
       Middlewares:
         TrustedProxyMiddleware: '%$SilverStripe\Control\Middleware\TrustedProxyMiddleware'
@@ -16,3 +17,11 @@ SilverStripe\Core\Injector\Injector:
   SilverStripe\Control\TrustedProxyMiddleware:
     properties:
       TrustedProxyIPs: "`SS_TRUSTED_PROXY_IPS`"
+---
+Name: errorrequestprocessors
+After:
+  - requestprocessors
+---
+SilverStripe\Core\Injector\Injector:
+  # Note: If Director config changes, take note it will affect this config too
+  SilverStripe\Core\Startup\ErrorDirector: '%$SilverStripe\Control\Director'

docs/en/02_Developer_Guides/05_Extending/05_Injector.md

@@ -252,11 +252,31 @@ assign a reference to that service.
 	:::yaml
 	Injector:
 	  JSONServiceDefinition:
+	    class: JSONServiceImplementor
 	    properties:
 	      Serialiser: JSONSerialiser
 	  GZIPJSONProvider: %$JSONServiceDefinition
 
 
+`Injector::inst()->get('GZIPJSONProvider')` will then be an instance of `JSONServiceImplementor` with the injected
+properties.
+
+It is important here to note that the 'class' property of the parent service will be inherited directly as well.
+If class is not specified, then the class will be inherited from the outer service name, not the inner service name.
+
+For example with this config:
+
+    :::yaml
+	Injector:
+	  Connector:
+	    properties:
+	      AsString: true
+	  ServiceConnector: %$Connector
+
+
+Both `Connector` and `ServiceConnector` will have the `AsString` property set to true, but the resulting
+instances will be classes which match their respective service names, due to the lack of a `class` specification. 
+
 ## Testing with Injector
 
 In situations where injector states must be temporarily overridden, it is possible to create nested Injector instances 

docs/en/04_Changelogs/4.0.0.md

@@ -442,6 +442,9 @@ Upgrade subclasses
 -class MyClass extends Object
 -{
 -}
++use SilverStripe\Core\Extensible;
++use SilverStripe\Core\Injector\Injectable;
++use SilverStripe\Core\Config\Configurable;
 +class MyClass
 +{
 +    use Extensible;

src/Core/Startup/ErrorControlChainMiddleware.php

@@ -6,9 +6,8 @@ use SilverStripe\Control\Director;
 use SilverStripe\Control\HTTPRequest;
 use SilverStripe\Control\HTTPResponse;
 use SilverStripe\Control\HTTPResponse_Exception;
-use SilverStripe\Core\Application;
 use SilverStripe\Control\Middleware\HTTPMiddleware;
-use SilverStripe\Security\Permission;
+use SilverStripe\Core\Application;
 use SilverStripe\Security\Security;
 
 /**
@@ -87,9 +86,10 @@ class ErrorControlChainMiddleware implements HTTPMiddleware
         // Ensure session is started
         $request->getSession()->init($request);
 
-        // Next, check if we're in dev mode, or the database doesn't have any security data, or we are admin
-        if (Director::isDev() || !Security::database_is_ready() || Permission::check('ADMIN')) {
-            return $reloadToken->reloadWithToken();
+        // Request with ErrorDirector
+        $result = ErrorDirector::singleton()->handleRequestWithToken($request, $reloadToken, $this->getApplication()->getKernel());
+        if ($result) {
+            return $result;
         }
 
         // Fail and redirect the user to the login page

src/Core/Startup/ErrorDirector.php

@@ -0,0 +1,47 @@
+<?php
+
+namespace SilverStripe\Core\Startup;
+
+use SilverStripe\Control\Director;
+use SilverStripe\Control\HTTPRequest;
+use SilverStripe\Control\HTTPResponse;
+use SilverStripe\Core\Injector\Injector;
+use SilverStripe\Core\Kernel;
+use SilverStripe\Security\Permission;
+use SilverStripe\Security\Security;
+
+/**
+ * Specialised Director class used by ErrorControlChain to handle error and redirect conditions
+ *
+ * @internal This class is experimental API and may change without warning
+ */
+class ErrorDirector extends Director
+{
+    /**
+     * Redirect with token if allowed, or null if not allowed
+     *
+     * @param HTTPRequest $request
+     * @param ParameterConfirmationToken $token
+     * @param Kernel $kernel
+     * @return null|HTTPResponse Redirection response, or null if not able to redirect
+     */
+    public function handleRequestWithToken(HTTPRequest $request, ParameterConfirmationToken $token, Kernel $kernel)
+    {
+        Injector::inst()->registerService($request, HTTPRequest::class);
+
+        // Next, check if we're in dev mode, or the database doesn't have any security data, or we are admin
+        $reload = function (HTTPRequest $request) use ($token, $kernel) {
+            if ($kernel->getEnvironment() === Kernel::DEV || !Security::database_is_ready() || Permission::check('ADMIN')) {
+                return $token->reloadWithToken();
+            }
+            return null;
+        };
+
+        try {
+            return $this->callMiddleware($request, $reload);
+        } finally {
+            // Ensure registered request is un-registered
+            Injector::inst()->unregisterNamedObject(HTTPRequest::class);
+        }
+    }
+}

tests/php/Control/DirectorTest.php

@@ -498,7 +498,7 @@ class DirectorTest extends SapphireTest
     }
 
     /**
-     * @covers \SilverStripe\Control\Director::extract_request_headers()
+     * @covers \SilverStripe\Control\HTTPRequestBuilder::extractRequestHeaders()
      */
     public function testExtractRequestHeaders()
     {

tests/php/Core/Startup/ErrorControlChainMiddlewareTest.php

@@ -0,0 +1,77 @@
+<?php
+
+namespace SilverStripe\Core\Tests\Startup;
+
+use SilverStripe\Control\Cookie;
+use SilverStripe\Control\HTTPApplication;
+use SilverStripe\Control\HTTPRequest;
+use SilverStripe\Control\HTTPResponse;
+use SilverStripe\Control\Session;
+use SilverStripe\Core\Kernel;
+use SilverStripe\Core\Startup\ErrorControlChainMiddleware;
+use SilverStripe\Core\Tests\Startup\ErrorControlChainMiddlewareTest\BlankKernel;
+use SilverStripe\Dev\SapphireTest;
+use SilverStripe\Security\Security;
+
+class ErrorControlChainMiddlewareTest extends SapphireTest
+{
+    protected $usesDatabase = true;
+
+    protected function setUp()
+    {
+        parent::setUp();
+        Security::force_database_is_ready(true);
+    }
+
+    protected function tearDown()
+    {
+        Security::clear_database_is_ready();
+        parent::tearDown();
+    }
+
+    public function testLiveFlushAdmin()
+    {
+        // Mock admin
+        $adminID = $this->logInWithPermission('ADMIN');
+        $this->logOut();
+
+        // Mock app
+        $app = new HTTPApplication(new BlankKernel(BASE_PATH));
+        $app->getKernel()->setEnvironment(Kernel::LIVE);
+
+        // Test being logged in as admin
+        $chain = new ErrorControlChainMiddleware($app);
+        $request = new HTTPRequest('GET', '/', ['flush' => 1]);
+        $request->setSession(new Session(['loggedInAs' => $adminID]));
+        $result = $chain->process($request, function () {
+            return null;
+        });
+
+        $this->assertInstanceOf(HTTPResponse::class, $result);
+        $location = $result->getHeader('Location');
+        $this->assertContains('?flush=1&flushtoken=', $location);
+        $this->assertNotContains('Security/login', $location);
+    }
+
+    public function testLiveFlushUnauthenticated()
+    {
+        // Mock app
+        $app = new HTTPApplication(new BlankKernel(BASE_PATH));
+        $app->getKernel()->setEnvironment(Kernel::LIVE);
+
+        // Test being logged in as no one
+        Security::setCurrentUser(null);
+        $chain = new ErrorControlChainMiddleware($app);
+        $request = new HTTPRequest('GET', '/', ['flush' => 1]);
+        $request->setSession(new Session(['loggedInAs' => 0]));
+        $result = $chain->process($request, function () {
+            return null;
+        });
+
+        // Should be directed to login, not to flush
+        $this->assertInstanceOf(HTTPResponse::class, $result);
+        $location = $result->getHeader('Location');
+        $this->assertNotContains('?flush=1&flushtoken=', $location);
+        $this->assertContains('Security/login', $location);
+    }
+}

tests/php/Core/Startup/ErrorControlChainMiddlewareTest/BlankKernel.php

@@ -0,0 +1,18 @@
+<?php
+
+namespace SilverStripe\Core\Tests\Startup\ErrorControlChainMiddlewareTest;
+
+use SilverStripe\Core\CoreKernel;
+
+class BlankKernel extends CoreKernel
+{
+    public function __construct($basePath)
+    {
+        // Noop
+    }
+
+    public function boot($flush = false)
+    {
+        // Noop
+    }
+}