Merge "Expose authn/z failure info to API in debug mode"

2 files changed, 102 insertions, 10 deletions

diff --git a/keystone/exception.py b/keystone/exception.py
index 4b9721cf..96caf322 100644
--- a/keystone/exception.py
+++ b/keystone/exception.py
@@ -15,6 +15,13 @@
 # under the License.
 import re
 
+from keystone.common import logging
+from keystone import config
+
+
+CONF = config.CONF
+LOG = logging.getLogger(__name__)
+
 
 class Error(StandardError):
     """Base error class.
@@ -27,9 +34,24 @@ class Error(StandardError):
 
     def __init__(self, message=None, **kwargs):
         """Use the doc string as the error message by default."""
-        message = message or self.__doc__ % kwargs
+
+        try:
+            message = self._build_message(message, **kwargs)
+        except KeyError:
+            # if you see this warning in your logs, please raise a bug report
+            LOG.warning('missing expected exception kwargs (programmer error)')
+            message = self.__doc__
+
         super(Error, self).__init__(message)
 
+    def _build_message(self, message, **kwargs):
+        """Builds and returns an exception message.
+
+        :raises: KeyError given insufficient kwargs
+
+        """
+        return message or self.__doc__ % kwargs
+
     def __str__(self):
         """Cleans up line breaks and indentation from doc strings."""
         string = super(Error, self).__str__()
@@ -51,13 +73,24 @@ class ValidationError(Error):
     title = 'Bad Request'
 
 
-class Unauthorized(Error):
+class SecurityError(Error):
+    """Avoids exposing details of security failures, unless in debug mode."""
+
+    def _build_message(self, message, **kwargs):
+        """Only returns detailed messages in debug mode."""
+        if CONF.debug:
+            return message or self.__doc__ % kwargs
+        else:
+            return self.__doc__ % kwargs
+
+
+class Unauthorized(SecurityError):
     """The request you have made requires authentication."""
     code = 401
     title = 'Not Authorized'
 
 
-class Forbidden(Error):
+class Forbidden(SecurityError):
     """You are not authorized to perform the requested action."""
     code = 403
     title = 'Not Authorized'
diff --git a/tests/test_exception.py b/tests/test_exception.py
index 1cbc5810..eb67098b 100644
--- a/tests/test_exception.py
+++ b/tests/test_exception.py
@@ -16,12 +16,16 @@
 
 import uuid
 
+from keystone import config
 from keystone.common import wsgi
 from keystone import exception
 from keystone.openstack.common import jsonutils
 from keystone import test
 
 
+CONF = config.CONF
+
+
 class ExceptionTestCase(test.TestCase):
     def setUp(self):
         pass
@@ -54,7 +58,7 @@ class ExceptionTestCase(test.TestCase):
 
         """
         for cls in [x for x in exception.__dict__.values() if callable(x)]:
-            if cls is not exception.Error:
+            if cls is not exception.Error and isinstance(cls, exception.Error):
                 self.assertValidJsonRendering(cls(message='Overriden.'))
 
     def test_validation_error(self):
@@ -65,14 +69,69 @@ class ExceptionTestCase(test.TestCase):
         self.assertIn(target, str(e))
         self.assertIn(attribute, str(e))
 
-    def test_forbidden_action(self):
-        action = uuid.uuid4().hex
-        e = exception.ForbiddenAction(action=action)
-        self.assertValidJsonRendering(e)
-        self.assertIn(action, str(e))
-
     def test_not_found(self):
         target = uuid.uuid4().hex
         e = exception.NotFound(target=target)
         self.assertValidJsonRendering(e)
         self.assertIn(target, str(e))
+
+
+class SecurityErrorTestCase(ExceptionTestCase):
+    """Tests whether security-related info is exposed to the API user."""
+    def test_unauthorized_exposure(self):
+        CONF.debug = False
+
+        risky_info = uuid.uuid4().hex
+        e = exception.Unauthorized(message=risky_info)
+        self.assertValidJsonRendering(e)
+        self.assertNotIn(risky_info, str(e))
+
+    def test_unauthroized_exposure_in_debug(self):
+        CONF.debug = True
+
+        risky_info = uuid.uuid4().hex
+        e = exception.Unauthorized(message=risky_info)
+        self.assertValidJsonRendering(e)
+        self.assertIn(risky_info, str(e))
+
+    def test_foribdden_exposure(self):
+        CONF.debug = False
+
+        risky_info = uuid.uuid4().hex
+        e = exception.Forbidden(message=risky_info)
+        self.assertValidJsonRendering(e)
+        self.assertNotIn(risky_info, str(e))
+
+    def test_forbidden_exposure_in_Debug(self):
+        CONF.debug = True
+
+        risky_info = uuid.uuid4().hex
+        e = exception.Forbidden(message=risky_info)
+        self.assertValidJsonRendering(e)
+        self.assertIn(risky_info, str(e))
+
+    def test_forbidden_action_exposure(self):
+        CONF.debug = False
+
+        risky_info = uuid.uuid4().hex
+
+        e = exception.ForbiddenAction(message=risky_info)
+        self.assertValidJsonRendering(e)
+        self.assertNotIn(risky_info, str(e))
+
+        e = exception.ForbiddenAction(action=risky_info)
+        self.assertValidJsonRendering(e)
+        self.assertIn(risky_info, str(e))
+
+    def test_forbidden_action_exposure_in_debug(self):
+        CONF.debug = True
+
+        risky_info = uuid.uuid4().hex
+
+        e = exception.ForbiddenAction(message=risky_info)
+        self.assertValidJsonRendering(e)
+        self.assertIn(risky_info, str(e))
+
+        e = exception.ForbiddenAction(action=risky_info)
+        self.assertValidJsonRendering(e)
+        self.assertIn(risky_info, str(e))