From e8b1c3ec8169952be0baa3f4a0a2c42a2770dcb5 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 2 May 2019 16:32:34 +0100 Subject: [PATCH 01/13] Move constant to constants.py --- synapse/api/constants.py | 2 ++ synapse/handlers/directory.py | 7 +++---- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/synapse/api/constants.py b/synapse/api/constants.py index 0860b7590532..eaecb8e55060 100644 --- a/synapse/api/constants.py +++ b/synapse/api/constants.py @@ -20,6 +20,8 @@ # 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): diff --git a/synapse/handlers/directory.py b/synapse/handlers/directory.py index 50c587aa6196..a9dcab99e5b9 100644 --- a/synapse/handlers/directory.py +++ b/synapse/handlers/directory.py @@ -19,7 +19,7 @@ from twisted.internet import defer -from synapse.api.constants import EventTypes +from synapse.api.constants import EventTypes, MAX_ALIAS_LENGTH from synapse.api.errors import ( AuthError, CodeMessageException, @@ -36,7 +36,6 @@ class DirectoryHandler(BaseHandler): - MAX_ALIAS_LENGTH = 255 def __init__(self, hs): super(DirectoryHandler, self).__init__(hs) @@ -105,10 +104,10 @@ def create_association(self, requester, room_alias, room_id, servers=None, 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, ) From 5fa7496dd7fec48f1a409375dda1bce2a51fbeb9 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 2 May 2019 16:32:54 +0100 Subject: [PATCH 02/13] Add checks to incoming m.room.aliases events --- synapse/handlers/message.py | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py index 224d34ef3ac8..5a043b38d62d 100644 --- a/synapse/handlers/message.py +++ b/synapse/handlers/message.py @@ -22,7 +22,7 @@ from twisted.internet import defer from twisted.internet.defer import succeed -from synapse.api.constants import EventTypes, Membership +from synapse.api.constants import EventTypes, MAX_ALIAS_LENGTH, Membership from synapse.api.errors import ( AuthError, Codes, @@ -228,6 +228,7 @@ def __init__(self, hs): 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) @@ -320,6 +321,26 @@ def create_event(self, requester, event_dict, token_id=None, txn_id=None, target, e ) + if builder.type == EventTypes.Aliases: + if "aliases" in builder.content: + for alias in builder.content["aliases"]: + if len(alias) > MAX_ALIAS_LENGTH: + raise SynapseError( + 400, + ("Can't create aliases longer than" + " %s characters" % MAX_ALIAS_LENGTH), + Codes.INVALID_PARAM, + ) + + if self.require_membership_for_aliases: + rooms_for_user = yield self.store.get_rooms_for_user(builder.sender) + if builder.room_id not in rooms_for_user: + raise AuthError( + 403, + "You must be in the room to create an alias for it", + ) + + is_exempt = yield self._is_exempt_from_privacy_policy(builder, requester) if require_consent and not is_exempt: yield self.assert_accepted_privacy_policy(requester) From 74ce5f86d4a26c87c9584edaaf75d891b30e49e7 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 2 May 2019 16:33:16 +0100 Subject: [PATCH 03/13] Add tests --- tests/rest/client/v1/test_directory.py | 168 +++++++++++++++++++++++++ 1 file changed, 168 insertions(+) create mode 100644 tests/rest/client/v1/test_directory.py diff --git a/tests/rest/client/v1/test_directory.py b/tests/rest/client/v1/test_directory.py new file mode 100644 index 000000000000..cd6f3f604cff --- /dev/null +++ b/tests/rest/client/v1/test_directory.py @@ -0,0 +1,168 @@ +# -*- 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.client.v1 import admin, directory, login, room +from synapse.util.stringutils import random_string +from synapse.types import RoomAlias + +from tests import unittest + + +class DirectoryTestCase(unittest.HomeserverTestCase): + + servlets = [ + admin.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 \ No newline at end of file From 4f5089315732097e5b50bd306e53ccc72fa7241f Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 2 May 2019 16:38:57 +0100 Subject: [PATCH 04/13] Lint + changelog --- changelog.d/5128.bugfix | 1 + synapse/handlers/directory.py | 2 +- synapse/handlers/message.py | 2 +- tests/rest/client/v1/test_directory.py | 6 +++--- 4 files changed, 6 insertions(+), 5 deletions(-) create mode 100644 changelog.d/5128.bugfix diff --git a/changelog.d/5128.bugfix b/changelog.d/5128.bugfix new file mode 100644 index 000000000000..46df1e9fd550 --- /dev/null +++ b/changelog.d/5128.bugfix @@ -0,0 +1 @@ +Add some missing limitations to room alias creation. diff --git a/synapse/handlers/directory.py b/synapse/handlers/directory.py index a9dcab99e5b9..a12f9508d879 100644 --- a/synapse/handlers/directory.py +++ b/synapse/handlers/directory.py @@ -19,7 +19,7 @@ from twisted.internet import defer -from synapse.api.constants import EventTypes, MAX_ALIAS_LENGTH +from synapse.api.constants import MAX_ALIAS_LENGTH, EventTypes from synapse.api.errors import ( AuthError, CodeMessageException, diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py index 5a043b38d62d..592e6435749d 100644 --- a/synapse/handlers/message.py +++ b/synapse/handlers/message.py @@ -22,7 +22,7 @@ from twisted.internet import defer from twisted.internet.defer import succeed -from synapse.api.constants import EventTypes, MAX_ALIAS_LENGTH, Membership +from synapse.api.constants import MAX_ALIAS_LENGTH, EventTypes, Membership from synapse.api.errors import ( AuthError, Codes, diff --git a/tests/rest/client/v1/test_directory.py b/tests/rest/client/v1/test_directory.py index cd6f3f604cff..f30a57e82f8b 100644 --- a/tests/rest/client/v1/test_directory.py +++ b/tests/rest/client/v1/test_directory.py @@ -16,8 +16,8 @@ import json from synapse.rest.client.v1 import admin, directory, login, room -from synapse.util.stringutils import random_string from synapse.types import RoomAlias +from synapse.util.stringutils import random_string from tests import unittest @@ -80,7 +80,7 @@ def test_room_creation_too_long(self): # 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)), + "room_alias_name": random_string(256 - len(self.hs.hostname)), } request_data = json.dumps(data) request, channel = self.make_request( @@ -165,4 +165,4 @@ def ensure_membership(self, membership): # 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 \ No newline at end of file + pass From 29cd740ac67e3e269403929129af2a4a5bf67a74 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 2 May 2019 16:41:31 +0100 Subject: [PATCH 05/13] Lint --- synapse/api/constants.py | 1 + synapse/handlers/message.py | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/api/constants.py b/synapse/api/constants.py index eaecb8e55060..8547a635350a 100644 --- a/synapse/api/constants.py +++ b/synapse/api/constants.py @@ -23,6 +23,7 @@ # the maximum length for a room alias is 255 characters MAX_ALIAS_LENGTH = 255 + class Membership(object): """Represents the membership states of a user in a room.""" diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py index 592e6435749d..2e77e01e9f63 100644 --- a/synapse/handlers/message.py +++ b/synapse/handlers/message.py @@ -340,7 +340,6 @@ def create_event(self, requester, event_dict, token_id=None, txn_id=None, "You must be in the room to create an alias for it", ) - is_exempt = yield self._is_exempt_from_privacy_policy(builder, requester) if require_consent and not is_exempt: yield self.assert_accepted_privacy_policy(requester) From 69c5288fd68afabcf5f3df9ba1e3202ecb719d15 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Fri, 3 May 2019 11:59:01 +0100 Subject: [PATCH 06/13] Update docstring Make docstring clearer about what's returned by context.get_current_state_ids and context.get_prev_state_ids --- synapse/events/snapshot.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/synapse/events/snapshot.py b/synapse/events/snapshot.py index 368b5f6ae449..fa09c132a0a1 100644 --- a/synapse/events/snapshot.py +++ b/synapse/events/snapshot.py @@ -187,7 +187,9 @@ def get_current_state_ids(self, store): Returns: Deferred[dict[(str, str), str]|None]: Returns None if state_group - is None, which happens when the associated event is an outlier. + 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: @@ -205,7 +207,9 @@ def get_prev_state_ids(self, store): Returns: Deferred[dict[(str, str), str]|None]: Returns None if state_group - is None, which happens when the associated event is an outlier. + 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: From 7b32c15689f487bccd7d1b766f1c6ddc72ecf633 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Fri, 3 May 2019 12:03:14 +0100 Subject: [PATCH 07/13] Update import for admin servlets --- tests/rest/client/v1/test_directory.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/rest/client/v1/test_directory.py b/tests/rest/client/v1/test_directory.py index f30a57e82f8b..4804000ec7ac 100644 --- a/tests/rest/client/v1/test_directory.py +++ b/tests/rest/client/v1/test_directory.py @@ -15,7 +15,8 @@ import json -from synapse.rest.client.v1 import admin, directory, login, room +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 @@ -25,7 +26,7 @@ class DirectoryTestCase(unittest.HomeserverTestCase): servlets = [ - admin.register_servlets, + register_servlets, directory.register_servlets, login.register_servlets, room.register_servlets, From dcd2e60f7596d932c0a289253bb2d45551cb2613 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Fri, 3 May 2019 12:34:45 +0100 Subject: [PATCH 08/13] Incorporate review --- synapse/events/validator.py | 15 +++++++++++++-- synapse/handlers/message.py | 33 ++++++++++++++------------------- 2 files changed, 27 insertions(+), 21 deletions(-) diff --git a/synapse/events/validator.py b/synapse/events/validator.py index 514273c79204..3efe9877ad54 100644 --- a/synapse/events/validator.py +++ b/synapse/events/validator.py @@ -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 @@ def validate_new(self, event): 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" + " %s 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 diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py index 2e77e01e9f63..90df4f4c8376 100644 --- a/synapse/handlers/message.py +++ b/synapse/handlers/message.py @@ -321,25 +321,6 @@ def create_event(self, requester, event_dict, token_id=None, txn_id=None, target, e ) - if builder.type == EventTypes.Aliases: - if "aliases" in builder.content: - for alias in builder.content["aliases"]: - if len(alias) > MAX_ALIAS_LENGTH: - raise SynapseError( - 400, - ("Can't create aliases longer than" - " %s characters" % MAX_ALIAS_LENGTH), - Codes.INVALID_PARAM, - ) - - if self.require_membership_for_aliases: - rooms_for_user = yield self.store.get_rooms_for_user(builder.sender) - if builder.room_id not in rooms_for_user: - raise AuthError( - 403, - "You must be in the room to create an alias for it", - ) - is_exempt = yield self._is_exempt_from_privacy_policy(builder, requester) if require_consent and not is_exempt: yield self.assert_accepted_privacy_policy(requester) @@ -356,6 +337,20 @@ def create_event(self, requester, event_dict, token_id=None, txn_id=None, prev_events_and_hashes=prev_events_and_hashes, ) + if builder.type == EventTypes.Aliases and self.require_membership_for_aliases: + # Ideally we'd do this check in event_auth.check(), however this function + # describes a spec'd algorithm and this limitation is lacking from the spec, + # so we're doing it outside of check(). When/if this additional constraint + # gets added to the spec, this check should be moved to 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: + raise AuthError( + 403, + "You must be in the room to create an alias for it", + ) + self.validator.validate_new(event) defer.returnValue((event, context)) From 77b6024e1769718087b65a205e5571b38d5de94c Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Fri, 3 May 2019 12:36:14 +0100 Subject: [PATCH 09/13] Remove condition to see what happens --- synapse/handlers/message.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py index 90df4f4c8376..4038ff7f6e89 100644 --- a/synapse/handlers/message.py +++ b/synapse/handlers/message.py @@ -337,7 +337,7 @@ def create_event(self, requester, event_dict, token_id=None, txn_id=None, prev_events_and_hashes=prev_events_and_hashes, ) - if builder.type == EventTypes.Aliases and self.require_membership_for_aliases: + if builder.type == EventTypes.Aliases: # Ideally we'd do this check in event_auth.check(), however this function # describes a spec'd algorithm and this limitation is lacking from the spec, # so we're doing it outside of check(). When/if this additional constraint From 630f7810fad5688828372568a42ef1bd0223d6a5 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Fri, 3 May 2019 13:52:06 +0100 Subject: [PATCH 10/13] lint --- synapse/handlers/message.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py index 4038ff7f6e89..20230a7ea4c3 100644 --- a/synapse/handlers/message.py +++ b/synapse/handlers/message.py @@ -22,7 +22,7 @@ from twisted.internet import defer from twisted.internet.defer import succeed -from synapse.api.constants import MAX_ALIAS_LENGTH, EventTypes, Membership +from synapse.api.constants import EventTypes, Membership from synapse.api.errors import ( AuthError, Codes, From c67d4d7f1537c4d6755309307721b6f6d1bdac32 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Fri, 3 May 2019 13:52:56 +0100 Subject: [PATCH 11/13] Re-add config check --- synapse/handlers/message.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py index 20230a7ea4c3..0f841bebdbef 100644 --- a/synapse/handlers/message.py +++ b/synapse/handlers/message.py @@ -337,7 +337,7 @@ def create_event(self, requester, event_dict, token_id=None, txn_id=None, prev_events_and_hashes=prev_events_and_hashes, ) - if builder.type == EventTypes.Aliases: + if builder.type == EventTypes.Aliases and self.require_membership_for_aliases: # Ideally we'd do this check in event_auth.check(), however this function # describes a spec'd algorithm and this limitation is lacking from the spec, # so we're doing it outside of check(). When/if this additional constraint From e910c29444c1f491a111a9bd05c0a2fe046e5460 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Fri, 3 May 2019 14:01:34 +0100 Subject: [PATCH 12/13] Doc --- synapse/handlers/message.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py index 0f841bebdbef..af241c2afdfb 100644 --- a/synapse/handlers/message.py +++ b/synapse/handlers/message.py @@ -337,6 +337,11 @@ def create_event(self, requester, event_dict, token_id=None, txn_id=None, 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, and this code is evaluated each time a new m.room.aliases event is + # created, which includes hitting a /directory route, therefore not including it + # would render the similar check in synapse.handlers.directory pointless. if builder.type == EventTypes.Aliases and self.require_membership_for_aliases: # Ideally we'd do this check in event_auth.check(), however this function # describes a spec'd algorithm and this limitation is lacking from the spec, From 89e7efdaa3d8df03e503100b4eaeaeffbbfb4a2e Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Wed, 8 May 2019 16:16:17 +0100 Subject: [PATCH 13/13] Incorporate review --- synapse/events/validator.py | 2 +- synapse/handlers/message.py | 24 +++++++++++++++++------- 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/synapse/events/validator.py b/synapse/events/validator.py index 3efe9877ad54..711af512b27e 100644 --- a/synapse/events/validator.py +++ b/synapse/events/validator.py @@ -63,7 +63,7 @@ def validate_new(self, event): raise SynapseError( 400, ("Can't create aliases longer than" - " %s characters" % MAX_ALIAS_LENGTH), + " %d characters" % (MAX_ALIAS_LENGTH,)), Codes.INVALID_PARAM, ) diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py index af241c2afdfb..e5afeadf68e1 100644 --- a/synapse/handlers/message.py +++ b/synapse/handlers/message.py @@ -339,18 +339,28 @@ def create_event(self, requester, event_dict, token_id=None, txn_id=None, # 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, and this code is evaluated each time a new m.room.aliases event is - # created, which includes hitting a /directory route, therefore not including it - # would render the similar check in synapse.handlers.directory pointless. + # 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 this check in event_auth.check(), however this function - # describes a spec'd algorithm and this limitation is lacking from the spec, - # so we're doing it outside of check(). When/if this additional constraint - # gets added to the spec, this check should be moved to event_auth.check(). + # 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",