diff --git a/changelog.d/7022.misc b/changelog.d/7022.misc new file mode 100644 index 000000000000..f506c811f83b --- /dev/null +++ b/changelog.d/7022.misc @@ -0,0 +1 @@ +Add validation of 3pid in admin api ``PUT /_synapse/admin/v2/users/``. \ No newline at end of file diff --git a/synapse/api/errors.py b/synapse/api/errors.py index 616942b057b0..413ad0d5c2bf 100644 --- a/synapse/api/errors.py +++ b/synapse/api/errors.py @@ -55,6 +55,7 @@ class Codes(object): THREEPID_IN_USE = "M_THREEPID_IN_USE" THREEPID_NOT_FOUND = "M_THREEPID_NOT_FOUND" THREEPID_DENIED = "M_THREEPID_DENIED" + INVALID_THREEPID = "M_INVALID_THREEPID" INVALID_USERNAME = "M_INVALID_USERNAME" SERVER_NOT_TRUSTED = "M_SERVER_NOT_TRUSTED" CONSENT_NOT_GIVEN = "M_CONSENT_NOT_GIVEN" diff --git a/synapse/rest/admin/users.py b/synapse/rest/admin/users.py index 80f959248dcd..0d3a7e25f1ea 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 check_3pid_allowed, check_3pid_valid_format logger = logging.getLogger(__name__) @@ -195,8 +196,35 @@ 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"]: + # 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 = threepid["address"].lower() + if not check_3pid_valid_format(threepid["medium"], address): + raise SynapseError( + 400, + "Third party identifier has an invalid format", + Codes.INVALID_THREEPID, + ) + + if not check_3pid_allowed(self.hs, threepid["medium"], address): + raise SynapseError( + 403, + "Your email domain or account phone number is not authorized on this server", + Codes.THREEPID_DENIED, + ) + + existing_user_id = await self.store.get_user_id_by_threepid( + threepid["medium"], address + ) + + if existing_user_id is not None: + raise SynapseError( + 400, "Threepid is already in use", Codes.THREEPID_IN_USE + ) + await self.auth_handler.add_threepid( - user_id, threepid["medium"], threepid["address"], current_time + user_id, threepid["medium"], address, current_time ) if "avatar_url" in body: @@ -271,8 +299,35 @@ async def on_PUT(self, request, user_id): current_time = self.hs.get_clock().time_msec() for threepid in body["threepids"]: + # 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 = threepid["address"].lower() + if not check_3pid_valid_format(threepid["medium"], address): + raise SynapseError( + 400, + "Third party identifier has an invalid format", + Codes.INVALID_THREEPID, + ) + + if not check_3pid_allowed(self.hs, threepid["medium"], address): + raise SynapseError( + 403, + "Your email domain or account phone number is not authorized on this server", + Codes.THREEPID_DENIED, + ) + + existing_user_id = await self.store.get_user_id_by_threepid( + threepid["medium"], address + ) + + if existing_user_id is not None: + raise SynapseError( + 400, "Threepid is already in use", Codes.THREEPID_IN_USE + ) + await self.auth_handler.add_threepid( - user_id, threepid["medium"], threepid["address"], current_time + user_id, threepid["medium"], address, current_time ) if "avatar_url" in body: diff --git a/synapse/rest/client/v2_alpha/account.py b/synapse/rest/client/v2_alpha/account.py index dc837d6c7582..4448e5f1cc19 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 check_3pid_allowed, check_3pid_valid_format from ._base import client_patterns, interactive_auth_handler @@ -88,6 +88,13 @@ async def on_POST(self, request): send_attempt = body["send_attempt"] next_link = body.get("next_link") # Optional param + if not check_3pid_valid_format("email", email): + raise SynapseError( + 400, + "Third party identifier has an invalid format", + Codes.INVALID_THREEPID, + ) + if not check_3pid_allowed(self.hs, "email", email): raise SynapseError( 403, @@ -363,6 +370,13 @@ async def on_POST(self, request): send_attempt = body["send_attempt"] next_link = body.get("next_link") # Optional param + if not check_3pid_valid_format("email", email): + raise SynapseError( + 400, + "Third party identifier has an invalid format", + Codes.INVALID_THREEPID, + ) + if not check_3pid_allowed(self.hs, "email", email): raise SynapseError( 403, @@ -428,6 +442,13 @@ async def on_POST(self, request): msisdn = phone_number_to_msisdn(country, phone_number) + if not check_3pid_valid_format("msisdn", msisdn): + raise SynapseError( + 400, + "Third party identifier has an invalid format", + Codes.INVALID_THREEPID, + ) + if not check_3pid_allowed(self.hs, "msisdn", msisdn): raise SynapseError( 403, diff --git a/synapse/rest/client/v2_alpha/register.py b/synapse/rest/client/v2_alpha/register.py index a09189b1b469..5dac8062cb16 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 check_3pid_allowed, check_3pid_valid_format from ._base import client_patterns, interactive_auth_handler @@ -123,6 +123,13 @@ async def on_POST(self, request): send_attempt = body["send_attempt"] next_link = body.get("next_link") # Optional param + if not check_3pid_valid_format("email", email): + raise SynapseError( + 400, + "Third party identifier has an invalid format", + Codes.INVALID_THREEPID, + ) + if not check_3pid_allowed(self.hs, "email", email): raise SynapseError( 403, @@ -190,6 +197,13 @@ async def on_POST(self, request): msisdn = phone_number_to_msisdn(country, phone_number) + if not check_3pid_valid_format("msisdn", msisdn): + raise SynapseError( + 400, + "Third party identifier has an invalid format", + Codes.INVALID_THREEPID, + ) + if not check_3pid_allowed(self.hs, "msisdn", msisdn): raise SynapseError( 403, @@ -514,6 +528,13 @@ async def on_POST(self, request): medium = auth_result[login_type]["medium"] address = auth_result[login_type]["address"] + if not check_3pid_valid_format(medium, address): + raise SynapseError( + 400, + "Third party identifier has an invalid format", + Codes.INVALID_THREEPID, + ) + if not check_3pid_allowed(self.hs, medium, address): raise SynapseError( 403, diff --git a/synapse/util/threepids.py b/synapse/util/threepids.py index 3ec1dfb0c2ea..18704bb4ac49 100644 --- a/synapse/util/threepids.py +++ b/synapse/util/threepids.py @@ -48,3 +48,24 @@ def check_3pid_allowed(hs, medium, address): return True return False + + +def check_3pid_valid_format(medium, address) -> bool: + """Checks whether 3pid has a valid format + + Args: + medium (str): 3pid medium - e.g. email, msisdn + address (str): 3pid address to check (e.g. "wotan@matrix.org") + Returns: + bool: whether the email address has a valid format + """ + + # medium must be "email" or "msisdn" + if medium == "email": + regex = r"^[^@]+@[^@]+\.[^@]+$" + return bool(re.fullmatch(regex, address)) + # no validation/pattern for "msisdn" at the moment + elif medium == "msisdn": + return True + else: + return False diff --git a/tests/rest/admin/test_user.py b/tests/rest/admin/test_user.py index 6416fb5d2a3e..cac562ce2f17 100644 --- a/tests/rest/admin/test_user.py +++ b/tests/rest/admin/test_user.py @@ -22,6 +22,7 @@ import synapse.rest.admin from synapse.api.constants import UserTypes +from synapse.api.errors import Codes from synapse.rest.client.v1 import login from tests import unittest @@ -599,6 +600,79 @@ def test_set_threepid(self): self.assertEqual("email", channel.json_body["threepids"][0]["medium"]) self.assertEqual("bob3@bob.bob", channel.json_body["threepids"][0]["address"]) + def test_set_duplicate_threepid(self): + """ + Test setting duplicate threepid for different user. + """ + self.hs.config.registration_shared_secret = None + + # Add duplicate threepid with different notations + body = json.dumps( + { + "threepids": [ + {"medium": "email", "address": "bob3@bob.bob"}, + {"medium": "email", "address": "BOB3@bob.BOB"}, + ] + } + ) + + request, channel = self.make_request( + "PUT", + self.url_other_user, + access_token=self.admin_user_tok, + content=body.encode(encoding="utf_8"), + ) + self.render(request) + + self.assertEqual(400, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual(Codes.THREEPID_IN_USE, channel.json_body["errcode"]) + + def test_set_invalid_threepid(self): + """ + Test setting invalid threepid for an other user. + """ + self.hs.config.registration_shared_secret = None + + # Add threepid to user + body = json.dumps({"threepids": [{"medium": "email", "address": "bob3@bob"}]}) + + request, channel = self.make_request( + "PUT", + self.url_other_user, + access_token=self.admin_user_tok, + content=body.encode(encoding="utf_8"), + ) + self.render(request) + + self.assertEqual(400, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual(Codes.INVALID_THREEPID, channel.json_body["errcode"]) + + def test_set_not_allowed_threepid(self): + """ + Test setting not allowed threepid for an other user. + """ + self.hs.config.registration_shared_secret = None + self.hs.config.allowed_local_3pids = [ + {"medium": "email", "pattern": r".*@matrix\.org"} + ] + + # Add threepid to user + body = json.dumps( + {"threepids": [{"medium": "email", "address": "bob3@bob.bob"}]} + ) + + request, channel = self.make_request( + "PUT", + self.url_other_user, + access_token=self.admin_user_tok, + content=body.encode(encoding="utf_8"), + ) + self.render(request) + + self.assertEqual(403, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual(Codes.THREEPID_DENIED, channel.json_body["errcode"], + ) + def test_deactivate_user(self): """ Test deactivating another user. @@ -693,7 +767,7 @@ def test_accidental_deactivation_prevention(self): self.assertEqual(200, int(channel.result["code"]), msg=channel.result["body"]) self.assertEqual("@bob:test", channel.json_body["name"]) self.assertEqual("bob", channel.json_body["displayname"]) - self.assertEqual(0, channel.json_body["deactivated"]) + self.assertEqual(False, channel.json_body["deactivated"]) # Change password (and use a str for deactivate instead of a bool) body = json.dumps({"password": "abc123", "deactivated": "false"}) # oops! @@ -719,4 +793,4 @@ def test_accidental_deactivation_prevention(self): self.assertEqual("bob", channel.json_body["displayname"]) # Ensure they're still alive - self.assertEqual(0, channel.json_body["deactivated"]) + self.assertEqual(False, channel.json_body["deactivated"])