GH-306 Keyring API is now used to store JMX passwords

Author: thurka

visualvm/jmx/nbproject/project.xml

@@ -51,6 +51,14 @@
                         <specification-version>1.48</specification-version>
                     </run-dependency>
                 </dependency>
+                <dependency>
+                    <code-name-base>org.netbeans.modules.keyring</code-name-base>
+                    <build-prerequisite/>
+                    <compile-dependency/>
+                    <run-dependency>
+                        <specification-version>1.40</specification-version>
+                    </run-dependency>
+                </dependency>
                 <dependency>
                     <code-name-base>org.netbeans.modules.sendopts</code-name-base>
                     <build-prerequisite/>

visualvm/jmx/src/org/graalvm/visualvm/jmx/CredentialsProvider.java

@@ -32,6 +32,8 @@
 import org.graalvm.visualvm.application.Application;
 import org.graalvm.visualvm.core.datasource.Storage;
 import org.graalvm.visualvm.core.datasupport.Utils;
+import org.graalvm.visualvm.jmx.impl.JmxApplication;
+import org.netbeans.api.keyring.Keyring;
 
 /**
  * EnvironmentProvider adding the JMXConnector.CREDENTIALS property to the JMX
@@ -109,7 +111,7 @@ public Custom(String username, char[] password, boolean persistent) {
 
 
         public Map<String, ?> getEnvironment(Application application, Storage storage) {
-            return createMap(user, pword == null ? null : Arrays.copyOf(pword, pword.length));
+            return createMap(user, getPassword(storage));
         }
 
         public String getEnvironmentId(Storage storage) {
@@ -120,11 +122,24 @@ public String getEnvironmentId(Storage storage) {
         public void saveEnvironment(Storage storage) {
             if (!persistent) return;
             storage.setCustomProperty(PROPERTY_USER, user);
-            storage.setCustomProperty(PROPERTY_PWORD, new String(pword));
+            String keyringId = JmxApplication.createId(null, this, storage);
+            char[] pw = getPassword(storage);
+            if (pw != null) {
+                Keyring.save(keyringId, pw, "VisualVM - JMX password for "+user);    // NOI18N
+            } else {
+                Keyring.delete(keyringId);
+            }
         }
 
 
         String getUsername(Storage storage) { return user; }
+
+        private char[] getPassword(Storage storage) {
+            if (hasPassword(storage)) {
+                return decodePassword(Arrays.copyOf(pword, pword.length));
+            }
+            return null;
+        }
 
         boolean hasPassword(Storage storage) { return pword != null &&
                                                pword.length > 0; }
@@ -143,8 +158,7 @@ public static class Persistent extends CredentialsProvider {
 
         public Map<String, ?> getEnvironment(Application application, Storage storage) {
             String user = storage.getCustomProperty(PROPERTY_USER);
-            String pword = storage.getCustomProperty(PROPERTY_PWORD);
-            return createMap(user, pword == null ? null : pword.toCharArray());
+            return createMap(user, getPassword(storage));
         }
 
         public String getEnvironmentId(Storage storage) {
@@ -159,9 +173,27 @@ public String getEnvironmentId(Storage storage) {
         String getUsername(Storage storage) { return storage.getCustomProperty(
                                                      PROPERTY_USER); }
 
-        boolean hasPassword(Storage storage) {
+        private char[] getPassword(Storage storage) {
+            String keyringId = JmxApplication.createId(null, this, storage);
+            char[] pw = Keyring.read(keyringId);
+            if (pw != null) {
+                return pw;
+            }
+            // read old settings
             String pword = storage.getCustomProperty(PROPERTY_PWORD);
-            return pword != null && !pword.isEmpty();
+            if (pword != null && !pword.isEmpty()) {
+                // migrate old settings to Keyring
+                // Keyring.save(keyringId, decodePassword(pword.toCharArray()), "VisualVM - JMX password for "+getUsername(storage));       // NOI18N
+                // storage.clearCustomProperty(PROPERTY_PWORD);
+                //return getPassword(storage);
+                return decodePassword(pword.toCharArray());
+            }
+            return null;
+        }
+
+        boolean hasPassword(Storage storage) {
+            char[] pword = getPassword(storage);
+            return pword != null && pword.length>0;
         }
 
         boolean isPersistent(Storage storage) {
@@ -178,10 +210,9 @@ boolean isPersistent(Storage storage) {
         Map<String, Object>  map = new HashMap<>();
 
         if (username != null && !username.isEmpty()) {
-            map.put(JMXConnector.CREDENTIALS, new String[] { username, pword == null ? null : new String(decodePassword(pword)) });
-        } else {
-            if (pword != null) Arrays.fill(pword, (char)0);
+            map.put(JMXConnector.CREDENTIALS, new String[] { username, pword == null ? null : new String(pword) });
         }
+        if (pword != null) Arrays.fill(pword, (char)0);
 
         return map;
     }

visualvm/jmx/src/org/graalvm/visualvm/jmx/impl/JmxApplication.java

@@ -79,7 +79,7 @@ public final class JmxApplication extends Application {
     // Note: storage may be null, in this case the JmxApplication isn't persistent
     // and creates a temporary storage just like any other regular Application
     public JmxApplication(Host host, JMXServiceURL url, EnvironmentProvider envProvider, Storage storage) {
-        super(host, createId(url, envProvider, storage), STATE_UNAVAILABLE);
+        super(host, createId(url.toString(), envProvider, storage), STATE_UNAVAILABLE);
         this.url = url;
         this.envProvider = envProvider;
         this.storage = storage;
@@ -166,11 +166,11 @@ public String toString() {
         return "JmxApplication [id: " + getId() + "]";   // NOI18N
     }
 
-    private static String createId(JMXServiceURL url, EnvironmentProvider envProvider,
+    public static String createId(String urlId, EnvironmentProvider envProvider,
                                    Storage storage) {
-        // url.toString will always be used
-        String urlId = url.toString();
-        
+        if (urlId == null) {
+            urlId = storage.getCustomProperty(JmxApplicationProvider.PROPERTY_CONNECTION_STRING);
+        }
         // No envProvider -> return just url.toString()
         if (envProvider == null) return urlId;
 

visualvm/jmx/src/org/graalvm/visualvm/jmx/impl/JmxApplicationProvider.java

@@ -98,7 +98,7 @@ public class JmxApplicationProvider {
                                                 CURRENT_SNAPSHOT_VERSION_MINOR;
 
     public static final String PROPERTY_RETRY_WITHOUT_SSL = "prop_retry_without_ssl"; // NOI18N
-    private static final String PROPERTY_CONNECTION_STRING = "prop_conn_string";    // NOI18N
+    static final String PROPERTY_CONNECTION_STRING = "prop_conn_string";    // NOI18N
     private static final String PROPERTY_HOSTNAME = "prop_conn_hostname";   // NOI18N
     private static final String PROPERTY_ENV_PROVIDER_ID = "prop_env_provider_id"; // NOI18N