From 4c5987adf2f0336ce10ead647d2a9325114c6c6c Mon Sep 17 00:00:00 2001
From: Matt Oswalt <oswaltm@brocade.com>
Date: Tue, 14 Feb 2017 16:30:24 -0800
Subject: [PATCH 01/13] Initial work on fixing `st2 login` bug.

In st2client/base.py, there's a conditional that detects if the username and password
is provided, and if so, try to retrieve a token and update the token attribute of the
client before returning. However, in a traditional `st2 login` scenario, only the
'username' field is populated, so this code will never run.

Previous tests were done on systems that effectively had auth disabled (doh!), so while
the config file and token files were being changed/generated correctly, API requests
did not carry the 'X-Auth-Token' header. Since auth was disabled, this was no problem.
However, on a real installation, this feature just plainly did not work.

In short, this allows the 'st2 login' feature to work, and it also provides unit tests
to help prevent this kind of thing

Signed-off-by: Matt Oswalt <oswaltm@brocade.com>
---
 st2client/st2client/base.py        |  2 +-
 st2client/st2client/models/core.py |  1 +
 st2client/st2client/shell.py       |  3 +--
 st2client/tests/base.py            |  4 +++-
 st2client/tests/unit/test_auth.py  | 11 +++++++++++
 5 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/st2client/st2client/base.py b/st2client/st2client/base.py
index f90dedb8c0..c1bc62f489 100644
--- a/st2client/st2client/base.py
+++ b/st2client/st2client/base.py
@@ -124,7 +124,7 @@ def get_client(self, args, debug=False):
         password = credentials.get('password', None)
         cache_token = rc_config.get('cli', {}).get('cache_token', False)
 
-        if username and password:
+        if credentials:
             # Credentials are provided, try to authenticate agaist the API
             try:
                 token = self._get_auth_token(client=client, username=username, password=password,
diff --git a/st2client/st2client/models/core.py b/st2client/st2client/models/core.py
index f2352502f4..6b37af992f 100644
--- a/st2client/st2client/models/core.py
+++ b/st2client/st2client/models/core.py
@@ -289,6 +289,7 @@ def get_by_name(self, name, **kwargs):
 
     @add_auth_token_to_kwargs_from_env
     def create(self, instance, **kwargs):
+
         url = '/%s' % self.resource.get_url_path_name()
         response = self.client.post(url, instance.serialize(), **kwargs)
         if response.status_code != 200:
diff --git a/st2client/st2client/shell.py b/st2client/st2client/shell.py
index c26c302984..529e04fe08 100755
--- a/st2client/st2client/shell.py
+++ b/st2client/st2client/shell.py
@@ -65,8 +65,7 @@ class Shell(BaseCLIApp):
     LOG = LOGGER
 
     SKIP_AUTH_CLASSES = [
-        TokenCreateCommand.__name__,
-        LoginCommand.__name__
+        TokenCreateCommand.__name__
     ]
 
     def __init__(self):
diff --git a/st2client/tests/base.py b/st2client/tests/base.py
index 49d2fa6fa8..9f40ff0329 100644
--- a/st2client/tests/base.py
+++ b/st2client/tests/base.py
@@ -77,6 +77,8 @@ class BaseCLITestCase(unittest2.TestCase):
     stdout = six.moves.StringIO()
     stderr = six.moves.StringIO()
 
+    DEFAULT_SKIP_CONFIG = '1'
+
     def setUp(self):
         super(BaseCLITestCase, self).setUp()
 
@@ -86,7 +88,7 @@ def setUp(self):
             if var in os.environ:
                 del os.environ[var]
 
-        os.environ['ST2_CLI_SKIP_CONFIG'] = '1'
+        os.environ['ST2_CLI_SKIP_CONFIG'] = self.DEFAULT_SKIP_CONFIG
 
         if self.capture_output:
             # Make sure we reset it for each test class instance
diff --git a/st2client/tests/unit/test_auth.py b/st2client/tests/unit/test_auth.py
index 1e4c72c664..c157d208ba 100644
--- a/st2client/tests/unit/test_auth.py
+++ b/st2client/tests/unit/test_auth.py
@@ -61,6 +61,9 @@ class TestLoginBase(base.BaseCLITestCase):
     def __init__(self, *args, **kwargs):
         super(TestLoginBase, self).__init__(*args, **kwargs)
 
+        # We're overriding the default behavior for CLI test cases here
+        self.DEFAULT_SKIP_CONFIG = '0'
+
         self.parser = argparse.ArgumentParser()
         self.parser.add_argument('-t', '--token', dest='token')
         self.parser.add_argument('--api-key', dest='api_key')
@@ -154,6 +157,14 @@ def runTest(self, mock_gp):
 
         self.shell.run(args)
 
+        # TODO(mierdin): This tests that this particular command sends X-Auth-Token but you should
+        # also test other commands after this token has been installed
+        kwargs = {
+            'headers': {'X-Auth-Token': self.TOKEN['token'], 'content-type': 'application/json'},
+            'auth': ('st2admin', 'Password1!')
+        }
+        requests.post.assert_called_with('http://127.0.0.1:9100/tokens', '{}', **kwargs)
+
         with open(self.CONFIG_FILE, 'r') as config_file:
 
             for line in config_file.readlines():

From 89057ee8142d40a4748914291a5adb679ff7a954 Mon Sep 17 00:00:00 2001
From: Matt Oswalt <oswaltm@brocade.com>
Date: Tue, 14 Feb 2017 16:49:31 -0800
Subject: [PATCH 02/13] Removed dumb extra space

Signed-off-by: Matt Oswalt <oswaltm@brocade.com>
---
 st2client/st2client/models/core.py | 1 -
 1 file changed, 1 deletion(-)

diff --git a/st2client/st2client/models/core.py b/st2client/st2client/models/core.py
index 6b37af992f..f2352502f4 100644
--- a/st2client/st2client/models/core.py
+++ b/st2client/st2client/models/core.py
@@ -289,7 +289,6 @@ def get_by_name(self, name, **kwargs):
 
     @add_auth_token_to_kwargs_from_env
     def create(self, instance, **kwargs):
-
         url = '/%s' % self.resource.get_url_path_name()
         response = self.client.post(url, instance.serialize(), **kwargs)
         if response.status_code != 200:

From f755d03e2a9b88ebb4fa59871febff50cfa8e7e8 Mon Sep 17 00:00:00 2001
From: Matt Oswalt <oswaltm@brocade.com>
Date: Tue, 14 Feb 2017 17:03:32 -0800
Subject: [PATCH 03/13] Removed unused import

Signed-off-by: Matt Oswalt <oswaltm@brocade.com>
---
 st2client/st2client/shell.py | 1 -
 1 file changed, 1 deletion(-)

diff --git a/st2client/st2client/shell.py b/st2client/st2client/shell.py
index 529e04fe08..ec297f6403 100755
--- a/st2client/st2client/shell.py
+++ b/st2client/st2client/shell.py
@@ -50,7 +50,6 @@
 from st2client.config import set_config
 from st2client.exceptions.operations import OperationFailureException
 from st2client.utils.logging import LogLevelFilter, set_log_level_for_all_loggers
-from st2client.commands.auth import TokenCreateCommand, LoginCommand
 
 __all__ = [
     'Shell'

From 4b8a1460ec7190f494fd4ccb2224dc3485fc8e62 Mon Sep 17 00:00:00 2001
From: Tomaz Muraus <tomaz@tomaz.me>
Date: Wed, 15 Feb 2017 10:54:16 +0100
Subject: [PATCH 04/13] Add missing import and logincommand class back so st2
 login works.

---
 st2client/st2client/shell.py | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/st2client/st2client/shell.py b/st2client/st2client/shell.py
index ec297f6403..94951c6f2b 100755
--- a/st2client/st2client/shell.py
+++ b/st2client/st2client/shell.py
@@ -50,6 +50,9 @@
 from st2client.config import set_config
 from st2client.exceptions.operations import OperationFailureException
 from st2client.utils.logging import LogLevelFilter, set_log_level_for_all_loggers
+from st2client.commands.auth import TokenCreateCommand
+from st2client.commands.auth import LoginCommand
+
 
 __all__ = [
     'Shell'
@@ -64,7 +67,8 @@ class Shell(BaseCLIApp):
     LOG = LOGGER
 
     SKIP_AUTH_CLASSES = [
-        TokenCreateCommand.__name__
+        TokenCreateCommand.__name__,
+        LoginCommand.__name__
     ]
 
     def __init__(self):

From 7d09f74002032ac083d49db41004f71b648c9c47 Mon Sep 17 00:00:00 2001
From: Tomaz Muraus <tomaz@tomaz.me>
Date: Wed, 15 Feb 2017 10:58:40 +0100
Subject: [PATCH 05/13] Skip auth on st2 whoami command.

We really just want to read a username from config, etc. and don't want
to perform any additional auth requests.
---
 st2client/st2client/shell.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/st2client/st2client/shell.py b/st2client/st2client/shell.py
index 94951c6f2b..915cd89612 100755
--- a/st2client/st2client/shell.py
+++ b/st2client/st2client/shell.py
@@ -52,6 +52,7 @@
 from st2client.utils.logging import LogLevelFilter, set_log_level_for_all_loggers
 from st2client.commands.auth import TokenCreateCommand
 from st2client.commands.auth import LoginCommand
+from st2client.commands.auth import WhoamiCommand
 
 
 __all__ = [
@@ -68,7 +69,8 @@ class Shell(BaseCLIApp):
 
     SKIP_AUTH_CLASSES = [
         TokenCreateCommand.__name__,
-        LoginCommand.__name__
+        LoginCommand.__name__,
+        WhoamiCommand.__name__
     ]
 
     def __init__(self):

From cd89193fd69bda32a1f9591faa1e9ea6f8e953a0 Mon Sep 17 00:00:00 2001
From: Tomaz Muraus <tomaz@tomaz.me>
Date: Wed, 15 Feb 2017 11:02:06 +0100
Subject: [PATCH 06/13] Update the test - no token is sent in the headers to
 the auth api, just username and password and token is returned back.

---
 st2client/tests/unit/test_auth.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/st2client/tests/unit/test_auth.py b/st2client/tests/unit/test_auth.py
index c157d208ba..06fa109de6 100644
--- a/st2client/tests/unit/test_auth.py
+++ b/st2client/tests/unit/test_auth.py
@@ -160,7 +160,7 @@ def runTest(self, mock_gp):
         # TODO(mierdin): This tests that this particular command sends X-Auth-Token but you should
         # also test other commands after this token has been installed
         kwargs = {
-            'headers': {'X-Auth-Token': self.TOKEN['token'], 'content-type': 'application/json'},
+            'headers': {'content-type': 'application/json'},
             'auth': ('st2admin', 'Password1!')
         }
         requests.post.assert_called_with('http://127.0.0.1:9100/tokens', '{}', **kwargs)

From 545c7baef5225e9e9c0a10d97c25dec3b822c87c Mon Sep 17 00:00:00 2001
From: Tomaz Muraus <tomaz@tomaz.me>
Date: Wed, 15 Feb 2017 11:06:15 +0100
Subject: [PATCH 07/13] Add test case which verifies that st2 login works
 correctly and that X-Auth-Token is indeed sent in the header in subsequent
 requests after st2 login to st2 API.

---
 st2client/tests/unit/test_auth.py | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/st2client/tests/unit/test_auth.py b/st2client/tests/unit/test_auth.py
index 06fa109de6..7135e31fd1 100644
--- a/st2client/tests/unit/test_auth.py
+++ b/st2client/tests/unit/test_auth.py
@@ -145,6 +145,9 @@ class TestLoginIntPwdAndConfig(TestLoginBase):
     @mock.patch.object(
         requests, 'post',
         mock.MagicMock(return_value=base.FakeResponse(json.dumps(TOKEN), 200, 'OK')))
+    @mock.patch.object(
+        requests, 'get',
+        mock.MagicMock(return_value=base.FakeResponse(json.dumps({}), 200, 'OK')))
     @mock.patch('st2client.commands.auth.getpass')
     def runTest(self, mock_gp):
         '''Test 'st2 login' functionality with interactive password entry
@@ -157,18 +160,14 @@ def runTest(self, mock_gp):
 
         self.shell.run(args)
 
-        # TODO(mierdin): This tests that this particular command sends X-Auth-Token but you should
-        # also test other commands after this token has been installed
-        kwargs = {
+        expected_kwargs = {
             'headers': {'content-type': 'application/json'},
             'auth': ('st2admin', 'Password1!')
         }
-        requests.post.assert_called_with('http://127.0.0.1:9100/tokens', '{}', **kwargs)
+        requests.post.assert_called_with('http://127.0.0.1:9100/tokens', '{}', **expected_kwargs)
 
         with open(self.CONFIG_FILE, 'r') as config_file:
-
             for line in config_file.readlines():
-
                 # Make sure certain values are not present
                 self.assertFalse('password' in line)
                 self.assertFalse('olduser' in line)
@@ -180,6 +179,18 @@ def runTest(self, mock_gp):
             # validate token was created
             self.assertTrue(os.path.isfile('%stoken-%s' % (self.DOTST2_PATH, expected_username)))
 
+        # Validate token is sent on subsequent requests to st2 API
+        args = ['--config', self.CONFIG_FILE, 'pack', 'list']
+        self.shell.run(args)
+
+        expected_kwargs = {
+            'headers': {
+                'X-Auth-Token': self.TOKEN['token']
+            },
+            'params': {}
+        }
+        requests.get.assert_called_with('http://127.0.0.1:9101/v1/packs', **expected_kwargs)
+
 
 class TestLoginWritePwdOkay(TestLoginBase):
 

From a59dd26584ba6924957a3123f81de65c747db2cc Mon Sep 17 00:00:00 2001
From: Tomaz Muraus <tomaz@tomaz.me>
Date: Wed, 15 Feb 2017 11:10:36 +0100
Subject: [PATCH 08/13] Fix typo.

---
 st2client/st2client/commands/auth.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/st2client/st2client/commands/auth.py b/st2client/st2client/commands/auth.py
index c37ba6955a..1735e7ea11 100644
--- a/st2client/st2client/commands/auth.py
+++ b/st2client/st2client/commands/auth.py
@@ -98,7 +98,7 @@ def __init__(self, resource, *args, **kwargs):
         self.parser.add_argument('-w', '--write-password', action='store_true', default=False,
                                  dest='write_password',
                                  help='Write the password in plain text to the config file '
-                                      '(default is to omit it')
+                                      '(default is to omit it)')
 
     def run(self, args, **kwargs):
 

From 3418ffea3f2e0edce4d82ef0b8fe67c282ce9330 Mon Sep 17 00:00:00 2001
From: Tomaz Muraus <tomaz@tomaz.me>
Date: Wed, 15 Feb 2017 16:56:02 +0100
Subject: [PATCH 09/13] Print a message / notice about token expiration if the
 user doesn't use --write-password flag.

---
 st2client/st2client/commands/auth.py | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/st2client/st2client/commands/auth.py b/st2client/st2client/commands/auth.py
index 1735e7ea11..7023981aee 100644
--- a/st2client/st2client/commands/auth.py
+++ b/st2client/st2client/commands/auth.py
@@ -17,8 +17,10 @@
 import json
 import logging
 
+from oslo_config import cfg
 from six.moves.configparser import ConfigParser
 
+from st2common.config import parse_args
 from st2client.base import BaseCLIApp
 from st2client import config_parser
 from st2client import models
@@ -142,12 +144,24 @@ def run(self, args, **kwargs):
     def run_and_print(self, args, **kwargs):
         try:
             self.run(args, **kwargs)
-            print("Logged in as %s" % (args.username))
         except Exception as e:
-            print("Failed to log in as %s: %s" % (args.username, str(e)))
+            print('Failed to log in as %s: %s' % (args.username, str(e)))
             if self.app.client.debug:
                 raise
 
+        print('Logged in as %s' % (args.username))
+
+        if not args.write_password:
+            parse_args(args=[])
+            token_expire_hours = (cfg.CONF.auth.token_ttl / 60 / 60)
+
+            print('')
+            print('Note: You didn\'t use --write-password option so the password hasn\'t been '
+                  'stored in the client config and you will need to login again in %s hours when '
+                  'the auth token expires.' % (token_expire_hours))
+            print('As an alternative you can run st2 login command with the "--write-password" '
+                  'flag, but keep it mind this will cause it to store the password in plain-text '
+                  'in the client config file.')
 
 class WhoamiCommand(resource.ResourceCommand):
     display_attributes = ['user', 'token', 'expiry']

From 772991b2a02c49ec20bff7b7dfbf45f860ff4db5 Mon Sep 17 00:00:00 2001
From: Tomaz Muraus <tomaz@tomaz.me>
Date: Wed, 15 Feb 2017 17:11:55 +0100
Subject: [PATCH 10/13] Fix lint.

---
 st2client/st2client/commands/auth.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/st2client/st2client/commands/auth.py b/st2client/st2client/commands/auth.py
index 7023981aee..c6ae5b52e6 100644
--- a/st2client/st2client/commands/auth.py
+++ b/st2client/st2client/commands/auth.py
@@ -163,6 +163,7 @@ def run_and_print(self, args, **kwargs):
                   'flag, but keep it mind this will cause it to store the password in plain-text '
                   'in the client config file.')
 
+
 class WhoamiCommand(resource.ResourceCommand):
     display_attributes = ['user', 'token', 'expiry']
 

From 62cbad8a8cdd419b84ed1c0617e8060414c59d56 Mon Sep 17 00:00:00 2001
From: Tomaz Muraus <tomaz@tomaz.me>
Date: Wed, 15 Feb 2017 17:13:10 +0100
Subject: [PATCH 11/13] Update message.

---
 st2client/st2client/commands/auth.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/st2client/st2client/commands/auth.py b/st2client/st2client/commands/auth.py
index c6ae5b52e6..22a2dbc5f2 100644
--- a/st2client/st2client/commands/auth.py
+++ b/st2client/st2client/commands/auth.py
@@ -159,9 +159,9 @@ def run_and_print(self, args, **kwargs):
             print('Note: You didn\'t use --write-password option so the password hasn\'t been '
                   'stored in the client config and you will need to login again in %s hours when '
                   'the auth token expires.' % (token_expire_hours))
-            print('As an alternative you can run st2 login command with the "--write-password" '
+            print('As an alternative, you can run st2 login command with the "--write-password" '
                   'flag, but keep it mind this will cause it to store the password in plain-text '
-                  'in the client config file.')
+                  'in the client config file (~/.st2/config).')
 
 
 class WhoamiCommand(resource.ResourceCommand):

From 5f7b2e57572eac609e4ce1888bf7addbe4760f04 Mon Sep 17 00:00:00 2001
From: Tomaz Muraus <tomaz@tomaz.me>
Date: Wed, 15 Feb 2017 17:20:20 +0100
Subject: [PATCH 12/13] st2clien't can't rely on st2common so we need to
 hard-code this default value.

---
 st2client/st2client/commands/auth.py | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/st2client/st2client/commands/auth.py b/st2client/st2client/commands/auth.py
index 22a2dbc5f2..99079fd114 100644
--- a/st2client/st2client/commands/auth.py
+++ b/st2client/st2client/commands/auth.py
@@ -17,10 +17,8 @@
 import json
 import logging
 
-from oslo_config import cfg
 from six.moves.configparser import ConfigParser
 
-from st2common.config import parse_args
 from st2client.base import BaseCLIApp
 from st2client import config_parser
 from st2client import models
@@ -152,8 +150,9 @@ def run_and_print(self, args, **kwargs):
         print('Logged in as %s' % (args.username))
 
         if not args.write_password:
-            parse_args(args=[])
-            token_expire_hours = (cfg.CONF.auth.token_ttl / 60 / 60)
+            # Note: Client can't depend and import from st2common so we need to hard-code this
+            # default value
+            token_expire_hours = 24
 
             print('')
             print('Note: You didn\'t use --write-password option so the password hasn\'t been '

From 7ddca45a36e65f885091dc2273f250106c522801 Mon Sep 17 00:00:00 2001
From: Tomaz Muraus <tomaz@tomaz.me>
Date: Wed, 15 Feb 2017 17:53:54 +0100
Subject: [PATCH 13/13] Update comment to avoid lint failure.

---
 st2client/st2client/commands/auth.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/st2client/st2client/commands/auth.py b/st2client/st2client/commands/auth.py
index 99079fd114..98a37820c5 100644
--- a/st2client/st2client/commands/auth.py
+++ b/st2client/st2client/commands/auth.py
@@ -150,7 +150,7 @@ def run_and_print(self, args, **kwargs):
         print('Logged in as %s' % (args.username))
 
         if not args.write_password:
-            # Note: Client can't depend and import from st2common so we need to hard-code this
+            # Note: Client can't depend and import from common so we need to hard-code this
             # default value
             token_expire_hours = 24