diff --git a/changelog.d/20174.bugfix b/changelog.d/20174.bugfix new file mode 100644 index 00000000000..f83f5c9a2cb --- /dev/null +++ b/changelog.d/20174.bugfix @@ -0,0 +1 @@ +Return a 404 error from `GET /_matrix/client/v3/profile/{userId}/displayname` and `GET /_matrix/client/v3/profile/{userId}/avatar_url` when the field is not set, as required by the spec, instead of a 200 response with a `null` value. diff --git a/docs/upgrade.md b/docs/upgrade.md index a3c529f1f16..683375b81e8 100644 --- a/docs/upgrade.md +++ b/docs/upgrade.md @@ -118,6 +118,18 @@ stacking them up. You can monitor the currently running background updates with [the Admin API](usage/administration/admin_api/background_updates.html#status). +# Upgrading to v1.160.0 + +## Fetching an unset `displayname` or `avatar_url` now returns a 404 error + +`GET /_matrix/client/v3/profile/{userId}/displayname` and +`GET /_matrix/client/v3/profile/{userId}/avatar_url` now return a 404 error with +`M_NOT_FOUND` when the requested field is not set, as required by +[the spec](https://spec.matrix.org/latest/client-server-api/#get_matrixclientv3profileuseridkeyname), +instead of a 200 response with a `null` value. This matches the existing +behaviour of custom profile fields on the same endpoint. Clients relying on the +previous non-spec-compliant behaviour may need updating. + # Upgrading to v1.159.0 ## Change of signing key expiry date for the Debian/Ubuntu package repository (2026) diff --git a/synapse/rest/client/profile.py b/synapse/rest/client/profile.py index 4431aa2b2c9..6436cb0a959 100644 --- a/synapse/rest/client/profile.py +++ b/synapse/rest/client/profile.py @@ -154,6 +154,18 @@ async def on_GET( else: field_value = await self.profile_handler.get_profile_field(user, field_name) + if ( + field_name in (ProfileFields.DISPLAYNAME, ProfileFields.AVATAR_URL) + and field_value is None + ): + # displayname and avatar_url cannot hold a JSON null (unlike custom + # fields), so None means the field is unset, for which the spec + # requires a 404: + # https://spec.matrix.org/latest/client-server-api/#get_matrixclientv3profileuseridkeyname + raise SynapseError( + HTTPStatus.NOT_FOUND, "Profile was not found", Codes.NOT_FOUND + ) + return 200, {field_name: field_value} async def on_PUT( diff --git a/tests/rest/client/test_profile.py b/tests/rest/client/test_profile.py index 023a376ed16..cb4c4abd852 100644 --- a/tests/rest/client/test_profile.py +++ b/tests/rest/client/test_profile.py @@ -118,6 +118,24 @@ def test_get_displayname_other(self) -> None: res = self._get_displayname(self.other) self.assertEqual(res, "Bob") + def test_get_unset_displayname(self) -> None: + """Fetching an unset displayname should return 404, as per the spec: + https://spec.matrix.org/latest/client-server-api/#get_matrixclientv3profileuseridkeyname + """ + # The owner's displayname defaults to their localpart at registration; + # clear it. + channel = self.make_request( + "PUT", + "/profile/%s/displayname" % (self.owner,), + content={"displayname": ""}, + access_token=self.owner_tok, + ) + self.assertEqual(channel.code, 200, channel.result) + + channel = self.make_request("GET", "/profile/%s/displayname" % (self.owner,)) + self.assertEqual(channel.code, 404, channel.result) + self.assertEqual(channel.json_body["errcode"], Codes.NOT_FOUND) + def test_set_displayname_other(self) -> None: channel = self.make_request( "PUT", @@ -168,6 +186,14 @@ def test_get_avatar_url_other(self) -> None: res = self._get_avatar_url(self.other) self.assertIsNone(res) + def test_get_unset_avatar_url(self) -> None: + """Fetching an unset avatar_url should return 404, as per the spec: + https://spec.matrix.org/latest/client-server-api/#get_matrixclientv3profileuseridkeyname + """ + channel = self.make_request("GET", "/profile/%s/avatar_url" % (self.owner,)) + self.assertEqual(channel.code, 404, channel.result) + self.assertEqual(channel.json_body["errcode"], Codes.NOT_FOUND) + def test_set_avatar_url_other(self) -> None: channel = self.make_request( "PUT", @@ -178,24 +204,24 @@ def test_set_avatar_url_other(self) -> None: self.assertEqual(channel.code, 400, channel.result) def _get_displayname(self, name: str | None = None) -> str | None: + """Fetch a user's displayname, returning None if it is unset (404).""" channel = self.make_request( "GET", "/profile/%s/displayname" % (name or self.owner,) ) + if channel.code == 404: + return None self.assertEqual(channel.code, 200, channel.result) - # FIXME: If a user has no displayname set, Synapse returns 200 and omits a - # displayname from the response. This contradicts the spec, see - # https://github.com/matrix-org/synapse/issues/13137. - return channel.json_body.get("displayname") + return channel.json_body["displayname"] def _get_avatar_url(self, name: str | None = None) -> str | None: + """Fetch a user's avatar_url, returning None if it is unset (404).""" channel = self.make_request( "GET", "/profile/%s/avatar_url" % (name or self.owner,) ) + if channel.code == 404: + return None self.assertEqual(channel.code, 200, channel.result) - # FIXME: If a user has no avatar set, Synapse returns 200 and omits an - # avatar_url from the response. This contradicts the spec, see - # https://github.com/matrix-org/synapse/issues/13137. - return channel.json_body.get("avatar_url") + return channel.json_body["avatar_url"] @unittest.override_config({"max_avatar_size": 50}) def test_avatar_size_limit_global(self) -> None: @@ -824,6 +850,16 @@ def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: self.owner_tok = self.login("owner", "pass") self.profile_url = "/profile/%s" % (self.owner) + # Set an avatar for the owner, since fetching an unset avatar_url + # returns 404 rather than exercising the access checks. + channel = self.make_request( + "PUT", + f"{self.profile_url}/avatar_url", + content={"avatar_url": "mxc://test/pic"}, + access_token=self.owner_tok, + ) + self.assertEqual(channel.code, 200, channel.result) + # User requesting the profile. self.requester = self.register_user("requester", "pass") self.requester_tok = self.login("requester", "pass") @@ -905,6 +941,16 @@ def test_can_lookup_own_profile(self) -> None: """Tests that a user can lookup their own profile without having to be in a room if 'require_auth_for_profile_requests' is set to true in the server's config. """ + # Set an avatar, since fetching an unset avatar_url returns 404 rather + # than exercising the access checks. + channel = self.make_request( + "PUT", + "/profile/" + self.requester + "/avatar_url", + content={"avatar_url": "mxc://test/pic"}, + access_token=self.requester_tok, + ) + self.assertEqual(channel.code, 200, channel.result) + channel = self.make_request( "GET", "/profile/" + self.requester, access_token=self.requester_tok )