From e4bc1de0c5488ef86a5939f9472adfbc08c2dec3 Mon Sep 17 00:00:00 2001 From: Johannes Kulik Date: Mon, 30 Jun 2025 15:52:34 +0200 Subject: [PATCH] Support provider-only tags We want to set tags on projects that normal users cannot remove or edit. Instead, only operators of the cloud provider should be able to - hence we call them provider tags. We identify provider tags by matching on prefixes of tags. These prefixes are configurable via the `provider_tag_prefix` config option, which can bet set multiple times to have multiple prefixes. It's meant to be used e.g. like this "cloudname::special" with `provider_tag_prefix = cloudname::`. To make the roles allowed to edit these tags configurable, we introduce a new set of policies. Since these policies check additional restrictions on already existing actions, we extended existing actions with another layer ":provider_tags" added. A scheme like this is already in use a lot in Neutron to limit users being allowed to set certain attribute of e.g. networks. Since customers might not be aware of provider keys in their tooling, we allow them to send requests that change the tags they are allowed to edit while keeping the provider tags in tact - even if they aren't sent in the same request. This should make it easier for the customers to e.g. delete all their tags on a project while not having to filter in tags they don't care about. Change-Id: I9a144b1efdbd2481dcb929288a3fee97a4772279 --- doc/source/getting-started/policy_mapping.rst | 6 + keystone/api/projects.py | 66 ++++- keystone/common/policies/project.py | 47 +++- keystone/conf/default.py | 8 + keystone/tests/unit/test_v3_resource.py | 255 ++++++++++++++++++ 5 files changed, 380 insertions(+), 2 deletions(-) diff --git a/doc/source/getting-started/policy_mapping.rst b/doc/source/getting-started/policy_mapping.rst index a7cb27cfa7..643d86243f 100644 --- a/doc/source/getting-started/policy_mapping.rst +++ b/doc/source/getting-started/policy_mapping.rst @@ -49,7 +49,9 @@ identity:get_project GET /v3/projects/{pro identity:list_projects GET /v3/projects identity:list_user_projects GET /v3/users/{user_id}/projects identity:create_project POST /v3/projects +identity:create_project:provider_tags identity:update_project PATCH /v3/projects/{project_id} +identity:update_project:provider_tags identity:delete_project DELETE /v3/projects/{project_id} identity:get_project_tag GET /v3/projects/{project_id}/tags/{tag_name} @@ -57,9 +59,13 @@ identity:get_project_tag GET /v3/projects/{pro identity:list_project_tags GET /v3/projects/{project_id}/tags HEAD /v3/projects/{project_id}/tags identity:create_project_tag PUT /v3/projects/{project_id}/tags/{tag_name} +identity:create_project_tag:provider_tags identity:update_project_tags PUT /v3/projects/{project_id}/tags +identity:update_project_tags:provider_tags identity:delete_project_tag DELETE /v3/projects/{project_id}/tags/{tag_name} +identity:delete_project_tag:provider_tags identity:delete_project_tags DELETE /v3/projects/{project_id}/tags +identity:delete_project_tags:provider_tags identity:get_user GET /v3/users/{user_id} identity:list_users GET /v3/users diff --git a/keystone/api/projects.py b/keystone/api/projects.py index 9cd524b084..f787679b42 100644 --- a/keystone/api/projects.py +++ b/keystone/api/projects.py @@ -12,6 +12,7 @@ # This file handles all flask-restful resources for /v3/projects +from collections.abc import Iterable import functools import flask @@ -47,6 +48,49 @@ def _build_project_target_enforcement(): return target +def is_provider_tag(tag: str) -> bool: + """Check if the provided tag is a provider tag.""" + return tag.startswith(tuple(CONF.provider_tag_prefix)) + + +def get_provider_tags(tags: Iterable[str]) -> set[str]: + """Return the provider tags from the given tags.""" + return set(t for t in tags if is_provider_tag(t)) + + +def validate_tags(project_id: str, tags: Iterable[str], policy: str + ) -> list[str]: + """Return the tags the user is allowed to change. + + We check whether the list of tags contains provider tags. If it does, we + check if the user is allowed to change provider tags. If they are not + allowed, we restore the original provider tags in the list. + """ + original_ptags = get_provider_tags( + PROVIDERS.resource_api.list_project_tags(project_id) + ) + ptags = get_provider_tags(tags) + removed_ptags = original_ptags - ptags + new_ptags = ptags - original_ptags + if removed_ptags or new_ptags: + # NOTE(jkulik): Adding/removing provider tags needs special + # privileges but the user might just send us an update of their own + # tags instead of including the provider tags, so we try to handle + # this gracefully here by updating the non-provider tags + # nonetheless. + try: + ENFORCER.enforce_call( + action='identity:%s:provider_tags' % policy, + build_target=_build_project_target_enforcement + ) + except exception.Forbidden: + # User is not allowed to change provider tags so we restore the + # original provider tags to allow them to update their tags. + tags = (set(tags) | removed_ptags) - new_ptags + + return list(tags) + + class ProjectResource(ks_flask.ResourceBase): collection_key = 'projects' member_key = 'project' @@ -182,6 +226,11 @@ def post(self): if not project.get('parent_id'): project['parent_id'] = project.get('domain_id') project = self._normalize_dict(project) + if get_provider_tags(project.get('tags', [])): + ENFORCER.enforce_call( + action='identity:create_project:provider_tags', + target_attr=target + ) try: ref = PROVIDERS.resource_api.create_project( project['id'], @@ -203,6 +252,9 @@ def patch(self, project_id): project = self.request_body_json.get('project', {}) validation.lazy_validate(schema.project_update, project) self._require_matching_id(project) + if project.get('tags') is not None: + project['tags'] = validate_tags( + project_id, project['tags'], 'update_project') ref = PROVIDERS.resource_api.update_project( project_id, project, @@ -264,6 +316,7 @@ def put(self, project_id): ) tags = self.request_body_json.get('tags', {}) validation.lazy_validate(schema.project_tags_update, tags) + tags = validate_tags(project_id, tags, 'update_project_tags') ref = PROVIDERS.resource_api.update_project_tags( project_id, tags, initiator=self.audit_initiator) return self.wrap_member(ref) @@ -277,7 +330,8 @@ def delete(self, project_id): action='identity:delete_project_tags', build_target=_build_project_target_enforcement ) - PROVIDERS.resource_api.update_project_tags(project_id, []) + tags = validate_tags(project_id, [], 'delete_project_tags') + PROVIDERS.resource_api.update_project_tags(project_id, tags) return None, http.client.NO_CONTENT @@ -308,6 +362,11 @@ def put(self, project_id, value): tags = PROVIDERS.resource_api.list_project_tags(project_id) tags.append(value) validation.lazy_validate(schema.project_tags_update, tags) + if is_provider_tag(value): + ENFORCER.enforce_call( + action='identity:create_project_tag:provider_tags', + build_target=_build_project_target_enforcement + ) PROVIDERS.resource_api.create_project_tag( project_id, value, @@ -327,6 +386,11 @@ def delete(self, project_id, value): action='identity:delete_project_tag', build_target=_build_project_target_enforcement ) + if is_provider_tag(value): + ENFORCER.enforce_call( + action='identity:delete_project_tag:provider_tags', + build_target=_build_project_target_enforcement + ) PROVIDERS.resource_api.delete_project_tag(project_id, value) return None, http.client.NO_CONTENT diff --git a/keystone/common/policies/project.py b/keystone/common/policies/project.py index 8796b9c388..e6a93ea0e7 100644 --- a/keystone/common/policies/project.py +++ b/keystone/common/policies/project.py @@ -65,6 +65,8 @@ '(role:admin and domain_id:%(target.project.domain_id)s)' ) +RULE_CLOUD_ADMIN_OR_SERVICE = 'rule:cloud_admin or rule:service_role' + DEPRECATED_REASON = ( "The project API is now aware of system scope and default roles." ) @@ -184,6 +186,13 @@ operations=[{'path': '/v3/projects', 'method': 'POST'}], deprecated_rule=deprecated_create_project), + policy.DocumentedRuleDefault( + name=base.IDENTITY % 'create_project:provider_tags', + check_str=RULE_CLOUD_ADMIN_OR_SERVICE, + scope_types=['system', 'domain', 'project'], + description='Create project with provider tags.', + operations=[{'path': '/v3/projects', + 'method': 'POST'}]), policy.DocumentedRuleDefault( name=base.IDENTITY % 'update_project', check_str=base.RULE_ADMIN_REQUIRED, @@ -192,6 +201,13 @@ operations=[{'path': '/v3/projects/{project_id}', 'method': 'PATCH'}], deprecated_rule=deprecated_update_project), + policy.DocumentedRuleDefault( + name=base.IDENTITY % 'update_project:provider_tags', + check_str=RULE_CLOUD_ADMIN_OR_SERVICE, + scope_types=['system', 'domain', 'project'], + description='Update project with provider tags.', + operations=[{'path': '/v3/projects/{project_id}', + 'method': 'PATCH'}]), policy.DocumentedRuleDefault( name=base.IDENTITY % 'delete_project', check_str=base.RULE_ADMIN_REQUIRED, @@ -228,6 +244,14 @@ operations=[{'path': '/v3/projects/{project_id}/tags', 'method': 'PUT'}], deprecated_rule=deprecated_update_project_tag), + policy.DocumentedRuleDefault( + name=base.IDENTITY % 'update_project_tags:provider_tags', + check_str=RULE_CLOUD_ADMIN_OR_SERVICE, + scope_types=['system', 'domain', 'project'], + description='Replace all tags on a project with the new set of tags ' + 'that includes provider tags.', + operations=[{'path': '/v3/projects/{project_id}/tags', + 'method': 'PUT'}]), policy.DocumentedRuleDefault( name=base.IDENTITY % 'create_project_tag', check_str=base.RULE_ADMIN_REQUIRED, @@ -236,6 +260,13 @@ operations=[{'path': '/v3/projects/{project_id}/tags/{value}', 'method': 'PUT'}], deprecated_rule=deprecated_create_project_tag), + policy.DocumentedRuleDefault( + name=base.IDENTITY % 'create_project_tag:provider_tags', + check_str=RULE_CLOUD_ADMIN_OR_SERVICE, + scope_types=['system', 'domain', 'project'], + description='Add a single provider tag to a project.', + operations=[{'path': '/v3/projects/{project_id}/tags/{value}', + 'method': 'PUT'}]), policy.DocumentedRuleDefault( name=base.IDENTITY % 'delete_project_tags', check_str=base.RULE_ADMIN_REQUIRED, @@ -244,6 +275,13 @@ operations=[{'path': '/v3/projects/{project_id}/tags', 'method': 'DELETE'}], deprecated_rule=deprecated_delete_project_tags), + policy.DocumentedRuleDefault( + name=base.IDENTITY % 'delete_project_tags:provider_tags', + check_str=RULE_CLOUD_ADMIN_OR_SERVICE, + scope_types=['system', 'domain', 'project'], + description='Remove all tags from a project including provider tags.', + operations=[{'path': '/v3/projects/{project_id}/tags', + 'method': 'DELETE'}]), policy.DocumentedRuleDefault( name=base.IDENTITY % 'delete_project_tag', check_str=base.RULE_ADMIN_REQUIRED, @@ -251,7 +289,14 @@ description='Delete a specified tag from project.', operations=[{'path': '/v3/projects/{project_id}/tags/{value}', 'method': 'DELETE'}], - deprecated_rule=deprecated_delete_project_tag) + deprecated_rule=deprecated_delete_project_tag), + policy.DocumentedRuleDefault( + name=base.IDENTITY % 'delete_project_tag:provider_tags', + check_str=RULE_CLOUD_ADMIN_OR_SERVICE, + scope_types=['system', 'domain', 'project'], + description='Delete a specified provider tag from project.', + operations=[{'path': '/v3/projects/{project_id}/tags/{value}', + 'method': 'DELETE'}]), ] diff --git a/keystone/conf/default.py b/keystone/conf/default.py index 9a1284b0ec..5be4c3ac56 100644 --- a/keystone/conf/default.py +++ b/keystone/conf/default.py @@ -115,6 +115,13 @@ default_tag=tag_1 """)) +provider_tag_prefix = cfg.MultiStrOpt( + 'provider_tag_prefix', + default=[], + help=utils.fmt(""" +Prefixes for `tag`(s) on projects that need special privileges. +""")) + notification_format = cfg.StrOpt( 'notification_format', default='cadf', @@ -158,6 +165,7 @@ insecure_debug, default_publisher_id, default_tag, + provider_tag_prefix, notification_format, notification_opt_out, ] diff --git a/keystone/tests/unit/test_v3_resource.py b/keystone/tests/unit/test_v3_resource.py index d90c85b0c1..1b3284e25d 100644 --- a/keystone/tests/unit/test_v3_resource.py +++ b/keystone/tests/unit/test_v3_resource.py @@ -10,12 +10,15 @@ # License for the specific language governing permissions and limitations # under the License. +import contextlib import uuid import http.client from testtools import matchers +from unittest import mock from keystone.common import provider_api +from keystone.common import rbac_enforcer import keystone.conf from keystone.credential.providers import fernet as credential_fernet from keystone import exception @@ -27,6 +30,7 @@ CONF = keystone.conf.CONF PROVIDERS = provider_api.ProviderAPIs +ENFORCER = rbac_enforcer.RBACEnforcer _DEFAULT_TAG = ['single_tag'] _DEFAULT_TAGS = [None, [], ['vc-a-0', 'tag_1', 'tag_2'], _DEFAULT_TAG] @@ -1876,6 +1880,257 @@ def test_update_project_tags_with_too_many_tags(self): body={'tags': tags}, expected_status=http.client.BAD_REQUEST) + @contextlib.contextmanager + def _provider_tags_rule_forbidden(self, name): + """Patch provider_tags rule to be forbidden.""" + policy = f"identity:{name}:provider_tags" + orig_fn = ENFORCER()._enforce + + def _enforce(*args, **kwargs): + if kwargs.get('action') == policy: + raise exception.ForbiddenAction(action=kwargs['action']) + return orig_fn(*args, **kwargs) + + with mock.patch.object(ENFORCER, '_enforce', side_effect=_enforce): + yield + + def test_provider_tags_project_post(self): + """Normal users cannot create projects while specifying provider tags. + + Normal users get an error back on project creation. + Admins can create projects with provider tags. + """ + self.config_fixture.config(provider_tag_prefix=['provider-']) + tags = ['special', 'provider-special'] + + with self._provider_tags_rule_forbidden('create_project'): + ref = unit.new_project_ref(domain_id=self.domain_id, tags=tags) + r = self.post( + '/projects', + body={'project': ref}, + expected_status=http.client.FORBIDDEN) + + ref = unit.new_project_ref(domain_id=self.domain_id, tags=tags) + r = self.post( + '/projects', + body={'project': ref}) + self.assertValidProjectResponse(r, ref) + + def test_provider_tags_project_post_empty_list(self): + """Normal users cannot create projects while specifying provider tags. + + Normal users get an error back on project creation. + Admins can create projects with provider tags. + """ + self.config_fixture.config(provider_tag_prefix=['provider-']) + tags = [] + + with self._provider_tags_rule_forbidden('create_project'): + ref = unit.new_project_ref(domain_id=self.domain_id, tags=tags) + r = self.post( + '/projects', + body={'project': ref}) + self.assertValidProjectResponse(r, ref) + + ref = unit.new_project_ref(domain_id=self.domain_id, tags=tags) + r = self.post( + '/projects', + body={'project': ref}) + self.assertValidProjectResponse(r, ref) + + def test_provider_tags_project_patch(self): + """Normal users cannot add/remove provider tags when updating project. + + We keep provider-tags or don't add them for normal users. + Admins can remove/add provider-tags when updating a project. + """ + project, _ = self._create_project_and_tags() + self.config_fixture.config(provider_tag_prefix=['provider-']) + tags = ['special', 'provider-special'] + PROVIDERS.resource_api.update_project_tags(project['id'], tags) + + with self._provider_tags_rule_forbidden('update_project'): + project['tags'] = ['more-special'] + self.patch( + f"/projects/{project['id']}", + body={'project': project}) + self.assertListEqual( + sorted(['more-special', 'provider-special']), + PROVIDERS.resource_api.list_project_tags(project['id'])) + + project['tags'] = ['special', 'provider-more-special'] + self.patch( + f"/projects/{project['id']}", + body={'project': project}) + self.assertListEqual( + sorted(['special', 'provider-special']), + PROVIDERS.resource_api.list_project_tags(project['id'])) + + project['tags'] = [] + self.patch( + f"/projects/{project['id']}", + body={'project': project}) + self.assertListEqual( + ['provider-special'], + PROVIDERS.resource_api.list_project_tags(project['id'])) + + project['tags'] = ['provider-more-special'] + self.patch( + f"/projects/{project['id']}", + body={'project': project}) + self.assertListEqual( + ['provider-more-special'], + PROVIDERS.resource_api.list_project_tags(project['id'])) + + project['tags'] = [] + self.patch( + f"/projects/{project['id']}", + body={'project': project}) + self.assertListEqual( + [], + PROVIDERS.resource_api.list_project_tags(project['id'])) + + def test_provider_tags_project_tags_put(self): + """Normal users cannot add provider tags when setting project tags. + + List of tags gets filtered for provider tags. The rest is set. + For admins, the whole list is set. + """ + project, _ = self._create_project_and_tags() + self.config_fixture.config(provider_tag_prefix=['provider-']) + tags = ['special', 'provider-special'] + + with self._provider_tags_rule_forbidden('update_project_tags'): + self.put( + f"/projects/{project['id']}/tags", + body={'tags': tags}, + expected_status=http.client.OK) + self.assertListEqual( + ['special'], + PROVIDERS.resource_api.list_project_tags(project['id'])) + + self.put( + '/projects/%(project_id)s/tags' % {'project_id': project['id']}, + body={'tags': tags}, + expected_status=http.client.OK) + self.assertListEqual( + sorted(tags), + sorted(PROVIDERS.resource_api.list_project_tags(project['id']))) + + def test_provider_tags_project_tags_put_empty_list(self): + """Normal users cannot remove provider tags with an empty list. + + All tags get remove except the provider tags. + For admins, all tags get removed. + """ + project, _ = self._create_project_and_tags() + self.config_fixture.config(provider_tag_prefix=['provider-']) + tags = ['special', 'provider-special'] + PROVIDERS.resource_api.update_project_tags(project['id'], tags) + new_tags = [] + + with self._provider_tags_rule_forbidden('update_project_tags'): + self.put( + f"/projects/{project['id']}/tags", + body={'tags': new_tags}, + expected_status=http.client.OK) + self.assertListEqual( + ['provider-special'], + PROVIDERS.resource_api.list_project_tags(project['id'])) + + self.put( + '/projects/%(project_id)s/tags' % {'project_id': project['id']}, + body={'tags': new_tags}, + expected_status=http.client.OK) + self.assertListEqual( + [], + sorted(PROVIDERS.resource_api.list_project_tags(project['id']))) + + def test_provider_tags_project_tags_delete(self): + """Normal users cannot delete provider tags. + + All but the provider tags get deleted. + For admins, everything gets deleted. + """ + project, _ = self._create_project_and_tags() + self.config_fixture.config(provider_tag_prefix=['provider-']) + tags = ['special', 'provider-special'] + PROVIDERS.resource_api.update_project_tags(project['id'], tags) + + with self._provider_tags_rule_forbidden('delete_project_tags'): + self.delete( + f"/projects/{project['id']}/tags", + expected_status=http.client.NO_CONTENT) + self.assertListEqual( + ['provider-special'], + PROVIDERS.resource_api.list_project_tags(project['id'])) + + PROVIDERS.resource_api.update_project_tags(project['id'], tags) + self.delete( + f"/projects/{project['id']}/tags", + expected_status=http.client.NO_CONTENT) + self.assertListEqual( + [], + PROVIDERS.resource_api.list_project_tags(project['id'])) + + def test_provider_tags_project_tag_put(self): + """Normal users cannot add a provider tag. + + Normal users get an error back. + Admins can add the tag. + """ + project, _ = self._create_project_and_tags() + self.config_fixture.config(provider_tag_prefix=['provider-']) + PROVIDERS.resource_api.update_project_tags(project['id'], []) + tags = ['special', 'provider-special'] + + with self._provider_tags_rule_forbidden('create_project_tag'): + self.put( + f"/projects/{project['id']}/tags/{tags[0]}", + expected_status=http.client.CREATED) + self.put( + f"/projects/{project['id']}/tags/{tags[1]}", + expected_status=http.client.FORBIDDEN) + self.assertListEqual( + ['special'], + PROVIDERS.resource_api.list_project_tags(project['id'])) + + self.put( + f"/projects/{project['id']}/tags/{tags[1]}", + expected_status=http.client.CREATED) + self.assertListEqual( + sorted(tags), + sorted(PROVIDERS.resource_api.list_project_tags(project['id']))) + + def test_provider_tags_project_tag_delete(self): + """Normal users cannot delete a provider tag. + + Normal users get an erro back for provider tags. + Admins can remove the tag. + """ + project, _ = self._create_project_and_tags() + self.config_fixture.config(provider_tag_prefix=['provider-']) + tags = ['special', 'provider-special'] + PROVIDERS.resource_api.update_project_tags(project['id'], tags) + + with self._provider_tags_rule_forbidden('delete_project_tag'): + self.delete( + f"/projects/{project['id']}/tags/{tags[0]}", + expected_status=http.client.NO_CONTENT) + self.delete( + f"/projects/{project['id']}/tags/{tags[1]}", + expected_status=http.client.FORBIDDEN) + self.assertListEqual( + ['provider-special'], + PROVIDERS.resource_api.list_project_tags(project['id'])) + + self.delete( + f"/projects/{project['id']}/tags/{tags[1]}", + expected_status=http.client.NO_CONTENT) + self.assertListEqual( + [], + PROVIDERS.resource_api.list_project_tags(project['id'])) + def test_list_projects_by_user_with_inherited_role(self): """Ensure the cache is invalidated when creating/deleting a project.""" domain_ref = unit.new_domain_ref()