diff --git a/changelog.d/20328.bugfix b/changelog.d/20328.bugfix new file mode 100644 index 0000000000..54d33d0d47 --- /dev/null +++ b/changelog.d/20328.bugfix @@ -0,0 +1 @@ +Filter out [MSC4426](https://github.com/matrix-org/matrix-spec-proposals/pull/4426) User Status fields for remote user profile lookups which become stale on clients without [MSC4259](https://github.com/matrix-org/matrix-spec-proposals/pull/4259) to get updates about profiles over federation. \ No newline at end of file diff --git a/synapse/handlers/profile.py b/synapse/handlers/profile.py index 7521975cc5..692ce5bef9 100644 --- a/synapse/handlers/profile.py +++ b/synapse/handlers/profile.py @@ -66,6 +66,12 @@ MAX_AVATAR_URL_LEN = 1000 MAX_CUSTOM_FIELD_LEN = 255 UPDATE_JOIN_STATES_ACTION_NAME = "update_join_states" UPDATE_JOIN_STATES_LOCK_NAME = "update_join_states_lock" +MSC4426_FIELDS = ( + "m.status", + "m.call", + "org.matrix.msc4426.status", + "org.matrix.msc4426.call", +) class ProfileHandler: @@ -161,6 +167,17 @@ class ProfileHandler: args={"user_id": user_id}, ignore_backoff=ignore_backoff, ) + + # Strip out MSC4426 User Status fields as we currently don't have support + # for federating these. Without this, clients will cache stale data + # of user status fields they may receive through a federated profile + # fetch. Until we start delivering profile fields for remote users in + # the sync stream, like we do for local users, we should strip these + # profile fields away to avoid confusing users. + # TODO: remove when profile fields are pushed over federation and the + # profile updates stream is adjusted to deliver remote profile fields. + for key in MSC4426_FIELDS: + result.pop(key, None) return result except RequestSendFailed as e: raise SynapseError(502, "Failed to fetch profile") from e @@ -613,6 +630,16 @@ class ProfileHandler: return field_value else: + # Strip out MSC4426 User Status fields as we currently don't have support + # for federating these. Without this, clients will cache stale data + # of user status fields they may receive through a federated profile field + # fetch. Until we start delivering profile fields for remote users in + # the sync stream, like we do for local users, we should strip these + # profile fields away to avoid confusing users. + # TODO: remove when profile fields are pushed over federation and the + # profile updates stream is adjusted to deliver remote profile fields. + if field_name in MSC4426_FIELDS: + raise SynapseError(404, "Profile was not found", Codes.NOT_FOUND) try: result = await self.federation.make_query( destination=target_user.domain, diff --git a/tests/handlers/test_profile.py b/tests/handlers/test_profile.py index 8ea932c46e..6a37252908 100644 --- a/tests/handlers/test_profile.py +++ b/tests/handlers/test_profile.py @@ -1234,6 +1234,130 @@ class ProfileTestCase(unittest.HomeserverTestCase): self.get_success(self.handler.check_avatar_size_and_mime_type(remote_mxc)) ) + @parameterized.expand( + [ + "m.status", + "m.call", + "org.matrix.msc4426.status", + "org.matrix.msc4426.call", + ] + ) + def test_get_profile_filters_out_msc4426_fields_for_remote_users( + self, field_name: str + ) -> None: + # Set some fields for our local user + self.get_success( + self.handler.set_field( + target_user=self.frank, + requester=synapse.types.create_requester(self.frank), + field_name=field_name, + new_value="value", + ) + ) + self.get_success( + self.handler.set_field( + target_user=self.frank, + requester=synapse.types.create_requester(self.frank), + field_name="some_other_field", + new_value="value", + ) + ) + + # Mock the remote user lookup + self.mock_federation.make_query.return_value = { + field_name: "value", + "some_other_field": "value", + } + + # Test our local user + local_profile = self.get_success( + self.handler.get_profile( + user_id=self.frank.to_string(), + ) + ) + self.assertEqual( + local_profile, + { + "displayname": "1234abcd", + field_name: "value", + "some_other_field": "value", + }, + ) + + # Test our remote user + remote_profile = self.get_success( + self.handler.get_profile( + user_id=self.alice.to_string(), + ) + ) + self.assertEqual( + remote_profile, + { + "some_other_field": "value", + }, + ) + + # Mock the remote user lookup again to only return the filtered field + self.mock_federation.make_query.return_value = { + field_name: "value", + } + + # Ensure empty dictionary + remote_profile = self.get_success( + self.handler.get_profile( + user_id=self.alice.to_string(), + ) + ) + self.assertEqual( + remote_profile, + {}, + ) + + @parameterized.expand( + [ + "m.status", + "m.call", + "org.matrix.msc4426.status", + "org.matrix.msc4426.call", + ] + ) + def test_get_profile_field_filters_out_msc4426_fields_for_remote_users( + self, field_name: str + ) -> None: + # Set a field value for our local user + self.get_success( + self.handler.set_field( + target_user=self.frank, + requester=synapse.types.create_requester(self.frank), + field_name=field_name, + new_value="value", + ) + ) + + # No need to mock the remote user query since we shouldn't do one if + # the field is one that we want to ignore. + + # Test our local user + field_value = self.get_success( + self.handler.get_profile_field( + target_user=self.frank, + field_name=field_name, + ) + ) + self.assertEqual( + field_value, + "value", + ) + + # Test our remote user + self.get_failure( + self.handler.get_profile_field( + target_user=self.alice, + field_name=field_name, + ), + SynapseError, + ) + def _setup_local_files(self, names_and_props: dict[str, dict[str, Any]]) -> None: """Stores metadata about files in the database.