From 255645055783f442e3475dd4121692ba056698ca Mon Sep 17 00:00:00 2001
From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
Date: Fri, 17 Jan 2020 15:32:51 +0000
Subject: [PATCH] Block new registrations based on a domain blacklist (#2)

---
 README.md                                     | 18 +++++-
 .../mapping_provider.py                       | 60 ++++++++++++++++---
 .../saml_maps/nameformat_basic.py             |  5 ++
 .../saml_maps/nameformat_uri.py               | 11 ++++
 setup.cfg                                     |  3 +-
 tests/test_attributes.py                      | 29 +++++++--
 tests/test_saml_response.xml                  | 15 ++---
 7 files changed, 119 insertions(+), 22 deletions(-)
 create mode 100644 matrix_synapse_saml_mozilla/saml_maps/nameformat_basic.py
 create mode 100644 matrix_synapse_saml_mozilla/saml_maps/nameformat_uri.py

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 @@
         <ns0:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema" xsi:type="xs:string">testuser@domain.com</ns0:AttributeValue>
       </ns0:Attribute>
       <ns0:Attribute Name="displayName" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:basic">
-        <ns0:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema" xsi:type="xs:string">Jan de
-                 Mooij</ns0:AttributeValue>
+        <ns0:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema" xsi:type="xs:string">Test Testuser</ns0:AttributeValue>
       </ns0:Attribute>
       <ns0:Attribute Name="givenname" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:basic">
-        <ns0:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema" xsi:type="xs:string">Jan</ns0:AttributeValue>
+        <ns0:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema" xsi:type="xs:string">Test</ns0:AttributeValue>
       </ns0:Attribute>
       <ns0:Attribute Name="surname" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:basic">
-        <ns0:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema" xsi:type="xs:string">de
-                 Mooij</ns0:AttributeValue>
+        <ns0:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema" xsi:type="xs:string">Testuser</ns0:AttributeValue>
       </ns0:Attribute>
       <ns0:Attribute Name="http://schemas.xmlsoap.org/claims/Group" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri">
         <ns0:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema" xsi:type="xs:string">everyone</ns0:AttributeValue>
@@ -55,12 +53,11 @@
         <ns0:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema" xsi:type="xs:boolean">false</ns0:AttributeValue>
       </ns0:Attribute>
       <ns0:Attribute Name="http://schemas.auth0.com/nickname" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri">
-        <ns0:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema" xsi:type="xs:string">Jan de
-                 Mooij</ns0:AttributeValue>
+        <ns0:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema" xsi:type="xs:string">Test Testuser</ns0:AttributeValue>
       </ns0:Attribute>
       <ns0:Attribute Name="http://schemas.auth0.com/emails" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri">
         <ns0:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema" xsi:type="xs:string">testuser@domain.com</ns0:AttributeValue>
-        <ns0:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema" xsi:type="xs:string">other@domain.com</ns0:AttributeValue>
+        <ns0:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema" xsi:type="xs:string">other@otherdomain.com</ns0:AttributeValue>
       </ns0:Attribute>
       <ns0:Attribute Name="http://schemas.auth0.com/dn" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri">
         <ns0:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema" xsi:type="xs:string">mail=testuser@domain.com,o=com,dc=domain</ns0:AttributeValue>
@@ -69,7 +66,7 @@
         <ns0:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema" xsi:type="xs:string">mail=testuser@domain.com,o=com,dc=domain</ns0:AttributeValue>
       </ns0:Attribute>
       <ns0:Attribute Name="http://schemas.auth0.com/email_aliases" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri">
-        <ns0:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema" xsi:type="xs:string">other@domain.com</ns0:AttributeValue>
+        <ns0:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema" xsi:type="xs:string">other@otherdomain.com</ns0:AttributeValue>
       </ns0:Attribute>
       <ns0:Attribute Name="http://schemas.auth0.com/_HRData" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri">
         <ns0:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema" xsi:type="xs:anyType">[object Object]</ns0:AttributeValue>