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

Fix inconsistent handling of upper and lower cases of email addresses. #7021

Merged
merged 15 commits into from
Jul 3, 2020
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/7021.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix inconsistent handling of upper and lower cases of email addresses that is use only case folding (MSC2265).
dklimpel marked this conversation as resolved.
Show resolved Hide resolved
15 changes: 3 additions & 12 deletions synapse/handlers/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
from synapse.module_api import ModuleApi
from synapse.push.mailer import load_jinja2_templates
from synapse.types import Requester, UserID
from synapse.util.threepids import canonicalise_email

from ._base import BaseHandler

Expand Down Expand Up @@ -899,17 +900,8 @@ 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).
dklimpel marked this conversation as resolved.
Show resolved Hide resolved
if medium == "email":
address = address.lower()
address = canonicalise_email(address)

await self.store.user_add_threepid(
user_id, medium, address, validated_at, self.hs.get_clock().time_msec()
Expand All @@ -935,9 +927,8 @@ async def delete_threepid(
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 = await 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"])
dklimpel marked this conversation as resolved.
Show resolved Hide resolved
await self.auth_handler.add_threepid(
user_id, threepid["medium"], threepid["address"], current_time
)
Expand Down Expand Up @@ -272,6 +275,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 @@ -28,6 +28,7 @@
from synapse.rest.well_known import WellKnownBuilder
from synapse.types import UserID
from synapse.util.msisdn import phone_number_to_msisdn
from synapse.util.threepids import canonicalise_email

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -192,10 +193,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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for the record, I have checked that there are no users on matrix.org that will be locked out by this change (there are no users with non-ascii characters in their email addresses). I'm reasonably happy to assume that if it's not a problem on matrix.org, it's not a problem anywhere.


# 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
15 changes: 5 additions & 10 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, random_string
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,7 +84,7 @@ async def on_POST(self, request):
client_secret = body["client_secret"]
assert_valid_client_secret(client_secret)

email = body["email"]
email = canonicalise_email(body["email"])
send_attempt = body["send_attempt"]
next_link = body.get("next_link") # Optional param

Expand Down Expand Up @@ -261,10 +261,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)
dklimpel marked this conversation as resolved.
Show resolved Hide resolved
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 @@ -379,7 +376,7 @@ async def on_POST(self, request):
client_secret = body["client_secret"]
assert_valid_client_secret(client_secret)

email = body["email"]
email = canonicalise_email(body["email"])
send_attempt = body["send_attempt"]
next_link = body.get("next_link") # Optional param

Expand All @@ -390,9 +387,7 @@ async def on_POST(self, request):
Codes.THREEPID_DENIED,
)

existing_user_id = await self.store.get_user_id_by_threepid(
"email", body["email"]
)
existing_user_id = await self.store.get_user_id_by_threepid("email", email)

if existing_user_id is not None:
if self.config.request_token_inhibit_3pid_errors:
Expand Down
13 changes: 9 additions & 4 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, random_string
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,7 +119,7 @@ async def on_POST(self, request):
client_secret = body["client_secret"]
assert_valid_client_secret(client_secret)

email = body["email"]
email = canonicalise_email(body["email"])
send_attempt = body["send_attempt"]
next_link = body.get("next_link") # Optional param

Expand All @@ -131,7 +131,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:
Expand Down Expand Up @@ -568,7 +568,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"]
address = auth_result[login_type]["address"]
if medium == "email":
address = canonicalise_email(
auth_result[login_type]["address"]
)
else:
address = auth_result[login_type]["address"]
dklimpel marked this conversation as resolved.
Show resolved Hide resolved

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:
dklimpel marked this conversation as resolved.
Show resolved Hide resolved
"""'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")
dklimpel marked this conversation as resolved.
Show resolved Hide resolved
Returns:
(str) The canonical form of the email address
dklimpel marked this conversation as resolved.
Show resolved Hide resolved
Raises:
SynapseError if the number could not be parsed.
"""

try:
address = address.split("@")
return address[0].casefold() + "@" + address[1].lower()
dklimpel marked this conversation as resolved.
Show resolved Hide resolved
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"
)