SCrypt Support #1727

Closed
wants to merge 5 commits into
from

Conversation

Projects
None yet
6 participants

damien commented Mar 19, 2012

I wanted to use SCrypt in an app I've just started on and noticed Devise didn't currently support it. After seeing #847 I decided to try to add it in myself.

This is a pretty short commit with no tests. The only dependency required for this generator is scrypt (gem install scrypt). I'd add some, but I wasn't sure what the convention here was. If someone could point me in the correct direction I'd be happy to update this pull request with whatever test coverage is needed.

Owner

josevalim commented Mar 19, 2012

Thanks for the pull request. A few things:

  1. We should require scrypt at the top of the encryptor file;

  2. We should add the Scrypt encryptor to the autoload list: https://github.com/plataformatec/devise/blob/master/lib/devise.rb#L24

  3. If scrypt works like bcrypt, it means the salt is managed by the library, which means we will need to tackle this a bit differently (but don't worry, I will manage this part later);

  4. Finally, could you please add a test? Thanks!

damien commented Mar 19, 2012

@josevalim: Thanks for all the feedback! I just discovered where the Encrytpors tests are located, so I'll write a few more and implement your suggestions later this evening.

damien commented Mar 20, 2012

So, some bad news: the scrypt gem segfaults while running under the Devise test suite.

I suspect it may be more an issue of running scrypt under Bundler, but I have yet to verify or find a work-around for the issue. If anyone has suggestions, ideas, or knows enough C to help out, please check out pbhogan/scrypt#4

josevalim closed this Apr 17, 2012

The scrypt gem has been updated to 1.10 and seems to have fixed the segfaults. In the mean time though encryptable has been moved to a separate gem. So I took damien's code and refactored it for devise-encryptable with the new version of scrypt. Things seem to work. I haven't been able to generate the segfaults, though damien did mention in the scrypt#4 issue that he was getting it intermittently. I am running the tests as many times as possible(100+ atm) till I am sure that the segfaults issue is fixed.

https://github.com/rovermicrover/devise-encryptable/tree/scrypt

Rovermicrovers-MacBook-Air:devise-encryptable rovermicrover$ rake
/Users/rovermicrover/.rvm/rubies/ruby-1.9.3-head/bin/ruby -I"lib:lib:test" -I"/Users/rovermicrover/.rvm/gems/ruby-1.9.3->head/gems/rake-0.9.2.2/lib" "/Users/rovermicrover/.rvm/gems/ruby-1.9.3-head/gems/rake->0.9.2.2/lib/rake/rake_test_loader.rb" "test/*_/__test.rb"
Run options: --seed 20376

Running tests:

.................

Finished tests in 0.924101s, 18.3963 tests/s, 22.7248 assertions/s.

17 tests, 21 assertions, 0 failures, 0 errors, 0 skips

While I think this proves that scrypt should now be workable with devise I am unsure if this would be something you all would now want to merge into the devise-encryptable gem or if this is something you would want to put into devise itself and just have an option for scrypt or bcrypt. If its a merge into devise-encryptable gem I will create a pull request, other wise I would be up to adding it into devise itself if you would find that agreeable, and then submit a pull request for that.

Also atm I did all the merging by hand sense I was afraid to try and merge devise and the devise-encryptable with damien code because the force is not with me when it comes to complicated merges. I did make sure to add his github into the commit message though.

I hope this helps.

damien commented Jun 1, 2012

@rovermicrover you, sir, are awesome! I'll run my own tests later and see if the problem is resolved with the newer scrypt gem for me as well.

Owner

josevalim commented Jun 1, 2012

This is great. We will bake in a new style encryptors API that work well
with BCrypt and SCrypt in the next release, which will be a great
opportunity to add support.

Sent from my iPhone

Hey guys, we've built a devise-scrypt gem that works with the new devise-encryptors API and allows to use scrypt.

I chose to use the actual SCrypt::Password utilities provided by the scrypt gem instead of working with the raw hashing functions as in the examples above. This leads to two salts being actually used: The one generated and passed in by devise, which is simply appendend to the plain text password encryption input, and the salt that the SCrypt algorithm generates itself.

👍 for using scrypt

Is there a working example? devise-encryptable throws a password_salt= not found on the User model object during registration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment