From 900a17ccc0f6bc243c5d5479695e0b60a5b84144 Mon Sep 17 00:00:00 2001
From: asmita singh <asmita.singh@nttdata.com>
Date: Tue, 27 Nov 2018 11:03:58 +0000
Subject: [PATCH] Return 401 error for invalid token instead of 204

For `/` and `/versions` API requests, if an invalid token is passed,
Blazar returns a 204 status code.

This patch fixes this issue by returning a 401 error for invalid tokens.
If the token is valid, it will return information about all supported
versions.

Change-Id: If4fda2eaebd0dd24906f35eeb64d0af65d0e5ee8
Closes-Bug: #1790625
Closes-Bug: #1803944
---
 blazar/api/app.py                         | 20 ++++++++++++++++---
 blazar/tests/api/test_version_selector.py | 24 ++++++++++++++++++++++-
 2 files changed, 40 insertions(+), 4 deletions(-)

diff --git a/blazar/api/app.py b/blazar/api/app.py
index 77ddb095..393317e7 100644
--- a/blazar/api/app.py
+++ b/blazar/api/app.py
@@ -14,6 +14,7 @@
 # limitations under the License.
 
 from oslo_serialization import jsonutils
+import six
 
 from blazar.api.v1 import app as v1_app
 from blazar.api.v2 import app as v2_app
@@ -31,8 +32,13 @@ class VersionSelectorApplication(object):
     def _append_versions_from_app(self, versions, app, environ):
         tmp_versions = app(environ, self.internal_start_response)
         if self._status.startswith("300"):
-            tmp_versions = jsonutils.loads(tmp_versions.pop())
+            # In case of v1, app returns ClosingIterator generator object,
+            # whereas in case of v2, it returns list.
+            # So convert it to iterator to get the versions.
+            app_iter = iter(tmp_versions)
+            tmp_versions = jsonutils.loads(six.next(app_iter))
             versions['versions'].extend(tmp_versions['versions'])
+        return tmp_versions
 
     def internal_start_response(self, status, response_headers, exc_info=None):
         self._status = status
@@ -44,8 +50,16 @@ class VersionSelectorApplication(object):
 
         if environ['PATH_INFO'] == '/' or environ['PATH_INFO'] == '/versions':
             versions = {'versions': []}
-            self._append_versions_from_app(versions, self.v1, environ)
-            self._append_versions_from_app(versions, self.v2, environ)
+            tmp_versions = self._append_versions_from_app(versions, self.v1,
+                                                          environ)
+            # Both v1 and v2 apps run auth_token middleware. So simply
+            # validate token for v1. If it fails no need to call v2 app.
+            if self._status.startswith("401"):
+                start_response(self._status,
+                               [("Content-Type", "application/json")])
+                return tmp_versions
+            self._append_versions_from_app(versions, self.v2,
+                                           environ)
             if len(versions['versions']):
                 start_response("300 Multiple Choices",
                                [("Content-Type", "application/json")])
diff --git a/blazar/tests/api/test_version_selector.py b/blazar/tests/api/test_version_selector.py
index 79ccf21a..0af9ddbe 100644
--- a/blazar/tests/api/test_version_selector.py
+++ b/blazar/tests/api/test_version_selector.py
@@ -35,7 +35,13 @@ class FakeWSGIApp(object):
 
     def __call__(self, environ, start_response):
         start_response(self.status_code, [])
-        return [jsonutils.dump_as_bytes(self.versions)]
+        if self.status_code == "300 Multiple Choices":
+            return [jsonutils.dump_as_bytes(self.versions)]
+        elif self.status_code == "401 Unauthorized":
+            return ['{"error": '
+                    '{"message": "The request you have '
+                    'made requires authentication.",'
+                    '"code": 401, "title": "Unauthorized"}}']
 
 
 class TestVersionDiscovery(tests.TestCase):
@@ -98,6 +104,22 @@ class TestVersionDiscovery(tests.TestCase):
         self.assertEqual([], versions_raw)
         self.start_response_mock.assert_called_with("204 No Content", [])
 
+    def test_unauthorized_token(self):
+        self.v1_make_app.return_value = FakeWSGIApp(1, "401 Unauthorized")
+        version_selector = api.VersionSelectorApplication()
+        environ = {'PATH_INFO': self.path}
+
+        versions_raw = version_selector(environ, self.start_response)
+        self.assertEqual(['{"error": '
+                          '{"message": "The request you have '
+                          'made requires authentication.",'
+                          '"code": 401, "title": "Unauthorized"}}'],
+                         versions_raw)
+
+        self.start_response_mock.assert_called_with(
+            "401 Unauthorized",
+            [("Content-Type", "application/json")])
+
 
 class TestVersionSelectorApplication(tests.TestCase):
     def start_response(self, status, response_headers):