From 505fa14cd68e13d066a5770a229ba0d7fa88d2a9 Mon Sep 17 00:00:00 2001
From: Dean Troyer <dtroyer@gmail.com>
Date: Fri, 27 Feb 2015 09:19:12 -0600
Subject: [PATCH] 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
---
examples/osc-lib.py | 2 +-
openstackclient/api/auth.py | 6 +-
openstackclient/common/clientmanager.py | 71 +++++++++++--------
openstackclient/shell.py | 10 ++-
.../tests/common/test_clientmanager.py | 61 ++++++++++------
5 files changed, 90 insertions(+), 60 deletions(-)
diff --git a/examples/osc-lib.py b/examples/osc-lib.py
index 69fc5d9849..2960a2f74e 100755
--- a/examples/osc-lib.py
+++ b/examples/osc-lib.py
@@ -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,
)
diff --git a/openstackclient/api/auth.py b/openstackclient/api/auth.py
index 14bb01d7b1..d44f2f9d63 100644
--- a/openstackclient/api/auth.py
+++ b/openstackclient/api/auth.py
@@ -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
diff --git a/openstackclient/common/clientmanager.py b/openstackclient/common/clientmanager.py
index 974936f899..c430791933 100644
--- a/openstackclient/common/clientmanager.py
+++ b/openstackclient/common/clientmanager.py
@@ -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
diff --git a/openstackclient/shell.py b/openstackclient/shell.py
index 246f51b177..b61da2b941 100644
--- a/openstackclient/shell.py
+++ b/openstackclient/shell.py
@@ -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
diff --git a/openstackclient/tests/common/test_clientmanager.py b/openstackclient/tests/common/test_clientmanager.py
index 3b2b976b5d..f3826ca370 100644
--- a/openstackclient/tests/common/test_clientmanager.py
+++ b/openstackclient/tests/common/test_clientmanager.py
@@ -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,
+ )