Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Add configurable room list publishing rules #4647

Merged
merged 9 commits into from
Feb 15, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/4647.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add configurable room list publishing rules
170 changes: 144 additions & 26 deletions synapse/config/room_directory.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,73 +20,176 @@

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 """
# The `alias_creation` option controls who's allowed to create aliases
# 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.
erikjohnston marked this conversation as resolved.
Show resolved Hide resolved
#
# 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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what does it even mean to publish a room in the room directory when it doesn't have an alias, ooi?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a perfectly legit thing to do, while remote users wouldn't be able to join the room local users still would happily be able to.

Though I have a suspicion we might require an alias before we actually publish the room

# 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):
erikjohnston marked this conversation as resolved.
Show resolved Hide resolved
"""
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
Expand All @@ -96,7 +199,22 @@ def matches(self, user_id, alias):
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
29 changes: 24 additions & 5 deletions synapse/handlers/directory.py
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,9 @@ def create_association(self, requester, room_alias, room_id, servers=None,
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?
Expand Down Expand Up @@ -395,9 +397,9 @@ def edit_published_room_list(self, requester, room_id, visibility):
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"
Expand All @@ -415,7 +417,24 @@ def edit_published_room_list(self, requester, room_id, visibility):

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,
Expand Down
25 changes: 25 additions & 0 deletions synapse/storage/state.py
Original file line number Diff line number Diff line change
Expand Up @@ -548,6 +548,31 @@ def _get_filtered_current_state_ids_txn(txn):
_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
Expand Down
Loading