On 08/31/2011 01:54 PM, jprovazn@redhat.com wrote:
https://www.aeolusproject.org/redmine/issues/2118 This patchset replaces authlogic with warden which is used also in katello. This is first step which allows us to add more auth strategies (ldap, oauth) or use devise on top of warden, this also synchronizes us with katello auth.
What's not very good in this solution is lack of password handling/validations, which authlogic handled so now we have to handle this ourselves. This can be solved in some next step by using devise which is based on warden.
Password reset (or db recreation) is required with this patch because encrypted password has different format.
aeolus-devel mailing list aeolus-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/aeolus-devel
Looking good, ACK.
3 notes:
First, the Pool Family permissions patch was pushed after you sent this and it doesn't have the tests fixed for the Warden switch, so some of the tests are failing (both rspec and cucumber).
Could you please make sure all tests pass before pushing this?
Second, the migrations here invalidate all the existing users -- they will not be able to log in again.
I'm not sure if we need to worry about that now, especially since the fix probably won't be very easy. But it's good to keep it in mind.
Lastly, we shouldn't be implementing our own password crypto (the util/password.rb file). It looks okay on glance (at least we're stretching the hashes), but it's a slippery slope.
If Warden doesn't provide us with default secure way to encrypt password, we should just go ahead and use bcrypt:
https://github.com/codahale/bcrypt-ruby
It's packaged in Fedora, the technology has been vetted by security researchers over the years and we will not be bitten in the arse if we go with it. Can't say the same about a homebrew solution that had right about zero cryptographers banging at it.
We don't have to implement bcrypt in this patchset -- go ahead and push this. But we should do it soon. Definitely before any okay-for-public-consumption releases. Note that Devise uses brcypt by default so if we switch to that, this problem is solved.
I created a Redmine task so we can track it:
https://www.aeolusproject.org/redmine/issues/2226
Thomas