Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Allow to specify roles for mass-assignment as array #1829

Merged
merged 2 commits into from

4 participants

@wildchild

Currently we must write:

class User
  attr_accessible :email
  attr_accessible :email, :as => :admin
end

I believe alternate syntax is useful:

class User
  attr_accessible :email, :as => [:default, :admin]
end
@dmathieu dmathieu commented on the diff
activemodel/lib/active_model/mass_assignment_security.rb
@@ -111,7 +111,10 @@ module ActiveModel
role = options[:as] || :default
@dmathieu Collaborator

As there can be several roles now, this variable should be roles to improve readability.

But there can be just one role. It's actually role(s).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
activemodel/lib/active_model/mass_assignment_security.rb
@@ -111,7 +111,10 @@ module ActiveModel
role = options[:as] || :default
self._protected_attributes = protected_attributes_configs.dup
- self._protected_attributes[role] = self.protected_attributes(role) + args
+
+ Array(role).each do |name|
@josevalim Owner

Please require and use Array.wrap here. It is a convention inside Rails source code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@josevalim
Owner

I have added a comment. Besides that, everything is great. Thanks for the pull request.

@wildchild

Done. Could it be applied to 3.1-stable?

@josevalim josevalim merged commit 4389fc9 into rails:master
@josevalim
Owner

Please provide a pull request for 3-1-stable.

@crizCraig

Shouldn't the active_authorizer be set on a role by role basis as well? For example if I set

attr_protected :as => :admin

All attributes will be accessible to all roles.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
View
11 activemodel/lib/active_model/mass_assignment_security.rb
@@ -1,5 +1,6 @@
require 'active_support/core_ext/class/attribute'
require 'active_support/core_ext/string/inflections'
+require 'active_support/core_ext/array/wrap'
require 'active_model/mass_assignment_security/permission_set'
require 'active_model/mass_assignment_security/sanitizer'
@@ -111,7 +112,10 @@ def attr_protected(*args)
role = options[:as] || :default
@dmathieu Collaborator

As there can be several roles now, this variable should be roles to improve readability.

But there can be just one role. It's actually role(s).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
self._protected_attributes = protected_attributes_configs.dup
- self._protected_attributes[role] = self.protected_attributes(role) + args
+
+ Array.wrap(role).each do |name|
+ self._protected_attributes[name] = self.protected_attributes(name) + args
+ end
self._active_authorizer = self._protected_attributes
end
@@ -170,7 +174,10 @@ def attr_accessible(*args)
role = options[:as] || :default
self._accessible_attributes = accessible_attributes_configs.dup
- self._accessible_attributes[role] = self.accessible_attributes(role) + args
+
+ Array.wrap(role).each do |name|
+ self._accessible_attributes[name] = self.accessible_attributes(name) + args
+ end
self._active_authorizer = self._accessible_attributes
end
View
14 activemodel/test/cases/mass_assignment_security_test.rb
@@ -43,6 +43,20 @@ def test_attributes_accessible_with_admin_role
assert_equal expected, sanitized
end
+ def test_attributes_accessible_with_roles_given_as_array
+ user = Account.new
+ expected = { "name" => "John Smith", "email" => "john@smith.com" }
+ sanitized = user.sanitize_for_mass_assignment(expected.merge("admin" => true))
+ assert_equal expected, sanitized
+ end
+
+ def test_attributes_accessible_with_admin_role_when_roles_given_as_array
+ user = Account.new
+ expected = { "name" => "John Smith", "email" => "john@smith.com", "admin" => true }
+ sanitized = user.sanitize_for_mass_assignment(expected.merge("super_powers" => true), :admin)
+ assert_equal expected, sanitized
+ end
+
def test_attributes_protected_by_default
firm = Firm.new
expected = { }
View
10 activemodel/test/models/mass_assignment_specific.rb
@@ -20,6 +20,14 @@ class Person
public :sanitize_for_mass_assignment
end
+class Account
+ include ActiveModel::MassAssignmentSecurity
+ attr_accessible :name, :email, :as => [:default, :admin]
+ attr_accessible :admin, :as => :admin
+
+ public :sanitize_for_mass_assignment
+end
+
class Firm
include ActiveModel::MassAssignmentSecurity
@@ -65,4 +73,4 @@ def self.attributes_protected_by_default
class TightDescendant < TightPerson
attr_accessible :phone_number
attr_accessible :super_powers, :as => :admin
-end
+end
Something went wrong with that request. Please try again.