Support shorter tokens used with authentication

README.md

@@ -4,7 +4,7 @@ A Synapse plugin module to manage account validity using validation emails.
 
 This module requires:
 
-* Synapse >= 1.34.0
+* Synapse >= 1.38.0
 * sqlite3 >= 3.24.0 (if using SQLite with Synapse)
 
 ## Installation
@@ -17,33 +17,62 @@ pip install synapse-email-account-validity
 
 ## Config
 
-Add the following in your Synapse config under `account_validity`:
+Add the following in your Synapse config:
 
 ```yaml
+modules:
   - module: email_account_validity.EmailAccountValidity
     config:
       # The maximum amount of time an account can stay valid for without being renewed.
       period: 6w
       # How long before an account expires should Synapse send it a renewal email.
       renew_at: 1w
-```
-
-Also under the HTTP client `listener`, configure an `additional_resource` as per below:
-
-```yaml
-    additional_resources:
-      "/_synapse/client/email_account_validity":
-        module: email_account_validity.EmailAccountValidityServlet
-        config:
-          # The maximum amount of time an account can stay valid for without being
-          # renewed.
-          period: 6w
+      # Whether to include a link to click in the emails sent to users. If false, only a
+      # renewal token is sent, in which case it is generated so it's simpler, and the
+      # user will need to copy it into a compatible client that will send an
+      # authenticated request to the server.
+      # Defaults to true.
+      send_links: true
 ```
 
 The syntax for durations is the same as in the rest of Synapse's configuration file.
 
-If they are not already there, copy the [templates](/email_account_validity/templates)
-into Synapse's templates directory.
+Configuration parameters with matching names that appear both in `account_validity` and
+`listeners` __must__ have the same value in both places, otherwise the module will not
+behave correctly.
+
+## Templates
+
+The templates the module will use are:
+
+* `notice_expiry.(html|txt)`: The content of the renewal email. It gets passed the
+  following variables:
+    * `app_name`: The value configured for `app_name` in the Synapse configuration file
+      (under the `email` section).
+    * `display_name`: The display name of the user needing renewal.
+    * `expiration_ts`: A timestamp in milliseconds representing when the account will
+      expire. Templates can use the `format_ts` (with a date format as the function's
+      parameter) to format this timestamp into a human-readable date.
+    * `url`: The URL the user is supposed to click on to renew their account. If
+      `send_links` is set to `false` in the module's configuration, the value of this
+      variable will be `None`.
+    * `renewal_token`: The token to use in order to renew the user's account. If
+      `send_links` is set to `false`, templates should prefer this variable to `url`.
+* `account_renewed.html`: The HTML to display to a user when they successfully renew
+  their account. It gets passed the following vaiables:
+    * `expiration_ts`: A timestamp in milliseconds representing when the account will
+      expire. Templates can use the `format_ts` (with a date format as the function's
+      parameter) to format this timestamp into a human-readable date.
+* `account_previously_renewed.html`: The HTML to display to a user when they try to renew
+  their account with a token that's valid but previously used. It gets passed the same
+  variables as `account_renewed.html`.
+* `invalid_token.html`: The HTML to display to a user when they try to renew their account
+  with the wrong token. It doesn't get passed any variable.
+
+You can find and change the default templates [here](https://github.com/matrix-org/synapse-email-account-validity/tree/main/email_account_validity/templates).
+
+Note that the templates directory contains two files that aren't templates (`mail.css`
+and `mail-expiry.css`), but are used by email templates to apply visual adjustments.
 
 ## Routes
 

email_account_validity/__init__.py

@@ -16,12 +16,11 @@
 from pkg_resources import DistributionNotFound, get_distribution
 
 from email_account_validity.account_validity import EmailAccountValidity
-from email_account_validity.servlets import EmailAccountValidityServlet
 
 try:
     __version__ = get_distribution(__name__).version
 except DistributionNotFound:
     # package is not installed
     pass
 
-__all__ = ["EmailAccountValidity", "EmailAccountValidityServlet"]
+__all__ = ["EmailAccountValidity"]

email_account_validity/_base.py

@@ -14,31 +14,32 @@
 # limitations under the License.
 
 import logging
+import os
+import time
 from typing import Any, Optional, Tuple
 
 from twisted.web.server import Request
 
-from synapse.api.errors import StoreError, SynapseError
-from synapse.http.servlet import parse_json_object_from_request
-from synapse.module_api import ModuleApi
-from synapse.types import UserID
-from synapse.util import stringutils
+from synapse.module_api import ModuleApi, parse_json_object_from_request, UserID
+from synapse.module_api.errors import SynapseError
 
 from email_account_validity._store import EmailAccountValidityStore
+from email_account_validity._utils import (
+    random_digit_string,
+    random_string,
+    UNAUTHENTICATED_TOKEN_REGEX,
+)
 
 logger = logging.getLogger(__name__)
 
 
 class EmailAccountValidityBase:
-    def __init__(self, config: Any, api: ModuleApi, store: EmailAccountValidityStore):
+    def __init__(self, config: dict, api: ModuleApi):
         self._api = api
-        self._store = store
+        self._store = EmailAccountValidityStore(config, api)
 
         self._period = config.get("period")
-
-        (self._template_html, self._template_text,) = api.read_templates(
-            ["notice_expiry.html", "notice_expiry.txt"],
-        )
+        self._send_links = config.get("send_links", True)
 
         if "renew_email_subject" in config:
             renew_email_subject = config["renew_email_subject"]
@@ -48,10 +49,15 @@ def __init__(self, config: Any, api: ModuleApi, store: EmailAccountValidityStore
         try:
             app_name = self._api.email_app_name
             self._renew_email_subject = renew_email_subject % {"app": app_name}
-        except Exception:
+        except (KeyError, TypeError):
             # If substitution failed, fall back to the bare strings.
             self._renew_email_subject = renew_email_subject
 
+        (self._template_html, self._template_text,) = api.read_templates(
+            ["notice_expiry.html", "notice_expiry.txt"],
+            os.path.join(os.path.dirname(os.path.abspath(__file__)), "templates"),
+        )
+
     async def send_renewal_email_to_user(self, user_id: str) -> None:
         """
         Send a renewal email for a specific user.
@@ -102,38 +108,52 @@ async def send_renewal_email(self, user_id: str, expiration_ts: int):
             display_name = profile.display_name
             if display_name is None:
                 display_name = user_id
-        except StoreError:
+        except SynapseError:
             display_name = user_id
 
-        renewal_token = await self.generate_renewal_token(user_id)
+        # If the user isn't expected to click on a link, but instead to copy the token
+        # into their client, we generate a different kind of token, simpler and shorter,
+        # because a) we don't need it to be unique to the whole table and b) we want the
+        # user to be able to be easily type it back into their client.
+        if self._send_links:
+            renewal_token = await self.generate_unauthenticated_renewal_token(user_id)
+        else:
+            renewal_token = await self.generate_authenticated_renewal_token(user_id)
 
-        url = "%s_synapse/client/email_account_validity/renew?token=%s" % (
-            self._api.public_baseurl,
-            renewal_token,
-        )
+        if self._send_links:
+            url = "%s_synapse/client/email_account_validity/renew?token=%s" % (
+                self._api.public_baseurl,
+                renewal_token,
+            )
+        else:
+            url = None
 
         template_vars = {
+            "app_name": self._api.email_app_name,
             "display_name": display_name,
             "expiration_ts": expiration_ts,
             "url": url,
+            "renewal_token": renewal_token,
         }
 
         html_text = self._template_html.render(**template_vars)
         plain_text = self._template_text.render(**template_vars)
 
         for address in addresses:
             await self._api.send_mail(
-                address,
-                self._renew_email_subject,
-                html_text,
-                plain_text,
+                recipient=address,
+                subject=self._renew_email_subject,
+                html=html_text,
+                text=plain_text,
             )
 
         await self._store.set_renewal_mail_status(user_id=user_id, email_sent=True)
 
-    async def generate_renewal_token(self, user_id: str) -> str:
-        """Generates a 32-byte long random string that will be inserted into the
-        user's renewal email's unique link, then saves it into the database.
+    async def generate_authenticated_renewal_token(self, user_id: str) -> str:
+        """Generates a 8-digit long random string then saves it into the database.
+
+        This token is to be sent to the user over email so that the user can copy it into
+        their client to renew their account.
 
         Args:
             user_id: ID of the user to generate a string for.
@@ -142,19 +162,42 @@ async def generate_renewal_token(self, user_id: str) -> str:
             The generated string.
 
         Raises:
-            StoreError(500): Couldn't generate a unique string after 5 attempts.
+            SynapseError(500): Couldn't generate a unique string after 5 attempts.
+        """
+        renewal_token = random_digit_string(8)
+        await self._store.set_renewal_token_for_user(user_id, renewal_token)
+        return renewal_token
+
+    async def generate_unauthenticated_renewal_token(self, user_id: str) -> str:
+        """Generates a 32-letter long random string then saves it into the database.
+
+        This token is to be sent to the user over email in a link that the user will then
+        click to renew their account.
+
+        Args:
+            user_id: ID of the user to generate a string for.
+
+        Returns:
+            The generated string.
+
+        Raises:
+            SynapseError(500): Couldn't generate a unique string after 5 attempts.
         """
         attempts = 0
         while attempts < 5:
             try:
-                renewal_token = stringutils.random_string(32)
+                renewal_token = random_string(32)
                 await self._store.set_renewal_token_for_user(user_id, renewal_token)
                 return renewal_token
-            except StoreError:
+            except SynapseError:
                 attempts += 1
-        raise StoreError(500, "Couldn't generate a unique string as refresh string.")
+        raise SynapseError(500, "Couldn't generate a unique string as refresh string.")
 
-    async def renew_account(self, renewal_token: str) -> Tuple[bool, bool, int]:
+    async def renew_account(
+        self,
+        renewal_token: str,
+        user_id: Optional[str] = None,
+    ) -> Tuple[bool, bool, int]:
         """Renews the account attached to a given renewal token by pushing back the
         expiration date by the current validity period in the server's configuration.
 
@@ -164,25 +207,29 @@ async def renew_account(self, renewal_token: str) -> Tuple[bool, bool, int]:
 
         Args:
             renewal_token: Token sent with the renewal request.
+            user_id: The Matrix ID of the user to renew, if the renewal request was
+                authenticated.
+
         Returns:
             A tuple containing:
               * A bool representing whether the token is valid and unused.
               * A bool which is `True` if the token is valid, but stale.
               * An int representing the user's expiry timestamp as milliseconds since the
                 epoch, or 0 if the token was invalid.
         """
-        if self._period is None:
-            # If a period hasn't been provided in the config, then it means this function
-            # was called from a place it shouldn't have been, e.g. the /send_mail servlet.
-            raise SynapseError(500, "Tried to renew account in unexpected place")
+        # If we were not able to authenticate the user requesting a renewal, and the
+        # token needs authentication, consider the token neither valid nor stale.
+        if user_id is None and not UNAUTHENTICATED_TOKEN_REGEX.match(renewal_token):
+            return False, False, 0
 
+        # Verify if the token, or the (token, user_id) tuple, exists.
         try:
             (
                 user_id,
                 current_expiration_ts,
                 token_used_ts,
-            ) = await self._store.get_user_from_renewal_token(renewal_token)
-        except StoreError:
+            ) = await self._store.validate_renewal_token(renewal_token, user_id)
+        except SynapseError:
             return False, False, 0
 
         # Check whether this token has already been used.
@@ -230,7 +277,7 @@ async def renew_account_for_user(
             New expiration date for this account, as a timestamp in
             milliseconds since epoch.
         """
-        now = self._api.current_time_ms()
+        now = int(time.time() * 1000)
         if expiration_ts is None:
             expiration_ts = now + self._period