From cbe7d4dfdbceba91f9a9d698d56b81fe3034ca16 Mon Sep 17 00:00:00 2001
From: Franz Liedke <franz@develophp.org>
Date: Sat, 14 Sep 2019 20:33:23 +0200
Subject: [PATCH] Restore beta.9 behavior of assertCan()

In flarum/core#1854, I changed the implementation of `assertCan()` to be
more aware of the user's log-in status. I came across this when unifying
our API's response status code when actors are not authenticated or not
authorized to do something.

@luceos rightfully had to tweak this again in 8e3eb59, because the
behavior changed for one of the few API endpoints that checked for a
permission that even guests can have.

It turns out having this complex behavior in `assertCan()` is quite
misleading, because the name suggests a simple permission check and
nothing more.

Where we actually want to differ between HTTP 401 and 403, we can do
this using two method calls, and enforce it with our tests.

If this turns out to be problematic or extremely common, we can revisit
this and introduce a method with a different, better name in the future.

This commit restores the method's behavior in the last release, so we
also avoid another breaking change for extensions.
---
 .../src/Group/Command/CreateGroupHandler.php    |  1 +
 .../core/src/User/AssertPermissionTrait.php     | 17 +++--------------
 .../src/User/Command/RegisterUserHandler.php    |  2 +-
 .../tests/integration/api/users/ListTest.php    |  2 +-
 4 files changed, 6 insertions(+), 16 deletions(-)

diff --git a/framework/core/src/Group/Command/CreateGroupHandler.php b/framework/core/src/Group/Command/CreateGroupHandler.php
index 036443213..8a000679e 100644
--- a/framework/core/src/Group/Command/CreateGroupHandler.php
+++ b/framework/core/src/Group/Command/CreateGroupHandler.php
@@ -49,6 +49,7 @@ class CreateGroupHandler
         $actor = $command->actor;
         $data = $command->data;
 
+        $this->assertRegistered($actor);
         $this->assertCan($actor, 'createGroup');
 
         $group = Group::build(
diff --git a/framework/core/src/User/AssertPermissionTrait.php b/framework/core/src/User/AssertPermissionTrait.php
index 7646e8080..4845d85dd 100644
--- a/framework/core/src/User/AssertPermissionTrait.php
+++ b/framework/core/src/User/AssertPermissionTrait.php
@@ -55,28 +55,17 @@ trait AssertPermissionTrait
      * @param User $actor
      * @param string $ability
      * @param mixed $arguments
-     * @throws NotAuthenticatedException
      * @throws PermissionDeniedException
      */
     protected function assertCan(User $actor, $ability, $arguments = [])
     {
-        // Identify whether guest or user has the permission.
-        $can = $actor->can($ability, $arguments);
-
-        // For non-authenticated users, we throw a different exception to signal
-        // that logging in may help.
-        if (! $can) {
-            $this->assertRegistered($actor);
-        }
-
-        // If we're logged in, then we need to communicate that the current
-        // account simply does not have enough permissions.
-        $this->assertPermission($can);
+        $this->assertPermission(
+            $actor->can($ability, $arguments)
+        );
     }
 
     /**
      * @param User $actor
-     * @throws NotAuthenticatedException
      * @throws PermissionDeniedException
      */
     protected function assertAdmin(User $actor)
diff --git a/framework/core/src/User/Command/RegisterUserHandler.php b/framework/core/src/User/Command/RegisterUserHandler.php
index f2d1818f8..e839e1c2b 100644
--- a/framework/core/src/User/Command/RegisterUserHandler.php
+++ b/framework/core/src/User/Command/RegisterUserHandler.php
@@ -74,7 +74,7 @@ class RegisterUserHandler
         $data = $command->data;
 
         if (! $this->settings->get('allow_sign_up')) {
-            $this->assertPermission($actor->can('administrate'));
+            $this->assertAdmin($actor);
         }
 
         $password = Arr::get($data, 'attributes.password');
diff --git a/framework/core/tests/integration/api/users/ListTest.php b/framework/core/tests/integration/api/users/ListTest.php
index bbae90ecf..f1c8660b4 100644
--- a/framework/core/tests/integration/api/users/ListTest.php
+++ b/framework/core/tests/integration/api/users/ListTest.php
@@ -50,7 +50,7 @@ class ListTest extends TestCase
             $this->request('GET', '/api/users')
         );
 
-        $this->assertEquals(401, $response->getStatusCode());
+        $this->assertEquals(403, $response->getStatusCode());
     }
 
     /**