diff --git a/keystone/api/os_federation.py b/keystone/api/os_federation.py index 22abd1437a..7067f2917e 100644 --- a/keystone/api/os_federation.py +++ b/keystone/api/os_federation.py @@ -21,10 +21,11 @@ from oslo_serialization import jsonutils from keystone.api._shared import authentication from keystone.api._shared import json_home_relations +from keystone.api import validation from keystone.common import provider_api from keystone.common import rbac_enforcer from keystone.common import render_token -from keystone.common import validation +from keystone.common import validation as ks_validation import keystone.conf from keystone import exception from keystone.federation import schema @@ -44,7 +45,9 @@ IDP_ID_PARAMETER_RELATION = _build_param_relation(parameter_name='idp_id') PROTOCOL_ID_PARAMETER_RELATION = _build_param_relation( parameter_name='protocol_id' ) -SP_ID_PARAMETER_RELATION = _build_param_relation(parameter_name='sp_id') +SERVICE_PROVIDER_ID_PARAMETER_RELATION = _build_param_relation( + parameter_name='service_provider_id' +) def _combine_lists_uniquely(a, b): @@ -145,7 +148,7 @@ class IdentityProvidersResource(_ResourceBase): """ ENFORCER.enforce_call(action='identity:create_identity_provider') idp = self.request_body_json.get('identity_provider', {}) - validation.lazy_validate(schema.identity_provider_create, idp) + ks_validation.lazy_validate(schema.identity_provider_create, idp) idp = self._normalize_dict(idp) idp.setdefault('enabled', False) idp_ref = PROVIDERS.federation_api.create_idp(idp_id, idp) @@ -154,7 +157,7 @@ class IdentityProvidersResource(_ResourceBase): def patch(self, idp_id): ENFORCER.enforce_call(action='identity:update_identity_provider') idp = self.request_body_json.get('identity_provider', {}) - validation.lazy_validate(schema.identity_provider_update, idp) + ks_validation.lazy_validate(schema.identity_provider_update, idp) idp = self._normalize_dict(idp) idp_ref = PROVIDERS.federation_api.update_idp(idp_id, idp) return self.wrap_member(idp_ref) @@ -223,7 +226,7 @@ class IDPProtocolsCRUDResource(_IdentityProvidersProtocolsResourceBase): """ ENFORCER.enforce_call(action='identity:create_protocol') protocol = self.request_body_json.get('protocol', {}) - validation.lazy_validate(schema.protocol_create, protocol) + ks_validation.lazy_validate(schema.protocol_create, protocol) protocol = self._normalize_dict(protocol) ref = PROVIDERS.federation_api.create_protocol( idp_id, protocol_id, protocol @@ -238,7 +241,7 @@ class IDPProtocolsCRUDResource(_IdentityProvidersProtocolsResourceBase): """ ENFORCER.enforce_call(action='identity:update_protocol') protocol = self.request_body_json.get('protocol', {}) - validation.lazy_validate(schema.protocol_update, protocol) + ks_validation.lazy_validate(schema.protocol_update, protocol) ref = PROVIDERS.federation_api.update_protocol( idp_id, protocol_id, protocol ) @@ -360,23 +363,15 @@ class ServiceProvidersResource(_ResourceBase): 'sp_url', ] ) - _id_path_param_name_override = 'sp_id' api_prefix = '/OS-FEDERATION' - def get(self, sp_id=None): - if sp_id is not None: - return self._get_service_provider(sp_id) - return self._list_service_providers() - - def _get_service_provider(self, sp_id): - """Get a service provider. - - GET/HEAD /OS-FEDERATION/service_providers/{sp_id} - """ - ENFORCER.enforce_call(action='identity:get_service_provider') - return self.wrap_member(PROVIDERS.federation_api.get_sp(sp_id)) - - def _list_service_providers(self): + @validation.request_query_schema( + schema.service_provider_index_request_query + ) + @validation.response_body_schema( + schema.service_provider_index_response_body + ) + def get(self): """List service providers. GET/HEAD /OS-FEDERATION/service_providers @@ -392,39 +387,76 @@ class ServiceProvidersResource(_ResourceBase): ] return self.wrap_collection(refs, hints=hints) - def put(self, sp_id): + +class ServiceProviderResource(_ResourceBase): + collection_key = 'service_providers' + member_key = 'service_provider' + _public_parameters = frozenset( + [ + 'auth_url', + 'id', + 'enabled', + 'description', + 'links', + 'relay_state_prefix', + 'sp_url', + ] + ) + api_prefix = '/OS-FEDERATION' + + @validation.request_query_schema( + schema.service_provider_index_request_query + ) + @validation.response_body_schema(schema.service_provider_response_body) + def get(self, service_provider_id): + """Get a service provider. + + GET/HEAD /OS-FEDERATION/service_providers/{service_provider_id} + """ + ENFORCER.enforce_call(action='identity:get_service_provider') + return self.wrap_member( + PROVIDERS.federation_api.get_sp(service_provider_id) + ) + + @validation.request_body_schema( + schema.service_provider_create_request_body + ) + @validation.response_body_schema(schema.service_provider_response_body) + def put(self, service_provider_id): """Create a service provider. - PUT /OS-FEDERATION/service_providers/{sp_id} + PUT /OS-FEDERATION/service_providers/{service_provider_id} """ ENFORCER.enforce_call(action='identity:create_service_provider') sp = self.request_body_json.get('service_provider', {}) - validation.lazy_validate(schema.service_provider_create, sp) sp = self._normalize_dict(sp) sp.setdefault('enabled', False) sp.setdefault('relay_state_prefix', CONF.saml.relay_state_prefix) - sp_ref = PROVIDERS.federation_api.create_sp(sp_id, sp) + sp_ref = PROVIDERS.federation_api.create_sp(service_provider_id, sp) return self.wrap_member(sp_ref), http.client.CREATED - def patch(self, sp_id): + @validation.request_body_schema( + schema.service_provider_update_request_body + ) + @validation.response_body_schema(schema.service_provider_response_body) + def patch(self, service_provider_id): """Update a service provider. - PATCH /OS-FEDERATION/service_providers/{sp_id} + PATCH /OS-FEDERATION/service_providers/{service_provider_id} """ ENFORCER.enforce_call(action='identity:update_service_provider') sp = self.request_body_json.get('service_provider', {}) - validation.lazy_validate(schema.service_provider_update, sp) sp = self._normalize_dict(sp) - sp_ref = PROVIDERS.federation_api.update_sp(sp_id, sp) + sp_ref = PROVIDERS.federation_api.update_sp(service_provider_id, sp) return self.wrap_member(sp_ref) - def delete(self, sp_id): + def delete(self, service_provider_id): """Delete a service provider. - DELETE /OS-FEDERATION/service_providers/{sp_id} + DELETE /OS-FEDERATION/service_providers/{service_provider_id} """ ENFORCER.enforce_call(action='identity:delete_service_provider') - PROVIDERS.federation_api.delete_sp(sp_id) + PROVIDERS.federation_api.delete_sp(service_provider_id) return None, http.client.NO_CONTENT @@ -570,8 +602,26 @@ class OSFederationServiceProvidersAPI(ks_flask.APIBase): _name = 'service_providers' _import_name = __name__ _api_url_prefix = '/OS-FEDERATION' - resources = [ServiceProvidersResource] - resource_mapping = [] + resource_mapping = [ + ks_flask.construct_resource_map( + resource=ServiceProvidersResource, + url='/service_providers', + resource_kwargs={}, + rel="service_providers", + resource_relation_func=_build_resource_relation, + path_vars=None, + ), + ks_flask.construct_resource_map( + resource=ServiceProviderResource, + url='/service_providers/', + resource_kwargs={}, + rel="service_provider", + resource_relation_func=_build_resource_relation, + path_vars={ + 'service_provider_id': SERVICE_PROVIDER_ID_PARAMETER_RELATION + }, + ), + ] APIs = ( diff --git a/keystone/api/validation/parameter_types.py b/keystone/api/validation/parameter_types.py index 60d4dc21f4..d2644441c2 100644 --- a/keystone/api/validation/parameter_types.py +++ b/keystone/api/validation/parameter_types.py @@ -83,6 +83,13 @@ tags: dict[str, Any] = { }, } +url: dict[str, Any] = { + "type": "string", + "minLength": 0, + "maxLength": 225, + "pattern": "^[a-zA-Z0-9+.-]+:.+", +} + id_string: dict[str, Any] = { "type": "string", "minLength": 1, diff --git a/keystone/federation/schema.py b/keystone/federation/schema.py index 6dced4f7d5..e05cb33451 100644 --- a/keystone/federation/schema.py +++ b/keystone/federation/schema.py @@ -10,8 +10,12 @@ # License for the specific language governing permissions and limitations # under the License. +from typing import Any + +from keystone.api.validation import parameter_types +from keystone.api.validation import response_types from keystone.common import validation -from keystone.common.validation import parameter_types +from keystone.common.validation import parameter_types as ks_parameter_types basic_property_id = { 'type': 'object', @@ -46,34 +50,112 @@ saml_create = { _service_provider_properties = { # NOTE(rodrigods): The database accepts URLs with 256 as max length, # but parameter_types.url uses 225 as max length. - 'auth_url': parameter_types.url, - 'sp_url': parameter_types.url, - 'description': validation.nullable(parameter_types.description), - 'enabled': parameter_types.boolean, - 'relay_state_prefix': validation.nullable(parameter_types.description), + "auth_url": { + **parameter_types.url, + "description": "The URL to authenticate against", + }, + "sp_url": { + **parameter_types.url, + "description": "The service provider's URL", + }, + "description": { + "type": ["string", "null"], + "description": "The description of the service provider", + }, + "enabled": { + **parameter_types.boolean, + "description": "Whether the service provider is enabled or not", + }, + "relay_state_prefix": { + "type": ["string", "null"], + "description": "The prefix of the RelayState SAML attribute", + }, } -service_provider_create = { - 'type': 'object', - 'properties': _service_provider_properties, - # NOTE(rodrigods): 'id' is not required since it is passed in the URL - 'required': ['auth_url', 'sp_url'], - 'additionalProperties': False, +service_provider_schema: dict[str, Any] = { + "type": "object", + "description": "A service provider object", + "properties": { + "id": { + "type": "string", + "readOnly": True, + "description": "The service provider ID", + }, + "links": response_types.resource_links, + **_service_provider_properties, + }, + "additionalProperties": True, } -service_provider_update = { - 'type': 'object', - 'properties': _service_provider_properties, - # Make sure at least one property is being updated - 'minProperties': 1, - 'additionalProperties': False, +service_provider_index_request_query: dict[str, Any] = { + "type": "object", + "properties": { + "id": {"type": "string", "description": "The service provider ID"}, + "enabled": { + **parameter_types.boolean, + "description": "Whether the service provider is enabled or not", + }, + }, + "additionalProperties": False, +} + +service_provider_index_response_body: dict[str, Any] = { + "type": "object", + "properties": { + "service_providers": { + "type": "array", + "items": service_provider_schema, + "description": "A list of service provider objects", + }, + "links": response_types.links, + "truncated": response_types.truncated, + }, + "additionalProperties": False, +} + +service_provider_response_body: dict[str, Any] = { + "type": "object", + "description": "A service provider object", + "properties": {"service_provider": service_provider_schema}, + "additionalProperties": False, +} + +service_provider_create_request_body: dict[str, Any] = { + "type": "object", + "description": "A service provider object", + "properties": { + "service_provider": { + "type": "object", + "properties": _service_provider_properties, + "required": ["auth_url", "sp_url"], + "additionalProperties": False, + } + }, + "required": ["service_provider"], + "additionalProperties": False, +} + +service_provider_update_request_body: dict[str, Any] = { + "type": "object", + "description": "A service provider object", + "properties": { + # NOTE(rodrigods): 'id' is not required since it is passed in the URL + "service_provider": { + "type": "object", + "properties": _service_provider_properties, + "additionalProperties": False, + "minProperties": 1, + } + }, + "required": ["service_provider"], + "additionalProperties": False, } _identity_provider_properties_create = { - 'enabled': parameter_types.boolean, - 'description': validation.nullable(parameter_types.description), - 'domain_id': validation.nullable(parameter_types.id_string), - 'authorization_ttl': validation.nullable(parameter_types.integer_min0), + 'enabled': ks_parameter_types.boolean, + 'description': validation.nullable(ks_parameter_types.description), + 'domain_id': validation.nullable(ks_parameter_types.id_string), + 'authorization_ttl': validation.nullable(ks_parameter_types.integer_min0), 'remote_ids': { 'type': ['array', 'null'], 'items': {'type': 'string'}, @@ -82,9 +164,9 @@ _identity_provider_properties_create = { } _identity_provider_properties_update = { - 'enabled': parameter_types.boolean, - 'description': validation.nullable(parameter_types.description), - 'authorization_ttl': validation.nullable(parameter_types.integer_min0), + 'enabled': ks_parameter_types.boolean, + 'description': validation.nullable(ks_parameter_types.description), + 'authorization_ttl': validation.nullable(ks_parameter_types.integer_min0), 'remote_ids': { 'type': ['array', 'null'], 'items': {'type': 'string'}, @@ -109,7 +191,7 @@ identity_provider_update = { _remote_id_attribute_properties = {'type': 'string', 'maxLength': 64} _protocol_properties = { - 'mapping_id': parameter_types.mapping_id_string, + 'mapping_id': ks_parameter_types.mapping_id_string, 'remote_id_attribute': _remote_id_attribute_properties, } diff --git a/keystone/tests/unit/test_validation.py b/keystone/tests/unit/test_validation.py index 9b6f55cfa2..3efa780ff8 100644 --- a/keystone/tests/unit/test_validation.py +++ b/keystone/tests/unit/test_validation.py @@ -2095,24 +2095,28 @@ class ServiceProviderValidationTestCase(unit.BaseTestCase): self.valid_auth_url = 'https://' + uuid.uuid4().hex + '.com' self.valid_sp_url = 'https://' + uuid.uuid4().hex + '.com' - create = federation_schema.service_provider_create - update = federation_schema.service_provider_update + create = federation_schema.service_provider_create_request_body + update = federation_schema.service_provider_update_request_body self.create_sp_validator = validators.SchemaValidator(create) self.update_sp_validator = validators.SchemaValidator(update) def test_validate_sp_request(self): """Test that we validate `auth_url` and `sp_url` in request.""" request_to_validate = { - 'auth_url': self.valid_auth_url, - 'sp_url': self.valid_sp_url, + 'service_provider': { + 'auth_url': self.valid_auth_url, + 'sp_url': self.valid_sp_url, + } } self.create_sp_validator.validate(request_to_validate) def test_validate_sp_request_with_invalid_auth_url_fails(self): """Validate request fails with invalid `auth_url`.""" request_to_validate = { - 'auth_url': uuid.uuid4().hex, - 'sp_url': self.valid_sp_url, + 'service_provider': { + 'auth_url': uuid.uuid4().hex, + 'sp_url': self.valid_sp_url, + } } self.assertRaises( exception.SchemaValidationError, @@ -2123,8 +2127,10 @@ class ServiceProviderValidationTestCase(unit.BaseTestCase): def test_validate_sp_request_with_invalid_sp_url_fails(self): """Validate request fails with invalid `sp_url`.""" request_to_validate = { - 'auth_url': self.valid_auth_url, - 'sp_url': uuid.uuid4().hex, + 'service_provider': { + 'auth_url': self.valid_auth_url, + 'sp_url': uuid.uuid4().hex, + } } self.assertRaises( exception.SchemaValidationError, @@ -2134,13 +2140,17 @@ class ServiceProviderValidationTestCase(unit.BaseTestCase): def test_validate_sp_request_without_auth_url_fails(self): """Validate request fails without `auth_url`.""" - request_to_validate = {'sp_url': self.valid_sp_url} + request_to_validate = { + 'service_provider': {'sp_url': self.valid_sp_url} + } self.assertRaises( exception.SchemaValidationError, self.create_sp_validator.validate, request_to_validate, ) - request_to_validate = {'auth_url': None, 'sp_url': self.valid_sp_url} + request_to_validate = { + 'service_provider': {'auth_url': None, 'sp_url': self.valid_sp_url} + } self.assertRaises( exception.SchemaValidationError, self.create_sp_validator.validate, @@ -2149,13 +2159,20 @@ class ServiceProviderValidationTestCase(unit.BaseTestCase): def test_validate_sp_request_without_sp_url_fails(self): """Validate request fails without `sp_url`.""" - request_to_validate = {'auth_url': self.valid_auth_url} + request_to_validate = { + 'service_provider': {'auth_url': self.valid_auth_url} + } self.assertRaises( exception.SchemaValidationError, self.create_sp_validator.validate, request_to_validate, ) - request_to_validate = {'auth_url': self.valid_auth_url, 'sp_url': None} + request_to_validate = { + 'service_provider': { + 'auth_url': self.valid_auth_url, + 'sp_url': None, + } + } self.assertRaises( exception.SchemaValidationError, self.create_sp_validator.validate, @@ -2166,9 +2183,11 @@ class ServiceProviderValidationTestCase(unit.BaseTestCase): """Validate `enabled` as boolean-like values.""" for valid_enabled in _VALID_ENABLED_FORMATS: request_to_validate = { - 'auth_url': self.valid_auth_url, - 'sp_url': self.valid_sp_url, - 'enabled': valid_enabled, + 'service_provider': { + 'auth_url': self.valid_auth_url, + 'sp_url': self.valid_sp_url, + 'enabled': valid_enabled, + } } self.create_sp_validator.validate(request_to_validate) @@ -2176,9 +2195,11 @@ class ServiceProviderValidationTestCase(unit.BaseTestCase): """Exception is raised when `enabled` isn't a boolean-like value.""" for invalid_enabled in _INVALID_ENABLED_FORMATS: request_to_validate = { - 'auth_url': self.valid_auth_url, - 'sp_url': self.valid_sp_url, - 'enabled': invalid_enabled, + 'service_provider': { + 'auth_url': self.valid_auth_url, + 'sp_url': self.valid_sp_url, + 'enabled': invalid_enabled, + } } self.assertRaises( exception.SchemaValidationError, @@ -2189,18 +2210,22 @@ class ServiceProviderValidationTestCase(unit.BaseTestCase): def test_validate_sp_request_with_valid_description(self): """Test that we validate `description` in create requests.""" request_to_validate = { - 'auth_url': self.valid_auth_url, - 'sp_url': self.valid_sp_url, - 'description': 'My Service Provider', + 'service_provider': { + 'auth_url': self.valid_auth_url, + 'sp_url': self.valid_sp_url, + 'description': 'My Service Provider', + } } self.create_sp_validator.validate(request_to_validate) def test_validate_sp_request_with_invalid_description_fails(self): """Exception is raised when `description` as a non-string value.""" request_to_validate = { - 'auth_url': self.valid_auth_url, - 'sp_url': self.valid_sp_url, - 'description': False, + 'service_provider': { + 'auth_url': self.valid_auth_url, + 'sp_url': self.valid_sp_url, + 'description': False, + } } self.assertRaises( exception.SchemaValidationError, @@ -2212,10 +2237,12 @@ class ServiceProviderValidationTestCase(unit.BaseTestCase): """Exception raised when passing extra fields in the body.""" # 'id' can't be passed in the body since it is passed in the URL request_to_validate = { - 'id': 'ACME', - 'auth_url': self.valid_auth_url, - 'sp_url': self.valid_sp_url, - 'description': 'My Service Provider', + 'service_provider': { + 'id': 'ACME', + 'auth_url': self.valid_auth_url, + 'sp_url': self.valid_sp_url, + 'description': 'My Service Provider', + } } self.assertRaises( exception.SchemaValidationError, @@ -2225,7 +2252,9 @@ class ServiceProviderValidationTestCase(unit.BaseTestCase): def test_validate_sp_update_request(self): """Test that we validate a update request.""" - request_to_validate = {'description': uuid.uuid4().hex} + request_to_validate = { + 'service_provider': {'description': uuid.uuid4().hex} + } self.update_sp_validator.validate(request_to_validate) def test_validate_sp_update_request_with_no_parameters_fails(self): @@ -2239,13 +2268,15 @@ class ServiceProviderValidationTestCase(unit.BaseTestCase): def test_validate_sp_update_request_with_invalid_auth_url_fails(self): """Exception raised when updating with invalid `auth_url`.""" - request_to_validate = {'auth_url': uuid.uuid4().hex} + request_to_validate = { + 'service_provider': {'auth_url': uuid.uuid4().hex} + } self.assertRaises( exception.SchemaValidationError, self.update_sp_validator.validate, request_to_validate, ) - request_to_validate = {'auth_url': None} + request_to_validate = {'service_provider': {'auth_url': None}} self.assertRaises( exception.SchemaValidationError, self.update_sp_validator.validate, @@ -2254,13 +2285,15 @@ class ServiceProviderValidationTestCase(unit.BaseTestCase): def test_validate_sp_update_request_with_invalid_sp_url_fails(self): """Exception raised when updating with invalid `sp_url`.""" - request_to_validate = {'sp_url': uuid.uuid4().hex} + request_to_validate = { + 'service_provider': {'sp_url': uuid.uuid4().hex} + } self.assertRaises( exception.SchemaValidationError, self.update_sp_validator.validate, request_to_validate, ) - request_to_validate = {'sp_url': None} + request_to_validate = {'service_provider': {'sp_url': None}} self.assertRaises( exception.SchemaValidationError, self.update_sp_validator.validate, diff --git a/keystone/tests/unit/test_versions.py b/keystone/tests/unit/test_versions.py index 2c579cdcff..e865686da4 100644 --- a/keystone/tests/unit/test_versions.py +++ b/keystone/tests/unit/test_versions.py @@ -118,8 +118,10 @@ MAPPING_ID_PARAM_RELATION = json_home.build_v3_extension_parameter_relation( 'OS-FEDERATION', '1.0', 'mapping_id' ) -SP_ID_PARAMETER_RELATION = json_home.build_v3_extension_parameter_relation( - 'OS-FEDERATION', '1.0', 'sp_id' +SERVICE_PROVIDER_ID_PARAMETER_RELATION = ( + json_home.build_v3_extension_parameter_relation( + 'OS-FEDERATION', '1.0', 'service_provider_id' + ) ) CONSUMER_ID_PARAMETER_RELATION = ( @@ -470,8 +472,10 @@ V3_JSON_HOME_RESOURCES = { }, }, _build_federation_rel(resource_name='service_provider'): { - 'href-template': '/OS-FEDERATION/service_providers/{sp_id}', - 'href-vars': {'sp_id': SP_ID_PARAMETER_RELATION}, + 'href-template': '/OS-FEDERATION/service_providers/{service_provider_id}', + 'href-vars': { + 'service_provider_id': SERVICE_PROVIDER_ID_PARAMETER_RELATION + }, }, _build_federation_rel(resource_name='mapping'): { 'href-template': '/OS-FEDERATION/mappings/{mapping_id}',