From 79ee5697a863cae0cd0da94b23f83492f3538e18 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Grimpard?= Date: Tue, 5 Sep 2023 16:44:55 +0200 Subject: [PATCH 01/14] Update config_documentation.md SSO CAS registration configuration --- docs/usage/configuration/config_documentation.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/docs/usage/configuration/config_documentation.md b/docs/usage/configuration/config_documentation.md index 0b1725816e3d..5b0c986e4184 100644 --- a/docs/usage/configuration/config_documentation.md +++ b/docs/usage/configuration/config_documentation.md @@ -3428,6 +3428,10 @@ Has the following sub-options: and the values must match the given value. Alternately if the given value is `None` then any value is allowed (the attribute just must exist). All of the listed attributes must match for the login to be permitted. +* `enable_registration`: set to 'false' to disable automatic registration of new + 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. Example configuration: ```yaml @@ -3439,6 +3443,7 @@ cas_config: required_attributes: userGroup: "staff" department: None + enable_registration: true ``` --- ### `sso` From 883b9426af799384cbee7b96306132ae8079bbfc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Grimpard?= Date: Wed, 6 Sep 2023 14:10:17 +0200 Subject: [PATCH 02/14] Add configuration setting "enable_registration" --- synapse/config/cas.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/synapse/config/cas.py b/synapse/config/cas.py index 6e2d9addbf4c..41357929da45 100644 --- a/synapse/config/cas.py +++ b/synapse/config/cas.py @@ -57,6 +57,8 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None: required_attributes ) + self.cas_enable_registration = cas_config.get("enable_registration") + 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") @@ -67,6 +69,7 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None: self.cas_protocol_version = None self.cas_displayname_attribute = None self.cas_required_attributes = [] + self.cas_enable_registration = None # CAS uses a legacy required attributes mapping, not the one provided by From 01d2b771c0777fd9eb3c4f85cd2e27ff73f29e7a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Grimpard?= Date: Wed, 6 Sep 2023 14:13:02 +0200 Subject: [PATCH 03/14] Handle "registration_enabled" parameter for CAS --- synapse/handlers/cas.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/synapse/handlers/cas.py b/synapse/handlers/cas.py index a85054545356..b5b8b9bd35cf 100644 --- a/synapse/handlers/cas.py +++ b/synapse/handlers/cas.py @@ -70,6 +70,7 @@ def __init__(self, hs: "HomeServer"): self._cas_protocol_version = hs.config.cas.cas_protocol_version 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._http_client = hs.get_proxied_http_client() @@ -395,4 +396,5 @@ async def grandfather_existing_users() -> Optional[str]: client_redirect_url, cas_response_to_user_attributes, grandfather_existing_users, + registration_enabled=self._cas_enable_registration, ) From dad4426c468fd654b2a9d82844a15dfa5cdc1e31 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Grimpard?= Date: Wed, 6 Sep 2023 14:18:09 +0200 Subject: [PATCH 04/14] Changelog +Add the ability to enable/disable registrations when in the CAS flow --- changelog.d/16262.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/16262.feature diff --git a/changelog.d/16262.feature b/changelog.d/16262.feature new file mode 100644 index 000000000000..7c8e7e349bca --- /dev/null +++ b/changelog.d/16262.feature @@ -0,0 +1 @@ +Add the ability to enable/disable registrations when in the CAS flow. Contributed by Aurélien Grimpard. From 8cec3c7f9c53b35e4039f6a2c3adce1d933e6dbb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Grimpard?= Date: Wed, 6 Sep 2023 14:38:17 +0200 Subject: [PATCH 05/14] enable_registration parameter to boolean Co-authored-by: Patrick Cloke --- 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 41357929da45..3a8f7f81be0e 100644 --- a/synapse/config/cas.py +++ b/synapse/config/cas.py @@ -69,7 +69,7 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None: self.cas_protocol_version = None self.cas_displayname_attribute = None self.cas_required_attributes = [] - self.cas_enable_registration = None + self.cas_enable_registration = False # CAS uses a legacy required attributes mapping, not the one provided by From 46cd6aaf4d7a5eed2ed79281bc9ce3fd2934b7a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Grimpard?= Date: Wed, 6 Sep 2023 14:39:03 +0200 Subject: [PATCH 06/14] enable_registration parameter to true for backwards compatibility Co-authored-by: Patrick Cloke --- 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 3a8f7f81be0e..bbc8f4307350 100644 --- a/synapse/config/cas.py +++ b/synapse/config/cas.py @@ -57,7 +57,7 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None: required_attributes ) - self.cas_enable_registration = cas_config.get("enable_registration") + self.cas_enable_registration = cas_config.get("enable_registration", True) self.idp_name = cas_config.get("idp_name", "CAS") self.idp_icon = cas_config.get("idp_icon") From c42c8f639a7292f9fea4baaa4da549b98cb172c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Grimpard?= Date: Wed, 6 Sep 2023 15:18:43 +0200 Subject: [PATCH 07/14] Add test that ensures new users are not registered if the enabled registration flag is disabled --- 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 8582b1cd1e9e..167c07d172a5 100644 --- a/tests/handlers/test_cas.py +++ b/tests/handlers/test_cas.py @@ -197,6 +197,29 @@ def test_required_attributes(self) -> None: auth_provider_session_id=None, ) + @override_config( + { + "cas_config": { + "enable_registration": False + } + } + ) + def test_map_cas_user_does_not_register_new_user(self) -> None: + """Ensures new users are not registered if the enabled registration flag is disabled.""" + + 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", "") + ) + self.complete_sso_login.assert_not_called() + self.assertRenderedError( + "mapping_error", + "User does not exist and registrations are disabled", + ) + def _mock_request() -> Mock: """Returns a mock which will stand in as a SynapseRequest""" From c4c02594fd3a9b0a9ed3625ca0843626f5916160 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Grimpard?= Date: Wed, 6 Sep 2023 15:30:30 +0200 Subject: [PATCH 08/14] Fix lint issue Co-authored-by: Patrick Cloke --- tests/handlers/test_cas.py | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/tests/handlers/test_cas.py b/tests/handlers/test_cas.py index 167c07d172a5..255675485738 100644 --- a/tests/handlers/test_cas.py +++ b/tests/handlers/test_cas.py @@ -197,16 +197,10 @@ def test_required_attributes(self) -> None: auth_provider_session_id=None, ) - @override_config( - { - "cas_config": { - "enable_registration": False - } - } - ) + @override_config({"cas_config": {"enable_registration": False}}) def test_map_cas_user_does_not_register_new_user(self) -> None: """Ensures new users are not registered if the enabled registration flag is disabled.""" - + auth_handler = self.hs.get_auth_handler() auth_handler.complete_sso_login = AsyncMock() # type: ignore[method-assign] cas_response = CasResponse("test_user", {}) From 434d435196062884eaa0348e12fb955f658a44ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Grimpard?= Date: Wed, 6 Sep 2023 15:55:55 +0200 Subject: [PATCH 09/14] Fix auth handler test Co-authored-by: Patrick Cloke --- 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 255675485738..2de597639a7a 100644 --- a/tests/handlers/test_cas.py +++ b/tests/handlers/test_cas.py @@ -208,7 +208,7 @@ def test_map_cas_user_does_not_register_new_user(self) -> None: self.get_success( self.handler._handle_cas_response(request, cas_response, "redirect_uri", "") ) - self.complete_sso_login.assert_not_called() + auth_handler.complete_sso_login.assert_not_called() self.assertRenderedError( "mapping_error", "User does not exist and registrations are disabled", From c2a9c2402018f660384c9adcba08b9c9d88043a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Grimpard?= Date: Wed, 6 Sep 2023 16:22:23 +0200 Subject: [PATCH 10/14] Fix auth_handler issue --- tests/handlers/test_cas.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/tests/handlers/test_cas.py b/tests/handlers/test_cas.py index 2de597639a7a..ae1ffd319a7e 100644 --- a/tests/handlers/test_cas.py +++ b/tests/handlers/test_cas.py @@ -208,11 +208,15 @@ def test_map_cas_user_does_not_register_new_user(self) -> None: self.get_success( self.handler._handle_cas_response(request, cas_response, "redirect_uri", "") ) - auth_handler.complete_sso_login.assert_not_called() - self.assertRenderedError( - "mapping_error", - "User does not exist and registrations are disabled", - ) + auth_handler.complete_sso_login.assert_not_called( + "@test_user:test", + "cas", + request, + "redirect_uri", + None, + new_user=True, + auth_provider_session_id=None, + ) def _mock_request() -> Mock: From 510635c20ab710658aa9ccc290535d33c8bb3bab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Grimpard?= Date: Wed, 6 Sep 2023 16:27:18 +0200 Subject: [PATCH 11/14] lint + comment --- tests/handlers/test_cas.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/handlers/test_cas.py b/tests/handlers/test_cas.py index ae1ffd319a7e..fd6276e996e0 100644 --- a/tests/handlers/test_cas.py +++ b/tests/handlers/test_cas.py @@ -201,13 +201,17 @@ def test_required_attributes(self) -> None: def test_map_cas_user_does_not_register_new_user(self) -> None: """Ensures new users are not registered if the enabled registration flag is disabled.""" + # 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 was not called as expected auth_handler.complete_sso_login.assert_not_called( "@test_user:test", "cas", @@ -216,8 +220,7 @@ 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: """Returns a mock which will stand in as a SynapseRequest""" From e08f9356e613144c418dac403fa63547c11a6e82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Grimpard?= Date: Wed, 6 Sep 2023 16:33:35 +0200 Subject: [PATCH 12/14] fix assert_not_called parameters --- tests/handlers/test_cas.py | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/tests/handlers/test_cas.py b/tests/handlers/test_cas.py index fd6276e996e0..45824aa814f6 100644 --- a/tests/handlers/test_cas.py +++ b/tests/handlers/test_cas.py @@ -204,7 +204,7 @@ def test_map_cas_user_does_not_register_new_user(self) -> None: # 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( @@ -212,15 +212,7 @@ 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( - "@test_user:test", - "cas", - request, - "redirect_uri", - None, - new_user=True, - auth_provider_session_id=None, - ) + auth_handler.complete_sso_login.assert_not_called() def _mock_request() -> Mock: """Returns a mock which will stand in as a SynapseRequest""" From 764ee972602ceebe346eed6b75eeef6b9cff1164 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Grimpard?= Date: Wed, 6 Sep 2023 16:36:35 +0200 Subject: [PATCH 13/14] lint ! --- tests/handlers/test_cas.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/handlers/test_cas.py b/tests/handlers/test_cas.py index 45824aa814f6..13e2cd153a08 100644 --- a/tests/handlers/test_cas.py +++ b/tests/handlers/test_cas.py @@ -214,6 +214,7 @@ 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() + def _mock_request() -> Mock: """Returns a mock which will stand in as a SynapseRequest""" mock = Mock( From b6dfca1b48b2b3e8a9761c0c85013607dca3c0e8 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 6 Sep 2023 12:27:37 -0400 Subject: [PATCH 14/14] Note the version. --- docs/usage/configuration/config_documentation.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/usage/configuration/config_documentation.md b/docs/usage/configuration/config_documentation.md index 5b0c986e4184..9dea19c4d2c6 100644 --- a/docs/usage/configuration/config_documentation.md +++ b/docs/usage/configuration/config_documentation.md @@ -3433,6 +3433,8 @@ Has the following sub-options: automatically registering users that have a valid SSO login but do not have a pre-registered account. Defaults to true. + *Added in Synapse 1.93.0.* + Example configuration: ```yaml cas_config: