Allow SAML username provider plugins

docs/saml_mapping_providers.md

@@ -16,24 +16,29 @@ where SAML mapping providers come into play.
 
 External mapping providers are provided to Synapse in the form of an external
 Python module. Retrieve this module from [PyPi](https://pypi.org) or elsewhere,
-then tell Synapse where to look for the handler class by changing the
-`saml2_config.user_mapping_provider` config option.
+then tell Synapse where to look for the handler class by editing the
+`saml2_config.user_mapping_provider.module` config option.
 
-`saml2_config.user_mapping_provider_config` allows you to provide custom
+`saml2_config.user_mapping_provider.config` allows you to provide custom
 configuration options to the module. Check with the module's documentation for
-what options it provides (if any).
+what options it provides (if any). The options listed by default are for the
+user mapping provider built in to Synapse. If using a custom module, you should
+comment these options out and use those specified by the module instead.
 
 ## Building a Custom Mapping Provider
 
-A custom mapping provider must specify the following method:
+A custom mapping provider must specify the following methods:
 
-* `saml_response_to_user_attributes(self, saml_response, config, failures)`
+* `__init__(self, parsed_config)`
+   - Arguments:
+     - `config` - A configuration object that is the return value of the
+       `parse_config` method. You should set any configuration options needed
+       by the module here.
+* `saml_response_to_user_attributes(self, saml_response, failures)`
     - Arguments:
-      - `config` - A dictionary with the contents of the homeserver config's
-                   `saml2_config.user_mapping_provider_config` option.
       - `saml_response` - A `saml2.response.AuthnResponse` object to extract user
                           information from.
-      - `failures` - An int that represents the amount of times the returned
+      - `failures` - An `int` that represents the amount of times the returned
                      mxid localpart mapping has failed.  This should be used
                      to create a deduplicated mxid localpart which should be
                      returned instead.  For example, if this method returns
@@ -46,6 +51,26 @@ A custom mapping provider must specify the following method:
       to build a new user. The following keys are allowed:
        * `mxid_localpart` - Required. The mxid localpart of the new user.
        * `displayname` - The displayname of the new user.
+* `parse_config(config)`
+    - This method should have the `@staticmethod` decoration.
+    - Arguments:
+        - `config` - A `dict` representing the parsed content of the
+          `saml2_config.user_mapping_provider.config` homeserver config option.
+           Runs on homeserver startup. Providers should extract any option values
+           they need here.
+    - Whatever is returned will be passed back to the user mapping provider module's
+      `__init__` method during construction.
+* `get_required_attributes(config)`
+    - This method should have the `@staticmethod` decoration.
+    - Arguments:
+        - `config` - A `dict` representing the parsed content of the
+          `saml2_config.user_mapping_provider.config` homeserver config option.
+           Runs on homeserver startup. Providers should extract any option values
+           they need here.
+    - Returns a tuple of two sets. The first set equates to the saml auth
+      response attributes that are required for the module to function, whereas
+      the second set consists of those attributes which can be used if available,
+      but are not necessary.
 
 ## Synapse's Default Provider
 

synapse/config/saml2_config.py

@@ -14,11 +14,15 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import logging
+
 from synapse.python_dependencies import DependencyException, check_requirements
 from synapse.util.module_loader import load_module, load_python_module
 
 from ._base import Config, ConfigError
 
+logger = logging.getLogger(__name__)
+
 
 def _dict_merge(merge_dict, into_dict):
     """Do a deep merge of two dicts
@@ -86,42 +90,64 @@ def read_config(self, config, **kwargs):
         )
         user_mapping_provider_config = user_mapping_provider_dict.get("config", {})
 
-        if user_mapping_provider_module == default_mapping_provider:
-            # Handle deprecated options in default module config
+        # Retrieve an instance of the module's class
+        # Pass the config dictionary to the module for processing
+        (
+            self.saml2_user_mapping_provider_class,
+            self.saml2_user_mapping_provider_config,
+        ) = load_module(
+            {
+                "module": user_mapping_provider_module,
+                "config": user_mapping_provider_config,
+            }
+        )
+
+        # Ensure loaded user mapping module has defined all necessary methods
+        required_methods = [
+            "parse_config",
+            "get_required_attributes",
+            "saml_response_to_user_attributes",
+        ]
+        missing_methods = [
+            method
+            for method in required_methods
+            if not hasattr(self.saml2_user_mapping_provider_class, method)
+        ]
+        if missing_methods:
+            raise ConfigError(
+                "Class specified by saml2_config."
+                "user_mapping_provider.module is missing required "
+                "methods: %s" % (", ".join(missing_methods),)
+            )
 
-            # If mxid_source_attribute is defined in the deprecated location, use
-            # that instead for backwards compatibility
+        if user_mapping_provider_module == default_mapping_provider:
+            # Load deprecated options for use by the default module
             old_mxid_source_attribute = saml2_config.get("mxid_source_attribute")
             if old_mxid_source_attribute:
-                user_mapping_provider_config[
-                    "mxid_source_attribute"
-                ] = old_mxid_source_attribute
-
-                # Provide a deprecation warning
-                self.using_old_mxid_source_attribute = True
-            else:
-                # If the option doesn't exist in saml2_config, and it's not set under
-                # user_mapping_provider_config, set it to a default value
-                user_mapping_provider_config.setdefault("mxid_source_attribute", "uid")
-
-            # If mxid_mapping is defined, use that instead for backwards compatibility
+                logger.warning(
+                    "The config option saml2_config.mxid_source_attribute is deprecated. "
+                    "Please use saml2_config.user_mapping_provider.config"
+                    ".mxid_source_attribute instead."
+                )
+            self.saml2_user_mapping_provider_config[
+                "old_mxid_source_attribute"
+            ] = old_mxid_source_attribute
+
             old_mxid_mapping = saml2_config.get("mxid_mapping")
             if old_mxid_mapping:
-                user_mapping_provider_config["mxid_mapping"] = old_mxid_mapping
-
-                # Provide a deprecation warning
-                self.using_old_mxid_mapping = True
-            else:
-                # If the option doesn't exist in saml2_config, and it's not set under
-                # user_mapping_provider_config, set it to a default value
-                user_mapping_provider_config.setdefault("mxid_mapping", "hexencode")
-
-        # We don't use nor provide the module's config here
-        self.saml2_user_mapping_provider_class, _ = load_module(
-            {"module": user_mapping_provider_module}
+                logger.warning(
+                    "The config option saml2_config.mxid_mapping is deprecated. Please use "
+                    "saml2_config.user_mapping_provider.config.mxid_mapping instead."
+                )
+            self.saml2_user_mapping_provider_config[
+                "old_mxid_mapping"
+            ] = old_mxid_mapping
+
+        saml2_config_dict = self._default_saml_config_dict(
+            *self.saml2_user_mapping_provider_class.get_required_attributes(
+                self.saml2_user_mapping_provider_config
+            )
         )
-
-        saml2_config_dict = self._default_saml_config_dict()
         _dict_merge(
             merge_dict=saml2_config.get("sp_config", {}), into_dict=saml2_config_dict
         )
@@ -141,16 +167,26 @@ def read_config(self, config, **kwargs):
             saml2_config.get("saml_session_lifetime", "5m")
         )
 
-    def _default_saml_config_dict(self):
+    def _default_saml_config_dict(
+        self, required_attributes: set, optional_attributes: set
+    ):
+        """Generate a configuration dictionary with required and optional attributes that
+        will be needed to process new user registration
+
+        Args:
+            required_attributes: SAML auth response attributes that are necessary to function
+            optional_attributes: SAML auth response attributes that can be used to add
+                additional information to Synapse user accounts, but are not required
+
+        Returns:
+            dict: A SAML configuration dictionary
+        """
         import saml2
 
         public_baseurl = self.public_baseurl
         if public_baseurl is None:
             raise ConfigError("saml2_config requires a public_baseurl to be set")
 
-        required_attributes = {"uid"}
-
-        optional_attributes = {"displayName"}
         if self.saml2_grandfathered_mxid_source_attribute:
             optional_attributes.add(self.saml2_grandfathered_mxid_source_attribute)
         optional_attributes -= required_attributes

synapse/handlers/saml_handler.py

@@ -21,6 +21,7 @@
 from saml2.client import Saml2Client
 
 from synapse.api.errors import SynapseError
+from synapse.config import ConfigError
 from synapse.http.servlet import parse_string
 from synapse.rest.client.v1.login import SSOAuthHandler
 from synapse.types import (
@@ -72,22 +73,6 @@ def __init__(self, hs):
         # a lock on the mappings
         self._mapping_lock = Linearizer(name="saml_mapping", clock=self._clock)
 
-        # Check for deprecated config options
-        if hasattr(hs.config, "using_old_mxid_source_attribute"):
-            logger.warning(
-                "The config option saml2_config.mxid_source_attribute is "
-                "deprecated. Please use "
-                "saml2_config.user_mapping_provider.config.mxid_source_attribute "
-                "instead."
-            )
-        if hasattr(hs.config, "using_old_mxid_mapping"):
-            logger.warning(
-                "The config option saml2_config.mxid_mapping is "
-                "deprecated. Please use "
-                "saml2_config.user_mapping_provider.config.mxid_mapping "
-                "instead."
-            )
-
     def handle_redirect_request(self, client_redirect_url):
         """Handle an incoming request to /login/sso/redirect
 
@@ -199,13 +184,9 @@ async def _map_saml_response_to_user(self, resp_bytes):
 
             # Map saml response to user attributes using the configured mapping provider
             for i in range(1000):
-                try:
-                    attribute_dict = self._user_mapping_provider.saml_response_to_user_attributes(
-                        saml2_auth, i
-                    )
-                except Exception:
-                    logging.exception("Error in SAML mapping provider plugin")
-                    raise
+                attribute_dict = self._user_mapping_provider.saml_response_to_user_attributes(
+                    saml2_auth, i
+                )
 
                 localpart = attribute_dict.get("mxid_localpart")
                 if not localpart:
@@ -272,23 +253,21 @@ def dot_replace_for_mxid(username: str) -> str:
 class DefaultSamlMappingProvider(object):
     __version__ = "0.0.1"
 
-    def __init__(self, config: dict):
+    def __init__(self, parsed_config):
         """The default SAML user mapping provider
 
         Args:
-            config: Module configuration dictionary
+            parsed_config: Module configuration
         """
-        self._mxid_source_attribute = config["mxid_source_attribute"]
-        self._mxid_mapping = config["mxid_mapping"]
+        self._mxid_source_attribute = parsed_config.mxid_source_attribute
+        self._mxid_mapping = parsed_config.mxid_mapping
 
     def saml_response_to_user_attributes(
         self, saml_response: saml2.response.AuthnResponse, failures: int = 0,
     ) -> dict:
         """Maps some text from a SAML response to attributes of a new user
 
         Args:
-            config: A configuration dictionary
-
             saml_response: A SAML auth response object
 
             failures: How many times a call to this function with this
@@ -324,3 +303,53 @@ def saml_response_to_user_attributes(
             "mxid_localpart": localpart,
             "displayname": displayname,
         }
+
+    @staticmethod
+    def parse_config(config: dict):
+        """Parse the dict provided by the homeserver's config
+        Args:
+            config: A dictionary containing configuration options for this provider
+        Returns:
+            _SamlConfig: A custom config object
+        """
+
+        class _SamlConfig(object):
+            pass
+
+        saml_config = _SamlConfig()
+
+        # Handle deprecated options
+
+        # If mxid_source_attribute or mxid_mapping is defined in the deprecated location, use
+        # that instead for backwards compatibility
+        saml_config.mxid_source_attribute = config[
+            "old_mxid_source_attribute"
+        ] or config.get("mxid_source_attribute", "uid")
+
+        # If mxid_mapping is defined, use that instead for backwards compatibility
+        mapping_type = config["old_mxid_mapping"] or config.get(
+            "mxid_mapping", "hexencode"
+        )
+        try:
+            saml_config.mxid_mapper = MXID_MAPPER_MAP[mapping_type]
+        except KeyError:
+            raise ConfigError(
+                "saml2_config.user_mapping_provider.config: %s is not a valid "
+                "mxid_mapping value" % (mapping_type,)
+            )
+        return saml_config
+
+    @staticmethod
+    def get_required_saml_attributes(config: dict):
+        """Returns the required attributes of a SAML
+
+        Args:
+            config: A dictionary containing configuration options for this provider
+
+        Returns:
+            tuple[set,set]: The first set equates to the saml auth response attributes that
+                are required for the module to function, whereas the second set consists of
+                those attributes which can be used if available, but are not necessary
+        """
+        saml_config = DefaultSamlMappingProvider.parse_config(config)
+        return {"uid", saml_config.mxid_source_attribute}, {"displayName"}

synapse/util/module_loader.py

@@ -25,22 +25,18 @@ def load_module(provider):
     (the config dict).
 
     Returns
-        Tuple of (provider class, parsed config object|None, if a config was not passed)
+        Tuple of (provider class, parsed config object
     """
     # We need to import the module, and then pick the class out of
     # that, so we split based on the last dot.
     module, clz = provider["module"].rsplit(".", 1)
     module = importlib.import_module(module)
     provider_class = getattr(module, clz)
-    provider_config = provider.get("config")
-
-    if provider.get("config"):
-        try:
-            provider_config = provider_class.parse_config(provider_config)
-        except Exception as e:
-            raise ConfigError(
-                "Failed to parse config for %r: %r" % (provider["module"], e)
-            )
+
+    try:
+        provider_config = provider_class.parse_config(provider["config"])
+    except Exception as e:
+        raise ConfigError("Failed to parse config for %r: %r" % (provider["module"], e))
 
     return provider_class, provider_config