From ba71e029182b5890914552713ee2b999870598a6 Mon Sep 17 00:00:00 2001
From: David Robertson <davidr@element.io>
Date: Tue, 5 Jul 2022 18:51:56 +0100
Subject: [PATCH] Validate `/client/account/3pid/email/requestToken`

---
 synapse/rest/client/account.py    | 52 ++++++++++++-------------------
 tests/rest/client/test_account.py |  8 ++---
 2 files changed, 24 insertions(+), 36 deletions(-)

diff --git a/synapse/rest/client/account.py b/synapse/rest/client/account.py
index 31394bb27298..b4cd5d9d313c 100644
--- a/synapse/rest/client/account.py
+++ b/synapse/rest/client/account.py
@@ -361,28 +361,16 @@ async def on_POST(self, request: SynapseRequest) -> Tuple[int, JsonDict]:
                     "Adding emails have been disabled due to lack of an email config"
                 )
             raise SynapseError(
-                400, "Adding an email to your account is disabled on this server"
+                HTTPStatus.NOT_FOUND,
+                "Adding an email to your account is disabled on this server",
+                Codes.NOT_FOUND,
             )
 
-        body = parse_json_object_from_request(request)
-        assert_params_in_dict(body, ["client_secret", "email", "send_attempt"])
-        client_secret = body["client_secret"]
-        assert_valid_client_secret(client_secret)
-
-        # Canonicalise the email address. The addresses are all stored canonicalised
-        # in the database.
-        # This ensures that the validation email is sent to the canonicalised address
-        # as it will later be entered into the database.
-        # Otherwise the email will be sent to "FOO@bar.com" and stored as
-        # "foo@bar.com" in database.
-        try:
-            email = validate_email(body["email"])
-        except ValueError as e:
-            raise SynapseError(400, str(e))
-        send_attempt = body["send_attempt"]
-        next_link = body.get("next_link")  # Optional param
+        body = parse_and_validate_json_object_from_request(
+            request, EmailRequestTokenBody
+        )
 
-        if not await check_3pid_allowed(self.hs, "email", email):
+        if not await check_3pid_allowed(self.hs, "email", body.email):
             raise SynapseError(
                 403,
                 "Your email domain is not authorized on this server",
@@ -390,14 +378,14 @@ async def on_POST(self, request: SynapseRequest) -> Tuple[int, JsonDict]:
             )
 
         await self.identity_handler.ratelimit_request_token_requests(
-            request, "email", email
+            request, "email", body.email
         )
 
-        if next_link:
+        if body.next_link:
             # Raise if the provided next_link value isn't valid
-            assert_valid_next_link(self.hs, next_link)
+            assert_valid_next_link(self.hs, body.next_link)
 
-        existing_user_id = await self.store.get_user_id_by_threepid("email", email)
+        existing_user_id = await self.store.get_user_id_by_threepid("email", body.email)
 
         if existing_user_id is not None:
             if self.config.server.request_token_inhibit_3pid_errors:
@@ -416,26 +404,26 @@ async def on_POST(self, request: SynapseRequest) -> Tuple[int, JsonDict]:
             # Have the configured identity server handle the request
             ret = await self.identity_handler.requestEmailToken(
                 self.hs.config.registration.account_threepid_delegate_email,
-                email,
-                client_secret,
-                send_attempt,
-                next_link,
+                body.email,
+                body.client_secret,
+                body.send_attempt,
+                body.next_link,
             )
         else:
             # Send threepid validation emails from Synapse
             sid = await self.identity_handler.send_threepid_validation(
-                email,
-                client_secret,
-                send_attempt,
+                body.email,
+                body.client_secret,
+                body.send_attempt,
                 self.mailer.send_add_threepid_mail,
-                next_link,
+                body.next_link,
             )
 
             # Wrap the session id in a JSON object
             ret = {"sid": sid}
 
         threepid_send_requests.labels(type="email", reason="add_threepid").observe(
-            send_attempt
+            body.send_attempt
         )
 
         return 200, ret
diff --git a/tests/rest/client/test_account.py b/tests/rest/client/test_account.py
index 982c710e9a12..229f6969cc92 100644
--- a/tests/rest/client/test_account.py
+++ b/tests/rest/client/test_account.py
@@ -645,21 +645,21 @@ def test_add_valid_email_second_time_canonicalise(self) -> None:
     def test_add_email_no_at(self) -> None:
         self._request_token_invalid_email(
             "address-without-at.bar",
-            expected_errcode=Codes.UNKNOWN,
+            expected_errcode=Codes.BAD_JSON,
             expected_error="Unable to parse email address",
         )
 
     def test_add_email_two_at(self) -> None:
         self._request_token_invalid_email(
             "foo@foo@test.bar",
-            expected_errcode=Codes.UNKNOWN,
+            expected_errcode=Codes.BAD_JSON,
             expected_error="Unable to parse email address",
         )
 
     def test_add_email_bad_format(self) -> None:
         self._request_token_invalid_email(
             "user@bad.example.net@good.example.com",
-            expected_errcode=Codes.UNKNOWN,
+            expected_errcode=Codes.BAD_JSON,
             expected_error="Unable to parse email address",
         )
 
@@ -995,7 +995,7 @@ def _request_token_invalid_email(
         )
         self.assertEqual(400, channel.code, msg=channel.result["body"])
         self.assertEqual(expected_errcode, channel.json_body["errcode"])
-        self.assertEqual(expected_error, channel.json_body["error"])
+        self.assertIn(expected_error, channel.json_body["error"])
 
     def _validate_token(self, link: str) -> None:
         # Remove the host