Skip to content

Commit

Permalink
Disable password and apikey persistency if keyring is unavailable (#1…
Browse files Browse the repository at this point in the history 
…0224)
  • Loading branch information
@calixtus
calixtus authored Aug 28, 2023
1 parent 3e1b7ab commit 6ac018b
Show file tree
Hide file tree
Showing 10 changed files with 123 additions and 14 deletions.
5 changes: 4 additions & 1 deletion src/main/java/org/jabref/gui/preferences/network/NetworkTab.fxml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
<?import javafx.scene.control.Button?>
<?import javafx.scene.control.CheckBox?>
<?import javafx.scene.control.Label?>
<?import javafx.scene.control.SplitPane?>
<?import javafx.scene.control.TableColumn?>
<?import javafx.scene.control.TableView?>
<?import javafx.scene.control.TextField?>
Expand Down Expand Up @@ -52,7 +53,9 @@
<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" />
<CheckBox fx:id="proxyPersistPassword" text="%Persist password between sessions" GridPane.columnIndex="2" GridPane.rowIndex="5"/>
<SplitPane fx:id="persistentTooltipWrapper" GridPane.columnIndex="2" GridPane.rowIndex="5">
<CheckBox fx:id="proxyPersistPassword" text="%Persist password between sessions"/>
</SplitPane>
<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
13 changes: 12 additions & 1 deletion src/main/java/org/jabref/gui/preferences/network/NetworkTab.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,11 @@
import javafx.scene.control.Button;
import javafx.scene.control.CheckBox;
import javafx.scene.control.Label;
import javafx.scene.control.SplitPane;
import javafx.scene.control.TableColumn;
import javafx.scene.control.TableView;
import javafx.scene.control.TextField;
import javafx.scene.control.Tooltip;
import javafx.scene.input.MouseEvent;

import org.jabref.gui.Globals;
Expand Down Expand Up @@ -53,6 +55,7 @@ public class NetworkTab extends AbstractPreferenceTabView<NetworkTabViewModel> i
@FXML private CustomPasswordField proxyPassword;
@FXML private Button checkConnectionButton;
@FXML private CheckBox proxyPersistPassword;
@FXML private SplitPane persistentTooltipWrapper; // The disabled persistPassword control does not show tooltips

@FXML private TableView<CustomCertificateViewModel> customCertificatesTable;
@FXML private TableColumn<CustomCertificateViewModel, String> certIssuer;
Expand Down Expand Up @@ -109,7 +112,15 @@ public void initialize() {
proxyPassword.textProperty().bindBidirectional(viewModel.proxyPasswordProperty());
proxyPassword.disableProperty().bind(proxyCustomAndAuthentication.not());
proxyPersistPassword.selectedProperty().bindBidirectional(viewModel.proxyPersistPasswordProperty());
proxyPersistPassword.disableProperty().bind(proxyCustomAndAuthentication.not());
proxyPersistPassword.disableProperty().bind(
proxyCustomAndAuthentication.and(viewModel.passwordPersistAvailable()).not());
EasyBind.subscribe(viewModel.passwordPersistAvailable(), available -> {
if (!available) {
persistentTooltipWrapper.setTooltip(new Tooltip(Localization.lang("Credential store not available.")));
} else {
persistentTooltipWrapper.setTooltip(null);
}
});

proxyPassword.setRight(IconTheme.JabRefIcons.PASSWORD_REVEALED.getGraphicNode());
proxyPassword.getRight().addEventFilter(MouseEvent.MOUSE_PRESSED, this::proxyPasswordReveal);
Expand Down
15 changes: 10 additions & 5 deletions src/main/java/org/jabref/gui/preferences/network/NetworkTabViewModel.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

import javafx.beans.property.BooleanProperty;
import javafx.beans.property.ListProperty;
import javafx.beans.property.ReadOnlyBooleanProperty;
import javafx.beans.property.SimpleBooleanProperty;
import javafx.beans.property.SimpleListProperty;
import javafx.beans.property.SimpleStringProperty;
Expand All @@ -27,10 +28,10 @@
import org.jabref.logic.net.ProxyRegisterer;
import org.jabref.logic.net.URLDownload;
import org.jabref.logic.net.ssl.SSLCertificate;
import org.jabref.logic.net.ssl.SSLPreferences;
import org.jabref.logic.net.ssl.TrustStoreManager;
import org.jabref.logic.remote.RemotePreferences;
import org.jabref.logic.remote.RemoteUtil;
import org.jabref.logic.util.OS;
import org.jabref.logic.util.StandardFileType;
import org.jabref.model.strings.StringUtil;
import org.jabref.model.util.FileUpdateMonitor;
Expand All @@ -55,6 +56,7 @@ public class NetworkTabViewModel implements PreferenceTabViewModel {
private final StringProperty proxyUsernameProperty = new SimpleStringProperty("");
private final StringProperty proxyPasswordProperty = new SimpleStringProperty("");
private final BooleanProperty proxyPersistPasswordProperty = new SimpleBooleanProperty();
private final BooleanProperty passwordPersistAvailable = new SimpleBooleanProperty();
private final ListProperty<CustomCertificateViewModel> customCertificateListProperty = new SimpleListProperty<>(FXCollections.observableArrayList());

private final Validator remotePortValidator;
Expand All @@ -70,7 +72,6 @@ public class NetworkTabViewModel implements PreferenceTabViewModel {
private final RemotePreferences remotePreferences;
private final ProxyPreferences proxyPreferences;
private final ProxyPreferences backupProxyPreferences;
private final SSLPreferences sslPreferences;
private final InternalPreferences internalPreferences;

private final TrustStoreManager trustStoreManager;
Expand All @@ -83,7 +84,6 @@ public NetworkTabViewModel(DialogService dialogService, PreferencesService prefe
this.fileUpdateMonitor = fileUpdateMonitor;
this.remotePreferences = preferences.getRemotePreferences();
this.proxyPreferences = preferences.getProxyPreferences();
this.sslPreferences = preferences.getSSLPreferences();
this.internalPreferences = preferences.getInternalPreferences();

backupProxyPreferences = new ProxyPreferences(
Expand Down Expand Up @@ -136,13 +136,13 @@ public NetworkTabViewModel(DialogService dialogService, PreferencesService prefe

proxyPasswordValidator = new FunctionBasedValidator<>(
proxyPasswordProperty,
input -> input.length() > 0,
input -> !input.isBlank(),
ValidationMessage.error(String.format("%s > %s %n %n %s",
Localization.lang("Network"),
Localization.lang("Proxy configuration"),
Localization.lang("Please specify a password"))));

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

@Override
Expand All @@ -163,6 +163,7 @@ private void setProxyValues() {
proxyUsernameProperty.setValue(proxyPreferences.getUsername());
proxyPasswordProperty.setValue(proxyPreferences.getPassword());
proxyPersistPasswordProperty.setValue(proxyPreferences.shouldPersistPassword());
passwordPersistAvailable.setValue(OS.isKeyringAvailable());
}

private void setSSLValues() {
Expand Down Expand Up @@ -354,6 +355,10 @@ public BooleanProperty proxyPersistPasswordProperty() {
return proxyPersistPasswordProperty;
}

public ReadOnlyBooleanProperty passwordPersistAvailable() {
return passwordPersistAvailable;
}

public ListProperty<CustomCertificateViewModel> customCertificateListProperty() {
return customCertificateListProperty;
}
Expand Down
7 changes: 6 additions & 1 deletion src/main/java/org/jabref/gui/preferences/websearch/WebSearchTab.fxml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
<?import javafx.scene.control.CheckBox?>
<?import javafx.scene.control.ComboBox?>
<?import javafx.scene.control.Label?>
<?import javafx.scene.control.SplitPane?>
<?import javafx.scene.control.TextField?>
<?import javafx.scene.layout.GridPane?>
<?import javafx.scene.layout.HBox?>
Expand Down Expand Up @@ -39,8 +40,12 @@
<CheckBox fx:id="useCustomApiKey" text="%Custom">
</CheckBox>
</HBox>
<HBox alignment="CENTER_LEFT" >

<HBox alignment="CENTER_LEFT" spacing="10.0">
<Button fx:id="testCustomApiKey" text="%Check connection" onAction="#checkCustomApiKey"
prefWidth="200.0" GridPane.columnIndex="1" GridPane.rowIndex="6"/>
<SplitPane fx:id="persistentTooltipWrapper">
<CheckBox fx:id="persistApiKeys" text="%Persist api keys between sessions"/>
</SplitPane>
</HBox>
</fx:root>
16 changes: 16 additions & 0 deletions src/main/java/org/jabref/gui/preferences/websearch/WebSearchTab.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@
import javafx.scene.control.Button;
import javafx.scene.control.CheckBox;
import javafx.scene.control.ComboBox;
import javafx.scene.control.SplitPane;
import javafx.scene.control.TextField;
import javafx.scene.control.Tooltip;

import org.jabref.gui.preferences.AbstractPreferenceTabView;
import org.jabref.gui.preferences.PreferencesTab;
Expand All @@ -14,6 +16,7 @@
import org.jabref.logic.preferences.FetcherApiKey;

import com.airhacks.afterburner.views.ViewLoader;
import com.tobiasdiez.easybind.EasyBind;

public class WebSearchTab extends AbstractPreferenceTabView<WebSearchTabViewModel> implements PreferencesTab {

Expand All @@ -33,6 +36,9 @@ public class WebSearchTab extends AbstractPreferenceTabView<WebSearchTabViewMode
@FXML private CheckBox useCustomApiKey;
@FXML private Button testCustomApiKey;

@FXML private CheckBox persistApiKeys;
@FXML private SplitPane persistentTooltipWrapper; // The disabled persistApiKeys control does not show tooltips

public WebSearchTab() {
ViewLoader.view(this)
.root(this)
Expand Down Expand Up @@ -77,6 +83,16 @@ public void initialize() {
customApiKey.disableProperty().bind(useCustomApiKey.selectedProperty().not());
testCustomApiKey.disableProperty().bind(useCustomApiKey.selectedProperty().not());

persistApiKeys.selectedProperty().bindBidirectional(viewModel.getApikeyPersistProperty());
persistApiKeys.disableProperty().bind(viewModel.apiKeyPersistAvailable().not());
EasyBind.subscribe(viewModel.apiKeyPersistAvailable(), available -> {
if (!available) {
persistentTooltipWrapper.setTooltip(new Tooltip(Localization.lang("Credential store not available.")));
} else {
persistentTooltipWrapper.setTooltip(null);
}
});

apiKeySelector.setItems(viewModel.fetcherApiKeys());
viewModel.selectedApiKeyProperty().bind(apiKeySelector.valueProperty());

Expand Down
19 changes: 18 additions & 1 deletion src/main/java/org/jabref/gui/preferences/websearch/WebSearchTabViewModel.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import javafx.beans.property.BooleanProperty;
import javafx.beans.property.ListProperty;
import javafx.beans.property.ObjectProperty;
import javafx.beans.property.ReadOnlyBooleanProperty;
import javafx.beans.property.SimpleBooleanProperty;
import javafx.beans.property.SimpleListProperty;
import javafx.beans.property.SimpleObjectProperty;
Expand All @@ -28,6 +29,7 @@
import org.jabref.logic.net.URLDownload;
import org.jabref.logic.preferences.DOIPreferences;
import org.jabref.logic.preferences.FetcherApiKey;
import org.jabref.logic.util.OS;
import org.jabref.preferences.FilePreferences;
import org.jabref.preferences.PreferencesService;

Expand All @@ -45,6 +47,8 @@ public class WebSearchTabViewModel implements PreferenceTabViewModel {

private final ListProperty<FetcherApiKey> apiKeys = new SimpleListProperty<>();
private final ObjectProperty<FetcherApiKey> selectedApiKeyProperty = new SimpleObjectProperty<>();
private final BooleanProperty apikeyPersistProperty = new SimpleBooleanProperty();
private final BooleanProperty apikeyPersistAvailableProperty = new SimpleBooleanProperty();

private final DialogService dialogService;
private final PreferencesService preferencesService;
Expand Down Expand Up @@ -76,6 +80,8 @@ public void setValues() {
grobidURLProperty.setValue(grobidPreferences.getGrobidURL());

apiKeys.setValue(FXCollections.observableArrayList(preferencesService.getImporterPreferences().getApiKeys()));
apikeyPersistAvailableProperty.setValue(OS.isKeyringAvailable());
apikeyPersistProperty.setValue(preferencesService.getImporterPreferences().shouldPersistCustomKeys());
}

@Override
Expand All @@ -92,8 +98,11 @@ public void storeSettings() {
doiPreferences.setUseCustom(useCustomDOIProperty.get());
doiPreferences.setDefaultBaseURI(useCustomDOINameProperty.getValue().trim());

importerPreferences.setPersistCustomKeys(apikeyPersistProperty.get());
preferencesService.getImporterPreferences().getApiKeys().clear();
preferencesService.getImporterPreferences().getApiKeys().addAll(apiKeys);
if (apikeyPersistAvailableProperty.get()) {
preferencesService.getImporterPreferences().getApiKeys().addAll(apiKeys);
}
}

public BooleanProperty enableWebSearchProperty() {
Expand Down Expand Up @@ -136,6 +145,14 @@ public BooleanProperty shouldDownloadLinkedOnlineFiles() {
return shouldDownloadLinkedOnlineFiles;
}

public ReadOnlyBooleanProperty apiKeyPersistAvailable() {
return apikeyPersistAvailableProperty;
}

public BooleanProperty getApikeyPersistProperty() {
return apikeyPersistProperty;
}

public void checkCustomApiKey() {
final String apiKeyName = selectedApiKeyProperty.get().getName();

Expand Down
17 changes: 16 additions & 1 deletion src/main/java/org/jabref/logic/importer/ImporterPreferences.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,19 +21,22 @@ public class ImporterPreferences {
private final ObjectProperty<Path> importWorkingDirectory;
private final ObservableSet<FetcherApiKey> apiKeys;
private final ObservableSet<CustomImporter> customImporters;
private final BooleanProperty persistCustomKeys;

public ImporterPreferences(boolean importerEnabled,
boolean generateNewKeyOnImport,
Path importWorkingDirectory,
boolean warnAboutDuplicatesOnImport,
Set<CustomImporter> customImporters,
Set<FetcherApiKey> apiKeys) {
Set<FetcherApiKey> apiKeys,
boolean persistCustomKeys) {
this.importerEnabled = new SimpleBooleanProperty(importerEnabled);
this.generateNewKeyOnImport = new SimpleBooleanProperty(generateNewKeyOnImport);
this.importWorkingDirectory = new SimpleObjectProperty<>(importWorkingDirectory);
this.warnAboutDuplicatesOnImport = new SimpleBooleanProperty(warnAboutDuplicatesOnImport);
this.customImporters = FXCollections.observableSet(customImporters);
this.apiKeys = FXCollections.observableSet(apiKeys);
this.persistCustomKeys = new SimpleBooleanProperty(persistCustomKeys);
}

public boolean areImporterEnabled() {
Expand Down Expand Up @@ -96,4 +99,16 @@ public void setCustomImporters(Set<CustomImporter> importers) {
customImporters.clear();
customImporters.addAll(importers);
}

public boolean shouldPersistCustomKeys() {
return persistCustomKeys.get();
}

public BooleanProperty persistCustomKeysProperty() {
return persistCustomKeys;
}

public void setPersistCustomKeys(boolean persistCustomKeys) {
this.persistCustomKeys.set(persistCustomKeys);
}
}
25 changes: 25 additions & 0 deletions src/main/java/org/jabref/logic/util/OS.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,11 @@
import org.jabref.gui.desktop.os.OSX;
import org.jabref.gui.desktop.os.Windows;

import com.github.javakeyring.BackendNotSupportedException;
import com.github.javakeyring.Keyring;
import com.github.javakeyring.PasswordAccessException;
import org.slf4j.LoggerFactory;

/**
* Operating system (OS) detection
*
Expand Down Expand Up @@ -42,4 +47,24 @@ public static NativeDesktop getNativeDesktop() {
}
return new DefaultDesktop();
}

public static boolean isKeyringAvailable() {
try (Keyring keyring = Keyring.create()) {
keyring.setPassword("JabRef", "keyringTest", "keyringTest");
if (!"keyringTest".equals(keyring.getPassword("JabRef", "keyringTest"))) {
return false;
}
keyring.deletePassword("JabRef", "keyringTest");
} catch (BackendNotSupportedException ex) {
LoggerFactory.getLogger(OS.class).warn("Credential store not supported.");
return false;
} catch (PasswordAccessException ex) {
LoggerFactory.getLogger(OS.class).warn("Password storage in credential store failed.");
return false;
} catch (Exception ex) {
LoggerFactory.getLogger(OS.class).warn("Connection to credential store failed");
return false;
}
return true;
}
}
16 changes: 12 additions & 4 deletions src/main/java/org/jabref/preferences/JabRefPreferences.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,7 @@ public class JabRefPreferences implements PreferencesService {
// Web search
private static final String FETCHER_CUSTOM_KEY_NAMES = "fetcherCustomKeyNames";
private static final String FETCHER_CUSTOM_KEY_USES = "fetcherCustomKeyUses";
private static final String FETCHER_CUSTOM_KEY_PERSIST = "fetcherCustomKeyPersist";

// SSL
private static final String TRUSTSTORE_PATH = "truststorePath";
Expand Down Expand Up @@ -667,6 +668,7 @@ private JabRefPreferences() {

defaults.put(FETCHER_CUSTOM_KEY_NAMES, "Springer;IEEEXplore;SAO/NASA ADS;ScienceDirect;Biodiversity Heritage");
defaults.put(FETCHER_CUSTOM_KEY_USES, "FALSE;FALSE;FALSE;FALSE;FALSE");
defaults.put(FETCHER_CUSTOM_KEY_PERSIST, Boolean.FALSE);

defaults.put(USE_OWNER, Boolean.FALSE);
defaults.put(OVERWRITE_OWNER, Boolean.FALSE);
Expand Down Expand Up @@ -2785,13 +2787,14 @@ public ImporterPreferences getImporterPreferences() {
Path.of(get(IMPORT_WORKING_DIRECTORY)),
getBoolean(WARN_ABOUT_DUPLICATES_IN_INSPECTION),
getCustomImportFormats(),
getFetcherKeys()
);
getFetcherKeys(),
getBoolean(FETCHER_CUSTOM_KEY_PERSIST));

EasyBind.listen(importerPreferences.importerEnabledProperty(), (obs, oldValue, newValue) -> putBoolean(IMPORTERS_ENABLED, newValue));
EasyBind.listen(importerPreferences.generateNewKeyOnImportProperty(), (obs, oldValue, newValue) -> putBoolean(GENERATE_KEY_ON_IMPORT, newValue));
EasyBind.listen(importerPreferences.importWorkingDirectoryProperty(), (obs, oldValue, newValue) -> put(IMPORT_WORKING_DIRECTORY, newValue.toString()));
EasyBind.listen(importerPreferences.warnAboutDuplicatesOnImportProperty(), (obs, oldValue, newValue) -> putBoolean(WARN_ABOUT_DUPLICATES_IN_INSPECTION, newValue));
EasyBind.listen(importerPreferences.persistCustomKeysProperty(), (obs, oldValue, newValue) -> putBoolean(FETCHER_CUSTOM_KEY_PERSIST, newValue));
importerPreferences.getApiKeys().addListener((InvalidationListener) c -> storeFetcherKeys(importerPreferences.getApiKeys()));
importerPreferences.getCustomImporters().addListener((InvalidationListener) c -> storeCustomImportFormats(importerPreferences.getCustomImporters()));

Expand Down Expand Up @@ -2856,7 +2859,7 @@ private List<String> getFetcherKeysFromKeyring(List<String> names) {
getInternalPreferences().getUserAndHost())
.decrypt());
} catch (PasswordAccessException ex) {
LOGGER.warn("No api key stored for {} fetcher", fetcher);
LOGGER.debug("No api key stored for {} fetcher", fetcher);
keys.add("");
}
}
Expand All @@ -2880,7 +2883,12 @@ private void storeFetcherKeys(Set<FetcherApiKey> fetcherApiKeys) {

putStringList(FETCHER_CUSTOM_KEY_NAMES, names);
putStringList(FETCHER_CUSTOM_KEY_USES, uses);
storeFetcherKeysToKeyring(names, keys);

if (getBoolean(FETCHER_CUSTOM_KEY_PERSIST)) {
storeFetcherKeysToKeyring(names, keys);
} else {
clearCustomFetcherKeys();
}
}

private void storeFetcherKeysToKeyring(List<String> names, List<String> keys) {
Expand Down
Loading

0 comments on commit 6ac018b

Please sign in to comment.