Add a module type for account validity

changelog.d/9884.feature

@@ -0,0 +1 @@
+Add a module type for the account validity feature.

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.

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

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 = (

synapse/handlers/account_validity.py

@@ -29,10 +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]
-# Legacy callbacks to allow for a smoother transition between the legacy native account
-# validity feature and synapse-email-account-validity.
+# 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]
@@ -48,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
  )
@@ -108,6 +107,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")
@@ -135,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

synapse/module_api/__init__.py

@@ -159,18 +159,16 @@ 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
 
- def get_user_by_req(
+ async def get_user_by_req(
  self,
  req: SynapseRequest,
  allow_guest: bool = False,
@@ -187,13 +185,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.
@@ -222,6 +223,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.
 
@@ -556,16 +583,19 @@ 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.
 
  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.
@@ -631,32 +661,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

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():