Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Commit

Permalink
Change validation of mail address from 'lower()' to 'casefold'
Browse files Browse the repository at this point in the history
  • Loading branch information
dklimpel committed May 2, 2020
1 parent ced5716 commit 61ee8a4
Show file tree
Hide file tree
Showing 8 changed files with 87 additions and 42 deletions.
2 changes: 1 addition & 1 deletion changelog.d/7016.bugfix → changelog.d/7021.bugfix
Original file line number Diff line number Diff line change
@@ -1 +1 @@
Fix inconsistent handling of upper and lower cases of email addresses that is use only lower cases.
Fix inconsistent handling of upper and lower cases of email addresses that is use only case folding (MSC2265).
15 changes: 3 additions & 12 deletions synapse/handlers/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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()
Expand All @@ -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(
Expand Down
5 changes: 5 additions & 0 deletions synapse/rest/admin/users.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
historical_admin_path_patterns,
)
from synapse.types import UserID
from synapse.util.threepids import canonicalise_email

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -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
)
Expand Down Expand Up @@ -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
)
Expand Down
6 changes: 2 additions & 4 deletions synapse/rest/client/v1/login.py
Original file line number Diff line number Diff line change
Expand Up @@ -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__)

Expand Down Expand Up @@ -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.
Expand Down
21 changes: 5 additions & 16 deletions synapse/rest/client/v2_alpha/account.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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"]
Expand Down Expand Up @@ -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

Expand All @@ -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)
Expand Down
17 changes: 8 additions & 9 deletions synapse/rest/client/v2_alpha/register.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down
22 changes: 22 additions & 0 deletions synapse/util/threepids.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
import logging
import re

from synapse.api.errors import SynapseError

logger = logging.getLogger(__name__)


Expand Down Expand Up @@ -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")
41 changes: 41 additions & 0 deletions tests/util/test_threepids.py
Original file line number Diff line number Diff line change
@@ -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"
)

0 comments on commit 61ee8a4

Please sign in to comment.