Added checks for CertRequest and Cert Resources

Ticket 749

4 files changed, 79 insertions, 35 deletions

diff --git a/base/common/src/com/netscape/certsrv/cert/CertClient.java b/base/common/src/com/netscape/certsrv/cert/CertClient.java
index aa94483bd..9de548cad 100644
--- a/base/common/src/com/netscape/certsrv/cert/CertClient.java
+++ b/base/common/src/com/netscape/certsrv/cert/CertClient.java
@@ -31,7 +31,7 @@ import com.netscape.certsrv.request.RequestId;
 public class CertClient extends Client {
 
     public CertResource certClient;
-    public CertRequestResource certRequestResource;
+    public CertRequestResource certRequestClient;
 
     public CertClient(PKIClient client, String subsystem) throws URISyntaxException {
         super(client, subsystem, "cert");
@@ -40,7 +40,7 @@ public class CertClient extends Client {
 
     public void init() throws URISyntaxException {
         certClient = createProxy(CertResource.class);
-        certRequestResource = createProxy(CertRequestResource.class);
+        certRequestClient = createProxy(CertRequestResource.class);
     }
 
     public CertData getCert(CertId id) {
@@ -72,56 +72,56 @@ public class CertClient extends Client {
     }
 
     public CertRequestInfos enrollRequest(CertEnrollmentRequest data) {
-        return certRequestResource.enrollCert(data);
+        return certRequestClient.enrollCert(data);
     }
 
     public CertRequestInfo getRequest(RequestId id) {
-        return certRequestResource.getRequestInfo(id);
+        return certRequestClient.getRequestInfo(id);
     }
 
     public CertReviewResponse reviewRequest(RequestId id) {
-        return certRequestResource.reviewRequest(id);
+        return certRequestClient.reviewRequest(id);
     }
 
     public void approveRequest(RequestId id, CertReviewResponse data) {
-        certRequestResource.approveRequest(id, data);
+        certRequestClient.approveRequest(id, data);
     }
 
     public void rejectRequest(RequestId id, CertReviewResponse data) {
-        certRequestResource.rejectRequest(id, data);
+        certRequestClient.rejectRequest(id, data);
     }
 
     public void cancelRequest(RequestId id, CertReviewResponse data) {
-        certRequestResource.cancelRequest(id, data);
+        certRequestClient.cancelRequest(id, data);
     }
 
     public void updateRequest(RequestId id, CertReviewResponse data) {
-        certRequestResource.updateRequest(id, data);
+        certRequestClient.updateRequest(id, data);
     }
 
     public void validateRequest(RequestId id, CertReviewResponse data) {
-        certRequestResource.validateRequest(id, data);
+        certRequestClient.validateRequest(id, data);
     }
 
     public void assignRequest(RequestId id, CertReviewResponse data) {
-        certRequestResource.assignRequest(id, data);
+        certRequestClient.assignRequest(id, data);
     }
 
     public void unassignRequest(RequestId id, CertReviewResponse data) {
-        certRequestResource.unassignRequest(id, data);
+        certRequestClient.unassignRequest(id, data);
     }
 
     public CertRequestInfos listRequests(String requestState, String requestType, RequestId start, Integer pageSize,
             Integer maxResults, Integer maxTime) {
-        return certRequestResource.listRequests(requestState, requestType, start, pageSize, maxResults, maxTime);
+        return certRequestClient.listRequests(requestState, requestType, start, pageSize, maxResults, maxTime);
     }
 
     public CertEnrollmentRequest getEnrollmentTemplate(String id) {
-        return certRequestResource.getEnrollmentTemplate(id);
+        return certRequestClient.getEnrollmentTemplate(id);
     }
 
     public ProfileDataInfos listEnrollmentTemplates() {
-        return certRequestResource.listEnrollmentTemplates();
+        return certRequestClient.listEnrollmentTemplates();
     }
 
 }
diff --git a/base/java-tools/src/com/netscape/cmstools/cert/CertRequestFindCLI.java b/base/java-tools/src/com/netscape/cmstools/cert/CertRequestFindCLI.java
index dbfc0ee87..79970393c 100644
--- a/base/java-tools/src/com/netscape/cmstools/cert/CertRequestFindCLI.java
+++ b/base/java-tools/src/com/netscape/cmstools/cert/CertRequestFindCLI.java
@@ -83,7 +83,7 @@ public class CertRequestFindCLI extends CLI {
         if (requestType != null && requestType.equals("all")) requestType = null;
 
         try {
-            certRequests = certCLI.certClient.certRequestResource.listRequests(requestState, requestType, start, size, maxResults, maxTime);
+            certRequests = certCLI.certClient.certRequestClient.listRequests(requestState, requestType, start, size, maxResults, maxTime);
         } catch (PKIException e) {
             System.err.println("Error: Cannot list certificate requests. " + e.getMessage());
             System.exit(-1);
diff --git a/base/server/cms/src/com/netscape/cms/servlet/cert/CertService.java b/base/server/cms/src/com/netscape/cms/servlet/cert/CertService.java
index 87912e60e..483ae139c 100644
--- a/base/server/cms/src/com/netscape/cms/servlet/cert/CertService.java
+++ b/base/server/cms/src/com/netscape/cms/servlet/cert/CertService.java
@@ -113,12 +113,6 @@ public class CertService extends PKIService implements CertResource {
         repo = authority.getCertificateRepository();
     }
 
-    private void validateRequest(CertId id) {
-        if (id == null) {
-            throw new BadRequestException("Invalid id in CertResourceService.validateRequest.");
-        }
-    }
-
     @Override
     public CertData getCert(CertId id) {
         return getCert(id, false);
@@ -130,7 +124,9 @@ public class CertService extends PKIService implements CertResource {
     }
 
     public CertData getCert(CertId id, boolean generateNonce) {
-        validateRequest(id);
+        if (id == null) {
+            throw new BadRequestException("Unable to get certificate: Invalid id.");
+        }
 
         CertRetrievalRequest data = new CertRetrievalRequest();
         data.setCertId(id);
@@ -161,6 +157,20 @@ public class CertService extends PKIService implements CertResource {
     }
 
     public CertRequestInfo revokeCert(CertId id, CertRevokeRequest request, boolean caCert) {
+        if (id == null) {
+            CMS.debug("revokeCert: id is null");
+            throw new BadRequestException("Unable to revoke cert: invalid id");
+        }
+        if (request == null) {
+            CMS.debug("revokeCert: request is null");
+            throw new BadRequestException("Unable to revoke cert: invalid request");
+        }
+
+        // check cert actually exists.  This will throw a CertNotFoundException
+        // if the cert does not exist
+        @SuppressWarnings("unused")
+        CertData data = getCert(id);
+
         RevocationReason revReason = request.getReason();
         if (revReason == RevocationReason.REMOVE_FROM_CRL) {
             CertUnrevokeRequest unrevRequest = new CertUnrevokeRequest();
@@ -284,6 +294,20 @@ public class CertService extends PKIService implements CertResource {
 
     @Override
     public CertRequestInfo unrevokeCert(CertId id, CertUnrevokeRequest request) {
+        if (id == null) {
+            CMS.debug("unrevokeCert: id is null");
+            throw new BadRequestException("Unable to unrevoke cert: invalid id");
+        }
+        if (request == null) {
+            CMS.debug("unrevokeCert: request is null");
+            throw new BadRequestException("Unable to unrevoke cert: invalid request");
+        }
+
+        // check cert actually exists.  This will throw a CertNotFoundException
+        // if the cert does not exist
+        @SuppressWarnings("unused")
+        CertData data = getCert(id);
+
         RevocationProcessor processor;
         try {
             processor = new RevocationProcessor("caDoUnrevoke", getLocale(headers));
diff --git a/base/server/cms/src/com/netscape/cms/servlet/request/CertRequestService.java b/base/server/cms/src/com/netscape/cms/servlet/request/CertRequestService.java
index 620084208..56da823d2 100644
--- a/base/server/cms/src/com/netscape/cms/servlet/request/CertRequestService.java
+++ b/base/server/cms/src/com/netscape/cms/servlet/request/CertRequestService.java
@@ -86,8 +86,12 @@ public class CertRequestService extends PKIService implements CertRequestResourc
     /**
      * Used to retrieve key request info for a specific request
      */
+    @Override
     public CertRequestInfo getRequestInfo(RequestId id) {
-        // auth and authz
+        if (id == null) {
+            CMS.debug("getRequestInfo: id is null");
+            throw new BadRequestException("Unable to get request: invalid id");
+        }
         CertRequestInfo info;
 
         CertRequestDAO dao = new CertRequestDAO();
@@ -100,7 +104,6 @@ public class CertRequestService extends PKIService implements CertRequestResourc
         }
 
         if (info == null) {
-            // request does not exist
             throw new RequestNotFoundException(id);
         }
 
@@ -108,15 +111,18 @@ public class CertRequestService extends PKIService implements CertRequestResourc
     }
 
     // Enrollment - used to test integration with a browser
+    @Override
     public CertRequestInfos enrollCert(MultivaluedMap<String, String> form) {
         CertEnrollmentRequest data = new CertEnrollmentRequest(form);
         return enrollCert(data);
     }
 
+    @Override
     public CertRequestInfos enrollCert(CertEnrollmentRequest data) {
         CertRequestInfos infos;
         if (data == null) {
-            throw new BadRequestException("Bad data input into CertRequestResourceService.enrollCert!");
+            CMS.debug("enrollCert: data is null");
+            throw new BadRequestException("Unable to create enrollment reequest: Invalid input data");
         }
         CertRequestDAO dao = new CertRequestDAO();
 
@@ -135,33 +141,44 @@ public class CertRequestService extends PKIService implements CertRequestResourc
             throw new PKIException(e.toString());
         }
 
+        // this will return an error code of 200, instead of 201
+        // because it is possible to create more than one request
+        // as a result of this enrollment
+
         return infos;
     }
 
+    @Override
     public void approveRequest(RequestId id, CertReviewResponse data) {
         changeRequestState(id, data, "approve");
     }
 
+    @Override
     public void rejectRequest(RequestId id, CertReviewResponse data) {
         changeRequestState(id, data, "reject");
     }
 
+    @Override
     public void cancelRequest(RequestId id, CertReviewResponse data) {
         changeRequestState(id, data, "cancel");
     }
 
+    @Override
     public void updateRequest(RequestId id, CertReviewResponse data) {
         changeRequestState(id, data, "update");
     }
 
+    @Override
     public void validateRequest(RequestId id, CertReviewResponse data) {
         changeRequestState(id, data, "validate");
     }
 
+    @Override
     public void unassignRequest(RequestId id, CertReviewResponse data) {
         changeRequestState(id, data, "unassign");
     }
 
+    @Override
     public void assignRequest(RequestId id, CertReviewResponse data) {
         changeRequestState(id, data, "assign");
     }
@@ -195,12 +212,16 @@ public class CertRequestService extends PKIService implements CertRequestResourc
             throw new PKIException("Problem approving request in CertRequestResource.assignRequest! " + e);
         } catch (RequestNotFoundException e) {
             CMS.debug(e);
-            throw new BadRequestException(CMS.getUserMessage(getLocale(headers), "CMS_REQUEST_NOT_FOUND", id.toString()));
+            throw e;
         }
     }
 
+    @Override
     public CertReviewResponse reviewRequest(@PathParam("id") RequestId id) {
-     // auth and authz
+        if (id == null) {
+            CMS.debug("reviewRequest: id is null");
+            throw new BadRequestException("Unable to review request: invalid id");
+        }
         CertReviewResponse info;
 
         CertRequestDAO dao = new CertRequestDAO();
@@ -223,10 +244,9 @@ public class CertRequestService extends PKIService implements CertRequestResourc
     /**
      * Used to generate list of cert requests based on the search parameters
      */
+    @Override
     public CertRequestInfos listRequests(String requestState, String requestType,
             RequestId start, Integer pageSize, Integer maxResults, Integer maxTime) {
-        // auth and authz
-
         // get ldap filter
         String filter = createSearchFilter(requestState, requestType);
         CMS.debug("listRequests: filter is " + filter);
@@ -276,17 +296,17 @@ public class CertRequestService extends PKIService implements CertRequestResourc
 
     @Override
     public CertEnrollmentRequest getEnrollmentTemplate(String profileId) {
+        if (profileId == null) {
+            CMS.debug("getEnrollmenTemplate: invalid request. profileId is null");
+            throw new BadRequestException("Invalid ProfileId");
+        }
+
         IProfileSubsystem ps = (IProfileSubsystem) CMS.getSubsystem(IProfileSubsystem.ID);
         if (ps == null) {
             CMS.debug("getEnrollmentTemplate: ps is null");
             throw new PKIException("Error modifying profile state.  Profile Service not available");
         }
 
-        if (profileId == null) {
-            CMS.debug("getEnrollmenTemplate: invalid request. profileId is null");
-            throw new BadRequestException("Invalid ProfileId");
-        }
-
         IProfile profile = null;
         try {
             profile = ps.getProfile(profileId);