Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Correctly await on_logged_out callbacks #11786

Merged
merged 6 commits into from
Jan 20, 2022
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/11786.bugfix
Original file line number Diff line number Diff line change
@@ -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.
DMRobertson marked this conversation as resolved.
Show resolved Hide resolved
2 changes: 1 addition & 1 deletion synapse/handlers/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Argh are there any tests of the module API which could/should have caught this?

Related: python/mypy#2499, https://bugs.python.org/issue30491

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm good point, I'll add a test for this

except Exception as e:
logger.warning("Failed to run module API callback %s: %s", callback, e)
continue
23 changes: 22 additions & 1 deletion tests/handlers/test_password_providers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -719,6 +721,25 @@ 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()
babolivier marked this conversation as resolved.
Show resolved Hide resolved

def _get_login_flows(self) -> JsonDict:
channel = self.make_request("GET", "/_matrix/client/r0/login")
self.assertEqual(channel.code, 200, channel.result)
Expand Down