Merge "Fix missing argument bug in oslo common policy"

2 files changed, 27 insertions, 16 deletions

diff --git a/openstack/common/policy.py b/openstack/common/policy.py
index d4d9aa1..5705d78 100644
--- a/openstack/common/policy.py
+++ b/openstack/common/policy.py
@@ -285,7 +285,7 @@ class BaseCheck(object):
         pass
 
     @abc.abstractmethod
-    def __call__(self, target, cred):
+    def __call__(self, target, cred, enforcer):
         """Triggers if instance of the class is called.
 
         Performs the check. Returns False to reject the access or a
@@ -303,7 +303,7 @@ class FalseCheck(BaseCheck):
 
         return "!"
 
-    def __call__(self, target, cred):
+    def __call__(self, target, cred, enforcer):
         """Check the policy."""
 
         return False
@@ -317,7 +317,7 @@ class TrueCheck(BaseCheck):
 
         return "@"
 
-    def __call__(self, target, cred):
+    def __call__(self, target, cred, enforcer):
         """Check the policy."""
 
         return True
@@ -363,13 +363,13 @@ class NotCheck(BaseCheck):
 
         return "not %s" % self.rule
 
-    def __call__(self, target, cred):
+    def __call__(self, target, cred, enforcer):
         """Check the policy.
 
         Returns the logical inverse of the wrapped check.
         """
 
-        return not self.rule(target, cred)
+        return not self.rule(target, cred, enforcer)
 
 
 class AndCheck(BaseCheck):
@@ -391,7 +391,7 @@ class AndCheck(BaseCheck):
 
         return "(%s)" % ' and '.join(str(r) for r in self.rules)
 
-    def __call__(self, target, cred):
+    def __call__(self, target, cred, enforcer):
         """Check the policy.
 
         Requires that all rules accept in order to return True.
@@ -434,7 +434,7 @@ class OrCheck(BaseCheck):
 
         return "(%s)" % ' or '.join(str(r) for r in self.rules)
 
-    def __call__(self, target, cred):
+    def __call__(self, target, cred, enforcer):
         """Check the policy.
 
         Requires that at least one rule accept in order to return True.
diff --git a/tests/unit/test_policy.py b/tests/unit/test_policy.py
index 24b23a4..b7d38a3 100644
--- a/tests/unit/test_policy.py
+++ b/tests/unit/test_policy.py
@@ -159,6 +159,17 @@ class EnforcerTest(PolicyBaseTestCase):
         self.enforcer.clear()
         self.assertEqual(self.enforcer.rules, {})
 
+    def test_rule_with_check(self):
+        rules_json = """{
+                        "deny_stack_user": "not role:stack_user",
+                        "cloudwatch:PutMetricData": ""
+                        }"""
+        rules = policy.Rules.load_json(rules_json)
+        self.enforcer.set_rules(rules)
+        action = "cloudwatch:PutMetricData"
+        creds = {'roles': ''}
+        self.assertEqual(self.enforcer.enforce(action, {}, creds), True)
+
 
 class FakeCheck(policy.BaseCheck):
     def __init__(self, result=None):
@@ -228,7 +239,7 @@ class FalseCheckTestCase(utils.BaseTestCase):
     def test_call(self):
         check = policy.FalseCheck()
 
-        self.assertEqual(check('target', 'creds'), False)
+        self.assertEqual(check('target', 'creds', None), False)
 
 
 class TrueCheckTestCase(utils.BaseTestCase):
@@ -240,7 +251,7 @@ class TrueCheckTestCase(utils.BaseTestCase):
     def test_call(self):
         check = policy.TrueCheck()
 
-        self.assertEqual(check('target', 'creds'), True)
+        self.assertEqual(check('target', 'creds', None), True)
 
 
 class CheckForTest(policy.Check):
@@ -276,15 +287,15 @@ class NotCheckTestCase(utils.BaseTestCase):
         rule = mock.Mock(return_value=True)
         check = policy.NotCheck(rule)
 
-        self.assertEqual(check('target', 'cred'), False)
-        rule.assert_called_once_with('target', 'cred')
+        self.assertEqual(check('target', 'cred', None), False)
+        rule.assert_called_once_with('target', 'cred', None)
 
     def test_call_false(self):
         rule = mock.Mock(return_value=False)
         check = policy.NotCheck(rule)
 
-        self.assertEqual(check('target', 'cred'), True)
-        rule.assert_called_once_with('target', 'cred')
+        self.assertEqual(check('target', 'cred', None), True)
+        rule.assert_called_once_with('target', 'cred', None)
 
 
 class OrCheckTestCase(utils.BaseTestCase):
@@ -308,7 +319,7 @@ class OrCheckTestCase(utils.BaseTestCase):
         rules = [mock.Mock(return_value=False), mock.Mock(return_value=False)]
         check = policy.OrCheck(rules)
 
-        self.assertEqual(check('target', 'cred'), False)
+        self.assertEqual(check('target', 'cred', None), False)
         rules[0].assert_called_once_with('target', 'cred')
         rules[1].assert_called_once_with('target', 'cred')
 
@@ -316,7 +327,7 @@ class OrCheckTestCase(utils.BaseTestCase):
         rules = [mock.Mock(return_value=True), mock.Mock(return_value=False)]
         check = policy.OrCheck(rules)
 
-        self.assertEqual(check('target', 'cred'), True)
+        self.assertEqual(check('target', 'cred', None), True)
         rules[0].assert_called_once_with('target', 'cred')
         self.assertFalse(rules[1].called)
 
@@ -324,7 +335,7 @@ class OrCheckTestCase(utils.BaseTestCase):
         rules = [mock.Mock(return_value=False), mock.Mock(return_value=True)]
         check = policy.OrCheck(rules)
 
-        self.assertEqual(check('target', 'cred'), True)
+        self.assertEqual(check('target', 'cred', None), True)
         rules[0].assert_called_once_with('target', 'cred')
         rules[1].assert_called_once_with('target', 'cred')