cancancan icon indicating copy to clipboard operation
cancancan copied to clipboard

Granting read permission on intermediate STI table prevents any records being returned

Open owst opened this issue 3 years ago • 4 comments

Steps to reproduce

Similar (but I think a different issue) to #809.

Granting read permission on an intermediate STI subclass as well as the STI base class leads to no records being returned by accessible_by. With the following STI hierarchy:

class Vehicle < ActiveRecord::Base
class Car < Vehicle
class Motorbike < Vehicle
class Ferrari < Car
class Suzuki < Motorbike

Given

can :read, Vehicle
can :read, Car

Vehicle.accessible_by(ability, :index) # Expected to include all Car/Motorbike instances, but is actually empty

compared to

can :read, Vehicle

Vehicle.accessible_by(ability, :index) # Expected and does include all Car/Motorbike instances

as there is no conflict in permission granted the result is surprising.

For whatever reason the query being issued includes a constraint on the intermediate sub-class type:

Vehicle Load (0.1ms)  SELECT "vehicles".* FROM "vehicles" WHERE "vehicles"."type" = ?  [["type", "Car"]]

and since no rows have this type no rows are returned.

Reproduction:

require 'bundler/inline'

gemfile(true) do
  source 'https://rubygems.org'
  gem 'rails', '= 6.1.7'
  gem 'cancancan', '= 3.4.0', require: false # require false to force rails to be required first
  gem 'sqlite3'
end

require 'active_record'
require 'cancancan'
require 'minitest/autorun'
require 'logger'

ActiveRecord::Base.establish_connection(adapter: 'sqlite3', database: ':memory:')
ActiveRecord::Base.logger = Logger.new(STDOUT)

ActiveRecord::Schema.define do
  create_table :vehicles, force: true do |t|
    t.string :type
  end
end

class Vehicle < ActiveRecord::Base; end
class Car < Vehicle; end
class Motorbike < Vehicle; end
class Ferrari < Car; end
class Suzuki < Motorbike; end

class Ability
  include CanCan::Ability

  def initialize
    can :read, Vehicle
    can :read, Car       # The next 2 lines are problematic - comment to prevent failure.
    can :read, Motorbike
    can :read, Suzuki    # The next 2 lines seem to have no effect on this issue
    can :read, Ferrari
  end
end

class BugTest < Minitest::Test
  def test_bug
    ferrari = Ferrari.create!
    suzuki = Suzuki.create!

    ability = Ability.new

    # These assertions pass
    assert_equal true, ability.can?(:index, Vehicle)
    assert_equal true, ability.can?(:index, Car)
    assert_equal true, ability.can?(:index, Ferrari)
    assert_equal true, ability.can?(:index, Motorbike)
    assert_equal true, ability.can?(:index, Suzuki)

    # This assertion fails (the actual array is empty)
    assert_equal [ferrari, suzuki], Vehicle.accessible_by(ability, :index).to_a
  end
end

Expected behavior

All subclass instances should be returned by accessible_by.

Actual behavior

No subclass instances are returned by accessible_by.

System configuration

Rails version: 6.1.7

Ruby version: 2.7.6

CanCanCan version 3.4.0

owst avatar Dec 21 '22 13:12 owst

Similarly to this comment https://github.com/CanCanCommunity/cancancan/pull/663#issuecomment-1141121182 adding this before the test "fixes" the test:

CanCan::RulesCompressor.class_eval do
  def compress(array)
    # Don't compress...
    array
  end
end

owst avatar Dec 21 '22 15:12 owst

I just wasted a few hours fighting with this error today 😔

a-chris avatar Apr 14 '23 19:04 a-chris

Similarly to this comment #663 (comment) adding this before the test "fixes" the test:

CanCan::RulesCompressor.class_eval do
  def compress(array)
    # Don't compress...
    array
  end
end

This solution didn't work for me, I figured out the problem is the StiNormalizer introduced in #649 so a possible solution could be to go back to the 3.1.0 version.

At the end I prefered to overwrite the StiDetector class to that it stops adding the type = ... filter to the query only for the models I need

class StiDetector
  class << self
    alias_method :sti_class_original?, :sti_class?

    def sti_class?(subject)
      if subject == MyModel
        false
      else
        sti_class_original?(subject)
      end
    end
  end
end

a-chris avatar Apr 17 '23 10:04 a-chris

I am getting the same issue for some reason cancancan 3.5.0 is adding an extra where condition type="'.

This patch worked fine: https://github.com/CanCanCommunity/cancancan/pull/663#issuecomment-752383109 But it breaks another part of my app. :(

System configuration

  • Rails version: 6.1.7
  • Ruby version: 2.7.7
  • CanCanCan version 3.5.0

rvalladares77 avatar May 25 '23 14:05 rvalladares77