From: Salvatore Orlando <salv.orlando@gmail.com>
Date: Tue, 19 Mar 2013 18:54:58 +0000 (+0100)
Subject: Enable authZ checks for member actions
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=dc110b71c115dc9d1ac708ae62aad7d1a7218dcf;p=openstack-build%2Fneutron-build.git

Enable authZ checks for member actions

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
---

diff --git a/etc/policy.json b/etc/policy.json
index f2c304b48..4e433eec0 100644
--- a/etc/policy.json
+++ b/etc/policy.json
@@ -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",
diff --git a/quantum/api/v2/base.py b/quantum/api/v2/base.py
index 8fc46377a..8ddb19118 100644
--- a/quantum/api/v2/base.py
+++ b/quantum/api/v2/base.py
@@ -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
diff --git a/quantum/db/l3_db.py b/quantum/db/l3_db.py
index 70d1ab4fd..e7e4849af 100644
--- a/quantum/db/l3_db.py
+++ b/quantum/db/l3_db.py
@@ -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)
diff --git a/quantum/policy.py b/quantum/policy.py
index e2b548444..67211611e 100644
--- a/quantum/policy.py
+++ b/quantum/policy.py
@@ -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()