add support for polymorphic associations in active record (#814)

* add support for polymorphic associations in active record

* make tests pass for polymorphic support and ensure backwards compatibility

* remove unnecessary and noisy puts statement

---------

Co-authored-by: Bryant Morrill <bryant.morrill@mx.com>

lib/cancan/conditions_matcher.rb

@@ -39,11 +39,13 @@ def matches_non_block_conditions(subject)
     def nested_subject_matches_conditions?(subject_hash)
       parent, child = subject_hash.first
 
-      matches_base_parent_conditions = matches_conditions_hash?(parent,
-                                                                @conditions[parent.class.name.downcase.to_sym] || {})
-
       adapter = model_adapter(parent)
 
+      parent_condition_name = adapter.parent_condition_name(parent, child)
+
+      matches_base_parent_conditions = matches_conditions_hash?(parent,
+                                                                @conditions[parent_condition_name] || {})
+
       matches_base_parent_conditions &&
         (!adapter.override_nested_subject_conditions_matching?(parent, child, @conditions) ||
           adapter.nested_subject_matches_conditions?(parent, child, @conditions))

lib/cancan/model_adapters/abstract_adapter.rb

@@ -35,6 +35,11 @@ def self.matches_conditions_hash?(_subject, _conditions)
         raise NotImplemented, 'This model adapter does not support matching on a conditions hash.'
       end
 
+      # Override if parent condition could be under a different key in conditions
+      def self.parent_condition_name(parent, _child)
+        parent.class.name.downcase.to_sym
+      end
+
       # Used above override_conditions_hash_matching to determine if this model adapter will override the
       # matching behavior for nested subject.
       # If this returns true then nested_subject_matches_conditions? will be called.

lib/cancan/model_adapters/active_record_adapter.rb

@@ -42,11 +42,53 @@ def nested_subject_matches_conditions?(parent, child, all_conditions)
 
         def parent_child_conditions(parent, child, all_conditions)
           child_class = child.is_a?(Class) ? child : child.class
+          parent_class = parent.is_a?(Class) ? parent : parent.class
+
           foreign_key = child_class.reflect_on_all_associations(:belongs_to).find do |association|
-            association.klass == parent.class
+            # Do not match on polymorphic associations or it will throw an error (klass cannot be determined)
+            !association.polymorphic? && association.klass == parent.class
+          end&.foreign_key&.to_sym
+
+          # Search again in case of polymorphic associations, this time matching on the :has_many side
+          # via the :as option, as well as klass
+          foreign_key ||= parent_class.reflect_on_all_associations(:has_many).find do |has_many_assoc|
+            !matching_parent_child_polymorphic_association(has_many_assoc, child_class).nil?
           end&.foreign_key&.to_sym
+
           foreign_key.nil? ? nil : all_conditions[foreign_key]
         end
+
+        def matching_parent_child_polymorphic_association(parent_assoc, child_class)
+          return nil unless parent_assoc.klass == child_class
+          return nil if parent_assoc&.options[:as].nil?
+
+          child_class.reflect_on_all_associations(:belongs_to).find do |child_assoc|
+            # Only match this way for polymorphic associations
+            child_assoc.polymorphic? && child_assoc.name == parent_assoc.options[:as]
+          end
+        end
+
+        def child_association_to_parent(parent, child)
+          child_class = child.is_a?(Class) ? child : child.class
+          parent_class = parent.is_a?(Class) ? parent : parent.class
+
+          association = child_class.reflect_on_all_associations(:belongs_to).find do |association|
+            # Do not match on polymorphic associations or it will throw an error (klass cannot be determined)
+            !association.polymorphic? && association.klass == parent.class
+          end
+
+          return association unless association.nil?
+
+          parent_class.reflect_on_all_associations(:has_many).each do |has_many_assoc|
+            association ||= matching_parent_child_polymorphic_association(has_many_assoc, child_class)
+          end
+
+          association
+        end
+
+        def parent_condition_name(parent, child)
+          child_association_to_parent(parent, child)&.name || parent.class.name.downcase.to_sym
+        end
       end
 
       # Returns conditions intended to be used inside a database query. Normally you will not call this

spec/cancan/model_adapters/active_record_adapter_spec.rb

@@ -60,6 +60,12 @@
         t.timestamps null: false
       end
 
+      create_table(:attachments) do |t|
+        t.integer :attachable_id
+        t.string :attachable_type
+        t.timestamps null: false
+      end
+
       create_table(:users) do |t|
         t.string :name
         t.timestamps null: false
@@ -86,6 +92,7 @@ class Article < ActiveRecord::Base
       has_many :mentioned_users, through: :mentions, source: :user
       belongs_to :user
       belongs_to :project
+      has_many :attachments, as: :attachable
 
       scope :unpopular, lambda {
         joins('LEFT OUTER JOIN comments ON (comments.post_id = posts.id)')
@@ -103,17 +110,23 @@ class Mention < ActiveRecord::Base
     class Comment < ActiveRecord::Base
       belongs_to :article
       belongs_to :project
+      has_many :attachments, as: :attachable
     end
 
     class LegacyComment < ActiveRecord::Base
       belongs_to :article, foreign_key: 'post_id'
       belongs_to :project
     end
 
+    class Attachment < ActiveRecord::Base
+      belongs_to :attachable, polymorphic: true
+    end
+
     class User < ActiveRecord::Base
       has_many :articles
       has_many :mentions
       has_many :mentioned_articles, through: :mentions, source: :article
+      has_many :comments, through: :articles
     end
 
     (@ability = double).extend(CanCan::Ability)
@@ -511,6 +524,10 @@ class User < ActiveRecord::Base
       @comment2 = Comment.create!(article: @article2)
       @legacy_comment1 = LegacyComment.create!(article: @article1)
       @legacy_comment2 = LegacyComment.create!(article: @article2)
+      @attachment1 = Attachment.create!(attachable: @article1)
+      @comment_attachment1 = Attachment.create!(attachable: @comment1)
+      @attachment2 = Attachment.create!(attachable: @article2)
+      @comment_attachment2 = Attachment.create!(attachable: @comment2)
     end
 
     context 'when conditions are defined using the parent model' do
@@ -520,6 +537,8 @@ class User < ActiveRecord::Base
           ability.can :manage, Article, user: user1
           ability.can :manage, Comment, article: user1.articles
           ability.can :manage, LegacyComment, article: user1.articles
+          ability.can :manage, Attachment, attachable: user1.articles
+          ability.can :manage, Attachment, attachable: user1.comments
         end
       end
 
@@ -533,6 +552,20 @@ class User < ActiveRecord::Base
         expect(ability.can?(:manage, { @article2 => LegacyComment })).to eq(false)
         expect(ability.can?(:manage, { @article2 => @legacy_comment2 })).to eq(false)
       end
+
+      context 'when the association is polymorphic' do
+        it 'verifies parent equality correctly' do
+          expect(ability.can?(:manage, { @article1 => Attachment })).to eq(true)
+          expect(ability.can?(:manage, { @article1 => @attachment1 })).to eq(true)
+          expect(ability.can?(:manage, { @comment1 => Attachment })).to eq(true)
+          expect(ability.can?(:manage, { @comment1 => @comment_attachment1 })).to eq(true)
+
+          expect(ability.can?(:manage, { @article2 => Attachment })).to eq(false)
+          expect(ability.can?(:manage, { @article2 => @attachment2 })).to eq(false)
+          expect(ability.can?(:manage, { @comment2 => Attachment })).to eq(false)
+          expect(ability.can?(:manage, { @comment2 => @comment_attachment2 })).to eq(false)
+        end
+      end
     end
 
     context 'when conditions are defined using the parent id' do
@@ -542,6 +575,7 @@ class User < ActiveRecord::Base
           ability.can :manage, Article, user_id: user1.id
           ability.can :manage, Comment, article_id: user1.article_ids
           ability.can :manage, LegacyComment, post_id: user1.article_ids
+          ability.can :manage, Attachment, attachable_id: user1.article_ids
         end
       end
 
@@ -555,6 +589,20 @@ class User < ActiveRecord::Base
         expect(ability.can?(:manage, { @article2 => LegacyComment })).to eq(false)
         expect(ability.can?(:manage, { @article2 => @legacy_comment2 })).to eq(false)
       end
+
+      context 'when the association is polymorphic' do
+        it 'verifies parent equality correctly' do
+          expect(ability.can?(:manage, { @article1 => Attachment })).to eq(true)
+          expect(ability.can?(:manage, { @article1 => @attachment1 })).to eq(true)
+          expect(ability.can?(:manage, { @comment1 => Attachment })).to eq(true)
+          expect(ability.can?(:manage, { @comment1 => @comment_attachment1 })).to eq(true)
+
+          expect(ability.can?(:manage, { @article2 => Attachment })).to eq(false)
+          expect(ability.can?(:manage, { @article2 => @attachment2 })).to eq(false)
+          expect(ability.can?(:manage, { @comment2 => Attachment })).to eq(false)
+          expect(ability.can?(:manage, { @comment2 => @comment_attachment2 })).to eq(false)
+        end
+      end
     end
   end