Fix trailing slash handling regression

Also added tests

Author: theseal

src/pyff/api.py

@@ -205,8 +205,9 @@ def _d(x: Optional[str], do_split: bool = True) -> tuple[Optional[str], Optional
     # Ugly workaround bc WSGI drops double-slashes.
     path = path.replace(':/', '://')
 
-    # Ugly workaround bc request.matchdict drops trailing slashes which could be part of the entityID
-    if request.path and request.path[-1] == "/":
+    # Ugly workaround bc request.matchdict drops trailing slashes which could be part of the entityID.
+    # Only for a non-empty path - a bare /entities/ means "everything", not the entity named "/".
+    if path and request.path and request.path[-1] == "/":
         path = path + "/"
 
     msg = "handling entry={}, alias={}, path={}"

src/pyff/test/data/metadata/test04-trailing-slash-sp.xml

@@ -0,0 +1,24 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<md:EntityDescriptor xmlns:md="urn:oasis:names:tc:SAML:2.0:metadata"
+                     entityID="https://sp.example.com/saml2/metadata/">
+  <md:SPSSODescriptor protocolSupportEnumeration="urn:oasis:names:tc:SAML:2.0:protocol">
+    <md:Extensions>
+      <mdui:UIInfo xmlns:mdui="urn:oasis:names:tc:SAML:metadata:ui">
+        <mdui:DisplayName xml:lang="en">Example Service</mdui:DisplayName>
+      </mdui:UIInfo>
+    </md:Extensions>
+    <md:NameIDFormat>urn:oasis:names:tc:SAML:2.0:nameid-format:transient</md:NameIDFormat>
+    <md:AssertionConsumerService Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST"
+                                 Location="https://sp.example.com/saml2/acs/" index="0"/>
+  </md:SPSSODescriptor>
+  <md:Organization>
+    <md:OrganizationName xml:lang="en">ExampleOrg</md:OrganizationName>
+    <md:OrganizationDisplayName xml:lang="en">The Example Organisation</md:OrganizationDisplayName>
+    <md:OrganizationURL xml:lang="en">https://www.example.com/</md:OrganizationURL>
+  </md:Organization>
+  <md:ContactPerson contactType="technical">
+    <md:Company>Example Organisation</md:Company>
+    <md:SurName>Example helpdesk</md:SurName>
+    <md:EmailAddress>helpdesk@example.com</md:EmailAddress>
+  </md:ContactPerson>
+</md:EntityDescriptor>

src/pyff/test/test_md_api.py

@@ -7,11 +7,13 @@
 
 import pytest
 import requests
+from lxml import etree
 from mako.lookup import TemplateLookup
 from wsgi_intercept.interceptor import RequestsInterceptor, UrllibInterceptor
 
 from pyff.api import mkapp
 from pyff.constants import config
+from pyff.samlmd import iter_entities
 from pyff.test import SignerTestCase
 from pyff.test.test_pipeline import PipeLineTest
 
@@ -226,3 +228,83 @@ def test_api_resources(self):
             assert (last_seen - now).total_seconds() < 60
 
             assert os.path.exists(os.path.join(config.local_copy_dir, urlescape(f'file://{self.test01}')))
+
+
+class PyFFAPITestTrailingSlash(PipeLineTest):
+    """
+    A trailing slash is part of an entityID, except when there is no entityID at all -
+    a bare /entities/ selects everything, just like /entities does.
+    """
+
+    mdx = None
+    app = None
+    idp = 'https://idp.example.com/saml2/idp/metadata.php'
+    sp = 'https://sp.example.com/saml2/metadata/'
+
+    @classmethod
+    def setUpClass(cls):
+        SignerTestCase.setUpClass()
+        config.local_copy_dir = tempfile.mkdtemp()
+        cls.test01 = os.path.join(cls.datadir, 'metadata', 'test01.xml')
+        cls.test04 = os.path.join(cls.datadir, 'metadata', 'test04-trailing-slash-sp.xml')
+        cls.mdx = tempfile.NamedTemporaryFile('w').name
+        with open(cls.mdx, "w") as fd:
+            fd.write(
+                f"""
+- when update:
+    - load:
+        - {cls.test01}
+        - {cls.test04}
+- when request:
+    - select
+    - pipe:
+        - when accept application/xml:
+            - finalize:
+                cacheDuration: PT5H
+                validUntil: P10D
+            - emit application/xml
+            - break
+"""
+            )
+        cls._app = mkapp(cls.mdx)
+        cls.app = lambda *args, **kwargs: cls._app
+
+    @classmethod
+    def tearDownClass(cls):
+        SignerTestCase.tearDownClass()
+        if os.path.exists(cls.mdx):
+            os.unlink(cls.mdx)
+        if os.path.exists(config.local_copy_dir):
+            shutil.rmtree(config.local_copy_dir)
+
+    def _entity_ids(self, url, path):
+        """Return the set of entityIDs the API serves for path"""
+        r = requests.get(f'{url}{path}', headers={'Accept': 'application/xml'})
+        assert r.status_code == 200, f'{path} -> {r.status_code}'
+        t = etree.fromstring(r.content)
+        return {e.get('entityID') for e in iter_entities(t)}
+
+    def test_entities_without_trailing_slash(self):
+        with RequestsInterceptor(self.app, host='127.0.0.1', port=80) as url:
+            assert requests.post(f'{url}/api/call/update').status_code == 200
+            assert self._entity_ids(url, '/entities') == {self.idp, self.sp}
+
+    def test_entities_with_trailing_slash(self):
+        with RequestsInterceptor(self.app, host='127.0.0.1', port=80) as url:
+            assert requests.post(f'{url}/api/call/update').status_code == 200
+            assert self._entity_ids(url, '/entities/') == {self.idp, self.sp}
+
+    def test_entity_id_keeps_its_trailing_slash(self):
+        """An entityID ending in a slash must not be truncated - cf. issue #298"""
+        with RequestsInterceptor(self.app, host='127.0.0.1', port=80) as url:
+            assert requests.post(f'{url}/api/call/update').status_code == 200
+            # the unescaped form - WSGI hands us a single slash after the scheme
+            assert self._entity_ids(url, '/entities/https:/sp.example.com/saml2/metadata/') == {self.sp}
+            # ... and the escaped form
+            assert self._entity_ids(url, f'/entities/{urlescape(self.sp, safe="")}') == {self.sp}
+
+    def test_entity_id_without_trailing_slash_is_not_found(self):
+        """Dropping the slash from an entityID that has one must not match it"""
+        with RequestsInterceptor(self.app, host='127.0.0.1', port=80) as url:
+            assert requests.post(f'{url}/api/call/update').status_code == 200
+            assert self._entity_ids(url, '/entities/https:/sp.example.com/saml2/metadata') == set()