diff --git a/neutron/conf/policies/security_group.py b/neutron/conf/policies/security_group.py index dbc2d7a6823..f1a74b5b5f5 100644 --- a/neutron/conf/policies/security_group.py +++ b/neutron/conf/policies/security_group.py @@ -20,8 +20,22 @@ SG_RESOURCE_PATH = '/security-groups/{id}' RULE_COLLECTION_PATH = '/security-group-rules' RULE_RESOURCE_PATH = '/security-group-rules/{id}' +RULE_ADMIN_OR_SG_OWNER = 'rule:admin_or_sg_owner' +RULE_ADMIN_OWNER_OR_SG_OWNER = 'rule:admin_owner_or_sg_owner' + rules = [ + policy.RuleDefault( + 'admin_or_sg_owner', + base.policy_or('rule:context_is_admin', + 'tenant_id:%(security_group:tenant_id)s'), + description='Rule for admin or security group owner access'), + policy.RuleDefault( + 'admin_owner_or_sg_owner', + base.policy_or('rule:owner', + RULE_ADMIN_OR_SG_OWNER), + description=('Rule for resource owner, ' + 'admin or security group owner access')), # TODO(amotoki): admin_or_owner is the right rule? # Does an empty string make more sense for create_security_group? policy.DocumentedRuleDefault( @@ -88,7 +102,7 @@ rules = [ ), policy.DocumentedRuleDefault( 'get_security_group_rule', - base.RULE_ADMIN_OR_OWNER, + RULE_ADMIN_OWNER_OR_SG_OWNER, 'Get a security group rule', [ { diff --git a/neutron/db/securitygroups_db.py b/neutron/db/securitygroups_db.py index 0065af4acd1..da5167d8dc5 100644 --- a/neutron/db/securitygroups_db.py +++ b/neutron/db/securitygroups_db.py @@ -680,8 +680,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) @@ -696,7 +701,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/policy.py b/neutron/policy.py index bc3ebd49ca9..ea5eff506d5 100644 --- a/neutron/policy.py +++ b/neutron/policy.py @@ -45,7 +45,8 @@ ADVSVC_CTX_POLICY = 'context_is_advsvc' # Identify the attribute used by a resource to reference another resource _RESOURCE_FOREIGN_KEYS = { - net_apidef.COLLECTION_NAME: 'network_id' + net_apidef.COLLECTION_NAME: 'network_id', + 'security_groups': 'security_group_id' } 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 `_.