Merge pull request #3161 from NotAFile/remove-v1auth

Make Client-Server API return 403 for invalid token

synapse/rest/client/v1/base.py

@@ -52,12 +52,16 @@ class ClientV1RestServlet(RestServlet):
     """A base Synapse REST Servlet for the client version 1 API.
     """
 
+    # This subclass was presumably created to allow the auth for the v1
+    # protocol version to be different, however this behaviour was removed.
+    # it may no longer be necessary
+
     def __init__(self, hs):
         """
         Args:
             hs (synapse.server.HomeServer):
         """
         self.hs = hs
         self.builder_factory = hs.get_event_builder_factory()
-        self.auth = hs.get_v1auth()
+        self.auth = hs.get_auth()
         self.txns = HttpTransactionCache(hs.get_clock())

synapse/rest/client/v1/pusher.py

@@ -150,7 +150,7 @@ def __init__(self, hs):
         super(RestServlet, self).__init__()
         self.hs = hs
         self.notifier = hs.get_notifier()
-        self.auth = hs.get_v1auth()
+        self.auth = hs.get_auth()
         self.pusher_pool = self.hs.get_pusherpool()
 
     @defer.inlineCallbacks

synapse/server.py

@@ -105,7 +105,6 @@ def build_DEPENDENCY(self)
         'federation_client',
         'federation_server',
         'handlers',
-        'v1auth',
         'auth',
         'state_handler',
         'state_resolution_handler',
@@ -225,15 +224,6 @@ def build_http_client_context_factory(self):
     def build_simple_http_client(self):
         return SimpleHttpClient(self)
 
-    def build_v1auth(self):
-        orf = Auth(self)
-        # Matrix spec makes no reference to what HTTP status code is returned,
-        # but the V1 API uses 403 where it means 401, and the webclient
-        # relies on this behaviour, so V1 gets its own copy of the auth
-        # with backwards compat behaviour.
-        orf.TOKEN_NOT_FOUND_HTTP_STATUS = 403
-        return orf
-
     def build_state_handler(self):
         return StateHandler(self)
 

tests/rest/client/v1/test_events.py

@@ -148,11 +148,16 @@ def tearDown(self):
 
     @defer.inlineCallbacks
     def test_stream_basic_permissions(self):
-        # invalid token, expect 403
+        # invalid token, expect 401
+        # note: this is in violation of the original v1 spec, which expected
+        # 403. However, since the v1 spec no longer exists and the v1
+        # implementation is now part of the r0 implementation, the newer
+        # behaviour is used instead to be consistent with the r0 spec.
+        # see issue #2602
         (code, response) = yield self.mock_resource.trigger_get(
             "/events?access_token=%s" % ("invalid" + self.token, )
         )
-        self.assertEquals(403, code, msg=str(response))
+        self.assertEquals(401, code, msg=str(response))
 
         # valid token, expect content
         (code, response) = yield self.mock_resource.trigger_get(

tests/rest/client/v1/test_profile.py

@@ -52,7 +52,7 @@ def setUp(self):
         def _get_user_by_req(request=None, allow_guest=False):
             return synapse.types.create_requester(myid)
 
-        hs.get_v1auth().get_user_by_req = _get_user_by_req
+        hs.get_auth().get_user_by_req = _get_user_by_req
 
         profile.register_servlets(hs, self.mock_resource)
 

tests/rest/client/v1/test_rooms.py

@@ -60,7 +60,7 @@ def get_user_by_access_token(token=None, allow_guest=False):
                 "token_id": 1,
                 "is_guest": False,
             }
-        hs.get_v1auth().get_user_by_access_token = get_user_by_access_token
+        hs.get_auth().get_user_by_access_token = get_user_by_access_token
 
         def _insert_client_ip(*args, **kwargs):
             return defer.succeed(None)
@@ -70,7 +70,7 @@ def _insert_client_ip(*args, **kwargs):
 
         synapse.rest.client.v1.room.register_servlets(hs, self.mock_resource)
 
-        self.auth = hs.get_v1auth()
+        self.auth = hs.get_auth()
 
         # create some rooms under the name rmcreator_id
         self.uncreated_rmid = "!aa:test"
@@ -425,7 +425,7 @@ def get_user_by_access_token(token=None, allow_guest=False):
                 "token_id": 1,
                 "is_guest": False,
             }
-        hs.get_v1auth().get_user_by_access_token = get_user_by_access_token
+        hs.get_auth().get_user_by_access_token = get_user_by_access_token
 
         def _insert_client_ip(*args, **kwargs):
             return defer.succeed(None)
@@ -507,7 +507,7 @@ def get_user_by_access_token(token=None, allow_guest=False):
                 "token_id": 1,
                 "is_guest": False,
             }
-        hs.get_v1auth().get_user_by_access_token = get_user_by_access_token
+        hs.get_auth().get_user_by_access_token = get_user_by_access_token
 
         def _insert_client_ip(*args, **kwargs):
             return defer.succeed(None)
@@ -597,7 +597,7 @@ def get_user_by_access_token(token=None, allow_guest=False):
                 "is_guest": False,
             }
 
-        hs.get_v1auth().get_user_by_access_token = get_user_by_access_token
+        hs.get_auth().get_user_by_access_token = get_user_by_access_token
 
         def _insert_client_ip(*args, **kwargs):
             return defer.succeed(None)
@@ -711,7 +711,7 @@ def get_user_by_access_token(token=None, allow_guest=False):
                 "token_id": 1,
                 "is_guest": False,
             }
-        hs.get_v1auth().get_user_by_access_token = get_user_by_access_token
+        hs.get_auth().get_user_by_access_token = get_user_by_access_token
 
         def _insert_client_ip(*args, **kwargs):
             return defer.succeed(None)
@@ -843,7 +843,7 @@ def get_user_by_access_token(token=None, allow_guest=False):
                 "token_id": 1,
                 "is_guest": False,
             }
-        hs.get_v1auth().get_user_by_access_token = get_user_by_access_token
+        hs.get_auth().get_user_by_access_token = get_user_by_access_token
 
         def _insert_client_ip(*args, **kwargs):
             return defer.succeed(None)
@@ -945,7 +945,7 @@ def get_user_by_access_token(token=None, allow_guest=False):
                 "token_id": 1,
                 "is_guest": False,
             }
-        hs.get_v1auth().get_user_by_access_token = get_user_by_access_token
+        hs.get_auth().get_user_by_access_token = get_user_by_access_token
 
         def _insert_client_ip(*args, **kwargs):
             return defer.succeed(None)
@@ -1017,7 +1017,7 @@ def get_user_by_access_token(token=None, allow_guest=False):
                 "token_id": 1,
                 "is_guest": False,
             }
-        hs.get_v1auth().get_user_by_access_token = get_user_by_access_token
+        hs.get_auth().get_user_by_access_token = get_user_by_access_token
 
         def _insert_client_ip(*args, **kwargs):
             return defer.succeed(None)

tests/rest/client/v1/test_typing.py

@@ -68,7 +68,7 @@ def get_user_by_access_token(token=None, allow_guest=False):
                 "is_guest": False,
             }
 
-        hs.get_v1auth().get_user_by_access_token = get_user_by_access_token
+        hs.get_auth().get_user_by_access_token = get_user_by_access_token
 
         def _insert_client_ip(*args, **kwargs):
             return defer.succeed(None)