Merge pull request #191 from Ouranosinc/missing_return_fix

Add return statement

HISTORY.rst

@@ -13,6 +13,12 @@ Features / Changes
 * add constant ``MAGPIE_LOG_PRINT`` (default: ``False``) to enforce printing logs to console
   (equivalent to specifying a ``sys.stdout/stderr StreamHandler`` in ``magpie.ini``, but is not enforced anymore)
 * update logging config to avoid duplicate outputs and adjust code to respect specified config.
+* add some typing for ACL methods
+
+Bug Fixes
+~~~~~~~~~~~~~~~~~~~~~
+* fix ``Permission`` enum vs literal string usage during ACL resolution for some services and return enums when calling
+  ``ServiceInterface.permission_requested`` method.
 
 1.1.0 (2019-05-28)
 ---------------------

magpie/constants.py

@@ -74,6 +74,7 @@ def _get_default_log_level():
         _default_log_lvl = _settings.get("level", _default_log_lvl)
     except Exception:
         pass
+    return _default_log_lvl
 
 
 # ===========================

magpie/definitions/typedefs.py

@@ -54,6 +54,7 @@
     ServiceOrResourceType = Union[models.Service, models.Resource]
     ResourcePermissionType = Union[models.GroupPermission, models.UserPermission]
     AnyPermissionType = Union[Permission, ResourcePermissionType, Str]
+    AccessControlListType = List[Tuple[Str, Str, Str]]
 
     TestAppOrUrlType = Union[Str, TestApp]
     AnyMagpieTestType = Union[Type[Base_Magpie_TestCase], Base_Magpie_TestCase, TestAppOrUrlType]

magpie/services.py

@@ -15,7 +15,9 @@
 from typing import TYPE_CHECKING
 from six import with_metaclass
 if TYPE_CHECKING:
-    from magpie.definitions.typedefs import Str, List, Dict, Type, ResourcePermissionType  # noqa: F401
+    from magpie.definitions.typedefs import (  # noqa: F401
+        AccessControlListType, Str, List, Dict, Type, ResourcePermissionType
+    )
     from magpie.definitions.pyramid_definitions import Request  # noqa: F401
 
 
@@ -59,12 +61,14 @@ class ServiceInterface(with_metaclass(ServiceMeta)):
     def __init__(self, service, request):
         self.service = service
         self.request = request
-        self.acl = []
+        self.acl = []                   # type: AccessControlListType
         self.parser = ows_parser_factory(request)
         self.parser.parse(self.params_expected)
 
     @property
     def __acl__(self):
+        # type: () -> AccessControlListType
+        """List of access control rules defining (outcome, user/group, permission) combinations."""
         raise NotImplementedError
 
     def expand_acl(self, resource, user):
@@ -87,9 +91,13 @@ def expand_acl(self, resource, user):
                         self.acl.append((outcome, EVERYONE, perm_name,))
 
     def permission_requested(self):
-        # type: () -> Str
+        # type: () -> Permission
         try:
-            return self.parser.params[u"request"]
+            req = self.parser.params[u"request"]
+            perm = Permission.get(req)
+            if perm is None:
+                raise NotImplementedError("Undefined 'Permission' from 'request' parameter: {!s}".format(req))
+            return perm
         except KeyError as ex:
             # if 'ServiceInterface', 'params_expected' is empty and will raise a KeyError
             raise NotImplementedError("Exception: [{!r}] for class '{}'.".format(ex, type(self)))
@@ -225,7 +233,7 @@ def __acl__(self):
                 netcdf_file = netcdf_file.rsplit("/", 1)[0]
 
         else:
-            return [(ALLOW, EVERYONE, permission_requested,)]
+            return [(ALLOW, EVERYONE, permission_requested.value,)]
 
         if netcdf_file:
             ax.verify_param("outputs/", paramCompare=netcdf_file, httpError=HTTPNotFound,
@@ -349,10 +357,10 @@ def _route_acl(self, sub_api_route=None):
         # process read/write-match specific permission access
         # (convert exact route 'match' to read/write counterparts only if matching last item's permissions)
         for i in range(match_index, len(self.acl)):
-            if self.acl[i][2] == Permission.READ_MATCH:
-                self.acl[i] = (self.acl[i][0], self.acl[i][1], Permission.READ)
-            if self.acl[i][2] == Permission.WRITE_MATCH:
-                self.acl[i] = (self.acl[i][0], self.acl[i][1], Permission.WRITE)
+            if Permission.get(self.acl[i][2]) == Permission.READ_MATCH:
+                self.acl[i] = (self.acl[i][0], self.acl[i][1], Permission.READ.value)
+            if Permission.get(self.acl[i][2]) == Permission.WRITE_MATCH:
+                self.acl[i] = (self.acl[i][0], self.acl[i][1], Permission.WRITE.value)
 
         return self.acl
 
@@ -470,7 +478,7 @@ def __acl__(self):
         return self.acl
 
     def permission_requested(self):
-        return Permission.READ.value
+        return Permission.READ
 
 
 SERVICE_TYPE_DICT = dict()

magpie/utils.py

@@ -328,7 +328,7 @@ def values(cls):
     def get(cls, key_or_value, default=None):
         # type: (AnyKey, Optional[Any]) -> Optional[_TC]
         """
-        Finds a enum entry by defined name or its value.
+        Finds an enum entry by defined name or its value.
         Returns the entry directly if it is already a valid enum.
         """
         if key_or_value in cls: