From bd9ebc5d27f00f30b86b387bb191bfc82e0780f2 Mon Sep 17 00:00:00 2001
From: Alexis Lee <alexisl@hp.com>
Date: Thu, 19 Dec 2013 19:26:06 +0000
Subject: [PATCH] Don't call CS if a token + URL are provided

Adds --os-auth-token (matching Glance client at least) to accept a
pre-obtained authentication token.

CS will be called iff:
      One or both of token and URL are not provided;
  AND the cache is empty/disabled or we don't have a tenant-id.

Removed some code altering the auth request to a GET if a token is
supplied - did not work for me and I think the path was dead previously.
Fixed test to account for this change.

Completes blueprint token-endpoint-instantiation

Change-Id: I67410b80e506bb80c152223cd113b7139a62a536
---
 novaclient/client.py               |  42 ++++++-----
 novaclient/shell.py                | 108 ++++++++++++++++++-----------
 novaclient/tests/test_client.py    |  20 ++++++
 novaclient/tests/v1_1/test_auth.py |  15 ++--
 novaclient/v1_1/client.py          |   3 +-
 novaclient/v3/client.py            |   3 +-
 6 files changed, 127 insertions(+), 64 deletions(-)

diff --git a/novaclient/client.py b/novaclient/client.py
index 0ae8bb285..d89331322 100644
--- a/novaclient/client.py
+++ b/novaclient/client.py
@@ -48,13 +48,19 @@ class HTTPClient(object):
                  timings=False, bypass_url=None,
                  os_cache=False, no_cache=True,
                  http_log_debug=False, auth_system='keystone',
-                 auth_plugin=None,
+                 auth_plugin=None, auth_token=None,
                  cacert=None, tenant_id=None):
         self.user = user
         self.password = password
         self.projectid = projectid
         self.tenant_id = tenant_id
 
+        # This will be called by #_get_password if self.password is None.
+        # EG if a password can only be obtained by prompting the user, but a
+        # token is available, you don't want to prompt until the token has
+        # been proven invalid
+        self.password_func = None
+
         if auth_system and auth_system != 'keystone' and not auth_plugin:
             raise exceptions.AuthSystemNotFound(auth_system)
 
@@ -80,8 +86,8 @@ class HTTPClient(object):
 
         self.times = []  # [("item", starttime, endtime), ...]
 
-        self.management_url = None
-        self.auth_token = None
+        self.management_url = self.bypass_url or None
+        self.auth_token = auth_token
         self.proxy_token = proxy_token
         self.proxy_tenant_id = proxy_tenant_id
         self.keyring_saver = None
@@ -239,6 +245,11 @@ class HTTPClient(object):
             except exceptions.Unauthorized:
                 raise e
 
+    def _get_password(self):
+        if not self.password and self.password_func:
+            self.password = self.password_func()
+        return self.password
+
     def get(self, url, **kwargs):
         return self._cs_request(url, 'GET', **kwargs)
 
@@ -324,6 +335,10 @@ class HTTPClient(object):
                 self.version = part
                 break
 
+        if self.auth_token and self.management_url:
+            self._save_keys()
+            return
+
         # TODO(sandy): Assume admin endpoint is 35357 for now.
         # Ideally this is going to have to be provided by the service catalog.
         new_netloc = netloc.replace(':%d' % port, ':%d' % (35357,))
@@ -367,8 +382,13 @@ class HTTPClient(object):
         elif not self.management_url:
             raise exceptions.Unauthorized('Nova Client')
 
+        self._save_keys()
+
+    def _save_keys(self):
         # Store the token/mgmt url in the keyring for later requests.
-        if self.keyring_saver and self.os_cache and not self.keyring_saved:
+        if (self.keyring_saver and self.os_cache and not self.keyring_saved
+                and self.auth_token and self.management_url
+                and self.tenant_id):
             self.keyring_saver.save(self.auth_token,
                                     self.management_url,
                                     self.tenant_id)
@@ -380,7 +400,7 @@ class HTTPClient(object):
             raise exceptions.NoTokenLookupException()
 
         headers = {'X-Auth-User': self.user,
-                   'X-Auth-Key': self.password}
+                   'X-Auth-Key': self._get_password()}
         if self.projectid:
             headers['X-Auth-Project-Id'] = self.projectid
 
@@ -409,7 +429,7 @@ class HTTPClient(object):
         else:
             body = {"auth": {
                     "passwordCredentials": {"username": self.user,
-                                            "password": self.password}}}
+                                            "password": self._get_password()}}}
 
         if self.tenant_id:
             body['auth']['tenantId'] = self.tenant_id
@@ -423,16 +443,6 @@ class HTTPClient(object):
         method = "POST"
         token_url = url + "/tokens"
 
-        # if we have a valid auth token, use it instead of generating a new one
-        if self.auth_token:
-            kwargs.setdefault('headers', {})['X-Auth-Token'] = self.auth_token
-            token_url += "/" + self.auth_token
-            method = "GET"
-            body = None
-
-        if self.auth_token and self.tenant_id and self.management_url:
-            return None
-
         # Make sure we follow redirects when trying to reach Keystone
         resp, respbody = self._time_request(
             token_url,
diff --git a/novaclient/shell.py b/novaclient/shell.py
index c4a0c9ec3..a4254c929 100644
--- a/novaclient/shell.py
+++ b/novaclient/shell.py
@@ -82,6 +82,7 @@ class SecretsHelper(object):
         self.args = args
         self.client = client
         self.key = None
+        self._password = None
 
     def _validate_string(self, text):
         if text is None or len(text) == 0:
@@ -143,11 +144,21 @@ class SecretsHelper(object):
 
     @property
     def password(self):
-        if self._validate_string(self.args.os_password):
-            return self.args.os_password
-        verify_pass = strutils.bool_from_string(
-            utils.env("OS_VERIFY_PASSWORD", default=False), True)
-        return self._prompt_password(verify_pass)
+        # Cache password so we prompt user at most once
+        if self._password:
+            pass
+        elif self._validate_string(self.args.os_password):
+            self._password = self.args.os_password
+        else:
+            verify_pass = strutils.bool_from_string(
+                utils.env("OS_VERIFY_PASSWORD", default=False), True)
+            self._password = self._prompt_password(verify_pass)
+        if not self._password:
+            raise exc.CommandError(
+                'Expecting a password provided via either '
+                '--os-password, env[OS_PASSWORD], or '
+                'prompted response')
+        return self._password
 
     @property
     def management_url(self):
@@ -260,6 +271,10 @@ class OpenStackComputeShell(object):
             type=positive_non_zero_float,
             help="Set HTTP call timeout (in seconds)")
 
+        parser.add_argument('--os-auth-token',
+                default=utils.env('OS_AUTH_TOKEN'),
+                help='Defaults to env[OS_AUTH_TOKEN]')
+
         parser.add_argument('--os-username',
             metavar='<auth-user-name>',
             default=utils.env('OS_USERNAME', 'NOVA_USERNAME'),
@@ -491,7 +506,6 @@ class OpenStackComputeShell(object):
                             format=streamformat)
 
     def main(self, argv):
-
         # Parse args once to find version and debug settings
         parser = self.get_base_parser()
         (options, args) = parser.parse_known_args(argv)
@@ -532,27 +546,37 @@ class OpenStackComputeShell(object):
             self.do_bash_completion(args)
             return 0
 
-        (os_username, os_tenant_name, os_tenant_id, os_auth_url,
-                os_region_name, os_auth_system, endpoint_type, insecure,
-                service_type, service_name, volume_service_name,
-                bypass_url, os_cache, cacert, timeout) = (
-                        args.os_username,
-                        args.os_tenant_name, args.os_tenant_id,
-                        args.os_auth_url,
-                        args.os_region_name, args.os_auth_system,
-                        args.endpoint_type, args.insecure, args.service_type,
-                        args.service_name, args.volume_service_name,
-                        args.bypass_url, args.os_cache,
-                        args.os_cacert, args.timeout)
+        os_username = args.os_username
+        os_password = None  # Fetched and set later as needed
+        os_tenant_name = args.os_tenant_name
+        os_tenant_id = args.os_tenant_id
+        os_auth_url = args.os_auth_url
+        os_region_name = args.os_region_name
+        os_auth_system = args.os_auth_system
+        endpoint_type = args.endpoint_type
+        insecure = args.insecure
+        service_type = args.service_type
+        service_name = args.service_name
+        volume_service_name = args.volume_service_name
+        bypass_url = args.bypass_url
+        os_cache = args.os_cache
+        cacert = args.os_cacert
+        timeout = args.timeout
+
+        # We may have either, both or none of these.
+        # If we have both, we don't need USERNAME, PASSWORD etc.
+        # Fill in the blanks from the SecretsHelper if possible.
+        # Finally, authenticate unless we have both.
+        # Note if we don't auth we probably don't have a tenant ID so we can't
+        # cache the token.
+        auth_token = args.os_auth_token if args.os_auth_token else None
+        management_url = bypass_url if bypass_url else None
 
         if os_auth_system and os_auth_system != "keystone":
             auth_plugin = novaclient.auth_plugin.load_plugin(os_auth_system)
         else:
             auth_plugin = None
 
-        # Fetched and set later as needed
-        os_password = None
-
         if not endpoint_type:
             endpoint_type = DEFAULT_NOVA_ENDPOINT_TYPE
 
@@ -567,9 +591,14 @@ class OpenStackComputeShell(object):
                     DEFAULT_OS_COMPUTE_API_VERSION]
             service_type = utils.get_service_type(args.func) or service_type
 
+        # If we have an auth token but no management_url, we must auth anyway.
+        # Expired tokens are handled by client.py:_cs_request
+        must_auth = not (utils.isunauthenticated(args.func)
+                         or (auth_token and management_url))
+
         #FIXME(usrleon): Here should be restrict for project id same as
         # for os_username or os_password but for compatibility it is not.
-        if not utils.isunauthenticated(args.func):
+        if must_auth:
             if auth_plugin:
                 auth_plugin.parse_opts(args)
 
@@ -613,7 +642,7 @@ class OpenStackComputeShell(object):
                 region_name=os_region_name, endpoint_type=endpoint_type,
                 extensions=self.extensions, service_type=service_type,
                 service_name=service_name, auth_system=os_auth_system,
-                auth_plugin=auth_plugin,
+                auth_plugin=auth_plugin, auth_token=auth_token,
                 volume_service_name=volume_service_name,
                 timings=args.timings, bypass_url=bypass_url,
                 os_cache=os_cache, http_log_debug=options.debug,
@@ -621,7 +650,7 @@ class OpenStackComputeShell(object):
 
         # Now check for the password/token of which pieces of the
         # identifying keyring key can come from the underlying client
-        if not utils.isunauthenticated(args.func):
+        if must_auth:
             helper = SecretsHelper(args, self.cs.client)
             if (auth_plugin and auth_plugin.opts and
                     "os_password" not in auth_plugin.opts):
@@ -629,31 +658,26 @@ class OpenStackComputeShell(object):
             else:
                 use_pw = True
 
-            tenant_id, auth_token, management_url = (helper.tenant_id,
-                                                     helper.auth_token,
-                                                     helper.management_url)
+            tenant_id = helper.tenant_id
+            # Allow commandline to override cache
+            if not auth_token:
+                auth_token = helper.auth_token
+            if not management_url:
+                management_url = helper.management_url
             if tenant_id and auth_token and management_url:
                 self.cs.client.tenant_id = tenant_id
                 self.cs.client.auth_token = auth_token
                 self.cs.client.management_url = management_url
-                # authenticate just sets up some values in this case, no REST
-                # calls
-                self.cs.authenticate()
-            if use_pw:
-                # Auth using token must have failed or not happened
-                # at all, so now switch to password mode and save
-                # the token when its gotten... using our keyring
-                # saver
-                os_password = helper.password
-                if not os_password:
-                    raise exc.CommandError(
-                        'Expecting a password provided via either '
-                        '--os-password, env[OS_PASSWORD], or '
-                        'prompted response')
-                self.cs.client.password = os_password
+                self.cs.client.password_fun = lambda: helper.password
+            elif use_pw:
+                # We're missing something, so auth with user/pass and save
+                # the result in our helper.
+                self.cs.client.password = helper.password
                 self.cs.client.keyring_saver = helper
 
         try:
+            # This does a couple of bits which are useful even if we've
+            # got the token + service URL already. It exits fast in that case.
             if not utils.isunauthenticated(args.func):
                 self.cs.authenticate()
         except exc.Unauthorized:
diff --git a/novaclient/tests/test_client.py b/novaclient/tests/test_client.py
index ded0d9add..0d2cc4cf7 100644
--- a/novaclient/tests/test_client.py
+++ b/novaclient/tests/test_client.py
@@ -196,3 +196,23 @@ class ClientTest(utils.TestCase):
                                            auth_url="foo/v2")
         cs.authenticate()
         self.assertTrue(mock_authenticate.called)
+
+    def test_get_password_simple(self):
+        cs = novaclient.client.HTTPClient("user", "password", "", "")
+        cs.password_func = mock.Mock()
+        self.assertEqual(cs._get_password(), "password")
+        self.assertFalse(cs.password_func.called)
+
+    def test_get_password_none(self):
+        cs = novaclient.client.HTTPClient("user", None, "", "")
+        self.assertEqual(cs._get_password(), None)
+
+    def test_get_password_func(self):
+        cs = novaclient.client.HTTPClient("user", None, "", "")
+        cs.password_func = mock.Mock(return_value="password")
+        self.assertEqual(cs._get_password(), "password")
+        cs.password_func.assert_called_once_with()
+
+        cs.password_func = mock.Mock()
+        self.assertEqual(cs._get_password(), "password")
+        self.assertFalse(cs.password_func.called)
diff --git a/novaclient/tests/v1_1/test_auth.py b/novaclient/tests/v1_1/test_auth.py
index 1754f8a2a..7344bc76e 100644
--- a/novaclient/tests/v1_1/test_auth.py
+++ b/novaclient/tests/v1_1/test_auth.py
@@ -379,15 +379,22 @@ class AuthenticateAgainstKeystoneTests(utils.TestCase):
                 'User-Agent': cs.client.USER_AGENT,
                 'Content-Type': 'application/json',
                 'Accept': 'application/json',
-                'X-Auth-Token': "FAKE_ID",
+            }
+            body = {
+                'auth': {
+                    'token': {
+                        'id': cs.client.auth_token,
+                    },
+                    'tenantName': cs.client.projectid,
+                },
             }
 
-            token_url = cs.client.auth_url + "/tokens/FAKE_ID"
+            token_url = cs.client.auth_url + "/tokens"
             mock_request.assert_called_with(
-                "GET",
+                "POST",
                 token_url,
                 headers=headers,
-                data="null",
+                data=json.dumps(body),
                 allow_redirects=True,
                 **self.TEST_REQUEST_BASE)
 
diff --git a/novaclient/v1_1/client.py b/novaclient/v1_1/client.py
index a89581eac..382929dcc 100644
--- a/novaclient/v1_1/client.py
+++ b/novaclient/v1_1/client.py
@@ -73,7 +73,7 @@ class Client(object):
                   volume_service_name=None, timings=False,
                   bypass_url=None, os_cache=False, no_cache=True,
                   http_log_debug=False, auth_system='keystone',
-                  auth_plugin=None,
+                  auth_plugin=None, auth_token=None,
                   cacert=None, tenant_id=None):
         # FIXME(comstud): Rename the api_key argument above when we
         # know it's not being used as keyword argument
@@ -131,6 +131,7 @@ class Client(object):
                                     projectid=project_id,
                                     tenant_id=tenant_id,
                                     auth_url=auth_url,
+                                    auth_token=auth_token,
                                     insecure=insecure,
                                     timeout=timeout,
                                     auth_system=auth_system,
diff --git a/novaclient/v3/client.py b/novaclient/v3/client.py
index 16c5a33b3..37489616f 100644
--- a/novaclient/v3/client.py
+++ b/novaclient/v3/client.py
@@ -57,7 +57,7 @@ class Client(object):
                   volume_service_name=None, timings=False,
                   bypass_url=None, os_cache=False, no_cache=True,
                   http_log_debug=False, auth_system='keystone',
-                  auth_plugin=None,
+                  auth_plugin=None, auth_token=None,
                   cacert=None, tenant_id=None):
         self.projectid = project_id
         self.tenant_id = tenant_id
@@ -96,6 +96,7 @@ class Client(object):
                                     timeout=timeout,
                                     auth_system=auth_system,
                                     auth_plugin=auth_plugin,
+                                    auth_token=auth_token,
                                     proxy_token=proxy_token,
                                     proxy_tenant_id=proxy_tenant_id,
                                     region_name=region_name,