Merge pull request #634 from Ouranosinc/thredds-prefix-path-fix

Author: fmigneault

CHANGES.rst

@@ -9,7 +9,7 @@ Changes
 `Unreleased <https://github.com/Ouranosinc/Magpie/tree/master>`_ (latest)
 ------------------------------------------------------------------------------------
 
-* Nothing new for the moment.
+* Allow ``metadata_type.prefixes`` and ``data_type.prefixes`` in ``ServiceTHREDDS`` configuration to contain ``/`` character.
 
 .. _changes_4.1.1:
 
@@ -563,7 +563,7 @@ Features / Changes
   `Ouranosinc/Magpie#473 <https://github.com/Ouranosinc/Magpie/pull/473>`_,
   see also
   `Avoid Accessing the Authentication Policy
-  <https://docs.pylonsproject.org/projects/pyramid_tm/en/latest/#avoid-accessing-the-authentication-policy>`_).
+  <https://docs.pylonsproject.org/projects/pyramid-tm/en/latest/#avoid-accessing-the-authentication-policy>`_).
 
 .. _changes_3.17.1:
 

docs/services.rst

@@ -301,6 +301,8 @@ by the below configuration.
               - dap4
               - wcs
               - wms
+              - ncss/grid
+              - ncss/point
 
 ..  warning:: Regular Expression Patterns
 
@@ -322,6 +324,11 @@ by the below configuration.
     API entrypoint and `Tomcat` service running it. If this feature is not needed, it can be disabled by setting the
     parameter to ``null``.
 
+.. versionchanged:: 4.2.0
+    ``prefixes`` can now contain a ``/`` character. This allows `ServiceTHREDDS`_ to properly handle `THREDDS`_ services
+    that have multiple path parts. For example, starting with `THREDDS`_ version 5, the ``ncss`` service contains two
+    sub-services which are accessed using the path prefixes ``ncss/grid`` and ``ncss/point``.
+
 Assuming a proxy intended to receive incoming requests configured with :class:`magpie.adapter.MagpieAdapter` such that
 ``{PROXY_URL}`` is the base path, the following path would point toward the registered service with the above YAML
 configuration.

magpie/services.py

@@ -1382,6 +1382,21 @@ def get_path_parts(self):
                 return path_parts[1:]  # remove extra '' added by split
         return path_parts
 
+    def _get_path_prefix_permission(self, path_parts):
+        # type: (List[str]) -> Tuple[Optional[Str], PermissionRequested]
+        cfg = self.get_config()
+        # first to favor BROWSE over READ prefix conflicts
+        for prefixes, permission in ((cfg["metadata_type"]["prefixes"], Permission.BROWSE),
+                                     (cfg["data_type"]["prefixes"], Permission.READ)):
+            for pattern_prefix in prefixes:
+                if not path_parts and pattern_prefix is None:
+                    return pattern_prefix, permission
+                if pattern_prefix is not None:
+                    path_prefix = "/".join(path_parts[:pattern_prefix.count("/") + 1])
+                    if self.is_match(path_prefix, pattern_prefix) is not None:
+                        return path_prefix, permission
+        return None, None
+
     @staticmethod
     def is_match(value, pattern):
         # type: (Str, Str) -> Optional[Str]
@@ -1396,11 +1411,14 @@ def is_match(value, pattern):
     def resource_requested(self):
         # type: () -> TargetResourceRequested
         path_parts = self.get_path_parts()
-
+        path_prefix, _ = self._get_path_prefix_permission(path_parts)
         # handle optional prefix as targeting the service directly
         if not path_parts or len(path_parts) < 2:
             return self.service, True
-        path_parts = path_parts[1:]
+        if path_prefix:
+            path_parts = path_parts[path_prefix.count("/") + 1:]
+        else:
+            path_parts = path_parts[1:]
         cfg = self.get_config()
 
         # find deepest possible resource matching either Directory or File by name
@@ -1431,21 +1449,9 @@ def resource_requested(self):
 
     def permission_requested(self):
         # type: () -> PermissionRequested
-        cfg = self.get_config()
         path_parts = self.get_path_parts()
-        path_prefix = None  # in case of no `<prefix>`, simulate as `null`
-        if path_parts:
-            path_prefix = path_parts[0]
-        for prefixes, permission in [
-            (cfg["metadata_type"]["prefixes"], Permission.BROWSE),  # first to favor BROWSE over READ prefix conflicts
-            (cfg["data_type"]["prefixes"], Permission.READ),
-        ]:
-            for pattern_prefix in prefixes:  # type: Str
-                if path_prefix is None and pattern_prefix is None:
-                    return permission
-                if self.is_match(path_prefix, pattern_prefix) is not None:
-                    return permission
-        return None  # automatically deny
+        _, permission = self._get_path_prefix_permission(path_parts)
+        return permission
 
 
 class ServiceGeoserverWPS(ServiceGeoserverBase, ServiceWPS):  # order important to call overridden class properties

tests/test_services.py

@@ -504,6 +504,7 @@ def test_ServiceTHREDDS_custom_config(self):
                                     # only allow these variants, other should be blocked ("dap4", "wcs", "wms")
                                     - dodsC
                                     - fileServer
+                                    - ncss/grid
                             metadata_type:
                                 prefixes:
                                     # only allow these variants, others should be blocked ("ncml", "uddc", "iso")
@@ -531,7 +532,7 @@ def test_ServiceTHREDDS_custom_config(self):
         utils.check_val_is_in("data_type", svc_config)
         utils.check_val_is_in("metadata_type", svc_config)
         utils.check_val_equal(svc_config["file_patterns"], [".*\\.ncml", ".*\\.nc"])
-        utils.check_val_equal(svc_config["data_type"]["prefixes"], ["dodsC", "fileServer"])
+        utils.check_val_equal(svc_config["data_type"]["prefixes"], ["dodsC", "fileServer", "ncss/grid"])
         utils.check_val_equal(svc_config["metadata_type"]["prefixes"], [None, "catalog"])
 
         # create resources
@@ -565,7 +566,7 @@ def test_ServiceTHREDDS_custom_config(self):
         # (same resource because matching '.*.nc' regex, NCML file matched by other '.*.ncml' regex, so other Resource)
         # only accessible via specified data prefixes
         allowed_files = [file_name, file_html_name]
-        known_prefixes = ["dodsC", "fileServer"]
+        known_prefixes = ["dodsC", "fileServer", "ncss/grid"]
         for prefix in known_prefixes:
             for test_file in allowed_files:
                 path = "/ows/proxy/{}/{}/{}/{}".format(svc_name, prefix, dir_name, test_file)
@@ -580,7 +581,8 @@ def test_ServiceTHREDDS_custom_config(self):
             utils.check_raises(lambda: self.ows.check_request(req), OWSAccessForbidden, msg=msg)
 
         # using unknown prefixes, otherwise allowed file should always be denied
-        unknown_prefixes = ["ncml", "dap4"]  # purposely take normally allowed THREDDS prefixes, validate active config
+        # purposely take normally allowed THREDDS prefixes, validate active config
+        unknown_prefixes = ["ncml", "dap4", "ncss/point", "ncss", "grid"]
         allowed_resources = [dir_name, "{}/{}".format(dir_name, file_name), "{}/{}".format(dir_name, file_html_name)]
         for prefix in unknown_prefixes:
             for target in allowed_resources: