From fc57d2e1acdc96115adcbe7db3820e7e8475f7b3 Mon Sep 17 00:00:00 2001 From: "Endi S. Dewata" Date: Mon, 29 Jun 2015 10:00:08 -0400 Subject: [PATCH] Cleaned up SystemConfigService.validateRequest(). The configure() in SystemConfigService method has been modified to log only the error message in normal responses but log the full stack trace when unexpected issues occur. The validateData() in SystemConfigService has been renamed to validateRequest() for clarity. The log messages have been modified to include the invalid values entered in the request. --- .../cms/servlet/test/ConfigurationTest.java | 2 +- .../certsrv/system/SystemConfigClient.java | 2 +- .../certsrv/system/SystemConfigResource.java | 2 +- .../dogtagpki/server/rest/SystemConfigService.java | 69 ++++++++++++---------- 4 files changed, 41 insertions(+), 34 deletions(-) diff --git a/base/common/functional/src/com/netscape/cms/servlet/test/ConfigurationTest.java b/base/common/functional/src/com/netscape/cms/servlet/test/ConfigurationTest.java index bf4dc892822df7d32be5d83c64e99c4dc7620529..69994fa389d31f15e314feba6bfbd3d4803622ba 100644 --- a/base/common/functional/src/com/netscape/cms/servlet/test/ConfigurationTest.java +++ b/base/common/functional/src/com/netscape/cms/servlet/test/ConfigurationTest.java @@ -76,7 +76,7 @@ public class ConfigurationTest { System.exit(1); } - public static void main(String args[]) throws NoSuchAlgorithmException, TokenException, IOException, InvalidBERException { + public static void main(String args[]) throws Exception { String host = null; String port = null; String cstype = null; diff --git a/base/common/src/com/netscape/certsrv/system/SystemConfigClient.java b/base/common/src/com/netscape/certsrv/system/SystemConfigClient.java index 242f0053119d04fcbf635f245bbde1fd58a19ff5..8208915808ebf5992e37b8e6d2ba54bedfadf232 100644 --- a/base/common/src/com/netscape/certsrv/system/SystemConfigClient.java +++ b/base/common/src/com/netscape/certsrv/system/SystemConfigClient.java @@ -40,7 +40,7 @@ public class SystemConfigClient extends Client { configClient = createProxy(SystemConfigResource.class); } - public ConfigurationResponse configure(ConfigurationRequest data) { + public ConfigurationResponse configure(ConfigurationRequest data) throws Exception { return configClient.configure(data); } } diff --git a/base/common/src/com/netscape/certsrv/system/SystemConfigResource.java b/base/common/src/com/netscape/certsrv/system/SystemConfigResource.java index 0cebb607433aea8571ff524df42872e9ae781c43..9c570eb2b6749c2eac1fc36ccf21b98f458369dd 100644 --- a/base/common/src/com/netscape/certsrv/system/SystemConfigResource.java +++ b/base/common/src/com/netscape/certsrv/system/SystemConfigResource.java @@ -29,5 +29,5 @@ public interface SystemConfigResource { @POST @Path("configure") - public ConfigurationResponse configure(ConfigurationRequest data); + public ConfigurationResponse configure(ConfigurationRequest data) throws Exception; } diff --git a/base/server/cms/src/org/dogtagpki/server/rest/SystemConfigService.java b/base/server/cms/src/org/dogtagpki/server/rest/SystemConfigService.java index 2de087badfa3f1b87ccf2295f00fc6c490c53517..75e3065faedb59f6e38a1778be7df40e2056ea0f 100644 --- a/base/server/cms/src/org/dogtagpki/server/rest/SystemConfigService.java +++ b/base/server/cms/src/org/dogtagpki/server/rest/SystemConfigService.java @@ -111,28 +111,38 @@ public class SystemConfigService extends PKIService implements SystemConfigResou * @see com.netscape.cms.servlet.csadmin.SystemConfigurationResource#configure(com.netscape.cms.servlet.csadmin.data.ConfigurationData) */ @Override - public ConfigurationResponse configure(ConfigurationRequest request) { + public ConfigurationResponse configure(ConfigurationRequest request) throws Exception { + + CMS.debug("SystemConfigService: configure()"); + try { ConfigurationResponse response = new ConfigurationResponse(); configure(request, response); return response; - } catch (Throwable t) { - CMS.debug(t); - throw t; + } catch (PKIException e) { // normal responses + CMS.debug(e.getMessage()); // log the response + throw e; + + } catch (Exception e) { // unexpected exceptions + CMS.debug(e); // show stack trace for troubleshooting + throw e; + + } catch (Error e) { // system errors + CMS.debug(e); // show stack trace for troubleshooting + throw e; } } - public void configure(ConfigurationRequest data, ConfigurationResponse response) { + public void configure(ConfigurationRequest data, ConfigurationResponse response) throws Exception { + if (csState.equals("1")) { throw new BadRequestException("System is already configured"); } - CMS.debug("SystemConfigService(): configure() called"); - CMS.debug(data.toString()); - - validateData(data); + CMS.debug("SystemConfigService: request: " + data); + validateRequest(data); Collection certList = getCertList(data); @@ -1020,22 +1030,15 @@ public class SystemConfigService extends PKIService implements SystemConfigResou } } - private void validateData(ConfigurationRequest data) { - // get required info from CS.cfg - String preopPin; - try { - preopPin = cs.getString("preop.pin"); - } catch (Exception e) { - CMS.debug("validateData: Failed to get required config form CS.cfg"); - e.printStackTrace(); - throw new PKIException("Unable to retrieve required configuration from configuration files"); - } + private void validateRequest(ConfigurationRequest data) throws Exception { - // get the preop pin and validate it + // validate installation pin String pin = data.getPin(); if (pin == null) { throw new BadRequestException("No preop pin provided"); } + + String preopPin = cs.getString("preop.pin"); if (!preopPin.equals(pin)) { throw new BadRequestException("Incorrect pin provided"); } @@ -1067,6 +1070,7 @@ public class SystemConfigService extends PKIService implements SystemConfigResou if (data.getSecurityDomainName() == null) { throw new BadRequestException("Security Domain Name is not provided"); } + } else if (domainType.equals(ConfigurationRequest.EXISTING_DOMAIN) || domainType.equals(ConfigurationRequest.NEW_SUBDOMAIN)) { if (data.getStandAlone()) { @@ -1079,11 +1083,11 @@ public class SystemConfigService extends PKIService implements SystemConfigResou } try { - @SuppressWarnings("unused") - URL admin_u = new URL(domainURI); // check for invalid URL + new URL(domainURI); } catch (MalformedURLException e) { - throw new BadRequestException("Invalid security domain URI"); + throw new BadRequestException("Invalid security domain URI: " + domainURI, e); } + if ((data.getSecurityDomainUser() == null) || (data.getSecurityDomainPassword() == null)) { throw new BadRequestException("Security domain user or password not provided"); } @@ -1109,11 +1113,13 @@ public class SystemConfigService extends PKIService implements SystemConfigResou throw new BadRequestException("Clone selected, but no clone URI provided"); } try { - @SuppressWarnings("unused") - URL url = new URL(cloneUri); // check for invalid URL + URL url = new URL(cloneUri); // confirm protocol is https + if (!"https".equals(url.getProtocol())) { + throw new BadRequestException("Clone URI must use HTTPS protocol: " + cloneUri); + } } catch (MalformedURLException e) { - throw new BadRequestException("Invalid clone URI"); + throw new BadRequestException("Invalid clone URI: " + cloneUri, e); } if (data.getToken().equals(ConfigurationRequest.TOKEN_DEFAULT)) { @@ -1133,6 +1139,7 @@ public class SystemConfigService extends PKIService implements SystemConfigResou throw new BadRequestException("P12 password should not be provided since HSM clones must share their HSM master's private keys"); } } + } else { data.setClone("false"); } @@ -1145,7 +1152,7 @@ public class SystemConfigService extends PKIService implements SystemConfigResou try { Integer.parseInt(data.getDsPort()); // check for errors } catch (NumberFormatException e) { - throw new BadRequestException("Internal database port is invalid"); + throw new BadRequestException("Internal database port is invalid: " + data.getDsPort(), e); } String basedn = data.getBaseDN(); @@ -1173,7 +1180,7 @@ public class SystemConfigService extends PKIService implements SystemConfigResou try { Integer.parseInt(masterReplicationPort); // check for errors } catch (NumberFormatException e) { - throw new BadRequestException("Master replication port is invalid"); + throw new BadRequestException("Master replication port is invalid: " + masterReplicationPort, e); } } @@ -1181,8 +1188,8 @@ public class SystemConfigService extends PKIService implements SystemConfigResou if (cloneReplicationPort != null && cloneReplicationPort.length() > 0) { try { Integer.parseInt(cloneReplicationPort); // check for errors - } catch (Exception e) { - throw new BadRequestException("Clone replication port is invalid"); + } catch (NumberFormatException e) { + throw new BadRequestException("Clone replication port is invalid: " + cloneReplicationPort, e); } } @@ -1293,7 +1300,7 @@ public class SystemConfigService extends PKIService implements SystemConfigResou try { Integer.parseInt(data.getAuthdbPort()); // check for errors } catch (NumberFormatException e) { - throw new BadRequestException("Authdb port is invalid"); + throw new BadRequestException("Authentication Database port is invalid: " + data.getAuthdbPort(), e); } // TODO check connection with authdb -- 1.9.3