Feature: Add strong parameters to the PasswordsController

[hakeem0114]

Problem

The Devise::PasswordsController uses unsanitized resource_params during password reset, which could lead to security issues.

Proposal

• Add a new :reset_password action to the DEFAULT_PERMITTED_ATTRIBUTES

DEFAULT_PERMITTED_ATTRIBUTES = {
      sign_in: [:password, :remember_me],
      sign_up: [:password, :password_confirmation],
      account_update: [:password, :password_confirmation, :current_password]
      reset_password: [:reset_password_token, :password, :password_confirmation]
    }

• Use it in the Devise::PasswordsController.

    def resource_params
      devise_parameter_sanitizer.sanitize(:reset_password)
    end

This will ensure the parameters used in the Devise::PasswordsControllerare sanitized, maintaining consistency with other controllers like RegistrationController and SessionController.

[timdiggins]

@hakeem0114 I don't agree. The code you reference is

https://github.com/heartcombo/devise/blob/fec67f98f26fcd9a79072e4581b1bd40d0c7fa1d/app/controllers/devise_controller.rb#L221-L223

but params.fetch (see https://api.rubyonrails.org/classes/ActionController/Parameters.html#method-i-fetch) still returns a ActionController::Parameters instance (unless the resource name is not present in the params, but in this case it will return an empty hash) so it is still subject to ActionController::Parameters prevention of mass-assignment (like #require but without requiring the resource name), so doesn't AFAICT introduce a security problem needing fixing

[hakeem0114]

@timdiggins Thanks for the comment! This is a feature enhancement proposal not a bug. I can't seem to add a label on this issue for some reason. When resource_params is called in the PasswordsController, it returns the entire resource (:user) instead of a sanitized one like the other controllers.

[timdiggins]

@hakeem0114 Ah - ok, if it's a feature request, what benefit does it bring? (the problem statement still implies to me that it's a security issue)

FYI After more looking, I think that in fact the security in devise (from not mass assigning) doesn't really come from strong parameters but from a very similar (but unconnected) route -- see Devise::Models::Authenticatable::ClassMethods# find_or_initialize_with_errors