Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New devise_attr_accessible option #2071

Closed
wants to merge 1 commit into from

Conversation

VadimPushtaev
Copy link

Following #716

Problem

Devise uses update_attributes in its controllers, so fields like :name and :password should be attr_accessible in your model. But it should be client's decision, not Devises's, what fields should be attr_accessible. Some people prefer to control all params by their own, for example.

There must be a way to say to Devise "don't rely on attr_accessible". But in this case, we want to provide some list of acceptable attributes to Devise so it can control them by its own.

Solution

I implement new option: devise_attr_accissble.

If it is set:

  1. Devise uses update_attributes params, :without_protection => true, so attr_accessible doesn't matter anymore.
  2. Devise checks that any param in resource is among devise_attr_accissble list.

Example

This is going to work:

class User < ActiveRecord::Base
  devise :database_authenticatable, :registerable, :encryptable,
         :recoverable, :rememberable, :trackable, :validatable, :confirmable,
         :devise_attr_accessible => [ :name, :password, :password_confirmation ]

  attr_accessible #nothing
end

Devise uses update_attributes in its controllers, so fields like :name
and :password should be attr_accessible in your model. If you don't want
them to be attr_accissble, you can use this option. If it is set, Devise
uses update_attributes :without_protection, but doesn't accept any param
not from this list:

config.devise_attr_accessible = [:name, :password, :password_confirmation]
@josevalim
Copy link
Contributor

I will leave this open, but if we are going to solve this problem, it should be along the same lines as Rails 4 parameter. I.e. moving the logic to the controller instead of the model.

@VadimPushtaev
Copy link
Author

Could you please explain what do you mean? What's "Rails 4 parameter"? Thanks in advance.

@VadimPushtaev
Copy link
Author

@rafaelfranca Thanks, I love this! ProtectedAttributes disgruntles me so much.

@josevalim As far as I understand, something like my devise_attr_accessible will be necessary in Rails 4 controller, but still optional in Rails 3. Do you want to defer my commit until you start to think about new default Devise controller? Do you have any plan about it? It would be awesome to join them :)

@rewritten
Copy link

On the contrary, the Devise controller can mark the params as permitted, so it will enable any field update. This is probably the whole point of the StrongParameters, to move the "accessibility" of parameters into the controllers, where it belong. So the Devise controller can ensure just :name and :password are accepted in the password reset screen.

@VadimPushtaev
Copy link
Author

As far as I understand it's up to programmer to add some field in password reset screen. And devise should know about this.

@josevalim josevalim closed this Dec 13, 2012
@josevalim
Copy link
Contributor

This was closed in favor of #2172 which also includes a way to handle the parameters permission in the controller.

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

Successfully merging this pull request may close these issues.

None yet

4 participants