diff options
| -rw-r--r-- | CHANGES.md | 15 | ||||
| -rw-r--r-- | synapse/__init__.py | 2 | ||||
| -rw-r--r-- | synapse/handlers/auth.py | 37 | ||||
| -rw-r--r-- | synapse/rest/client/v2_alpha/register.py | 1 | ||||
| -rw-r--r-- | synapse/storage/data_stores/main/roommember.py | 3 | ||||
| -rw-r--r-- | tests/rest/client/v2_alpha/test_auth.py | 55 | ||||
| -rw-r--r-- | tests/storage/test_roommember.py | 50 |
7 files changed, 95 insertions, 68 deletions
diff --git a/CHANGES.md b/CHANGES.md index f516dcf0d0..7dbe072fed 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,3 +1,18 @@ +Synapse 1.13.0rc2 (2020-05-14) +============================== + +Bugfixes +-------- + +- Fix a long-standing bug which could cause messages not to be sent over federation, when state events with state keys matching user IDs (such as custom user statuses) were received. ([\#7376](https://github.com/matrix-org/synapse/issues/7376)) +- Restore compatibility with non-compliant clients during the user interactive authentication process, fixing a problem introduced in v1.13.0rc1. ([\#7483](https://github.com/matrix-org/synapse/issues/7483)) + +Internal Changes +---------------- + +- Fix linting errors in new version of Flake8. ([\#7470](https://github.com/matrix-org/synapse/issues/7470)) + + Synapse 1.13.0rc1 (2020-05-11) ============================== diff --git a/synapse/__init__.py b/synapse/__init__.py index 6cd16a820b..977e26a048 100644 --- a/synapse/__init__.py +++ b/synapse/__init__.py @@ -36,7 +36,7 @@ try: except ImportError: pass -__version__ = "1.13.0rc1" +__version__ = "1.13.0rc2" if bool(os.environ.get("SYNAPSE_TEST_PATCH_LOG_CONTEXTS", False)): # We import here so that we don't have to install a bunch of deps when diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py index 65c66a00b1..524281d2f1 100644 --- a/synapse/handlers/auth.py +++ b/synapse/handlers/auth.py @@ -252,7 +252,6 @@ class AuthHandler(BaseHandler): clientdict: Dict[str, Any], clientip: str, description: str, - validate_clientdict: bool = True, ) -> Tuple[dict, dict, str]: """ Takes a dictionary sent by the client in the login / registration @@ -278,10 +277,6 @@ class AuthHandler(BaseHandler): description: A human readable string to be displayed to the user that describes the operation happening on their account. - validate_clientdict: Whether to validate that the operation happening - on the account has not changed. If this is false, - the client dict is persisted instead of validated. - Returns: A tuple of (creds, params, session_id). @@ -346,26 +341,30 @@ class AuthHandler(BaseHandler): # Ensure that the queried operation does not vary between stages of # the UI authentication session. This is done by generating a stable - # comparator based on the URI, method, and client dict (minus the - # auth dict) and storing it during the initial query. Subsequent + # comparator and storing it during the initial query. Subsequent # queries ensure that this comparator has not changed. - if validate_clientdict: - session_comparator = (session.uri, session.method, session.clientdict) - comparator = (uri, method, clientdict) - else: - session_comparator = (session.uri, session.method) # type: ignore - comparator = (uri, method) # type: ignore - - if session_comparator != comparator: + # + # The comparator is based on the requested URI and HTTP method. The + # client dict (minus the auth dict) should also be checked, but some + # clients are not spec compliant, just warn for now if the client + # dict changes. + if (session.uri, session.method) != (uri, method): raise SynapseError( 403, "Requested operation has changed during the UI authentication session.", ) - # For backwards compatibility the registration endpoint persists - # changes to the client dict instead of validating them. - if not validate_clientdict: - await self.store.set_ui_auth_clientdict(sid, clientdict) + if session.clientdict != clientdict: + logger.warning( + "Requested operation has changed during the UI " + "authentication session. A future version of Synapse " + "will remove this capability." + ) + + # For backwards compatibility, changes to the client dict are + # persisted as clients modify them throughout their user interactive + # authentication flow. + await self.store.set_ui_auth_clientdict(sid, clientdict) if not authdict: raise InteractiveAuthIncompleteError( diff --git a/synapse/rest/client/v2_alpha/register.py b/synapse/rest/client/v2_alpha/register.py index e77dd6bf92..af08cc6cce 100644 --- a/synapse/rest/client/v2_alpha/register.py +++ b/synapse/rest/client/v2_alpha/register.py @@ -516,7 +516,6 @@ class RegisterRestServlet(RestServlet): body, self.hs.get_ip_from_request(request), "register a new account", - validate_clientdict=False, ) # Check that we're not trying to register a denied 3pid. diff --git a/synapse/storage/data_stores/main/roommember.py b/synapse/storage/data_stores/main/roommember.py index 04ffdcf934..48810a3e91 100644 --- a/synapse/storage/data_stores/main/roommember.py +++ b/synapse/storage/data_stores/main/roommember.py @@ -566,7 +566,8 @@ class RoomMemberWorkerStore(EventsWorkerStore): if key[0] == EventTypes.Member ] for etype, state_key in context.delta_ids: - users_in_room.pop(state_key, None) + if etype == EventTypes.Member: + users_in_room.pop(state_key, None) # We check if we have any of the member event ids in the event cache # before we ask the DB diff --git a/tests/rest/client/v2_alpha/test_auth.py b/tests/rest/client/v2_alpha/test_auth.py index a56c50a5b7..293ccfba2b 100644 --- a/tests/rest/client/v2_alpha/test_auth.py +++ b/tests/rest/client/v2_alpha/test_auth.py @@ -133,47 +133,6 @@ class FallbackAuthTests(unittest.HomeserverTestCase): # We're given a registered user. self.assertEqual(channel.json_body["user_id"], "@user:test") - def test_legacy_registration(self): - """ - Registration allows the parameters to vary through the process. - """ - - # Make the initial request to register. (Later on a different password - # will be used.) - # Returns a 401 as per the spec - channel = self.register( - 401, {"username": "user", "type": "m.login.password", "password": "bar"}, - ) - - # Grab the session - session = channel.json_body["session"] - # Assert our configured public key is being given - self.assertEqual( - channel.json_body["params"]["m.login.recaptcha"]["public_key"], "brokencake" - ) - - # Complete the recaptcha step. - self.recaptcha(session, 200) - - # also complete the dummy auth - self.register(200, {"auth": {"session": session, "type": "m.login.dummy"}}) - - # Now we should have fulfilled a complete auth flow, including - # the recaptcha fallback step. Make the initial request again, but - # with a changed password. This still completes. - channel = self.register( - 200, - { - "username": "user", - "type": "m.login.password", - "password": "foo", # Note that this is different. - "auth": {"session": session}, - }, - ) - - # We're given a registered user. - self.assertEqual(channel.json_body["user_id"], "@user:test") - def test_complete_operation_unknown_session(self): """ Attempting to mark an invalid session as complete should error. @@ -282,9 +241,15 @@ class UIAuthTests(unittest.HomeserverTestCase): }, ) - def test_cannot_change_body(self): + def test_can_change_body(self): """ - The initial requested client dict cannot be modified during the user interactive authentication session. + The client dict can be modified during the user interactive authentication session. + + Note that it is not spec compliant to modify the client dict during a + user interactive authentication session, but many clients currently do. + + When Synapse is updated to be spec compliant, the call to re-use the + session ID should be rejected. """ # Create a second login. self.login("test", self.user_pass) @@ -302,9 +267,9 @@ class UIAuthTests(unittest.HomeserverTestCase): self.assertIn({"stages": ["m.login.password"]}, channel.json_body["flows"]) # Make another request providing the UI auth flow, but try to delete the - # second device. This results in an error. + # second device. self.delete_devices( - 403, + 200, { "devices": [device_ids[1]], "auth": { diff --git a/tests/storage/test_roommember.py b/tests/storage/test_roommember.py index 00df0ea68e..5dd46005e6 100644 --- a/tests/storage/test_roommember.py +++ b/tests/storage/test_roommember.py @@ -22,6 +22,8 @@ from synapse.rest.client.v1 import login, room from synapse.types import Requester, UserID from tests import unittest +from tests.test_utils import event_injection +from tests.utils import TestHomeServer class RoomMemberStoreTestCase(unittest.HomeserverTestCase): @@ -38,7 +40,7 @@ class RoomMemberStoreTestCase(unittest.HomeserverTestCase): ) return hs - def prepare(self, reactor, clock, hs): + def prepare(self, reactor, clock, hs: TestHomeServer): # We can't test the RoomMemberStore on its own without the other event # storage logic @@ -114,6 +116,52 @@ class RoomMemberStoreTestCase(unittest.HomeserverTestCase): # It now knows about Charlie's server. self.assertEqual(self.store._known_servers_count, 2) + def test_get_joined_users_from_context(self): + room = self.helper.create_room_as(self.u_alice, tok=self.t_alice) + bob_event = event_injection.inject_member_event( + self.hs, room, self.u_bob, Membership.JOIN + ) + + # first, create a regular event + event, context = event_injection.create_event( + self.hs, + room_id=room, + sender=self.u_alice, + prev_event_ids=[bob_event.event_id], + type="m.test.1", + content={}, + ) + + users = self.get_success( + self.store.get_joined_users_from_context(event, context) + ) + self.assertEqual(users.keys(), {self.u_alice, self.u_bob}) + + # Regression test for #7376: create a state event whose key matches bob's + # user_id, but which is *not* a membership event, and persist that; then check + # that `get_joined_users_from_context` returns the correct users for the next event. + non_member_event = event_injection.inject_event( + self.hs, + room_id=room, + sender=self.u_bob, + prev_event_ids=[bob_event.event_id], + type="m.test.2", + state_key=self.u_bob, + content={}, + ) + event, context = event_injection.create_event( + self.hs, + room_id=room, + sender=self.u_alice, + prev_event_ids=[non_member_event.event_id], + type="m.test.3", + content={}, + ) + users = self.get_success( + self.store.get_joined_users_from_context(event, context) + ) + self.assertEqual(users.keys(), {self.u_alice, self.u_bob}) + class CurrentStateMembershipUpdateTestCase(unittest.HomeserverTestCase): def prepare(self, reactor, clock, homeserver): |