From 892cbf502591567baa9fe631e56adfc20580f281 Mon Sep 17 00:00:00 2001 From: Dave Chen Date: Wed, 13 Jan 2016 16:53:10 +0800 Subject: [PATCH] Add schema for OAuth1 consumer API Add schema validation on create/update of OAuth1 consumer. This patch also remove the unnecessary code after schema validation enforcement. - Remove the check of `secret` in request body for update consumer API. It is covered by this schema definition. 'not': { 'required': ['secret'] } - Remove the check of `description` to consistent with other entities, such as `user`, `group`. It is covered by the following schema definition. 'description': validation.nullable(parameter_types.description) Partially implements: bp schema-validation-extent Change-Id: I4d7e6188e8120aa4bcb4a27a22a34d7b395d5f49 --- keystone/oauth1/backends/sql.py | 2 - keystone/oauth1/controllers.py | 10 ++--- keystone/oauth1/schema.py | 34 ++++++++++++++++ keystone/tests/unit/test_validation.py | 56 ++++++++++++++++++++++++++ 4 files changed, 94 insertions(+), 8 deletions(-) create mode 100644 keystone/oauth1/schema.py diff --git a/keystone/oauth1/backends/sql.py b/keystone/oauth1/backends/sql.py index a82b96a1f8..75ba804b5c 100644 --- a/keystone/oauth1/backends/sql.py +++ b/keystone/oauth1/backends/sql.py @@ -102,8 +102,6 @@ class OAuth1(core.Oauth1DriverV8): def create_consumer(self, consumer): consumer['secret'] = uuid.uuid4().hex - if not consumer.get('description'): - consumer['description'] = None session = sql.get_session() with session.begin(): consumer_ref = Consumer.from_dict(consumer) diff --git a/keystone/oauth1/controllers.py b/keystone/oauth1/controllers.py index 3da6fd2855..db793ced3e 100644 --- a/keystone/oauth1/controllers.py +++ b/keystone/oauth1/controllers.py @@ -21,11 +21,13 @@ from oslo_utils import timeutils from keystone.common import controller from keystone.common import dependency from keystone.common import utils +from keystone.common import validation from keystone.common import wsgi from keystone import exception from keystone.i18n import _ from keystone import notifications from keystone.oauth1 import core as oauth1 +from keystone.oauth1 import schema from keystone.oauth1 import validator @@ -56,6 +58,7 @@ class ConsumerCrudV3(controller.V3Controller): return controller.V3Controller.base_url(context, path=path) @controller.protected() + @validation.validated(schema.consumer_create, 'consumer') def create_consumer(self, context, consumer): ref = self._assign_unique_id(self._normalize_dict(consumer)) initiator = notifications._get_request_audit_info(context) @@ -63,10 +66,10 @@ class ConsumerCrudV3(controller.V3Controller): return ConsumerCrudV3.wrap_member(context, consumer_ref) @controller.protected() + @validation.validated(schema.consumer_update, 'consumer') def update_consumer(self, context, consumer_id, consumer): self._require_matching_id(consumer_id, consumer) ref = self._normalize_dict(consumer) - self._validate_consumer_ref(ref) initiator = notifications._get_request_audit_info(context) ref = self.oauth_api.update_consumer(consumer_id, ref, initiator) return ConsumerCrudV3.wrap_member(context, ref) @@ -90,11 +93,6 @@ class ConsumerCrudV3(controller.V3Controller): initiator = notifications._get_request_audit_info(context) self.oauth_api.delete_consumer(consumer_id, initiator) - def _validate_consumer_ref(self, consumer): - if 'secret' in consumer: - msg = _('Cannot change consumer secret') - raise exception.ValidationError(message=msg) - @dependency.requires('oauth_api') class AccessTokenCrudV3(controller.V3Controller): diff --git a/keystone/oauth1/schema.py b/keystone/oauth1/schema.py new file mode 100644 index 0000000000..51c11afeeb --- /dev/null +++ b/keystone/oauth1/schema.py @@ -0,0 +1,34 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# 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 keystone.common import validation +from keystone.common.validation import parameter_types + +_consumer_properties = { + 'description': validation.nullable(parameter_types.description) +} + +consumer_create = { + 'type': 'object', + 'properties': _consumer_properties, + 'additionalProperties': True +} + +consumer_update = { + 'type': 'object', + 'properties': _consumer_properties, + 'not': { + 'required': ['secret'] + }, + 'minProperties': 1, + 'additionalProperties': True +} diff --git a/keystone/tests/unit/test_validation.py b/keystone/tests/unit/test_validation.py index 007adc0292..2749d91b1b 100644 --- a/keystone/tests/unit/test_validation.py +++ b/keystone/tests/unit/test_validation.py @@ -25,6 +25,7 @@ from keystone.credential import schema as credential_schema from keystone import exception from keystone.federation import schema as federation_schema from keystone.identity import schema as identity_schema +from keystone.oauth1 import schema as oauth1_schema from keystone.policy import schema as policy_schema from keystone.resource import schema as resource_schema from keystone.tests import unit @@ -83,6 +84,8 @@ _VALID_ENABLED_FORMATS = [True, False] _INVALID_ENABLED_FORMATS = ['some string', 1, 0, 'True', 'False'] +_INVALID_DESC_FORMATS = [False, 1, 2.0] + _VALID_URLS = ['https://example.com', 'http://EXAMPLE.com/v3', 'http://localhost', 'http://127.0.0.1:5000', 'http://1.1.1.1', 'http://255.255.255.255', @@ -2038,3 +2041,56 @@ class FederationProtocolValidationTestCase(unit.BaseTestCase): self.assertRaises(exception.SchemaValidationError, self.protocol_validator.validate, request_to_validate) + + +class OAuth1ValidationTestCase(unit.BaseTestCase): + """Test for V3 Identity OAuth1 API validation.""" + + def setUp(self): + super(OAuth1ValidationTestCase, self).setUp() + + create = oauth1_schema.consumer_create + update = oauth1_schema.consumer_update + self.create_consumer_validator = validators.SchemaValidator(create) + self.update_consumer_validator = validators.SchemaValidator(update) + + def test_validate_consumer_request_succeeds(self): + """Test that we validate a consumer request successfully.""" + request_to_validate = {'description': uuid.uuid4().hex, + 'name': uuid.uuid4().hex} + self.create_consumer_validator.validate(request_to_validate) + self.update_consumer_validator.validate(request_to_validate) + + def test_validate_consumer_request_with_no_parameters(self): + """Test that schema validation with empty request body.""" + request_to_validate = {} + self.create_consumer_validator.validate(request_to_validate) + # At least one property should be given. + self.assertRaises(exception.SchemaValidationError, + self.update_consumer_validator.validate, + request_to_validate) + + def test_validate_consumer_request_with_invalid_description_fails(self): + """Exception is raised when `description` as a non-string value.""" + for invalid_desc in _INVALID_DESC_FORMATS: + request_to_validate = {'description': invalid_desc} + self.assertRaises(exception.SchemaValidationError, + self.create_consumer_validator.validate, + request_to_validate) + + self.assertRaises(exception.SchemaValidationError, + self.update_consumer_validator.validate, + request_to_validate) + + def test_validate_update_consumer_request_fails_with_secret(self): + """Exception raised when secret is given.""" + request_to_validate = {'secret': uuid.uuid4().hex} + self.assertRaises(exception.SchemaValidationError, + self.update_consumer_validator.validate, + request_to_validate) + + def test_validate_consumer_request_with_none_desc(self): + """Test that schema validation with None desc.""" + request_to_validate = {'description': None} + self.create_consumer_validator.validate(request_to_validate) + self.update_consumer_validator.validate(request_to_validate)