Hi,
I'd like to propose some changes to password storage in amarok, in
particular the way KWallet is dealt with, and what is done if not available.
I noticed that there seem to be a number of different ways password
storage is done in amarok, with each plugin having some unique way of
doing things -- Bart made me aware of the fact that some might be buggy
when KWallet isn't available (I originally submitted a patch copying the
LastFM password storage to Magnatune).
Most plugins use KWallet, but some resort to plaintext if KWallet isn't
available (and some ask the user to allow this) -- meaning similar code
is replicated over many plugins [some plugins only use plaintext
currently]. I propose to write a wrapper class ("PasswordManager" ?),
which uses KWallet if available, but if KWallet turns out not to be
available then the user is asked once whether to use plaintext storage,
with this setting being remembered across all plugins. The end result
is that a plugin only needs to instantiate PasswordManager with the
plugin name, and then all the dealing with KWallet, or if necessary
amarokrc, is done in once central place.
I would also add the option to PasswordManager to check for existing
plaintext passwords, importing them to KWallet as necessary, to ease
migration from older to newer versions of amarok (I could also add a
panel to the amarok config allowing configuration of the password
settings, i.e. to allow migration from plaintext to KWallet in case
KWallet wasn't initally available, but becomes available; or the reverse
-- that would be a later stage).
Another issue is that some plugins set a variable to store the fact that
a password has been stored (in order to load KWallet on startup) but
don't remove this variable if the login details are removed, meaning
KWallet is always opened if a login has ever been set, even if no longer
necessary. To solve such issues, nternally I would store a
isPasswordStored variable in amarokrc for each plugin, which is set
depending on whether username/password are empty or not. This would
probably be in addition to any isMember variables stored by the plugin,
since that way you could still store passwords but disable a login as
desired.
A rough outline of what PasswordManager would look like to the world:
PasswordManager(QString aPluginName, bool migratePlaintext=false);
// aPluginName has to be the same as that used when getting the config
group for the plugin
QString username(); //Returns empty string if isPasswordStored == false,
otherwise deals with KWallet.
QString password(); //as above
void setUsername(QString username);
void setPassword(QString password);
In amarokrc it would store:
General: disableKWallet
<Per plugin>: isPasswordStored
[If ignoreWallet] <Per Plugin>: username and password.
Before I actually start writing such a patch: does this all look
sensible and useful?
Cheers,
Andrzej Hunt
_______________________________________________
Amarok-devel mailing list
Amarok-devel@kde.org
https://mail.kde.org/mailman/listinfo/amarok-devel
