Fix inconsistent status codes

HTTP 401 should be used when logging in (i.e. authenticating) would make
a difference; HTTP 403 is reserved for requests that fail because the
already authenticated user is not authorized (i.e. lacking permissions)
to do something.

framework/core/src/Api/Controller/ListNotificationsController.php

@@ -15,12 +15,14 @@ use Flarum\Api\Serializer\NotificationSerializer;
 use Flarum\Discussion\Discussion;
 use Flarum\Http\UrlGenerator;
 use Flarum\Notification\NotificationRepository;
-use Flarum\User\Exception\PermissionDeniedException;
+use Flarum\User\AssertPermissionTrait;
 use Psr\Http\Message\ServerRequestInterface;
 use Tobscure\JsonApi\Document;
 
 class ListNotificationsController extends AbstractListController
 {
+    use AssertPermissionTrait;
+
     /**
      * {@inheritdoc}
      */
@@ -67,9 +69,7 @@ class ListNotificationsController extends AbstractListController
     {
         $actor = $request->getAttribute('actor');
 
-        if ($actor->isGuest()) {
-            throw new PermissionDeniedException;
-        }
+        $this->assertRegistered($actor);
 
         $actor->markNotificationsAsRead()->save();
 

framework/core/src/Api/Controller/ListUsersController.php

@@ -14,7 +14,7 @@ namespace Flarum\Api\Controller;
 use Flarum\Api\Serializer\UserSerializer;
 use Flarum\Http\UrlGenerator;
 use Flarum\Search\SearchCriteria;
-use Flarum\User\Exception\PermissionDeniedException;
+use Flarum\User\AssertPermissionTrait;
 use Flarum\User\Search\UserSearcher;
 use Illuminate\Support\Arr;
 use Psr\Http\Message\ServerRequestInterface;
@@ -22,6 +22,8 @@ use Tobscure\JsonApi\Document;
 
 class ListUsersController extends AbstractListController
 {
+    use AssertPermissionTrait;
+
     /**
      * {@inheritdoc}
      */
@@ -70,9 +72,8 @@ class ListUsersController extends AbstractListController
     {
         $actor = $request->getAttribute('actor');
 
-        if ($actor->cannot('viewUserList')) {
-            throw new PermissionDeniedException;
-        }
+        $this->assertRegistered($actor);
+        $this->assertCan($actor, 'viewUserList');
 
         $query = Arr::get($this->extractFilter($request), 'q');
         $sort = $this->extractSort($request);

framework/core/src/Foundation/ErrorServiceProvider.php

@@ -31,6 +31,7 @@ class ErrorServiceProvider extends AbstractServiceProvider
 
                 // 401 Unauthorized
                 'invalid_access_token' => 401,
+                'not_authenticated' => 401,
 
                 // 403 Forbidden
                 'forbidden' => 403,

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(

framework/core/src/User/AssertPermissionTrait.php

@@ -11,12 +11,13 @@
 
 namespace Flarum\User;
 
+use Flarum\User\Exception\NotAuthenticatedException;
 use Flarum\User\Exception\PermissionDeniedException;
 
 trait AssertPermissionTrait
 {
     /**
-     * @param $condition
+     * @param bool $condition
      * @throws PermissionDeniedException
      */
     protected function assertPermission($condition)
@@ -26,6 +27,17 @@ trait AssertPermissionTrait
         }
     }
 
+    /**
+     * @param bool $condition
+     * @throws NotAuthenticatedException
+     */
+    protected function assertAuthentication($condition)
+    {
+        if (! $condition) {
+            throw new NotAuthenticatedException;
+        }
+    }
+
     /**
      * @param User $actor
      * @param string $ability
@@ -39,20 +51,11 @@ trait AssertPermissionTrait
 
     /**
      * @param User $actor
-     * @throws \Flarum\User\Exception\PermissionDeniedException
-     */
-    protected function assertGuest(User $actor)
-    {
-        $this->assertPermission($actor->isGuest());
-    }
-
-    /**
-     * @param User $actor
-     * @throws PermissionDeniedException
+     * @throws NotAuthenticatedException
      */
     protected function assertRegistered(User $actor)
     {
-        $this->assertPermission(! $actor->isGuest());
+        $this->assertAuthentication(! $actor->isGuest());
     }
 
     /**

framework/core/src/User/Exception/NotAuthenticatedException.php

@@ -0,0 +1,23 @@
+<?php
+
+/*
+ * This file is part of Flarum.
+ *
+ * (c) Toby Zerner <toby.zerner@gmail.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Flarum\User\Exception;
+
+use Exception;
+use Flarum\Foundation\KnownError;
+
+class NotAuthenticatedException extends Exception implements KnownError
+{
+    public function getType(): string
+    {
+        return 'not_authenticated';
+    }
+}

framework/core/tests/integration/api/Auth/AuthenticateWithApiKeyTest.php

@@ -65,7 +65,7 @@ class AuthenticateWithApiKeyTest extends TestCase
 
         $response = $api->send(CreateGroupController::class, new Guest);
 
-        $this->assertEquals(403, $response->getStatusCode());
+        $this->assertEquals(401, $response->getStatusCode());
     }
 
     /**

framework/core/tests/integration/api/Controller/CreateGroupControllerTest.php

@@ -80,7 +80,7 @@ class CreateGroupControllerTest extends ApiControllerTestCase
     /**
      * @test
      */
-    public function unauthorized_user_cannot_create_group()
+    public function normal_user_cannot_create_group()
     {
         $this->actor = User::find(2);
 

framework/core/tests/integration/api/Controller/ListNotificationsControllerTest.php

@@ -36,7 +36,7 @@ class ListNotificationsControllerTest extends ApiControllerTestCase
     {
         $response = $this->callWith();
 
-        $this->assertEquals(403, $response->getStatusCode());
+        $this->assertEquals(401, $response->getStatusCode());
     }
 
     /**

framework/core/tests/integration/api/Controller/ListUsersControllerTest.php

@@ -42,7 +42,7 @@ class ListUsersControllerTest extends ApiControllerTestCase
     {
         $response = $this->callWith();
 
-        $this->assertEquals(403, $response->getStatusCode());
+        $this->assertEquals(401, $response->getStatusCode());
     }
 
     /**