Merge pull request #3573 from StackStorm/rbac_rule_creation_fix

Allow users to create rules which reference actions which don't exist in the system when RBAC is enabled

Author: Kami

CHANGELOG.rst

@@ -36,8 +36,11 @@ Fixed
   ``http_proxy`` or ``https_proxy`` environment variables for ``st2api`` and ``st2actionrunner``
   processes and pack commands will work with proxy. Refer to documentation for details on
   proxy configuration. (bug-fix) #3137
-* Fix no-member linting error on U16 by ignoring mistralclient.api.v2.executions module.
+* Fix an API bug and allow users to create rules which reference actions which don't yet exist in
+  the system when RBAC is enabled and user doesn't have system admin permission. (bug fix)
+  #3572 #3573
 
+  Reported by sibirajal.
 
 2.3.1 - July 07, 2017
 ---------------------

st2api/tests/unit/controllers/v1/test_rules_rbac.py

@@ -65,6 +65,11 @@ def setUp(self):
             fixtures_pack=FIXTURES_PACK,
             fixtures_dict={'rules': [file_name]})['rules'][file_name]
 
+        file_name = 'rule_action_doesnt_exist.yaml'
+        RuleControllerRBACTestCase.RULE_3 = self.fixtures_loader.load_fixtures(
+            fixtures_pack=FIXTURES_PACK,
+            fixtures_dict={'rules': [file_name]})['rules'][file_name]
+
         # Insert mock users, roles and assignments
 
         # Users
@@ -156,6 +161,17 @@ def test_post_webhook_trigger_no_trigger_and_action_permission(self):
         self.assertEqual(resp.status_code, httplib.FORBIDDEN)
         self.assertEqual(resp.json['faultstring'], expected_msg)
 
+    def test_post_user_has_no_permission_on_action_which_doesnt_exist_in_db(self):
+        # User has rule_create, but no action_execute on the action which doesn't exist in the db
+        user_db = self.users['rule_create_webhook_create']
+        self.use_user(user_db)
+
+        resp = self.__do_post(RuleControllerRBACTestCase.RULE_3)
+        expected_msg = ('User "rule_create_webhook_create" doesn\'t have required (action_execute)'
+                        ' permission to use action wolfpack.action-doesnt-exist-woo')
+        self.assertEqual(resp.status_code, httplib.FORBIDDEN)
+        self.assertEqual(resp.json['faultstring'], expected_msg)
+
     def test_post_no_webhook_trigger(self):
         # Test a scenario when user with only "rule_create" permission selects a non-webhook
         # trigger for which we don't perform any permission checking right now

st2common/st2common/rbac/resolvers.py

@@ -212,6 +212,7 @@ def _user_has_resource_permission(self, user_db, pack_uid, resource_uid, permiss
         self._log('Checking user resource permissions', extra=log_context)
 
         # First check the system role permissions
+        self._log('Checking grants via system role permissions', extra=log_context)
         has_system_role_permission = self._user_has_system_role_permission(
             user_db=user_db, permission_type=permission_type)
 
@@ -235,6 +236,7 @@ def _user_has_resource_permission(self, user_db, pack_uid, resource_uid, permiss
             permission_types = [permission_type]
 
         # Check direct grants on the specified resource
+        self._log('Checking direct grans on the specified resource', extra=log_context)
         resource_types = [self.resource_type]
         permission_grants = get_all_permission_grants_for_user(user_db=user_db,
                                                                resource_uid=resource_uid,
@@ -245,6 +247,7 @@ def _user_has_resource_permission(self, user_db, pack_uid, resource_uid, permiss
             return True
 
         # Check grants on the parent pack
+        self._log('Checking grants on the parent resource', extra=log_context)
         resource_types = [ResourceType.PACK]
         permission_grants = get_all_permission_grants_for_user(user_db=user_db,
                                                                resource_uid=pack_uid,

st2common/st2common/rbac/utils.py

@@ -21,6 +21,8 @@
 
 from oslo_config import cfg
 
+from st2common.models.db.action import ActionDB
+from st2common.models.system.common import ResourceReference
 from st2common.exceptions.rbac import AccessDeniedError
 from st2common.exceptions.rbac import ResourceTypeAccessDeniedError
 from st2common.exceptions.rbac import ResourceAccessDeniedError
@@ -162,11 +164,20 @@ def user_has_rule_action_permission(user_db, action_ref):
     """
     Check that the currently logged-in has necessary permissions on the action used / referenced
     inside the rule.
+
+    Note: Rules can reference actions which don't yet exist in the system.
     """
     if not cfg.CONF.rbac.enable:
         return True
 
     action_db = action_utils.get_action_by_ref(ref=action_ref)
+
+    if not action_db:
+        # We allow rules to be created for actions which don't yet exist in the
+        # system
+        ref = ResourceReference.from_string_reference(ref=action_ref)
+        action_db = ActionDB(pack=ref.pack, name=ref.name, ref=action_ref)
+
     action_resolver = resolvers.get_resolver_for_resource_type(ResourceType.ACTION)
     has_action_permission = action_resolver.user_has_resource_db_permission(
         user_db=user_db, resource_db=action_db, permission_type=PermissionType.ACTION_EXECUTE)

st2tests/st2tests/fixtures/generic/rules/rule_action_doesnt_exist.yaml

@@ -0,0 +1,21 @@
+---
+action:
+  parameters:
+    ip1: '{{trigger.t1_p}}'
+    ip2: '{{trigger}}'
+  ref: wolfpack.action-doesnt-exist-woo
+criteria:
+  trigger.t1_p:
+    pattern: t1_p_v
+    type: equals
+description: ''
+enabled: true
+name: rule_action_doesnt_exist
+pack: examples
+tags:
+- name: tag1
+  value: dont-care
+- name: tag2
+  value: dont-care
+trigger:
+  type: wolfpack.triggertype-1