From 472d91da988fadb013fb0bd647f778b2295cd088 Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Wed, 20 Jul 2022 10:27:06 +0100 Subject: [PATCH 01/19] Saving changes --- synapse/api/errors.py | 1 + synapse/event_auth.py | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/synapse/api/errors.py b/synapse/api/errors.py index 1c74e131f2b2..1ac1d7bfd193 100644 --- a/synapse/api/errors.py +++ b/synapse/api/errors.py @@ -79,6 +79,7 @@ class Codes(str, Enum): WEAK_PASSWORD = "M_WEAK_PASSWORD" INVALID_SIGNATURE = "M_INVALID_SIGNATURE" USER_DEACTIVATED = "M_USER_DEACTIVATED" + ALREADY_IN_ROOM = "M_ALREADY_IN_ROOM" # The account has been suspended on the server. # By opposition to `USER_DEACTIVATED`, this is a reversible measure diff --git a/synapse/event_auth.py b/synapse/event_auth.py index 965cb265daf8..eadc3234be8f 100644 --- a/synapse/event_auth.py +++ b/synapse/event_auth.py @@ -30,7 +30,7 @@ JoinRules, Membership, ) -from synapse.api.errors import AuthError, EventSizeError, SynapseError +from synapse.api.errors import AuthError, Codes, EventSizeError, SynapseError from synapse.api.room_versions import ( KNOWN_ROOM_VERSIONS, EventFormatVersions, @@ -484,7 +484,7 @@ def _is_membership_change_allowed( if target_banned: raise AuthError(403, "%s is banned from the room" % (target_user_id,)) elif target_in_room: # the target is already in the room. - raise AuthError(403, "%s is already in the room." % target_user_id) + raise AuthError(403, "%s is already in the room." % target_user_id, errcode=Codes.ALREADY_IN_ROOM) else: if user_level < invite_level: raise AuthError(403, "You don't have permission to invite users") From 366bc6d1d4948355f3ade051f0bd88f364116b33 Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Wed, 20 Jul 2022 18:27:26 +0100 Subject: [PATCH 02/19] Add unstable spec auth errors --- synapse/api/errors.py | 23 ++++++++++++++++++++++- synapse/appservice/__init__.py | 9 +++++++++ synapse/appservice/api.py | 3 +++ synapse/event_auth.py | 22 ++++++++++++---------- 4 files changed, 46 insertions(+), 11 deletions(-) diff --git a/synapse/api/errors.py b/synapse/api/errors.py index 1ac1d7bfd193..2efbea05e3d5 100644 --- a/synapse/api/errors.py +++ b/synapse/api/errors.py @@ -79,7 +79,8 @@ class Codes(str, Enum): WEAK_PASSWORD = "M_WEAK_PASSWORD" INVALID_SIGNATURE = "M_INVALID_SIGNATURE" USER_DEACTIVATED = "M_USER_DEACTIVATED" - ALREADY_IN_ROOM = "M_ALREADY_IN_ROOM" + ALREADY_JOINED = "M_ALREADY_JOINED" + INSUFFICIENT_POWER = "M_INSUFFICIENT_POWER" # The account has been suspended on the server. # By opposition to `USER_DEACTIVATED`, this is a reversible measure @@ -307,6 +308,26 @@ def __init__( ): super().__init__(code, msg, errcode, additional_fields) +class UnstableSpecAuthError(AuthError): + """An error raised when a new error code is being proposed to replace a previous one. + This error will return a "org.matrix.unstable.errcode" property with the new error code, + with the previous error code still being defined in the "errcode" property + """ + + def __init__( + self, + code: int, + msg: str, + errcode: str, + previous_errcode: str = Codes.FORBIDDEN, + additional_fields: Optional[dict] = None, + ): + self.previous_errcode = previous_errcode + super().__init__(code, msg, errcode, additional_fields) + + + def error_dict(self) -> "JsonDict": + return cs_error(self.msg, self.previous_errcode, **{"org.matrix.unstable.errcode": self.errcode}, **self._additional_fields) class InvalidClientCredentialsError(SynapseError): """An error raised when there was a problem with the authorisation credentials diff --git a/synapse/appservice/__init__.py b/synapse/appservice/__init__.py index 0dfa00df44c7..c965307879b2 100644 --- a/synapse/appservice/__init__.py +++ b/synapse/appservice/__init__.py @@ -67,6 +67,9 @@ class ApplicationService: # values. NS_LIST = [NS_USERS, NS_ALIASES, NS_ROOMS] + # We assume all appservices support an authorisation header + use_auth_header = True + def __init__( self, token: str, @@ -353,6 +356,12 @@ def __str__(self) -> str: dict_copy["hs_token"] = "" return "ApplicationService: %s" % (dict_copy,) + def can_use_auth_header(self) -> bool: + return self.use_auth_header + + def mark_auth_header_as_unsupported(self): + self.use_auth_header = False + class AppServiceTransaction: """Represents an application service transaction.""" diff --git a/synapse/appservice/api.py b/synapse/appservice/api.py index 0963fb3bb4fb..ccce0e78a49e 100644 --- a/synapse/appservice/api.py +++ b/synapse/appservice/api.py @@ -313,6 +313,7 @@ async def push_bulk( uri=uri, json_body=body, args={"access_token": service.hs_token}, + headers={"Authorization": [f"Bearer {service.hs_token}"]}, ) if logger.isEnabledFor(logging.DEBUG): logger.debug( @@ -326,6 +327,8 @@ async def push_bulk( sent_todevice_counter.labels(service.id).inc(len(to_device_messages)) return True except CodeMessageException as e: + if e.code == 403: + service.mark_auth_header_as_unsupported() logger.warning( "push_bulk to %s received code=%s msg=%s", uri, diff --git a/synapse/event_auth.py b/synapse/event_auth.py index eadc3234be8f..046fd1d0028a 100644 --- a/synapse/event_auth.py +++ b/synapse/event_auth.py @@ -30,7 +30,7 @@ JoinRules, Membership, ) -from synapse.api.errors import AuthError, Codes, EventSizeError, SynapseError +from synapse.api.errors import AuthError, Codes, EventSizeError, SynapseError, UnstableSpecAuthError from synapse.api.room_versions import ( KNOWN_ROOM_VERSIONS, EventFormatVersions, @@ -291,7 +291,7 @@ def check_state_dependent_auth_rules( invite_level = get_named_level(auth_dict, "invite", 0) if user_level < invite_level: - raise AuthError(403, "You don't have permission to invite users") + raise UnstableSpecAuthError(403, "You don't have permission to invite users", errcode=Codes.INSUFFICIENT_POWER) else: logger.debug("Allowing! %s", event) return @@ -484,10 +484,10 @@ def _is_membership_change_allowed( if target_banned: raise AuthError(403, "%s is banned from the room" % (target_user_id,)) elif target_in_room: # the target is already in the room. - raise AuthError(403, "%s is already in the room." % target_user_id, errcode=Codes.ALREADY_IN_ROOM) + raise UnstableSpecAuthError(403, "%s is already in the room." % target_user_id, errcode=Codes.ALREADY_JOINED) else: if user_level < invite_level: - raise AuthError(403, "You don't have permission to invite users") + raise UnstableSpecAuthError(403, "You don't have permission to invite users", errcode=Codes.INSUFFICIENT_POWER) elif Membership.JOIN == membership: # Joins are valid iff caller == target and: # * They are not banned. @@ -549,15 +549,15 @@ def _is_membership_change_allowed( elif Membership.LEAVE == membership: # TODO (erikj): Implement kicks. if target_banned and user_level < ban_level: - raise AuthError(403, "You cannot unban user %s." % (target_user_id,)) + raise UnstableSpecAuthError(403, "You cannot unban user %s." % (target_user_id,), errcode=Codes.INSUFFICIENT_POWER) elif target_user_id != event.user_id: kick_level = get_named_level(auth_events, "kick", 50) if user_level < kick_level or user_level <= target_level: - raise AuthError(403, "You cannot kick user %s." % target_user_id) + raise UnstableSpecAuthError(403, "You cannot kick user %s." % target_user_id, errcode=Codes.INSUFFICIENT_POWER) elif Membership.BAN == membership: if user_level < ban_level or user_level <= target_level: - raise AuthError(403, "You don't have permission to ban") + raise UnstableSpecAuthError(403, "You don't have permission to ban", errcode=Codes.INSUFFICIENT_POWER) elif room_version.msc2403_knocking and Membership.KNOCK == membership: if join_rule != JoinRules.KNOCK and ( not room_version.msc3787_knock_restricted_join_rule @@ -567,7 +567,7 @@ def _is_membership_change_allowed( elif target_user_id != event.user_id: raise AuthError(403, "You cannot knock for other users") elif target_in_room: - raise AuthError(403, "You cannot knock on a room you are already in") + raise UnstableSpecAuthError(403, "You cannot knock on a room you are already in", errcode=Codes.ALREADY_JOINED) elif caller_invited: raise AuthError(403, "You are already invited to this room") elif target_banned: @@ -638,10 +638,11 @@ def _can_send_event(event: "EventBase", auth_events: StateMap["EventBase"]) -> b user_level = get_user_power_level(event.user_id, auth_events) if user_level < send_level: - raise AuthError( + raise UnstableSpecAuthError( 403, "You don't have permission to post that to the room. " + "user_level (%d) < send_level (%d)" % (user_level, send_level), + errcode=Codes.INSUFFICIENT_POWER, ) # Check state_key @@ -716,9 +717,10 @@ def check_historical( historical_level = get_named_level(auth_events, "historical", 100) if user_level < historical_level: - raise AuthError( + raise UnstableSpecAuthError( 403, 'You don\'t have permission to send send historical related events ("insertion", "batch", and "marker")', + errcode=Codes.INSUFFICIENT_POWER ) From 60c9cf299e0f8a8ae8c2d3f31da0b215d8574600 Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Wed, 20 Jul 2022 18:30:00 +0100 Subject: [PATCH 03/19] lint --- synapse/api/errors.py | 16 ++++++++----- synapse/event_auth.py | 52 +++++++++++++++++++++++++++++++++++-------- 2 files changed, 54 insertions(+), 14 deletions(-) diff --git a/synapse/api/errors.py b/synapse/api/errors.py index 2efbea05e3d5..b27b797d7c17 100644 --- a/synapse/api/errors.py +++ b/synapse/api/errors.py @@ -308,10 +308,11 @@ def __init__( ): super().__init__(code, msg, errcode, additional_fields) + class UnstableSpecAuthError(AuthError): - """An error raised when a new error code is being proposed to replace a previous one. - This error will return a "org.matrix.unstable.errcode" property with the new error code, - with the previous error code still being defined in the "errcode" property + """An error raised when a new error code is being proposed to replace a previous one. + This error will return a "org.matrix.unstable.errcode" property with the new error code, + with the previous error code still being defined in the "errcode" property """ def __init__( @@ -325,9 +326,14 @@ def __init__( self.previous_errcode = previous_errcode super().__init__(code, msg, errcode, additional_fields) - def error_dict(self) -> "JsonDict": - return cs_error(self.msg, self.previous_errcode, **{"org.matrix.unstable.errcode": self.errcode}, **self._additional_fields) + return cs_error( + self.msg, + self.previous_errcode, + **{"org.matrix.unstable.errcode": self.errcode}, + **self._additional_fields, + ) + class InvalidClientCredentialsError(SynapseError): """An error raised when there was a problem with the authorisation credentials diff --git a/synapse/event_auth.py b/synapse/event_auth.py index 046fd1d0028a..0248d7e0c4c1 100644 --- a/synapse/event_auth.py +++ b/synapse/event_auth.py @@ -30,7 +30,13 @@ JoinRules, Membership, ) -from synapse.api.errors import AuthError, Codes, EventSizeError, SynapseError, UnstableSpecAuthError +from synapse.api.errors import ( + AuthError, + Codes, + EventSizeError, + SynapseError, + UnstableSpecAuthError, +) from synapse.api.room_versions import ( KNOWN_ROOM_VERSIONS, EventFormatVersions, @@ -291,7 +297,11 @@ def check_state_dependent_auth_rules( invite_level = get_named_level(auth_dict, "invite", 0) if user_level < invite_level: - raise UnstableSpecAuthError(403, "You don't have permission to invite users", errcode=Codes.INSUFFICIENT_POWER) + raise UnstableSpecAuthError( + 403, + "You don't have permission to invite users", + errcode=Codes.INSUFFICIENT_POWER, + ) else: logger.debug("Allowing! %s", event) return @@ -484,10 +494,18 @@ def _is_membership_change_allowed( if target_banned: raise AuthError(403, "%s is banned from the room" % (target_user_id,)) elif target_in_room: # the target is already in the room. - raise UnstableSpecAuthError(403, "%s is already in the room." % target_user_id, errcode=Codes.ALREADY_JOINED) + raise UnstableSpecAuthError( + 403, + "%s is already in the room." % target_user_id, + errcode=Codes.ALREADY_JOINED, + ) else: if user_level < invite_level: - raise UnstableSpecAuthError(403, "You don't have permission to invite users", errcode=Codes.INSUFFICIENT_POWER) + raise UnstableSpecAuthError( + 403, + "You don't have permission to invite users", + errcode=Codes.INSUFFICIENT_POWER, + ) elif Membership.JOIN == membership: # Joins are valid iff caller == target and: # * They are not banned. @@ -549,15 +567,27 @@ def _is_membership_change_allowed( elif Membership.LEAVE == membership: # TODO (erikj): Implement kicks. if target_banned and user_level < ban_level: - raise UnstableSpecAuthError(403, "You cannot unban user %s." % (target_user_id,), errcode=Codes.INSUFFICIENT_POWER) + raise UnstableSpecAuthError( + 403, + "You cannot unban user %s." % (target_user_id,), + errcode=Codes.INSUFFICIENT_POWER, + ) elif target_user_id != event.user_id: kick_level = get_named_level(auth_events, "kick", 50) if user_level < kick_level or user_level <= target_level: - raise UnstableSpecAuthError(403, "You cannot kick user %s." % target_user_id, errcode=Codes.INSUFFICIENT_POWER) + raise UnstableSpecAuthError( + 403, + "You cannot kick user %s." % target_user_id, + errcode=Codes.INSUFFICIENT_POWER, + ) elif Membership.BAN == membership: if user_level < ban_level or user_level <= target_level: - raise UnstableSpecAuthError(403, "You don't have permission to ban", errcode=Codes.INSUFFICIENT_POWER) + raise UnstableSpecAuthError( + 403, + "You don't have permission to ban", + errcode=Codes.INSUFFICIENT_POWER, + ) elif room_version.msc2403_knocking and Membership.KNOCK == membership: if join_rule != JoinRules.KNOCK and ( not room_version.msc3787_knock_restricted_join_rule @@ -567,7 +597,11 @@ def _is_membership_change_allowed( elif target_user_id != event.user_id: raise AuthError(403, "You cannot knock for other users") elif target_in_room: - raise UnstableSpecAuthError(403, "You cannot knock on a room you are already in", errcode=Codes.ALREADY_JOINED) + raise UnstableSpecAuthError( + 403, + "You cannot knock on a room you are already in", + errcode=Codes.ALREADY_JOINED, + ) elif caller_invited: raise AuthError(403, "You are already invited to this room") elif target_banned: @@ -720,7 +754,7 @@ def check_historical( raise UnstableSpecAuthError( 403, 'You don\'t have permission to send send historical related events ("insertion", "batch", and "marker")', - errcode=Codes.INSUFFICIENT_POWER + errcode=Codes.INSUFFICIENT_POWER, ) From 4cdb8a6855535c01654a60697432133914f841e1 Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Wed, 20 Jul 2022 18:33:20 +0100 Subject: [PATCH 04/19] prefixing --- synapse/api/errors.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/synapse/api/errors.py b/synapse/api/errors.py index b27b797d7c17..499242c30124 100644 --- a/synapse/api/errors.py +++ b/synapse/api/errors.py @@ -79,8 +79,11 @@ class Codes(str, Enum): WEAK_PASSWORD = "M_WEAK_PASSWORD" INVALID_SIGNATURE = "M_INVALID_SIGNATURE" USER_DEACTIVATED = "M_USER_DEACTIVATED" - ALREADY_JOINED = "M_ALREADY_JOINED" - INSUFFICIENT_POWER = "M_INSUFFICIENT_POWER" + + # Part of MSC3848 + # https://github.com/matrix-org/matrix-spec-proposals/pull/3848 + ALREADY_JOINED = "ORG.MATRIX.MSC3848.ALREADY_JOINED" + INSUFFICIENT_POWER = "ORG.MATRIX.MSC3848.INSUFFICIENT_POWER" # The account has been suspended on the server. # By opposition to `USER_DEACTIVATED`, this is a reversible measure From f3defc61a0c1a9dbb1315ca66e71807535972cdc Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Wed, 20 Jul 2022 18:35:59 +0100 Subject: [PATCH 05/19] notes --- changelog.d/13343.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/13343.feature diff --git a/changelog.d/13343.feature b/changelog.d/13343.feature new file mode 100644 index 000000000000..44355706e789 --- /dev/null +++ b/changelog.d/13343.feature @@ -0,0 +1 @@ +Add new unstable error codes `ORG.MATRIX.MSC3848.ALREADY_JOINED` and `ORG.MATRIX.MSC3848.INSUFFICIENT_POWER` described in MSC3848. \ No newline at end of file From 8d6bafa3d918bce4a05d5f3f0f7c03371ce8a901 Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Wed, 20 Jul 2022 18:37:48 +0100 Subject: [PATCH 06/19] Drop mistakenly committed changes --- synapse/appservice/__init__.py | 9 --------- synapse/appservice/api.py | 3 --- 2 files changed, 12 deletions(-) diff --git a/synapse/appservice/__init__.py b/synapse/appservice/__init__.py index c965307879b2..0dfa00df44c7 100644 --- a/synapse/appservice/__init__.py +++ b/synapse/appservice/__init__.py @@ -67,9 +67,6 @@ class ApplicationService: # values. NS_LIST = [NS_USERS, NS_ALIASES, NS_ROOMS] - # We assume all appservices support an authorisation header - use_auth_header = True - def __init__( self, token: str, @@ -356,12 +353,6 @@ def __str__(self) -> str: dict_copy["hs_token"] = "" return "ApplicationService: %s" % (dict_copy,) - def can_use_auth_header(self) -> bool: - return self.use_auth_header - - def mark_auth_header_as_unsupported(self): - self.use_auth_header = False - class AppServiceTransaction: """Represents an application service transaction.""" diff --git a/synapse/appservice/api.py b/synapse/appservice/api.py index ccce0e78a49e..0963fb3bb4fb 100644 --- a/synapse/appservice/api.py +++ b/synapse/appservice/api.py @@ -313,7 +313,6 @@ async def push_bulk( uri=uri, json_body=body, args={"access_token": service.hs_token}, - headers={"Authorization": [f"Bearer {service.hs_token}"]}, ) if logger.isEnabledFor(logging.DEBUG): logger.debug( @@ -327,8 +326,6 @@ async def push_bulk( sent_todevice_counter.labels(service.id).inc(len(to_device_messages)) return True except CodeMessageException as e: - if e.code == 403: - service.mark_auth_header_as_unsupported() logger.warning( "push_bulk to %s received code=%s msg=%s", uri, From b302d34dc5a36c9940bdc4c2a7b3c96ab4720eaf Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Fri, 22 Jul 2022 10:23:23 +0100 Subject: [PATCH 07/19] Add NOT_JOINED --- changelog.d/13343.feature | 2 +- synapse/api/auth.py | 11 ++++++++--- synapse/api/errors.py | 1 + synapse/event_auth.py | 6 +++++- synapse/handlers/message.py | 13 +++++++++++-- synapse/handlers/room_summary.py | 5 +++-- 6 files changed, 29 insertions(+), 9 deletions(-) diff --git a/changelog.d/13343.feature b/changelog.d/13343.feature index 44355706e789..c151251e54ab 100644 --- a/changelog.d/13343.feature +++ b/changelog.d/13343.feature @@ -1 +1 @@ -Add new unstable error codes `ORG.MATRIX.MSC3848.ALREADY_JOINED` and `ORG.MATRIX.MSC3848.INSUFFICIENT_POWER` described in MSC3848. \ No newline at end of file +Add new unstable error codes `ORG.MATRIX.MSC3848.ALREADY_JOINED`, `ORG.MATRIX.MSC3848.NOT_JOINED`, and `ORG.MATRIX.MSC3848.INSUFFICIENT_POWER` described in MSC3848. \ No newline at end of file diff --git a/synapse/api/auth.py b/synapse/api/auth.py index 6e6eaf3805bd..82e6475ef571 100644 --- a/synapse/api/auth.py +++ b/synapse/api/auth.py @@ -26,6 +26,7 @@ Codes, InvalidClientTokenError, MissingClientTokenError, + UnstableSpecAuthError, ) from synapse.appservice import ApplicationService from synapse.http import get_request_user_agent @@ -106,8 +107,11 @@ async def check_user_in_room( forgot = await self.store.did_forget(user_id, room_id) if not forgot: return membership, member_event_id - - raise AuthError(403, "User %s not in room %s" % (user_id, room_id)) + raise UnstableSpecAuthError( + 403, + "User %s not in room %s" % (user_id, room_id), + errcode=Codes.NOT_JOINED, + ) async def get_user_by_req( self, @@ -600,8 +604,9 @@ async def check_user_in_room_or_world_readable( == HistoryVisibility.WORLD_READABLE ): return Membership.JOIN, None - raise AuthError( + raise UnstableSpecAuthError( 403, "User %s not in room %s, and room previews are disabled" % (user_id, room_id), + errcode=Codes.NOT_JOINED, ) diff --git a/synapse/api/errors.py b/synapse/api/errors.py index 499242c30124..3d6a00216e64 100644 --- a/synapse/api/errors.py +++ b/synapse/api/errors.py @@ -83,6 +83,7 @@ class Codes(str, Enum): # Part of MSC3848 # https://github.com/matrix-org/matrix-spec-proposals/pull/3848 ALREADY_JOINED = "ORG.MATRIX.MSC3848.ALREADY_JOINED" + NOT_JOINED = "ORG.MATRIX.MSC3848.NOT_JOINED" INSUFFICIENT_POWER = "ORG.MATRIX.MSC3848.INSUFFICIENT_POWER" # The account has been suspended on the server. diff --git a/synapse/event_auth.py b/synapse/event_auth.py index 0248d7e0c4c1..389b0c5d533d 100644 --- a/synapse/event_auth.py +++ b/synapse/event_auth.py @@ -484,7 +484,11 @@ def _is_membership_change_allowed( return if not caller_in_room: # caller isn't joined - raise AuthError(403, "%s not in room %s." % (event.user_id, event.room_id)) + raise UnstableSpecAuthError( + 403, + "%s not in room %s." % (event.user_id, event.room_id), + errcode=Codes.NOT_JOINED, + ) if Membership.INVITE == membership: # TODO (erikj): We should probably handle this more intelligently diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py index bd7baef0510b..06adae09d97c 100644 --- a/synapse/handlers/message.py +++ b/synapse/handlers/message.py @@ -41,6 +41,7 @@ NotFoundError, ShadowBanError, SynapseError, + UnstableSpecAuthError, UnsupportedRoomVersionError, ) from synapse.api.room_versions import KNOWN_ROOM_VERSIONS @@ -149,7 +150,11 @@ async def get_room_data( "Attempted to retrieve data from a room for a user that has never been in it. " "This should not have happened." ) - raise SynapseError(403, "User not in room", errcode=Codes.FORBIDDEN) + raise UnstableSpecAuthError( + 403, + "User not in room", + errcode=Codes.NOT_JOINED, + ) return data @@ -334,7 +339,11 @@ async def get_joined_members(self, requester: Requester, room_id: str) -> dict: break else: # Loop fell through, AS has no interested users in room - raise AuthError(403, "Appservice not in room") + raise UnstableSpecAuthError( + 403, + "Appservice not in room", + errcode=Codes.NOT_JOINED, + ) return { user_id: { diff --git a/synapse/handlers/room_summary.py b/synapse/handlers/room_summary.py index 13098f56ed26..732b0310bcd6 100644 --- a/synapse/handlers/room_summary.py +++ b/synapse/handlers/room_summary.py @@ -28,11 +28,11 @@ RoomTypes, ) from synapse.api.errors import ( - AuthError, Codes, NotFoundError, StoreError, SynapseError, + UnstableSpecAuthError, UnsupportedRoomVersionError, ) from synapse.api.ratelimiting import Ratelimiter @@ -175,10 +175,11 @@ async def _get_room_hierarchy( # First of all, check that the room is accessible. if not await self._is_local_room_accessible(requested_room_id, requester): - raise AuthError( + raise UnstableSpecAuthError( 403, "User %s not in room %s, and room previews are disabled" % (requester, requested_room_id), + errcode=Codes.NOT_JOINED, ) # If this is continuing a previous session, pull the persisted data. From 8d08d0f1260a67e384524a1031f0329c3d9123b8 Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Mon, 25 Jul 2022 15:07:11 +0100 Subject: [PATCH 08/19] Use msc namespace --- synapse/api/errors.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/synapse/api/errors.py b/synapse/api/errors.py index 3d6a00216e64..9b0ab399e2d5 100644 --- a/synapse/api/errors.py +++ b/synapse/api/errors.py @@ -316,7 +316,9 @@ def __init__( class UnstableSpecAuthError(AuthError): """An error raised when a new error code is being proposed to replace a previous one. This error will return a "org.matrix.unstable.errcode" property with the new error code, - with the previous error code still being defined in the "errcode" property + with the previous error code still being defined in the "errcode" property. + + This error will include `org.matrix.MSC3848.unstable.errcode` in the C-S error body. """ def __init__( @@ -334,7 +336,7 @@ def error_dict(self) -> "JsonDict": return cs_error( self.msg, self.previous_errcode, - **{"org.matrix.unstable.errcode": self.errcode}, + **{"org.matrix.MSC3848.unstable.errcode": self.errcode}, **self._additional_fields, ) From ae053f41ce926fc2811a284fb7274d93f81dae33 Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Mon, 25 Jul 2022 15:08:49 +0100 Subject: [PATCH 09/19] namespace --- synapse/api/errors.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/synapse/api/errors.py b/synapse/api/errors.py index 9b0ab399e2d5..d88a3d43ef75 100644 --- a/synapse/api/errors.py +++ b/synapse/api/errors.py @@ -318,7 +318,7 @@ class UnstableSpecAuthError(AuthError): This error will return a "org.matrix.unstable.errcode" property with the new error code, with the previous error code still being defined in the "errcode" property. - This error will include `org.matrix.MSC3848.unstable.errcode` in the C-S error body. + This error will include `org.matrix.msc3848.unstable.errcode` in the C-S error body. """ def __init__( @@ -336,7 +336,7 @@ def error_dict(self) -> "JsonDict": return cs_error( self.msg, self.previous_errcode, - **{"org.matrix.MSC3848.unstable.errcode": self.errcode}, + **{"org.matrix.msc3848.unstable.errcode": self.errcode}, **self._additional_fields, ) From eb870dd70d50cab68adc5da237a343e8977556b4 Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Mon, 25 Jul 2022 16:53:38 +0100 Subject: [PATCH 10/19] Make errors configurable --- synapse/api/errors.py | 7 +++++-- synapse/config/experimental.py | 3 +++ synapse/federation/federation_server.py | 8 +++++++- synapse/handlers/auth.py | 3 +++ synapse/http/server.py | 19 +++++++++++++++++-- 5 files changed, 35 insertions(+), 5 deletions(-) diff --git a/synapse/api/errors.py b/synapse/api/errors.py index d88a3d43ef75..360e1f36e38f 100644 --- a/synapse/api/errors.py +++ b/synapse/api/errors.py @@ -332,11 +332,14 @@ def __init__( self.previous_errcode = previous_errcode super().__init__(code, msg, errcode, additional_fields) - def error_dict(self) -> "JsonDict": + def error_dict(self, allow_unstable_fields: bool = False) -> "JsonDict": + fields = {} + if allow_unstable_fields: + fields["org.matrix.msc3848.unstable.errcode"] = self.errcode return cs_error( self.msg, self.previous_errcode, - **{"org.matrix.msc3848.unstable.errcode": self.errcode}, + **fields, **self._additional_fields, ) diff --git a/synapse/config/experimental.py b/synapse/config/experimental.py index ee443cea0054..1902222d7b67 100644 --- a/synapse/config/experimental.py +++ b/synapse/config/experimental.py @@ -90,3 +90,6 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None: # MSC3827: Filtering of /publicRooms by room type self.msc3827_enabled: bool = experimental.get("msc3827_enabled", False) + + # MSC3848: Introduce errcodes for specific event sending failures + self.msc3848_enabled: bool = experimental.get("msc3848_enabled", False) diff --git a/synapse/federation/federation_server.py b/synapse/federation/federation_server.py index ae550d3f4de6..8350d66e4116 100644 --- a/synapse/federation/federation_server.py +++ b/synapse/federation/federation_server.py @@ -42,6 +42,7 @@ IncompatibleRoomVersionError, NotFoundError, SynapseError, + UnstableSpecAuthError, UnsupportedRoomVersionError, ) from synapse.api.room_versions import KNOWN_ROOM_VERSIONS, RoomVersion @@ -469,7 +470,12 @@ async def process_pdus_for_room(room_id: str) -> None: ) for pdu in pdus_by_room[room_id]: event_id = pdu.event_id - pdu_results[event_id] = e.error_dict() + if isinstance(e, UnstableSpecAuthError): + pdu_results[event_id] = e.error_dict( + self.hs.config.experimental.msc3848_enabled + ) + else: + pdu_results[event_id] = e.error_dict() return for pdu in pdus_by_room[room_id]: diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py index 3d83236b0cd7..099e46ce78d0 100644 --- a/synapse/handlers/auth.py +++ b/synapse/handlers/auth.py @@ -50,6 +50,7 @@ LoginError, StoreError, SynapseError, + UnstableSpecAuthError, UserDeactivatedError, ) from synapse.api.ratelimiting import Ratelimiter @@ -562,6 +563,8 @@ async def check_ui_auth( await self.store.mark_ui_auth_stage_complete( session.session_id, login_type, result ) + except UnstableSpecAuthError as e: + errordict = e.error_dict(self.hs.config.experimental.msc3848_enabled) except LoginError as e: # this step failed. Merge the error dict into the response # so that the client can have another go. diff --git a/synapse/http/server.py b/synapse/http/server.py index cf2d6f904b9f..b591249468c6 100644 --- a/synapse/http/server.py +++ b/synapse/http/server.py @@ -57,6 +57,7 @@ RedirectException, SynapseError, UnrecognizedRequestError, + UnstableSpecAuthError, ) from synapse.http.site import SynapseRequest from synapse.logging.context import defer_to_thread, preserve_fn, run_in_background @@ -155,14 +156,20 @@ def is_method_cancellable(method: Callable[..., Any]) -> bool: return getattr(method, "cancellable", False) -def return_json_error(f: failure.Failure, request: SynapseRequest) -> None: +def return_json_error( + f: failure.Failure, request: SynapseRequest, allow_unstable_fields=False +) -> None: """Sends a JSON error response to clients.""" if f.check(SynapseError): # mypy doesn't understand that f.check asserts the type. exc: SynapseError = f.value # type: ignore error_code = exc.code - error_dict = exc.error_dict() + if f.check(UnstableSpecAuthError): + unstable_exc: UnstableSpecAuthError = f.value # type: ignore + error_dict = unstable_exc.error_dict(allow_unstable_fields) + else: + error_dict = exc.error_dict() logger.info("%s SynapseError: %s - %s", request, error_code, exc.msg) elif f.check(CancelledError): @@ -575,6 +582,14 @@ async def _async_render(self, request: SynapseRequest) -> Tuple[int, Any]: return callback_return + def _send_error_response( + self, + f: failure.Failure, + request: SynapseRequest, + ) -> None: + """Implements _AsyncResource._send_error_response""" + return_json_error(f, request, self.hs.config.experimental.msc3848_enabled) + class DirectServeHtmlResource(_AsyncResource): """A resource that will call `self._async_on_` on new requests, From 1a97541b239a4c250b8de02d8c0bc2a6c995af85 Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Mon, 25 Jul 2022 16:56:45 +0100 Subject: [PATCH 11/19] tidy up if --- synapse/http/server.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/synapse/http/server.py b/synapse/http/server.py index b591249468c6..be23a2e1e2c5 100644 --- a/synapse/http/server.py +++ b/synapse/http/server.py @@ -161,16 +161,17 @@ def return_json_error( ) -> None: """Sends a JSON error response to clients.""" - if f.check(SynapseError): + if f.check(UnstableSpecAuthError): + # mypy doesn't understand that f.check asserts the type. + exc: UnstableSpecAuthError = f.value # type: ignore + error_code = exc.code + error_dict = exc.error_dict(allow_unstable_fields) + logger.info("%s SynapseError: %s - %s", request, error_code, exc.msg) + elif f.check(SynapseError): # mypy doesn't understand that f.check asserts the type. exc: SynapseError = f.value # type: ignore error_code = exc.code - if f.check(UnstableSpecAuthError): - unstable_exc: UnstableSpecAuthError = f.value # type: ignore - error_dict = unstable_exc.error_dict(allow_unstable_fields) - else: - error_dict = exc.error_dict() - + error_dict = exc.error_dict() logger.info("%s SynapseError: %s - %s", request, error_code, exc.msg) elif f.check(CancelledError): error_code = HTTP_STATUS_REQUEST_CANCELLED From 1d34f6b5887dd59b1025fdc0597a5e212bcb3ad6 Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Mon, 25 Jul 2022 17:00:34 +0100 Subject: [PATCH 12/19] make lint happy --- synapse/http/server.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/http/server.py b/synapse/http/server.py index be23a2e1e2c5..d3f1fe48ff75 100644 --- a/synapse/http/server.py +++ b/synapse/http/server.py @@ -157,7 +157,7 @@ def is_method_cancellable(method: Callable[..., Any]) -> bool: def return_json_error( - f: failure.Failure, request: SynapseRequest, allow_unstable_fields=False + f: failure.Failure, request: SynapseRequest, allow_unstable_fields: bool = False ) -> None: """Sends a JSON error response to clients.""" From 4f7629b5a05ce6818b9e95f5d21dc9b2f1fbf4a2 Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Wed, 27 Jul 2022 12:17:01 +0100 Subject: [PATCH 13/19] Pass in config to all error types --- synapse/api/errors.py | 25 +++++++++++---------- synapse/federation/federation_server.py | 8 +------ synapse/handlers/auth.py | 5 +---- synapse/http/server.py | 9 ++++---- tests/rest/client/test_third_party_rules.py | 4 ++-- 5 files changed, 22 insertions(+), 29 deletions(-) diff --git a/synapse/api/errors.py b/synapse/api/errors.py index 360e1f36e38f..fdbb78e6585b 100644 --- a/synapse/api/errors.py +++ b/synapse/api/errors.py @@ -23,6 +23,7 @@ from twisted.web import http +from synapse.config.homeserver import HomeServerConfig from synapse.util import json_decoder if typing.TYPE_CHECKING: @@ -173,7 +174,7 @@ def __init__( else: self._additional_fields = dict(additional_fields) - def error_dict(self) -> "JsonDict": + def error_dict(self, config: HomeServerConfig) -> "JsonDict": return cs_error(self.msg, self.errcode, **self._additional_fields) @@ -219,7 +220,7 @@ def __init__(self, msg: str, consent_uri: str): ) self._consent_uri = consent_uri - def error_dict(self) -> "JsonDict": + def error_dict(self, config: HomeServerConfig) -> "JsonDict": return cs_error(self.msg, self.errcode, consent_uri=self._consent_uri) @@ -332,9 +333,9 @@ def __init__( self.previous_errcode = previous_errcode super().__init__(code, msg, errcode, additional_fields) - def error_dict(self, allow_unstable_fields: bool = False) -> "JsonDict": + def error_dict(self, config: HomeServerConfig) -> "JsonDict": fields = {} - if allow_unstable_fields: + if config.experimental.msc3848_enabled: fields["org.matrix.msc3848.unstable.errcode"] = self.errcode return cs_error( self.msg, @@ -375,8 +376,8 @@ def __init__( super().__init__(msg=msg, errcode="M_UNKNOWN_TOKEN") self._soft_logout = soft_logout - def error_dict(self) -> "JsonDict": - d = super().error_dict() + def error_dict(self, config: HomeServerConfig) -> "JsonDict": + d = super().error_dict(config) d["soft_logout"] = self._soft_logout return d @@ -399,7 +400,7 @@ def __init__( self.limit_type = limit_type super().__init__(code, msg, errcode=errcode) - def error_dict(self) -> "JsonDict": + def error_dict(self, config: HomeServerConfig) -> "JsonDict": return cs_error( self.msg, self.errcode, @@ -434,7 +435,7 @@ def __init__( super().__init__(code, msg, errcode) self.error_url = error_url - def error_dict(self) -> "JsonDict": + def error_dict(self, config: HomeServerConfig) -> "JsonDict": return cs_error(self.msg, self.errcode, error_url=self.error_url) @@ -451,7 +452,7 @@ def __init__( super().__init__(code, msg, errcode) self.retry_after_ms = retry_after_ms - def error_dict(self) -> "JsonDict": + def error_dict(self, config: HomeServerConfig) -> "JsonDict": return cs_error(self.msg, self.errcode, retry_after_ms=self.retry_after_ms) @@ -466,7 +467,7 @@ def __init__(self, current_version: str): super().__init__(403, "Wrong room_keys version", Codes.WRONG_ROOM_KEYS_VERSION) self.current_version = current_version - def error_dict(self) -> "JsonDict": + def error_dict(self, config: HomeServerConfig) -> "JsonDict": return cs_error(self.msg, self.errcode, current_version=self.current_version) @@ -506,7 +507,7 @@ def __init__(self, room_version: str): self._room_version = room_version - def error_dict(self) -> "JsonDict": + def error_dict(self, config: HomeServerConfig) -> "JsonDict": return cs_error(self.msg, self.errcode, room_version=self._room_version) @@ -552,7 +553,7 @@ def __init__(self, content_keep_ms: Optional[int] = None): ) self.content_keep_ms = content_keep_ms - def error_dict(self) -> "JsonDict": + def error_dict(self, config: HomeServerConfig) -> "JsonDict": extra = {} if self.content_keep_ms is not None: extra = {"fi.mau.msc2815.content_keep_ms": self.content_keep_ms} diff --git a/synapse/federation/federation_server.py b/synapse/federation/federation_server.py index 8350d66e4116..1d60137411d6 100644 --- a/synapse/federation/federation_server.py +++ b/synapse/federation/federation_server.py @@ -42,7 +42,6 @@ IncompatibleRoomVersionError, NotFoundError, SynapseError, - UnstableSpecAuthError, UnsupportedRoomVersionError, ) from synapse.api.room_versions import KNOWN_ROOM_VERSIONS, RoomVersion @@ -470,12 +469,7 @@ async def process_pdus_for_room(room_id: str) -> None: ) for pdu in pdus_by_room[room_id]: event_id = pdu.event_id - if isinstance(e, UnstableSpecAuthError): - pdu_results[event_id] = e.error_dict( - self.hs.config.experimental.msc3848_enabled - ) - else: - pdu_results[event_id] = e.error_dict() + pdu_results[event_id] = e.error_dict(self.hs.config) return for pdu in pdus_by_room[room_id]: diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py index 099e46ce78d0..bfa553504442 100644 --- a/synapse/handlers/auth.py +++ b/synapse/handlers/auth.py @@ -50,7 +50,6 @@ LoginError, StoreError, SynapseError, - UnstableSpecAuthError, UserDeactivatedError, ) from synapse.api.ratelimiting import Ratelimiter @@ -563,12 +562,10 @@ async def check_ui_auth( await self.store.mark_ui_auth_stage_complete( session.session_id, login_type, result ) - except UnstableSpecAuthError as e: - errordict = e.error_dict(self.hs.config.experimental.msc3848_enabled) except LoginError as e: # this step failed. Merge the error dict into the response # so that the client can have another go. - errordict = e.error_dict() + errordict = e.error_dict(self.hs.config) creds = await self.store.get_completed_ui_auth_stages(session.session_id) for f in flows: diff --git a/synapse/http/server.py b/synapse/http/server.py index d3f1fe48ff75..47b396b62b0e 100644 --- a/synapse/http/server.py +++ b/synapse/http/server.py @@ -59,6 +59,7 @@ UnrecognizedRequestError, UnstableSpecAuthError, ) +from synapse.config.homeserver import HomeServerConfig from synapse.http.site import SynapseRequest from synapse.logging.context import defer_to_thread, preserve_fn, run_in_background from synapse.logging.opentracing import active_span, start_active_span, trace_servlet @@ -157,7 +158,7 @@ def is_method_cancellable(method: Callable[..., Any]) -> bool: def return_json_error( - f: failure.Failure, request: SynapseRequest, allow_unstable_fields: bool = False + f: failure.Failure, request: SynapseRequest, config: HomeServerConfig ) -> None: """Sends a JSON error response to clients.""" @@ -165,13 +166,13 @@ def return_json_error( # mypy doesn't understand that f.check asserts the type. exc: UnstableSpecAuthError = f.value # type: ignore error_code = exc.code - error_dict = exc.error_dict(allow_unstable_fields) + error_dict = exc.error_dict(config) logger.info("%s SynapseError: %s - %s", request, error_code, exc.msg) elif f.check(SynapseError): # mypy doesn't understand that f.check asserts the type. exc: SynapseError = f.value # type: ignore error_code = exc.code - error_dict = exc.error_dict() + error_dict = exc.error_dict(config) logger.info("%s SynapseError: %s - %s", request, error_code, exc.msg) elif f.check(CancelledError): error_code = HTTP_STATUS_REQUEST_CANCELLED @@ -589,7 +590,7 @@ def _send_error_response( request: SynapseRequest, ) -> None: """Implements _AsyncResource._send_error_response""" - return_json_error(f, request, self.hs.config.experimental.msc3848_enabled) + return_json_error(f, request, self.hs.config) class DirectServeHtmlResource(_AsyncResource): diff --git a/tests/rest/client/test_third_party_rules.py b/tests/rest/client/test_third_party_rules.py index 9a48e9286fe4..4841e939a0a8 100644 --- a/tests/rest/client/test_third_party_rules.py +++ b/tests/rest/client/test_third_party_rules.py @@ -185,12 +185,12 @@ def test_third_party_rules_workaround_synapse_errors_pass_through(self) -> None: """ class NastyHackException(SynapseError): - def error_dict(self) -> JsonDict: + def error_dict(self, config) -> JsonDict: """ This overrides SynapseError's `error_dict` to nastily inject JSON into the error response. """ - result = super().error_dict() + result = super().error_dict(config) result["nasty"] = "very" return result From 5b3ef13d608e2224026ff442a4940bb26a679b61 Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Wed, 27 Jul 2022 12:31:19 +0100 Subject: [PATCH 14/19] Appease python --- synapse/api/errors.py | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/synapse/api/errors.py b/synapse/api/errors.py index fdbb78e6585b..02473adc43a9 100644 --- a/synapse/api/errors.py +++ b/synapse/api/errors.py @@ -23,11 +23,11 @@ from twisted.web import http -from synapse.config.homeserver import HomeServerConfig from synapse.util import json_decoder if typing.TYPE_CHECKING: from synapse.types import JsonDict + from synapse.config.homeserver import HomeServerConfig logger = logging.getLogger(__name__) @@ -174,7 +174,7 @@ def __init__( else: self._additional_fields = dict(additional_fields) - def error_dict(self, config: HomeServerConfig) -> "JsonDict": + def error_dict(self, config: "HomeServerConfig") -> "JsonDict": return cs_error(self.msg, self.errcode, **self._additional_fields) @@ -220,7 +220,7 @@ def __init__(self, msg: str, consent_uri: str): ) self._consent_uri = consent_uri - def error_dict(self, config: HomeServerConfig) -> "JsonDict": + def error_dict(self, config: "HomeServerConfig") -> "JsonDict": return cs_error(self.msg, self.errcode, consent_uri=self._consent_uri) @@ -333,7 +333,7 @@ def __init__( self.previous_errcode = previous_errcode super().__init__(code, msg, errcode, additional_fields) - def error_dict(self, config: HomeServerConfig) -> "JsonDict": + def error_dict(self, config: "HomeServerConfig") -> "JsonDict": fields = {} if config.experimental.msc3848_enabled: fields["org.matrix.msc3848.unstable.errcode"] = self.errcode @@ -376,7 +376,7 @@ def __init__( super().__init__(msg=msg, errcode="M_UNKNOWN_TOKEN") self._soft_logout = soft_logout - def error_dict(self, config: HomeServerConfig) -> "JsonDict": + def error_dict(self, config: "HomeServerConfig") -> "JsonDict": d = super().error_dict(config) d["soft_logout"] = self._soft_logout return d @@ -400,7 +400,7 @@ def __init__( self.limit_type = limit_type super().__init__(code, msg, errcode=errcode) - def error_dict(self, config: HomeServerConfig) -> "JsonDict": + def error_dict(self, config: "HomeServerConfig") -> "JsonDict": return cs_error( self.msg, self.errcode, @@ -435,7 +435,7 @@ def __init__( super().__init__(code, msg, errcode) self.error_url = error_url - def error_dict(self, config: HomeServerConfig) -> "JsonDict": + def error_dict(self, config: "HomeServerConfig") -> "JsonDict": return cs_error(self.msg, self.errcode, error_url=self.error_url) @@ -452,7 +452,7 @@ def __init__( super().__init__(code, msg, errcode) self.retry_after_ms = retry_after_ms - def error_dict(self, config: HomeServerConfig) -> "JsonDict": + def error_dict(self, config: "HomeServerConfig") -> "JsonDict": return cs_error(self.msg, self.errcode, retry_after_ms=self.retry_after_ms) @@ -467,7 +467,7 @@ def __init__(self, current_version: str): super().__init__(403, "Wrong room_keys version", Codes.WRONG_ROOM_KEYS_VERSION) self.current_version = current_version - def error_dict(self, config: HomeServerConfig) -> "JsonDict": + def error_dict(self, config: "HomeServerConfig") -> "JsonDict": return cs_error(self.msg, self.errcode, current_version=self.current_version) @@ -507,7 +507,7 @@ def __init__(self, room_version: str): self._room_version = room_version - def error_dict(self, config: HomeServerConfig) -> "JsonDict": + def error_dict(self, config: "HomeServerConfig") -> "JsonDict": return cs_error(self.msg, self.errcode, room_version=self._room_version) @@ -553,7 +553,7 @@ def __init__(self, content_keep_ms: Optional[int] = None): ) self.content_keep_ms = content_keep_ms - def error_dict(self, config: HomeServerConfig) -> "JsonDict": + def error_dict(self, config: "HomeServerConfig") -> "JsonDict": extra = {} if self.content_keep_ms is not None: extra = {"fi.mau.msc2815.content_keep_ms": self.content_keep_ms} From 780ed562a5bdd0abddb166586800f79a5d54327f Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Wed, 27 Jul 2022 12:33:18 +0100 Subject: [PATCH 15/19] reorder --- synapse/api/errors.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/api/errors.py b/synapse/api/errors.py index 02473adc43a9..135ee6c21c31 100644 --- a/synapse/api/errors.py +++ b/synapse/api/errors.py @@ -26,8 +26,8 @@ from synapse.util import json_decoder if typing.TYPE_CHECKING: - from synapse.types import JsonDict from synapse.config.homeserver import HomeServerConfig + from synapse.types import JsonDict logger = logging.getLogger(__name__) From 7ff61c9212aefe0cc4ceab7d86419d6467c0a5a9 Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Wed, 27 Jul 2022 12:34:59 +0100 Subject: [PATCH 16/19] lint again --- synapse/api/errors.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/api/errors.py b/synapse/api/errors.py index 135ee6c21c31..598baaab4f94 100644 --- a/synapse/api/errors.py +++ b/synapse/api/errors.py @@ -26,7 +26,7 @@ from synapse.util import json_decoder if typing.TYPE_CHECKING: - from synapse.config.homeserver import HomeServerConfig + from synapse.config.homeserver import HomeServerConfig from synapse.types import JsonDict logger = logging.getLogger(__name__) From 584351bd90b5810fe53b9ad97bdaf229ecd28f62 Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Wed, 27 Jul 2022 12:39:21 +0100 Subject: [PATCH 17/19] Make homeserver config optional --- synapse/api/errors.py | 22 ++++++++++----------- synapse/http/server.py | 12 +++-------- tests/rest/client/test_third_party_rules.py | 3 ++- 3 files changed, 16 insertions(+), 21 deletions(-) diff --git a/synapse/api/errors.py b/synapse/api/errors.py index 598baaab4f94..957e325b08ea 100644 --- a/synapse/api/errors.py +++ b/synapse/api/errors.py @@ -174,7 +174,7 @@ def __init__( else: self._additional_fields = dict(additional_fields) - def error_dict(self, config: "HomeServerConfig") -> "JsonDict": + def error_dict(self, config: Optional["HomeServerConfig"]) -> "JsonDict": return cs_error(self.msg, self.errcode, **self._additional_fields) @@ -220,7 +220,7 @@ def __init__(self, msg: str, consent_uri: str): ) self._consent_uri = consent_uri - def error_dict(self, config: "HomeServerConfig") -> "JsonDict": + def error_dict(self, config: Optional["HomeServerConfig"]) -> "JsonDict": return cs_error(self.msg, self.errcode, consent_uri=self._consent_uri) @@ -333,9 +333,9 @@ def __init__( self.previous_errcode = previous_errcode super().__init__(code, msg, errcode, additional_fields) - def error_dict(self, config: "HomeServerConfig") -> "JsonDict": + def error_dict(self, config: Optional["HomeServerConfig"]) -> "JsonDict": fields = {} - if config.experimental.msc3848_enabled: + if config != None and config.experimental.msc3848_enabled: fields["org.matrix.msc3848.unstable.errcode"] = self.errcode return cs_error( self.msg, @@ -376,7 +376,7 @@ def __init__( super().__init__(msg=msg, errcode="M_UNKNOWN_TOKEN") self._soft_logout = soft_logout - def error_dict(self, config: "HomeServerConfig") -> "JsonDict": + def error_dict(self, config: Optional["HomeServerConfig"]) -> "JsonDict": d = super().error_dict(config) d["soft_logout"] = self._soft_logout return d @@ -400,7 +400,7 @@ def __init__( self.limit_type = limit_type super().__init__(code, msg, errcode=errcode) - def error_dict(self, config: "HomeServerConfig") -> "JsonDict": + def error_dict(self, config: Optional["HomeServerConfig"]) -> "JsonDict": return cs_error( self.msg, self.errcode, @@ -435,7 +435,7 @@ def __init__( super().__init__(code, msg, errcode) self.error_url = error_url - def error_dict(self, config: "HomeServerConfig") -> "JsonDict": + def error_dict(self, config: Optional["HomeServerConfig"]) -> "JsonDict": return cs_error(self.msg, self.errcode, error_url=self.error_url) @@ -452,7 +452,7 @@ def __init__( super().__init__(code, msg, errcode) self.retry_after_ms = retry_after_ms - def error_dict(self, config: "HomeServerConfig") -> "JsonDict": + def error_dict(self, config: Optional["HomeServerConfig"]) -> "JsonDict": return cs_error(self.msg, self.errcode, retry_after_ms=self.retry_after_ms) @@ -467,7 +467,7 @@ def __init__(self, current_version: str): super().__init__(403, "Wrong room_keys version", Codes.WRONG_ROOM_KEYS_VERSION) self.current_version = current_version - def error_dict(self, config: "HomeServerConfig") -> "JsonDict": + def error_dict(self, config: Optional["HomeServerConfig"]) -> "JsonDict": return cs_error(self.msg, self.errcode, current_version=self.current_version) @@ -507,7 +507,7 @@ def __init__(self, room_version: str): self._room_version = room_version - def error_dict(self, config: "HomeServerConfig") -> "JsonDict": + def error_dict(self, config: Optional["HomeServerConfig"]) -> "JsonDict": return cs_error(self.msg, self.errcode, room_version=self._room_version) @@ -553,7 +553,7 @@ def __init__(self, content_keep_ms: Optional[int] = None): ) self.content_keep_ms = content_keep_ms - def error_dict(self, config: "HomeServerConfig") -> "JsonDict": + def error_dict(self, config: Optional["HomeServerConfig"]) -> "JsonDict": extra = {} if self.content_keep_ms is not None: extra = {"fi.mau.msc2815.content_keep_ms": self.content_keep_ms} diff --git a/synapse/http/server.py b/synapse/http/server.py index 47b396b62b0e..d2e38edf29d8 100644 --- a/synapse/http/server.py +++ b/synapse/http/server.py @@ -158,17 +158,11 @@ def is_method_cancellable(method: Callable[..., Any]) -> bool: def return_json_error( - f: failure.Failure, request: SynapseRequest, config: HomeServerConfig + f: failure.Failure, request: SynapseRequest, config: HomeServerConfig|None ) -> None: """Sends a JSON error response to clients.""" - if f.check(UnstableSpecAuthError): - # mypy doesn't understand that f.check asserts the type. - exc: UnstableSpecAuthError = f.value # type: ignore - error_code = exc.code - error_dict = exc.error_dict(config) - logger.info("%s SynapseError: %s - %s", request, error_code, exc.msg) - elif f.check(SynapseError): + if f.check(SynapseError): # mypy doesn't understand that f.check asserts the type. exc: SynapseError = f.value # type: ignore error_code = exc.code @@ -459,7 +453,7 @@ def _send_error_response( request: SynapseRequest, ) -> None: """Implements _AsyncResource._send_error_response""" - return_json_error(f, request) + return_json_error(f, request, None) @attr.s(slots=True, frozen=True, auto_attribs=True) diff --git a/tests/rest/client/test_third_party_rules.py b/tests/rest/client/test_third_party_rules.py index 4841e939a0a8..4852d126bf85 100644 --- a/tests/rest/client/test_third_party_rules.py +++ b/tests/rest/client/test_third_party_rules.py @@ -20,6 +20,7 @@ from synapse.api.constants import EventTypes, LoginType, Membership from synapse.api.errors import SynapseError from synapse.api.room_versions import RoomVersion +from synapse.config.homeserver import HomeServerConfig from synapse.events import EventBase from synapse.events.third_party_rules import load_legacy_third_party_event_rules from synapse.rest import admin @@ -185,7 +186,7 @@ def test_third_party_rules_workaround_synapse_errors_pass_through(self) -> None: """ class NastyHackException(SynapseError): - def error_dict(self, config) -> JsonDict: + def error_dict(self, config: HomeServerConfig) -> JsonDict: """ This overrides SynapseError's `error_dict` to nastily inject JSON into the error response. From 1c19e5f8b0c3c52309cb3671659281973d6ed303 Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Wed, 27 Jul 2022 12:40:40 +0100 Subject: [PATCH 18/19] Fix optionals --- synapse/api/errors.py | 2 +- synapse/http/server.py | 3 +-- tests/rest/client/test_third_party_rules.py | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/synapse/api/errors.py b/synapse/api/errors.py index 957e325b08ea..e6dea89c6d09 100644 --- a/synapse/api/errors.py +++ b/synapse/api/errors.py @@ -335,7 +335,7 @@ def __init__( def error_dict(self, config: Optional["HomeServerConfig"]) -> "JsonDict": fields = {} - if config != None and config.experimental.msc3848_enabled: + if config is not None and config.experimental.msc3848_enabled: fields["org.matrix.msc3848.unstable.errcode"] = self.errcode return cs_error( self.msg, diff --git a/synapse/http/server.py b/synapse/http/server.py index d2e38edf29d8..ba67617fb4b8 100644 --- a/synapse/http/server.py +++ b/synapse/http/server.py @@ -57,7 +57,6 @@ RedirectException, SynapseError, UnrecognizedRequestError, - UnstableSpecAuthError, ) from synapse.config.homeserver import HomeServerConfig from synapse.http.site import SynapseRequest @@ -158,7 +157,7 @@ def is_method_cancellable(method: Callable[..., Any]) -> bool: def return_json_error( - f: failure.Failure, request: SynapseRequest, config: HomeServerConfig|None + f: failure.Failure, request: SynapseRequest, config: HomeServerConfig | None ) -> None: """Sends a JSON error response to clients.""" diff --git a/tests/rest/client/test_third_party_rules.py b/tests/rest/client/test_third_party_rules.py index 4852d126bf85..18a719540978 100644 --- a/tests/rest/client/test_third_party_rules.py +++ b/tests/rest/client/test_third_party_rules.py @@ -186,7 +186,7 @@ def test_third_party_rules_workaround_synapse_errors_pass_through(self) -> None: """ class NastyHackException(SynapseError): - def error_dict(self, config: HomeServerConfig) -> JsonDict: + def error_dict(self, config: Optional[HomeServerConfig]) -> JsonDict: """ This overrides SynapseError's `error_dict` to nastily inject JSON into the error response. From c430045ab634a5f94192a03b9a9ab982d91724fe Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Wed, 27 Jul 2022 12:49:07 +0100 Subject: [PATCH 19/19] mark as optional to appease code --- synapse/http/server.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/http/server.py b/synapse/http/server.py index ba67617fb4b8..19f42159b8bd 100644 --- a/synapse/http/server.py +++ b/synapse/http/server.py @@ -157,7 +157,7 @@ def is_method_cancellable(method: Callable[..., Any]) -> bool: def return_json_error( - f: failure.Failure, request: SynapseRequest, config: HomeServerConfig | None + f: failure.Failure, request: SynapseRequest, config: Optional[HomeServerConfig] ) -> None: """Sends a JSON error response to clients."""