diff --git a/changelog.d/20104.bugfix b/changelog.d/20104.bugfix new file mode 100644 index 0000000000..f04945660c --- /dev/null +++ b/changelog.d/20104.bugfix @@ -0,0 +1 @@ +Fix a regression where `client_secret` request parameters were not validated against the character set required by the spec on Pydantic-validated endpoints, regressed in Synapse 1.66.0 (originally fixed for https://github.com/matrix-org/synapse/issues/6766). diff --git a/synapse/types/rest/client/__init__.py b/synapse/types/rest/client/__init__.py index 17647e621d..dbd4455b82 100644 --- a/synapse/types/rest/client/__init__.py +++ b/synapse/types/rest/client/__init__.py @@ -62,7 +62,7 @@ class AuthenticationData(RequestBodyModel): ClientSecretStr = Annotated[ str, StringConstraints( - pattern="[0-9a-zA-Z.=_-]", + pattern="^[0-9a-zA-Z.=_-]+$", min_length=1, max_length=255, strict=True, diff --git a/tests/rest/client/test_models.py b/tests/rest/client/test_models.py index f297856830..e5dc922023 100644 --- a/tests/rest/client/test_models.py +++ b/tests/rest/client/test_models.py @@ -23,7 +23,7 @@ from typing import Literal from pydantic import BaseModel, ValidationError -from synapse.types.rest.client import EmailRequestTokenBody +from synapse.types.rest.client import ClientSecretStr, EmailRequestTokenBody class ThreepidMediumEnumTestCase(stdlib_unittest.TestCase): @@ -48,6 +48,44 @@ class ThreepidMediumEnumTestCase(stdlib_unittest.TestCase): self.Model.model_validate({"medium": 123}) +class ClientSecretStrTestCase(stdlib_unittest.TestCase): + class Model(BaseModel): + client_secret: ClientSecretStr + + def test_accepts_valid_client_secrets(self) -> None: + """Secrets consisting entirely of `[0-9a-zA-Z.=_-]` are accepted.""" + for client_secret in ( + "this.is-a_valid=secret", + "foobar", + "a", + "0123456789", + "a" * 255, + ): + with self.subTest(client_secret=client_secret): + model = self.Model.model_validate({"client_secret": client_secret}) + self.assertEqual(model.client_secret, client_secret) + + def test_rejects_client_secrets_with_invalid_characters(self) -> None: + for client_secret in ( + "foo bar", + "secret!", + "café", + # Little bobby tables + "Robert'; DROP TABLE students;--", + ): + with self.subTest(client_secret=client_secret): + with self.assertRaises(ValidationError): + self.Model.model_validate({"client_secret": client_secret}) + + def test_rejects_empty_client_secret(self) -> None: + with self.assertRaises(ValidationError): + self.Model.model_validate({"client_secret": ""}) + + def test_rejects_overlong_client_secret(self) -> None: + with self.assertRaises(ValidationError): + self.Model.model_validate({"client_secret": "a" * 256}) + + class EmailRequestTokenBodyTestCase(stdlib_unittest.TestCase): base_request = { "client_secret": "hunter2",