Do checks on aliases for incoming m.room.aliases events (#5128)
Follow-up to #5124 Also added a bunch of checks to make sure everything (both the stuff added on #5124 and this PR) works as intended.
This commit is contained in:
parent
de655e669a
commit
1473058b5e
|
@ -0,0 +1 @@
|
|||
Add some missing limitations to room alias creation.
|
|
@ -20,6 +20,9 @@
|
|||
# the "depth" field on events is limited to 2**63 - 1
|
||||
MAX_DEPTH = 2**63 - 1
|
||||
|
||||
# the maximum length for a room alias is 255 characters
|
||||
MAX_ALIAS_LENGTH = 255
|
||||
|
||||
|
||||
class Membership(object):
|
||||
|
||||
|
|
|
@ -188,6 +188,8 @@ class EventContext(object):
|
|||
Returns:
|
||||
Deferred[dict[(str, str), str]|None]: Returns None if state_group
|
||||
is None, which happens when the associated event is an outlier.
|
||||
Maps a (type, state_key) to the event ID of the state event matching
|
||||
this tuple.
|
||||
"""
|
||||
|
||||
if not self._fetching_state_deferred:
|
||||
|
@ -206,6 +208,8 @@ class EventContext(object):
|
|||
Returns:
|
||||
Deferred[dict[(str, str), str]|None]: Returns None if state_group
|
||||
is None, which happens when the associated event is an outlier.
|
||||
Maps a (type, state_key) to the event ID of the state event matching
|
||||
this tuple.
|
||||
"""
|
||||
|
||||
if not self._fetching_state_deferred:
|
||||
|
|
|
@ -15,8 +15,8 @@
|
|||
|
||||
from six import string_types
|
||||
|
||||
from synapse.api.constants import EventTypes, Membership
|
||||
from synapse.api.errors import SynapseError
|
||||
from synapse.api.constants import MAX_ALIAS_LENGTH, EventTypes, Membership
|
||||
from synapse.api.errors import Codes, SynapseError
|
||||
from synapse.api.room_versions import EventFormatVersions
|
||||
from synapse.types import EventID, RoomID, UserID
|
||||
|
||||
|
@ -56,6 +56,17 @@ class EventValidator(object):
|
|||
if not isinstance(getattr(event, s), string_types):
|
||||
raise SynapseError(400, "'%s' not a string type" % (s,))
|
||||
|
||||
if event.type == EventTypes.Aliases:
|
||||
if "aliases" in event.content:
|
||||
for alias in event.content["aliases"]:
|
||||
if len(alias) > MAX_ALIAS_LENGTH:
|
||||
raise SynapseError(
|
||||
400,
|
||||
("Can't create aliases longer than"
|
||||
" %d characters" % (MAX_ALIAS_LENGTH,)),
|
||||
Codes.INVALID_PARAM,
|
||||
)
|
||||
|
||||
def validate_builder(self, event):
|
||||
"""Validates that the builder/event has roughly the right format. Only
|
||||
checks values that we expect a proto event to have, rather than all the
|
||||
|
|
|
@ -19,7 +19,7 @@ import string
|
|||
|
||||
from twisted.internet import defer
|
||||
|
||||
from synapse.api.constants import EventTypes
|
||||
from synapse.api.constants import MAX_ALIAS_LENGTH, EventTypes
|
||||
from synapse.api.errors import (
|
||||
AuthError,
|
||||
CodeMessageException,
|
||||
|
@ -36,7 +36,6 @@ logger = logging.getLogger(__name__)
|
|||
|
||||
|
||||
class DirectoryHandler(BaseHandler):
|
||||
MAX_ALIAS_LENGTH = 255
|
||||
|
||||
def __init__(self, hs):
|
||||
super(DirectoryHandler, self).__init__(hs)
|
||||
|
@ -105,10 +104,10 @@ class DirectoryHandler(BaseHandler):
|
|||
|
||||
user_id = requester.user.to_string()
|
||||
|
||||
if len(room_alias.to_string()) > self.MAX_ALIAS_LENGTH:
|
||||
if len(room_alias.to_string()) > MAX_ALIAS_LENGTH:
|
||||
raise SynapseError(
|
||||
400,
|
||||
"Can't create aliases longer than %s characters" % self.MAX_ALIAS_LENGTH,
|
||||
"Can't create aliases longer than %s characters" % MAX_ALIAS_LENGTH,
|
||||
Codes.INVALID_PARAM,
|
||||
)
|
||||
|
||||
|
|
|
@ -228,6 +228,7 @@ class EventCreationHandler(object):
|
|||
self.ratelimiter = hs.get_ratelimiter()
|
||||
self.notifier = hs.get_notifier()
|
||||
self.config = hs.config
|
||||
self.require_membership_for_aliases = hs.config.require_membership_for_aliases
|
||||
|
||||
self.send_event_to_master = ReplicationSendEventRestServlet.make_client(hs)
|
||||
|
||||
|
@ -336,6 +337,35 @@ class EventCreationHandler(object):
|
|||
prev_events_and_hashes=prev_events_and_hashes,
|
||||
)
|
||||
|
||||
# In an ideal world we wouldn't need the second part of this condition. However,
|
||||
# this behaviour isn't spec'd yet, meaning we should be able to deactivate this
|
||||
# behaviour. Another reason is that this code is also evaluated each time a new
|
||||
# m.room.aliases event is created, which includes hitting a /directory route.
|
||||
# Therefore not including this condition here would render the similar one in
|
||||
# synapse.handlers.directory pointless.
|
||||
if builder.type == EventTypes.Aliases and self.require_membership_for_aliases:
|
||||
# Ideally we'd do the membership check in event_auth.check(), which
|
||||
# describes a spec'd algorithm for authenticating events received over
|
||||
# federation as well as those created locally. As of room v3, aliases events
|
||||
# can be created by users that are not in the room, therefore we have to
|
||||
# tolerate them in event_auth.check().
|
||||
prev_state_ids = yield context.get_prev_state_ids(self.store)
|
||||
prev_event_id = prev_state_ids.get((EventTypes.Member, event.sender))
|
||||
prev_event = yield self.store.get_event(prev_event_id, allow_none=True)
|
||||
if not prev_event or prev_event.membership != Membership.JOIN:
|
||||
logger.warning(
|
||||
("Attempt to send `m.room.aliases` in room %s by user %s but"
|
||||
" membership is %s"),
|
||||
event.room_id,
|
||||
event.sender,
|
||||
prev_event.membership if prev_event else None,
|
||||
)
|
||||
|
||||
raise AuthError(
|
||||
403,
|
||||
"You must be in the room to create an alias for it",
|
||||
)
|
||||
|
||||
self.validator.validate_new(event)
|
||||
|
||||
defer.returnValue((event, context))
|
||||
|
|
|
@ -0,0 +1,169 @@
|
|||
# -*- coding: utf-8 -*-
|
||||
# Copyright 2019 New Vector Ltd
|
||||
#
|
||||
# Licensed under the Apache License, Version 2.0 (the "License");
|
||||
# you may not use this file except in compliance with the License.
|
||||
# You may obtain a copy of the License at
|
||||
#
|
||||
# http://www.apache.org/licenses/LICENSE-2.0
|
||||
#
|
||||
# Unless required by applicable law or agreed to in writing, software
|
||||
# distributed under the License is distributed on an "AS IS" BASIS,
|
||||
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||
# See the License for the specific language governing permissions and
|
||||
# limitations under the License.
|
||||
|
||||
import json
|
||||
|
||||
from synapse.rest.admin import register_servlets
|
||||
from synapse.rest.client.v1 import directory, login, room
|
||||
from synapse.types import RoomAlias
|
||||
from synapse.util.stringutils import random_string
|
||||
|
||||
from tests import unittest
|
||||
|
||||
|
||||
class DirectoryTestCase(unittest.HomeserverTestCase):
|
||||
|
||||
servlets = [
|
||||
register_servlets,
|
||||
directory.register_servlets,
|
||||
login.register_servlets,
|
||||
room.register_servlets,
|
||||
]
|
||||
|
||||
def make_homeserver(self, reactor, clock):
|
||||
config = self.default_config()
|
||||
config.require_membership_for_aliases = True
|
||||
|
||||
self.hs = self.setup_test_homeserver(config=config)
|
||||
|
||||
return self.hs
|
||||
|
||||
def prepare(self, reactor, clock, homeserver):
|
||||
self.room_owner = self.register_user("room_owner", "test")
|
||||
self.room_owner_tok = self.login("room_owner", "test")
|
||||
|
||||
self.room_id = self.helper.create_room_as(
|
||||
self.room_owner, tok=self.room_owner_tok,
|
||||
)
|
||||
|
||||
self.user = self.register_user("user", "test")
|
||||
self.user_tok = self.login("user", "test")
|
||||
|
||||
def test_state_event_not_in_room(self):
|
||||
self.ensure_user_left_room()
|
||||
self.set_alias_via_state_event(403)
|
||||
|
||||
def test_directory_endpoint_not_in_room(self):
|
||||
self.ensure_user_left_room()
|
||||
self.set_alias_via_directory(403)
|
||||
|
||||
def test_state_event_in_room_too_long(self):
|
||||
self.ensure_user_joined_room()
|
||||
self.set_alias_via_state_event(400, alias_length=256)
|
||||
|
||||
def test_directory_in_room_too_long(self):
|
||||
self.ensure_user_joined_room()
|
||||
self.set_alias_via_directory(400, alias_length=256)
|
||||
|
||||
def test_state_event_in_room(self):
|
||||
self.ensure_user_joined_room()
|
||||
self.set_alias_via_state_event(200)
|
||||
|
||||
def test_directory_in_room(self):
|
||||
self.ensure_user_joined_room()
|
||||
self.set_alias_via_directory(200)
|
||||
|
||||
def test_room_creation_too_long(self):
|
||||
url = "/_matrix/client/r0/createRoom"
|
||||
|
||||
# We use deliberately a localpart under the length threshold so
|
||||
# that we can make sure that the check is done on the whole alias.
|
||||
data = {
|
||||
"room_alias_name": random_string(256 - len(self.hs.hostname)),
|
||||
}
|
||||
request_data = json.dumps(data)
|
||||
request, channel = self.make_request(
|
||||
"POST", url, request_data, access_token=self.user_tok,
|
||||
)
|
||||
self.render(request)
|
||||
self.assertEqual(channel.code, 400, channel.result)
|
||||
|
||||
def test_room_creation(self):
|
||||
url = "/_matrix/client/r0/createRoom"
|
||||
|
||||
# Check with an alias of allowed length. There should already be
|
||||
# a test that ensures it works in test_register.py, but let's be
|
||||
# as cautious as possible here.
|
||||
data = {
|
||||
"room_alias_name": random_string(5),
|
||||
}
|
||||
request_data = json.dumps(data)
|
||||
request, channel = self.make_request(
|
||||
"POST", url, request_data, access_token=self.user_tok,
|
||||
)
|
||||
self.render(request)
|
||||
self.assertEqual(channel.code, 200, channel.result)
|
||||
|
||||
def set_alias_via_state_event(self, expected_code, alias_length=5):
|
||||
url = ("/_matrix/client/r0/rooms/%s/state/m.room.aliases/%s"
|
||||
% (self.room_id, self.hs.hostname))
|
||||
|
||||
data = {
|
||||
"aliases": [
|
||||
self.random_alias(alias_length),
|
||||
],
|
||||
}
|
||||
request_data = json.dumps(data)
|
||||
|
||||
request, channel = self.make_request(
|
||||
"PUT", url, request_data, access_token=self.user_tok,
|
||||
)
|
||||
self.render(request)
|
||||
self.assertEqual(channel.code, expected_code, channel.result)
|
||||
|
||||
def set_alias_via_directory(self, expected_code, alias_length=5):
|
||||
url = "/_matrix/client/r0/directory/room/%s" % self.random_alias(alias_length)
|
||||
data = {
|
||||
"room_id": self.room_id,
|
||||
}
|
||||
request_data = json.dumps(data)
|
||||
|
||||
request, channel = self.make_request(
|
||||
"PUT", url, request_data, access_token=self.user_tok,
|
||||
)
|
||||
self.render(request)
|
||||
self.assertEqual(channel.code, expected_code, channel.result)
|
||||
|
||||
def random_alias(self, length):
|
||||
return RoomAlias(
|
||||
random_string(length),
|
||||
self.hs.hostname,
|
||||
).to_string()
|
||||
|
||||
def ensure_user_left_room(self):
|
||||
self.ensure_membership("leave")
|
||||
|
||||
def ensure_user_joined_room(self):
|
||||
self.ensure_membership("join")
|
||||
|
||||
def ensure_membership(self, membership):
|
||||
try:
|
||||
if membership == "leave":
|
||||
self.helper.leave(
|
||||
room=self.room_id,
|
||||
user=self.user,
|
||||
tok=self.user_tok,
|
||||
)
|
||||
if membership == "join":
|
||||
self.helper.join(
|
||||
room=self.room_id,
|
||||
user=self.user,
|
||||
tok=self.user_tok,
|
||||
)
|
||||
except AssertionError:
|
||||
# We don't care whether the leave request didn't return a 200 (e.g.
|
||||
# if the user isn't already in the room), because we only want to
|
||||
# make sure the user isn't in the room.
|
||||
pass
|
Loading…
Reference in New Issue