From a4906a4f57a249b8abfbf02aa420dc0b972769bd Mon Sep 17 00:00:00 2001 From: Al Bailey Date: Thu, 4 Aug 2022 18:25:32 +0000 Subject: [PATCH] Debian: Remove sysinv references from sw-patch sysinv files were being imported to provide authentication features like policy enforcement and request contexts. Those are now replaced with oslo imports. Test Plan: (Debian) PASS: AIO-DX bootstrap/unlock PASS: CLI upload/apply/host-install RR patch PASS: Horizon patching operations work PASS: NFV patching operations work PASS: no (new) errors in patching logs Story: 2009969 Task: 45998 Signed-off-by: Al Bailey Change-Id: I15d80441201755673f827529469a7f4feaa7f0ee --- sw-patch/bin/policy.json | 3 - sw-patch/cgcs-patch/cgcs_patch/authapi/acl.py | 19 +-- sw-patch/cgcs-patch/cgcs_patch/authapi/app.py | 13 +- .../cgcs-patch/cgcs_patch/authapi/context.py | 39 +++++ .../cgcs-patch/cgcs_patch/authapi/hooks.py | 55 ++++--- .../cgcs-patch/cgcs_patch/authapi/policy.py | 140 +++++++----------- .../cgcs-patch/cgcs_patch/patch_controller.py | 7 + .../cgcs_patch/tests/api/__init__.py | 0 .../cgcs_patch/tests/api/auth_config.py | 23 +++ .../cgcs-patch/cgcs_patch/tests/api/config.py | 23 +++ .../cgcs_patch/tests/api/patching.conf | 1 + .../cgcs_patch/tests/api/policy.json | 0 .../cgcs_patch/tests/api/test_api.py | 42 ++++++ .../cgcs_patch/tests/api/test_authapi.py | 43 ++++++ sw-patch/cgcs-patch/requirements.txt | 4 + sw-patch/cgcs-patch/tox.ini | 1 - 16 files changed, 289 insertions(+), 124 deletions(-) create mode 100644 sw-patch/cgcs-patch/cgcs_patch/authapi/context.py create mode 100644 sw-patch/cgcs-patch/cgcs_patch/tests/api/__init__.py create mode 100644 sw-patch/cgcs-patch/cgcs_patch/tests/api/auth_config.py create mode 100644 sw-patch/cgcs-patch/cgcs_patch/tests/api/config.py create mode 100644 sw-patch/cgcs-patch/cgcs_patch/tests/api/patching.conf create mode 100644 sw-patch/cgcs-patch/cgcs_patch/tests/api/policy.json create mode 100644 sw-patch/cgcs-patch/cgcs_patch/tests/api/test_api.py create mode 100644 sw-patch/cgcs-patch/cgcs_patch/tests/api/test_authapi.py diff --git a/sw-patch/bin/policy.json b/sw-patch/bin/policy.json index 94ac3a5b..2c63c085 100644 --- a/sw-patch/bin/policy.json +++ b/sw-patch/bin/policy.json @@ -1,5 +1,2 @@ { - "admin": "role:admin or role:administrator", - "admin_api": "is_admin:True", - "default": "rule:admin_api" } diff --git a/sw-patch/cgcs-patch/cgcs_patch/authapi/acl.py b/sw-patch/cgcs-patch/cgcs_patch/authapi/acl.py index ff2b2356..3b4fd1ed 100755 --- a/sw-patch/cgcs-patch/cgcs_patch/authapi/acl.py +++ b/sw-patch/cgcs-patch/cgcs_patch/authapi/acl.py @@ -1,16 +1,14 @@ -""" -Copyright (c) 2014-2017 Wind River Systems, Inc. - -SPDX-License-Identifier: Apache-2.0 - -""" +# +# Copyright (c) 2014-2022 Wind River Systems, Inc. +# +# SPDX-License-Identifier: Apache-2.0 +# +"""Access Control Lists (ACL's) control access the API server.""" from cgcs_patch.authapi import auth_token OPT_GROUP_NAME = 'keystone_authtoken' - - -"""Access Control Lists (ACL's) control access the API server.""" +OPT_GROUP_PROVIDER = 'keystonemiddleware.auth_token' def install(app, conf, public_routes): @@ -23,8 +21,7 @@ def install(app, conf, public_routes): :return: The same WSGI application with ACL installed. """ - - keystone_config = dict(conf.items(OPT_GROUP_NAME)) + keystone_config = dict(conf.get(OPT_GROUP_NAME)) return auth_token.AuthTokenMiddleware(app, conf=keystone_config, public_api_routes=public_routes) diff --git a/sw-patch/cgcs-patch/cgcs_patch/authapi/app.py b/sw-patch/cgcs-patch/cgcs_patch/authapi/app.py index d96d2bd3..a22cf46a 100755 --- a/sw-patch/cgcs-patch/cgcs_patch/authapi/app.py +++ b/sw-patch/cgcs-patch/cgcs_patch/authapi/app.py @@ -1,11 +1,10 @@ """ -Copyright (c) 2014-2017 Wind River Systems, Inc. +Copyright (c) 2014-2022 Wind River Systems, Inc. SPDX-License-Identifier: Apache-2.0 """ -import configparser from oslo_config import cfg import pecan @@ -14,7 +13,6 @@ from cgcs_patch.authapi import config from cgcs_patch.authapi import hooks from cgcs_patch.authapi import policy - auth_opts = [ cfg.StrOpt('auth_strategy', default='keystone', @@ -32,9 +30,6 @@ def get_pecan_config(): def setup_app(pecan_config=None, extra_hooks=None): - config_parser = configparser.RawConfigParser() - config_parser.read('/etc/patching/patching.conf') - policy.init() app_hooks = [hooks.ConfigHook(), @@ -47,7 +42,7 @@ def setup_app(pecan_config=None, extra_hooks=None): pecan_config = get_pecan_config() if pecan_config.app.enable_acl: - app_hooks.append(hooks.AdminAuthHook()) + app_hooks.append(hooks.AccessPolicyHook()) pecan.configuration.set_config(dict(pecan_config), overwrite=True) @@ -61,8 +56,10 @@ def setup_app(pecan_config=None, extra_hooks=None): guess_content_type_from_ext=False, # Avoid mime-type lookup ) + # config_parser must contain the keystone_auth if pecan_config.app.enable_acl: - return acl.install(app, config_parser, pecan_config.app.acl_public_routes) + CONF.import_group(acl.OPT_GROUP_NAME, acl.OPT_GROUP_PROVIDER) + return acl.install(app, CONF, pecan_config.app.acl_public_routes) return app diff --git a/sw-patch/cgcs-patch/cgcs_patch/authapi/context.py b/sw-patch/cgcs-patch/cgcs_patch/authapi/context.py new file mode 100644 index 00000000..47044c32 --- /dev/null +++ b/sw-patch/cgcs-patch/cgcs_patch/authapi/context.py @@ -0,0 +1,39 @@ +# +# Copyright (c) 2022 Wind River Systems, Inc. +# +# SPDX-License-Identifier: Apache-2.0 +# + +from oslo_context import context + + +# Patching calls into fault. so only FM service type +# needs to be preserved in the service catalog +REQUIRED_SERVICE_TYPES = ('faultmanagement',) + + +class RequestContext(context.RequestContext): + """Extends security contexts from the OpenStack common library.""" + + def __init__(self, is_public_api=False, service_catalog=None, **kwargs): + """Stores several additional request parameters: + """ + super(RequestContext, self).__init__(**kwargs) + self.is_public_api = is_public_api + if service_catalog: + # Only include required parts of service_catalog + self.service_catalog = [s for s in service_catalog + if s.get('type') in REQUIRED_SERVICE_TYPES] + else: + # if list is empty or none + self.service_catalog = [] + + def to_dict(self): + value = super(RequestContext, self).to_dict() + value.update({'is_public_api': self.is_public_api, + 'service_catalog': self.service_catalog}) + return value + + +def make_context(*args, **kwargs): + return RequestContext(*args, **kwargs) diff --git a/sw-patch/cgcs-patch/cgcs_patch/authapi/hooks.py b/sw-patch/cgcs-patch/cgcs_patch/authapi/hooks.py index e7434ac4..83a878ae 100755 --- a/sw-patch/cgcs-patch/cgcs_patch/authapi/hooks.py +++ b/sw-patch/cgcs-patch/cgcs_patch/authapi/hooks.py @@ -21,12 +21,12 @@ # SPDX-License-Identifier: Apache-2.0 # from oslo_config import cfg +from oslo_serialization import jsonutils from pecan import hooks from webob import exc -from sysinv.common import context -from sysinv.openstack.common import policy - +from cgcs_patch.authapi.context import RequestContext +from cgcs_patch.authapi import policy from cgcs_patch import utils @@ -56,6 +56,9 @@ class ContextHook(hooks.PecanHook): The flag is set to True, if X-Roles contains either an administrator or admin substring. Otherwise it is set to False. + X-Project-Name: + Used for context.project_name. + """ def __init__(self, public_api_routes): self.public_api_routes = public_api_routes @@ -66,36 +69,54 @@ class ContextHook(hooks.PecanHook): user_id = state.request.headers.get('X-User', user_id) tenant = state.request.headers.get('X-Tenant-Id') tenant = state.request.headers.get('X-Tenant', tenant) + project_name = state.request.headers.get('X-Project-Name') domain_id = state.request.headers.get('X-User-Domain-Id') domain_name = state.request.headers.get('X-User-Domain-Name') auth_token = state.request.headers.get('X-Auth-Token', None) creds = {'roles': state.request.headers.get('X-Roles', '').split(',')} + catalog_header = state.request.headers.get('X-Service-Catalog') + service_catalog = None + if catalog_header: + try: + service_catalog = jsonutils.loads(catalog_header) + except ValueError: + raise exc.HTTPInternalServerError( + 'Invalid service catalog json.') - is_admin = policy.check('admin', state.request.headers, creds) + is_admin = policy.authorize('admin_api', {}, creds, do_raise=False) path = utils.safe_rstrip(state.request.path, '/') is_public_api = path in self.public_api_routes - state.request.context = context.RequestContext( + state.request.context = RequestContext( auth_token=auth_token, user=user_id, tenant=tenant, domain_id=domain_id, domain_name=domain_name, is_admin=is_admin, - is_public_api=is_public_api) + is_public_api=is_public_api, + project_name=project_name, + roles=creds['roles'], + service_catalog=service_catalog) -class AdminAuthHook(hooks.PecanHook): - """Verify that the user has admin rights. - - Checks whether the request context is an admin context and - rejects the request otherwise. - +class AccessPolicyHook(hooks.PecanHook): + """Verify that the user has the needed credentials + to execute the action. """ def before(self, state): - ctx = state.request.context - is_admin_api = policy.check('admin_api', {}, ctx.to_dict()) - - if not is_admin_api and not ctx.is_public_api: - raise exc.HTTPForbidden() + controller = state.controller.__self__ + if hasattr(controller, 'enforce_policy'): + controller_method = state.controller.__name__ + controller.enforce_policy(controller_method, + state.request) + else: + context = state.request.context + is_admin_api = policy.authorize( + 'admin_api', + {}, + context.to_dict(), + do_raise=False) + if not is_admin_api and not context.is_public_api: + raise exc.HTTPForbidden() diff --git a/sw-patch/cgcs-patch/cgcs_patch/authapi/policy.py b/sw-patch/cgcs-patch/cgcs_patch/authapi/policy.py index 496320aa..94cbb4f3 100755 --- a/sw-patch/cgcs-patch/cgcs_patch/authapi/policy.py +++ b/sw-patch/cgcs-patch/cgcs_patch/authapi/policy.py @@ -16,104 +16,76 @@ # # Copyright (c) 2014-2022 Wind River Systems, Inc. # +# SPDX-License-Identifier: Apache-2.0 +# """Policy Engine For Patching.""" -import os.path - -from sysinv.common import exception -from sysinv.openstack.common import policy - -from cgcs_patch import utils +from oslo_config import cfg +from oslo_policy import policy -_POLICY_PATH = None -_POLICY_CACHE = {} +base_rules = [ + policy.RuleDefault('admin_required', 'role:admin or is_admin:1', + description='Who is considered an admin'), + policy.RuleDefault('admin_api', 'rule:admin_required', + description='admin API requirement'), + policy.RuleDefault('default', 'rule:admin_api', + description='default rule'), +] + +CONF = cfg.CONF +_ENFORCER = None -def reset(): - global _POLICY_PATH - global _POLICY_CACHE - _POLICY_PATH = None - _POLICY_CACHE = {} - policy.reset() +def init(policy_file=None, rules=None, + default_rule=None, use_conf=True, overwrite=True): + """Init an Enforcer class. + oslo policy supports change policy rule dynamically. + policy.enforce will reload the policy rules if it detects + the policy files have been touched. -def init(): - global _POLICY_PATH - global _POLICY_CACHE - if not _POLICY_PATH: - _POLICY_PATH = '/etc/patching/policy.json' - if not os.path.exists(_POLICY_PATH): - raise exception.ConfigNotFound(message='/etc/patching/policy.json') - utils.read_cached_file(_POLICY_PATH, _POLICY_CACHE, - reload_func=_set_rules) - - -def _set_rules(data): - default_rule = "rule:admin_api" - rules = policy.Rules.load_rules(data, default_rule, []) - policy.set_rules(rules) - - -def enforce(context, action, target, do_raise=True): - """Verifies that the action is valid on the target in this context. - - :param context: sysinv context - :param action: string representing the action to be checked - this should be colon separated for clarity. - i.e. ``compute:create_instance``, - ``compute:attach_volume``, - ``volume:attach_volume`` - :param target: dictionary representing the object of the action - for object creation this should be a dictionary representing the - location of the object e.g. ``{'project_id': context.project_id}`` - :param do_raise: if True (the default), raises PolicyNotAuthorized; - if False, returns False - - :raises sysinv.exception.PolicyNotAuthorized: if verification fails - and do_raise is True. - - :return: returns a non-False value (not necessarily "True") if - authorized, and the exact value False if not authorized and - do_raise is False. + :param policy_file: Custom policy file to use, if none is + specified, ``conf.policy_file`` will be + used. + :param rules: Default dictionary / Rules to use. It will be + considered just in the first instantiation. If + :meth:`load_rules` with ``force_reload=True``, + :meth:`clear` or :meth:`set_rules` with + ``overwrite=True`` is called this will be overwritten. + :param default_rule: Default rule to use, conf.default_rule will + be used if none is specified. + :param use_conf: Whether to load rules from cache or config file. + :param overwrite: Whether to overwrite existing rules when reload rules + from config file. """ + global _ENFORCER + if not _ENFORCER: + # https://docs.openstack.org/oslo.policy/latest/user/usage.html + _ENFORCER = policy.Enforcer(CONF, + policy_file=policy_file, + rules=rules, + default_rule=default_rule, + use_conf=use_conf, + overwrite=overwrite) + _ENFORCER.register_defaults(base_rules) + return _ENFORCER + + +def authorize(rule, target, creds, do_raise=True): + """A wrapper around 'authorize' from 'oslo_policy.policy'.""" init() + return _ENFORCER.authorize(rule, target, creds, do_raise=do_raise) - credentials = context.to_dict() - # Add the exception arguments if asked to do a raise - extra = {} - if do_raise: - extra.update(exc=exception.PolicyNotAuthorized, action=action) - - return policy.check(action, target, credentials, **extra) +def default_target(context): + return {'project_id': context.project_id, 'user_id': context.user_id} def check_is_admin(context): - """Whether or not role contains 'admin' role according to policy setting. - + """Whether or not roles contains 'admin' role according to policy setting. """ - init() - - credentials = context.to_dict() - target = credentials - - return policy.check('context_is_admin', target, credentials) - - -@policy.register('context_is_admin') -class IsAdminCheck(policy.Check): - """An explicit check for is_admin.""" - - def __init__(self, kind, match): - """Initialize the check.""" - - self.expected = (match.lower() == 'true') - - super(IsAdminCheck, self).__init__(kind, str(self.expected)) - - def __call__(self, target, creds): - """Determine whether is_admin matches the requested value.""" - - return creds['is_admin'] == self.expected + return authorize('context_is_admin', # rule + default_target(context), # target + context) # creds diff --git a/sw-patch/cgcs-patch/cgcs_patch/patch_controller.py b/sw-patch/cgcs-patch/cgcs_patch/patch_controller.py index 29101db9..2f299be7 100644 --- a/sw-patch/cgcs-patch/cgcs_patch/patch_controller.py +++ b/sw-patch/cgcs-patch/cgcs_patch/patch_controller.py @@ -2588,6 +2588,13 @@ class PatchControllerMainThread(threading.Thread): def main(): + # The following call to CONF is to ensure the oslo config + # has been called to specify a valid config dir. + # Otherwise oslo_policy will fail when it looks for its files. + CONF( + (), # Required to load an anonymous configuration + default_config_files=['/etc/patching/patching.conf', ] + ) configure_logging() cfg.read_config() diff --git a/sw-patch/cgcs-patch/cgcs_patch/tests/api/__init__.py b/sw-patch/cgcs-patch/cgcs_patch/tests/api/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/sw-patch/cgcs-patch/cgcs_patch/tests/api/auth_config.py b/sw-patch/cgcs-patch/cgcs_patch/tests/api/auth_config.py new file mode 100644 index 00000000..a6e80078 --- /dev/null +++ b/sw-patch/cgcs-patch/cgcs_patch/tests/api/auth_config.py @@ -0,0 +1,23 @@ +""" +Copyright (c) 2022 Wind River Systems, Inc. + +SPDX-License-Identifier: Apache-2.0 + +""" + +# Server Specific Configurations +server = { + 'port': '5487', + 'host': '127.0.0.1' +} + +# Pecan Application Configurations +app = { + 'root': 'cgcs_patch.api.controllers.root.RootController', + 'modules': ['cgcs_patch.authapi'], + 'static_root': '%(confdir)s/public', + 'template_path': '%(confdir)s/../templates', + 'debug': False, + 'enable_acl': True, + 'acl_public_routes': [], +} diff --git a/sw-patch/cgcs-patch/cgcs_patch/tests/api/config.py b/sw-patch/cgcs-patch/cgcs_patch/tests/api/config.py new file mode 100644 index 00000000..06390e2d --- /dev/null +++ b/sw-patch/cgcs-patch/cgcs_patch/tests/api/config.py @@ -0,0 +1,23 @@ +""" +Copyright (c) 2022 Wind River Systems, Inc. + +SPDX-License-Identifier: Apache-2.0 + +""" + +# Server Specific Configurations +server = { + 'port': '5487', + 'host': '127.0.0.1' +} + +# Pecan Application Configurations +app = { + 'root': 'cgcs_patch.api.controllers.root.RootController', + 'modules': ['cgcs_patch.authapi'], + 'static_root': '%(confdir)s/public', + 'template_path': '%(confdir)s/../templates', + 'debug': False, + 'enable_acl': False, + 'acl_public_routes': [], +} diff --git a/sw-patch/cgcs-patch/cgcs_patch/tests/api/patching.conf b/sw-patch/cgcs-patch/cgcs_patch/tests/api/patching.conf new file mode 100644 index 00000000..db4574b5 --- /dev/null +++ b/sw-patch/cgcs-patch/cgcs_patch/tests/api/patching.conf @@ -0,0 +1 @@ +[DEFAULT] diff --git a/sw-patch/cgcs-patch/cgcs_patch/tests/api/policy.json b/sw-patch/cgcs-patch/cgcs_patch/tests/api/policy.json new file mode 100644 index 00000000..e69de29b diff --git a/sw-patch/cgcs-patch/cgcs_patch/tests/api/test_api.py b/sw-patch/cgcs-patch/cgcs_patch/tests/api/test_api.py new file mode 100644 index 00000000..3edf92d6 --- /dev/null +++ b/sw-patch/cgcs-patch/cgcs_patch/tests/api/test_api.py @@ -0,0 +1,42 @@ +# +# Copyright (c) 2022 Wind River Systems, Inc. +# +# SPDX-License-Identifier: Apache-2.0 +# + +import os +from oslo_config import cfg +from pecan import set_config +from pecan.testing import load_test_app +from unittest import TestCase + + +class SWPatchAPITest(TestCase): + """API Tests for sw-patch""" + + def setUp(self): + # trigger oslo_config to load a config file + # so that it can co-locate a policy file + config_file = os.path.join(os.path.dirname(__file__), + 'patching.conf') + cfg.CONF((), + default_config_files=[config_file, ]) + # config.py sets acl to False + self.app = load_test_app(os.path.join( + os.path.dirname(__file__), + 'config.py' + )) + + def tearDown(self): + set_config({}, overwrite=True) + + +class TestRootController(SWPatchAPITest): + + def test_get(self): + response = self.app.get('/') + assert response.status_int == 200 + + def test_get_not_found(self): + response = self.app.get('/a/bogus/url', expect_errors=True) + assert response.status_int == 404 diff --git a/sw-patch/cgcs-patch/cgcs_patch/tests/api/test_authapi.py b/sw-patch/cgcs-patch/cgcs_patch/tests/api/test_authapi.py new file mode 100644 index 00000000..dccce09b --- /dev/null +++ b/sw-patch/cgcs-patch/cgcs_patch/tests/api/test_authapi.py @@ -0,0 +1,43 @@ +# +# Copyright (c) 2022 Wind River Systems, Inc. +# +# SPDX-License-Identifier: Apache-2.0 +# + +import os +from oslo_config import cfg +from pecan import set_config +from pecan.testing import load_test_app +from unittest import TestCase + + +class SWPatchAuthAPITest(TestCase): + """Auth API Tests for sw-patch""" + + def setUp(self): + # trigger oslo_config to load a config file + # so that it can co-locate a policy file + config_file = os.path.join(os.path.dirname(__file__), + 'patching.conf') + cfg.CONF((), + default_config_files=[config_file, ]) + # auth_config.py sets acl to True + self.app = load_test_app(os.path.join( + os.path.dirname(__file__), + 'auth_config.py' + )) + + def tearDown(self): + set_config({}, overwrite=True) + + +class TestRootControllerNoAuth(SWPatchAuthAPITest): + """We are not authenticated. Commands return 401""" + + def test_get(self): + response = self.app.get('/', expect_errors=True) + assert response.status_int == 401 + + def test_get_not_found(self): + response = self.app.get('/a/bogus/url', expect_errors=True) + assert response.status_int == 401 diff --git a/sw-patch/cgcs-patch/requirements.txt b/sw-patch/cgcs-patch/requirements.txt index 38555d04..faac4383 100644 --- a/sw-patch/cgcs-patch/requirements.txt +++ b/sw-patch/cgcs-patch/requirements.txt @@ -2,11 +2,15 @@ # of appearance. Changing the order has an impact on the overall integration # process, which may cause wedges in the gate later. +keystoneauth1 keystonemiddleware lxml oslo.config +oslo.policy +oslo.serialization netaddr pecan pycryptodomex requests_toolbelt sh +WebOb diff --git a/sw-patch/cgcs-patch/tox.ini b/sw-patch/cgcs-patch/tox.ini index 1a77cf55..908f02e7 100644 --- a/sw-patch/cgcs-patch/tox.ini +++ b/sw-patch/cgcs-patch/tox.ini @@ -17,7 +17,6 @@ basepython = python3 deps = -r{toxinidir}/requirements.txt -r{toxinidir}/test-requirements.txt -e{[tox]stxdir}/fault/fm-api/source - -e{[tox]stxdir}/config/sysinv/sysinv/sysinv -e{[tox]stxdir}/config/tsconfig/tsconfig install_command = pip install \