Ensure all unauthorized reponses return 403

* Clean up authorization vs authentication failures internally
* Remove ambiguous exception.NotAuthorized in favour of exception.Forbidden for authorization failures
* Add exception.NotAuthenticated to make authentication failures more clear
* Fixes bug 956206

Change-Id: I39ce0fcd77d4f06273040a2aa4913a9be911ceab

bin/glance

@@ -62,7 +62,7 @@ def catch_error(action):
             try:
                 ret = func(*args, **kwargs)
                 return SUCCESS if ret is None else ret
-            except (exception.NotAuthorized, exception.Forbidden):
+            except exception.Forbidden:
                 print "Not authorized to make this request. Check "\
                       "your credentials (OS_AUTH_USER, OS_AUTH_KEY, ...)."
                 return FAILURE
@@ -392,7 +392,7 @@ to spell field names correctly."""
         except exception.NotFound:
             print "No image with ID %s was found" % image_id
             return FAILURE
-        except (exception.NotAuthorized, exception.Forbidden):
+        except exception.Forbidden:
             print "You do not have permission to update image %s" % image_id
             return FAILURE
         except Exception, e:
@@ -443,7 +443,7 @@ Deletes an image from Glance"""
     except exception.NotFound:
         print "No image with ID %s was found" % image_id
         return FAILURE
-    except (exception.NotAuthorized, exception.Forbidden):
+    except exception.Forbidden:
         print "You do not have permission to delete image %s" % image_id
         return FAILURE
 

bin/glance-cache-manage

@@ -60,7 +60,7 @@ def catch_error(action):
                 print ("Cache management middleware not enabled on host %s" %
                        options.host)
                 return FAILURE
-            except exception.NotAuthorized:
+            except exception.Forbidden:
                 print "Not authorized to make this request. Check "\
                       "your credentials (OS_AUTH_USER, OS_AUTH_KEY, ...)."
                 return FAILURE

glance/api/cached_images.py

@@ -46,7 +46,7 @@ def _enforce(self, req):
         """Authorize request against 'manage_image_cache' policy"""
         try:
             self.policy.enforce(req.context, 'manage_image_cache', {})
-        except exception.NotAuthorized:
+        except exception.Forbidden:
             raise webob.exc.HTTPForbidden()
 
     def get_cached_images(self, req):

glance/api/policy.py

@@ -84,7 +84,7 @@ def enforce(self, context, action, target):
            :param context: Glance request context
            :param action: String representing the action to be checked
            :param object: Dictionary representing the object of the action.
-           :raises: `glance.common.exception.NotAuthorized`
+           :raises: `glance.common.exception.Forbidden`
            :returns: None
         """
         self.load_rules()
@@ -99,4 +99,4 @@ def enforce(self, context, action, target):
         try:
             policy.enforce(match_list, target, credentials)
         except policy.NotAuthorized:
-            raise exception.NotAuthorized(action=action)
+            raise exception.Forbidden(action=action)

glance/api/v1/controller.py

@@ -44,11 +44,6 @@ def get_image_meta_or_404(self, request, image_id):
             logger.debug(msg)
             raise webob.exc.HTTPNotFound(
                     msg, request=request, content_type='text/plain')
-        except exception.NotAuthorized:
-            msg = _("Unauthorized image access")
-            logger.debug(msg)
-            raise webob.exc.HTTPUnauthorized(msg, request=request,
-                                content_type='text/plain')
         except exception.Forbidden:
             msg = _("Forbidden image access")
             logger.debug(msg)

glance/api/v1/images.py

@@ -29,7 +29,6 @@
                        HTTPConflict,
                        HTTPBadRequest,
                        HTTPForbidden,
-                       HTTPUnauthorized,
                        HTTPRequestEntityTooLarge,
                        HTTPServiceUnavailable,
                       )
@@ -103,8 +102,8 @@ def _enforce(self, req, action):
         """Authorize an action against our policies"""
         try:
             self.policy.enforce(req.context, action, {})
-        except exception.NotAuthorized:
-            raise HTTPUnauthorized()
+        except exception.Forbidden:
+            raise HTTPForbidden()
 
     def index(self, req):
         """
@@ -325,10 +324,6 @@ def _reserve(self, req, image_meta):
             for line in msg.split('\n'):
                 logger.error(line)
             raise HTTPBadRequest(msg, request=req, content_type="text/plain")
-        except exception.NotAuthorized:
-            msg = _("Not authorized to reserve image.")
-            logger.error(msg)
-            raise HTTPUnauthorized(msg, request=req, content_type="text/plain")
         except exception.Forbidden:
             msg = _("Forbidden to reserve image.")
             logger.error(msg)
@@ -429,14 +424,6 @@ def _upload(self, req, image_meta):
             self.notifier.error('image.upload', msg)
             raise HTTPConflict(msg, request=req)
 
-        except exception.NotAuthorized, e:
-            msg = _("Unauthorized upload attempt: %s") % e
-            logger.error(msg)
-            self._safe_kill(req, image_id)
-            self.notifier.error('image.upload', msg)
-            raise HTTPUnauthorized(msg, request=req,
-                                   content_type='text/plain')
-
         except exception.Forbidden, e:
             msg = _("Forbidden upload attempt: %s") % e
             logger.error(msg)
@@ -698,12 +685,6 @@ def update(self, req, id, image_meta, image_data):
                 logger.info(line)
             self.notifier.info('image.update', msg)
             raise HTTPNotFound(msg, request=req, content_type="text/plain")
-        except exception.NotAuthorized, e:
-            msg = ("Unable to update image: %(e)s" % locals())
-            for line in msg.split('\n'):
-                logger.info(line)
-            self.notifier.info('image.update', msg)
-            raise HTTPUnauthorized(msg, request=req, content_type="text/plain")
         except exception.Forbidden, e:
             msg = ("Forbidden to update image: %(e)s" % locals())
             for line in msg.split('\n'):
@@ -760,12 +741,6 @@ def delete(self, req, id):
                 logger.info(line)
             self.notifier.info('image.delete', msg)
             raise HTTPNotFound(msg, request=req, content_type="text/plain")
-        except exception.NotAuthorized, e:
-            msg = ("Unable to delete image: %(e)s" % locals())
-            for line in msg.split('\n'):
-                logger.info(line)
-            self.notifier.info('image.delete', msg)
-            raise HTTPUnauthorized(msg, request=req, content_type="text/plain")
         except exception.Forbidden, e:
             msg = ("Forbidden to delete image: %(e)s" % locals())
             for line in msg.split('\n'):

glance/api/v1/members.py

@@ -52,10 +52,6 @@ def index(self, req, image_id):
             msg = _("Image with identifier %s not found") % image_id
             logger.debug(msg)
             raise webob.exc.HTTPNotFound(msg)
-        except exception.NotAuthorized:
-            msg = _("Unauthorized image access")
-            logger.debug(msg)
-            raise webob.exc.HTTPUnauthorized(msg)
         except exception.Forbidden:
             msg = _("Unauthorized image access")
             logger.debug(msg)
@@ -77,7 +73,7 @@ def delete(self, req, image_id, id):
             msg = "%s" % e
             logger.debug(msg)
             raise webob.exc.HTTPNotFound(msg)
-        except (exception.NotAuthorized, exception.Forbidden), e:
+        except exception.Forbidden, e:
             msg = "%s" % e
             logger.debug(msg)
             raise webob.exc.HTTPNotFound(msg)
@@ -120,7 +116,7 @@ def update(self, req, image_id, id, body=None):
             msg = "%s" % e
             logger.debug(msg)
             raise webob.exc.HTTPNotFound(msg)
-        except (exception.NotAuthorized, exception.Forbidden), e:
+        except exception.Forbidden, e:
             msg = "%s" % e
             logger.debug(msg)
             raise webob.exc.HTTPNotFound(msg)
@@ -152,7 +148,7 @@ def update_all(self, req, image_id, body):
             msg = "%s" % e
             logger.debug(msg)
             raise webob.exc.HTTPNotFound(msg)
-        except (exception.NotAuthorized, exception.Forbidden), e:
+        except exception.Forbidden, e:
             msg = "%s" % e
             logger.debug(msg)
             raise webob.exc.HTTPNotFound(msg)
@@ -178,7 +174,7 @@ def index_shared_images(self, req, id):
             msg = "%s" % e
             logger.debug(msg)
             raise webob.exc.HTTPNotFound(msg)
-        except (exception.NotAuthorized, exception.Forbidden), e:
+        except exception.Forbidden, e:
             msg = "%s" % e
             logger.debug(msg)
             raise webob.exc.HTTPForbidden(msg)

glance/common/auth.py

@@ -178,7 +178,7 @@ def _management_url(self, resp):
         elif resp.status == 400:
             raise exception.AuthBadRequest(url=token_url)
         elif resp.status == 401:
-            raise exception.NotAuthorized()
+            raise exception.NotAuthenticated()
         elif resp.status == 404:
             raise exception.AuthUrlNotFound(url=token_url)
         else:
@@ -246,7 +246,7 @@ def get_endpoint(service_catalog):
         elif resp.status == 400:
             raise exception.AuthBadRequest(url=token_url)
         elif resp.status == 401:
-            raise exception.NotAuthorized()
+            raise exception.NotAuthenticated()
         elif resp.status == 404:
             raise exception.AuthUrlNotFound(url=token_url)
         else:

glance/common/client.py

@@ -48,15 +48,15 @@
 CHUNKSIZE = 65536
 
 
-def handle_unauthorized(func):
+def handle_unauthenticated(func):
     """
     Wrap a function to re-authenticate and retry.
     """
     @functools.wraps(func)
     def wrapped(self, *args, **kwargs):
         try:
             return func(self, *args, **kwargs)
-        except (exception.NotAuthorized, exception.Forbidden):
+        except exception.NotAuthenticated:
             self._authenticate(force_reauth=True)
             return func(self, *args, **kwargs)
     return wrapped
@@ -399,7 +399,7 @@ def _authenticate(self, force_reauth=False):
         if management_url and self.configure_via_auth:
             self.configure_from_url(management_url)
 
-    @handle_unauthorized
+    @handle_unauthenticated
     def do_request(self, method, action, body=None, headers=None,
                    params=None):
         """
@@ -531,7 +531,7 @@ def _retry(res):
             elif status_code in self.REDIRECT_RESPONSE_CODES:
                 raise exception.RedirectException(res.getheader('Location'))
             elif status_code == httplib.UNAUTHORIZED:
-                raise exception.NotAuthorized(res.read())
+                raise exception.NotAuthenticated(res.read())
             elif status_code == httplib.FORBIDDEN:
                 raise exception.Forbidden(res.read())
             elif status_code == httplib.NOT_FOUND:

glance/common/context.py

@@ -89,7 +89,7 @@ def process_request(self, req):
            to determine permissions.
 
         2. An X-Auth-Token was passed in, but the Identity-Status is not
-           confirmed. For now, just raising a NotAuthorized exception.
+           confirmed. For now, just raising a NotAuthenticated exception.
 
         3. X-Auth-Token is omitted. If we were using Keystone, then the
            tokenauth middleware would have rejected the request, so we must be
@@ -108,8 +108,8 @@ def process_request(self, req):
             else:
                 # 2. Indentity-Status not confirmed
                 # FIXME(sirp): not sure what the correct behavior in this case
-                # is; just raising NotAuthorized for now
-                raise exception.NotAuthorized()
+                # is; just raising NotAuthenticated for now
+                raise exception.NotAuthenticated()
         else:
             # 3. Auth-token is ommited, assume NoAuth
             user = None

glance/common/exception.py

@@ -109,15 +109,20 @@ class AuthorizationFailure(GlanceException):
     message = _("Authorization failed.")
 
 
-class NotAuthorized(GlanceException):
-    message = _("You are not authorized to complete this action.")
+class NotAuthenticated(GlanceException):
+    message = _("You are not authenticated.")
 
 
 class Forbidden(GlanceException):
-    message = _("That action is forbidden.")
+    message = _("You are not authorized to complete this action.")
+
+
+class ForbiddenPublicImage(Forbidden):
+    message = _("You are not authorized to complete this action.")
 
 
-class NotAuthorizedPublicImage(NotAuthorized):
+#NOTE(bcwaldon): here for backwards-compatability, need to deprecate.
+class NotAuthorized(Forbidden):
     message = _("You are not authorized to complete this action.")
 
 

glance/registry/api/v1/images.py

@@ -274,7 +274,7 @@ def show(self, req, id):
             image = db_api.image_get(req.context, id)
         except exception.NotFound:
             raise exc.HTTPNotFound()
-        except exception.NotAuthorized:
+        except exception.Forbidden:
             # If it's private and doesn't belong to them, don't let on
             # that it exists
             msg = _("Access by %(user)s to image %(id)s "
@@ -300,15 +300,15 @@ def delete(self, req, id):
         try:
             db_api.image_destroy(req.context, id)
 
-        except exception.NotAuthorizedPublicImage:
+        except exception.ForbiddenPublicImage:
             # If it's private and doesn't belong to them, don't let on
             # that it exists
             msg = _("Access by %(user)s to delete public image %(id)s denied")
             args = {'user': req.context.user, 'id': id}
             logger.info(msg % args)
             raise exc.HTTPForbidden()
 
-        except exception.NotAuthorized:
+        except exception.Forbidden:
             msg = _("Access by %(user)s to delete private image %(id)s denied")
             args = {'user': req.context.user, 'id': id}
             logger.info(msg % args)
@@ -397,12 +397,12 @@ def update(self, req, id, body):
             raise exc.HTTPNotFound(body='Image not found',
                                request=req,
                                content_type='text/plain')
-        except exception.NotAuthorizedPublicImage:
+        except exception.ForbiddenPublicImage:
             msg = _("Access by %(user)s to update public image %(id)s denied")
             logger.info(msg % {'user': req.context.user, 'id': id})
             raise exc.HTTPForbidden()
 
-        except exception.NotAuthorized:
+        except exception.Forbidden:
             # If it's private and doesn't belong to them, don't let on
             # that it exists
             msg = _("Access by %(user)s to update private image %(id)s denied")

glance/registry/api/v1/members.py

@@ -41,7 +41,7 @@ def index(self, req, image_id):
             image = db_api.image_get(req.context, image_id)
         except exception.NotFound:
             raise webob.exc.HTTPNotFound()
-        except exception.NotAuthorized:
+        except exception.Forbidden:
             # If it's private and doesn't belong to them, don't let on
             # that it exists
             msg = _("Access by %(user)s to image %(id)s "
@@ -75,7 +75,7 @@ def update_all(self, req, image_id, body):
             image = db_api.image_get(req.context, image_id, session=session)
         except exception.NotFound:
             raise webob.exc.HTTPNotFound()
-        except exception.NotAuthorized:
+        except exception.Forbidden:
             # If it's private and doesn't belong to them, don't let on
             # that it exists
             msg = _("Access by %(user)s to image %(id)s "
@@ -177,7 +177,7 @@ def update(self, req, image_id, id, body=None):
             image = db_api.image_get(req.context, image_id)
         except exception.NotFound:
             raise webob.exc.HTTPNotFound()
-        except exception.NotAuthorized:
+        except exception.Forbidden:
             # If it's private and doesn't belong to them, don't let on
             # that it exists
             msg = _("Access by %(user)s to image %(id)s "
@@ -232,7 +232,7 @@ def delete(self, req, image_id, id):
             image = db_api.image_get(req.context, image_id)
         except exception.NotFound:
             raise webob.exc.HTTPNotFound()
-        except exception.NotAuthorized:
+        except exception.Forbidden:
             # If it's private and doesn't belong to them, don't let on
             # that it exists
             msg = _("Access by %(user)s to image %(id)s "

glance/registry/db/api.py

@@ -136,9 +136,9 @@ def check_mutate_authorization(context, image_ref):
         logger.info(_("Attempted to modify image user did not own."))
         msg = _("You do not own this image")
         if image_ref.is_public:
-            exc_class = exception.NotAuthorizedPublicImage
+            exc_class = exception.ForbiddenPublicImage
         else:
-            exc_class = exception.NotAuthorized
+            exc_class = exception.Forbidden
 
         raise exc_class(msg)
 
@@ -207,7 +207,7 @@ def image_get(context, image_id, session=None, force_show_deleted=False):
 
     # Make sure they can look at it
     if not context.is_image_visible(image):
-        raise exception.NotAuthorized("Image not visible to you")
+        raise exception.Forbidden("Image not visible to you")
 
     return image
 
@@ -618,7 +618,7 @@ def image_member_get(context, member_id, session=None):
 
     # Make sure they can look at it
     if not context.is_image_visible(member.image):
-        raise exception.NotAuthorized("Image not visible to you")
+        raise exception.Forbidden("Image not visible to you")
 
     return member