Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement MSC4133 to support custom profile fields. #17488

Open
wants to merge 36 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
e7ab5a1
Implement MSC4133 to support custom profile fields.
clokep Jul 25, 2024
a80c0eb
Newsfragment
clokep Jul 25, 2024
dcacf35
Move delta.
clokep Jul 25, 2024
60b3f00
Copy & paste error.
clokep Jul 31, 2024
d2ab9c9
Clarify docstring.
clokep Jul 31, 2024
ce405ab
Correct license.
clokep Jul 31, 2024
e4f9d7e
Fix typo.
clokep Jul 31, 2024
2c28bd5
Add more methods.
clokep Aug 9, 2024
00c02ac
Merge remote-tracking branch 'refs/remotes/origin/custom-fields' into…
clokep Aug 9, 2024
f0e02aa
Merge remote-tracking branch 'upstream/develop' into custom-fields
clokep Aug 9, 2024
8030ba2
Add comment.
clokep Aug 9, 2024
735e8cc
Get tests passing.
clokep Aug 9, 2024
8d9b74e
Review comments.
clokep Aug 12, 2024
3658b6a
Merge remote-tracking branch 'upstream/develop' into custom-fields
clokep Sep 5, 2024
02b852a
Handle TODO about limiting overall size.
clokep Sep 5, 2024
00ad911
Ensure fields exist.
clokep Sep 6, 2024
87ff9f8
Fix-up capabilities.
clokep Sep 9, 2024
4b44da3
Add to /versions.
clokep Sep 10, 2024
cf7499a
Merge remote-tracking branch 'upstream/develop' into custom-fields
clokep Sep 10, 2024
d28bf25
Rename delta.
clokep Sep 10, 2024
52fe59f
Updates from MSC.
clokep Oct 9, 2024
66878c4
Merge remote-tracking branch 'upstream/develop' into custom-fields
clokep Oct 31, 2024
1c98d96
Get postgres working again.
clokep Oct 31, 2024
749e27d
Working on sqlite again.
clokep Nov 1, 2024
6936daf
Move schema delta again.
clokep Nov 1, 2024
64fe222
Fix-up max length checks.
clokep Nov 4, 2024
6c777b2
Fix-up fetching individual fields.
clokep Nov 6, 2024
7cc1264
Merge remote-tracking branch 'upstream/develop' into custom-fields
clokep Nov 7, 2024
91a29e5
Merge remote-tracking branch 'upstream/develop' into custom-fields
clokep Nov 14, 2024
4318bdd
Merge remote-tracking branch 'upstream/develop' into custom-fields
clokep Dec 5, 2024
11cc48a
Remove the PATCH endpoint.
clokep Dec 5, 2024
adb64d8
Match the MSC error codes.
clokep Dec 6, 2024
069d6f9
Merge branch 'develop' into custom-fields
clokep Dec 19, 2024
2a327bf
Remove PUT endpoint.
clokep Dec 20, 2024
2357ae9
Merge remote-tracking branch 'refs/remotes/origin/custom-fields' into…
clokep Dec 20, 2024
f176237
Merge remote-tracking branch 'upstream/develop' into custom-fields
clokep Dec 20, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/17488.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Implement [MSC4133](https://github.com/matrix-org/matrix-spec-proposals/pull/4133) for custom profile fields.
5 changes: 5 additions & 0 deletions synapse/api/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -318,3 +318,8 @@ class ApprovalNoticeMedium:
class Direction(enum.Enum):
BACKWARDS = "b"
FORWARDS = "f"


class ProfileFields:
DISPLAYNAME: Final = "displayname"
AVATAR_URL: Final = "avatar_url"
6 changes: 6 additions & 0 deletions synapse/config/experimental.py
Original file line number Diff line number Diff line change
Expand Up @@ -447,3 +447,9 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None:

# MSC4151: Report room API (Client-Server API)
self.msc4151_enabled: bool = experimental.get("msc4151_enabled", False)

# MSC4156: Migrate server_name to via
self.msc4156_enabled: bool = experimental.get("msc4156_enabled", False)

# MSC4133: Custom profile fields
self.msc4133_enabled: bool = experimental.get("msc4133_enabled", False)
clokep marked this conversation as resolved.
Show resolved Hide resolved
152 changes: 145 additions & 7 deletions synapse/handlers/profile.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import random
from typing import TYPE_CHECKING, List, Optional, Union

from synapse.api.constants import ProfileFields
from synapse.api.errors import (
AuthError,
Codes,
Expand All @@ -42,6 +43,9 @@

MAX_DISPLAYNAME_LEN = 256
MAX_AVATAR_URL_LEN = 1000
# Field name length is specced at 255, value is server controlled.
MAX_CUSTOM_FIELD_LEN = 255
MAX_CUSTOM_VALUE_LEN = 255


class ProfileHandler:
Expand Down Expand Up @@ -89,13 +93,27 @@ async def get_profile(self, user_id: str, ignore_backoff: bool = True) -> JsonDi

if self.hs.is_mine(target_user):
profileinfo = await self.store.get_profileinfo(target_user)
if profileinfo.display_name is None and profileinfo.avatar_url is None:
extra_fields = {}
if self.hs.config.experimental.msc4133_enabled:
extra_fields = await self.store.get_profile_fields(target_user)

if (
profileinfo.display_name is None
and profileinfo.avatar_url is None
and not extra_fields
):
raise SynapseError(404, "Profile was not found", Codes.NOT_FOUND)

return {
"displayname": profileinfo.display_name,
"avatar_url": profileinfo.avatar_url,
}
# Do not include display name or avatar are denoted if unset.
ret = {}
if profileinfo.display_name is not None:
ret[ProfileFields.DISPLAYNAME] = profileinfo.display_name
if profileinfo.avatar_url is not None:
ret[ProfileFields.AVATAR_URL] = profileinfo.avatar_url
if extra_fields:
ret.update(extra_fields)

return ret
else:
try:
result = await self.federation.make_query(
Expand Down Expand Up @@ -399,6 +417,115 @@ async def check_avatar_size_and_mime_type(self, mxc: str) -> bool:

return True

async def get_profile_field(
clokep marked this conversation as resolved.
Show resolved Hide resolved
self, target_user: UserID, field_name: str
) -> Optional[str]:
clokep marked this conversation as resolved.
Show resolved Hide resolved
"""
Fetch a user's profile from the database for local users and over federation
for remote users.

Args:
target_user: The user ID to fetch the profile for.
field_name: The field to fetch the profile for.

Returns:
The value for the profile field or None if the field does not exist.
"""
if self.hs.is_mine(target_user):
try:
field_value = await self.store.get_profile_field(
target_user, field_name
)
except StoreError as e:
if e.code == 404:
raise SynapseError(404, "Profile was not found", Codes.NOT_FOUND)
clokep marked this conversation as resolved.
Show resolved Hide resolved
raise

return field_value
else:
try:
result = await self.federation.make_query(
destination=target_user.domain,
query_type="profile",
args={"user_id": target_user.to_string(), "field": field_name},
ignore_backoff=True,
)
except RequestSendFailed as e:
raise SynapseError(502, "Failed to fetch profile") from e
except HttpResponseException as e:
raise e.to_synapse_error()

return result.get(field_name)

async def set_profile_field(
self,
target_user: UserID,
requester: Requester,
field_name: str,
new_value: str,
by_admin: bool = False,
deactivation: bool = False,
) -> None:
"""Set a new profile field for a user.

Args:
target_user: the user whose avatar URL is to be changed.
requester: The user attempting to make this change.
field_name: The name of the profile field to update.
new_value: The new field value for this user.
by_admin: Whether this change was made by an administrator.
deactivation: Whether this change was made while deactivating the user.
"""
if not self.hs.is_mine(target_user):
raise SynapseError(400, "User is not hosted on this homeserver")

if not by_admin and target_user != requester.user:
raise AuthError(400, "Cannot set another user's profile")

if not isinstance(new_value, str):
raise SynapseError(
400, f"'{field_name}' must be a string", errcode=Codes.INVALID_PARAM
)

await self.store.set_profile_field(target_user, field_name, new_value)

# Custom fields do not propagate into the user directory *or* rooms.
profile = await self.store.get_profileinfo(target_user)
await self._third_party_rules.on_profile_update(
target_user.to_string(), profile, by_admin, deactivation
)

async def delete_profile_field(
self,
target_user: UserID,
requester: Requester,
field_name: str,
by_admin: bool = False,
deactivation: bool = False,
) -> None:
"""Set a new avatar URL for a user.

Args:
target_user: the user whose avatar URL is to be changed.
requester: The user attempting to make this change.
field_name: The name of the profile field to update.
by_admin: Whether this change was made by an administrator.
deactivation: Whether this change was made while deactivating the user.
"""
if not self.hs.is_mine(target_user):
raise SynapseError(400, "User is not hosted on this homeserver")

if not by_admin and target_user != requester.user:
raise AuthError(400, "Cannot set another user's profile")

await self.store.delete_profile_field(target_user, field_name)

# Custom fields do not propagate into the user directory *or* rooms.
profile = await self.store.get_profileinfo(target_user)
await self._third_party_rules.on_profile_update(
target_user.to_string(), profile, by_admin, deactivation
)

async def on_profile_query(self, args: JsonDict) -> JsonDict:
"""Handles federation profile query requests."""

Expand All @@ -417,11 +544,22 @@ async def on_profile_query(self, args: JsonDict) -> JsonDict:

response = {}
try:
if just_field is None or just_field == "displayname":
if just_field is None or just_field == ProfileFields.DISPLAYNAME:
response["displayname"] = await self.store.get_profile_displayname(user)

if just_field is None or just_field == "avatar_url":
if just_field is None or just_field == ProfileFields.AVATAR_URL:
response["avatar_url"] = await self.store.get_profile_avatar_url(user)

if self.hs.config.experimental.msc4133_enabled:
if just_field is None:
response.update(await self.store.get_profile_fields(user))
elif just_field not in (
ProfileFields.DISPLAYNAME,
ProfileFields.AVATAR_URL,
):
response[just_field] = await self.store.get_profile_field(
user, just_field
)
except StoreError as e:
if e.code == 404:
raise SynapseError(404, "Profile was not found", Codes.NOT_FOUND)
Expand Down
2 changes: 1 addition & 1 deletion synapse/handlers/sso.py
Original file line number Diff line number Diff line change
Expand Up @@ -813,7 +813,7 @@ def is_allowed_mime_type(content_type: str) -> bool:

# bail if user already has the same avatar
profile = await self._profile_handler.get_profile(user_id)
if profile["avatar_url"] is not None:
if "avatar_url" in profile:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we still need to check whether profile["avatar_url"] is None before attempting to .split on it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suspect I need to split this to a separate PR to make it clear what the change is.

server_name = profile["avatar_url"].split("/")[-2]
media_id = profile["avatar_url"].split("/")[-1]
if self._is_mine_server_name(server_name):
Expand Down
2 changes: 1 addition & 1 deletion synapse/http/servlet.py
Original file line number Diff line number Diff line change
Expand Up @@ -964,7 +964,7 @@ def register(self, http_server: HttpServer) -> None:
"""Register this servlet with the given HTTP server."""
patterns = getattr(self, "PATTERNS", None)
if patterns:
for method in ("GET", "PUT", "POST", "DELETE"):
for method in ("GET", "PUT", "POST", "DELETE", "PATCH"):
if hasattr(self, "on_%s" % (method,)):
servlet_classname = self.__class__.__name__
method_handler = getattr(self, "on_%s" % (method,))
Expand Down
4 changes: 2 additions & 2 deletions synapse/module_api/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -1110,10 +1110,10 @@ async def update_room_membership(

# Set the profile where it needs to be set.
if "avatar_url" not in content:
content["avatar_url"] = profile["avatar_url"]
content["avatar_url"] = profile.get("avatar_url")

if "displayname" not in content:
content["displayname"] = profile["displayname"]
content["displayname"] = profile.get("displayname")

event_id, _ = await self._hs.get_room_member_handler().update_membership(
requester=requester,
Expand Down
5 changes: 5 additions & 0 deletions synapse/rest/client/capabilities.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,11 @@ async def on_GET(self, request: SynapseRequest) -> Tuple[int, JsonDict]:
"enabled": self.config.experimental.msc3664_enabled,
}

if self.config.experimental.msc4133_enabled:
response["capabilities"]["k.tcpip.msc4133.profile_fields"] = {
True,
}

return HTTPStatus.OK, response


Expand Down
Loading
Loading