Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Store proxy password and apikeys in native OS credential store #10044

Merged
merged 23 commits into from
Jul 1, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve
- We enabled the user to change the name of a field in a custom entry type by double-clicking on it. [#9840](https://github.com/JabRef/jabref/issues/9840)
- We integrated two mail actions ("As Email" and "To Kindle") under a new "Send" option in the right-click & Tools menus. The Kindle option creates an email targeted to the user's Kindle email, which can be set in preferences under "External programs" [#6186](https://github.com/JabRef/jabref/issues/6186)
- We added an option to clear recent libraries' history. [#10003](https://github.com/JabRef/jabref/issues/10003)
- We added an option to encrypt and remember the proxy password. [#8055](https://github.com/JabRef/jabref/issues/8055)[#10044](https://github.com/JabRef/jabref/issues/10044)

### Changed

Expand Down Expand Up @@ -70,6 +71,7 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve
- We improved the error message when no terminal was found [#9607](https://github.com/JabRef/jabref/issues/9607)
- In the context of the "systematic literature functionality", we changed the name "database" to "catalog" to use a separate term for online catalogs in comparison to SQL databases. [#9951](https://github.com/JabRef/jabref/pull/9951)
- We now show more fields (including Special Fields) in the dropdown selection for "Save sort order" in the library properties and for "Export sort order" in the preferences. [#10010](https://github.com/JabRef/jabref/issues/10010)
- We now encrypt and store the custom API keys in the OS native credential store. [#10044](https://github.com/JabRef/jabref/issues/10044)

### Fixed

Expand All @@ -81,7 +83,7 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve
- 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 Merge Entries Toolbar configuration was not saved after hitting 'Merge Entries' button. [#9091](https://github.com/JabRef/jabref/issues/9091)
- 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)
- We fixed an issue where the password is stored in clear text if the user wants to use a 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 exceeded four digits. [#8797](https://github.com/JabRef/jabref/issues/8797)
- We fixed the issue where the size of the global search window was not retained after closing. [#9362](https://github.com/JabRef/jabref/issues/9362)
Expand Down
1 change: 1 addition & 0 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ dependencies {

implementation 'io.github.java-diff-utils:java-diff-utils:4.12'
implementation 'info.debatty:java-string-similarity:2.0.0'
implementation 'com.github.javakeyring:java-keyring:1.0.2'

antlr4 'org.antlr:antlr4:4.13.0'
implementation 'org.antlr:antlr4-runtime:4.13.0'
Expand Down
2 changes: 2 additions & 0 deletions src/main/java/module-info.java
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,8 @@

requires com.h2database.mvstore;

requires java.keyring;

requires org.jooq.jool;

// fulltext search
Expand Down
35 changes: 28 additions & 7 deletions src/main/java/org/jabref/gui/JabRefGUI.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import java.sql.SQLException;
import java.util.ArrayList;
import java.util.List;
import java.util.Optional;
import java.util.stream.Collectors;

import javafx.application.Platform;
Expand All @@ -25,6 +26,7 @@
import org.jabref.logic.shared.exception.InvalidDBMSConnectionPropertiesException;
import org.jabref.logic.shared.exception.NotASharedDatabaseException;
import org.jabref.logic.util.WebViewStore;
import org.jabref.model.strings.StringUtil;
import org.jabref.model.util.FileUpdateMonitor;
import org.jabref.preferences.GuiPreferences;
import org.jabref.preferences.PreferencesService;
Expand Down Expand Up @@ -71,13 +73,32 @@ public JabRefGUI(Stage mainStage,
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());
});
setupProxy();
}

private void setupProxy() {
if (!preferencesService.getProxyPreferences().shouldUseProxy()
|| !preferencesService.getProxyPreferences().shouldUseAuthentication()) {
return;
}

if (preferencesService.getProxyPreferences().shouldPersistPassword()
&& StringUtil.isNotBlank(preferencesService.getProxyPreferences().getPassword())) {
ProxyRegisterer.register(preferencesService.getProxyPreferences());
return;
}

DialogService dialogService = Injector.instantiateModelOrService(DialogService.class);
Optional<String> password = dialogService.showPasswordDialogAndWait(
Localization.lang("Proxy configuration"),
Localization.lang("Proxy requires password"),
Localization.lang("Password"));

if (password.isPresent()) {
preferencesService.getProxyPreferences().setPassword(password.get());
ProxyRegisterer.register(preferencesService.getProxyPreferences());
} else {
LOGGER.warn("No proxy password specified");
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@
<CheckBox fx:id="inspectionWarningDuplicate"
text="%Warn about unresolved duplicates when closing inspection window"/>
<CheckBox fx:id="confirmDelete" text="%Show confirmation dialog when deleting entries"/>

<CheckBox fx:id="collectTelemetry" text="%Collect and share telemetry data to help improve JabRef"/>

<Label styleClass="sectionHeader" text="%Libraries"/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
<TextField fx:id="proxyUsername" prefWidth="200.0" GridPane.columnIndex="1" GridPane.rowIndex="4" />
<Label fx:id="proxyPasswordLabel" text="%Password" GridPane.rowIndex="5" />
<CustomPasswordField fx:id="proxyPassword" prefWidth="200.0" GridPane.columnIndex="1" GridPane.rowIndex="5" />
<Label fx:id="proxyAttentionLabel" styleClass="info-message" text="%Password kept for this session only." GridPane.columnIndex="2" GridPane.rowIndex="5" />
<CheckBox fx:id="proxyPersistPassword" text="%Persist password between sessions" GridPane.columnIndex="2" GridPane.rowIndex="5"/>
<Button fx:id="checkConnectionButton" onAction="#checkConnection" prefWidth="200.0" text="%Check connection" GridPane.columnIndex="1" GridPane.rowIndex="6" />
</GridPane>
<Label styleClass="sectionHeader" text="%SSL Configuration" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,8 @@ public class NetworkTab extends AbstractPreferenceTabView<NetworkTabViewModel> i
@FXML private TextField proxyUsername;
@FXML private Label proxyPasswordLabel;
@FXML private CustomPasswordField proxyPassword;
@FXML private Label proxyAttentionLabel;
@FXML private Button checkConnectionButton;
@FXML private CheckBox proxyPersistPassword;

@FXML private TableView<CustomCertificateViewModel> customCertificatesTable;
@FXML private TableColumn<CustomCertificateViewModel, String> certIssuer;
Expand Down Expand Up @@ -105,7 +105,8 @@ public void initialize() {
proxyPasswordLabel.disableProperty().bind(proxyCustomAndAuthentication.not());
proxyPassword.textProperty().bindBidirectional(viewModel.proxyPasswordProperty());
proxyPassword.disableProperty().bind(proxyCustomAndAuthentication.not());
proxyAttentionLabel.disableProperty().bind(proxyCustomAndAuthentication.not());
proxyPersistPassword.selectedProperty().bindBidirectional(viewModel.proxyPersistPasswordProperty());
proxyPersistPassword.disableProperty().bind(proxyCustomAndAuthentication.not());

proxyPassword.setRight(IconTheme.JabRefIcons.PASSWORD_REVEALED.getGraphicNode());
proxyPassword.getRight().addEventFilter(MouseEvent.MOUSE_PRESSED, this::proxyPasswordReveal);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import java.nio.file.Path;
import java.util.Collections;
import java.util.List;
import java.util.Objects;
import java.util.Optional;
import java.util.concurrent.atomic.AtomicBoolean;

Expand Down Expand Up @@ -43,12 +42,8 @@
import de.saxsys.mvvmfx.utils.validation.ValidationStatus;
import de.saxsys.mvvmfx.utils.validation.Validator;
import kong.unirest.UnirestException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

public class NetworkTabViewModel implements PreferenceTabViewModel {
private static final Logger LOGGER = LoggerFactory.getLogger(NetworkTabViewModel.class);

private final BooleanProperty remoteServerProperty = new SimpleBooleanProperty();
private final StringProperty remotePortProperty = new SimpleStringProperty("");
private final BooleanProperty proxyUseProperty = new SimpleBooleanProperty();
Expand All @@ -57,6 +52,7 @@ public class NetworkTabViewModel implements PreferenceTabViewModel {
private final BooleanProperty proxyUseAuthenticationProperty = new SimpleBooleanProperty();
private final StringProperty proxyUsernameProperty = new SimpleStringProperty("");
private final StringProperty proxyPasswordProperty = new SimpleStringProperty("");
private final BooleanProperty proxyPersistPasswordProperty = new SimpleBooleanProperty();
private final ListProperty<CustomCertificateViewModel> customCertificateListProperty = new SimpleListProperty<>(FXCollections.observableArrayList());

private final Validator remotePortValidator;
Expand Down Expand Up @@ -92,7 +88,8 @@ public NetworkTabViewModel(DialogService dialogService, PreferencesService prefe
proxyPreferences.getPort(),
proxyPreferences.shouldUseAuthentication(),
proxyPreferences.getUsername(),
proxyPreferences.getPassword());
proxyPreferences.getPassword(),
proxyPreferences.shouldPersistPassword());

remotePortValidator = new FunctionBasedValidator<>(
remotePortProperty,
Expand Down Expand Up @@ -140,6 +137,7 @@ public NetworkTabViewModel(DialogService dialogService, PreferencesService prefe
Localization.lang("Network"),
Localization.lang("Proxy configuration"),
Localization.lang("Please specify a password"))));

this.trustStoreManager = new TrustStoreManager(Path.of(sslPreferences.getTruststorePath()));
}

Expand All @@ -159,6 +157,7 @@ private void setProxyValues() {
proxyUseAuthenticationProperty.setValue(proxyPreferences.shouldUseAuthentication());
proxyUsernameProperty.setValue(proxyPreferences.getUsername());
proxyPasswordProperty.setValue(proxyPreferences.getPassword());
proxyPersistPasswordProperty.setValue(proxyPreferences.shouldPersistPassword());
}

private void setSSLValues() {
Expand All @@ -181,24 +180,6 @@ private void setSSLValues() {

@Override
public void storeSettings() {
storeRemoteSettings();
storeProxySettings(new ProxyPreferences(
proxyUseProperty.getValue(),
proxyHostnameProperty.getValue().trim(),
proxyPortProperty.getValue().trim(),
proxyUseAuthenticationProperty.getValue(),
proxyUsernameProperty.getValue().trim(),
proxyPasswordProperty.getValue()
));
storeSSLSettings();
}

private void storeRemoteSettings() {
RemotePreferences newRemotePreferences = new RemotePreferences(
remotePreferences.getPort(),
remoteServerProperty.getValue()
);

getPortAsInt(remotePortProperty.getValue()).ifPresent(newPort -> {
if (remotePreferences.isDifferentPort(newPort)) {
remotePreferences.setPort(newPort);
Expand All @@ -212,25 +193,16 @@ private void storeRemoteSettings() {
remotePreferences.setUseRemoteServer(false);
Globals.REMOTE_LISTENER.stop();
}
}

private void storeProxySettings(ProxyPreferences newProxyPreferences) {
if (Objects.equals(newProxyPreferences, proxyPreferences)) {
// nothing changed; thus, nothing to store
return;
}

ProxyRegisterer.register(newProxyPreferences);
proxyPreferences.setUseProxy(proxyUseProperty.getValue());
proxyPreferences.setHostname(proxyHostnameProperty.getValue().trim());
proxyPreferences.setPort(proxyPortProperty.getValue().trim());
proxyPreferences.setUseAuthentication(proxyUseAuthenticationProperty.getValue());
proxyPreferences.setUsername(proxyUsernameProperty.getValue().trim());
proxyPreferences.setPersistPassword(proxyPersistPasswordProperty.getValue()); // Set before the password to actually persist
proxyPreferences.setPassword(proxyPasswordProperty.getValue());
ProxyRegisterer.register(proxyPreferences);

proxyPreferences.setUseProxy(newProxyPreferences.shouldUseProxy());
proxyPreferences.setHostname(newProxyPreferences.getHostname());
proxyPreferences.setPort(newProxyPreferences.getPort());
proxyPreferences.setUseAuthentication(newProxyPreferences.shouldUseAuthentication());
proxyPreferences.setUsername(newProxyPreferences.getUsername());
proxyPreferences.setPassword(newProxyPreferences.getPassword());
}

public void storeSSLSettings() {
trustStoreManager.flush();
}

Expand Down Expand Up @@ -299,15 +271,14 @@ public void checkConnection() {

final String testUrl = "http://jabref.org";

// Workaround for testing, since the URLDownload uses stored proxy settings, see
// preferences.storeProxyPreferences(...) below.
storeProxySettings(new ProxyPreferences(
ProxyRegisterer.register(new ProxyPreferences(
proxyUseProperty.getValue(),
proxyHostnameProperty.getValue().trim(),
proxyPortProperty.getValue().trim(),
proxyUseAuthenticationProperty.getValue(),
proxyUsernameProperty.getValue().trim(),
proxyPasswordProperty.getValue()
proxyPasswordProperty.getValue(),
proxyPersistPasswordProperty.getValue()
));

URLDownload urlDownload;
Expand All @@ -324,7 +295,7 @@ public void checkConnection() {
dialogService.showErrorDialogAndWait(dialogTitle, connectionFailedText);
}

storeProxySettings(backupProxyPreferences);
ProxyRegisterer.register(backupProxyPreferences);
}

@Override
Expand Down Expand Up @@ -368,6 +339,10 @@ public StringProperty proxyPasswordProperty() {
return proxyPasswordProperty;
}

public BooleanProperty proxyPersistPasswordProperty() {
return proxyPersistPasswordProperty;
}

public ListProperty<CustomCertificateViewModel> customCertificateListProperty() {
return customCertificateListProperty;
}
Expand Down
29 changes: 23 additions & 6 deletions src/main/java/org/jabref/logic/net/ProxyPreferences.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,22 +15,25 @@ public class ProxyPreferences {
private final BooleanProperty useAuthentication;
private final StringProperty username;
private final StringProperty password;
private final BooleanProperty persistPassword;

public ProxyPreferences(Boolean useProxy,
String hostname,
String port,
Boolean useAuthentication,
String username,
String password) {
String password,
boolean persistPassword) {
this.useProxy = new SimpleBooleanProperty(useProxy);
this.hostname = new SimpleStringProperty(hostname);
this.port = new SimpleStringProperty(port);
this.useAuthentication = new SimpleBooleanProperty(useAuthentication);
this.username = new SimpleStringProperty(username);
this.password = new SimpleStringProperty(password);
this.persistPassword = new SimpleBooleanProperty(persistPassword);
}

public final Boolean shouldUseProxy() {
public final boolean shouldUseProxy() {
return useProxy.getValue();
}

Expand Down Expand Up @@ -66,8 +69,8 @@ public void setPort(String port) {
this.port.set(port);
}

public final Boolean shouldUseAuthentication() {
return useAuthentication.getValue();
public final boolean shouldUseAuthentication() {
return useAuthentication.get();
}

public BooleanProperty useAuthenticationProperty() {
Expand Down Expand Up @@ -102,6 +105,18 @@ public void setPassword(String password) {
this.password.set(password);
}

public boolean shouldPersistPassword() {
return persistPassword.get();
}

public BooleanProperty persistPasswordProperty() {
return persistPassword;
}

public void setPersistPassword(boolean persistPassword) {
this.persistPassword.set(persistPassword);
}

@Override
public boolean equals(Object o) {
if (this == o) {
Expand All @@ -118,7 +133,8 @@ public boolean equals(Object o) {
&& Objects.equals(port.getValue(), other.port.getValue())
&& Objects.equals(useAuthentication.getValue(), other.useAuthentication.getValue())
&& Objects.equals(username.getValue(), other.username.getValue())
&& Objects.equals(password.getValue(), other.password.getValue());
&& Objects.equals(password.getValue(), other.password.getValue())
&& Objects.equals(persistPassword.getValue(), other.persistPassword.getValue());
}

@Override
Expand All @@ -129,6 +145,7 @@ public int hashCode() {
port.getValue(),
useAuthentication.getValue(),
username.getValue(),
password.getValue());
password.getValue(),
persistPassword.getValue());
}
}
Loading