Skip to content
This repository

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 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 December 12, 2012
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

Showing 1 unique commit by 1 author.

Sep 25, 2012
Vadim Pushtaev 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
This page is out of date. Refresh to see the latest.
8  app/controllers/devise/registrations_controller.rb
@@ -40,7 +40,7 @@ def update
40 40
     self.resource = resource_class.to_adapter.get!(send(:"current_#{resource_name}").to_key)
41 41
     prev_unconfirmed_email = resource.unconfirmed_email if resource.respond_to?(:unconfirmed_email)
42 42
 
43  
-    if resource.update_with_password(resource_params)
  43
+    if resource.update_with_password(resource_params, :without_protection => resource_class.devise_attr_accessible)
44 44
       if is_navigational_format?
45 45
         flash_key = update_needs_confirmation?(resource, prev_unconfirmed_email) ?
46 46
           :update_needs_confirmation : :updated
@@ -82,9 +82,9 @@ def update_needs_confirmation?(resource, previous)
82 82
 
83 83
   # Build a devise resource passing in the session. Useful to move
84 84
   # temporary session data to the newly created user.
85  
-  def build_resource(hash=nil)
86  
-    hash ||= resource_params || {}
87  
-    self.resource = resource_class.new_with_session(hash, session)
  85
+  def build_resource(params=nil)
  86
+    params ||= resource_params || {}
  87
+    self.resource = resource_class.new_with_session(params, session, :without_protection => resource_class.devise_attr_accessible)
88 88
   end
89 89
 
90 90
   # The path used after sign up. You need to overwrite this method
18  app/controllers/devise_controller.rb
@@ -28,8 +28,9 @@ def resource_class
28 28
     devise_mapping.to
29 29
   end
30 30
 
  31
+  # Returns resource params
31 32
   def resource_params
32  
-    params[resource_name]
  33
+    check_resource_params(params[resource_name])
33 34
   end
34 35
 
35 36
   # Returns a signed in resource from session (if one exists)
@@ -189,4 +190,19 @@ def request_format
189 190
   def is_navigational_format?
190 191
     Devise.navigational_formats.include?(request_format)
191 192
   end
  193
+
  194
+  # Raise an exception if devise_attr_accessible is set
  195
+  # and inappropriate param is among "params".
  196
+  def check_resource_params(params)
  197
+    if resource_class.devise_attr_accessible && params
  198
+      protected_params = params.keys.select do |param|
  199
+        !resource_class.devise_attr_accessible.index(param.to_sym)
  200
+      end
  201
+      unless protected_params.empty?
  202
+        raise "#{protected_params} are not devise_attr_accessible"
  203
+      end
  204
+    end
  205
+
  206
+    params
  207
+  end
192 208
 end
4  lib/devise.rb
@@ -51,6 +51,10 @@ module Strategies
51 51
   mattr_accessor :stretches
52 52
   @@stretches = 10
53 53
 
  54
+  # List of accessible attributes
  55
+  mattr_accessor :devise_attr_accessible
  56
+  @@devise_attr_accessible = false
  57
+
54 58
   # Keys used when authenticating a user.
55 59
   mattr_accessor :authentication_keys
56 60
   @@authentication_keys = [ :email ]
2  lib/devise/models/authenticatable.rb
@@ -172,7 +172,7 @@ def strip_whitespace
172 172
       end
173 173
 
174 174
       module ClassMethods
175  
-        Devise::Models.config(self, :authentication_keys, :request_keys, :strip_whitespace_keys,
  175
+        Devise::Models.config(self, :devise_attr_accessible, :authentication_keys, :request_keys, :strip_whitespace_keys,
176 176
           :case_insensitive_keys, :http_authenticatable, :params_authenticatable, :skip_session_storage)
177 177
 
178 178
         def serialize_into_session(record)
4  lib/devise/models/registerable.rb
@@ -16,8 +16,8 @@ module ClassMethods
16 16
         #
17 17
         # By default discards all information sent by the session by calling
18 18
         # new with params.
19  
-        def new_with_session(params, session)
20  
-          new(params)
  19
+        def new_with_session(params, session, *options)
  20
+          new(params, *options)
21 21
         end
22 22
       end
23 23
     end
7  lib/generators/templates/devise.rb
@@ -72,6 +72,13 @@
72 72
   # passing :skip => :sessions to `devise_for` in your config/routes.rb
73 73
   config.skip_session_storage = [:http_auth]
74 74
 
  75
+  # Devise uses update_attributes in its controllers, so fields like :name
  76
+  # and :password should be attr_accessible in your model. If you don't want
  77
+  # them to be attr_accissble, you can use this option. If it is set, Devise
  78
+  # uses update_attributes :without_protection, but doesn't accept any param
  79
+  # not from this list
  80
+  # config.devise_attr_accessible = [:name, :password, :password_confirmation]
  81
+
75 82
   # ==> Configuration for :database_authenticatable
76 83
   # For bcrypt, this is the cost for hashing the password and defaults to 10. If
77 84
   # using other encryptors, it sets how many times you want the password re-encrypted.
16  test/integration/registerable_test.rb
@@ -61,6 +61,20 @@ def user_sign_up
61 61
     assert_not user.confirmed?
62 62
   end
63 63
 
  64
+  test 'a guest user shold not be able to sign up with protected params if devise_attr_accissble is truly' do
  65
+    swap Devise, :devise_attr_accessible => [ :email ] do # now user_sign_up() is hack!
  66
+      assert_raise RuntimeError do  
  67
+        user_sign_up
  68
+      end
  69
+    end
  70
+  end
  71
+
  72
+  test 'a guest user shold be able to sign up without protected params if devise_attr_accissble is truly' do
  73
+    swap Devise, :devise_attr_accessible => [ :email, :password, :password_confirmation ] do
  74
+      user_sign_up # Doesn't raise RuntimeError
  75
+    end
  76
+  end
  77
+ 
64 78
   test 'a guest user should receive the confirmation instructions from the default mailer' do
65 79
     user_sign_up
66 80
     assert_equal ['please-change-me@config-initializers-devise.com'], ActionMailer::Base.deliveries.first.from
@@ -342,4 +356,4 @@ class ReconfirmableRegistrationTest < ActionController::IntegrationTest
342 356
     assert_equal "admin.new@example.com", Admin.first.unconfirmed_email
343 357
     assert Admin.first.valid_password?('pas123')
344 358
   end
345  
-end
  359
+end
2  test/rails_app/lib/shared_user.rb
@@ -14,7 +14,7 @@ module SharedUser
14 14
   end
15 15
 
16 16
   module ExtendMethods
17  
-    def new_with_session(params, session)
  17
+    def new_with_session(params, session, options)
18 18
       super.tap do |user|
19 19
         if data = session["devise.facebook_data"]
20 20
           user.email = data["email"]
Commit_comment_tip

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.