diff --git a/UPGRADE.rst b/UPGRADE.rst
index d869b7111b..228222d534 100644
--- a/UPGRADE.rst
+++ b/UPGRADE.rst
@@ -39,7 +39,7 @@ instructions that may be required are listed later in this document.
./synctl restart
-To check whether your update was sucessful, you can check the Server header
+To check whether your update was successful, you can check the Server header
returned by the Client-Server API:
.. code:: bash
diff --git a/changelog.d/4643.misc b/changelog.d/4643.misc
new file mode 100644
index 0000000000..556cdd2240
--- /dev/null
+++ b/changelog.d/4643.misc
@@ -0,0 +1 @@
+Reduce number of exceptions we log
diff --git a/changelog.d/4647.feature b/changelog.d/4647.feature
new file mode 100644
index 0000000000..5a5b1dcebb
--- /dev/null
+++ b/changelog.d/4647.feature
@@ -0,0 +1 @@
+Add configurable room list publishing rules
diff --git a/changelog.d/4657.misc b/changelog.d/4657.misc
new file mode 100644
index 0000000000..8872765819
--- /dev/null
+++ b/changelog.d/4657.misc
@@ -0,0 +1 @@
+Fix various spelling mistakes.
diff --git a/synapse/config/key.py b/synapse/config/key.py
index dce4b19a2d..499ffd4e06 100644
--- a/synapse/config/key.py
+++ b/synapse/config/key.py
@@ -56,7 +56,7 @@ class KeyConfig(Config):
if not self.macaroon_secret_key:
# Unfortunately, there are people out there that don't have this
# set. Lets just be "nice" and derive one from their secret key.
- logger.warn("Config is missing missing macaroon_secret_key")
+ logger.warn("Config is missing macaroon_secret_key")
seed = bytes(self.signing_key[0])
self.macaroon_secret_key = hashlib.sha256(seed).digest()
diff --git a/synapse/config/room_directory.py b/synapse/config/room_directory.py
index 9da13ab11b..c8e0abbae7 100644
--- a/synapse/config/room_directory.py
+++ b/synapse/config/room_directory.py
@@ -20,12 +20,37 @@ from ._base import Config, ConfigError
class RoomDirectoryConfig(Config):
def read_config(self, config):
- alias_creation_rules = config["alias_creation_rules"]
+ alias_creation_rules = config.get("alias_creation_rules")
- self._alias_creation_rules = [
- _AliasRule(rule)
- for rule in alias_creation_rules
- ]
+ if alias_creation_rules is not None:
+ self._alias_creation_rules = [
+ _RoomDirectoryRule("alias_creation_rules", rule)
+ for rule in alias_creation_rules
+ ]
+ else:
+ self._alias_creation_rules = [
+ _RoomDirectoryRule(
+ "alias_creation_rules", {
+ "action": "allow",
+ }
+ )
+ ]
+
+ room_list_publication_rules = config.get("room_list_publication_rules")
+
+ if room_list_publication_rules is not None:
+ self._room_list_publication_rules = [
+ _RoomDirectoryRule("room_list_publication_rules", rule)
+ for rule in room_list_publication_rules
+ ]
+ else:
+ self._room_list_publication_rules = [
+ _RoomDirectoryRule(
+ "room_list_publication_rules", {
+ "action": "allow",
+ }
+ )
+ ]
def default_config(self, config_dir_path, server_name, **kwargs):
return """
@@ -33,60 +58,138 @@ class RoomDirectoryConfig(Config):
# on this server.
#
# The format of this option is a list of rules that contain globs that
- # match against user_id and the new alias (fully qualified with server
- # name). The action in the first rule that matches is taken, which can
- # currently either be "allow" or "deny".
+ # match against user_id, room_id and the new alias (fully qualified with
+ # server name). The action in the first rule that matches is taken,
+ # which can currently either be "allow" or "deny".
+ #
+ # Missing user_id/room_id/alias fields default to "*".
+ #
+ # If no rules match the request is denied. An empty list means no one
+ # can create aliases.
+ #
+ # Options for the rules include:
+ #
+ # user_id: Matches against the creator of the alias
+ # alias: Matches against the alias being created
+ # room_id: Matches against the room ID the alias is being pointed at
+ # action: Whether to "allow" or "deny" the request if the rule matches
+ #
+ # The default is:
+ #
+ # alias_creation_rules:
+ # - user_id: "*"
+ # alias: "*"
+ # room_id: "*"
+ # action: allow
+
+ # The `room_list_publication_rules` option controls who can publish and
+ # which rooms can be published in the public room list.
+ #
+ # The format of this option is the same as that for
+ # `alias_creation_rules`.
#
- # If no rules match the request is denied.
- alias_creation_rules:
- - user_id: "*"
- alias: "*"
- action: allow
+ # If the room has one or more aliases associated with it, only one of
+ # the aliases needs to match the alias rule. If there are no aliases
+ # then only rules with `alias: *` match.
+ #
+ # If no rules match the request is denied. An empty list means no one
+ # can publish rooms.
+ #
+ # Options for the rules include:
+ #
+ # user_id: Matches agaisnt the creator of the alias
+ # room_id: Matches against the room ID being published
+ # alias: Matches against any current local or canonical aliases
+ # associated with the room
+ # action: Whether to "allow" or "deny" the request if the rule matches
+ #
+ # The default is:
+ #
+ # room_list_publication_rules:
+ # - user_id: "*"
+ # alias: "*"
+ # room_id: "*"
+ # action: allow
"""
- def is_alias_creation_allowed(self, user_id, alias):
+ def is_alias_creation_allowed(self, user_id, room_id, alias):
"""Checks if the given user is allowed to create the given alias
Args:
user_id (str)
+ room_id (str)
alias (str)
Returns:
boolean: True if user is allowed to crate the alias
"""
for rule in self._alias_creation_rules:
- if rule.matches(user_id, alias):
+ if rule.matches(user_id, room_id, [alias]):
+ return rule.action == "allow"
+
+ return False
+
+ def is_publishing_room_allowed(self, user_id, room_id, aliases):
+ """Checks if the given user is allowed to publish the room
+
+ Args:
+ user_id (str)
+ room_id (str)
+ aliases (list[str]): any local aliases associated with the room
+
+ Returns:
+ boolean: True if user can publish room
+ """
+ for rule in self._room_list_publication_rules:
+ if rule.matches(user_id, room_id, aliases):
return rule.action == "allow"
return False
-class _AliasRule(object):
- def __init__(self, rule):
+class _RoomDirectoryRule(object):
+ """Helper class to test whether a room directory action is allowed, like
+ creating an alias or publishing a room.
+ """
+
+ def __init__(self, option_name, rule):
+ """
+ Args:
+ option_name (str): Name of the config option this rule belongs to
+ rule (dict): The rule as specified in the config
+ """
+
action = rule["action"]
- user_id = rule["user_id"]
- alias = rule["alias"]
+ user_id = rule.get("user_id", "*")
+ room_id = rule.get("room_id", "*")
+ alias = rule.get("alias", "*")
if action in ("allow", "deny"):
self.action = action
else:
raise ConfigError(
- "alias_creation_rules rules can only have action of 'allow'"
- " or 'deny'"
+ "%s rules can only have action of 'allow'"
+ " or 'deny'" % (option_name,)
)
+ self._alias_matches_all = alias == "*"
+
try:
self._user_id_regex = glob_to_regex(user_id)
self._alias_regex = glob_to_regex(alias)
+ self._room_id_regex = glob_to_regex(room_id)
except Exception as e:
raise ConfigError("Failed to parse glob into regex: %s", e)
- def matches(self, user_id, alias):
- """Tests if this rule matches the given user_id and alias.
+ def matches(self, user_id, room_id, aliases):
+ """Tests if this rule matches the given user_id, room_id and aliases.
Args:
user_id (str)
- alias (str)
+ room_id (str)
+ aliases (list[str]): The associated aliases to the room. Will be a
+ single element for testing alias creation, and can be empty for
+ testing room publishing.
Returns:
boolean
@@ -96,7 +199,22 @@ class _AliasRule(object):
if not self._user_id_regex.match(user_id):
return False
- if not self._alias_regex.match(alias):
+ if not self._room_id_regex.match(room_id):
return False
- return True
+ # We only have alias checks left, so we can short circuit if the alias
+ # rule matches everything.
+ if self._alias_matches_all:
+ return True
+
+ # If we are not given any aliases then this rule only matches if the
+ # alias glob matches all aliases, which we checked above.
+ if not aliases:
+ return False
+
+ # Otherwise, we just need one alias to match
+ for alias in aliases:
+ if self._alias_regex.match(alias):
+ return True
+
+ return False
diff --git a/synapse/crypto/keyring.py b/synapse/crypto/keyring.py
index 3a96980bed..cce40fdd2d 100644
--- a/synapse/crypto/keyring.py
+++ b/synapse/crypto/keyring.py
@@ -35,7 +35,7 @@ from unpaddedbase64 import decode_base64
from twisted.internet import defer
-from synapse.api.errors import Codes, SynapseError
+from synapse.api.errors import Codes, RequestSendFailed, SynapseError
from synapse.util import logcontext, unwrapFirstError
from synapse.util.logcontext import (
LoggingContext,
@@ -656,7 +656,7 @@ def _handle_key_deferred(verify_request):
try:
with PreserveLoggingContext():
_, key_id, verify_key = yield verify_request.deferred
- except IOError as e:
+ except (IOError, RequestSendFailed) as e:
logger.warn(
"Got IOError when downloading keys for %s: %s %s",
server_name, type(e).__name__, str(e),
diff --git a/synapse/groups/attestations.py b/synapse/groups/attestations.py
index b04f4234ca..786149be65 100644
--- a/synapse/groups/attestations.py
+++ b/synapse/groups/attestations.py
@@ -42,7 +42,7 @@ from signedjson.sign import sign_json
from twisted.internet import defer
-from synapse.api.errors import SynapseError
+from synapse.api.errors import RequestSendFailed, SynapseError
from synapse.metrics.background_process_metrics import run_as_background_process
from synapse.types import get_domain_from_id
from synapse.util.logcontext import run_in_background
@@ -191,6 +191,11 @@ class GroupAttestionRenewer(object):
yield self.store.update_attestation_renewal(
group_id, user_id, attestation
)
+ except RequestSendFailed as e:
+ logger.warning(
+ "Failed to renew attestation of %r in %r: %s",
+ user_id, group_id, e,
+ )
except Exception:
logger.exception("Error renewing attestation of %r in %r",
user_id, group_id)
diff --git a/synapse/handlers/device.py b/synapse/handlers/device.py
index 8955cde4ed..6eddb10e0d 100644
--- a/synapse/handlers/device.py
+++ b/synapse/handlers/device.py
@@ -20,7 +20,7 @@ from twisted.internet import defer
from synapse.api import errors
from synapse.api.constants import EventTypes
-from synapse.api.errors import FederationDeniedError
+from synapse.api.errors import FederationDeniedError, RequestSendFailed
from synapse.types import RoomStreamToken, get_domain_from_id
from synapse.util import stringutils
from synapse.util.async_helpers import Linearizer
@@ -504,7 +504,7 @@ class DeviceListEduUpdater(object):
origin = get_domain_from_id(user_id)
try:
result = yield self.federation.query_user_devices(origin, user_id)
- except NotRetryingDestination:
+ except (NotRetryingDestination, RequestSendFailed):
# TODO: Remember that we are now out of sync and try again
# later
logger.warn(
diff --git a/synapse/handlers/directory.py b/synapse/handlers/directory.py
index 6bb254f899..8b113307d2 100644
--- a/synapse/handlers/directory.py
+++ b/synapse/handlers/directory.py
@@ -112,7 +112,9 @@ class DirectoryHandler(BaseHandler):
403, "This user is not permitted to create this alias",
)
- if not self.config.is_alias_creation_allowed(user_id, room_alias.to_string()):
+ if not self.config.is_alias_creation_allowed(
+ user_id, room_id, room_alias.to_string(),
+ ):
# Lets just return a generic message, as there may be all sorts of
# reasons why we said no. TODO: Allow configurable error messages
# per alias creation rule?
@@ -395,9 +397,9 @@ class DirectoryHandler(BaseHandler):
room_id (str)
visibility (str): "public" or "private"
"""
- if not self.spam_checker.user_may_publish_room(
- requester.user.to_string(), room_id
- ):
+ user_id = requester.user.to_string()
+
+ if not self.spam_checker.user_may_publish_room(user_id, room_id):
raise AuthError(
403,
"This user is not permitted to publish rooms to the room list"
@@ -415,7 +417,24 @@ class DirectoryHandler(BaseHandler):
yield self.auth.check_can_change_room_list(room_id, requester.user)
- yield self.store.set_room_is_public(room_id, visibility == "public")
+ making_public = visibility == "public"
+ if making_public:
+ room_aliases = yield self.store.get_aliases_for_room(room_id)
+ canonical_alias = yield self.store.get_canonical_alias_for_room(room_id)
+ if canonical_alias:
+ room_aliases.append(canonical_alias)
+
+ if not self.config.is_publishing_room_allowed(
+ user_id, room_id, room_aliases,
+ ):
+ # Lets just return a generic message, as there may be all sorts of
+ # reasons why we said no. TODO: Allow configurable error messages
+ # per alias creation rule?
+ raise SynapseError(
+ 403, "Not allowed to publish room",
+ )
+
+ yield self.store.set_room_is_public(room_id, making_public)
@defer.inlineCallbacks
def edit_published_appservice_room_list(self, appservice_id, network_id,
diff --git a/synapse/handlers/groups_local.py b/synapse/handlers/groups_local.py
index 173315af6c..02c508acec 100644
--- a/synapse/handlers/groups_local.py
+++ b/synapse/handlers/groups_local.py
@@ -20,7 +20,7 @@ from six import iteritems
from twisted.internet import defer
-from synapse.api.errors import HttpResponseException, SynapseError
+from synapse.api.errors import HttpResponseException, RequestSendFailed, SynapseError
from synapse.types import get_domain_from_id
logger = logging.getLogger(__name__)
@@ -46,13 +46,19 @@ def _create_rerouter(func_name):
# when the remote end responds with things like 403 Not
# In Group, we can communicate that to the client instead
# of a 500.
- def h(failure):
+ def http_response_errback(failure):
failure.trap(HttpResponseException)
e = failure.value
if e.code == 403:
raise e.to_synapse_error()
return failure
- d.addErrback(h)
+
+ def request_failed_errback(failure):
+ failure.trap(RequestSendFailed)
+ raise SynapseError(502, "Failed to contact group server")
+
+ d.addErrback(http_response_errback)
+ d.addErrback(request_failed_errback)
return d
return f
diff --git a/synapse/storage/state.py b/synapse/storage/state.py
index d14a7b2538..6ddc4055d2 100644
--- a/synapse/storage/state.py
+++ b/synapse/storage/state.py
@@ -548,6 +548,31 @@ class StateGroupWorkerStore(EventsWorkerStore, SQLBaseStore):
_get_filtered_current_state_ids_txn,
)
+ @defer.inlineCallbacks
+ def get_canonical_alias_for_room(self, room_id):
+ """Get canonical alias for room, if any
+
+ Args:
+ room_id (str)
+
+ Returns:
+ Deferred[str|None]: The canonical alias, if any
+ """
+
+ state = yield self.get_filtered_current_state_ids(room_id, StateFilter.from_types(
+ [(EventTypes.CanonicalAlias, "")]
+ ))
+
+ event_id = state.get((EventTypes.CanonicalAlias, ""))
+ if not event_id:
+ return
+
+ event = yield self.get_event(event_id, allow_none=True)
+ if not event:
+ return
+
+ defer.returnValue(event.content.get("canonical_alias"))
+
@cached(max_entries=10000, iterable=True)
def get_state_group_delta(self, state_group):
"""Given a state group try to return a previous group and a delta between
diff --git a/tests/config/test_room_directory.py b/tests/config/test_room_directory.py
index f37a17d618..3dc2631523 100644
--- a/tests/config/test_room_directory.py
+++ b/tests/config/test_room_directory.py
@@ -36,6 +36,8 @@ class RoomDirectoryConfigTestCase(unittest.TestCase):
- user_id: "@gah:example.com"
alias: "#goo:example.com"
action: "allow"
+
+ room_list_publication_rules: []
""")
rd_config = RoomDirectoryConfig()
@@ -43,25 +45,102 @@ class RoomDirectoryConfigTestCase(unittest.TestCase):
self.assertFalse(rd_config.is_alias_creation_allowed(
user_id="@bob:example.com",
+ room_id="!test",
alias="#test:example.com",
))
self.assertTrue(rd_config.is_alias_creation_allowed(
user_id="@test:example.com",
+ room_id="!test",
alias="#unofficial_st:example.com",
))
self.assertTrue(rd_config.is_alias_creation_allowed(
user_id="@foobar:example.com",
+ room_id="!test",
alias="#test:example.com",
))
self.assertTrue(rd_config.is_alias_creation_allowed(
user_id="@gah:example.com",
+ room_id="!test",
alias="#goo:example.com",
))
self.assertFalse(rd_config.is_alias_creation_allowed(
user_id="@test:example.com",
+ room_id="!test",
alias="#test:example.com",
))
+
+ def test_room_publish_acl(self):
+ config = yaml.load("""
+ alias_creation_rules: []
+
+ room_list_publication_rules:
+ - user_id: "*bob*"
+ alias: "*"
+ action: "deny"
+ - user_id: "*"
+ alias: "#unofficial_*"
+ action: "allow"
+ - user_id: "@foo*:example.com"
+ alias: "*"
+ action: "allow"
+ - user_id: "@gah:example.com"
+ alias: "#goo:example.com"
+ action: "allow"
+ - room_id: "!test-deny"
+ action: "deny"
+ """)
+
+ rd_config = RoomDirectoryConfig()
+ rd_config.read_config(config)
+
+ self.assertFalse(rd_config.is_publishing_room_allowed(
+ user_id="@bob:example.com",
+ room_id="!test",
+ aliases=["#test:example.com"],
+ ))
+
+ self.assertTrue(rd_config.is_publishing_room_allowed(
+ user_id="@test:example.com",
+ room_id="!test",
+ aliases=["#unofficial_st:example.com"],
+ ))
+
+ self.assertTrue(rd_config.is_publishing_room_allowed(
+ user_id="@foobar:example.com",
+ room_id="!test",
+ aliases=[],
+ ))
+
+ self.assertTrue(rd_config.is_publishing_room_allowed(
+ user_id="@gah:example.com",
+ room_id="!test",
+ aliases=["#goo:example.com"],
+ ))
+
+ self.assertFalse(rd_config.is_publishing_room_allowed(
+ user_id="@test:example.com",
+ room_id="!test",
+ aliases=["#test:example.com"],
+ ))
+
+ self.assertTrue(rd_config.is_publishing_room_allowed(
+ user_id="@foobar:example.com",
+ room_id="!test-deny",
+ aliases=[],
+ ))
+
+ self.assertFalse(rd_config.is_publishing_room_allowed(
+ user_id="@gah:example.com",
+ room_id="!test-deny",
+ aliases=[],
+ ))
+
+ self.assertTrue(rd_config.is_publishing_room_allowed(
+ user_id="@test:example.com",
+ room_id="!test",
+ aliases=["#unofficial_st:example.com", "#blah:example.com"],
+ ))
diff --git a/tests/handlers/test_directory.py b/tests/handlers/test_directory.py
index 8ae6556c0a..9bf395e923 100644
--- a/tests/handlers/test_directory.py
+++ b/tests/handlers/test_directory.py
@@ -121,6 +121,7 @@ class TestCreateAliasACL(unittest.HomeserverTestCase):
"action": "allow",
}
]
+ config["room_list_publication_rules"] = []
rd_config = RoomDirectoryConfig()
rd_config.read_config(config)
|
