openstack/python-openstackclient
Code Issues Proposed changes

Fix auth-required for help command

Browse Source 
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
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
examples/osc-lib.py
View File

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

6
openstackclient/api/auth.py
View File

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

71
openstackclient/common/clientmanager.py
View File

@ -56,14 +56,14 @@ class ClientManager(object):
def __init__( def __init__(
self, self,
auth_options, cli_options,
api_version=None, api_version=None,
verify=True, verify=True,
pw_func=None, pw_func=None,
): ):
"""Set up a ClientManager """Set up a ClientManager
:param auth_options: :param cli_options:
Options collected from the command-line, environment, or wherever Options collected from the command-line, environment, or wherever
:param api_version: :param api_version:
Dict of API versions: key is API name, value is the 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 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 # If no auth type is named by the user, select one based on
# the supplied options # 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 # 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 # Horrible hack alert...must handle prompt for null password if
# password auth is requested. # password auth is requested.
if (self.auth_plugin_name.endswith('password') and if (self.auth_plugin_name.endswith('password') and
not auth_options.os_password): not self._cli_options.os_password):
auth_options.os_password = pw_func() self._cli_options.os_password = self.pw_callback()
(auth_plugin, self._auth_params) = auth.build_auth_params( (auth_plugin, self._auth_params) = auth.build_auth_params(
self.auth_plugin_name, self.auth_plugin_name,
auth_options, self._cli_options,
) )
self._url = auth_options.os_url default_domain = self._cli_options.os_default_domain
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
# NOTE(stevemar): If PROJECT_DOMAIN_ID or PROJECT_DOMAIN_NAME is # NOTE(stevemar): If PROJECT_DOMAIN_ID or PROJECT_DOMAIN_NAME is
# present, then do not change the behaviour. Otherwise, set the # present, then do not change the behaviour. Otherwise, set the
# PROJECT_DOMAIN_ID to 'OS_DEFAULT_DOMAIN' for better usability. # PROJECT_DOMAIN_ID to 'OS_DEFAULT_DOMAIN' for better usability.
@ -125,20 +151,6 @@ class ClientManager(object):
elif 'tenant_name' in self._auth_params: elif 'tenant_name' in self._auth_params:
self._project_name = self._auth_params['tenant_name'] 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) LOG.info('Using auth plugin: %s' % self.auth_plugin_name)
self.auth = auth_plugin.load_from_options(**self._auth_params) self.auth = auth_plugin.load_from_options(**self._auth_params)
# needed by SAML authentication # needed by SAML authentication
@ -146,7 +158,7 @@ class ClientManager(object):
self.session = session.Session( self.session = session.Session(
auth=self.auth, auth=self.auth,
session=request_session, session=request_session,
verify=verify, verify=self._verify,
) )
return return
@ -155,6 +167,7 @@ class ClientManager(object):
def auth_ref(self): def auth_ref(self):
"""Dereference will trigger an auth if it hasn't already""" """Dereference will trigger an auth if it hasn't already"""
if not self._auth_ref: if not self._auth_ref:
self.setup_auth()
LOG.debug("Get auth_ref") LOG.debug("Get auth_ref")
self._auth_ref = self.auth.get_auth_ref(self.session) self._auth_ref = self.auth.get_auth_ref(self.session)
return self._auth_ref return self._auth_ref

10
openstackclient/shell.py
View File

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

61
openstackclient/tests/common/test_clientmanager.py
View File

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