From e00ebee05318edbd18f49df0fd34697d0e1417ed Mon Sep 17 00:00:00 2001 From: Slawek Kaplonski Date: Thu, 12 Sep 2019 22:02:52 +0200 Subject: [PATCH] List SG rules which belongs to tenant's SG In case when user's security group contains rules created e.g. by admin, and such rules has got admin's tenant as tenant_id, owner of security group should be able to see those rules. Some time ago this was addressed for request: GET /v2.0/security-groups/ But it is also required to behave in same way for GET /v2.0/security-group-rules So this patch fixes this behaviour for listing of security group rules. To achieve that this patch also adds new policy rule: ADMIN_OWNER_OR_SG_OWNER which is similar to already existing ADMIN_OWNER_OR_NETWORK_OWNER used e.g. for listing or creating ports. Conflicts: etc/policy.json neutron/policy.py Change-Id: I09114712582d2d38d14cf1683b87a8ce3a8e8c3c Closes-Bug: #1824248 (cherry picked from commit b898d2e3c08b50e576ee849fbe8614c66f360c62) (cherry picked from commit 36d1086569627af5dafd734333a7ebc4bc060d77) --- etc/policy.json | 4 +++- neutron/api/v2/attributes.py | 3 ++- neutron/db/securitygroups_db.py | 14 ++++++++++++-- neutron/tests/etc/policy.json | 4 +++- ...-for-security-group-owner-6635dd3e4c6ab5ee.yaml | 6 ++++++ 5 files changed, 26 insertions(+), 5 deletions(-) create mode 100644 releasenotes/notes/show-all-security-group-rules-for-security-group-owner-6635dd3e4c6ab5ee.yaml diff --git a/etc/policy.json b/etc/policy.json index 02171ce1b25..e25bad690b8 100644 --- a/etc/policy.json +++ b/etc/policy.json @@ -13,6 +13,8 @@ "shared_address_scopes": "field:address_scopes:shared=True", "external": "field:networks:router:external=True", "default": "rule:admin_or_owner", + "admin_or_sg_owner": "rule:context_is_admin or tenant_id:%(security_group:tenant_id)s", + "admin_owner_or_sg_owner": "rule:owner or rule:admin_or_sg_owner", "create_subnet": "rule:admin_or_network_owner", "create_subnet:segment_id": "rule:admin_only", @@ -230,7 +232,7 @@ "update_security_group": "rule:admin_or_owner", "delete_security_group": "rule:admin_or_owner", "get_security_group_rules": "rule:admin_or_owner", - "get_security_group_rule": "rule:admin_or_owner", + "get_security_group_rule": "rule:admin_owner_or_sg_owner", "create_security_group_rule": "rule:admin_or_owner", "delete_security_group_rule": "rule:admin_or_owner", diff --git a/neutron/api/v2/attributes.py b/neutron/api/v2/attributes.py index 33d75f33420..9159b6048a4 100644 --- a/neutron/api/v2/attributes.py +++ b/neutron/api/v2/attributes.py @@ -34,7 +34,8 @@ RESOURCE_ATTRIBUTE_MAP = attrs.RESOURCES # Identify the attribute used by a resource to reference another resource RESOURCE_FOREIGN_KEYS = { - net_def.COLLECTION_NAME: 'network_id' + net_def.COLLECTION_NAME: 'network_id', + 'security_groups': 'security_group_id' } diff --git a/neutron/db/securitygroups_db.py b/neutron/db/securitygroups_db.py index 6ac30c75ee7..df5ea39d9c4 100644 --- a/neutron/db/securitygroups_db.py +++ b/neutron/db/securitygroups_db.py @@ -679,8 +679,13 @@ class SecurityGroupDbMixin(ext_sg.SecurityGroupPluginBase): pager = base_obj.Pager( sorts=sorts, marker=marker, limit=limit, page_reverse=page_reverse) + # NOTE(slaweq): use admin context here to be able to get all rules + # which fits filters' criteria. Later in policy engine rules will be + # filtered and only those which are allowed according to policy will + # be returned rule_objs = sg_obj.SecurityGroupRule.get_objects( - context, _pager=pager, validate_filters=False, **filters + context_lib.get_admin_context(), _pager=pager, + validate_filters=False, **filters ) return [ self._make_security_group_rule_dict(obj.db_obj, fields) @@ -695,7 +700,12 @@ class SecurityGroupDbMixin(ext_sg.SecurityGroupPluginBase): @db_api.retry_if_session_inactive() def get_security_group_rule(self, context, id, fields=None): - security_group_rule = self._get_security_group_rule(context, id) + # NOTE(slaweq): use admin context here to be able to get all rules + # which fits filters' criteria. Later in policy engine rules will be + # filtered and only those which are allowed according to policy will + # be returned + security_group_rule = self._get_security_group_rule( + context_lib.get_admin_context(), id) return self._make_security_group_rule_dict( security_group_rule.db_obj, fields) diff --git a/neutron/tests/etc/policy.json b/neutron/tests/etc/policy.json index 02171ce1b25..e25bad690b8 100644 --- a/neutron/tests/etc/policy.json +++ b/neutron/tests/etc/policy.json @@ -13,6 +13,8 @@ "shared_address_scopes": "field:address_scopes:shared=True", "external": "field:networks:router:external=True", "default": "rule:admin_or_owner", + "admin_or_sg_owner": "rule:context_is_admin or tenant_id:%(security_group:tenant_id)s", + "admin_owner_or_sg_owner": "rule:owner or rule:admin_or_sg_owner", "create_subnet": "rule:admin_or_network_owner", "create_subnet:segment_id": "rule:admin_only", @@ -230,7 +232,7 @@ "update_security_group": "rule:admin_or_owner", "delete_security_group": "rule:admin_or_owner", "get_security_group_rules": "rule:admin_or_owner", - "get_security_group_rule": "rule:admin_or_owner", + "get_security_group_rule": "rule:admin_owner_or_sg_owner", "create_security_group_rule": "rule:admin_or_owner", "delete_security_group_rule": "rule:admin_or_owner", diff --git a/releasenotes/notes/show-all-security-group-rules-for-security-group-owner-6635dd3e4c6ab5ee.yaml b/releasenotes/notes/show-all-security-group-rules-for-security-group-owner-6635dd3e4c6ab5ee.yaml new file mode 100644 index 00000000000..647d0e69af1 --- /dev/null +++ b/releasenotes/notes/show-all-security-group-rules-for-security-group-owner-6635dd3e4c6ab5ee.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + Owners of security groups now see all security group rules which belong to + the security group, even if the rule was created by the admin user. + Fixes bug `1824248 `_.