Using Synapse Admin API to add a user fails when request contains "msisdn" third-party ID

[thomasweston12]

Description

According to the documentation here you should be able to set a list of third-party IDs in the request body when adding a User. These can have a "medium" field of either "email" for an email address, or "msisdn" for a phone number.

When attempting to add a new user with a third-party ID of type "msisdn", the request fails, returning a 500 Internal Server Error, and an error code of "M_UNKNOWN".

Checking the logs, it appears that Synapse is treating the address field of the "msisdn" third-party ID as an email address. This can be seen from a log statement saying "Couldn't parse email address [redacted-phone-number]".

I've had a brief look at the code, and it seems that the issue may caused by the for loop in synapse/rest/admin/users.py when adding a new user (line 367 of users.py).

Synapse appears to iterate across all of the third party IDs from the request body, and calls "self.auth_handler.add_threepid()" on them. After adding a threepid, there is an if statement checking if email notifications are enabled, and enabled for new users. If this resolves to true, "self.pusher_pool.add_pusher()" is called.

The arguments passed to the add_pusher() method are hard-coded to email, even when the threepid is of type "msisdn". This seems to cause the add_pusher() method to attempt to parse a phone number as an email address and fail, leading to a 500 internal server error.

Steps to reproduce

Expected Functionality

• Make HTTP PUT request to create a new user.
• Use the /_synapse/admin/v2/users/@redacted.user:domain endpoint, with a request body of:

{
    "password": "mysecretpassword",
    "threepids": [
        {
            "medium": "msisdn",
            "address": "12345678901"
        }
    ]
}

• Returns a success response, the User is created, and the User has a third-party ID set.

Actual Functionality

• Make HTTP PUT request to create a new user.
• Use the /_synapse/admin/v2/users/@redacted.user:domain endpoint, with a request body of:

{
    "password": "mysecretpassword",
    "threepids": [
        {
            "medium": "msisdn",
            "address": "12345678901"
        }
    ]
}

• 500 Internal Server Error is returned with response body:

{
    "errcode": "M_UNKNOWN",
    "error": "Internal server error"
}

Note: Matrix user ID, password, msisdn phone number changed for privacy

Homeserver

Self-hosted homeserver, on my local network, non-public

Synapse Version

1.50.1

Installation Method

Other (please mention below)

Platform

deployed in docker container on CentOS 7, using matrix-docker-ansible-deploy project.

Relevant log output

2022-07-04 13:37:56,550 - synapse.util.threepids - 83 - DEBUG - PUT-589707 - Couldn't parse email address [redacted-phone-number]
Failed handle request via 'UserRestServletV2': <XForwardedForRequest at 0x7fb581821ee0 method='PUT' uri='/_synapse/admin/v2/users/@redacted.user:redacted.domain' clientproto='HTTP/1.0' site='8008'>
Traceback (most recent call last):
File "/usr/local/lib/python3.8/site-packages/synapse/http/server.py", line 269, in _async_render_wrapper
callback_return = await self._async_render(request)
File "/usr/local/lib/python3.8/site-packages/synapse/http/server.py", line 471, in _async_render
callback_return = await raw_callback_return
File "/usr/local/lib/python3.8/site-packages/synapse/rest/admin/users.py", line 372, in on_PUT
await self.pusher_pool.add_pusher(
File "/usr/local/lib/python3.8/site-packages/synapse/push/pusherpool.py", line 118, in add_pusher
"email", canonicalise_email(pushkey)
File "/usr/local/lib/python3.8/site-packages/synapse/util/threepids.py", line 84, in canonicalise_email
raise ValueError("Unable to parse email address")
ValueError: Unable to parse email address

Anything else that would be useful to know?

I can use the same endpoint to "modify" an existing User, and add a "msisdn" third-party ID to them. This works successfully. The issue only seems to occur when using the endpoint to "add" a new User.

[DMRobertson]

I think this is an oversight in #7267. We should only be calling add_pusher if medium == "email". Should be easy enough to fix (he says...)

[DMRobertson]

To fix this:

1. (Optional.) Write a test case which reproduces the failure.
2. Add the condition `medium == "email" to here:
   

   synapse/synapse/rest/admin/users.py

   Lines 373 to 376 in fcd8703

   	if (
   	self.hs.config.email.email_enable_notifs
   	and self.hs.config.email.email_notif_for_new_users
   	):