From 4f4a3e0972c68fd646c98d0d0b40cfb9652ae499 Mon Sep 17 00:00:00 2001 From: Artem Goncharov Date: Thu, 9 Jan 2025 12:20:46 +0100 Subject: [PATCH] Add jsonschema for Group resource Change-Id: I3518dd0e2d5375d4902263c52dfc4bb6f9b13304 --- keystone/api/groups.py | 76 ++++++++++++++++------- keystone/identity/schema.py | 83 ++++++++++++++++++++++---- keystone/tests/unit/test_validation.py | 29 ++++----- 3 files changed, 140 insertions(+), 48 deletions(-) diff --git a/keystone/api/groups.py b/keystone/api/groups.py index d56ac22ae9..98cc66e39e 100644 --- a/keystone/api/groups.py +++ b/keystone/api/groups.py @@ -18,10 +18,11 @@ import http.client import flask import flask_restful +from keystone.api import validation from keystone.common import json_home from keystone.common import provider_api from keystone.common import rbac_enforcer -from keystone.common import validation +from keystone.common import validation as ks_validation import keystone.conf from keystone import exception from keystone.identity import schema @@ -54,23 +55,10 @@ class GroupsResource(ks_flask.ResourceBase): api='identity_api', method='get_group' ) - def get(self, group_id=None): - if group_id is not None: - return self._get_group(group_id) - return self._list_groups() - - def _get_group(self, group_id): - """Get a group reference. - - GET/HEAD /groups/{group_id} - """ - ENFORCER.enforce_call( - action='identity:get_group', - build_target=_build_group_target_enforcement, - ) - return self.wrap_member(PROVIDERS.identity_api.get_group(group_id)) - - def _list_groups(self): + @validation.request_query_schema(schema.group_index_request_query) + @validation.request_body_schema(None) + @validation.response_body_schema(schema.group_index_response_body) + def get(self): """List groups. GET/HEAD /groups @@ -95,6 +83,9 @@ class GroupsResource(ks_flask.ResourceBase): refs = filtered_refs return self.wrap_collection(refs, hints=hints) + @validation.request_query_schema(None) + @validation.request_body_schema(schema.group_create_request_body) + @validation.response_body_schema(schema.group_create_response_body) def post(self): """Create group. @@ -105,7 +96,6 @@ class GroupsResource(ks_flask.ResourceBase): ENFORCER.enforce_call( action='identity:create_group', target_attr=target ) - validation.lazy_validate(schema.group_create, group) group = self._normalize_dict(group) group = self._normalize_domain_id(group) ref = PROVIDERS.identity_api.create_group( @@ -113,7 +103,32 @@ class GroupsResource(ks_flask.ResourceBase): ) return self.wrap_member(ref), http.client.CREATED - def patch(self, group_id): + +class GroupResource(ks_flask.ResourceBase): + collection_key = 'groups' + member_key = 'group' + get_member_from_driver = PROVIDERS.deferred_provider_lookup( + api='identity_api', method='get_group' + ) + + @validation.request_query_schema(None) + @validation.request_body_schema(None) + @validation.response_body_schema(schema.group_get_response_body) + def get(self, group_id: str): + """Get a group reference. + + GET/HEAD /groups/{group_id} + """ + ENFORCER.enforce_call( + action='identity:get_group', + build_target=_build_group_target_enforcement, + ) + return self.wrap_member(PROVIDERS.identity_api.get_group(group_id)) + + @validation.request_query_schema(None) + @validation.request_body_schema(schema.group_update_request_body) + @validation.response_body_schema(schema.group_update_response_body) + def patch(self, group_id: str): """Update group. PATCH /groups/{group_id} @@ -123,14 +138,16 @@ class GroupsResource(ks_flask.ResourceBase): build_target=_build_group_target_enforcement, ) group = self.request_body_json.get('group', {}) - validation.lazy_validate(schema.group_update, group) self._require_matching_id(group) ref = PROVIDERS.identity_api.update_group( group_id, group, initiator=self.audit_initiator ) return self.wrap_member(ref) - def delete(self, group_id): + @validation.request_query_schema(None) + @validation.request_body_schema(None) + @validation.response_body_schema(None) + def delete(self, group_id: str): """Delete group. DELETE /groups/{group_id} @@ -245,8 +262,21 @@ class UserGroupCRUDResource(flask_restful.Resource): class GroupAPI(ks_flask.APIBase): _name = 'groups' _import_name = __name__ - resources = [GroupsResource] resource_mapping = [ + ks_flask.construct_resource_map( + resource=GroupsResource, + url='/groups', + resource_kwargs={}, + rel='groups', + path_vars={}, + ), + ks_flask.construct_resource_map( + resource=GroupResource, + url='/groups/', + resource_kwargs={}, + rel='group', + path_vars={'group_id': json_home.Parameters.GROUP_ID}, + ), ks_flask.construct_resource_map( resource=GroupUsersResource, url='/groups//users', diff --git a/keystone/identity/schema.py b/keystone/identity/schema.py index 064542cb9e..4837d381c3 100644 --- a/keystone/identity/schema.py +++ b/keystone/identity/schema.py @@ -186,26 +186,87 @@ user_update_request: dict[str, Any] = { user_update_response_body: dict[str, Any] = user_get_response_body -_group_properties = { +group_index_request_query: dict[str, Any] = { + "type": "object", + "properties": { + "domain_id": parameter_types.domain_id, + "name": parameter_types.name, + }, + "additionalProperties": False, +} + +_group_properties: dict[str, Any] = { 'description': validation.nullable(parameter_types.description), 'domain_id': parameter_types.id_string, + 'id': {"type": "string", "description": "The user ID.", "readOnly": True}, 'name': _identity_name, } -group_create = { - 'type': 'object', - 'properties': _group_properties, - 'required': ['name'], - 'additionalProperties': True, +group_schema: dict[str, Any] = { + "type": "object", + "properties": _group_properties, + # NOTE(gtema) Group resource supports additional attributes which are stored + # in the `extra` DB field + "additionalProperties": True, } -group_update = { - 'type': 'object', - 'properties': _group_properties, - 'minProperties': 1, - 'additionalProperties': True, +group_index_response_body: dict[str, Any] = { + "type": "object", + "properties": { + "groups": { + "type": "array", + "items": group_schema, + "description": "A list of group objects", + }, + "links": response_types.links, + "truncated": response_types.truncated, + }, + "required": ["groups"], + "additionalProperties": False, } +group_get_response_body: dict[str, Any] = { + "type": "object", + "properties": {"group": group_schema}, + "required": ["group"], + "additionalProperties": False, +} + +group_create_request_body: dict[str, Any] = { + 'type': 'object', + 'properties': { + 'group': { + 'type': 'object', + 'properties': _group_properties, + 'required': ['name'], + 'additionalProperties': True, + } + }, + "required": ["group"], + "additionalProperties": False, +} + +group_create_response_body = group_get_response_body + +group_update_properties = copy.deepcopy(_group_properties) +# It is not allowed anymore to update domain of the existing group +group_update_properties.pop("domain_id", None) +group_update_request_body: dict[str, Any] = { + 'type': 'object', + 'properties': { + 'group': { + 'type': 'object', + 'properties': group_update_properties, + 'minProperties': 1, + 'additionalProperties': True, + } + }, + "required": ["group"], + 'additionalProperties': False, +} + +group_update_response_body = group_get_response_body + _password_change_properties = { 'original_password': {'type': 'string'}, 'password': {'type': 'string'}, diff --git a/keystone/tests/unit/test_validation.py b/keystone/tests/unit/test_validation.py index 674d8b8b5c..3771d19d2e 100644 --- a/keystone/tests/unit/test_validation.py +++ b/keystone/tests/unit/test_validation.py @@ -2583,28 +2583,30 @@ class GroupValidationTestCase(unit.BaseTestCase): self.group_name = uuid.uuid4().hex - create = identity_schema.group_create - update = identity_schema.group_update + create = identity_schema.group_create_request_body + update = identity_schema.group_update_request_body self.create_group_validator = validators.SchemaValidator(create) self.update_group_validator = validators.SchemaValidator(update) def test_validate_group_create_succeeds(self): """Validate create group requests.""" - request_to_validate = {'name': self.group_name} + request_to_validate = {'group': {'name': self.group_name}} self.create_group_validator.validate(request_to_validate) def test_validate_group_create_succeeds_with_all_parameters(self): """Validate create group requests with all parameters.""" request_to_validate = { - 'name': self.group_name, - 'description': uuid.uuid4().hex, - 'domain_id': uuid.uuid4().hex, + 'group': { + 'name': self.group_name, + 'description': uuid.uuid4().hex, + 'domain_id': uuid.uuid4().hex, + } } self.create_group_validator.validate(request_to_validate) def test_validate_group_create_fails_without_group_name(self): """Exception raised when group name is not provided in request.""" - request_to_validate = {'description': uuid.uuid4().hex} + request_to_validate = {'group': {'description': uuid.uuid4().hex}} self.assertRaises( exception.SchemaValidationError, self.create_group_validator.validate, @@ -2614,15 +2616,14 @@ class GroupValidationTestCase(unit.BaseTestCase): def test_validate_group_create_succeeds_with_extra_parameters(self): """Validate extra attributes on group create requests.""" request_to_validate = { - 'name': self.group_name, - 'other_attr': uuid.uuid4().hex, + 'group': {'name': self.group_name, 'other_attr': uuid.uuid4().hex} } self.create_group_validator.validate(request_to_validate) def test_validate_group_create_fails_with_invalid_name(self): """Exception when validating a create request with invalid `name`.""" for invalid_name in _INVALID_NAMES: - request_to_validate = {'name': invalid_name} + request_to_validate = {'group': {'name': invalid_name}} self.assertRaises( exception.SchemaValidationError, self.create_group_validator.validate, @@ -2631,12 +2632,12 @@ class GroupValidationTestCase(unit.BaseTestCase): def test_validate_group_update_succeeds(self): """Validate group update requests.""" - request_to_validate = {'description': uuid.uuid4().hex} + request_to_validate = {'group': {'description': uuid.uuid4().hex}} self.update_group_validator.validate(request_to_validate) def test_validate_group_update_fails_with_no_parameters(self): """Exception raised when no parameters passed in on update.""" - request_to_validate = {} + request_to_validate = {'group': {}} self.assertRaises( exception.SchemaValidationError, self.update_group_validator.validate, @@ -2645,13 +2646,13 @@ class GroupValidationTestCase(unit.BaseTestCase): def test_validate_group_update_succeeds_with_extra_parameters(self): """Validate group update requests with extra parameters.""" - request_to_validate = {'other_attr': uuid.uuid4().hex} + request_to_validate = {'group': {'other_attr': uuid.uuid4().hex}} self.update_group_validator.validate(request_to_validate) def test_validate_group_update_fails_with_invalid_name(self): """Exception when validating an update request with invalid `name`.""" for invalid_name in _INVALID_NAMES: - request_to_validate = {'name': invalid_name} + request_to_validate = {'group': {'name': invalid_name}} self.assertRaises( exception.SchemaValidationError, self.update_group_validator.validate,