]> review.fuel-infra Code Review - openstack-build/neutron-build.git/commitdiff
Enable authZ checks for member actions
authorSalvatore Orlando <salv.orlando@gmail.com>
Tue, 19 Mar 2013 18:54:58 +0000 (19:54 +0100)
committerSalvatore Orlando <salv.orlando@gmail.com>
Sat, 30 Mar 2013 01:06:38 +0000 (02:06 +0100)
Blueprint make-authz-orthogonal

This implements work item #1 of the blueprint.
This patch enables authZ checks for 'member actions' in the base
controller and removes explicit checks from l3_db.
This patch also addresses a small glitch in the policy engine which
was assuming the request always had a body.

Change-Id: I7e0f386eedcfff24ea1fee7294bbadd6c5ec781c

etc/policy.json
quantum/api/v2/base.py
quantum/db/l3_db.py
quantum/policy.py

index f2c304b4849717496e10bac524e6173de62e94fb..4e433eec011e9ec205bf9d9492f4ba40e013daf0 100644 (file)
@@ -12,8 +12,6 @@
 
     "extension:router:view": "rule:regular_user",
     "extension:router:set": "rule:admin_only",
-    "extension:router:add_router_interface": "rule:admin_or_owner",
-    "extension:router:remove_router_interface": "rule:admin_or_owner",
 
     "extension:port_binding:view": "rule:admin_only",
     "extension:port_binding:set": "rule:admin_only",
index 8fc46377a365b8c42ef88e867b8700c0ac66c1ee..8ddb1911890222e55797a9acdfb7e07a81bb8339 100644 (file)
@@ -141,13 +141,25 @@ class Controller(object):
     def __getattr__(self, name):
         if name in self._member_actions:
             def _handle_action(request, id, **kwargs):
-                if 'body' in kwargs:
-                    body = kwargs.pop('body')
-                    return getattr(self._plugin, name)(request.context, id,
-                                                       body, **kwargs)
-                else:
-                    return getattr(self._plugin, name)(request.context, id,
-                                                       **kwargs)
+                arg_list = [request.context, id]
+                # Fetch the resource and verify if the user can access it
+                try:
+                    resource = self._item(request, id, True)
+                except exceptions.PolicyNotAuthorized:
+                    raise webob.exc.HTTPNotFound()
+                body = kwargs.pop('body', None)
+                # Explicit comparison with None to distinguish from {}
+                if body is not None:
+                    arg_list.append(body)
+                # TODO(salvatore-orlando): bp/make-authz-ortogonal
+                # The body of the action request should be included
+                # in the info passed to the policy engine
+                # Enforce policy, if any, for this action
+                # It is ok to raise a 403 because accessibility to the
+                # object was checked earlier in this method
+                policy.enforce(request.context, name, resource,
+                               plugin=self._plugin)
+                return getattr(self._plugin, name)(*arg_list, **kwargs)
             return _handle_action
         else:
             raise AttributeError
index 70d1ab4fdd857aa9008481e6ac7a6f3013d0f95e..e7e4849af4b12aa662eeb2d9993a3b4c2dafafe5 100644 (file)
@@ -310,19 +310,10 @@ class L3_NAT_db_mixin(l3.RouterPluginBase):
             pass
 
     def add_router_interface(self, context, router_id, interface_info):
-        # make sure router exists
-        router = self._get_router(context, router_id)
         if not interface_info:
             msg = _("Either subnet_id or port_id must be specified")
             raise q_exc.BadRequest(resource='router', msg=msg)
 
-        try:
-            policy.enforce(context,
-                           "extension:router:add_router_interface",
-                           self._make_router_dict(router))
-        except q_exc.PolicyNotAuthorized:
-            raise l3.RouterNotFound(router_id=router_id)
-
         if 'port_id' in interface_info:
             if 'subnet_id' in interface_info:
                 msg = _("Cannot specify both subnet-id and port-id")
@@ -394,15 +385,6 @@ class L3_NAT_db_mixin(l3.RouterPluginBase):
                     router_id=router_id, subnet_id=subnet_id)
 
     def remove_router_interface(self, context, router_id, interface_info):
-        # make sure router exists
-        router = self._get_router(context, router_id)
-        try:
-            policy.enforce(context,
-                           "extension:router:remove_router_interface",
-                           self._make_router_dict(router))
-        except q_exc.PolicyNotAuthorized:
-            raise l3.RouterNotFound(router_id=router_id)
-
         if not interface_info:
             msg = _("Either subnet_id or port_id must be specified")
             raise q_exc.BadRequest(resource='router', msg=msg)
index e2b548444c9476a98ccadb6d6f353d47e49ab918..67211611e67404e91567000ff55028d35acd5394 100644 (file)
@@ -174,6 +174,9 @@ def check(context, action, target, plugin=None):
     :return: Returns True if access is permitted else False.
     """
     init()
+    # Compare with None to distinguish case in which target is {}
+    if target is None:
+        target = {}
     real_target = _build_target(action, target, plugin, context)
     match_rule = _build_match_rule(action, real_target)
     credentials = context.to_dict()
@@ -196,6 +199,9 @@ def enforce(context, action, target, plugin=None):
     """
 
     init()
+    # Compare with None to distinguish case in which target is {}
+    if target is None:
+        target = {}
     real_target = _build_target(action, target, plugin, context)
     match_rule = _build_match_rule(action, real_target)
     credentials = context.to_dict()