From 517f03dffa17549a4d4b14640973cfc7893b95ee Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 20 Jan 2022 16:36:03 +0000 Subject: [PATCH 1/6] Correctly await on_logged_out callbacks --- synapse/handlers/auth.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py index 2389c9ac5214..bd1a3225638a 100644 --- a/synapse/handlers/auth.py +++ b/synapse/handlers/auth.py @@ -2281,7 +2281,7 @@ async def on_logged_out( # call all of the on_logged_out callbacks for callback in self.on_logged_out_callbacks: try: - callback(user_id, device_id, access_token) + await callback(user_id, device_id, access_token) except Exception as e: logger.warning("Failed to run module API callback %s: %s", callback, e) continue From b22989d56c5c04793cc41dbb105d00bf758b6295 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 20 Jan 2022 16:37:59 +0000 Subject: [PATCH 2/6] Changelog --- changelog.d/11786.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/11786.bugfix diff --git a/changelog.d/11786.bugfix b/changelog.d/11786.bugfix new file mode 100644 index 000000000000..306875f2dd63 --- /dev/null +++ b/changelog.d/11786.bugfix @@ -0,0 +1 @@ +Fix a bug introduced in Synapse 1.46.0 that prevented `on_logged_out` module callbacks from being correctly awaited by Synapse. From 31ae2c8790bb7ef7265b7634548def8bfac7879d Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 20 Jan 2022 17:00:22 +0000 Subject: [PATCH 3/6] Add test --- tests/handlers/test_password_providers.py | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/tests/handlers/test_password_providers.py b/tests/handlers/test_password_providers.py index 08e9730d4dfa..1a51ae6b012a 100644 --- a/tests/handlers/test_password_providers.py +++ b/tests/handlers/test_password_providers.py @@ -22,11 +22,12 @@ import synapse from synapse.handlers.auth import load_legacy_password_auth_providers from synapse.module_api import ModuleApi -from synapse.rest.client import devices, login +from synapse.rest.client import devices, login, logout from synapse.types import JsonDict from tests import unittest from tests.server import FakeChannel +from tests.test_utils import make_awaitable from tests.unittest import override_config # (possibly experimental) login flows we expect to appear in the list after the normal @@ -155,6 +156,7 @@ class PasswordAuthProviderTests(unittest.HomeserverTestCase): synapse.rest.admin.register_servlets, login.register_servlets, devices.register_servlets, + logout.register_servlets, ] def setUp(self): @@ -719,6 +721,20 @@ def custom_auth_no_local_user_fallback_test_body(self): channel = self._send_password_login("localuser", "localpass") self.assertEqual(channel.code, 400, channel.result) + def test_on_logged_out(self): + """Tests that the on_logged_out callback is called when the user logs out.""" + self.register_user("rin", "password") + tok = self.login("rin", "password") + + on_logged_out = Mock(return_value=make_awaitable(None)) + self.hs.get_password_auth_provider().on_logged_out_callbacks.append(on_logged_out) + + channel = self.make_request( + "POST", "/_matrix/client/v3/logout", {}, access_token=tok, + ) + self.assertEqual(channel.code, 200) + on_logged_out.assert_called_once() + def _get_login_flows(self) -> JsonDict: channel = self.make_request("GET", "/_matrix/client/r0/login") self.assertEqual(channel.code, 200, channel.result) From 6bf3b627555613deb27cb1abbe18b23a21e2a12f Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 20 Jan 2022 17:02:01 +0000 Subject: [PATCH 4/6] Lint --- tests/handlers/test_password_providers.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/tests/handlers/test_password_providers.py b/tests/handlers/test_password_providers.py index 1a51ae6b012a..acc475106e10 100644 --- a/tests/handlers/test_password_providers.py +++ b/tests/handlers/test_password_providers.py @@ -727,10 +727,15 @@ def test_on_logged_out(self): tok = self.login("rin", "password") on_logged_out = Mock(return_value=make_awaitable(None)) - self.hs.get_password_auth_provider().on_logged_out_callbacks.append(on_logged_out) + self.hs.get_password_auth_provider().on_logged_out_callbacks.append( + on_logged_out + ) channel = self.make_request( - "POST", "/_matrix/client/v3/logout", {}, access_token=tok, + "POST", + "/_matrix/client/v3/logout", + {}, + access_token=tok, ) self.assertEqual(channel.code, 200) on_logged_out.assert_called_once() From 556d9eb9968b1e5e927eee40da04537ef0301d29 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 20 Jan 2022 17:28:31 +0000 Subject: [PATCH 5/6] Fix tests --- tests/handlers/test_password_providers.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/handlers/test_password_providers.py b/tests/handlers/test_password_providers.py index acc475106e10..cc2bdebe4462 100644 --- a/tests/handlers/test_password_providers.py +++ b/tests/handlers/test_password_providers.py @@ -726,7 +726,12 @@ def test_on_logged_out(self): self.register_user("rin", "password") tok = self.login("rin", "password") - on_logged_out = Mock(return_value=make_awaitable(None)) + self.called = False + + async def on_logged_out(user_id, device_id, access_token): + self.called = True + + on_logged_out = Mock(side_effect=on_logged_out) self.hs.get_password_auth_provider().on_logged_out_callbacks.append( on_logged_out ) @@ -739,6 +744,7 @@ def test_on_logged_out(self): ) self.assertEqual(channel.code, 200) on_logged_out.assert_called_once() + self.assertTrue(self.called) def _get_login_flows(self) -> JsonDict: channel = self.make_request("GET", "/_matrix/client/r0/login") From 9e974cfd9dedca75d3d787c47bb89ab6f46c32e7 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 20 Jan 2022 17:32:53 +0000 Subject: [PATCH 6/6] Lint --- tests/handlers/test_password_providers.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/handlers/test_password_providers.py b/tests/handlers/test_password_providers.py index cc2bdebe4462..2add72b28a3a 100644 --- a/tests/handlers/test_password_providers.py +++ b/tests/handlers/test_password_providers.py @@ -27,7 +27,6 @@ from tests import unittest from tests.server import FakeChannel -from tests.test_utils import make_awaitable from tests.unittest import override_config # (possibly experimental) login flows we expect to appear in the list after the normal