diff --git a/CHANGELOG.rst b/CHANGELOG.rst index e7f9132dbd..058a69ea1f 100644 --- a/CHANGELOG.rst +++ b/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 --------------------- diff --git a/st2api/tests/unit/controllers/v1/test_rules_rbac.py b/st2api/tests/unit/controllers/v1/test_rules_rbac.py index af1847b11e..b6fc7f76cc 100644 --- a/st2api/tests/unit/controllers/v1/test_rules_rbac.py +++ b/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 diff --git a/st2common/st2common/rbac/resolvers.py b/st2common/st2common/rbac/resolvers.py index 3f86bca852..394f0fb03b 100644 --- a/st2common/st2common/rbac/resolvers.py +++ b/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, diff --git a/st2common/st2common/rbac/utils.py b/st2common/st2common/rbac/utils.py index ad29b3be94..b44e70c9ae 100644 --- a/st2common/st2common/rbac/utils.py +++ b/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) diff --git a/st2tests/st2tests/fixtures/generic/rules/rule_action_doesnt_exist.yaml b/st2tests/st2tests/fixtures/generic/rules/rule_action_doesnt_exist.yaml new file mode 100644 index 0000000000..a1af3cc9c0 --- /dev/null +++ b/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