fix: Use superior oslo.policy authorize over enforce

`oslo.policy` supports both enforce and authorize. authorize is
stricter because it'll raise an exception if the policy action is
not found in the list of registered rules. This means that attempting
to enforce anything not found in ``armada.common.policies`` will error
out with a 'Policy not registered' message and 403 status code.

This problem manifests itself through such cases: [0]

Please reference the oslo.policy docs on authorize [1] and
enforce [2] to better understand the discrepancy between the
two.

[0] https://review.openstack.org/#/c/610117/1
[1] feac3dcbfe/oslo_policy/policy.py (L960)
[2] feac3dcbfe/oslo_policy/policy.py (L792)

Change-Id: I5b0a28a2b5fb4dff150f13a56013a7a9b694c756

armada/common/policy.py

@@ -13,12 +13,15 @@
 import functools
 
 from oslo_config import cfg
+from oslo_log import log as logging
 from oslo_policy import policy
+from oslo_utils import excutils
 
 from armada.common import policies
 from armada.exceptions import base_exception as exc
 
 CONF = cfg.CONF
+LOG = logging.getLogger(__name__)
 _ENFORCER = None
 
 
@@ -41,9 +44,30 @@ def _enforce_policy(action, target, credentials, do_raise=True):
     if do_raise:
         extras.update(exc=exc.ActionForbidden, do_raise=do_raise)
 
-    _ENFORCER.enforce(action, target, credentials.to_policy_view(), **extras)
+    # `oslo.policy` supports both enforce and authorize. authorize is
+    # stricter because it'll raise an exception if the policy action is
+    # not found in the list of registered rules. This means that attempting
+    # to enforce anything not found in ``armada.common.policies`` will error
+    # out with a 'Policy not registered' message and 403 status code.
+    try:
+        _ENFORCER.authorize(action, target, credentials.to_policy_view(),
+                            **extras)
+    except policy.PolicyNotRegistered as e:
+        LOG.exception('Policy not registered for %(action)s',
+                      {'action': action})
+        raise exc.ActionForbidden()
+    except Exception as e:
+        with excutils.save_and_reraise_exception():
+            LOG.debug(
+                'Policy check for %(action)s failed with credentials '
+                '%(credentials)s', {
+                    'action': action,
+                    'credentials': credentials
+                })
 
 
+# NOTE(felipemonteiro): This naming is OK. It's just kept around for legacy
+# reasons. What's important is that authorize is used above.
 def enforce(rule):
 
     def decorator(func):

armada/tests/unit/common/test_policy.py

@@ -29,8 +29,8 @@ class PolicyTestCase(testtools.TestCase):
         super(PolicyTestCase, self).setUp()
         self.rules = {
             "true": [],
-            "example:allowed": [],
-            "example:disallowed": [["false:false"]]
+            "armada:validate_manifest": [],
+            "armada:create_endpoints": [["false:false"]]
         }
         self.useFixture(fixtures.RealPolicyFixture(False))
         self._set_rules()
@@ -41,24 +41,30 @@ class PolicyTestCase(testtools.TestCase):
         curr_rules = common_policy.Rules.from_dict(self.rules)
         policy._ENFORCER.set_rules(curr_rules)
 
-    @mock.patch('armada.api.ArmadaRequestContext')
-    def test_enforce_nonexistent_action(self, mock_ctx):
+    @mock.patch.object(policy, 'LOG', autospec=True)
+    @mock.patch('armada.api.ArmadaRequestContext', autospec=True)
+    def test_enforce_nonexistent_action(self, mock_ctx, mock_log):
+        """Validates that unregistered default policy throws exception."""
         action = "example:nope"
         mock_ctx.to_policy_view.return_value = self.credentials
 
         self.assertRaises(exc.ActionForbidden, policy._enforce_policy, action,
                           self.target, mock_ctx)
+        mock_log.exception.assert_called_once_with(
+            'Policy not registered for %(action)s', {'action': 'example:nope'})
 
-    @mock.patch('armada.api.ArmadaRequestContext')
-    def test_enforce_good_action(self, mock_ctx):
-        action = "example:allowed"
+    @mock.patch('armada.api.ArmadaRequestContext', autospec=True)
+    def test_enforce_allowed_action(self, mock_ctx):
+        """Validates that allowed policy action can be performed."""
+        action = "armada:validate_manifest"
         mock_ctx.to_policy_view.return_value = self.credentials
 
         policy._enforce_policy(action, self.target, mock_ctx)
 
-    @mock.patch('armada.api.ArmadaRequestContext')
-    def test_enforce_bad_action(self, mock_ctx):
-        action = "example:disallowed"
+    @mock.patch('armada.api.ArmadaRequestContext', autospec=True)
+    def test_enforce_disallowed_action(self, mock_ctx):
+        """Validates that disallowed policy action cannot be performed."""
+        action = "armada:create_endpoints"
         mock_ctx.to_policy_view.return_value = self.credentials
 
         self.assertRaises(exc.ActionForbidden, policy._enforce_policy, action,