diff --git a/changelog.d/17949.feature b/changelog.d/17949.feature new file mode 100644 index 0000000000000000000000000000000000000000..ee9bb51e032a83498aeb29ce32296bbf2b4471b2 --- /dev/null +++ b/changelog.d/17949.feature @@ -0,0 +1 @@ +Add functionality to be able to use multiple values in SSO feature `attribute_requirements`. diff --git a/docs/usage/configuration/config_documentation.md b/docs/usage/configuration/config_documentation.md index a1e671ab8e40b45781c514e0b598d0cb6b8f6a53..a5f23149ec506d8e499b3b253ad0e9110c37662a 100644 --- a/docs/usage/configuration/config_documentation.md +++ b/docs/usage/configuration/config_documentation.md @@ -3337,8 +3337,9 @@ This setting has the following sub-options: The default is 'uid'. * `attribute_requirements`: It is possible to configure Synapse to only allow logins if SAML attributes match particular values. The requirements can be listed under - `attribute_requirements` as shown in the example. All of the listed attributes must - match for the login to be permitted. + `attribute_requirements` as shown in the example. All of the listed attributes must + match for the login to be permitted. Values can be specified in a `one_of` list to allow + multiple values for an attribute. * `idp_entityid`: If the metadata XML contains multiple IdP entities then the `idp_entityid` option must be set to the entity to redirect users to. Most deployments only have a single IdP entity and so should omit this option. @@ -3419,7 +3420,9 @@ saml2_config: - attribute: userGroup value: "staff" - attribute: department - value: "sales" + one_of: + - "sales" + - "admins" idp_entityid: 'https://our_idp/entityid' ``` diff --git a/synapse/config/sso.py b/synapse/config/sso.py index d7a2187e7dc11d0a6e65fe6185c64bcea52d9a96..97b85e47ea475f585ab82e422763a3d7d41b5113 100644 --- a/synapse/config/sso.py +++ b/synapse/config/sso.py @@ -19,7 +19,7 @@ # # import logging -from typing import Any, Dict, Optional +from typing import Any, Dict, List, Optional import attr @@ -43,13 +43,23 @@ class SsoAttributeRequirement: """Object describing a single requirement for SSO attributes.""" attribute: str - # If a value is not given, than the attribute must simply exist. - value: Optional[str] + # If neither value nor one_of is given, the attribute must simply exist. This is + # only true for CAS configs which use a different JSON schema than the one below. + value: Optional[str] = None + one_of: Optional[List[str]] = None JSON_SCHEMA = { "type": "object", - "properties": {"attribute": {"type": "string"}, "value": {"type": "string"}}, - "required": ["attribute", "value"], + "properties": { + "attribute": {"type": "string"}, + "value": {"type": "string"}, + "one_of": {"type": "array", "items": {"type": "string"}}, + }, + "required": ["attribute"], + "oneOf": [ + {"required": ["value"]}, + {"required": ["one_of"]}, + ], } diff --git a/synapse/handlers/sso.py b/synapse/handlers/sso.py index cee2eefbb379a5e23ea267cee477a04a92d6b79d..531ed57110ef9d18decb16804e22f8c7d3841672 100644 --- a/synapse/handlers/sso.py +++ b/synapse/handlers/sso.py @@ -1277,12 +1277,16 @@ def _check_attribute_requirement( return False # If the requirement is None, the attribute existing is enough. - if req.value is None: + if req.value is None and req.one_of is None: return True values = attributes[req.attribute] if req.value in values: return True + if req.one_of: + for value in req.one_of: + if value in values: + return True logger.info( "SSO attribute %s did not match required value '%s' (was '%s')", diff --git a/tests/handlers/test_oidc.py b/tests/handlers/test_oidc.py index a81501979d410f328b7a492ab1c44c94e374f982..1b43ee43c65f13e6ad177de26f38d60fb912e68f 100644 --- a/tests/handlers/test_oidc.py +++ b/tests/handlers/test_oidc.py @@ -1267,6 +1267,38 @@ class OidcHandlerTestCase(HomeserverTestCase): auth_provider_session_id=None, ) + @override_config( + { + "oidc_config": { + **DEFAULT_CONFIG, + "attribute_requirements": [ + {"attribute": "test", "one_of": ["foo", "bar"]} + ], + } + } + ) + def test_attribute_requirements_one_of(self) -> None: + """Test that auth succeeds if userinfo attribute has multiple values and CONTAINS required value""" + # userinfo with "test": ["bar"] attribute should succeed. + userinfo = { + "sub": "tester", + "username": "tester", + "test": ["bar"], + } + request, _ = self.start_authorization(userinfo) + self.get_success(self.handler.handle_oidc_callback(request)) + + # check that the auth handler got called as expected + self.complete_sso_login.assert_called_once_with( + "@tester:test", + self.provider.idp_id, + request, + ANY, + None, + new_user=True, + auth_provider_session_id=None, + ) + @override_config( { "oidc_config": { diff --git a/tests/handlers/test_saml.py b/tests/handlers/test_saml.py index 6ab8fda6e7388aa475cdaf1b17c11da43fb8ae78..1aca3548267a4dba53e41cfc0c64e6b57897cdda 100644 --- a/tests/handlers/test_saml.py +++ b/tests/handlers/test_saml.py @@ -363,6 +363,52 @@ class SamlHandlerTestCase(HomeserverTestCase): auth_provider_session_id=None, ) + @override_config( + { + "saml2_config": { + "attribute_requirements": [ + {"attribute": "userGroup", "one_of": ["staff", "admin"]}, + ], + }, + } + ) + def test_attribute_requirements_one_of(self) -> None: + """The required attributes can be comma-separated.""" + + # stub out the auth handler + auth_handler = self.hs.get_auth_handler() + auth_handler.complete_sso_login = AsyncMock() # type: ignore[method-assign] + + # The response doesn't have the proper department. + saml_response = FakeAuthnResponse( + {"uid": "test_user", "username": "test_user", "userGroup": ["nogroup"]} + ) + request = _mock_request() + self.get_success( + self.handler._handle_authn_response(request, saml_response, "redirect_uri") + ) + auth_handler.complete_sso_login.assert_not_called() + + # Add the proper attributes and it should succeed. + saml_response = FakeAuthnResponse( + {"uid": "test_user", "username": "test_user", "userGroup": ["admin"]} + ) + request.reset_mock() + self.get_success( + self.handler._handle_authn_response(request, saml_response, "redirect_uri") + ) + + # check that the auth handler got called as expected + auth_handler.complete_sso_login.assert_called_once_with( + "@test_user:test", + "saml", + 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"""