Disabling 3pid changes still sends validation email

[SPiRiT369]

I want to disable removing/adding email addresses, but only allow this during registration.

Reproduce the bug with:

• Set enable_3pid_changes: false on configuration file.
• On client side (e.g. Element Web) - try to add another email address to your account.
• Synapse will send you a verification mail.
• Go back to client side to proceed with adding the verified email.
• You'll only then receive:
  

Synapse should deny the request at start, and not send verification email.

[anoadragon453]

The process of adding an email during registration is:

1. Request an email via POST /_matrix/client/v3/register/email/requestToken.
2. Verify that email by navigating via your browser to a URL provided by the homeserver.
3. Send a request to POST /_matrix/client/v3/register to complete the registration.

For adding an email to your account after it's already been created, the process is:

1. Request an email via POST /_matrix/client/v3/account/3pid/email/requestToken (notice the difference in path)
2. Verify that email by navigating via your browser to a URL provided by the homeserver.
3. Send a request to POST /_matrix/client/v3/account/3pid/add.

Currently the check behind enable_3pid_changes is employed in step 3 of adding an email to your account. I don't see a problem with additionally adding the check to step 1 as well. This would prevent an email from being sent out, and would not affect registration (or password resets).

[santhoshivan23]

@anoadragon453 I guess the check already exists in step 1.

This is the POST handler for the endpoint account/3pid/email/requestToken

    async def on_POST(self, request: SynapseRequest) -> Tuple[int, JsonDict]:
        if self.config.email.threepid_behaviour_email == ThreepidBehaviour.OFF:
            if self.config.email.local_threepid_handling_disabled_due_to_email_config:
                logger.warning(
                    "Adding emails have been disabled due to lack of an email config"
                )
            raise SynapseError(
                400, "Adding an email to your account is disabled on this server"
            )

[a-0-dev]

I would like to bump this issue as it is causing some usability questions in my institution as well. The two relevant points in synapse code are:

synapse/synapse/rest/client/account.py

Line 340 in 1a1abdd

async def on_POST(self, request: SynapseRequest) -> Tuple[int, JsonDict]:

("step 1", as far as I can see the check is not performed in this function)

synapse/synapse/rest/client/account.py

Line 645 in 1a1abdd

async def on_POST(self, request: SynapseRequest) -> Tuple[int, JsonDict]:

("step 3", here the check is performed)

If it somehow helps with the process I can of course post some merge request, but I assume since it's such a minor change it's easier for you (synapse devs) to just change it yourself (?)

Anyways, thanks for your great work!

[JeyRathnam]

Would the fix for this issue be to send back http code 400 and message : "3PID changes are disabled on this server" when enable_3pid_changes is set to false? If so, I have a draft PR, need a bit of help fixing up test case.

Test case: tests.rest.client.test_account.ThreepidEmailRestTestCase.test_add_email_if_disabled calls _request_token

synapse/tests/rest/client/test_account.py

Line 693 in 54c012c

session_id = self._request_token(self.email, client_secret)

_request_token() checks for http status code 200 (OK) and fails as the change would return 400 now

synapse/tests/rest/client/test_account.py

Line 979 in 54c012c

if channel.code != expect_code:

[squahtx]

Would the fix for this issue be to send back http code 400 and message : "3PID changes are disabled on this server" when enable_3pid_changes is set to false? If so, I have a draft PR, need a bit of help fixing up test case.

Yes, that's the fix proposed by anoadragon453 in #12277 (comment).

[JeyRathnam]

I have PR #14682 ready for review.