From 02f63acb61630dc7f5f9146ad348aea73742ce1d Mon Sep 17 00:00:00 2001 From: kaspernj Date: Tue, 3 May 2022 13:25:26 +0000 Subject: [PATCH 1/3] Allow can? check on a single resource with a relation query --- lib/cancan/conditions_matcher.rb | 17 ++++++++++++++++- .../active_record_adapter_spec.rb | 15 +++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/lib/cancan/conditions_matcher.rb b/lib/cancan/conditions_matcher.rb index b30d3e2a..70183db0 100644 --- a/lib/cancan/conditions_matcher.rb +++ b/lib/cancan/conditions_matcher.rb @@ -82,7 +82,12 @@ def matches_hash_conditions(adapter, subject, conditions) end end - def condition_match?(attribute, value) + def condition_match?(attribute, value) # rubocop:disable Metrics/MethodLength + if defined?(ActiveRecord) && + (value.is_a?(ActiveRecord::AssociationRelation) || value.is_a?(ActiveRecord::Relation)) + return condition_match_query?(attribute, value) + end + case value when Hash hash_condition_match?(attribute, value) @@ -95,6 +100,16 @@ def condition_match?(attribute, value) end end + def condition_match_query?(attribute, query) + selects = query.values[:select] + + if selects&.length != 1 + raise "Only one column should be selected and not #{selects&.length || 0} for: #{value.to_sql}" + end + + query.where(selects[0] => attribute).any? + end + def hash_condition_match?(attribute, value) if attribute.is_a?(Array) || (defined?(ActiveRecord) && attribute.is_a?(ActiveRecord::Relation)) attribute.to_a.any? { |element| matches_conditions_hash?(element, value) } diff --git a/spec/cancan/model_adapters/active_record_adapter_spec.rb b/spec/cancan/model_adapters/active_record_adapter_spec.rb index 90065108..6b378bbf 100644 --- a/spec/cancan/model_adapters/active_record_adapter_spec.rb +++ b/spec/cancan/model_adapters/active_record_adapter_spec.rb @@ -1052,6 +1052,21 @@ class CustomPkTransaction < ActiveRecord::Base expect(CustomPkTransaction.accessible_by(ability)).to match_array([transaction1]) end + + it 'allows scope usage on can?' do + user1 = User.create! + user2 = User.create! + + ability = Ability.new(user1) + ability.can :read, Article, user_id: User.where(id: user1.id).select(:id) + + article1 = Article.create!(user: user1) + article2 = Article.create!(user: user2) + + expect(ability).to be_able_to :read, article1 + expect(ability).not_to be_able_to :read, article2 + expect(Article.accessible_by(ability)).to eq [article1] + end end end From 64143c533351331aaf86fc92d2968834f5557488 Mon Sep 17 00:00:00 2001 From: kaspernj Date: Mon, 23 Jan 2023 10:13:37 +0000 Subject: [PATCH 2/3] Fixed wrong variable --- lib/cancan/conditions_matcher.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/cancan/conditions_matcher.rb b/lib/cancan/conditions_matcher.rb index ed7c02b1..4234ab33 100644 --- a/lib/cancan/conditions_matcher.rb +++ b/lib/cancan/conditions_matcher.rb @@ -108,7 +108,7 @@ def condition_match_query?(attribute, query) selects = query.values[:select] if selects&.length != 1 - raise "Only one column should be selected and not #{selects&.length || 0} for: #{value.to_sql}" + raise "Only one column should be selected and not #{selects&.length || 0} for: #{query.to_sql}" end query.where(selects[0] => attribute).any? From 4f31fc7bcdf1ce3667c74ec9030516805c1c1ca4 Mon Sep 17 00:00:00 2001 From: kaspernj Date: Mon, 23 Jan 2023 10:13:44 +0000 Subject: [PATCH 3/3] Added test for multiple columns selected --- .../model_adapters/active_record_adapter_spec.rb | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/spec/cancan/model_adapters/active_record_adapter_spec.rb b/spec/cancan/model_adapters/active_record_adapter_spec.rb index db7693fd..4a965709 100644 --- a/spec/cancan/model_adapters/active_record_adapter_spec.rb +++ b/spec/cancan/model_adapters/active_record_adapter_spec.rb @@ -1274,6 +1274,18 @@ class CustomPkTransaction < ActiveRecord::Base expect(ability).not_to be_able_to :read, article2 expect(Article.accessible_by(ability)).to eq [article1] end + + it 'raises an error if multiple columns are selected' do + user1 = User.create! + + ability = Ability.new(user1) + ability.can :read, Article, user_id: User.where(id: user1.id).select(:id, :created_at) + + article1 = Article.create!(user: user1) + + expect { ability.can?(:read, article1) } + .to raise_error(/\AOnly one column should be selected and not 2 for: SELECT/) + end end end