diff options
| -rw-r--r-- | CHANGES.md | 15 | ||||
| -rw-r--r-- | docker/Dockerfile | 2 | ||||
| -rw-r--r-- | docs/sso_mapping_providers.md | 7 | ||||
| -rw-r--r-- | synapse/__init__.py | 2 | ||||
| -rw-r--r-- | synapse/handlers/oidc_handler.py | 2 | ||||
| -rw-r--r-- | synapse/handlers/sso.py | 27 | ||||
| -rw-r--r-- | synapse/python_dependencies.py | 13 | ||||
| -rw-r--r-- | tests/handlers/test_oidc.py | 3 | ||||
| -rw-r--r-- | tests/handlers/test_saml.py | 28 |
9 files changed, 81 insertions, 18 deletions
diff --git a/CHANGES.md b/CHANGES.md index e7aaebb1f3..d5e578ee3a 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 `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) ============================== diff --git a/docker/Dockerfile b/docker/Dockerfile index 791cd6936b..afd896ffc1 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/docs/sso_mapping_providers.md b/docs/sso_mapping_providers.md index c13b4f7155..7714b1d844 100644 --- a/docs/sso_mapping_providers.md +++ b/docs/sso_mapping_providers.md @@ -170,6 +170,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/__init__.py b/synapse/__init__.py index d33a99f230..2e354f2cc6 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 diff --git a/synapse/handlers/oidc_handler.py b/synapse/handlers/oidc_handler.py index 55c4377890..c605f7082a 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 f42b90e1bc..47ad96f97e 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/synapse/python_dependencies.py b/synapse/python_dependencies.py index aab77fc453..c899ca14d3 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.) diff --git a/tests/handlers/test_oidc.py b/tests/handlers/test_oidc.py index 23ea1e3543..1d99a45436 100644 --- a/tests/handlers/test_oidc.py +++ b/tests/handlers/test_oidc.py @@ -690,8 +690,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 e1e13a5faf..45dc17aba5 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/") |