From ced5716b983fc4c462ac6d5568d727f3a369d927 Mon Sep 17 00:00:00 2001
From: dklimpel <5740567+dklimpel@users.noreply.github.com>
Date: Mon, 2 Mar 2020 22:29:58 +0100
Subject: [PATCH 01/12] Fix inconsistent handling of upper and lower cases of
 email addresses.

---
 changelog.d/7016.bugfix                  |  1 +
 synapse/rest/client/v2_alpha/account.py  | 12 +++++++++---
 synapse/rest/client/v2_alpha/register.py | 12 +++++++++---
 3 files changed, 19 insertions(+), 6 deletions(-)
 create mode 100644 changelog.d/7016.bugfix

diff --git a/changelog.d/7016.bugfix b/changelog.d/7016.bugfix
new file mode 100644
index 000000000000..46944a341310
--- /dev/null
+++ b/changelog.d/7016.bugfix
@@ -0,0 +1 @@
+Fix inconsistent handling of upper and lower cases of email addresses that is use only lower cases.
\ No newline at end of file
diff --git a/synapse/rest/client/v2_alpha/account.py b/synapse/rest/client/v2_alpha/account.py
index dc837d6c7582..d4e61a4082b1 100644
--- a/synapse/rest/client/v2_alpha/account.py
+++ b/synapse/rest/client/v2_alpha/account.py
@@ -84,7 +84,10 @@ async def on_POST(self, request):
         client_secret = body["client_secret"]
         assert_valid_client_secret(client_secret)
 
-        email = body["email"]
+        # For emails, transform the address to lowercase.
+        # We store all email addreses as lowercase in the DB.
+        # (See add_threepid in synapse/handlers/auth.py)
+        email = body["email"].lower()
         send_attempt = body["send_attempt"]
         next_link = body.get("next_link")  # Optional param
 
@@ -359,7 +362,10 @@ async def on_POST(self, request):
         client_secret = body["client_secret"]
         assert_valid_client_secret(client_secret)
 
-        email = body["email"]
+        # For emails, transform the address to lowercase.
+        # We store all email addreses as lowercase in the DB.
+        # (See add_threepid in synapse/handlers/auth.py)
+        email = body["email"].lower()
         send_attempt = body["send_attempt"]
         next_link = body.get("next_link")  # Optional param
 
@@ -371,7 +377,7 @@ async def on_POST(self, request):
             )
 
         existing_user_id = await self.store.get_user_id_by_threepid(
-            "email", body["email"]
+            "email", email
         )
 
         if existing_user_id is not None:
diff --git a/synapse/rest/client/v2_alpha/register.py b/synapse/rest/client/v2_alpha/register.py
index a09189b1b469..25fe3cbf172d 100644
--- a/synapse/rest/client/v2_alpha/register.py
+++ b/synapse/rest/client/v2_alpha/register.py
@@ -119,7 +119,10 @@ async def on_POST(self, request):
         client_secret = body["client_secret"]
         assert_valid_client_secret(client_secret)
 
-        email = body["email"]
+        # For emails, transform the address to lowercase.
+        # We store all email addreses as lowercase in the DB.
+        # (See add_threepid in synapse/handlers/auth.py)
+        email = body["email"].lower()
         send_attempt = body["send_attempt"]
         next_link = body.get("next_link")  # Optional param
 
@@ -131,7 +134,7 @@ async def on_POST(self, request):
             )
 
         existing_user_id = await self.hs.get_datastore().get_user_id_by_threepid(
-            "email", body["email"]
+            "email", email
         )
 
         if existing_user_id is not None:
@@ -552,7 +555,10 @@ async def on_POST(self, request):
                 for login_type in [LoginType.EMAIL_IDENTITY, LoginType.MSISDN]:
                     if login_type in auth_result:
                         medium = auth_result[login_type]["medium"]
-                        address = auth_result[login_type]["address"]
+                        # For emails, transform the address to lowercase.
+                        # We store all email addreses as lowercase in the DB.
+                        # (See add_threepid in synapse/handlers/auth.py)
+                        address = auth_result[login_type]["address"].lower()
 
                         existing_user_id = await self.store.get_user_id_by_threepid(
                             medium, address

From 61ee8a4dd0afb4362c149b62191166d1f575dbfd Mon Sep 17 00:00:00 2001
From: dklimpel <5740567+dklimpel@users.noreply.github.com>
Date: Sat, 2 May 2020 21:14:20 +0200
Subject: [PATCH 02/12] Change validation of mail address from 'lower()' to
 'casefold'

---
 changelog.d/{7016.bugfix => 7021.bugfix} |  2 +-
 synapse/handlers/auth.py                 | 15 ++-------
 synapse/rest/admin/users.py              |  5 +++
 synapse/rest/client/v1/login.py          |  6 ++--
 synapse/rest/client/v2_alpha/account.py  | 21 +++---------
 synapse/rest/client/v2_alpha/register.py | 17 +++++-----
 synapse/util/threepids.py                | 22 +++++++++++++
 tests/util/test_threepids.py             | 41 ++++++++++++++++++++++++
 8 files changed, 87 insertions(+), 42 deletions(-)
 rename changelog.d/{7016.bugfix => 7021.bugfix} (58%)
 create mode 100644 tests/util/test_threepids.py

diff --git a/changelog.d/7016.bugfix b/changelog.d/7021.bugfix
similarity index 58%
rename from changelog.d/7016.bugfix
rename to changelog.d/7021.bugfix
index 46944a341310..708e09c3f946 100644
--- a/changelog.d/7016.bugfix
+++ b/changelog.d/7021.bugfix
@@ -1 +1 @@
-Fix inconsistent handling of upper and lower cases of email addresses that is use only lower cases.
\ No newline at end of file
+Fix inconsistent handling of upper and lower cases of email addresses that is use only case folding (MSC2265).
\ No newline at end of file
diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py
index 48a88d3c2aaf..2698108a5241 100644
--- a/synapse/handlers/auth.py
+++ b/synapse/handlers/auth.py
@@ -42,6 +42,7 @@
 from synapse.module_api import ModuleApi
 from synapse.types import UserID
 from synapse.util.caches.expiringcache import ExpiringCache
+from synapse.util.threepids import canonicalise_email
 
 from ._base import BaseHandler
 
@@ -824,17 +825,8 @@ def add_threepid(self, user_id, medium, address, validated_at):
                 errcode=Codes.INVALID_PARAM,
             )
 
-        # 'Canonicalise' email addresses down to lower case.
-        # We've now moving towards the homeserver being the entity that
-        # is responsible for validating threepids used for resetting passwords
-        # on accounts, so in future Synapse will gain knowledge of specific
-        # types (mediums) of threepid. For now, we still use the existing
-        # infrastructure, but this is the start of synapse gaining knowledge
-        # of specific types of threepid (and fixes the fact that checking
-        # for the presence of an email address during password reset was
-        # case sensitive).
         if medium == "email":
-            address = address.lower()
+            address = canonicalise_email(address)
 
         yield self.store.user_add_threepid(
             user_id, medium, address, validated_at, self.hs.get_clock().time_msec()
@@ -860,9 +852,8 @@ def delete_threepid(self, user_id, medium, address, id_server=None):
             unbind API.
         """
 
-        # 'Canonicalise' email addresses as per above
         if medium == "email":
-            address = address.lower()
+            address = canonicalise_email(address)
 
         identity_handler = self.hs.get_handlers().identity_handler
         result = yield identity_handler.try_unbind_threepid(
diff --git a/synapse/rest/admin/users.py b/synapse/rest/admin/users.py
index 80f959248dcd..5f82a14ad211 100644
--- a/synapse/rest/admin/users.py
+++ b/synapse/rest/admin/users.py
@@ -36,6 +36,7 @@
     historical_admin_path_patterns,
 )
 from synapse.types import UserID
+from synapse.util.threepids import canonicalise_email
 
 logger = logging.getLogger(__name__)
 
@@ -195,6 +196,8 @@ async def on_PUT(self, request, user_id):
                 # add new threepids to user
                 current_time = self.hs.get_clock().time_msec()
                 for threepid in body["threepids"]:
+                    if threepid["medium"] == "email":
+                        threepid["address"] = canonicalise_email(threepid["address"])
                     await self.auth_handler.add_threepid(
                         user_id, threepid["medium"], threepid["address"], current_time
                     )
@@ -271,6 +274,8 @@ async def on_PUT(self, request, user_id):
 
                 current_time = self.hs.get_clock().time_msec()
                 for threepid in body["threepids"]:
+                    if threepid["medium"] == "email":
+                        threepid["address"] = canonicalise_email(threepid["address"])
                     await self.auth_handler.add_threepid(
                         user_id, threepid["medium"], threepid["address"], current_time
                     )
diff --git a/synapse/rest/client/v1/login.py b/synapse/rest/client/v1/login.py
index 2c99536678ff..f58321386537 100644
--- a/synapse/rest/client/v1/login.py
+++ b/synapse/rest/client/v1/login.py
@@ -33,6 +33,7 @@
 from synapse.rest.well_known import WellKnownBuilder
 from synapse.types import UserID, map_username_to_mxid_localpart
 from synapse.util.msisdn import phone_number_to_msisdn
+from synapse.util.threepids import canonicalise_email
 
 logger = logging.getLogger(__name__)
 
@@ -197,10 +198,7 @@ async def _do_other_login(self, login_submission):
                 raise SynapseError(400, "Invalid thirdparty identifier")
 
             if medium == "email":
-                # For emails, transform the address to lowercase.
-                # We store all email addreses as lowercase in the DB.
-                # (See add_threepid in synapse/handlers/auth.py)
-                address = address.lower()
+                address = canonicalise_email(address)
 
             # We also apply account rate limiting using the 3PID as a key, as
             # otherwise using 3PID bypasses the ratelimiting based on user ID.
diff --git a/synapse/rest/client/v2_alpha/account.py b/synapse/rest/client/v2_alpha/account.py
index d4e61a4082b1..451d99eb8198 100644
--- a/synapse/rest/client/v2_alpha/account.py
+++ b/synapse/rest/client/v2_alpha/account.py
@@ -31,7 +31,7 @@
 from synapse.push.mailer import Mailer, load_jinja2_templates
 from synapse.util.msisdn import phone_number_to_msisdn
 from synapse.util.stringutils import assert_valid_client_secret
-from synapse.util.threepids import check_3pid_allowed
+from synapse.util.threepids import canonicalise_email, check_3pid_allowed
 
 from ._base import client_patterns, interactive_auth_handler
 
@@ -84,10 +84,7 @@ async def on_POST(self, request):
         client_secret = body["client_secret"]
         assert_valid_client_secret(client_secret)
 
-        # For emails, transform the address to lowercase.
-        # We store all email addreses as lowercase in the DB.
-        # (See add_threepid in synapse/handlers/auth.py)
-        email = body["email"].lower()
+        email = canonicalise_email(body["email"])
         send_attempt = body["send_attempt"]
         next_link = body.get("next_link")  # Optional param
 
@@ -251,10 +248,7 @@ async def on_POST(self, request):
                 if "medium" not in threepid or "address" not in threepid:
                     raise SynapseError(500, "Malformed threepid")
                 if threepid["medium"] == "email":
-                    # For emails, transform the address to lowercase.
-                    # We store all email addreses as lowercase in the DB.
-                    # (See add_threepid in synapse/handlers/auth.py)
-                    threepid["address"] = threepid["address"].lower()
+                    threepid["address"] = canonicalise_email(threepid["address"])
                 # if using email, we must know about the email they're authing with!
                 threepid_user_id = await self.datastore.get_user_id_by_threepid(
                     threepid["medium"], threepid["address"]
@@ -362,10 +356,7 @@ async def on_POST(self, request):
         client_secret = body["client_secret"]
         assert_valid_client_secret(client_secret)
 
-        # For emails, transform the address to lowercase.
-        # We store all email addreses as lowercase in the DB.
-        # (See add_threepid in synapse/handlers/auth.py)
-        email = body["email"].lower()
+        email = canonicalise_email(body["email"])
         send_attempt = body["send_attempt"]
         next_link = body.get("next_link")  # Optional param
 
@@ -376,9 +367,7 @@ async def on_POST(self, request):
                 Codes.THREEPID_DENIED,
             )
 
-        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", email)
 
         if existing_user_id is not None:
             raise SynapseError(400, "Email is already in use", Codes.THREEPID_IN_USE)
diff --git a/synapse/rest/client/v2_alpha/register.py b/synapse/rest/client/v2_alpha/register.py
index 25fe3cbf172d..663c5572e5f5 100644
--- a/synapse/rest/client/v2_alpha/register.py
+++ b/synapse/rest/client/v2_alpha/register.py
@@ -50,7 +50,7 @@
 from synapse.util.msisdn import phone_number_to_msisdn
 from synapse.util.ratelimitutils import FederationRateLimiter
 from synapse.util.stringutils import assert_valid_client_secret
-from synapse.util.threepids import check_3pid_allowed
+from synapse.util.threepids import canonicalise_email, check_3pid_allowed
 
 from ._base import client_patterns, interactive_auth_handler
 
@@ -119,10 +119,7 @@ async def on_POST(self, request):
         client_secret = body["client_secret"]
         assert_valid_client_secret(client_secret)
 
-        # For emails, transform the address to lowercase.
-        # We store all email addreses as lowercase in the DB.
-        # (See add_threepid in synapse/handlers/auth.py)
-        email = body["email"].lower()
+        email = canonicalise_email(body["email"])
         send_attempt = body["send_attempt"]
         next_link = body.get("next_link")  # Optional param
 
@@ -555,10 +552,12 @@ async def on_POST(self, request):
                 for login_type in [LoginType.EMAIL_IDENTITY, LoginType.MSISDN]:
                     if login_type in auth_result:
                         medium = auth_result[login_type]["medium"]
-                        # For emails, transform the address to lowercase.
-                        # We store all email addreses as lowercase in the DB.
-                        # (See add_threepid in synapse/handlers/auth.py)
-                        address = auth_result[login_type]["address"].lower()
+                        if medium == "email":
+                            address = canonicalise_email(
+                                auth_result[login_type]["address"]
+                            )
+                        else:
+                            address = auth_result[login_type]["address"]
 
                         existing_user_id = await self.store.get_user_id_by_threepid(
                             medium, address
diff --git a/synapse/util/threepids.py b/synapse/util/threepids.py
index 3ec1dfb0c2ea..6821bcead157 100644
--- a/synapse/util/threepids.py
+++ b/synapse/util/threepids.py
@@ -16,6 +16,8 @@
 import logging
 import re
 
+from synapse.api.errors import SynapseError
+
 logger = logging.getLogger(__name__)
 
 
@@ -48,3 +50,23 @@ def check_3pid_allowed(hs, medium, address):
         return True
 
     return False
+
+
+def canonicalise_email(address) -> str:
+    """'Canonicalise' email address
+    Case folding of local part of email address and lowercase domain part
+    See MSC2265, https://github.com/matrix-org/matrix-doc/pull/2265
+
+    Args:
+        address (str): email address within that medium (e.g. "wotan@matrix.org")
+    Returns:
+        (str) The canonical form of the email address
+    Raises:
+        SynapseError if the number could not be parsed.
+    """
+
+    try:
+        address = address.split("@")
+        return address[0].casefold() + "@" + address[1].lower()
+    except IndexError:
+        raise SynapseError(400, "Unable to parse email address")
diff --git a/tests/util/test_threepids.py b/tests/util/test_threepids.py
new file mode 100644
index 000000000000..a8bf510f5bfd
--- /dev/null
+++ b/tests/util/test_threepids.py
@@ -0,0 +1,41 @@
+# -*- coding: utf-8 -*-
+# Copyright 2020 Dirk Klimpel
+#
+# 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 synapse.api.errors import SynapseError
+from synapse.util.threepids import canonicalise_email
+
+from tests.unittest import HomeserverTestCase
+
+
+class CanonicaliseEmailTests(HomeserverTestCase):
+    def test_invalid_format(self):
+        with self.assertRaises(SynapseError) as cm:
+            canonicalise_email("address-without-at.bar")
+        e = cm.exception
+        self.assertEqual(e.code, 400)
+
+    def test_valid_format(self):
+        self.assertEqual(canonicalise_email("foo@test.bar"), "foo@test.bar")
+
+    def test_domain_to_lower(self):
+        self.assertEqual(canonicalise_email("foo@TEST.BAR"), "foo@test.bar")
+
+    def test_domain_with_umlaut(self):
+        self.assertEqual(canonicalise_email("foo@Öumlaut.com"), "foo@öumlaut.com")
+
+    def test_address_casefold(self):
+        self.assertEqual(
+            canonicalise_email("Strauß@Example.com"), "strauss@example.com"
+        )

From 288d2db08360c993785586f45d140deb3631b734 Mon Sep 17 00:00:00 2001
From: dklimpel <5740567+dklimpel@users.noreply.github.com>
Date: Wed, 13 May 2020 23:37:20 +0200
Subject: [PATCH 03/12] update after review

better email address validation
additional tests in `tests.rest.client.v2_alpha.test_account`
---
 synapse/handlers/auth.py                   |  10 ++
 synapse/rest/admin/users.py                |   5 -
 synapse/rest/client/v1/login.py            |   3 +
 synapse/rest/client/v2_alpha/account.py    |   3 +
 synapse/rest/client/v2_alpha/register.py   |   7 +-
 synapse/util/threepids.py                  |  18 +++-
 tests/rest/client/v2_alpha/test_account.py | 104 +++++++++++++++------
 tests/util/test_threepids.py               |  19 +++-
 8 files changed, 123 insertions(+), 46 deletions(-)

diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py
index dd60e0f314ec..a561881dd972 100644
--- a/synapse/handlers/auth.py
+++ b/synapse/handlers/auth.py
@@ -900,6 +900,15 @@ async def add_threepid(
                 errcode=Codes.INVALID_PARAM,
             )
 
+        # 'Canonicalise' email addresses down to lower case.
+        # We've now moving towards the homeserver being the entity that
+        # is responsible for validating threepids used for resetting passwords
+        # on accounts, so in future Synapse will gain knowledge of specific
+        # types (mediums) of threepid. For now, we still use the existing
+        # infrastructure, but this is the start of synapse gaining knowledge
+        # of specific types of threepid (and fixes the fact that checking
+        # for the presence of an email address during password reset was
+        # case sensitive).
         if medium == "email":
             address = canonicalise_email(address)
 
@@ -927,6 +936,7 @@ async def delete_threepid(
             unbind API.
         """
 
+        # 'Canonicalise' email addresses as per above
         if medium == "email":
             address = canonicalise_email(address)
 
diff --git a/synapse/rest/admin/users.py b/synapse/rest/admin/users.py
index 27e0ee29ba26..593ce011e888 100644
--- a/synapse/rest/admin/users.py
+++ b/synapse/rest/admin/users.py
@@ -36,7 +36,6 @@
     historical_admin_path_patterns,
 )
 from synapse.types import UserID
-from synapse.util.threepids import canonicalise_email
 
 logger = logging.getLogger(__name__)
 
@@ -196,8 +195,6 @@ async def on_PUT(self, request, user_id):
                 # add new threepids to user
                 current_time = self.hs.get_clock().time_msec()
                 for threepid in body["threepids"]:
-                    if threepid["medium"] == "email":
-                        threepid["address"] = canonicalise_email(threepid["address"])
                     await self.auth_handler.add_threepid(
                         user_id, threepid["medium"], threepid["address"], current_time
                     )
@@ -275,8 +272,6 @@ async def on_PUT(self, request, user_id):
 
                 current_time = self.hs.get_clock().time_msec()
                 for threepid in body["threepids"]:
-                    if threepid["medium"] == "email":
-                        threepid["address"] = canonicalise_email(threepid["address"])
                     await self.auth_handler.add_threepid(
                         user_id, threepid["medium"], threepid["address"], current_time
                     )
diff --git a/synapse/rest/client/v1/login.py b/synapse/rest/client/v1/login.py
index 5ede84b38067..1259913249d9 100644
--- a/synapse/rest/client/v1/login.py
+++ b/synapse/rest/client/v1/login.py
@@ -192,6 +192,9 @@ async def _do_other_login(self, login_submission):
             if medium is None or address is None:
                 raise SynapseError(400, "Invalid thirdparty identifier")
 
+            # For emails, canonicalise the address.
+            # We store all email addreses as cononised in the DB.
+            # (See add_threepid in synapse/handlers/auth.py)
             if medium == "email":
                 address = canonicalise_email(address)
 
diff --git a/synapse/rest/client/v2_alpha/account.py b/synapse/rest/client/v2_alpha/account.py
index db1e944763f1..49160363486c 100644
--- a/synapse/rest/client/v2_alpha/account.py
+++ b/synapse/rest/client/v2_alpha/account.py
@@ -261,6 +261,9 @@ async def on_POST(self, request):
                 if "medium" not in threepid or "address" not in threepid:
                     raise SynapseError(500, "Malformed threepid")
                 if threepid["medium"] == "email":
+                    # For emails, canonicalise the address.
+                    # We store all email addreses as cononised in the DB.
+                    # (See add_threepid in synapse/handlers/auth.py)
                     threepid["address"] = canonicalise_email(threepid["address"])
                 # if using email, we must know about the email they're authing with!
                 threepid_user_id = await self.datastore.get_user_id_by_threepid(
diff --git a/synapse/rest/client/v2_alpha/register.py b/synapse/rest/client/v2_alpha/register.py
index 23b2be399cc3..13844a3e080e 100644
--- a/synapse/rest/client/v2_alpha/register.py
+++ b/synapse/rest/client/v2_alpha/register.py
@@ -568,12 +568,9 @@ async def on_POST(self, request):
                 for login_type in [LoginType.EMAIL_IDENTITY, LoginType.MSISDN]:
                     if login_type in auth_result:
                         medium = auth_result[login_type]["medium"]
+                        address = auth_result[login_type]["address"]
                         if medium == "email":
-                            address = canonicalise_email(
-                                auth_result[login_type]["address"]
-                            )
-                        else:
-                            address = auth_result[login_type]["address"]
+                            address = canonicalise_email(address)
 
                         existing_user_id = await self.store.get_user_id_by_threepid(
                             medium, address
diff --git a/synapse/util/threepids.py b/synapse/util/threepids.py
index 6821bcead157..09975e0bbee3 100644
--- a/synapse/util/threepids.py
+++ b/synapse/util/threepids.py
@@ -13,6 +13,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import email.utils
 import logging
 import re
 
@@ -62,11 +63,18 @@ def canonicalise_email(address) -> str:
     Returns:
         (str) The canonical form of the email address
     Raises:
-        SynapseError if the number could not be parsed.
+        SynapseError if the address could not be parsed.
     """
 
-    try:
-        address = address.split("@")
-        return address[0].casefold() + "@" + address[1].lower()
-    except IndexError:
+    address = address.strip()
+    # Validate address
+    # See https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2019-11340
+    parsedAddress = email.utils.parseaddr(address)[1]
+
+    # parseaddr does not find missing "@"
+    regex = r"^[^@]+@[^@]+\.[^@]+$"
+    if parsedAddress == '' or not bool(re.fullmatch(regex, address)):
+        logger.debug("Couldn't parse email address %s", address)
         raise SynapseError(400, "Unable to parse email address")
+    address = address.split("@")
+    return address[0].casefold() + "@" + address[1].lower()
diff --git a/tests/rest/client/v2_alpha/test_account.py b/tests/rest/client/v2_alpha/test_account.py
index 0d6936fd36e2..74794bc26a54 100644
--- a/tests/rest/client/v2_alpha/test_account.py
+++ b/tests/rest/client/v2_alpha/test_account.py
@@ -386,44 +386,39 @@ def prepare(self, reactor, clock, hs):
         self.email = "test@example.com"
         self.url_3pid = b"account/3pid"
 
-    def test_add_email(self):
-        """Test adding an email to profile
-        """
-        client_secret = "foobar"
-        session_id = self._request_token(self.email, client_secret)
+    def test_add_valid_email(self):
+        self.get_success(self._add_email(self.email, self.email))
 
-        self.assertEquals(len(self.email_attempts), 1)
-        link = self._get_link_from_email()
+    def test_add_email_no_at(self):
+        self.get_success(
+            self._request_token_invalid_email("address-without-at.bar")
+        )
 
-        self._validate_token(link)
+    def test_add_email_two_at(self):
+        self.get_success(
+            self._request_token_invalid_email("foo@foo@test.bar")
+        )
 
-        request, channel = self.make_request(
-            "POST",
-            b"/_matrix/client/unstable/account/3pid/add",
-            {
-                "client_secret": client_secret,
-                "sid": session_id,
-                "auth": {
-                    "type": "m.login.password",
-                    "user": self.user_id,
-                    "password": "test",
-                },
-            },
-            access_token=self.user_id_tok,
+    def test_add_email_bad_format(self):
+        self.get_success(
+            self._request_token_invalid_email("user@bad.example.net@good.example.com")
         )
 
-        self.render(request)
-        self.assertEqual(200, int(channel.result["code"]), msg=channel.result["body"])
+    def test_add_email_domain_to_lower(self):
+        self.get_success(self._add_email("foo@TEST.BAR", "foo@test.bar"))
 
-        # Get user
-        request, channel = self.make_request(
-            "GET", self.url_3pid, access_token=self.user_id_tok,
+    def test_add_email_domain_with_umlaut(self):
+        self.get_success(self._add_email("foo@Öumlaut.com", "foo@öumlaut.com"))
+
+    def test_add_email_address_casefold(self):
+        self.get_success(
+            self._add_email("Strauß@Example.com", "strauss@example.com")
         )
-        self.render(request)
 
-        self.assertEqual(200, int(channel.result["code"]), msg=channel.result["body"])
-        self.assertEqual("email", channel.json_body["threepids"][0]["medium"])
-        self.assertEqual(self.email, channel.json_body["threepids"][0]["address"])
+    def test_address_trim(self):
+        self.get_success(
+            self._add_email(" foo@test.bar ", "foo@test.bar")
+        )
 
     def test_add_email_if_disabled(self):
         """Test adding email to profile when doing so is disallowed
@@ -616,6 +611,16 @@ def _request_token(self, email, client_secret):
 
         return channel.json_body["sid"]
 
+    def _request_token_invalid_email(self, email, client_secret="foobar"):
+        request, channel = self.make_request(
+            "POST",
+            b"account/3pid/email/requestToken",
+            {"client_secret": client_secret, "email": email, "send_attempt": 1},
+        )
+        self.render(request)
+        self.assertEqual(400, int(channel.result["code"]), msg=channel.result["body"])
+        self.assertEqual(Codes.UNKNOWN, channel.json_body["errcode"])
+
     def _validate_token(self, link):
         # Remove the host
         path = link.replace("https://example.com", "")
@@ -643,3 +648,42 @@ def _get_link_from_email(self):
         assert match, "Could not find link in email"
 
         return match.group(0)
+
+    def _add_email(self, request_email, expected_email):
+        """Test adding an email to profile
+        """
+        client_secret = "foobar"
+        session_id = self._request_token(request_email, client_secret)
+
+        self.assertEquals(len(self.email_attempts), 1)
+        link = self._get_link_from_email()
+
+        self._validate_token(link)
+
+        request, channel = self.make_request(
+            "POST",
+            b"/_matrix/client/unstable/account/3pid/add",
+            {
+                "client_secret": client_secret,
+                "sid": session_id,
+                "auth": {
+                    "type": "m.login.password",
+                    "user": self.user_id,
+                    "password": "test",
+                },
+            },
+            access_token=self.user_id_tok,
+        )
+
+        self.render(request)
+        self.assertEqual(200, int(channel.result["code"]), msg=channel.result["body"])
+
+        # Get user
+        request, channel = self.make_request(
+            "GET", self.url_3pid, access_token=self.user_id_tok,
+        )
+        self.render(request)
+
+        self.assertEqual(200, int(channel.result["code"]), msg=channel.result["body"])
+        self.assertEqual("email", channel.json_body["threepids"][0]["medium"])
+        self.assertEqual(expected_email, channel.json_body["threepids"][0]["address"])
diff --git a/tests/util/test_threepids.py b/tests/util/test_threepids.py
index a8bf510f5bfd..d0a0e995bfd6 100644
--- a/tests/util/test_threepids.py
+++ b/tests/util/test_threepids.py
@@ -20,12 +20,24 @@
 
 
 class CanonicaliseEmailTests(HomeserverTestCase):
-    def test_invalid_format(self):
+    def test_no_at(self):
         with self.assertRaises(SynapseError) as cm:
             canonicalise_email("address-without-at.bar")
         e = cm.exception
         self.assertEqual(e.code, 400)
 
+    def test_two_at(self):
+        with self.assertRaises(SynapseError) as cm:
+            canonicalise_email("foo@foo@test.bar")
+        e = cm.exception
+        self.assertEqual(e.code, 400)
+
+    def test_bad_format(self):
+        with self.assertRaises(SynapseError) as cm:
+            canonicalise_email("user@bad.example.net@good.example.com")
+        e = cm.exception
+        self.assertEqual(e.code, 400)
+
     def test_valid_format(self):
         self.assertEqual(canonicalise_email("foo@test.bar"), "foo@test.bar")
 
@@ -39,3 +51,8 @@ def test_address_casefold(self):
         self.assertEqual(
             canonicalise_email("Strauß@Example.com"), "strauss@example.com"
         )
+
+    def test_address_trim(self):
+        self.assertEqual(
+            canonicalise_email(" foo@test.bar "), "foo@test.bar"
+        )

From 790c0df3f8ded00c7c88b80083ead5ce79305ba8 Mon Sep 17 00:00:00 2001
From: dklimpel <5740567+dklimpel@users.noreply.github.com>
Date: Wed, 13 May 2020 23:42:54 +0200
Subject: [PATCH 04/12] code style

---
 synapse/util/threepids.py                  |  2 +-
 tests/rest/client/v2_alpha/test_account.py | 16 ++++------------
 tests/util/test_threepids.py               |  4 +---
 3 files changed, 6 insertions(+), 16 deletions(-)

diff --git a/synapse/util/threepids.py b/synapse/util/threepids.py
index 09975e0bbee3..ffa41ad8f0d7 100644
--- a/synapse/util/threepids.py
+++ b/synapse/util/threepids.py
@@ -73,7 +73,7 @@ def canonicalise_email(address) -> str:
 
     # parseaddr does not find missing "@"
     regex = r"^[^@]+@[^@]+\.[^@]+$"
-    if parsedAddress == '' or not bool(re.fullmatch(regex, address)):
+    if parsedAddress == "" or not bool(re.fullmatch(regex, address)):
         logger.debug("Couldn't parse email address %s", address)
         raise SynapseError(400, "Unable to parse email address")
     address = address.split("@")
diff --git a/tests/rest/client/v2_alpha/test_account.py b/tests/rest/client/v2_alpha/test_account.py
index 74794bc26a54..9dc9752fb1ec 100644
--- a/tests/rest/client/v2_alpha/test_account.py
+++ b/tests/rest/client/v2_alpha/test_account.py
@@ -390,14 +390,10 @@ def test_add_valid_email(self):
         self.get_success(self._add_email(self.email, self.email))
 
     def test_add_email_no_at(self):
-        self.get_success(
-            self._request_token_invalid_email("address-without-at.bar")
-        )
+        self.get_success(self._request_token_invalid_email("address-without-at.bar"))
 
     def test_add_email_two_at(self):
-        self.get_success(
-            self._request_token_invalid_email("foo@foo@test.bar")
-        )
+        self.get_success(self._request_token_invalid_email("foo@foo@test.bar"))
 
     def test_add_email_bad_format(self):
         self.get_success(
@@ -411,14 +407,10 @@ def test_add_email_domain_with_umlaut(self):
         self.get_success(self._add_email("foo@Öumlaut.com", "foo@öumlaut.com"))
 
     def test_add_email_address_casefold(self):
-        self.get_success(
-            self._add_email("Strauß@Example.com", "strauss@example.com")
-        )
+        self.get_success(self._add_email("Strauß@Example.com", "strauss@example.com"))
 
     def test_address_trim(self):
-        self.get_success(
-            self._add_email(" foo@test.bar ", "foo@test.bar")
-        )
+        self.get_success(self._add_email(" foo@test.bar ", "foo@test.bar"))
 
     def test_add_email_if_disabled(self):
         """Test adding email to profile when doing so is disallowed
diff --git a/tests/util/test_threepids.py b/tests/util/test_threepids.py
index d0a0e995bfd6..cf7c97a41012 100644
--- a/tests/util/test_threepids.py
+++ b/tests/util/test_threepids.py
@@ -53,6 +53,4 @@ def test_address_casefold(self):
         )
 
     def test_address_trim(self):
-        self.assertEqual(
-            canonicalise_email(" foo@test.bar "), "foo@test.bar"
-        )
+        self.assertEqual(canonicalise_email(" foo@test.bar "), "foo@test.bar")

From 5cd421b8f6b45b797aacc923f5db88e987bc9f13 Mon Sep 17 00:00:00 2001
From: Dirk Klimpel <5740567+dklimpel@users.noreply.github.com>
Date: Fri, 15 May 2020 17:15:25 +0200
Subject: [PATCH 05/12] Apply suggestions from code review

Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
---
 synapse/rest/client/v1/login.py         | 2 +-
 synapse/rest/client/v2_alpha/account.py | 2 +-
 synapse/util/threepids.py               | 6 +++---
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/synapse/rest/client/v1/login.py b/synapse/rest/client/v1/login.py
index 6533f3abd458..c2d39e57fdc2 100644
--- a/synapse/rest/client/v1/login.py
+++ b/synapse/rest/client/v1/login.py
@@ -197,7 +197,7 @@ async def _do_other_login(self, login_submission):
                 raise SynapseError(400, "Invalid thirdparty identifier")
 
             # For emails, canonicalise the address.
-            # We store all email addreses as cononised in the DB.
+            # We store all email addresses canonicalised in the DB.
             # (See add_threepid in synapse/handlers/auth.py)
             if medium == "email":
                 address = canonicalise_email(address)
diff --git a/synapse/rest/client/v2_alpha/account.py b/synapse/rest/client/v2_alpha/account.py
index 49160363486c..60de77fbda95 100644
--- a/synapse/rest/client/v2_alpha/account.py
+++ b/synapse/rest/client/v2_alpha/account.py
@@ -262,7 +262,7 @@ async def on_POST(self, request):
                     raise SynapseError(500, "Malformed threepid")
                 if threepid["medium"] == "email":
                     # For emails, canonicalise the address.
-                    # We store all email addreses as cononised in the DB.
+                    # We store all email addresses canonicalised in the DB.
                     # (See add_threepid in synapse/handlers/auth.py)
                     threepid["address"] = canonicalise_email(threepid["address"])
                 # if using email, we must know about the email they're authing with!
diff --git a/synapse/util/threepids.py b/synapse/util/threepids.py
index ffa41ad8f0d7..afcec54263c2 100644
--- a/synapse/util/threepids.py
+++ b/synapse/util/threepids.py
@@ -53,15 +53,15 @@ def check_3pid_allowed(hs, medium, address):
     return False
 
 
-def canonicalise_email(address) -> str:
+def canonicalise_email(address: str) -> str:
     """'Canonicalise' email address
     Case folding of local part of email address and lowercase domain part
     See MSC2265, https://github.com/matrix-org/matrix-doc/pull/2265
 
     Args:
-        address (str): email address within that medium (e.g. "wotan@matrix.org")
+        address: email address to be canonicalised
     Returns:
-        (str) The canonical form of the email address
+        The canonical form of the email address
     Raises:
         SynapseError if the address could not be parsed.
     """

From 8aecb7fe75f8fda27c3955947f96d4a27b30dcd9 Mon Sep 17 00:00:00 2001
From: dklimpel <5740567+dklimpel@users.noreply.github.com>
Date: Sun, 17 May 2020 21:02:07 +0200
Subject: [PATCH 06/12] Update error handling after review to `ValueError`

---
 synapse/handlers/auth.py                   | 10 ++++++++--
 synapse/rest/client/v1/login.py            |  5 ++++-
 synapse/rest/client/v2_alpha/account.py    | 15 ++++++++++++---
 synapse/rest/client/v2_alpha/register.py   | 10 ++++++++--
 synapse/util/threepids.py                  |  7 +++----
 tests/rest/client/v2_alpha/test_account.py |  1 +
 tests/util/test_threepids.py               | 13 +++----------
 7 files changed, 39 insertions(+), 22 deletions(-)

diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py
index d397fbca6d8c..8243502fee69 100644
--- a/synapse/handlers/auth.py
+++ b/synapse/handlers/auth.py
@@ -936,7 +936,10 @@ async def add_threepid(
         # for the presence of an email address during password reset was
         # case sensitive).
         if medium == "email":
-            address = canonicalise_email(address)
+            try:
+                address = canonicalise_email(address)
+            except ValueError as e:
+                raise SynapseError(400, str(e))
 
         await self.store.user_add_threepid(
             user_id, medium, address, validated_at, self.hs.get_clock().time_msec()
@@ -964,7 +967,10 @@ async def delete_threepid(
 
         # 'Canonicalise' email addresses as per above
         if medium == "email":
-            address = canonicalise_email(address)
+            try:
+                address = canonicalise_email(address)
+            except ValueError as e:
+                raise SynapseError(400, str(e))
 
         identity_handler = self.hs.get_handlers().identity_handler
         result = await identity_handler.try_unbind_threepid(
diff --git a/synapse/rest/client/v1/login.py b/synapse/rest/client/v1/login.py
index c2d39e57fdc2..4244699bc347 100644
--- a/synapse/rest/client/v1/login.py
+++ b/synapse/rest/client/v1/login.py
@@ -200,7 +200,10 @@ async def _do_other_login(self, login_submission):
             # We store all email addresses canonicalised in the DB.
             # (See add_threepid in synapse/handlers/auth.py)
             if medium == "email":
-                address = canonicalise_email(address)
+                try:
+                    address = canonicalise_email(address)
+                except ValueError as e:
+                    raise SynapseError(400, str(e))
 
             # We also apply account rate limiting using the 3PID as a key, as
             # otherwise using 3PID bypasses the ratelimiting based on user ID.
diff --git a/synapse/rest/client/v2_alpha/account.py b/synapse/rest/client/v2_alpha/account.py
index 60de77fbda95..9711bde6b9cb 100644
--- a/synapse/rest/client/v2_alpha/account.py
+++ b/synapse/rest/client/v2_alpha/account.py
@@ -84,7 +84,10 @@ async def on_POST(self, request):
         client_secret = body["client_secret"]
         assert_valid_client_secret(client_secret)
 
-        email = canonicalise_email(body["email"])
+        try:
+            email = canonicalise_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
 
@@ -264,7 +267,10 @@ async def on_POST(self, request):
                     # For emails, canonicalise the address.
                     # We store all email addresses canonicalised in the DB.
                     # (See add_threepid in synapse/handlers/auth.py)
-                    threepid["address"] = canonicalise_email(threepid["address"])
+                    try:
+                        threepid["address"] = canonicalise_email(threepid["address"])
+                    except ValueError as e:
+                        raise SynapseError(400, str(e))
                 # if using email, we must know about the email they're authing with!
                 threepid_user_id = await self.datastore.get_user_id_by_threepid(
                     threepid["medium"], threepid["address"]
@@ -379,7 +385,10 @@ async def on_POST(self, request):
         client_secret = body["client_secret"]
         assert_valid_client_secret(client_secret)
 
-        email = canonicalise_email(body["email"])
+        try:
+            email = canonicalise_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
 
diff --git a/synapse/rest/client/v2_alpha/register.py b/synapse/rest/client/v2_alpha/register.py
index cc607bdec7c6..5e2f1d87e66c 100644
--- a/synapse/rest/client/v2_alpha/register.py
+++ b/synapse/rest/client/v2_alpha/register.py
@@ -119,7 +119,10 @@ async def on_POST(self, request):
         client_secret = body["client_secret"]
         assert_valid_client_secret(client_secret)
 
-        email = canonicalise_email(body["email"])
+        try:
+            email = canonicalise_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
 
@@ -571,7 +574,10 @@ async def on_POST(self, request):
                         medium = auth_result[login_type]["medium"]
                         address = auth_result[login_type]["address"]
                         if medium == "email":
-                            address = canonicalise_email(address)
+                            try:
+                                address = canonicalise_email(address)
+                            except ValueError as e:
+                                raise SynapseError(400, str(e))
 
                         existing_user_id = await self.store.get_user_id_by_threepid(
                             medium, address
diff --git a/synapse/util/threepids.py b/synapse/util/threepids.py
index afcec54263c2..ed7b9cda0623 100644
--- a/synapse/util/threepids.py
+++ b/synapse/util/threepids.py
@@ -17,8 +17,6 @@
 import logging
 import re
 
-from synapse.api.errors import SynapseError
-
 logger = logging.getLogger(__name__)
 
 
@@ -63,7 +61,7 @@ def canonicalise_email(address: str) -> str:
     Returns:
         The canonical form of the email address
     Raises:
-        SynapseError if the address could not be parsed.
+        ValueError if the address could not be parsed.
     """
 
     address = address.strip()
@@ -75,6 +73,7 @@ def canonicalise_email(address: str) -> str:
     regex = r"^[^@]+@[^@]+\.[^@]+$"
     if parsedAddress == "" or not bool(re.fullmatch(regex, address)):
         logger.debug("Couldn't parse email address %s", address)
-        raise SynapseError(400, "Unable to parse email address")
+        raise ValueError("Unable to parse email address")
+
     address = address.split("@")
     return address[0].casefold() + "@" + address[1].lower()
diff --git a/tests/rest/client/v2_alpha/test_account.py b/tests/rest/client/v2_alpha/test_account.py
index 9dc9752fb1ec..8472f9b0c031 100644
--- a/tests/rest/client/v2_alpha/test_account.py
+++ b/tests/rest/client/v2_alpha/test_account.py
@@ -612,6 +612,7 @@ def _request_token_invalid_email(self, email, client_secret="foobar"):
         self.render(request)
         self.assertEqual(400, int(channel.result["code"]), msg=channel.result["body"])
         self.assertEqual(Codes.UNKNOWN, channel.json_body["errcode"])
+        self.assertEqual("Unable to parse email address", channel.json_body["error"])
 
     def _validate_token(self, link):
         # Remove the host
diff --git a/tests/util/test_threepids.py b/tests/util/test_threepids.py
index cf7c97a41012..5513724d87cf 100644
--- a/tests/util/test_threepids.py
+++ b/tests/util/test_threepids.py
@@ -13,7 +13,6 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-from synapse.api.errors import SynapseError
 from synapse.util.threepids import canonicalise_email
 
 from tests.unittest import HomeserverTestCase
@@ -21,22 +20,16 @@
 
 class CanonicaliseEmailTests(HomeserverTestCase):
     def test_no_at(self):
-        with self.assertRaises(SynapseError) as cm:
+        with self.assertRaises(ValueError):
             canonicalise_email("address-without-at.bar")
-        e = cm.exception
-        self.assertEqual(e.code, 400)
 
     def test_two_at(self):
-        with self.assertRaises(SynapseError) as cm:
+        with self.assertRaises(ValueError):
             canonicalise_email("foo@foo@test.bar")
-        e = cm.exception
-        self.assertEqual(e.code, 400)
 
     def test_bad_format(self):
-        with self.assertRaises(SynapseError) as cm:
+        with self.assertRaises(ValueError):
             canonicalise_email("user@bad.example.net@good.example.com")
-        e = cm.exception
-        self.assertEqual(e.code, 400)
 
     def test_valid_format(self):
         self.assertEqual(canonicalise_email("foo@test.bar"), "foo@test.bar")

From 27866381f18d11c8fb44607ad8ee8c0f34364321 Mon Sep 17 00:00:00 2001
From: dklimpel <5740567+dklimpel@users.noreply.github.com>
Date: Sun, 17 May 2020 21:31:02 +0200
Subject: [PATCH 07/12] Revert error handling in `auth.py`

---
 synapse/handlers/auth.py | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py
index 8243502fee69..d397fbca6d8c 100644
--- a/synapse/handlers/auth.py
+++ b/synapse/handlers/auth.py
@@ -936,10 +936,7 @@ async def add_threepid(
         # for the presence of an email address during password reset was
         # case sensitive).
         if medium == "email":
-            try:
-                address = canonicalise_email(address)
-            except ValueError as e:
-                raise SynapseError(400, str(e))
+            address = canonicalise_email(address)
 
         await self.store.user_add_threepid(
             user_id, medium, address, validated_at, self.hs.get_clock().time_msec()
@@ -967,10 +964,7 @@ async def delete_threepid(
 
         # 'Canonicalise' email addresses as per above
         if medium == "email":
-            try:
-                address = canonicalise_email(address)
-            except ValueError as e:
-                raise SynapseError(400, str(e))
+            address = canonicalise_email(address)
 
         identity_handler = self.hs.get_handlers().identity_handler
         result = await identity_handler.try_unbind_threepid(

From 35560eb1d19cc14c6da825edccde87b8352a256f Mon Sep 17 00:00:00 2001
From: Dirk Klimpel <5740567+dklimpel@users.noreply.github.com>
Date: Tue, 2 Jun 2020 14:41:46 +0200
Subject: [PATCH 08/12] Apply suggestions from code review

Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
---
 changelog.d/7021.bugfix | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/changelog.d/7021.bugfix b/changelog.d/7021.bugfix
index 708e09c3f946..140fe37b2d29 100644
--- a/changelog.d/7021.bugfix
+++ b/changelog.d/7021.bugfix
@@ -1 +1 @@
-Fix inconsistent handling of upper and lower cases of email addresses that is use only case folding (MSC2265).
\ No newline at end of file
+Fix inconsistent handling of upper and lower case in email addresses when used as identifiers for login, etc. Contributed by @dklimpel.

From 19218d1dc1aa9024d384486f91fd5f71ec9b20c0 Mon Sep 17 00:00:00 2001
From: dklimpel <5740567+dklimpel@users.noreply.github.com>
Date: Wed, 17 Jun 2020 11:53:15 +0200
Subject: [PATCH 09/12] Add tests, comments, password reset with mail from
 database

---
 synapse/rest/client/v2_alpha/account.py    | 30 +++++++-
 synapse/rest/client/v2_alpha/register.py   |  8 ++
 synapse/util/threepids.py                  |  3 +-
 tests/rest/client/v2_alpha/test_account.py | 90 ++++++++++++++++++++--
 4 files changed, 122 insertions(+), 9 deletions(-)

diff --git a/synapse/rest/client/v2_alpha/account.py b/synapse/rest/client/v2_alpha/account.py
index 9711bde6b9cb..a1c31e782135 100644
--- a/synapse/rest/client/v2_alpha/account.py
+++ b/synapse/rest/client/v2_alpha/account.py
@@ -47,6 +47,7 @@ def __init__(self, hs):
         self.datastore = hs.get_datastore()
         self.config = hs.config
         self.identity_handler = hs.get_handlers().identity_handler
+        self.account_validity_handler = hs.get_account_validity_handler()
 
         if self.config.threepid_behaviour_email == ThreepidBehaviour.LOCAL:
             template_html, template_text = load_jinja2_templates(
@@ -84,6 +85,11 @@ async def on_POST(self, request):
         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 allows the user to reset his password without having to
+        # know the exact spelling (eg. upper and lower case) of address in the database.
+        # Stored in the database "foo@bar.com"
+        # User requests with "FOO@bar.com" would raise a Not Found error
         try:
             email = canonicalise_email(body["email"])
         except ValueError as e:
@@ -110,13 +116,27 @@ async def on_POST(self, request):
 
             raise SynapseError(400, "Email not found", Codes.THREEPID_NOT_FOUND)
 
+        # Get the email addresses of the user from database
+        # The user can own more than one address. Lookup for the right address.
+        # The email will be sent to the stored address.
+        # It should prevent potential account hijack via password reset form,
+        # if some compare algorithm are not exactly.
+        addresses = await self.account_validity_handler._get_email_addresses_for_user(
+            existing_user_id
+        )
+        for address in addresses:
+            if address == email:
+                email_from_database = address
+        if not email_from_database:
+            raise SynapseError(400, "Email not found", Codes.THREEPID_NOT_FOUND)
+
         if self.config.threepid_behaviour_email == ThreepidBehaviour.REMOTE:
             assert self.hs.config.account_threepid_delegate_email
 
             # Have the configured identity server handle the request
             ret = await self.identity_handler.requestEmailToken(
                 self.hs.config.account_threepid_delegate_email,
-                email,
+                email_from_database,
                 client_secret,
                 send_attempt,
                 next_link,
@@ -124,7 +144,7 @@ async def on_POST(self, request):
         else:
             # Send password reset emails from Synapse
             sid = await self.identity_handler.send_threepid_validation(
-                email,
+                email_from_database,
                 client_secret,
                 send_attempt,
                 self.mailer.send_password_reset_mail,
@@ -385,6 +405,12 @@ async def on_POST(self, request):
         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 = canonicalise_email(body["email"])
         except ValueError as e:
diff --git a/synapse/rest/client/v2_alpha/register.py b/synapse/rest/client/v2_alpha/register.py
index 5e2f1d87e66c..142f568636a0 100644
--- a/synapse/rest/client/v2_alpha/register.py
+++ b/synapse/rest/client/v2_alpha/register.py
@@ -119,6 +119,10 @@ async def on_POST(self, request):
         client_secret = body["client_secret"]
         assert_valid_client_secret(client_secret)
 
+        # For emails, canonicalise the address.
+        # We store all email addresses canonicalised in the DB.
+        # (See on_POST in EmailThreepidRequestTokenRestServlet
+        # in synapse/rest/client/v2_alpha/account.py)
         try:
             email = canonicalise_email(body["email"])
         except ValueError as e:
@@ -573,6 +577,10 @@ async def on_POST(self, request):
                     if login_type in auth_result:
                         medium = auth_result[login_type]["medium"]
                         address = auth_result[login_type]["address"]
+                        # For emails, canonicalise the address.
+                        # We store all email addresses canonicalised in the DB.
+                        # (See on_POST in EmailThreepidRequestTokenRestServlet
+                        # in synapse/rest/client/v2_alpha/account.py)
                         if medium == "email":
                             try:
                                 address = canonicalise_email(address)
diff --git a/synapse/util/threepids.py b/synapse/util/threepids.py
index ed7b9cda0623..4c755260eda8 100644
--- a/synapse/util/threepids.py
+++ b/synapse/util/threepids.py
@@ -75,5 +75,6 @@ def canonicalise_email(address: str) -> str:
         logger.debug("Couldn't parse email address %s", address)
         raise ValueError("Unable to parse email address")
 
-    address = address.split("@")
+    # double check, split starting from the right and only at first `@`
+    address = address.rsplit("@", 1)
     return address[0].casefold() + "@" + address[1].lower()
diff --git a/tests/rest/client/v2_alpha/test_account.py b/tests/rest/client/v2_alpha/test_account.py
index 8472f9b0c031..c2f12535b8a7 100644
--- a/tests/rest/client/v2_alpha/test_account.py
+++ b/tests/rest/client/v2_alpha/test_account.py
@@ -108,6 +108,46 @@ def test_basic_password_reset(self):
         # Assert we can't log in with the old password
         self.attempt_wrong_password_login("kermit", old_password)
 
+    def test_basic_password_reset_canonicalise_email(self):
+        """Test basic password reset flow
+        Request password reset with different spelling
+        """
+        old_password = "monkey"
+        new_password = "kangeroo"
+
+        user_id = self.register_user("kermit", old_password)
+        self.login("kermit", old_password)
+
+        email_profile = "test@example.com"
+        email_passwort_reset = "TEST@EXAMPLE.COM"
+
+        # Add a threepid
+        self.get_success(
+            self.store.user_add_threepid(
+                user_id=user_id,
+                medium="email",
+                address=email_profile,
+                validated_at=0,
+                added_at=0,
+            )
+        )
+
+        client_secret = "foobar"
+        session_id = self._request_token(email_passwort_reset, client_secret)
+
+        self.assertEquals(len(self.email_attempts), 1)
+        link = self._get_link_from_email()
+
+        self._validate_token(link)
+
+        self._reset_password(new_password, session_id, client_secret)
+
+        # Assert we can log in with the new password
+        self.login("kermit", new_password)
+
+        # Assert we can't log in with the old password
+        self.attempt_wrong_password_login("kermit", old_password)
+
     def test_cant_reset_password_without_clicking_link(self):
         """Test that we do actually need to click the link in the email
         """
@@ -389,15 +429,51 @@ def prepare(self, reactor, clock, hs):
     def test_add_valid_email(self):
         self.get_success(self._add_email(self.email, self.email))
 
+    def test_add_valid_email_second_time(self):
+        self.get_success(self._add_email(self.email, self.email))
+        self.get_success(
+            self._request_token_invalid_email(
+                self.email,
+                expected_errcode=Codes.THREEPID_IN_USE,
+                expected_error="Email is already in use",
+            )
+        )
+
+    def test_add_valid_email_second_time_canonicalise(self):
+        self.get_success(self._add_email(self.email, self.email))
+        self.get_success(
+            self._request_token_invalid_email(
+                "TEST@EXAMPLE.COM",
+                expected_errcode=Codes.THREEPID_IN_USE,
+                expected_error="Email is already in use",
+            )
+        )
+
     def test_add_email_no_at(self):
-        self.get_success(self._request_token_invalid_email("address-without-at.bar"))
+        self.get_success(
+            self._request_token_invalid_email(
+                "address-without-at.bar",
+                expected_errcode=Codes.UNKNOWN,
+                expected_error="Unable to parse email address",
+            )
+        )
 
     def test_add_email_two_at(self):
-        self.get_success(self._request_token_invalid_email("foo@foo@test.bar"))
+        self.get_success(
+            self._request_token_invalid_email(
+                "foo@foo@test.bar",
+                expected_errcode=Codes.UNKNOWN,
+                expected_error="Unable to parse email address",
+            )
+        )
 
     def test_add_email_bad_format(self):
         self.get_success(
-            self._request_token_invalid_email("user@bad.example.net@good.example.com")
+            self._request_token_invalid_email(
+                "user@bad.example.net@good.example.com",
+                expected_errcode=Codes.UNKNOWN,
+                expected_error="Unable to parse email address",
+            )
         )
 
     def test_add_email_domain_to_lower(self):
@@ -603,7 +679,9 @@ def _request_token(self, email, client_secret):
 
         return channel.json_body["sid"]
 
-    def _request_token_invalid_email(self, email, client_secret="foobar"):
+    def _request_token_invalid_email(
+        self, email, expected_errcode, expected_error, client_secret="foobar",
+    ):
         request, channel = self.make_request(
             "POST",
             b"account/3pid/email/requestToken",
@@ -611,8 +689,8 @@ def _request_token_invalid_email(self, email, client_secret="foobar"):
         )
         self.render(request)
         self.assertEqual(400, int(channel.result["code"]), msg=channel.result["body"])
-        self.assertEqual(Codes.UNKNOWN, channel.json_body["errcode"])
-        self.assertEqual("Unable to parse email address", channel.json_body["error"])
+        self.assertEqual(expected_errcode, channel.json_body["errcode"])
+        self.assertEqual(expected_error, channel.json_body["error"])
 
     def _validate_token(self, link):
         # Remove the host

From f418a209ecc1be558ab2f9576ed799d88c7a4abd Mon Sep 17 00:00:00 2001
From: dklimpel <5740567+dklimpel@users.noreply.github.com>
Date: Wed, 17 Jun 2020 19:43:31 +0200
Subject: [PATCH 10/12] Apply suggestions from code review

---
 synapse/rest/client/v2_alpha/account.py | 14 +++++---------
 synapse/util/threepids.py               |  8 +-------
 2 files changed, 6 insertions(+), 16 deletions(-)

diff --git a/synapse/rest/client/v2_alpha/account.py b/synapse/rest/client/v2_alpha/account.py
index a1c31e782135..a36b71dc77a1 100644
--- a/synapse/rest/client/v2_alpha/account.py
+++ b/synapse/rest/client/v2_alpha/account.py
@@ -119,16 +119,12 @@ async def on_POST(self, request):
         # Get the email addresses of the user from database
         # The user can own more than one address. Lookup for the right address.
         # The email will be sent to the stored address.
-        # It should prevent potential account hijack via password reset form,
-        # if some compare algorithm are not exactly.
+        # This avoids a potential account hijack by requesting a password reset to
+        # an email address which is controlled by the attacker but which, after
+        # canonicalisation, matches the one in our database.
         addresses = await self.account_validity_handler._get_email_addresses_for_user(
             existing_user_id
         )
-        for address in addresses:
-            if address == email:
-                email_from_database = address
-        if not email_from_database:
-            raise SynapseError(400, "Email not found", Codes.THREEPID_NOT_FOUND)
 
         if self.config.threepid_behaviour_email == ThreepidBehaviour.REMOTE:
             assert self.hs.config.account_threepid_delegate_email
@@ -136,7 +132,7 @@ async def on_POST(self, request):
             # Have the configured identity server handle the request
             ret = await self.identity_handler.requestEmailToken(
                 self.hs.config.account_threepid_delegate_email,
-                email_from_database,
+                email,
                 client_secret,
                 send_attempt,
                 next_link,
@@ -144,7 +140,7 @@ async def on_POST(self, request):
         else:
             # Send password reset emails from Synapse
             sid = await self.identity_handler.send_threepid_validation(
-                email_from_database,
+                email,
                 client_secret,
                 send_attempt,
                 self.mailer.send_password_reset_mail,
diff --git a/synapse/util/threepids.py b/synapse/util/threepids.py
index 4c755260eda8..880f7f7ac388 100644
--- a/synapse/util/threepids.py
+++ b/synapse/util/threepids.py
@@ -13,7 +13,6 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-import email.utils
 import logging
 import re
 
@@ -65,13 +64,8 @@ def canonicalise_email(address: str) -> str:
     """
 
     address = address.strip()
-    # Validate address
-    # See https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2019-11340
-    parsedAddress = email.utils.parseaddr(address)[1]
 
-    # parseaddr does not find missing "@"
-    regex = r"^[^@]+@[^@]+\.[^@]+$"
-    if parsedAddress == "" or not bool(re.fullmatch(regex, address)):
+    if len(address.split("@")) != 2:
         logger.debug("Couldn't parse email address %s", address)
         raise ValueError("Unable to parse email address")
 

From 4b2aff93f1a068b3af14844cb4b6dc1afdf711d8 Mon Sep 17 00:00:00 2001
From: dklimpel <5740567+dklimpel@users.noreply.github.com>
Date: Wed, 17 Jun 2020 19:51:35 +0200
Subject: [PATCH 11/12] minor correction

---
 synapse/rest/client/v2_alpha/account.py | 15 ++++-----------
 1 file changed, 4 insertions(+), 11 deletions(-)

diff --git a/synapse/rest/client/v2_alpha/account.py b/synapse/rest/client/v2_alpha/account.py
index a36b71dc77a1..45b38b10ad53 100644
--- a/synapse/rest/client/v2_alpha/account.py
+++ b/synapse/rest/client/v2_alpha/account.py
@@ -47,7 +47,6 @@ def __init__(self, hs):
         self.datastore = hs.get_datastore()
         self.config = hs.config
         self.identity_handler = hs.get_handlers().identity_handler
-        self.account_validity_handler = hs.get_account_validity_handler()
 
         if self.config.threepid_behaviour_email == ThreepidBehaviour.LOCAL:
             template_html, template_text = load_jinja2_templates(
@@ -104,6 +103,10 @@ async def on_POST(self, request):
                 Codes.THREEPID_DENIED,
             )
 
+        # The email will be sent to the stored address.
+        # This avoids a potential account hijack by requesting a password reset to
+        # an email address which is controlled by the attacker but which, after
+        # canonicalisation, matches the one in our database.
         existing_user_id = await self.hs.get_datastore().get_user_id_by_threepid(
             "email", email
         )
@@ -116,16 +119,6 @@ async def on_POST(self, request):
 
             raise SynapseError(400, "Email not found", Codes.THREEPID_NOT_FOUND)
 
-        # Get the email addresses of the user from database
-        # The user can own more than one address. Lookup for the right address.
-        # The email will be sent to the stored address.
-        # This avoids a potential account hijack by requesting a password reset to
-        # an email address which is controlled by the attacker but which, after
-        # canonicalisation, matches the one in our database.
-        addresses = await self.account_validity_handler._get_email_addresses_for_user(
-            existing_user_id
-        )
-
         if self.config.threepid_behaviour_email == ThreepidBehaviour.REMOTE:
             assert self.hs.config.account_threepid_delegate_email
 

From 6e634e9493e03219d8b564470fbeb61fe5214345 Mon Sep 17 00:00:00 2001
From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
Date: Fri, 3 Jul 2020 13:34:41 +0100
Subject: [PATCH 12/12] Update synapse/util/threepids.py

---
 synapse/util/threepids.py | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/synapse/util/threepids.py b/synapse/util/threepids.py
index 880f7f7ac388..43c2e0ac230c 100644
--- a/synapse/util/threepids.py
+++ b/synapse/util/threepids.py
@@ -65,10 +65,9 @@ def canonicalise_email(address: str) -> str:
 
     address = address.strip()
 
-    if len(address.split("@")) != 2:
+    parts = address.split("@")
+    if len(parts) != 2:
         logger.debug("Couldn't parse email address %s", address)
         raise ValueError("Unable to parse email address")
 
-    # double check, split starting from the right and only at first `@`
-    address = address.rsplit("@", 1)
-    return address[0].casefold() + "@" + address[1].lower()
+    return parts[0].casefold() + "@" + parts[1].lower()