From 69e5e2c70ff0bff5dcff1bf99a007327c47a6325 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 20 Jan 2021 19:03:04 +0000 Subject: [PATCH 1/2] Add a check for duplicate IdP ids --- changelog.d/9184.misc | 1 + synapse/config/oidc_config.py | 12 ++++++++++++ 2 files changed, 13 insertions(+) create mode 100644 changelog.d/9184.misc diff --git a/changelog.d/9184.misc b/changelog.d/9184.misc new file mode 100644 index 000000000000..70da3d6cf59c --- /dev/null +++ b/changelog.d/9184.misc @@ -0,0 +1 @@ +Emit an error at startup if different Identity Providers are configured with the same `idp_id`. diff --git a/synapse/config/oidc_config.py b/synapse/config/oidc_config.py index 8cb0c42f36f1..ddabb61df53a 100644 --- a/synapse/config/oidc_config.py +++ b/synapse/config/oidc_config.py @@ -15,6 +15,7 @@ # limitations under the License. import string +from collections import Counter from typing import Iterable, Optional, Tuple, Type import attr @@ -43,6 +44,17 @@ def read_config(self, config, **kwargs): except DependencyException as e: raise ConfigError(e.message) from e + # check we don't have any duplicate idp_ids + # XXX: this won't detect clashes with other IdP providers using other SSO + # mechanisms (such as SAML or CAS); that will be detected when we set up the + # listeners but by then synapse will have forked, so it's not ideal. + c = Counter([i.idp_id for i in self.oidc_providers]) + for idp_id, count in c.items(): + if count > 1: + raise ConfigError( + "Multiple OIDC providers have the idp_id %r." % idp_id + ) + public_baseurl = self.public_baseurl self.oidc_callback_url = public_baseurl + "_synapse/oidc/callback" From ac3d44df0750fe3ada62de74bbc8662c55f8e17f Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 21 Jan 2021 11:49:22 +0000 Subject: [PATCH 2/2] Update out-of-date-comment https://github.com/matrix-org/synapse/pull/9189 makes this comment redundant. --- synapse/config/oidc_config.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/synapse/config/oidc_config.py b/synapse/config/oidc_config.py index ddabb61df53a..6c9fde9ce22f 100644 --- a/synapse/config/oidc_config.py +++ b/synapse/config/oidc_config.py @@ -44,10 +44,9 @@ def read_config(self, config, **kwargs): except DependencyException as e: raise ConfigError(e.message) from e - # check we don't have any duplicate idp_ids - # XXX: this won't detect clashes with other IdP providers using other SSO - # mechanisms (such as SAML or CAS); that will be detected when we set up the - # listeners but by then synapse will have forked, so it's not ideal. + # check we don't have any duplicate idp_ids now. (The SSO handler will also + # check for duplicates when the REST listeners get registered, but that happens + # after synapse has forked so doesn't give nice errors.) c = Counter([i.idp_id for i in self.oidc_providers]) for idp_id, count in c.items(): if count > 1: