Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allows CAS SSO flow to provide user IDs composed of numbers only #17098

Merged
merged 19 commits into from
May 14, 2024
Merged
1 change: 1 addition & 0 deletions changelog.d/17098.feature
Original file line number Diff line number Diff line change
@@ -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.
11 changes: 11 additions & 0 deletions docs/usage/configuration/config_documentation.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
agrimpard marked this conversation as resolved.
Show resolved Hide resolved

*Added in Synapse 1.93.0.*

Expand All @@ -3534,6 +3543,8 @@ cas_config:
userGroup: "staff"
department: None
enable_registration: true
allow_numeric_ids: true
numeric_ids_prefix: "NUMERICUSER"
```
---
### `sso`
Expand Down
14 changes: 14 additions & 0 deletions synapse/config/cas.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"(
"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")
Expand All @@ -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"
agrimpard marked this conversation as resolved.
Show resolved Hide resolved


# CAS uses a legacy required attributes mapping, not the one provided by
Expand Down
5 changes: 5 additions & 0 deletions synapse/handlers/cas.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down Expand Up @@ -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
Expand Down
27 changes: 27 additions & 0 deletions tests/handlers/test_cas.py
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,33 @@ 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", "")
)

# check that the auth handler got called as expected
auth_handler.complete_sso_login.assert_called_once_with(
"@1234:test",
agrimpard marked this conversation as resolved.
Show resolved Hide resolved
"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"""
Expand Down
Loading