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

8 files changed, 26 insertions, 97 deletions

diff --git a/etc/ironic/policy.json b/etc/ironic/policy.json
index a889e0bb0..94ac3a5b8 100644
--- a/etc/ironic/policy.json
+++ b/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"
 }
diff --git a/ironic/api/acl.py b/ironic/api/acl.py
index 85fa5325c..453b67b46 100644
--- a/ironic/api/acl.py
+++ b/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()
diff --git a/ironic/api/app.py b/ironic/api/app.py
index 8bb865baa..ec42b6c54 100644
--- a/ironic/api/app.py
+++ b/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)
 
diff --git a/ironic/api/hooks.py b/ironic/api/hooks.py
index 5162eba7f..20739a948 100644
--- a/ironic/api/hooks.py
+++ b/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()
diff --git a/ironic/common/policy.py b/ironic/common/policy.py
index 1c764daca..4326556c8 100644
--- a/ironic/common/policy.py
+++ b/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
diff --git a/ironic/tests/api/base.py b/ironic/tests/api/base.py
index f79c4412c..df227084c 100644
--- a/ironic/tests/api/base.py
+++ b/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()
 
diff --git a/ironic/tests/fake_policy.py b/ironic/tests/fake_policy.py
index 15e19db95..3851966b1 100644
--- a/ironic/tests/fake_policy.py
+++ b/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"
 }
 """
diff --git a/ironic/tests/policy.json b/ironic/tests/policy.json
deleted file mode 100644
index a889e0bb0..000000000
--- a/ironic/tests/policy.json
+++ /dev/null
@@ -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",
-}