From a13a6df2cabc86aa3be032698a002eb1633b340b Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 8 Mar 2023 18:19:18 +0000 Subject: [PATCH 1/4] Fix missing conditional --- synapse/events/third_party_rules.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/synapse/events/third_party_rules.py b/synapse/events/third_party_rules.py index 3e4d52c8d803..61d4530be784 100644 --- a/synapse/events/third_party_rules.py +++ b/synapse/events/third_party_rules.py @@ -247,6 +247,11 @@ def register_third_party_rules_callbacks( on_add_user_third_party_identifier ) + if on_remove_user_third_party_identifier is not None: + self._on_remove_user_third_party_identifier_callbacks.append( + on_remove_user_third_party_identifier + ) + async def check_event_allowed( self, event: EventBase, From 0e9d64aecf8d3aa68bbd350b54ac0f28afeb8a96 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 8 Mar 2023 18:27:40 +0000 Subject: [PATCH 2/4] Update tests to catch this case. The tests were modifying the callback object directly instead of using the method that modules would use, thus bypassing the missing code. --- tests/rest/client/test_third_party_rules.py | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/tests/rest/client/test_third_party_rules.py b/tests/rest/client/test_third_party_rules.py index 3b9951370752..355024ff81a0 100644 --- a/tests/rest/client/test_third_party_rules.py +++ b/tests/rest/client/test_third_party_rules.py @@ -941,18 +941,16 @@ def test_on_add_and_remove_user_third_party_identifier(self) -> None: just before associating and removing a 3PID to/from an account. """ # Pretend to be a Synapse module and register both callbacks as mocks. - third_party_rules = self.hs.get_third_party_event_rules() on_add_user_third_party_identifier_callback_mock = Mock( return_value=make_awaitable(None) ) on_remove_user_third_party_identifier_callback_mock = Mock( return_value=make_awaitable(None) ) - third_party_rules._on_threepid_bind_callbacks.append( - on_add_user_third_party_identifier_callback_mock - ) - third_party_rules._on_threepid_bind_callbacks.append( - on_remove_user_third_party_identifier_callback_mock + third_party_rules = self.hs.get_third_party_event_rules() + third_party_rules.register_third_party_rules_callbacks( + on_add_user_third_party_identifier=on_add_user_third_party_identifier_callback_mock, + on_remove_user_third_party_identifier=on_remove_user_third_party_identifier_callback_mock, ) # Register an admin user. @@ -1008,12 +1006,12 @@ def test_on_remove_user_third_party_identifier_is_called_on_deactivate( when a user is deactivated and their third-party ID associations are deleted. """ # Pretend to be a Synapse module and register both callbacks as mocks. - third_party_rules = self.hs.get_third_party_event_rules() on_remove_user_third_party_identifier_callback_mock = Mock( return_value=make_awaitable(None) ) - third_party_rules._on_threepid_bind_callbacks.append( - on_remove_user_third_party_identifier_callback_mock + third_party_rules = self.hs.get_third_party_event_rules() + third_party_rules.register_third_party_rules_callbacks( + on_threepid_bind=on_remove_user_third_party_identifier_callback_mock, ) # Register an admin user. @@ -1039,6 +1037,9 @@ def test_on_remove_user_third_party_identifier_is_called_on_deactivate( ) self.assertEqual(channel.code, 200, channel.json_body) + # Check that the mock was not called on the act of adding a third-party ID. + on_remove_user_third_party_identifier_callback_mock.assert_not_called() + # Now deactivate the user. channel = self.make_request( "PUT", From 9cad8cf53f6866c90c13d3d1dc7af8192f88a9a4 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 8 Mar 2023 18:35:58 +0000 Subject: [PATCH 3/4] changelog --- changelog.d/15227.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/15227.bugfix diff --git a/changelog.d/15227.bugfix b/changelog.d/15227.bugfix new file mode 100644 index 000000000000..eaa26c8f7fa3 --- /dev/null +++ b/changelog.d/15227.bugfix @@ -0,0 +1 @@ +Fix a bug introduced in Synapse 1.79.0rc1 where attempting to register a `on_remove_user_third_party_identifier` module API callback would be a no-op. \ No newline at end of file From e384ff24ce7e342f5adb95061cc06189acdda58d Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Thu, 9 Mar 2023 16:57:43 +0000 Subject: [PATCH 4/4] fix the tests like I said I would --- tests/rest/client/test_third_party_rules.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/rest/client/test_third_party_rules.py b/tests/rest/client/test_third_party_rules.py index 355024ff81a0..753ecc8d161a 100644 --- a/tests/rest/client/test_third_party_rules.py +++ b/tests/rest/client/test_third_party_rules.py @@ -1011,7 +1011,7 @@ def test_on_remove_user_third_party_identifier_is_called_on_deactivate( ) third_party_rules = self.hs.get_third_party_event_rules() third_party_rules.register_third_party_rules_callbacks( - on_threepid_bind=on_remove_user_third_party_identifier_callback_mock, + on_remove_user_third_party_identifier=on_remove_user_third_party_identifier_callback_mock, ) # Register an admin user.