Merge pull request #422 from creative-commoners/pulls/4.2/disable-cac…

…he-on-endpoints

FIX Disable HTTP caching on all relevant MFA API endpoints

src/Authenticator/LoginHandler.php

@@ -7,6 +7,7 @@
 use Psr\Log\LoggerInterface;
 use SilverStripe\Control\HTTPRequest;
 use SilverStripe\Control\HTTPResponse;
+use SilverStripe\Control\Middleware\HTTPCacheControlMiddleware;
 use SilverStripe\Core\Injector\Injector;
 use SilverStripe\MFA\Exception\InvalidMethodException;
 use SilverStripe\MFA\Exception\MemberNotFoundException;
@@ -159,9 +160,13 @@ public function mfa(HTTPRequest $request)
      */
     public function getSchema(): HTTPResponse
     {
+        // Prevent caching of response
+        HTTPCacheControlMiddleware::singleton()->disableCache(true);
+
         try {
             $member = $this->getMember();
             $schema = SchemaGenerator::create()->getSchema($member);
+
             return $this->jsonResponse(
                 $schema + [
                     'endpoints' => [
@@ -186,6 +191,9 @@ public function getSchema(): HTTPResponse
      */
     public function startRegistration(HTTPRequest $request): HTTPResponse
     {
+        // Prevent caching of response
+        HTTPCacheControlMiddleware::singleton()->disableCache(true);
+
         $store = $this->getStore();
         $sessionMember = $store ? $store->getMember() : null;
         $loggedInMember = Security::getCurrentUser();
@@ -344,6 +352,9 @@ public function skipRegistration(HTTPRequest $request): HTTPResponse
      */
     public function startVerification(HTTPRequest $request): HTTPResponse
     {
+        // Prevent caching of response
+        HTTPCacheControlMiddleware::singleton()->disableCache(true);
+
         $store = $this->getStore();
         // If we don't have a valid member we shouldn't be here, or if sudo mode is not active yet.
         if (!$store || !$store->getMember() || !$this->getSudoModeService()->check($request->getSession())) {

src/Controller/AdminRegistrationController.php

@@ -8,6 +8,7 @@
 use SilverStripe\Admin\LeftAndMain;
 use SilverStripe\Control\HTTPRequest;
 use SilverStripe\Control\HTTPResponse;
+use SilverStripe\Control\Middleware\HTTPCacheControlMiddleware;
 use SilverStripe\Core\Injector\Injector;
 use SilverStripe\MFA\Extension\MemberExtension;
 use SilverStripe\MFA\RequestHandler\BaseHandlerTrait;
@@ -66,6 +67,9 @@ class AdminRegistrationController extends LeftAndMain
      */
     public function startRegistration(HTTPRequest $request): HTTPResponse
     {
+        // Prevent caching of response
+        HTTPCacheControlMiddleware::singleton()->disableCache(true);
+
         // Create a fresh store from the current logged in user
         $member = Security::getCurrentUser();
         $store = $this->createStore($member);

src/RequestHandler/VerificationHandlerTrait.php

@@ -4,7 +4,6 @@
 
 use SilverStripe\Control\HTTPRequest;
 use SilverStripe\Control\HTTPResponse;
-use SilverStripe\Control\Middleware\HTTPCacheControlMiddleware;
 use SilverStripe\Core\Config\Config;
 use SilverStripe\MFA\Exception\InvalidMethodException;
 use SilverStripe\MFA\Method\MethodInterface;
@@ -76,9 +75,6 @@ protected function createStartVerificationResponse(
         $token->reset();
         $data[$token->getName()] = $token->getValue();
 
-        // Prevent caching of response
-        HTTPCacheControlMiddleware::singleton()->disableCache(true);
-
         // Respond with our method
         return $response->setBody(json_encode($data));
     }

tests/php/Authenticator/LoginHandlerTest.php

@@ -335,24 +335,42 @@ public function testStartVerificationIncludesACSRFToken()
         $this->assertTrue(SecurityToken::inst()->check($response->SecurityID));
     }
 
-    // This is testing that HTTP caching headers that disable caching are set
-    // in VerificationHandlerTrait::createStartVerificationResponse()
-    // VerificationHandlerTrait is used by LoginHandler
-    public function testStartVerificationHttpCacheHeadersDisabled()
+    /**
+     * Test that HTTP caching is disabled in requests to the schema endpoint
+     */
+    public function testSchemaDisablesHTTPCaching()
+    {
+        $middleware = HTTPCacheControlMiddleware::singleton();
+        $middleware->enableCache(true);
+        $this->assertSame('enabled', $middleware->getState());
+
+        $this->get(Controller::join_links(Security::login_url(), 'default/mfa/schema'));
+        $this->assertSame('disabled', $middleware->getState());
+    }
+
+    /**
+     * Test that HTTP caching is disabled in requests to the registration endpoint
+     */
+    public function testStartRegistrationDisablesHTTPCaching()
+    {
+        $middleware = HTTPCacheControlMiddleware::singleton();
+        $middleware->enableCache(true);
+        $this->assertSame('enabled', $middleware->getState());
+
+        $this->get(Controller::join_links(Security::login_url(), 'default/mfa/register/basic-math'));
+        $this->assertSame('disabled', $middleware->getState());
+    }
+
+    /**
+     * Test that HTTP caching is disabled in requests to the verification endpoint
+     */
+    public function testStartVerificationDisablesHTTPCaching()
     {
-        /** @var Member $member */
-        SecurityToken::enable();
-        $handler = new LoginHandler('mfa', $this->createMock(MemberAuthenticator::class));
-        $member = $this->objFromFixture(Member::class, 'robbie');
-        $store = new SessionStore($member);
-        $handler->setStore($store);
-        $request = new HTTPRequest('GET', '/');
-        $request->setSession(new Session([]));
-        $request->setRouteParams(['Method' => 'basic-math']);
         $middleware = HTTPCacheControlMiddleware::singleton();
         $middleware->enableCache(true);
         $this->assertSame('enabled', $middleware->getState());
-        $handler->startVerification($request);
+
+        $this->get(Controller::join_links(Security::login_url(), 'default/mfa/verify/basic-math'));
         $this->assertSame('disabled', $middleware->getState());
     }
 

tests/php/Controller/AdminRegistrationControllerTest.php

@@ -7,6 +7,7 @@
 use SilverStripe\Admin\AdminRootController;
 use SilverStripe\Control\Controller;
 use SilverStripe\Control\HTTPRequest;
+use SilverStripe\Control\Middleware\HTTPCacheControlMiddleware;
 use SilverStripe\Core\Config\Config;
 use SilverStripe\Core\Injector\Injector;
 use SilverStripe\Dev\FunctionalTest;
@@ -84,6 +85,19 @@ public function testStartRegistrationReturns200Response()
         $this->assertSame(200, $result->getStatusCode());
     }
 
+    /**
+     * Test that HTTP caching is disabled in requests to the registration endpoint
+     */
+    public function testStartRegistrationDisablesHTTPCaching()
+    {
+        $middleware = HTTPCacheControlMiddleware::singleton();
+        $middleware->enableCache(true);
+        $this->assertSame('enabled', $middleware->getState());
+
+        $this->get(Controller::join_links(AdminRootController::admin_url(), 'mfa', 'register/foo'));
+        $this->assertSame('disabled', $middleware->getState());
+    }
+
     public function testFinishRegistrationGracefullyHandlesInvalidSessions()
     {
         $this->logInAs($this->objFromFixture(Member::class, 'sally_smith'));