diff --git a/changelog.d/6986.feature b/changelog.d/6986.feature
new file mode 100644
index 0000000000..16dea8bd7f
--- /dev/null
+++ b/changelog.d/6986.feature
@@ -0,0 +1 @@
+Users with a power level sufficient to modify the canonical alias of a room can now delete room aliases.
diff --git a/synapse/api/auth.py b/synapse/api/auth.py
index 5ca18b4301..c1ade1333b 100644
--- a/synapse/api/auth.py
+++ b/synapse/api/auth.py
@@ -539,7 +539,7 @@ class Auth(object):
@defer.inlineCallbacks
def check_can_change_room_list(self, room_id: str, user: UserID):
- """Check if the user is allowed to edit the room's entry in the
+ """Determine whether the user is allowed to edit the room's entry in the
published room list.
Args:
@@ -570,12 +570,7 @@ class Auth(object):
)
user_level = event_auth.get_user_power_level(user_id, auth_events)
- if user_level < send_level:
- raise AuthError(
- 403,
- "This server requires you to be a moderator in the room to"
- " edit its room list entry",
- )
+ return user_level >= send_level
@staticmethod
def has_access_token(request):
diff --git a/synapse/handlers/directory.py b/synapse/handlers/directory.py
index 61eb49059b..1d842c369b 100644
--- a/synapse/handlers/directory.py
+++ b/synapse/handlers/directory.py
@@ -15,7 +15,7 @@
import logging
import string
-from typing import List
+from typing import Iterable, List, Optional
from twisted.internet import defer
@@ -28,6 +28,7 @@ from synapse.api.errors import (
StoreError,
SynapseError,
)
+from synapse.appservice import ApplicationService
from synapse.types import Requester, RoomAlias, UserID, get_domain_from_id
from ._base import BaseHandler
@@ -55,7 +56,13 @@ class DirectoryHandler(BaseHandler):
self.spam_checker = hs.get_spam_checker()
@defer.inlineCallbacks
- def _create_association(self, room_alias, room_id, servers=None, creator=None):
+ def _create_association(
+ self,
+ room_alias: RoomAlias,
+ room_id: str,
+ servers: Optional[Iterable[str]] = None,
+ creator: Optional[str] = None,
+ ):
# general association creation for both human users and app services
for wchar in string.whitespace:
@@ -81,17 +88,21 @@ class DirectoryHandler(BaseHandler):
@defer.inlineCallbacks
def create_association(
- self, requester, room_alias, room_id, servers=None, check_membership=True,
+ self,
+ requester: Requester,
+ room_alias: RoomAlias,
+ room_id: str,
+ servers: Optional[List[str]] = None,
+ check_membership: bool = True,
):
"""Attempt to create a new alias
Args:
- requester (Requester)
- room_alias (RoomAlias)
- room_id (str)
- servers (list[str]|None): List of servers that others servers
- should try and join via
- check_membership (bool): Whether to check if the user is in the room
+ requester
+ room_alias
+ room_id
+ servers: Iterable of servers that others servers should try and join via
+ check_membership: Whether to check if the user is in the room
before the alias can be set (if the server's config requires it).
Returns:
@@ -145,15 +156,15 @@ class DirectoryHandler(BaseHandler):
yield self._create_association(room_alias, room_id, servers, creator=user_id)
@defer.inlineCallbacks
- def delete_association(self, requester, room_alias):
+ def delete_association(self, requester: Requester, room_alias: RoomAlias):
"""Remove an alias from the directory
(this is only meant for human users; AS users should call
delete_appservice_association)
Args:
- requester (Requester):
- room_alias (RoomAlias):
+ requester
+ room_alias
Returns:
Deferred[unicode]: room id that the alias used to point to
@@ -189,16 +200,16 @@ class DirectoryHandler(BaseHandler):
room_id = yield self._delete_association(room_alias)
try:
- yield self._update_canonical_alias(
- requester, requester.user.to_string(), room_id, room_alias
- )
+ yield self._update_canonical_alias(requester, user_id, room_id, room_alias)
except AuthError as e:
logger.info("Failed to update alias events: %s", e)
return room_id
@defer.inlineCallbacks
- def delete_appservice_association(self, service, room_alias):
+ def delete_appservice_association(
+ self, service: ApplicationService, room_alias: RoomAlias
+ ):
if not service.is_interested_in_alias(room_alias.to_string()):
raise SynapseError(
400,
@@ -208,7 +219,7 @@ class DirectoryHandler(BaseHandler):
yield self._delete_association(room_alias)
@defer.inlineCallbacks
- def _delete_association(self, room_alias):
+ def _delete_association(self, room_alias: RoomAlias):
if not self.hs.is_mine(room_alias):
raise SynapseError(400, "Room alias must be local")
@@ -217,7 +228,7 @@ class DirectoryHandler(BaseHandler):
return room_id
@defer.inlineCallbacks
- def get_association(self, room_alias):
+ def get_association(self, room_alias: RoomAlias):
room_id = None
if self.hs.is_mine(room_alias):
result = yield self.get_association_from_room_alias(room_alias)
@@ -282,7 +293,9 @@ class DirectoryHandler(BaseHandler):
)
@defer.inlineCallbacks
- def _update_canonical_alias(self, requester, user_id, room_id, room_alias):
+ def _update_canonical_alias(
+ self, requester: Requester, user_id: str, room_id: str, room_alias: RoomAlias
+ ):
"""
Send an updated canonical alias event if the removed alias was set as
the canonical alias or listed in the alt_aliases field.
@@ -331,7 +344,7 @@ class DirectoryHandler(BaseHandler):
)
@defer.inlineCallbacks
- def get_association_from_room_alias(self, room_alias):
+ def get_association_from_room_alias(self, room_alias: RoomAlias):
result = yield self.store.get_association_from_room_alias(room_alias)
if not result:
# Query AS to see if it exists
@@ -339,7 +352,7 @@ class DirectoryHandler(BaseHandler):
result = yield as_handler.query_room_alias_exists(room_alias)
return result
- def can_modify_alias(self, alias, user_id=None):
+ def can_modify_alias(self, alias: RoomAlias, user_id: Optional[str] = None):
# Any application service "interested" in an alias they are regexing on
# can modify the alias.
# Users can only modify the alias if ALL the interested services have
@@ -360,22 +373,42 @@ class DirectoryHandler(BaseHandler):
return defer.succeed(True)
@defer.inlineCallbacks
- def _user_can_delete_alias(self, alias, user_id):
+ def _user_can_delete_alias(self, alias: RoomAlias, user_id: str):
+ """Determine whether a user can delete an alias.
+
+ One of the following must be true:
+
+ 1. The user created the alias.
+ 2. The user is a server administrator.
+ 3. The user has a power-level sufficient to send a canonical alias event
+ for the current room.
+
+ """
creator = yield self.store.get_room_alias_creator(alias.to_string())
if creator is not None and creator == user_id:
return True
- is_admin = yield self.auth.is_server_admin(UserID.from_string(user_id))
- return is_admin
+ # Resolve the alias to the corresponding room.
+ room_mapping = yield self.get_association(alias)
+ room_id = room_mapping["room_id"]
+ if not room_id:
+ return False
+
+ res = yield self.auth.check_can_change_room_list(
+ room_id, UserID.from_string(user_id)
+ )
+ return res
@defer.inlineCallbacks
- def edit_published_room_list(self, requester, room_id, visibility):
+ def edit_published_room_list(
+ self, requester: Requester, room_id: str, visibility: str
+ ):
"""Edit the entry of the room in the published room list.
requester
- room_id (str)
- visibility (str): "public" or "private"
+ room_id
+ visibility: "public" or "private"
"""
user_id = requester.user.to_string()
@@ -400,7 +433,15 @@ class DirectoryHandler(BaseHandler):
if room is None:
raise SynapseError(400, "Unknown room")
- yield self.auth.check_can_change_room_list(room_id, requester.user)
+ can_change_room_list = yield self.auth.check_can_change_room_list(
+ room_id, requester.user
+ )
+ if not can_change_room_list:
+ raise AuthError(
+ 403,
+ "This server requires you to be a moderator in the room to"
+ " edit its room list entry",
+ )
making_public = visibility == "public"
if making_public:
@@ -421,16 +462,16 @@ class DirectoryHandler(BaseHandler):
@defer.inlineCallbacks
def edit_published_appservice_room_list(
- self, appservice_id, network_id, room_id, visibility
+ self, appservice_id: str, network_id: str, room_id: str, visibility: str
):
"""Add or remove a room from the appservice/network specific public
room list.
Args:
- appservice_id (str): ID of the appservice that owns the list
- network_id (str): The ID of the network the list is associated with
- room_id (str)
- visibility (str): either "public" or "private"
+ appservice_id: ID of the appservice that owns the list
+ network_id: The ID of the network the list is associated with
+ room_id
+ visibility: either "public" or "private"
"""
if visibility not in ["public", "private"]:
raise SynapseError(400, "Invalid visibility setting")
diff --git a/tests/handlers/test_directory.py b/tests/handlers/test_directory.py
index 3397cfa485..5e40adba52 100644
--- a/tests/handlers/test_directory.py
+++ b/tests/handlers/test_directory.py
@@ -18,6 +18,7 @@ from mock import Mock
from twisted.internet import defer
+import synapse
import synapse.api.errors
from synapse.api.constants import EventTypes
from synapse.config.room_directory import RoomDirectoryConfig
@@ -87,52 +88,131 @@ class DirectoryTestCase(unittest.HomeserverTestCase):
ignore_backoff=True,
)
- def test_delete_alias_not_allowed(self):
- """Removing an alias should be denied if a user does not have the proper permissions."""
- room_id = "!8765qwer:test"
+ def test_incoming_fed_query(self):
+ self.get_success(
+ self.store.create_room_alias_association(
+ self.your_room, "!8765asdf:test", ["test"]
+ )
+ )
+
+ response = self.get_success(
+ self.handler.on_directory_query({"room_alias": "#your-room:test"})
+ )
+
+ self.assertEquals({"room_id": "!8765asdf:test", "servers": ["test"]}, response)
+
+
+class TestDeleteAlias(unittest.HomeserverTestCase):
+ servlets = [
+ synapse.rest.admin.register_servlets,
+ login.register_servlets,
+ room.register_servlets,
+ directory.register_servlets,
+ ]
+
+ def prepare(self, reactor, clock, hs):
+ self.store = hs.get_datastore()
+ self.handler = hs.get_handlers().directory_handler
+ self.state_handler = hs.get_state_handler()
+
+ # Create user
+ self.admin_user = self.register_user("admin", "pass", admin=True)
+ self.admin_user_tok = self.login("admin", "pass")
+
+ # Create a test room
+ self.room_id = self.helper.create_room_as(
+ self.admin_user, tok=self.admin_user_tok
+ )
+
+ self.test_alias = "#test:test"
+ self.room_alias = RoomAlias.from_string(self.test_alias)
+
+ # Create a test user.
+ self.test_user = self.register_user("user", "pass", admin=False)
+ self.test_user_tok = self.login("user", "pass")
+ self.helper.join(room=self.room_id, user=self.test_user, tok=self.test_user_tok)
+
+ def _create_alias(self, user):
+ # Create a new alias to this room.
self.get_success(
- self.store.create_room_alias_association(self.my_room, room_id, ["test"])
+ self.store.create_room_alias_association(
+ self.room_alias, self.room_id, ["test"], user
+ )
)
+ def test_delete_alias_not_allowed(self):
+ """A user that doesn't meet the expected guidelines cannot delete an alias."""
+ self._create_alias(self.admin_user)
self.get_failure(
self.handler.delete_association(
- create_requester("@user:test"), self.my_room
+ create_requester(self.test_user), self.room_alias
),
synapse.api.errors.AuthError,
)
- def test_delete_alias(self):
- """Removing an alias should work when a user does has the proper permissions."""
- room_id = "!8765qwer:test"
- user_id = "@user:test"
- self.get_success(
- self.store.create_room_alias_association(
- self.my_room, room_id, ["test"], user_id
+ def test_delete_alias_creator(self):
+ """An alias creator can delete their own alias."""
+ # Create an alias from a different user.
+ self._create_alias(self.test_user)
+
+ # Delete the user's alias.
+ result = self.get_success(
+ self.handler.delete_association(
+ create_requester(self.test_user), self.room_alias
)
)
+ self.assertEquals(self.room_id, result)
+ # Confirm the alias is gone.
+ self.get_failure(
+ self.handler.get_association(self.room_alias),
+ synapse.api.errors.SynapseError,
+ )
+
+ def test_delete_alias_admin(self):
+ """A server admin can delete an alias created by another user."""
+ # Create an alias from a different user.
+ self._create_alias(self.test_user)
+
+ # Delete the user's alias as the admin.
result = self.get_success(
- self.handler.delete_association(create_requester(user_id), self.my_room)
+ self.handler.delete_association(
+ create_requester(self.admin_user), self.room_alias
+ )
)
- self.assertEquals(room_id, result)
+ self.assertEquals(self.room_id, result)
- # The alias should not be found.
+ # Confirm the alias is gone.
self.get_failure(
- self.handler.get_association(self.my_room), synapse.api.errors.SynapseError
+ self.handler.get_association(self.room_alias),
+ synapse.api.errors.SynapseError,
)
- def test_incoming_fed_query(self):
- self.get_success(
- self.store.create_room_alias_association(
- self.your_room, "!8765asdf:test", ["test"]
- )
+ def test_delete_alias_sufficient_power(self):
+ """A user with a sufficient power level should be able to delete an alias."""
+ self._create_alias(self.admin_user)
+
+ # Increase the user's power level.
+ self.helper.send_state(
+ self.room_id,
+ "m.room.power_levels",
+ {"users": {self.test_user: 100}},
+ tok=self.admin_user_tok,
)
- response = self.get_success(
- self.handler.on_directory_query({"room_alias": "#your-room:test"})
+ # They can now delete the alias.
+ result = self.get_success(
+ self.handler.delete_association(
+ create_requester(self.test_user), self.room_alias
+ )
)
+ self.assertEquals(self.room_id, result)
- self.assertEquals({"room_id": "!8765asdf:test", "servers": ["test"]}, response)
+ # Confirm the alias is gone.
+ self.get_failure(
+ self.handler.get_association(self.room_alias),
+ synapse.api.errors.SynapseError,
+ )
class CanonicalAliasTestCase(unittest.HomeserverTestCase):
diff --git a/tox.ini b/tox.ini
index 097ebb8774..7622aa19f1 100644
--- a/tox.ini
+++ b/tox.ini
@@ -185,6 +185,7 @@ commands = mypy \
synapse/federation/federation_client.py \
synapse/federation/sender \
synapse/federation/transport \
+ synapse/handlers/directory.py \
synapse/handlers/presence.py \
synapse/handlers/sync.py \
synapse/handlers/ui_auth \
|
