Clean up exception handling for access_tokens (#5656)

First of all, let's get rid of `TOKEN_NOT_FOUND_HTTP_STATUS`. It was a hack we
did at one point when it was possible to return either a 403 or a 401 if the
creds were missing. We always return a 401 in these cases now (thankfully), so
it's not needed.

Let's also stop abusing `AuthError` for these cases. Honestly they have nothing
that relates them to the other places that `AuthError` is used, other than the
fact that they are loosely under the 'Auth' banner. It makes no sense for them
to share exception classes.

Instead, let's add a couple of new exception classes: `InvalidClientTokenError`
and `MissingClientTokenError`, for the `M_UNKNOWN_TOKEN` and `M_MISSING_TOKEN`
cases respectively - and an `InvalidClientCredentialsError` base class for the
two of them.

1 files changed, 24 insertions, 7 deletions

diff --git a/tests/api/test_auth.py b/tests/api/test_auth.py

index ddf2b578b3..ee92ceeb60 100644
--- a/tests/api/test_auth.py
+++ b/tests/api/test_auth.py
@@ -21,7 +21,14 @@ from twisted.internet import defer
 
 import synapse.handlers.auth
 from synapse.api.auth import Auth
-from synapse.api.errors import AuthError, Codes, ResourceLimitError
+from synapse.api.errors import (
+    AuthError,
+    Codes,
+    InvalidClientCredentialsError,
+    InvalidClientTokenError,
+    MissingClientTokenError,
+    ResourceLimitError,
+)
 from synapse.types import UserID
 
 from tests import unittest
@@ -70,7 +77,9 @@ class AuthTestCase(unittest.TestCase):
         request.args[b"access_token"] = [self.test_token]
         request.requestHeaders.getRawHeaders = mock_getRawHeaders()
         d = self.auth.get_user_by_req(request)
-        self.failureResultOf(d, AuthError)
+        f = self.failureResultOf(d, InvalidClientTokenError).value
+        self.assertEqual(f.code, 401)
+        self.assertEqual(f.errcode, "M_UNKNOWN_TOKEN")
 
     def test_get_user_by_req_user_missing_token(self):
         user_info = {"name": self.test_user, "token_id": "ditto"}
@@ -79,7 +88,9 @@ class AuthTestCase(unittest.TestCase):
         request = Mock(args={})
         request.requestHeaders.getRawHeaders = mock_getRawHeaders()
         d = self.auth.get_user_by_req(request)
-        self.failureResultOf(d, AuthError)
+        f = self.failureResultOf(d, MissingClientTokenError).value
+        self.assertEqual(f.code, 401)
+        self.assertEqual(f.errcode, "M_MISSING_TOKEN")
 
     @defer.inlineCallbacks
     def test_get_user_by_req_appservice_valid_token(self):
@@ -133,7 +144,9 @@ class AuthTestCase(unittest.TestCase):
         request.args[b"access_token"] = [self.test_token]
         request.requestHeaders.getRawHeaders = mock_getRawHeaders()
         d = self.auth.get_user_by_req(request)
-        self.failureResultOf(d, AuthError)
+        f = self.failureResultOf(d, InvalidClientTokenError).value
+        self.assertEqual(f.code, 401)
+        self.assertEqual(f.errcode, "M_UNKNOWN_TOKEN")
 
     def test_get_user_by_req_appservice_bad_token(self):
         self.store.get_app_service_by_token = Mock(return_value=None)
@@ -143,7 +156,9 @@ class AuthTestCase(unittest.TestCase):
         request.args[b"access_token"] = [self.test_token]
         request.requestHeaders.getRawHeaders = mock_getRawHeaders()
         d = self.auth.get_user_by_req(request)
-        self.failureResultOf(d, AuthError)
+        f = self.failureResultOf(d, InvalidClientTokenError).value
+        self.assertEqual(f.code, 401)
+        self.assertEqual(f.errcode, "M_UNKNOWN_TOKEN")
 
     def test_get_user_by_req_appservice_missing_token(self):
         app_service = Mock(token="foobar", url="a_url", sender=self.test_user)
@@ -153,7 +168,9 @@ class AuthTestCase(unittest.TestCase):
         request = Mock(args={})
         request.requestHeaders.getRawHeaders = mock_getRawHeaders()
         d = self.auth.get_user_by_req(request)
-        self.failureResultOf(d, AuthError)
+        f = self.failureResultOf(d, MissingClientTokenError).value
+        self.assertEqual(f.code, 401)
+        self.assertEqual(f.errcode, "M_MISSING_TOKEN")
 
     @defer.inlineCallbacks
     def test_get_user_by_req_appservice_valid_token_valid_user_id(self):
@@ -280,7 +297,7 @@ class AuthTestCase(unittest.TestCase):
         request.args[b"access_token"] = [guest_tok.encode("ascii")]
         request.requestHeaders.getRawHeaders = mock_getRawHeaders()
 
-        with self.assertRaises(AuthError) as cm:
+        with self.assertRaises(InvalidClientCredentialsError) as cm:
             yield self.auth.get_user_by_req(request, allow_guest=True)
 
         self.assertEqual(401, cm.exception.code)