Filter out MSC4426 User Status profile fields for remote users (#20328)

Several clients, including Element Web/Desktop and Element X, have
already implemented
[MSC4426](https://github.com/matrix-org/matrix-spec-proposals/pull/4426)
User Status profile fields. However, we have only implemented sending
profile fields down sync for local users
([MSC4429](https://github.com/matrix-org/matrix-spec-proposals/pull/4429)/[MSC4262](https://github.com/matrix-org/matrix-spec-proposals/pull/4262)),
since federation support for pushing profile field updates from server
to server has not yet been implemented
([MSC4259](https://github.com/matrix-org/matrix-spec-proposals/pull/4259)).

This has created user confusion. The before mentioned clients use the
profile and/or profile field fetch endpoints to fetch missing profile
information, regardless of the user being local or remote. Since the
sync response isn't delivering these profiles, the client does a
fallback fetch, causing a one time update of the profile information
over federation.

This causes stale data to be cached in the client for the remote user,
which in the case of EW for example is never cleared.

This filtering should be temporary until MSC4259 is implemented and the
remote profile data also syncs via the profile update stream to clients.

I suggest making this not configurable so we don't have to maintain a
configuration property, as hopefully this would only be a case for a few
months at most.

### Pull Request Checklist

<!-- Please read
https://element-hq.github.io/synapse/latest/development/contributing_guide.html
before submitting your pull request -->

* [x] Pull request is based on the develop branch
* [x] Pull request includes a [changelog
file](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#changelog).
The entry should:
- Be a short description of your change which makes sense to users.
"Fixed a bug that prevented receiving messages from other servers."
instead of "Moved X method from `EventStore` to `EventWorkerStore`.".
  - Use markdown where necessary, mostly for `code blocks`.
  - End with either a period (.) or an exclamation mark (!).
  - Start with a capital letter.
- Feel free to credit yourself, by adding a sentence "Contributed by
@github_username." or "Contributed by [Your Name]." to the end of the
entry.
* [x] [Code
style](https://element-hq.github.io/synapse/latest/code_style.html) is
correct (run the
[linters](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#run-the-linters))

---------

Co-authored-by: Eric Eastwood <madlittlemods@gmail.com>

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.

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,

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.