Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

New devise_attr_accessible option #2071

Closed
wants to merge 1 commit into from

4 participants

Vadim Pushtaev José Valim Rafael Mendonça França Saverio Trioni
Vadim Pushtaev

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
Vadim Pushtaev VadimPushtaev New devise_attr_accessible option
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]
dc9237d
José Valim
Owner

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.

Vadim Pushtaev

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

Vadim Pushtaev

@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 :)

Saverio Trioni

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.

Vadim Pushtaev

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

José Valim josevalim closed this
José Valim
Owner

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
Commits on Sep 25, 2012
  1. Vadim Pushtaev

    New devise_attr_accessible option

    VadimPushtaev authored
    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]
This page is out of date. Refresh to see the latest.
8 app/controllers/devise/registrations_controller.rb
View
@@ -40,7 +40,7 @@ def update
self.resource = resource_class.to_adapter.get!(send(:"current_#{resource_name}").to_key)
prev_unconfirmed_email = resource.unconfirmed_email if resource.respond_to?(:unconfirmed_email)
- if resource.update_with_password(resource_params)
+ if resource.update_with_password(resource_params, :without_protection => resource_class.devise_attr_accessible)
if is_navigational_format?
flash_key = update_needs_confirmation?(resource, prev_unconfirmed_email) ?
:update_needs_confirmation : :updated
@@ -82,9 +82,9 @@ def update_needs_confirmation?(resource, previous)
# Build a devise resource passing in the session. Useful to move
# temporary session data to the newly created user.
- def build_resource(hash=nil)
- hash ||= resource_params || {}
- self.resource = resource_class.new_with_session(hash, session)
+ def build_resource(params=nil)
+ params ||= resource_params || {}
+ self.resource = resource_class.new_with_session(params, session, :without_protection => resource_class.devise_attr_accessible)
end
# The path used after sign up. You need to overwrite this method
18 app/controllers/devise_controller.rb
View
@@ -28,8 +28,9 @@ def resource_class
devise_mapping.to
end
+ # Returns resource params
def resource_params
- params[resource_name]
+ check_resource_params(params[resource_name])
end
# Returns a signed in resource from session (if one exists)
@@ -189,4 +190,19 @@ def request_format
def is_navigational_format?
Devise.navigational_formats.include?(request_format)
end
+
+ # Raise an exception if devise_attr_accessible is set
+ # and inappropriate param is among "params".
+ def check_resource_params(params)
+ if resource_class.devise_attr_accessible && params
+ protected_params = params.keys.select do |param|
+ !resource_class.devise_attr_accessible.index(param.to_sym)
+ end
+ unless protected_params.empty?
+ raise "#{protected_params} are not devise_attr_accessible"
+ end
+ end
+
+ params
+ end
end
4 lib/devise.rb
View
@@ -51,6 +51,10 @@ module Strategies
mattr_accessor :stretches
@@stretches = 10
+ # List of accessible attributes
+ mattr_accessor :devise_attr_accessible
+ @@devise_attr_accessible = false
+
# Keys used when authenticating a user.
mattr_accessor :authentication_keys
@@authentication_keys = [ :email ]
2  lib/devise/models/authenticatable.rb
View
@@ -172,7 +172,7 @@ def strip_whitespace
end
module ClassMethods
- Devise::Models.config(self, :authentication_keys, :request_keys, :strip_whitespace_keys,
+ Devise::Models.config(self, :devise_attr_accessible, :authentication_keys, :request_keys, :strip_whitespace_keys,
:case_insensitive_keys, :http_authenticatable, :params_authenticatable, :skip_session_storage)
def serialize_into_session(record)
4 lib/devise/models/registerable.rb
View
@@ -16,8 +16,8 @@ module ClassMethods
#
# By default discards all information sent by the session by calling
# new with params.
- def new_with_session(params, session)
- new(params)
+ def new_with_session(params, session, *options)
+ new(params, *options)
end
end
end
7 lib/generators/templates/devise.rb
View
@@ -72,6 +72,13 @@
# passing :skip => :sessions to `devise_for` in your config/routes.rb
config.skip_session_storage = [:http_auth]
+ # 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]
+
# ==> Configuration for :database_authenticatable
# 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.
16 test/integration/registerable_test.rb
View
@@ -61,6 +61,20 @@ def user_sign_up
assert_not user.confirmed?
end
+ test 'a guest user shold not be able to sign up with protected params if devise_attr_accissble is truly' do
+ swap Devise, :devise_attr_accessible => [ :email ] do # now user_sign_up() is hack!
+ assert_raise RuntimeError do
+ user_sign_up
+ end
+ end
+ end
+
+ test 'a guest user shold be able to sign up without protected params if devise_attr_accissble is truly' do
+ swap Devise, :devise_attr_accessible => [ :email, :password, :password_confirmation ] do
+ user_sign_up # Doesn't raise RuntimeError
+ end
+ end
+
test 'a guest user should receive the confirmation instructions from the default mailer' do
user_sign_up
assert_equal ['please-change-me@config-initializers-devise.com'], ActionMailer::Base.deliveries.first.from
@@ -342,4 +356,4 @@ class ReconfirmableRegistrationTest < ActionController::IntegrationTest
assert_equal "admin.new@example.com", Admin.first.unconfirmed_email
assert Admin.first.valid_password?('pas123')
end
-end
+end
2  test/rails_app/lib/shared_user.rb
View
@@ -14,7 +14,7 @@ module SharedUser
end
module ExtendMethods
- def new_with_session(params, session)
+ def new_with_session(params, session, options)
super.tap do |user|
if data = session["devise.facebook_data"]
user.email = data["email"]
Something went wrong with that request. Please try again.