Repository: EMCECS/python-ecsclient

Issue:
Two logouts issued when only one logout() executed.
I am using a single logout() command, but the requests sent by the code I see two been issued.  The problem is the first one succeed (200) and the second call fails (probably because the first 200).

Discussion:
coveralls: 
[![Coverage Status](https://coveralls.io/builds/10825849/badge)](https://coveralls.io/builds/10825849)

Coverage increased (+0.3%) to 55.697% when pulling **799e48d6f2d54291c624f05e5a8774b4fbcf0da3 on bug-logout** into **546e342440f7ca0274d222179c6eca790d1d6573 on master**.

coveralls: 
[![Coverage Status](https://coveralls.io/builds/10825849/badge)](https://coveralls.io/builds/10825849)

Coverage increased (+0.3%) to 55.697% when pulling **799e48d6f2d54291c624f05e5a8774b4fbcf0da3 on bug-logout** into **546e342440f7ca0274d222179c6eca790d1d6573 on master**.

coveralls: 
[![Coverage Status](https://coveralls.io/builds/10825849/badge)](https://coveralls.io/builds/10825849)

Coverage increased (+0.3%) to 55.697% when pulling **799e48d6f2d54291c624f05e5a8774b4fbcf0da3 on bug-logout** into **546e342440f7ca0274d222179c6eca790d1d6573 on master**.


Patch:
diff --git a/ecsclient/authentication.py b/ecsclient/authentication.py
index 0e8ae84..31fb0c4 100644
--- a/ecsclient/authentication.py
+++ b/ecsclient/authentication.py
@@ -23,6 +23,11 @@ class Authentication(object):
         :param force: If you have multiple sessions running simultaneously this
         forces the termination of all tokens to the current user
         """
+
+        if not self.conn.get_current_token():
+            log.warning('Not logging out since the client has no token set up')
+            return
+
         params = {
             'force': force
         }
diff --git a/ecsclient/baseclient.py b/ecsclient/baseclient.py
index e9dd6cf..485238a 100644
--- a/ecsclient/baseclient.py
+++ b/ecsclient/baseclient.py
@@ -84,11 +84,20 @@ class Client(object):
         """
         return self._token_request.get_new_token()
 
+    def get_current_token(self):
+        """
+        Get the current token in use. None if the client is logged out or not yet logged in
+        """
+        return self._token_request.token
+
     def remove_cached_token(self):
         """
         Remove the cached token file, this is useful if you switch users
         and want to use a different token
         """
+        self.token = None
+        self._token_request.token = None
+
         if os.path.isfile(self.token_path):
             log.debug("Removing cached token '{0}'".format(self.token_path))
             os.remove(self.token_path)


Test patch:
diff --git a/tests/unit/test_authentication.py b/tests/unit/test_authentication.py
index 814018f..82de270 100644
--- a/tests/unit/test_authentication.py
+++ b/tests/unit/test_authentication.py
@@ -1,3 +1,4 @@
+import logging
 import testtools
 from mock import mock
 from requests.auth import _basic_auth_str
@@ -12,6 +13,7 @@ class TestAuthentication(testtools.TestCase):
     LOGOUT_URL = 'http://127.0.0.1:4443/logout'
 
     def setUp(self, *args, **kwargs):
+        # logging.basicConfig(level=logging.DEBUG)
         super(TestAuthentication, self).setUp(*args, **kwargs)
         self.client = Client(username='someone',
                              password='password',
@@ -19,61 +21,105 @@ class TestAuthentication(testtools.TestCase):
                              token_endpoint='http://127.0.0.1:4443/login')
         self.requests_mock = self.useFixture(fixture.Fixture())
 
-    @mock.patch('ecsclient.baseclient.os.path.isfile')
-    def test_get_token_valid_credentials(self, mock_isfile):
-        mock_isfile.return_value = False
-        self.requests_mock.register_uri('GET', self.LOGIN_URL, headers={'X-SDS-AUTH-TOKEN': 'token'})
+    def test_get_token_valid_credentials(self):
+        self.requests_mock.register_uri('GET', self.LOGIN_URL, headers={'X-SDS-AUTH-TOKEN': 'FAKE-TOKEN-123'})
         self.assertIsNone(self.client.token)
-        self.assertIsNone(self.client._token_request._get_existing_token())
 
         token = self.client.get_token()
 
-        self.assertEqual(token, 'token')
-        self.assertEqual(self.client._token_request._get_existing_token(), 'token')
+        self.assertEqual(token, 'FAKE-TOKEN-123')
+        self.assertEqual(self.client._token_request.token, 'FAKE-TOKEN-123')
         self.assertEqual(self.requests_mock.last_request.method, 'GET')
         self.assertEqual(self.requests_mock.last_request.url, self.LOGIN_URL)
         self.assertEqual(self.requests_mock.last_request.headers['authorization'],
                          _basic_auth_str('someone', 'password'))
 
-    @mock.patch('ecsclient.baseclient.os.path.isfile')
-    def test_get_token_invalid_credentials(self, mock_isfile):
-        mock_isfile.return_value = False
+    def test_get_token_invalid_credentials(self):
         self.requests_mock.register_uri('GET', self.LOGIN_URL, status_code=401, text='body')
 
         with super(testtools.TestCase, self).assertRaises(ECSClientException) as error:
             self.client.get_token()
 
         exception = error.exception
+        self.assertIsNone(self.client._token_request.token)
         self.assertEqual(exception.message, 'Invalid username or password')
         self.assertEqual(exception.http_response_content, 'body')
         self.assertEqual(exception.http_status, 401)
-        self.assertIsNone(self.client._token_request._get_existing_token())
         self.assertEqual(self.requests_mock.last_request.method, 'GET')
         self.assertEqual(self.requests_mock.last_request.url, self.LOGIN_URL)
         self.assertEqual(self.requests_mock.last_request.headers['authorization'],
                          _basic_auth_str('someone', 'password'))
 
-    @mock.patch('ecsclient.common.token_request.TokenRequest.get_token')
-    def test_logout(self, mock_get_token):
-        mock_get_token.return_value = 'token'
+    @mock.patch('ecsclient.baseclient.os.remove')
+    @mock.patch('ecsclient.baseclient.os.path.isfile')
+    def test_logout(self, mock_isfile, mock_remove):
+        self.client.token = 'FAKE-TOKEN-123'
+        self.client._token_request.token = 'FAKE-TOKEN-123'
         self.requests_mock.register_uri('GET', self.LOGOUT_URL, text="{'user': 'someone'}")
+        mock_isfile.return_value = True
+        mock_remove.return_value = True
 
         resp = self.client.authentication.logout()
 
         self.assertEqual(resp, "{'user': 'someone'}")
+        self.assertIsNone(self.client.token)
+        self.assertIsNone(self.client._token_request.token)
+        mock_isfile.assert_called_with('/tmp/ecsclient.tkn')
+        mock_remove.assert_called_with('/tmp/ecsclient.tkn')
         self.assertEqual(self.requests_mock.last_request.method, 'GET')
         self.assertEqual(self.requests_mock.last_request.url, self.LOGOUT_URL)
-        self.assertEqual(self.requests_mock.last_request.headers['x-sds-auth-token'], 'token')
+        self.assertEqual(self.requests_mock.last_request.headers['x-sds-auth-token'], 'FAKE-TOKEN-123')
 
-    @mock.patch('ecsclient.common.token_request.TokenRequest.get_token')
-    def test_logout_force(self, mock_get_token):
-        mock_get_token.return_value = 'token'
+    @mock.patch('ecsclient.baseclient.os.remove')
+    @mock.patch('ecsclient.baseclient.os.path.isfile')
+    def test_logout_force(self, mock_isfile, mock_remove):
+        self.client.token = 'FAKE-TOKEN-123'
+        self.client._token_request.token = 'FAKE-TOKEN-123'
         self.requests_mock.register_uri('GET', self.LOGOUT_URL + '?force=True', text="{'user': 'someone'}")
+        mock_isfile.return_value = True
+        mock_remove.return_value = True
 
         resp = self.client.authentication.logout(force=True)
 
         self.assertEqual(resp, "{'user': 'someone'}")
+        self.assertIsNone(self.client.token)
+        self.assertIsNone(self.client._token_request.token)
+        mock_isfile.assert_called_with('/tmp/ecsclient.tkn')
+        mock_remove.assert_called_with('/tmp/ecsclient.tkn')
         self.assertEqual(self.requests_mock.last_request.method, 'GET')
         self.assertEqual(self.requests_mock.last_request.url, self.LOGOUT_URL + '?force=True')
         self.assertEqual(self.requests_mock.last_request.qs['force'], ['true'])
-        self.assertEqual(self.requests_mock.last_request.headers['x-sds-auth-token'], 'token')
+        self.assertEqual(self.requests_mock.last_request.headers['x-sds-auth-token'], 'FAKE-TOKEN-123')
+
+    def test_logout_when_logged_out(self):
+        self.client._token_request.token = 'FAKE-TOKEN-123'
+        self.client._token_request.cache_token = False
+        self.requests_mock.register_uri('GET', self.LOGOUT_URL, text="{'user': 'someone'}")
+        self.requests_mock.register_uri('GET', 'http://127.0.0.1:4443/user/whoami')
+
+        resp = self.client.authentication.logout()
+
+        self.assertEqual(resp, "{'user': 'someone'}")
+
+        resp2 = self.client.authentication.logout()
+
+        self.assertIsNone(resp2)
+
+    def test_logout_and_reconnect(self):
+        self.client.token = 'FAKE-TOKEN-123'
+        self.client._token_request.token = 'FAKE-TOKEN-123'
+        self.client._token_request.cache_token = False
+        self.requests_mock.register_uri('GET', self.LOGOUT_URL, text="{'user': 'someone'}")
+
+        self.client.authentication.logout()
+
+        self.assertIsNone(self.client.token)
+        self.assertIsNone(self.client._token_request.token)
+
+        self.requests_mock.register_uri('GET', self.LOGIN_URL, headers={'X-SDS-AUTH-TOKEN': 'NEW-TOKEN-123'})
+
+        self.client.get('login')
+
+        self.assertEqual(self.client._token_request.token, 'NEW-TOKEN-123')
+
+
diff --git a/tests/unit/test_ecsclient.py b/tests/unit/test_ecsclient.py
index 0d5cf14..d24ad9c 100644
--- a/tests/unit/test_ecsclient.py
+++ b/tests/unit/test_ecsclient.py
@@ -66,6 +66,7 @@ class TestEcsClient(unittest.TestCase):
                    password='password',
                    ecs_endpoint='https://192.168.1.10')
         exception = error.exception.message
+        mock_isfile.assert_called_with('/tmp/ecsclient.tkn')
         self.assertEqual("'token_endpoint' not provided and missing 'token'|'token_path'", str(exception))
 
     def test_client_without_credentials(self):
@@ -112,6 +113,7 @@ class TestEcsClient(unittest.TestCase):
     def test_client_init_with_token_path(self, mock_isfile):
         mock_isfile.return_value = True
         c = Client(version='3',
-                   token_path='/tmp/token.tkn',
+                   token_path='/tmp/mytoken.tkn',
                    ecs_endpoint='https://192.168.1.10')
         self.assertTrue(hasattr(c, 'token_path'))
+        mock_isfile.assert_called_with('/tmp/mytoken.tkn')
diff --git a/tests/unit/test_node.py b/tests/unit/test_node.py
index c6383c8..5c01719 100644
--- a/tests/unit/test_node.py
+++ b/tests/unit/test_node.py
@@ -45,29 +45,33 @@ class TestNode(testtools.TestCase):
         self.response = MagicMock()
         self.requests_mock = self.useFixture(fixture.Fixture())
 
-    @mock.patch('ecsclient.common.token_request.TokenRequest.get_new_token')
-    def test_get_nodes_should_throw_ecsclientexception(self, mock_get_new_token):
+    @mock.patch('ecsclient.common.token_request.TokenRequest.get_token')
+    def test_get_nodes_throw_exception(self, mock_get_token):
         self.requests_mock.register_uri('GET', 'https://127.0.0.1:4443/vdc/nodes',
                                         status_code=http_client.INTERNAL_SERVER_ERROR,
                                         text='Server Error')
-        mock_get_new_token.return_value = 'FAKE-TOKEN-123'
+        mock_get_token.return_value = 'FAKE-TOKEN-123'
 
         with super(testtools.TestCase, self).assertRaises(ECSClientException) as error:
             self.client.node.get_nodes()
 
         exception = error.exception
+        self.assertEqual(self.requests_mock.last_request.method, 'GET')
+        self.assertEqual(self.requests_mock.last_request.url, 'https://127.0.0.1:4443/vdc/nodes')
+        self.assertEqual(self.requests_mock.last_request.headers['x-sds-auth-token'], 'FAKE-TOKEN-123')
         self.assertEqual(exception.http_response_content, 'Server Error')
         self.assertEqual(exception.http_status, http_client.INTERNAL_SERVER_ERROR)
 
-    def test_get_nodes(self):
-        self.response.status_code = http_client.OK
-        self.response.body = self.returned_json
-        self.response.json = MagicMock(return_value=self.returned_json)
-        self.requests = MagicMock(return_value=self.response)
-        self.requests.get.side_effect = [self.response]
+    @mock.patch('ecsclient.common.token_request.TokenRequest.get_token')
+    def test_get_nodes(self, mock_get_token):
+        mock_get_token.return_value = 'FAKE-TOKEN-123'
+        self.requests_mock.register_uri('GET', 'https://127.0.0.1:4443/vdc/nodes',
+                                        status_code=http_client.OK,
+                                        json=self.returned_json)
+
+        response = self.client.node.get_nodes()
 
-        with patch('ecsclient.common.token_request.TokenRequest.'
-                   '_get_existing_token', return_value='FAKE-TOKEN-123'):
-            with patch('ecsclient.baseclient.requests.Session.get', self.requests):
-                returned_json = self.client.node.get_nodes()
-                self.assertEqual(returned_json, self.returned_json)
+        self.assertEqual(self.requests_mock.last_request.method, 'GET')
+        self.assertEqual(self.requests_mock.last_request.url, 'https://127.0.0.1:4443/vdc/nodes')
+        self.assertEqual(self.requests_mock.last_request.headers['x-sds-auth-token'], 'FAKE-TOKEN-123')
+        self.assertEqual(response, self.returned_json)