From 197e43e3b7bc447d36ebdd690f1e8133b4f6262b Mon Sep 17 00:00:00 2001 From: Ben Soares Date: Wed, 29 Jul 2020 12:44:24 +0100 Subject: [PATCH] JAL-3275 JAL-3633: JAL-3275 Turned Preferences into a Singleton that checks renews if it's been closed. Removed some debugging for JAL-3633. Added focus/highlight to proxy password box when needed. Forced WS lookup wheb proxy settings changed. --- src/jalview/bin/Cache.java | 127 +++++++++++++++++------------- src/jalview/gui/Desktop.java | 2 +- src/jalview/gui/Preferences.java | 70 ++++++++++++----- src/jalview/gui/WsPreferences.java | 17 ++-- src/jalview/jbgui/GPreferences.java | 145 ++++++++++++++++++++++++++--------- 5 files changed, 243 insertions(+), 118 deletions(-) diff --git a/src/jalview/bin/Cache.java b/src/jalview/bin/Cache.java index 8269f32..e96fef1 100755 --- a/src/jalview/bin/Cache.java +++ b/src/jalview/bin/Cache.java @@ -34,6 +34,7 @@ import java.net.PasswordAuthentication; import java.net.URL; import java.text.DateFormat; import java.text.SimpleDateFormat; +import java.util.Arrays; import java.util.Collections; import java.util.Date; import java.util.Enumeration; @@ -54,7 +55,6 @@ import org.apache.log4j.SimpleLayout; import jalview.datamodel.PDBEntry; import jalview.gui.Preferences; import jalview.gui.UserDefinedColours; -import jalview.jbgui.GPreferences; import jalview.schemes.ColourSchemeLoader; import jalview.schemes.ColourSchemes; import jalview.schemes.UserColourScheme; @@ -1316,17 +1316,28 @@ public class Cache { String proxyType = Cache.getDefault("USE_PROXY", Cache.PROXYTYPE_SYSTEM); - if (proxyType.equals(Cache.PROXYTYPE_NONE)) + if (previousProxyType != null + && !proxyType.equals(Cache.PROXYTYPE_CUSTOM) // always apply + // customProxy + && proxyType.equals(previousProxyType)) { - if (!previousProxyType.equals(proxyType)) - Cache.log.info("Setting no proxy settings"); - Cache.setProxyProperties(null, null, null, null, null, null, null, - null, null); + // no change + return; } - else if (proxyType.equals(Cache.PROXYTYPE_CUSTOM)) + switch (proxyType) { + case Cache.PROXYTYPE_NONE: if (!previousProxyType.equals(proxyType)) - Cache.log.info("Setting custom proxy settings"); + { + Cache.log.info("Setting no proxy settings"); + Cache.setProxyProperties(null, null, null, null, null, null, null, + null, null); + } + break; + case Cache.PROXYTYPE_CUSTOM: + // always re-set a custom proxy -- it might have changed, particularly + // password + Cache.log.info("Setting custom proxy settings"); boolean proxyAuthSet = Cache.getDefault("PROXY_AUTH", false); Cache.setProxyProperties(Cache.getDefault("PROXY_SERVER", null), Cache.getDefault("PROXY_PORT", null), @@ -1338,11 +1349,9 @@ public class Cache proxyAuthSet ? Cache.getDefault("PROXY_AUTH_USERNAME", "") : null, proxyAuthSet ? Cache.proxyAuthPassword : null, "localhost"); - } - else // systemProxy should be selected and is sensible default anyway - { - if (!previousProxyType.equals(proxyType)) - Cache.log.info("Setting system proxy settings"); + break; + default: // system proxy settings by default + Cache.log.info("Setting system proxy settings"); Cache.resetProxyProperties(); } } @@ -1357,39 +1366,54 @@ public class Cache setOrClearSystemProperty("https.proxyHost", httpsHost); setOrClearSystemProperty("https.proxyPort", httpsPort); setOrClearSystemProperty("http.proxyUser", httpUser); - setOrClearSystemProperty("http.proxyPassword", httpPassword); setOrClearSystemProperty("https.proxyUser", httpsUser); - setOrClearSystemProperty("https.proxyPassword", httpsPassword); + // note: passwords for http.proxyPassword and https.proxyPassword are sent + // via the Authenticator, properties do not need to be set + // are we using a custom proxy (password prompt might be required)? boolean customProxySet = getDefault("USE_PROXY", PROXYTYPE_SYSTEM) .equals(PROXYTYPE_CUSTOM); + + /* + * A bug in Java means the AuthCache does not get reset, so once it has working credentials, + * it never asks for more, so changing the Authenticator has no effect (as getPasswordAuthentication() + * is not re-called). + * This could lead to password leak to a hostile proxy server, so I'm putting in a hack to clear + * the AuthCache. + * see https://www.generacodice.com/en/articolo/154918/Reset-the-Authenticator-credentials + * ... + * Turns out this is only accessible in Java 8, and not in Java 9 onwards, so commenting out + */ + /* + try + { + sun.net.www.protocol.http.AuthCacheValue + .setAuthCache(new sun.net.www.protocol.http.AuthCacheImpl()); + } catch (Throwable t) + { + Cache.error(t.getMessage()); + Cache.debug(getStackTraceString(t)); + } + */ + if (httpUser != null || httpsUser != null) { try { + Cache.debug("CACHE Proxy: setting new Authenticator with httpUser='" + + httpUser + "' httpPassword='" + + (proxyAuthPassword == null ? "null" + : new String(proxyAuthPassword)) + + "'"); // DELETE THIS LINE (password in logs) Authenticator.setDefault(new Authenticator() { @Override protected PasswordAuthentication getPasswordAuthentication() { - Cache.debug( - "*** START PasswordAuthentication.getPasswordAuthentication()"); - Cache.debug("*** getRequestorType()=" + getRequestorType()); if (getRequestorType() == RequestorType.PROXY) { String protocol = getRequestingProtocol(); boolean needProxyPasswordSet = false; - Cache.debug("*** customProxySet = " + customProxySet); - Cache.debug("*** protocol = " + protocol); - Cache.debug("*** httpUser = " + httpUser); - Cache.debug( - "*** httpPassword = \"" + (httpPassword == null ? null - : new String(httpPassword)) + "\""); - Cache.debug("*** httpsUser = " + httpsUser); - Cache.debug("*** httpsPassword = \"" - + (httpsPassword == null ? null - : new String(httpsPassword)) - + "\""); if (customProxySet && // we have a username but no password for the scheme being // requested @@ -1406,13 +1430,12 @@ public class Cache // open Preferences -> Connections String message = MessageManager .getString("label.proxy_password_required"); - Cache.debug("***+ TRYING TO OPEN PREFERENCES"); - openPreferencesConnectionsForProxyPassword(message); - Cache.debug("***+ AFTER TRYING TO OPEN PREFERENCES"); + Preferences.openPreferences(Preferences.CONNECTIONS_TAB, + message); + Preferences.getPreferences().proxyAuthPasswordFocus(); } else { - Cache.debug("***+ TRYING TO GET PASSWORDAUTHENTICATION"); try { if (protocol.equalsIgnoreCase("http") @@ -1420,9 +1443,12 @@ public class Cache && getRequestingPort() == Integer .valueOf(httpPort)) { - Cache.debug("***+ RETURNING PasswordAuthentication(\"" - + httpUser + "\", \"" + new String(httpPassword) - + "\""); + char[] displayHttpPw = new char[httpPassword.length]; + Arrays.fill(displayHttpPw, '*'); + Cache.debug( + "AUTHENTICATOR returning PasswordAuthentication(\"" + + httpUser + "\", '" + + new String(displayHttpPw) + "')"); return new PasswordAuthentication(httpUser, httpPassword); } @@ -1431,8 +1457,12 @@ public class Cache && getRequestingPort() == Integer .valueOf(httpsPort)) { - Cache.debug("***+ RETURNING PasswordAuthentication(\"" - + httpsUser + "\", \"" + httpsPassword + "\""); + char[] displayHttpsPw = new char[httpPassword.length]; + Arrays.fill(displayHttpsPw, '*'); + Cache.debug( + "AUTHENTICATOR returning PasswordAuthentication(\"" + + httpsUser + "\", '" + displayHttpsPw + + "'"); return new PasswordAuthentication(httpsUser, httpsPassword); } @@ -1442,18 +1472,18 @@ public class Cache + httpPort + ", https:" + httpsPort + "]"); } Cache.debug( - "***+ AFTER TRYING TO GET PASSWORDAUTHENTICATION"); + "AUTHENTICATOR after trying to get PasswordAuthentication"); } } // non proxy request - Cache.debug("***+ Returning null"); + Cache.debug("AUTHENTICATOR returning null"); return null; } }); // required to re-enable basic authentication (should be okay for a // local proxy) Cache.debug( - "***+ Setting jdk.http.auth.tunneling.disabledSchemes to ''"); + "AUTHENTICATOR setting property 'jdk.http.auth.tunneling.disabledSchemes' to \"\""); System.setProperty("jdk.http.auth.tunneling.disabledSchemes", ""); } catch (SecurityException e) { @@ -1465,27 +1495,16 @@ public class Cache { // reset the Authenticator to protect http.proxyUser and // http.proxyPassword Just In Case - Cache.debug("***+ Setting default Authenticator to null"); + Cache.debug("AUTHENTICATOR setting default Authenticator to null"); Authenticator.setDefault(null); } // nonProxyHosts not currently configurable in Preferences - Cache.debug("***+ Setting http.nonProxyHosts property to \"" + Cache.debug("AUTHENTICATOR setting property 'http.nonProxyHosts' to \"" + nonProxyHosts + "\""); setOrClearSystemProperty("http.nonProxyHosts", nonProxyHosts); } - private static void openPreferencesConnectionsForProxyPassword( - String message) - { - // - Cache.info("Opening Preferences for proxy password"); - // Desktop.instance.preferences_actionPerformed(null); - Cache.debug("***+########## TRYING TO OPEN PREFERENCES: " + message); - Preferences p = new Preferences(GPreferences.CONNECTIONS_TAB, message); - p.grabFocus(); - } - public static void setOrClearSystemProperty(String key, char[] value) { setOrClearSystemProperty(key, diff --git a/src/jalview/gui/Desktop.java b/src/jalview/gui/Desktop.java index 37d542f..67d273c 100644 --- a/src/jalview/gui/Desktop.java +++ b/src/jalview/gui/Desktop.java @@ -1625,7 +1625,7 @@ public class Desktop extends jalview.jbgui.GDesktop @Override protected void preferences_actionPerformed(ActionEvent e) { - new Preferences(); + Preferences.openPreferences(); } /** diff --git a/src/jalview/gui/Preferences.java b/src/jalview/gui/Preferences.java index da33f39..65b333e 100755 --- a/src/jalview/gui/Preferences.java +++ b/src/jalview/gui/Preferences.java @@ -129,6 +129,8 @@ public class Preferences extends GPreferences private String previousProxyType; + private static Preferences INSTANCE = null; // add "final" + /** * Holds name and link separated with | character. Sequence ID must be * $SEQUENCE_ID$ or $SEQUENCE_ID=/.possible | chars ./=$ @@ -188,22 +190,43 @@ public class Preferences extends GPreferences private OptionsParam textOpt = new OptionsParam( MessageManager.getString("action.text"), "Text"); - /** - * Creates a new Preferences object. - */ - public Preferences() + // get singleton Preferences instance + public static Preferences getPreferences() + { + return getPreferences(0, null); + } + + public static Preferences getPreferences(int selectTab, String message) { - new Preferences(0); + if (INSTANCE == null || INSTANCE.frame == null + || INSTANCE.frame.isClosed()) + { + INSTANCE = new Preferences(); + } + INSTANCE.selectTab(selectTab); + INSTANCE.setMessage(message); + return INSTANCE; } - public Preferences(int selectTab) + public static void openPreferences() { - new Preferences(selectTab, null); + openPreferences(0, null); } - public Preferences(int selectTab, String message) + public static void openPreferences(int selectTab, String message) { - super(selectTab, message); + Preferences p = getPreferences(selectTab, message); + p.frame.show(); + p.frame.moveToFront(); + p.frame.grabFocus(); + } + + /** + * Creates a new Preferences object. + */ + private Preferences() + { + super(); frame = new JInternalFrame(); frame.setContentPane(this); if (!Platform.isJS()) @@ -574,7 +597,7 @@ public class Preferences extends GPreferences proxyAuthUsernameTB .setText(Cache.getDefault("PROXY_AUTH_USERNAME", "")); // we are not storing or retrieving proxy password from .jalview_properties - proxyAuthPasswordTB.setText(Cache.proxyAuthPassword == null ? "" + proxyAuthPasswordPB.setText(Cache.proxyAuthPassword == null ? "" : new String(Cache.proxyAuthPassword)); setCustomProxyEnabled(); @@ -675,6 +698,11 @@ public class Preferences extends GPreferences return; } + /* + * Set proxy settings first (to be before web services refresh) + */ + saveProxySettings(); + /* * Save Visual settings */ @@ -818,6 +846,8 @@ public class Preferences extends GPreferences /* * Save Connections settings */ + // Proxy settings set first (to catch web services) + Cache.setOrRemove("DEFAULT_BROWSER", defaultBrowser.getText()); jalview.util.BrowserLauncher.resetBrowser(); @@ -848,9 +878,6 @@ public class Preferences extends GPreferences Cache.applicationProperties.setProperty("DEFAULT_URL", sequenceUrlLinks.getPrimaryUrlId()); - // Proxy settings - saveProxySettings(); - Cache.setProperty("VERSION_CHECK", Boolean.toString(versioncheck.isSelected())); if (Cache.getProperty("USAGESTATS") != null || usagestats.isSelected()) @@ -949,10 +976,10 @@ public class Preferences extends GPreferences public void saveProxySettings() { - Cache.applicationProperties.setProperty("USE_PROXY", - customProxy.isSelected() ? Cache.PROXYTYPE_CUSTOM - : noProxy.isSelected() ? Cache.PROXYTYPE_NONE - : Cache.PROXYTYPE_SYSTEM); + String newProxyType = customProxy.isSelected() ? Cache.PROXYTYPE_CUSTOM + : noProxy.isSelected() ? Cache.PROXYTYPE_NONE + : Cache.PROXYTYPE_SYSTEM; + Cache.applicationProperties.setProperty("USE_PROXY", newProxyType); Cache.setOrRemove("PROXY_SERVER", proxyServerHttpTB.getText()); Cache.setOrRemove("PROXY_PORT", proxyPortHttpTB.getText()); Cache.setOrRemove("PROXY_SERVER_HTTPS", proxyServerHttpsTB.getText()); @@ -960,8 +987,15 @@ public class Preferences extends GPreferences Cache.setOrRemove("PROXY_AUTH", Boolean.toString(proxyAuth.isSelected())); Cache.setOrRemove("PROXY_AUTH_USERNAME", proxyAuthUsernameTB.getText()); - Cache.proxyAuthPassword = proxyAuthPasswordTB.getPassword(); + Cache.proxyAuthPassword = proxyAuthPasswordPB.getPassword(); Cache.setProxyPropertiesFromPreferences(previousProxyType); + if (newProxyType.equals(Cache.PROXYTYPE_CUSTOM) + || !newProxyType.equals(previousProxyType)) + { + // force a re-lookup of ws if proxytype is custom or has changed + wsPrefs.update++; + } + previousProxyType = newProxyType; } /** diff --git a/src/jalview/gui/WsPreferences.java b/src/jalview/gui/WsPreferences.java index 5186a26..e37f77c 100644 --- a/src/jalview/gui/WsPreferences.java +++ b/src/jalview/gui/WsPreferences.java @@ -20,12 +20,6 @@ */ package jalview.gui; -import jalview.bin.Cache; -import jalview.jbgui.GWsPreferences; -import jalview.util.MessageManager; -import jalview.ws.jws2.Jws2Discoverer; -import jalview.ws.rest.RestServiceDescription; - import java.awt.BorderLayout; import java.awt.Color; import java.awt.Component; @@ -37,13 +31,18 @@ import java.util.List; import java.util.Vector; import javax.swing.JLabel; -import javax.swing.JOptionPane; import javax.swing.JPanel; import javax.swing.JTable; import javax.swing.JTextField; import javax.swing.table.AbstractTableModel; import javax.swing.table.TableCellRenderer; +import jalview.bin.Cache; +import jalview.jbgui.GWsPreferences; +import jalview.util.MessageManager; +import jalview.ws.jws2.Jws2Discoverer; +import jalview.ws.rest.RestServiceDescription; + public class WsPreferences extends GWsPreferences { @@ -634,7 +633,9 @@ public class WsPreferences extends GWsPreferences /** * state counters for ensuring that updates only happen if config has changed. */ - private long update = 0, lastrefresh = 0; + protected long update = 0; + + private long lastrefresh = 0; /* * (non-Javadoc) diff --git a/src/jalview/jbgui/GPreferences.java b/src/jalview/jbgui/GPreferences.java index fb0b2a4..ab60f90 100755 --- a/src/jalview/jbgui/GPreferences.java +++ b/src/jalview/jbgui/GPreferences.java @@ -69,6 +69,8 @@ import javax.swing.border.EtchedBorder; import javax.swing.border.TitledBorder; import javax.swing.event.ChangeEvent; import javax.swing.event.ChangeListener; +import javax.swing.event.DocumentEvent; +import javax.swing.event.DocumentListener; import javax.swing.table.TableCellEditor; import javax.swing.table.TableCellRenderer; @@ -262,7 +264,7 @@ public class GPreferences extends JPanel protected JTextField proxyAuthUsernameTB = new JTextField(); - protected JPasswordField proxyAuthPasswordTB = new JPasswordField(); + protected JPasswordField proxyAuthPasswordPB = new JPasswordField(); protected JTextField defaultBrowser = new JTextField(); @@ -274,6 +276,8 @@ public class GPreferences extends JPanel protected JRadioButton customProxy = new JRadioButton(); + protected JButton applyProxyButton = new JButton(); + protected JCheckBox usagestats = new JCheckBox(); protected JCheckBox questionnaire = new JCheckBox(); @@ -373,24 +377,18 @@ public class GPreferences extends JPanel protected JTextArea backupfilesExampleLabel = new JTextArea(); + private final JTabbedPane tabbedPane = new JTabbedPane(); + + private JLabel messageLabel = new JLabel("", JLabel.CENTER); + /** * Creates a new GPreferences object. */ public GPreferences() { - new GPreferences(0); - } - - public GPreferences(int selectTab) - { - new GPreferences(selectTab, null); - } - - public GPreferences(int selectTab, String message) - { try { - jbInit(selectTab, message); + jbInit(); } catch (Exception ex) { ex.printStackTrace(); @@ -404,28 +402,11 @@ public class GPreferences extends JPanel */ private void jbInit() throws Exception { - jbInit(0); - } - - private void jbInit(int selectTab) throws Exception - { - jbInit(selectTab, null); - } - - public final static int CONNECTIONS_TAB = 5; - - private void jbInit(int selectTab, String message) throws Exception - { - final JTabbedPane tabbedPane = new JTabbedPane(); + // final JTabbedPane tabbedPane = new JTabbedPane(); this.setLayout(new BorderLayout()); - if (message != null) - { - JLabel messageLabel = new JLabel(message, JLabel.CENTER); - messageLabel.setFont(LABEL_FONT_BOLD); - messageLabel.setForeground(Color.RED.darker()); - this.add(messageLabel, BorderLayout.NORTH); - } + // message label at top + this.add(messageLabel, BorderLayout.NORTH); JPanel okCancelPanel = initOkCancelPanel(); this.add(tabbedPane, BorderLayout.CENTER); @@ -473,6 +454,7 @@ public class GPreferences extends JPanel /* * Handler to validate a tab before leaving it - currently only for * Structure. + * Adding a clearMessage() so messages are cleared when changing tabs. */ tabbedPane.addChangeListener(new ChangeListener() { @@ -491,10 +473,47 @@ public class GPreferences extends JPanel } } lastTab = tabbedPane.getSelectedComponent(); + + clearMessage(); } }); + } + public void setMessage(String message) + { + if (message != null) + { + messageLabel.setText(message); + messageLabel.setFont(LABEL_FONT_BOLD); + messageLabel.setForeground(Color.RED.darker()); + messageLabel.revalidate(); + messageLabel.repaint(); + } + // note message not cleared if message is null. call clearMessage() + // directly. + this.revalidate(); + this.repaint(); + } + + public void clearMessage() + { + // only repaint if message exists + if (messageLabel.getText() != null + && messageLabel.getText().length() > 0) + { + messageLabel.setText(""); + messageLabel.revalidate(); + messageLabel.repaint(); + this.revalidate(); + this.repaint(); + } + } + + public final static int CONNECTIONS_TAB = 5; + + public void selectTab(int selectTab) + { // select a given tab - currently only for Connections switch (selectTab) { @@ -503,7 +522,6 @@ public class GPreferences extends JPanel break; default: } - } /** @@ -972,8 +990,36 @@ public class GPreferences extends JPanel proxyPortHttpsTB.setColumns(4); proxyAuthUsernameTB.setFont(LABEL_FONT); proxyAuthUsernameTB.setColumns(30); - proxyAuthPasswordTB.setFont(LABEL_FONT); - proxyAuthPasswordTB.setColumns(30); + proxyAuthPasswordPB.setFont(LABEL_FONT); + proxyAuthPasswordPB.setColumns(30); + proxyAuthPasswordPB.getDocument() + .addDocumentListener(new DocumentListener() + { + @Override + public void changedUpdate(DocumentEvent e) + { + if (!proxyAuthPasswordPB.getBackground() + .equals(Color.WHITE)) + proxyAuthPasswordPB.setBackground(Color.WHITE); + } + + @Override + public void insertUpdate(DocumentEvent e) + { + if (!proxyAuthPasswordPB.getBackground() + .equals(Color.WHITE)) + proxyAuthPasswordPB.setBackground(Color.WHITE); + } + + @Override + public void removeUpdate(DocumentEvent e) + { + if (!proxyAuthPasswordPB.getBackground() + .equals(Color.WHITE)) + proxyAuthPasswordPB.setBackground(Color.WHITE); + } + + }); // Label for Port text box portLabel.setFont(LABEL_FONT); @@ -1165,7 +1211,7 @@ public class GPreferences extends JPanel c.gridx++; c.weightx = 1.0; c.anchor = GridBagConstraints.LINE_START; - upPanel.add(proxyAuthPasswordTB, c); + upPanel.add(proxyAuthPasswordPB, c); c.gridx++; c.weightx = 0.4; @@ -1175,9 +1221,34 @@ public class GPreferences extends JPanel gbc.gridy++; proxyPanel.add(upPanel, gbc); + applyProxyButton.setText(MessageManager.getString("action.apply")); + applyProxyButton.addActionListener(new ActionListener() + { + @Override + public void actionPerformed(ActionEvent e) + { + saveProxySettings(); + } + }); + gbc.gridy++; + gbc.fill = GridBagConstraints.NONE; + gbc.anchor = GridBagConstraints.LINE_END; + proxyPanel.add(applyProxyButton, gbc); + return proxyPanel; } + public void proxyAuthPasswordFocus() + { + proxyAuthPasswordPB.grabFocus(); + proxyAuthPasswordPB.setBackground(Color.PINK); + } + + public void saveProxySettings() + { + // overridden in Preferences + } + private String displayUserHostPort(String user, String host, String port) { boolean hostBlank = (host == null || host.isEmpty()); @@ -3096,7 +3167,7 @@ public class GPreferences extends JPanel proxyAuthPasswordLabel.setEnabled(enabled); passwordNotStoredLabel.setEnabled(enabled); proxyAuthUsernameTB.setEnabled(enabled); - proxyAuthPasswordTB.setEnabled(enabled); + proxyAuthPasswordPB.setEnabled(enabled); } public void setCustomProxyEnabled() -- 1.7.10.2