Implement MSC3848: Introduce errcodes for specific event sending failures (#13343)

Implements MSC3848
This commit is contained in:
Will Hunt 2022-07-27 13:44:40 +01:00 committed by GitHub
parent 39be5bc550
commit 502f075e96
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 144 additions and 36 deletions

View File

@ -0,0 +1 @@
Add new unstable error codes `ORG.MATRIX.MSC3848.ALREADY_JOINED`, `ORG.MATRIX.MSC3848.NOT_JOINED`, and `ORG.MATRIX.MSC3848.INSUFFICIENT_POWER` described in MSC3848.

View File

@ -26,6 +26,7 @@ from synapse.api.errors import (
Codes, Codes,
InvalidClientTokenError, InvalidClientTokenError,
MissingClientTokenError, MissingClientTokenError,
UnstableSpecAuthError,
) )
from synapse.appservice import ApplicationService from synapse.appservice import ApplicationService
from synapse.http import get_request_user_agent from synapse.http import get_request_user_agent
@ -106,8 +107,11 @@ class Auth:
forgot = await self.store.did_forget(user_id, room_id) forgot = await self.store.did_forget(user_id, room_id)
if not forgot: if not forgot:
return membership, member_event_id return membership, member_event_id
raise UnstableSpecAuthError(
raise AuthError(403, "User %s not in room %s" % (user_id, room_id)) 403,
"User %s not in room %s" % (user_id, room_id),
errcode=Codes.NOT_JOINED,
)
async def get_user_by_req( async def get_user_by_req(
self, self,
@ -600,8 +604,9 @@ class Auth:
== HistoryVisibility.WORLD_READABLE == HistoryVisibility.WORLD_READABLE
): ):
return Membership.JOIN, None return Membership.JOIN, None
raise AuthError( raise UnstableSpecAuthError(
403, 403,
"User %s not in room %s, and room previews are disabled" "User %s not in room %s, and room previews are disabled"
% (user_id, room_id), % (user_id, room_id),
errcode=Codes.NOT_JOINED,
) )

View File

@ -26,6 +26,7 @@ from twisted.web import http
from synapse.util import json_decoder from synapse.util import json_decoder
if typing.TYPE_CHECKING: if typing.TYPE_CHECKING:
from synapse.config.homeserver import HomeServerConfig
from synapse.types import JsonDict from synapse.types import JsonDict
logger = logging.getLogger(__name__) logger = logging.getLogger(__name__)
@ -80,6 +81,12 @@ class Codes(str, Enum):
INVALID_SIGNATURE = "M_INVALID_SIGNATURE" INVALID_SIGNATURE = "M_INVALID_SIGNATURE"
USER_DEACTIVATED = "M_USER_DEACTIVATED" USER_DEACTIVATED = "M_USER_DEACTIVATED"
# 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. # The account has been suspended on the server.
# By opposition to `USER_DEACTIVATED`, this is a reversible measure # By opposition to `USER_DEACTIVATED`, this is a reversible measure
# that can possibly be appealed and reverted. # that can possibly be appealed and reverted.
@ -167,7 +174,7 @@ class SynapseError(CodeMessageException):
else: else:
self._additional_fields = dict(additional_fields) self._additional_fields = dict(additional_fields)
def error_dict(self) -> "JsonDict": def error_dict(self, config: Optional["HomeServerConfig"]) -> "JsonDict":
return cs_error(self.msg, self.errcode, **self._additional_fields) return cs_error(self.msg, self.errcode, **self._additional_fields)
@ -213,7 +220,7 @@ class ConsentNotGivenError(SynapseError):
) )
self._consent_uri = consent_uri self._consent_uri = consent_uri
def error_dict(self) -> "JsonDict": def error_dict(self, config: Optional["HomeServerConfig"]) -> "JsonDict":
return cs_error(self.msg, self.errcode, consent_uri=self._consent_uri) return cs_error(self.msg, self.errcode, consent_uri=self._consent_uri)
@ -307,6 +314,37 @@ class AuthError(SynapseError):
super().__init__(code, msg, errcode, additional_fields) 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.
This error will include `org.matrix.msc3848.unstable.errcode` in the C-S error body.
"""
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, config: Optional["HomeServerConfig"]) -> "JsonDict":
fields = {}
if config is not None and config.experimental.msc3848_enabled:
fields["org.matrix.msc3848.unstable.errcode"] = self.errcode
return cs_error(
self.msg,
self.previous_errcode,
**fields,
**self._additional_fields,
)
class InvalidClientCredentialsError(SynapseError): class InvalidClientCredentialsError(SynapseError):
"""An error raised when there was a problem with the authorisation credentials """An error raised when there was a problem with the authorisation credentials
in a client request. in a client request.
@ -338,8 +376,8 @@ class InvalidClientTokenError(InvalidClientCredentialsError):
super().__init__(msg=msg, errcode="M_UNKNOWN_TOKEN") super().__init__(msg=msg, errcode="M_UNKNOWN_TOKEN")
self._soft_logout = soft_logout self._soft_logout = soft_logout
def error_dict(self) -> "JsonDict": def error_dict(self, config: Optional["HomeServerConfig"]) -> "JsonDict":
d = super().error_dict() d = super().error_dict(config)
d["soft_logout"] = self._soft_logout d["soft_logout"] = self._soft_logout
return d return d
@ -362,7 +400,7 @@ class ResourceLimitError(SynapseError):
self.limit_type = limit_type self.limit_type = limit_type
super().__init__(code, msg, errcode=errcode) super().__init__(code, msg, errcode=errcode)
def error_dict(self) -> "JsonDict": def error_dict(self, config: Optional["HomeServerConfig"]) -> "JsonDict":
return cs_error( return cs_error(
self.msg, self.msg,
self.errcode, self.errcode,
@ -397,7 +435,7 @@ class InvalidCaptchaError(SynapseError):
super().__init__(code, msg, errcode) super().__init__(code, msg, errcode)
self.error_url = error_url self.error_url = error_url
def error_dict(self) -> "JsonDict": def error_dict(self, config: Optional["HomeServerConfig"]) -> "JsonDict":
return cs_error(self.msg, self.errcode, error_url=self.error_url) return cs_error(self.msg, self.errcode, error_url=self.error_url)
@ -414,7 +452,7 @@ class LimitExceededError(SynapseError):
super().__init__(code, msg, errcode) super().__init__(code, msg, errcode)
self.retry_after_ms = retry_after_ms self.retry_after_ms = retry_after_ms
def error_dict(self) -> "JsonDict": def error_dict(self, config: Optional["HomeServerConfig"]) -> "JsonDict":
return cs_error(self.msg, self.errcode, retry_after_ms=self.retry_after_ms) return cs_error(self.msg, self.errcode, retry_after_ms=self.retry_after_ms)
@ -429,7 +467,7 @@ class RoomKeysVersionError(SynapseError):
super().__init__(403, "Wrong room_keys version", Codes.WRONG_ROOM_KEYS_VERSION) super().__init__(403, "Wrong room_keys version", Codes.WRONG_ROOM_KEYS_VERSION)
self.current_version = current_version self.current_version = current_version
def error_dict(self) -> "JsonDict": def error_dict(self, config: Optional["HomeServerConfig"]) -> "JsonDict":
return cs_error(self.msg, self.errcode, current_version=self.current_version) return cs_error(self.msg, self.errcode, current_version=self.current_version)
@ -469,7 +507,7 @@ class IncompatibleRoomVersionError(SynapseError):
self._room_version = room_version self._room_version = room_version
def error_dict(self) -> "JsonDict": def error_dict(self, config: Optional["HomeServerConfig"]) -> "JsonDict":
return cs_error(self.msg, self.errcode, room_version=self._room_version) return cs_error(self.msg, self.errcode, room_version=self._room_version)
@ -515,7 +553,7 @@ class UnredactedContentDeletedError(SynapseError):
) )
self.content_keep_ms = content_keep_ms self.content_keep_ms = content_keep_ms
def error_dict(self) -> "JsonDict": def error_dict(self, config: Optional["HomeServerConfig"]) -> "JsonDict":
extra = {} extra = {}
if self.content_keep_ms is not None: if self.content_keep_ms is not None:
extra = {"fi.mau.msc2815.content_keep_ms": self.content_keep_ms} extra = {"fi.mau.msc2815.content_keep_ms": self.content_keep_ms}

View File

@ -90,3 +90,6 @@ class ExperimentalConfig(Config):
# MSC3827: Filtering of /publicRooms by room type # MSC3827: Filtering of /publicRooms by room type
self.msc3827_enabled: bool = experimental.get("msc3827_enabled", False) 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)

View File

@ -30,7 +30,13 @@ from synapse.api.constants import (
JoinRules, JoinRules,
Membership, Membership,
) )
from synapse.api.errors import AuthError, EventSizeError, SynapseError from synapse.api.errors import (
AuthError,
Codes,
EventSizeError,
SynapseError,
UnstableSpecAuthError,
)
from synapse.api.room_versions import ( from synapse.api.room_versions import (
KNOWN_ROOM_VERSIONS, KNOWN_ROOM_VERSIONS,
EventFormatVersions, EventFormatVersions,
@ -291,7 +297,11 @@ def check_state_dependent_auth_rules(
invite_level = get_named_level(auth_dict, "invite", 0) invite_level = get_named_level(auth_dict, "invite", 0)
if user_level < invite_level: 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: else:
logger.debug("Allowing! %s", event) logger.debug("Allowing! %s", event)
return return
@ -474,7 +484,11 @@ def _is_membership_change_allowed(
return return
if not caller_in_room: # caller isn't joined 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: if Membership.INVITE == membership:
# TODO (erikj): We should probably handle this more intelligently # TODO (erikj): We should probably handle this more intelligently
@ -484,10 +498,18 @@ def _is_membership_change_allowed(
if target_banned: if target_banned:
raise AuthError(403, "%s is banned from the room" % (target_user_id,)) raise AuthError(403, "%s is banned from the room" % (target_user_id,))
elif target_in_room: # the target is already in the room. elif target_in_room: # the target is already in the room.
raise AuthError(403, "%s is already in the room." % target_user_id) raise UnstableSpecAuthError(
403,
"%s is already in the room." % target_user_id,
errcode=Codes.ALREADY_JOINED,
)
else: else:
if user_level < invite_level: 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: elif Membership.JOIN == membership:
# Joins are valid iff caller == target and: # Joins are valid iff caller == target and:
# * They are not banned. # * They are not banned.
@ -549,15 +571,27 @@ def _is_membership_change_allowed(
elif Membership.LEAVE == membership: elif Membership.LEAVE == membership:
# TODO (erikj): Implement kicks. # TODO (erikj): Implement kicks.
if target_banned and user_level < ban_level: 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: elif target_user_id != event.user_id:
kick_level = get_named_level(auth_events, "kick", 50) kick_level = get_named_level(auth_events, "kick", 50)
if user_level < kick_level or user_level <= target_level: 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: elif Membership.BAN == membership:
if user_level < ban_level or user_level <= target_level: 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: elif room_version.msc2403_knocking and Membership.KNOCK == membership:
if join_rule != JoinRules.KNOCK and ( if join_rule != JoinRules.KNOCK and (
not room_version.msc3787_knock_restricted_join_rule not room_version.msc3787_knock_restricted_join_rule
@ -567,7 +601,11 @@ def _is_membership_change_allowed(
elif target_user_id != event.user_id: elif target_user_id != event.user_id:
raise AuthError(403, "You cannot knock for other users") raise AuthError(403, "You cannot knock for other users")
elif target_in_room: 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: elif caller_invited:
raise AuthError(403, "You are already invited to this room") raise AuthError(403, "You are already invited to this room")
elif target_banned: elif target_banned:
@ -638,10 +676,11 @@ def _can_send_event(event: "EventBase", auth_events: StateMap["EventBase"]) -> b
user_level = get_user_power_level(event.user_id, auth_events) user_level = get_user_power_level(event.user_id, auth_events)
if user_level < send_level: if user_level < send_level:
raise AuthError( raise UnstableSpecAuthError(
403, 403,
"You don't have permission to post that to the room. " "You don't have permission to post that to the room. "
+ "user_level (%d) < send_level (%d)" % (user_level, send_level), + "user_level (%d) < send_level (%d)" % (user_level, send_level),
errcode=Codes.INSUFFICIENT_POWER,
) )
# Check state_key # Check state_key
@ -716,9 +755,10 @@ def check_historical(
historical_level = get_named_level(auth_events, "historical", 100) historical_level = get_named_level(auth_events, "historical", 100)
if user_level < historical_level: if user_level < historical_level:
raise AuthError( raise UnstableSpecAuthError(
403, 403,
'You don\'t have permission to send send historical related events ("insertion", "batch", and "marker")', 'You don\'t have permission to send send historical related events ("insertion", "batch", and "marker")',
errcode=Codes.INSUFFICIENT_POWER,
) )

View File

@ -469,7 +469,7 @@ class FederationServer(FederationBase):
) )
for pdu in pdus_by_room[room_id]: for pdu in pdus_by_room[room_id]:
event_id = pdu.event_id event_id = pdu.event_id
pdu_results[event_id] = e.error_dict() pdu_results[event_id] = e.error_dict(self.hs.config)
return return
for pdu in pdus_by_room[room_id]: for pdu in pdus_by_room[room_id]:

View File

@ -565,7 +565,7 @@ class AuthHandler:
except LoginError as e: except LoginError as e:
# this step failed. Merge the error dict into the response # this step failed. Merge the error dict into the response
# so that the client can have another go. # 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) creds = await self.store.get_completed_ui_auth_stages(session.session_id)
for f in flows: for f in flows:

View File

@ -41,6 +41,7 @@ from synapse.api.errors import (
NotFoundError, NotFoundError,
ShadowBanError, ShadowBanError,
SynapseError, SynapseError,
UnstableSpecAuthError,
UnsupportedRoomVersionError, UnsupportedRoomVersionError,
) )
from synapse.api.room_versions import KNOWN_ROOM_VERSIONS from synapse.api.room_versions import KNOWN_ROOM_VERSIONS
@ -149,7 +150,11 @@ class MessageHandler:
"Attempted to retrieve data from a room for a user that has never been in it. " "Attempted to retrieve data from a room for a user that has never been in it. "
"This should not have happened." "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 return data
@ -334,7 +339,11 @@ class MessageHandler:
break break
else: else:
# Loop fell through, AS has no interested users in room # 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 { return {
user_id: { user_id: {

View File

@ -28,11 +28,11 @@ from synapse.api.constants import (
RoomTypes, RoomTypes,
) )
from synapse.api.errors import ( from synapse.api.errors import (
AuthError,
Codes, Codes,
NotFoundError, NotFoundError,
StoreError, StoreError,
SynapseError, SynapseError,
UnstableSpecAuthError,
UnsupportedRoomVersionError, UnsupportedRoomVersionError,
) )
from synapse.api.ratelimiting import Ratelimiter from synapse.api.ratelimiting import Ratelimiter
@ -175,10 +175,11 @@ class RoomSummaryHandler:
# First of all, check that the room is accessible. # First of all, check that the room is accessible.
if not await self._is_local_room_accessible(requested_room_id, requester): if not await self._is_local_room_accessible(requested_room_id, requester):
raise AuthError( raise UnstableSpecAuthError(
403, 403,
"User %s not in room %s, and room previews are disabled" "User %s not in room %s, and room previews are disabled"
% (requester, requested_room_id), % (requester, requested_room_id),
errcode=Codes.NOT_JOINED,
) )
# If this is continuing a previous session, pull the persisted data. # If this is continuing a previous session, pull the persisted data.

View File

@ -58,6 +58,7 @@ from synapse.api.errors import (
SynapseError, SynapseError,
UnrecognizedRequestError, UnrecognizedRequestError,
) )
from synapse.config.homeserver import HomeServerConfig
from synapse.http.site import SynapseRequest from synapse.http.site import SynapseRequest
from synapse.logging.context import defer_to_thread, preserve_fn, run_in_background 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 from synapse.logging.opentracing import active_span, start_active_span, trace_servlet
@ -155,15 +156,16 @@ def is_method_cancellable(method: Callable[..., Any]) -> bool:
return getattr(method, "cancellable", False) return getattr(method, "cancellable", False)
def return_json_error(f: failure.Failure, request: SynapseRequest) -> None: def return_json_error(
f: failure.Failure, request: SynapseRequest, config: Optional[HomeServerConfig]
) -> None:
"""Sends a JSON error response to clients.""" """Sends a JSON error response to clients."""
if f.check(SynapseError): if f.check(SynapseError):
# mypy doesn't understand that f.check asserts the type. # mypy doesn't understand that f.check asserts the type.
exc: SynapseError = f.value # type: ignore exc: SynapseError = f.value # type: ignore
error_code = exc.code 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) logger.info("%s SynapseError: %s - %s", request, error_code, exc.msg)
elif f.check(CancelledError): elif f.check(CancelledError):
error_code = HTTP_STATUS_REQUEST_CANCELLED error_code = HTTP_STATUS_REQUEST_CANCELLED
@ -450,7 +452,7 @@ class DirectServeJsonResource(_AsyncResource):
request: SynapseRequest, request: SynapseRequest,
) -> None: ) -> None:
"""Implements _AsyncResource._send_error_response""" """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) @attr.s(slots=True, frozen=True, auto_attribs=True)
@ -575,6 +577,14 @@ class JsonResource(DirectServeJsonResource):
return callback_return 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)
class DirectServeHtmlResource(_AsyncResource): class DirectServeHtmlResource(_AsyncResource):
"""A resource that will call `self._async_on_<METHOD>` on new requests, """A resource that will call `self._async_on_<METHOD>` on new requests,

View File

@ -20,6 +20,7 @@ from twisted.test.proto_helpers import MemoryReactor
from synapse.api.constants import EventTypes, LoginType, Membership from synapse.api.constants import EventTypes, LoginType, Membership
from synapse.api.errors import SynapseError from synapse.api.errors import SynapseError
from synapse.api.room_versions import RoomVersion from synapse.api.room_versions import RoomVersion
from synapse.config.homeserver import HomeServerConfig
from synapse.events import EventBase from synapse.events import EventBase
from synapse.events.third_party_rules import load_legacy_third_party_event_rules from synapse.events.third_party_rules import load_legacy_third_party_event_rules
from synapse.rest import admin from synapse.rest import admin
@ -185,12 +186,12 @@ class ThirdPartyRulesTestCase(unittest.FederatingHomeserverTestCase):
""" """
class NastyHackException(SynapseError): class NastyHackException(SynapseError):
def error_dict(self) -> JsonDict: def error_dict(self, config: Optional[HomeServerConfig]) -> JsonDict:
""" """
This overrides SynapseError's `error_dict` to nastily inject This overrides SynapseError's `error_dict` to nastily inject
JSON into the error response. JSON into the error response.
""" """
result = super().error_dict() result = super().error_dict(config)
result["nasty"] = "very" result["nasty"] = "very"
return result return result