Skip to content

Bump secret-service to 1.2.0 #159

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

Merged
infeo merged 7 commits into from
Aug 24, 2026
Merged

Bump secret-service to 1.2.0 #159

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
7347609
Use nested records due to secret-service 1.2.0
purejava Aug 7, 2026
f4a8958
Fixes suggested by coderabbitai
purejava Aug 9, 2026
1ca3953
Use guarded pattern matching in switch
purejava Aug 10, 2026
28c3347
Check number of returned search items
purejava Aug 10, 2026
74a7c13
Add explaining comment for DBus prompt
purejava Aug 10, 2026
2f1889d
Use more of the guarded pattern matching in switch
purejava Aug 19, 2026
52c3e0e
Size check should be the same everywhere
purejava Aug 19, 2026
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
2 changes: 1 addition & 1 deletion pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
<jackson.version>2.22.1</jackson.version>
<secret-service.version>2.0.1-alpha</secret-service.version>
<kdewallet.version>1.4.0</kdewallet.version>
<secret-service-02.version>1.1.1</secret-service-02.version>
<secret-service-02.version>1.2.0</secret-service-02.version>
<flatpakupdateportal.version>1.1.1</flatpakupdateportal.version>
<appindicator.version>1.4.2</appindicator.version>

Expand Down
280 changes: 225 additions & 55 deletions src/main/java/org/cryptomator/linux/keychain/SecretServiceKeychainAccess.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,21 @@
import org.cryptomator.integrations.keychain.KeychainAccessProvider;
import org.freedesktop.dbus.DBusPath;
import org.purejava.secret.api.Collection;
import org.purejava.secret.api.DBusMessageHandler;
import org.purejava.secret.api.EncryptedSession;
import org.purejava.secret.api.Item;
import org.purejava.secret.api.Pair;
import org.purejava.secret.api.Static;
import org.purejava.secret.api.Util;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.Objects;

import static org.purejava.secret.api.DBusMessageHandler.DBusResult.*;

@Priority(1100)
@OperatingSystem(OperatingSystem.Value.LINUX)
@DisplayName("Secret Service")
Expand All @@ -36,9 +39,15 @@ public SecretServiceKeychainAccess() {
session.getService().addCollectionCreatedHandler(collection -> LOG.debug("Collection {} created", collection.getPath()));
session.getService().addCollectionDeletedHandler(collection -> LOG.debug("Collection {} deleted", collection.getPath()));
var getAlias = session.getService().readAlias("default");
if (getAlias.isSuccess() && "/".equals(getAlias.value().getPath())) {
// default alias is not set; set it to the login keyring
session.getService().setAlias("default", new DBusPath(Static.DBusPath.LOGIN_COLLECTION));
switch (getAlias) {
case Success<DBusPath> success-> {
if ("/".equals(success.value().getPath())) {
// default alias is not set; set it to the login keyring
session.getService().setAlias("default", new DBusPath(Static.DBusPath.LOGIN_COLLECTION));
}
}
case Failure<DBusPath> failure
-> LOG.warn("Getting the collection with the \"default\" alias failed with: {}", failure.error().getMessage());
}
collection.addItemChangedHandler(item -> LOG.debug("Item {} changed", item.getPath()));
collection.addItemCreatedHandler(item -> LOG.debug("Item {} created", item.getPath()));
Expand All @@ -50,98 +59,248 @@ public SecretServiceKeychainAccess() {
public void storePassphrase(String key, String displayName, CharSequence passphrase) throws KeychainAccessException {
try {
var call = collection.searchItems(withKey(key));
if (call.isSuccess()) {
if (call.value().isEmpty()) {
List<DBusPath> lockable = new ArrayList<>();
lockable.add(new DBusPath(collection.getDBusPath()));
var promptNeededToUnlock = session.getService().unlock(lockable);
if (promptNeededToUnlock.isSuccess() && !"/".equals(promptNeededToUnlock.value().b.getPath())) {
Util.promptAndGetResultAsArrayList(promptNeededToUnlock.value().b);
switch (call) {
case DBusMessageHandler.DBusResult.Success<List<DBusPath>> success
when success.value().isEmpty() -> {
List<DBusPath> lockable = List.of(new DBusPath(collection.getDBusPath()));
var unlockResult = session.getService().unlock(lockable);

switch (unlockResult) {
case Success<Pair<List<DBusPath>, DBusPath>> unlockSuccess -> {
/* The unlockResult variable may contain a prompt: when you call unlock for the collection
in order to be able to store the secret within the collection, depending on whether
a prompt is needed to unlock the collection or not, a prompt needs to be handled or not.*/
var prompt = unlockSuccess.value().b;
if (!"/".equals(prompt.getPath())) {
Util.promptAndGetResultAsArrayList(prompt);
}
}
case Failure<Pair<List<DBusPath>, DBusPath>> unlockFailure ->
LOG.warn("Failed to unlock collection {}",
collection.getDBusPath(),
unlockFailure.error());
}
var itemProps = Item.createProperties(LABEL_FOR_SECRET_IN_KEYRING, withKeyAndName(key, displayName));

var itemProps = Item.createProperties(
LABEL_FOR_SECRET_IN_KEYRING,
withKeyAndName(key, displayName)
);
var secret = session.encrypt(passphrase);
var created = collection.createItem(itemProps, secret, false);
if (!created.isSuccess()) {
throw new KeychainAccessException("Storing password failed", created.error());

switch (created) {
case Success<Pair<DBusPath, DBusPath>> successful ->
LOG.debug("Created item {} on collection {}",
successful.value().a.getPath(),
collection.getDBusPath());
case Failure<Pair<DBusPath, DBusPath>> failure ->
throw new KeychainAccessException(
"Storing password failed for collection "
+ collection.getDBusPath(),
failure.error()
);
}
} else {
changePassphrase(key, displayName, passphrase);
}
} else {
throw new KeychainAccessException("Storing password failed", call.error());
case DBusMessageHandler.DBusResult.Success<List<DBusPath>> _ ->
changePassphrase(key, displayName, passphrase);

case DBusMessageHandler.DBusResult.Failure<List<DBusPath>> failure ->
throw new KeychainAccessException(
"Storing password failed for collection "
+ collection.getDBusPath(),
failure.error()
);
}
} catch (KeychainAccessException e) {
throw e;
} catch (Exception e) {
throw new KeychainAccessException("Storing password failed.", e);
throw new KeychainAccessException(
"Storing password failed for collection "
+ collection.getDBusPath(),
e
);
}
}

@Override
public char[] loadPassphrase(String key) throws KeychainAccessException {
try {
var call = collection.searchItems(withKey(key));
if (call.isSuccess()) {
if (!call.value().isEmpty()) {
var path = call.value().getFirst();

return switch (call) {

case Success<List<DBusPath>> success
when success.value().isEmpty() ->
null;

case Success<List<DBusPath>> success
when success.value().size() > 1 ->
throw new KeychainAccessException(
"Expected exactly one item, but found "
+ success.value().size()
);

case Success<List<DBusPath>> success -> {
var path = success.value().getFirst();

@overheadhunter overheadhunter Aug 9, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of silently returning getFirst, shouldn't it be three different cases:

  1. empty → null
  2. exactly one item → success
  3. more than one items → ambiguous exception? Or at least log a warning?

@purejava purejava Aug 10, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test for an empty search response that returns null is already there:

if (success.value().isEmpty()) {
	return null;
}

var path = success.value().getFirst();

If the search would return more than one items, there would be two or more vaults with the same ID or the search would be broken. That's unlikely. But, I think, a test for only one returned item and a log message in case the result differs from that expectation does improve the code.

@overheadhunter overheadhunter Aug 18, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My suggestion was to pull it up all to the switch, so we avoid further checks in the happy path.

switch (call) {
	case Failure<List<DBusPath>> failure -> throw new KeychainAccessException("Loading password failed for collection " + collection.getDBusPath(), failure.error());
	case Success<List<DBusPath>> success when success.value().isEmpty() -> null;
	case Success<List<DBusPath>> success when success.value().size() != 1 -> throw new KeychainAccessException("Expected exactly one item, but found " + success.value().size());
	case Success<List<DBusPath>> success -> {
		// ...
	}
}

@purejava purejava Aug 19, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@overheadhunter Yeah, that's even better. Thanks!


session.getService().ensureUnlocked(path);

var secret = new Item(path).getSecret(session.getSession());
return session.decrypt(secret);
} else {
return null;
yield session.decrypt(secret);
}
} else {
throw new KeychainAccessException("Loading password failed", call.error());
}

case Failure<List<DBusPath>> failure ->
throw new KeychainAccessException(
"Loading password failed for collection "
+ collection.getDBusPath(),
failure.error()
);
};

} catch (KeychainAccessException e) {
throw e;
} catch (Exception e) {
throw new KeychainAccessException("Loading password failed.", e);
throw new KeychainAccessException(
"Loading password failed for collection "
+ collection.getDBusPath(),
e
);
}
}

@Override
public void deletePassphrase(String key) throws KeychainAccessException {
try {
var call = collection.searchItems(withKey(key));
if (call.isSuccess()) {
if (!call.value().isEmpty()) {
var path = call.value().getFirst();

switch (call) {

case Success<List<DBusPath>> success
when success.value().isEmpty() ->
LOG.debug("Deleting entry with {}={} failed: No such item found",
ID_KEY,
key);

case Success<List<DBusPath>> success
when success.value().size() > 1 ->
throw new KeychainAccessException(
"Expected exactly one item, but found "
+ success.value().size()
);

case Success<List<DBusPath>> success -> {
var path = success.value().getFirst();
Comment thread
purejava marked this conversation as resolved.
session.getService().ensureUnlocked(path);
var item = new Item(path);
var deleted = item.delete();
if (!deleted.isSuccess()) {
throw new KeychainAccessException("Deleting password failed", deleted.error());

switch (item.delete()) {
case Success<DBusPath> _ ->
LOG.debug("Deleted item {} from collection {}",
path.getPath(),
collection.getDBusPath());

case Failure<DBusPath> failure -> {
LOG.warn("Failed to delete item {} from collection {}",
path.getPath(),
collection.getDBusPath(),
failure.error());

throw new KeychainAccessException(
"Deleting password failed for collection "
+ collection.getDBusPath(),
failure.error()
);
}
}
} else {
LOG.debug("Deleting entry with {}={} failed: No such item found", ID_KEY, key);
}
} else {
throw new KeychainAccessException("Deleting password failed", call.error());

case Failure<List<DBusPath>> failure ->
throw new KeychainAccessException(
"Deleting password failed for collection "
+ collection.getDBusPath(),
failure.error()
);
}

} catch (KeychainAccessException e) {
throw e;
} catch (Exception e) {
throw new KeychainAccessException("Deleting password failed", e);
throw new KeychainAccessException(
"Deleting password failed for collection "
+ collection.getDBusPath(),
e
);
}
}

@Override
public void changePassphrase(String key, String displayName, CharSequence passphrase) throws KeychainAccessException {
public void changePassphrase(String key, String displayName, CharSequence passphrase)
throws KeychainAccessException {
try {
var call = collection.searchItems(withKey(key));
if (call.isSuccess()) {
if (!call.value().isEmpty()) {
session.getService().ensureUnlocked(call.value().getFirst());

switch (call) {

case Success<List<DBusPath>> success
when success.value().isEmpty() ->
throw new KeychainAccessException(
"Vault " + key + " not found, updating failed"
);

case Success<List<DBusPath>> success
when success.value().size() > 1 ->
throw new KeychainAccessException(
"Expected exactly one item, but found "
+ success.value().size()
);

case Success<List<DBusPath>> success -> {
var path = success.value().getFirst();
Comment thread
purejava marked this conversation as resolved.
session.getService().ensureUnlocked(path);
var secret = session.encrypt(passphrase);
var itemProps = Item.createProperties(LABEL_FOR_SECRET_IN_KEYRING, withKeyAndName(key, displayName));
var itemProps = Item.createProperties(
LABEL_FOR_SECRET_IN_KEYRING,
withKeyAndName(key, displayName)
);
var updated = collection.createItem(itemProps, secret, true);
if (!updated.isSuccess()) {
throw new KeychainAccessException("Updating password failed", updated.error());

switch (updated) {
case Success<Pair<DBusPath, DBusPath>> _ ->
LOG.debug("Updated item {} in collection {}",
path.getPath(),
collection.getDBusPath());

case Failure<Pair<DBusPath, DBusPath>> failure -> {
LOG.warn("Failed to update item {} in collection {}",
path.getPath(),
collection.getDBusPath(),
failure.error());

throw new KeychainAccessException(
"Updating password failed for collection "
+ collection.getDBusPath(),
failure.error()
);
}
}
} else {
var msg = "Vault " + key + " not found, updating failed";
throw new KeychainAccessException(msg);
}
} else {
throw new KeychainAccessException("Updating password failed", call.error());

case Failure<List<DBusPath>> failure ->
throw new KeychainAccessException(
"Updating password failed for collection "
+ collection.getDBusPath(),
failure.error()
);
}

} catch (KeychainAccessException e) {
throw e;
} catch (Exception e) {
throw new KeychainAccessException("Updating password failed", e);
throw new KeychainAccessException(
"Updating password failed for collection "
+ collection.getDBusPath(),
e
);
}
}

Expand All @@ -158,8 +317,19 @@ public boolean isSupported() {

@Override
public boolean isLocked() {
var call = collection.isLocked();
return !call.isSuccess() || call.value();
return switch (collection.isLocked()) {
case Success<Boolean> success ->
success.value(); // yields the value

case Failure<Boolean> failure -> {
LOG.warn(
"Failed to determine lock state of collection {}",
collection.getDBusPath(),
failure.error()
);
yield true;
}
};
}

private Map<String, String> withKey(String key) {
Expand Down
Loading