From d2cc0dc5663657ae80550954269e19a6a8157501 Mon Sep 17 00:00:00 2001 From: Rick Bartra Date: Fri, 20 Jul 2018 17:42:09 -0400 Subject: [PATCH] Add Policy enforcement for several Metadata Definition delete APIs Several Metadata Definition delete APIs do not have RBAC. This patchset add policy enforcment to the following APIs: - `Delete namespace` - `Delete object` - `Remove resource type association` - `Remove property definition` - `Delete tag definition` - `Delete all tag definitions` The following actions are enforce and added to the policy.json: - `delete_metadef_namespace` - `delete_metadef_object` - `remove_metadef_resource_type_association` - `remove_metadef_property` - `delete_metadef_tag` - `delete_metadef_tags` Most other APIs have policy enforcement, so the ones above should as well. Without adding policy enforcement for the above APIs, all roles can peform the delete APIs noted above. Change-Id: I8cd6eb26b0d3401fa4667384c31e4c56d838d42b Closes-Bug: #1782840 Co-Authored-By: julian.sy@att.com --- doc/source/admin/troubleshooting.rst | 7 +- glance/api/policy.py | 47 +++ glance/policies/metadef.py | 9 + glance/tests/etc/policy.json | 6 + glance/tests/unit/test_policy.py | 286 ++++++++++++++++++ ..._metadef_delete_apis-95d2b16cc444840a.yaml | 16 + 6 files changed, 370 insertions(+), 1 deletion(-) create mode 100644 releasenotes/notes/add_policy_enforcement_for_metadef_delete_apis-95d2b16cc444840a.yaml diff --git a/doc/source/admin/troubleshooting.rst b/doc/source/admin/troubleshooting.rst index 60e9b1e84f..d92ef178e9 100644 --- a/doc/source/admin/troubleshooting.rst +++ b/doc/source/admin/troubleshooting.rst @@ -194,21 +194,26 @@ can also be used to set policies for Image service actions. "get_metadef_objects":"", "modify_metadef_object":"", "add_metadef_object":"", + "delete_metadef_object":"", "list_metadef_resource_types":"", "get_metadef_resource_type":"", "add_metadef_resource_type_association":"", + "remove_metadef_resource_type_association":"", "get_metadef_property":"", "get_metadef_properties":"", "modify_metadef_property":"", "add_metadef_property":"", + "remove_metadef_property":"", "get_metadef_tag":"", "get_metadef_tags":"", "modify_metadef_tag":"", "add_metadef_tag":"", - "add_metadef_tags":"" + "add_metadef_tags":"", + "delete_metadef_tag":"", + "delete_metadef_tags":"" } For each parameter, use ``"rule:restricted"`` to restrict access to all diff --git a/glance/api/policy.py b/glance/api/policy.py index f14dbc1877..18a30ced98 100644 --- a/glance/api/policy.py +++ b/glance/api/policy.py @@ -454,6 +454,10 @@ class MetadefNamespaceProxy(glance.domain.proxy.MetadefNamespace): self.policy = policy super(MetadefNamespaceProxy, self).__init__(namespace) + def delete(self): + self.policy.enforce(self.context, 'delete_metadef_namespace', {}) + return super(MetadefNamespaceProxy, self).delete() + class MetadefNamespaceRepoProxy(glance.domain.proxy.MetadefNamespaceRepo): @@ -483,6 +487,14 @@ class MetadefNamespaceRepoProxy(glance.domain.proxy.MetadefNamespaceRepo): self.policy.enforce(self.context, 'add_metadef_namespace', {}) return super(MetadefNamespaceRepoProxy, self).add(namespace) + def remove(self, namespace): + self.policy.enforce(self.context, 'delete_metadef_namespace', {}) + return super(MetadefNamespaceRepoProxy, self).remove(namespace) + + def remove_tags(self, namespace): + self.policy.enforce(self.context, 'delete_metadef_tags', {}) + return super(MetadefNamespaceRepoProxy, self).remove_tags(namespace) + class MetadefNamespaceFactoryProxy( glance.domain.proxy.MetadefNamespaceFactory): @@ -507,6 +519,10 @@ class MetadefObjectProxy(glance.domain.proxy.MetadefObject): self.policy = policy super(MetadefObjectProxy, self).__init__(meta_object) + def delete(self): + self.policy.enforce(self.context, 'delete_metadef_object', {}) + return super(MetadefObjectProxy, self).delete() + class MetadefObjectRepoProxy(glance.domain.proxy.MetadefObjectRepo): @@ -536,6 +552,10 @@ class MetadefObjectRepoProxy(glance.domain.proxy.MetadefObjectRepo): self.policy.enforce(self.context, 'add_metadef_object', {}) return super(MetadefObjectRepoProxy, self).add(meta_object) + def remove(self, meta_object): + self.policy.enforce(self.context, 'delete_metadef_object', {}) + return super(MetadefObjectRepoProxy, self).remove(meta_object) + class MetadefObjectFactoryProxy(glance.domain.proxy.MetadefObjectFactory): @@ -559,6 +579,11 @@ class MetadefResourceTypeProxy(glance.domain.proxy.MetadefResourceType): self.policy = policy super(MetadefResourceTypeProxy, self).__init__(meta_resource_type) + def delete(self): + self.policy.enforce(self.context, + 'remove_metadef_resource_type_association', {}) + return super(MetadefResourceTypeProxy, self).delete() + class MetadefResourceTypeRepoProxy( glance.domain.proxy.MetadefResourceTypeRepo): @@ -586,6 +611,12 @@ class MetadefResourceTypeRepoProxy( 'add_metadef_resource_type_association', {}) return super(MetadefResourceTypeRepoProxy, self).add(resource_type) + def remove(self, *args, **kwargs): + self.policy.enforce(self.context, + 'remove_metadef_resource_type_association', {}) + return super(MetadefResourceTypeRepoProxy, + self).remove(*args, **kwargs) + class MetadefResourceTypeFactoryProxy( glance.domain.proxy.MetadefResourceTypeFactory): @@ -610,6 +641,10 @@ class MetadefPropertyProxy(glance.domain.proxy.MetadefProperty): self.policy = policy super(MetadefPropertyProxy, self).__init__(namespace_property) + def delete(self): + self.policy.enforce(self.context, 'remove_metadef_property', {}) + return super(MetadefPropertyProxy, self).delete() + class MetadefPropertyRepoProxy(glance.domain.proxy.MetadefPropertyRepo): @@ -643,6 +678,10 @@ class MetadefPropertyRepoProxy(glance.domain.proxy.MetadefPropertyRepo): return super(MetadefPropertyRepoProxy, self).add( namespace_property) + def remove(self, *args, **kwargs): + self.policy.enforce(self.context, 'remove_metadef_property', {}) + return super(MetadefPropertyRepoProxy, self).remove(*args, **kwargs) + class MetadefPropertyFactoryProxy(glance.domain.proxy.MetadefPropertyFactory): @@ -665,6 +704,10 @@ class MetadefTagProxy(glance.domain.proxy.MetadefTag): self.policy = policy super(MetadefTagProxy, self).__init__(meta_tag) + def delete(self): + self.policy.enforce(self.context, 'delete_metadef_tag', {}) + return super(MetadefTagProxy, self).delete() + class MetadefTagRepoProxy(glance.domain.proxy.MetadefTagRepo): @@ -698,6 +741,10 @@ class MetadefTagRepoProxy(glance.domain.proxy.MetadefTagRepo): self.policy.enforce(self.context, 'add_metadef_tags', {}) return super(MetadefTagRepoProxy, self).add_tags(meta_tags) + def remove(self, meta_tag): + self.policy.enforce(self.context, 'delete_metadef_tag', {}) + return super(MetadefTagRepoProxy, self).remove(meta_tag) + class MetadefTagFactoryProxy(glance.domain.proxy.MetadefTagFactory): diff --git a/glance/policies/metadef.py b/glance/policies/metadef.py index 55ddd6c827..5f5f9d68dd 100644 --- a/glance/policies/metadef.py +++ b/glance/policies/metadef.py @@ -20,11 +20,14 @@ metadef_policies = [ policy.RuleDefault(name="modify_metadef_namespace", check_str="rule:default"), policy.RuleDefault(name="add_metadef_namespace", check_str="rule:default"), + policy.RuleDefault(name="delete_metadef_namespace", + check_str="rule:default"), policy.RuleDefault(name="get_metadef_object", check_str="rule:default"), policy.RuleDefault(name="get_metadef_objects", check_str="rule:default"), policy.RuleDefault(name="modify_metadef_object", check_str="rule:default"), policy.RuleDefault(name="add_metadef_object", check_str="rule:default"), + policy.RuleDefault(name="delete_metadef_object", check_str="rule:default"), policy.RuleDefault(name="list_metadef_resource_types", check_str="rule:default"), @@ -32,6 +35,8 @@ metadef_policies = [ check_str="rule:default"), policy.RuleDefault(name="add_metadef_resource_type_association", check_str="rule:default"), + policy.RuleDefault(name="remove_metadef_resource_type_association", + check_str="rule:default"), policy.RuleDefault(name="get_metadef_property", check_str="rule:default"), policy.RuleDefault(name="get_metadef_properties", @@ -39,12 +44,16 @@ metadef_policies = [ policy.RuleDefault(name="modify_metadef_property", check_str="rule:default"), policy.RuleDefault(name="add_metadef_property", check_str="rule:default"), + policy.RuleDefault(name="remove_metadef_property", + check_str="rule:default"), policy.RuleDefault(name="get_metadef_tag", check_str="rule:default"), policy.RuleDefault(name="get_metadef_tags", check_str="rule:default"), policy.RuleDefault(name="modify_metadef_tag", check_str="rule:default"), policy.RuleDefault(name="add_metadef_tag", check_str="rule:default"), policy.RuleDefault(name="add_metadef_tags", check_str="rule:default"), + policy.RuleDefault(name="delete_metadef_tag", check_str="rule:default"), + policy.RuleDefault(name="delete_metadef_tags", check_str="rule:default"), ] diff --git a/glance/tests/etc/policy.json b/glance/tests/etc/policy.json index fb19f4fa13..a084ab20d0 100644 --- a/glance/tests/etc/policy.json +++ b/glance/tests/etc/policy.json @@ -36,26 +36,32 @@ "get_metadef_namespaces":"", "modify_metadef_namespace":"", "add_metadef_namespace":"", + "delete_metadef_namespace": "", "get_metadef_object":"", "get_metadef_objects":"", "modify_metadef_object":"", "add_metadef_object":"", + "delete_metadef_object": "", "list_metadef_resource_types":"", "get_metadef_resource_type":"", "add_metadef_resource_type_association":"", + "remove_metadef_resource_type_association": "", "get_metadef_property":"", "get_metadef_properties":"", "modify_metadef_property":"", "add_metadef_property":"", + "remove_metadef_property": "", "get_metadef_tag":"", "get_metadef_tags":"", "modify_metadef_tag":"", "add_metadef_tag":"", "add_metadef_tags":"", + "delete_metadef_tag": "", + "delete_metadef_tags": "", "deactivate": "", "reactivate": "" diff --git a/glance/tests/unit/test_policy.py b/glance/tests/unit/test_policy.py index 3dea40be59..376a1edcf5 100644 --- a/glance/tests/unit/test_policy.py +++ b/glance/tests/unit/test_policy.py @@ -160,6 +160,94 @@ class TaskFactoryStub(object): return 'new_task' +class MdNamespaceRepoStub(object): + def add(self, namespace): + return 'mdns_add' + + def get(self, namespace): + return 'mdns_get' + + def list(self, *args, **kwargs): + return ['mdns_list'] + + def save(self, namespace): + return 'mdns_save' + + def remove(self, namespace): + return 'mdns_remove' + + def remove_tags(self, namespace): + return 'mdtags_remove' + + +class MdObjectRepoStub(object): + def add(self, obj): + return 'mdobj_add' + + def get(self, ns, obj_name): + return 'mdobj_get' + + def list(self, *args, **kwargs): + return ['mdobj_list'] + + def save(self, obj): + return 'mdobj_save' + + def remove(self, obj): + return 'mdobj_remove' + + +class MdResourceTypeRepoStub(object): + def add(self, rt): + return 'mdrt_add' + + def get(self, *args, **kwargs): + return 'mdrt_get' + + def list(self, *args, **kwargs): + return ['mdrt_list'] + + def remove(self, *args, **kwargs): + return 'mdrt_remove' + + +class MdPropertyRepoStub(object): + def add(self, prop): + return 'mdprop_add' + + def get(self, ns, prop_name): + return 'mdprop_get' + + def list(self, *args, **kwargs): + return ['mdprop_list'] + + def save(self, prop): + return 'mdprop_save' + + def remove(self, prop): + return 'mdprop_remove' + + +class MdTagRepoStub(object): + def add(self, tag): + return 'mdtag_add' + + def add_tags(self, tags): + return ['mdtag_add_tags'] + + def get(self, ns, tag_name): + return 'mdtag_get' + + def list(self, *args, **kwargs): + return ['mdtag_list'] + + def save(self, tag): + return 'mdtag_save' + + def remove(self, tag): + return 'mdtag_remove' + + class TestPolicyEnforcer(base.IsolatedUnitTest): def test_policy_enforce_unregistered(self): @@ -604,6 +692,204 @@ class TestTaskPolicy(test_utils.BaseTestCase): task_repo.add(task) +class TestMetadefPolicy(test_utils.BaseTestCase): + def setUp(self): + self.fakens = mock.Mock() + self.fakeobj = mock.Mock() + self.fakert = mock.Mock() + self.fakeprop = mock.Mock() + self.faketag = mock.Mock() + self.policy = unit_test_utils.FakePolicyEnforcer() + super(TestMetadefPolicy, self).setUp() + + def test_md_namespace_not_allowed(self): + rules = {'get_metadef_namespace': False, + 'get_metadef_namespaces': False, + 'modify_metadef_namespace': False, + 'add_metadef_namespace': False, + 'delete_metadef_namespace': False} + self.policy.set_rules(rules) + mdns_repo = glance.api.policy.MetadefNamespaceRepoProxy( + MdNamespaceRepoStub(), {}, self.policy) + self.assertRaises(exception.Forbidden, mdns_repo.add, self.fakens) + self.assertRaises(exception.Forbidden, mdns_repo.get, self.fakens) + self.assertRaises(exception.Forbidden, mdns_repo.list) + self.assertRaises(exception.Forbidden, mdns_repo.remove, self.fakens) + self.assertRaises(exception.Forbidden, mdns_repo.save, self.fakens) + + def test_md_namespace_allowed(self): + rules = {'get_metadef_namespace': True, + 'get_metadef_namespaces': True, + 'modify_metadef_namespace': True, + 'add_metadef_namespace': True, + 'delete_metadef_namespace': True} + self.policy.set_rules(rules) + mdns_repo = glance.api.policy.MetadefNamespaceRepoProxy( + MdNamespaceRepoStub(), {}, self.policy) + self.assertEqual(None, mdns_repo.add(self.fakens)) + self.assertEqual('mdns_get', + mdns_repo.get(self.fakens).namespace_input) + self.assertEqual(['mdns_list'], + [ns.namespace_input for ns in mdns_repo.list()]) + self.assertEqual('mdns_save', + mdns_repo.save(self.fakens).namespace_input) + self.assertEqual('mdns_remove', + mdns_repo.remove(self.fakens).namespace_input) + + def test_md_object_not_allowed(self): + rules = {'get_metadef_object': False, + 'get_metadef_objects': False, + 'modify_metadef_object': False, + 'add_metadef_object': False, + 'delete_metadef_object': False} + self.policy.set_rules(rules) + mdobj_repo = glance.api.policy.MetadefObjectRepoProxy( + MdObjectRepoStub(), {}, self.policy) + self.assertRaises(exception.Forbidden, mdobj_repo.add, self.fakeobj) + self.assertRaises(exception.Forbidden, mdobj_repo.get, self.fakens, + self.fakeobj) + self.assertRaises(exception.Forbidden, mdobj_repo.list) + self.assertRaises(exception.Forbidden, mdobj_repo.remove, self.fakeobj) + self.assertRaises(exception.Forbidden, mdobj_repo.save, self.fakeobj) + + def test_md_object_allowed(self): + rules = {'get_metadef_object': True, + 'get_metadef_objects': True, + 'modify_metadef_object': True, + 'add_metadef_object': True, + 'delete_metadef_object': True} + self.policy.set_rules(rules) + mdobj_repo = glance.api.policy.MetadefObjectRepoProxy( + MdObjectRepoStub(), {}, self.policy) + self.assertEqual(None, mdobj_repo.add(self.fakeobj)) + self.assertEqual('mdobj_get', + mdobj_repo.get(self.fakens, 'fakeobj').meta_object) + self.assertEqual(['mdobj_list'], + [obj.meta_object for obj in mdobj_repo.list()]) + self.assertEqual('mdobj_save', + mdobj_repo.save(self.fakeobj).meta_object) + self.assertEqual('mdobj_remove', + mdobj_repo.remove(self.fakeobj).meta_object) + + def test_md_resource_type_not_allowed(self): + rules = {'get_metadef_resource_type': False, + 'list_metadef_resource_types': False, + 'add_metadef_resource_type_association': False, + 'remove_metadef_resource_type_association': False} + self.policy.set_rules(rules) + mdrt_repo = glance.api.policy.MetadefResourceTypeRepoProxy( + MdResourceTypeRepoStub(), {}, self.policy) + self.assertRaises(exception.Forbidden, mdrt_repo.add, self.fakert) + self.assertRaises(exception.Forbidden, mdrt_repo.get, self.fakert) + self.assertRaises(exception.Forbidden, mdrt_repo.list) + self.assertRaises(exception.Forbidden, mdrt_repo.remove, self.fakert) + + def test_md_resource_type_allowed(self): + rules = {'get_metadef_resource_type': True, + 'list_metadef_resource_types': True, + 'add_metadef_resource_type_association': True, + 'remove_metadef_resource_type_association': True} + self.policy.set_rules(rules) + mdrt_repo = glance.api.policy.MetadefResourceTypeRepoProxy( + MdResourceTypeRepoStub(), {}, self.policy) + self.assertEqual(None, mdrt_repo.add(self.fakert)) + self.assertEqual( + 'mdrt_get', mdrt_repo.get(self.fakens, + 'fakert').meta_resource_type) + self.assertEqual(['mdrt_list'], + [rt.meta_resource_type for rt in mdrt_repo.list()]) + self.assertEqual('mdrt_remove', + mdrt_repo.remove(self.fakert).meta_resource_type) + + def test_md_property_not_allowed(self): + rules = {'get_metadef_property': False, + 'get_metadef_properties': False, + 'modify_metadef_property': False, + 'add_metadef_property': False, + 'remove_metadef_property': False} + self.policy.set_rules(rules) + mdprop_repo = glance.api.policy.MetadefPropertyRepoProxy( + MdPropertyRepoStub(), {}, self.policy) + self.assertRaises(exception.Forbidden, mdprop_repo.add, self.fakeprop) + self.assertRaises(exception.Forbidden, mdprop_repo.get, self.fakens, + self.fakeprop) + self.assertRaises(exception.Forbidden, mdprop_repo.list) + self.assertRaises(exception.Forbidden, mdprop_repo.remove, + self.fakeprop) + self.assertRaises(exception.Forbidden, mdprop_repo.save, self.fakeprop) + + def test_md_property_allowed(self): + rules = {'get_metadef_property': True, + 'get_metadef_properties': True, + 'modify_metadef_property': True, + 'add_metadef_property': True, + 'remove_metadef_property': True} + self.policy.set_rules(rules) + mdprop_repo = glance.api.policy.MetadefPropertyRepoProxy( + MdPropertyRepoStub(), {}, self.policy) + self.assertEqual(None, mdprop_repo.add(self.fakeprop)) + self.assertEqual( + 'mdprop_get', mdprop_repo.get(self.fakens, + 'fakeprop').namespace_property) + self.assertEqual(['mdprop_list'], + [prop.namespace_property for prop + in mdprop_repo.list()]) + self.assertEqual('mdprop_save', + mdprop_repo.save(self.fakeprop).namespace_property) + self.assertEqual('mdprop_remove', + mdprop_repo.remove(self.fakeprop).namespace_property) + + def test_md_tag_not_allowed(self): + rules = {'get_metadef_tag': False, + 'get_metadef_tags': False, + 'modify_metadef_tag': False, + 'add_metadef_tag': False, + 'add_metadef_tags': False, + 'delete_metadef_tag': False, + 'delete_metadef_tags': False} + self.policy.set_rules(rules) + mdtag_repo = glance.api.policy.MetadefTagRepoProxy( + MdTagRepoStub(), {}, self.policy) + mdns_repo = glance.api.policy.MetadefNamespaceRepoProxy( + MdNamespaceRepoStub(), {}, self.policy) + self.assertRaises(exception.Forbidden, mdtag_repo.add, self.faketag) + self.assertRaises(exception.Forbidden, mdtag_repo.add_tags, + [self.faketag]) + self.assertRaises(exception.Forbidden, mdtag_repo.get, self.fakens, + self.faketag) + self.assertRaises(exception.Forbidden, mdtag_repo.list) + self.assertRaises(exception.Forbidden, mdtag_repo.remove, self.faketag) + self.assertRaises(exception.Forbidden, mdns_repo.remove_tags, + self.fakens) + self.assertRaises(exception.Forbidden, mdtag_repo.save, self.faketag) + + def test_md_tag_allowed(self): + rules = {'get_metadef_tag': True, + 'get_metadef_tags': True, + 'modify_metadef_tag': True, + 'add_metadef_tag': True, + 'add_metadef_tags': True, + 'delete_metadef_tag': True, + 'delete_metadef_tags': True} + self.policy.set_rules(rules) + mdtag_repo = glance.api.policy.MetadefTagRepoProxy( + MdTagRepoStub(), {}, self.policy) + mdns_repo = glance.api.policy.MetadefNamespaceRepoProxy( + MdNamespaceRepoStub(), {}, self.policy) + self.assertEqual(None, mdtag_repo.add(self.faketag)) + self.assertEqual(None, mdtag_repo.add_tags([self.faketag])) + self.assertEqual('mdtag_get', + mdtag_repo.get(self.fakens, 'faketag').base) + self.assertEqual(['mdtag_list'], + [tag.base for tag in mdtag_repo.list()]) + self.assertEqual('mdtag_save', + mdtag_repo.save(self.faketag).base) + self.assertEqual('mdtag_remove', + mdtag_repo.remove(self.faketag).base) + self.assertEqual('mdtags_remove', + mdns_repo.remove_tags(self.fakens).base) + + class TestContextPolicyEnforcer(base.IsolatedUnitTest): def _do_test_policy_influence_context_admin(self, diff --git a/releasenotes/notes/add_policy_enforcement_for_metadef_delete_apis-95d2b16cc444840a.yaml b/releasenotes/notes/add_policy_enforcement_for_metadef_delete_apis-95d2b16cc444840a.yaml new file mode 100644 index 0000000000..bd63c88b3a --- /dev/null +++ b/releasenotes/notes/add_policy_enforcement_for_metadef_delete_apis-95d2b16cc444840a.yaml @@ -0,0 +1,16 @@ +--- +features: + - | + Policy enforcement for several Metadata Definition delete APIs are + added in this release. The following actions are enforced + and added to the policy.json: + + - ``delete_metadef_namespace`` + - ``delete_metadef_object`` + - ``remove_metadef_resource_type_association`` + - ``remove_metadef_property`` + - ``delete_metadef_tag`` + - ``delete_metadef_tags`` + + This prevents roles that should not have access to these APIs from + performing the APIs associated with the actions above.