[Catalyst] Outcome of the "Security issue with hashed passwords in C:P:A:Password"?

J. Shirley jshirley at gmail.com
Fri Apr 9 16:04:58 GMT 2010


On Fri, Apr 9, 2010 at 8:51 AM, Evan Carroll <lists at evancarroll.com> wrote:
>> Without any unnecessary commentary, here is the implementation of the
>> password_(pre|post)_salt_field, without other features that should be
>> patched separately.
>>
>> http://codepeek.com/paste/4bbf456c0ae3049443a742a2
>
> I appreciate your efforts.
>
> Without any unnecessary commentary (yes, it is all necessary) you
> can't pry code from my moosified version out, without reading what
> Moose was offering (namely, the methods you're using like get_config
> -- I take it you liked it). Your version is a slight disservice as
> you're ignoring the test patches and docs patches. So what I have 4
> patches -- completely lacking negative criticism might I add -- they
> also put the dev team in the direction they want to go with Moose.
>


Please provide a link to the tests, as I didn't see any modifications
in the diffs.

get_config could be easily applied.  I wouldn't commit this until
there are adequate tests, anyway.

My specific problems with your patch is:
1) You arbitrarily change the style.  It's difficult to find
functional changes when every line in the diff is changed so you can
change the way spaces are.  You did this same thing with
Catalyst::View::Email.  If you want to propose stylistic changes, do
that in a separate patch from functional changes.
2) Adding the salt from the database field doesn't require
Mooseification.  It does require tests.  I didn't see tests.
3) There isn't a problem with going towards Moose.  There is, however,
a problem with doing 3 different things at once and just saying, "It's
a point release".

This is due diligence, nothing more.  I don't like to see large
changes to any code I use.  I've already had several points of
breakage with DBIC (one severe) that are holding me back to previous
versions (and still no tuits to fully chase it down beyond a ML post)

Authentication is a big deal, and an important component, I want to
see good patches go in but care needs to be taken.  Understand that
your method of submitting patches puts a huge burden on those
accepting and integrating those patches.  Your brazen attitude makes
you easy to ignore, even if the point you raise is valid.



More information about the Catalyst mailing list