diff --git a/changelog.d/5760.feature b/changelog.d/5760.feature new file mode 100644 index 000000000000..90302d793eb6 --- /dev/null +++ b/changelog.d/5760.feature @@ -0,0 +1 @@ +Force the access rule to be "restricted" if the join rule is "public". diff --git a/changelog.d/5780.misc b/changelog.d/5780.misc new file mode 100644 index 000000000000..b7eb56e625b2 --- /dev/null +++ b/changelog.d/5780.misc @@ -0,0 +1 @@ +Allow looping calls to be given arguments. diff --git a/changelog.d/5807.feature b/changelog.d/5807.feature new file mode 100644 index 000000000000..8b7d29a23cb4 --- /dev/null +++ b/changelog.d/5807.feature @@ -0,0 +1 @@ +Allow defining HTML templates to serve the user on account renewal attempt when using the account validity feature. diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index 24ee11d86e5b..63051dd56f0a 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -860,6 +860,16 @@ uploads_path: "DATADIR/uploads" # period: 6w # renew_at: 1w # renew_email_subject: "Renew your %(app)s account" +# # Directory in which Synapse will try to find the HTML files to serve to the +# # user when trying to renew an account. Optional, defaults to +# # synapse/res/templates. +# template_dir: "res/templates" +# # HTML to be displayed to the user after they successfully renewed their +# # account. Optional. +# account_renewed_html_path: "account_renewed.html" +# # HTML to be displayed when the user tries to renew an account with an invalid +# # renewal token. Optional. +# invalid_token_html_path: "invalid_token.html" # The user must provide all of the below types of 3PID when registering. # diff --git a/synapse/config/registration.py b/synapse/config/registration.py index 5041cfa4ee7b..14752298e9e1 100644 --- a/synapse/config/registration.py +++ b/synapse/config/registration.py @@ -13,8 +13,11 @@ # See the License for the specific language governing permissions and # limitations under the License. +import os from distutils.util import strtobool +import pkg_resources + from synapse.config._base import Config, ConfigError from synapse.types import RoomAlias from synapse.util.stringutils import random_string_with_symbols @@ -41,8 +44,36 @@ def __init__(self, config, synapse_config): self.startup_job_max_delta = self.period * 10. / 100. - if self.renew_by_email_enabled and "public_baseurl" not in synapse_config: - raise ConfigError("Can't send renewal emails without 'public_baseurl'") + if self.renew_by_email_enabled: + if "public_baseurl" not in synapse_config: + raise ConfigError("Can't send renewal emails without 'public_baseurl'") + + template_dir = config.get("template_dir") + + if not template_dir: + template_dir = pkg_resources.resource_filename("synapse", "res/templates") + + if "account_renewed_html_path" in config: + file_path = os.path.join(template_dir, config["account_renewed_html_path"]) + + self.account_renewed_html_content = self.read_file( + file_path, "account_validity.account_renewed_html_path" + ) + else: + self.account_renewed_html_content = ( + "Your account has been successfully renewed." + ) + + if "invalid_token_html_path" in config: + file_path = os.path.join(template_dir, config["invalid_token_html_path"]) + + self.invalid_token_html_content = self.read_file( + file_path, "account_validity.invalid_token_html_path" + ) + else: + self.invalid_token_html_content = ( + "Invalid renewal token." + ) class RegistrationConfig(Config): @@ -161,6 +192,16 @@ def default_config(self, generate_secrets=False, **kwargs): # period: 6w # renew_at: 1w # renew_email_subject: "Renew your %%(app)s account" + # # Directory in which Synapse will try to find the HTML files to serve to the + # # user when trying to renew an account. Optional, defaults to + # # synapse/res/templates. + # template_dir: "res/templates" + # # HTML to be displayed to the user after they successfully renewed their + # # account. Optional. + # account_renewed_html_path: "account_renewed.html" + # # HTML to be displayed when the user tries to renew an account with an invalid + # # renewal token. Optional. + # invalid_token_html_path: "invalid_token.html" # The user must provide all of the below types of 3PID when registering. # diff --git a/synapse/handlers/account_validity.py b/synapse/handlers/account_validity.py index 5e0b92eb1cc2..396f0059f711 100644 --- a/synapse/handlers/account_validity.py +++ b/synapse/handlers/account_validity.py @@ -223,11 +223,19 @@ def renew_account(self, renewal_token): Args: renewal_token (str): Token sent with the renewal request. + Returns: + bool: Whether the provided token is valid. """ - user_id = yield self.store.get_user_from_renewal_token(renewal_token) + try: + user_id = yield self.store.get_user_from_renewal_token(renewal_token) + except StoreError: + defer.returnValue(False) + logger.debug("Renewing an account for user %s", user_id) yield self.renew_account_for_user(user_id) + defer.returnValue(True) + @defer.inlineCallbacks def renew_account_for_user(self, user_id, expiration_ts=None, email_sent=False): """Renews the account attached to a given user by pushing back the diff --git a/synapse/res/templates/account_renewed.html b/synapse/res/templates/account_renewed.html new file mode 100644 index 000000000000..894da030afb7 --- /dev/null +++ b/synapse/res/templates/account_renewed.html @@ -0,0 +1 @@ +Your account has been successfully renewed. diff --git a/synapse/res/templates/invalid_token.html b/synapse/res/templates/invalid_token.html new file mode 100644 index 000000000000..6bd2b9836437 --- /dev/null +++ b/synapse/res/templates/invalid_token.html @@ -0,0 +1 @@ +Invalid renewal token. diff --git a/synapse/rest/client/v2_alpha/account_validity.py b/synapse/rest/client/v2_alpha/account_validity.py index 63bdc3356434..8091b7828591 100644 --- a/synapse/rest/client/v2_alpha/account_validity.py +++ b/synapse/rest/client/v2_alpha/account_validity.py @@ -40,6 +40,8 @@ def __init__(self, hs): self.hs = hs self.account_activity_handler = hs.get_account_validity_handler() self.auth = hs.get_auth() + self.success_html = hs.config.account_validity.account_renewed_html_content + self.failure_html = hs.config.account_validity.invalid_token_html_content @defer.inlineCallbacks def on_GET(self, request): @@ -47,14 +49,21 @@ def on_GET(self, request): raise SynapseError(400, "Missing renewal token") renewal_token = request.args[b"token"][0] - yield self.account_activity_handler.renew_account(renewal_token.decode('utf8')) + token_valid = yield self.account_activity_handler.renew_account( + renewal_token.decode("utf8") + ) - request.setResponseCode(200) + if token_valid: + status_code = 200 + response = self.success_html + else: + status_code = 404 + response = self.failure_html + + request.setResponseCode(status_code) request.setHeader(b"Content-Type", b"text/html; charset=utf-8") - request.setHeader(b"Content-Length", b"%d" % ( - len(AccountValidityRenewServlet.SUCCESS_HTML), - )) - request.write(AccountValidityRenewServlet.SUCCESS_HTML) + request.setHeader(b"Content-Length", b"%d" % (len(response),)) + request.write(response.encode("utf8")) finish_request(request) defer.returnValue(None) diff --git a/synapse/storage/events_worker.py b/synapse/storage/events_worker.py index cc7df5cf14df..5dc49822b59d 100644 --- a/synapse/storage/events_worker.py +++ b/synapse/storage/events_worker.py @@ -255,6 +255,26 @@ def get_events_as_list( # didn't have the redacted event at the time, so we recheck on read # instead. if not allow_rejected and entry.event.type == EventTypes.Redaction: + orig_event_info = yield self._simple_select_one( + table="events", + keyvalues={"event_id": entry.event.redacts}, + retcols=["sender", "room_id", "type"], + allow_none=True, + ) + + if not orig_event_info: + # We don't have the event that is being redacted, so we + # assume that the event isn't authorized for now. (If we + # later receive the event, then we will always redact + # it anyway, since we have this redaction) + continue + + if orig_event_info["room_id"] != entry.event.room_id: + # Don't process redactions if the redacted event doesn't belong to the + # redaction's room. + logger.info("Ignoring redation in another room.") + continue + if entry.event.internal_metadata.need_to_check_redaction(): # XXX: we need to avoid calling get_event here. # @@ -277,27 +297,13 @@ def get_events_as_list( # 2. have _get_event_from_row just call the first half of # that - orig_sender = yield self._simple_select_one_onecol( - table="events", - keyvalues={"event_id": entry.event.redacts}, - retcol="sender", - allow_none=True, - ) - expected_domain = get_domain_from_id(entry.event.sender) if ( - orig_sender - and get_domain_from_id(orig_sender) == expected_domain + get_domain_from_id(orig_event_info["sender"]) == expected_domain ): # This redaction event is allowed. Mark as not needing a # recheck. entry.event.internal_metadata.recheck_redaction = False - else: - # We don't have the event that is being redacted, so we - # assume that the event isn't authorized for now. (If we - # later receive the event, then we will always redact - # it anyway, since we have this redaction) - continue if allow_rejected or not entry.event.rejected_reason: if check_redacted and entry.redacted_event: @@ -532,7 +538,7 @@ def _get_event_from_row( ) redacted_event = None - if redacted: + if redacted and original_ev.type != EventTypes.Redaction: redacted_event = prune_event(original_ev) redaction_id = yield self._simple_select_one_onecol( @@ -564,9 +570,18 @@ def _get_event_from_row( # recheck. because.internal_metadata.recheck_redaction = False else: - # Senders don't match, so the event isn't actually redacted + # Senders don't match, so the event isn't actually + # redacted redacted_event = None + if because.room_id != original_ev.room_id: + redacted_event = None + else: + # The lack of a redaction likely means that the redaction is invalid + # and therefore not returned by get_event, so it should be safe to + # just ignore it here. + redacted_event = None + cache_entry = _EventCacheEntry( event=original_ev, redacted_event=redacted_event ) diff --git a/synapse/third_party_rules/access_rules.py b/synapse/third_party_rules/access_rules.py index e3f97bdf3aa8..1a295ea7cea4 100644 --- a/synapse/third_party_rules/access_rules.py +++ b/synapse/third_party_rules/access_rules.py @@ -17,7 +17,7 @@ from twisted.internet import defer -from synapse.api.constants import EventTypes +from synapse.api.constants import EventTypes, JoinRules, RoomCreationPreset from synapse.api.errors import SynapseError from synapse.config._base import ConfigError from synapse.types import get_domain_from_id @@ -94,35 +94,43 @@ def on_create_room(self, requester, config, is_requester_admin): default rule to the initial state. """ is_direct = config.get("is_direct") - rule = None + preset = config.get("preset") + access_rule = None + join_rule = None # If there's a rules event in the initial state, check if it complies with the # spec for im.vector.room.access_rules and deny the request if not. for event in config.get("initial_state", []): if event["type"] == ACCESS_RULES_TYPE: - rule = event["content"].get("rule") + access_rule = event["content"].get("rule") # Make sure the event has a valid content. - if rule is None: + if access_rule is None: raise SynapseError(400, "Invalid access rule") # Make sure the rule name is valid. - if rule not in VALID_ACCESS_RULES: + if access_rule not in VALID_ACCESS_RULES: raise SynapseError(400, "Invalid access rule") # Make sure the rule is "direct" if the room is a direct chat. if ( - (is_direct and rule != ACCESS_RULE_DIRECT) - or (rule == ACCESS_RULE_DIRECT and not is_direct) + (is_direct and access_rule != ACCESS_RULE_DIRECT) + or (access_rule == ACCESS_RULE_DIRECT and not is_direct) ): raise SynapseError(400, "Invalid access rule") - # If there's no rules event in the initial state, create one with the default - # setting. - if not rule: + if event["type"] == EventTypes.JoinRules: + join_rule = event["content"].get("join_rule") + + if access_rule is None: + # If there's no access rules event in the initial state, create one with the + # default setting. if is_direct: default_rule = ACCESS_RULE_DIRECT else: + # If the default value for non-direct chat changes, we should make another + # case here for rooms created with either a "public" join_rule or the + # "public_chat" preset to make sure those keep defaulting to "restricted" default_rule = ACCESS_RULE_RESTRICTED if not config.get("initial_state"): @@ -136,11 +144,22 @@ def on_create_room(self, requester, config, is_requester_admin): } }) - rule = default_rule + access_rule = default_rule + + # Check that the preset or the join rule in use is compatible with the access + # rule, whether it's a user-defined one or the default one (i.e. if it involves + # a "public" join rule, the access rule must be "restricted"). + if ( + ( + join_rule == JoinRules.PUBLIC + or preset == RoomCreationPreset.PUBLIC_CHAT + ) and access_rule != ACCESS_RULE_RESTRICTED + ): + raise SynapseError(400, "Invalid access rule") # Check if the creator can override values for the power levels. allowed = self._is_power_level_content_allowed( - config.get("power_level_content_override", {}), rule, + config.get("power_level_content_override", {}), access_rule, ) if not allowed: raise SynapseError(400, "Invalid power levels content override") @@ -148,7 +167,9 @@ def on_create_room(self, requester, config, is_requester_admin): # Second loop for events we need to know the current rule to process. for event in config.get("initial_state", []): if event["type"] == EventTypes.PowerLevels: - allowed = self._is_power_level_content_allowed(event["content"], rule) + allowed = self._is_power_level_content_allowed( + event["content"], access_rule + ) if not allowed: raise SynapseError(400, "Invalid power levels content") @@ -213,6 +234,9 @@ def check_event_allowed(self, event, state_events): if event.type == EventTypes.Member or event.type == EventTypes.ThirdPartyInvite: return self._on_membership_or_invite(event, rule, state_events) + if event.type == EventTypes.JoinRules: + return self._on_join_rule_change(event, rule) + return True def _on_rules_change(self, event, state_events): @@ -232,6 +256,12 @@ def _on_rules_change(self, event, state_events): if new_rule not in VALID_ACCESS_RULES: return False + # We must not allow rooms with the "public" join rule to be given any other access + # rule than "restricted". + join_rule = self._get_join_rule_from_state(state_events) + if join_rule == JoinRules.PUBLIC and new_rule != ACCESS_RULE_RESTRICTED: + return False + # Make sure we don't apply "direct" if the room has more than two members. if new_rule == ACCESS_RULE_DIRECT: existing_members, threepid_tokens = self._get_members_and_tokens_from_state( @@ -381,7 +411,6 @@ def _is_power_level_content_allowed(self, content, access_rule): access_rule (str): The access rule in place in this room. Returns: bool, True if the event can be allowed, False otherwise. - """ # Check if we need to apply the restrictions with the current rule. if access_rule not in RULES_WITH_RESTRICTED_POWER_LEVELS: @@ -405,6 +434,33 @@ def _is_power_level_content_allowed(self, content, access_rule): return True + def _on_join_rule_change(self, event, rule): + """Check whether a join rule change is allowed. A join rule change is always + allowed unless the new join rule is "public" and the current access rule isn't + "restricted". + The rationale is that external users (those whose server would be denied access + to rooms enforcing the "restricted" access rule) should always rely on non- + external users for access to rooms, therefore they shouldn't be able to access + rooms that don't require an invite to be joined. + + Note that we currently rely on the default access rule being "restricted": during + room creation, the m.room.join_rules event will be sent *before* the + im.vector.room.access_rules one, so the access rule that will be considered here + in this case will be the default "restricted" one. This is fine since the + "restricted" access rule allows any value for the join rule, but we should keep + that in mind if we need to change the default access rule in the future. + + Args: + event (synapse.events.EventBase): The event to check. + rule (str): The name of the rule to apply. + Returns: + bool, True if the event can be allowed, False otherwise. + """ + if event.content.get('join_rule') == JoinRules.PUBLIC: + return rule == ACCESS_RULE_RESTRICTED + + return True + @staticmethod def _get_rule_from_state(state_events): """Extract the rule to be applied from the given set of state events. @@ -422,6 +478,21 @@ def _get_rule_from_state(state_events): rule = access_rules.content.get("rule") return rule + @staticmethod + def _get_join_rule_from_state(state_events): + """Extract the room's join rule from the given set of state events. + + Args: + state_events (dict[tuple[event type, state key], EventBase]): The set of state + events. + Returns: + str, the name of the join rule (either "public", or "invite") + """ + join_rule_event = state_events.get((EventTypes.JoinRules, "")) + if join_rule_event is None: + return None + return join_rule_event.content.get("join_rule") + @staticmethod def _get_members_and_tokens_from_state(state_events): """Retrieves from a list of state events the list of users that have a diff --git a/synapse/util/__init__.py b/synapse/util/__init__.py index 0ae7e2ef3b3e..8f5a526800fa 100644 --- a/synapse/util/__init__.py +++ b/synapse/util/__init__.py @@ -58,7 +58,7 @@ def time_msec(self): """Returns the current system time in miliseconds since epoch.""" return int(self.time() * 1000) - def looping_call(self, f, msec): + def looping_call(self, f, msec, *args, **kwargs): """Call a function repeatedly. Waits `msec` initially before calling `f` for the first time. @@ -66,8 +66,10 @@ def looping_call(self, f, msec): Args: f(function): The function to call repeatedly. msec(float): How long to wait between calls in milliseconds. + *args: Postional arguments to pass to function. + **kwargs: Key arguments to pass to function. """ - call = task.LoopingCall(f) + call = task.LoopingCall(f, *args, **kwargs) call.clock = self._reactor d = call.start(msec / 1000.0, now=False) d.addErrback( diff --git a/tests/rest/client/test_room_access_rules.py b/tests/rest/client/test_room_access_rules.py index df48a89e9305..7e23add6b79b 100644 --- a/tests/rest/client/test_room_access_rules.py +++ b/tests/rest/client/test_room_access_rules.py @@ -22,7 +22,7 @@ from twisted.internet import defer -from synapse.api.constants import EventTypes +from synapse.api.constants import EventTypes, JoinRules, RoomCreationPreset from synapse.rest import admin from synapse.rest.client.v1 import login, room from synapse.third_party_rules.access_rules import ( @@ -156,6 +156,84 @@ def test_create_room_direct_invalid_rule(self): """ self.create_room(direct=True, rule=ACCESS_RULE_RESTRICTED, expected_code=400) + def test_public_room(self): + """Tests that it's not possible to have a room with the public join rule and an + access rule that's not restricted. + """ + # Creating a room with the public_chat preset should succeed and set the access + # rule to restricted. + preset_room_id = self.create_room(preset=RoomCreationPreset.PUBLIC_CHAT) + self.assertEqual( + self.current_rule_in_room(preset_room_id), ACCESS_RULE_RESTRICTED, + ) + + # Creating a room with the public join rule in its initial state should succeed + # and set the access rule to restricted. + init_state_room_id = self.create_room(initial_state=[{ + "type": "m.room.join_rules", + "content": { + "join_rule": JoinRules.PUBLIC, + }, + }]) + self.assertEqual( + self.current_rule_in_room(init_state_room_id), ACCESS_RULE_RESTRICTED, + ) + + # Changing access rule to unrestricted should fail. + self.change_rule_in_room( + preset_room_id, ACCESS_RULE_UNRESTRICTED, expected_code=403, + ) + self.change_rule_in_room( + init_state_room_id, ACCESS_RULE_UNRESTRICTED, expected_code=403, + ) + + # Changing access rule to direct should fail. + self.change_rule_in_room( + preset_room_id, ACCESS_RULE_DIRECT, expected_code=403, + ) + self.change_rule_in_room( + init_state_room_id, ACCESS_RULE_DIRECT, expected_code=403, + ) + + # Changing join rule to public in an unrestricted room should fail. + self.change_join_rule_in_room( + self.unrestricted_room, JoinRules.PUBLIC, expected_code=403, + ) + # Changing join rule to public in an direct room should fail. + self.change_join_rule_in_room( + self.direct_rooms[0], JoinRules.PUBLIC, expected_code=403, + ) + + # Creating a new room with the public_chat preset and an access rule that isn't + # restricted should fail. + self.create_room( + preset=RoomCreationPreset.PUBLIC_CHAT, rule=ACCESS_RULE_UNRESTRICTED, + expected_code=400, + ) + self.create_room( + preset=RoomCreationPreset.PUBLIC_CHAT, rule=ACCESS_RULE_DIRECT, + expected_code=400, + ) + + # Creating a room with the public join rule in its initial state and an access + # rule that isn't restricted should fail. + self.create_room( + initial_state=[{ + "type": "m.room.join_rules", + "content": { + "join_rule": JoinRules.PUBLIC, + }, + }], rule=ACCESS_RULE_UNRESTRICTED, expected_code=400, + ) + self.create_room( + initial_state=[{ + "type": "m.room.join_rules", + "content": { + "join_rule": JoinRules.PUBLIC, + }, + }], rule=ACCESS_RULE_DIRECT, expected_code=400, + ) + def test_restricted(self): """Tests that in restricted mode we're unable to invite users from blacklisted servers but can invite other users. @@ -405,9 +483,13 @@ def test_change_rules(self): expected_code=403, ) - def create_room(self, direct=False, rule=None, expected_code=200): + def create_room( + self, direct=False, rule=None, preset=RoomCreationPreset.TRUSTED_PRIVATE_CHAT, + initial_state=None, expected_code=200, + ): content = { "is_direct": direct, + "preset": preset, } if rule: @@ -419,6 +501,12 @@ def create_room(self, direct=False, rule=None, expected_code=200): } }] + if initial_state: + if "initial_state" not in content: + content["initial_state"] = [] + + content["initial_state"] += initial_state + request, channel = self.make_request( "POST", "/_matrix/client/r0/createRoom", @@ -457,6 +545,20 @@ def change_rule_in_room(self, room_id, new_rule, expected_code=200): self.assertEqual(channel.code, expected_code, channel.result) + def change_join_rule_in_room(self, room_id, new_join_rule, expected_code=200): + data = { + "join_rule": new_join_rule, + } + request, channel = self.make_request( + "PUT", + "/_matrix/client/r0/rooms/%s/state/%s" % (room_id, EventTypes.JoinRules), + json.dumps(data), + access_token=self.tok, + ) + self.render(request) + + self.assertEqual(channel.code, expected_code, channel.result) + def send_threepid_invite(self, address, room_id, expected_code=200): params = { "id_server": "testis", diff --git a/tests/rest/client/v2_alpha/test_register.py b/tests/rest/client/v2_alpha/test_register.py index f56a34a41d33..af1e60059121 100644 --- a/tests/rest/client/v2_alpha/test_register.py +++ b/tests/rest/client/v2_alpha/test_register.py @@ -375,6 +375,8 @@ def make_homeserver(self, reactor, clock): "renew_at": 172800000, # Time in ms for 2 days "renew_by_email_enabled": True, "renew_email_subject": "Renew your account", + "account_renewed_html_path": "account_renewed.html", + "invalid_token_html_path": "invalid_token.html", } # Email config. @@ -425,6 +427,19 @@ def test_renewal_email(self): self.render(request) self.assertEquals(channel.result["code"], b"200", channel.result) + # Check that we're getting HTML back. + content_type = None + for header in channel.result.get("headers", []): + if header[0] == b"Content-Type": + content_type = header[1] + self.assertEqual(content_type, b"text/html; charset=utf-8", channel.result) + + # Check that the HTML we're getting is the one we expect on a successful renewal. + expected_html = self.hs.config.account_validity.account_renewed_html_content + self.assertEqual( + channel.result["body"], expected_html.encode("utf8"), channel.result + ) + # Move 3 days forward. If the renewal failed, every authed request with # our access token should be denied from now, otherwise they should # succeed. @@ -433,6 +448,28 @@ def test_renewal_email(self): self.render(request) self.assertEquals(channel.result["code"], b"200", channel.result) + def test_renewal_invalid_token(self): + # Hit the renewal endpoint with an invalid token and check that it behaves as + # expected, i.e. that it responds with 404 Not Found and the correct HTML. + url = "/_matrix/client/unstable/account_validity/renew?token=123" + request, channel = self.make_request(b"GET", url) + self.render(request) + self.assertEquals(channel.result["code"], b"404", channel.result) + + # Check that we're getting HTML back. + content_type = None + for header in channel.result.get("headers", []): + if header[0] == b"Content-Type": + content_type = header[1] + self.assertEqual(content_type, b"text/html; charset=utf-8", channel.result) + + # Check that the HTML we're getting is the one we expect when using an + # invalid/unknown token. + expected_html = self.hs.config.account_validity.invalid_token_html_content + self.assertEqual( + channel.result["body"], expected_html.encode("utf8"), channel.result + ) + def test_manual_email_send(self): self.email_attempts = []