diff --git a/README.md b/README.md index af0b874..97b6b50 100644 --- a/README.md +++ b/README.md @@ -42,7 +42,15 @@ listeners: Synapse allows SAML mapping providers to specify custom configuration through the `saml2_config.user_mapping_provider.config` option. -There are no options currently supported by this provider. +Currently the following options are supported: + + * `use_name_id_for_remote_uid`: if set to `False`, we will use the SAML + attribute mapped to `uid` to identify the remote user instead of the `NameID` + from the assertion. `True` by default. + + * `domain_block_file`: should point a file containing a list of domains (one + per line); users who have an email address on any of these domains will be + blocked from registration. ## Implementation notes @@ -63,3 +71,11 @@ errors in the codebase. This repository uses `unittest` to run the tests located in the `tests` directory. They can be ran with `tox -e tests`. + +### Making a release + +``` +git tag vX.Y +python3 setup.py sdist +twine upload dist/matrix-synapse-saml-mozilla-X.Y.tar.gz +``` diff --git a/matrix_synapse_saml_mozilla/mapping_provider.py b/matrix_synapse_saml_mozilla/mapping_provider.py index 4f8891c..c3c405a 100644 --- a/matrix_synapse_saml_mozilla/mapping_provider.py +++ b/matrix_synapse_saml_mozilla/mapping_provider.py @@ -16,7 +16,7 @@ import logging import random import string import time -from typing import Tuple +from typing import Set, Tuple import attr import saml2.response @@ -40,7 +40,8 @@ MAPPING_SESSION_VALIDITY_PERIOD_MS = 15 * 60 * 1000 @attr.s class SamlConfig(object): - use_name_id_for_remote_uid = attr.ib(type=bool) + use_name_id_for_remote_uid = attr.ib(type=bool, default=True) + domain_block_list = attr.ib(type=Set[str], default={}) class SamlMappingProvider(object): @@ -55,6 +56,8 @@ class SamlMappingProvider(object): self._random = random.SystemRandom() self._config = parsed_config + logger.info("Domain block list: %s", self._config.domain_block_list) + def get_remote_user_id( self, saml_response: saml2.response.AuthnResponse, client_redirect_url: str ): @@ -69,7 +72,7 @@ class SamlMappingProvider(object): try: return saml_response.ava["uid"][0] except KeyError: - logger.warning("SAML2 response lacks a 'uid' attestation") + logger.warning("SAML2 response lacks a 'uid' attribute") raise CodeMessageException(400, "'uid' not in SAML2 response") def saml_response_to_user_attributes( @@ -97,6 +100,29 @@ class SamlMappingProvider(object): expire_old_sessions() + # check the user's emails against our block list + if "emails" not in saml_response.ava: + logger.warning("SAML2 response lacks an 'emails' attribute") + raise CodeMessageException(400, "'emails' not in SAML2 response") + + for email in saml_response.ava["emails"]: + parts = email.rsplit("@", 1) + if len(parts) != 2: + logger.warning( + "Rejecting registration from remote user %s with unparsable email %s", + remote_user_id, + email, + ) + raise CodeMessageException(403, "Forbidden") + + if parts[1].lower() in self._config.domain_block_list: + logger.warning( + "Rejecting registration from remote user %s with blacklisted email %s", + remote_user_id, + email, + ) + raise CodeMessageException(403, "Forbidden") + # make up a cryptorandom session id session_id = "".join( self._random.choice(string.ascii_letters) for _ in range(16) @@ -128,9 +154,23 @@ class SamlMappingProvider(object): Returns: SamlConfig: A custom config object """ - return SamlConfig( - use_name_id_for_remote_uid=config.get("use_name_id_for_remote_uid"), - ) + parsed = SamlConfig() + if "use_name_id_for_remote_uid" in config: + parsed.use_name_id_for_remote_uid = config["use_name_id_for_remote_uid"] + + domain_block_file = config.get("domain_block_file") + if domain_block_file: + try: + with open(domain_block_file, encoding="ascii") as fh: + parsed.domain_block_list = { + line.strip().lower() for line in fh.readlines() + } + except Exception as e: + raise Exception( + "Error reading domain block file %s: %s" % (domain_block_file, e) + ) + + return parsed @staticmethod def get_saml_attributes(config: SamlConfig) -> Tuple[set, set]: @@ -145,4 +185,10 @@ class SamlMappingProvider(object): second set consists of those attributes which can be used if available, but are not necessary """ - return {"uid"}, {"displayName"} + required = set() + optional = {"uid", "emails", "displayName"} + + if not config.use_name_id_for_remote_uid: + required += "uid" + + return required, optional diff --git a/matrix_synapse_saml_mozilla/saml_maps/nameformat_basic.py b/matrix_synapse_saml_mozilla/saml_maps/nameformat_basic.py new file mode 100644 index 0000000..b2e8734 --- /dev/null +++ b/matrix_synapse_saml_mozilla/saml_maps/nameformat_basic.py @@ -0,0 +1,5 @@ +MAP = { + "identifier": "urn:oasis:names:tc:SAML:2.0:attrname-format:basic", + "fro": {"displayName": "displayName"}, + "to": {"displayName": "displayName"}, +} diff --git a/matrix_synapse_saml_mozilla/saml_maps/nameformat_uri.py b/matrix_synapse_saml_mozilla/saml_maps/nameformat_uri.py new file mode 100644 index 0000000..a719b24 --- /dev/null +++ b/matrix_synapse_saml_mozilla/saml_maps/nameformat_uri.py @@ -0,0 +1,11 @@ +MAP = { + "identifier": "urn:oasis:names:tc:SAML:2.0:attrname-format:uri", + "fro": { + "http://schemas.xmlsoap.org/ws/2005/05/identity/claims/nameidentifier": "uid", + "http://schemas.auth0.com/emails": "emails", + }, + "to": { + "emails": "http://schemas.auth0.com/emails", + "uid": "http://schemas.xmlsoap.org/ws/2005/05/identity/claims/nameidentifier", + }, +} diff --git a/setup.cfg b/setup.cfg index f3d6356..c3fb81f 100644 --- a/setup.cfg +++ b/setup.cfg @@ -1,7 +1,8 @@ [flake8] max-line-length = 90 # W503 requires that binary operators be at the end, not start, of lines. Erik doesn't like it. -ignore = W503 +# E501: Line too long (black enforces this for us) +ignore = W503,E501 [isort] line_length = 88 diff --git a/tests/test_attributes.py b/tests/test_attributes.py index f99fada..8083d60 100644 --- a/tests/test_attributes.py +++ b/tests/test_attributes.py @@ -22,9 +22,10 @@ from saml2.config import SPConfig from saml2.response import AuthnResponse from saml2.sigver import CryptoBackend, SecurityContext -from synapse.api.errors import RedirectException +from synapse.api.errors import CodeMessageException, RedirectException from matrix_synapse_saml_mozilla._sessions import username_mapping_sessions +from matrix_synapse_saml_mozilla.mapping_provider import SamlConfig, SamlMappingProvider from . import create_mapping_provider @@ -33,6 +34,7 @@ class FakeResponse: def __init__(self, source_uid, display_name): self.ava = { "uid": [source_uid], + "emails": [], } if display_name: @@ -49,7 +51,13 @@ def _load_test_response() -> AuthnResponse: ).decode("utf-8") config = SPConfig() - config.load({}) + config.load( + { + "attribute_map_dir": pkg_resources.resource_filename( + "matrix_synapse_saml_mozilla", "saml_maps" + ) + } + ) assert config.attribute_converters is not None response = AuthnResponse( @@ -57,6 +65,7 @@ def _load_test_response() -> AuthnResponse: attribute_converters=config.attribute_converters, entity_id="https://host/_matrix/saml2/metadata.xml", allow_unsolicited=True, + allow_unknown_attributes=True, # tell it not to check the `destination` asynchop=False, # tell it not to check the issue time @@ -70,7 +79,7 @@ def _load_test_response() -> AuthnResponse: class SamlUserAttributeTestCase(unittest.TestCase): def test_get_remote_user_id_from_name_id(self): resp = _load_test_response() - provider = create_mapping_provider({"use_name_id_for_remote_uid": True}) + provider = create_mapping_provider() remote_user_id = provider.get_remote_user_id(resp, "",) self.assertEqual(remote_user_id, "test@domain.com") @@ -78,7 +87,7 @@ class SamlUserAttributeTestCase(unittest.TestCase): """Creates a dummy response, feeds it to the provider and checks that it redirects to the username picker. """ - provider = create_mapping_provider() + provider = create_mapping_provider({"use_name_id_for_remote_uid": False}) response = FakeResponse(123435, "Jonny") # we expect this to redirect to the username picker @@ -105,3 +114,15 @@ class SamlUserAttributeTestCase(unittest.TestCase): expected_expiry = (time.time() + 15 * 60) * 1000 self.assertGreaterEqual(session.expiry_time_ms, expected_expiry - 1000) self.assertLessEqual(session.expiry_time_ms, expected_expiry + 1000) + + def test_reject_blacklisted_email(self): + config = SamlConfig( + use_name_id_for_remote_uid=True, domain_block_list={"otherdomain.com"} + ) + provider = SamlMappingProvider(config, None) + resp = _load_test_response() + + with self.assertRaises(CodeMessageException) as e: + provider.saml_response_to_user_attributes(resp, 0, "http://client/") + + self.assertEqual(e.exception.code, 403) diff --git a/tests/test_saml_response.xml b/tests/test_saml_response.xml index d733056..bd42dbc 100644 --- a/tests/test_saml_response.xml +++ b/tests/test_saml_response.xml @@ -26,15 +26,13 @@ testuser@domain.com - Jan de - Mooij + Test Testuser - Jan + Test - de - Mooij + Testuser everyone @@ -55,12 +53,11 @@ false - Jan de - Mooij + Test Testuser testuser@domain.com - other@domain.com + other@otherdomain.com mail=testuser@domain.com,o=com,dc=domain @@ -69,7 +66,7 @@ mail=testuser@domain.com,o=com,dc=domain - other@domain.com + other@otherdomain.com [object Object]