From 8fba6689c2551eeb62be123a6d11fc82e648b122 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Grimpard?= Date: Tue, 16 Apr 2024 17:46:19 +0200 Subject: [PATCH 01/19] Update cas.py Allows CAS SSO flow to provide user IDs composed of numbers only which will be prefixed --- synapse/config/cas.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/synapse/config/cas.py b/synapse/config/cas.py index d23dcf96b20..6803853ac6b 100644 --- a/synapse/config/cas.py +++ b/synapse/config/cas.py @@ -66,6 +66,18 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None: self.cas_enable_registration = cas_config.get("enable_registration", True) + self.cas_allow_numeric_ids = cas_config.get("allow_numeric_ids") + self.cas_numeric_ids_prefix = cas_config.get("numeric_ids_prefix") + if ( + self.cas_numeric_ids_prefix is not None + and self.cas_numeric_ids_prefix.isalnum() is False + ): + raise ConfigError( + "Only alphanumeric characters are allowed for numeric IDs prefix" + % (self.cas_numeric_ids_prefix,), + ("cas_config", "numeric_ids_prefix"), + ) + self.idp_name = cas_config.get("idp_name", "CAS") self.idp_icon = cas_config.get("idp_icon") self.idp_brand = cas_config.get("idp_brand") @@ -77,6 +89,8 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None: self.cas_displayname_attribute = None self.cas_required_attributes = [] self.cas_enable_registration = False + self.cas_allow_numeric_ids = False + self.cas_numeric_ids_prefix = "U" # CAS uses a legacy required attributes mapping, not the one provided by From f57e354f7ba02a698ba9273e492cc44cdf2ebba6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Grimpard?= Date: Tue, 16 Apr 2024 17:49:11 +0200 Subject: [PATCH 02/19] Update cas.py Allows CAS SSO flow to provide user IDs composed of numbers only which will be prefixed --- synapse/handlers/cas.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/synapse/handlers/cas.py b/synapse/handlers/cas.py index 153123ee831..27bc8a8c74e 100644 --- a/synapse/handlers/cas.py +++ b/synapse/handlers/cas.py @@ -78,6 +78,8 @@ def __init__(self, hs: "HomeServer"): self._cas_displayname_attribute = hs.config.cas.cas_displayname_attribute self._cas_required_attributes = hs.config.cas.cas_required_attributes self._cas_enable_registration = hs.config.cas.cas_enable_registration + self._cas_allow_numeric_ids = hs.config.cas.cas_allow_numeric_ids + self._cas_numeric_ids_prefix = hs.config.cas.cas_numeric_ids_prefix self._http_client = hs.get_proxied_http_client() @@ -188,6 +190,9 @@ def _parse_cas_response(self, cas_response_body: bytes) -> CasResponse: for child in root[0]: if child.tag.endswith("user"): user = child.text + # if numeric user IDs are allowed and username is numeric then we add the prefix so Synapse can handle it + if self._cas_allow_numeric_ids is True and user.isdigit(): + user = f"{self._cas_numeric_ids_prefix}{user}" if child.tag.endswith("attributes"): for attribute in child: # ElementTree library expands the namespace in From c2f016891c5275965839029129f0257a18c79ec7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Grimpard?= Date: Tue, 16 Apr 2024 17:51:00 +0200 Subject: [PATCH 03/19] Update config_documentation.md Allows CAS SSO flow to provide user IDs composed of numbers only which will be prefixed --- docs/usage/configuration/config_documentation.md | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/docs/usage/configuration/config_documentation.md b/docs/usage/configuration/config_documentation.md index 985f90c8a16..37cf657104f 100644 --- a/docs/usage/configuration/config_documentation.md +++ b/docs/usage/configuration/config_documentation.md @@ -3520,6 +3520,15 @@ Has the following sub-options: users. This allows the CAS SSO flow to be limited to sign in only, rather than automatically registering users that have a valid SSO login but do not have a pre-registered account. Defaults to true. +* `allow_numeric_ids`: set to 'true' allow numeric user IDs (default false). + This allows CAS SSO flow to provide user IDs composed of numbers only. + These identifiers will be prefixed by the letter "U" by default. + The prefix can be configured using the "numeric_ids_prefix" option. + Be careful to choose the prefix correctly to avoid any possible conflicts + (e.g. user 1234 becomes U1234 when a user U1234 already exists). +* `numeric_ids_prefix`: the prefix you wish to add in front of a numeric user ID + when the "allow_numeric_ids" option is set to "true". + By default, the prefix is the letter "U" and only alphanumeric characters are allowed. *Added in Synapse 1.93.0.* @@ -3534,6 +3543,8 @@ cas_config: userGroup: "staff" department: None enable_registration: true + allow_numeric_ids: true + numeric_ids_prefix: "NUMERICUSER" ``` --- ### `sso` From 6c7113fb2c7612af55648e10c17cd6637feb4212 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Grimpard?= Date: Tue, 16 Apr 2024 17:53:37 +0200 Subject: [PATCH 04/19] Update test_cas.py Allows CAS SSO flow to provide user IDs composed of numbers only which will be prefixed --- tests/handlers/test_cas.py | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/tests/handlers/test_cas.py b/tests/handlers/test_cas.py index f41f7d36ad1..a2ef2532365 100644 --- a/tests/handlers/test_cas.py +++ b/tests/handlers/test_cas.py @@ -221,6 +221,29 @@ def test_map_cas_user_does_not_register_new_user(self) -> None: # check that the auth handler was not called as expected auth_handler.complete_sso_login.assert_not_called() + @override_config({"cas_config": {"allow_numeric_ids": True, "numeric_ids_prefix": "NUMERICUSER"}}) + def test_map_cas_user_does_not_register_new_user(self) -> None: + """Ensures new users with numeric user IDs are registered if the allow_numeric_ids flag is enabled.""" + + # stub out the auth handler + auth_handler = self.hs.get_auth_handler() + auth_handler.complete_sso_login = AsyncMock() # type: ignore[method-assign] + + cas_response = CasResponse("test_user", {}) + request = _mock_request() + self.get_success( + self.handler._handle_cas_response(request, cas_response, "redirect_uri", "") + ) + + # check that the auth handler got called as expected + auth_handler.complete_sso_login.assert_called_once_with( + "@1234:test", + "cas", + request, + "redirect_uri", + None, + new_user=True, + auth_provider_session_id=None, def _mock_request() -> Mock: """Returns a mock which will stand in as a SynapseRequest""" From 8788cbb914e3667ab8c110cc6f9e8280f04ccbdc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Grimpard?= Date: Tue, 16 Apr 2024 17:59:40 +0200 Subject: [PATCH 05/19] Create 17098.feature --- changelog.d/17098.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/17098.feature diff --git a/changelog.d/17098.feature b/changelog.d/17098.feature new file mode 100644 index 00000000000..43e06481b2c --- /dev/null +++ b/changelog.d/17098.feature @@ -0,0 +1 @@ +Add the ability to allow numeric user IDs with a specific prefix when in the CAS flow. Contributed by Aurélien Grimpard. From 981a7ef6fdf7c62ef16484ef43375c1433128966 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Grimpard?= Date: Tue, 16 Apr 2024 18:05:57 +0200 Subject: [PATCH 06/19] Update test_cas.py lint --- tests/handlers/test_cas.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/handlers/test_cas.py b/tests/handlers/test_cas.py index a2ef2532365..5ed8cbf1143 100644 --- a/tests/handlers/test_cas.py +++ b/tests/handlers/test_cas.py @@ -245,7 +245,7 @@ def test_map_cas_user_does_not_register_new_user(self) -> None: new_user=True, auth_provider_session_id=None, -def _mock_request() -> Mock: + def _mock_request() -> Mock: """Returns a mock which will stand in as a SynapseRequest""" mock = Mock( spec=[ From 5ad3ad4ab8c31c523396e429b3cb120382b92678 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Grimpard?= Date: Tue, 16 Apr 2024 18:10:50 +0200 Subject: [PATCH 07/19] Update test_cas.py Fix lint + miss c/p --- tests/handlers/test_cas.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/handlers/test_cas.py b/tests/handlers/test_cas.py index 5ed8cbf1143..191aa7ad186 100644 --- a/tests/handlers/test_cas.py +++ b/tests/handlers/test_cas.py @@ -244,8 +244,10 @@ def test_map_cas_user_does_not_register_new_user(self) -> None: None, new_user=True, auth_provider_session_id=None, + ) + - def _mock_request() -> Mock: +def _mock_request() -> Mock: """Returns a mock which will stand in as a SynapseRequest""" mock = Mock( spec=[ From 0068b1000fdf5cdac45baa2800104c18b013eb03 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Grimpard?= Date: Tue, 16 Apr 2024 18:14:39 +0200 Subject: [PATCH 08/19] Update test_cas.py Reformate proposed by lint test ... --- tests/handlers/test_cas.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/handlers/test_cas.py b/tests/handlers/test_cas.py index 191aa7ad186..5a0d8b8540b 100644 --- a/tests/handlers/test_cas.py +++ b/tests/handlers/test_cas.py @@ -221,7 +221,9 @@ def test_map_cas_user_does_not_register_new_user(self) -> None: # check that the auth handler was not called as expected auth_handler.complete_sso_login.assert_not_called() - @override_config({"cas_config": {"allow_numeric_ids": True, "numeric_ids_prefix": "NUMERICUSER"}}) + @override_config( + {"cas_config": {"allow_numeric_ids": True, "numeric_ids_prefix": "NUMERICUSER"}} + ) def test_map_cas_user_does_not_register_new_user(self) -> None: """Ensures new users with numeric user IDs are registered if the allow_numeric_ids flag is enabled.""" From fc6306bbb1335ba354f2ce6d01df19a885679d9c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Grimpard?= Date: Tue, 16 Apr 2024 18:22:45 +0200 Subject: [PATCH 09/19] Update test_cas.py unique def for test --- tests/handlers/test_cas.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/handlers/test_cas.py b/tests/handlers/test_cas.py index 5a0d8b8540b..f16bd9c40fd 100644 --- a/tests/handlers/test_cas.py +++ b/tests/handlers/test_cas.py @@ -224,7 +224,7 @@ def test_map_cas_user_does_not_register_new_user(self) -> None: @override_config( {"cas_config": {"allow_numeric_ids": True, "numeric_ids_prefix": "NUMERICUSER"}} ) - def test_map_cas_user_does_not_register_new_user(self) -> None: + def test_register_user_with_numeric_ids_when_allowed(self) -> None: """Ensures new users with numeric user IDs are registered if the allow_numeric_ids flag is enabled.""" # stub out the auth handler From 589a567e200cbf0fd59661e24c5acc8f4d3d3918 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Grimpard?= Date: Tue, 16 Apr 2024 18:25:37 +0200 Subject: [PATCH 10/19] Update cas.py Fix useless % --- synapse/config/cas.py | 1 - 1 file changed, 1 deletion(-) diff --git a/synapse/config/cas.py b/synapse/config/cas.py index 6803853ac6b..d7d1ba58bb0 100644 --- a/synapse/config/cas.py +++ b/synapse/config/cas.py @@ -74,7 +74,6 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None: ): raise ConfigError( "Only alphanumeric characters are allowed for numeric IDs prefix" - % (self.cas_numeric_ids_prefix,), ("cas_config", "numeric_ids_prefix"), ) From 96e29e4ac5ba6fb33b5f16c3e9e09ed5172099eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Grimpard?= Date: Tue, 16 Apr 2024 18:29:35 +0200 Subject: [PATCH 11/19] Update cas.py reformate proposed by lint test --- synapse/config/cas.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/synapse/config/cas.py b/synapse/config/cas.py index d7d1ba58bb0..50b1d7efb53 100644 --- a/synapse/config/cas.py +++ b/synapse/config/cas.py @@ -73,8 +73,9 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None: and self.cas_numeric_ids_prefix.isalnum() is False ): raise ConfigError( - "Only alphanumeric characters are allowed for numeric IDs prefix" - ("cas_config", "numeric_ids_prefix"), + "Only alphanumeric characters are allowed for numeric IDs prefix"( + "cas_config", "numeric_ids_prefix" + ), ) self.idp_name = cas_config.get("idp_name", "CAS") From 50474bc5dad523cea281f8712670c463c47e3f85 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Grimpard?= Date: Thu, 18 Apr 2024 11:17:27 +0200 Subject: [PATCH 12/19] Update cas.py fix coma --- synapse/config/cas.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/synapse/config/cas.py b/synapse/config/cas.py index 50b1d7efb53..846f34b2841 100644 --- a/synapse/config/cas.py +++ b/synapse/config/cas.py @@ -73,7 +73,8 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None: and self.cas_numeric_ids_prefix.isalnum() is False ): raise ConfigError( - "Only alphanumeric characters are allowed for numeric IDs prefix"( + "Only alphanumeric characters are allowed for numeric IDs prefix", + ( "cas_config", "numeric_ids_prefix" ), ) From db52967df8ee5fbebd460abc71f9693768eec676 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Grimpard?= Date: Thu, 18 Apr 2024 11:18:23 +0200 Subject: [PATCH 13/19] Update cas.py --- synapse/handlers/cas.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/synapse/handlers/cas.py b/synapse/handlers/cas.py index 27bc8a8c74e..f6442634857 100644 --- a/synapse/handlers/cas.py +++ b/synapse/handlers/cas.py @@ -191,7 +191,11 @@ def _parse_cas_response(self, cas_response_body: bytes) -> CasResponse: if child.tag.endswith("user"): user = child.text # if numeric user IDs are allowed and username is numeric then we add the prefix so Synapse can handle it - if self._cas_allow_numeric_ids is True and user.isdigit(): + if ( + self._cas_allow_numeric_ids + and user is not None + and user.isdigit() + ): user = f"{self._cas_numeric_ids_prefix}{user}" if child.tag.endswith("attributes"): for attribute in child: From 3f6a8d22d037b74d2e79919157a99dda7b7672af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Grimpard?= Date: Thu, 18 Apr 2024 11:24:01 +0200 Subject: [PATCH 14/19] Update cas.py --- synapse/handlers/cas.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/synapse/handlers/cas.py b/synapse/handlers/cas.py index f6442634857..cc3d641b7d8 100644 --- a/synapse/handlers/cas.py +++ b/synapse/handlers/cas.py @@ -191,11 +191,7 @@ def _parse_cas_response(self, cas_response_body: bytes) -> CasResponse: if child.tag.endswith("user"): user = child.text # if numeric user IDs are allowed and username is numeric then we add the prefix so Synapse can handle it - if ( - self._cas_allow_numeric_ids - and user is not None - and user.isdigit() - ): + if self._cas_allow_numeric_ids and user is not None and user.isdigit(): user = f"{self._cas_numeric_ids_prefix}{user}" if child.tag.endswith("attributes"): for attribute in child: From 93da7f41476f390c84ac0a43ed4e14764b297657 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Grimpard?= Date: Thu, 18 Apr 2024 11:24:16 +0200 Subject: [PATCH 15/19] Update cas.py --- synapse/config/cas.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/synapse/config/cas.py b/synapse/config/cas.py index 846f34b2841..f5f5e808677 100644 --- a/synapse/config/cas.py +++ b/synapse/config/cas.py @@ -74,9 +74,7 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None: ): raise ConfigError( "Only alphanumeric characters are allowed for numeric IDs prefix", - ( - "cas_config", "numeric_ids_prefix" - ), + ("cas_config", "numeric_ids_prefix"), ) self.idp_name = cas_config.get("idp_name", "CAS") From 00b0705a825e374079dcbba5b0d25311dfa380c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Grimpard?= Date: Thu, 18 Apr 2024 11:38:35 +0200 Subject: [PATCH 16/19] Update test_cas.py Removed test, need real devops ! --- tests/handlers/test_cas.py | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/tests/handlers/test_cas.py b/tests/handlers/test_cas.py index f16bd9c40fd..1482b9d4e3c 100644 --- a/tests/handlers/test_cas.py +++ b/tests/handlers/test_cas.py @@ -237,17 +237,6 @@ def test_register_user_with_numeric_ids_when_allowed(self) -> None: self.handler._handle_cas_response(request, cas_response, "redirect_uri", "") ) - # check that the auth handler got called as expected - auth_handler.complete_sso_login.assert_called_once_with( - "@1234:test", - "cas", - request, - "redirect_uri", - None, - new_user=True, - auth_provider_session_id=None, - ) - def _mock_request() -> Mock: """Returns a mock which will stand in as a SynapseRequest""" From 7b84f085447fe55dd5c1403f4668d0dc3d2d6a8b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Grimpard?= Date: Thu, 18 Apr 2024 11:40:08 +0200 Subject: [PATCH 17/19] Update test_cas.py Remove test, need real devops ! --- tests/handlers/test_cas.py | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/tests/handlers/test_cas.py b/tests/handlers/test_cas.py index 1482b9d4e3c..f41f7d36ad1 100644 --- a/tests/handlers/test_cas.py +++ b/tests/handlers/test_cas.py @@ -221,22 +221,6 @@ def test_map_cas_user_does_not_register_new_user(self) -> None: # check that the auth handler was not called as expected auth_handler.complete_sso_login.assert_not_called() - @override_config( - {"cas_config": {"allow_numeric_ids": True, "numeric_ids_prefix": "NUMERICUSER"}} - ) - def test_register_user_with_numeric_ids_when_allowed(self) -> None: - """Ensures new users with numeric user IDs are registered if the allow_numeric_ids flag is enabled.""" - - # stub out the auth handler - auth_handler = self.hs.get_auth_handler() - auth_handler.complete_sso_login = AsyncMock() # type: ignore[method-assign] - - cas_response = CasResponse("test_user", {}) - request = _mock_request() - self.get_success( - self.handler._handle_cas_response(request, cas_response, "redirect_uri", "") - ) - def _mock_request() -> Mock: """Returns a mock which will stand in as a SynapseRequest""" From ab9678f8e19352bfbb1dd52d9096fb42ae560197 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Grimpard?= Date: Thu, 18 Apr 2024 13:46:49 +0200 Subject: [PATCH 18/19] Update cas.py lower case prefix for uid --- synapse/config/cas.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/config/cas.py b/synapse/config/cas.py index f5f5e808677..fa59c350c15 100644 --- a/synapse/config/cas.py +++ b/synapse/config/cas.py @@ -89,7 +89,7 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None: self.cas_required_attributes = [] self.cas_enable_registration = False self.cas_allow_numeric_ids = False - self.cas_numeric_ids_prefix = "U" + self.cas_numeric_ids_prefix = "u" # CAS uses a legacy required attributes mapping, not the one provided by From e5ddad0c48b4e9e48ff59509cfb204319a1b3268 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Grimpard?= Date: Thu, 18 Apr 2024 13:49:03 +0200 Subject: [PATCH 19/19] Update config_documentation.md lower case prefix uid --- docs/usage/configuration/config_documentation.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/usage/configuration/config_documentation.md b/docs/usage/configuration/config_documentation.md index 37cf657104f..c179d173cbf 100644 --- a/docs/usage/configuration/config_documentation.md +++ b/docs/usage/configuration/config_documentation.md @@ -3522,13 +3522,13 @@ Has the following sub-options: a pre-registered account. Defaults to true. * `allow_numeric_ids`: set to 'true' allow numeric user IDs (default false). This allows CAS SSO flow to provide user IDs composed of numbers only. - These identifiers will be prefixed by the letter "U" by default. + These identifiers will be prefixed by the letter "u" by default. The prefix can be configured using the "numeric_ids_prefix" option. Be careful to choose the prefix correctly to avoid any possible conflicts - (e.g. user 1234 becomes U1234 when a user U1234 already exists). + (e.g. user 1234 becomes u1234 when a user u1234 already exists). * `numeric_ids_prefix`: the prefix you wish to add in front of a numeric user ID when the "allow_numeric_ids" option is set to "true". - By default, the prefix is the letter "U" and only alphanumeric characters are allowed. + By default, the prefix is the letter "u" and only alphanumeric characters are allowed. *Added in Synapse 1.93.0.* @@ -3544,7 +3544,7 @@ cas_config: department: None enable_registration: true allow_numeric_ids: true - numeric_ids_prefix: "NUMERICUSER" + numeric_ids_prefix: "numericuser" ``` --- ### `sso`