From 9b052e4c059893791dd7adc4aa32d40ac098e788 Mon Sep 17 00:00:00 2001 From: Morgan Fainberg Date: Thu, 11 Oct 2018 12:32:59 -0700 Subject: [PATCH] Register exceptions with a Flask Error Handler Exceptions are now handled in the Flask APP instead of in the legacy webob Application code (at this point that code was living in the URL Normalizing Middleware). All Keystone API exceptions (derived from keystone.exception.Error) are automatically registered on definition with the keystone.exception.KEYSTONE_API_EXCEPTIONS set. This set is processed once the app is created in keystone.server.application to the flask-friendly handler. TypeError and generic Exception are registered to an explicit error handler that converts TypeError to ValidationError (BAD_REQUEST) and all other Exceptions to UnexpectedError (INTERNAL SERVER ERROR). These exceptions are then emitted in a "jsonify-ed" manner to the client. Two other minor changes were required: * Unenforced API decorator had it's core functionality split into a dedicated function that can be called in the case of an error being raised in a "before_request" function (such as validation in the JSON Body before request func. * The JSON Body before request func now explicitly sets the api to "unenforced_ok" if it is raising an exception. This prevents the flask "was this API enforced" assertion from failing because @unenforced_api was never run (the ValidationError was raised prior to the resource's method being called). Change-Id: I0d0ef6a774eb86b4769238ed34d7703232ce86c3 Partial-Bug: #1776504 --- keystone/exception.py | 17 ++++ keystone/server/flask/application.py | 81 +++++++++++++++++++ keystone/server/flask/common.py | 9 ++- .../flask/request_processing/json_body.py | 7 ++ 4 files changed, 113 insertions(+), 1 deletion(-) diff --git a/keystone/exception.py b/keystone/exception.py index d5d27d97cc..94f159d50c 100644 --- a/keystone/exception.py +++ b/keystone/exception.py @@ -24,6 +24,8 @@ from keystone.i18n import _ CONF = keystone.conf.CONF LOG = log.getLogger(__name__) +KEYSTONE_API_EXCEPTIONS = set([]) + # Tests use this to make exception message format errors fatal _FATAL_EXCEPTION_FORMAT_ERRORS = False @@ -44,6 +46,21 @@ def _format_with_unicode_kwargs(msg_format, kwargs): return msg_format % kwargs +class _KeystoneExceptionMeta(type): + """Automatically Register the Exceptions in 'KEYSTONE_API_EXCEPTIONS' list. + + The `KEYSTONE_API_EXCEPTIONS` list is utilized by flask to register a + handler to emit sane details when the exception occurs. + """ + + def __new__(mcs, name, bases, class_dict): + """Create a new instance and register with KEYSTONE_API_EXCEPTIONS.""" + cls = type.__new__(mcs, name, bases, class_dict) + KEYSTONE_API_EXCEPTIONS.add(cls) + return cls + + +@six.add_metaclass(_KeystoneExceptionMeta) class Error(Exception): """Base error class. diff --git a/keystone/server/flask/application.py b/keystone/server/flask/application.py index acc883a624..be7c60d926 100644 --- a/keystone/server/flask/application.py +++ b/keystone/server/flask/application.py @@ -16,11 +16,15 @@ import functools import sys import flask +import oslo_i18n from oslo_log import log from oslo_middleware import healthcheck +import six import werkzeug.wsgi import keystone.api +from keystone import exception +from keystone.server.flask import common as ks_flask from keystone.server.flask.request_processing import json_body from keystone.server.flask.request_processing import req_logging @@ -50,6 +54,68 @@ def _add_vary_x_auth_token_header(response): return response +def _best_match_language(): + """Determine the best available locale. + + This returns best available locale based on the Accept-Language HTTP + header passed in the request. + """ + if not flask.request.accept_languages: + return None + return flask.request.accept_languages.best_match( + oslo_i18n.get_available_languages('keystone')) + + +def _handle_keystone_exception(error): + # Handle logging + if isinstance(error, exception.Unauthorized): + LOG.warning( + "Authorization failed. %(exception)s from %(remote_addr)s", + {'exception': error, 'remote_addr': flask.request.remote_addr}) + elif isinstance(error, exception.UnexpectedError): + LOG.exception(six.text_type(error)) + else: + LOG.warning(six.text_type(error)) + + # Render the exception to something user "friendly" + error_message = error.args[0] + message = oslo_i18n.translate(error_message, _best_match_language()) + if message is error_message: + # translate() didn't do anything because it wasn't a Message, + # convert to a string. + message = six.text_type(message) + + body = dict( + error={ + 'code': error.code, + 'title': error.title, + 'message': message} + ) + + if isinstance(error, exception.AuthPluginException): + body['error']['identity'] = error.authentication + + # Create the response and set status code. + response = flask.jsonify(body) + response.status_code = error.code + + # Add the appropriate WWW-Authenticate header for Unauthorized + if isinstance(error, exception.Unauthorized): + url = ks_flask.base_url() + response.headers['WWW-Authenticate'] = 'Keystone uri="%s"' % url + return response + + +def _handle_unknown_keystone_exception(error): + # translate a python exception to something we can properly render as + # an API error. + if isinstance(error, TypeError): + new_exc = exception.ValidationError(error) + else: + new_exc = exception.UnexpectedError(error) + return _handle_keystone_exception(new_exc) + + @fail_gracefully def application_factory(name='public'): if name not in ('admin', 'public'): @@ -58,6 +124,21 @@ def application_factory(name='public'): app = flask.Flask(name) + # Register Error Handler Function for Keystone Errors. + # NOTE(morgan): Flask passes errors to an error handling function. All of + # keystone's api errors are explicitly registered in + # keystone.exception.KEYSTONE_API_EXCEPTIONS and those are in turn + # registered here to ensure a proper error is bubbled up to the end user + # instead of a 500 error. + for exc in exception.KEYSTONE_API_EXCEPTIONS: + app.register_error_handler(exc, _handle_keystone_exception) + + # Register extra (python) exceptions with the proper exception handler, + # specifically TypeError and generic exception, these will render as + # 500 errors, but presented in a "web-ified" manner + app.register_error_handler(TypeError, _handle_unknown_keystone_exception) + app.register_error_handler(Exception, _handle_unknown_keystone_exception) + # Add core before request functions app.before_request(req_logging.log_request_info) app.before_request(json_body.json_body_before_request) diff --git a/keystone/server/flask/common.py b/keystone/server/flask/common.py index 1db7199dca..520c17f44e 100644 --- a/keystone/server/flask/common.py +++ b/keystone/server/flask/common.py @@ -1027,6 +1027,13 @@ def full_url(path=''): return '%(url)s%(query_string)s' % subs +def set_unenforced_ok(): + # Does the work for unenforced_api. This must be used outside of a + # decorator in some limited, such as when a ValidationError is raised up + # from a "before_request" function (body_json checker is a prime example) + setattr(flask.g, enforcer._ENFORCEMENT_CHECK_ATTR, True) + + def unenforced_api(f): """Decorate a resource method to mark is as an unenforced API. @@ -1040,6 +1047,6 @@ def unenforced_api(f): """ @functools.wraps(f) def wrapper(*args, **kwargs): - setattr(flask.g, enforcer._ENFORCEMENT_CHECK_ATTR, True) + set_unenforced_ok() return f(*args, **kwargs) return wrapper diff --git a/keystone/server/flask/request_processing/json_body.py b/keystone/server/flask/request_processing/json_body.py index 1203401581..676ef791e0 100644 --- a/keystone/server/flask/request_processing/json_body.py +++ b/keystone/server/flask/request_processing/json_body.py @@ -17,6 +17,7 @@ from werkzeug import exceptions as werkzeug_exceptions from keystone import exception from keystone.i18n import _ +from keystone.server.flask import common as ks_flask_common def json_body_before_request(): @@ -46,9 +47,15 @@ def json_body_before_request(): raise werkzeug_exceptions.BadRequest( _('resulting JSON load was not a dict')) else: + # We no longer need enforcement on this API, set unenforced_ok + # we already hit a validation error. + ks_flask_common.set_unenforced_ok() raise exception.ValidationError(attribute='application/json', target='Content-Type header') except werkzeug_exceptions.BadRequest: + # We no longer need enforcement on this API, set unenforced_ok + # we already hit a validation error. + ks_flask_common.set_unenforced_ok() raise exception.ValidationError(attribute='valid JSON', target='request body')