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()