diff --git a/keystone/api/credentials.py b/keystone/api/credentials.py index 46e7790df4..7f46e19299 100644 --- a/keystone/api/credentials.py +++ b/keystone/api/credentials.py @@ -18,9 +18,10 @@ import http.client import flask from oslo_serialization import jsonutils +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 import keystone.conf from keystone.credential import schema from keystone import exception @@ -46,7 +47,35 @@ def _build_target_enforcement(): return target -class CredentialResource(ks_flask.ResourceBase): +def _blob_to_json(ref: dict) -> dict: + """Serialize blob as string.""" + # credentials stored via ec2tokens before the fix for #1259584 + # need json_serailzing, as that's the documented API format + blob = ref.get('blob') + if isinstance(blob, dict): + ref = ref.copy() + ref['blob'] = jsonutils.dumps(blob) + return ref + + +def _validate_blob_json(ref: dict) -> dict: + """Validate `blob` is a valid object.""" + try: + blob = jsonutils.loads(ref.get('blob')) + except (ValueError, TabError): + raise exception.ValidationError( + message=_('Invalid blob in credential') + ) + if not blob or not isinstance(blob, dict): + raise exception.ValidationError(attribute='blob', target='credential') + if blob.get('access') is None: + raise exception.ValidationError( + attribute='access', target='credential' + ) + return blob + + +class CredentialsResource(ks_flask.ResourceBase): collection_key = 'credentials' member_key = 'credential' @@ -82,7 +111,7 @@ class CredentialResource(ks_flask.ResourceBase): ): # Generates an assigns a unique identifier to a credential reference. if ref.get('type', '').lower() == 'ec2': - blob = self._validate_blob_json(ref) + blob = _validate_blob_json(ref) ref = ref.copy() ref['id'] = hashlib.sha256( blob['access'].encode('utf8') @@ -104,7 +133,13 @@ class CredentialResource(ks_flask.ResourceBase): else: return super()._assign_unique_id(ref) - def _list_credentials(self): + @validation.request_query_schema(schema.index_request_query) + @validation.response_body_schema(schema.index_response_body) + def get(self): + """List credentials. + + GET /v3/credentials + """ filters = ['user_id', 'type'] if not self.oslo_context.system_scope: target = {'credential': {'user_id': self.oslo_context.user_id}} @@ -136,34 +171,22 @@ class CredentialResource(ks_flask.ResourceBase): except exception.Forbidden: pass refs = filtered_refs - refs = [self._blob_to_json(r) for r in refs] + refs = [_blob_to_json(r) for r in refs] return self.wrap_collection(refs, hints=hints) - def _get_credential(self, credential_id): - ENFORCER.enforce_call( - action='identity:get_credential', - build_target=_build_target_enforcement, - ) - credential = PROVIDERS.credential_api.get_credential(credential_id) - return self.wrap_member(self._blob_to_json(credential)) - - def get(self, credential_id=None): - # Get Credential or List of credentials. - if credential_id is None: - # No Parameter passed means that we're doing a LIST action. - return self._list_credentials() - else: - return self._get_credential(credential_id) - + @validation.request_body_schema(schema.create_request_body) + @validation.response_body_schema(schema.credential_response_body) def post(self): - # Create a new credential + """Create new credentials. + + POST /v3/credentials + """ credential = self.request_body_json.get('credential', {}) target = {} target['credential'] = credential ENFORCER.enforce_call( action='identity:create_credential', target_attr=target ) - validation.lazy_validate(schema.credential_create, credential) trust_id = getattr(self.oslo_context, 'trust_id', None) app_cred_id = getattr( self.auth_context['token'], 'application_credential_id', None @@ -182,9 +205,14 @@ class CredentialResource(ks_flask.ResourceBase): ) return self.wrap_member(ref), http.client.CREATED + +class CredentialResource(ks_flask.ResourceBase): + collection_key = 'credentials' + member_key = 'credential' + def _validate_blob_update_keys(self, credential, ref): if credential.get('type', '').lower() == 'ec2': - new_blob = self._validate_blob_json(ref) + new_blob = _validate_blob_json(ref) old_blob = credential.get('blob') if isinstance(old_blob, str): old_blob = jsonutils.loads(old_blob) @@ -199,8 +227,27 @@ class CredentialResource(ks_flask.ResourceBase): message = _('%s can not be updated for credential') % key raise exception.ValidationError(message=message) - def patch(self, credential_id): - # Update Credential + @validation.response_body_schema(schema.credential_response_body) + def get(self, credential_id: str): + """Retrieve existing credentials. + + GET /v3/credentials/{credential_id} + """ + # Get Credential. + ENFORCER.enforce_call( + action='identity:get_credential', + build_target=_build_target_enforcement, + ) + credential = PROVIDERS.credential_api.get_credential(credential_id) + return self.wrap_member(_blob_to_json(credential)) + + @validation.request_body_schema(schema.update_request_body) + @validation.response_body_schema(schema.credential_response_body) + def patch(self, credential_id: str): + """Update existing credentials. + + PATCH /v3/credentials/{credential_id} + """ ENFORCER.enforce_call( action='identity:update_credential', build_target=_build_target_enforcement, @@ -208,7 +255,6 @@ class CredentialResource(ks_flask.ResourceBase): current = PROVIDERS.credential_api.get_credential(credential_id) credential = self.request_body_json.get('credential', {}) - validation.lazy_validate(schema.credential_update, credential) self._validate_blob_update_keys(current.copy(), credential.copy()) self._require_matching_id(credential) # Check that the user hasn't illegally modified the owner or scope @@ -221,8 +267,11 @@ class CredentialResource(ks_flask.ResourceBase): ) return self.wrap_member(ref) - def delete(self, credential_id): - # Delete credentials + def delete(self, credential_id: str): + """Delete credentials. + + DELETE /v3/credentials/{credential_id} + """ ENFORCER.enforce_call( action='identity:delete_credential', build_target=_build_target_enforcement, @@ -240,8 +289,26 @@ class CredentialAPI(ks_flask.APIBase): _name = 'credentials' _import_name = __name__ - resource_mapping = [] - resources = [CredentialResource] + resource_mapping = [ + ks_flask.construct_resource_map( + resource=CredentialsResource, + url='/credentials', + resource_kwargs={}, + rel="credentials", + path_vars=None, + ), + ks_flask.construct_resource_map( + resource=CredentialResource, + url='/credentials/', + resource_kwargs={}, + rel="credential", + path_vars={ + 'credential_id': json_home.build_v3_parameter_relation( + "credential_id" + ) + }, + ), + ] APIs = (CredentialAPI,) diff --git a/keystone/common/validation/validators.py b/keystone/common/validation/validators.py index 60a7e3a79c..2ee02923f6 100644 --- a/keystone/common/validation/validators.py +++ b/keystone/common/validation/validators.py @@ -54,7 +54,8 @@ def validate_password(password): class SchemaValidator: """Resource reference validator class.""" - validator_org = jsonschema.Draft4Validator + # Use 2020 Schema consistently in all other OpenStack services + validator_org = jsonschema.Draft202012Validator def __init__(self, schema): # NOTE(lbragstad): If at some point in the future we want to extend diff --git a/keystone/credential/schema.py b/keystone/credential/schema.py index a5cdb088df..1cd17d01d1 100644 --- a/keystone/credential/schema.py +++ b/keystone/credential/schema.py @@ -9,36 +9,133 @@ # WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the # License for the specific language governing permissions and limitations # under the License. +from typing import Any +from keystone.api.validation import response_types +# Individual properties of the `Credential` _credential_properties = { - 'blob': {'type': 'string'}, - 'project_id': {'type': 'string'}, - 'type': {'type': 'string'}, - 'user_id': {'type': 'string'}, + "blob": { + "type": "string", + "description": "The credential itself, as a serialized blob.", + }, + "project_id": { + "type": ["string", "null"], + "description": "The ID for the project. Mandatory for `EC2` type.", + }, + "type": { + "type": "string", + "description": ( + "The credential type, such as ec2 or cert. The implementation" + " determines the list of supported types." + ), + }, + "user_id": { + "type": "string", + "format": "uuid", + "description": "The ID of the user who owns the credential.", + }, } -credential_create = { - 'type': 'object', - 'properties': _credential_properties, - 'additionalProperties': True, - 'oneOf': [ - { - 'title': 'ec2 credential requires project_id', - 'required': ['blob', 'type', 'user_id', 'project_id'], - 'properties': {'type': {'enum': ['ec2']}}, +# Common schema of the `Credential` resource +credential_schema: dict[str, Any] = { + "type": "object", + "description": "A credential object.", + "properties": { + "id": { + "type": "string", + "readOnly": True, + "description": "The UUID for the credential.", }, - { - 'title': 'non-ec2 credential does not require project_id', - 'required': ['blob', 'type', 'user_id'], - 'properties': {'type': {'not': {'enum': ['ec2']}}}, - }, - ], + "links": response_types.resource_links, + **_credential_properties, + }, + "additionalProperties": True, } -credential_update = { - 'type': 'object', - 'properties': _credential_properties, - 'minProperties': 1, - 'additionalProperties': True, +# Query parameters of the `/credentials` API +index_request_query: dict[str, Any] = { + "type": "object", + "properties": { + "user_id": { + "type": "string", + "format": "uuid", + "description": "Filters the response by a user ID.", + }, + "type": { + "type": "string", + "description": ( + "The credential type, such as ec2 or cert. The implementation" + " determines the list of supported types." + ), + }, + }, +} + +# Response of the `/credentials` API +index_response_body: dict[str, Any] = { + "type": "object", + "properties": { + "credentials": { + "type": "array", + "items": credential_schema, + "description": "A list of credential objects.", + }, + "links": response_types.links, + "truncated": response_types.truncated, + }, + "additionalProperties": False, +} + +# Response of the diverse `/credentials` APIs returning single +# credential +credential_response_body: dict[str, Any] = { + "type": "object", + "description": "A credential object.", + "properties": {"credential": credential_schema}, + "additionalProperties": False, +} + +# Request body of the `POST /credentials` operation +create_request_body: dict[str, Any] = { + "type": "object", + "description": "A credential object.", + "properties": { + "credential": { + "type": "object", + "properties": { + "id": { + "type": "string", + "description": "The UUID for the credential.", + }, + **_credential_properties, + }, + "additionalProperties": True, + "required": ["blob", "type", "user_id"], + # Generating client code for conditionally optional parameters is + # hard. Use if-then-else for validation keeping the schema itself + # static. + "if": {"properties": {"type": {"const": "ec2"}}}, + "then": { + "title": "ec2 credential requires project_id", + "required": ["blob", "type", "user_id", "project_id"], + }, + } + }, + "required": ["credential"], +} + +# Request body of the `PATCH /credentials/{credential_id}` operation +update_request_body: dict[str, Any] = { + "type": "object", + "description": "A credential object.", + "properties": { + "credential": { + "type": "object", + "properties": _credential_properties, + "additionalProperties": True, + "minProperties": 1, + }, + }, + "required": ["credential"], } diff --git a/keystone/tests/unit/test_validation.py b/keystone/tests/unit/test_validation.py index af05b56fc6..a735f2864a 100644 --- a/keystone/tests/unit/test_validation.py +++ b/keystone/tests/unit/test_validation.py @@ -1061,24 +1061,29 @@ class CredentialValidationTestCase(unit.BaseTestCase): def setUp(self): super().setUp() - create = credential_schema.credential_create - update = credential_schema.credential_update + # Use full request body schemas + create = credential_schema.create_request_body + update = credential_schema.update_request_body self.create_credential_validator = validators.SchemaValidator(create) self.update_credential_validator = validators.SchemaValidator(update) def test_validate_credential_succeeds(self): """Test that we validate a credential request.""" request_to_validate = { - 'blob': 'some string', - 'project_id': uuid.uuid4().hex, - 'type': 'ec2', - 'user_id': uuid.uuid4().hex, + 'credential': { + 'blob': 'some string', + 'project_id': uuid.uuid4().hex, + 'type': 'ec2', + 'user_id': uuid.uuid4().hex, + } } self.create_credential_validator.validate(request_to_validate) def test_validate_credential_without_blob_fails(self): """Exception raised without `blob` in create request.""" - request_to_validate = {'type': 'ec2', 'user_id': uuid.uuid4().hex} + request_to_validate = { + 'credential': {'type': 'ec2', 'user_id': uuid.uuid4().hex} + } self.assertRaises( exception.SchemaValidationError, self.create_credential_validator.validate, @@ -1087,7 +1092,9 @@ class CredentialValidationTestCase(unit.BaseTestCase): def test_validate_credential_without_user_id_fails(self): """Exception raised without `user_id` in create request.""" - request_to_validate = {'blob': 'some credential blob', 'type': 'ec2'} + request_to_validate = { + 'credential': {'blob': 'some credential blob', 'type': 'ec2'} + } self.assertRaises( exception.SchemaValidationError, self.create_credential_validator.validate, @@ -1097,8 +1104,10 @@ class CredentialValidationTestCase(unit.BaseTestCase): def test_validate_credential_without_type_fails(self): """Exception raised without `type` in create request.""" request_to_validate = { - 'blob': 'some credential blob', - 'user_id': uuid.uuid4().hex, + 'credential': { + 'blob': 'some credential blob', + 'user_id': uuid.uuid4().hex, + } } self.assertRaises( exception.SchemaValidationError, @@ -1113,9 +1122,11 @@ class CredentialValidationTestCase(unit.BaseTestCase): and no `project_id` is provided in create request. """ request_to_validate = { - 'blob': 'some credential blob', - 'type': 'ec2', - 'user_id': uuid.uuid4().hex, + 'credential': { + 'blob': 'some credential blob', + 'type': 'ec2', + 'user_id': uuid.uuid4().hex, + } } self.assertRaises( exception.SchemaValidationError, @@ -1129,10 +1140,12 @@ class CredentialValidationTestCase(unit.BaseTestCase): for c_type in cred_types: request_to_validate = { - 'blob': 'some blob', - 'project_id': uuid.uuid4().hex, - 'type': c_type, - 'user_id': uuid.uuid4().hex, + 'credential': { + 'blob': 'some blob', + 'project_id': uuid.uuid4().hex, + 'type': c_type, + 'user_id': uuid.uuid4().hex, + } } # Make sure an exception isn't raised self.create_credential_validator.validate(request_to_validate) @@ -1147,9 +1160,11 @@ class CredentialValidationTestCase(unit.BaseTestCase): for c_type in cred_types: request_to_validate = { - 'blob': 'some blob', - 'type': c_type, - 'user_id': uuid.uuid4().hex, + 'credential': { + 'blob': 'some blob', + 'type': c_type, + 'user_id': uuid.uuid4().hex, + } } # Make sure an exception isn't raised self.create_credential_validator.validate(request_to_validate) @@ -1157,21 +1172,25 @@ class CredentialValidationTestCase(unit.BaseTestCase): def test_validate_credential_with_extra_parameters_succeeds(self): """Validate create request with extra parameters.""" request_to_validate = { - 'blob': 'some string', - 'extra': False, - 'project_id': uuid.uuid4().hex, - 'type': 'ec2', - 'user_id': uuid.uuid4().hex, + 'credential': { + 'blob': 'some string', + 'extra': False, + 'project_id': uuid.uuid4().hex, + 'type': 'ec2', + 'user_id': uuid.uuid4().hex, + } } self.create_credential_validator.validate(request_to_validate) def test_validate_credential_update_succeeds(self): """Test that a credential request is properly validated.""" request_to_validate = { - 'blob': 'some string', - 'project_id': uuid.uuid4().hex, - 'type': 'ec2', - 'user_id': uuid.uuid4().hex, + 'credential': { + 'blob': 'some string', + 'project_id': uuid.uuid4().hex, + 'type': 'ec2', + 'user_id': uuid.uuid4().hex, + } } self.update_credential_validator.validate(request_to_validate) @@ -1187,11 +1206,13 @@ class CredentialValidationTestCase(unit.BaseTestCase): def test_validate_credential_update_with_extra_parameters_succeeds(self): """Validate credential update with extra parameters.""" request_to_validate = { - 'blob': 'some string', - 'extra': False, - 'project_id': uuid.uuid4().hex, - 'type': 'ec2', - 'user_id': uuid.uuid4().hex, + 'credential': { + 'blob': 'some string', + 'extra': False, + 'project_id': uuid.uuid4().hex, + 'type': 'ec2', + 'user_id': uuid.uuid4().hex, + } } self.update_credential_validator.validate(request_to_validate)