Remove redundant check after enforcing schema validation
check_immutable_params() turn out to be redundant since schema validation that is enforce has the constraint that no additional properties is allowed, any call with immutable params will be caught by json schema. The patch remove the check from controller and the relevant testcases since the restful testcase will never get to the call of the API actually. The patch also remove the method check_immutable_params and relevant testcases since it's not used anywhere. Partially implements: bp schema-validation-extent Change-Id: I017c14e4d1229951e389a9e8d48384b54e197dd3
This commit is contained in:
Dave Chen
Steve Martinelli
parent
1a67c2a7ef
commit
fc8d7422a8
@ -425,8 +425,6 @@ class V3Controller(wsgi.Application):
|
||||
|
||||
Class parameters:
|
||||
|
||||
* `_mutable_parameters` - set of parameters that can be changed by users.
|
||||
Usually used by cls.check_immutable_params()
|
||||
* `_public_parameters` - set of parameters that are exposed to the user.
|
||||
Usually used by cls.filter_params()
|
||||
|
||||
@ -811,38 +809,12 @@ class V3Controller(wsgi.Application):
|
||||
utils.flatten_dict(policy_dict))
|
||||
LOG.debug('RBAC: Authorization granted')
|
||||
|
||||
@classmethod
|
||||
def check_immutable_params(cls, ref):
|
||||
"""Raise exception when disallowed parameter is in ref.
|
||||
|
||||
Check whether the ref dictionary representing a request has only
|
||||
mutable parameters included. If not, raise an exception. This method
|
||||
checks only root-level keys from a ref dictionary.
|
||||
|
||||
:param ref: a dictionary representing deserialized request to be
|
||||
stored
|
||||
:raises: :class:`keystone.exception.ImmutableAttributeError`
|
||||
|
||||
"""
|
||||
ref_keys = set(ref.keys())
|
||||
blocked_keys = ref_keys.difference(cls._mutable_parameters)
|
||||
|
||||
if not blocked_keys:
|
||||
# No immutable parameters changed
|
||||
return
|
||||
|
||||
exception_args = {'target': cls.__name__,
|
||||
'attributes': ', '.join(blocked_keys)}
|
||||
raise exception.ImmutableAttributeError(**exception_args)
|
||||
|
||||
@classmethod
|
||||
def filter_params(cls, ref):
|
||||
"""Remove unspecified parameters from the dictionary.
|
||||
|
||||
This function removes unspecified parameters from the dictionary. See
|
||||
check_immutable_parameters for corresponding function that raises
|
||||
exceptions. This method checks only root-level keys from a ref
|
||||
dictionary.
|
||||
This function removes unspecified parameters from the dictionary.
|
||||
This method checks only root-level keys from a ref dictionary.
|
||||
|
||||
:param ref: a dictionary representing deserialized response to be
|
||||
serialized
|
||||
|
||||
@ -55,7 +55,6 @@ class IdentityProvider(_ControllerBase):
|
||||
collection_name = 'identity_providers'
|
||||
member_name = 'identity_provider'
|
||||
|
||||
_mutable_parameters = frozenset(['description', 'enabled', 'remote_ids'])
|
||||
_public_parameters = frozenset(['id', 'enabled', 'description',
|
||||
'remote_ids', 'links'
|
||||
])
|
||||
@ -95,7 +94,6 @@ class IdentityProvider(_ControllerBase):
|
||||
def create_identity_provider(self, context, idp_id, identity_provider):
|
||||
identity_provider = self._normalize_dict(identity_provider)
|
||||
identity_provider.setdefault('enabled', False)
|
||||
IdentityProvider.check_immutable_params(identity_provider)
|
||||
idp_ref = self.federation_api.create_idp(idp_id, identity_provider)
|
||||
response = IdentityProvider.wrap_member(context, idp_ref)
|
||||
return wsgi.render_response(body=response, status=('201', 'Created'))
|
||||
@ -119,7 +117,6 @@ class IdentityProvider(_ControllerBase):
|
||||
@validation.validated(schema.identity_provider_update, 'identity_provider')
|
||||
def update_identity_provider(self, context, idp_id, identity_provider):
|
||||
identity_provider = self._normalize_dict(identity_provider)
|
||||
IdentityProvider.check_immutable_params(identity_provider)
|
||||
idp_ref = self.federation_api.update_idp(idp_id, identity_provider)
|
||||
return IdentityProvider.wrap_member(context, idp_ref)
|
||||
|
||||
@ -128,8 +125,8 @@ class IdentityProvider(_ControllerBase):
|
||||
class FederationProtocol(_ControllerBase):
|
||||
"""A federation protocol representation.
|
||||
|
||||
See IdentityProvider docstring for explanation on _mutable_parameters
|
||||
and _public_parameters class attributes.
|
||||
See keystone.common.controller.V3Controller docstring for explanation
|
||||
on _public_parameters class attributes.
|
||||
|
||||
"""
|
||||
|
||||
@ -137,7 +134,6 @@ class FederationProtocol(_ControllerBase):
|
||||
member_name = 'protocol'
|
||||
|
||||
_public_parameters = frozenset(['id', 'mapping_id', 'links'])
|
||||
_mutable_parameters = frozenset(['mapping_id'])
|
||||
|
||||
@classmethod
|
||||
def _add_self_referential_link(cls, context, ref):
|
||||
@ -184,7 +180,6 @@ class FederationProtocol(_ControllerBase):
|
||||
@validation.validated(schema.federation_protocol_schema, 'protocol')
|
||||
def create_protocol(self, context, idp_id, protocol_id, protocol):
|
||||
ref = self._normalize_dict(protocol)
|
||||
FederationProtocol.check_immutable_params(ref)
|
||||
ref = self.federation_api.create_protocol(idp_id, protocol_id, ref)
|
||||
response = FederationProtocol.wrap_member(context, ref)
|
||||
return wsgi.render_response(body=response, status=('201', 'Created'))
|
||||
@ -193,7 +188,6 @@ class FederationProtocol(_ControllerBase):
|
||||
@validation.validated(schema.federation_protocol_schema, 'protocol')
|
||||
def update_protocol(self, context, idp_id, protocol_id, protocol):
|
||||
ref = self._normalize_dict(protocol)
|
||||
FederationProtocol.check_immutable_params(ref)
|
||||
ref = self.federation_api.update_protocol(idp_id, protocol_id,
|
||||
protocol)
|
||||
return FederationProtocol.wrap_member(context, ref)
|
||||
@ -466,8 +460,6 @@ class ServiceProvider(_ControllerBase):
|
||||
collection_name = 'service_providers'
|
||||
member_name = 'service_provider'
|
||||
|
||||
_mutable_parameters = frozenset(['auth_url', 'description', 'enabled',
|
||||
'relay_state_prefix', 'sp_url'])
|
||||
_public_parameters = frozenset(['auth_url', 'id', 'enabled', 'description',
|
||||
'links', 'relay_state_prefix', 'sp_url'])
|
||||
|
||||
@ -478,7 +470,6 @@ class ServiceProvider(_ControllerBase):
|
||||
service_provider.setdefault('enabled', False)
|
||||
service_provider.setdefault('relay_state_prefix',
|
||||
CONF.saml.relay_state_prefix)
|
||||
ServiceProvider.check_immutable_params(service_provider)
|
||||
sp_ref = self.federation_api.create_sp(sp_id, service_provider)
|
||||
response = ServiceProvider.wrap_member(context, sp_ref)
|
||||
return wsgi.render_response(body=response, status=('201', 'Created'))
|
||||
@ -502,7 +493,6 @@ class ServiceProvider(_ControllerBase):
|
||||
@validation.validated(schema.service_provider_update, 'service_provider')
|
||||
def update_service_provider(self, context, sp_id, service_provider):
|
||||
service_provider = self._normalize_dict(service_provider)
|
||||
ServiceProvider.check_immutable_params(service_provider)
|
||||
sp_ref = self.federation_api.update_sp(sp_id, service_provider)
|
||||
return ServiceProvider.wrap_member(context, sp_ref)
|
||||
|
||||
|
||||
@ -1,54 +0,0 @@
|
||||
# Copyright 2014 CERN.
|
||||
#
|
||||
# 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.
|
||||
|
||||
import uuid
|
||||
|
||||
import six
|
||||
from six.moves import range
|
||||
from testtools import matchers
|
||||
|
||||
from keystone.common import controller
|
||||
from keystone import exception
|
||||
from keystone.tests import unit
|
||||
|
||||
|
||||
class V3ControllerTestCase(unit.TestCase):
|
||||
"""Tests for the V3Controller class."""
|
||||
|
||||
def setUp(self):
|
||||
super(V3ControllerTestCase, self).setUp()
|
||||
|
||||
class ControllerUnderTest(controller.V3Controller):
|
||||
_mutable_parameters = frozenset(['hello', 'world'])
|
||||
|
||||
self.api = ControllerUnderTest()
|
||||
|
||||
def test_check_immutable_params(self):
|
||||
"""Pass valid parameters to the method and expect no failure."""
|
||||
ref = {
|
||||
'hello': uuid.uuid4().hex,
|
||||
'world': uuid.uuid4().hex
|
||||
}
|
||||
self.api.check_immutable_params(ref)
|
||||
|
||||
def test_check_immutable_params_fail(self):
|
||||
"""Pass invalid parameter to the method and expect failure."""
|
||||
ref = {uuid.uuid4().hex: uuid.uuid4().hex for _ in range(3)}
|
||||
|
||||
ex = self.assertRaises(exception.ImmutableAttributeError,
|
||||
self.api.check_immutable_params, ref)
|
||||
ex_msg = six.text_type(ex)
|
||||
self.assertThat(ex_msg, matchers.Contains(self.api.__class__.__name__))
|
||||
for key in ref.keys():
|
||||
self.assertThat(ex_msg, matchers.Contains(key))
|
||||
1
tox.ini
1
tox.ini
@ -67,7 +67,6 @@ commands =
|
||||
keystone/tests/unit/test_token_provider.py \
|
||||
keystone/tests/unit/test_url_middleware.py \
|
||||
keystone/tests/unit/test_v2_controller.py \
|
||||
keystone/tests/unit/test_v3_controller.py \
|
||||
keystone/tests/unit/test_validation.py \
|
||||
keystone/tests/unit/test_wsgi.py \
|
||||
keystone/tests/unit/token/test_pki_provider.py \
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user