From 73b03722f446bf182f5f7a0ed318dffd55513bd3 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Date: Tue, 19 Jan 2021 14:56:54 +0000 Subject: [PATCH] Fix error messages from OIDC config parsing (#9153) Make sure we report the correct config path for errors in the OIDC configs. --- changelog.d/9153.feature | 1 + synapse/config/oidc_config.py | 25 +++++++++++++++---------- 2 files changed, 16 insertions(+), 10 deletions(-) create mode 100644 changelog.d/9153.feature diff --git a/changelog.d/9153.feature b/changelog.d/9153.feature new file mode 100644 index 000000000..01a24dcf4 --- /dev/null +++ b/changelog.d/9153.feature @@ -0,0 +1 @@ +Add support for multiple SSO Identity Providers. diff --git a/synapse/config/oidc_config.py b/synapse/config/oidc_config.py index c7fa74937..80a24cfbc 100644 --- a/synapse/config/oidc_config.py +++ b/synapse/config/oidc_config.py @@ -15,7 +15,7 @@ # limitations under the License. import string -from typing import Iterable, Optional, Type +from typing import Iterable, Optional, Tuple, Type import attr @@ -280,8 +280,8 @@ def _parse_oidc_provider_configs(config: JsonDict) -> Iterable["OidcProviderConf """ validate_config(MAIN_CONFIG_SCHEMA, config, ()) - for p in config.get("oidc_providers") or []: - yield _parse_oidc_config_dict(p) + for i, p in enumerate(config.get("oidc_providers") or []): + yield _parse_oidc_config_dict(p, ("oidc_providers", "" % (i,))) # for backwards-compatibility, it is also possible to provide a single "oidc_config" # object with an "enabled: True" property. @@ -291,10 +291,12 @@ def _parse_oidc_provider_configs(config: JsonDict) -> Iterable["OidcProviderConf # it matches OIDC_PROVIDER_CONFIG_SCHEMA (see the comments on OIDC_CONFIG_SCHEMA # above), so now we need to validate it. validate_config(OIDC_PROVIDER_CONFIG_SCHEMA, oidc_config, ("oidc_config",)) - yield _parse_oidc_config_dict(oidc_config) + yield _parse_oidc_config_dict(oidc_config, ("oidc_config",)) -def _parse_oidc_config_dict(oidc_config: JsonDict) -> "OidcProviderConfig": +def _parse_oidc_config_dict( + oidc_config: JsonDict, config_path: Tuple[str, ...] +) -> "OidcProviderConfig": """Take the configuration dict and parse it into an OidcProviderConfig Raises: @@ -305,7 +307,7 @@ def _parse_oidc_config_dict(oidc_config: JsonDict) -> "OidcProviderConfig": ump_config.setdefault("config", {}) (user_mapping_provider_class, user_mapping_provider_config,) = load_module( - ump_config, ("oidc_config", "user_mapping_provider") + ump_config, config_path + ("user_mapping_provider",) ) # Ensure loaded user mapping module has defined all necessary methods @@ -320,9 +322,9 @@ def _parse_oidc_config_dict(oidc_config: JsonDict) -> "OidcProviderConfig": ] if missing_methods: raise ConfigError( - "Class specified by oidc_config." - "user_mapping_provider.module is missing required " - "methods: %s" % (", ".join(missing_methods),) + "Class %s is missing required " + "methods: %s" % (user_mapping_provider_class, ", ".join(missing_methods),), + config_path + ("user_mapping_provider", "module"), ) # MSC2858 will appy certain limits in what can be used as an IdP id, so let's @@ -331,7 +333,10 @@ def _parse_oidc_config_dict(oidc_config: JsonDict) -> "OidcProviderConfig": valid_idp_chars = set(string.ascii_letters + string.digits + "-._~") if any(c not in valid_idp_chars for c in idp_id): - raise ConfigError('idp_id may only contain A-Z, a-z, 0-9, "-", ".", "_", "~"') + raise ConfigError( + 'idp_id may only contain A-Z, a-z, 0-9, "-", ".", "_", "~"', + config_path + ("idp_id",), + ) return OidcProviderConfig( idp_id=idp_id,