Merge "Remove redundant check after enforcing schema validation"
This commit is contained in:
commit
bc483fdac5
@ -405,8 +405,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()
|
||||
|
||||
@ -800,38 +798,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
@ -59,7 +59,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/tests/ \
|
||||
|
Loading…
Reference in New Issue
Block a user