Replace JSON Body middleware with flask-native func
Replace the JSON Body middleware with flask-native before-request function. The body filtering and storing data in request.environ['openstack.params'] was not used in the code base and has been dropped. Test Changes: * JSON Body middleware has been removed, no testing of the removed code * JSON Body Before Request Method has been implemented and associated testing (mirroring the JSON Body middleware code). * Test entry points no longer looks for JSON Body middleware. Change-Id: I84491865870b6bf2b8f094b524ee8b77510f0054 Partial-Bug: #1776504
This commit is contained in:
parent
35c9bb7eff
commit
ee9b035cf1
|
@ -13,66 +13,13 @@
|
||||||
# under the License.
|
# under the License.
|
||||||
|
|
||||||
from oslo_log import log
|
from oslo_log import log
|
||||||
from oslo_serialization import jsonutils
|
|
||||||
|
|
||||||
from keystone.common import wsgi
|
from keystone.common import wsgi
|
||||||
from keystone import exception
|
|
||||||
|
|
||||||
|
|
||||||
LOG = log.getLogger(__name__)
|
LOG = log.getLogger(__name__)
|
||||||
|
|
||||||
|
|
||||||
class JsonBodyMiddleware(wsgi.Middleware):
|
|
||||||
"""Middleware to allow method arguments to be passed as serialized JSON.
|
|
||||||
|
|
||||||
Accepting arguments as JSON is useful for accepting data that may be more
|
|
||||||
complex than simple primitives.
|
|
||||||
|
|
||||||
Filters out the parameters `self`, `context` and anything beginning with
|
|
||||||
an underscore.
|
|
||||||
|
|
||||||
"""
|
|
||||||
|
|
||||||
def process_request(self, request):
|
|
||||||
# Abort early if we don't have any work to do
|
|
||||||
params_json = request.body
|
|
||||||
if not params_json:
|
|
||||||
return
|
|
||||||
|
|
||||||
# Reject unrecognized content types. Empty string indicates
|
|
||||||
# the client did not explicitly set the header
|
|
||||||
if request.content_type not in ('application/json', ''):
|
|
||||||
e = exception.ValidationError(attribute='application/json',
|
|
||||||
target='Content-Type header')
|
|
||||||
return wsgi.render_exception(e, request=request)
|
|
||||||
|
|
||||||
params_parsed = {}
|
|
||||||
try:
|
|
||||||
params_parsed = jsonutils.loads(params_json)
|
|
||||||
except ValueError:
|
|
||||||
e = exception.ValidationError(attribute='valid JSON',
|
|
||||||
target='request body')
|
|
||||||
return wsgi.render_exception(e, request=request)
|
|
||||||
finally:
|
|
||||||
if not params_parsed:
|
|
||||||
params_parsed = {}
|
|
||||||
|
|
||||||
if not isinstance(params_parsed, dict):
|
|
||||||
e = exception.ValidationError(attribute='valid JSON object',
|
|
||||||
target='request body')
|
|
||||||
return wsgi.render_exception(e, request=request)
|
|
||||||
|
|
||||||
params = {}
|
|
||||||
for k, v in params_parsed.items():
|
|
||||||
if k in ('self', 'context'):
|
|
||||||
continue
|
|
||||||
if k.startswith('_'):
|
|
||||||
continue
|
|
||||||
params[k] = v
|
|
||||||
|
|
||||||
request.environ[wsgi.PARAMS_ENV] = params
|
|
||||||
|
|
||||||
|
|
||||||
class NormalizingFilter(wsgi.Middleware):
|
class NormalizingFilter(wsgi.Middleware):
|
||||||
"""Middleware filter to handle URL normalization."""
|
"""Middleware filter to handle URL normalization."""
|
||||||
|
|
||||||
|
|
|
@ -25,6 +25,7 @@ import werkzeug.wsgi
|
||||||
|
|
||||||
import keystone.api
|
import keystone.api
|
||||||
from keystone.common import wsgi as keystone_wsgi
|
from keystone.common import wsgi as keystone_wsgi
|
||||||
|
from keystone.server.flask.request_processing import json_body
|
||||||
|
|
||||||
# TODO(morgan): _MOVED_API_PREFIXES to be removed when the legacy dispatch
|
# TODO(morgan): _MOVED_API_PREFIXES to be removed when the legacy dispatch
|
||||||
# support is removed.
|
# support is removed.
|
||||||
|
@ -179,6 +180,11 @@ def application_factory(name='public'):
|
||||||
# NOTE(morgan): The Flask App actually dispatches nothing until we migrate
|
# NOTE(morgan): The Flask App actually dispatches nothing until we migrate
|
||||||
# some routers to Flask-Blueprints, it is simply a placeholder.
|
# some routers to Flask-Blueprints, it is simply a placeholder.
|
||||||
app = flask.Flask(name)
|
app = flask.Flask(name)
|
||||||
|
|
||||||
|
# Add core before request functions
|
||||||
|
app.before_request(json_body.json_body_before_request)
|
||||||
|
|
||||||
|
# Add core after request functions
|
||||||
app.after_request(_add_vary_x_auth_token_header)
|
app.after_request(_add_vary_x_auth_token_header)
|
||||||
|
|
||||||
# NOTE(morgan): Configure the Flask Environment for our needs.
|
# NOTE(morgan): Configure the Flask Environment for our needs.
|
||||||
|
|
|
@ -54,9 +54,6 @@ _APP_MIDDLEWARE = (
|
||||||
_Middleware(namespace='keystone.server_middleware',
|
_Middleware(namespace='keystone.server_middleware',
|
||||||
ep='url_normalize',
|
ep='url_normalize',
|
||||||
conf={}),
|
conf={}),
|
||||||
_Middleware(namespace='keystone.server_middleware',
|
|
||||||
ep='json_body',
|
|
||||||
conf={}),
|
|
||||||
_Middleware(namespace='keystone.server_middleware',
|
_Middleware(namespace='keystone.server_middleware',
|
||||||
ep='osprofiler',
|
ep='osprofiler',
|
||||||
conf={}),
|
conf={}),
|
||||||
|
|
|
@ -0,0 +1,54 @@
|
||||||
|
# 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.
|
||||||
|
|
||||||
|
# Before request processing for JSON Body enforcement
|
||||||
|
|
||||||
|
import flask
|
||||||
|
from werkzeug import exceptions as werkzeug_exceptions
|
||||||
|
|
||||||
|
from keystone import exception
|
||||||
|
from keystone.i18n import _
|
||||||
|
|
||||||
|
|
||||||
|
def json_body_before_request():
|
||||||
|
"""Enforce JSON Request Body."""
|
||||||
|
# TODO(morgan): Allow other content-types when OpenAPI Doc or improved
|
||||||
|
# federation is implemented for known/valid paths. This function should
|
||||||
|
# be removed long term.
|
||||||
|
|
||||||
|
# exit if there is nothing to be done, (no body)
|
||||||
|
if not flask.request.get_data():
|
||||||
|
return None
|
||||||
|
|
||||||
|
try:
|
||||||
|
# flask does loading for us for json, use the flask default loader
|
||||||
|
# in the case that the data is *not* json or a dict, we should see a
|
||||||
|
# raise of werkzeug.exceptions.BadRequest, re-spin this to the keystone
|
||||||
|
# ValidationError message (as expected by our contract)
|
||||||
|
|
||||||
|
# Explicitly check if the content is supposed to be json.
|
||||||
|
if (flask.request.is_json
|
||||||
|
or flask.request.headers['Content-Type'] == ''):
|
||||||
|
json_decoded = flask.request.get_json(force=True)
|
||||||
|
if not isinstance(json_decoded, dict):
|
||||||
|
# In the case that the returned value was not a dict, force
|
||||||
|
# a raise that will be caught the same way that a Decode error
|
||||||
|
# would be handled.
|
||||||
|
raise werkzeug_exceptions.BadRequest(
|
||||||
|
_('resulting JSON load was not a dict'))
|
||||||
|
else:
|
||||||
|
raise exception.ValidationError(attribute='application/json',
|
||||||
|
target='Content-Type header')
|
||||||
|
|
||||||
|
except werkzeug_exceptions.BadRequest:
|
||||||
|
raise exception.ValidationError(attribute='valid JSON',
|
||||||
|
target='request body')
|
|
@ -26,6 +26,7 @@ from keystone.common import rbac_enforcer
|
||||||
import keystone.conf
|
import keystone.conf
|
||||||
from keystone import exception
|
from keystone import exception
|
||||||
from keystone.server.flask import common as flask_common
|
from keystone.server.flask import common as flask_common
|
||||||
|
from keystone.server.flask.request_processing import json_body
|
||||||
from keystone.tests.unit import rest
|
from keystone.tests.unit import rest
|
||||||
|
|
||||||
|
|
||||||
|
@ -654,3 +655,44 @@ class TestKeystoneFlaskCommon(rest.RestfulTestCase):
|
||||||
self.assertTrue(ref['links']['self'].startswith(
|
self.assertTrue(ref['links']['self'].startswith(
|
||||||
'https://localhost/v3/%s' % view_arg)
|
'https://localhost/v3/%s' % view_arg)
|
||||||
)
|
)
|
||||||
|
|
||||||
|
def test_json_body_before_req_func_valid_json(self):
|
||||||
|
with self.test_request_context(
|
||||||
|
headers={'Content-Type': 'application/json'},
|
||||||
|
data='{"key": "value"}'):
|
||||||
|
# No exception should be raised, everything is happy.
|
||||||
|
json_body.json_body_before_request()
|
||||||
|
|
||||||
|
def test_json_body_before_req_func_invalid_json(self):
|
||||||
|
with self.test_request_context(
|
||||||
|
headers={'Content-Type': 'application/json'},
|
||||||
|
data='invalid JSON'):
|
||||||
|
# keystone.exception.ValidationError should be raised
|
||||||
|
self.assertRaises(exception.ValidationError,
|
||||||
|
json_body.json_body_before_request)
|
||||||
|
|
||||||
|
def test_json_body_before_req_func_no_content_type(self):
|
||||||
|
# Unset
|
||||||
|
with self.test_request_context(data='{"key": "value"}'):
|
||||||
|
# No exception should be raised, everything is happy.
|
||||||
|
json_body.json_body_before_request()
|
||||||
|
|
||||||
|
# Explicitly set to ''
|
||||||
|
with self.test_request_context(
|
||||||
|
headers={'Content-Type': ''}, data='{"key": "value"}'):
|
||||||
|
# No exception should be raised, everything is happy.
|
||||||
|
json_body.json_body_before_request()
|
||||||
|
|
||||||
|
def test_json_body_before_req_func_unrecognized_content_type(self):
|
||||||
|
with self.test_request_context(
|
||||||
|
headers={'Content-Type': 'unrecognized/content-type'},
|
||||||
|
data='{"key": "value"'):
|
||||||
|
# keystone.exception.ValidationError should be raised
|
||||||
|
self.assertRaises(exception.ValidationError,
|
||||||
|
json_body.json_body_before_request)
|
||||||
|
|
||||||
|
def test_json_body_before_req_func_unrecognized_conten_type_no_body(self):
|
||||||
|
with self.test_request_context(
|
||||||
|
headers={'Content-Type': 'unrecognized/content-type'}):
|
||||||
|
# No exception should be raised, everything is happy.
|
||||||
|
json_body.json_body_before_request()
|
||||||
|
|
|
@ -23,7 +23,6 @@ class TestEntryPoints(test.TestCase):
|
||||||
'build_auth_context',
|
'build_auth_context',
|
||||||
'cors',
|
'cors',
|
||||||
'debug',
|
'debug',
|
||||||
'json_body',
|
|
||||||
'request_id',
|
'request_id',
|
||||||
'sizelimit',
|
'sizelimit',
|
||||||
'url_normalize',
|
'url_normalize',
|
||||||
|
|
|
@ -103,57 +103,6 @@ class MiddlewareRequestTestBase(unit.TestCase):
|
||||||
return self._do_middleware_response(*args, **kwargs).request
|
return self._do_middleware_response(*args, **kwargs).request
|
||||||
|
|
||||||
|
|
||||||
class JsonBodyMiddlewareTest(MiddlewareRequestTestBase):
|
|
||||||
|
|
||||||
MIDDLEWARE_CLASS = middleware.JsonBodyMiddleware
|
|
||||||
|
|
||||||
def test_request_with_params(self):
|
|
||||||
headers = {'Content-Type': 'application/json'}
|
|
||||||
params = '{"arg1": "one", "arg2": ["a"]}'
|
|
||||||
req = self._do_middleware_request(params=params,
|
|
||||||
headers=headers,
|
|
||||||
method='post')
|
|
||||||
self.assertEqual({"arg1": "one", "arg2": ["a"]},
|
|
||||||
req.environ[wsgi.PARAMS_ENV])
|
|
||||||
|
|
||||||
def test_malformed_json(self):
|
|
||||||
headers = {'Content-Type': 'application/json'}
|
|
||||||
self._do_middleware_response(params='{"arg1": "on',
|
|
||||||
headers=headers,
|
|
||||||
method='post',
|
|
||||||
status=http_client.BAD_REQUEST)
|
|
||||||
|
|
||||||
def test_not_dict_body(self):
|
|
||||||
headers = {'Content-Type': 'application/json'}
|
|
||||||
resp = self._do_middleware_response(params='42',
|
|
||||||
headers=headers,
|
|
||||||
method='post',
|
|
||||||
status=http_client.BAD_REQUEST)
|
|
||||||
|
|
||||||
self.assertIn('valid JSON object', resp.json['error']['message'])
|
|
||||||
|
|
||||||
def test_no_content_type(self):
|
|
||||||
headers = {'Content-Type': ''}
|
|
||||||
params = '{"arg1": "one", "arg2": ["a"]}'
|
|
||||||
req = self._do_middleware_request(params=params,
|
|
||||||
headers=headers,
|
|
||||||
method='post')
|
|
||||||
self.assertEqual({"arg1": "one", "arg2": ["a"]},
|
|
||||||
req.environ[wsgi.PARAMS_ENV])
|
|
||||||
|
|
||||||
def test_unrecognized_content_type(self):
|
|
||||||
headers = {'Content-Type': 'text/plain'}
|
|
||||||
self._do_middleware_response(params='{"arg1": "one", "arg2": ["a"]}',
|
|
||||||
headers=headers,
|
|
||||||
method='post',
|
|
||||||
status=http_client.BAD_REQUEST)
|
|
||||||
|
|
||||||
def test_unrecognized_content_type_without_body(self):
|
|
||||||
headers = {'Content-Type': 'text/plain'}
|
|
||||||
req = self._do_middleware_request(headers=headers)
|
|
||||||
self.assertEqual({}, req.environ.get(wsgi.PARAMS_ENV, {}))
|
|
||||||
|
|
||||||
|
|
||||||
class AuthContextMiddlewareTest(test_backend_sql.SqlTests,
|
class AuthContextMiddlewareTest(test_backend_sql.SqlTests,
|
||||||
MiddlewareRequestTestBase):
|
MiddlewareRequestTestBase):
|
||||||
|
|
||||||
|
|
|
@ -196,5 +196,4 @@ keystone.server_middleware =
|
||||||
request_id = oslo_middleware:RequestId
|
request_id = oslo_middleware:RequestId
|
||||||
build_auth_context = keystone.middleware:AuthContextMiddleware
|
build_auth_context = keystone.middleware:AuthContextMiddleware
|
||||||
token_auth = keystone.middleware:TokenAuthMiddleware
|
token_auth = keystone.middleware:TokenAuthMiddleware
|
||||||
json_body = keystone.middleware:JsonBodyMiddleware
|
|
||||||
debug = oslo_middleware:Debug
|
debug = oslo_middleware:Debug
|
||||||
|
|
Loading…
Reference in New Issue