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.

Sign up for GitHub

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

Jump to bottom

Disable password and apikey persistency if keyring is unavailable #10224

Merged
merged 7 commits into from
Aug 28, 2023
Merged

Disable password and apikey persistency if keyring is unavailable #10224

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
06df64b
Introduced method isKeyRingAvailable
calixtus Aug 28, 2023
0c53639
Made password persistency dependend on keyring available
calixtus Aug 28, 2023
1b233f2
Made api key persistency dependend on keyring available
calixtus Aug 28, 2023
4110d08
l10n
calixtus Aug 28, 2023
b510a74
Merge remote-tracking branch 'upstream/main' into keyringAvailable
calixtus Aug 28, 2023
b54d56d
Added log messages
calixtus Aug 28, 2023
3f2ff59
Downgraded warning message about no key stored
calixtus Aug 28, 2023
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
5 changes: 4 additions & 1 deletion src/main/java/org/jabref/gui/preferences/network/NetworkTab.fxml
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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;
Siedlerchr marked this conversation as resolved.
Show resolved Hide resolved
}
return true;
}
}
16 changes: 12 additions & 4 deletions src/main/java/org/jabref/preferences/JabRefPreferences.java
View file
Open in desktop
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
Loading