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.

2 files changed, 33 insertions, 4 deletions

diff --git a/synapse/handlers/directory.py b/synapse/handlers/directory.py

index 50c587aa61..a12f9508d8 100644
--- a/synapse/handlers/directory.py
+++ b/synapse/handlers/directory.py
@@ -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,
             )
 
diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py

index 224d34ef3a..e5afeadf68 100644
--- a/synapse/handlers/message.py
+++ b/synapse/handlers/message.py
@@ -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))