Merge "Fix auth-required for help command"
This commit is contained in:
		@@ -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,
 | 
			
		||||
    )
 | 
			
		||||
 
 | 
			
		||||
@@ -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
 | 
			
		||||
 | 
			
		||||
 
 | 
			
		||||
@@ -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
 | 
			
		||||
 
 | 
			
		||||
@@ -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
 | 
			
		||||
 
 | 
			
		||||
@@ -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,
 | 
			
		||||
        )
 | 
			
		||||
 
 | 
			
		||||
		Reference in New Issue
	
	Block a user