diff --git a/software-client/software_client/client.py b/software-client/software_client/client.py index 84e74ae5..d9b90fa7 100644 --- a/software-client/software_client/client.py +++ b/software-client/software_client/client.py @@ -111,9 +111,6 @@ def get_client(api_version, auth_mode, session=None, service_type=SERVICE_TYPE, session = _make_session(**kwargs) if not endpoint: - exception_msg = ('Either provide Keystone credentials or ' - 'user-defined endpoint and token or ' - 'execute software command as root (sudo)') if session: try: interface = kwargs.get('os_endpoint_type') @@ -122,12 +119,14 @@ def get_client(api_version, auth_mode, session=None, service_type=SERVICE_TYPE, interface=interface, region_name=region_name) except Exception as e: + msg = ('Failed to get openstack endpoint') raise exc.EndpointException( ('%(message)s, error was: %(error)s') % - {'message': exception_msg, 'error': e}) + {'message': msg, 'error': e}) elif local_root: endpoint = API_ENDPOINT else: + exception_msg = ('Missing / invalid authorization credentials') raise exc.AmbigiousAuthSystem(exception_msg) if endpoint: diff --git a/software-client/software_client/common/http.py b/software-client/software_client/common/http.py index a96d0ad5..f2de3f79 100644 --- a/software-client/software_client/common/http.py +++ b/software-client/software_client/common/http.py @@ -111,6 +111,44 @@ class ServiceCatalog(object): return matching_endpoints[0][endpoint_type] +def _extract_error_json_text(body_json): + error_json = {} + if 'error_message' in body_json: + raw_msg = body_json['error_message'] + if 'error' in raw_msg: + raw_error = jsonutils.loads(raw_msg) + error_json = {'faultstring': raw_error.get('error'), + 'debuginfo': raw_error.get('info')} + elif 'error_message' in raw_msg: + raw_error = jsonutils.loads(raw_msg) + raw_msg = raw_error['error_message'] + error_json = jsonutils.loads(raw_msg) + return error_json + + +def _extract_error_json(body, resp): + """Return error_message from the HTTP response body.""" + try: + content_type = resp.headers.get("Content-Type", "") + except AttributeError: + content_type = "" + if content_type.startswith("application/json"): + try: + body_json = resp.json() + return _extract_error_json_text(body_json) + except AttributeError: + body_json = jsonutils.loads(body) + return _extract_error_json_text(body_json) + except ValueError: + return {} + else: + try: + body_json = jsonutils.loads(body) + return _extract_error_json_text(body_json) + except ValueError: + return {} + + class SessionClient(adapter.LegacyJsonAdapter): def __init__(self, *args, **kwargs): @@ -136,8 +174,16 @@ class SessionClient(adapter.LegacyJsonAdapter): endpoint_filter.setdefault('service_type', self.service_type) endpoint_filter.setdefault('region_name', self.region_name) - return self.session.request(url, method, + resp = self.session.request(url, method, raise_exc=False, **kwargs) + if 400 <= resp.status_code < 600: + error_json = _extract_error_json(resp.content, resp) + raise exceptions.from_response( + resp, error_json.get('faultstring'), + error_json.get('debuginfo'), method, url) + elif resp.status_code in (300, 301, 302, 305): + raise exceptions.from_response(resp, method=method, url=url) + return resp def json_request(self, method, url, **kwargs): kwargs.setdefault('headers', {}) @@ -183,8 +229,6 @@ class SessionClient(adapter.LegacyJsonAdapter): body = None return resp, body - - def raw_request(self, method, url, **kwargs): kwargs.setdefault('headers', {}) kwargs['headers'].setdefault('Content-Type', @@ -222,6 +266,14 @@ class SessionClient(adapter.LegacyJsonAdapter): headers = {'Content-Type': enc.content_type, "X-Auth-Token": self.session.get_token()} response = requests.post(requests_url, data=enc, headers=headers) + if kwargs.get('check_exceptions'): + if response.status_code != 200: + err_message = _extract_error_json(response.text, response) + fault_text = ( + err_message.get("faultstring") + or "Unknown error in SessionClient while uploading request with multipart" + ) + raise exceptions.HTTPBadRequest(fault_text) return response.json() @@ -356,6 +408,26 @@ class HTTPClient(httplib2.Http): else: self.http_log_resp(_logger, resp, body) + status_code = self.get_status_code(resp) + if status_code == 401: + raise exceptions.HTTPUnauthorized(body) + elif status_code == 403: + reason = "Not allowed/Proper role is needed" + if body_str is not None: + error_json = self._extract_error_json(body_str) + reason = error_json.get('faultstring') + if reason is None: + reason = error_json.get('description') + raise exceptions.Forbidden(reason) + elif 400 <= status_code < 600: + _logger.warn("Request returned failure status: %s", status_code) # pylint: disable=deprecated-method + error_json = self._extract_error_json(body_str) + raise exceptions.from_response( + resp, error_json.get('faultstring'), + error_json.get('debuginfo'), *args) + elif status_code in (300, 301, 302, 305): + raise exceptions.from_response(resp, *args) + return resp, body def json_request(self, method, url, **kwargs): @@ -383,6 +455,9 @@ class HTTPClient(httplib2.Http): content_type = resp['content-type'] \ if resp.get('content-type', None) else None + # Add status_code attribute to make compatible with session resp + setattr(resp, 'status_code', resp.status) + if resp.status == 204 or resp.status == 205 or content_type is None: return resp, list() @@ -395,8 +470,6 @@ class HTTPClient(httplib2.Http): else: body = None - # Add status_code attribute to make compatible with session resp - setattr(resp, 'status_code', resp.status) return resp, body def multipart_request(self, method, url, **kwargs): @@ -491,6 +564,21 @@ class HTTPClient(httplib2.Http): ################# # UTILS ################# + def _extract_error_json(self, body): + error_json = {} + try: + body_json = json.loads(body) + if 'error' in body_json: + error_json = {'faultstring': body_json.get('error'), + 'debuginfo': body_json.get('info')} + elif 'error_message' in body_json: + raw_msg = body_json['error_message'] + error_json = json.loads(raw_msg) + except ValueError: + return {} + + return error_json + def _strip_credentials(self, kwargs): if kwargs.get('body') and self.password: log_kwargs = kwargs.copy() diff --git a/software-client/software_client/exc.py b/software-client/software_client/exc.py index 8345456d..0e7a81ed 100644 --- a/software-client/software_client/exc.py +++ b/software-client/software_client/exc.py @@ -43,6 +43,133 @@ class ClientException(Exception): """DEPRECATED.""" +class HTTPException(ClientException): + """Base exception for all HTTP-derived exceptions.""" + code = 'N/A' + + def __init__(self, details=None): + super(HTTPException, self).__init__() + self.details = details + + def __str__(self): + return str(self.details) or "%s (HTTP %s)" % (self.__class__.__name__, + self.code) + + +class HTTPMultipleChoices(HTTPException): + code = 300 + + def __str__(self): + self.details = ("Requested version of Software API is not" + "available.") + return "%s (HTTP %s) %s" % (self.__class__.__name__, self.code, + self.details) + + +class BadRequest(HTTPException): + """DEPRECATED.""" + code = 400 + + +class HTTPBadRequest(BadRequest): + pass + + +class Unauthorized(HTTPException): + """DEPRECATED.""" + code = 401 + + +class HTTPUnauthorized(Unauthorized): + pass + + +class Forbidden(HTTPException): + """DEPRECATED.""" + code = 403 + + +class HTTPForbidden(Forbidden): + pass + + +class NotFound(HTTPException): + """DEPRECATED.""" + code = 404 + + +class HTTPNotFound(NotFound): + pass + + +class HTTPMethodNotAllowed(HTTPException): + code = 405 + + +class Conflict(HTTPException): + """DEPRECATED.""" + code = 409 + + +class HTTPConflict(Conflict): + pass + + +class OverLimit(HTTPException): + """DEPRECATED.""" + code = 413 + + +class HTTPOverLimit(OverLimit): + pass + + +class HTTPInternalServerError(HTTPException): + code = 500 + + +class HTTPNotImplemented(HTTPException): + code = 501 + + +class HTTPBadGateway(HTTPException): + code = 502 + + +class ServiceUnavailable(HTTPException): + """DEPRECATED.""" + code = 503 + + +class HTTPServiceUnavailable(ServiceUnavailable): + pass + + +# NOTE(bcwaldon): Build a mapping of HTTP codes to corresponding exception +# classes +_code_map = {} +for obj_name in dir(sys.modules[__name__]): + if obj_name.startswith('HTTP'): + obj = getattr(sys.modules[__name__], obj_name) + _code_map[obj.code] = obj + + +def from_response(response, message=None, traceback=None, + method=None, url=None): + """Return an instance of an HTTPException based on httplib response.""" + cls = None + if hasattr(response, 'status_code'): + cls = _code_map.get(response.status_code, HTTPException) + elif hasattr(response, 'status_int'): + cls = _code_map.get(response.status_int, HTTPException) + elif hasattr(response, 'status'): + cls = _code_map.get(response.status, HTTPException) + else: + # No status code: return a generic exception + return Exception("Unexpected error in response: %s" % message) + return cls(message) + + class NoTokenLookupException(Exception): """DEPRECATED.""" pass # pylint: disable=unnecessary-pass diff --git a/software-client/software_client/software_client.py b/software-client/software_client/software_client.py index 91dac8ad..0df7ef29 100644 --- a/software-client/software_client/software_client.py +++ b/software-client/software_client/software_client.py @@ -392,17 +392,12 @@ class SoftwareClientShell(object): args.os_endpoint_type = endpoint_type client = sclient.get_client(api_version, auth_mode, **(args.__dict__)) - return args.func(client, args) - # TODO(bqian) reenable below once Exception classes are defined - """ try: - args.func(client, args) + return args.func(client, args) except exc.Unauthorized: raise exc.CommandError("Invalid Identity credentials.") except exc.HTTPForbidden: raise exc.CommandError("Error: Forbidden") - """ - def do_bash_completion(self, args): """Prints all of the commands and options to stdout. @@ -452,7 +447,6 @@ def main(): except Exception as e: print(e, file=sys.stderr) sys.exit(1) - sys.exit(0) if __name__ == "__main__": diff --git a/software/software/api/controllers/v1/software.py b/software/software/api/controllers/v1/software.py index e34ffc86..24c90bb1 100644 --- a/software/software/api/controllers/v1/software.py +++ b/software/software/api/controllers/v1/software.py @@ -6,8 +6,8 @@ SPDX-License-Identifier: Apache-2.0 """ import cgi import json +import logging import os -from oslo_log import log from pecan import expose from pecan import request from pecan import Response @@ -20,7 +20,7 @@ import software.utils as utils import software.constants as constants -LOG = log.getLogger(__name__) +LOG = logging.getLogger('main_logger') class SoftwareAPIController(object): diff --git a/software/software/authapi/app.py b/software/software/authapi/app.py index 708623e6..cb9879f1 100755 --- a/software/software/authapi/app.py +++ b/software/software/authapi/app.py @@ -12,6 +12,7 @@ from software.authapi import acl from software.authapi import config from software.authapi import hooks from software.authapi import policy +from software.parsable_error import ParsableErrorMiddleware from software.utils import ExceptionHook auth_opts = [ @@ -55,6 +56,7 @@ def setup_app(pecan_config=None, extra_hooks=None): debug=False, force_canonical=getattr(pecan_config.app, 'force_canonical', True), hooks=app_hooks, + wrap_app=ParsableErrorMiddleware, guess_content_type_from_ext=False, # Avoid mime-type lookup ) diff --git a/software/software/authapi/hooks.py b/software/software/authapi/hooks.py index 38370e4a..7a9b2350 100755 --- a/software/software/authapi/hooks.py +++ b/software/software/authapi/hooks.py @@ -121,14 +121,8 @@ class AccessPolicyHook(hooks.PecanHook): except Exception: raise exc.HTTPForbidden() else: - method = state.request.method - if method == 'GET': - has_api_access = policy.authorize( - 'reader_in_system_projects', {}, - context.to_dict(), do_raise=False) - else: - has_api_access = policy.authorize( - 'admin_in_system_projects', {}, - context.to_dict(), do_raise=False) + has_api_access = policy.authorize( + 'admin_in_system_projects', {}, + context.to_dict(), do_raise=False) if not has_api_access: raise exc.HTTPForbidden() diff --git a/software/software/authapi/policy.py b/software/software/authapi/policy.py index 6716e438..a0b1f0d4 100755 --- a/software/software/authapi/policy.py +++ b/software/software/authapi/policy.py @@ -30,10 +30,6 @@ base_rules = [ 'role:admin and (project_name:admin or ' + 'project_name:services)', description='Admin user in system projects.'), - policy.RuleDefault('reader_in_system_projects', - 'role:reader and (project_name:admin or ' + - 'project_name:services)', - description='Reader user in system projects.'), policy.RuleDefault('default', 'rule:admin_in_system_projects', description='Default rule.'), ] diff --git a/software/software/parsable_error.py b/software/software/parsable_error.py new file mode 100644 index 00000000..b526fd37 --- /dev/null +++ b/software/software/parsable_error.py @@ -0,0 +1,119 @@ +# Copyright (c) 2018-2024 Wind River Systems, Inc. +# Copyright © 2012 New Dream Network, LLC (DreamHost) +# +# Author: Doug Hellmann +# +# 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. +# + +""" +Middleware to replace the plain text message body of an error +response with one formatted so the client can parse it. + +Based on pecan.middleware.errordocument +""" +import html +import json +import logging +import six +import webob +from xml import etree as et + + +LOG = logging.getLogger('main_logger') + +# As per webob.exc code: +# https://github.com/Pylons/webob/blob/master/src/webob/exc.py +# The explanation field is added to the HTTP exception as following: +# ${explanation}

+WEBOB_EXPL_SEP = "

" + + +class ParsableErrorMiddleware(object): + """Replace error body with something the client can parse. + """ + def __init__(self, app): + self.app = app + + def __call__(self, environ, start_response): + # Request for this state, modified by replace_start_response() + # and used when an error is being reported. + state = {} + + def replacement_start_response(status, headers, exc_info=None): + """Overrides the default response to make errors parsable. + """ + try: + status_code = int(status.split(' ')[0]) + state['status_code'] = status_code + except (ValueError, TypeError): # pragma: nocover + raise Exception(( + 'ErrorDocumentMiddleware received an invalid ' + 'status %s' % status + )) + else: + if (state['status_code'] // 100) not in (2, 3): + # Remove some headers so we can replace them later + # when we have the full error message and can + # compute the length. + headers = [(h, v) + for (h, v) in headers + if h not in ('Content-Length', 'Content-Type') + ] + # Save the headers in case we need to modify them. + state['headers'] = headers + return start_response(status, headers, exc_info) + + app_iter = self.app(environ, replacement_start_response) + if (state['status_code'] // 100) not in (2, 3): + req = webob.Request(environ) + if (req.accept.best_match(['application/json', 'application/xml']) == + 'application/xml'): + + try: + # simple check xml is valid + body = [et.ElementTree.tostring( + et.ElementTree.fromstring('' + + '\n'.join(app_iter) + ''))] + except et.ElementTree.ParseError as err: + LOG.error('Error parsing HTTP response: %s' % err) + body = ['%s' % state['status_code'] + + ''] + state['headers'].append(('Content-Type', 'application/xml')) + else: + if six.PY3: + app_iter = [i.decode('utf-8') for i in app_iter] + # Parse explanation field from webob.exc and add it as + # 'faultstring' to be processed by cgts-client + fault = None + app_data = '\n'.join(app_iter) + for data in app_data.split("\n"): + if WEBOB_EXPL_SEP in str(data): + # Remove separator, trailing and leading white spaces + fault = str(data).replace(WEBOB_EXPL_SEP, "").strip() + break + if fault is None: + body = [json.dumps({'error_message': app_data})] + else: + # HTML unescape converts HTML entities back into their + # corresponding chars for human-readable text + fault = html.unescape(fault) + body = [json.dumps({'error_message': + json.dumps({'faultstring': fault})})] + if six.PY3: + body = [item.encode('utf-8') for item in body] + state['headers'].append(('Content-Type', 'application/json')) + state['headers'].append(('Content-Length', str(len(body[0])))) + else: + body = app_iter + return body diff --git a/software/software/utilities/utils.py b/software/software/utilities/utils.py index a4b9b9b5..433d652c 100644 --- a/software/software/utilities/utils.py +++ b/software/software/utilities/utils.py @@ -5,6 +5,7 @@ # import keyring +import logging import os import psycopg2 from psycopg2.extras import RealDictCursor @@ -20,9 +21,7 @@ from software.utilities.constants import KEYRING_PERMDIR from software.utilities import constants -from oslo_log import log - -LOG = log.getLogger(__name__) +LOG = logging.getLogger('main_logger') DB_CONNECTION = "postgresql://%s:%s@127.0.0.1/%s\n" KUBERNETES_CONF_PATH = "/etc/kubernetes" diff --git a/software/software/utils.py b/software/software/utils.py index ffe3aa25..b12c3320 100644 --- a/software/software/utils.py +++ b/software/software/utils.py @@ -48,10 +48,14 @@ class ExceptionHook(hooks.PecanHook): data = dict(info=e.info, warning=e.warning, error=e.error) else: err_msg = "Internal error occurred. Error signature [%s]" % signature + try: + # If exception contains error details, send that to user + if str(e): + err_msg = "Error \"%s\", Error signature [%s]" % (str(e), signature) + except Exception: + pass LOG.error(err_msg) LOG.exception(e) - # Unexpected exceptions, exception message is not sent to the user. - # Instead state as internal error data = dict(info="", warning="", error=err_msg) return webob.Response(json.dumps(data), status=status)