[BUG] Fix issues with multiple authenticators for a single task (#7149)

Using multiple 2FA authenticators, logging out, resetting password etc. proved to be handled wrong.
Example scenario:
The result is an error, because the `renderWrappedController` was called, despite the responses being a set of either array with Content or Form, or a redirect action.

The default action should be followed and not try to render if there is nothing to render

Because the logout (or changepassword, or resetpassword, etc.) has already been handled, the first response is the default authenticator's response. This _could_ be a form (in case of logout without valid token), a content set (reset password) or a form (change password).

This edge case only happens when there are multiple authenticators supporting the requested method that is _not_ login.

src/Security/MemberAuthenticator/LogoutHandler.php

@@ -3,6 +3,7 @@
 namespace SilverStripe\Security\MemberAuthenticator;
 
 use SilverStripe\Control\Director;
+use SilverStripe\Control\HTTPResponse;
 use SilverStripe\Control\RequestHandler;
 use SilverStripe\Core\Injector\Injector;
 use SilverStripe\ORM\ValidationResult;
@@ -44,7 +45,7 @@ class LogoutHandler extends RequestHandler
      * {@link __construct constructor} was set to TRUE and the user was
      * currently logged in.
      *
-     * @return bool|Member
+     * @return array|HTTPResponse
      */
     public function logout()
     {

src/Security/Security.php

@@ -723,7 +723,7 @@ class Security extends Controller implements TemplateGlobalProvider
             $handlers,
             _t(__CLASS__.'.LOGOUT', 'Log out'),
             $this->getTemplatesFor('logout'),
-            [$this, 'aggregateTabbedForms']
+            [$this, 'aggregateAuthenticatorResponses']
         );
     }
 
@@ -733,7 +733,7 @@ class Security extends Controller implements TemplateGlobalProvider
      *
      * @param int $service
      * @param HTTPRequest $request
-     * @return Authenticator[]
+     * @return array|Authenticator[]
      * @throws HTTPResponse_Exception
      */
     protected function getServiceAuthenticatorsFromRequest($service, HTTPRequest $request)
@@ -805,6 +805,40 @@ class Security extends Controller implements TemplateGlobalProvider
         ];
     }
 
+    /**
+     * We have three possible scenarios.
+     * We get back Content (e.g. Password Reset)
+     * We get back a Form (no token set for logout)
+     * We get back a HTTPResponse, telling us to redirect.
+     * Return the first one, which is the default response, as that covers all required scenarios
+     *
+     * @param array $results
+     * @return array|HTTPResponse
+     */
+    protected function aggregateAuthenticatorResponses($results)
+    {
+        $error = false;
+        $result = null;
+        foreach ($results as $authName => $singleResult) {
+            if (($singleResult instanceof HTTPResponse) ||
+                (is_array($singleResult) &&
+                    (isset($singleResult['Content']) || isset($singleResult['Form'])))
+            ) {
+                // return the first successful response
+                return $singleResult;
+            } else {
+                // Not a valid response
+                $error = true;
+            }
+        }
+
+        if ($error) {
+            throw new \LogicException('No authenticators found compatible with logout operation');
+        }
+
+        return $result;
+    }
+
     /**
      * Delegate to a number of handlers and aggregate the results. This is used, for example, to
      * build the log-in page where there are multiple authenticators active.
@@ -833,8 +867,13 @@ class Security extends Controller implements TemplateGlobalProvider
             $handlers
         );
 
-        $fragments = call_user_func_array($aggregator, [$results]);
-        return $this->renderWrappedController($title, $fragments, $templates);
+        $response = call_user_func_array($aggregator, [$results]);
+        // The return could be a HTTPResponse, in which we don't want to call the render
+        if (is_array($response)) {
+            return $this->renderWrappedController($title, $response, $templates);
+        }
+
+        return $response;
     }
 
     /**
@@ -920,7 +959,7 @@ class Security extends Controller implements TemplateGlobalProvider
             $handlers,
             _t('SilverStripe\\Security\\Security.LOSTPASSWORDHEADER', 'Lost Password'),
             $this->getTemplatesFor('lostpassword'),
-            [$this, 'aggregateTabbedForms']
+            [$this, 'aggregateAuthenticatorResponses']
         );
     }
 
@@ -949,7 +988,7 @@ class Security extends Controller implements TemplateGlobalProvider
             $handlers,
             _t('SilverStripe\\Security\\Security.CHANGEPASSWORDHEADER', 'Change your password'),
             $this->getTemplatesFor('changepassword'),
-            [$this, 'aggregateTabbedForms']
+            [$this, 'aggregateAuthenticatorResponses']
         );
     }