From 7a124958a0e98759a93680515932d78718124056 Mon Sep 17 00:00:00 2001 From: Ernie Miller Date: Thu, 17 Dec 2015 16:47:44 -0500 Subject: [PATCH] Support Devise paranoid mode The overridden methods in UserPasswordsController were inadvertently missing out on Devise's "paranoid mode", which protects against user enumeration attacks. Current dependency is Devise ~> 3.4.1 according to the gemspec, which calls a separate method to determine redirect location. All existing specs still pass. Added a spec for nonexistent users. The situation with existing users requires a good deal more setup since it would trigger the e-mail. --- README.md | 4 ++++ config/locales/en.yml | 1 + .../spree/admin/user_passwords_controller.rb | 18 ++------------ .../spree/user_passwords_controller.rb | 18 ++------------ .../spree/user_passwords_controller_spec.rb | 24 +++++++++++++++++++ 5 files changed, 33 insertions(+), 32 deletions(-) diff --git a/README.md b/README.md index 68b073cce..ce1cb1b59 100644 --- a/README.md +++ b/README.md @@ -108,6 +108,10 @@ You need to do a quick one-time creation of a test application and then you can bundle exec rake test_app +You'll also need `phantomjs` (an NPM package) installed. + + npm install -g phantomjs + Then run the rspec tests. bundle exec rake spec diff --git a/config/locales/en.yml b/config/locales/en.yml index 459af3739..7fc450a29 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -35,6 +35,7 @@ en: cannot_be_blank: Your password cannot be blank. no_token: "You can't access this page without coming from a password reset email. If you do come from a password reset email, please make sure you used the full URL provided." send_instructions: You will receive an email with instructions about how to reset your password in a few minutes. + send_paranoid_instructions: If your email address exists in our database, you will receive a password recovery link at your email address in a few minutes. updated: Your password was changed successfully. You are now signed in. user_registrations: destroyed: Bye! Your account was successfully cancelled. We hope to see you again soon. diff --git a/lib/controllers/backend/spree/admin/user_passwords_controller.rb b/lib/controllers/backend/spree/admin/user_passwords_controller.rb index 708283580..3d0582b2c 100644 --- a/lib/controllers/backend/spree/admin/user_passwords_controller.rb +++ b/lib/controllers/backend/spree/admin/user_passwords_controller.rb @@ -9,22 +9,8 @@ class Spree::Admin::UserPasswordsController < Devise::PasswordsController helper 'spree/admin/tables' layout 'spree/layouts/admin' - # Overridden due to bug in Devise. - # respond_with resource, :location => new_session_path(resource_name) - # is generating bad url /session/new.user - # - # overridden to: - # respond_with resource, :location => spree.login_path - # - def create - self.resource = resource_class.send_reset_password_instructions(params[resource_name]) - - if resource.errors.empty? - set_flash_message(:notice, :send_instructions) if is_navigational_format? - respond_with resource, :location => spree.admin_login_path - else - respond_with_navigational(resource) { render :new } - end + def after_sending_reset_password_instructions_path_for(resource_name) + spree.admin_login_path end # Devise::PasswordsController allows for blank passwords. diff --git a/lib/controllers/frontend/spree/user_passwords_controller.rb b/lib/controllers/frontend/spree/user_passwords_controller.rb index 6f9717178..06f77e6f9 100644 --- a/lib/controllers/frontend/spree/user_passwords_controller.rb +++ b/lib/controllers/frontend/spree/user_passwords_controller.rb @@ -6,22 +6,8 @@ class Spree::UserPasswordsController < Devise::PasswordsController include Spree::Core::ControllerHelpers::Order include Spree::Core::ControllerHelpers::Store - # Overridden due to bug in Devise. - # respond_with resource, :location => new_session_path(resource_name) - # is generating bad url /session/new.user - # - # overridden to: - # respond_with resource, :location => spree.login_path - # - def create - self.resource = resource_class.send_reset_password_instructions(params[resource_name]) - - if resource.errors.empty? - set_flash_message(:notice, :send_instructions) if is_navigational_format? - respond_with resource, :location => spree.login_path - else - respond_with_navigational(resource) { render :new } - end + def after_sending_reset_password_instructions_path_for(resource_name) + spree.login_path end # Devise::PasswordsController allows for blank passwords. diff --git a/spec/controllers/spree/user_passwords_controller_spec.rb b/spec/controllers/spree/user_passwords_controller_spec.rb index 826e9c106..65a4a2a6c 100644 --- a/spec/controllers/spree/user_passwords_controller_spec.rb +++ b/spec/controllers/spree/user_passwords_controller_spec.rb @@ -41,4 +41,28 @@ end end end + + context '#create' do + + context 'when resetting password' do + it 'puts an error on the object' do + spree_post :create, spree_user: {email: 'made-up-email@made-up-domain.com'} + expect(response).to be_success + expect(assigns(:spree_user).kind_of?(Spree::User)).to eq true + expect(assigns(:spree_user).errors.messages[:email].first).to eq I18n.t(:not_found, scope: [:errors, :messages]) + end + + context 'with paranoid mode' do + before { Devise.paranoid = true } + after { Devise.paranoid = false } + it 'does not indicate whether the user exists' do + spree_post :create, spree_user: {email: 'made-up-email@made-up-domain.com'} + expect(response).to redirect_to spree.login_path + expect(flash[:notice]).to eq I18n.t(:send_paranoid_instructions, scope: [:devise, :user_passwords, :spree_user]) + end + end + + end + + end end