better error handling for config

DataONEorg · Feb 26, 2024 · c99b320 · c99b320
1 parent bc17681
commit c99b320
Show file tree

Hide file tree

Showing 2 changed files with 50 additions and 10 deletions.
diff --git a/src/main/java/org/dataone/portal/TokenGenerator.java b/src/main/java/org/dataone/portal/TokenGenerator.java
@@ -4,6 +4,7 @@
 import java.math.BigInteger;
 import java.net.URL;
 import java.nio.file.Files;
+import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.security.cert.Certificate;
 import java.security.interfaces.RSAPrivateKey;
@@ -235,8 +236,9 @@ protected synchronized void setPublicKeys() throws IOException {
         log.debug("local certificate FileNames to be loaded: \n"
                       + Arrays.toString(certificateFileNames));
         for (String certFileName : certificateFileNames) {
-            if (!Files.isReadable(Paths.get(certFileName))) {
-                log.warn("Certificate file " + certFileName + " does not exist.");
+            Path certPath = Paths.get(certFileName);
+            if (Files.isDirectory(certPath) || !Files.isReadable(certPath)) {
+                log.warn("No readable Certificate file found at path: " + certFileName);
                 continue;
             }
             RSAPublicKey currentKey = (RSAPublicKey) CertificateManager.getInstance()

diff --git a/src/test/java/org/dataone/portal/TokenGeneratorTest.java b/src/test/java/org/dataone/portal/TokenGeneratorTest.java
@@ -166,11 +166,21 @@ public void testGetSession_multipleCerts() throws Exception {
     }
 
     @Test
-    public void testSetPublicKeys_multipleCerts() throws Exception {
+    public void testSetPublicKeys_singleCerts() throws Exception {
 
         String orig = Settings.getConfiguration().getString(PUB_CERT_KEY);
         String bogusLocalCert = "/tmp/nonExistentCert.pem";
 
+        ///////////////////////////////////////////
+        // Verify 1 local cert & 1 server cert case
+        // (i.e. backwards compatible)
+        ///////////////////////////////////////////
+        Settings.getConfiguration().setProperty(PUB_CERT_KEY, LOCAL_CERT_1);
+        TokenGenerator.getInstance().setPublicKeys();
+        // should be 2 public keys total: one from disk & one from CN server
+        assertEquals(Arrays.toString(TokenGenerator.publicKeys.toArray()), 2,
+                     TokenGenerator.publicKeys.size());
+
         ///////////////////////////////////////////
         // Verify code can handle missing config
         // (i.e. backwards compatible)
@@ -187,16 +197,43 @@ public void testSetPublicKeys_multipleCerts() throws Exception {
         assertEquals(Arrays.toString(TokenGenerator.publicKeys.toArray()), 1,
                      TokenGenerator.publicKeys.size());
 
-        ///////////////////////////////////////////
-        // Verify 1 local cert & 1 server cert case
-        // (i.e. backwards compatible)
-        ///////////////////////////////////////////
-        Settings.getConfiguration().setProperty(PUB_CERT_KEY, LOCAL_CERT_1);
+        // Key present, but empty value
+        Settings.getConfiguration().addProperty(PUB_CERT_KEY, "");
         TokenGenerator.getInstance().setPublicKeys();
-        // should be 2 public keys total: one from disk & one from CN server
-        assertEquals(Arrays.toString(TokenGenerator.publicKeys.toArray()), 2,
+        // should be 1 public key total: none from disk & one from CN server
+        assertEquals(Arrays.toString(TokenGenerator.publicKeys.toArray()), 1,
+                     TokenGenerator.publicKeys.size());
+
+        // Value set to paths that are invalid in this context (i.e. no filename)
+        Settings.getConfiguration().addProperty(PUB_CERT_KEY, ".");
+        TokenGenerator.getInstance().setPublicKeys();
+        // should be 1 public key total: none from disk & one from CN server
+        assertEquals(Arrays.toString(TokenGenerator.publicKeys.toArray()), 1,
+                     TokenGenerator.publicKeys.size());
+
+        Settings.getConfiguration().addProperty(PUB_CERT_KEY, "/");
+        TokenGenerator.getInstance().setPublicKeys();
+        // should be 1 public key total: none from disk & one from CN server
+        assertEquals(Arrays.toString(TokenGenerator.publicKeys.toArray()), 1,
+                     TokenGenerator.publicKeys.size());
+
+        // Value set to non-existent path
+        Settings.getConfiguration().addProperty(PUB_CERT_KEY, bogusLocalCert);
+        TokenGenerator.getInstance().setPublicKeys();
+        // should be 1 public key total: none from disk & one from CN server
+        assertEquals(Arrays.toString(TokenGenerator.publicKeys.toArray()), 1,
                      TokenGenerator.publicKeys.size());
 
+        // clean up
+        Settings.getConfiguration().setProperty(PUB_CERT_KEY, orig);
+    }
+
+    @Test
+    public void testSetPublicKeys_multipleCerts() throws Exception {
+
+        String orig = Settings.getConfiguration().getString(PUB_CERT_KEY);
+        String bogusLocalCert = "/tmp/nonExistentCert.pem";
+
         ///////////////////////////////////////////
         // Verify 1 present & 1 missing local cert
         ///////////////////////////////////////////
@@ -220,6 +257,7 @@ public void testSetPublicKeys_multipleCerts() throws Exception {
         assertEquals(Arrays.toString(TokenGenerator.publicKeys.toArray()), 2,
                      TokenGenerator.publicKeys.size());
 
+        // clean up
         Settings.getConfiguration().setProperty(PUB_CERT_KEY, orig);
     }