Realms - Address comments from review

Review comments addressed:
1. when archiving or generating keys, realm is checked
2. when no plugin is found for a realm, access is denied.
3. rename mFoo to foo for new variables.
4. add chaining of exceptions
5. remove attributes from KeyArchivalRequest etc. when realm is null
6. Add more detail to denial in BasicGroupAuthz

Part of Trac Ticket 2041

2 files changed, 67 insertions, 32 deletions

diff --git a/base/kra/src/org/dogtagpki/server/kra/rest/KeyRequestService.java b/base/kra/src/org/dogtagpki/server/kra/rest/KeyRequestService.java
index 8504f0ea2..41d78af53 100644
--- a/base/kra/src/org/dogtagpki/server/kra/rest/KeyRequestService.java
+++ b/base/kra/src/org/dogtagpki/server/kra/rest/KeyRequestService.java
@@ -38,6 +38,7 @@ import org.mozilla.jss.crypto.SymmetricKey;
 import com.netscape.certsrv.apps.CMS;
 import com.netscape.certsrv.authentication.IAuthToken;
 import com.netscape.certsrv.authorization.EAuthzAccessDenied;
+import com.netscape.certsrv.authorization.EAuthzUnknownRealm;
 import com.netscape.certsrv.base.BadRequestException;
 import com.netscape.certsrv.base.EBaseException;
 import com.netscape.certsrv.base.PKIException;
@@ -171,15 +172,25 @@ public class KeyRequestService extends PKIService implements KeyRequestResource
             if (getRequestor() == null) {
                 throw new UnauthorizedException("Archival must be performed by an agent");
             }
+
+            String realm = data.getRealm();
+            if (realm != null) {
+                authz.checkRealm(realm, getAuthToken(), null, "keyRequest", "archive");
+            }
             response = dao.submitRequest(data, uriInfo, getRequestor());
             auditArchivalRequestMade(response.getRequestInfo().getRequestId(), ILogger.SUCCESS, data.getClientKeyId());
 
             return createCreatedResponse(response, new URI(response.getRequestInfo().getRequestURL()));
-
+        } catch (EAuthzAccessDenied e) {
+            auditArchivalRequestMade(null, ILogger.FAILURE, data.getClientKeyId());
+            throw new UnauthorizedException("Not authorized to generate request in this realm", e);
+        } catch (EAuthzUnknownRealm e) {
+            auditArchivalRequestMade(null, ILogger.FAILURE, data.getClientKeyId());
+            throw new BadRequestException("Invalid realm", e);
         } catch (EBaseException | URISyntaxException e) {
             e.printStackTrace();
             auditArchivalRequestMade(null, ILogger.FAILURE, data.getClientKeyId());
-            throw new PKIException(e.toString());
+            throw new PKIException(e.toString(), e);
         }
     }
 
@@ -216,7 +227,7 @@ public class KeyRequestService extends PKIService implements KeyRequestResource
         } catch (EBaseException | URISyntaxException e) {
             e.printStackTrace();
             auditRecoveryRequestMade(null, ILogger.FAILURE, data.getKeyId());
-            throw new PKIException(e.toString());
+            throw new PKIException(e.toString(), e);
         }
     }
 
@@ -233,11 +244,11 @@ public class KeyRequestService extends PKIService implements KeyRequestResource
             dao.approveRequest(id, getRequestor(), getAuthToken());
             auditRecoveryRequestChange(id, ILogger.SUCCESS, "approve");
         } catch (EAuthzAccessDenied e) {
-            throw new UnauthorizedException("Not authorized to approve request");
+            throw new UnauthorizedException("Not authorized to approve request", e);
         } catch (EBaseException e) {
             e.printStackTrace();
             auditRecoveryRequestChange(id, ILogger.FAILURE, "approve");
-            throw new PKIException(e.toString());
+            throw new PKIException(e.toString(), e);
         }
 
         return createNoContentResponse();
@@ -254,11 +265,11 @@ public class KeyRequestService extends PKIService implements KeyRequestResource
             dao.rejectRequest(id, getAuthToken());
             auditRecoveryRequestChange(id, ILogger.SUCCESS, "reject");
         }catch (EAuthzAccessDenied e) {
-            throw new UnauthorizedException("Not authorized to reject request");
+            throw new UnauthorizedException("Not authorized to reject request", e);
         } catch (EBaseException e) {
             e.printStackTrace();
             auditRecoveryRequestChange(id, ILogger.FAILURE, "reject");
-            throw new PKIException(e.toString());
+            throw new PKIException(e.toString(), e);
         }
 
         return createNoContentResponse();
@@ -275,11 +286,11 @@ public class KeyRequestService extends PKIService implements KeyRequestResource
             dao.cancelRequest(id, getAuthToken());
             auditRecoveryRequestChange(id, ILogger.SUCCESS, "cancel");
         } catch (EAuthzAccessDenied e) {
-            throw new UnauthorizedException("Not authorized to cancel request");
+            throw new UnauthorizedException("Not authorized to cancel request", e);
         } catch (EBaseException e) {
             e.printStackTrace();
             auditRecoveryRequestChange(id, ILogger.FAILURE, "cancel");
-            throw new PKIException(e.toString());
+            throw new PKIException(e.toString(), e);
         }
 
         return createNoContentResponse();
@@ -295,10 +306,12 @@ public class KeyRequestService extends PKIService implements KeyRequestResource
             try {
                 authz.checkRealm(realm, getAuthToken(), null, "keyRequests", "list");
             } catch (EAuthzAccessDenied e) {
-                throw new UnauthorizedException("Not authorized to list these requests");
+                throw new UnauthorizedException("Not authorized to list these requests", e);
+            } catch (EAuthzUnknownRealm e) {
+                throw new BadRequestException("Invalid realm", e);
             } catch (EBaseException e) {
                 CMS.debug("listRequests: unable to authorize realm" + e);
-                throw new PKIException(e.toString());
+                throw new PKIException(e.toString(), e);
             }
         }
         // get ldap filter
@@ -317,7 +330,7 @@ public class KeyRequestService extends PKIService implements KeyRequestResource
         } catch (EBaseException e) {
             CMS.debug("listRequests: error in obtaining request results" + e);
             e.printStackTrace();
-            throw new PKIException(e.toString());
+            throw new PKIException(e.toString(), e);
         }
         return createOKResponse(requests);
     }
@@ -426,7 +439,7 @@ public class KeyRequestService extends PKIService implements KeyRequestResource
             request = requestClazz.getDeclaredConstructor(ResourceMessage.class).newInstance(data);
         } catch (ClassNotFoundException | NoSuchMethodException | SecurityException | InstantiationException
                 | IllegalAccessException | IllegalArgumentException | InvocationTargetException e) {
-            throw new BadRequestException("Invalid request class." + e);
+            throw new BadRequestException("Invalid request class." + e, e);
         }
 
         if (request instanceof KeyArchivalRequest) {
@@ -453,16 +466,26 @@ public class KeyRequestService extends PKIService implements KeyRequestResource
             if (getRequestor() == null) {
                 throw new UnauthorizedException("Key generation must be performed by an agent");
             }
+            String realm = data.getRealm();
+            if (realm != null) {
+                authz.checkRealm(realm, getAuthToken(), null, "keyRequest", "generateSymkey");
+            }
+
             response = dao.submitRequest(data, uriInfo, getRequestor());
             auditSymKeyGenRequestMade(response.getRequestInfo().getRequestId(), ILogger.SUCCESS,
                     data.getClientKeyId());
 
             return createCreatedResponse(response, new URI(response.getRequestInfo().getRequestURL()));
-
+        } catch (EAuthzAccessDenied e) {
+            auditSymKeyGenRequestMade(null, ILogger.FAILURE, data.getClientKeyId());
+            throw new UnauthorizedException("Not authorized to generate request in this realm", e);
+        } catch (EAuthzUnknownRealm e) {
+            auditSymKeyGenRequestMade(null, ILogger.FAILURE, data.getClientKeyId());
+            throw new BadRequestException("Invalid realm", e);
         } catch (EBaseException | URISyntaxException e) {
             e.printStackTrace();
             auditSymKeyGenRequestMade(null, ILogger.FAILURE, data.getClientKeyId());
-            throw new PKIException(e.toString());
+            throw new PKIException(e.toString(), e);
         }
     }
 
@@ -477,16 +500,26 @@ public class KeyRequestService extends PKIService implements KeyRequestResource
             if (getRequestor() == null) {
                 throw new UnauthorizedException("Key generation must be performed by an agent");
             }
+            String realm = data.getRealm();
+            if (realm != null) {
+                authz.checkRealm(realm, getAuthToken(), null, "keyRequest", "generateAsymkey");
+            }
+
             response = dao.submitRequest(data, uriInfo, getRequestor());
             auditAsymKeyGenRequestMade(response.getRequestInfo().getRequestId(), ILogger.SUCCESS,
                     data.getClientKeyId());
 
             return createCreatedResponse(response, new URI(response.getRequestInfo().getRequestURL()));
-
+        } catch (EAuthzAccessDenied e) {
+            auditAsymKeyGenRequestMade(null, ILogger.FAILURE, data.getClientKeyId());
+            throw new UnauthorizedException("Not authorized to generate request in this realm", e);
+        } catch (EAuthzUnknownRealm e) {
+            auditAsymKeyGenRequestMade(null, ILogger.FAILURE, data.getClientKeyId());
+            throw new BadRequestException("Invalid realm", e);
         } catch (EBaseException | URISyntaxException e) {
             e.printStackTrace();
             auditAsymKeyGenRequestMade(null, ILogger.FAILURE, data.getClientKeyId());
-            throw new PKIException(e.toString());
+            throw new PKIException(e.toString(), e);
         }
     }
 
diff --git a/base/kra/src/org/dogtagpki/server/kra/rest/KeyService.java b/base/kra/src/org/dogtagpki/server/kra/rest/KeyService.java
index 52df7696f..255d8d614 100644
--- a/base/kra/src/org/dogtagpki/server/kra/rest/KeyService.java
+++ b/base/kra/src/org/dogtagpki/server/kra/rest/KeyService.java
@@ -44,6 +44,7 @@ import org.jboss.resteasy.plugins.providers.atom.Link;
 import com.netscape.certsrv.apps.CMS;
 import com.netscape.certsrv.authentication.IAuthToken;
 import com.netscape.certsrv.authorization.EAuthzAccessDenied;
+import com.netscape.certsrv.authorization.EAuthzUnknownRealm;
 import com.netscape.certsrv.base.BadRequestException;
 import com.netscape.certsrv.base.EBaseException;
 import com.netscape.certsrv.base.HTTPGoneException;
@@ -150,7 +151,7 @@ public class KeyService extends PKIService implements KeyResource {
         } catch (EBaseException e) {
             CMS.debug(e);
             auditRetrieveKey(ILogger.FAILURE, requestID, null, auditInfo + ";" + e.getMessage());
-            throw new PKIException(e.getMessage());
+            throw new PKIException(e.getMessage(), e);
         }
 
         String type = request.getRequestType();
@@ -170,7 +171,7 @@ public class KeyService extends PKIService implements KeyResource {
         } catch (Exception e) {
             CMS.debug(e);
             auditRetrieveKey(ILogger.FAILURE, requestID, keyId, auditInfo + ";" + e.getMessage());
-            throw new PKIException(e.getMessage());
+            throw new PKIException(e.getMessage(), e);
         }
 
         if (keyData == null) {
@@ -348,7 +349,7 @@ public class KeyService extends PKIService implements KeyResource {
             CMS.debug(logMessage);
 
             e1.printStackTrace();
-            throw new PKIException(logMessage + e1.getMessage());
+            throw new PKIException(logMessage + e1.getMessage(), e1);
         }
         if (reqInfo == null) {
             // request not found
@@ -377,7 +378,7 @@ public class KeyService extends PKIService implements KeyResource {
             logMessage = e.getMessage();
             CMS.debug(logMessage);
 
-            throw new PKIException(logMessage);
+            throw new PKIException(logMessage, e);
         }
         String originator = request.getExtDataInString(IRequest.ATTR_REQUEST_OWNER);
         if (! originator.equals(retriever)) {
@@ -423,10 +424,12 @@ public class KeyService extends PKIService implements KeyResource {
             try {
                 authz.checkRealm(realm, getAuthToken(), null, "keys", "list");
             } catch (EAuthzAccessDenied e) {
-                throw new UnauthorizedException("Not authorized to list these keys");
+                throw new UnauthorizedException("Not authorized to list these keys", e);
+            } catch (EAuthzUnknownRealm e) {
+                throw new BadRequestException("Invalid realm", e);
             } catch (EBaseException e) {
                 CMS.debug("listRequests: unable to authorize realm" + e);
-                throw new PKIException(e.toString());
+                throw new PKIException(e.toString(), e);
             }
         }
 
@@ -475,7 +478,7 @@ public class KeyService extends PKIService implements KeyResource {
             auditRetrieveKey(ILogger.FAILURE, null, clientKeyID, e.getMessage() + auditInfo);
 
             e.printStackTrace();
-            throw new PKIException(e.getMessage());
+            throw new PKIException(e.getMessage(), e);
         }
         auditRetrieveKey(ILogger.SUCCESS, null, clientKeyID, auditInfo);
 
@@ -508,10 +511,10 @@ public class KeyService extends PKIService implements KeyResource {
                 try {
                     authz.checkRealm(info.getRealm(), getAuthToken(), info.getOwnerName(), "key", "read");
                 } catch (EAuthzAccessDenied e) {
-                    throw new UnauthorizedException("Not authorized to read this key");
+                    throw new UnauthorizedException("Not authorized to read this key", e);
                 } catch (EBaseException e) {
                     CMS.debug("listRequests: unable to authorize realm" + e);
-                    throw new PKIException(e.toString());
+                    throw new PKIException(e.toString(), e);
                 }
 
                 auditRetrieveKey(ILogger.SUCCESS, null, clientKeyID, auditInfo);
@@ -686,18 +689,17 @@ public class KeyService extends PKIService implements KeyResource {
         } catch (EAuthzAccessDenied e) {
             auditInfo = method + "Unauthorized access for key record";
             auditRetrieveKey(ILogger.FAILURE, null, keyId, auditInfo);
-            throw new UnauthorizedException(auditInfo);
+            throw new UnauthorizedException(auditInfo, e);
         } catch (EDBRecordNotFoundException e) {
             auditInfo = method + e.getMessage();
             auditRetrieveKey(ILogger.FAILURE, null, keyId, auditInfo);
-
-            throw new KeyNotFoundException(keyId);
+            throw new KeyNotFoundException(keyId, "key not found", e);
         } catch (Exception e) {
             auditInfo = method + "Unable to retrieve key record: " + e.getMessage();
             auditRetrieveKey(ILogger.FAILURE, null, keyId, auditInfo);
             CMS.debug(auditInfo);
             e.printStackTrace();
-            throw new PKIException(e.getMessage());
+            throw new PKIException(e.getMessage(), e);
         }
     }
 
@@ -735,14 +737,14 @@ public class KeyService extends PKIService implements KeyResource {
             CMS.debug(auditInfo);
             auditKeyStatusChange(ILogger.FAILURE, keyId.toString(),
                     (info!=null)?info.getStatus():null, status, auditInfo);
-            throw new KeyNotFoundException(keyId);
+            throw new KeyNotFoundException(keyId, "key not found to modify", e);
         } catch (Exception e) {
             auditInfo = auditInfo + ":" + e.getMessage();
             CMS.debug(auditInfo);
             auditKeyStatusChange(ILogger.FAILURE, keyId.toString(),
                     (info!=null)?info.getStatus():null, status, auditInfo);
             e.printStackTrace();
-            throw new PKIException(e.getMessage());
+            throw new PKIException(e.getMessage(), e);
         }
     }