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.

src/Api/Controller/ListNotificationsController.php

@@ -13,12 +13,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}
      */
@@ -65,9 +67,7 @@ class ListNotificationsController extends AbstractListController
     {
         $actor = $request->getAttribute('actor');
 
-        if ($actor->isGuest()) {
+        $this->assertRegistered($actor);
-            throw new PermissionDeniedException;
-        }
 
         $actor->markNotificationsAsRead()->save();
 

src/Api/Controller/ListUsersController.php

@@ -12,7 +12,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;
@@ -20,6 +20,8 @@ use Tobscure\JsonApi\Document;
 
 class ListUsersController extends AbstractListController
 {
+    use AssertPermissionTrait;
+
     /**
      * {@inheritdoc}
      */
@@ -68,9 +70,8 @@ class ListUsersController extends AbstractListController
     {
         $actor = $request->getAttribute('actor');
 
-        if ($actor->cannot('viewUserList')) {
+        $this->assertRegistered($actor);
-            throw new PermissionDeniedException;
+        $this->assertCan($actor, 'viewUserList');
-        }
 
         $query = Arr::get($this->extractFilter($request), 'q');
         $sort = $this->extractSort($request);

src/Foundation/ErrorServiceProvider.php

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

src/Group/Command/CreateGroupHandler.php

@@ -47,6 +47,7 @@ class CreateGroupHandler
         $actor = $command->actor;
         $data = $command->data;
 
+        $this->assertRegistered($actor);
         $this->assertCan($actor, 'createGroup');
 
         $group = Group::build(

src/User/AssertPermissionTrait.php

@@ -9,12 +9,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)
@@ -24,6 +25,17 @@ trait AssertPermissionTrait
         }
     }
 
+    /**
+     * @param bool $condition
+     * @throws NotAuthenticatedException
+     */
+    protected function assertAuthentication($condition)
+    {
+        if (! $condition) {
+            throw new NotAuthenticatedException;
+        }
+    }
+
     /**
      * @param User $actor
      * @param string $ability
@@ -37,20 +49,11 @@ trait AssertPermissionTrait
 
     /**
      * @param User $actor
-     * @throws \Flarum\User\Exception\PermissionDeniedException
+     * @throws NotAuthenticatedException
-     */
-    protected function assertGuest(User $actor)
-    {
-        $this->assertPermission($actor->isGuest());
-    }
-
-    /**
-     * @param User $actor
-     * @throws PermissionDeniedException
      */
     protected function assertRegistered(User $actor)
     {
-        $this->assertPermission(! $actor->isGuest());
+        $this->assertAuthentication(! $actor->isGuest());
     }
 
     /**

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';
+    }
+}

tests/integration/api/Auth/AuthenticateWithApiKeyTest.php

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

tests/integration/api/Controller/CreateGroupControllerTest.php

@@ -78,7 +78,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);
 

tests/integration/api/Controller/ListNotificationsControllerTest.php

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

tests/integration/api/Controller/ListUsersControllerTest.php

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