Fix auth-required for help command

When we got picky with the auth arguments we broke using help without
any auth config supplied.  This rearranges things a bit to do the argument
checking when the deferred auth request to Identity occurs so commands
that do not need auth have a chance to live short but useful lives.

Closes-Bug: #1399588
Change-Id: I8ceac491cf65e25eddb62ab2713f471fe686756d
This commit is contained in:
Dean Troyer 2015-02-27 09:19:12 -06:00
parent 9400effd4b
commit 505fa14cd6
5 changed files with 90 additions and 60 deletions

View File

@ -59,7 +59,7 @@ def run(opts):
# Collect the auth and config options together and give them to
# ClientManager and it will wrangle all of the goons into place.
client_manager = clientmanager.ClientManager(
auth_options=opts,
cli_options=opts,
verify=verify,
api_version=api_version,
)

View File

@ -85,9 +85,9 @@ def select_auth_plugin(options):
# let keystoneclient figure it out itself
auth_plugin_name = 'token'
else:
raise exc.CommandError(
"Authentication type must be selected with --os-auth-type"
)
# The ultimate default is similar to the original behaviour,
# but this time with version discovery
auth_plugin_name = 'osc_password'
LOG.debug("Auth plugin %s selected" % auth_plugin_name)
return auth_plugin_name

View File

@ -56,14 +56,14 @@ class ClientManager(object):
def __init__(
self,
auth_options,
cli_options,
api_version=None,
verify=True,
pw_func=None,
):
"""Set up a ClientManager
:param auth_options:
:param cli_options:
Options collected from the command-line, environment, or wherever
:param api_version:
Dict of API versions: key is API name, value is the version
@ -77,31 +77,57 @@ class ClientManager(object):
returns a string containing the password
"""
self._cli_options = cli_options
self._api_version = api_version
self._pw_callback = pw_func
self._url = self._cli_options.os_url
self._region_name = self._cli_options.os_region_name
self.timing = self._cli_options.timing
self._auth_ref = None
self.session = None
# verify is the Requests-compatible form
self._verify = verify
# also store in the form used by the legacy client libs
self._cacert = None
if isinstance(verify, bool):
self._insecure = not verify
else:
self._cacert = verify
self._insecure = False
# Get logging from root logger
root_logger = logging.getLogger('')
LOG.setLevel(root_logger.getEffectiveLevel())
def setup_auth(self):
"""Set up authentication
This is deferred until authentication is actually attempted because
it gets in the way of things that do not require auth.
"""
# If no auth type is named by the user, select one based on
# the supplied options
self.auth_plugin_name = auth.select_auth_plugin(auth_options)
self.auth_plugin_name = auth.select_auth_plugin(self._cli_options)
# Basic option checking to avoid unhelpful error messages
auth.check_valid_auth_options(auth_options, self.auth_plugin_name)
auth.check_valid_auth_options(self._cli_options, self.auth_plugin_name)
# Horrible hack alert...must handle prompt for null password if
# password auth is requested.
if (self.auth_plugin_name.endswith('password') and
not auth_options.os_password):
auth_options.os_password = pw_func()
not self._cli_options.os_password):
self._cli_options.os_password = self.pw_callback()
(auth_plugin, self._auth_params) = auth.build_auth_params(
self.auth_plugin_name,
auth_options,
self._cli_options,
)
self._url = auth_options.os_url
self._region_name = auth_options.os_region_name
self._api_version = api_version
self._auth_ref = None
self.timing = auth_options.timing
default_domain = auth_options.os_default_domain
default_domain = self._cli_options.os_default_domain
# NOTE(stevemar): If PROJECT_DOMAIN_ID or PROJECT_DOMAIN_NAME is
# present, then do not change the behaviour. Otherwise, set the
# PROJECT_DOMAIN_ID to 'OS_DEFAULT_DOMAIN' for better usability.
@ -125,20 +151,6 @@ class ClientManager(object):
elif 'tenant_name' in self._auth_params:
self._project_name = self._auth_params['tenant_name']
# verify is the Requests-compatible form
self._verify = verify
# also store in the form used by the legacy client libs
self._cacert = None
if isinstance(verify, bool):
self._insecure = not verify
else:
self._cacert = verify
self._insecure = False
# Get logging from root logger
root_logger = logging.getLogger('')
LOG.setLevel(root_logger.getEffectiveLevel())
LOG.info('Using auth plugin: %s' % self.auth_plugin_name)
self.auth = auth_plugin.load_from_options(**self._auth_params)
# needed by SAML authentication
@ -146,7 +158,7 @@ class ClientManager(object):
self.session = session.Session(
auth=self.auth,
session=request_session,
verify=verify,
verify=self._verify,
)
return
@ -155,6 +167,7 @@ class ClientManager(object):
def auth_ref(self):
"""Dereference will trigger an auth if it hasn't already"""
if not self._auth_ref:
self.setup_auth()
LOG.debug("Get auth_ref")
self._auth_ref = self.auth.get_auth_ref(self.session)
return self._auth_ref

View File

@ -22,7 +22,6 @@ import traceback
from cliff import app
from cliff import command
from cliff import complete
from cliff import help
import openstackclient
@ -70,10 +69,9 @@ class OpenStackShell(app.App):
def __init__(self):
# Patch command.Command to add a default auth_required = True
command.Command.auth_required = True
command.Command.best_effort = False
# But not help
# Some commands do not need authentication
help.HelpCommand.auth_required = False
complete.CompleteCommand.best_effort = True
super(OpenStackShell, self).__init__(
description=__doc__.strip(),
@ -294,7 +292,7 @@ class OpenStackShell(app.App):
self.verify = not self.options.insecure
self.client_manager = clientmanager.ClientManager(
auth_options=self.options,
cli_options=self.options,
verify=self.verify,
api_version=self.api_version,
pw_func=prompt_for_password,
@ -308,7 +306,7 @@ class OpenStackShell(app.App):
cmd.__class__.__module__,
cmd.__class__.__name__,
)
if cmd.auth_required and cmd.best_effort:
if cmd.auth_required:
try:
# Trigger the Identity client to initialize
self.client_manager.auth_ref

View File

@ -80,12 +80,16 @@ class TestClientManager(utils.TestCase):
def test_client_manager_token_endpoint(self):
client_manager = clientmanager.ClientManager(
auth_options=FakeOptions(os_token=fakes.AUTH_TOKEN,
os_url=fakes.AUTH_URL,
os_auth_type='token_endpoint'),
cli_options=FakeOptions(
os_token=fakes.AUTH_TOKEN,
os_url=fakes.AUTH_URL,
os_auth_type='token_endpoint',
),
api_version=API_VERSION,
verify=True
)
client_manager.setup_auth()
self.assertEqual(
fakes.AUTH_URL,
client_manager._url,
@ -104,12 +108,15 @@ class TestClientManager(utils.TestCase):
def test_client_manager_token(self):
client_manager = clientmanager.ClientManager(
auth_options=FakeOptions(os_token=fakes.AUTH_TOKEN,
os_auth_url=fakes.AUTH_URL,
os_auth_type='v2token'),
cli_options=FakeOptions(
os_token=fakes.AUTH_TOKEN,
os_auth_url=fakes.AUTH_URL,
os_auth_type='v2token',
),
api_version=API_VERSION,
verify=True
)
client_manager.setup_auth()
self.assertEqual(
fakes.AUTH_URL,
@ -125,13 +132,16 @@ class TestClientManager(utils.TestCase):
def test_client_manager_password(self):
client_manager = clientmanager.ClientManager(
auth_options=FakeOptions(os_auth_url=fakes.AUTH_URL,
os_username=fakes.USERNAME,
os_password=fakes.PASSWORD,
os_project_name=fakes.PROJECT_NAME),
cli_options=FakeOptions(
os_auth_url=fakes.AUTH_URL,
os_username=fakes.USERNAME,
os_password=fakes.PASSWORD,
os_project_name=fakes.PROJECT_NAME,
),
api_version=API_VERSION,
verify=False,
)
client_manager.setup_auth()
self.assertEqual(
fakes.AUTH_URL,
@ -182,14 +192,17 @@ class TestClientManager(utils.TestCase):
def test_client_manager_password_verify_ca(self):
client_manager = clientmanager.ClientManager(
auth_options=FakeOptions(os_auth_url=fakes.AUTH_URL,
os_username=fakes.USERNAME,
os_password=fakes.PASSWORD,
os_project_name=fakes.PROJECT_NAME,
os_auth_type='v2password'),
cli_options=FakeOptions(
os_auth_url=fakes.AUTH_URL,
os_username=fakes.USERNAME,
os_password=fakes.PASSWORD,
os_project_name=fakes.PROJECT_NAME,
os_auth_type='v2password',
),
api_version=API_VERSION,
verify='cafile',
)
client_manager.setup_auth()
self.assertFalse(client_manager._insecure)
self.assertTrue(client_manager._verify)
@ -199,10 +212,12 @@ class TestClientManager(utils.TestCase):
auth_params['os_auth_type'] = auth_plugin_name
auth_params['os_identity_api_version'] = api_version
client_manager = clientmanager.ClientManager(
auth_options=FakeOptions(**auth_params),
cli_options=FakeOptions(**auth_params),
api_version=API_VERSION,
verify=True
)
client_manager.setup_auth()
self.assertEqual(
auth_plugin_name,
client_manager.auth_plugin_name,
@ -228,8 +243,12 @@ class TestClientManager(utils.TestCase):
self._select_auth_plugin(params, 'XXX', 'password')
def test_client_manager_select_auth_plugin_failure(self):
self.assertRaises(exc.CommandError,
clientmanager.ClientManager,
auth_options=FakeOptions(os_auth_plugin=''),
api_version=API_VERSION,
verify=True)
client_manager = clientmanager.ClientManager(
cli_options=FakeOptions(os_auth_plugin=''),
api_version=API_VERSION,
verify=True,
)
self.assertRaises(
exc.CommandError,
client_manager.setup_auth,
)