Skip to content
This repository was archived by the owner on Apr 26, 2024. It is now read-only.

Fix application service not being able to join remote federated room without a profile set #13131

Merged
MadLittleMods merged 2 commits into from
Jul 5, 2022
Merged

Fix application service not being able to join remote federated room without a profile set #13131

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fa6de7d
Application service can join remote federated room without profile set
MadLittleMods Jun 29, 2022
f14991a
Add changelog
MadLittleMods Jun 29, 2022
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/13131.bugfix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix application service not being able to join remote federated room without a profile set.
32 changes: 23 additions & 9 deletions synapse/handlers/room_member.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -830,10 +830,17 @@ async def update_membership_locked(

content["membership"] = Membership.JOIN

profile = self.profile_handler
if not content_specified:
content["displayname"] = await profile.get_displayname(target)
content["avatar_url"] = await profile.get_avatar_url(target)
try:
profile = self.profile_handler
if not content_specified:
content["displayname"] = await profile.get_displayname(target)
content["avatar_url"] = await profile.get_avatar_url(target)
Copy link
Copy Markdown
Contributor Author

@MadLittleMods MadLittleMods Jun 29, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Feels like these profile functions would be better if they just returned None when there is no profile for the local user. Profiles aren't required.

  • synapse/synapse/handlers/profile.py

    Lines 104 to 129 in e714b8a

    async def get_displayname(self, target_user: UserID) -> Optional[str]:
    if self.hs.is_mine(target_user):
    try:
    displayname = await self.store.get_profile_displayname(
    target_user.localpart
    )
    except StoreError as e:
    if e.code == 404:
    raise SynapseError(404, "Profile was not found", Codes.NOT_FOUND)
    raise
    return displayname
    else:
    try:
    result = await self.federation.make_query(
    destination=target_user.domain,
    query_type="profile",
    args={"user_id": target_user.to_string(), "field": "displayname"},
    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("displayname")

  • synapse/synapse/handlers/profile.py

    Lines 201 to 225 in e714b8a

    async def get_avatar_url(self, target_user: UserID) -> Optional[str]:
    if self.hs.is_mine(target_user):
    try:
    avatar_url = await self.store.get_profile_avatar_url(
    target_user.localpart
    )
    except StoreError as e:
    if e.code == 404:
    raise SynapseError(404, "Profile was not found", Codes.NOT_FOUND)
    raise
    return avatar_url
    else:
    try:
    result = await self.federation.make_query(
    destination=target_user.domain,
    query_type="profile",
    args={"user_id": target_user.to_string(), "field": "avatar_url"},
    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("avatar_url")

Copy link
Copy Markdown
Member

@erikjohnston erikjohnston Jul 5, 2022

Choose a reason for hiding this comment

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

Yeah, I agree. Pretty much everywhere uses the same pattern as here, and we already return None if the user sets and then deletes their display name

except Exception as e:
logger.info(
"Failed to get profile information while processing remote join for %r: %s",
target,
e,
)
Copy link
Copy Markdown
Contributor Author

@MadLittleMods MadLittleMods Jun 29, 2022

Choose a reason for hiding this comment

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

Same pattern that we already use in

synapse/synapse/handlers/message.py

Lines 640 to 652 in e714b8a

try:
if "displayname" not in content:
displayname = await profile.get_displayname(target)
if displayname is not None:
content["displayname"] = displayname
if "avatar_url" not in content:
avatar_url = await profile.get_avatar_url(target)
if avatar_url is not None:
content["avatar_url"] = avatar_url
except Exception as e:
logger.info(
"Failed to get profile information for %r: %s", target, e
)


if requester.is_guest:
content["kind"] = "guest"
Expand Down Expand Up @@ -910,11 +917,18 @@ async def update_membership_locked(

content["membership"] = Membership.KNOCK

profile = self.profile_handler
if "displayname" not in content:
content["displayname"] = await profile.get_displayname(target)
if "avatar_url" not in content:
content["avatar_url"] = await profile.get_avatar_url(target)
try:
profile = self.profile_handler
if "displayname" not in content:
content["displayname"] = await profile.get_displayname(target)
if "avatar_url" not in content:
content["avatar_url"] = await profile.get_avatar_url(target)
except Exception as e:
logger.info(
"Failed to get profile information while processing remote knock for %r: %s",
target,
e,
)

return await self.remote_knock(
remote_room_hosts, room_id, target, content
Expand Down