From ab97b6e33c6e141d185ec203d24ea8266f924900 Mon Sep 17 00:00:00 2001 From: Jason Robinson Date: Thu, 17 Jan 2019 15:52:57 +0200 Subject: [PATCH 01/14] Fix a test docstring in frontend proxy tests Signed-off-by: Jason Robinson --- tests/app/test_frontend_proxy.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 From 899e60be80736e3b747eb567171296829e96f716 Mon Sep 17 00:00:00 2001 From: Jason Robinson Date: Thu, 17 Jan 2019 15:55:37 +0200 Subject: [PATCH 02/14] Add parameterized Python module to test dependencies Allows running parameterized tests. BSD license. Signed-off-by: Jason Robinson --- synapse/python_dependencies.py | 2 +- tox.ini | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/synapse/python_dependencies.py b/synapse/python_dependencies.py index 882e844eb1..b5555fbba9 100644 --- a/synapse/python_dependencies.py +++ b/synapse/python_dependencies.py @@ -81,7 +81,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/tox.ini b/tox.ini index a0f5486829..9b6cb2036c 100644 --- a/tox.ini +++ b/tox.ini @@ -8,6 +8,7 @@ deps = python-subunit junitxml coverage + parameterized # cyptography 2.2 requires setuptools >= 18.5 # From 4f8f41c824dc326903863625ce79e5386ad0f8e1 Mon Sep 17 00:00:00 2001 From: Jason Robinson Date: Thu, 17 Jan 2019 15:58:27 +0200 Subject: [PATCH 03/14] Make FederationReaderServer _http_listen use self.get_reactor() For all the homeserver classes, only the FrontendProxyServer passes its reactor when doing the http listen. Looking at previous PR's looks like this was introduced to make it possible to write a test, otherwise when you try to run a test with the test homeserver it tries to do a real bind to a port. Passing the reactor that the homeserver is instantiated with should probably be the right thing to do anyway? Signed-off-by: Jason Robinson --- synapse/app/federation_reader.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/synapse/app/federation_reader.py b/synapse/app/federation_reader.py index 228a297fb8..ea594f0f1a 100644 --- a/synapse/app/federation_reader.py +++ b/synapse/app/federation_reader.py @@ -99,7 +99,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) From 6d255990983405fe7eb1322c9d02414c859d1e96 Mon Sep 17 00:00:00 2001 From: Jason Robinson Date: Thu, 17 Jan 2019 15:59:28 +0200 Subject: [PATCH 04/14] Add tests for the openid lister for FederationReaderServer Check all possible variants of openid and federation listener on/off possibilities. Signed-off-by: Jason Robinson --- tests/app/test_openid_listener.py | 66 +++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) create mode 100644 tests/app/test_openid_listener.py diff --git a/tests/app/test_openid_listener.py b/tests/app/test_openid_listener.py new file mode 100644 index 0000000000..cf3baad96f --- /dev/null +++ b/tests/app/test_openid_listener.py @@ -0,0 +1,66 @@ +# -*- coding: utf-8 -*- +# Copyright 2018 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 patch, Mock +from parameterized import parameterized + +from synapse.app.federation_reader import FederationReaderServer + +from tests.unittest import HomeserverTestCase + + +@patch("synapse.app.homeserver.KeyApiV2Resource", new=Mock()) +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"].children[b"v1"] + ) + 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) From a17bac171f301e54d6bd3d4c769f4f005c38f112 Mon Sep 17 00:00:00 2001 From: Jason Robinson Date: Fri, 18 Jan 2019 10:02:08 +0200 Subject: [PATCH 05/14] Make SynapseHomeServer _http_listener use self.get_reactor() For all the homeserver classes, only the FrontendProxyServer passes its reactor when doing the http listen. Looking at previous PR's looks like this was introduced to make it possible to write a test, otherwise when you try to run a test with the test homeserver it tries to do a real bind to a port. Passing the reactor that the homeserver is instantiated with should probably be the right thing to do anyway? Signed-off-by: Jason Robinson --- synapse/app/homeserver.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/synapse/app/homeserver.py b/synapse/app/homeserver.py index f3ac3d19f0..5652c6201c 100755 --- a/synapse/app/homeserver.py +++ b/synapse/app/homeserver.py @@ -130,6 +130,7 @@ class SynapseHomeServer(HomeServer): self.version_string, ), self.tls_server_context_factory, + reactor=self.get_reactor(), ) else: @@ -142,7 +143,8 @@ class SynapseHomeServer(HomeServer): listener_config, root_resource, self.version_string, - ) + ), + reactor=self.get_reactor(), ) logger.info("Synapse now listening on port %d", port) From 5336e49b39b6ad73491531c68694c92dade028be Mon Sep 17 00:00:00 2001 From: Jason Robinson Date: Fri, 18 Jan 2019 10:03:32 +0200 Subject: [PATCH 06/14] Add tests for the openid lister for SynapseHomeServer Check all possible variants of openid and federation listener on/off possibilities. Signed-off-by: Jason Robinson --- tests/app/test_openid_listener.py | 49 ++++++++++++++++++++++++++++++- 1 file changed, 48 insertions(+), 1 deletion(-) diff --git a/tests/app/test_openid_listener.py b/tests/app/test_openid_listener.py index cf3baad96f..329aadc3d3 100644 --- a/tests/app/test_openid_listener.py +++ b/tests/app/test_openid_listener.py @@ -1,5 +1,5 @@ # -*- coding: utf-8 -*- -# Copyright 2018 New Vector Ltd +# 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. @@ -16,6 +16,7 @@ from mock import patch, Mock from parameterized import parameterized from synapse.app.federation_reader import FederationReaderServer +from synapse.app.homeserver import SynapseHomeServer from tests.unittest import HomeserverTestCase @@ -64,3 +65,49 @@ class FederationReaderOpenIDListenerTests(HomeserverTestCase): 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"].children[b"v1"] + ) + 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) From 82e13662c03a41c085e784a594b423711e0caffa Mon Sep 17 00:00:00 2001 From: Jason Robinson Date: Mon, 21 Jan 2019 01:54:43 +0200 Subject: [PATCH 07/14] Split federation OpenID userinfo endpoint out of the federation resource This allows the OpenID userinfo endpoint to be active even if the federation resource is not active. The OpenID userinfo endpoint is called by integration managers to verify user actions using the client API OpenID access token. Without this verification, the integration manager cannot know that the access token is valid. The OpenID userinfo endpoint will be loaded in the case that either "federation" or "openid" resource is defined. The new "openid" resource is defaulted to active in default configuration. Signed-off-by: Jason Robinson --- synapse/app/federation_reader.py | 7 ++ synapse/app/homeserver.py | 9 +++ synapse/config/server.py | 9 ++- synapse/federation/transport/server.py | 106 ++++++++++++++++--------- 4 files changed, 89 insertions(+), 42 deletions(-) diff --git a/synapse/app/federation_reader.py b/synapse/app/federation_reader.py index ea594f0f1a..99e8a4cf6a 100644 --- a/synapse/app/federation_reader.py +++ b/synapse/app/federation_reader.py @@ -87,6 +87,13 @@ 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()) diff --git a/synapse/app/homeserver.py b/synapse/app/homeserver.py index 5652c6201c..0a924d7a80 100755 --- a/synapse/app/homeserver.py +++ b/synapse/app/homeserver.py @@ -95,6 +95,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), )) @@ -192,6 +196,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 fb57791098..556f1efee5 100644 --- a/synapse/config/server.py +++ b/synapse/config/server.py @@ -151,7 +151,7 @@ class ServerConfig(Config): "compress": gzip_responses, }, { - "names": ["federation"], + "names": ["federation", "openid"], "compress": False, } ] @@ -170,7 +170,7 @@ class ServerConfig(Config): "compress": gzip_responses, }, { - "names": ["federation"], + "names": ["federation", "openid"], "compress": False, } ] @@ -328,7 +328,7 @@ class ServerConfig(Config): # that can do automatic compression. compress: true - - names: [federation] # Federation APIs + - names: [federation, openid] # Federation APIs compress: false # optional list of additional endpoints which can be loaded via @@ -350,7 +350,7 @@ class ServerConfig(Config): resources: - names: [client] compress: true - - names: [federation] + - names: [federation, openid] compress: false # Turn on the twisted ssh manhole service on localhost on the given @@ -477,6 +477,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 4557a9e66e..49b80beecb 100644 --- a/synapse/federation/transport/server.py +++ b/synapse/federation/transport/server.py @@ -43,9 +43,10 @@ logger = logging.getLogger(__name__) class TransportLayerServer(JsonResource): """Handles incoming federation HTTP requests""" - def __init__(self, hs): + def __init__(self, hs, servlet_groups=None): self.hs = hs self.clock = hs.get_clock() + self.servlet_groups = servlet_groups super(TransportLayerServer, self).__init__(hs, canonical_json=False) @@ -67,6 +68,7 @@ class TransportLayerServer(JsonResource): resource=self, ratelimiter=self.ratelimiter, authenticator=self.authenticator, + servlet_groups=self.servlet_groups, ) @@ -1308,10 +1310,12 @@ FEDERATION_SERVLET_CLASSES = ( FederationClientKeysClaimServlet, FederationThirdPartyInviteExchangeServlet, On3pidBindServlet, - OpenIdUserInfo, FederationVersionServlet, ) +OPENID_SERVLET_CLASSES = ( + OpenIdUserInfo, +) ROOM_LIST_CLASSES = ( PublicRoomList, @@ -1350,44 +1354,70 @@ 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): - 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) +def register_servlets(hs, resource, authenticator, ratelimiter, servlet_groups=None): + if not servlet_groups: + servlet_groups = DEFAULT_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 "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) - 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 "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) - for servletclass in GROUP_ATTESTATION_SERVLET_CLASSES: - servletclass( - handler=hs.get_groups_attestation_renewer(), - 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) From 1d2c69fee897cf052cfa03f0cc6f9f419c898bb1 Mon Sep 17 00:00:00 2001 From: Jason Robinson Date: Mon, 21 Jan 2019 01:59:18 +0200 Subject: [PATCH 08/14] Add changelog for openid resource addition Signed-off-by: Jason Robinson --- changelog.d/4420.feature | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 changelog.d/4420.feature diff --git a/changelog.d/4420.feature b/changelog.d/4420.feature new file mode 100644 index 0000000000..5e684d01e0 --- /dev/null +++ b/changelog.d/4420.feature @@ -0,0 +1,13 @@ +New listener resource for the federation API "openid/userinfo" endpoint + +Integration managers use the OpenID userinfo endpoint in the federation API to verify that user +OpenID access tokens are valid. If the federation resource is disabled, integration managers will not be able +to verify the access token, causing a broken experience for users. The OpenID userinfo endpoint has now been split +to a separate `openid` resource, which is enabled by default in newly generated configuration. It is also enabled +automatically if the federation resource is enabled. + +If your homeserver runs federation enabled, this change does not require any actions. + +If you run a homeserver with federation disabled, we recommend adding the `openid` resource to your homeserver +configuration in the `type: http` listener `resources` list to allow your users access to +integration manager features. From d39b7b6d38b0d9876ad305491ea05af92ea35b04 Mon Sep 17 00:00:00 2001 From: Jason Robinson Date: Tue, 22 Jan 2019 11:00:17 +0200 Subject: [PATCH 09/14] Document `servlet_groups` parameters Signed-off-by: Jason Robinson --- synapse/federation/transport/server.py | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/synapse/federation/transport/server.py b/synapse/federation/transport/server.py index 49b80beecb..9c1c9c39e1 100644 --- a/synapse/federation/transport/server.py +++ b/synapse/federation/transport/server.py @@ -44,6 +44,16 @@ class TransportLayerServer(JsonResource): """Handles incoming federation HTTP requests""" 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 @@ -1365,6 +1375,19 @@ DEFAULT_SERVLET_GROUPS = ( def register_servlets(hs, resource, authenticator, ratelimiter, servlet_groups=None): + """Initialize and register servlet classes. + + 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 From 0516dc4d85f1018beb241df6833117b8b49d08d3 Mon Sep 17 00:00:00 2001 From: Jason Robinson Date: Tue, 22 Jan 2019 11:04:10 +0200 Subject: [PATCH 10/14] Remove openid resource from default config Instead document it commented out. Signed-off-by: Jason Robinson --- synapse/config/server.py | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/synapse/config/server.py b/synapse/config/server.py index 556f1efee5..eebbfccafe 100644 --- a/synapse/config/server.py +++ b/synapse/config/server.py @@ -151,7 +151,7 @@ class ServerConfig(Config): "compress": gzip_responses, }, { - "names": ["federation", "openid"], + "names": ["federation"], "compress": False, } ] @@ -170,7 +170,7 @@ class ServerConfig(Config): "compress": gzip_responses, }, { - "names": ["federation", "openid"], + "names": ["federation"], "compress": False, } ] @@ -328,8 +328,13 @@ class ServerConfig(Config): # that can do automatic compression. compress: true - - names: [federation, openid] # Federation APIs + - 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 @@ -350,8 +355,12 @@ class ServerConfig(Config): resources: - names: [client] compress: true - - names: [federation, openid] + - 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. From db33634b1dc47167cffce31ff4ae44c5d3fae2af Mon Sep 17 00:00:00 2001 From: Jason Robinson Date: Tue, 22 Jan 2019 11:05:22 +0200 Subject: [PATCH 11/14] Collapse changelog to one line Signed-off-by: Jason Robinson --- changelog.d/4420.feature | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/changelog.d/4420.feature b/changelog.d/4420.feature index 5e684d01e0..05e777c624 100644 --- a/changelog.d/4420.feature +++ b/changelog.d/4420.feature @@ -1,13 +1 @@ -New listener resource for the federation API "openid/userinfo" endpoint - -Integration managers use the OpenID userinfo endpoint in the federation API to verify that user -OpenID access tokens are valid. If the federation resource is disabled, integration managers will not be able -to verify the access token, causing a broken experience for users. The OpenID userinfo endpoint has now been split -to a separate `openid` resource, which is enabled by default in newly generated configuration. It is also enabled -automatically if the federation resource is enabled. - -If your homeserver runs federation enabled, this change does not require any actions. - -If you run a homeserver with federation disabled, we recommend adding the `openid` resource to your homeserver -configuration in the `type: http` listener `resources` list to allow your users access to -integration manager features. +Federation OpenID listener resource can now be activated even if federation is disabled From a47fac9af6b17ae60c381675caece6e846faefff Mon Sep 17 00:00:00 2001 From: Jason Robinson Date: Tue, 22 Jan 2019 11:07:28 +0200 Subject: [PATCH 12/14] Fix sorting of imports in tests. Remove an unnecessary mock Signed-off-by: Jason Robinson --- tests/app/test_openid_listener.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/app/test_openid_listener.py b/tests/app/test_openid_listener.py index 329aadc3d3..c1ebc93f52 100644 --- a/tests/app/test_openid_listener.py +++ b/tests/app/test_openid_listener.py @@ -12,7 +12,8 @@ # 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 patch, Mock +from mock import Mock, patch + from parameterized import parameterized from synapse.app.federation_reader import FederationReaderServer @@ -21,7 +22,6 @@ from synapse.app.homeserver import SynapseHomeServer from tests.unittest import HomeserverTestCase -@patch("synapse.app.homeserver.KeyApiV2Resource", new=Mock()) class FederationReaderOpenIDListenerTests(HomeserverTestCase): def make_homeserver(self, reactor, clock): hs = self.setup_test_homeserver( From 1838ef1ac39d624e21786120b402d04a0c4485ba Mon Sep 17 00:00:00 2001 From: Jason Robinson Date: Wed, 23 Jan 2019 10:38:13 +0200 Subject: [PATCH 13/14] Fix openid tests after rebase Signed-off-by: Jason Robinson --- tests/app/test_openid_listener.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/app/test_openid_listener.py b/tests/app/test_openid_listener.py index c1ebc93f52..46a3b61a3c 100644 --- a/tests/app/test_openid_listener.py +++ b/tests/app/test_openid_listener.py @@ -54,7 +54,7 @@ class FederationReaderOpenIDListenerTests(HomeserverTestCase): site = self.reactor.tcpServers[0][1] try: self.resource = ( - site.resource.children[b"_matrix"].children[b"federation"].children[b"v1"] + site.resource.children[b"_matrix"].children[b"federation"] ) except KeyError: if expectation == "no_resource": @@ -100,7 +100,7 @@ class SynapseHomeserverOpenIDListenerTests(HomeserverTestCase): site = self.reactor.tcpServers[0][1] try: self.resource = ( - site.resource.children[b"_matrix"].children[b"federation"].children[b"v1"] + site.resource.children[b"_matrix"].children[b"federation"] ) except KeyError: if expectation == "no_resource": From 6f680241bd7f53c3a3f912c257d2bfa437dfd486 Mon Sep 17 00:00:00 2001 From: Jason Robinson Date: Wed, 23 Jan 2019 10:53:48 +0200 Subject: [PATCH 14/14] Fix flake8 issues Signed-off-by: Jason Robinson --- synapse/app/federation_reader.py | 5 ++++- synapse/config/server.py | 2 +- tests/app/test_openid_listener.py | 10 ++++++++-- 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/synapse/app/federation_reader.py b/synapse/app/federation_reader.py index 99e8a4cf6a..2c99ce8c64 100644 --- a/synapse/app/federation_reader.py +++ b/synapse/app/federation_reader.py @@ -92,7 +92,10 @@ class FederationReaderServer(HomeServer): # is not specified since federation resource includes openid # resource. resources.update({ - FEDERATION_PREFIX: TransportLayerServer(self, servlet_groups=["openid"]), + FEDERATION_PREFIX: TransportLayerServer( + self, + servlet_groups=["openid"], + ), }) root_resource = create_resource_tree(resources, NoResource()) diff --git a/synapse/config/server.py b/synapse/config/server.py index eebbfccafe..4eefd06f4a 100644 --- a/synapse/config/server.py +++ b/synapse/config/server.py @@ -330,7 +330,7 @@ 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] diff --git a/tests/app/test_openid_listener.py b/tests/app/test_openid_listener.py index 46a3b61a3c..590abc1e92 100644 --- a/tests/app/test_openid_listener.py +++ b/tests/app/test_openid_listener.py @@ -61,7 +61,10 @@ class FederationReaderOpenIDListenerTests(HomeserverTestCase): return raise - request, channel = self.make_request("GET", "/_matrix/federation/v1/openid/userinfo") + request, channel = self.make_request( + "GET", + "/_matrix/federation/v1/openid/userinfo", + ) self.render(request) self.assertEqual(channel.code, 401) @@ -107,7 +110,10 @@ class SynapseHomeserverOpenIDListenerTests(HomeserverTestCase): return raise - request, channel = self.make_request("GET", "/_matrix/federation/v1/openid/userinfo") + request, channel = self.make_request( + "GET", + "/_matrix/federation/v1/openid/userinfo", + ) self.render(request) self.assertEqual(channel.code, 401)