Skip to content

Commit

Permalink
Implement detection for sti for including subclasses in check (#689)
Browse files Browse the repository at this point in the history
* Implement detection for sti for including subclasses in check

* Lint

* use sti_name

* Add inverse check for can/cannot on child class
  • Loading branch information
Liberatys authored Mar 23, 2022
1 parent c97a152 commit 40bb117
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 4 deletions.
4 changes: 4 additions & 0 deletions lib/cancan/class_matcher.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
require_relative 'sti_detector'

# This class is responsible for matching classes and their subclasses as well as
# upmatching classes to their ancestors.
# This is used to generate sti connections
Expand All @@ -12,6 +14,8 @@ def self.matches_subject_class?(subjects, subject)
def self.matching_class_check(subject, sub, has_subclasses)
matches = matches_class_or_is_related(subject, sub)
if has_subclasses
return matches unless StiDetector.sti_class?(sub)

matches || subject.subclasses.include?(sub)
else
matches
Expand Down
8 changes: 4 additions & 4 deletions lib/cancan/model_adapters/sti_normalizer.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
require_relative '../sti_detector'

# this class is responsible for detecting sti classes and creating new rules for the
# relevant subclasses, using the inheritance_column as a merger
module CanCan
Expand All @@ -20,9 +22,7 @@ def normalize(rules)
private

def update_rule(subject, rule, rules_cache)
return false unless subject.respond_to?(:descends_from_active_record?)
return false if subject == :all || subject.descends_from_active_record?
return false unless subject < ActiveRecord::Base
return false unless StiDetector.sti_class?(subject)

rules_cache.push(build_rule_for_subclass(rule, subject))
true
Expand All @@ -31,7 +31,7 @@ def update_rule(subject, rule, rules_cache)
# create a new rule for the subclasses that links on the inheritance_column
def build_rule_for_subclass(rule, subject)
CanCan::Rule.new(rule.base_behavior, rule.actions, subject.superclass,
rule.conditions.merge(subject.inheritance_column => subject.name), rule.block)
rule.conditions.merge(subject.inheritance_column => subject.sti_name), rule.block)
end
end
end
Expand Down
12 changes: 12 additions & 0 deletions lib/cancan/sti_detector.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# frozen_string_literal: true

class StiDetector
def self.sti_class?(subject)
return false unless defined?(ActiveRecord::Base)
return false unless subject.respond_to?(:descends_from_active_record?)
return false if subject == :all || subject.descends_from_active_record?
return false unless subject < ActiveRecord::Base

true
end
end
37 changes: 37 additions & 0 deletions spec/cancan/model_adapters/active_record_adapter_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1104,6 +1104,43 @@ class JsonTransaction < ActiveRecord::Base
end
end

context 'with rule application to subclass for non sti class' do
before do
ActiveRecord::Schema.define do
create_table :parents, force: true

create_table :children, force: true
end

class ApplicationRecord < ActiveRecord::Base
self.abstract_class = true
end

class Parent < ActiveRecord::Base
end

class Child < Parent
end
end

it 'cannot rules are not effecting parent class' do
u1 = User.create!(name: 'pippo')
ability = Ability.new(u1)
ability.can :manage, Parent
ability.cannot :manage, Child
expect(ability).not_to be_able_to(:index, Child)
expect(ability).to be_able_to(:index, Parent)
end

it 'can rules are not effecting parent class' do
u1 = User.create!(name: 'pippo')
ability = Ability.new(u1)
ability.can :manage, Child
expect(ability).to be_able_to(:index, Child)
expect(ability).not_to be_able_to(:index, Parent)
end
end

context 'when STI is in use' do
before do
ActiveRecord::Schema.define do
Expand Down

0 comments on commit 40bb117

Please sign in to comment.