[Psi-Devel] [PATCH] Password Manager Daemon (pwmd) support. When enabled in the Application section of the Options dialog, accounts will retrieve account authentication details from pwmd. See README.PWMD for details.

[Ben Kibbey]

On Wed, Jan 05, 2011 at 01:37:37PM +0500, Rion wrote:
> since its quite non-standard feature for most DE i think this feature
> should be made as a plugin.

It seems a plugin hook is needed while authenticating. I could be wrong
though. I didn't see one and also don't know anything about the Qt
plugin architecture.

> from other things:
> 1) bad code style

How so?

> 2) it seems HAVE_LIBPWMD defined nowhere (should be smth like
> configure option)

It's defined in pwmdedit/include.pri which is included in src.pri.  I
think your right though, a configure option would be better.

> 3) some code parts look strange. for example PwmdPrivate allocating
> (isnt it simpler to use stack allocation?).

Could be. See 4).

>    or calling restoreWidgets(). WhatIsThis can live in ui file, why is
>    it in cpp?

Not sure why it's there. :) The rest of the Psi code has these too.

>    in other words add comments or rewrite unclear code.
> 4) code dublication

There are cases where PwmdPrivate::get() could be made a static member.
Thats a good idea. Rather than allocating a new object and pwmd handle,
just return a GET result from the static member.

> 5) no fallback mechanisms

I'm not sure what you mean here. The purpose of pwmd is to store the
credentials of an account on the pwmd server and not the local Psi
configuration file. If a failure occurs while connecting to pwmd, the
account is treated as if a login failure occurred (unless pwmd is
disabled in the Psi options, of course). The exception is when changing
a password for an account. Since the password is only updated on the
pwmd server after Psi changes the password on the account server and in
the case of a pwmd failure, the local Psi configuration file is updated
to the new passphrase and the user is notified of this.

Thanks for the feedback. An authentication plugin method seems to be
needed but I don't really want to be the one to write it. Maybe later
sometime if no one else is willing to do it.

Please tell me of any other ideas or criticisms.

-- 
Ben Kibbey
[XMPP: bjk AT thiessen DOT org] - [IRC: (bjk) FreeNode/OFTC]