Don't require users be logged in to access the Logout controller, so users with no Spaces can log out

Summary:
Fixes T13310. Use cases in the form "users with no access to any spaces can not <do things>" are generally unsupported (that is, we consider this to mean that the install is misconfigured), but "log out" is a somewhat more reasonable sort of thing to do and easy to support.

Drop the requirement that users be logged in to access the Logout controller. This skips the check for access to any Spaces and allows users with no Spaces to log out.

For users who are already logged out, this just redirects home with no effect.

Test Plan:
  - As a user with access to no Spaces, logged out. (Before: error; after: worked).
  - As a logged-out user, logged out (was redirected).
  - As a normal user, logged out (normal logout).

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13310

Differential Revision: https://secure.phabricator.com/D20578

src/applications/auth/controller/PhabricatorLogoutController.php

@@ -4,7 +4,25 @@ final class PhabricatorLogoutController
   extends PhabricatorAuthController {
 
   public function shouldRequireLogin() {
-    return true;
+    // See T13310. We allow access to the "Logout" controller even if you are
+    // not logged in: otherwise, users who do not have access to any Spaces can
+    // not log out.
+
+    // When you try to access a controller which requires you be logged in,
+    // and you do not have access to any Spaces, an access check fires first
+    // and prevents access with a "No Access to Spaces" error. If this
+    // controller requires users be logged in, users who are trying to log out
+    // and also have no access to Spaces get the error instead of a logout
+    // workflow and are trapped.
+
+    // By permitting access to this controller even if you are not logged in,
+    // we bypass the Spaces check and allow users who have no access to Spaces
+    // to log out.
+
+    // This incidentally allows users who are already logged out to access the
+    // controller, but this is harmless: we just no-op these requests.
+
+    return false;
   }
 
   public function shouldRequireEmailVerification() {