Fixed return code for user and group services.

The user and group services have been modified to return consistent HTTP
return codes under various situations. The UGSubsystem has been modified
to capture any LDAP exceptions and throw the proper PKIException subclass
that represents the appropriate HTTP error code for the situation.

Ticket #669, #749

4 files changed, 65 insertions, 132 deletions

diff --git a/base/server/cms/src/com/netscape/cms/servlet/admin/GroupMemberProcessor.java b/base/server/cms/src/com/netscape/cms/servlet/admin/GroupMemberProcessor.java
index 399b97d0c..9f976d401 100644
--- a/base/server/cms/src/com/netscape/cms/servlet/admin/GroupMemberProcessor.java
+++ b/base/server/cms/src/com/netscape/cms/servlet/admin/GroupMemberProcessor.java
@@ -254,7 +254,7 @@ public class GroupMemberProcessor extends Processor {
         } catch (Exception e) {
             log(ILogger.LL_FAILURE, e.toString());
             auditAddGroupMember(groupID, groupMemberData, ILogger.FAILURE);
-            throw new PKIException(getUserMessage("CMS_USRGRP_USER_ADD_FAILED"));
+            throw new PKIException(getUserMessage("CMS_USRGRP_GROUP_MODIFY_FAILED"));
         }
     }
 
@@ -372,7 +372,7 @@ public class GroupMemberProcessor extends Processor {
         } catch (Exception e) {
             log(ILogger.LL_FAILURE, e.toString());
             auditDeleteGroupMember(groupID, groupMemberData, ILogger.FAILURE);
-            throw new PKIException(getUserMessage("CMS_USRGRP_USER_ADD_FAILED"));
+            throw new PKIException(getUserMessage("CMS_USRGRP_GROUP_MODIFY_FAILED"));
         }
     }
 
diff --git a/base/server/cms/src/com/netscape/cms/servlet/admin/GroupService.java b/base/server/cms/src/com/netscape/cms/servlet/admin/GroupService.java
index 3fd84da6c..3f250517a 100644
--- a/base/server/cms/src/com/netscape/cms/servlet/admin/GroupService.java
+++ b/base/server/cms/src/com/netscape/cms/servlet/admin/GroupService.java
@@ -38,6 +38,7 @@ import com.netscape.certsrv.apps.CMS;
 import com.netscape.certsrv.base.BadRequestException;
 import com.netscape.certsrv.base.EBaseException;
 import com.netscape.certsrv.base.PKIException;
+import com.netscape.certsrv.base.ResourceNotFoundException;
 import com.netscape.certsrv.common.OpDef;
 import com.netscape.certsrv.common.ScopeDef;
 import com.netscape.certsrv.group.GroupCollection;
@@ -207,23 +208,18 @@ public class GroupService extends PKIService implements GroupResource {
             }
 
             // allow adding a group with no members
-            try {
-                userGroupManager.addGroup(group);
+            userGroupManager.addGroup(group);
 
-                auditAddGroup(groupID, groupData, ILogger.SUCCESS);
+            auditAddGroup(groupID, groupData, ILogger.SUCCESS);
 
-                // read the data back
-                groupData = getGroup(groupID);
+            // read the data back
+            groupData = getGroup(groupID);
 
-                return Response
-                        .created(groupData.getLink().getHref())
-                        .entity(groupData)
-                        .type(MediaType.APPLICATION_XML)
-                        .build();
-
-            } catch (Exception e) {
-                throw new PKIException(getUserMessage("CMS_USRGRP_GROUP_ADD_FAILED", headers));
-            }
+            return Response
+                    .created(groupData.getLink().getHref())
+                    .entity(groupData)
+                    .type(MediaType.APPLICATION_XML)
+                    .build();
 
         } catch (PKIException e) {
             auditAddGroup(groupID, groupData, ILogger.FAILURE);
@@ -263,27 +259,25 @@ public class GroupService extends PKIService implements GroupResource {
 
             IGroup group = userGroupManager.getGroupFromName(groupID);
 
+            if (group == null) {
+                throw new ResourceNotFoundException("Group " + groupID + "  not found.");
+            }
+
             group.set("description", groupData.getDescription());
 
             // allow adding a group with no members, except "Certificate
             // Server Administrators"
-            try {
-                userGroupManager.modifyGroup(group);
-
-                auditModifyGroup(groupID, groupData, ILogger.SUCCESS);
+            userGroupManager.modifyGroup(group);
 
-                // read the data back
-                groupData = getGroup(groupID);
+            auditModifyGroup(groupID, groupData, ILogger.SUCCESS);
 
-                return Response
-                        .ok(groupData)
-                        .type(MediaType.APPLICATION_XML)
-                        .build();
+            // read the data back
+            groupData = getGroup(groupID);
 
-            } catch (Exception e) {
-                log(ILogger.LL_FAILURE, e.toString());
-                throw new PKIException(getUserMessage("CMS_USRGRP_GROUP_MODIFY_FAILED", headers));
-            }
+            return Response
+                    .ok(groupData)
+                    .type(MediaType.APPLICATION_XML)
+                    .build();
 
         } catch (PKIException e) {
             auditModifyGroup(groupID, groupData, ILogger.FAILURE);
diff --git a/base/server/cms/src/com/netscape/cms/servlet/admin/UserService.java b/base/server/cms/src/com/netscape/cms/servlet/admin/UserService.java
index 827541e2f..3f172abeb 100644
--- a/base/server/cms/src/com/netscape/cms/servlet/admin/UserService.java
+++ b/base/server/cms/src/com/netscape/cms/servlet/admin/UserService.java
@@ -39,7 +39,6 @@ import javax.ws.rs.core.Request;
 import javax.ws.rs.core.Response;
 import javax.ws.rs.core.UriInfo;
 
-import netscape.ldap.LDAPException;
 import netscape.security.pkcs.PKCS7;
 import netscape.security.x509.X509CertImpl;
 
@@ -49,7 +48,6 @@ import org.mozilla.jss.CryptoManager;
 import org.mozilla.jss.crypto.InternalCertificate;
 
 import com.netscape.certsrv.apps.CMS;
-import com.netscape.certsrv.base.BadRequestDataException;
 import com.netscape.certsrv.base.BadRequestException;
 import com.netscape.certsrv.base.EBaseException;
 import com.netscape.certsrv.base.ForbiddenException;
@@ -62,7 +60,6 @@ import com.netscape.certsrv.common.OpDef;
 import com.netscape.certsrv.common.ScopeDef;
 import com.netscape.certsrv.dbs.certdb.CertId;
 import com.netscape.certsrv.group.GroupMemberData;
-import com.netscape.certsrv.ldap.LDAPExceptionConverter;
 import com.netscape.certsrv.logging.IAuditor;
 import com.netscape.certsrv.logging.ILogger;
 import com.netscape.certsrv.password.IPasswordCheck;
@@ -187,7 +184,7 @@ public class UserService extends PKIService implements UserResource {
             if (userID == null) {
                 log(ILogger.LL_FAILURE, CMS.getLogMessage("ADMIN_SRVLT_NULL_RS_ID"));
 
-                throw new BadRequestDataException(getUserMessage("CMS_ADMIN_SRVLT_NULL_RS_ID", headers));
+                throw new BadRequestException(getUserMessage("CMS_ADMIN_SRVLT_NULL_RS_ID", headers));
             }
 
             IUser user;
@@ -266,13 +263,13 @@ public class UserService extends PKIService implements UserResource {
         try {
             if (userID == null) {
                 log(ILogger.LL_FAILURE, CMS.getLogMessage("ADMIN_SRVLT_NULL_RS_ID"));
-                throw new BadRequestDataException(getUserMessage("CMS_ADMIN_SRVLT_NULL_RS_ID", headers));
+                throw new BadRequestException(getUserMessage("CMS_ADMIN_SRVLT_NULL_RS_ID", headers));
             }
 
             if (userID.indexOf(BACK_SLASH) != -1) {
                 // backslashes (BS) are not allowed
                 log(ILogger.LL_FAILURE, CMS.getLogMessage("ADMIN_SRVLT_RS_ID_BS"));
-                throw new BadRequestDataException(getUserMessage("CMS_ADMIN_SRVLT_RS_ID_BS", headers));
+                throw new BadRequestException(getUserMessage("CMS_ADMIN_SRVLT_RS_ID_BS", headers));
             }
 
             if (userID.equals(SYSTEM_USER)) {
@@ -288,7 +285,7 @@ public class UserService extends PKIService implements UserResource {
                 String msg = getUserMessage("CMS_USRGRP_USER_ADD_FAILED_1", headers, "full name");
 
                 log(ILogger.LL_FAILURE, msg);
-                throw new BadRequestDataException(msg);
+                throw new BadRequestException(msg);
 
             } else {
                 user.setFullName(fname);
@@ -337,43 +334,24 @@ public class UserService extends PKIService implements UserResource {
             String csType = cs.getString("cs.type");
             if (tpsProfiles != null) {
                 if (!csType.equals("TPS")) {
-                    throw new BadRequestDataException("Cannot set tpsProfiles on a non-TPS subsystem");
+                    throw new BadRequestException("Cannot set tpsProfiles on a non-TPS subsystem");
                 }
                 String[] profiles = tpsProfiles.split(",");
                 user.setTpsProfiles(Arrays.asList(profiles));
             }
 
-            try {
-                userGroupManager.addUser(user);
-
-                auditAddUser(userID, userData, ILogger.SUCCESS);
-
-                // read the data back
-                userData = getUser(userID);
-
-                return Response
-                        .created(userData.getLink().getHref())
-                        .entity(userData)
-                        .type(MediaType.APPLICATION_XML)
-                        .build();
-
-            } catch (EUsrGrpException e) {
-                log(ILogger.LL_FAILURE, e.toString());
+            userGroupManager.addUser(user);
 
-                if (user.getUserID() == null) {
-                    throw new BadRequestDataException(getUserMessage("CMS_USRGRP_USER_ADD_FAILED_1", headers, "uid"));
-                } else {
-                    throw new PKIException(e.getMessage(), e);
-                }
+            auditAddUser(userID, userData, ILogger.SUCCESS);
 
-            } catch (LDAPException e) {
-                log(ILogger.LL_FAILURE, CMS.getLogMessage("ADMIN_SRVLT_ADD_USER_FAIL", e.toString()));
-                throw LDAPExceptionConverter.toPKIException(e);
+            // read the data back
+            userData = getUser(userID);
 
-            } catch (Exception e) {
-                log(ILogger.LL_FAILURE, e.toString());
-                throw new PKIException(e.getMessage(), e);
-            }
+            return Response
+                    .created(userData.getLink().getHref())
+                    .entity(userData)
+                    .type(MediaType.APPLICATION_XML)
+                    .build();
 
         } catch (PKIException e) {
             auditAddUser(userID, userData, ILogger.FAILURE);
@@ -407,7 +385,7 @@ public class UserService extends PKIService implements UserResource {
         try {
             if (userID == null) {
                 log(ILogger.LL_FAILURE, CMS.getLogMessage("ADMIN_SRVLT_NULL_RS_ID"));
-                throw new BadRequestDataException(getUserMessage("CMS_ADMIN_SRVLT_NULL_RS_ID", headers));
+                throw new BadRequestException(getUserMessage("CMS_ADMIN_SRVLT_NULL_RS_ID", headers));
             }
 
             IUser user = userGroupManager.createUser(userID);
@@ -447,29 +425,23 @@ public class UserService extends PKIService implements UserResource {
             String csType = cs.getString("cs.type");
             if (tpsProfiles != null) {
                 if (!csType.equals("TPS")) {
-                    throw new BadRequestDataException("Cannot set tpsProfiles on a non-TPS subsystem");
+                    throw new BadRequestException("Cannot set tpsProfiles on a non-TPS subsystem");
                 }
                 String[] profiles = tpsProfiles.split(",");
                 user.setTpsProfiles(Arrays.asList(profiles));
             }
 
-            try {
-                userGroupManager.modifyUser(user);
-
-                auditModifyUser(userID, userData, ILogger.SUCCESS);
+            userGroupManager.modifyUser(user);
 
-                // read the data back
-                userData = getUser(userID);
+            auditModifyUser(userID, userData, ILogger.SUCCESS);
 
-                return Response
-                        .ok(userData)
-                        .type(MediaType.APPLICATION_XML)
-                        .build();
+            // read the data back
+            userData = getUser(userID);
 
-            } catch (Exception e) {
-                log(ILogger.LL_FAILURE, e.toString());
-                throw new PKIException(getUserMessage("CMS_USRGRP_USER_MOD_FAILED", headers));
-            }
+            return Response
+                    .ok(userData)
+                    .type(MediaType.APPLICATION_XML)
+                    .build();
 
         } catch (PKIException e) {
             auditModifyUser(userID, userData, ILogger.FAILURE);
@@ -509,35 +481,27 @@ public class UserService extends PKIService implements UserResource {
             }
 
             // get list of groups, and see if uid belongs to any
-            Enumeration<IGroup> groups;
+            Enumeration<IGroup> groups = userGroupManager.findGroups("*");
 
-            try {
-                groups = userGroupManager.findGroups("*");
+            while (groups.hasMoreElements()) {
+                IGroup group = groups.nextElement();
+                if (!group.isMember(userID)) continue;
 
-            } catch (Exception e) {
-                throw new PKIException(getUserMessage("CMS_INTERNAL_ERROR", headers));
+                userGroupManager.removeUserFromGroup(group, userID);
             }
 
-            try {
-                while (groups.hasMoreElements()) {
-                    IGroup group = groups.nextElement();
-                    if (!group.isMember(userID)) continue;
+            // comes out clean of group membership...now remove user
+            userGroupManager.removeUser(userID);
 
-                    userGroupManager.removeUserFromGroup(group, userID);
-                }
-
-                // comes out clean of group membership...now remove user
-                userGroupManager.removeUser(userID);
-
-                auditDeleteUser(userID, ILogger.SUCCESS);
-
-            } catch (Exception e) {
-                throw new PKIException(getUserMessage("CMS_USRGRP_SRVLT_FAIL_USER_RMV", headers));
-            }
+            auditDeleteUser(userID, ILogger.SUCCESS);
 
         } catch (PKIException e) {
             auditDeleteUser(userID, ILogger.FAILURE);
             throw e;
+
+        } catch (EBaseException e) {
+            auditDeleteUser(userID, ILogger.FAILURE);
+            throw new PKIException(e.getMessage());
         }
     }
 
@@ -864,13 +828,6 @@ public class UserService extends PKIService implements UserResource {
                 log(ILogger.LL_FAILURE, CMS.getLogMessage("USRGRP_SRVLT_CERT_NOT_YET_VALID",
                         String.valueOf(cert.getSubjectDN())));
                 throw new BadRequestException(getUserMessage("CMS_USRGRP_SRVLT_CERT_NOT_YET_VALID", headers));
-
-            } catch (LDAPException e) {
-                if (e.getLDAPResultCode() == LDAPException.ATTRIBUTE_OR_VALUE_EXISTS) {
-                    throw new PKIException(getUserMessage("CMS_USRGRP_SRVLT_USER_CERT_EXISTS", headers));
-                } else {
-                    throw new PKIException(getUserMessage("CMS_USRGRP_USER_MOD_FAILED", headers));
-                }
             }
 
         } catch (PKIException e) {
diff --git a/base/server/cms/src/com/netscape/cms/servlet/admin/UsrGrpAdminServlet.java b/base/server/cms/src/com/netscape/cms/servlet/admin/UsrGrpAdminServlet.java
index 2cd337123..c4eed9068 100644
--- a/base/server/cms/src/com/netscape/cms/servlet/admin/UsrGrpAdminServlet.java
+++ b/base/server/cms/src/com/netscape/cms/servlet/admin/UsrGrpAdminServlet.java
@@ -31,7 +31,6 @@ import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
-import netscape.ldap.LDAPException;
 import netscape.security.pkcs.PKCS7;
 import netscape.security.x509.X509CertImpl;
 
@@ -40,6 +39,7 @@ import org.mozilla.jss.crypto.InternalCertificate;
 
 import com.netscape.certsrv.apps.CMS;
 import com.netscape.certsrv.authorization.IAuthzSubsystem;
+import com.netscape.certsrv.base.ConflictingOperationException;
 import com.netscape.certsrv.base.EBaseException;
 import com.netscape.certsrv.base.ICertPrettyPrint;
 import com.netscape.certsrv.base.ISubsystem;
@@ -900,21 +900,7 @@ public class UsrGrpAdminServlet extends AdminServlet {
                             CMS.getUserMessage(getLocale(req), "CMS_USRGRP_USER_ADD_FAILED"), null, resp);
                 }
                 return;
-            } catch (LDAPException e) {
-                log(ILogger.LL_FAILURE, CMS.getLogMessage("ADMIN_SRVLT_ADD_USER_FAIL", e.toString()));
 
-                // store a message in the signed audit log file
-                auditMessage = CMS.getLogMessage(
-                            LOGGING_SIGNED_AUDIT_CONFIG_ROLE,
-                            auditSubjectID,
-                            ILogger.FAILURE,
-                            auditParams(req));
-
-                audit(auditMessage);
-
-                sendResponse(ERROR,
-                        CMS.getUserMessage(getLocale(req), "CMS_USRGRP_USER_ADD_FAILED"), null, resp);
-                return;
             } catch (Exception e) {
                 log(ILogger.LL_FAILURE, e.toString());
 
@@ -1251,7 +1237,7 @@ public class UsrGrpAdminServlet extends AdminServlet {
                         CMS.getUserMessage(getLocale(req), "CMS_USRGRP_SRVLT_CERT_NOT_YET_VALID"), null, resp);
                 return;
 
-            } catch (LDAPException e) {
+            } catch (ConflictingOperationException e) {
                 // store a message in the signed audit log file
                 auditMessage = CMS.getLogMessage(
                             LOGGING_SIGNED_AUDIT_CONFIG_ROLE,
@@ -1261,14 +1247,10 @@ public class UsrGrpAdminServlet extends AdminServlet {
 
                 audit(auditMessage);
 
-                if (e.getLDAPResultCode() == LDAPException.ATTRIBUTE_OR_VALUE_EXISTS) {
-                    sendResponse(ERROR,
-                            CMS.getUserMessage(getLocale(req), "CMS_USRGRP_SRVLT_USER_CERT_EXISTS"), null, resp);
-                } else {
-                    sendResponse(ERROR,
-                            CMS.getUserMessage(getLocale(req), "CMS_USRGRP_USER_MOD_FAILED"), null, resp);
-                }
+                sendResponse(ERROR,
+                        CMS.getUserMessage(getLocale(req), "CMS_USRGRP_SRVLT_USER_CERT_EXISTS"), null, resp);
                 return;
+
             } catch (Exception e) {
                 log(ILogger.LL_FAILURE, e.toString());