Support trying multiple localparts for OpenID Connect.

synapse/handlers/oidc_handler.py

@@ -39,12 +39,7 @@
 from synapse.handlers.sso import MappingException
 from synapse.http.site import SynapseRequest
 from synapse.logging.context import make_deferred_yieldable
-from synapse.types import (
-    JsonDict,
-    UserID,
-    contains_invalid_mxid_characters,
-    map_username_to_mxid_localpart,
-)
+from synapse.types import JsonDict, map_username_to_mxid_localpart
 from synapse.util import json_decoder
 
 if TYPE_CHECKING:
@@ -94,9 +89,6 @@ class OidcHandler(BaseHandler):
     """Handles requests related to the OpenID Connect login flow.
     """
 
-    # The number of attempts to ask the mapping provider for when generating an MXID.
-    _MAP_USERNAME_RETRIES = 1000
-
     def __init__(self, hs: "HomeServer"):
         super().__init__(hs)
         self._callback_url = hs.config.oidc_callback_url  # type: str
@@ -873,95 +865,41 @@ async def _map_userinfo_to_user(
         # to be strings.
         remote_user_id = str(remote_user_id)
 
-        # first of all, check if we already have a mapping for this user
-        previously_registered_user_id = await self._sso_handler.get_sso_user_by_remote_user_id(
-            self._auth_provider_id, remote_user_id,
-        )
-        if previously_registered_user_id:
-            return previously_registered_user_id
-
         # Older mapping providers don't accept the `failures` argument, so we
         # try and detect support.
         mapper_args = inspect.getfullargspec(
             self._user_mapping_provider.map_user_attributes
         )
         supports_failures = "failures" in mapper_args.args
 
-        # Otherwise, generate a new user.
-        for i in range(self._MAP_USERNAME_RETRIES):
-            try:
-                if supports_failures:
-                    attributes = await self._user_mapping_provider.map_user_attributes(
-                        userinfo, token, i
-                    )
-                else:
-                    attributes = await self._user_mapping_provider.map_user_attributes(  # type: ignore
-                        userinfo, token
-                    )
-            except Exception as e:
-                raise MappingException(
-                    "Could not extract user attributes from OIDC response: " + str(e)
-                )
-
-            logger.debug(
-                "Retrieved user attributes from user mapping provider: %r", attributes
-            )
+        async def oidc_response_to_user_attributes(userinfo, token, failures):
+            """
+            Call the mapping provider to map the OIDC userinfo and token to user attributes.
 
-            localpart = attributes["localpart"]
-            if not localpart:
-                raise MappingException(
-                    "Error parsing OIDC response: OIDC mapping provider plugin "
-                    "did not return a localpart value"
+            This is backwards compatibility for abstraction for the SSO handler.
+            """
+            if supports_failures:
+                attributes = await self._user_mapping_provider.map_user_attributes(
+                    userinfo, token, failures
                 )
-
-            user_id = UserID(localpart, self.server_name).to_string()
-            users = await self.store.get_users_by_id_case_insensitive(user_id)
-            if users:
-                if self._allow_existing_users:
-                    if len(users) == 1:
-                        registered_user_id = next(iter(users))
-                        break
-                    elif user_id in users:
-                        registered_user_id = user_id
-                        break
-                    else:
-                        raise MappingException(
-                            "Attempted to login as '{}' but it matches more than one user inexactly: {}".format(
-                                user_id, list(users.keys())
-                            )
-                        )
-                else:
-                    # This mxid is taken
-                    if not supports_failures:
-                        raise MappingException(
-                            "mxid '{}' is already taken".format(user_id)
-                        )
             else:
-                # Since the localpart is provided via a potentially untrusted module,
-                # ensure the MXID is valid before registering.
-                if contains_invalid_mxid_characters(localpart):
-                    raise MappingException("localpart is invalid: %s" % (localpart,))
-
-                # It's the first time this user is logging in and the mapped mxid was
-                # not taken, register the user
-                registered_user_id = await self._registration_handler.register_user(
-                    localpart=localpart,
-                    default_display_name=attributes["display_name"],
-                    user_agent_ips=(user_agent, ip_address),
+                attributes = await self._user_mapping_provider.map_user_attributes(  # type: ignore
+                    userinfo, token
                 )
-                break
 
-        else:
-            # Unable to generate a username in 1000 iterations
-            # Break and return error to the user
-            raise MappingException(
-                "Unable to generate a Matrix ID from the OpenID response"
-            )
+            return attributes
+
+        # TODO Support existing users.
 
-        await self.store.record_user_external_id(
-            self._auth_provider_id, remote_user_id, registered_user_id,
+        return await self._sso_handler.get_mxid_from_sso(
+            self._auth_provider_id,
+            remote_user_id,
+            user_agent,
+            ip_address,
+            oidc_response_to_user_attributes,
+            userinfo,
+            token,
         )
-        return registered_user_id
 
 
 UserAttribute = TypedDict(

synapse/handlers/saml_handler.py

@@ -31,7 +31,6 @@
 from synapse.module_api import ModuleApi
 from synapse.types import (
     UserID,
-    contains_invalid_mxid_characters,
     map_username_to_mxid_localpart,
     mxid_localpart_allowed_characters,
 )
@@ -250,14 +249,26 @@ async def _map_saml_response_to_user(
                 "Failed to extract remote user id from SAML response"
             )
 
-        with (await self._mapping_lock.queue(self._auth_provider_id)):
-            # first of all, check if we already have a mapping for this user
-            previously_registered_user_id = await self._sso_handler.get_sso_user_by_remote_user_id(
-                self._auth_provider_id, remote_user_id,
+        async def saml_response_to_remapped_user_attributes(
+            saml_response, client_redirect_url, failures
+        ):
+            """
+            Call the mapping provider to map a SAML response to user attributes and coerce the result into the standard form.
+
+            This is backwards compatibility for abstraction for the SSO handler.
+            """
+            # Remap the arguments to the mapping provider.
+            result = self._user_mapping_provider.saml_response_to_user_attributes(
+                saml_response, failures, client_redirect_url
             )
-            if previously_registered_user_id:
-                return previously_registered_user_id
+            # Remap some of the results.
+            if "mxid_localpart" in result:
+                result["localpart"] = result.pop("mxid_localpart")
+            if "displayname" in result:
+                result["display_name"] = result.pop("displayname")
+            return result
 
+        with (await self._mapping_lock.queue(self._auth_provider_id)):
             # backwards-compatibility hack: see if there is an existing user with a
             # suitable mapping from the uid
             if (
@@ -284,59 +295,15 @@ async def _map_saml_response_to_user(
                     )
                     return registered_user_id
 
-            # Map saml response to user attributes using the configured mapping provider
-            for i in range(1000):
-                attribute_dict = self._user_mapping_provider.saml_response_to_user_attributes(
-                    saml2_auth, i, client_redirect_url=client_redirect_url,
-                )
-
-                logger.debug(
-                    "Retrieved SAML attributes from user mapping provider: %s "
-                    "(attempt %d)",
-                    attribute_dict,
-                    i,
-                )
-
-                localpart = attribute_dict.get("mxid_localpart")
-                if not localpart:
-                    raise MappingException(
-                        "Error parsing SAML2 response: SAML mapping provider plugin "
-                        "did not return a mxid_localpart value"
-                    )
-
-                displayname = attribute_dict.get("displayname")
-                emails = attribute_dict.get("emails", [])
-
-                # Check if this mxid already exists
-                if not await self.store.get_users_by_id_case_insensitive(
-                    UserID(localpart, self.server_name).to_string()
-                ):
-                    # This mxid is free
-                    break
-            else:
-                # Unable to generate a username in 1000 iterations
-                # Break and return error to the user
-                raise MappingException(
-                    "Unable to generate a Matrix ID from the SAML response"
-                )
-
-            # Since the localpart is provided via a potentially untrusted module,
-            # ensure the MXID is valid before registering.
-            if contains_invalid_mxid_characters(localpart):
-                raise MappingException("localpart is invalid: %s" % (localpart,))
-
-            logger.debug("Mapped SAML user to local part %s", localpart)
-            registered_user_id = await self._registration_handler.register_user(
-                localpart=localpart,
-                default_display_name=displayname,
-                bind_emails=emails,
-                user_agent_ips=(user_agent, ip_address),
-            )
-
-            await self.store.record_user_external_id(
-                self._auth_provider_id, remote_user_id, registered_user_id
+            return await self._sso_handler.get_mxid_from_sso(
+                self._auth_provider_id,
+                remote_user_id,
+                user_agent,
+                ip_address,
+                saml_response_to_remapped_user_attributes,
+                saml2_auth,
+                client_redirect_url,
             )
-            return registered_user_id
 
     def expire_sessions(self):
         expire_before = self.clock.time_msec() - self._saml2_session_lifetime

synapse/handlers/sso.py

@@ -13,10 +13,11 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 import logging
-from typing import TYPE_CHECKING, Optional
+from typing import TYPE_CHECKING, Any, Awaitable, Callable, Optional
 
 from synapse.handlers._base import BaseHandler
 from synapse.http.server import respond_with_html
+from synapse.types import UserID, contains_invalid_mxid_characters
 
 if TYPE_CHECKING:
     from synapse.server import HomeServer
@@ -30,8 +31,12 @@ class MappingException(Exception):
 
 
 class SsoHandler(BaseHandler):
+    # The number of attempts to ask the mapping provider for when generating an MXID.
+    _MAP_USERNAME_RETRIES = 1000
+
     def __init__(self, hs: "HomeServer"):
         super().__init__(hs)
+        self._registration_handler = hs.get_registration_handler()
         self._error_template = hs.config.sso_error_template
 
     def render_error(
@@ -94,3 +99,98 @@ async def get_sso_user_by_remote_user_id(
 
         # No match.
         return None
+
+    async def get_mxid_from_sso(
+        self,
+        auth_provider_id: str,
+        remote_user_id: str,
+        user_agent: str,
+        ip_address: str,
+        mapper: "Callable[..., Awaitable[str]]",
+        *args: Any,
+        **kwargs: Any,
+    ):
+        """
+
+        Args:
+            auth_provider_id: A unique identifier for this SSO provider, e.g.
+                "oidc" or "saml".
+            remote_user_id: The unique identifier from the SSO provider.
+            user_agent: The user agent of the client making the request.
+            ip_address: The IP address of the client making the request.
+            mapper: A callable to generate the user attributes. Note that the
+                current number of failures is passed in as the last argument.
+            args: Additional arguments for mapper.
+            kwargs: Additional keyword arguments for mapper.
+
+        Returns:
+             The user ID associated with the SSO response.
+
+        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.
+
+        """
+        # first of all, check if we already have a mapping for this user
+        previously_registered_user_id = await self.get_sso_user_by_remote_user_id(
+            auth_provider_id, remote_user_id,
+        )
+        if previously_registered_user_id:
+            return previously_registered_user_id
+
+        # Otherwise, generate a new user.
+        for i in range(self._MAP_USERNAME_RETRIES):
+            try:
+                attributes = await mapper(*args, i, **kwargs)
+            except Exception as e:
+                raise MappingException(
+                    "Could not extract user attributes from SSO response: " + str(e)
+                )
+
+            logger.debug(
+                "Retrieved user attributes from user mapping provider: %r (attempt %d)",
+                attributes,
+                i,
+            )
+
+            localpart = attributes["localpart"]
+            if not localpart:
+                raise MappingException(
+                    "Error parsing SSO response: SSO mapping provider plugin "
+                    "did not return a localpart value"
+                )
+
+            # Other potential attributes returned by the mapping provider.
+            display_name = attributes.get("display_name")
+            emails = attributes.get("emails", [])
+
+            # Check if this mxid already exists
+            user_id = UserID(localpart, self.server_name).to_string()
+            if not await self.store.get_users_by_id_case_insensitive(user_id):
+                # This mxid is free
+                break
+        else:
+            # Unable to generate a username in 1000 iterations
+            # Break and return error to the user
+            raise MappingException(
+                "Unable to generate a Matrix ID from the SSO response"
+            )
+
+        # Since the localpart is provided via a potentially untrusted module,
+        # ensure the MXID is valid before registering.
+        if contains_invalid_mxid_characters(localpart):
+            raise MappingException("localpart is invalid: %s" % (localpart,))
+
+        logger.debug("Mapped SAML user to local part %s", localpart)
+        registered_user_id = await self._registration_handler.register_user(
+            localpart=localpart,
+            default_display_name=display_name,
+            bind_emails=emails,
+            user_agent_ips=(user_agent, ip_address),
+        )
+
+        await self.store.record_user_external_id(
+            auth_provider_id, remote_user_id, registered_user_id
+        )
+        return registered_user_id