Merge pull request #9652 from JacobTrossing/fix-for-issue-8055

Protect proxy password by not storing it between sessions

CHANGELOG.md

@@ -46,6 +46,7 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve
 - We fixed an issue where the command line export using `--exportMatches` flag does not create an output bib file [#9581](https://github.com/JabRef/jabref/issues/9581)
 - We fixed an issue where custom field in the custom entry types could not be set to mulitline [#9609](https://github.com/JabRef/jabref/issues/9609)
 - We fixed an issue where the Office XML exporter did not resolve BibTeX-Strings when exporting entries [forum#3741](https://discourse.jabref.org/t/exporting-bibtex-constant-strings-to-ms-office-2007-xml/3741)
+- We fixed an issue where the password is saved locally if user wants to use proxy with authentication [#8055](https://github.com/JabRef/jabref/issues/8055)
 - JabRef is now more relaxed when parsing field content: In case a field content ended with `\`, the combination `\}` was treated as plain `}`. [#9668](https://github.com/JabRef/jabref/issues/9668)
 - We resolved an issue that cut off the number of group entries when it exceedet four digits. [#8797](https://github.com/JabRef/jabref/issues/8797)
 

src/main/java/org/jabref/gui/DialogService.java

@@ -20,6 +20,7 @@
 import org.jabref.gui.util.FileDialogConfiguration;
 import org.jabref.logic.l10n.Localization;
 
+import org.controlsfx.control.textfield.CustomPasswordField;
 import org.controlsfx.dialog.ProgressDialog;
 
 /**
@@ -161,6 +162,14 @@ boolean showConfirmationDialogWithOptOutAndWait(String title, String content,
                                                     String okButtonLabel, String cancelButtonLabel,
                                                     String optOutMessage, Consumer<Boolean> optOutAction);
 
+    /**
+     * This will create and display new {@link CustomPasswordField} that doesn't show the text, and two buttons
+     * one cancel and one ok.
+     *
+     * @return the entered password if pressed "OK", null otherwise
+     */
+    Optional<String> showPasswordDialogAndWait(String title, String header, String content);
+
     /**
      * Shows a custom dialog without returning any results.
      *

src/main/java/org/jabref/gui/JabRefDialogService.java

@@ -26,6 +26,7 @@
 import javafx.scene.control.DialogPane;
 import javafx.scene.control.Label;
 import javafx.scene.control.TextInputDialog;
+import javafx.scene.layout.HBox;
 import javafx.scene.layout.Pane;
 import javafx.scene.layout.Region;
 import javafx.scene.layout.VBox;
@@ -49,6 +50,7 @@
 import com.tobiasdiez.easybind.EasyBind;
 import org.controlsfx.control.Notifications;
 import org.controlsfx.control.TaskProgressView;
+import org.controlsfx.control.textfield.CustomPasswordField;
 import org.controlsfx.dialog.ExceptionDialog;
 import org.controlsfx.dialog.ProgressDialog;
 import org.slf4j.Logger;
@@ -270,6 +272,30 @@ public <R> Optional<R> showCustomDialogAndWait(javafx.scene.control.Dialog<R> di
         return dialog.showAndWait();
     }
 
+    @Override
+    public Optional<String> showPasswordDialogAndWait(String title, String header, String content) {
+        javafx.scene.control.Dialog<String> dialog = new javafx.scene.control.Dialog<>();
+        dialog.setTitle(title);
+        dialog.setHeaderText(header);
+
+        CustomPasswordField passwordField = new CustomPasswordField();
+
+        HBox box = new HBox();
+        box.setSpacing(10);
+        box.getChildren().addAll(new Label(content), passwordField);
+        dialog.setTitle(title);
+        dialog.getDialogPane().setContent(box);
+
+        dialog.getDialogPane().getButtonTypes().addAll(ButtonType.CANCEL, ButtonType.OK);
+        dialog.setResultConverter(dialogButton -> {
+            if (dialogButton == ButtonType.OK) {
+                return passwordField.getText();
+            }
+            return null;
+        });
+        return dialog.showAndWait();
+    }
+
     @Override
     public <V> void showProgressDialog(String title, String content, Task<V> task) {
         ProgressDialog progressDialog = new ProgressDialog(task);

src/main/java/org/jabref/gui/JabRefGUI.java

@@ -20,13 +20,15 @@
 import org.jabref.gui.shared.SharedDatabaseUIManager;
 import org.jabref.logic.importer.ParserResult;
 import org.jabref.logic.l10n.Localization;
+import org.jabref.logic.net.ProxyRegisterer;
 import org.jabref.logic.shared.DatabaseNotSupportedException;
 import org.jabref.logic.shared.exception.InvalidDBMSConnectionPropertiesException;
 import org.jabref.logic.shared.exception.NotASharedDatabaseException;
 import org.jabref.logic.util.WebViewStore;
 import org.jabref.preferences.GuiPreferences;
 import org.jabref.preferences.PreferencesService;
 
+import com.airhacks.afterburner.injection.Injector;
 import impl.org.controlsfx.skin.DecorationPane;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -61,6 +63,15 @@ public JabRefGUI(Stage mainStage, List<ParserResult> databases, boolean isBlank,
                 Globals.TASK_EXECUTOR,
                 preferencesService.getInternalPreferences())
                 .checkForNewVersionDelayed();
+
+        if (preferencesService.getProxyPreferences().shouldUseProxy() && preferencesService.getProxyPreferences().shouldUseAuthentication()) {
+            DialogService dialogService = Injector.instantiateModelOrService(DialogService.class);
+            dialogService.showPasswordDialogAndWait(Localization.lang("Proxy configuration"), Localization.lang("Proxy requires password"), Localization.lang("Password"))
+                .ifPresent(newPassword -> {
+                    preferencesService.getProxyPreferences().setPassword(newPassword);
+                    ProxyRegisterer.register(preferencesService.getProxyPreferences());
+                });
+        }
     }
 
     private void openWindow(Stage mainStage) {

src/main/java/org/jabref/preferences/JabRefPreferences.java

@@ -1564,14 +1564,13 @@ public ProxyPreferences getProxyPreferences() {
                 get(PROXY_PORT),
                 getBoolean(PROXY_USE_AUTHENTICATION),
                 get(PROXY_USERNAME),
-                get(PROXY_PASSWORD));
+                (String) defaults.get(PROXY_PASSWORD));
 
         EasyBind.listen(proxyPreferences.useProxyProperty(), (obs, oldValue, newValue) -> putBoolean(PROXY_USE, newValue));
         EasyBind.listen(proxyPreferences.hostnameProperty(), (obs, oldValue, newValue) -> put(PROXY_HOSTNAME, newValue));
         EasyBind.listen(proxyPreferences.portProperty(), (obs, oldValue, newValue) -> put(PROXY_PORT, newValue));
         EasyBind.listen(proxyPreferences.useAuthenticationProperty(), (obs, oldValue, newValue) -> putBoolean(PROXY_USE_AUTHENTICATION, newValue));
         EasyBind.listen(proxyPreferences.usernameProperty(), (obs, oldValue, newValue) -> put(PROXY_USERNAME, newValue));
-        EasyBind.listen(proxyPreferences.passwordProperty(), (obs, oldValue, newValue) -> put(PROXY_PASSWORD, newValue));
 
         return proxyPreferences;
     }

src/main/resources/l10n/JabRef_en.properties

@@ -1,3 +1,5 @@
+Proxy\ requires\ password=Proxy requires password
+
 Could\ not\ delete\ empty\ entries.=Could not delete empty entries.
 
 Delete\ empty\ entries=Delete empty entries

src/test/java/org/jabref/logic/net/ProxyTest.java

@@ -0,0 +1,37 @@
+package org.jabref.logic.net;
+
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+public class ProxyTest {
+   /**
+    * The test checks if ProxyPreference class is still able to store password and use it from memory,
+    * even though it's no longer stored in register.
+    */
+   @Test
+   public void testProxyPreferencesStorePassword() {
+       // mock data
+       Boolean useProxy = true;
+       String hostname = "testName";
+       String port = "8080";
+       Boolean useAuthentication = true;
+       String username = "testUserName";
+       String password = "testPassword";
+       // Creates proxy preference
+       ProxyPreferences proxyPref = new ProxyPreferences(
+               useProxy,
+               hostname,
+               port,
+               useAuthentication,
+               username,
+               password);
+       // Check if mock data is stored in object memory and can be extracted
+       assertEquals(proxyPref.shouldUseProxy(), true);
+       assertEquals(proxyPref.getHostname(), hostname);
+       assertEquals(proxyPref.getPort(), port);
+       assertEquals(proxyPref.shouldUseAuthentication(), true);
+       assertEquals(proxyPref.getUsername(), username);
+       assertEquals(proxyPref.getPassword(), password);
+   }
+}