Review use of Http 417 status code (#48)

DataONEorg · Jul 8, 2020 · ede0e88 · ede0e88
1 parent fd9bb77
commit ede0e88
Show file tree

Hide file tree

Showing 5 changed files with 35 additions and 21 deletions.
diff --git a/src/main/java/org/dataone/bookkeeper/resources/CustomersResource.java b/src/main/java/org/dataone/bookkeeper/resources/CustomersResource.java
@@ -169,7 +169,7 @@ public Customer create(@Context SecurityContext context,
             customer = customerStore.getCustomer(id);
         } catch (Exception e) {
             String message = "Couldn't insert the customer: " + e.getMessage();
-            throw new WebApplicationException(message, Response.Status.EXPECTATION_FAILED);
+            throw new WebApplicationException(message, Response.Status.INTERNAL_SERVER_ERROR);
         }
         return customer;
     }
@@ -252,7 +252,7 @@ public Customer update(@Context SecurityContext context,
             Integer id = customerStore.update(customer);
         } catch (Exception e) {
             String message = "Couldn't update the customer: " + e.getMessage();
-            throw new WebApplicationException(message, Response.Status.EXPECTATION_FAILED);
+            throw new WebApplicationException(message, Response.Status.INTERNAL_SERVER_ERROR);
         }
         return customer;
     }

diff --git a/src/main/java/org/dataone/bookkeeper/resources/OrdersResource.java b/src/main/java/org/dataone/bookkeeper/resources/OrdersResource.java
@@ -163,7 +163,7 @@ public OrderList listOrders(
             }
         } catch (Exception e) {
             String message = "Couldn't list orders: " + e.getMessage();
-            throw new WebApplicationException(message, Response.Status.EXPECTATION_FAILED);
+            throw new WebApplicationException(message, Response.Status.INTERNAL_SERVER_ERROR);
         }
 
         // TODO: Incorporate paging params - new OrderList(start, count, total, orders)
@@ -216,7 +216,7 @@ public Order create(@Context SecurityContext context,
             order = orderStore.getOrder(id);
         } catch (Exception e) {
             String message = "Couldn't insert the order: " + e.getMessage();
-            throw new WebApplicationException(message, Response.Status.EXPECTATION_FAILED);
+            throw new WebApplicationException(message, Response.Status.INTERNAL_SERVER_ERROR);
         }
         return order;
     }
@@ -284,7 +284,7 @@ public Order update(@Context SecurityContext context,
         if ( ! isAdmin ) {
             if ( ! existing.getCustomer().equals(caller.getId()) ) {
                 throw new WebApplicationException(
-                    "Customer doesn't have access to this order.", Response.Status.EXPECTATION_FAILED
+                    "Customer doesn't have access to this order.", Response.Status.UNAUTHORIZED
                 );
             }
         }
@@ -322,7 +322,7 @@ public Order update(@Context SecurityContext context,
             orderStore.update(order);
         } catch (Exception e) {
             String message = "Couldn't update the order: " + e.getMessage();
-            throw new WebApplicationException(message, Response.Status.EXPECTATION_FAILED);
+            throw new WebApplicationException(message, Response.Status.INTERNAL_SERVER_ERROR);
         }
         return order;
     }
@@ -357,7 +357,7 @@ public Order pay(@Context SecurityContext context,
             if ( ! isAdmin ) {
                 if ( ! order.getCustomer().equals(caller.getId()) ) {
                     throw new WebApplicationException(
-                        "Customer doesn't have access to this order.", Response.Status.EXPECTATION_FAILED
+                        "Customer doesn't have access to this order.", Response.Status.UNAUTHORIZED
                     );
                 }
             }
@@ -431,7 +431,7 @@ public Order pay(@Context SecurityContext context,
 
         } catch (Exception e) {
             String message = "Couldn't pay the order: " + e.getMessage();
-            throw new WebApplicationException(message, Response.Status.EXPECTATION_FAILED);
+            throw new WebApplicationException(message, Response.Status.INTERNAL_SERVER_ERROR);
         }
         return order;
     }

diff --git a/src/main/java/org/dataone/bookkeeper/resources/ProductsResource.java b/src/main/java/org/dataone/bookkeeper/resources/ProductsResource.java
@@ -140,7 +140,7 @@ public Product create(
             product = productStore.getProduct(id);
         } catch (Exception e) {
             String message = "Couldn't insert the product: " + e.getMessage();
-            throw new WebApplicationException(message, Response.Status.EXPECTATION_FAILED);
+            throw new WebApplicationException(message, Response.Status.INTERNAL_SERVER_ERROR);
         }
         return product;
     }
@@ -197,7 +197,7 @@ public Product update(
             productStore.update(product);
         } catch (Exception e) {
             String message = "Couldn't update the product: " + e.getMessage();
-            throw new WebApplicationException(message, Response.Status.EXPECTATION_FAILED);
+            throw new WebApplicationException(message, Response.Status.INTERNAL_SERVER_ERROR);
         }
         return product;
     }

diff --git a/src/main/java/org/dataone/bookkeeper/resources/QuotasResource.java b/src/main/java/org/dataone/bookkeeper/resources/QuotasResource.java
@@ -136,7 +136,7 @@ public QuotaList listQuotas(
 
                     /* Caller is not admin and is not associated with any of the specified subscribers. */
                     if (subjects.size() == 0) {
-                        throw new WebApplicationException(caller.getSubject() + " requested subscribers don't exist or requestor doesn't have priviledge to view them.", Response.Status.FORBIDDEN);
+                        throw new WebApplicationException("The requested subscribers don't exist or requestor doesn't have priviledge to view them.", Response.Status.FORBIDDEN);
                     }
                 } else {
                     /* Admin caller, so can see quotas for all requested subscribers */
@@ -174,11 +174,18 @@ public QuotaList listQuotas(
                 message += " " + e.getCause();
             }
 
-            throw new WebApplicationException(message, Response.Status.EXPECTATION_FAILED);
+            throw new WebApplicationException(message, Response.Status.INTERNAL_SERVER_ERROR);
         }
 
         if (quotas == null || quotas.size() == 0) {
-            throw new WebApplicationException("The requested quotas were not found or requestor does not have privilege to retrieve them.", Response.Status.NOT_FOUND);
+            if (! isAdmin || isProxy) {
+                // If not an admin user or is a proxy user, we have no way to determine if they didn't have enough
+                // privilege or if the quotas don't exist.
+                throw new WebApplicationException("The requested quotas were not found or requestor does not have privilege to view them.", Response.Status.NOT_FOUND);
+            } else {
+                // Admin user can see any existing quota, so can't be a priv issue.
+                throw new WebApplicationException("The requested quotas were not found.", Response.Status.NOT_FOUND);
+            }
         }
 
         // TODO: Incorporate paging params - new QuotaList(start, count, total, quotas)
@@ -209,7 +216,7 @@ public Quota create(
             quota = quotaStore.getQuota(id);
         } catch (Exception e) {
             String message = "Couldn't insert the quota: " + e.getMessage();
-            throw new WebApplicationException(message, Response.Status.EXPECTATION_FAILED);
+            throw new WebApplicationException(message, Response.Status.INTERNAL_SERVER_ERROR);
         }
         return quota;
     }
@@ -283,7 +290,7 @@ public Quota update(
             updatedQuota = quotaStore.update(quota);
         } catch (Exception e) {
             String message = "Couldn't update the quota: " + e.getMessage();
-            throw new WebApplicationException(message, Response.Status.EXPECTATION_FAILED);
+            throw new WebApplicationException(message, Response.Status.INTERNAL_SERVER_ERROR);
         }
         return updatedQuota;
     }

diff --git a/src/main/java/org/dataone/bookkeeper/resources/UsagesResource.java b/src/main/java/org/dataone/bookkeeper/resources/UsagesResource.java
@@ -129,7 +129,7 @@ public UsageList listUsages(@Context SecurityContext context,
 
                     /* Caller is not admin and is not associated with any of the specified subscribers. */
                     if (subjects.size() == 0) {
-                        throw new WebApplicationException(caller.getSubject() + " requested subscribers don't exist or requestor doesn't have priviledge to view them.", Response.Status.FORBIDDEN);
+                        throw new WebApplicationException("The requested subscribers don't exist or requestor doesn't have privilege to view them.", Response.Status.FORBIDDEN);
                     }
                 } else {
                     /* Admin caller, so can see quotas for all requested subscribers */
@@ -168,7 +168,7 @@ public UsageList listUsages(@Context SecurityContext context,
                     usage = usageStore.findUsageByInstanceIdQuotaIdAndSubjects(instanceId, quotaId, subjects);
                 }
                 if(usage == null) {
-                    throw new WebApplicationException("The requested usage was not found or requestor does not have privilege to retrieve it.", Response.Status.NOT_FOUND);
+                    usages = null;
                 } else {
                     usages = new ArrayList<>();
                     usages.add(usage);
@@ -204,7 +204,14 @@ public UsageList listUsages(@Context SecurityContext context,
             }
 
             if (usages == null || usages.size() == 0) {
-                throw new WebApplicationException("The requested usages were not found or requestor does not have privilege to retrieve them.", Response.Status.NOT_FOUND);
+                if (! isAdmin || isProxy) {
+                    // If not an admin user or is a proxy user, we have no way to determine if they didn't have enough
+                    // privilege or if the usage doesn't exist.
+                    throw new WebApplicationException("The requested usages were not found or requestor does not have privilege to view them.", Response.Status.NOT_FOUND);
+                } else {
+                    // Admin user can see any existing usage, so can't be a priv issue.
+                    throw new WebApplicationException("The requested usage was not found.", Response.Status.NOT_FOUND);
+                }
             } else {
                 // Filter by status, if requested.
                 if(status != null) {
@@ -260,7 +267,7 @@ public Usage create(
                 usage = usageStore.getUsage(id);
             } catch (Exception e) {
                 String message = "Couldn't insert the usage: " + e.getMessage();
-                throw new WebApplicationException(message, Response.Status.EXPECTATION_FAILED);
+                throw new WebApplicationException(message, Response.Status.INTERNAL_SERVER_ERROR);
             }
             return usage;
         } else {
@@ -315,7 +322,7 @@ public Usage retrieve(
             }
         } catch (Exception e) {
             String message = "The requested usage could not be retrieved: " + e.getMessage();
-            throw new WebApplicationException(message, Response.Status.EXPECTATION_FAILED);
+            throw new WebApplicationException(message, Response.Status.INTERNAL_SERVER_ERROR);
         }
     }
 
@@ -349,7 +356,7 @@ public Usage update(
                 updatedUsage = usageStore.update(usage);
             } catch (Exception e) {
                 String message = "Couldn't update the usage: " + e.getMessage();
-                throw new WebApplicationException(message, Response.Status.EXPECTATION_FAILED);
+                throw new WebApplicationException(message, Response.Status.INTERNAL_SERVER_ERROR);
             }
         } else {
             throw new WebApplicationException("Admin privilege is required to update a usage, " + caller.getSubject() + " is not authorized.", Response.Status.FORBIDDEN);