Merge pull request #4420 from matrix-org/jaywink/openid-listener

New listener resource for the federation API "openid/userinfo" endpoint

9 files changed, 252 insertions, 46 deletions

diff --git a/changelog.d/4420.feature b/changelog.d/4420.feature
new file mode 100644
index 0000000000..05e777c624
--- /dev/null
+++ b/changelog.d/4420.feature
@@ -0,0 +1 @@
+Federation OpenID listener resource can now be activated even if federation is disabled
diff --git a/synapse/app/federation_reader.py b/synapse/app/federation_reader.py
index 42886dbfc1..6ee2b76dcd 100644
--- a/synapse/app/federation_reader.py
+++ b/synapse/app/federation_reader.py
@@ -86,6 +86,16 @@ class FederationReaderServer(HomeServer):
                     resources.update({
                         FEDERATION_PREFIX: TransportLayerServer(self),
                     })
+                if name == "openid" and "federation" not in res["names"]:
+                    # Only load the openid resource separately if federation resource
+                    # is not specified since federation resource includes openid
+                    # resource.
+                    resources.update({
+                        FEDERATION_PREFIX: TransportLayerServer(
+                            self,
+                            servlet_groups=["openid"],
+                        ),
+                    })
 
         root_resource = create_resource_tree(resources, NoResource())
 
@@ -98,7 +108,8 @@ class FederationReaderServer(HomeServer):
                 listener_config,
                 root_resource,
                 self.version_string,
-            )
+            ),
+            reactor=self.get_reactor()
         )
 
         logger.info("Synapse federation reader now listening on port %d", port)
diff --git a/synapse/app/homeserver.py b/synapse/app/homeserver.py
index 1a341568ac..d1cab07bb6 100755
--- a/synapse/app/homeserver.py
+++ b/synapse/app/homeserver.py
@@ -99,6 +99,10 @@ class SynapseHomeServer(HomeServer):
         resources = {}
         for res in listener_config["resources"]:
             for name in res["names"]:
+                if name == "openid" and "federation" in res["names"]:
+                    # Skip loading openid resource if federation is defined
+                    # since federation resource will include openid
+                    continue
                 resources.update(self._configure_named_resource(
                     name, res.get("compress", False),
                 ))
@@ -134,6 +138,7 @@ class SynapseHomeServer(HomeServer):
                     self.version_string,
                 ),
                 self.tls_server_context_factory,
+                reactor=self.get_reactor(),
             )
 
         else:
@@ -146,7 +151,8 @@ class SynapseHomeServer(HomeServer):
                     listener_config,
                     root_resource,
                     self.version_string,
-                )
+                ),
+                reactor=self.get_reactor(),
             )
 
     def _configure_named_resource(self, name, compress=False):
@@ -193,6 +199,11 @@ class SynapseHomeServer(HomeServer):
                 FEDERATION_PREFIX: TransportLayerServer(self),
             })
 
+        if name == "openid":
+            resources.update({
+                FEDERATION_PREFIX: TransportLayerServer(self, servlet_groups=["openid"]),
+            })
+
         if name in ["static", "client"]:
             resources.update({
                 STATIC_PREFIX: File(
diff --git a/synapse/config/server.py b/synapse/config/server.py
index 268a43ff00..f0a60cc712 100644
--- a/synapse/config/server.py
+++ b/synapse/config/server.py
@@ -335,6 +335,11 @@ class ServerConfig(Config):
               - names: [federation]  # Federation APIs
                 compress: false
 
+              # # If federation is disabled synapse can still expose the open ID endpoint
+              # # to allow integrations to authenticate users
+              # - names: [openid]
+              #   compress: false
+
             # optional list of additional endpoints which can be loaded via
             # dynamic modules
             # additional_resources:
@@ -356,6 +361,10 @@ class ServerConfig(Config):
                 compress: true
               - names: [federation]
                 compress: false
+              # # If federation is disabled synapse can still expose the open ID endpoint
+              # # to allow integrations to authenticate users
+              # - names: [openid]
+              #   compress: false
 
           # Turn on the twisted ssh manhole service on localhost on the given
           # port.
@@ -480,6 +489,7 @@ KNOWN_RESOURCES = (
     'keys',
     'media',
     'metrics',
+    'openid',
     'replication',
     'static',
     'webclient',
diff --git a/synapse/federation/transport/server.py b/synapse/federation/transport/server.py
index 67ae0212c3..a2396ab466 100644
--- a/synapse/federation/transport/server.py
+++ b/synapse/federation/transport/server.py
@@ -43,9 +43,20 @@ logger = logging.getLogger(__name__)
 class TransportLayerServer(JsonResource):
     """Handles incoming federation HTTP requests"""
 
-    def __init__(self, hs):
+    def __init__(self, hs, servlet_groups=None):
+        """Initialize the TransportLayerServer
+
+        Will by default register all servlets. For custom behaviour, pass in
+        a list of servlet_groups to register.
+
+        Args:
+            hs (synapse.server.HomeServer): homeserver
+            servlet_groups (list[str], optional): List of servlet groups to register.
+                Defaults to ``DEFAULT_SERVLET_GROUPS``.
+        """
         self.hs = hs
         self.clock = hs.get_clock()
+        self.servlet_groups = servlet_groups
 
         super(TransportLayerServer, self).__init__(hs, canonical_json=False)
 
@@ -67,6 +78,7 @@ class TransportLayerServer(JsonResource):
             resource=self,
             ratelimiter=self.ratelimiter,
             authenticator=self.authenticator,
+            servlet_groups=self.servlet_groups,
         )
 
 
@@ -1308,10 +1320,12 @@ FEDERATION_SERVLET_CLASSES = (
     FederationClientKeysClaimServlet,
     FederationThirdPartyInviteExchangeServlet,
     On3pidBindServlet,
-    OpenIdUserInfo,
     FederationVersionServlet,
 )
 
+OPENID_SERVLET_CLASSES = (
+    OpenIdUserInfo,
+)
 
 ROOM_LIST_CLASSES = (
     PublicRoomList,
@@ -1350,44 +1364,83 @@ GROUP_ATTESTATION_SERVLET_CLASSES = (
     FederationGroupsRenewAttestaionServlet,
 )
 
+DEFAULT_SERVLET_GROUPS = (
+    "federation",
+    "room_list",
+    "group_server",
+    "group_local",
+    "group_attestation",
+    "openid",
+)
+
+
+def register_servlets(hs, resource, authenticator, ratelimiter, servlet_groups=None):
+    """Initialize and register servlet classes.
 
-def register_servlets(hs, resource, authenticator, ratelimiter):
-    for servletclass in FEDERATION_SERVLET_CLASSES:
-        servletclass(
-            handler=hs.get_federation_server(),
-            authenticator=authenticator,
-            ratelimiter=ratelimiter,
-            server_name=hs.hostname,
-        ).register(resource)
-
-    for servletclass in ROOM_LIST_CLASSES:
-        servletclass(
-            handler=hs.get_room_list_handler(),
-            authenticator=authenticator,
-            ratelimiter=ratelimiter,
-            server_name=hs.hostname,
-        ).register(resource)
-
-    for servletclass in GROUP_SERVER_SERVLET_CLASSES:
-        servletclass(
-            handler=hs.get_groups_server_handler(),
-            authenticator=authenticator,
-            ratelimiter=ratelimiter,
-            server_name=hs.hostname,
-        ).register(resource)
-
-    for servletclass in GROUP_LOCAL_SERVLET_CLASSES:
-        servletclass(
-            handler=hs.get_groups_local_handler(),
-            authenticator=authenticator,
-            ratelimiter=ratelimiter,
-            server_name=hs.hostname,
-        ).register(resource)
-
-    for servletclass in GROUP_ATTESTATION_SERVLET_CLASSES:
-        servletclass(
-            handler=hs.get_groups_attestation_renewer(),
-            authenticator=authenticator,
-            ratelimiter=ratelimiter,
-            server_name=hs.hostname,
-        ).register(resource)
+    Will by default register all servlets. For custom behaviour, pass in
+    a list of servlet_groups to register.
+
+    Args:
+        hs (synapse.server.HomeServer): homeserver
+        resource (TransportLayerServer): resource class to register to
+        authenticator (Authenticator): authenticator to use
+        ratelimiter (util.ratelimitutils.FederationRateLimiter): ratelimiter to use
+        servlet_groups (list[str], optional): List of servlet groups to register.
+            Defaults to ``DEFAULT_SERVLET_GROUPS``.
+    """
+    if not servlet_groups:
+        servlet_groups = DEFAULT_SERVLET_GROUPS
+
+    if "federation" in servlet_groups:
+        for servletclass in FEDERATION_SERVLET_CLASSES:
+            servletclass(
+                handler=hs.get_federation_server(),
+                authenticator=authenticator,
+                ratelimiter=ratelimiter,
+                server_name=hs.hostname,
+            ).register(resource)
+
+    if "openid" in servlet_groups:
+        for servletclass in OPENID_SERVLET_CLASSES:
+            servletclass(
+                handler=hs.get_federation_server(),
+                authenticator=authenticator,
+                ratelimiter=ratelimiter,
+                server_name=hs.hostname,
+            ).register(resource)
+
+    if "room_list" in servlet_groups:
+        for servletclass in ROOM_LIST_CLASSES:
+            servletclass(
+                handler=hs.get_room_list_handler(),
+                authenticator=authenticator,
+                ratelimiter=ratelimiter,
+                server_name=hs.hostname,
+            ).register(resource)
+
+    if "group_server" in servlet_groups:
+        for servletclass in GROUP_SERVER_SERVLET_CLASSES:
+            servletclass(
+                handler=hs.get_groups_server_handler(),
+                authenticator=authenticator,
+                ratelimiter=ratelimiter,
+                server_name=hs.hostname,
+            ).register(resource)
+
+    if "group_local" in servlet_groups:
+        for servletclass in GROUP_LOCAL_SERVLET_CLASSES:
+            servletclass(
+                handler=hs.get_groups_local_handler(),
+                authenticator=authenticator,
+                ratelimiter=ratelimiter,
+                server_name=hs.hostname,
+            ).register(resource)
+
+    if "group_attestation" in servlet_groups:
+        for servletclass in GROUP_ATTESTATION_SERVLET_CLASSES:
+            servletclass(
+                handler=hs.get_groups_attestation_renewer(),
+                authenticator=authenticator,
+                ratelimiter=ratelimiter,
+                server_name=hs.hostname,
+            ).register(resource)
diff --git a/synapse/python_dependencies.py b/synapse/python_dependencies.py
index 756721e304..5d087ee26b 100644
--- a/synapse/python_dependencies.py
+++ b/synapse/python_dependencies.py
@@ -85,7 +85,7 @@ CONDITIONAL_REQUIREMENTS = {
 
     "saml2": ["pysaml2>=4.5.0"],
     "url_preview": ["lxml>=3.5.0"],
-    "test": ["mock>=2.0"],
+    "test": ["mock>=2.0", "parameterized"],
 }
 
 
diff --git a/tests/app/test_frontend_proxy.py b/tests/app/test_frontend_proxy.py
index a83f567ebd..8bdbc608a9 100644
--- a/tests/app/test_frontend_proxy.py
+++ b/tests/app/test_frontend_proxy.py
@@ -59,7 +59,7 @@ class FrontendProxyTests(HomeserverTestCase):
 
     def test_listen_http_with_presence_disabled(self):
         """
-        When presence is on, the stub servlet will register.
+        When presence is off, the stub servlet will register.
         """
         # Presence is off
         self.hs.config.use_presence = False
diff --git a/tests/app/test_openid_listener.py b/tests/app/test_openid_listener.py
new file mode 100644
index 0000000000..590abc1e92
--- /dev/null
+++ b/tests/app/test_openid_listener.py
@@ -0,0 +1,119 @@
+# -*- coding: utf-8 -*-
+# Copyright 2019 New Vector Ltd
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+from mock import Mock, patch
+
+from parameterized import parameterized
+
+from synapse.app.federation_reader import FederationReaderServer
+from synapse.app.homeserver import SynapseHomeServer
+
+from tests.unittest import HomeserverTestCase
+
+
+class FederationReaderOpenIDListenerTests(HomeserverTestCase):
+    def make_homeserver(self, reactor, clock):
+        hs = self.setup_test_homeserver(
+            http_client=None, homeserverToUse=FederationReaderServer,
+        )
+        return hs
+
+    @parameterized.expand([
+        (["federation"], "auth_fail"),
+        ([], "no_resource"),
+        (["openid", "federation"], "auth_fail"),
+        (["openid"], "auth_fail"),
+    ])
+    def test_openid_listener(self, names, expectation):
+        """
+        Test different openid listener configurations.
+
+        401 is success here since it means we hit the handler and auth failed.
+        """
+        config = {
+            "port": 8080,
+            "bind_addresses": ["0.0.0.0"],
+            "resources": [{"names": names}],
+        }
+
+        # Listen with the config
+        self.hs._listen_http(config)
+
+        # Grab the resource from the site that was told to listen
+        site = self.reactor.tcpServers[0][1]
+        try:
+            self.resource = (
+                site.resource.children[b"_matrix"].children[b"federation"]
+            )
+        except KeyError:
+            if expectation == "no_resource":
+                return
+            raise
+
+        request, channel = self.make_request(
+            "GET",
+            "/_matrix/federation/v1/openid/userinfo",
+        )
+        self.render(request)
+
+        self.assertEqual(channel.code, 401)
+
+
+@patch("synapse.app.homeserver.KeyApiV2Resource", new=Mock())
+class SynapseHomeserverOpenIDListenerTests(HomeserverTestCase):
+    def make_homeserver(self, reactor, clock):
+        hs = self.setup_test_homeserver(
+            http_client=None, homeserverToUse=SynapseHomeServer,
+        )
+        return hs
+
+    @parameterized.expand([
+        (["federation"], "auth_fail"),
+        ([], "no_resource"),
+        (["openid", "federation"], "auth_fail"),
+        (["openid"], "auth_fail"),
+    ])
+    def test_openid_listener(self, names, expectation):
+        """
+        Test different openid listener configurations.
+
+        401 is success here since it means we hit the handler and auth failed.
+        """
+        config = {
+            "port": 8080,
+            "bind_addresses": ["0.0.0.0"],
+            "resources": [{"names": names}],
+        }
+
+        # Listen with the config
+        self.hs._listener_http(config, config)
+
+        # Grab the resource from the site that was told to listen
+        site = self.reactor.tcpServers[0][1]
+        try:
+            self.resource = (
+                site.resource.children[b"_matrix"].children[b"federation"]
+            )
+        except KeyError:
+            if expectation == "no_resource":
+                return
+            raise
+
+        request, channel = self.make_request(
+            "GET",
+            "/_matrix/federation/v1/openid/userinfo",
+        )
+        self.render(request)
+
+        self.assertEqual(channel.code, 401)
diff --git a/tox.ini b/tox.ini
index 9b2d78ed6d..3e2dba2925 100644
--- a/tox.ini
+++ b/tox.ini
@@ -8,6 +8,7 @@ deps =
     python-subunit
     junitxml
     coverage
+    parameterized
 
     # cyptography 2.2 requires setuptools >= 18.5
     #