Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,11 @@ gem 'rails', '~> 4.2.6'
gem 'activerecord-session_store'
gem 'ahoy_matey'
gem 'american_date'
gem 'bcrypt'
gem 'browserify-rails'
gem 'coffee-rails', '~> 4.1.0'
gem 'devise', '~> 4.1'
gem 'devise-encryptable', github: '18F/devise-encryptable', branch: 'pbkdf2-encryptor'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was going to ask if this is something we can PR back to the main lib, but I see you already did that! heartcombo/devise-encryptable#12

Maybe add Pr link to Gemfile in case we forget to switch this out when it is merged?

gem 'dotiw'
gem 'figaro'
gem 'foundation_emails'
Expand Down
18 changes: 14 additions & 4 deletions Gemfile.lock
Original file line number Diff line number Diff line change
@@ -1,3 +1,11 @@
GIT
remote: https://github.com/18F/devise-encryptable.git
revision: 401e61c0e63c792e472fa18550ab0c74bcde65c7
branch: pbkdf2-encryptor
specs:
devise-encryptable (0.2.0)
devise (>= 2.1.0)

GIT
remote: https://github.com/18F/identity-proofer-gem.git
revision: d013230ddeba07d6f37a58c06351218bb5f525fe
Expand Down Expand Up @@ -299,7 +307,7 @@ GEM
mime-types-data (~> 3.2015)
mime-types-data (3.2016.0521)
mini_portile2 (2.1.0)
minitest (5.9.0)
minitest (5.9.1)
multi_json (1.12.1)
multi_xml (0.5.5)
nenv (0.3.0)
Expand Down Expand Up @@ -392,7 +400,7 @@ GEM
rake (>= 0.8.7)
thor (>= 0.18.1, < 2.0)
rainbow (2.1.0)
rake (11.2.2)
rake (11.3.0)
randexp (0.1.7)
rb-fsevent (0.9.7)
rb-inotify (0.9.7)
Expand All @@ -409,7 +417,7 @@ GEM
rainbow (~> 2.0)
referer-parser (0.3.0)
request_store (1.3.1)
responders (2.2.0)
responders (2.3.0)
railties (>= 4.2.0, < 5.1)
rotp (3.2.0)
rqrcode (0.10.1)
Expand Down Expand Up @@ -602,6 +610,7 @@ DEPENDENCIES
activerecord-session_store
ahoy_matey
american_date
bcrypt
better_errors
binding_of_caller
brakeman
Expand All @@ -618,6 +627,7 @@ DEPENDENCIES
database_cleaner
derailed
devise (~> 4.1)
devise-encryptable!
dotiw
email_spec
factory_girl_rails
Expand Down Expand Up @@ -682,4 +692,4 @@ DEPENDENCIES
xmlenc (~> 0.6.4)

BUNDLED WITH
1.12.5
1.13.1
2 changes: 1 addition & 1 deletion app/forms/recovery_code_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ def submit
attr_reader :user, :code, :success

def valid_recovery_code?
Devise::Encryptor.compare(User, user.recovery_code, code)
RecoveryCodeGenerator.compare(user.recovery_code, code)
end

def result
Expand Down
2 changes: 1 addition & 1 deletion app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ class User < ActiveRecord::Base

after_validation :set_default_role, if: :new_record?

devise :confirmable, :database_authenticatable, :recoverable, :registerable,
devise :confirmable, :database_authenticatable, :encryptable, :recoverable, :registerable,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same comment as other PR -- thoughts on listing each item on its own line?

:timeoutable, :trackable, :two_factor_authenticatable, :omniauthable,
omniauth_providers: [:saml]

Expand Down
11 changes: 10 additions & 1 deletion app/services/recovery_code_generator.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,13 @@
class RecoveryCodeGenerator
STRETCHES = 12
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what does this mean?


def self.compare(hashed_code, recovery_code)
return false if hashed_code.blank?
bcrypt = BCrypt::Password.new(hashed_code)
password = BCrypt::Engine.hash_secret(recovery_code, bcrypt.salt)
Devise.secure_compare(password, hashed_code)
end

def initialize(user, length: 16)
@user = user
@length = length
Expand All @@ -15,7 +24,7 @@ def create
attr_reader :length, :user

def hashed_code
Devise::Encryptor.digest(User, raw_recovery_code)
BCrypt::Password.create(raw_recovery_code, cost: STRETCHES).to_s
end

def raw_recovery_code
Expand Down
12 changes: 5 additions & 7 deletions config/initializers/devise.rb
Original file line number Diff line number Diff line change
Expand Up @@ -91,12 +91,10 @@
# For bcrypt, this is the cost for hashing the password and defaults to 10. If
# using other encryptors, it sets how many times you want the password re-encrypted.
#
# Limiting the stretches to just one in testing will increase the performance of
# your test suite dramatically. However, it is STRONGLY RECOMMENDED to not use
# a value less than 10 in other environments. Note that, for bcrypt (the default
# encryptor), the cost increases exponentially with the number of stretches (e.g.
# a value of 20 is already extremely slow: approx. 60 seconds for 1 calculation).
config.stretches = Rails.env.test? ? 1 : 12
# Limiting the stretches in testing will increase the performance of
# your test suite dramatically.
# For pbkdf2, 100k iterations ~~ 0.125sec per encryption.
config.stretches = Rails.env.test? ? 1001 : 100_000

# Setup a pepper to generate the encrypted password.
# config.pepper = 'b1108db70077daa6b0be8863b3b611ade79090f24be0e2f53aea51a59bca8462265fb4407110b182a69fdf2fef56589da7c2dbeaf3081b21794387f052e1ea47'
Expand Down Expand Up @@ -204,7 +202,7 @@
# REST_AUTH_SITE_KEY to pepper).
#
# Require the `devise-encryptable` gem when using anything other than bcrypt
# config.encryptor = :sha512
config.encryptor = :pbkdf2

# ==> Scopes configuration
# Turn scoped views on. Before rendering "sessions/new", it will first check for
Expand Down
5 changes: 5 additions & 0 deletions db/migrate/20160929214801_add_users_salt.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class AddUsersSalt < ActiveRecord::Migration
def change
add_column :users, :password_salt, :string
end
end
3 changes: 2 additions & 1 deletion db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
#
# It's strongly recommended that you check this file into your version control system.

ActiveRecord::Schema.define(version: 20160915202036) do
ActiveRecord::Schema.define(version: 20160929214801) do

# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"
Expand Down Expand Up @@ -132,6 +132,7 @@
t.datetime "idv_attempted_at"
t.integer "idv_attempts", default: 0
t.string "recovery_code"
t.string "password_salt"
end

add_index "users", ["confirmation_token"], name: "index_users_on_confirmation_token", unique: true, using: :btree
Expand Down