attr_accessible and attr_protected with roles should be RailsAdmin fi…

…rst class citizen.

README.md

@@ -29,6 +29,8 @@ Supported ORMs:
 
 ## <a name="notices">Notices</a>
 
+`:attr_accessible` is now taken into account: restricted fields are not editable anymore, and mass-assignement security isn't bypassed anymore. Be careful if you whitelist attributes, you'll need to whitelist association 'id' methods as well : `division_id`, `player_ids`, `commentable_type`, `commentable_id`, etc.
+
 Default scopes are now fully *active* in list views (ordering is overriden, obvisously) as they used to a while ago. This is not configurable (that would bring consistency issues with cancan scoping which brings default scope). If you don't want some default scopes in RailsAdmin, either move your scoping rules to cancan, or activate your default scope conditionnaly on user/url prefix.
 
 Configuration with ActiveRecord::Base#rails_admin is not recommended anymore and should be
@@ -1421,6 +1423,37 @@ Or even scope it like this:
     end
 
 ## <a name="authorization">Authorization</a>
+
+`:attr_accessible` and `:attr_blacklisted` are taken into account: restricted fields are not editable (read_only). 
+If you whitelist attributes, don't forget to whitelist accessible associations' 'id' methods as well : `division_id`, `player_ids`, `commentable_type`, `commentable_id`, etc.
+:attr_accessible specifies a list of accessible methods for mass-assignment in your ActiveModel models. By default, RailsAdmin uses role :default (default in ActiveModel).
+If the role you specify isn't used in your whitelist declarations, you'll free access to all attributes.
+Keep in mind that `'key' != :key`
+You can change role with a block evaluated in the context of the controller (you'll have access to view and _current_user) : 
+
+    RailsAdmin.config do |config|
+      config.attr_accessible_role do
+        _current_user.roles.first
+      end
+    end
+
+If you don't want read_only fields to be visible in your forms:
+
+    RailsAdmin.config do |c|
+      c.reload_between_requests = false # strongly advised, since mass-assignement slows things down a lot.
+      c.models do
+        edit do
+          fields do
+            visible do 
+              visible && !read_only
+            end
+          end
+        end
+      end
+    end
+
+
+
 Authorization can be added using the `authorize_with` method. If you pass a block
 it will be triggered through a before filter on every action in Rails Admin.
 

app/controllers/rails_admin/application_controller.rb

@@ -8,7 +8,7 @@ class ApplicationController < ::ApplicationController
     before_filter :_authorize!
     before_filter :set_plugin_name
 
-    helper_method :_current_user
+    helper_method :_current_user, :_attr_accessible_role
 
     def get_model
       model_name = to_model_name(params[:model_name])
@@ -42,7 +42,11 @@ def _authorize!
     def _current_user
       instance_eval &RailsAdmin::Config.current_user_method
     end
-
+
+    def _attr_accessible_role
+      instance_eval &RailsAdmin::Config.attr_accessible_role
+    end
+
     def set_plugin_name
       @plugin_name = "RailsAdmin"
     end

app/controllers/rails_admin/main_controller.rb

@@ -89,7 +89,7 @@ def new
         @authorization_adapter.authorize(:new, @abstract_model, @object)
       end
       if object_params = params[@abstract_model.to_param]
-        @object.attributes = @object.attributes.merge(object_params)
+        @object.set_attributes(@object.attributes.merge(object_params), _attr_accessible_role)
       end
       @page_name = t("admin.actions.create").capitalize + " " + @model_config.label.downcase
       @page_type = @abstract_model.pretty_name.downcase
@@ -109,7 +109,7 @@ def create
         end
         @authorization_adapter.authorize(:create, @abstract_model, @object)
       end
-      @object.attributes = @attributes
+      @object.set_attributes(@attributes, _attr_accessible_role)
       @page_name = t("admin.actions.create").capitalize + " " + @model_config.label.downcase
       @page_type = @abstract_model.pretty_name.downcase
 
@@ -158,9 +158,9 @@ def update
 
       @old_object = @object.dup
 
-      @model_config.update.fields.each {|f| f.parse_input(@attributes) if f.respond_to?(:parse_input) }
+      @model_config.update.fields.map {|f| f.parse_input(@attributes) if f.respond_to?(:parse_input) }
 
-      @object.attributes = @attributes
+      @object.set_attributes(@attributes, _attr_accessible_role)
 
       if @object.save
         AbstractHistory.create_update_history @abstract_model, @object, @cached_assocations_hash, associations_hash, @modified_assoc, @old_object, _current_user
@@ -293,7 +293,7 @@ def get_bulk_objects(ids)
     end
 
     def get_sort_hash
-      params[:sort] = params[:sort_reverse] = nil unless @model_config.list.visible_fields.map {|f| f.name.to_s}.include? params[:sort]
+      params[:sort] = params[:sort_reverse] = nil unless @model_config.list.with(:view => self, :object => @abstract_model.model.new).visible_fields.map {|f| f.name.to_s}.include? params[:sort]
 
       params[:sort] ||= @model_config.list.sort_by.to_s
       params[:sort_reverse] ||= 'false'

app/views/rails_admin/main/_form_fieldset.html.haml

@@ -1,4 +1,4 @@
-- if (fields = fieldset.fields.map{ |f| f.with(:form => form, :object => object, :view => self) }.select(&:visible)).length > 0
+- if (fields = fieldset.fields.map{ |f| f.with(:form => form, :object => object, :view => self) }.select(&:visible?)).length > 0
   %fieldset
     %legend= fieldset.label
 

app/views/rails_admin/main/edit.html.haml

@@ -12,6 +12,6 @@
     .inner
       = render(:partial => 'layouts/rails_admin/flash', :locals => {:flash => flash})
       = send(@model_config.update.form_builder, @object, :url => update_path(@abstract_model, @object.id), :html => { :method => "put", :multipart => true, :class => "form" }) do |form|
-        - @model_config.edit.with(:object => @object).visible_groups.each do |fieldset|
+        - @model_config.edit.with(:form => form, :object => @object, :view => self).visible_groups.each do |fieldset|
           = render :partial => 'form_fieldset', :locals => { :fieldset => fieldset, :form => form, :object => @object }
         = render :partial => 'submit_buttons'

app/views/rails_admin/main/export.html.haml

@@ -1,5 +1,5 @@
 - params = request.params.except(:action, :controller, :utf8, :page, :per_page, :format, :authenticity_token)
-- visible_fields = @model_config.export.visible_fields
+- visible_fields = @model_config.export.with(:view => self, :object => @abstract_model.model.new).visible_fields
 
 #block-tables.block
   = breadcrumbs_for :export, @abstract_model
@@ -36,7 +36,7 @@
         - visible_fields.select{ |f| f.association? && !f.association[:polymorphic] }.each do |field|
           - association = field.association
           - associated_model = association[:type] == :belongs_to ? association[:parent_model] : association[:child_model]
-          - fields = RailsAdmin.config(associated_model).export.visible_fields.select{ |f| !f.association? }
+          - fields = RailsAdmin.config(associated_model).export.with(:view => self, :object => associated_model.new).visible_fields.select{ |f| !f.association? }
           %fieldset
             %legend= t('admin.export.fields_from_associated', :name => field.label)
             - fields.each do |associated_model_field|

app/views/rails_admin/main/list.html.haml

@@ -3,7 +3,7 @@
   params = request.params.except(:authenticity_token, :action, :controller, :utf8, :bulk_export)
   sort = params[:sort]
   sort_reverse = params[:sort_reverse]
-  properties = @model_config.list.visible_fields.map{|property| property.with(:view => self)}
+  properties = @model_config.list.with(:view => self, :object => @abstract_model.model.new).visible_fields
   # columns paginate
   @filterable_fields = @model_config.list.fields.select(&:filterable?)
   @style, @other, properties = get_column_set(properties)

app/views/rails_admin/main/new.html.haml

@@ -8,6 +8,6 @@
     .inner
       = render(:partial => 'layouts/rails_admin/flash', :locals => {:flash => flash})
       = send(@model_config.create.form_builder, @object, :url => create_path(:model_name => @abstract_model.to_param, :id => @object.id), :html => { :multipart => true, :class => "form" }) do |form|
-        - @model_config.create.with(:object => @object).visible_groups.each do |fieldset|
+        - @model_config.create.with(:form => form, :object => @object, :view => self).visible_groups.each do |fieldset|
           = render :partial => 'form_fieldset', :locals => { :fieldset => fieldset, :form => form, :object => @object }
         = render :partial => 'submit_buttons'

app/views/rails_admin/main/show.html.haml

@@ -15,7 +15,7 @@
     .inner
       = render(:partial => 'layouts/rails_admin/flash', :locals => {:flash => flash})
       - @model_config.show.with(:object => @object, :view => self).visible_groups.each do |fieldset|
-        - unless (fields = fieldset.visible_fields.map{|f| f.with(:object => @object, :view => self)}).empty?
+        - unless (fields = fieldset.with(:object => @object, :view => self).visible_fields).empty?
           - if !(values = fields.map{ |f| f.pretty_value.presence }).compact.empty? || !RailsAdmin::config.compact_show_view
             .fieldset
               %h4= fieldset.label

lib/rails_admin/abstract_object.rb

@@ -13,8 +13,8 @@ def initialize(object)
       self.object = object
     end
 
-    def attributes=(attributes)
-      object.assign_attributes(attributes, :without_protection => true)
+    def set_attributes(attributes, role = nil)
+      object.assign_attributes(attributes, :as => role)
     end
 
     def method_missing(name, *args, &block)

lib/rails_admin/config.rb

@@ -30,6 +30,8 @@ class AuthenticationNotConfigured < StandardError; end
         end
       end
     end
+
+    DEFAULT_ATTR_ACCESSIBLE_ROLE = Proc.new { :default }
 
     DEFAULT_AUTHORIZE = Proc.new {}
 
@@ -114,6 +116,12 @@ def authenticate_with(&blk)
         @authenticate = blk if blk
         @authenticate || DEFAULT_AUTHENTICATION
       end
+
+
+      def attr_accessible_role(&blk)
+        @attr_accessible_role = blk if blk
+        @attr_accessible_role || DEFAULT_ATTR_ACCESSIBLE_ROLE
+      end
 
       # Setup authorization to be run as a before filter
       # This is run inside the controller instance so you can setup any authorization you need to.

lib/rails_admin/config/fields/association.rb

@@ -40,7 +40,7 @@ def association
         register_instance_option(:visible?) do
           @visible ||= !self.associated_model_config.excluded?
         end
-
+        
         # use the association name as a key, not the association key anymore!
         register_instance_option(:label) do
           @label ||= abstract_model.model.human_attribute_name association[:name]

lib/rails_admin/config/fields/base.rb

@@ -33,7 +33,7 @@ def initialize(parent, name, properties)
             extend RailsAdmin::Config::Fields::Groupable
           end
         end
-
+        
         register_instance_option(:css_class) do
           self.class.instance_variable_get("@css_class")
         end
@@ -51,7 +51,11 @@ def column_css_class(*args, &block)
         end
 
         register_instance_option(:read_only) do
-          false
+          role = bindings[:view].controller.send(:_attr_accessible_role)
+          klass = bindings[:object].class
+          whitelist = klass.accessible_attributes(role).map(&:to_s)
+          blacklist = klass.protected_attributes(role).map(&:to_s)
+          self.method_name.to_s.in?(blacklist) || (whitelist.any? ? !self.method_name.to_s.in?(whitelist) : false)
         end
 
         register_instance_option(:truncated?) do

lib/rails_admin/config/fields/group.rb

@@ -45,7 +45,7 @@ def fields_of_type(type, &block)
 
         # Reader for fields that are marked as visible
         def visible_fields
-          fields.select {|f| f.visible? }
+          fields.select {|f| f.with(bindings).visible? }.map{|f| f.with(bindings)}
         end
 
         # Configurable group label which by default is group's name humanized.

lib/rails_admin/config/has_fields.rb

@@ -105,7 +105,7 @@ def fields_of_type(type, &block)
 
       # Get all fields defined as visible.
       def visible_fields
-        fields.select {|f| f.visible? }
+        fields.select {|f| f.with(bindings).visible? }.map{|f| f.with(bindings)}
       end
     end
   end

spec/dummy_app/app/models/field_test.rb

@@ -1,3 +1,6 @@
 class FieldTest < ActiveRecord::Base
   has_one :comment, :as => :commentable
+  attr_accessible :string_field, :text_field, :integer_field, :float_field, :decimal_field, :datetime_field, :timestamp_field, :time_field, :date_field, :boolean_field, :created_at, :updated_at, :format
+  attr_accessible :string_field, :text_field, :integer_field, :float_field, :decimal_field, :datetime_field, :timestamp_field, :time_field, :date_field, :boolean_field, :created_at, :updated_at, :format, :restricted_field, :as => :custom_role
+  attr_accessible :string_field, :text_field, :integer_field, :float_field, :decimal_field, :datetime_field, :timestamp_field, :time_field, :date_field, :boolean_field, :created_at, :updated_at, :format, :protected_field, :as => :extra_safe_role
 end

spec/dummy_app/app/models/team.rb

@@ -11,7 +11,7 @@ class Team < ActiveRecord::Base
   has_many :players, :inverse_of => :team
   has_and_belongs_to_many :fans
   has_many :comments, :as => :commentable
-
+  
   def player_names_truncated
     players.map{|p| p.name}.join(", ")[0..32]
   end

spec/dummy_app/app/models/user.rb

@@ -3,11 +3,16 @@ class User < ActiveRecord::Base
   # :token_authenticatable, :confirmable, :lockable and :timeoutable
   devise :database_authenticatable, :registerable, :recoverable, :rememberable, :trackable, :validatable
 
-  # Setup accessible (or protected) attributes for your model
-  attr_accessible :email, :password, :password_confirmation, :remember_me
+  serialize :roles, Array
 
+  # Setup accessible (or protected) attributes for your model
+  #attr_accessible :email, :password, :password_confirmation, :remember_me, :roles, :avatar
+
+
   # Add Paperclip support for avatars
   has_attached_file :avatar, :styles => { :medium => "300x300>", :thumb => "100x100>" }
-
-  serialize :roles, Array
+
+  def attr_accessible_role
+    :custom_role
+  end
 end

spec/dummy_app/db/migrate/20110831090841_add_protected_field_and_restricted_field_to_field_tests.rb

@@ -0,0 +1,6 @@
+class AddProtectedFieldAndRestrictedFieldToFieldTests < ActiveRecord::Migration
+  def change
+    add_column :field_tests, :restricted_field, :string
+    add_column :field_tests, :protected_field, :string
+  end
+end

spec/factories.rb

@@ -45,6 +45,10 @@
     sequence(:password) { |n| "password" }
   end
 
+  factory :field_test do
+  end
+
+
   factory :comment do
     sequence(:content) do |n| <<-EOF
         Lorém --#{n}-- ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod

spec/lib/abstract_object_spec.rb

@@ -21,7 +21,7 @@
 
     describe "a record without associations" do
       before do
-        object.attributes = { :name => name, :number => number, :position => position, :suspended => suspended, :team_id => nil }
+        object.set_attributes({ :name => name, :number => number, :position => position, :suspended => suspended, :team_id => nil })
       end
 
       it "should create a Player with given attributes" do
@@ -31,7 +31,7 @@
         player.name.should == name
         player.number.should == number
         player.position.should == position
-        player.suspended.should == suspended
+        player.suspended.should == false # protected
         player.draft.should == nil
         player.team.should == nil
       end
@@ -41,7 +41,7 @@
       let(:draft) { Factory :draft }
 
       before do
-        object.attributes = { :name => name, :number => number, :position => position, :suspended => suspended, :team_id => nil, :draft_id => draft.id }
+        object.set_attributes({ :name => name, :number => number, :position => position, :suspended => suspended, :team_id => nil, :draft_id => draft.id })
       end
 
       it "should create a Player with given attributes" do
@@ -51,7 +51,7 @@
         player.name.should == name
         player.number.should == number
         player.position.should == position
-        player.suspended.should == suspended
+        player.suspended.should == false # protected
         player.draft.should == draft.reload
         player.team.should == nil
       end
@@ -65,7 +65,7 @@
       let(:divisions) { [Factory(:division), Factory(:division)] }
 
       before do
-        object.attributes = { :name  => name, :division_ids => divisions.map(&:id) }
+        object.set_attributes({ :name  => name, :division_ids => divisions.map(&:id) })
       end
 
       it "should create a League with given attributes and associations" do
@@ -89,7 +89,7 @@
       let(:new_number) { player.number + 29 }
 
       before do
-        object.attributes = { :number => new_number, :team_id => new_team.id, :suspended => new_suspended, :draft_id => new_draft }
+        object.set_attributes({ :number => new_number, :team_id => new_team.id, :suspended => new_suspended, :draft_id => new_draft })
         object.save
       end
 
@@ -98,7 +98,7 @@
         object.number.should == new_number
         object.name.should == name
         object.draft.should == nil
-        object.suspended.should == new_suspended
+        object.suspended.should == true # protected
         object.team.should == new_team
       end
     end

spec/lib/rails_admin_spec.rb

@@ -25,6 +25,22 @@
       end
     end
   end
+
+  describe ".attr_accessible_role" do
+    it "sould be :default by default" do
+      RailsAdmin.config.attr_accessible_role.call.should == :default
+    end
+
+    it "sould be configurable with user role for example" do
+      RailsAdmin.config do |config|
+        config.attr_accessible_role do
+          :admin
+        end
+      end
+
+      RailsAdmin.config.attr_accessible_role.call.should == :admin
+    end
+  end
 
   describe ".authorize_with" do
     context "given a key for a extension with authorization" do