Remove the requirement to authenticate for /admin/server_version. (#5122)

This endpoint isn't much use for its intended purpose if you first need to get
yourself an admin's auth token.

I've restricted it to the `/_synapse/admin` path to make it a bit easier to
lock down for those concerned about exposing this information. I don't imagine
anyone is using it in anger currently.

5 files changed, 32 insertions, 38 deletions

diff --git a/changelog.d/5122.misc b/changelog.d/5122.misc
new file mode 100644
index 0000000000..e1be8a6210
--- /dev/null
+++ b/changelog.d/5122.misc
@@ -0,0 +1 @@
+Remove the requirement to authenticate for /admin/server_version.
diff --git a/docs/admin_api/version_api.rst b/docs/admin_api/version_api.rst
index 6d66543b96..833d9028be 100644
--- a/docs/admin_api/version_api.rst
+++ b/docs/admin_api/version_api.rst
@@ -10,8 +10,6 @@ The api is::
 
     GET /_synapse/admin/v1/server_version
 
-including an ``access_token`` of a server admin.
-
 It returns a JSON body like the following:
 
 .. code:: json
diff --git a/synapse/rest/admin/__init__.py b/synapse/rest/admin/__init__.py
index 0ce89741f0..744d85594f 100644
--- a/synapse/rest/admin/__init__.py
+++ b/synapse/rest/admin/__init__.py
@@ -88,21 +88,16 @@ class UsersRestServlet(RestServlet):
 
 
 class VersionServlet(RestServlet):
-    PATTERNS = historical_admin_path_patterns("/server_version")
+    PATTERNS = (re.compile("^/_synapse/admin/v1/server_version$"), )
 
     def __init__(self, hs):
-        self.auth = hs.get_auth()
-
-    @defer.inlineCallbacks
-    def on_GET(self, request):
-        yield assert_requester_is_admin(self.auth, request)
-
-        ret = {
+        self.res = {
             'server_version': get_version_string(synapse),
             'python_version': platform.python_version(),
         }
 
-        defer.returnValue((200, ret))
+    def on_GET(self, request):
+        return 200, self.res
 
 
 class UserRegisterServlet(RestServlet):
@@ -830,6 +825,7 @@ class AdminRestResource(JsonResource):
 
         register_servlets_for_client_rest_resource(hs, self)
         SendServerNoticeServlet(hs).register(self)
+        VersionServlet(hs).register(self)
 
 
 def register_servlets_for_client_rest_resource(hs, http_server):
@@ -847,7 +843,6 @@ def register_servlets_for_client_rest_resource(hs, http_server):
     QuarantineMediaInRoom(hs).register(http_server)
     ListMediaInRoom(hs).register(http_server)
     UserRegisterServlet(hs).register(http_server)
-    VersionServlet(hs).register(http_server)
     DeleteGroupAdminRestServlet(hs).register(http_server)
     AccountValidityRenewServlet(hs).register(http_server)
     # don't add more things here: new servlets should only be exposed on
diff --git a/tests/rest/admin/test_admin.py b/tests/rest/admin/test_admin.py
index db4cfd8550..da19a83918 100644
--- a/tests/rest/admin/test_admin.py
+++ b/tests/rest/admin/test_admin.py
@@ -21,6 +21,8 @@ from mock import Mock
 
 import synapse.rest.admin
 from synapse.api.constants import UserTypes
+from synapse.http.server import JsonResource
+from synapse.rest.admin import VersionServlet
 from synapse.rest.client.v1 import events, login, room
 from synapse.rest.client.v2_alpha import groups
 
@@ -28,20 +30,15 @@ from tests import unittest
 
 
 class VersionTestCase(unittest.HomeserverTestCase):
+    url = '/_synapse/admin/v1/server_version'
 
-    servlets = [
-        synapse.rest.admin.register_servlets_for_client_rest_resource,
-        login.register_servlets,
-    ]
-
-    url = '/_matrix/client/r0/admin/server_version'
+    def create_test_json_resource(self):
+        resource = JsonResource(self.hs)
+        VersionServlet(self.hs).register(resource)
+        return resource
 
     def test_version_string(self):
-        self.register_user("admin", "pass", admin=True)
-        self.admin_token = self.login("admin", "pass")
-
-        request, channel = self.make_request("GET", self.url,
-                                             access_token=self.admin_token)
+        request, channel = self.make_request("GET", self.url, shorthand=False)
         self.render(request)
 
         self.assertEqual(200, int(channel.result["code"]),
@@ -49,17 +46,6 @@ class VersionTestCase(unittest.HomeserverTestCase):
         self.assertEqual({'server_version', 'python_version'},
                          set(channel.json_body.keys()))
 
-    def test_inaccessible_to_non_admins(self):
-        self.register_user("unprivileged-user", "pass", admin=False)
-        user_token = self.login("unprivileged-user", "pass")
-
-        request, channel = self.make_request("GET", self.url,
-                                             access_token=user_token)
-        self.render(request)
-
-        self.assertEqual(403, int(channel.result['code']),
-                         msg=channel.result['body'])
-
 
 class UserRegisterTestCase(unittest.HomeserverTestCase):
 
diff --git a/tests/unittest.py b/tests/unittest.py
index 8c65736a51..029a88d770 100644
--- a/tests/unittest.py
+++ b/tests/unittest.py
@@ -181,10 +181,7 @@ class HomeserverTestCase(TestCase):
             raise Exception("A homeserver wasn't returned, but %r" % (self.hs,))
 
         # Register the resources
-        self.resource = JsonResource(self.hs)
-
-        for servlet in self.servlets:
-            servlet(self.hs, self.resource)
+        self.resource = self.create_test_json_resource()
 
         from tests.rest.client.v1.utils import RestHelper
 
@@ -230,6 +227,23 @@ class HomeserverTestCase(TestCase):
         hs = self.setup_test_homeserver()
         return hs
 
+    def create_test_json_resource(self):
+        """
+        Create a test JsonResource, with the relevant servlets registerd to it
+
+        The default implementation calls each function in `servlets` to do the
+        registration.
+
+        Returns:
+            JsonResource:
+        """
+        resource = JsonResource(self.hs)
+
+        for servlet in self.servlets:
+            servlet(self.hs, resource)
+
+        return resource
+
     def default_config(self, name="test"):
         """
         Get a default HomeServer config object.