From 295c209cdd9364a5f277470da66d06a3d4133ad7 Mon Sep 17 00:00:00 2001 From: Jordan Bancino Date: Fri, 4 Dec 2020 08:01:06 -0500 Subject: [PATCH 1/4] Remove version pin prometheus_client dependency (#8875) This removes the version pin of the `prometheus_client` dependency, in direct response to #8831. If merged, this will close #8831 As far as I can tell, no other changes are needed, but as I'm no synapse expert, I'm relying heavily on CI and maintainer reviews for this. My very primitive test of synapse with prometheus_client v0.9.0 on my home server didn't bring up any issues, so we'll see what happens. Signed-off-by: Jordan Bancino --- changelog.d/8875.misc | 1 + docker/Dockerfile | 2 +- synapse/python_dependencies.py | 13 +++++-------- 3 files changed, 7 insertions(+), 9 deletions(-) create mode 100644 changelog.d/8875.misc diff --git a/changelog.d/8875.misc b/changelog.d/8875.misc new file mode 100644 index 000000000..5a56a6296 --- /dev/null +++ b/changelog.d/8875.misc @@ -0,0 +1 @@ +Add support for the latest third-party libraries. Contributed by Jordan Bancino. diff --git a/docker/Dockerfile b/docker/Dockerfile index 791cd6936..afd896ffc 100644 --- a/docker/Dockerfile +++ b/docker/Dockerfile @@ -37,7 +37,7 @@ RUN pip install --prefix="/install" --no-warn-script-location \ jaeger-client \ opentracing \ # Match the version constraints of Synapse - "prometheus_client>=0.4.0,<0.9.0" \ + "prometheus_client>=0.4.0" \ psycopg2 \ pycparser \ pyrsistent \ diff --git a/synapse/python_dependencies.py b/synapse/python_dependencies.py index aab77fc45..c899ca14d 100644 --- a/synapse/python_dependencies.py +++ b/synapse/python_dependencies.py @@ -40,6 +40,10 @@ logger = logging.getLogger(__name__) # Note that these both represent runtime dependencies (and the versions # installed are checked at runtime). # +# Also note that we replicate these constraints in the Synapse Dockerfile while +# pre-installing dependencies. If these constraints are updated here, the same +# change should be made in the Dockerfile. +# # [1] https://pip.pypa.io/en/stable/reference/pip_install/#requirement-specifiers. REQUIREMENTS = [ @@ -69,14 +73,7 @@ REQUIREMENTS = [ "msgpack>=0.5.2", "phonenumbers>=8.2.0", # we use GaugeHistogramMetric, which was added in prom-client 0.4.0. - # prom-client has a history of breaking backwards compatibility between - # minor versions (https://github.com/prometheus/client_python/issues/317), - # so we also pin the minor version. - # - # Note that we replicate these constraints in the Synapse Dockerfile while - # pre-installing dependencies. If these constraints are updated here, the - # same change should be made in the Dockerfile. - "prometheus_client>=0.4.0,<0.9.0", + "prometheus_client>=0.4.0", # we use attr.validators.deep_iterable, which arrived in 19.1.0 (Note: # Fedora 31 only has 19.1, so if we want to upgrade we should wait until 33 # is out in November.) From 22c6c19f91d7325c82eddfada696826adad69e5b Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 4 Dec 2020 08:25:15 -0500 Subject: [PATCH 2/4] Fix a regression that mapping providers should be able to redirect users. (#8878) This was broken in #8801. --- changelog.d/8878.bugfix | 1 + docs/sso_mapping_providers.md | 7 +++++++ synapse/handlers/oidc_handler.py | 2 +- synapse/handlers/sso.py | 27 ++++++++++++++++++++++----- tests/handlers/test_oidc.py | 3 +-- tests/handlers/test_saml.py | 28 ++++++++++++++++++++++++++++ 6 files changed, 60 insertions(+), 8 deletions(-) create mode 100644 changelog.d/8878.bugfix diff --git a/changelog.d/8878.bugfix b/changelog.d/8878.bugfix new file mode 100644 index 000000000..e53005ee1 --- /dev/null +++ b/changelog.d/8878.bugfix @@ -0,0 +1 @@ +Fix a regression in v1.24.0rc1 which failed to allow SAML mapping providers which were unable to redirect users to an additional page. diff --git a/docs/sso_mapping_providers.md b/docs/sso_mapping_providers.md index dee53b5d4..ab2a64891 100644 --- a/docs/sso_mapping_providers.md +++ b/docs/sso_mapping_providers.md @@ -168,6 +168,13 @@ A custom mapping provider must specify the following methods: the value of `mxid_localpart`. * `emails` - A list of emails for the new user. If not provided, will default to an empty list. + + Alternatively it can raise a `synapse.api.errors.RedirectException` to + redirect the user to another page. This is useful to prompt the user for + additional information, e.g. if you want them to provide their own username. + It is the responsibility of the mapping provider to either redirect back + to `client_redirect_url` (including any additional information) or to + complete registration using methods from the `ModuleApi`. ### Default SAML Mapping Provider diff --git a/synapse/handlers/oidc_handler.py b/synapse/handlers/oidc_handler.py index 55c437789..c605f7082 100644 --- a/synapse/handlers/oidc_handler.py +++ b/synapse/handlers/oidc_handler.py @@ -888,7 +888,7 @@ class OidcHandler(BaseHandler): # continue to already be in use. Note that the error raised is # arbitrary and will get turned into a MappingException. if failures: - raise RuntimeError( + raise MappingException( "Mapping provider does not support de-duplicating Matrix IDs" ) diff --git a/synapse/handlers/sso.py b/synapse/handlers/sso.py index f42b90e1b..47ad96f97 100644 --- a/synapse/handlers/sso.py +++ b/synapse/handlers/sso.py @@ -17,6 +17,7 @@ from typing import TYPE_CHECKING, Awaitable, Callable, List, Optional import attr +from synapse.api.errors import RedirectException from synapse.handlers._base import BaseHandler from synapse.http.server import respond_with_html from synapse.types import UserID, contains_invalid_mxid_characters @@ -28,7 +29,9 @@ logger = logging.getLogger(__name__) class MappingException(Exception): - """Used to catch errors when mapping the UserInfo object + """Used to catch errors when mapping an SSO response to user attributes. + + Note that the msg that is raised is shown to end-users. """ @@ -145,6 +148,14 @@ class SsoHandler(BaseHandler): sso_to_matrix_id_mapper: A callable to generate the user attributes. The only parameter is an integer which represents the amount of times the returned mxid localpart mapping has failed. + + It is expected that the mapper can raise two exceptions, which + will get passed through to the caller: + + MappingException if there was a problem mapping the response + to the user. + RedirectException to redirect to an additional page (e.g. + to prompt the user for more information). grandfather_existing_users: A callable which can return an previously existing matrix ID. The SSO ID is then linked to the returned matrix ID. @@ -154,8 +165,8 @@ class SsoHandler(BaseHandler): Raises: MappingException if there was a problem mapping the response to a user. - RedirectException: some mapping providers may raise this if they need - to redirect to an interstitial page. + RedirectException: if the mapping provider needs to redirect the user + to an additional page. (e.g. to prompt for more information) """ # first of all, check if we already have a mapping for this user @@ -179,10 +190,16 @@ class SsoHandler(BaseHandler): for i in range(self._MAP_USERNAME_RETRIES): try: attributes = await sso_to_matrix_id_mapper(i) + except (RedirectException, MappingException): + # Mapping providers are allowed to issue a redirect (e.g. to ask + # the user for more information) and can issue a mapping exception + # if a name cannot be generated. + raise except Exception as e: + # Any other exception is unexpected. raise MappingException( - "Could not extract user attributes from SSO response: " + str(e) - ) + "Could not extract user attributes from SSO response." + ) from e logger.debug( "Retrieved user attributes from user mapping provider: %r (attempt %d)", diff --git a/tests/handlers/test_oidc.py b/tests/handlers/test_oidc.py index d485af52f..a308c46da 100644 --- a/tests/handlers/test_oidc.py +++ b/tests/handlers/test_oidc.py @@ -705,8 +705,7 @@ class OidcHandlerTestCase(HomeserverTestCase): MappingException, ) self.assertEqual( - str(e.value), - "Could not extract user attributes from SSO response: Mapping provider does not support de-duplicating Matrix IDs", + str(e.value), "Mapping provider does not support de-duplicating Matrix IDs", ) @override_config({"oidc_config": {"allow_existing_users": True}}) diff --git a/tests/handlers/test_saml.py b/tests/handlers/test_saml.py index e1e13a5fa..45dc17aba 100644 --- a/tests/handlers/test_saml.py +++ b/tests/handlers/test_saml.py @@ -14,6 +14,7 @@ import attr +from synapse.api.errors import RedirectException from synapse.handlers.sso import MappingException from tests.unittest import HomeserverTestCase, override_config @@ -49,6 +50,13 @@ class TestMappingProvider: return {"mxid_localpart": localpart, "displayname": None} +class TestRedirectMappingProvider(TestMappingProvider): + def saml_response_to_user_attributes( + self, saml_response, failures, client_redirect_url + ): + raise RedirectException(b"https://custom-saml-redirect/") + + class SamlHandlerTestCase(HomeserverTestCase): def default_config(self): config = super().default_config() @@ -166,3 +174,23 @@ class SamlHandlerTestCase(HomeserverTestCase): self.assertEqual( str(e.value), "Unable to generate a Matrix ID from the SSO response" ) + + @override_config( + { + "saml2_config": { + "user_mapping_provider": { + "module": __name__ + ".TestRedirectMappingProvider" + }, + } + } + ) + def test_map_saml_response_redirect(self): + saml_response = FakeAuthnResponse({"uid": "test", "username": "test_user"}) + redirect_url = "" + e = self.get_failure( + self.handler._map_saml_response_to_user( + saml_response, redirect_url, "user-agent", "10.10.10.10" + ), + RedirectException, + ) + self.assertEqual(e.value.location, b"https://custom-saml-redirect/") From 693dab487c8de75ca1e7573474a3d4429ce8b313 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 4 Dec 2020 08:48:04 -0500 Subject: [PATCH 3/4] 1.24.0rc2 --- CHANGES.md | 15 +++++++++++++++ changelog.d/8875.misc | 1 - changelog.d/8878.bugfix | 1 - synapse/__init__.py | 2 +- 4 files changed, 16 insertions(+), 3 deletions(-) delete mode 100644 changelog.d/8875.misc delete mode 100644 changelog.d/8878.bugfix diff --git a/CHANGES.md b/CHANGES.md index e7aaebb1f..26d413844 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,3 +1,18 @@ +Synapse 1.24.0rc2 (2020-12-04) +============================== + +Bugfixes +-------- + +- Fix a regression in v1.24.0rc1 which failed to allow SAML mapping providers which were unable to redirect users to an additional page. ([\#8878](https://github.com/matrix-org/synapse/issues/8878)) + + +Internal Changes +---------------- + +- Add support for the latest third-party libraries. Contributed by Jordan Bancino. ([\#8875](https://github.com/matrix-org/synapse/issues/8875)) + + Synapse 1.24.0rc1 (2020-12-02) ============================== diff --git a/changelog.d/8875.misc b/changelog.d/8875.misc deleted file mode 100644 index 5a56a6296..000000000 --- a/changelog.d/8875.misc +++ /dev/null @@ -1 +0,0 @@ -Add support for the latest third-party libraries. Contributed by Jordan Bancino. diff --git a/changelog.d/8878.bugfix b/changelog.d/8878.bugfix deleted file mode 100644 index e53005ee1..000000000 --- a/changelog.d/8878.bugfix +++ /dev/null @@ -1 +0,0 @@ -Fix a regression in v1.24.0rc1 which failed to allow SAML mapping providers which were unable to redirect users to an additional page. diff --git a/synapse/__init__.py b/synapse/__init__.py index d33a99f23..2e354f2cc 100644 --- a/synapse/__init__.py +++ b/synapse/__init__.py @@ -48,7 +48,7 @@ try: except ImportError: pass -__version__ = "1.24.0rc1" +__version__ = "1.24.0rc2" if bool(os.environ.get("SYNAPSE_TEST_PATCH_LOG_CONTEXTS", False)): # We import here so that we don't have to install a bunch of deps when From 2602514f34aab76934d27791400d7405c1da6336 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 4 Dec 2020 09:00:32 -0500 Subject: [PATCH 4/4] Minor update to CHANGES. --- CHANGES.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGES.md b/CHANGES.md index 26d413844..d5e578ee3 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -10,7 +10,7 @@ Bugfixes Internal Changes ---------------- -- Add support for the latest third-party libraries. Contributed by Jordan Bancino. ([\#8875](https://github.com/matrix-org/synapse/issues/8875)) +- Add support for the `prometheus_client` newer than 0.9.0. Contributed by Jordan Bancino. ([\#8875](https://github.com/matrix-org/synapse/issues/8875)) Synapse 1.24.0rc1 (2020-12-02)