Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
owst opened this issue Dec 21, 2022 · 4 comments

Comments

@owst
Copy link

owst commented Dec 21, 2022

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
Copy link
Author

owst commented Dec 21, 2022

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

@a-chris
Copy link

a-chris commented Apr 14, 2023

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

@a-chris
Copy link

a-chris commented Apr 17, 2023

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

@rvalladares77
Copy link

rvalladares77 commented May 25, 2023

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

This patch worked fine: #663 (comment)
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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants