Add validation of format for 3pid and add validation of 3pid in admin api

changelog.d/7022.misc

@@ -0,0 +1 @@
+Add validation of 3pid in admin api ``PUT /_synapse/admin/v2/users/<user_id>``.

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"

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:

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,

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,

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

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"])