-
Notifications
You must be signed in to change notification settings - Fork 162
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
Prefer kwallet over libsecret for KDE #111
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -78,11 +78,19 @@ static DesktopEnvironment detectDesktopEnvironment() { | |
|
||
static KeyringBackend detectKeyringBackend() | ||
{ | ||
/* Libsecret unifies access to KDE and GNOME | ||
* password services. */ | ||
if (LibSecretKeyring::isAvailable()) { | ||
return Backend_LibSecretKeyring; | ||
} | ||
/* The secret service dbus api, accessible through libsecret, is supposed | ||
* to unify password services. | ||
* | ||
* Unfortunately at the time of Kubuntu 18.04 the secret service backend | ||
* in KDE is gnome-keyring-daemon - using it has several complications: | ||
* - the default collection isn't opened on session start, so users need | ||
* to manually unlock it when the first application uses it | ||
* - it's separate from the kwallet5 keyring, so switching to it means the | ||
* existing keyring data can't be accessed anymore | ||
* | ||
* Thus we still prefer kwallet backends on KDE even if libsecret is | ||
* available. | ||
*/ | ||
|
||
switch (detectDesktopEnvironment()) { | ||
case DesktopEnv_Kde4: | ||
|
@@ -91,13 +99,18 @@ static KeyringBackend detectKeyringBackend() | |
case DesktopEnv_Plasma5: | ||
return Backend_Kwallet5; | ||
break; | ||
// fall through | ||
|
||
// fall through | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (nitpick: fall through what? there is already one return AND a redundent break :-) ) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. I didn't add it. |
||
case DesktopEnv_Gnome: | ||
case DesktopEnv_Unity: | ||
case DesktopEnv_Xfce: | ||
case DesktopEnv_Other: | ||
default: | ||
if ( GnomeKeyring::isAvailable() ) { | ||
if (LibSecretKeyring::isAvailable()) { | ||
return Backend_LibSecretKeyring; | ||
} | ||
|
||
if ( GnomeKeyring::isAvailable() ) { | ||
return Backend_GnomeKeyring; | ||
} else { | ||
return Backend_Kwallet4; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please change to Backend_Kwallet5 So this would cover the use case that we are running Plasma 5, but neither isKwallet5Available() nor LibSecretKeyring::isAvailable() (because the application just started at startup before kwallet5d is running) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right, that might happen during startup. So: Plasma: Try kwallet5, try libsecret, try gnome-keyring, fallback kwallet5 ? |
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we check somehow if kwallet5 is running. And if it is not, then we'd fallback to libsecret?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will check.