[dancer-users] Problem with D::P::A::E::Provider::Base.pm

[David Precious]

On Mon, 10 Dec 2012 20:57:15 -0500
"Rick Bragg" <rbragg at gmnet.net> wrote:

> Hi everyone!
> 
> I really like what I see in Dancer::Plugin::Auth::Extensible,
> however, there is a real problem in the Provider::Base.pm
> match_password() sub, and the solution would probably break some
> current (poorly implemented) sites.
> 
> First, it is hard coded to use "Crypt::SaltedHash->validate($correct,
> $given)" for validation, (not to mention It does not even let you set
> the salt length so it defaults to 4) then to my horror, if that
> fails, it tries to match it in plain text!  Yikes!

Yeah, it should at least make the salt length configurable, good point,
and attempting to match as plain text should be a configurable option
too (probably defaulted to off).

> The reason this is really bad is that at the back end where user
> validation is concerned, there should be NO guessing and helping from
> the scrip! This sub should not try to help out at all.  I would argue
> there should be no option for plain text period.  You should also
> have to specify the hashing module you want (i.e.
> Dancer::Plugin::Passphrase; Crypt::SaltedHash; etc... (in case one is
> compromised, or just outdated you could switch to another at will)
> And you should have to specify the algorithm to use.  There should
> never be defaults here.  It should just fail if this stuff is not
> specified period.

The reason I used Crypt::SaltedHash there is it's good at working out
what hashing scheme is in use and just doing the right thing.  If it's
at all difficult to configure or understand, users might decide not to
use it and just use plain text passwords instead; whilst I think they
should have that choice, I think it should be seriously discouraged :)


> One thing I like to be able to do is set the password in a user's
> account to '', or '1' or '0' or anything as a really quick way to
> force the user to reset their password because they would NEVER be
> able to log in with that.  The way it is now, you have to set a
> random string and hash it or something crazy...

Yeah, that's a good point.  A check for an empty password and returning
an immediate failure if that's the case would be sensible.


> I'm willing to help out however I can because I'm re-writing a fairly
> large site (from Interchange) and I don't want to write lots of patch
> code on top when things should be done at the modules.  My time is
> short, and I'm really hurting for cash, but let me know if there is
> anything I can do to help.

I know the feeling, but if you get time, pull requests would be
welcome.  I should be able to implement some of the above reasonably
soon anyway I should think.


> Again, I really like what I'm seeing in D::P::A::E and in Dancer
> overall!!

Glad to hear you're liking it!

-- 
David Precious ("bigpresh") <davidp at preshweb.co.uk>
http://www.preshweb.co.uk/     www.preshweb.co.uk/twitter
www.preshweb.co.uk/linkedin    www.preshweb.co.uk/facebook
www.preshweb.co.uk/cpan        www.preshweb.co.uk/github