From 98fb610cc043e4f6ba77f78aaecef6b646bf61d6 Mon Sep 17 00:00:00 2001 From: 3nprob <74199244+3nprob@users.noreply.github.com> Date: Fri, 29 Jul 2022 10:29:23 +0000 Subject: Revert "Drop support for delegating email validation (#13192)" (#13406) Reverts commit fa71bb18b527d1a3e2629b48640ea67fff2f8c59, and tweaks documentation. Signed-off-by: 3nprob --- synapse/rest/client/account.py | 106 ++++++++++++++++++++++++++++------------- 1 file changed, 72 insertions(+), 34 deletions(-) (limited to 'synapse/rest/client/account.py') diff --git a/synapse/rest/client/account.py b/synapse/rest/client/account.py index 0cc87a4001..50edc6b7d3 100644 --- a/synapse/rest/client/account.py +++ b/synapse/rest/client/account.py @@ -28,6 +28,7 @@ from synapse.api.errors import ( SynapseError, ThreepidValidationError, ) +from synapse.config.emailconfig import ThreepidBehaviour from synapse.handlers.ui_auth import UIAuthSessionDataConstants from synapse.http.server import HttpServer, finish_request, respond_with_html from synapse.http.servlet import ( @@ -63,7 +64,7 @@ class EmailPasswordRequestTokenRestServlet(RestServlet): self.config = hs.config self.identity_handler = hs.get_identity_handler() - if self.config.email.can_verify_email: + if self.config.email.threepid_behaviour_email == ThreepidBehaviour.LOCAL: self.mailer = Mailer( hs=self.hs, app_name=self.config.email.email_app_name, @@ -72,10 +73,11 @@ class EmailPasswordRequestTokenRestServlet(RestServlet): ) async def on_POST(self, request: SynapseRequest) -> Tuple[int, JsonDict]: - if not self.config.email.can_verify_email: - logger.warning( - "User password resets have been disabled due to lack of email config" - ) + if self.config.email.threepid_behaviour_email == ThreepidBehaviour.OFF: + if self.config.email.local_threepid_handling_disabled_due_to_email_config: + logger.warning( + "User password resets have been disabled due to lack of email config" + ) raise SynapseError( 400, "Email-based password resets have been disabled on this server" ) @@ -127,21 +129,35 @@ class EmailPasswordRequestTokenRestServlet(RestServlet): raise SynapseError(400, "Email not found", Codes.THREEPID_NOT_FOUND) - # Send password reset emails from Synapse - sid = await self.identity_handler.send_threepid_validation( - email, - client_secret, - send_attempt, - self.mailer.send_password_reset_mail, - next_link, - ) + if self.config.email.threepid_behaviour_email == ThreepidBehaviour.REMOTE: + assert self.hs.config.registration.account_threepid_delegate_email + + # Have the configured identity server handle the request + ret = await self.identity_handler.request_email_token( + self.hs.config.registration.account_threepid_delegate_email, + email, + client_secret, + send_attempt, + next_link, + ) + else: + # Send password reset emails from Synapse + sid = await self.identity_handler.send_threepid_validation( + email, + client_secret, + send_attempt, + self.mailer.send_password_reset_mail, + next_link, + ) + + # Wrap the session id in a JSON object + ret = {"sid": sid} threepid_send_requests.labels(type="email", reason="password_reset").observe( send_attempt ) - # Wrap the session id in a JSON object - return 200, {"sid": sid} + return 200, ret class PasswordRestServlet(RestServlet): @@ -333,7 +349,7 @@ class EmailThreepidRequestTokenRestServlet(RestServlet): self.identity_handler = hs.get_identity_handler() self.store = self.hs.get_datastores().main - if self.config.email.can_verify_email: + if self.config.email.threepid_behaviour_email == ThreepidBehaviour.LOCAL: self.mailer = Mailer( hs=self.hs, app_name=self.config.email.email_app_name, @@ -342,10 +358,11 @@ class EmailThreepidRequestTokenRestServlet(RestServlet): ) async def on_POST(self, request: SynapseRequest) -> Tuple[int, JsonDict]: - if not self.config.email.can_verify_email: - logger.warning( - "Adding emails have been disabled due to lack of an email config" - ) + 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" ) @@ -396,20 +413,35 @@ class EmailThreepidRequestTokenRestServlet(RestServlet): raise SynapseError(400, "Email is already in use", Codes.THREEPID_IN_USE) - sid = await self.identity_handler.send_threepid_validation( - email, - client_secret, - send_attempt, - self.mailer.send_add_threepid_mail, - next_link, - ) + if self.config.email.threepid_behaviour_email == ThreepidBehaviour.REMOTE: + assert self.hs.config.registration.account_threepid_delegate_email + + # Have the configured identity server handle the request + ret = await self.identity_handler.request_email_token( + self.hs.config.registration.account_threepid_delegate_email, + email, + client_secret, + send_attempt, + next_link, + ) + else: + # Send threepid validation emails from Synapse + sid = await self.identity_handler.send_threepid_validation( + email, + client_secret, + send_attempt, + self.mailer.send_add_threepid_mail, + next_link, + ) + + # Wrap the session id in a JSON object + ret = {"sid": sid} threepid_send_requests.labels(type="email", reason="add_threepid").observe( send_attempt ) - # Wrap the session id in a JSON object - return 200, {"sid": sid} + return 200, ret class MsisdnThreepidRequestTokenRestServlet(RestServlet): @@ -502,19 +534,25 @@ class AddThreepidEmailSubmitTokenServlet(RestServlet): self.config = hs.config self.clock = hs.get_clock() self.store = hs.get_datastores().main - if self.config.email.can_verify_email: + if self.config.email.threepid_behaviour_email == ThreepidBehaviour.LOCAL: self._failure_email_template = ( self.config.email.email_add_threepid_template_failure_html ) async def on_GET(self, request: Request) -> None: - if not self.config.email.can_verify_email: - logger.warning( - "Adding emails have been disabled due to lack of an email config" - ) + 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" ) + elif self.config.email.threepid_behaviour_email == ThreepidBehaviour.REMOTE: + raise SynapseError( + 400, + "This homeserver is not validating threepids.", + ) sid = parse_string(request, "sid", required=True) token = parse_string(request, "token", required=True) -- cgit 1.5.1 From d642ce4b3258012da6c024b0b5d1396d2a3e69dd Mon Sep 17 00:00:00 2001 From: David Robertson Date: Mon, 15 Aug 2022 20:05:57 +0100 Subject: Use Pydantic to systematically validate a first batch of endpoints in `synapse.rest.client.account`. (#13188) --- changelog.d/13188.feature | 1 + mypy.ini | 2 +- poetry.lock | 54 +++++++++++++- pyproject.toml | 3 + synapse/http/servlet.py | 25 +++++++ synapse/rest/client/account.py | 148 ++++++++++++++++---------------------- synapse/rest/client/models.py | 69 ++++++++++++++++++ synapse/rest/models.py | 23 ++++++ tests/rest/client/test_account.py | 10 +-- tests/rest/client/test_models.py | 53 ++++++++++++++ 10 files changed, 296 insertions(+), 92 deletions(-) create mode 100644 changelog.d/13188.feature create mode 100644 synapse/rest/client/models.py create mode 100644 synapse/rest/models.py create mode 100644 tests/rest/client/test_models.py (limited to 'synapse/rest/client/account.py') diff --git a/changelog.d/13188.feature b/changelog.d/13188.feature new file mode 100644 index 0000000000..4c39b74289 --- /dev/null +++ b/changelog.d/13188.feature @@ -0,0 +1 @@ +Improve validation of request bodies for the following client-server API endpoints: [`/account/password`](https://spec.matrix.org/v1.3/client-server-api/#post_matrixclientv3accountpassword), [`/account/password/email/requestToken`](https://spec.matrix.org/v1.3/client-server-api/#post_matrixclientv3accountpasswordemailrequesttoken), [`/account/deactivate`](https://spec.matrix.org/v1.3/client-server-api/#post_matrixclientv3accountdeactivate) and [`/account/3pid/email/requestToken`](https://spec.matrix.org/v1.3/client-server-api/#post_matrixclientv3account3pidemailrequesttoken). diff --git a/mypy.ini b/mypy.ini index 6add272990..e2034e411f 100644 --- a/mypy.ini +++ b/mypy.ini @@ -1,6 +1,6 @@ [mypy] namespace_packages = True -plugins = mypy_zope:plugin, scripts-dev/mypy_synapse_plugin.py +plugins = pydantic.mypy, mypy_zope:plugin, scripts-dev/mypy_synapse_plugin.py follow_imports = normal check_untyped_defs = True show_error_codes = True diff --git a/poetry.lock b/poetry.lock index 1acdb5da56..651659ec98 100644 --- a/poetry.lock +++ b/poetry.lock @@ -778,6 +778,21 @@ category = "main" optional = false python-versions = ">=2.7, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*" +[[package]] +name = "pydantic" +version = "1.9.1" +description = "Data validation and settings management using python type hints" +category = "main" +optional = false +python-versions = ">=3.6.1" + +[package.dependencies] +typing-extensions = ">=3.7.4.3" + +[package.extras] +dotenv = ["python-dotenv (>=0.10.4)"] +email = ["email-validator (>=1.0.3)"] + [[package]] name = "pyflakes" version = "2.4.0" @@ -1563,7 +1578,7 @@ url_preview = ["lxml"] [metadata] lock-version = "1.1" python-versions = "^3.7.1" -content-hash = "c24bbcee7e86dbbe7cdbf49f91a25b310bf21095452641e7440129f59b077f78" +content-hash = "7de518bf27967b3547eab8574342cfb67f87d6b47b4145c13de11112141dbf2d" [metadata.files] attrs = [ @@ -2260,6 +2275,43 @@ pycparser = [ {file = "pycparser-2.21-py2.py3-none-any.whl", hash = "sha256:8ee45429555515e1f6b185e78100aea234072576aa43ab53aefcae078162fca9"}, {file = "pycparser-2.21.tar.gz", hash = "sha256:e644fdec12f7872f86c58ff790da456218b10f863970249516d60a5eaca77206"}, ] +pydantic = [ + {file = "pydantic-1.9.1-cp310-cp310-macosx_10_9_x86_64.whl", hash = "sha256:c8098a724c2784bf03e8070993f6d46aa2eeca031f8d8a048dff277703e6e193"}, + {file = "pydantic-1.9.1-cp310-cp310-macosx_11_0_arm64.whl", hash = "sha256:c320c64dd876e45254bdd350f0179da737463eea41c43bacbee9d8c9d1021f11"}, + {file = "pydantic-1.9.1-cp310-cp310-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:18f3e912f9ad1bdec27fb06b8198a2ccc32f201e24174cec1b3424dda605a310"}, + {file = "pydantic-1.9.1-cp310-cp310-manylinux_2_5_i686.manylinux1_i686.manylinux_2_17_i686.manylinux2014_i686.whl", hash = "sha256:c11951b404e08b01b151222a1cb1a9f0a860a8153ce8334149ab9199cd198131"}, + {file = "pydantic-1.9.1-cp310-cp310-musllinux_1_1_i686.whl", hash = "sha256:8bc541a405423ce0e51c19f637050acdbdf8feca34150e0d17f675e72d119580"}, + {file = "pydantic-1.9.1-cp310-cp310-musllinux_1_1_x86_64.whl", hash = "sha256:e565a785233c2d03724c4dc55464559639b1ba9ecf091288dd47ad9c629433bd"}, + {file = "pydantic-1.9.1-cp310-cp310-win_amd64.whl", hash = "sha256:a4a88dcd6ff8fd47c18b3a3709a89adb39a6373f4482e04c1b765045c7e282fd"}, + {file = "pydantic-1.9.1-cp36-cp36m-macosx_10_9_x86_64.whl", hash = "sha256:447d5521575f18e18240906beadc58551e97ec98142266e521c34968c76c8761"}, + {file = "pydantic-1.9.1-cp36-cp36m-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:985ceb5d0a86fcaa61e45781e567a59baa0da292d5ed2e490d612d0de5796918"}, + {file = "pydantic-1.9.1-cp36-cp36m-manylinux_2_5_i686.manylinux1_i686.manylinux_2_17_i686.manylinux2014_i686.whl", hash = "sha256:059b6c1795170809103a1538255883e1983e5b831faea6558ef873d4955b4a74"}, + {file = "pydantic-1.9.1-cp36-cp36m-musllinux_1_1_i686.whl", hash = "sha256:d12f96b5b64bec3f43c8e82b4aab7599d0157f11c798c9f9c528a72b9e0b339a"}, + {file = "pydantic-1.9.1-cp36-cp36m-musllinux_1_1_x86_64.whl", hash = "sha256:ae72f8098acb368d877b210ebe02ba12585e77bd0db78ac04a1ee9b9f5dd2166"}, + {file = "pydantic-1.9.1-cp36-cp36m-win_amd64.whl", hash = "sha256:79b485767c13788ee314669008d01f9ef3bc05db9ea3298f6a50d3ef596a154b"}, + {file = "pydantic-1.9.1-cp37-cp37m-macosx_10_9_x86_64.whl", hash = "sha256:494f7c8537f0c02b740c229af4cb47c0d39840b829ecdcfc93d91dcbb0779892"}, + {file = "pydantic-1.9.1-cp37-cp37m-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:f0f047e11febe5c3198ed346b507e1d010330d56ad615a7e0a89fae604065a0e"}, + {file = "pydantic-1.9.1-cp37-cp37m-manylinux_2_5_i686.manylinux1_i686.manylinux_2_17_i686.manylinux2014_i686.whl", hash = "sha256:969dd06110cb780da01336b281f53e2e7eb3a482831df441fb65dd30403f4608"}, + {file = "pydantic-1.9.1-cp37-cp37m-musllinux_1_1_i686.whl", hash = "sha256:177071dfc0df6248fd22b43036f936cfe2508077a72af0933d0c1fa269b18537"}, + {file = "pydantic-1.9.1-cp37-cp37m-musllinux_1_1_x86_64.whl", hash = "sha256:9bcf8b6e011be08fb729d110f3e22e654a50f8a826b0575c7196616780683380"}, + {file = "pydantic-1.9.1-cp37-cp37m-win_amd64.whl", hash = "sha256:a955260d47f03df08acf45689bd163ed9df82c0e0124beb4251b1290fa7ae728"}, + {file = "pydantic-1.9.1-cp38-cp38-macosx_10_9_x86_64.whl", hash = "sha256:9ce157d979f742a915b75f792dbd6aa63b8eccaf46a1005ba03aa8a986bde34a"}, + {file = "pydantic-1.9.1-cp38-cp38-macosx_11_0_arm64.whl", hash = "sha256:0bf07cab5b279859c253d26a9194a8906e6f4a210063b84b433cf90a569de0c1"}, + {file = "pydantic-1.9.1-cp38-cp38-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:5d93d4e95eacd313d2c765ebe40d49ca9dd2ed90e5b37d0d421c597af830c195"}, + {file = "pydantic-1.9.1-cp38-cp38-manylinux_2_5_i686.manylinux1_i686.manylinux_2_17_i686.manylinux2014_i686.whl", hash = "sha256:1542636a39c4892c4f4fa6270696902acb186a9aaeac6f6cf92ce6ae2e88564b"}, + {file = "pydantic-1.9.1-cp38-cp38-musllinux_1_1_i686.whl", hash = "sha256:a9af62e9b5b9bc67b2a195ebc2c2662fdf498a822d62f902bf27cccb52dbbf49"}, + {file = "pydantic-1.9.1-cp38-cp38-musllinux_1_1_x86_64.whl", hash = "sha256:fe4670cb32ea98ffbf5a1262f14c3e102cccd92b1869df3bb09538158ba90fe6"}, + {file = "pydantic-1.9.1-cp38-cp38-win_amd64.whl", hash = "sha256:9f659a5ee95c8baa2436d392267988fd0f43eb774e5eb8739252e5a7e9cf07e0"}, + {file = "pydantic-1.9.1-cp39-cp39-macosx_10_9_x86_64.whl", hash = "sha256:b83ba3825bc91dfa989d4eed76865e71aea3a6ca1388b59fc801ee04c4d8d0d6"}, + {file = "pydantic-1.9.1-cp39-cp39-macosx_11_0_arm64.whl", hash = "sha256:1dd8fecbad028cd89d04a46688d2fcc14423e8a196d5b0a5c65105664901f810"}, + {file = "pydantic-1.9.1-cp39-cp39-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:02eefd7087268b711a3ff4db528e9916ac9aa18616da7bca69c1871d0b7a091f"}, + {file = "pydantic-1.9.1-cp39-cp39-manylinux_2_5_i686.manylinux1_i686.manylinux_2_17_i686.manylinux2014_i686.whl", hash = "sha256:7eb57ba90929bac0b6cc2af2373893d80ac559adda6933e562dcfb375029acee"}, + {file = "pydantic-1.9.1-cp39-cp39-musllinux_1_1_i686.whl", hash = "sha256:4ce9ae9e91f46c344bec3b03d6ee9612802682c1551aaf627ad24045ce090761"}, + {file = "pydantic-1.9.1-cp39-cp39-musllinux_1_1_x86_64.whl", hash = "sha256:72ccb318bf0c9ab97fc04c10c37683d9eea952ed526707fabf9ac5ae59b701fd"}, + {file = "pydantic-1.9.1-cp39-cp39-win_amd64.whl", hash = "sha256:61b6760b08b7c395975d893e0b814a11cf011ebb24f7d869e7118f5a339a82e1"}, + {file = "pydantic-1.9.1-py3-none-any.whl", hash = "sha256:4988c0f13c42bfa9ddd2fe2f569c9d54646ce84adc5de84228cfe83396f3bd58"}, + {file = "pydantic-1.9.1.tar.gz", hash = "sha256:1ed987c3ff29fff7fd8c3ea3a3ea877ad310aae2ef9889a119e22d3f2db0691a"}, +] pyflakes = [ {file = "pyflakes-2.4.0-py2.py3-none-any.whl", hash = "sha256:3bb3a3f256f4b7968c9c788781e4ff07dce46bdf12339dcda61053375426ee2e"}, {file = "pyflakes-2.4.0.tar.gz", hash = "sha256:05a85c2872edf37a4ed30b0cce2f6093e1d0581f8c19d7393122da7e25b2b24c"}, diff --git a/pyproject.toml b/pyproject.toml index a9f59a676f..4f1e0b5c19 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -158,6 +158,9 @@ packaging = ">=16.1" # At the time of writing, we only use functions from the version `importlib.metadata` # which shipped in Python 3.8. This corresponds to version 1.4 of the backport. importlib_metadata = { version = ">=1.4", python = "<3.8" } +# This is the most recent version of Pydantic with available on common distros. +pydantic = ">=1.7.4" + # Optional Dependencies diff --git a/synapse/http/servlet.py b/synapse/http/servlet.py index 4ff840ca0e..26aaabfb34 100644 --- a/synapse/http/servlet.py +++ b/synapse/http/servlet.py @@ -23,9 +23,12 @@ from typing import ( Optional, Sequence, Tuple, + Type, + TypeVar, overload, ) +from pydantic import BaseModel, ValidationError from typing_extensions import Literal from twisted.web.server import Request @@ -694,6 +697,28 @@ def parse_json_object_from_request( return content +Model = TypeVar("Model", bound=BaseModel) + + +def parse_and_validate_json_object_from_request( + request: Request, model_type: Type[Model] +) -> Model: + """Parse a JSON object from the body of a twisted HTTP request, then deserialise and + validate using the given pydantic model. + + Raises: + SynapseError if the request body couldn't be decoded as JSON or + if it wasn't a JSON object. + """ + content = parse_json_object_from_request(request, allow_empty_body=False) + try: + instance = model_type.parse_obj(content) + except ValidationError as e: + raise SynapseError(HTTPStatus.BAD_REQUEST, str(e), errcode=Codes.BAD_JSON) + + return instance + + def assert_params_in_dict(body: JsonDict, required: Iterable[str]) -> None: absent = [] for k in required: diff --git a/synapse/rest/client/account.py b/synapse/rest/client/account.py index 50edc6b7d3..e5ee63133b 100644 --- a/synapse/rest/client/account.py +++ b/synapse/rest/client/account.py @@ -15,10 +15,11 @@ # limitations under the License. import logging import random -from http import HTTPStatus from typing import TYPE_CHECKING, Optional, Tuple from urllib.parse import urlparse +from pydantic import StrictBool, StrictStr, constr + from twisted.web.server import Request from synapse.api.constants import LoginType @@ -34,12 +35,15 @@ from synapse.http.server import HttpServer, finish_request, respond_with_html from synapse.http.servlet import ( RestServlet, assert_params_in_dict, + parse_and_validate_json_object_from_request, parse_json_object_from_request, parse_string, ) from synapse.http.site import SynapseRequest from synapse.metrics import threepid_send_requests from synapse.push.mailer import Mailer +from synapse.rest.client.models import AuthenticationData, EmailRequestTokenBody +from synapse.rest.models import RequestBodyModel from synapse.types import JsonDict from synapse.util.msisdn import phone_number_to_msisdn from synapse.util.stringutils import assert_valid_client_secret, random_string @@ -82,32 +86,16 @@ class EmailPasswordRequestTokenRestServlet(RestServlet): 400, "Email-based password resets have been disabled on this server" ) - body = parse_json_object_from_request(request) - - assert_params_in_dict(body, ["client_secret", "email", "send_attempt"]) - - # Extract params from body - client_secret = body["client_secret"] - assert_valid_client_secret(client_secret) - - # Canonicalise the email address. The addresses are all stored canonicalised - # in the database. This allows the user to reset his password without having to - # know the exact spelling (eg. upper and lower case) of address in the database. - # Stored in the database "foo@bar.com" - # User requests with "FOO@bar.com" would raise a Not Found error - try: - email = validate_email(body["email"]) - except ValueError as e: - raise SynapseError(400, str(e)) - send_attempt = body["send_attempt"] - next_link = body.get("next_link") # Optional param + body = parse_and_validate_json_object_from_request( + request, EmailRequestTokenBody + ) - if next_link: + if body.next_link: # Raise if the provided next_link value isn't valid - assert_valid_next_link(self.hs, next_link) + assert_valid_next_link(self.hs, body.next_link) await self.identity_handler.ratelimit_request_token_requests( - request, "email", email + request, "email", body.email ) # The email will be sent to the stored address. @@ -115,7 +103,7 @@ class EmailPasswordRequestTokenRestServlet(RestServlet): # an email address which is controlled by the attacker but which, after # canonicalisation, matches the one in our database. existing_user_id = await self.hs.get_datastores().main.get_user_id_by_threepid( - "email", email + "email", body.email ) if existing_user_id is None: @@ -135,26 +123,26 @@ class EmailPasswordRequestTokenRestServlet(RestServlet): # Have the configured identity server handle the request ret = await self.identity_handler.request_email_token( self.hs.config.registration.account_threepid_delegate_email, - email, - client_secret, - send_attempt, - next_link, + body.email, + body.client_secret, + body.send_attempt, + body.next_link, ) else: # Send password reset emails from Synapse sid = await self.identity_handler.send_threepid_validation( - email, - client_secret, - send_attempt, + body.email, + body.client_secret, + body.send_attempt, self.mailer.send_password_reset_mail, - next_link, + body.next_link, ) # Wrap the session id in a JSON object ret = {"sid": sid} threepid_send_requests.labels(type="email", reason="password_reset").observe( - send_attempt + body.send_attempt ) return 200, ret @@ -172,16 +160,23 @@ class PasswordRestServlet(RestServlet): self.password_policy_handler = hs.get_password_policy_handler() self._set_password_handler = hs.get_set_password_handler() + class PostBody(RequestBodyModel): + auth: Optional[AuthenticationData] = None + logout_devices: StrictBool = True + if TYPE_CHECKING: + # workaround for https://github.com/samuelcolvin/pydantic/issues/156 + new_password: Optional[StrictStr] = None + else: + new_password: Optional[constr(max_length=512, strict=True)] = None + @interactive_auth_handler async def on_POST(self, request: SynapseRequest) -> Tuple[int, JsonDict]: - body = parse_json_object_from_request(request) + body = parse_and_validate_json_object_from_request(request, self.PostBody) # we do basic sanity checks here because the auth layer will store these # in sessions. Pull out the new password provided to us. - new_password = body.pop("new_password", None) + new_password = body.new_password if new_password is not None: - if not isinstance(new_password, str) or len(new_password) > 512: - raise SynapseError(400, "Invalid password") self.password_policy_handler.validate_password(new_password) # there are two possibilities here. Either the user does not have an @@ -201,7 +196,7 @@ class PasswordRestServlet(RestServlet): params, session_id = await self.auth_handler.validate_user_via_ui_auth( requester, request, - body, + body.dict(), "modify your account password", ) except InteractiveAuthIncompleteError as e: @@ -224,7 +219,7 @@ class PasswordRestServlet(RestServlet): result, params, session_id = await self.auth_handler.check_ui_auth( [[LoginType.EMAIL_IDENTITY]], request, - body, + body.dict(), "modify your account password", ) except InteractiveAuthIncompleteError as e: @@ -299,37 +294,33 @@ class DeactivateAccountRestServlet(RestServlet): self.auth_handler = hs.get_auth_handler() self._deactivate_account_handler = hs.get_deactivate_account_handler() + class PostBody(RequestBodyModel): + auth: Optional[AuthenticationData] = None + id_server: Optional[StrictStr] = None + # Not specced, see https://github.com/matrix-org/matrix-spec/issues/297 + erase: StrictBool = False + @interactive_auth_handler async def on_POST(self, request: SynapseRequest) -> Tuple[int, JsonDict]: - body = parse_json_object_from_request(request) - erase = body.get("erase", False) - if not isinstance(erase, bool): - raise SynapseError( - HTTPStatus.BAD_REQUEST, - "Param 'erase' must be a boolean, if given", - Codes.BAD_JSON, - ) + body = parse_and_validate_json_object_from_request(request, self.PostBody) requester = await self.auth.get_user_by_req(request) # allow ASes to deactivate their own users if requester.app_service: await self._deactivate_account_handler.deactivate_account( - requester.user.to_string(), erase, requester + requester.user.to_string(), body.erase, requester ) return 200, {} await self.auth_handler.validate_user_via_ui_auth( requester, request, - body, + body.dict(), "deactivate your account", ) result = await self._deactivate_account_handler.deactivate_account( - requester.user.to_string(), - erase, - requester, - id_server=body.get("id_server"), + requester.user.to_string(), body.erase, requester, id_server=body.id_server ) if result: id_server_unbind_result = "success" @@ -364,28 +355,15 @@ class EmailThreepidRequestTokenRestServlet(RestServlet): "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" + 400, + "Adding an email to your account is disabled on this server", ) - body = parse_json_object_from_request(request) - assert_params_in_dict(body, ["client_secret", "email", "send_attempt"]) - client_secret = body["client_secret"] - assert_valid_client_secret(client_secret) - - # Canonicalise the email address. The addresses are all stored canonicalised - # in the database. - # This ensures that the validation email is sent to the canonicalised address - # as it will later be entered into the database. - # Otherwise the email will be sent to "FOO@bar.com" and stored as - # "foo@bar.com" in database. - try: - email = validate_email(body["email"]) - except ValueError as e: - raise SynapseError(400, str(e)) - send_attempt = body["send_attempt"] - next_link = body.get("next_link") # Optional param + body = parse_and_validate_json_object_from_request( + request, EmailRequestTokenBody + ) - if not await check_3pid_allowed(self.hs, "email", email): + if not await check_3pid_allowed(self.hs, "email", body.email): raise SynapseError( 403, "Your email domain is not authorized on this server", @@ -393,14 +371,14 @@ class EmailThreepidRequestTokenRestServlet(RestServlet): ) await self.identity_handler.ratelimit_request_token_requests( - request, "email", email + request, "email", body.email ) - if next_link: + if body.next_link: # Raise if the provided next_link value isn't valid - assert_valid_next_link(self.hs, next_link) + assert_valid_next_link(self.hs, body.next_link) - existing_user_id = await self.store.get_user_id_by_threepid("email", email) + existing_user_id = await self.store.get_user_id_by_threepid("email", body.email) if existing_user_id is not None: if self.config.server.request_token_inhibit_3pid_errors: @@ -419,26 +397,26 @@ class EmailThreepidRequestTokenRestServlet(RestServlet): # Have the configured identity server handle the request ret = await self.identity_handler.request_email_token( self.hs.config.registration.account_threepid_delegate_email, - email, - client_secret, - send_attempt, - next_link, + body.email, + body.client_secret, + body.send_attempt, + body.next_link, ) else: # Send threepid validation emails from Synapse sid = await self.identity_handler.send_threepid_validation( - email, - client_secret, - send_attempt, + body.email, + body.client_secret, + body.send_attempt, self.mailer.send_add_threepid_mail, - next_link, + body.next_link, ) # Wrap the session id in a JSON object ret = {"sid": sid} threepid_send_requests.labels(type="email", reason="add_threepid").observe( - send_attempt + body.send_attempt ) return 200, ret diff --git a/synapse/rest/client/models.py b/synapse/rest/client/models.py new file mode 100644 index 0000000000..3150602997 --- /dev/null +++ b/synapse/rest/client/models.py @@ -0,0 +1,69 @@ +# Copyright 2022 The Matrix.org Foundation C.I.C. +# +# 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 typing import TYPE_CHECKING, Dict, Optional + +from pydantic import Extra, StrictInt, StrictStr, constr, validator + +from synapse.rest.models import RequestBodyModel +from synapse.util.threepids import validate_email + + +class AuthenticationData(RequestBodyModel): + """ + Data used during user-interactive authentication. + + (The name "Authentication Data" is taken directly from the spec.) + + Additional keys will be present, depending on the `type` field. Use `.dict()` to + access them. + """ + + class Config: + extra = Extra.allow + + session: Optional[StrictStr] = None + type: Optional[StrictStr] = None + + +class EmailRequestTokenBody(RequestBodyModel): + if TYPE_CHECKING: + client_secret: StrictStr + else: + # See also assert_valid_client_secret() + client_secret: constr( + regex="[0-9a-zA-Z.=_-]", # noqa: F722 + min_length=0, + max_length=255, + strict=True, + ) + email: StrictStr + id_server: Optional[StrictStr] + id_access_token: Optional[StrictStr] + next_link: Optional[StrictStr] + send_attempt: StrictInt + + @validator("id_access_token", always=True) + def token_required_for_identity_server( + cls, token: Optional[str], values: Dict[str, object] + ) -> Optional[str]: + if values.get("id_server") is not None and token is None: + raise ValueError("id_access_token is required if an id_server is supplied.") + return token + + # Canonicalise the email address. The addresses are all stored canonicalised + # in the database. This allows the user to reset his password without having to + # know the exact spelling (eg. upper and lower case) of address in the database. + # Without this, an email stored in the database as "foo@bar.com" would cause + # user requests for "FOO@bar.com" to raise a Not Found error. + _email_validator = validator("email", allow_reuse=True)(validate_email) diff --git a/synapse/rest/models.py b/synapse/rest/models.py new file mode 100644 index 0000000000..ac39cda8e5 --- /dev/null +++ b/synapse/rest/models.py @@ -0,0 +1,23 @@ +from pydantic import BaseModel, Extra + + +class RequestBodyModel(BaseModel): + """A custom version of Pydantic's BaseModel which + + - ignores unknown fields and + - does not allow fields to be overwritten after construction, + + but otherwise uses Pydantic's default behaviour. + + Ignoring unknown fields is a useful default. It means that clients can provide + unstable field not known to the server without the request being refused outright. + + Subclassing in this way is recommended by + https://pydantic-docs.helpmanual.io/usage/model_config/#change-behaviour-globally + """ + + class Config: + # By default, ignore fields that we don't recognise. + extra = Extra.ignore + # By default, don't allow fields to be reassigned after parsing. + allow_mutation = False diff --git a/tests/rest/client/test_account.py b/tests/rest/client/test_account.py index 7ae926dc9c..c1a7fb2f8a 100644 --- a/tests/rest/client/test_account.py +++ b/tests/rest/client/test_account.py @@ -488,7 +488,7 @@ class DeactivateTestCase(unittest.HomeserverTestCase): channel = self.make_request( "POST", "account/deactivate", request_data, access_token=tok ) - self.assertEqual(channel.code, 200) + self.assertEqual(channel.code, 200, channel.json_body) class WhoamiTestCase(unittest.HomeserverTestCase): @@ -641,21 +641,21 @@ class ThreepidEmailRestTestCase(unittest.HomeserverTestCase): def test_add_email_no_at(self) -> None: self._request_token_invalid_email( "address-without-at.bar", - expected_errcode=Codes.UNKNOWN, + expected_errcode=Codes.BAD_JSON, expected_error="Unable to parse email address", ) def test_add_email_two_at(self) -> None: self._request_token_invalid_email( "foo@foo@test.bar", - expected_errcode=Codes.UNKNOWN, + expected_errcode=Codes.BAD_JSON, expected_error="Unable to parse email address", ) def test_add_email_bad_format(self) -> None: self._request_token_invalid_email( "user@bad.example.net@good.example.com", - expected_errcode=Codes.UNKNOWN, + expected_errcode=Codes.BAD_JSON, expected_error="Unable to parse email address", ) @@ -1001,7 +1001,7 @@ class ThreepidEmailRestTestCase(unittest.HomeserverTestCase): HTTPStatus.BAD_REQUEST, channel.code, msg=channel.result["body"] ) self.assertEqual(expected_errcode, channel.json_body["errcode"]) - self.assertEqual(expected_error, channel.json_body["error"]) + self.assertIn(expected_error, channel.json_body["error"]) def _validate_token(self, link: str) -> None: # Remove the host diff --git a/tests/rest/client/test_models.py b/tests/rest/client/test_models.py new file mode 100644 index 0000000000..a9da00665e --- /dev/null +++ b/tests/rest/client/test_models.py @@ -0,0 +1,53 @@ +# Copyright 2022 The Matrix.org Foundation C.I.C. +# +# 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. +import unittest + +from pydantic import ValidationError + +from synapse.rest.client.models import EmailRequestTokenBody + + +class EmailRequestTokenBodyTestCase(unittest.TestCase): + base_request = { + "client_secret": "hunter2", + "email": "alice@wonderland.com", + "send_attempt": 1, + } + + def test_token_required_if_id_server_provided(self) -> None: + with self.assertRaises(ValidationError): + EmailRequestTokenBody.parse_obj( + { + **self.base_request, + "id_server": "identity.wonderland.com", + } + ) + with self.assertRaises(ValidationError): + EmailRequestTokenBody.parse_obj( + { + **self.base_request, + "id_server": "identity.wonderland.com", + "id_access_token": None, + } + ) + + def test_token_typechecked_when_id_server_provided(self) -> None: + with self.assertRaises(ValidationError): + EmailRequestTokenBody.parse_obj( + { + **self.base_request, + "id_server": "identity.wonderland.com", + "id_access_token": 1337, + } + ) -- cgit 1.5.1 From 3a245f6cfe3f35f5a37bcd91f3242ef59dc71332 Mon Sep 17 00:00:00 2001 From: reivilibre Date: Fri, 19 Aug 2022 11:03:29 +0000 Subject: Fix validation problem that occurs when a user tries to deactivate their account or change their password. (#13563) --- changelog.d/13563.feature | 1 + synapse/rest/client/account.py | 6 +++--- tests/handlers/test_deactivate_account.py | 15 +++++++++++++++ 3 files changed, 19 insertions(+), 3 deletions(-) create mode 100644 changelog.d/13563.feature (limited to 'synapse/rest/client/account.py') diff --git a/changelog.d/13563.feature b/changelog.d/13563.feature new file mode 100644 index 0000000000..4c39b74289 --- /dev/null +++ b/changelog.d/13563.feature @@ -0,0 +1 @@ +Improve validation of request bodies for the following client-server API endpoints: [`/account/password`](https://spec.matrix.org/v1.3/client-server-api/#post_matrixclientv3accountpassword), [`/account/password/email/requestToken`](https://spec.matrix.org/v1.3/client-server-api/#post_matrixclientv3accountpasswordemailrequesttoken), [`/account/deactivate`](https://spec.matrix.org/v1.3/client-server-api/#post_matrixclientv3accountdeactivate) and [`/account/3pid/email/requestToken`](https://spec.matrix.org/v1.3/client-server-api/#post_matrixclientv3account3pidemailrequesttoken). diff --git a/synapse/rest/client/account.py b/synapse/rest/client/account.py index e5ee63133b..9041e29d6c 100644 --- a/synapse/rest/client/account.py +++ b/synapse/rest/client/account.py @@ -196,7 +196,7 @@ class PasswordRestServlet(RestServlet): params, session_id = await self.auth_handler.validate_user_via_ui_auth( requester, request, - body.dict(), + body.dict(exclude_unset=True), "modify your account password", ) except InteractiveAuthIncompleteError as e: @@ -219,7 +219,7 @@ class PasswordRestServlet(RestServlet): result, params, session_id = await self.auth_handler.check_ui_auth( [[LoginType.EMAIL_IDENTITY]], request, - body.dict(), + body.dict(exclude_unset=True), "modify your account password", ) except InteractiveAuthIncompleteError as e: @@ -316,7 +316,7 @@ class DeactivateAccountRestServlet(RestServlet): await self.auth_handler.validate_user_via_ui_auth( requester, request, - body.dict(), + body.dict(exclude_unset=True), "deactivate your account", ) result = await self._deactivate_account_handler.deactivate_account( diff --git a/tests/handlers/test_deactivate_account.py b/tests/handlers/test_deactivate_account.py index 82baa8f154..7b9b711521 100644 --- a/tests/handlers/test_deactivate_account.py +++ b/tests/handlers/test_deactivate_account.py @@ -322,3 +322,18 @@ class DeactivateAccountTestCase(HomeserverTestCase): ) ), ) + + def test_deactivate_account_needs_auth(self) -> None: + """ + Tests that making a request to /deactivate with an empty body + succeeds in starting the user-interactive auth flow. + """ + req = self.make_request( + "POST", + "account/deactivate", + {}, + access_token=self.token, + ) + + self.assertEqual(req.code, 401, req) + self.assertEqual(req.json_body["flows"], [{"stages": ["m.login.password"]}]) -- cgit 1.5.1 From 956e015413d3da417c1058e3e72d97b3d1bc8170 Mon Sep 17 00:00:00 2001 From: David Robertson Date: Tue, 23 Aug 2022 12:40:00 +0100 Subject: Drop support for delegating email validation, round 2 (#13596) --- CHANGES.md | 12 +++ changelog.d/13596.removal | 1 + docs/upgrade.md | 19 ++++ docs/usage/configuration/config_documentation.md | 5 +- synapse/app/homeserver.py | 3 +- synapse/config/emailconfig.py | 46 ++-------- synapse/config/registration.py | 13 +-- synapse/handlers/identity.py | 56 +----------- synapse/handlers/ui_auth/checkers.py | 21 +---- synapse/rest/client/account.py | 108 ++++++++--------------- synapse/rest/client/register.py | 59 +++++-------- synapse/rest/synapse/client/password_reset.py | 8 +- tests/rest/client/test_register.py | 2 +- 13 files changed, 108 insertions(+), 245 deletions(-) create mode 100644 changelog.d/13596.removal (limited to 'synapse/rest/client/account.py') diff --git a/CHANGES.md b/CHANGES.md index 778713f528..14fafc260d 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,6 +1,12 @@ Synapse 1.66.0rc1 (2022-08-23) ============================== +This release removes the ability for homeservers to delegate email ownership +verification and password reset confirmation to identity servers. This removal +was originally planned for Synapse 1.64, but was later deferred until now. + +See the [upgrade notes](https://matrix-org.github.io/synapse/v1.66/upgrade.html#upgrading-to-v1660) for more details. + Features -------- @@ -33,6 +39,12 @@ Improved Documentation - Fix the doc and some warnings that were referring to the nonexistent `custom_templates_directory` setting (instead of `custom_template_directory`). ([\#13538](https://github.com/matrix-org/synapse/issues/13538)) +Deprecations and Removals +------------------------- + +- Remove the ability for homeservers to delegate email ownership verification + and password reset confirmation to identity servers. See [upgrade notes](https://matrix-org.github.io/synapse/v1.66/upgrade.html#upgrading-to-v1660) for more details. + Internal Changes ---------------- diff --git a/changelog.d/13596.removal b/changelog.d/13596.removal new file mode 100644 index 0000000000..6c12ae75b4 --- /dev/null +++ b/changelog.d/13596.removal @@ -0,0 +1 @@ +Remove the ability for homeservers to delegate email ownership verification and password reset confirmation to identity servers. See [upgrade notes](https://github.com/matrix-org/synapse/blob/release-v1.66/docs/upgrade.md#upgrading-to-v1660) for more details. \ No newline at end of file diff --git a/docs/upgrade.md b/docs/upgrade.md index 47a74b67de..0ab5bfeaf0 100644 --- a/docs/upgrade.md +++ b/docs/upgrade.md @@ -89,6 +89,25 @@ process, for example: dpkg -i matrix-synapse-py3_1.3.0+stretch1_amd64.deb ``` +# Upgrading to v1.66.0 + +## Delegation of email validation no longer supported + +As of this version, Synapse no longer allows the tasks of verifying email address +ownership, and password reset confirmation, to be delegated to an identity server. +This removal was previously planned for Synapse 1.64.0, but was +[delayed](https://github.com/matrix-org/synapse/issues/13421) until now to give +homeserver administrators more notice of the change. + +To continue to allow users to add email addresses to their homeserver accounts, +and perform password resets, make sure that Synapse is configured with a working +email server in the [`email` configuration +section](https://matrix-org.github.io/synapse/latest/usage/configuration/config_documentation.html#email) +(including, at a minimum, a `notif_from` setting.) + +Specifying an `email` setting under `account_threepid_delegates` will now cause +an error at startup. + # Upgrading to v1.64.0 ## Deprecation of the ability to delegate e-mail verification to identity servers diff --git a/docs/usage/configuration/config_documentation.md b/docs/usage/configuration/config_documentation.md index cc72966823..8ae018e628 100644 --- a/docs/usage/configuration/config_documentation.md +++ b/docs/usage/configuration/config_documentation.md @@ -2182,7 +2182,10 @@ their account. by the Matrix Identity Service API [specification](https://matrix.org/docs/spec/identity_service/latest).) -*Updated in Synapse 1.64.0*: The `email` option is deprecated. +*Deprecated in Synapse 1.64.0*: The `email` option is deprecated. + +*Removed in Synapse 1.66.0*: The `email` option has been removed. +If present, Synapse will report a configuration error on startup. Example configuration: ```yaml diff --git a/synapse/app/homeserver.py b/synapse/app/homeserver.py index d98012adeb..68993d91a9 100644 --- a/synapse/app/homeserver.py +++ b/synapse/app/homeserver.py @@ -44,7 +44,6 @@ from synapse.app._base import ( register_start, ) from synapse.config._base import ConfigError, format_config_error -from synapse.config.emailconfig import ThreepidBehaviour from synapse.config.homeserver import HomeServerConfig from synapse.config.server import ListenerConfig from synapse.federation.transport.server import TransportLayerServer @@ -202,7 +201,7 @@ class SynapseHomeServer(HomeServer): } ) - if self.config.email.threepid_behaviour_email == ThreepidBehaviour.LOCAL: + if self.config.email.can_verify_email: from synapse.rest.synapse.client.password_reset import ( PasswordResetSubmitTokenResource, ) diff --git a/synapse/config/emailconfig.py b/synapse/config/emailconfig.py index 66a6dbf1fe..a3af35b7c4 100644 --- a/synapse/config/emailconfig.py +++ b/synapse/config/emailconfig.py @@ -18,7 +18,6 @@ import email.utils import logging import os -from enum import Enum from typing import Any import attr @@ -136,40 +135,22 @@ class EmailConfig(Config): self.email_enable_notifs = email_config.get("enable_notifs", False) - self.threepid_behaviour_email = ( - # Have Synapse handle the email sending if account_threepid_delegates.email - # is not defined - # msisdn is currently always remote while Synapse does not support any method of - # sending SMS messages - ThreepidBehaviour.REMOTE - if self.root.registration.account_threepid_delegate_email - else ThreepidBehaviour.LOCAL - ) - if config.get("trust_identity_server_for_password_resets"): raise ConfigError( - 'The config option "trust_identity_server_for_password_resets" has been removed.' - "Please consult the configuration manual at docs/usage/configuration/config_documentation.md for " - "details and update your config file." + 'The config option "trust_identity_server_for_password_resets" ' + "is no longer supported. Please remove it from the config file." ) - self.local_threepid_handling_disabled_due_to_email_config = False - if ( - self.threepid_behaviour_email == ThreepidBehaviour.LOCAL - and email_config == {} - ): - # We cannot warn the user this has happened here - # Instead do so when a user attempts to reset their password - self.local_threepid_handling_disabled_due_to_email_config = True - - self.threepid_behaviour_email = ThreepidBehaviour.OFF + # If we have email config settings, assume that we can verify ownership of + # email addresses. + self.can_verify_email = email_config != {} # Get lifetime of a validation token in milliseconds self.email_validation_token_lifetime = self.parse_duration( email_config.get("validation_token_lifetime", "1h") ) - if self.threepid_behaviour_email == ThreepidBehaviour.LOCAL: + if self.can_verify_email: missing = [] if not self.email_notif_from: missing.append("email.notif_from") @@ -360,18 +341,3 @@ class EmailConfig(Config): "Config option email.invite_client_location must be a http or https URL", path=("email", "invite_client_location"), ) - - -class ThreepidBehaviour(Enum): - """ - Enum to define the behaviour of Synapse with regards to when it contacts an identity - server for 3pid registration and password resets - - REMOTE = use an external server to send tokens - LOCAL = send tokens ourselves - OFF = disable registration via 3pid and password resets - """ - - REMOTE = "remote" - LOCAL = "local" - OFF = "off" diff --git a/synapse/config/registration.py b/synapse/config/registration.py index 01fb0331bc..a888d976f2 100644 --- a/synapse/config/registration.py +++ b/synapse/config/registration.py @@ -13,7 +13,6 @@ # See the License for the specific language governing permissions and # limitations under the License. import argparse -import logging from typing import Any, Optional from synapse.api.constants import RoomCreationPreset @@ -21,15 +20,11 @@ from synapse.config._base import Config, ConfigError from synapse.types import JsonDict, RoomAlias, UserID from synapse.util.stringutils import random_string_with_symbols, strtobool -logger = logging.getLogger(__name__) - -LEGACY_EMAIL_DELEGATE_WARNING = """\ -Delegation of email verification to an identity server is now deprecated. To +NO_EMAIL_DELEGATE_ERROR = """\ +Delegation of email verification to an identity server is no longer supported. To continue to allow users to add email addresses to their accounts, and use them for password resets, configure Synapse with an SMTP server via the `email` setting, and remove `account_threepid_delegates.email`. - -This will be an error in a future version. """ @@ -64,9 +59,7 @@ class RegistrationConfig(Config): account_threepid_delegates = config.get("account_threepid_delegates") or {} if "email" in account_threepid_delegates: - logger.warning(LEGACY_EMAIL_DELEGATE_WARNING) - - self.account_threepid_delegate_email = account_threepid_delegates.get("email") + raise ConfigError(NO_EMAIL_DELEGATE_ERROR) self.account_threepid_delegate_msisdn = account_threepid_delegates.get("msisdn") self.default_identity_server = config.get("default_identity_server") self.allow_guest_access = config.get("allow_guest_access", False) diff --git a/synapse/handlers/identity.py b/synapse/handlers/identity.py index e5afe84df9..9571d461c8 100644 --- a/synapse/handlers/identity.py +++ b/synapse/handlers/identity.py @@ -26,7 +26,6 @@ from synapse.api.errors import ( SynapseError, ) from synapse.api.ratelimiting import Ratelimiter -from synapse.config.emailconfig import ThreepidBehaviour from synapse.http import RequestTimedOutError from synapse.http.client import SimpleHttpClient from synapse.http.site import SynapseRequest @@ -416,48 +415,6 @@ class IdentityHandler: return session_id - async def request_email_token( - self, - id_server: str, - email: str, - client_secret: str, - send_attempt: int, - next_link: Optional[str] = None, - ) -> JsonDict: - """ - Request an external server send an email on our behalf for the purposes of threepid - validation. - - Args: - id_server: The identity server to proxy to - email: The email to send the message to - client_secret: The unique client_secret sends by the user - send_attempt: Which attempt this is - next_link: A link to redirect the user to once they submit the token - - Returns: - The json response body from the server - """ - params = { - "email": email, - "client_secret": client_secret, - "send_attempt": send_attempt, - } - if next_link: - params["next_link"] = next_link - - try: - data = await self.http_client.post_json_get_json( - id_server + "/_matrix/identity/api/v1/validate/email/requestToken", - params, - ) - return data - except HttpResponseException as e: - logger.info("Proxied requestToken failed: %r", e) - raise e.to_synapse_error() - except RequestTimedOutError: - raise SynapseError(500, "Timed out contacting identity server") - async def requestMsisdnToken( self, id_server: str, @@ -531,18 +488,7 @@ class IdentityHandler: validation_session = None # Try to validate as email - if self.hs.config.email.threepid_behaviour_email == ThreepidBehaviour.REMOTE: - # Remote emails will only be used if a valid identity server is provided. - assert ( - self.hs.config.registration.account_threepid_delegate_email is not None - ) - - # Ask our delegated email identity server - validation_session = await self.threepid_from_creds( - self.hs.config.registration.account_threepid_delegate_email, - threepid_creds, - ) - elif self.hs.config.email.threepid_behaviour_email == ThreepidBehaviour.LOCAL: + if self.hs.config.email.can_verify_email: # Get a validated session matching these details validation_session = await self.store.get_threepid_validation_session( "email", client_secret, sid=sid, validated=True diff --git a/synapse/handlers/ui_auth/checkers.py b/synapse/handlers/ui_auth/checkers.py index 05cebb5d4d..a744d68c64 100644 --- a/synapse/handlers/ui_auth/checkers.py +++ b/synapse/handlers/ui_auth/checkers.py @@ -19,7 +19,6 @@ from twisted.web.client import PartialDownloadError from synapse.api.constants import LoginType from synapse.api.errors import Codes, LoginError, SynapseError -from synapse.config.emailconfig import ThreepidBehaviour from synapse.util import json_decoder if TYPE_CHECKING: @@ -153,7 +152,7 @@ class _BaseThreepidAuthChecker: logger.info("Getting validated threepid. threepidcreds: %r", (threepid_creds,)) - # msisdns are currently always ThreepidBehaviour.REMOTE + # msisdns are currently always verified via the IS if medium == "msisdn": if not self.hs.config.registration.account_threepid_delegate_msisdn: raise SynapseError( @@ -164,18 +163,7 @@ class _BaseThreepidAuthChecker: threepid_creds, ) elif medium == "email": - if ( - self.hs.config.email.threepid_behaviour_email - == ThreepidBehaviour.REMOTE - ): - assert self.hs.config.registration.account_threepid_delegate_email - threepid = await identity_handler.threepid_from_creds( - self.hs.config.registration.account_threepid_delegate_email, - threepid_creds, - ) - elif ( - self.hs.config.email.threepid_behaviour_email == ThreepidBehaviour.LOCAL - ): + if self.hs.config.email.can_verify_email: threepid = None row = await self.store.get_threepid_validation_session( medium, @@ -227,10 +215,7 @@ class EmailIdentityAuthChecker(UserInteractiveAuthChecker, _BaseThreepidAuthChec _BaseThreepidAuthChecker.__init__(self, hs) def is_enabled(self) -> bool: - return self.hs.config.email.threepid_behaviour_email in ( - ThreepidBehaviour.REMOTE, - ThreepidBehaviour.LOCAL, - ) + return self.hs.config.email.can_verify_email async def check_auth(self, authdict: dict, clientip: str) -> Any: return await self._check_threepid("email", authdict) diff --git a/synapse/rest/client/account.py b/synapse/rest/client/account.py index 9041e29d6c..1f9a8ccc23 100644 --- a/synapse/rest/client/account.py +++ b/synapse/rest/client/account.py @@ -29,7 +29,6 @@ from synapse.api.errors import ( SynapseError, ThreepidValidationError, ) -from synapse.config.emailconfig import ThreepidBehaviour from synapse.handlers.ui_auth import UIAuthSessionDataConstants from synapse.http.server import HttpServer, finish_request, respond_with_html from synapse.http.servlet import ( @@ -68,7 +67,7 @@ class EmailPasswordRequestTokenRestServlet(RestServlet): self.config = hs.config self.identity_handler = hs.get_identity_handler() - if self.config.email.threepid_behaviour_email == ThreepidBehaviour.LOCAL: + if self.config.email.can_verify_email: self.mailer = Mailer( hs=self.hs, app_name=self.config.email.email_app_name, @@ -77,11 +76,10 @@ class EmailPasswordRequestTokenRestServlet(RestServlet): ) 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( - "User password resets have been disabled due to lack of email config" - ) + if not self.config.email.can_verify_email: + logger.warning( + "User password resets have been disabled due to lack of email config" + ) raise SynapseError( 400, "Email-based password resets have been disabled on this server" ) @@ -117,35 +115,20 @@ class EmailPasswordRequestTokenRestServlet(RestServlet): raise SynapseError(400, "Email not found", Codes.THREEPID_NOT_FOUND) - if self.config.email.threepid_behaviour_email == ThreepidBehaviour.REMOTE: - assert self.hs.config.registration.account_threepid_delegate_email - - # Have the configured identity server handle the request - ret = await self.identity_handler.request_email_token( - self.hs.config.registration.account_threepid_delegate_email, - body.email, - body.client_secret, - body.send_attempt, - body.next_link, - ) - else: - # Send password reset emails from Synapse - sid = await self.identity_handler.send_threepid_validation( - body.email, - body.client_secret, - body.send_attempt, - self.mailer.send_password_reset_mail, - body.next_link, - ) - - # Wrap the session id in a JSON object - ret = {"sid": sid} - + # Send password reset emails from Synapse + sid = await self.identity_handler.send_threepid_validation( + body.email, + body.client_secret, + body.send_attempt, + self.mailer.send_password_reset_mail, + body.next_link, + ) threepid_send_requests.labels(type="email", reason="password_reset").observe( body.send_attempt ) - return 200, ret + # Wrap the session id in a JSON object + return 200, {"sid": sid} class PasswordRestServlet(RestServlet): @@ -340,7 +323,7 @@ class EmailThreepidRequestTokenRestServlet(RestServlet): self.identity_handler = hs.get_identity_handler() self.store = self.hs.get_datastores().main - if self.config.email.threepid_behaviour_email == ThreepidBehaviour.LOCAL: + if self.config.email.can_verify_email: self.mailer = Mailer( hs=self.hs, app_name=self.config.email.email_app_name, @@ -349,11 +332,10 @@ class EmailThreepidRequestTokenRestServlet(RestServlet): ) 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" - ) + if not self.config.email.can_verify_email: + 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", @@ -391,35 +373,21 @@ class EmailThreepidRequestTokenRestServlet(RestServlet): raise SynapseError(400, "Email is already in use", Codes.THREEPID_IN_USE) - if self.config.email.threepid_behaviour_email == ThreepidBehaviour.REMOTE: - assert self.hs.config.registration.account_threepid_delegate_email - - # Have the configured identity server handle the request - ret = await self.identity_handler.request_email_token( - self.hs.config.registration.account_threepid_delegate_email, - body.email, - body.client_secret, - body.send_attempt, - body.next_link, - ) - else: - # Send threepid validation emails from Synapse - sid = await self.identity_handler.send_threepid_validation( - body.email, - body.client_secret, - body.send_attempt, - self.mailer.send_add_threepid_mail, - body.next_link, - ) - - # Wrap the session id in a JSON object - ret = {"sid": sid} + # Send threepid validation emails from Synapse + sid = await self.identity_handler.send_threepid_validation( + body.email, + body.client_secret, + body.send_attempt, + self.mailer.send_add_threepid_mail, + body.next_link, + ) threepid_send_requests.labels(type="email", reason="add_threepid").observe( body.send_attempt ) - return 200, ret + # Wrap the session id in a JSON object + return 200, {"sid": sid} class MsisdnThreepidRequestTokenRestServlet(RestServlet): @@ -512,24 +480,18 @@ class AddThreepidEmailSubmitTokenServlet(RestServlet): self.config = hs.config self.clock = hs.get_clock() self.store = hs.get_datastores().main - if self.config.email.threepid_behaviour_email == ThreepidBehaviour.LOCAL: + if self.config.email.can_verify_email: self._failure_email_template = ( self.config.email.email_add_threepid_template_failure_html ) async def on_GET(self, request: Request) -> None: - 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" + if not self.config.email.can_verify_email: + logger.warning( + "Adding emails have been disabled due to lack of an email config" ) - elif self.config.email.threepid_behaviour_email == ThreepidBehaviour.REMOTE: raise SynapseError( - 400, - "This homeserver is not validating threepids.", + 400, "Adding an email to your account is disabled on this server" ) sid = parse_string(request, "sid", required=True) diff --git a/synapse/rest/client/register.py b/synapse/rest/client/register.py index 1b953d3fa0..20bab20c8f 100644 --- a/synapse/rest/client/register.py +++ b/synapse/rest/client/register.py @@ -31,7 +31,6 @@ from synapse.api.errors import ( ) from synapse.api.ratelimiting import Ratelimiter from synapse.config import ConfigError -from synapse.config.emailconfig import ThreepidBehaviour from synapse.config.homeserver import HomeServerConfig from synapse.config.ratelimiting import FederationRatelimitSettings from synapse.config.server import is_threepid_reserved @@ -74,7 +73,7 @@ class EmailRegisterRequestTokenRestServlet(RestServlet): self.identity_handler = hs.get_identity_handler() self.config = hs.config - if self.hs.config.email.threepid_behaviour_email == ThreepidBehaviour.LOCAL: + if self.hs.config.email.can_verify_email: self.mailer = Mailer( hs=self.hs, app_name=self.config.email.email_app_name, @@ -83,13 +82,10 @@ class EmailRegisterRequestTokenRestServlet(RestServlet): ) async def on_POST(self, request: SynapseRequest) -> Tuple[int, JsonDict]: - if self.hs.config.email.threepid_behaviour_email == ThreepidBehaviour.OFF: - if ( - self.hs.config.email.local_threepid_handling_disabled_due_to_email_config - ): - logger.warning( - "Email registration has been disabled due to lack of email config" - ) + if not self.hs.config.email.can_verify_email: + logger.warning( + "Email registration has been disabled due to lack of email config" + ) raise SynapseError( 400, "Email-based registration has been disabled on this server" ) @@ -138,35 +134,21 @@ class EmailRegisterRequestTokenRestServlet(RestServlet): raise SynapseError(400, "Email is already in use", Codes.THREEPID_IN_USE) - if self.config.email.threepid_behaviour_email == ThreepidBehaviour.REMOTE: - assert self.hs.config.registration.account_threepid_delegate_email - - # Have the configured identity server handle the request - ret = await self.identity_handler.request_email_token( - self.hs.config.registration.account_threepid_delegate_email, - email, - client_secret, - send_attempt, - next_link, - ) - else: - # Send registration emails from Synapse, - # wrapping the session id in a JSON object. - ret = { - "sid": await self.identity_handler.send_threepid_validation( - email, - client_secret, - send_attempt, - self.mailer.send_registration_mail, - next_link, - ) - } + # Send registration emails from Synapse + sid = await self.identity_handler.send_threepid_validation( + email, + client_secret, + send_attempt, + self.mailer.send_registration_mail, + next_link, + ) threepid_send_requests.labels(type="email", reason="register").observe( send_attempt ) - return 200, ret + # Wrap the session id in a JSON object + return 200, {"sid": sid} class MsisdnRegisterRequestTokenRestServlet(RestServlet): @@ -260,7 +242,7 @@ class RegistrationSubmitTokenServlet(RestServlet): self.clock = hs.get_clock() self.store = hs.get_datastores().main - if self.config.email.threepid_behaviour_email == ThreepidBehaviour.LOCAL: + if self.config.email.can_verify_email: self._failure_email_template = ( self.config.email.email_registration_template_failure_html ) @@ -270,11 +252,10 @@ class RegistrationSubmitTokenServlet(RestServlet): raise SynapseError( 400, "This medium is currently not supported for registration" ) - if self.config.email.threepid_behaviour_email == ThreepidBehaviour.OFF: - if self.config.email.local_threepid_handling_disabled_due_to_email_config: - logger.warning( - "User registration via email has been disabled due to lack of email config" - ) + if not self.config.email.can_verify_email: + logger.warning( + "User registration via email has been disabled due to lack of email config" + ) raise SynapseError( 400, "Email-based registration is disabled on this server" ) diff --git a/synapse/rest/synapse/client/password_reset.py b/synapse/rest/synapse/client/password_reset.py index 6ac9dbc7c9..b9402cfb75 100644 --- a/synapse/rest/synapse/client/password_reset.py +++ b/synapse/rest/synapse/client/password_reset.py @@ -17,7 +17,6 @@ from typing import TYPE_CHECKING, Tuple from twisted.web.server import Request from synapse.api.errors import ThreepidValidationError -from synapse.config.emailconfig import ThreepidBehaviour from synapse.http.server import DirectServeHtmlResource from synapse.http.servlet import parse_string from synapse.util.stringutils import assert_valid_client_secret @@ -46,9 +45,6 @@ class PasswordResetSubmitTokenResource(DirectServeHtmlResource): self.clock = hs.get_clock() self.store = hs.get_datastores().main - self._local_threepid_handling_disabled_due_to_email_config = ( - hs.config.email.local_threepid_handling_disabled_due_to_email_config - ) self._confirmation_email_template = ( hs.config.email.email_password_reset_template_confirmation_html ) @@ -59,8 +55,8 @@ class PasswordResetSubmitTokenResource(DirectServeHtmlResource): hs.config.email.email_password_reset_template_failure_html ) - # This resource should not be mounted if threepid behaviour is not LOCAL - assert hs.config.email.threepid_behaviour_email == ThreepidBehaviour.LOCAL + # This resource should only be mounted if email validation is enabled + assert hs.config.email.can_verify_email async def _async_render_GET(self, request: Request) -> Tuple[int, bytes]: sid = parse_string(request, "sid", required=True) diff --git a/tests/rest/client/test_register.py b/tests/rest/client/test_register.py index ab4277dd31..b781875d52 100644 --- a/tests/rest/client/test_register.py +++ b/tests/rest/client/test_register.py @@ -586,9 +586,9 @@ class RegisterRestServletTestCase(unittest.HomeserverTestCase): "require_at_registration": True, }, "account_threepid_delegates": { - "email": "https://id_server", "msisdn": "https://id_server", }, + "email": {"notif_from": "Synapse "}, } ) def test_advertised_flows_captcha_and_terms_and_3pids(self) -> None: -- cgit 1.5.1