From b31adc6ad2b918588bfaae50e0e67957c3b7c0b3 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Mon, 25 Nov 2019 17:01:51 +0000 Subject: [PATCH 01/35] Allow SAML username provider plugins --- docs/sample_config.yaml | 5 ++++ synapse/config/saml2_config.py | 15 ++++++++++- synapse/handlers/saml_handler.py | 45 +++++++++++++++++++++++++++----- synapse/util/module_loader.py | 12 ++++++--- 4 files changed, 66 insertions(+), 11 deletions(-) diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index 896159394c2b..e3c30ec7193b 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -1205,6 +1205,11 @@ saml2_config: # #mxid_mapping: dotreplace + # A third-party module can be provided here as a custom solution to mapping the + # above configured saml attribute onto a matrix ID. If this is defined, it will + # override the `mxid_mapping` option. + #mxid_mapping_provider: "mapping_provider.SamlMappingProvider" + # In previous versions of synapse, the mapping from SAML attribute to MXID was # always calculated dynamically rather than stored in a table. For backwards- # compatibility, we will look for user_ids matching such a pattern before diff --git a/synapse/config/saml2_config.py b/synapse/config/saml2_config.py index c5ea2d43a131..255cd2c18f6c 100644 --- a/synapse/config/saml2_config.py +++ b/synapse/config/saml2_config.py @@ -21,7 +21,7 @@ map_username_to_mxid_localpart, mxid_localpart_allowed_characters, ) -from synapse.util.module_loader import load_python_module +from synapse.util.module_loader import load_module, load_python_module from ._base import Config, ConfigError @@ -109,6 +109,14 @@ def read_config(self, config, **kwargs): except KeyError: raise ConfigError("%s is not a known mxid_mapping" % (mapping,)) + self.saml2_mapping_provider = None + mapping_provider_module = config.get("saml_mapping_provider") + if mapping_provider_module: + # Load configured module class + self.saml2_mapping_provider, _ = load_module( + {"mod_name": self.saml2_mapping_provider, "config": None} + ) + def _default_saml_config_dict(self): import saml2 @@ -226,6 +234,11 @@ def generate_config_section(self, config_dir_path, server_name, **kwargs): # #mxid_mapping: dotreplace + # A third-party module can be provided here as a custom solution to mapping the + # above configured saml attribute onto a matrix ID. If this is defined, it will + # override the `mxid_mapping` option. + #mxid_mapping_provider: "mapping_provider.SamlMappingProvider" + # In previous versions of synapse, the mapping from SAML attribute to MXID was # always calculated dynamically rather than stored in a table. For backwards- # compatibility, we will look for user_ids matching such a pattern before diff --git a/synapse/handlers/saml_handler.py b/synapse/handlers/saml_handler.py index cc9e6b9bd018..f425b9b8a4fe 100644 --- a/synapse/handlers/saml_handler.py +++ b/synapse/handlers/saml_handler.py @@ -43,6 +43,9 @@ def __init__(self, hs): ) self._mxid_mapper = hs.config.saml2_mxid_mapper + # plugin to do custom mapping from saml response to mxid + self._mapping_provider = hs.config.saml2_mapping_provider + # identifier for the external_ids table self._auth_provider_id = "saml" @@ -174,26 +177,56 @@ async def _map_saml_response_to_user(self, resp_bytes): return registered_user_id # figure out a new mxid for this user - base_mxid_localpart = self._mxid_mapper(mxid_source) + for i in range(1000): + # Use the provider's custom handler if available + if self._mapping_provider: + localpart = self._mapping_provider.mxid_source_to_mixd_localpart( + mxid_source, i + ) + else: + localpart = self.mxid_source_to_mxid_localpart(mxid_source, i) + logger.info("Allocating mxid for new user with localpart %s", localpart) - suffix = 0 - while True: - localpart = base_mxid_localpart + (str(suffix) if suffix else "") + # Check if this mxid already exists if not await self._datastore.get_users_by_id_case_insensitive( UserID(localpart, self._hostname).to_string() ): + # This mxid is free break - suffix += 1 - logger.info("Allocating mxid for new user with localpart %s", localpart) + else: + # Unable to generate a username in 1000 iterations + # Break and return error to the user + raise SynapseError( + 500, "Unable to generate a Matrix ID from the SAML response" + ) registered_user_id = await self._registration_handler.register_user( localpart=localpart, default_display_name=displayName ) + await self._datastore.record_user_external_id( self._auth_provider_id, remote_user_id, registered_user_id ) return registered_user_id + def mxid_source_to_mxid_localpart(self, mxid_source: str, failures: int = 0) -> str: + """Maps some text from a SAML response to the localpart of a new mxid + + Args: + mxid_source (str): The input text from a SAML auth response + + failures (int): How many times a call to this function with this mxid_source has + resulted in a failure (possibly due to the localpart already existing) + + Returns: + str: The localpart of a new mxid + """ + # Use the configured mapper for this mxid_source + base_mxid_localpart = self._mxid_mapper(mxid_source) + + # Append suffix integer if last call to this function failed to produce a usable mxid + return base_mxid_localpart + (str(failures) if failures else "") + def expire_sessions(self): expire_before = self._clock.time_msec() - self._saml2_session_lifetime to_expire = set() diff --git a/synapse/util/module_loader.py b/synapse/util/module_loader.py index 2705cbe5f892..bf5de227f3e3 100644 --- a/synapse/util/module_loader.py +++ b/synapse/util/module_loader.py @@ -33,10 +33,14 @@ def load_module(provider): module = importlib.import_module(module) provider_class = getattr(module, clz) - 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)) + provider_config = None + if "config" in provider: + 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 From 11805ba5bcdef2fdcca778f455ac79821c857b58 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Mon, 25 Nov 2019 17:10:16 +0000 Subject: [PATCH 02/35] Add changelog --- changelog.d/6411.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/6411.feature diff --git a/changelog.d/6411.feature b/changelog.d/6411.feature new file mode 100644 index 000000000000..ebea4a208d31 --- /dev/null +++ b/changelog.d/6411.feature @@ -0,0 +1 @@ +Allow custom SAML username mapping functinality through an external provider plugin. \ No newline at end of file From 82b81c0566e27a60cc1711949d8ca5c4755a6e0e Mon Sep 17 00:00:00 2001 From: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> Date: Wed, 27 Nov 2019 10:52:51 +0000 Subject: [PATCH 03/35] Apply suggestions from code review Co-Authored-By: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --- synapse/config/saml2_config.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/synapse/config/saml2_config.py b/synapse/config/saml2_config.py index 255cd2c18f6c..4f4285a416e0 100644 --- a/synapse/config/saml2_config.py +++ b/synapse/config/saml2_config.py @@ -234,10 +234,11 @@ def generate_config_section(self, config_dir_path, server_name, **kwargs): # #mxid_mapping: dotreplace - # A third-party module can be provided here as a custom solution to mapping the + # An external module can be provided here as a custom solution to mapping the # above configured saml attribute onto a matrix ID. If this is defined, it will # override the `mxid_mapping` option. - #mxid_mapping_provider: "mapping_provider.SamlMappingProvider" + # + #mxid_mapping_provider: mapping_provider.SamlMappingProvider # In previous versions of synapse, the mapping from SAML attribute to MXID was # always calculated dynamically rather than stored in a table. For backwards- From f3a3eb49ce80dc6f156aa980b0164748c6b8a23a Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 27 Nov 2019 11:04:38 +0000 Subject: [PATCH 04/35] typo --- synapse/handlers/saml_handler.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/handlers/saml_handler.py b/synapse/handlers/saml_handler.py index f425b9b8a4fe..342c85258510 100644 --- a/synapse/handlers/saml_handler.py +++ b/synapse/handlers/saml_handler.py @@ -180,7 +180,7 @@ async def _map_saml_response_to_user(self, resp_bytes): for i in range(1000): # Use the provider's custom handler if available if self._mapping_provider: - localpart = self._mapping_provider.mxid_source_to_mixd_localpart( + localpart = self._mapping_provider.mxid_source_to_mxid_localpart( mxid_source, i ) else: From da849c486c8f52ae60b5c309126d323db957c70e Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 27 Nov 2019 11:59:50 +0000 Subject: [PATCH 05/35] Provide a defeault mapping provider class --- docs/sample_config.yaml | 11 +++++- synapse/config/saml2_config.py | 21 +++++++---- synapse/handlers/saml_handler.py | 63 +++++++++++++++++++------------- synapse/util/module_loader.py | 14 +++---- 4 files changed, 67 insertions(+), 42 deletions(-) diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index e3c30ec7193b..d2f6c3ab1b68 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -1205,10 +1205,17 @@ saml2_config: # #mxid_mapping: dotreplace - # A third-party module can be provided here as a custom solution to mapping the + # An external module can be provided here as a custom solution to mapping the # above configured saml attribute onto a matrix ID. If this is defined, it will # override the `mxid_mapping` option. - #mxid_mapping_provider: "mapping_provider.SamlMappingProvider" + # + #mxid_mapping_provider: mapping_provider.SamlMappingProvider + + # Custom configuration values for the module. This will be passed as a + # Python dictionary to the module's `parse_config` method. + # + #mxid_mapping_provider_config: + # some_key: some_value # In previous versions of synapse, the mapping from SAML attribute to MXID was # always calculated dynamically rather than stored in a table. For backwards- diff --git a/synapse/config/saml2_config.py b/synapse/config/saml2_config.py index 4f4285a416e0..36a2582d526c 100644 --- a/synapse/config/saml2_config.py +++ b/synapse/config/saml2_config.py @@ -109,13 +109,14 @@ def read_config(self, config, **kwargs): except KeyError: raise ConfigError("%s is not a known mxid_mapping" % (mapping,)) - self.saml2_mapping_provider = None - mapping_provider_module = config.get("saml_mapping_provider") - if mapping_provider_module: - # Load configured module class - self.saml2_mapping_provider, _ = load_module( - {"mod_name": self.saml2_mapping_provider, "config": None} - ) + # Load configured module class and config + mapping_provider_module = config.get( + "saml_mapping_provider", "synapse.handlers.saml_handler.DefaultSamlMappingProvider" + ) + mapping_provider_config = config.get("saml_mapping_provider_config", {}) + self.saml2_mapping_provider, _ = load_module( + {"mod_name": mapping_provider_module, "config": mapping_provider_config} + ) def _default_saml_config_dict(self): import saml2 @@ -240,6 +241,12 @@ def generate_config_section(self, config_dir_path, server_name, **kwargs): # #mxid_mapping_provider: mapping_provider.SamlMappingProvider + # Custom configuration values for the module. This will be passed as a + # Python dictionary to the module's `parse_config` method. + # + #mxid_mapping_provider_config: + # some_key: some_value + # In previous versions of synapse, the mapping from SAML attribute to MXID was # always calculated dynamically rather than stored in a table. For backwards- # compatibility, we will look for user_ids matching such a pattern before diff --git a/synapse/handlers/saml_handler.py b/synapse/handlers/saml_handler.py index 342c85258510..75b931456d34 100644 --- a/synapse/handlers/saml_handler.py +++ b/synapse/handlers/saml_handler.py @@ -178,13 +178,9 @@ async def _map_saml_response_to_user(self, resp_bytes): # figure out a new mxid for this user for i in range(1000): - # Use the provider's custom handler if available - if self._mapping_provider: - localpart = self._mapping_provider.mxid_source_to_mxid_localpart( - mxid_source, i - ) - else: - localpart = self.mxid_source_to_mxid_localpart(mxid_source, i) + localpart = self._mapping_provider.mxid_source_to_mxid_localpart( + mxid_source, i + ) logger.info("Allocating mxid for new user with localpart %s", localpart) # Check if this mxid already exists @@ -209,24 +205,6 @@ async def _map_saml_response_to_user(self, resp_bytes): ) return registered_user_id - def mxid_source_to_mxid_localpart(self, mxid_source: str, failures: int = 0) -> str: - """Maps some text from a SAML response to the localpart of a new mxid - - Args: - mxid_source (str): The input text from a SAML auth response - - failures (int): How many times a call to this function with this mxid_source has - resulted in a failure (possibly due to the localpart already existing) - - Returns: - str: The localpart of a new mxid - """ - # Use the configured mapper for this mxid_source - base_mxid_localpart = self._mxid_mapper(mxid_source) - - # Append suffix integer if last call to this function failed to produce a usable mxid - return base_mxid_localpart + (str(failures) if failures else "") - def expire_sessions(self): expire_before = self._clock.time_msec() - self._saml2_session_lifetime to_expire = set() @@ -244,3 +222,38 @@ class Saml2SessionData: # time the session was created, in milliseconds creation_time = attr.ib() + + +class DefaultSamlMappingProvider(object): + __version__ = "0.0.1" + + def __init__(self, mxid_mapper: any): + self._mxid_mapper = mxid_mapper + + def mxid_source_to_mxid_localpart(self, mxid_source: str, failures: int = 0) -> str: + """Maps some text from a SAML response to the localpart of a new mxid + + Args: + mxid_source (str): The input text from a SAML auth response + + failures (int): How many times a call to this function with this + mxid_source has resulted in a failure (possibly due to the localpart + already existing) + + Returns: + str: The localpart of a new mxid + """ + # Use the configured mapper for this mxid_source + base_mxid_localpart = self._mxid_mapper(mxid_source) + + # Append suffix integer if last call to this function failed to produce + # a usable mxid + return base_mxid_localpart + (str(failures) if failures else "") + + @staticmethod + def parse_config(config): + """Parse the dict provided in the homeserver config. + + We currently do not use any config vars + """ + pass diff --git a/synapse/util/module_loader.py b/synapse/util/module_loader.py index bf5de227f3e3..b39a09b22518 100644 --- a/synapse/util/module_loader.py +++ b/synapse/util/module_loader.py @@ -33,14 +33,12 @@ def load_module(provider): module = importlib.import_module(module) provider_class = getattr(module, clz) - provider_config = None - if "config" in provider: - 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 From b1a0a5bda934dbdad5a15701a8ba33a5730d1053 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 27 Nov 2019 15:36:31 +0000 Subject: [PATCH 06/35] wip saml mapping provider documentation --- docs/saml_mapping_providers.md | 46 ++++++++++++++++++++++++++++++++++ docs/sample_config.yaml | 4 +-- synapse/config/saml2_config.py | 9 ++++--- synapse/util/module_loader.py | 4 +-- 4 files changed, 54 insertions(+), 9 deletions(-) create mode 100644 docs/saml_mapping_providers.md diff --git a/docs/saml_mapping_providers.md b/docs/saml_mapping_providers.md new file mode 100644 index 000000000000..2ada54b9c04a --- /dev/null +++ b/docs/saml_mapping_providers.md @@ -0,0 +1,46 @@ +# SAML Mapping Providers + +A SAML mapping provider is a Python class (loaded via a Python module) that +works out how to map attributes of a SAML response object to Matrix-specific +user attributes. Details such as user ID localpart, displayname, and even avatar +URLs are all things that can be mapped from talking to a SSO service. + +As an example, a SSO service may return the email address +"john.smith@example.com" for a user, whereas Synapse will need to figure out how +to turn that into a displayname when creating a Matrix user for this individual. +It may choose `John Smith`, or `Smith, John [Example.com]` or any number of +variations. As each Synapse configuration may want something different, this is +where SAML mapping providers come in. + +## Enabling Providers + +External mapping providers are provided to Synapse in the form of a 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. + +`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). + +## Building a Custom Mapping Provider + +A custom mapping provider must specify the following methods: + +* `mapping thing` + - Receives the `type` object from a SAML response. This method must return a + dictionary, which will then be used by Synapse to build a new user. The + following keys are allowed: + * displayname (required) + * user_id_localpart (required) + * avatar_url +* `parse_config(config: dict)` + - Receives the parsed content of the `saml2_config.user_mapping_provider` + homeserver config option. Runs on homeserver startup. Providers should + extract any option values they need here. + +## Synapse's Default Provider + +Synapse has a built-in SAML mapping provider if a custom provider isn't +specified in the config. It is located at +[`synapse.handlers.saml_handler.DefaultSamlMappingProvider`](../synapse/handlers/saml_handler.py). diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index d2f6c3ab1b68..d56bcf44311a 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -1209,12 +1209,12 @@ saml2_config: # above configured saml attribute onto a matrix ID. If this is defined, it will # override the `mxid_mapping` option. # - #mxid_mapping_provider: mapping_provider.SamlMappingProvider + #user_mapping_provider: mapping_provider.SamlMappingProvider # Custom configuration values for the module. This will be passed as a # Python dictionary to the module's `parse_config` method. # - #mxid_mapping_provider_config: + #user_mapping_provider_config: # some_key: some_value # In previous versions of synapse, the mapping from SAML attribute to MXID was diff --git a/synapse/config/saml2_config.py b/synapse/config/saml2_config.py index 36a2582d526c..090457fdda2e 100644 --- a/synapse/config/saml2_config.py +++ b/synapse/config/saml2_config.py @@ -111,9 +111,10 @@ def read_config(self, config, **kwargs): # Load configured module class and config mapping_provider_module = config.get( - "saml_mapping_provider", "synapse.handlers.saml_handler.DefaultSamlMappingProvider" + "user_mapping_provider", + "synapse.handlers.saml_handler.DefaultSamlMappingProvider", ) - mapping_provider_config = config.get("saml_mapping_provider_config", {}) + mapping_provider_config = config.get("user_mapping_provider_config", {}) self.saml2_mapping_provider, _ = load_module( {"mod_name": mapping_provider_module, "config": mapping_provider_config} ) @@ -239,12 +240,12 @@ def generate_config_section(self, config_dir_path, server_name, **kwargs): # above configured saml attribute onto a matrix ID. If this is defined, it will # override the `mxid_mapping` option. # - #mxid_mapping_provider: mapping_provider.SamlMappingProvider + #user_mapping_provider: mapping_provider.SamlMappingProvider # Custom configuration values for the module. This will be passed as a # Python dictionary to the module's `parse_config` method. # - #mxid_mapping_provider_config: + #user_mapping_provider_config: # some_key: some_value # In previous versions of synapse, the mapping from SAML attribute to MXID was diff --git a/synapse/util/module_loader.py b/synapse/util/module_loader.py index b39a09b22518..2705cbe5f892 100644 --- a/synapse/util/module_loader.py +++ b/synapse/util/module_loader.py @@ -36,9 +36,7 @@ def load_module(provider): 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) - ) + raise ConfigError("Failed to parse config for %r: %r" % (provider["module"], e)) return provider_class, provider_config From 466b28b077be60b3dd2d62909d1f5d487810ad79 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Thu, 28 Nov 2019 15:15:25 +0000 Subject: [PATCH 07/35] Add SAML mapping provider docs --- docs/saml_mapping_providers.md | 46 ++++++++++++++++++++++++++++++++++ docs/sample_config.yaml | 4 +-- synapse/config/saml2_config.py | 9 ++++--- synapse/util/module_loader.py | 4 +-- 4 files changed, 54 insertions(+), 9 deletions(-) create mode 100644 docs/saml_mapping_providers.md diff --git a/docs/saml_mapping_providers.md b/docs/saml_mapping_providers.md new file mode 100644 index 000000000000..b7c200caabcb --- /dev/null +++ b/docs/saml_mapping_providers.md @@ -0,0 +1,46 @@ +# SAML Mapping Providers + +A SAML mapping provider is a Python class (loaded via a Python module) that +works out how to map attributes of a SAML response object to Matrix-specific +user attributes. Details such as user ID localpart, displayname, and even avatar +URLs are all things that can be mapped from talking to a SSO service. + +As an example, a SSO service may return the email address +"john.smith@example.com" for a user, whereas Synapse will need to figure out how +to turn that into a displayname when creating a Matrix user for this individual. +It may choose `John Smith`, or `Smith, John [Example.com]` or any number of +variations. As each Synapse configuration may want something different, this is +where SAML mapping providers come in. + +## Enabling Providers + +External mapping providers are provided to Synapse in the form of a 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. + +`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). + +## Building a Custom Mapping Provider + +A custom mapping provider must specify the following methods: + +* `saml_response_to_user_attributes` + - Receives the `type` object from a SAML response. This method must return a + dictionary, which will then be used by Synapse to build a new user. The + following keys are allowed: + * displayname (required) + * user_id_localpart (required) + * avatar_url +* `parse_config(config: dict)` + - Receives the parsed content of the `saml2_config.user_mapping_provider` + homeserver config option. Runs on homeserver startup. Providers should + extract any option values they need here. + +## Synapse's Default Provider + +Synapse has a built-in SAML mapping provider if a custom provider isn't +specified in the config. It is located at +[`synapse.handlers.saml_handler.DefaultSamlMappingProvider`](../synapse/handlers/saml_handler.py). diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index d2f6c3ab1b68..d56bcf44311a 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -1209,12 +1209,12 @@ saml2_config: # above configured saml attribute onto a matrix ID. If this is defined, it will # override the `mxid_mapping` option. # - #mxid_mapping_provider: mapping_provider.SamlMappingProvider + #user_mapping_provider: mapping_provider.SamlMappingProvider # Custom configuration values for the module. This will be passed as a # Python dictionary to the module's `parse_config` method. # - #mxid_mapping_provider_config: + #user_mapping_provider_config: # some_key: some_value # In previous versions of synapse, the mapping from SAML attribute to MXID was diff --git a/synapse/config/saml2_config.py b/synapse/config/saml2_config.py index 36a2582d526c..090457fdda2e 100644 --- a/synapse/config/saml2_config.py +++ b/synapse/config/saml2_config.py @@ -111,9 +111,10 @@ def read_config(self, config, **kwargs): # Load configured module class and config mapping_provider_module = config.get( - "saml_mapping_provider", "synapse.handlers.saml_handler.DefaultSamlMappingProvider" + "user_mapping_provider", + "synapse.handlers.saml_handler.DefaultSamlMappingProvider", ) - mapping_provider_config = config.get("saml_mapping_provider_config", {}) + mapping_provider_config = config.get("user_mapping_provider_config", {}) self.saml2_mapping_provider, _ = load_module( {"mod_name": mapping_provider_module, "config": mapping_provider_config} ) @@ -239,12 +240,12 @@ def generate_config_section(self, config_dir_path, server_name, **kwargs): # above configured saml attribute onto a matrix ID. If this is defined, it will # override the `mxid_mapping` option. # - #mxid_mapping_provider: mapping_provider.SamlMappingProvider + #user_mapping_provider: mapping_provider.SamlMappingProvider # Custom configuration values for the module. This will be passed as a # Python dictionary to the module's `parse_config` method. # - #mxid_mapping_provider_config: + #user_mapping_provider_config: # some_key: some_value # In previous versions of synapse, the mapping from SAML attribute to MXID was diff --git a/synapse/util/module_loader.py b/synapse/util/module_loader.py index b39a09b22518..2705cbe5f892 100644 --- a/synapse/util/module_loader.py +++ b/synapse/util/module_loader.py @@ -36,9 +36,7 @@ def load_module(provider): 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) - ) + raise ConfigError("Failed to parse config for %r: %r" % (provider["module"], e)) return provider_class, provider_config From 0985eb60f754bd138adb9e62a3eedbca3fd612c3 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Thu, 28 Nov 2019 15:16:47 +0000 Subject: [PATCH 08/35] Pass the whole saml response object to the provider --- docs/sample_config.yaml | 32 +++++---- synapse/config/saml2_config.py | 47 ++++++++----- synapse/handlers/saml_handler.py | 116 ++++++++++++++++++------------- 3 files changed, 115 insertions(+), 80 deletions(-) diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index d56bcf44311a..88ade4ff9035 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -1192,19 +1192,6 @@ saml2_config: # #saml_session_lifetime: 5m - # The SAML attribute (after mapping via the attribute maps) to use to derive - # the Matrix ID from. 'uid' by default. - # - #mxid_source_attribute: displayName - - # The mapping system to use for mapping the saml attribute onto a matrix ID. - # Options include: - # * 'hexencode' (which maps unpermitted characters to '=xx') - # * 'dotreplace' (which replaces unpermitted characters with '.'). - # The default is 'hexencode'. - # - #mxid_mapping: dotreplace - # An external module can be provided here as a custom solution to mapping the # above configured saml attribute onto a matrix ID. If this is defined, it will # override the `mxid_mapping` option. @@ -1215,7 +1202,24 @@ saml2_config: # Python dictionary to the module's `parse_config` method. # #user_mapping_provider_config: - # some_key: some_value + # # The SAML attribute (after mapping via the attribute maps) to use to derive + # # the Matrix ID from. 'uid' by default. + # # + # # Note: This used to be configured by the saml2_config.mxid_source_attribute + # # option. If that is still defined, its value will be used instead. + # # + # #mxid_source_attribute: displayName + # + # # The mapping system to use for mapping the saml attribute onto a matrix ID. + # # Options include: + # # * 'hexencode' (which maps unpermitted characters to '=xx') + # # * 'dotreplace' (which replaces unpermitted characters with '.'). + # # The default is 'hexencode'. + # # + # # Note: This used to be configured by the saml2_config.mxid_mapping + # # option. If that is still defined, its value will be used instead. + # # + # #mxid_mapping: dotreplace # In previous versions of synapse, the mapping from SAML attribute to MXID was # always calculated dynamically rather than stored in a table. For backwards- diff --git a/synapse/config/saml2_config.py b/synapse/config/saml2_config.py index 090457fdda2e..0742cbaa69e7 100644 --- a/synapse/config/saml2_config.py +++ b/synapse/config/saml2_config.py @@ -75,10 +75,6 @@ def read_config(self, config, **kwargs): self.saml2_enabled = True - self.saml2_mxid_source_attribute = saml2_config.get( - "mxid_source_attribute", "uid" - ) - self.saml2_grandfathered_mxid_source_attribute = saml2_config.get( "grandfathered_mxid_source_attribute", "uid" ) @@ -115,6 +111,17 @@ def read_config(self, config, **kwargs): "synapse.handlers.saml_handler.DefaultSamlMappingProvider", ) mapping_provider_config = config.get("user_mapping_provider_config", {}) + + # If mxid_source_attribute is defined, use that instead for backwards compatibility + mxid_source_attribute = saml2_config.get("mxid_source_attribute") + if mxid_source_attribute: + mapping_provider_config["mxid_source_attribute"] = mxid_source_attribute + + # If mxid_mapping is defined, use that instead for backwards compatibility + mxid_mapping = saml2_config.get("mxid_mapping") + if mxid_mapping: + mapping_provider_config["mxid_mapping"] = mxid_mapping + self.saml2_mapping_provider, _ = load_module( {"mod_name": mapping_provider_module, "config": mapping_provider_config} ) @@ -223,19 +230,6 @@ def generate_config_section(self, config_dir_path, server_name, **kwargs): # #saml_session_lifetime: 5m - # The SAML attribute (after mapping via the attribute maps) to use to derive - # the Matrix ID from. 'uid' by default. - # - #mxid_source_attribute: displayName - - # The mapping system to use for mapping the saml attribute onto a matrix ID. - # Options include: - # * 'hexencode' (which maps unpermitted characters to '=xx') - # * 'dotreplace' (which replaces unpermitted characters with '.'). - # The default is 'hexencode'. - # - #mxid_mapping: dotreplace - # An external module can be provided here as a custom solution to mapping the # above configured saml attribute onto a matrix ID. If this is defined, it will # override the `mxid_mapping` option. @@ -246,7 +240,24 @@ def generate_config_section(self, config_dir_path, server_name, **kwargs): # Python dictionary to the module's `parse_config` method. # #user_mapping_provider_config: - # some_key: some_value + # # The SAML attribute (after mapping via the attribute maps) to use to derive + # # the Matrix ID from. 'uid' by default. + # # + # # Note: This used to be configured by the saml2_config.mxid_source_attribute + # # option. If that is still defined, its value will be used instead. + # # + # #mxid_source_attribute: displayName + # + # # The mapping system to use for mapping the saml attribute onto a matrix ID. + # # Options include: + # # * 'hexencode' (which maps unpermitted characters to '=xx') + # # * 'dotreplace' (which replaces unpermitted characters with '.'). + # # The default is 'hexencode'. + # # + # # Note: This used to be configured by the saml2_config.mxid_mapping + # # option. If that is still defined, its value will be used instead. + # # + # #mxid_mapping: dotreplace # In previous versions of synapse, the mapping from SAML attribute to MXID was # always calculated dynamically rather than stored in a table. For backwards- diff --git a/synapse/handlers/saml_handler.py b/synapse/handlers/saml_handler.py index 75b931456d34..84a74cae7866 100644 --- a/synapse/handlers/saml_handler.py +++ b/synapse/handlers/saml_handler.py @@ -16,6 +16,7 @@ import attr import saml2 +import saml2.response from saml2.client import Saml2Client from synapse.api.errors import SynapseError @@ -123,19 +124,39 @@ async def _map_saml_response_to_user(self, resp_bytes): logger.warning("SAML2 response lacks a 'uid' attestation") raise SynapseError(400, "uid not in SAML2 response") - try: - mxid_source = saml2_auth.ava[self._mxid_source_attribute][0] - except KeyError: - logger.warning( - "SAML2 response lacks a '%s' attestation", self._mxid_source_attribute - ) - raise SynapseError( - 400, "%s not in SAML2 response" % (self._mxid_source_attribute,) - ) - self._outstanding_requests_dict.pop(saml2_auth.in_response_to, None) - displayName = saml2_auth.ava.get("displayName", [None])[0] + # Map saml response to user attributes using the configured mapping provider + for i in range(1000): + try: + attribute_dict = self._mapping_provider.saml_response_to_user_attributes( + saml2_auth, i + ) + except Exception: + logging.exception("Error in SAML mapping provider plugin") + raise SynapseError(500, "Error parsing SAML2 response") + + localpart = attribute_dict.get("mxid_localpart") + if not localpart: + logger.error( + "SAML mapping provider plugin did not return a mxid_localpart object" + ) + raise SynapseError(500, "Error parsing SAML2 response") + + displayname = attribute_dict.get("displayname") + + # Check if this mxid already exists + if not await self._datastore.get_users_by_id_case_insensitive( + UserID(localpart, self._hostname).to_string() + ): + # This mxid is free + break + else: + # Unable to generate a username in 1000 iterations + # Break and return error to the user + raise SynapseError( + 500, "Unable to generate a Matrix ID from the 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 @@ -176,28 +197,8 @@ async def _map_saml_response_to_user(self, resp_bytes): ) return registered_user_id - # figure out a new mxid for this user - for i in range(1000): - localpart = self._mapping_provider.mxid_source_to_mxid_localpart( - mxid_source, i - ) - logger.info("Allocating mxid for new user with localpart %s", localpart) - - # Check if this mxid already exists - if not await self._datastore.get_users_by_id_case_insensitive( - UserID(localpart, self._hostname).to_string() - ): - # This mxid is free - break - else: - # Unable to generate a username in 1000 iterations - # Break and return error to the user - raise SynapseError( - 500, "Unable to generate a Matrix ID from the SAML response" - ) - registered_user_id = await self._registration_handler.register_user( - localpart=localpart, default_display_name=displayName + localpart=localpart, default_display_name=displayname ) await self._datastore.record_user_external_id( @@ -227,33 +228,52 @@ class Saml2SessionData: class DefaultSamlMappingProvider(object): __version__ = "0.0.1" - def __init__(self, mxid_mapper: any): - self._mxid_mapper = mxid_mapper + def __init__(self): + self._mxid_mapper = None + self._mxid_source_attribute = None - def mxid_source_to_mxid_localpart(self, mxid_source: str, failures: int = 0) -> str: - """Maps some text from a SAML response to the localpart of a new mxid + 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: - mxid_source (str): The input text from a SAML auth response + saml_response: A SAML auth response object - failures (int): How many times a call to this function with this - mxid_source has resulted in a failure (possibly due to the localpart - already existing) + failures: How many times a call to this function with this + saml_response has resulted in a failure Returns: - str: The localpart of a new mxid + dict: A dict containing new user attributes. Possible keys: + * mxid_localpart (str): Required. The localpart of the user's mxid + * displayname (str): The displayname of the user """ + try: + mxid_source = saml_response.ava[self._mxid_source_attribute][0] + except KeyError: + logger.warning( + "SAML2 response lacks a '%s' attestation", self._mxid_source_attribute + ) + raise SynapseError( + 400, "%s not in SAML2 response" % (self._mxid_source_attribute,) + ) + # Use the configured mapper for this mxid_source base_mxid_localpart = self._mxid_mapper(mxid_source) # Append suffix integer if last call to this function failed to produce # a usable mxid - return base_mxid_localpart + (str(failures) if failures else "") + localpart = base_mxid_localpart + (str(failures) if failures else "") - @staticmethod - def parse_config(config): - """Parse the dict provided in the homeserver config. + # Retrieve the display name from the saml response + displayname = saml_response.ava.get("displayName", [None])[0] - We currently do not use any config vars - """ - pass + return { + "mxid_localpart": localpart, + "displayname": displayname, + } + + def parse_config(self, config): + """Parse the dict provided by the homeserver config""" + self._mxid_source_attribute = config.get("mxid_source_attribute", "uid") + self._mxid_source_attribute = config.get("mxid_mapping", "hexencode") From 595e94764e83305a44b04c0efb7e020dc8b97756 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Thu, 28 Nov 2019 16:12:58 +0000 Subject: [PATCH 09/35] Keep saml2_mxid_source_attribute object for SamlHandler --- synapse/config/saml2_config.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/synapse/config/saml2_config.py b/synapse/config/saml2_config.py index 0742cbaa69e7..78d2b305fa5e 100644 --- a/synapse/config/saml2_config.py +++ b/synapse/config/saml2_config.py @@ -122,6 +122,10 @@ def read_config(self, config, **kwargs): if mxid_mapping: mapping_provider_config["mxid_mapping"] = mxid_mapping + self.saml2_mxid_source_attribute = mapping_provider_config[ + "mxid_source_attribute" + ] + self.saml2_mapping_provider, _ = load_module( {"mod_name": mapping_provider_module, "config": mapping_provider_config} ) From 6fec3a479a56b1a2d8d53961a44404629e7eb033 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Thu, 28 Nov 2019 16:15:17 +0000 Subject: [PATCH 10/35] Fix if statement logic --- synapse/config/saml2_config.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/synapse/config/saml2_config.py b/synapse/config/saml2_config.py index 78d2b305fa5e..6ec5d808c585 100644 --- a/synapse/config/saml2_config.py +++ b/synapse/config/saml2_config.py @@ -114,12 +114,12 @@ def read_config(self, config, **kwargs): # If mxid_source_attribute is defined, use that instead for backwards compatibility mxid_source_attribute = saml2_config.get("mxid_source_attribute") - if mxid_source_attribute: + if not mxid_source_attribute: mapping_provider_config["mxid_source_attribute"] = mxid_source_attribute # If mxid_mapping is defined, use that instead for backwards compatibility mxid_mapping = saml2_config.get("mxid_mapping") - if mxid_mapping: + if not mxid_mapping: mapping_provider_config["mxid_mapping"] = mxid_mapping self.saml2_mxid_source_attribute = mapping_provider_config[ From 7eecd6cabe698035e1da028e10b97aa28490e51e Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Thu, 28 Nov 2019 16:30:41 +0000 Subject: [PATCH 11/35] Load module before doing most of the rest of the saml config --- synapse/config/saml2_config.py | 78 ++++++++++---------------------- synapse/handlers/saml_handler.py | 47 +++++++++++++++---- 2 files changed, 61 insertions(+), 64 deletions(-) diff --git a/synapse/config/saml2_config.py b/synapse/config/saml2_config.py index 6ec5d808c585..e633f60ec870 100644 --- a/synapse/config/saml2_config.py +++ b/synapse/config/saml2_config.py @@ -14,13 +14,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -import re - from synapse.python_dependencies import DependencyException, check_requirements -from synapse.types import ( - map_username_to_mxid_localpart, - mxid_localpart_allowed_characters, -) from synapse.util.module_loader import load_module, load_python_module from ._base import Config, ConfigError @@ -79,34 +73,8 @@ def read_config(self, config, **kwargs): "grandfathered_mxid_source_attribute", "uid" ) - saml2_config_dict = self._default_saml_config_dict() - _dict_merge( - merge_dict=saml2_config.get("sp_config", {}), into_dict=saml2_config_dict - ) - - config_path = saml2_config.get("config_path", None) - if config_path is not None: - mod = load_python_module(config_path) - _dict_merge(merge_dict=mod.CONFIG, into_dict=saml2_config_dict) - - import saml2.config - - self.saml2_sp_config = saml2.config.SPConfig() - self.saml2_sp_config.load(saml2_config_dict) - - # session lifetime: in milliseconds - self.saml2_session_lifetime = self.parse_duration( - saml2_config.get("saml_session_lifetime", "5m") - ) - - mapping = saml2_config.get("mxid_mapping", "hexencode") - try: - self.saml2_mxid_mapper = MXID_MAPPER_MAP[mapping] - except KeyError: - raise ConfigError("%s is not a known mxid_mapping" % (mapping,)) - # Load configured module class and config - mapping_provider_module = config.get( + mapping_provider_module = saml2_config.get( "user_mapping_provider", "synapse.handlers.saml_handler.DefaultSamlMappingProvider", ) @@ -114,12 +82,12 @@ def read_config(self, config, **kwargs): # If mxid_source_attribute is defined, use that instead for backwards compatibility mxid_source_attribute = saml2_config.get("mxid_source_attribute") - if not mxid_source_attribute: + if mxid_source_attribute: mapping_provider_config["mxid_source_attribute"] = mxid_source_attribute # If mxid_mapping is defined, use that instead for backwards compatibility mxid_mapping = saml2_config.get("mxid_mapping") - if not mxid_mapping: + if mxid_mapping: mapping_provider_config["mxid_mapping"] = mxid_mapping self.saml2_mxid_source_attribute = mapping_provider_config[ @@ -130,6 +98,26 @@ def read_config(self, config, **kwargs): {"mod_name": mapping_provider_module, "config": 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 + ) + + config_path = saml2_config.get("config_path", None) + if config_path is not None: + mod = load_python_module(config_path) + _dict_merge(merge_dict=mod.CONFIG, into_dict=saml2_config_dict) + + import saml2.config + + self.saml2_sp_config = saml2.config.SPConfig() + self.saml2_sp_config.load(saml2_config_dict) + + # session lifetime: in milliseconds + self.saml2_session_lifetime = self.parse_duration( + saml2_config.get("saml_session_lifetime", "5m") + ) + def _default_saml_config_dict(self): import saml2 @@ -278,23 +266,3 @@ def generate_config_section(self, config_dir_path, server_name, **kwargs): """ % { "config_dir_path": config_dir_path } - - -DOT_REPLACE_PATTERN = re.compile( - ("[^%s]" % (re.escape("".join(mxid_localpart_allowed_characters)),)) -) - - -def dot_replace_for_mxid(username: str) -> str: - username = username.lower() - username = DOT_REPLACE_PATTERN.sub(".", username) - - # regular mxids aren't allowed to start with an underscore either - username = re.sub("^_", "", username) - return username - - -MXID_MAPPER_MAP = { - "hexencode": map_username_to_mxid_localpart, - "dotreplace": dot_replace_for_mxid, -} diff --git a/synapse/handlers/saml_handler.py b/synapse/handlers/saml_handler.py index 84a74cae7866..ddd38edf1191 100644 --- a/synapse/handlers/saml_handler.py +++ b/synapse/handlers/saml_handler.py @@ -13,6 +13,7 @@ # See the License for the specific language governing permissions and # limitations under the License. import logging +import re import attr import saml2 @@ -22,12 +23,24 @@ from synapse.api.errors import SynapseError from synapse.http.servlet import parse_string from synapse.rest.client.v1.login import SSOAuthHandler -from synapse.types import UserID, map_username_to_mxid_localpart +from synapse.types import ( + UserID, + map_username_to_mxid_localpart, + mxid_localpart_allowed_characters, +) from synapse.util.async_helpers import Linearizer logger = logging.getLogger(__name__) +@attr.s +class Saml2SessionData: + """Data we track about SAML2 sessions""" + + # time the session was created, in milliseconds + creation_time = attr.ib() + + class SamlHandler: def __init__(self, hs): self._saml_client = Saml2Client(hs.config.saml2_sp_config) @@ -42,7 +55,6 @@ def __init__(self, hs): self._grandfathered_mxid_source_attribute = ( hs.config.saml2_grandfathered_mxid_source_attribute ) - self._mxid_mapper = hs.config.saml2_mxid_mapper # plugin to do custom mapping from saml response to mxid self._mapping_provider = hs.config.saml2_mapping_provider @@ -217,20 +229,32 @@ def expire_sessions(self): del self._outstanding_requests_dict[reqid] -@attr.s -class Saml2SessionData: - """Data we track about SAML2 sessions""" +DOT_REPLACE_PATTERN = re.compile( + ("[^%s]" % (re.escape("".join(mxid_localpart_allowed_characters)),)) +) - # time the session was created, in milliseconds - creation_time = attr.ib() + +def dot_replace_for_mxid(username: str) -> str: + username = username.lower() + username = DOT_REPLACE_PATTERN.sub(".", username) + + # regular mxids aren't allowed to start with an underscore either + username = re.sub("^_", "", username) + return username + + +MXID_MAPPER_MAP = { + "hexencode": map_username_to_mxid_localpart, + "dotreplace": dot_replace_for_mxid, +} class DefaultSamlMappingProvider(object): __version__ = "0.0.1" def __init__(self): - self._mxid_mapper = None self._mxid_source_attribute = None + self._mxid_mapper = None def saml_response_to_user_attributes( self, saml_response: saml2.response.AuthnResponse, failures: int = 0, @@ -276,4 +300,9 @@ def saml_response_to_user_attributes( def parse_config(self, config): """Parse the dict provided by the homeserver config""" self._mxid_source_attribute = config.get("mxid_source_attribute", "uid") - self._mxid_source_attribute = config.get("mxid_mapping", "hexencode") + + mapping_type = config.get("mxid_mapper", "hexencode") + try: + self._mxid_mapper = MXID_MAPPER_MAP[mapping_type] + except KeyError: + raise Exception("%s is not a known mxid_mapping" % (mapping_type,)) From fcb31ef513824f87de5bd76b8630c81c685a8f9f Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Thu, 28 Nov 2019 16:40:25 +0000 Subject: [PATCH 12/35] Properly pull in backwards compatible options --- synapse/config/saml2_config.py | 16 ++++++++++------ synapse/handlers/saml_handler.py | 9 +++++++-- 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/synapse/config/saml2_config.py b/synapse/config/saml2_config.py index e633f60ec870..f0de0553f911 100644 --- a/synapse/config/saml2_config.py +++ b/synapse/config/saml2_config.py @@ -81,14 +81,18 @@ def read_config(self, config, **kwargs): mapping_provider_config = config.get("user_mapping_provider_config", {}) # If mxid_source_attribute is defined, use that instead for backwards compatibility - mxid_source_attribute = saml2_config.get("mxid_source_attribute") - if mxid_source_attribute: - mapping_provider_config["mxid_source_attribute"] = mxid_source_attribute + old_mxid_source_attribute = saml2_config.get("mxid_source_attribute") + if old_mxid_source_attribute: + mapping_provider_config["mxid_source_attribute"] = old_mxid_source_attribute + else: + mapping_provider_config.setdefault("mxid_source_attribute", "uid") # If mxid_mapping is defined, use that instead for backwards compatibility - mxid_mapping = saml2_config.get("mxid_mapping") - if mxid_mapping: - mapping_provider_config["mxid_mapping"] = mxid_mapping + old_mxid_mapping = saml2_config.get("mxid_source_attribute") + if old_mxid_mapping: + mapping_provider_config["mxid_mapping"] = old_mxid_mapping + else: + mapping_provider_config.setdefault("mxid_mapping", "hexencode") self.saml2_mxid_source_attribute = mapping_provider_config[ "mxid_source_attribute" diff --git a/synapse/handlers/saml_handler.py b/synapse/handlers/saml_handler.py index ddd38edf1191..32737b66f8c5 100644 --- a/synapse/handlers/saml_handler.py +++ b/synapse/handlers/saml_handler.py @@ -297,8 +297,13 @@ def saml_response_to_user_attributes( "displayname": displayname, } - def parse_config(self, config): - """Parse the dict provided by the homeserver config""" + @staticmethod + def parse_config(self, config: dict): + """Parse the dict provided by the homeserver's config + + Args: + config: A dictionary containing configuration options for this provider + """ self._mxid_source_attribute = config.get("mxid_source_attribute", "uid") mapping_type = config.get("mxid_mapper", "hexencode") From 0a42261c9b8253907ec07f484f7106480b0cf695 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Thu, 28 Nov 2019 16:42:47 +0000 Subject: [PATCH 13/35] Fix argument to load_module --- synapse/config/saml2_config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/config/saml2_config.py b/synapse/config/saml2_config.py index f0de0553f911..f3c7cccc37bc 100644 --- a/synapse/config/saml2_config.py +++ b/synapse/config/saml2_config.py @@ -99,7 +99,7 @@ def read_config(self, config, **kwargs): ] self.saml2_mapping_provider, _ = load_module( - {"mod_name": mapping_provider_module, "config": mapping_provider_config} + {"module": mapping_provider_module, "config": mapping_provider_config} ) saml2_config_dict = self._default_saml_config_dict() From 5f03ec039915f90828bec6684106b20f1a30b76d Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Thu, 28 Nov 2019 16:44:42 +0000 Subject: [PATCH 14/35] nostatic --- synapse/handlers/saml_handler.py | 1 - 1 file changed, 1 deletion(-) diff --git a/synapse/handlers/saml_handler.py b/synapse/handlers/saml_handler.py index 32737b66f8c5..967131626220 100644 --- a/synapse/handlers/saml_handler.py +++ b/synapse/handlers/saml_handler.py @@ -297,7 +297,6 @@ def saml_response_to_user_attributes( "displayname": displayname, } - @staticmethod def parse_config(self, config: dict): """Parse the dict provided by the homeserver's config From 81688f4222b6b9768f71f135563e378f8fff8404 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Thu, 28 Nov 2019 17:13:15 +0000 Subject: [PATCH 15/35] Remove parse_config method --- docs/saml_mapping_providers.md | 33 +++++++++++++++++----------- synapse/config/saml2_config.py | 16 ++++++++------ synapse/handlers/saml_handler.py | 37 +++++++++++--------------------- synapse/server.py | 2 +- synapse/util/module_loader.py | 14 +++++++----- 5 files changed, 52 insertions(+), 50 deletions(-) diff --git a/docs/saml_mapping_providers.md b/docs/saml_mapping_providers.md index b7c200caabcb..9c27fb466f16 100644 --- a/docs/saml_mapping_providers.md +++ b/docs/saml_mapping_providers.md @@ -25,19 +25,26 @@ what options it provides (if any). ## Building a Custom Mapping Provider -A custom mapping provider must specify the following methods: - -* `saml_response_to_user_attributes` - - Receives the `type` object from a SAML response. This method must return a - dictionary, which will then be used by Synapse to build a new user. The - following keys are allowed: - * displayname (required) - * user_id_localpart (required) - * avatar_url -* `parse_config(config: dict)` - - Receives the parsed content of the `saml2_config.user_mapping_provider` - homeserver config option. Runs on homeserver startup. Providers should - extract any option values they need here. +A custom mapping provider must specify the following method: + +* `saml_response_to_user_attributes(self, saml_response, config, failures)` + - Arguments: + - `saml_response` - A `saml2.response.AuthnResponse` object to extract user + information from. + - `config` - A dictionary with the contents of the homeserver config's + `saml2_config.user_mapping_provider_config` option. + - `failures` - 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 `john.doe` as the value of + `mxid_localpart` in the returned dict, and that is already taken + on the homeserver, this method will be called again with the same + parameters but with failures=1. The method should then return a + different `mxid_localpart` value, such as `john.doe1`. + - This method must return a dictionary, which will then be used by Synapse + 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. ## Synapse's Default Provider diff --git a/synapse/config/saml2_config.py b/synapse/config/saml2_config.py index f3c7cccc37bc..517c27e075be 100644 --- a/synapse/config/saml2_config.py +++ b/synapse/config/saml2_config.py @@ -78,28 +78,30 @@ def read_config(self, config, **kwargs): "user_mapping_provider", "synapse.handlers.saml_handler.DefaultSamlMappingProvider", ) - mapping_provider_config = config.get("user_mapping_provider_config", {}) + self.mapping_provider_config = config.get("user_mapping_provider_config", {}) # If mxid_source_attribute is defined, use that instead for backwards compatibility old_mxid_source_attribute = saml2_config.get("mxid_source_attribute") if old_mxid_source_attribute: - mapping_provider_config["mxid_source_attribute"] = old_mxid_source_attribute + self.mapping_provider_config[ + "mxid_source_attribute" + ] = old_mxid_source_attribute else: - mapping_provider_config.setdefault("mxid_source_attribute", "uid") + self.mapping_provider_config.setdefault("mxid_source_attribute", "uid") # If mxid_mapping is defined, use that instead for backwards compatibility old_mxid_mapping = saml2_config.get("mxid_source_attribute") if old_mxid_mapping: - mapping_provider_config["mxid_mapping"] = old_mxid_mapping + self.mapping_provider_config["mxid_mapping"] = old_mxid_mapping else: - mapping_provider_config.setdefault("mxid_mapping", "hexencode") + self.mapping_provider_config.setdefault("mxid_mapping", "hexencode") - self.saml2_mxid_source_attribute = mapping_provider_config[ + self.saml2_mxid_source_attribute = self.mapping_provider_config[ "mxid_source_attribute" ] self.saml2_mapping_provider, _ = load_module( - {"module": mapping_provider_module, "config": mapping_provider_config} + {"module": mapping_provider_module} ) saml2_config_dict = self._default_saml_config_dict() diff --git a/synapse/handlers/saml_handler.py b/synapse/handlers/saml_handler.py index 967131626220..e69a5bc869c2 100644 --- a/synapse/handlers/saml_handler.py +++ b/synapse/handlers/saml_handler.py @@ -142,7 +142,7 @@ async def _map_saml_response_to_user(self, resp_bytes): for i in range(1000): try: attribute_dict = self._mapping_provider.saml_response_to_user_attributes( - saml2_auth, i + self.saml2_mapping_provider_config, saml2_auth, i ) except Exception: logging.exception("Error in SAML mapping provider plugin") @@ -252,16 +252,17 @@ def dot_replace_for_mxid(username: str) -> str: class DefaultSamlMappingProvider(object): __version__ = "0.0.1" - def __init__(self): - self._mxid_source_attribute = None - self._mxid_mapper = None - def saml_response_to_user_attributes( - self, saml_response: saml2.response.AuthnResponse, failures: int = 0, + self, + config: dict, + 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 @@ -273,17 +274,19 @@ def saml_response_to_user_attributes( * displayname (str): The displayname of the user """ try: - mxid_source = saml_response.ava[self._mxid_source_attribute][0] + mxid_source = saml_response.ava[config["mxid_source_attribute"]][0] except KeyError: logger.warning( - "SAML2 response lacks a '%s' attestation", self._mxid_source_attribute + "SAML2 response lacks a '%s' attestation", + config["mxid_source_attribute"], ) raise SynapseError( - 400, "%s not in SAML2 response" % (self._mxid_source_attribute,) + 400, "%s not in SAML2 response" % (config["_mxid_source_attribute"],) ) # Use the configured mapper for this mxid_source - base_mxid_localpart = self._mxid_mapper(mxid_source) + mxid_mapper = MXID_MAPPER_MAP[config["mxid_mapping"]] + base_mxid_localpart = mxid_mapper(mxid_source) # Append suffix integer if last call to this function failed to produce # a usable mxid @@ -296,17 +299,3 @@ def saml_response_to_user_attributes( "mxid_localpart": localpart, "displayname": displayname, } - - def parse_config(self, config: dict): - """Parse the dict provided by the homeserver's config - - Args: - config: A dictionary containing configuration options for this provider - """ - self._mxid_source_attribute = config.get("mxid_source_attribute", "uid") - - mapping_type = config.get("mxid_mapper", "hexencode") - try: - self._mxid_mapper = MXID_MAPPER_MAP[mapping_type] - except KeyError: - raise Exception("%s is not a known mxid_mapping" % (mapping_type,)) diff --git a/synapse/server.py b/synapse/server.py index 90c3b072e8d3..bd0d1a35c732 100644 --- a/synapse/server.py +++ b/synapse/server.py @@ -543,7 +543,7 @@ def build_registration_handler(self): def build_account_validity_handler(self): return AccountValidityHandler(self) - def build_saml_handler(self): + def build_saml_handler(self,): from synapse.handlers.saml_handler import SamlHandler return SamlHandler(self) diff --git a/synapse/util/module_loader.py b/synapse/util/module_loader.py index 2705cbe5f892..e3e65894f465 100644 --- a/synapse/util/module_loader.py +++ b/synapse/util/module_loader.py @@ -32,11 +32,15 @@ def load_module(provider): module, clz = provider["module"].rsplit(".", 1) module = importlib.import_module(module) provider_class = getattr(module, clz) - - 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)) + provider_config = provider.get("config") + + if provider_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) + ) return provider_class, provider_config From e7cb32e61947d30d4446186c0a852bdff008f999 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Thu, 28 Nov 2019 17:18:06 +0000 Subject: [PATCH 16/35] Fix reference to user mapping provider config --- synapse/config/saml2_config.py | 18 ++++++++++++------ synapse/handlers/saml_handler.py | 5 ++++- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/synapse/config/saml2_config.py b/synapse/config/saml2_config.py index 517c27e075be..129e82a25636 100644 --- a/synapse/config/saml2_config.py +++ b/synapse/config/saml2_config.py @@ -78,25 +78,31 @@ def read_config(self, config, **kwargs): "user_mapping_provider", "synapse.handlers.saml_handler.DefaultSamlMappingProvider", ) - self.mapping_provider_config = config.get("user_mapping_provider_config", {}) + self.saml2_user_mapping_provider_config = config.get( + "user_mapping_provider_config", {} + ) # If mxid_source_attribute is defined, use that instead for backwards compatibility old_mxid_source_attribute = saml2_config.get("mxid_source_attribute") if old_mxid_source_attribute: - self.mapping_provider_config[ + self.saml2_user_mapping_provider_config[ "mxid_source_attribute" ] = old_mxid_source_attribute else: - self.mapping_provider_config.setdefault("mxid_source_attribute", "uid") + self.saml2_user_mapping_provider_config.setdefault( + "mxid_source_attribute", "uid" + ) # If mxid_mapping is defined, use that instead for backwards compatibility old_mxid_mapping = saml2_config.get("mxid_source_attribute") if old_mxid_mapping: - self.mapping_provider_config["mxid_mapping"] = old_mxid_mapping + self.saml2_user_mapping_provider_config["mxid_mapping"] = old_mxid_mapping else: - self.mapping_provider_config.setdefault("mxid_mapping", "hexencode") + self.saml2_user_mapping_provider_config.setdefault( + "mxid_mapping", "hexencode" + ) - self.saml2_mxid_source_attribute = self.mapping_provider_config[ + self.saml2_mxid_source_attribute = self.saml2_user_mapping_provider_config[ "mxid_source_attribute" ] diff --git a/synapse/handlers/saml_handler.py b/synapse/handlers/saml_handler.py index e69a5bc869c2..03df9d6739f4 100644 --- a/synapse/handlers/saml_handler.py +++ b/synapse/handlers/saml_handler.py @@ -55,6 +55,9 @@ def __init__(self, hs): self._grandfathered_mxid_source_attribute = ( hs.config.saml2_grandfathered_mxid_source_attribute ) + self._saml2_user_mapping_provider_config = ( + hs.config.saml2_user_mapping_provider_config + ) # plugin to do custom mapping from saml response to mxid self._mapping_provider = hs.config.saml2_mapping_provider @@ -142,7 +145,7 @@ async def _map_saml_response_to_user(self, resp_bytes): for i in range(1000): try: attribute_dict = self._mapping_provider.saml_response_to_user_attributes( - self.saml2_mapping_provider_config, saml2_auth, i + self._saml2_user_mapping_provider_config, saml2_auth, i ) except Exception: logging.exception("Error in SAML mapping provider plugin") From e892842e22486867f1cb858b2d24d2a8b1048481 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Thu, 28 Nov 2019 17:24:09 +0000 Subject: [PATCH 17/35] Pull user mapping provider module from the right place --- docs/saml_mapping_providers.md | 4 ++-- synapse/config/saml2_config.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/saml_mapping_providers.md b/docs/saml_mapping_providers.md index 9c27fb466f16..0085b06f745b 100644 --- a/docs/saml_mapping_providers.md +++ b/docs/saml_mapping_providers.md @@ -29,10 +29,10 @@ A custom mapping provider must specify the following method: * `saml_response_to_user_attributes(self, saml_response, config, failures)` - Arguments: - - `saml_response` - A `saml2.response.AuthnResponse` object to extract user - information from. - `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` - 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. diff --git a/synapse/config/saml2_config.py b/synapse/config/saml2_config.py index 129e82a25636..2e3b89185326 100644 --- a/synapse/config/saml2_config.py +++ b/synapse/config/saml2_config.py @@ -78,7 +78,7 @@ def read_config(self, config, **kwargs): "user_mapping_provider", "synapse.handlers.saml_handler.DefaultSamlMappingProvider", ) - self.saml2_user_mapping_provider_config = config.get( + self.saml2_user_mapping_provider_config = saml2_config.get( "user_mapping_provider_config", {} ) From 169d369daa74e4a6cd92ba9f2684893dde575598 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Thu, 28 Nov 2019 17:27:00 +0000 Subject: [PATCH 18/35] Create an instance of the module's class first --- synapse/config/saml2_config.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/synapse/config/saml2_config.py b/synapse/config/saml2_config.py index 2e3b89185326..20015d6f484f 100644 --- a/synapse/config/saml2_config.py +++ b/synapse/config/saml2_config.py @@ -106,9 +106,10 @@ def read_config(self, config, **kwargs): "mxid_source_attribute" ] - self.saml2_mapping_provider, _ = load_module( + user_mapping_provider_class, _ = load_module( {"module": mapping_provider_module} ) + self.saml2_mapping_provider = user_mapping_provider_class() saml2_config_dict = self._default_saml_config_dict() _dict_merge( From c0c75e5a3105320041a1c4d9018459e6e3796165 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Fri, 29 Nov 2019 12:00:24 +0000 Subject: [PATCH 19/35] Cleanup and fix var names --- docs/saml_mapping_providers.md | 21 +++++++++++---------- synapse/config/saml2_config.py | 29 ++++++++++++++++++----------- synapse/handlers/saml_handler.py | 4 ++-- 3 files changed, 31 insertions(+), 23 deletions(-) diff --git a/docs/saml_mapping_providers.md b/docs/saml_mapping_providers.md index 0085b06f745b..331d33b44fbe 100644 --- a/docs/saml_mapping_providers.md +++ b/docs/saml_mapping_providers.md @@ -10,11 +10,11 @@ As an example, a SSO service may return the email address to turn that into a displayname when creating a Matrix user for this individual. It may choose `John Smith`, or `Smith, John [Example.com]` or any number of variations. As each Synapse configuration may want something different, this is -where SAML mapping providers come in. +where SAML mapping providers come into play. ## Enabling Providers -External mapping providers are provided to Synapse in the form of a external +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. @@ -33,14 +33,15 @@ A custom mapping provider must specify the following method: `saml2_config.user_mapping_provider_config` option. - `saml_response` - A `saml2.response.AuthnResponse` object to extract user information from. - - `failures` - 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 `john.doe` as the value of - `mxid_localpart` in the returned dict, and that is already taken - on the homeserver, this method will be called again with the same - parameters but with failures=1. The method should then return a - different `mxid_localpart` value, such as `john.doe1`. + - `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 + `john.doe` as the value of `mxid_localpart` in the returned + dict, and that is already taken on the homeserver, this + method will be called again with the same parameters but + with failures=1. The method should then return a different + `mxid_localpart` value, such as `john.doe1`. - This method must return a dictionary, which will then be used by Synapse to build a new user. The following keys are allowed: * `mxid_localpart` - Required. The mxid localpart of the new user. diff --git a/synapse/config/saml2_config.py b/synapse/config/saml2_config.py index 20015d6f484f..dcab8a701faf 100644 --- a/synapse/config/saml2_config.py +++ b/synapse/config/saml2_config.py @@ -78,38 +78,45 @@ def read_config(self, config, **kwargs): "user_mapping_provider", "synapse.handlers.saml_handler.DefaultSamlMappingProvider", ) - self.saml2_user_mapping_provider_config = saml2_config.get( + saml2_user_mapping_provider_config = saml2_config.get( "user_mapping_provider_config", {} ) - # If mxid_source_attribute is defined, use that instead for backwards compatibility + # If mxid_source_attribute is defined in the deprecated location, use + # that instead for backwards compatibility old_mxid_source_attribute = saml2_config.get("mxid_source_attribute") if old_mxid_source_attribute: - self.saml2_user_mapping_provider_config[ + saml2_user_mapping_provider_config[ "mxid_source_attribute" ] = old_mxid_source_attribute else: - self.saml2_user_mapping_provider_config.setdefault( + # 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 + saml2_user_mapping_provider_config.setdefault( "mxid_source_attribute", "uid" ) # If mxid_mapping is defined, use that instead for backwards compatibility - old_mxid_mapping = saml2_config.get("mxid_source_attribute") + old_mxid_mapping = saml2_config.get("mxid_mapping") if old_mxid_mapping: - self.saml2_user_mapping_provider_config["mxid_mapping"] = old_mxid_mapping + saml2_user_mapping_provider_config["mxid_mapping"] = old_mxid_mapping else: - self.saml2_user_mapping_provider_config.setdefault( - "mxid_mapping", "hexencode" - ) + # 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 + saml2_user_mapping_provider_config.setdefault("mxid_mapping", "hexencode") - self.saml2_mxid_source_attribute = self.saml2_user_mapping_provider_config[ + # Some places in Synapse need this value directly + self.saml2_mxid_source_attribute = saml2_user_mapping_provider_config[ "mxid_source_attribute" ] + # We don't use nor provide the module's config here, and instead make it available + # under the hs' config object user_mapping_provider_class, _ = load_module( {"module": mapping_provider_module} ) - self.saml2_mapping_provider = user_mapping_provider_class() + self.saml2_user_mapping_provider = user_mapping_provider_class() + self.saml2_user_mapping_provider_config = saml2_user_mapping_provider_config saml2_config_dict = self._default_saml_config_dict() _dict_merge( diff --git a/synapse/handlers/saml_handler.py b/synapse/handlers/saml_handler.py index 03df9d6739f4..3b5687a642ab 100644 --- a/synapse/handlers/saml_handler.py +++ b/synapse/handlers/saml_handler.py @@ -60,7 +60,7 @@ def __init__(self, hs): ) # plugin to do custom mapping from saml response to mxid - self._mapping_provider = hs.config.saml2_mapping_provider + self._user_mapping_provider = hs.config.saml2_user_mapping_provider # identifier for the external_ids table self._auth_provider_id = "saml" @@ -144,7 +144,7 @@ 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._mapping_provider.saml_response_to_user_attributes( + attribute_dict = self._user_mapping_provider.saml_response_to_user_attributes( self._saml2_user_mapping_provider_config, saml2_auth, i ) except Exception: From 27b5f0f78fd1ed53ca91b68b30292fc1f08d22e0 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Fri, 29 Nov 2019 13:29:03 +0000 Subject: [PATCH 20/35] Remove comma --- synapse/server.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/server.py b/synapse/server.py index bd0d1a35c732..90c3b072e8d3 100644 --- a/synapse/server.py +++ b/synapse/server.py @@ -543,7 +543,7 @@ def build_registration_handler(self): def build_account_validity_handler(self): return AccountValidityHandler(self) - def build_saml_handler(self,): + def build_saml_handler(self): from synapse.handlers.saml_handler import SamlHandler return SamlHandler(self) From aac1ab5cb5c8b99d531913b12550cb7598145c3b Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 4 Dec 2019 11:26:53 +0000 Subject: [PATCH 21/35] Deprecation warning, module config, cleanup and processing reordering --- docs/sample_config.yaml | 77 ++++++++------- synapse/config/saml2_config.py | 157 +++++++++++++++++-------------- synapse/handlers/saml_handler.py | 109 ++++++++++++--------- synapse/util/module_loader.py | 11 +-- 4 files changed, 200 insertions(+), 154 deletions(-) diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index 88ade4ff9035..4a68cd94662c 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -1186,49 +1186,58 @@ saml2_config: # #config_path: "CONFDIR/sp_conf.py" - # the lifetime of a SAML session. This defines how long a user has to + # The lifetime of a SAML session. This defines how long a user has to # complete the authentication process, if allow_unsolicited is unset. # The default is 5 minutes. # #saml_session_lifetime: 5m - # An external module can be provided here as a custom solution to mapping the - # above configured saml attribute onto a matrix ID. If this is defined, it will - # override the `mxid_mapping` option. + # An external module can be provided here as a custom solution to + # mapping the above configured saml attribute onto a matrix ID. # - #user_mapping_provider: mapping_provider.SamlMappingProvider + user_mapping_provider: + # The custom module's class. Uncomment to use a custom module. + # + #module: mapping_provider.SamlMappingProvider - # Custom configuration values for the module. This will be passed as a - # Python dictionary to the module's `parse_config` method. - # - #user_mapping_provider_config: - # # The SAML attribute (after mapping via the attribute maps) to use to derive - # # the Matrix ID from. 'uid' by default. - # # - # # Note: This used to be configured by the saml2_config.mxid_source_attribute - # # option. If that is still defined, its value will be used instead. - # # - # #mxid_source_attribute: displayName - # - # # The mapping system to use for mapping the saml attribute onto a matrix ID. - # # Options include: - # # * 'hexencode' (which maps unpermitted characters to '=xx') - # # * 'dotreplace' (which replaces unpermitted characters with '.'). - # # The default is 'hexencode'. - # # - # # Note: This used to be configured by the saml2_config.mxid_mapping - # # option. If that is still defined, its value will be used instead. - # # - # #mxid_mapping: dotreplace - - # In previous versions of synapse, the mapping from SAML attribute to MXID was - # always calculated dynamically rather than stored in a table. For backwards- - # compatibility, we will look for user_ids matching such a pattern before - # creating a new account. + # Custom configuration values for the module. Below options are + # intended for the built-in provider, they should be changed if + # using a custom module. This section will be passed as a Python + # dictionary to the module's `parse_config` method. + # + config: + # The SAML attribute (after mapping via the attribute maps) to use + # to derive the Matrix ID from. 'uid' by default. + # + # Note: This used to be configured by the + # saml2_config.mxid_source_attribute option. If that is still + # defined, its value will be used instead. + # + #mxid_source_attribute: displayName + + # The mapping system to use for mapping the saml attribute onto a + # matrix ID. + # + # Options include: + # * 'hexencode' (which maps unpermitted characters to '=xx') + # * 'dotreplace' (which replaces unpermitted characters with + # '.'). + # The default is 'hexencode'. + # + # Note: This used to be configured by the + # saml2_config.mxid_mapping option. If that is still defined, its + # value will be used instead. + # + #mxid_mapping: dotreplace + + # In previous versions of synapse, the mapping from SAML attribute to + # MXID was always calculated dynamically rather than stored in a + # table. For backwards- compatibility, we will look for user_ids + # matching such a pattern before creating a new account. # # This setting controls the SAML attribute which will be used for this - # backwards-compatibility lookup. Typically it should be 'uid', but if the - # attribute maps are changed, it may be necessary to change it. + # backwards-compatibility lookup. Typically it should be 'uid', but if + # the attribute maps are changed, it may be necessary to change it. # # The default is 'uid'. # diff --git a/synapse/config/saml2_config.py b/synapse/config/saml2_config.py index dcab8a701faf..00bfd65e08ad 100644 --- a/synapse/config/saml2_config.py +++ b/synapse/config/saml2_config.py @@ -73,50 +73,58 @@ def read_config(self, config, **kwargs): "grandfathered_mxid_source_attribute", "uid" ) + user_mapping_provider_dict = saml2_config.get("user_mapping_provider") + if user_mapping_provider_dict is None: + user_mapping_provider_dict = {} + # Load configured module class and config - mapping_provider_module = saml2_config.get( - "user_mapping_provider", - "synapse.handlers.saml_handler.DefaultSamlMappingProvider", + default_mapping_provider = ( + "synapse.handlers.saml_handler.DefaultSamlMappingProvider" ) - saml2_user_mapping_provider_config = saml2_config.get( - "user_mapping_provider_config", {} + user_mapping_provider_module = user_mapping_provider_dict.get( + "module", default_mapping_provider, ) - - # If mxid_source_attribute is defined in the deprecated location, use - # that instead for backwards compatibility - old_mxid_source_attribute = saml2_config.get("mxid_source_attribute") - if old_mxid_source_attribute: - saml2_user_mapping_provider_config[ - "mxid_source_attribute" - ] = old_mxid_source_attribute - 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 - saml2_user_mapping_provider_config.setdefault( - "mxid_source_attribute", "uid" - ) - - # If mxid_mapping is defined, use that instead for backwards compatibility - old_mxid_mapping = saml2_config.get("mxid_mapping") - if old_mxid_mapping: - saml2_user_mapping_provider_config["mxid_mapping"] = old_mxid_mapping - 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 - saml2_user_mapping_provider_config.setdefault("mxid_mapping", "hexencode") + 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 + + # If mxid_source_attribute is defined in the deprecated location, use + # that instead for backwards compatibility + old_mxid_source_attribute = saml2_config.get("mxid_source_attribute") + if old_mxid_source_attribute: + self.using_old_mxid_source_attribute = True + user_mapping_provider_config[ + "mxid_source_attribute" + ] = old_mxid_source_attribute + 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 + old_mxid_mapping = saml2_config.get("mxid_mapping") + if old_mxid_mapping: + self.using_old_mxid_mapping = True + user_mapping_provider_config["mxid_mapping"] = old_mxid_mapping + 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") # Some places in Synapse need this value directly - self.saml2_mxid_source_attribute = saml2_user_mapping_provider_config[ + # TODO: This isn't going to be a thing without the default module + self.saml2_mxid_source_attribute = user_mapping_provider_config[ "mxid_source_attribute" ] - # We don't use nor provide the module's config here, and instead make it available - # under the hs' config object - user_mapping_provider_class, _ = load_module( - {"module": mapping_provider_module} + # We don't use nor provide the module's config here + self.saml2_user_mapping_provider_class, _ = load_module( + { + "module": user_mapping_provider_module, + "config": user_mapping_provider_config, + } ) - self.saml2_user_mapping_provider = user_mapping_provider_class() - self.saml2_user_mapping_provider_config = saml2_user_mapping_provider_config saml2_config_dict = self._default_saml_config_dict() _dict_merge( @@ -236,49 +244,58 @@ def generate_config_section(self, config_dir_path, server_name, **kwargs): # #config_path: "%(config_dir_path)s/sp_conf.py" - # the lifetime of a SAML session. This defines how long a user has to + # The lifetime of a SAML session. This defines how long a user has to # complete the authentication process, if allow_unsolicited is unset. # The default is 5 minutes. # #saml_session_lifetime: 5m - # An external module can be provided here as a custom solution to mapping the - # above configured saml attribute onto a matrix ID. If this is defined, it will - # override the `mxid_mapping` option. - # - #user_mapping_provider: mapping_provider.SamlMappingProvider - - # Custom configuration values for the module. This will be passed as a - # Python dictionary to the module's `parse_config` method. - # - #user_mapping_provider_config: - # # The SAML attribute (after mapping via the attribute maps) to use to derive - # # the Matrix ID from. 'uid' by default. - # # - # # Note: This used to be configured by the saml2_config.mxid_source_attribute - # # option. If that is still defined, its value will be used instead. - # # - # #mxid_source_attribute: displayName + # An external module can be provided here as a custom solution to + # mapping the above configured saml attribute onto a matrix ID. # - # # The mapping system to use for mapping the saml attribute onto a matrix ID. - # # Options include: - # # * 'hexencode' (which maps unpermitted characters to '=xx') - # # * 'dotreplace' (which replaces unpermitted characters with '.'). - # # The default is 'hexencode'. - # # - # # Note: This used to be configured by the saml2_config.mxid_mapping - # # option. If that is still defined, its value will be used instead. - # # - # #mxid_mapping: dotreplace - - # In previous versions of synapse, the mapping from SAML attribute to MXID was - # always calculated dynamically rather than stored in a table. For backwards- - # compatibility, we will look for user_ids matching such a pattern before - # creating a new account. + user_mapping_provider: + # The custom module's class. Uncomment to use a custom module. + # + #module: mapping_provider.SamlMappingProvider + + # Custom configuration values for the module. Below options are + # intended for the built-in provider, they should be changed if + # using a custom module. This section will be passed as a Python + # dictionary to the module's `parse_config` method. + # + config: + # The SAML attribute (after mapping via the attribute maps) to use + # to derive the Matrix ID from. 'uid' by default. + # + # Note: This used to be configured by the + # saml2_config.mxid_source_attribute option. If that is still + # defined, its value will be used instead. + # + #mxid_source_attribute: displayName + + # The mapping system to use for mapping the saml attribute onto a + # matrix ID. + # + # Options include: + # * 'hexencode' (which maps unpermitted characters to '=xx') + # * 'dotreplace' (which replaces unpermitted characters with + # '.'). + # The default is 'hexencode'. + # + # Note: This used to be configured by the + # saml2_config.mxid_mapping option. If that is still defined, its + # value will be used instead. + # + #mxid_mapping: dotreplace + + # In previous versions of synapse, the mapping from SAML attribute to + # MXID was always calculated dynamically rather than stored in a + # table. For backwards- compatibility, we will look for user_ids + # matching such a pattern before creating a new account. # # This setting controls the SAML attribute which will be used for this - # backwards-compatibility lookup. Typically it should be 'uid', but if the - # attribute maps are changed, it may be necessary to change it. + # backwards-compatibility lookup. Typically it should be 'uid', but if + # the attribute maps are changed, it may be necessary to change it. # # The default is 'uid'. # diff --git a/synapse/handlers/saml_handler.py b/synapse/handlers/saml_handler.py index 3b5687a642ab..941a4cae31b9 100644 --- a/synapse/handlers/saml_handler.py +++ b/synapse/handlers/saml_handler.py @@ -60,7 +60,9 @@ def __init__(self, hs): ) # plugin to do custom mapping from saml response to mxid - self._user_mapping_provider = hs.config.saml2_user_mapping_provider + self._user_mapping_provider = hs.config.saml2_user_mapping_provider_class( + hs.config.saml2_user_mapping_provider_config + ) # identifier for the external_ids table self._auth_provider_id = "saml" @@ -71,6 +73,22 @@ 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 @@ -137,42 +155,10 @@ async def _map_saml_response_to_user(self, resp_bytes): remote_user_id = saml2_auth.ava["uid"][0] except KeyError: logger.warning("SAML2 response lacks a 'uid' attestation") - raise SynapseError(400, "uid not in SAML2 response") + raise SynapseError(400, "'uid' not in SAML2 response") self._outstanding_requests_dict.pop(saml2_auth.in_response_to, None) - # 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( - self._saml2_user_mapping_provider_config, saml2_auth, i - ) - except Exception: - logging.exception("Error in SAML mapping provider plugin") - raise SynapseError(500, "Error parsing SAML2 response") - - localpart = attribute_dict.get("mxid_localpart") - if not localpart: - logger.error( - "SAML mapping provider plugin did not return a mxid_localpart object" - ) - raise SynapseError(500, "Error parsing SAML2 response") - - displayname = attribute_dict.get("displayname") - - # Check if this mxid already exists - if not await self._datastore.get_users_by_id_case_insensitive( - UserID(localpart, self._hostname).to_string() - ): - # This mxid is free - break - else: - # Unable to generate a username in 1000 iterations - # Break and return error to the user - raise SynapseError( - 500, "Unable to generate a Matrix ID from the 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 logger.info( @@ -212,6 +198,38 @@ async def _map_saml_response_to_user(self, resp_bytes): ) return registered_user_id + # 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 + + localpart = attribute_dict.get("mxid_localpart") + if not localpart: + logger.error( + "SAML mapping provider plugin did not return a mxid_localpart object" + ) + raise SynapseError(500, "Error parsing SAML2 response") + + displayname = attribute_dict.get("displayname") + + # Check if this mxid already exists + if not await self._datastore.get_users_by_id_case_insensitive( + UserID(localpart, self._hostname).to_string() + ): + # This mxid is free + break + else: + # Unable to generate a username in 1000 iterations + # Break and return error to the user + raise SynapseError( + 500, "Unable to generate a Matrix ID from the SAML response" + ) + registered_user_id = await self._registration_handler.register_user( localpart=localpart, default_display_name=displayname ) @@ -255,11 +273,17 @@ def dot_replace_for_mxid(username: str) -> str: class DefaultSamlMappingProvider(object): __version__ = "0.0.1" + def __init__(self, config: dict): + """The default SAML user mapping provider + + Args: + config: Module configuration dictionary + """ + self._mxid_source_attribute = config["mxid_source_attribute"] + self._mxid_mapping = config["mxid_mapping"] + def saml_response_to_user_attributes( - self, - config: dict, - saml_response: saml2.response.AuthnResponse, - failures: int = 0, + self, saml_response: saml2.response.AuthnResponse, failures: int = 0, ) -> dict: """Maps some text from a SAML response to attributes of a new user @@ -277,18 +301,17 @@ def saml_response_to_user_attributes( * displayname (str): The displayname of the user """ try: - mxid_source = saml_response.ava[config["mxid_source_attribute"]][0] + mxid_source = saml_response.ava[self._mxid_source_attribute][0] except KeyError: logger.warning( - "SAML2 response lacks a '%s' attestation", - config["mxid_source_attribute"], + "SAML2 response lacks a '%s' attestation", self._mxid_source_attribute, ) raise SynapseError( - 400, "%s not in SAML2 response" % (config["_mxid_source_attribute"],) + 400, "%s not in SAML2 response" % (self._mxid_source_attribute,) ) # Use the configured mapper for this mxid_source - mxid_mapper = MXID_MAPPER_MAP[config["mxid_mapping"]] + mxid_mapper = MXID_MAPPER_MAP[self._mxid_mapping] base_mxid_localpart = mxid_mapper(mxid_source) # Append suffix integer if last call to this function failed to produce diff --git a/synapse/util/module_loader.py b/synapse/util/module_loader.py index e3e65894f465..46c2e311bf99 100644 --- a/synapse/util/module_loader.py +++ b/synapse/util/module_loader.py @@ -34,13 +34,10 @@ def load_module(provider): provider_class = getattr(module, clz) provider_config = provider.get("config") - if provider_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 From 1d0f2b2f6f410bf760eaf90dc647262c0fd35a19 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 4 Dec 2019 15:19:34 +0000 Subject: [PATCH 22/35] Small fixes. Remove hs.config.saml2_mxid_source_attribute --- synapse/config/saml2_config.py | 21 ++++++++------------- synapse/handlers/saml_handler.py | 1 - synapse/util/module_loader.py | 13 ++++++++----- 3 files changed, 16 insertions(+), 19 deletions(-) diff --git a/synapse/config/saml2_config.py b/synapse/config/saml2_config.py index 00bfd65e08ad..9968ef3e711e 100644 --- a/synapse/config/saml2_config.py +++ b/synapse/config/saml2_config.py @@ -93,10 +93,12 @@ def read_config(self, config, **kwargs): # that instead for backwards compatibility old_mxid_source_attribute = saml2_config.get("mxid_source_attribute") if old_mxid_source_attribute: - self.using_old_mxid_source_attribute = True 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 @@ -105,25 +107,18 @@ def read_config(self, config, **kwargs): # If mxid_mapping is defined, use that instead for backwards compatibility old_mxid_mapping = saml2_config.get("mxid_mapping") if old_mxid_mapping: - self.using_old_mxid_mapping = True 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") - # Some places in Synapse need this value directly - # TODO: This isn't going to be a thing without the default module - self.saml2_mxid_source_attribute = user_mapping_provider_config[ - "mxid_source_attribute" - ] - # We don't use nor provide the module's config here self.saml2_user_mapping_provider_class, _ = load_module( - { - "module": user_mapping_provider_module, - "config": user_mapping_provider_config, - } + {"module": user_mapping_provider_module} ) saml2_config_dict = self._default_saml_config_dict() @@ -153,7 +148,7 @@ def _default_saml_config_dict(self): if public_baseurl is None: raise ConfigError("saml2_config requires a public_baseurl to be set") - required_attributes = {"uid", self.saml2_mxid_source_attribute} + required_attributes = {"uid"} optional_attributes = {"displayName"} if self.saml2_grandfathered_mxid_source_attribute: diff --git a/synapse/handlers/saml_handler.py b/synapse/handlers/saml_handler.py index 941a4cae31b9..ab2b60f2f450 100644 --- a/synapse/handlers/saml_handler.py +++ b/synapse/handlers/saml_handler.py @@ -51,7 +51,6 @@ def __init__(self, hs): self._datastore = hs.get_datastore() self._hostname = hs.hostname self._saml2_session_lifetime = hs.config.saml2_session_lifetime - self._mxid_source_attribute = hs.config.saml2_mxid_source_attribute self._grandfathered_mxid_source_attribute = ( hs.config.saml2_grandfathered_mxid_source_attribute ) diff --git a/synapse/util/module_loader.py b/synapse/util/module_loader.py index 46c2e311bf99..2f0ae679c651 100644 --- a/synapse/util/module_loader.py +++ b/synapse/util/module_loader.py @@ -25,7 +25,7 @@ def load_module(provider): (the config dict). Returns - Tuple of (provider class, parsed config object) + Tuple of (provider class, parsed config object|None, if a config was not passed) """ # We need to import the module, and then pick the class out of # that, so we split based on the last dot. @@ -34,10 +34,13 @@ def load_module(provider): provider_class = getattr(module, clz) provider_config = 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)) + 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) + ) return provider_class, provider_config From ba07fc235413e570ae9635f601ea2ee1a978fe41 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Fri, 6 Dec 2019 11:16:01 +0000 Subject: [PATCH 23/35] Non-functional logging change. Doc update --- docs/saml_mapping_providers.md | 14 +++++++------- synapse/config/saml2_config.py | 17 ++++++++++++++++- synapse/handlers/saml_handler.py | 26 +++----------------------- 3 files changed, 26 insertions(+), 31 deletions(-) diff --git a/docs/saml_mapping_providers.md b/docs/saml_mapping_providers.md index 331d33b44fbe..4ce1b6269b84 100644 --- a/docs/saml_mapping_providers.md +++ b/docs/saml_mapping_providers.md @@ -16,21 +16,21 @@ 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: -* `saml_response_to_user_attributes(self, saml_response, config, failures)` +* `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 diff --git a/synapse/config/saml2_config.py b/synapse/config/saml2_config.py index 9968ef3e711e..3fcfdca0461b 100644 --- a/synapse/config/saml2_config.py +++ b/synapse/config/saml2_config.py @@ -17,6 +17,11 @@ from synapse.python_dependencies import DependencyException, check_requirements from synapse.util.module_loader import load_module, load_python_module +import logging + +logger = logging.getLogger(__name__) + + from ._base import Config, ConfigError @@ -84,8 +89,8 @@ def read_config(self, config, **kwargs): user_mapping_provider_module = user_mapping_provider_dict.get( "module", default_mapping_provider, ) + logger.warning("hiiiiiii") 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 @@ -93,6 +98,11 @@ def read_config(self, config, **kwargs): # that instead for backwards compatibility old_mxid_source_attribute = saml2_config.get("mxid_source_attribute") if 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." + ) user_mapping_provider_config[ "mxid_source_attribute" ] = old_mxid_source_attribute @@ -107,6 +117,11 @@ def read_config(self, config, **kwargs): # If mxid_mapping is defined, use that instead for backwards compatibility old_mxid_mapping = saml2_config.get("mxid_mapping") if 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." + ) + user_mapping_provider_config["mxid_mapping"] = old_mxid_mapping # Provide a deprecation warning diff --git a/synapse/handlers/saml_handler.py b/synapse/handlers/saml_handler.py index ab2b60f2f450..5e14a1b64b52 100644 --- a/synapse/handlers/saml_handler.py +++ b/synapse/handlers/saml_handler.py @@ -72,22 +72,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 +183,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: From 22a6b3cb156a1befdfe4acbe541ea7fceecb245d Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Fri, 6 Dec 2019 14:23:38 +0000 Subject: [PATCH 24/35] Provider returns req/opt attrs, pass config to the provider, logging on startup --- docs/saml_mapping_providers.md | 29 ++++++++- synapse/config/saml2_config.py | 101 +++++++++++++++++++------------ synapse/handlers/saml_handler.py | 61 +++++++++++++++++-- synapse/util/module_loader.py | 11 ++-- 4 files changed, 147 insertions(+), 55 deletions(-) diff --git a/docs/saml_mapping_providers.md b/docs/saml_mapping_providers.md index 4ce1b6269b84..bc66ebaa2584 100644 --- a/docs/saml_mapping_providers.md +++ b/docs/saml_mapping_providers.md @@ -27,13 +27,18 @@ 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: +* `__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: - `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 diff --git a/synapse/config/saml2_config.py b/synapse/config/saml2_config.py index 3fcfdca0461b..bfce16f4f8f5 100644 --- a/synapse/config/saml2_config.py +++ b/synapse/config/saml2_config.py @@ -14,17 +14,16 @@ # 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 -import logging +from ._base import Config, ConfigError logger = logging.getLogger(__name__) -from ._base import Config, ConfigError - - def _dict_merge(merge_dict, into_dict): """Do a deep merge of two dicts @@ -89,13 +88,40 @@ def read_config(self, config, **kwargs): user_mapping_provider_module = user_mapping_provider_dict.get( "module", default_mapping_provider, ) - logger.warning("hiiiiiii") 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 - # If mxid_source_attribute is defined in the deprecated location, use - # that instead for backwards compatibility + # 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 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: logger.warning( @@ -103,40 +129,25 @@ def read_config(self, config, **kwargs): "Please use saml2_config.user_mapping_provider.config" ".mxid_source_attribute instead." ) - 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 + 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: logger.warning( "The config option saml2_config.mxid_mapping is deprecated. Please use " "saml2_config.user_mapping_provider.config.mxid_mapping instead." ) - - 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} + 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 ) @@ -156,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 diff --git a/synapse/handlers/saml_handler.py b/synapse/handlers/saml_handler.py index 5e14a1b64b52..d4f583c83cb3 100644 --- a/synapse/handlers/saml_handler.py +++ b/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 ( @@ -252,14 +253,14 @@ 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, @@ -267,8 +268,6 @@ def saml_response_to_user_attributes( """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 @@ -304,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"} diff --git a/synapse/util/module_loader.py b/synapse/util/module_loader.py index 2f0ae679c651..48898a21c314 100644 --- a/synapse/util/module_loader.py +++ b/synapse/util/module_loader.py @@ -34,13 +34,10 @@ def load_module(provider): 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 From 9b85d7c559ebe8b088f3e4a23e349e91a8bf2e33 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Fri, 6 Dec 2019 14:36:01 +0000 Subject: [PATCH 25/35] Remove modifications to load_module --- synapse/util/module_loader.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/synapse/util/module_loader.py b/synapse/util/module_loader.py index 48898a21c314..c370be789585 100644 --- a/synapse/util/module_loader.py +++ b/synapse/util/module_loader.py @@ -25,17 +25,16 @@ 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") try: - provider_config = provider_class.parse_config(provider_config) + provider_config = provider_class.parse_config(provider["config"]) except Exception as e: raise ConfigError("Failed to parse config for %r: %r" % (provider["module"], e)) From dcc6fe9f8f9322185fd60b7b89e82c9037c41fed Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Mon, 9 Dec 2019 17:29:04 +0000 Subject: [PATCH 26/35] Address review comments --- docs/saml_mapping_providers.md | 7 +-- synapse/config/saml2_config.py | 81 +++++++++++++++----------------- synapse/handlers/saml_handler.py | 60 ++++++++++------------- synapse/util/module_loader.py | 2 +- 4 files changed, 66 insertions(+), 84 deletions(-) diff --git a/docs/saml_mapping_providers.md b/docs/saml_mapping_providers.md index bc66ebaa2584..619307b2e84c 100644 --- a/docs/saml_mapping_providers.md +++ b/docs/saml_mapping_providers.md @@ -60,13 +60,10 @@ A custom mapping provider must specify the following methods: 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)` +* `get_saml_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. + - `config` - A object resulting from a call to `parse_config`. - 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, diff --git a/synapse/config/saml2_config.py b/synapse/config/saml2_config.py index bfce16f4f8f5..fcbcb75a61b3 100644 --- a/synapse/config/saml2_config.py +++ b/synapse/config/saml2_config.py @@ -23,6 +23,10 @@ logger = logging.getLogger(__name__) +DEFAULT_USER_MAPPING_PROVIDER = ( + "synapse.handlers.saml_handler.DefaultSamlMappingProvider" +) + def _dict_merge(merge_dict, into_dict): """Do a deep merge of two dicts @@ -77,35 +81,45 @@ def read_config(self, config, **kwargs): "grandfathered_mxid_source_attribute", "uid" ) - user_mapping_provider_dict = saml2_config.get("user_mapping_provider") - if user_mapping_provider_dict is None: - user_mapping_provider_dict = {} + # user_mapping_provider may be None if the key is present but has no value + ump_dict = saml2_config.get("user_mapping_provider") or {} - # Load configured module class and config - default_mapping_provider = ( - "synapse.handlers.saml_handler.DefaultSamlMappingProvider" - ) - user_mapping_provider_module = user_mapping_provider_dict.get( - "module", default_mapping_provider, - ) - user_mapping_provider_config = user_mapping_provider_dict.get("config", {}) + # Use the default user mapping provider if not set + ump_dict.setdefault("module", DEFAULT_USER_MAPPING_PROVIDER) + + # Ensure a config is present + ump_dict.setdefault("config", {}) + + if ump_dict["module"] == DEFAULT_USER_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: + logger.warning( + "The config option saml2_config.mxid_source_attribute is deprecated. " + "Please use saml2_config.user_mapping_provider.config" + ".mxid_source_attribute instead." + ) + ump_dict["config"]["mxid_source_attribute"] = old_mxid_source_attribute + + old_mxid_mapping = saml2_config.get("mxid_mapping") + if 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." + ) + ump_dict["config"]["mxid_mapping"] = old_mxid_mapping # 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, - } - ) + ) = load_module(ump_dict) # Ensure loaded user mapping module has defined all necessary methods + # Note parse_config() is already checked during the call to load_module required_methods = [ - "parse_config", - "get_required_attributes", + "get_attributes", "saml_response_to_user_attributes", ] missing_methods = [ @@ -120,31 +134,9 @@ def read_config(self, config, **kwargs): "methods: %s" % (", ".join(missing_methods),) ) - 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: - 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: - 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 - + # Get the desired saml auth response attributes from the module saml2_config_dict = self._default_saml_config_dict( - *self.saml2_user_mapping_provider_class.get_required_attributes( + *self.saml2_user_mapping_provider_class.get_saml_attributes( self.saml2_user_mapping_provider_config ) ) @@ -174,7 +166,8 @@ def _default_saml_config_dict( will be needed to process new user registration Args: - required_attributes: SAML auth response attributes that are necessary to function + 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 diff --git a/synapse/handlers/saml_handler.py b/synapse/handlers/saml_handler.py index d4f583c83cb3..1d8864004e31 100644 --- a/synapse/handlers/saml_handler.py +++ b/synapse/handlers/saml_handler.py @@ -191,7 +191,8 @@ async def _map_saml_response_to_user(self, resp_bytes): localpart = attribute_dict.get("mxid_localpart") if not localpart: logger.error( - "SAML mapping provider plugin did not return a mxid_localpart object" + "SAML mapping provider plugin did not return a " + "mxid_localpart object" ) raise SynapseError(500, "Error parsing SAML2 response") @@ -250,17 +251,23 @@ def dot_replace_for_mxid(username: str) -> str: } +@attr.s +class SamlConfig(object): + mxid_source_attribute = attr.ib() + mxid_mapper = attr.ib() + + class DefaultSamlMappingProvider(object): __version__ = "0.0.1" - def __init__(self, parsed_config): + def __init__(self, parsed_config: SamlConfig): """The default SAML user mapping provider Args: parsed_config: Module configuration """ self._mxid_source_attribute = parsed_config.mxid_source_attribute - self._mxid_mapping = parsed_config.mxid_mapping + self._mxid_mapper = parsed_config.mxid_mapper def saml_response_to_user_attributes( self, saml_response: saml2.response.AuthnResponse, failures: int = 0, @@ -289,8 +296,7 @@ def saml_response_to_user_attributes( ) # Use the configured mapper for this mxid_source - mxid_mapper = MXID_MAPPER_MAP[self._mxid_mapping] - base_mxid_localpart = mxid_mapper(mxid_source) + base_mxid_localpart = self._mxid_mapper(mxid_source) # Append suffix integer if last call to this function failed to produce # a usable mxid @@ -305,51 +311,37 @@ def saml_response_to_user_attributes( } @staticmethod - def parse_config(config: dict): + def parse_config(config: dict) -> SamlConfig: """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 + SamlConfig: A custom config object for this module """ - - 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" - ) + # Parse config options and use defaults where necessary + mxid_source_attribute = config.get("mxid_source_attribute", "uid") + mapping_type = config.get("mxid_mapping", "hexencode") try: - saml_config.mxid_mapper = MXID_MAPPER_MAP[mapping_type] + 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 + + return SamlConfig(mxid_source_attribute, mxid_mapper) @staticmethod - def get_required_saml_attributes(config: dict): + def get_saml_attributes(config: SamlConfig) -> tuple[set:set]: """Returns the required attributes of a SAML Args: - config: A dictionary containing configuration options for this provider + config: A SamlConfig object containing configuration params 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 + 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"} + return {"uid", config.mxid_source_attribute}, {"displayName"} diff --git a/synapse/util/module_loader.py b/synapse/util/module_loader.py index c370be789585..2705cbe5f892 100644 --- a/synapse/util/module_loader.py +++ b/synapse/util/module_loader.py @@ -25,7 +25,7 @@ def load_module(provider): (the config dict). Returns - Tuple of (provider class, parsed config object + 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. From e15fc6aa712b0aa901ffcb563aa67911cc0a22d6 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Mon, 9 Dec 2019 17:46:57 +0000 Subject: [PATCH 27/35] Fix typing statement --- synapse/handlers/saml_handler.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/synapse/handlers/saml_handler.py b/synapse/handlers/saml_handler.py index 1d8864004e31..f6362fe9a699 100644 --- a/synapse/handlers/saml_handler.py +++ b/synapse/handlers/saml_handler.py @@ -15,6 +15,7 @@ import logging import re +from typing import Tuple import attr import saml2 import saml2.response @@ -332,7 +333,7 @@ def parse_config(config: dict) -> SamlConfig: return SamlConfig(mxid_source_attribute, mxid_mapper) @staticmethod - def get_saml_attributes(config: SamlConfig) -> tuple[set:set]: + def get_saml_attributes(config: SamlConfig) -> Tuple[set, set]: """Returns the required attributes of a SAML Args: From f5dcbf5e0931cfa299974a9f94e9eaea5d36a8a5 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Mon, 9 Dec 2019 17:47:36 +0000 Subject: [PATCH 28/35] lint --- synapse/handlers/saml_handler.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/handlers/saml_handler.py b/synapse/handlers/saml_handler.py index f6362fe9a699..5520d2f1c25b 100644 --- a/synapse/handlers/saml_handler.py +++ b/synapse/handlers/saml_handler.py @@ -14,8 +14,8 @@ # limitations under the License. import logging import re - from typing import Tuple + import attr import saml2 import saml2.response From 22ead4fbb151d5b6feec413a59c48e1ca598d174 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Mon, 9 Dec 2019 18:00:07 +0000 Subject: [PATCH 29/35] Ensure parse_config doesn't fail if config: is None --- synapse/config/saml2_config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/config/saml2_config.py b/synapse/config/saml2_config.py index fcbcb75a61b3..eafb24e4a0d4 100644 --- a/synapse/config/saml2_config.py +++ b/synapse/config/saml2_config.py @@ -88,7 +88,7 @@ def read_config(self, config, **kwargs): ump_dict.setdefault("module", DEFAULT_USER_MAPPING_PROVIDER) # Ensure a config is present - ump_dict.setdefault("config", {}) + ump_dict["config"] = ump_dict.get("config") or {} if ump_dict["module"] == DEFAULT_USER_MAPPING_PROVIDER: # Load deprecated options for use by the default module From b8dd5abb7d38a54e16c64cc66174e6121d57d4db Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Mon, 9 Dec 2019 18:02:19 +0000 Subject: [PATCH 30/35] Fix attribute-retrieving method name --- synapse/config/saml2_config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/config/saml2_config.py b/synapse/config/saml2_config.py index eafb24e4a0d4..558b1a1995f3 100644 --- a/synapse/config/saml2_config.py +++ b/synapse/config/saml2_config.py @@ -119,7 +119,7 @@ def read_config(self, config, **kwargs): # Ensure loaded user mapping module has defined all necessary methods # Note parse_config() is already checked during the call to load_module required_methods = [ - "get_attributes", + "get_saml_attributes", "saml_response_to_user_attributes", ] missing_methods = [ From 87c9e7cf861b265cf649bb8a3647e6dc6ad3999d Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Mon, 9 Dec 2019 18:27:02 +0000 Subject: [PATCH 31/35] Add logging for what's returned by the provider --- synapse/handlers/saml_handler.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/synapse/handlers/saml_handler.py b/synapse/handlers/saml_handler.py index 5520d2f1c25b..94e114f84773 100644 --- a/synapse/handlers/saml_handler.py +++ b/synapse/handlers/saml_handler.py @@ -189,6 +189,9 @@ async def _map_saml_response_to_user(self, resp_bytes): saml2_auth, i ) + logger.info("Retrieved SAML attributes from user mapping provider: %s " + "(attempt %d)", attribute_dict, i) + localpart = attribute_dict.get("mxid_localpart") if not localpart: logger.error( From 9f01563a7c1c6f691f36ac684e2bdc5e267211db Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Tue, 10 Dec 2019 10:59:24 +0000 Subject: [PATCH 32/35] Add debug logging and fix config values being None --- synapse/handlers/saml_handler.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/synapse/handlers/saml_handler.py b/synapse/handlers/saml_handler.py index 94e114f84773..ec2a3ac8ee5a 100644 --- a/synapse/handlers/saml_handler.py +++ b/synapse/handlers/saml_handler.py @@ -189,8 +189,12 @@ async def _map_saml_response_to_user(self, resp_bytes): saml2_auth, i ) - logger.info("Retrieved SAML attributes from user mapping provider: %s " - "(attempt %d)", attribute_dict, i) + logger.debug( + "Retrieved SAML attributes from user mapping provider: %s " + "(attempt %d)", + attribute_dict, + i, + ) localpart = attribute_dict.get("mxid_localpart") if not localpart: @@ -323,13 +327,13 @@ def parse_config(config: dict) -> SamlConfig: SamlConfig: A custom config object for this module """ # Parse config options and use defaults where necessary - mxid_source_attribute = config.get("mxid_source_attribute", "uid") - mapping_type = config.get("mxid_mapping", "hexencode") + mxid_source_attribute = config.get("mxid_source_attribute") or "uid" + mapping_type = config.get("mxid_mapping") or "hexencode" try: mxid_mapper = MXID_MAPPER_MAP[mapping_type] except KeyError: raise ConfigError( - "saml2_config.user_mapping_provider.config: %s is not a valid " + "saml2_config.user_mapping_provider.config: '%s' is not a valid " "mxid_mapping value" % (mapping_type,) ) From e918990fd68ad4fbb26b92974a2e8ab5670061a5 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Tue, 10 Dec 2019 11:15:19 +0000 Subject: [PATCH 33/35] Polish --- docs/saml_mapping_providers.md | 11 ++++++----- synapse/config/saml2_config.py | 4 ++-- synapse/handlers/saml_handler.py | 2 ++ 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/docs/saml_mapping_providers.md b/docs/saml_mapping_providers.md index 619307b2e84c..92f2380488c3 100644 --- a/docs/saml_mapping_providers.md +++ b/docs/saml_mapping_providers.md @@ -31,9 +31,9 @@ A custom mapping provider must specify the following methods: * `__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. + - `parsed_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: - `saml_response` - A `saml2.response.AuthnResponse` object to extract user @@ -41,7 +41,7 @@ A custom mapping provider must specify the following methods: - `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 + returned instead. For example, if this method returns `john.doe` as the value of `mxid_localpart` in the returned dict, and that is already taken on the homeserver, this method will be called again with the same parameters but @@ -50,7 +50,8 @@ A custom mapping provider must specify the following methods: - This method must return a dictionary, which will then be used by Synapse 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. + * `displayname` - The displayname of the new user. If not provided, will default to + the value of `mxid_localpart`. * `parse_config(config)` - This method should have the `@staticmethod` decoration. - Arguments: diff --git a/synapse/config/saml2_config.py b/synapse/config/saml2_config.py index 558b1a1995f3..61a2a81c8fd8 100644 --- a/synapse/config/saml2_config.py +++ b/synapse/config/saml2_config.py @@ -99,7 +99,7 @@ def read_config(self, config, **kwargs): "Please use saml2_config.user_mapping_provider.config" ".mxid_source_attribute instead." ) - ump_dict["config"]["mxid_source_attribute"] = old_mxid_source_attribute + ump_dict["config"]["mxid_source_attribute"] = old_mxid_source_attribute old_mxid_mapping = saml2_config.get("mxid_mapping") if old_mxid_mapping: @@ -107,7 +107,7 @@ def read_config(self, config, **kwargs): "The config option saml2_config.mxid_mapping is deprecated. Please " "use saml2_config.user_mapping_provider.config.mxid_mapping instead." ) - ump_dict["config"]["mxid_mapping"] = old_mxid_mapping + ump_dict["config"]["mxid_mapping"] = old_mxid_mapping # Retrieve an instance of the module's class # Pass the config dictionary to the module for processing diff --git a/synapse/handlers/saml_handler.py b/synapse/handlers/saml_handler.py index ec2a3ac8ee5a..47d2d431927b 100644 --- a/synapse/handlers/saml_handler.py +++ b/synapse/handlers/saml_handler.py @@ -311,6 +311,7 @@ def saml_response_to_user_attributes( localpart = base_mxid_localpart + (str(failures) if failures else "") # Retrieve the display name from the saml response + # If displayname is None, the mxid_localpart will be used instead displayname = saml_response.ava.get("displayName", [None])[0] return { @@ -327,6 +328,7 @@ def parse_config(config: dict) -> SamlConfig: SamlConfig: A custom config object for this module """ # Parse config options and use defaults where necessary + # We use the 'or' syntax here as these options could be a valid value of None mxid_source_attribute = config.get("mxid_source_attribute") or "uid" mapping_type = config.get("mxid_mapping") or "hexencode" try: From 5b2281e209d3e487b7014da69a443928d9c87a29 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Tue, 10 Dec 2019 15:28:41 +0000 Subject: [PATCH 34/35] Comment cleanup --- docs/sample_config.yaml | 2 +- synapse/config/saml2_config.py | 2 +- synapse/handlers/saml_handler.py | 10 ++++++---- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index fd16f041b8c0..4d44e631d14f 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -1257,7 +1257,7 @@ saml2_config: #saml_session_lifetime: 5m # An external module can be provided here as a custom solution to - # mapping the above configured saml attribute onto a matrix ID. + # mapping attributes returned from a saml provider onto a matrix user. # user_mapping_provider: # The custom module's class. Uncomment to use a custom module. diff --git a/synapse/config/saml2_config.py b/synapse/config/saml2_config.py index 61a2a81c8fd8..b91414aa3546 100644 --- a/synapse/config/saml2_config.py +++ b/synapse/config/saml2_config.py @@ -275,7 +275,7 @@ def generate_config_section(self, config_dir_path, server_name, **kwargs): #saml_session_lifetime: 5m # An external module can be provided here as a custom solution to - # mapping the above configured saml attribute onto a matrix ID. + # mapping attributes returned from a saml provider onto a matrix user. # user_mapping_provider: # The custom module's class. Uncomment to use a custom module. diff --git a/synapse/handlers/saml_handler.py b/synapse/handlers/saml_handler.py index 47d2d431927b..dae359eb0a5b 100644 --- a/synapse/handlers/saml_handler.py +++ b/synapse/handlers/saml_handler.py @@ -56,9 +56,6 @@ def __init__(self, hs): self._grandfathered_mxid_source_attribute = ( hs.config.saml2_grandfathered_mxid_source_attribute ) - self._saml2_user_mapping_provider_config = ( - hs.config.saml2_user_mapping_provider_config - ) # plugin to do custom mapping from saml response to mxid self._user_mapping_provider = hs.config.saml2_user_mapping_provider_class( @@ -328,9 +325,14 @@ def parse_config(config: dict) -> SamlConfig: SamlConfig: A custom config object for this module """ # Parse config options and use defaults where necessary - # We use the 'or' syntax here as these options could be a valid value of None + + # `config` can contain "mxid_source_attribute: None". Using `config.get` + # here would still produce `None` even with the default parameter set. + # We use `or` syntax instead to set default if `config.get` returns None mxid_source_attribute = config.get("mxid_source_attribute") or "uid" mapping_type = config.get("mxid_mapping") or "hexencode" + + # Retrieve the associating mapping function try: mxid_mapper = MXID_MAPPER_MAP[mapping_type] except KeyError: From 8a2e6170a4bee33edde2c1794b799ceea187bad4 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Tue, 10 Dec 2019 16:27:12 +0000 Subject: [PATCH 35/35] Simplify pulling default from module config --- synapse/handlers/saml_handler.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/synapse/handlers/saml_handler.py b/synapse/handlers/saml_handler.py index dae359eb0a5b..0082f85c2666 100644 --- a/synapse/handlers/saml_handler.py +++ b/synapse/handlers/saml_handler.py @@ -325,12 +325,8 @@ def parse_config(config: dict) -> SamlConfig: SamlConfig: A custom config object for this module """ # Parse config options and use defaults where necessary - - # `config` can contain "mxid_source_attribute: None". Using `config.get` - # here would still produce `None` even with the default parameter set. - # We use `or` syntax instead to set default if `config.get` returns None - mxid_source_attribute = config.get("mxid_source_attribute") or "uid" - mapping_type = config.get("mxid_mapping") or "hexencode" + mxid_source_attribute = config.get("mxid_source_attribute", "uid") + mapping_type = config.get("mxid_mapping", "hexencode") # Retrieve the associating mapping function try: