Fix policies

Policy file contained malformed content so the policy
engine failed to parse it. That was the reason of rejecting
all requests, if authentication was enabled.

This patch also updates policies to get rid of unused policies
and use GenericCheck to check for admin API.

After changes mentioned above some unused code appeared in
ironic.common.policy and so it was cleaned up.

Closes-bug: #1236371
Change-Id: Ie1dbda11561a9e7068d240a19f9fb98eae121c94

etc/ironic/policy.json

@@ -1,6 +1,5 @@
 {
+    "admin": "role:admin or role:administrator",
     "admin_api": "is_admin:True",
-    "admin_or_owner": "is_admin:True or project_id:%(project_id)s",
-    "is_admin":  "role:admin or role:administrator",
-    "default": "rule:admin_or_owner",
+    "default": "rule:admin_api"
 }

ironic/api/acl.py

@@ -20,11 +20,8 @@
 
 from keystoneclient.middleware import auth_token as keystone_auth_token
 from oslo.config import cfg
-from pecan import hooks
-from webob import exc
 
 from ironic.api.middleware import auth_token
-from ironic.common import policy
 
 
 OPT_GROUP_NAME = 'keystone_authtoken'
@@ -56,17 +53,3 @@ def install(app, conf, public_routes):
     return auth_token.AuthTokenMiddleware(app,
                                           conf=keystone_config,
                                           public_api_routes=public_routes)
-
-
-class AdminAuthHook(hooks.PecanHook):
-    """Verify that the user has admin rights.
-
-    Checks whether the request context is an admin context and
-    rejects the request otherwise.
-
-    """
-    def before(self, state):
-        ctx = state.request.context
-
-        if not policy.check_is_admin(ctx) and not ctx.is_public_api:
-            raise exc.HTTPForbidden()

ironic/api/app.py

@@ -23,6 +23,7 @@ from ironic.api import acl
 from ironic.api import config
 from ironic.api import hooks
 from ironic.api import middleware
+from ironic.common import policy
 
 auth_opts = [
     cfg.StrOpt('auth_strategy',
@@ -41,6 +42,8 @@ def get_pecan_config():
 
 
 def setup_app(pecan_config=None, extra_hooks=None):
+    policy.init()
+
     app_hooks = [hooks.ConfigHook(),
                  hooks.DBHook(),
                  hooks.ContextHook(pecan_config.app.acl_public_routes),
@@ -52,7 +55,7 @@ def setup_app(pecan_config=None, extra_hooks=None):
         pecan_config = get_pecan_config()
 
     if pecan_config.app.enable_acl:
-        app_hooks.append(acl.AdminAuthHook())
+        app_hooks.append(hooks.AdminAuthHook())
 
     pecan.configuration.set_config(dict(pecan_config), overwrite=True)
 

ironic/api/hooks.py

@@ -18,6 +18,7 @@
 
 from oslo.config import cfg
 from pecan import hooks
+from webob import exc
 
 from ironic.common import context
 from ironic.common import utils
@@ -74,7 +75,7 @@ class ContextHook(hooks.PecanHook):
         auth_token = state.request.headers.get('X-Auth-Token', None)
         creds = {'roles': state.request.headers.get('X-Roles', '').split(',')}
 
-        is_admin = policy.check('is_admin', state.request.headers, creds)
+        is_admin = policy.check('admin', state.request.headers, creds)
 
         path = utils.safe_rstrip(state.request.path, '/')
         is_public_api = path in self.public_api_routes
@@ -94,3 +95,18 @@ class RPCHook(hooks.PecanHook):
 
     def before(self, state):
         state.request.rpcapi = rpcapi.ConductorAPI()
+
+
+class AdminAuthHook(hooks.PecanHook):
+    """Verify that the user has admin rights.
+
+    Checks whether the request context is an admin context and
+    rejects the request otherwise.
+
+    """
+    def before(self, state):
+        ctx = state.request.context
+        is_admin_api = policy.check('admin_api', {}, ctx.to_dict())
+
+        if not is_admin_api and not ctx.is_public_api:
+            raise exc.HTTPForbidden()

ironic/common/policy.py

@@ -66,66 +66,3 @@ def init():
 def _set_rules(data):
     default_rule = CONF.policy_default_rule
     policy.set_rules(policy.Rules.load_json(data, default_rule))
-
-
-def enforce(context, action, target, do_raise=True):
-    """Verifies that the action is valid on the target in this context.
-
-       :param context: ironic context
-       :param action: string representing the action to be checked
-           this should be colon separated for clarity.
-           i.e. ``compute:create_instance``,
-           ``compute:attach_volume``,
-           ``volume:attach_volume``
-       :param target: dictionary representing the object of the action
-           for object creation this should be a dictionary representing the
-           location of the object e.g. ``{'project_id': context.project_id}``
-       :param do_raise: if True (the default), raises PolicyNotAuthorized;
-           if False, returns False
-
-       :raises ironic.exception.PolicyNotAuthorized: if verification fails
-           and do_raise is True.
-
-       :return: returns a non-False value (not necessarily "True") if
-           authorized, and the exact value False if not authorized and
-           do_raise is False.
-    """
-    init()
-
-    credentials = context.to_dict()
-
-    # Add the exception arguments if asked to do a raise
-    extra = {}
-    if do_raise:
-        extra.update(exc=exception.PolicyNotAuthorized, action=action)
-
-    return policy.check(action, target, credentials, **extra)
-
-
-def check_is_admin(context):
-    """Whether or not role contains 'admin' role according to policy setting.
-
-    """
-    init()
-
-    credentials = context.to_dict()
-    target = credentials
-
-    return policy.check('context_is_admin', target, credentials)
-
-
-@policy.register('context_is_admin')
-class IsAdminCheck(policy.Check):
-    """An explicit check for is_admin."""
-
-    def __init__(self, kind, match):
-        """Initialize the check."""
-
-        self.expected = (match.lower() == 'true')
-
-        super(IsAdminCheck, self).__init__(kind, str(self.expected))
-
-    def __call__(self, target, creds):
-        """Determine whether is_admin matches the requested value."""
-
-        return creds['is_admin'] == self.expected

ironic/tests/api/base.py

@@ -42,8 +42,6 @@ class FunctionalTest(base.DbTestCase):
     def setUp(self):
         super(FunctionalTest, self).setUp()
         cfg.CONF.set_override("auth_version", "v2.0", group=acl.OPT_GROUP_NAME)
-        cfg.CONF.set_override("policy_file",
-                              self.path_get('tests/policy.json'))
         self.app = self._make_app()
         self.dbapi = dbapi.get_instance()
 

ironic/tests/fake_policy.py

@@ -17,9 +17,8 @@
 
 policy_data = """
 {
-    "admin_api": "role:admin",
-    "admin_or_owner": "is_admin:True or project_id:%(project_id)s",
-    "is_admin": "role:admin or role:administrator",
-    "default": "rule:admin_or_owner"
+    "admin": "role:admin or role:administrator",
+    "admin_api": "is_admin:True",
+    "default": "rule:admin_api"
 }
 """

ironic/tests/policy.json

@@ -1,6 +0,0 @@
-{
-    "admin_api": "is_admin:True",
-    "admin_or_owner": "is_admin:True or project_id:%(project_id)s",
-    "is_admin":  "role:admin or role:administrator",
-    "default": "rule:admin_or_owner",
-}