Change in rules behaviour affects RailsAdmin root actions

[dark-panda]

The change in c81cf44 to Cancan::Rule#initialize has introduced a small behavioural change that is affecting any authorization calls involving a nil subject. This was discovered using a custom RailsAdmin action that is root-level and thus has no real subject. In these cases, the calls to #can? have an action but just a nil subject.

The main difference in the behaviour can be seen in how the @subjects value is initialized when the subject argument is nil:

@subjects = [subject].flatten
# => [nil]

@subjects = Array(subject)
# => []

Because the @subjects value is an empty array as opposed to an array with a single nil element, none of the rules that would normally match against it are matching.

Perhaps the following would suffice as an easy patch?

@subjects = if subject.nil?
  [nil]
else
  Array(subject)
end

[coorasse]

can you please provide a test that shows the expected behaviour and what actually happens? You can use this gist: https://gist.github.com/coorasse/3f00f536563249125a37e15a1652648c

I see that the subjects are now [] instead of [nil] but this is internal behaviour and shouldn't change anything for you. I need to see a more real-life case of what is happening. Thanks

[dark-panda]

Sure, I could write out a test, but would the test be from the RailsAdmin perspective specifically? Basically the behaviour is exactly what you describe where subjects are [] versus [nil]. When creating custom RailsAdmin actions that return true for root?, there is no subject either as a model or as an instance, and the value that RailsAdmin passes is nil. Since there are no subjects to check on when the subjects value is [], the check always returns false.

At any rate, I can write out the tests, although perhaps a backtrace with method arguments from RailsAdmin might also be useful as well? What it basically boils down to is that RailsAdmin is asking can?(:root_action_name, nil) for any root-level actions when checking for authorization, and as near as I can tell the empty subjects array causes those checks to always return false regardless of the abilities configuration.

[coorasse]

I think this is a specific case for RailsAdmin. I don't see documented anywhere else the usage of can? without a subject. So, basically, RailsAdmin uses can? :dashboard, nil, am I right?

[coorasse]

I just added a test for you situation. I think the usage of cancancan here is wrong, or at least, not officially supported. The correct thing to do would be define can :read, :dashboard and call can? :read, :dashboard

[coorasse]

This issue should be reported to @mshibuya

[coorasse]

I think cancancan2 needs a new adapter for RailsAdmin. Until then you can us the following:

module RailsAdmin
  module Extensions
    module CanCanCan2
      class AuthorizationAdapter < RailsAdmin::Extensions::CanCanCan::AuthorizationAdapter
        def authorize(action, abstract_model = nil, model_object = nil)
          return unless action
          reaction, subject = fetch_action_and_subject(action, abstract_model, model_object)
          @controller.current_ability.authorize!(reaction, subject)
        end

        def authorized?(action, abstract_model = nil, model_object = nil)
          return unless action
          reaction, subject = fetch_action_and_subject(action, abstract_model, model_object)
          @controller.current_ability.can?(reaction, subject)
        end

        def fetch_action_and_subject(action, abstract_model, model_object)
          reaction = action
          subject = model_object || abstract_model&.model
          unless subject
            subject = reaction
            reaction = :read
          end
          return reaction, subject
        end
      end
    end
  end
end

RailsAdmin.add_extension(:cancancan2, RailsAdmin::Extensions::CanCanCan2, authorization: true)

and then:

RailsAdmin.config do |config|
 config.authorize_with :cancancan2
end

and finally change ability.rb replacing can :dashboard with can :read, :dashboard

[coorasse]

Since the issue is related to an incorrect usage of cancancan from rails_admin and in this issue there is a solution provided I'm going to close it.

[tsai146]

I am not saying this is right approach; however, it solved the issue for now until the change happened.

can :dashboar, :all

[andrew-aladev]

I think we need to have a link to this issue in readme. It is hard to find it.

[andrew-aladev]

I see task #402 about "we want to remove flatten because it is slow". This issue was fixed by replacing [fit].flatten(1) with Array(fit) which breaks nil value.

You said that this is intended, but it is not. You just matched change with Reduce unnecessary allocations in Rule creation.

We need to have explicit exception if subject is nil.

[coorasse]

Updated docs on RailsAdmin (https://github.com/sferik/rails_admin/wiki/CanCanCan). For the explicit exception I agree. PR is welcome 👍