Merge "only make token invalid when it really is" into stable/liberty2.3.1

3 files changed, 52 insertions, 19 deletions

diff --git a/keystonemiddleware/auth_token/__init__.py b/keystonemiddleware/auth_token/__init__.py
index d09f91b..a8afeb2 100644
--- a/keystonemiddleware/auth_token/__init__.py
+++ b/keystonemiddleware/auth_token/__init__.py
@@ -206,6 +206,7 @@ object is stored.
 
 """
 
+import binascii
 import datetime
 import logging
 
@@ -849,19 +850,19 @@ class AuthProtocol(_BaseAuthProtocol):
 
                 self._token_cache.store(token_hashes[0], data)
 
-        except (exceptions.ConnectionRefused, exceptions.RequestTimeout):
-            self.log.debug('Token validation failure.', exc_info=True)
-            self.log.warning(_LW('Authorization failed for token'))
-            raise exc.InvalidToken(_('Token authorization failed'))
-        except exc.ServiceError as e:
-            self.log.critical(_LC('Unable to obtain admin token: %s'), e)
+        except (exceptions.ConnectionRefused, exceptions.RequestTimeout,
+                exc.RevocationListError, exc.ServiceError) as e:
+            self.log.critical(_LC('Unable to validate token: %s'), e)
             raise webob.exc.HTTPServiceUnavailable()
-        except Exception:
+        except exc.InvalidToken:
             self.log.debug('Token validation failure.', exc_info=True)
             if token_hashes:
                 self._token_cache.store_invalid(token_hashes[0])
             self.log.warning(_LW('Authorization failed for token'))
-            raise exc.InvalidToken(_('Token authorization failed'))
+            raise
+        except Exception:
+            self.log.critical(_LC('Unable to validate token'), exc_info=True)
+            raise webob.exc.HTTPInternalServerError()
 
         return data
 
@@ -906,9 +907,10 @@ class AuthProtocol(_BaseAuthProtocol):
                 return cms.cms_verify(data, signing_cert_path,
                                       signing_ca_path,
                                       inform=inform).decode('utf-8')
-            except cms.subprocess.CalledProcessError as err:
+            except (exceptions.CMSError,
+                    cms.subprocess.CalledProcessError) as err:
                 self.log.warning(_LW('Verify error: %s'), err)
-                raise
+                raise exc.InvalidToken(_('Token authorization failed'))
 
         try:
             return verify()
@@ -940,7 +942,8 @@ class AuthProtocol(_BaseAuthProtocol):
             verified = self._cms_verify(uncompressed, inform=cms.PKIZ_CMS_FORM)
             return verified
         # TypeError If the signed_text is not zlib compressed
-        except TypeError:
+        # binascii.Error if signed_text has incorrect base64 padding (py34)
+        except (TypeError, binascii.Error):
             raise exc.InvalidToken(signed_text)
 
     def _fetch_signing_cert(self):
diff --git a/keystonemiddleware/auth_token/_identity.py b/keystonemiddleware/auth_token/_identity.py
index 98be3b2..6fbeac2 100644
--- a/keystonemiddleware/auth_token/_identity.py
+++ b/keystonemiddleware/auth_token/_identity.py
@@ -212,25 +212,28 @@ class IdentityServer(object):
         try:
             auth_ref = self._request_strategy.verify_token(user_token)
         except exceptions.NotFound as e:
-            self._LOG.warn(_LW('Authorization failed for token'))
-            self._LOG.warn(_LW('Identity response: %s'), e.response.text)
+            self._LOG.warning(_LW('Authorization failed for token'))
+            self._LOG.warning(_LW('Identity response: %s'), e.response.text)
+            raise exc.InvalidToken(_('Token authorization failed'))
         except exceptions.Unauthorized as e:
             self._LOG.info(_LI('Identity server rejected authorization'))
-            self._LOG.warn(_LW('Identity response: %s'), e.response.text)
+            self._LOG.warning(_LW('Identity response: %s'), e.response.text)
             if retry:
                 self._LOG.info(_LI('Retrying validation'))
                 return self.verify_token(user_token, False)
+            msg = _('Identity server rejected authorization necessary to '
+                    'fetch token data')
+            raise exc.ServiceError(msg)
         except exceptions.HttpError as e:
             self._LOG.error(
                 _LE('Bad response code while validating token: %s'),
                 e.http_status)
-            self._LOG.warn(_LW('Identity response: %s'), e.response.text)
+            self._LOG.warning(_LW('Identity response: %s'), e.response.text)
+            msg = _('Failed to fetch token data from identity server')
+            raise exc.ServiceError(msg)
         else:
             return auth_ref
 
-        msg = _('Failed to fetch token data from identity server')
-        raise exc.InvalidToken(msg)
-
     def fetch_revocation_list(self):
         try:
             data = self._request_strategy.fetch_revocation_list()
diff --git a/keystonemiddleware/tests/unit/auth_token/test_auth_token_middleware.py b/keystonemiddleware/tests/unit/auth_token/test_auth_token_middleware.py
index bb572aa..3fdd4a9 100644
--- a/keystonemiddleware/tests/unit/auth_token/test_auth_token_middleware.py
+++ b/keystonemiddleware/tests/unit/auth_token/test_auth_token_middleware.py
@@ -773,6 +773,33 @@ class CommonAuthTokenMiddlewareTest(object):
         resp = self.call_middleware(headers={'X-Auth-Token': token})
         self.assertEqual(401, resp.status_int)
 
+    def test_cached_revoked_error(self):
+        # When the token is cached and revocation list retrieval fails,
+        # 503 is returned
+        token = self.token_dict['uuid_token_default']
+        self.middleware._check_revocations_for_cached = True
+
+        # Token should be cached as ok after this.
+        resp = self.call_middleware(headers={'X-Auth-Token': token})
+        self.assertEqual(200, resp.status_int)
+
+        # Cause the revocation list to be fetched again next time so we can
+        # test the case where that retrieval fails
+        self.middleware._revocations._fetched_time = datetime.datetime.min
+        with mock.patch.object(self.middleware._revocations, '_fetch',
+                               side_effect=exc.RevocationListError):
+            resp = self.call_middleware(headers={'X-Auth-Token': token})
+            self.assertEqual(503, resp.status_int)
+
+    def test_unexpected_exception_in_validate_offline(self):
+        # When an unexpected exception is hit during _validate_offline,
+        # 500 is returned
+        token = self.token_dict['uuid_token_default']
+        with mock.patch.object(self.middleware, '_validate_offline',
+                               side_effect=Exception):
+            resp = self.call_middleware(headers={'X-Auth-Token': token})
+            self.assertEqual(500, resp.status_int)
+
     def test_cached_revoked_uuid(self):
         # When the UUID token is cached and revoked, 401 is returned.
         self._test_cache_revoked(self.token_dict['uuid_token_default'])
@@ -2085,7 +2112,7 @@ class CommonCompositeAuthTests(object):
         }
         self.update_expected_env(expected_env)
 
-        token = 'invalid-user-token'
+        token = 'invalid-token'
         service_token = 'invalid-service-token'
         resp = self.call_middleware(headers={'X-Auth-Token': token,
                                              'X-Service-Token': service_token})