Merge tag 'v1.24.0rc2' into develop

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](#8878))

Internal Changes
----------------

- Add support for the `prometheus_client` newer than 0.9.0. Contributed by Jordan Bancino. ([\#8875](#8875))

Author: clokep

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)
 ==============================
 

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 \

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
 

synapse/__init__.py

@@ -48,7 +48,7 @@
 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

synapse/handlers/oidc_handler.py

@@ -888,7 +888,7 @@ async def oidc_response_to_user_attributes(failures: int) -> UserAttributes:
                 # 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"
                     )
 

synapse/handlers/sso.py

@@ -17,6 +17,7 @@
 
 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 @@
 
 
 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 @@ async def get_mxid_from_sso(
             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 @@ async def get_mxid_from_sso(
 
         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 @@ async def get_mxid_from_sso(
         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)",

synapse/python_dependencies.py

@@ -40,6 +40,10 @@
 # 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 @@
     "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.)

tests/handlers/test_oidc.py

@@ -690,8 +690,7 @@ def test_map_userinfo_to_user(self):
             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}})

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 @@ def saml_response_to_user_attributes(
         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 @@ def test_map_saml_response_to_user_retries(self):
         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/")