From 8c976721cac606ec4d841ab225a2b59e1452f6b3 Mon Sep 17 00:00:00 2001 From: Lokesh S Date: Mon, 18 Jul 2016 10:19:37 +0000 Subject: [PATCH] Raise Not Authorized Exception when connection request is forbidden Currently it logs IncompatibleOneViewAPIVersion which confuses users to assume it is version issue. It can be wrong request call. Change-Id: I0bd69a9173d7318bb9e06a38ba54c043f9198b30 Closes-Bug: 1603928 --- oneview_client/client.py | 18 ++-- .../tests/unit/test_oneview_client.py | 82 ++++++++++++++++++- 2 files changed, 90 insertions(+), 10 deletions(-) diff --git a/oneview_client/client.py b/oneview_client/client.py index 21360f0..0f1edd3 100644 --- a/oneview_client/client.py +++ b/oneview_client/client.py @@ -129,13 +129,12 @@ class BaseClient(object): verify_ssl = self._get_verify_connection_option() - try: - versions = requests.get( - url, headers=headers, verify=verify_ssl - ).json() - return versions - except requests.RequestException as e: - raise exceptions.OneViewConnectionError(e.message) + response = requests.get( + url, headers=headers, verify=verify_ssl + ) + _check_request_status(response) + versions = response.json() + return versions # --- Requests --- def _prepare_and_do_request( @@ -655,8 +654,9 @@ def _check_request_status(response): repeat = False status = response.status_code - if status == 401: - raise exceptions.OneViewNotAuthorizedException() + if status in (401, 403): + error_code = response.json().get('errorCode') + raise exceptions.OneViewNotAuthorizedException(error_code) elif status == 404: raise exceptions.OneViewResourceNotFoundError() elif status in (408, 409,): diff --git a/oneview_client/tests/unit/test_oneview_client.py b/oneview_client/tests/unit/test_oneview_client.py index bf966a8..aa0e1d3 100644 --- a/oneview_client/tests/unit/test_oneview_client.py +++ b/oneview_client/tests/unit/test_oneview_client.py @@ -916,7 +916,8 @@ class OneViewClientTestCase(unittest.TestCase): mock_get_oneview_version.assert_called_once_with() @mock.patch.object(client.Client, 'get_oneview_version') - def test_verify_oneview_version_fail(self, mock_get_oneview_version): + def test_verify_oneview_version_under_supported(self, + mock_get_oneview_version): mock_get_oneview_version.return_value = { 'minimumVersion': 120, 'currentVersion': 120 @@ -926,6 +927,85 @@ class OneViewClientTestCase(unittest.TestCase): self.oneview_client.verify_oneview_version ) + @mock.patch.object(client.Client, 'get_oneview_version') + def test_verify_oneview_version_within_supported( + self, + mock_get_oneview_version + ): + mock_get_oneview_version.return_value = { + 'minimumVersion': 120, + 'currentVersion': 300 + } + self.oneview_client.verify_oneview_version() + mock_get_oneview_version.assert_called_once_with() + + @mock.patch.object(client.Client, 'get_oneview_version') + def test_verify_oneview_version_within_supported2( + self, + mock_get_oneview_version + ): + mock_get_oneview_version.return_value = { + 'minimumVersion': 200, + 'currentVersion': 300 + } + self.oneview_client.verify_oneview_version() + mock_get_oneview_version.assert_called_once_with() + + @mock.patch.object(client.Client, 'get_oneview_version') + def test_verify_oneview_version_over_supported(self, + mock_get_oneview_version): + mock_get_oneview_version.return_value = { + 'minimumVersion': 300, + 'currentVersion': 400 + } + self.assertRaises( + exceptions.IncompatibleOneViewAPIVersion, + self.oneview_client.verify_oneview_version + ) + + @mock.patch.object(client.Client, 'get_oneview_version') + def test_verify_oneview_version_not_authorized(self, + mock_get_oneview_version): + mock_get_oneview_version.side_effect = [ + exceptions.OneViewNotAuthorizedException + ] + + self.assertRaises( + exceptions.OneViewNotAuthorizedException, + self.oneview_client.verify_oneview_version + ) + + @mock.patch.object(requests, 'get') + def test_get_oneview_version_forbidden_with_json(self, mock_get): + response = mock_get.return_value + response.status_code = http_client.FORBIDDEN + response.json.return_value = { + 'errorSource': None, + 'recommendedActions': [ + 'Check the request URI, then resend the request.' + ], + 'nestedErrors': [], + 'errorCode': 'GENERIC_HTTP_403', + 'details': 'You are not allowed to access the requested resource.', + 'message': 'Forbidden', + 'data': {} + } + mock_get.return_value = response + self.assertRaises( + exceptions.OneViewNotAuthorizedException, + self.oneview_client.get_oneview_version + ) + + @mock.patch.object(requests, 'get') + def test_get_oneview_version_forbidden_without_json(self, mock_get): + response = mock_get.return_value + response.status_code = http_client.FORBIDDEN + mock_get.return_value = response + self.assertRaises( + exceptions.OneViewNotAuthorizedException, + self.oneview_client.get_oneview_version + ) + @mock.patch.object(client.Client, 'get_server_profile_from_hardware') def test_is_mac_compatible_with_server_profile( self, mock_get_server_profile_from_hardware