From 0cdbba7c6906a491e9bce828e3e7b02a290d55dd Mon Sep 17 00:00:00 2001 From: Sandro Kalbermatter Date: Thu, 23 May 2024 17:06:46 +0200 Subject: [PATCH] In permitted_attributes, switch from database column detection to Rails attributes detection. Addresses #838 (#839) * In permitted_attributes, switch from database column detection to Rails attributes detection. Fixes #838 * Temporarly revert "In permitted_attributes, switch from database column detection to Rails attributes detection. Fixes #838" This reverts commit 530a20248b705fffd5c60f350d823f8a13596b19. Reason: Adjust tests to obtain a before and after picture * Switch AbilitySpec's User testcase to an actual database model NamedUser in order to include Rails' actual column loading behavior * Demonstrate that the current implementation misses model attributes created by Rails' "attribute" call * Re-enable fix by reverting "Temporarly revert "In permitted_attributes, switch from database column detection to Rails attributes detection. Fixes #838"" This reverts commit 1195d3916abe2a5500582c261df2dc29e3309dd3. --- .../ability/strong_parameter_support.rb | 2 +- spec/cancan/ability_spec.rb | 22 ++++++++++++++----- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/lib/cancan/ability/strong_parameter_support.rb b/lib/cancan/ability/strong_parameter_support.rb index 31da74574..892d250a3 100644 --- a/lib/cancan/ability/strong_parameter_support.rb +++ b/lib/cancan/ability/strong_parameter_support.rb @@ -31,7 +31,7 @@ def get_attributes(rule, subject) klass = subject_class?(subject) ? subject : subject.class # empty attributes is an 'all' if rule.attributes.empty? && klass < ActiveRecord::Base - klass.column_names.map(&:to_sym) - Array(klass.primary_key) + klass.attribute_names.map(&:to_sym) - Array(klass.primary_key) else rule.attributes end diff --git a/spec/cancan/ability_spec.rb b/spec/cancan/ability_spec.rb index 486cbc425..3951f3b27 100644 --- a/spec/cancan/ability_spec.rb +++ b/spec/cancan/ability_spec.rb @@ -5,6 +5,21 @@ describe CanCan::Ability do before(:each) do (@ability = double).extend(CanCan::Ability) + + connect_db + ActiveRecord::Migration.verbose = false + ActiveRecord::Schema.define do + create_table(:named_users) do |t| + t.string :first_name + t.string :last_name + end + end + + unless defined?(NamedUser) + class NamedUser < ActiveRecord::Base + attribute :role, :string # Virtual only + end + end end it 'is able to :read anything' do @@ -651,13 +666,10 @@ def active? end it 'returns an array of permitted attributes for a given action and subject' do - user_class = Class.new(ActiveRecord::Base) - allow(user_class).to receive(:column_names).and_return(%w[first_name last_name]) - allow(user_class).to receive(:primary_key).and_return('id') - @ability.can :read, user_class + @ability.can :read, NamedUser @ability.can :read, Array, :special @ability.can :action, :subject, :attribute - expect(@ability.permitted_attributes(:read, user_class)).to eq(%i[first_name last_name]) + expect(@ability.permitted_attributes(:read, NamedUser)).to eq(%i[id first_name last_name role]) expect(@ability.permitted_attributes(:read, Array)).to eq([:special]) expect(@ability.permitted_attributes(:action, :subject)).to eq([:attribute]) end