From 2d71db2207952d5d7c03ca45f87c18b76e96f0b0 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Mon, 26 Apr 2021 15:30:22 +0100 Subject: [PATCH 01/50] Add additional required capabilities to the module API --- synapse/module_api/__init__.py | 202 +++++++++++++++++++++++++++++++-- 1 file changed, 195 insertions(+), 7 deletions(-) diff --git a/synapse/module_api/__init__.py b/synapse/module_api/__init__.py index a1a2b9aeccd3..eb22ea7c23df 100644 --- a/synapse/module_api/__init__.py +++ b/synapse/module_api/__init__.py @@ -12,17 +12,36 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. +import email.utils import logging -from typing import TYPE_CHECKING, Any, Generator, Iterable, List, Optional, Tuple +from email.mime.multipart import MIMEMultipart +from email.mime.text import MIMEText +from typing import ( + TYPE_CHECKING, + Any, + Callable, + Dict, + Generator, + Iterable, + List, + Optional, + Tuple, +) + +import jinja2 from twisted.internet import defer +from twisted.web.server import Request from synapse.events import EventBase from synapse.http.client import SimpleHttpClient from synapse.http.site import SynapseRequest from synapse.logging.context import make_deferred_yieldable, run_in_background +from synapse.metrics.background_process_metrics import wrap_as_background_process +from synapse.storage.databases.main.roommember import ProfileInfo from synapse.storage.state import StateFilter from synapse.types import JsonDict, UserID, create_requester +from synapse.util import Clock if TYPE_CHECKING: from synapse.server import HomeServer @@ -46,11 +65,22 @@ def __init__(self, hs, auth_handler): self._hs = hs self._store = hs.get_datastore() - self._auth = hs.get_auth() self._auth_handler = auth_handler self._server_name = hs.hostname self._presence_stream = hs.get_event_sources().sources["presence"] self._state = hs.get_state_handler() + self._sendmail = hs.get_sendmail() + self._clock = hs.get_clock() # type: Clock + + try: + app_name = self._hs.config.email_app_name + + self._from_string = self._hs.config.email_notif_from % {"app": app_name} + except Exception: + # If substitution failed, fall back to the bare strings. + self._from_string = self._hs.config.email_notif_from + + self._raw_from = email.utils.parseaddr(self._from_string)[1] # We expose these as properties below in order to attach a helpful docstring. self._http_client = hs.get_simple_http_client() # type: SimpleHttpClient @@ -81,14 +111,34 @@ def public_room_list_manager(self): """ return self._public_room_list_manager - def get_user_by_req(self, req, allow_guest=False): + @property + def public_baseurl(self): + """Allow accessing the configured public base URL for this homeserver.""" + return self._hs.config.public_baseurl + + @property + def email_app_name(self): + """Allow accessing the application name configured in the homeserver's + configuration. + """ + return self._hs.config.email_app_name + + def get_user_by_req( + self, + req: Request, + allow_guest: bool = False, + allow_expired: bool = False, + ): """Check the access_token provided for a request Args: - req (twisted.web.server.Request): Incoming HTTP request - allow_guest (bool): True if guest users should be allowed. If this + req: Incoming HTTP request + allow_guest: True if guest users should be allowed. If this is False, and the access token is for a guest user, an AuthError will be thrown + allow_expired: True if expired users should be allowed. If this + is False, and the access token is for an expired user, an + AuthError will be thrown Returns: twisted.internet.defer.Deferred[synapse.types.Requester]: the requester for this request @@ -96,7 +146,7 @@ def get_user_by_req(self, req, allow_guest=False): synapse.api.errors.AuthError: if no user by that token exists, or the token is invalid. """ - return self._auth.get_user_by_req(req, allow_guest) + return self._hs.get_auth().get_user_by_req(req, allow_guest, allow_expired) def get_qualified_user_id(self, username): """Qualify a user id, if necessary @@ -252,7 +302,7 @@ def invalidate_access_token(self, access_token): """ # see if the access token corresponds to a device user_info = yield defer.ensureDeferred( - self._auth.get_user_by_access_token(access_token) + self._hs.get_auth().get_user_by_access_token(access_token) ) device_id = user_info.get("device_id") user_id = user_info["user"].to_string() @@ -439,6 +489,144 @@ async def send_local_online_presence_to(self, users: Iterable[str]) -> None: presence_events ) + def background_call_async(self, f: Callable, *args, **kwargs): + """Wraps an async function as a background process and runs it. + + Args: + f(function): The function to call. + *args: Positional arguments to pass to function. + **kwargs: Key arguments to pass to function. + + """ + + @wrap_as_background_process(f.__name__) + async def background_call(*args, **kwargs): + await f(*args, **kwargs) + + if self._hs.config.run_background_tasks: + self._clock.call_later(0.0, background_call, *args, **kwargs) + else: + logger.warning( + "Not running looping call %s as the configuration forbids it", + f, + ) + + def looping_background_call_async(self, f: Callable, msec: float, *args, **kwargs): + """Wraps an async function as a background process and calls it repeatedly. + + Waits `msec` initially before calling `f` for the first time. + + Args: + f(function): The function to call repeatedly. + msec(float): How long to wait between calls in milliseconds. + *args: Positional arguments to pass to function. + **kwargs: Key arguments to pass to function. + """ + + @wrap_as_background_process(f.__name__) + async def background_call(*args, **kwargs): + await f(*args, **kwargs) + + if self._hs.config.run_background_tasks: + self._clock.looping_call(background_call, msec, *args, **kwargs) + else: + logger.warning( + "Not running looping call %s as the configuration forbids it", + f, + ) + + async def send_mail( + self, + recipient: str, + subject: str, + html: str, + text: str, + ): + """Send an email on behalf of the homeserver. + + Args: + recipient: The email address for the recipient. + subject: The email's subject. + html: The email's HTML content. + text: The email's text content. + """ + raw_to = email.utils.parseaddr(recipient)[1] + + multipart_msg = MIMEMultipart("alternative") + multipart_msg["Subject"] = subject + multipart_msg["From"] = self._from_string + multipart_msg["To"] = recipient + multipart_msg["Date"] = email.utils.formatdate() + multipart_msg["Message-ID"] = email.utils.make_msgid() + multipart_msg.attach(MIMEText(html, "html", "utf8")) + multipart_msg.attach(MIMEText(text, "plain", "utf8")) + + logger.info("Sending email to %s", recipient) + + await make_deferred_yieldable( + self._sendmail( + self._hs.config.email_smtp_host, + self._raw_from, + raw_to, + multipart_msg.as_string().encode("utf8"), + reactor=self._hs.get_reactor(), + port=self._hs.config.email_smtp_port, + requireAuthentication=self._hs.config.email_smtp_user is not None, + username=self._hs.config.email_smtp_user, + password=self._hs.config.email_smtp_pass, + requireTransportSecurity=self._hs.config.require_transport_security, + ) + ) + + def read_templates( + self, + filenames: List[str], + custom_template_directory: Optional[str] = None, + ) -> List[jinja2.Template]: + """Read and load the content of the template files at the given location. + By default, Synapse will look for these templates in its configured template + directory, but another directory to search in can be provided. + + Args: + filenames: The name of the template files to look for. + custom_template_directory: An additional directory to look for the files in. + + Returns: + A list containing the loaded templates, with the orders matching the one of + the filenames parameter. + """ + return self._hs.config.read_templates(filenames, custom_template_directory) + + async def get_profile_for_user(self, localpart: str) -> ProfileInfo: + """Lookup the profile info for the user with the given localpart. + + Args: + localpart: The localpart to lookup profile information for. + + Returns: + The profile information (i.e. display name and avatar URL). + """ + return await self._store.get_profileinfo(localpart) + + async def get_threepids_for_user(self, user_id: str) -> List[Dict[str, str]]: + """Lookup the threepids (email addresses and phone numbers) associated with the + given Matrix user ID. + + Args: + user_id: The Matrix user ID to lookup threepids for. + + Returns: + A list of threepids, each threepid being represented by a dictionary + containing a "medium" key which value is "email" for email addresses and + "msisdn" for phone numbers, and an "address" key which value is the + threepid's address. + """ + return await self._store.user_get_threepids(user_id) + + def current_time_ms(self) -> int: + """Get the current time in milliseconds.""" + return self._clock.time_msec() + class PublicRoomListManager: """Contains methods for adding to, removing from and querying whether a room From 163ca3886a13070b36e13be36e2060fbb2f201b1 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Mon, 26 Apr 2021 15:32:22 +0100 Subject: [PATCH 02/50] Change the account validity configuration * don't mention the legacy implementation anymore * allow configuring modules to handle account validity --- docs/sample_config.yaml | 99 ++++------------ synapse/config/account_validity.py | 179 ++++++++++++----------------- 2 files changed, 96 insertions(+), 182 deletions(-) diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index d260d762590a..4937d3f6500d 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -1371,87 +1371,28 @@ account_threepid_delegates: ## Account Validity ## -# Optional account validity configuration. This allows for accounts to be denied -# any request after a given period. -# -# Once this feature is enabled, Synapse will look for registered users without an -# expiration date at startup and will add one to every account it found using the -# current settings at that time. -# This means that, if a validity period is set, and Synapse is restarted (it will -# then derive an expiration date from the current validity period), and some time -# after that the validity period changes and Synapse is restarted, the users' -# expiration dates won't be updated unless their account is manually renewed. This -# date will be randomly selected within a range [now + period - d ; now + period], -# where d is equal to 10% of the validity period. +# Server admins can (optionally) get Synapse to check the validity of all user +# accounts against one or more custom modules. +# +# An account validity module must implement two APIs: +# +# * `user_expired`, which takes a Matrix user ID and returns one boolean to +# indicate whether the provided account has expired, and one boolean to +# indicate whether it succeeded in figuring out this info. If this second +# boolean value is False, the `user_expired function of the next module +# (in the order modules are configured in this file) is called. If there +# is no more module to use, Synapse will consider the account as not expired. +# +# * `on_user_registration`, which is called after any successful registration +# with the Matrix ID of the newly registered user. # account_validity: - # The account validity feature is disabled by default. Uncomment the - # following line to enable it. - # - #enabled: true - - # The period after which an account is valid after its registration. When - # renewing the account, its validity period will be extended by this amount - # of time. This parameter is required when using the account validity - # feature. - # - #period: 6w - - # The amount of time before an account's expiry date at which Synapse will - # send an email to the account's email address with a renewal link. By - # default, no such emails are sent. - # - # If you enable this setting, you will also need to fill out the 'email' and - # 'public_baseurl' configuration sections. - # - #renew_at: 1w - - # The subject of the email sent out with the renewal link. '%(app)s' can be - # used as a placeholder for the 'app_name' parameter from the 'email' - # section. - # - # Note that the placeholder must be written '%(app)s', including the - # trailing 's'. - # - # If this is not set, a default value is used. - # - #renew_email_subject: "Renew your %(app)s account" - - # Directory in which Synapse will try to find templates for the HTML files to - # serve to the user when trying to renew an account. If not set, default - # templates from within the Synapse package will be used. - # - # The currently available templates are: - # - # * account_renewed.html: Displayed to the user after they have successfully - # renewed their account. - # - # * account_previously_renewed.html: Displayed to the user if they attempt to - # renew their account with a token that is valid, but that has already - # been used. In this case the account is not renewed again. - # - # * invalid_token.html: Displayed to the user when they try to renew an account - # with an unknown or invalid renewal token. - # - # See https://github.com/matrix-org/synapse/tree/master/synapse/res/templates for - # default template contents. - # - # The file name of some of these templates can be configured below for legacy - # reasons. - # - #template_dir: "res/templates" - - # A custom file name for the 'account_renewed.html' template. - # - # If not set, the file is assumed to be named "account_renewed.html". - # - #account_renewed_html_path: "account_renewed.html" - - # A custom file name for the 'invalid_token.html' template. - # - # If not set, the file is assumed to be named "invalid_token.html". - # - #invalid_token_html_path: "invalid_token.html" + #- module: "my_custom_project.SomeAccountValidity" + # config: + # example_option: 'things' + #- module: "my_custom_project.SomeOtherAccountValidity" + # config: + # other_example_option: 'stuff' ## Metrics ### diff --git a/synapse/config/account_validity.py b/synapse/config/account_validity.py index c58a7d95a785..3d0e56d11e80 100644 --- a/synapse/config/account_validity.py +++ b/synapse/config/account_validity.py @@ -12,14 +12,70 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. +import logging + from synapse.config._base import Config, ConfigError +from synapse.util.module_loader import load_module + +logger = logging.getLogger(__name__) + +LEGACY_ACCOUNT_VALIDITY_IN_USE = """ +You are using the deprecated account validity feature. It is recommended to change your +configuration to be using one or more modules instead. This feature will be removed in a +future version of Synapse, at which point it will only be available through custom +modules. +See the sample documentation file for more information: +https://github.com/matrix-org/synapse/blob/master/docs/sample_config.yaml +--------------------------------------------------------------------------------------""" class AccountValidityConfig(Config): section = "account_validity" def read_config(self, config, **kwargs): - account_validity_config = config.get("account_validity") or {} + # Consider legacy account validity disabled unless proven otherwise + self.account_validity_enabled = False + self.account_validity_renew_by_email_enabled = False + + # Read and store template content. We need to do that regardless of whether the + # configuration is using modules or the legacy account validity implementation, + # because we need these templates to register the account validity servlets. + ( + self.account_validity_account_renewed_template, + self.account_validity_account_previously_renewed_template, + self.account_validity_invalid_token_template, + ) = self.read_templates( + [ + "account_renewed.html", + "account_previously_renewed.html", + "invalid_token.html", + ], + ) + + # Initialise the list of modules, which will stay empty if no modules or the + # legacy config was provided. + self.account_validity_modules = [] + + account_validity_config = config.get("account_validity") + if account_validity_config is None: + return + + if isinstance(account_validity_config, dict): + # If the configuration is for the legacy feature, then read it as such. + self.read_legacy_config(account_validity_config) + elif isinstance(account_validity_config, list): + for i, module in enumerate(account_validity_config): + config_path = ("account_validity", "" % i) + if not isinstance(module, dict): + raise ConfigError("expected a mapping", config_path) + + self.account_validity_modules.append(load_module(module, config_path)) + else: + raise ConfigError("account_validity syntax is incorrect") + + def read_legacy_config(self, account_validity_config): + logger.warning(LEGACY_ACCOUNT_VALIDITY_IN_USE) + self.account_validity_enabled = account_validity_config.get("enabled", False) self.account_validity_renew_by_email_enabled = ( "renew_at" in account_validity_config @@ -53,113 +109,30 @@ def read_config(self, config, **kwargs): if not self.public_baseurl: raise ConfigError("Can't send renewal emails without 'public_baseurl'") - # Load account validity templates. - account_validity_template_dir = account_validity_config.get("template_dir") - - account_renewed_template_filename = account_validity_config.get( - "account_renewed_html_path", "account_renewed.html" - ) - invalid_token_template_filename = account_validity_config.get( - "invalid_token_html_path", "invalid_token.html" - ) - - # Read and store template content - ( - self.account_validity_account_renewed_template, - self.account_validity_account_previously_renewed_template, - self.account_validity_invalid_token_template, - ) = self.read_templates( - [ - account_renewed_template_filename, - "account_previously_renewed.html", - invalid_token_template_filename, - ], - account_validity_template_dir, - ) - def generate_config_section(self, **kwargs): return """\ ## Account Validity ## - # Optional account validity configuration. This allows for accounts to be denied - # any request after a given period. + # Server admins can (optionally) get Synapse to check the validity of all user + # accounts against one or more custom modules. + # + # An account validity module must implement two APIs: + # + # * `user_expired`, which takes a Matrix user ID and returns one boolean to + # indicate whether the provided account has expired, and one boolean to + # indicate whether it succeeded in figuring out this info. If this second + # boolean value is False, the `user_expired function of the next module + # (in the order modules are configured in this file) is called. If there + # is no more module to use, Synapse will consider the account as not expired. # - # Once this feature is enabled, Synapse will look for registered users without an - # expiration date at startup and will add one to every account it found using the - # current settings at that time. - # This means that, if a validity period is set, and Synapse is restarted (it will - # then derive an expiration date from the current validity period), and some time - # after that the validity period changes and Synapse is restarted, the users' - # expiration dates won't be updated unless their account is manually renewed. This - # date will be randomly selected within a range [now + period - d ; now + period], - # where d is equal to 10% of the validity period. + # * `on_user_registration`, which is called after any successful registration + # with the Matrix ID of the newly registered user. # account_validity: - # The account validity feature is disabled by default. Uncomment the - # following line to enable it. - # - #enabled: true - - # The period after which an account is valid after its registration. When - # renewing the account, its validity period will be extended by this amount - # of time. This parameter is required when using the account validity - # feature. - # - #period: 6w - - # The amount of time before an account's expiry date at which Synapse will - # send an email to the account's email address with a renewal link. By - # default, no such emails are sent. - # - # If you enable this setting, you will also need to fill out the 'email' and - # 'public_baseurl' configuration sections. - # - #renew_at: 1w - - # The subject of the email sent out with the renewal link. '%(app)s' can be - # used as a placeholder for the 'app_name' parameter from the 'email' - # section. - # - # Note that the placeholder must be written '%(app)s', including the - # trailing 's'. - # - # If this is not set, a default value is used. - # - #renew_email_subject: "Renew your %(app)s account" - - # Directory in which Synapse will try to find templates for the HTML files to - # serve to the user when trying to renew an account. If not set, default - # templates from within the Synapse package will be used. - # - # The currently available templates are: - # - # * account_renewed.html: Displayed to the user after they have successfully - # renewed their account. - # - # * account_previously_renewed.html: Displayed to the user if they attempt to - # renew their account with a token that is valid, but that has already - # been used. In this case the account is not renewed again. - # - # * invalid_token.html: Displayed to the user when they try to renew an account - # with an unknown or invalid renewal token. - # - # See https://github.com/matrix-org/synapse/tree/master/synapse/res/templates for - # default template contents. - # - # The file name of some of these templates can be configured below for legacy - # reasons. - # - #template_dir: "res/templates" - - # A custom file name for the 'account_renewed.html' template. - # - # If not set, the file is assumed to be named "account_renewed.html". - # - #account_renewed_html_path: "account_renewed.html" - - # A custom file name for the 'invalid_token.html' template. - # - # If not set, the file is assumed to be named "invalid_token.html". - # - #invalid_token_html_path: "invalid_token.html" + #- module: "my_custom_project.SomeAccountValidity" + # config: + # example_option: 'things' + #- module: "my_custom_project.SomeOtherAccountValidity" + # config: + # other_example_option: 'stuff' """ From 2dd3a354b1eb827c2ea54c075bfaa6aa942a1d45 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Mon, 26 Apr 2021 15:33:07 +0100 Subject: [PATCH 03/50] Add an API for account validity modules --- synapse/account_validity/__init__.py | 109 +++++++++++++++++++++++++++ synapse/server.py | 5 ++ 2 files changed, 114 insertions(+) create mode 100644 synapse/account_validity/__init__.py diff --git a/synapse/account_validity/__init__.py b/synapse/account_validity/__init__.py new file mode 100644 index 000000000000..8fedd1b17511 --- /dev/null +++ b/synapse/account_validity/__init__.py @@ -0,0 +1,109 @@ +# -*- coding: utf-8 -*- +# Copyright 2021 The Matrix.org Foundation C.I.C. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +from typing import TYPE_CHECKING, Any, List, Tuple + +if TYPE_CHECKING: + import synapse.events + import synapse.server + + +class AccountValidity: + def __init__(self, hs: "synapse.server.HomeServer"): + self.modules = [] # type: List[Any] + api = hs.get_module_api() + + for module, config in hs.config.account_validity_modules: + self.modules.append(module(config=config, api=api)) + + async def on_legacy_send_mail(self, user_id: str): + """DEPRECATED: Function called when receiving a request on the deprecated + /_matrix/client/unstable/account_validity/send_mail endpoint. Modules that don't + reimplement the legacy email-based account validity feature should ignore this. + If several modules implement this hook, only the first one (in the orders modules + have been loaded at startup) gets called. + + Args: + user_id: The user ID to send a renewal email to. + + Raises: + NotImplementedError: No configured module implement this hook. + """ + implemented = False + for module in self.modules: + if hasattr(module, "on_legacy_send_mail"): + await module.on_legacy_send_mail(user_id) + return + + if not implemented: + raise NotImplementedError() + + async def on_legacy_renew(self, renewal_token: str) -> Tuple[bool, bool, int]: + """DEPRECATED: Function called when receiving a request on the deprecated + /_matrix/client/unstable/account_validity/renew endpoint. Modules that don't + reimplement the legacy email-based account validity feature should ignore this. + If several modules implement this hook, only the first one (in the orders modules + have been loaded at startup) gets called. + + Args: + renewal_token: The renewal token provided in the request. + + Raises: + NotImplementedError: No configured module implement this hook. + """ + implemented = False + for module in self.modules: + if hasattr(module, "on_legacy_renew"): + return await module.on_legacy_renew(renewal_token) + + if not implemented: + raise NotImplementedError() + + async def user_expired(self, user_id: str) -> bool: + """Check whether the user is expired. + + Modules are expected th return two booleans, the first one indicating whether the + user is expired, the second one indicating whether the module was able to + determine if the user was expired. + + If a module returns False on the second return value, the first one is ignored + and the next module (in the order the modules have been loaded at startup) is + called. If a module returns True on the second value, the first one is used and + this function returns. + If none of the modules have been able to determine whether the user is expired, + Synapse will consider them not expired to avoid locking them out of their account + in case of a technical incident. + + Args: + user_id: The user ID to check the expiration of. + + Returns: + Whether the user has expired. + """ + for module in self.modules: + expired, success = await module.user_expired(user_id) + if success: + return expired + + return False + + async def on_user_registration(self, user_id): + """Function called after successfully registering a new user. + + Args: + user_id: The Matrix ID of the newly registered user. + """ + for module in self.modules: + await module.on_user_registration(user_id) diff --git a/synapse/server.py b/synapse/server.py index 42d2fad8e869..a0ace918f14b 100644 --- a/synapse/server.py +++ b/synapse/server.py @@ -40,6 +40,7 @@ from twisted.mail.smtp import sendmail from twisted.web.iweb import IPolicyForHTTPS +from synapse.account_validity import AccountValidity from synapse.api.auth import Auth from synapse.api.filtering import Filtering from synapse.api.ratelimiting import Ratelimiter @@ -773,6 +774,10 @@ def get_outbound_redis_connection(self) -> Optional["RedisProtocol"]: reconnect=True, ) + @cache_in_self + def get_account_validity(self) -> AccountValidity: + return AccountValidity(self) + def should_send_federation(self) -> bool: "Should this server be sending federation traffic directly?" return self.config.send_federation From 850b3035ab20086e45c0b2ee8831bd0ef0a67116 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Mon, 26 Apr 2021 15:34:11 +0100 Subject: [PATCH 04/50] Plug the new account validity APIs onto the right places --- synapse/api/auth.py | 18 ++++++++++---- synapse/handlers/account_validity.py | 24 ++++++++++++++++++- synapse/handlers/register.py | 5 ++++ .../rest/client/v2_alpha/account_validity.py | 8 ++----- 4 files changed, 44 insertions(+), 11 deletions(-) diff --git a/synapse/api/auth.py b/synapse/api/auth.py index 872fd100cdd0..c0dbfed3669d 100644 --- a/synapse/api/auth.py +++ b/synapse/api/auth.py @@ -224,12 +224,22 @@ async def get_user_by_req( shadow_banned = user_info.shadow_banned # Deny the request if the user account has expired. - if self._account_validity_enabled and not allow_expired: - if await self.store.is_account_expired( - user_info.user_id, self.clock.time_msec() + if not allow_expired: + if await self.hs.get_account_validity().user_expired( + user_info.user_id + ) or ( + self._account_validity_enabled + and await self.store.is_account_expired( + user_info.user_id, self.clock.time_msec() + ) ): + # Raise the error if either an account validity module has determined + # the account has expired, or the legacy account validity + # implementation is enabled and determined the account has expired raise AuthError( - 403, "User account has expired", errcode=Codes.EXPIRED_ACCOUNT + 403, + "User account has expired", + errcode=Codes.EXPIRED_ACCOUNT, ) device_id = user_info.device_id diff --git a/synapse/handlers/account_validity.py b/synapse/handlers/account_validity.py index 5b927f10b3a9..3e64e72226d3 100644 --- a/synapse/handlers/account_validity.py +++ b/synapse/handlers/account_validity.py @@ -19,7 +19,7 @@ from email.mime.text import MIMEText from typing import TYPE_CHECKING, List, Optional, Tuple -from synapse.api.errors import StoreError, SynapseError +from synapse.api.errors import AuthError, StoreError, SynapseError from synapse.logging.context import make_deferred_yieldable from synapse.metrics.background_process_metrics import wrap_as_background_process from synapse.types import UserID @@ -39,6 +39,8 @@ def __init__(self, hs: "HomeServer"): self.sendmail = self.hs.get_sendmail() self.clock = self.hs.get_clock() + self._new_account_validity = hs.get_account_validity() + self._account_validity_enabled = ( hs.config.account_validity.account_validity_enabled ) @@ -109,6 +111,19 @@ async def send_renewal_email_to_user(self, user_id: str) -> None: Raises: SynapseError if the user is not set to renew. """ + # If a module supports sending a renewal email from here, do that, otherwise do + # the legacy dance. + try: + await self._new_account_validity.on_legacy_send_mail(user_id) + return + except NotImplementedError: + pass + + if not self._account_validity_renew_by_email_enabled: + raise AuthError( + 403, "Account renewal via email is disabled on this server." + ) + expiration_ts = await self.store.get_expiration_ts_for_user(user_id) # If this user isn't set to be expired, raise an error. @@ -253,6 +268,13 @@ async def renew_account(self, renewal_token: str) -> Tuple[bool, bool, int]: * An int representing the user's expiry timestamp as milliseconds since the epoch, or 0 if the token was invalid. """ + # If a module supports triggering a renew from here, do that, otherwise do the + # legacy dance. + try: + return await self._new_account_validity.on_legacy_renew(renewal_token) + except NotImplementedError: + pass + try: ( user_id, diff --git a/synapse/handlers/register.py b/synapse/handlers/register.py index 007fb1284091..b865377f120e 100644 --- a/synapse/handlers/register.py +++ b/synapse/handlers/register.py @@ -66,6 +66,7 @@ def __init__(self, hs: "HomeServer"): self.identity_handler = self.hs.get_identity_handler() self.ratelimiter = hs.get_registration_ratelimiter() self.macaroon_gen = hs.get_macaroon_generator() + self._account_validity = hs.get_account_validity() self._server_notices_mxid = hs.config.server_notices_mxid self._server_name = hs.hostname @@ -657,6 +658,10 @@ async def register_with_store( shadow_banned=shadow_banned, ) + # Only call the account validity module(s) on the main process, to avoid + # repeating e.g. database writes on all of the workers. + await self._account_validity.on_user_registration(user_id) + async def register_device( self, user_id: str, diff --git a/synapse/rest/client/v2_alpha/account_validity.py b/synapse/rest/client/v2_alpha/account_validity.py index 2d1ad3d3fbf0..c6c66c3aa223 100644 --- a/synapse/rest/client/v2_alpha/account_validity.py +++ b/synapse/rest/client/v2_alpha/account_validity.py @@ -14,7 +14,7 @@ import logging -from synapse.api.errors import AuthError, SynapseError +from synapse.api.errors import SynapseError from synapse.http.server import respond_with_html from synapse.http.servlet import RestServlet @@ -85,6 +85,7 @@ def __init__(self, hs): super().__init__() self.hs = hs + self.account_validity = hs.get_account_validity() self.account_activity_handler = hs.get_account_validity_handler() self.auth = hs.get_auth() self.account_validity_renew_by_email_enabled = ( @@ -92,11 +93,6 @@ def __init__(self, hs): ) async def on_POST(self, request): - if not self.account_validity_renew_by_email_enabled: - raise AuthError( - 403, "Account renewal via email is disabled on this server." - ) - requester = await self.auth.get_user_by_req(request, allow_expired=True) user_id = requester.user.to_string() await self.account_activity_handler.send_renewal_email_to_user(user_id) From f8c8ca88d44395bba660ff1ea69015e57b1274fc Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Mon, 26 Apr 2021 15:48:11 +0100 Subject: [PATCH 05/50] Changelog --- changelog.d/9884.removal | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/9884.removal diff --git a/changelog.d/9884.removal b/changelog.d/9884.removal new file mode 100644 index 000000000000..fd070a3f3e46 --- /dev/null +++ b/changelog.d/9884.removal @@ -0,0 +1 @@ +Deprecate the built-in account validity feature in favour of custom modules. From f492ef36f0247087617cabf744684e778e3a2932 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Mon, 26 Apr 2021 16:51:46 +0100 Subject: [PATCH 06/50] Allow modules to query whether the user is a server admin --- synapse/module_api/__init__.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/synapse/module_api/__init__.py b/synapse/module_api/__init__.py index eb22ea7c23df..2179282fad32 100644 --- a/synapse/module_api/__init__.py +++ b/synapse/module_api/__init__.py @@ -148,6 +148,17 @@ def get_user_by_req( """ return self._hs.get_auth().get_user_by_req(req, allow_guest, allow_expired) + async def is_user_admin(self, user_id: str) -> bool: + """Checks if a user is a server admin. + + Args: + user_id: The Matrix ID of the user to check. + + Returns: + True if the user is a server admin, False otherwise. + """ + return await self._store.is_server_admin(UserID.from_string(user_id)) + def get_qualified_user_id(self, username): """Qualify a user id, if necessary From 5596cfe2b03f1979669dfb3b0449ecc891eded9b Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Mon, 26 Apr 2021 17:28:52 +0100 Subject: [PATCH 07/50] The module API already exposes run_in_background which makes background_call_async redundant --- synapse/module_api/__init__.py | 22 ---------------------- 1 file changed, 22 deletions(-) diff --git a/synapse/module_api/__init__.py b/synapse/module_api/__init__.py index 2179282fad32..af90455b608f 100644 --- a/synapse/module_api/__init__.py +++ b/synapse/module_api/__init__.py @@ -500,28 +500,6 @@ async def send_local_online_presence_to(self, users: Iterable[str]) -> None: presence_events ) - def background_call_async(self, f: Callable, *args, **kwargs): - """Wraps an async function as a background process and runs it. - - Args: - f(function): The function to call. - *args: Positional arguments to pass to function. - **kwargs: Key arguments to pass to function. - - """ - - @wrap_as_background_process(f.__name__) - async def background_call(*args, **kwargs): - await f(*args, **kwargs) - - if self._hs.config.run_background_tasks: - self._clock.call_later(0.0, background_call, *args, **kwargs) - else: - logger.warning( - "Not running looping call %s as the configuration forbids it", - f, - ) - def looping_background_call_async(self, f: Callable, msec: float, *args, **kwargs): """Wraps an async function as a background process and calls it repeatedly. From 456a06c0550cb640c05e32ac9d7b455a33c86395 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Tue, 27 Apr 2021 11:56:44 +0100 Subject: [PATCH 08/50] Fix user authentication in the module API Also stop calling get_auth() in get_user_by_req, we don't need that anymore. --- synapse/module_api/__init__.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/synapse/module_api/__init__.py b/synapse/module_api/__init__.py index af90455b608f..5e043344d5a5 100644 --- a/synapse/module_api/__init__.py +++ b/synapse/module_api/__init__.py @@ -65,6 +65,7 @@ def __init__(self, hs, auth_handler): self._hs = hs self._store = hs.get_datastore() + self._auth = hs.get_auth() self._auth_handler = auth_handler self._server_name = hs.hostname self._presence_stream = hs.get_event_sources().sources["presence"] @@ -146,7 +147,7 @@ def get_user_by_req( synapse.api.errors.AuthError: if no user by that token exists, or the token is invalid. """ - return self._hs.get_auth().get_user_by_req(req, allow_guest, allow_expired) + return self._auth.get_user_by_req(req, allow_guest, allow_expired=allow_expired) async def is_user_admin(self, user_id: str) -> bool: """Checks if a user is a server admin. From 94706a91ef818282990df21a9a993e7d40363454 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Tue, 27 Apr 2021 16:27:31 +0100 Subject: [PATCH 09/50] Add a hook for the legacy admin API --- synapse/account_validity/__init__.py | 22 ++++++++++++++++++++++ synapse/rest/admin/users.py | 20 ++++++++++++-------- 2 files changed, 34 insertions(+), 8 deletions(-) diff --git a/synapse/account_validity/__init__.py b/synapse/account_validity/__init__.py index 8fedd1b17511..5fbef33572f3 100644 --- a/synapse/account_validity/__init__.py +++ b/synapse/account_validity/__init__.py @@ -71,6 +71,28 @@ async def on_legacy_renew(self, renewal_token: str) -> Tuple[bool, bool, int]: if not implemented: raise NotImplementedError() + async def on_legacy_admin_request(self, request) -> int: + """DEPRECATED: Function called when receiving a request on the deprecated + /_synapse/admin/account_validity endpoint, after the requester has been + identified as a server admin. Modules that don't reimplement the legacy + email-based account validity feature should ignore this. + If several modules implement this hook, only the first one (in the orders modules + have been loaded at startup) gets called. + + Args: + request: The admin request + + Raises: + NotImplementedError: No configured module implement this hook. + """ + implemented = False + for module in self.modules: + if hasattr(module, "on_legacy_admin_request"): + return await module.on_legacy_admin_request(request) + + if not implemented: + raise NotImplementedError() + async def user_expired(self, user_id: str) -> bool: """Check whether the user is expired. diff --git a/synapse/rest/admin/users.py b/synapse/rest/admin/users.py index edda7861fae5..82f14b695ebc 100644 --- a/synapse/rest/admin/users.py +++ b/synapse/rest/admin/users.py @@ -560,20 +560,24 @@ def __init__(self, hs): self.hs = hs self.account_activity_handler = hs.get_account_validity_handler() self.auth = hs.get_auth() + self.account_validity = hs.get_account_validity() async def on_POST(self, request: SynapseRequest) -> Tuple[int, JsonDict]: await assert_requester_is_admin(self.auth, request) - body = parse_json_object_from_request(request) + try: + expiration_ts = await self.account_validity.on_legacy_admin_request(request) + except NotImplementedError: + body = parse_json_object_from_request(request) - if "user_id" not in body: - raise SynapseError(400, "Missing property 'user_id' in the request body") + if "user_id" not in body: + raise SynapseError(400, "Missing property 'user_id' in the request body") - expiration_ts = await self.account_activity_handler.renew_account_for_user( - body["user_id"], - body.get("expiration_ts"), - not body.get("enable_renewal_emails", True), - ) + expiration_ts = await self.account_activity_handler.renew_account_for_user( + body["user_id"], + body.get("expiration_ts"), + not body.get("enable_renewal_emails", True), + ) res = {"expiration_ts": expiration_ts} return 200, res From 576b9f3830ab646debd313eaa64d390fa31556c2 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Tue, 27 Apr 2021 16:37:51 +0100 Subject: [PATCH 10/50] Fix comment with right URL path --- synapse/account_validity/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/synapse/account_validity/__init__.py b/synapse/account_validity/__init__.py index 5fbef33572f3..c8f8745d1705 100644 --- a/synapse/account_validity/__init__.py +++ b/synapse/account_validity/__init__.py @@ -73,8 +73,8 @@ async def on_legacy_renew(self, renewal_token: str) -> Tuple[bool, bool, int]: async def on_legacy_admin_request(self, request) -> int: """DEPRECATED: Function called when receiving a request on the deprecated - /_synapse/admin/account_validity endpoint, after the requester has been - identified as a server admin. Modules that don't reimplement the legacy + /_synapse/admin/v1/account_validity/validity endpoint, after the requester has + been identified as a server admin. Modules that don't reimplement the legacy email-based account validity feature should ignore this. If several modules implement this hook, only the first one (in the orders modules have been loaded at startup) gets called. From 907b655bb95640790092cc71912bc45e8ce908a2 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Tue, 27 Apr 2021 17:19:30 +0100 Subject: [PATCH 11/50] Add a deprecation notice to the upgrade notes --- UPGRADE.rst | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/UPGRADE.rst b/UPGRADE.rst index eff976017dd5..18f739bc7eb7 100644 --- a/UPGRADE.rst +++ b/UPGRADE.rst @@ -85,6 +85,19 @@ for example: wget https://packages.matrix.org/debian/pool/main/m/matrix-synapse-py3/matrix-synapse-py3_1.3.0+stretch1_amd64.deb dpkg -i matrix-synapse-py3_1.3.0+stretch1_amd64.deb +Upgrading to v1.34.0 +==================== + +Deprecation of the built-in account validity feature +---------------------------------------------------- + +As of Synapse v1.34.0, the built-in account validity feature is deprecated in favour of +plugin modules. If you are using this feature and want to keep its current behaviour, +please use the `synapse-email-account-validity `_ +module. + +Synapse's built-in account validity feature will be removed in a future release. + Upgrading to v1.33.0 ==================== From 9a9e83c43be76df8f81a9f9a30ae4fcb8246c38a Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Tue, 27 Apr 2021 17:20:59 +0100 Subject: [PATCH 12/50] Lint --- synapse/rest/admin/users.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/synapse/rest/admin/users.py b/synapse/rest/admin/users.py index 82f14b695ebc..b53d4f5e8706 100644 --- a/synapse/rest/admin/users.py +++ b/synapse/rest/admin/users.py @@ -571,7 +571,9 @@ async def on_POST(self, request: SynapseRequest) -> Tuple[int, JsonDict]: body = parse_json_object_from_request(request) if "user_id" not in body: - raise SynapseError(400, "Missing property 'user_id' in the request body") + raise SynapseError( + 400, "Missing property 'user_id' in the request body", + ) expiration_ts = await self.account_activity_handler.renew_account_for_user( body["user_id"], From 588fa411d72d04ca6e564cff92db9491edb59e91 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Tue, 27 Apr 2021 17:25:09 +0100 Subject: [PATCH 13/50] Lint --- synapse/rest/admin/users.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/synapse/rest/admin/users.py b/synapse/rest/admin/users.py index b53d4f5e8706..be46cd35dccb 100644 --- a/synapse/rest/admin/users.py +++ b/synapse/rest/admin/users.py @@ -572,7 +572,8 @@ async def on_POST(self, request: SynapseRequest) -> Tuple[int, JsonDict]: if "user_id" not in body: raise SynapseError( - 400, "Missing property 'user_id' in the request body", + 400, + "Missing property 'user_id' in the request body", ) expiration_ts = await self.account_activity_handler.renew_account_for_user( From 13a72ee0956c6cedc01d7997eb6c56588e68e3e0 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Tue, 27 Apr 2021 17:29:42 +0100 Subject: [PATCH 14/50] Mention the module in the config warning (+typo) --- synapse/config/account_validity.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/synapse/config/account_validity.py b/synapse/config/account_validity.py index 3d0e56d11e80..bd95e0a5eacf 100644 --- a/synapse/config/account_validity.py +++ b/synapse/config/account_validity.py @@ -24,8 +24,11 @@ configuration to be using one or more modules instead. This feature will be removed in a future version of Synapse, at which point it will only be available through custom modules. -See the sample documentation file for more information: +See the sample configuration file for more information: https://github.com/matrix-org/synapse/blob/master/docs/sample_config.yaml +The behaviour and features of the deprecated account validity feature have been ported +to a dedicated module: +https://github.com/matrix-org/synapse-email-account-validity --------------------------------------------------------------------------------------""" From fe2e8ca359996badd725e78cdf367b3467ee3c24 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Fri, 7 May 2021 17:26:43 +0100 Subject: [PATCH 15/50] Incorporate part of the review --- docs/account_validity.md | 50 ++++++++++++++++++++++++++++ docs/sample_config.yaml | 29 ++++++---------- synapse/account_validity/__init__.py | 20 +++++------ synapse/api/auth.py | 2 +- synapse/config/account_validity.py | 43 ++++++++---------------- synapse/module_api/__init__.py | 20 +++++------ 6 files changed, 93 insertions(+), 71 deletions(-) create mode 100644 docs/account_validity.md diff --git a/docs/account_validity.md b/docs/account_validity.md new file mode 100644 index 000000000000..e85230de4e6d --- /dev/null +++ b/docs/account_validity.md @@ -0,0 +1,50 @@ +# Account validity + +Synapse supports checking the validity of an account against one or more plugin module(s). + +An account validity plugin module is a Python class that exposes two functions: + +* `is_user_expired`, which checks if the account for the provided Matrix user ID has + expired. It returns a boolean to indicate whether the account has expired, or None if + it failed to figure it out. If this function returns `True`, Synapse will block any + request (apart from logout ones). If it returns `None`, Synapse will ask the next + module (in the order they appear in Synapse's configuration file), or consider the user + as not expired if it's reached the end of the list. +* `on_user_registration`, which is called after any successful registration + with the Matrix ID of the newly registered user. + + +## Example + +```python +import time +from typing import Optional + +from synapse.module_api import ModuleApi + +from my_module.store import Store, StoreException + +class ExampleAccountValidity: + def __init__(self, config: dict, api: ModuleApi): + self.config = config + self.api = api + self.store = Store(config, api) + + @staticmethod + def parse_config(config): + return config + + async def is_user_expired(self, user_id: str) -> Optional[bool]: + now_ms = time.time() * 1000 + + # Try to figure out what the expiration date for this account is and whether the + # account has expired, return None if that failed. + try: + expiration_ts = await self.store.get_expiration_ts(user_id) + return expiration_ts <= now_ms + except StoreException: + return None + + async def on_user_registration(self, user_id: str) -> None: + await self.store.insert_user(user_id) +``` \ No newline at end of file diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index 4937d3f6500d..f2c67917b53f 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -1374,25 +1374,16 @@ account_threepid_delegates: # Server admins can (optionally) get Synapse to check the validity of all user # accounts against one or more custom modules. # -# An account validity module must implement two APIs: -# -# * `user_expired`, which takes a Matrix user ID and returns one boolean to -# indicate whether the provided account has expired, and one boolean to -# indicate whether it succeeded in figuring out this info. If this second -# boolean value is False, the `user_expired function of the next module -# (in the order modules are configured in this file) is called. If there -# is no more module to use, Synapse will consider the account as not expired. -# -# * `on_user_registration`, which is called after any successful registration -# with the Matrix ID of the newly registered user. -# -account_validity: - #- module: "my_custom_project.SomeAccountValidity" - # config: - # example_option: 'things' - #- module: "my_custom_project.SomeOtherAccountValidity" - # config: - # other_example_option: 'stuff' +# See the documentation on how to write an account validity module: +# https://github.com/matrix-org/synapse/blob/master/docs/account_validity.md +# +account_validity_modules: + #- module: "my_custom_project.SomeAccountValidity" + # config: + # example_option: 'things' + #- module: "my_custom_project.SomeOtherAccountValidity" + # config: + # other_example_option: 'stuff' ## Metrics ### diff --git a/synapse/account_validity/__init__.py b/synapse/account_validity/__init__.py index c8f8745d1705..c210909f7045 100644 --- a/synapse/account_validity/__init__.py +++ b/synapse/account_validity/__init__.py @@ -13,7 +13,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -from typing import TYPE_CHECKING, Any, List, Tuple +from typing import TYPE_CHECKING, Any, List, Optional, Tuple if TYPE_CHECKING: import synapse.events @@ -93,17 +93,15 @@ async def on_legacy_admin_request(self, request) -> int: if not implemented: raise NotImplementedError() - async def user_expired(self, user_id: str) -> bool: + async def is_user_expired(self, user_id: str) -> bool: """Check whether the user is expired. - Modules are expected th return two booleans, the first one indicating whether the - user is expired, the second one indicating whether the module was able to - determine if the user was expired. + Modules are expected to return either a boolean indicating whether the user is + expired, or None if it was not able to figure it out. - If a module returns False on the second return value, the first one is ignored - and the next module (in the order the modules have been loaded at startup) is - called. If a module returns True on the second value, the first one is used and - this function returns. + If a module returns None, the next module (in the order the modules have been + loaded at startup) is called. If it returns a boolean, its value is used and the + function returns. If none of the modules have been able to determine whether the user is expired, Synapse will consider them not expired to avoid locking them out of their account in case of a technical incident. @@ -115,8 +113,8 @@ async def user_expired(self, user_id: str) -> bool: Whether the user has expired. """ for module in self.modules: - expired, success = await module.user_expired(user_id) - if success: + expired = await module.is_user_expired(user_id) # type: Optional[bool] + if expired is not None: return expired return False diff --git a/synapse/api/auth.py b/synapse/api/auth.py index c0dbfed3669d..2c4ffb84ecf5 100644 --- a/synapse/api/auth.py +++ b/synapse/api/auth.py @@ -225,7 +225,7 @@ async def get_user_by_req( # Deny the request if the user account has expired. if not allow_expired: - if await self.hs.get_account_validity().user_expired( + if await self.hs.get_account_validity().is_user_expired( user_info.user_id ) or ( self._account_validity_enabled diff --git a/synapse/config/account_validity.py b/synapse/config/account_validity.py index bd95e0a5eacf..348155c6f243 100644 --- a/synapse/config/account_validity.py +++ b/synapse/config/account_validity.py @@ -59,22 +59,16 @@ def read_config(self, config, **kwargs): # legacy config was provided. self.account_validity_modules = [] - account_validity_config = config.get("account_validity") - if account_validity_config is None: - return - - if isinstance(account_validity_config, dict): - # If the configuration is for the legacy feature, then read it as such. - self.read_legacy_config(account_validity_config) - elif isinstance(account_validity_config, list): - for i, module in enumerate(account_validity_config): + if config.get("account_validity_modules") is not None: + for i, module in enumerate(config.get("account_validity_modules")): config_path = ("account_validity", "" % i) if not isinstance(module, dict): raise ConfigError("expected a mapping", config_path) self.account_validity_modules.append(load_module(module, config_path)) - else: - raise ConfigError("account_validity syntax is incorrect") + elif config.get("account_validity") is not None: + # If the configuration is for the legacy feature, then read it as such. + self.read_legacy_config(config.get("account_validity")) def read_legacy_config(self, account_validity_config): logger.warning(LEGACY_ACCOUNT_VALIDITY_IN_USE) @@ -119,23 +113,14 @@ def generate_config_section(self, **kwargs): # Server admins can (optionally) get Synapse to check the validity of all user # accounts against one or more custom modules. # - # An account validity module must implement two APIs: - # - # * `user_expired`, which takes a Matrix user ID and returns one boolean to - # indicate whether the provided account has expired, and one boolean to - # indicate whether it succeeded in figuring out this info. If this second - # boolean value is False, the `user_expired function of the next module - # (in the order modules are configured in this file) is called. If there - # is no more module to use, Synapse will consider the account as not expired. - # - # * `on_user_registration`, which is called after any successful registration - # with the Matrix ID of the newly registered user. + # See the documentation on how to write an account validity module: + # https://github.com/matrix-org/synapse/blob/master/docs/account_validity.md # - account_validity: - #- module: "my_custom_project.SomeAccountValidity" - # config: - # example_option: 'things' - #- module: "my_custom_project.SomeOtherAccountValidity" - # config: - # other_example_option: 'stuff' + account_validity_modules: + #- module: "my_custom_project.SomeAccountValidity" + # config: + # example_option: 'things' + #- module: "my_custom_project.SomeOtherAccountValidity" + # config: + # other_example_option: 'stuff' """ diff --git a/synapse/module_api/__init__.py b/synapse/module_api/__init__.py index 5e043344d5a5..5512b5c2b1b0 100644 --- a/synapse/module_api/__init__.py +++ b/synapse/module_api/__init__.py @@ -77,8 +77,10 @@ def __init__(self, hs, auth_handler): app_name = self._hs.config.email_app_name self._from_string = self._hs.config.email_notif_from % {"app": app_name} - except Exception: - # If substitution failed, fall back to the bare strings. + except (KeyError, TypeError): + # If substitution failed (which can happen if the string contains + # placeholders other than just "app", or if the type of the placeholder is + # not a string), fall back to the bare strings. self._from_string = self._hs.config.email_notif_from self._raw_from = email.utils.parseaddr(self._from_string)[1] @@ -314,7 +316,7 @@ def invalidate_access_token(self, access_token): """ # see if the access token corresponds to a device user_info = yield defer.ensureDeferred( - self._hs.get_auth().get_user_by_access_token(access_token) + self._auth.get_user_by_access_token(access_token) ) device_id = user_info.get("device_id") user_id = user_info["user"].to_string() @@ -588,10 +590,10 @@ def read_templates( return self._hs.config.read_templates(filenames, custom_template_directory) async def get_profile_for_user(self, localpart: str) -> ProfileInfo: - """Lookup the profile info for the user with the given localpart. + """Look up the profile info for the user with the given localpart. Args: - localpart: The localpart to lookup profile information for. + localpart: The localpart to look up profile information for. Returns: The profile information (i.e. display name and avatar URL). @@ -599,11 +601,11 @@ async def get_profile_for_user(self, localpart: str) -> ProfileInfo: return await self._store.get_profileinfo(localpart) async def get_threepids_for_user(self, user_id: str) -> List[Dict[str, str]]: - """Lookup the threepids (email addresses and phone numbers) associated with the + """Look up the threepids (email addresses and phone numbers) associated with the given Matrix user ID. Args: - user_id: The Matrix user ID to lookup threepids for. + user_id: The Matrix user ID to look up threepids for. Returns: A list of threepids, each threepid being represented by a dictionary @@ -613,10 +615,6 @@ async def get_threepids_for_user(self, user_id: str) -> List[Dict[str, str]]: """ return await self._store.user_get_threepids(user_id) - def current_time_ms(self) -> int: - """Get the current time in milliseconds.""" - return self._clock.time_msec() - class PublicRoomListManager: """Contains methods for adding to, removing from and querying whether a room From 2147f06e718dbd5db47d192239f6138fa425c079 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 13 May 2021 12:11:12 +0100 Subject: [PATCH 16/50] Split multiplart email sending into a dedicated handler --- synapse/handlers/account_validity.py | 55 ++++--------------- synapse/handlers/send_email.py | 81 ++++++++++++++++++++++++++++ synapse/push/mailer.py | 52 +++--------------- synapse/server.py | 5 ++ 4 files changed, 104 insertions(+), 89 deletions(-) create mode 100644 synapse/handlers/send_email.py diff --git a/synapse/handlers/account_validity.py b/synapse/handlers/account_validity.py index 5b927f10b3a9..14b0f2d04d5a 100644 --- a/synapse/handlers/account_validity.py +++ b/synapse/handlers/account_validity.py @@ -15,12 +15,9 @@ import email.mime.multipart import email.utils import logging -from email.mime.multipart import MIMEMultipart -from email.mime.text import MIMEText from typing import TYPE_CHECKING, List, Optional, Tuple from synapse.api.errors import StoreError, SynapseError -from synapse.logging.context import make_deferred_yieldable from synapse.metrics.background_process_metrics import wrap_as_background_process from synapse.types import UserID from synapse.util import stringutils @@ -36,9 +33,11 @@ def __init__(self, hs: "HomeServer"): self.hs = hs self.config = hs.config self.store = self.hs.get_datastore() - self.sendmail = self.hs.get_sendmail() + self.send_email_handler = self.hs.get_send_email_handler() self.clock = self.hs.get_clock() + self.app_name = self.hs.config.email_app_name + self._account_validity_enabled = ( hs.config.account_validity.account_validity_enabled ) @@ -63,23 +62,10 @@ def __init__(self, hs: "HomeServer"): self._template_text = ( hs.config.account_validity.account_validity_template_text ) - account_validity_renew_email_subject = ( + self._renew_email_subject = ( hs.config.account_validity.account_validity_renew_email_subject ) - try: - app_name = hs.config.email_app_name - - self._subject = account_validity_renew_email_subject % {"app": app_name} - - self._from_string = hs.config.email_notif_from % {"app": app_name} - except Exception: - # If substitution failed, fall back to the bare strings. - self._subject = account_validity_renew_email_subject - self._from_string = hs.config.email_notif_from - - self._raw_from = email.utils.parseaddr(self._from_string)[1] - # Check the renewal emails to send and send them every 30min. if hs.config.run_background_tasks: self.clock.looping_call(self._send_renewal_emails, 30 * 60 * 1000) @@ -159,38 +145,17 @@ async def _send_renewal_email(self, user_id: str, expiration_ts: int) -> None: } html_text = self._template_html.render(**template_vars) - html_part = MIMEText(html_text, "html", "utf8") - plain_text = self._template_text.render(**template_vars) - text_part = MIMEText(plain_text, "plain", "utf8") for address in addresses: raw_to = email.utils.parseaddr(address)[1] - multipart_msg = MIMEMultipart("alternative") - multipart_msg["Subject"] = self._subject - multipart_msg["From"] = self._from_string - multipart_msg["To"] = address - multipart_msg["Date"] = email.utils.formatdate() - multipart_msg["Message-ID"] = email.utils.make_msgid() - multipart_msg.attach(text_part) - multipart_msg.attach(html_part) - - logger.info("Sending renewal email to %s", address) - - await make_deferred_yieldable( - self.sendmail( - self.hs.config.email_smtp_host, - self._raw_from, - raw_to, - multipart_msg.as_string().encode("utf8"), - reactor=self.hs.get_reactor(), - port=self.hs.config.email_smtp_port, - requireAuthentication=self.hs.config.email_smtp_user is not None, - username=self.hs.config.email_smtp_user, - password=self.hs.config.email_smtp_pass, - requireTransportSecurity=self.hs.config.require_transport_security, - ) + await self.send_email_handler.send_email( + email_address=raw_to, + subject=self._renew_email_subject, + app_name=self.app_name, + html=html_text, + text=plain_text, ) await self.store.set_renewal_mail_status(user_id=user_id, email_sent=True) diff --git a/synapse/handlers/send_email.py b/synapse/handlers/send_email.py new file mode 100644 index 000000000000..1db3135b09e6 --- /dev/null +++ b/synapse/handlers/send_email.py @@ -0,0 +1,81 @@ +# Copyright 2021 The Matrix.org C.I.C. Foundation +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import email.utils +from email.mime.text import MIMEText +from email.mime.multipart import MIMEMultipart +import logging +from typing import TYPE_CHECKING + +from synapse.logging.context import make_deferred_yieldable + +if TYPE_CHECKING: + from synapse.server import HomeServer + +logger = logging.getLogger(__name__) + + +class SendEmailHandler: + def __init__(self, hs: "HomeServer"): + self.hs = hs + self.sendmail = self.hs.get_sendmail() + + async def send_email( + self, + email_address: str, + subject: str, + app_name: str, + html: str, + text: str, + ) -> None: + """Send an email with the given information.""" + try: + from_string = self.hs.config.email_notif_from % {"app": app_name} + except TypeError: + from_string = self.hs.config.email_notif_from + + raw_from = email.utils.parseaddr(from_string)[1] + raw_to = email.utils.parseaddr(email_address)[1] + + if raw_to == "": + raise RuntimeError("Invalid 'to' address") + + html_part = MIMEText(html, "html", "utf8") + text_part = MIMEText(text, "plain", "utf8") + + multipart_msg = MIMEMultipart("alternative") + multipart_msg["Subject"] = subject + multipart_msg["From"] = from_string + multipart_msg["To"] = email_address + multipart_msg["Date"] = email.utils.formatdate() + multipart_msg["Message-ID"] = email.utils.make_msgid() + multipart_msg.attach(text_part) + multipart_msg.attach(html_part) + + logger.info("Sending email to %s" % email_address) + + await make_deferred_yieldable( + self.sendmail( + self.hs.config.email_smtp_host, + raw_from, + raw_to, + multipart_msg.as_string().encode("utf8"), + reactor=self.hs.get_reactor(), + port=self.hs.config.email_smtp_port, + requireAuthentication=self.hs.config.email_smtp_user is not None, + username=self.hs.config.email_smtp_user, + password=self.hs.config.email_smtp_pass, + requireTransportSecurity=self.hs.config.require_transport_security, + ) + ) diff --git a/synapse/push/mailer.py b/synapse/push/mailer.py index c4b43b0d3fc8..8c89d5ce3898 100644 --- a/synapse/push/mailer.py +++ b/synapse/push/mailer.py @@ -12,12 +12,8 @@ # See the License for the specific language governing permissions and # limitations under the License. -import email.mime.multipart -import email.utils import logging import urllib.parse -from email.mime.multipart import MIMEMultipart -from email.mime.text import MIMEText from typing import TYPE_CHECKING, Any, Dict, Iterable, List, Optional, TypeVar import bleach @@ -108,7 +104,7 @@ def __init__( self.template_html = template_html self.template_text = template_text - self.sendmail = self.hs.get_sendmail() + self.send_email_handler = hs.get_send_email_handler() self.store = self.hs.get_datastore() self.state_store = self.hs.get_storage().state self.macaroon_gen = self.hs.get_macaroon_generator() @@ -310,17 +306,6 @@ async def send_email( self, email_address: str, subject: str, extra_template_vars: Dict[str, Any] ) -> None: """Send an email with the given information and template text""" - try: - from_string = self.hs.config.email_notif_from % {"app": self.app_name} - except TypeError: - from_string = self.hs.config.email_notif_from - - raw_from = email.utils.parseaddr(from_string)[1] - raw_to = email.utils.parseaddr(email_address)[1] - - if raw_to == "": - raise RuntimeError("Invalid 'to' address") - template_vars = { "app_name": self.app_name, "server_name": self.hs.config.server.server_name, @@ -329,35 +314,14 @@ async def send_email( template_vars.update(extra_template_vars) html_text = self.template_html.render(**template_vars) - html_part = MIMEText(html_text, "html", "utf8") - plain_text = self.template_text.render(**template_vars) - text_part = MIMEText(plain_text, "plain", "utf8") - - multipart_msg = MIMEMultipart("alternative") - multipart_msg["Subject"] = subject - multipart_msg["From"] = from_string - multipart_msg["To"] = email_address - multipart_msg["Date"] = email.utils.formatdate() - multipart_msg["Message-ID"] = email.utils.make_msgid() - multipart_msg.attach(text_part) - multipart_msg.attach(html_part) - - logger.info("Sending email to %s" % email_address) - - await make_deferred_yieldable( - self.sendmail( - self.hs.config.email_smtp_host, - raw_from, - raw_to, - multipart_msg.as_string().encode("utf8"), - reactor=self.hs.get_reactor(), - port=self.hs.config.email_smtp_port, - requireAuthentication=self.hs.config.email_smtp_user is not None, - username=self.hs.config.email_smtp_user, - password=self.hs.config.email_smtp_pass, - requireTransportSecurity=self.hs.config.require_transport_security, - ) + + await self.send_email_handler.send_email( + email_address=email_address, + subject=subject, + app_name=self.app_name, + html=html_text, + text=plain_text, ) async def _get_room_vars( diff --git a/synapse/server.py b/synapse/server.py index 2337d2d9b450..fec0024c89a5 100644 --- a/synapse/server.py +++ b/synapse/server.py @@ -104,6 +104,7 @@ from synapse.handlers.room_member import RoomMemberHandler, RoomMemberMasterHandler from synapse.handlers.room_member_worker import RoomMemberWorkerHandler from synapse.handlers.search import SearchHandler +from synapse.handlers.send_email import SendEmailHandler from synapse.handlers.set_password import SetPasswordHandler from synapse.handlers.space_summary import SpaceSummaryHandler from synapse.handlers.sso import SsoHandler @@ -549,6 +550,10 @@ def get_deactivate_account_handler(self) -> DeactivateAccountHandler: def get_search_handler(self) -> SearchHandler: return SearchHandler(self) + @cache_in_self + def get_send_email_handler(self) -> SendEmailHandler: + return SendEmailHandler(self) + @cache_in_self def get_set_password_handler(self) -> SetPasswordHandler: return SetPasswordHandler(self) From 1df0c8ca3a8c16fa8a2a0471e47d71e07c71a88c Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 13 May 2021 12:11:52 +0100 Subject: [PATCH 17/50] Changelog --- changelog.d/9976.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/9976.misc diff --git a/changelog.d/9976.misc b/changelog.d/9976.misc new file mode 100644 index 000000000000..d5609f575479 --- /dev/null +++ b/changelog.d/9976.misc @@ -0,0 +1 @@ +Split multiplart email sending into a dedicated handler. From a26185ae33d289924a6e605726298763e6078518 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 13 May 2021 12:27:05 +0100 Subject: [PATCH 18/50] Lint --- changelog.d/{9976.misc => 9977.misc} | 0 synapse/handlers/send_email.py | 4 ++-- synapse/push/mailer.py | 1 - 3 files changed, 2 insertions(+), 3 deletions(-) rename changelog.d/{9976.misc => 9977.misc} (100%) diff --git a/changelog.d/9976.misc b/changelog.d/9977.misc similarity index 100% rename from changelog.d/9976.misc rename to changelog.d/9977.misc diff --git a/synapse/handlers/send_email.py b/synapse/handlers/send_email.py index 1db3135b09e6..71e829a35a0e 100644 --- a/synapse/handlers/send_email.py +++ b/synapse/handlers/send_email.py @@ -13,9 +13,9 @@ # limitations under the License. import email.utils -from email.mime.text import MIMEText -from email.mime.multipart import MIMEMultipart import logging +from email.mime.multipart import MIMEMultipart +from email.mime.text import MIMEText from typing import TYPE_CHECKING from synapse.logging.context import make_deferred_yieldable diff --git a/synapse/push/mailer.py b/synapse/push/mailer.py index 8c89d5ce3898..5f9ea5003a82 100644 --- a/synapse/push/mailer.py +++ b/synapse/push/mailer.py @@ -23,7 +23,6 @@ from synapse.api.errors import StoreError from synapse.config.emailconfig import EmailSubjectConfig from synapse.events import EventBase -from synapse.logging.context import make_deferred_yieldable from synapse.push.presentable_names import ( calculate_room_name, descriptor_from_member_events, From 76aed9e1c689846893a21527413d17291c23808e Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 13 May 2021 16:29:34 +0100 Subject: [PATCH 19/50] Incorporate review --- synapse/handlers/account_validity.py | 2 +- synapse/handlers/send_email.py | 43 +++++++++++++++++++--------- 2 files changed, 31 insertions(+), 14 deletions(-) diff --git a/synapse/handlers/account_validity.py b/synapse/handlers/account_validity.py index 5543ed57e722..42432ad70998 100644 --- a/synapse/handlers/account_validity.py +++ b/synapse/handlers/account_validity.py @@ -37,7 +37,7 @@ def __init__(self, hs: "HomeServer"): self.clock = self.hs.get_clock() self._new_account_validity = hs.get_account_validity() - self.app_name = self.hs.config.email_app_name + self._app_name = self.hs.config.email_app_name self._account_validity_enabled = ( hs.config.account_validity.account_validity_enabled diff --git a/synapse/handlers/send_email.py b/synapse/handlers/send_email.py index 71e829a35a0e..e9f6aef06f01 100644 --- a/synapse/handlers/send_email.py +++ b/synapse/handlers/send_email.py @@ -29,7 +29,16 @@ class SendEmailHandler: def __init__(self, hs: "HomeServer"): self.hs = hs - self.sendmail = self.hs.get_sendmail() + + self._sendmail = hs.get_sendmail() + self._reactor = hs.get_reactor() + + self._from = hs.config.email.email_notif_from + self._smtp_host = hs.config.email.email_smtp_host + self._smtp_port = hs.config.email.email_smtp_port + self._smtp_user = hs.config.email.email_smtp_user + self._smtp_pass = hs.config.email.email_smtp_pass + self._require_transport_security = hs.config.email.require_transport_security async def send_email( self, @@ -39,11 +48,19 @@ async def send_email( html: str, text: str, ) -> None: - """Send an email with the given information.""" + """Send a multipart email with the given information. + + Args: + email_address: The address to send the email to. + subject: The email's subject. + app_name: The app name to include in the From header. + html: The HTML content to include in the email. + text: The plain text content to include in the email. + """ try: - from_string = self.hs.config.email_notif_from % {"app": app_name} - except TypeError: - from_string = self.hs.config.email_notif_from + from_string = self._from % {"app": app_name} + except (KeyError, TypeError): + from_string = self._from raw_from = email.utils.parseaddr(from_string)[1] raw_to = email.utils.parseaddr(email_address)[1] @@ -66,16 +83,16 @@ async def send_email( logger.info("Sending email to %s" % email_address) await make_deferred_yieldable( - self.sendmail( - self.hs.config.email_smtp_host, + self._sendmail( + self._smtp_host, raw_from, raw_to, multipart_msg.as_string().encode("utf8"), - reactor=self.hs.get_reactor(), - port=self.hs.config.email_smtp_port, - requireAuthentication=self.hs.config.email_smtp_user is not None, - username=self.hs.config.email_smtp_user, - password=self.hs.config.email_smtp_pass, - requireTransportSecurity=self.hs.config.require_transport_security, + reactor=self._reactor, + port=self._smtp_port, + requireAuthentication=self._smtp_user is not None, + username=self._smtp_user, + password=self._smtp_pass, + requireTransportSecurity=self._require_transport_security, ) ) From efc82b7c014dba2112b7b8b1d1514007f60ab977 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 13 May 2021 17:30:00 +0200 Subject: [PATCH 20/50] Fix typo in changelog file Co-authored-by: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> --- changelog.d/9977.misc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog.d/9977.misc b/changelog.d/9977.misc index d5609f575479..093dffc6beac 100644 --- a/changelog.d/9977.misc +++ b/changelog.d/9977.misc @@ -1 +1 @@ -Split multiplart email sending into a dedicated handler. +Split multipart email sending into a dedicated handler. From 1df5d74d1379586972a50d8dbf79c66c764ef1e2 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 13 May 2021 16:29:34 +0100 Subject: [PATCH 21/50] Incorporate review --- synapse/handlers/account_validity.py | 2 +- synapse/handlers/send_email.py | 43 +++++++++++++++++++--------- 2 files changed, 31 insertions(+), 14 deletions(-) diff --git a/synapse/handlers/account_validity.py b/synapse/handlers/account_validity.py index 14b0f2d04d5a..43215cd0b3bc 100644 --- a/synapse/handlers/account_validity.py +++ b/synapse/handlers/account_validity.py @@ -36,7 +36,7 @@ def __init__(self, hs: "HomeServer"): self.send_email_handler = self.hs.get_send_email_handler() self.clock = self.hs.get_clock() - self.app_name = self.hs.config.email_app_name + self._app_name = self.hs.config.email_app_name self._account_validity_enabled = ( hs.config.account_validity.account_validity_enabled diff --git a/synapse/handlers/send_email.py b/synapse/handlers/send_email.py index 71e829a35a0e..e9f6aef06f01 100644 --- a/synapse/handlers/send_email.py +++ b/synapse/handlers/send_email.py @@ -29,7 +29,16 @@ class SendEmailHandler: def __init__(self, hs: "HomeServer"): self.hs = hs - self.sendmail = self.hs.get_sendmail() + + self._sendmail = hs.get_sendmail() + self._reactor = hs.get_reactor() + + self._from = hs.config.email.email_notif_from + self._smtp_host = hs.config.email.email_smtp_host + self._smtp_port = hs.config.email.email_smtp_port + self._smtp_user = hs.config.email.email_smtp_user + self._smtp_pass = hs.config.email.email_smtp_pass + self._require_transport_security = hs.config.email.require_transport_security async def send_email( self, @@ -39,11 +48,19 @@ async def send_email( html: str, text: str, ) -> None: - """Send an email with the given information.""" + """Send a multipart email with the given information. + + Args: + email_address: The address to send the email to. + subject: The email's subject. + app_name: The app name to include in the From header. + html: The HTML content to include in the email. + text: The plain text content to include in the email. + """ try: - from_string = self.hs.config.email_notif_from % {"app": app_name} - except TypeError: - from_string = self.hs.config.email_notif_from + from_string = self._from % {"app": app_name} + except (KeyError, TypeError): + from_string = self._from raw_from = email.utils.parseaddr(from_string)[1] raw_to = email.utils.parseaddr(email_address)[1] @@ -66,16 +83,16 @@ async def send_email( logger.info("Sending email to %s" % email_address) await make_deferred_yieldable( - self.sendmail( - self.hs.config.email_smtp_host, + self._sendmail( + self._smtp_host, raw_from, raw_to, multipart_msg.as_string().encode("utf8"), - reactor=self.hs.get_reactor(), - port=self.hs.config.email_smtp_port, - requireAuthentication=self.hs.config.email_smtp_user is not None, - username=self.hs.config.email_smtp_user, - password=self.hs.config.email_smtp_pass, - requireTransportSecurity=self.hs.config.require_transport_security, + reactor=self._reactor, + port=self._smtp_port, + requireAuthentication=self._smtp_user is not None, + username=self._smtp_user, + password=self._smtp_pass, + requireTransportSecurity=self._require_transport_security, ) ) From 0d7cb1601727d5dbfa490af319d744459806e5b8 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 13 May 2021 16:50:28 +0100 Subject: [PATCH 22/50] Typo --- synapse/handlers/account_validity.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/handlers/account_validity.py b/synapse/handlers/account_validity.py index 43215cd0b3bc..d752cf34f0cc 100644 --- a/synapse/handlers/account_validity.py +++ b/synapse/handlers/account_validity.py @@ -153,7 +153,7 @@ async def _send_renewal_email(self, user_id: str, expiration_ts: int) -> None: await self.send_email_handler.send_email( email_address=raw_to, subject=self._renew_email_subject, - app_name=self.app_name, + app_name=self._app_name, html=html_text, text=plain_text, ) From 7b553e616d5d5fcc06ca4ec1285f76076986a027 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Fri, 14 May 2021 12:15:56 +0100 Subject: [PATCH 23/50] Expose more things needed by the email account validity module on the module API --- synapse/module_api/__init__.py | 59 +++++++++++++++++----------------- synapse/module_api/errors.py | 7 +++- 2 files changed, 36 insertions(+), 30 deletions(-) diff --git a/synapse/module_api/__init__.py b/synapse/module_api/__init__.py index 5512b5c2b1b0..7ffb3b19de70 100644 --- a/synapse/module_api/__init__.py +++ b/synapse/module_api/__init__.py @@ -36,12 +36,20 @@ from synapse.events import EventBase from synapse.http.client import SimpleHttpClient from synapse.http.site import SynapseRequest +from synapse.http.servlet import parse_json_object_from_request +from synapse.http.server import ( + DirectServeHtmlResource, + DirectServeJsonResource, + respond_with_html, +) from synapse.logging.context import make_deferred_yieldable, run_in_background from synapse.metrics.background_process_metrics import wrap_as_background_process +from synapse.storage.database import DatabasePool, LoggingTransaction from synapse.storage.databases.main.roommember import ProfileInfo from synapse.storage.state import StateFilter from synapse.types import JsonDict, UserID, create_requester from synapse.util import Clock +from synapse.util.caches.descriptors import cached if TYPE_CHECKING: from synapse.server import HomeServer @@ -51,7 +59,20 @@ are loaded into Synapse. """ -__all__ = ["errors", "make_deferred_yieldable", "run_in_background", "ModuleApi"] +__all__ = [ + "errors", + "make_deferred_yieldable", + "parse_json_object_from_request", + "respond_with_html", + "run_in_background", + "cached", + "UserID", + "DatabasePool", + "LoggingTransaction", + "DirectServeHtmlResource", + "DirectServeJsonResource", + "ModuleApi", +] logger = logging.getLogger(__name__) @@ -61,7 +82,7 @@ class ModuleApi: can register new users etc if necessary. """ - def __init__(self, hs, auth_handler): + def __init__(self, hs: "HomeServer", auth_handler): self._hs = hs self._store = hs.get_datastore() @@ -70,8 +91,8 @@ def __init__(self, hs, auth_handler): self._server_name = hs.hostname self._presence_stream = hs.get_event_sources().sources["presence"] self._state = hs.get_state_handler() - self._sendmail = hs.get_sendmail() self._clock = hs.get_clock() # type: Clock + self._send_email_handler = hs.get_send_email_handler() try: app_name = self._hs.config.email_app_name @@ -542,32 +563,12 @@ async def send_mail( html: The email's HTML content. text: The email's text content. """ - raw_to = email.utils.parseaddr(recipient)[1] - - multipart_msg = MIMEMultipart("alternative") - multipart_msg["Subject"] = subject - multipart_msg["From"] = self._from_string - multipart_msg["To"] = recipient - multipart_msg["Date"] = email.utils.formatdate() - multipart_msg["Message-ID"] = email.utils.make_msgid() - multipart_msg.attach(MIMEText(html, "html", "utf8")) - multipart_msg.attach(MIMEText(text, "plain", "utf8")) - - logger.info("Sending email to %s", recipient) - - await make_deferred_yieldable( - self._sendmail( - self._hs.config.email_smtp_host, - self._raw_from, - raw_to, - multipart_msg.as_string().encode("utf8"), - reactor=self._hs.get_reactor(), - port=self._hs.config.email_smtp_port, - requireAuthentication=self._hs.config.email_smtp_user is not None, - username=self._hs.config.email_smtp_user, - password=self._hs.config.email_smtp_pass, - requireTransportSecurity=self._hs.config.require_transport_security, - ) + await self._send_email_handler.send_email( + email_address=recipient, + subject=subject, + app_name=None, + html=html, + text=text, ) def read_templates( diff --git a/synapse/module_api/errors.py b/synapse/module_api/errors.py index d24864c5492a..e14070eceaed 100644 --- a/synapse/module_api/errors.py +++ b/synapse/module_api/errors.py @@ -14,4 +14,9 @@ """Exception types which are exposed as part of the stable module API""" -from synapse.api.errors import RedirectException, SynapseError # noqa: F401 +from synapse.config._base import ConfigError +from synapse.api.errors import ( + InvalidClientCredentialsError, + RedirectException, + SynapseError, +)# noqa: F401 From d0a1cd545767371c54e05395fd9d72ceb3dddb12 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Wed, 19 May 2021 14:51:17 +0100 Subject: [PATCH 24/50] Update docs + lint --- UPGRADE.rst | 8 ++++++++ synapse/module_api/__init__.py | 4 ++-- synapse/module_api/errors.py | 6 +++--- 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/UPGRADE.rst b/UPGRADE.rst index 5965f9c1548c..f693e1105767 100644 --- a/UPGRADE.rst +++ b/UPGRADE.rst @@ -98,6 +98,14 @@ module. Synapse's built-in account validity feature will be removed in a future release. + +Removal of account validity template configuration settings +----------------------------------------------------------- + +In line with previous changes around templates management in Synapse, support has been +removed for the configuration settings ``template_dir``, ``account_renewed_html_path`` and +``invalid_token_html_path`` in the ``account_validity`` section of the configuration file. + Upgrading to v1.34.0 ==================== diff --git a/synapse/module_api/__init__.py b/synapse/module_api/__init__.py index 7ffb3b19de70..735b51baa6e7 100644 --- a/synapse/module_api/__init__.py +++ b/synapse/module_api/__init__.py @@ -35,13 +35,13 @@ from synapse.events import EventBase from synapse.http.client import SimpleHttpClient -from synapse.http.site import SynapseRequest -from synapse.http.servlet import parse_json_object_from_request from synapse.http.server import ( DirectServeHtmlResource, DirectServeJsonResource, respond_with_html, ) +from synapse.http.servlet import parse_json_object_from_request +from synapse.http.site import SynapseRequest from synapse.logging.context import make_deferred_yieldable, run_in_background from synapse.metrics.background_process_metrics import wrap_as_background_process from synapse.storage.database import DatabasePool, LoggingTransaction diff --git a/synapse/module_api/errors.py b/synapse/module_api/errors.py index e14070eceaed..ffeea0ee356c 100644 --- a/synapse/module_api/errors.py +++ b/synapse/module_api/errors.py @@ -14,9 +14,9 @@ """Exception types which are exposed as part of the stable module API""" -from synapse.config._base import ConfigError -from synapse.api.errors import ( +from synapse.api.errors import ( # noqa: F401 InvalidClientCredentialsError, RedirectException, SynapseError, -)# noqa: F401 +) +from synapse.config._base import ConfigError From 602d2ce301a767fbf876ad060634abf819ee05c8 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 20 May 2021 15:08:40 +0100 Subject: [PATCH 25/50] Get the email app name from the email config --- synapse/module_api/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/module_api/__init__.py b/synapse/module_api/__init__.py index 735b51baa6e7..0c2b4ead6381 100644 --- a/synapse/module_api/__init__.py +++ b/synapse/module_api/__init__.py @@ -145,7 +145,7 @@ def email_app_name(self): """Allow accessing the application name configured in the homeserver's configuration. """ - return self._hs.config.email_app_name + return self._hs.config.email.email_app_name def get_user_by_req( self, From 14885d321bbc3d2cafe812a0c80e87d283c81d77 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 20 May 2021 15:57:27 +0100 Subject: [PATCH 26/50] Fix types --- synapse/module_api/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/synapse/module_api/__init__.py b/synapse/module_api/__init__.py index dda0399d19c5..b45c04e1745e 100644 --- a/synapse/module_api/__init__.py +++ b/synapse/module_api/__init__.py @@ -141,7 +141,7 @@ def email_app_name(self): def get_user_by_req( self, - req: Request, + req: SynapseRequest, allow_guest: bool = False, allow_expired: bool = False, ): @@ -563,7 +563,7 @@ async def send_mail( await self._send_email_handler.send_email( email_address=recipient, subject=subject, - app_name=None, + app_name=self.email_app_name, html=html, text=text, ) From 3108884efea6e8069f55d91a30da70f51396f81c Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 20 May 2021 16:36:56 +0100 Subject: [PATCH 27/50] Fix imports --- synapse/module_api/__init__.py | 3 --- synapse/module_api/errors.py | 2 +- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/synapse/module_api/__init__.py b/synapse/module_api/__init__.py index b45c04e1745e..173fbccbf9c3 100644 --- a/synapse/module_api/__init__.py +++ b/synapse/module_api/__init__.py @@ -14,8 +14,6 @@ # limitations under the License. import email.utils import logging -from email.mime.multipart import MIMEMultipart -from email.mime.text import MIMEText from typing import ( TYPE_CHECKING, Any, @@ -31,7 +29,6 @@ import jinja2 from twisted.internet import defer -from twisted.web.server import Request from synapse.events import EventBase from synapse.http.client import SimpleHttpClient diff --git a/synapse/module_api/errors.py b/synapse/module_api/errors.py index ffeea0ee356c..98ea911a8195 100644 --- a/synapse/module_api/errors.py +++ b/synapse/module_api/errors.py @@ -19,4 +19,4 @@ RedirectException, SynapseError, ) -from synapse.config._base import ConfigError +from synapse.config._base import ConfigError # noqa: F401 From c52921eeedb61ddc3474c22d3feaa4e636f6cc0c Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 1 Jul 2021 18:50:19 +0100 Subject: [PATCH 28/50] Move the account validity module interface to the new system --- synapse/account_validity/__init__.py | 129 ------------------ synapse/api/auth.py | 3 +- synapse/config/account_validity.py | 36 +---- synapse/handlers/account_validity.py | 95 +++++++++++-- synapse/handlers/register.py | 4 +- synapse/module_api/__init__.py | 6 + synapse/rest/admin/users.py | 9 +- .../rest/client/v2_alpha/account_validity.py | 1 - synapse/server.py | 5 - 9 files changed, 104 insertions(+), 184 deletions(-) delete mode 100644 synapse/account_validity/__init__.py diff --git a/synapse/account_validity/__init__.py b/synapse/account_validity/__init__.py deleted file mode 100644 index c210909f7045..000000000000 --- a/synapse/account_validity/__init__.py +++ /dev/null @@ -1,129 +0,0 @@ -# -*- coding: utf-8 -*- -# Copyright 2021 The Matrix.org Foundation C.I.C. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -from typing import TYPE_CHECKING, Any, List, Optional, Tuple - -if TYPE_CHECKING: - import synapse.events - import synapse.server - - -class AccountValidity: - def __init__(self, hs: "synapse.server.HomeServer"): - self.modules = [] # type: List[Any] - api = hs.get_module_api() - - for module, config in hs.config.account_validity_modules: - self.modules.append(module(config=config, api=api)) - - async def on_legacy_send_mail(self, user_id: str): - """DEPRECATED: Function called when receiving a request on the deprecated - /_matrix/client/unstable/account_validity/send_mail endpoint. Modules that don't - reimplement the legacy email-based account validity feature should ignore this. - If several modules implement this hook, only the first one (in the orders modules - have been loaded at startup) gets called. - - Args: - user_id: The user ID to send a renewal email to. - - Raises: - NotImplementedError: No configured module implement this hook. - """ - implemented = False - for module in self.modules: - if hasattr(module, "on_legacy_send_mail"): - await module.on_legacy_send_mail(user_id) - return - - if not implemented: - raise NotImplementedError() - - async def on_legacy_renew(self, renewal_token: str) -> Tuple[bool, bool, int]: - """DEPRECATED: Function called when receiving a request on the deprecated - /_matrix/client/unstable/account_validity/renew endpoint. Modules that don't - reimplement the legacy email-based account validity feature should ignore this. - If several modules implement this hook, only the first one (in the orders modules - have been loaded at startup) gets called. - - Args: - renewal_token: The renewal token provided in the request. - - Raises: - NotImplementedError: No configured module implement this hook. - """ - implemented = False - for module in self.modules: - if hasattr(module, "on_legacy_renew"): - return await module.on_legacy_renew(renewal_token) - - if not implemented: - raise NotImplementedError() - - async def on_legacy_admin_request(self, request) -> int: - """DEPRECATED: Function called when receiving a request on the deprecated - /_synapse/admin/v1/account_validity/validity endpoint, after the requester has - been identified as a server admin. Modules that don't reimplement the legacy - email-based account validity feature should ignore this. - If several modules implement this hook, only the first one (in the orders modules - have been loaded at startup) gets called. - - Args: - request: The admin request - - Raises: - NotImplementedError: No configured module implement this hook. - """ - implemented = False - for module in self.modules: - if hasattr(module, "on_legacy_admin_request"): - return await module.on_legacy_admin_request(request) - - if not implemented: - raise NotImplementedError() - - async def is_user_expired(self, user_id: str) -> bool: - """Check whether the user is expired. - - Modules are expected to return either a boolean indicating whether the user is - expired, or None if it was not able to figure it out. - - If a module returns None, the next module (in the order the modules have been - loaded at startup) is called. If it returns a boolean, its value is used and the - function returns. - If none of the modules have been able to determine whether the user is expired, - Synapse will consider them not expired to avoid locking them out of their account - in case of a technical incident. - - Args: - user_id: The user ID to check the expiration of. - - Returns: - Whether the user has expired. - """ - for module in self.modules: - expired = await module.is_user_expired(user_id) # type: Optional[bool] - if expired is not None: - return expired - - return False - - async def on_user_registration(self, user_id): - """Function called after successfully registering a new user. - - Args: - user_id: The Matrix ID of the newly registered user. - """ - for module in self.modules: - await module.on_user_registration(user_id) diff --git a/synapse/api/auth.py b/synapse/api/auth.py index 2fc8fe1a9deb..8f0f2aaa0a3d 100644 --- a/synapse/api/auth.py +++ b/synapse/api/auth.py @@ -75,6 +75,7 @@ def __init__(self, hs: "HomeServer"): self.clock = hs.get_clock() self.store = hs.get_datastore() self.state = hs.get_state_handler() + self._account_validity_handler = hs.get_account_validity_handler() self.token_cache = LruCache( 10000, "token_cache" @@ -220,7 +221,7 @@ async def get_user_by_req( # Deny the request if the user account has expired. if not allow_expired: - if await self.hs.get_account_validity().is_user_expired( + if await self._account_validity_handler.is_user_expired( user_info.user_id ) or ( self._account_validity_enabled diff --git a/synapse/config/account_validity.py b/synapse/config/account_validity.py index f77873249525..6ce4312ad55e 100644 --- a/synapse/config/account_validity.py +++ b/synapse/config/account_validity.py @@ -56,21 +56,12 @@ def read_config(self, config, **kwargs): # Initialise the list of modules, which will stay empty if no modules or the # legacy config was provided. - self.account_validity_modules = [] - if config.get("account_validity_modules") is not None: - for i, module in enumerate(config.get("account_validity_modules")): - config_path = ("account_validity", "" % i) - if not isinstance(module, dict): - raise ConfigError("expected a mapping", config_path) + # If the configuration is for the legacy feature, then read it as such. + account_validity_config = config.get("account_validity") - self.account_validity_modules.append(load_module(module, config_path)) - elif config.get("account_validity") is not None: - # If the configuration is for the legacy feature, then read it as such. - self.read_legacy_config(config.get("account_validity")) - - def read_legacy_config(self, account_validity_config): - logger.warning(LEGACY_ACCOUNT_VALIDITY_IN_USE) + if account_validity_config: + logger.warning(LEGACY_ACCOUNT_VALIDITY_IN_USE) self.account_validity_enabled = account_validity_config.get("enabled", False) self.account_validity_renew_by_email_enabled = ( @@ -104,22 +95,3 @@ def read_legacy_config(self, account_validity_config): if self.account_validity_renew_by_email_enabled: if not self.public_baseurl: raise ConfigError("Can't send renewal emails without 'public_baseurl'") - - def generate_config_section(self, **kwargs): - return """\ - ## Account Validity ## - - # Server admins can (optionally) get Synapse to check the validity of all user - # accounts against one or more custom modules. - # - # See the documentation on how to write an account validity module: - # https://github.com/matrix-org/synapse/blob/master/docs/account_validity.md - # - account_validity_modules: - #- module: "my_custom_project.SomeAccountValidity" - # config: - # example_option: 'things' - #- module: "my_custom_project.SomeOtherAccountValidity" - # config: - # other_example_option: 'stuff' - """ diff --git a/synapse/handlers/account_validity.py b/synapse/handlers/account_validity.py index 6aada0c81d54..e26226eb6803 100644 --- a/synapse/handlers/account_validity.py +++ b/synapse/handlers/account_validity.py @@ -15,7 +15,9 @@ import email.mime.multipart import email.utils import logging -from typing import TYPE_CHECKING, List, Optional, Tuple +from typing import TYPE_CHECKING, Awaitable, Callable, List, Optional, Tuple + +from twisted.web.http import Request from synapse.api.errors import AuthError, StoreError, SynapseError from synapse.metrics.background_process_metrics import wrap_as_background_process @@ -27,6 +29,14 @@ logger = logging.getLogger(__name__) +IS_USER_EXPIRED_CALLBACK = Callable[[str], Awaitable[bool]] +ON_USER_REGISTRATION_CALLBACK = Callable[[str], Awaitable] +# Legacy callbacks to allow for a smoother transition between the legacy native account +# validity feature and synapse-email-account-validity. +ON_LEGACY_SEND_MAIL_CALLBACK = Callable[[str], Awaitable] +ON_LEGACY_RENEW_CALLBACK = Callable[[str], Awaitable[Tuple[bool, bool, int]]] +ON_LEGACY_ADMIN_REQUEST = Callable[[Request], Awaitable] + class AccountValidityHandler: def __init__(self, hs: "HomeServer"): @@ -35,7 +45,6 @@ def __init__(self, hs: "HomeServer"): self.store = self.hs.get_datastore() self.send_email_handler = self.hs.get_send_email_handler() self.clock = self.hs.get_clock() - self._new_account_validity = hs.get_account_validity() self._app_name = self.hs.config.email_app_name @@ -73,6 +82,76 @@ def __init__(self, hs: "HomeServer"): if hs.config.run_background_tasks: self.clock.looping_call(self._send_renewal_emails, 30 * 60 * 1000) + self._is_user_expired_callbacks: List[IS_USER_EXPIRED_CALLBACK] = [] + self._on_user_registration_callback: Optional[ + ON_USER_REGISTRATION_CALLBACK + ] = [] + self._on_legacy_send_mail_callback: Optional[ + ON_LEGACY_SEND_MAIL_CALLBACK + ] = None + self._on_legacy_renew_callback: Optional[ON_LEGACY_RENEW_CALLBACK] = None + + # The legacy admin requests callback isn't a protected attribute because we need + # to access it from the admin servlet, which is outside of this handler. + self.on_legacy_admin_request_callback: Optional[ON_LEGACY_ADMIN_REQUEST] = None + + def register_account_validity_callbacks( + self, + is_user_expired: Optional[IS_USER_EXPIRED_CALLBACK] = None, + on_user_registration: Optional[ON_USER_REGISTRATION_CALLBACK] = None, + on_legacy_send_mail: Optional[ON_LEGACY_SEND_MAIL_CALLBACK] = None, + on_legacy_renew: Optional[ON_LEGACY_RENEW_CALLBACK] = None, + on_legacy_admin_request: Optional[ON_LEGACY_ADMIN_REQUEST] = None, + ): + """Register callbacks from module for each hook.""" + if is_user_expired is not None: + self._is_user_expired_callbacks.append(is_user_expired) + + if on_user_registration is not None: + self._on_user_registration_callback = on_user_registration + + if on_legacy_send_mail is not None: + if self._on_legacy_send_mail_callback is not None: + raise RuntimeError("Tried to register on_legacy_send_mail twice") + + self._on_legacy_send_mail_callback = on_legacy_send_mail + + if on_legacy_renew is not None: + if self._on_legacy_renew_callback is not None: + raise RuntimeError("Tried to register on_legacy_renew twice") + + self._on_legacy_renew_callback = on_legacy_renew + + if on_legacy_admin_request is not None: + if self.on_legacy_admin_request_callback is not None: + raise RuntimeError("Tried to register on_legacy_admin_request twice") + + self.on_legacy_admin_request_callback = on_legacy_admin_request + + async def is_user_expired(self, user_id: str) -> bool: + """Checks if a user has expired against third-party modules. + + Args: + user_id: The user to check the expiry of. + + Returns: + Whether the user has expired. + """ + for callback in self._is_user_expired_callbacks: + if await callback(user_id) is True: + return True + + return False + + async def on_user_registration(self, user_id: str): + """Tell third-party modules about a user's registration. + + Args: + user_id: The ID of the newly registered user. + """ + for callback in self._on_user_registration_callback: + await callback(user_id) + @wrap_as_background_process("send_renewals") async def _send_renewal_emails(self) -> None: """Gets the list of users whose account is expiring in the amount of time @@ -100,11 +179,9 @@ async def send_renewal_email_to_user(self, user_id: str) -> None: """ # If a module supports sending a renewal email from here, do that, otherwise do # the legacy dance. - try: - await self._new_account_validity.on_legacy_send_mail(user_id) + if self._on_legacy_send_mail_callback is not None: + await self._on_legacy_send_mail_callback(user_id) return - except NotImplementedError: - pass if not self._account_validity_renew_by_email_enabled: raise AuthError( @@ -236,10 +313,8 @@ async def renew_account(self, renewal_token: str) -> Tuple[bool, bool, int]: """ # If a module supports triggering a renew from here, do that, otherwise do the # legacy dance. - try: - return await self._new_account_validity.on_legacy_renew(renewal_token) - except NotImplementedError: - pass + if self._on_legacy_renew_callback is not None: + return await self._on_legacy_renew_callback(renewal_token) try: ( diff --git a/synapse/handlers/register.py b/synapse/handlers/register.py index c4e7c140719c..056fe5e89f1b 100644 --- a/synapse/handlers/register.py +++ b/synapse/handlers/register.py @@ -77,7 +77,7 @@ def __init__(self, hs: "HomeServer"): self.identity_handler = self.hs.get_identity_handler() self.ratelimiter = hs.get_registration_ratelimiter() self.macaroon_gen = hs.get_macaroon_generator() - self._account_validity = hs.get_account_validity() + self._account_validity_handler = hs.get_account_validity_handler() self._server_notices_mxid = hs.config.server_notices_mxid self._server_name = hs.hostname @@ -703,7 +703,7 @@ async def register_with_store( # Only call the account validity module(s) on the main process, to avoid # repeating e.g. database writes on all of the workers. - await self._account_validity.on_user_registration(user_id) + await self._account_validity_handler.on_user_registration(user_id) async def register_device( self, diff --git a/synapse/module_api/__init__.py b/synapse/module_api/__init__.py index af394e98099e..ff74f656b84f 100644 --- a/synapse/module_api/__init__.py +++ b/synapse/module_api/__init__.py @@ -109,6 +109,7 @@ def __init__(self, hs: "HomeServer", auth_handler): self._public_room_list_manager = PublicRoomListManager(hs) self._spam_checker = hs.get_spam_checker() + self._account_validity_handler = hs.get_account_validity_handler() ################################################################################# # The following methods should only be called during the module's initialisation. @@ -118,6 +119,11 @@ def register_spam_checker_callbacks(self): """Registers callbacks for spam checking capabilities.""" return self._spam_checker.register_callbacks + @property + def register_account_validity_callbacks(self): + """Registers callbacks for account validity capabilities.""" + return self._account_validity_handler.register_account_validity_callbacks + def register_web_resource(self, path: str, resource: IResource): """Registers a web resource to be served at the given path. diff --git a/synapse/rest/admin/users.py b/synapse/rest/admin/users.py index 4f60f778c167..415618182ccb 100644 --- a/synapse/rest/admin/users.py +++ b/synapse/rest/admin/users.py @@ -556,14 +556,15 @@ def __init__(self, hs: "HomeServer"): self.hs = hs self.account_activity_handler = hs.get_account_validity_handler() self.auth = hs.get_auth() - self.account_validity = hs.get_account_validity() async def on_POST(self, request: SynapseRequest) -> Tuple[int, JsonDict]: await assert_requester_is_admin(self.auth, request) - try: - expiration_ts = await self.account_validity.on_legacy_admin_request(request) - except NotImplementedError: + if self.account_activity_handler.on_legacy_admin_request_callback: + expiration_ts = ( + self.account_activity_handler.on_legacy_admin_request_callback(request) + ) + else: body = parse_json_object_from_request(request) if "user_id" not in body: diff --git a/synapse/rest/client/v2_alpha/account_validity.py b/synapse/rest/client/v2_alpha/account_validity.py index c6c66c3aa223..3ebe40186153 100644 --- a/synapse/rest/client/v2_alpha/account_validity.py +++ b/synapse/rest/client/v2_alpha/account_validity.py @@ -85,7 +85,6 @@ def __init__(self, hs): super().__init__() self.hs = hs - self.account_validity = hs.get_account_validity() self.account_activity_handler = hs.get_account_validity_handler() self.auth = hs.get_auth() self.account_validity_renew_by_email_enabled = ( diff --git a/synapse/server.py b/synapse/server.py index ff993822c365..2c27d2a7e8bb 100644 --- a/synapse/server.py +++ b/synapse/server.py @@ -39,7 +39,6 @@ from twisted.web.iweb import IPolicyForHTTPS from twisted.web.resource import IResource -from synapse.account_validity import AccountValidity from synapse.api.auth import Auth from synapse.api.filtering import Filtering from synapse.api.ratelimiting import Ratelimiter @@ -813,10 +812,6 @@ def get_outbound_redis_connection(self) -> Optional["RedisProtocol"]: reconnect=True, ) - @cache_in_self - def get_account_validity(self) -> AccountValidity: - return AccountValidity(self) - def should_send_federation(self) -> bool: "Should this server be sending federation traffic directly?" return self.config.send_federation From c964099fbfee18764425eba574a2e1ea5ec7c28c Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Fri, 2 Jul 2021 09:50:24 +0100 Subject: [PATCH 29/50] Revert changes account validity config since the module config lives elsewhere now anyway --- synapse/config/account_validity.py | 53 ++++++++++++++---------------- 1 file changed, 25 insertions(+), 28 deletions(-) diff --git a/synapse/config/account_validity.py b/synapse/config/account_validity.py index 6ce4312ad55e..87b597075b5c 100644 --- a/synapse/config/account_validity.py +++ b/synapse/config/account_validity.py @@ -35,34 +35,7 @@ class AccountValidityConfig(Config): section = "account_validity" def read_config(self, config, **kwargs): - # Consider legacy account validity disabled unless proven otherwise - self.account_validity_enabled = False - self.account_validity_renew_by_email_enabled = False - - # Read and store template content. We need to do that regardless of whether the - # configuration is using modules or the legacy account validity implementation, - # because we need these templates to register the account validity servlets. - ( - self.account_validity_account_renewed_template, - self.account_validity_account_previously_renewed_template, - self.account_validity_invalid_token_template, - ) = self.read_templates( - [ - "account_renewed.html", - "account_previously_renewed.html", - "invalid_token.html", - ], - ) - - # Initialise the list of modules, which will stay empty if no modules or the - # legacy config was provided. - - # If the configuration is for the legacy feature, then read it as such. - account_validity_config = config.get("account_validity") - - if account_validity_config: - logger.warning(LEGACY_ACCOUNT_VALIDITY_IN_USE) - + account_validity_config = config.get("account_validity") or {} self.account_validity_enabled = account_validity_config.get("enabled", False) self.account_validity_renew_by_email_enabled = ( "renew_at" in account_validity_config @@ -95,3 +68,27 @@ def read_config(self, config, **kwargs): if self.account_validity_renew_by_email_enabled: if not self.public_baseurl: raise ConfigError("Can't send renewal emails without 'public_baseurl'") + + # Load account validity templates. + account_validity_template_dir = account_validity_config.get("template_dir") + + account_renewed_template_filename = account_validity_config.get( + "account_renewed_html_path", "account_renewed.html" + ) + invalid_token_template_filename = account_validity_config.get( + "invalid_token_html_path", "invalid_token.html" + ) + + # Read and store template content + ( + self.account_validity_account_renewed_template, + self.account_validity_account_previously_renewed_template, + self.account_validity_invalid_token_template, + ) = self.read_templates( + [ + account_renewed_template_filename, + "account_previously_renewed.html", + invalid_token_template_filename, + ], + account_validity_template_dir, + ) \ No newline at end of file From a6346e90c7520dfad40b2d799a244f2e60c23391 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Fri, 2 Jul 2021 09:52:58 +0100 Subject: [PATCH 30/50] Remove deprecation warning --- synapse/config/account_validity.py | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/synapse/config/account_validity.py b/synapse/config/account_validity.py index 87b597075b5c..404ef678c5bb 100644 --- a/synapse/config/account_validity.py +++ b/synapse/config/account_validity.py @@ -11,24 +11,7 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. -import logging - from synapse.config._base import Config, ConfigError -from synapse.util.module_loader import load_module - -logger = logging.getLogger(__name__) - -LEGACY_ACCOUNT_VALIDITY_IN_USE = """ -You are using the deprecated account validity feature. It is recommended to change your -configuration to be using one or more modules instead. This feature will be removed in a -future version of Synapse, at which point it will only be available through custom -modules. -See the sample configuration file for more information: -https://github.com/matrix-org/synapse/blob/master/docs/sample_config.yaml -The behaviour and features of the deprecated account validity feature have been ported -to a dedicated module: -https://github.com/matrix-org/synapse-email-account-validity ---------------------------------------------------------------------------------------""" class AccountValidityConfig(Config): From 51b5233ffcc24c384dffc0ac80e32f74dfd50be6 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Fri, 2 Jul 2021 09:54:50 +0100 Subject: [PATCH 31/50] Sample config --- docs/sample_config.yaml | 17 ----------------- synapse/config/account_validity.py | 2 +- 2 files changed, 1 insertion(+), 18 deletions(-) diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index 6479db967d72..45f4d332a032 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -1301,23 +1301,6 @@ account_threepid_delegates: #auto_join_rooms_for_guests: false -## Account Validity ## - -# Server admins can (optionally) get Synapse to check the validity of all user -# accounts against one or more custom modules. -# -# See the documentation on how to write an account validity module: -# https://github.com/matrix-org/synapse/blob/master/docs/account_validity.md -# -account_validity_modules: - #- module: "my_custom_project.SomeAccountValidity" - # config: - # example_option: 'things' - #- module: "my_custom_project.SomeOtherAccountValidity" - # config: - # other_example_option: 'stuff' - - ## Metrics ### # Enable collection and rendering of performance metrics diff --git a/synapse/config/account_validity.py b/synapse/config/account_validity.py index 404ef678c5bb..ec1fc06bef2c 100644 --- a/synapse/config/account_validity.py +++ b/synapse/config/account_validity.py @@ -74,4 +74,4 @@ def read_config(self, config, **kwargs): invalid_token_template_filename, ], account_validity_template_dir, - ) \ No newline at end of file + ) From c2185cf753f84e3a1e6f0847596f2e10248697e1 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Fri, 2 Jul 2021 11:30:19 +0100 Subject: [PATCH 32/50] Incorporate review comments --- synapse/api/auth.py | 5 ----- synapse/handlers/account_validity.py | 14 +++++++++--- synapse/module_api/__init__.py | 33 +++++++++++++++++++--------- 3 files changed, 34 insertions(+), 18 deletions(-) diff --git a/synapse/api/auth.py b/synapse/api/auth.py index 8f0f2aaa0a3d..497bd9ecd216 100644 --- a/synapse/api/auth.py +++ b/synapse/api/auth.py @@ -223,11 +223,6 @@ async def get_user_by_req( if not allow_expired: if await self._account_validity_handler.is_user_expired( user_info.user_id - ) or ( - self._account_validity_enabled - and await self.store.is_account_expired( - user_info.user_id, self.clock.time_msec() - ) ): # Raise the error if either an account validity module has determined # the account has expired, or the legacy account validity diff --git a/synapse/handlers/account_validity.py b/synapse/handlers/account_validity.py index e26226eb6803..9d4c63381ba9 100644 --- a/synapse/handlers/account_validity.py +++ b/synapse/handlers/account_validity.py @@ -137,9 +137,17 @@ async def is_user_expired(self, user_id: str) -> bool: Returns: Whether the user has expired. """ - for callback in self._is_user_expired_callbacks: - if await callback(user_id) is True: - return True + if self._is_user_expired_callbacks: + # If we have at least one module implementing this callback, ignore the + # legacy configuration and use the module(s) to determine if the user has + # expired. + for callback in self._is_user_expired_callbacks: + if await callback(user_id) is True: + return True + elif self._account_validity_enabled: + # Otherwise, if the legacy configuration is enabled, use it to figure out if + # the user has expired. + return await self.store.is_account_expired(user_id, self.clock.time_msec()) return False diff --git a/synapse/module_api/__init__.py b/synapse/module_api/__init__.py index ff74f656b84f..af8fa1d069f0 100644 --- a/synapse/module_api/__init__.py +++ b/synapse/module_api/__init__.py @@ -41,7 +41,7 @@ from synapse.http.servlet import parse_json_object_from_request from synapse.http.site import SynapseRequest from synapse.logging.context import make_deferred_yieldable, run_in_background -from synapse.metrics.background_process_metrics import wrap_as_background_process +from synapse.metrics.background_process_metrics import run_as_background_process from synapse.storage.database import DatabasePool, LoggingTransaction from synapse.storage.databases.main.roommember import ProfileInfo from synapse.storage.state import StateFilter @@ -552,24 +552,37 @@ async def send_local_online_presence_to(self, users: Iterable[str]) -> None: presence_events, destination ) - def looping_background_call_async(self, f: Callable, msec: float, *args, **kwargs): - """Wraps an async function as a background process and calls it repeatedly. + def looping_background_call( + self, + f: Callable, + msec: float, + desc: Optional[str] = None, + *args, + **kwargs, + ): + """Wraps a function as a background process and calls it repeatedly. Waits `msec` initially before calling `f` for the first time. Args: - f(function): The function to call repeatedly. - msec(float): How long to wait between calls in milliseconds. + f: The function to call repeatedly. + msec: How long to wait between calls in milliseconds. + desc: The background task's description. Default to the function's name. *args: Positional arguments to pass to function. **kwargs: Key arguments to pass to function. """ - - @wrap_as_background_process(f.__name__) - async def background_call(*args, **kwargs): - await f(*args, **kwargs) + if desc is None: + desc = f.__name__ if self._hs.config.run_background_tasks: - self._clock.looping_call(background_call, msec, *args, **kwargs) + self._clock.looping_call( + run_as_background_process, + msec, + desc, + f, + *args, + **kwargs, + ) else: logger.warning( "Not running looping call %s as the configuration forbids it", From a0ca66149d1047ca45d6123cadd88579c9596376 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Fri, 2 Jul 2021 12:57:19 +0100 Subject: [PATCH 33/50] Fix type of on_user_registration callbacks --- synapse/handlers/account_validity.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/synapse/handlers/account_validity.py b/synapse/handlers/account_validity.py index 9d4c63381ba9..0e44dd5d8d8a 100644 --- a/synapse/handlers/account_validity.py +++ b/synapse/handlers/account_validity.py @@ -83,8 +83,8 @@ def __init__(self, hs: "HomeServer"): self.clock.looping_call(self._send_renewal_emails, 30 * 60 * 1000) self._is_user_expired_callbacks: List[IS_USER_EXPIRED_CALLBACK] = [] - self._on_user_registration_callback: Optional[ - ON_USER_REGISTRATION_CALLBACK + self._on_user_registration_callbacks: Optional[ + List[ON_USER_REGISTRATION_CALLBACK] ] = [] self._on_legacy_send_mail_callback: Optional[ ON_LEGACY_SEND_MAIL_CALLBACK @@ -108,7 +108,7 @@ def register_account_validity_callbacks( self._is_user_expired_callbacks.append(is_user_expired) if on_user_registration is not None: - self._on_user_registration_callback = on_user_registration + self._on_user_registration_callbacks = on_user_registration if on_legacy_send_mail is not None: if self._on_legacy_send_mail_callback is not None: @@ -157,7 +157,7 @@ async def on_user_registration(self, user_id: str): Args: user_id: The ID of the newly registered user. """ - for callback in self._on_user_registration_callback: + for callback in self._on_user_registration_callbacks: await callback(user_id) @wrap_as_background_process("send_renewals") From 94ca9a728aca63cbca49283c526be312b7049178 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Fri, 2 Jul 2021 14:28:31 +0100 Subject: [PATCH 34/50] Fix types --- synapse/handlers/account_validity.py | 6 ++---- synapse/rest/admin/users.py | 2 +- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/synapse/handlers/account_validity.py b/synapse/handlers/account_validity.py index 0e44dd5d8d8a..a657b1a69024 100644 --- a/synapse/handlers/account_validity.py +++ b/synapse/handlers/account_validity.py @@ -83,9 +83,7 @@ def __init__(self, hs: "HomeServer"): self.clock.looping_call(self._send_renewal_emails, 30 * 60 * 1000) self._is_user_expired_callbacks: List[IS_USER_EXPIRED_CALLBACK] = [] - self._on_user_registration_callbacks: Optional[ - List[ON_USER_REGISTRATION_CALLBACK] - ] = [] + self._on_user_registration_callbacks: List[ON_USER_REGISTRATION_CALLBACK] = [] self._on_legacy_send_mail_callback: Optional[ ON_LEGACY_SEND_MAIL_CALLBACK ] = None @@ -108,7 +106,7 @@ def register_account_validity_callbacks( self._is_user_expired_callbacks.append(is_user_expired) if on_user_registration is not None: - self._on_user_registration_callbacks = on_user_registration + self._on_user_registration_callbacks.append(on_user_registration) if on_legacy_send_mail is not None: if self._on_legacy_send_mail_callback is not None: diff --git a/synapse/rest/admin/users.py b/synapse/rest/admin/users.py index 415618182ccb..06e6ccee4210 100644 --- a/synapse/rest/admin/users.py +++ b/synapse/rest/admin/users.py @@ -561,7 +561,7 @@ async def on_POST(self, request: SynapseRequest) -> Tuple[int, JsonDict]: await assert_requester_is_admin(self.auth, request) if self.account_activity_handler.on_legacy_admin_request_callback: - expiration_ts = ( + expiration_ts = await ( self.account_activity_handler.on_legacy_admin_request_callback(request) ) else: From 34dc6eac6392e0a47caea8ca0ba7f6b379618b03 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Fri, 2 Jul 2021 16:06:54 +0100 Subject: [PATCH 35/50] Restore the possibility for is_user_expired to return None --- synapse/handlers/account_validity.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/synapse/handlers/account_validity.py b/synapse/handlers/account_validity.py index a657b1a69024..661bb0a85930 100644 --- a/synapse/handlers/account_validity.py +++ b/synapse/handlers/account_validity.py @@ -29,7 +29,7 @@ logger = logging.getLogger(__name__) -IS_USER_EXPIRED_CALLBACK = Callable[[str], Awaitable[bool]] +IS_USER_EXPIRED_CALLBACK = Callable[[str], Awaitable[Optional[bool]]] ON_USER_REGISTRATION_CALLBACK = Callable[[str], Awaitable] # Legacy callbacks to allow for a smoother transition between the legacy native account # validity feature and synapse-email-account-validity. @@ -140,8 +140,9 @@ async def is_user_expired(self, user_id: str) -> bool: # legacy configuration and use the module(s) to determine if the user has # expired. for callback in self._is_user_expired_callbacks: - if await callback(user_id) is True: - return True + expired = await callback(user_id) + if expired is not None: + return expired elif self._account_validity_enabled: # Otherwise, if the legacy configuration is enabled, use it to figure out if # the user has expired. From 49c4582d9649358ea8cb56e542e2357808a1079f Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Fri, 2 Jul 2021 16:52:44 +0100 Subject: [PATCH 36/50] Document the account validity callbacks --- docs/account_validity.md | 50 ---------------------------------------- docs/modules.md | 45 ++++++++++++++++++++++++++++++++---- 2 files changed, 40 insertions(+), 55 deletions(-) delete mode 100644 docs/account_validity.md diff --git a/docs/account_validity.md b/docs/account_validity.md deleted file mode 100644 index e85230de4e6d..000000000000 --- a/docs/account_validity.md +++ /dev/null @@ -1,50 +0,0 @@ -# Account validity - -Synapse supports checking the validity of an account against one or more plugin module(s). - -An account validity plugin module is a Python class that exposes two functions: - -* `is_user_expired`, which checks if the account for the provided Matrix user ID has - expired. It returns a boolean to indicate whether the account has expired, or None if - it failed to figure it out. If this function returns `True`, Synapse will block any - request (apart from logout ones). If it returns `None`, Synapse will ask the next - module (in the order they appear in Synapse's configuration file), or consider the user - as not expired if it's reached the end of the list. -* `on_user_registration`, which is called after any successful registration - with the Matrix ID of the newly registered user. - - -## Example - -```python -import time -from typing import Optional - -from synapse.module_api import ModuleApi - -from my_module.store import Store, StoreException - -class ExampleAccountValidity: - def __init__(self, config: dict, api: ModuleApi): - self.config = config - self.api = api - self.store = Store(config, api) - - @staticmethod - def parse_config(config): - return config - - async def is_user_expired(self, user_id: str) -> Optional[bool]: - now_ms = time.time() * 1000 - - # Try to figure out what the expiration date for this account is and whether the - # account has expired, return None if that failed. - try: - expiration_ts = await self.store.get_expiration_ts(user_id) - return expiration_ts <= now_ms - except StoreException: - return None - - async def on_user_registration(self, user_id: str) -> None: - await self.store.insert_user(user_id) -``` \ No newline at end of file diff --git a/docs/modules.md b/docs/modules.md index 3a9fab61b8c3..a18c757229e0 100644 --- a/docs/modules.md +++ b/docs/modules.md @@ -91,12 +91,17 @@ are split in categories. A single module may implement callbacks from multiple c and is under no obligation to implement all callbacks from the categories it registers callbacks for. +Modules can register callbacks using one of the module API's `register_[...]_callbacks` +methods. The callbacks functions are passed to these methods as keyword arguments, with +the callback name as the argument name and the function as its value. This is demonstrated +in the example below. A `register_[...]_callbacks` method exists for each module type +documented in this section. + #### Spam checker callbacks -To register one of the callbacks described in this section, a module needs to use the -module API's `register_spam_checker_callbacks` method. The callback functions are passed -to `register_spam_checker_callbacks` as keyword arguments, with the callback name as the -argument name and the function as its value. This is demonstrated in the example below. +Spam checker callbacks allow module developers to implement spam mitigation actions for +Synapse instances. Spam checker callbacks can be registered using the module API's +`register_spam_checker_callbacks` method. The available spam checker callbacks are: @@ -115,7 +120,7 @@ async def user_may_invite(inviter: str, invitee: str, room_id: str) -> bool Called when processing an invitation. The module must return a `bool` indicating whether the inviter can invite the invitee to the given room. Both inviter and invitee are -represented by their Matrix user ID (i.e. `@alice:example.com`). +represented by their Matrix user ID (e.g. `@alice:example.com`). ```python async def user_may_create_room(user: str) -> bool @@ -188,6 +193,36 @@ async def check_media_file_for_spam( Called when storing a local or remote file. The module must return a boolean indicating whether the given file can be stored in the homeserver's media store. +#### Account validity callbacks + +Account validity callbacks allow module developers to add extra steps to verify the +validity on an account, i.e. see if a user can be granted access to their account on the +Synapse instance. Account validity callbacks can be registered using the module API's +`register_account_validity_callbacks` method. + +The available account validity callbacks are: + +```python +async def is_user_expired(user: str) -> Optional[bool] +``` + +Called when processing any authenticated request (except for logout requests). The module +can return a `bool` to indicate whether the user has expired and should be locked out of +their account, or `None` if the module wasn't able to figure it out. The user is +represented by their Matrix user ID (e.g. `@alice:example.com`). + +If the module returns `True`, the current request will be denied with the error code +`ORG_MATRIX_EXPIRED_ACCOUNT` and the HTTP status code 403. Note that this doesn't +invalidate the user's access token. + +```python +async def on_user_registration(user: str) +``` + +Called after successfully registering a user, in case the module needs to perform extra +operations to keep track of them. (e.g. add them to a database table). The user is +represented by their Matrix user ID. + ### Porting an existing module that uses the old interface In order to port a module that uses Synapse's old module interface, its author needs to: From 005725d3637a0450ce27da4e9b2052e348ff2daf Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Fri, 2 Jul 2021 16:59:10 +0100 Subject: [PATCH 37/50] Fix tests --- tests/test_state.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_state.py b/tests/test_state.py index 62f70958732c..8bf97ca85150 100644 --- a/tests/test_state.py +++ b/tests/test_state.py @@ -168,6 +168,7 @@ def setUp(self): "get_state_handler", "get_clock", "get_state_resolution_handler", + "get_account_validity_handler", "hostname", ] ) From f8754fed56e93de206c561877b6fdfc60fa9191c Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Mon, 5 Jul 2021 18:02:37 +0100 Subject: [PATCH 38/50] Document why we need legacy hooks --- synapse/handlers/account_validity.py | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/synapse/handlers/account_validity.py b/synapse/handlers/account_validity.py index 661bb0a85930..d41dfbba2d0d 100644 --- a/synapse/handlers/account_validity.py +++ b/synapse/handlers/account_validity.py @@ -31,8 +31,6 @@ IS_USER_EXPIRED_CALLBACK = Callable[[str], Awaitable[Optional[bool]]] ON_USER_REGISTRATION_CALLBACK = Callable[[str], Awaitable] -# Legacy callbacks to allow for a smoother transition between the legacy native account -# validity feature and synapse-email-account-validity. ON_LEGACY_SEND_MAIL_CALLBACK = Callable[[str], Awaitable] ON_LEGACY_RENEW_CALLBACK = Callable[[str], Awaitable[Tuple[bool, bool, int]]] ON_LEGACY_ADMIN_REQUEST = Callable[[Request], Awaitable] @@ -108,6 +106,19 @@ def register_account_validity_callbacks( if on_user_registration is not None: self._on_user_registration_callbacks.append(on_user_registration) + # The builtin account validity feature exposes 3 endpoints (send_mail, renew, and + # an admin one). As part of moving the feature into a module, we need to change + # the path from /_matrix/client/unstable/account_validity/... to + # /_synapse/client/account_validity, because: + # * the feature isn't part of the Matrix spec thus shouldn't live under /_matrix + # * because of the way we register servlets modules can't register resources + # under /_matrix/client + # We need to allow for a transition period between the old and new endpoints + # in order to allow for clients to update (and for emails to be processed). + # Therefore we need to allow modules (in practice just the one implementing the + # email-based account validity) to temporarily hook into the legacy endpoint so we + # can route the traffic coming into the old endpoints into the module, which is + # why we have the following three temporary hooks. if on_legacy_send_mail is not None: if self._on_legacy_send_mail_callback is not None: raise RuntimeError("Tried to register on_legacy_send_mail twice") From 58bb7b15f3c8cb1f66dc37bebf24e044a19ebb32 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Wed, 7 Jul 2021 18:41:38 +0200 Subject: [PATCH 39/50] Apply suggestions from code review Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --- docs/modules.md | 2 +- synapse/handlers/account_validity.py | 3 +++ synapse/module_api/__init__.py | 9 ++++----- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/docs/modules.md b/docs/modules.md index a18c757229e0..b4b03cb02647 100644 --- a/docs/modules.md +++ b/docs/modules.md @@ -92,7 +92,7 @@ and is under no obligation to implement all callbacks from the categories it reg callbacks for. Modules can register callbacks using one of the module API's `register_[...]_callbacks` -methods. The callbacks functions are passed to these methods as keyword arguments, with +methods. The callback functions are passed to these methods as keyword arguments, with the callback name as the argument name and the function as its value. This is demonstrated in the example below. A `register_[...]_callbacks` method exists for each module type documented in this section. diff --git a/synapse/handlers/account_validity.py b/synapse/handlers/account_validity.py index d41dfbba2d0d..7011b4dcea85 100644 --- a/synapse/handlers/account_validity.py +++ b/synapse/handlers/account_validity.py @@ -29,8 +29,11 @@ logger = logging.getLogger(__name__) +# Types for callbacks to be registered via the module api IS_USER_EXPIRED_CALLBACK = Callable[[str], Awaitable[Optional[bool]]] ON_USER_REGISTRATION_CALLBACK = Callable[[str], Awaitable] +# Temporary hooks to allow for a transition from `/_matrix/client` endpoints +# to `/_synapse/client/account_validity`. See `register_account_validity_callbacks`. ON_LEGACY_SEND_MAIL_CALLBACK = Callable[[str], Awaitable] ON_LEGACY_RENEW_CALLBACK = Callable[[str], Awaitable[Tuple[bool, bool, int]]] ON_LEGACY_ADMIN_REQUEST = Callable[[Request], Awaitable] diff --git a/synapse/module_api/__init__.py b/synapse/module_api/__init__.py index af8fa1d069f0..282399aedca9 100644 --- a/synapse/module_api/__init__.py +++ b/synapse/module_api/__init__.py @@ -159,14 +159,13 @@ def public_room_list_manager(self): return self._public_room_list_manager @property - def public_baseurl(self): - """Allow accessing the configured public base URL for this homeserver.""" + def public_baseurl(self) -> str: + """The configured public base URL for this homeserver.""" return self._hs.config.public_baseurl @property - def email_app_name(self): - """Allow accessing the application name configured in the homeserver's - configuration. + def email_app_name(self) -> str: + """The application name configured in the homeserver's configuration. """ return self._hs.config.email.email_app_name From 4fb4b49f792de2f46d6dace7c305fcd53024ef28 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Wed, 7 Jul 2021 18:05:26 +0100 Subject: [PATCH 40/50] Incorporate part of the review --- synapse/api/auth.py | 3 -- synapse/handlers/account_validity.py | 21 ++++----- synapse/module_api/__init__.py | 67 +++++++++++++++------------- 3 files changed, 43 insertions(+), 48 deletions(-) diff --git a/synapse/api/auth.py b/synapse/api/auth.py index 497bd9ecd216..237966a6c351 100644 --- a/synapse/api/auth.py +++ b/synapse/api/auth.py @@ -83,9 +83,6 @@ def __init__(self, hs: "HomeServer"): self._auth_blocking = AuthBlocking(self.hs) - self._account_validity_enabled = ( - hs.config.account_validity.account_validity_enabled - ) self._track_appservice_user_ips = hs.config.track_appservice_user_ips self._macaroon_secret_key = hs.config.macaroon_secret_key self._force_tracing_for_users = hs.config.tracing.force_tracing_for_users diff --git a/synapse/handlers/account_validity.py b/synapse/handlers/account_validity.py index 7011b4dcea85..b42e643a1ff0 100644 --- a/synapse/handlers/account_validity.py +++ b/synapse/handlers/account_validity.py @@ -49,8 +49,6 @@ def __init__(self, hs: "HomeServer"): self._app_name = self.hs.config.email_app_name - self._app_name = self.hs.config.email_app_name - self._account_validity_enabled = ( hs.config.account_validity.account_validity_enabled ) @@ -149,17 +147,14 @@ async def is_user_expired(self, user_id: str) -> bool: Returns: Whether the user has expired. """ - if self._is_user_expired_callbacks: - # If we have at least one module implementing this callback, ignore the - # legacy configuration and use the module(s) to determine if the user has - # expired. - for callback in self._is_user_expired_callbacks: - expired = await callback(user_id) - if expired is not None: - return expired - elif self._account_validity_enabled: - # Otherwise, if the legacy configuration is enabled, use it to figure out if - # the user has expired. + for callback in self._is_user_expired_callbacks: + expired = await callback(user_id) + if expired is not None: + return expired + + if self._account_validity_enabled: + # If no module could determine whether the user has expired and the legacy + # configuration is enabled, fall back to it. return await self.store.is_account_expired(user_id, self.clock.time_msec()) return False diff --git a/synapse/module_api/__init__.py b/synapse/module_api/__init__.py index 282399aedca9..f9641376ce08 100644 --- a/synapse/module_api/__init__.py +++ b/synapse/module_api/__init__.py @@ -169,7 +169,7 @@ def email_app_name(self) -> str: """ return self._hs.config.email.email_app_name - def get_user_by_req( + async def get_user_by_req( self, req: SynapseRequest, allow_guest: bool = False, @@ -186,13 +186,16 @@ def get_user_by_req( is False, and the access token is for an expired user, an AuthError will be thrown Returns: - twisted.internet.defer.Deferred[synapse.types.Requester]: - the requester for this request + synapse.types.Requester: the requester for this request Raises: - synapse.api.errors.AuthError: if no user by that token exists, + InvalidClientCredentialsError: if no user by that token exists, or the token is invalid. """ - return self._auth.get_user_by_req(req, allow_guest, allow_expired=allow_expired) + return await self._auth.get_user_by_req( + req, + allow_guest, + allow_expired=allow_expired, + ) async def is_user_admin(self, user_id: str) -> bool: """Checks if a user is a server admin. @@ -221,6 +224,32 @@ def get_qualified_user_id(self, username): return username return UserID(username, self._hs.hostname).to_string() + async def get_profile_for_user(self, localpart: str) -> ProfileInfo: + """Look up the profile info for the user with the given localpart. + + Args: + localpart: The localpart to look up profile information for. + + Returns: + The profile information (i.e. display name and avatar URL). + """ + return await self._store.get_profileinfo(localpart) + + async def get_threepids_for_user(self, user_id: str) -> List[Dict[str, str]]: + """Look up the threepids (email addresses and phone numbers) associated with the + given Matrix user ID. + + Args: + user_id: The Matrix user ID to look up threepids for. + + Returns: + A list of threepids, each threepid being represented by a dictionary + containing a "medium" key which value is "email" for email addresses and + "msisdn" for phone numbers, and an "address" key which value is the + threepid's address. + """ + return await self._store.user_get_threepids(user_id) + def check_user_exists(self, user_id): """Check if user exists. @@ -555,8 +584,8 @@ def looping_background_call( self, f: Callable, msec: float, - desc: Optional[str] = None, *args, + desc: Optional[str] = None, **kwargs, ): """Wraps a function as a background process and calls it repeatedly. @@ -630,32 +659,6 @@ def read_templates( """ return self._hs.config.read_templates(filenames, custom_template_directory) - async def get_profile_for_user(self, localpart: str) -> ProfileInfo: - """Look up the profile info for the user with the given localpart. - - Args: - localpart: The localpart to look up profile information for. - - Returns: - The profile information (i.e. display name and avatar URL). - """ - return await self._store.get_profileinfo(localpart) - - async def get_threepids_for_user(self, user_id: str) -> List[Dict[str, str]]: - """Look up the threepids (email addresses and phone numbers) associated with the - given Matrix user ID. - - Args: - user_id: The Matrix user ID to look up threepids for. - - Returns: - A list of threepids, each threepid being represented by a dictionary - containing a "medium" key which value is "email" for email addresses and - "msisdn" for phone numbers, and an "address" key which value is the - threepid's address. - """ - return await self._store.user_get_threepids(user_id) - class PublicRoomListManager: """Contains methods for adding to, removing from and querying whether a room From b45b45de9de7ae36034641bb9ad7f3904dc915fb Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Wed, 7 Jul 2021 18:13:16 +0100 Subject: [PATCH 41/50] Use the account validity handler in the pusher pool --- synapse/push/pusherpool.py | 24 ++++++++---------------- 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/synapse/push/pusherpool.py b/synapse/push/pusherpool.py index 579fcdf47250..716f3c869d46 100644 --- a/synapse/push/pusherpool.py +++ b/synapse/push/pusherpool.py @@ -62,10 +62,6 @@ def __init__(self, hs: "HomeServer"): self.store = self.hs.get_datastore() self.clock = self.hs.get_clock() - self._account_validity_enabled = ( - hs.config.account_validity.account_validity_enabled - ) - # We shard the handling of push notifications by user ID. self._pusher_shard_config = hs.config.push.pusher_shard_config self._instance_name = hs.get_instance_name() @@ -89,6 +85,8 @@ def __init__(self, hs: "HomeServer"): # map from user id to app_id:pushkey to pusher self.pushers = {} # type: Dict[str, Dict[str, Pusher]] + self._account_validity_handler = hs.get_account_validity_handler() + def start(self) -> None: """Starts the pushers off in a background process.""" if not self._should_start_pushers: @@ -238,12 +236,9 @@ async def _on_new_notifications(self, max_token: RoomStreamToken) -> None: for u in users_affected: # Don't push if the user account has expired - if self._account_validity_enabled: - expired = await self.store.is_account_expired( - u, self.clock.time_msec() - ) - if expired: - continue + expired = await self._account_validity_handler.is_user_expired(u) + if expired: + continue if u in self.pushers: for p in self.pushers[u].values(): @@ -268,12 +263,9 @@ async def on_new_receipts( for u in users_affected: # Don't push if the user account has expired - if self._account_validity_enabled: - expired = await self.store.is_account_expired( - u, self.clock.time_msec() - ) - if expired: - continue + expired = await self._account_validity_handler.is_user_expired(u) + if expired: + continue if u in self.pushers: for p in self.pushers[u].values(): From 972091de1c4b98be3aa2eb69e736fbf2f7cde2f0 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Wed, 7 Jul 2021 18:38:13 +0100 Subject: [PATCH 42/50] Lint --- synapse/module_api/__init__.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/synapse/module_api/__init__.py b/synapse/module_api/__init__.py index f9641376ce08..dc5cc0c6ef33 100644 --- a/synapse/module_api/__init__.py +++ b/synapse/module_api/__init__.py @@ -165,8 +165,7 @@ def public_baseurl(self) -> str: @property def email_app_name(self) -> str: - """The application name configured in the homeserver's configuration. - """ + """The application name configured in the homeserver's configuration.""" return self._hs.config.email.email_app_name async def get_user_by_req( From 07aa404dacef05718ba912b7e77da63b26334a29 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Wed, 7 Jul 2021 18:45:47 +0100 Subject: [PATCH 43/50] Fix changelog --- changelog.d/9884.feature | 1 + changelog.d/9884.removal | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) create mode 100644 changelog.d/9884.feature delete mode 100644 changelog.d/9884.removal diff --git a/changelog.d/9884.feature b/changelog.d/9884.feature new file mode 100644 index 000000000000..525fd2f93ce3 --- /dev/null +++ b/changelog.d/9884.feature @@ -0,0 +1 @@ +Add a module type for the account validity feature. diff --git a/changelog.d/9884.removal b/changelog.d/9884.removal deleted file mode 100644 index fd070a3f3e46..000000000000 --- a/changelog.d/9884.removal +++ /dev/null @@ -1 +0,0 @@ -Deprecate the built-in account validity feature in favour of custom modules. From ba4e0699759336e92d6da4f79ec12c84115c45b1 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 8 Jul 2021 17:25:21 +0100 Subject: [PATCH 44/50] Improve docs --- synapse/config/account_validity.py | 15 +++++++++++++++ synapse/module_api/__init__.py | 4 +++- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/synapse/config/account_validity.py b/synapse/config/account_validity.py index ec1fc06bef2c..6be4eafe5582 100644 --- a/synapse/config/account_validity.py +++ b/synapse/config/account_validity.py @@ -18,6 +18,21 @@ class AccountValidityConfig(Config): section = "account_validity" def read_config(self, config, **kwargs): + """Parses the old account validity config. The config format looks like this: + + account_validity: + enabled: true + period: 6w + renew_at: 1w + renew_email_subject: "Renew your %(app)s account" + template_dir: "res/templates" + account_renewed_html_path: "account_renewed.html" + invalid_token_html_path: "invalid_token.html" + + We expect admins to use modules for this feature (which is why it doesn't appear + in the sample config file), but we want to keep support for it around for a bit + for backwards compatibility. + """ account_validity_config = config.get("account_validity") or {} self.account_validity_enabled = account_validity_config.get("enabled", False) self.account_validity_renew_by_email_enabled = ( diff --git a/synapse/module_api/__init__.py b/synapse/module_api/__init__.py index dc5cc0c6ef33..913519b7f40b 100644 --- a/synapse/module_api/__init__.py +++ b/synapse/module_api/__init__.py @@ -592,7 +592,9 @@ def looping_background_call( Waits `msec` initially before calling `f` for the first time. Args: - f: The function to call repeatedly. + f: The function to call repeatedly. f can be either synchronous or + asynchronous, and must follow Synapse's logcontext rules. + More info about logcontexts is available at https://matrix-org.github.io/synapse/latest/log_contexts.html msec: How long to wait between calls in milliseconds. desc: The background task's description. Default to the function's name. *args: Positional arguments to pass to function. From d9ee0f9f25e481245d4de487ca62e600b87b7988 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Fri, 9 Jul 2021 14:37:07 +0100 Subject: [PATCH 45/50] Line break --- synapse/module_api/__init__.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/synapse/module_api/__init__.py b/synapse/module_api/__init__.py index 913519b7f40b..7bfef9d00b0f 100644 --- a/synapse/module_api/__init__.py +++ b/synapse/module_api/__init__.py @@ -594,7 +594,8 @@ def looping_background_call( Args: f: The function to call repeatedly. f can be either synchronous or asynchronous, and must follow Synapse's logcontext rules. - More info about logcontexts is available at https://matrix-org.github.io/synapse/latest/log_contexts.html + More info about logcontexts is available at + https://matrix-org.github.io/synapse/latest/log_contexts.html msec: How long to wait between calls in milliseconds. desc: The background task's description. Default to the function's name. *args: Positional arguments to pass to function. From c2b668927b5660b0780360b664461304f51d344c Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Wed, 14 Jul 2021 16:25:41 +0100 Subject: [PATCH 46/50] Incorporate review --- docs/modules.md | 4 ++-- synapse/module_api/__init__.py | 10 ++++++---- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/docs/modules.md b/docs/modules.md index b4b03cb02647..7b837b3f9528 100644 --- a/docs/modules.md +++ b/docs/modules.md @@ -63,7 +63,7 @@ Modules can register web resources onto Synapse's web server using the following API method: ```python -def ModuleApi.register_web_resource(path: str, resource: IResource) +def ModuleApi.register_web_resource(path: str, resource: IResource) -> None ``` The path is the full absolute path to register the resource at. For example, if you @@ -216,7 +216,7 @@ If the module returns `True`, the current request will be denied with the error invalidate the user's access token. ```python -async def on_user_registration(user: str) +async def on_user_registration(user: str) -> None ``` Called after successfully registering a user, in case the module needs to perform extra diff --git a/synapse/module_api/__init__.py b/synapse/module_api/__init__.py index 7bfef9d00b0f..ea0d56b3d4b8 100644 --- a/synapse/module_api/__init__.py +++ b/synapse/module_api/__init__.py @@ -45,7 +45,7 @@ from synapse.storage.database import DatabasePool, LoggingTransaction from synapse.storage.databases.main.roommember import ProfileInfo from synapse.storage.state import StateFilter -from synapse.types import JsonDict, UserID, create_requester +from synapse.types import JsonDict, UserID, create_requester, Requester from synapse.util import Clock from synapse.util.caches.descriptors import cached @@ -173,7 +173,7 @@ async def get_user_by_req( req: SynapseRequest, allow_guest: bool = False, allow_expired: bool = False, - ): + ) -> Requester: """Check the access_token provided for a request Args: @@ -184,8 +184,10 @@ async def get_user_by_req( allow_expired: True if expired users should be allowed. If this is False, and the access token is for an expired user, an AuthError will be thrown + Returns: - synapse.types.Requester: the requester for this request + The requester for this request + Raises: InvalidClientCredentialsError: if no user by that token exists, or the token is invalid. @@ -597,8 +599,8 @@ def looping_background_call( More info about logcontexts is available at https://matrix-org.github.io/synapse/latest/log_contexts.html msec: How long to wait between calls in milliseconds. - desc: The background task's description. Default to the function's name. *args: Positional arguments to pass to function. + desc: The background task's description. Default to the function's name. **kwargs: Key arguments to pass to function. """ if desc is None: From a4adb3d59ab4a6458c7279cef3f1b04e2f324b5a Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Wed, 14 Jul 2021 16:40:09 +0100 Subject: [PATCH 47/50] Lint --- synapse/module_api/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/module_api/__init__.py b/synapse/module_api/__init__.py index ea0d56b3d4b8..81e2f33d0a7d 100644 --- a/synapse/module_api/__init__.py +++ b/synapse/module_api/__init__.py @@ -45,7 +45,7 @@ from synapse.storage.database import DatabasePool, LoggingTransaction from synapse.storage.databases.main.roommember import ProfileInfo from synapse.storage.state import StateFilter -from synapse.types import JsonDict, UserID, create_requester, Requester +from synapse.types import JsonDict, Requester, UserID, create_requester from synapse.util import Clock from synapse.util.caches.descriptors import cached From c3debd57f7ab4f6283e835d4d31eb2b677b2e268 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Fri, 16 Jul 2021 16:52:28 +0200 Subject: [PATCH 48/50] Apply suggestions from code review Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --- synapse/handlers/account_validity.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/synapse/handlers/account_validity.py b/synapse/handlers/account_validity.py index b42e643a1ff0..2185d351391b 100644 --- a/synapse/handlers/account_validity.py +++ b/synapse/handlers/account_validity.py @@ -111,13 +111,19 @@ def register_account_validity_callbacks( # an admin one). As part of moving the feature into a module, we need to change # the path from /_matrix/client/unstable/account_validity/... to # /_synapse/client/account_validity, because: + # # * the feature isn't part of the Matrix spec thus shouldn't live under /_matrix - # * because of the way we register servlets modules can't register resources + # * the way we register servlets means that modules can't register resources # under /_matrix/client + # # We need to allow for a transition period between the old and new endpoints # in order to allow for clients to update (and for emails to be processed). - # Therefore we need to allow modules (in practice just the one implementing the - # email-based account validity) to temporarily hook into the legacy endpoint so we + # + # Once the email-account-validity module is loaded, it will take control of account + # validity by moving the rows from our `account_validity` table into its own table. + # + # Therefore, we need to allow modules (in practice just the one implementing the + # email-based account validity) to temporarily hook into the legacy endpoints so we # can route the traffic coming into the old endpoints into the module, which is # why we have the following three temporary hooks. if on_legacy_send_mail is not None: From e87c3cbbf797e7c7daae14f07d6f84d343366228 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Fri, 16 Jul 2021 15:57:55 +0100 Subject: [PATCH 49/50] Incorporate review --- synapse/handlers/account_validity.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/synapse/handlers/account_validity.py b/synapse/handlers/account_validity.py index b42e643a1ff0..fc992d6b4b32 100644 --- a/synapse/handlers/account_validity.py +++ b/synapse/handlers/account_validity.py @@ -318,6 +318,10 @@ async def renew_account(self, renewal_token: str) -> Tuple[bool, bool, int]: token is considered stale. A token is stale if the 'token_used_ts_ms' db column is non-null. + This method exists to support handling the legacy account validity /renew + endpoint. If a module implements the on_legacy_renew callback, then this process + is delegated to the module instead. + Args: renewal_token: Token sent with the renewal request. Returns: From 1a8af467d0341cc31547123a5ab733aa2393a7f3 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Fri, 16 Jul 2021 16:05:57 +0100 Subject: [PATCH 50/50] Lint --- synapse/handlers/account_validity.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/handlers/account_validity.py b/synapse/handlers/account_validity.py index e59d5a95a126..078accd634f1 100644 --- a/synapse/handlers/account_validity.py +++ b/synapse/handlers/account_validity.py @@ -121,7 +121,7 @@ def register_account_validity_callbacks( # # Once the email-account-validity module is loaded, it will take control of account # validity by moving the rows from our `account_validity` table into its own table. - # + # # Therefore, we need to allow modules (in practice just the one implementing the # email-based account validity) to temporarily hook into the legacy endpoints so we # can route the traffic coming into the old endpoints into the module, which is