[Admin] Unsupported use of can? with array of actions

[spaghetticode]

Solidus Version: all including master

Description In some views we're probably using cancancan in an unsupported way.

Instead of passing a single action, we provide an array with multiple values. For example, in app/views/spree/admin/users/orders.html.erb we're doing:

<% if can?([:admin, :edit], order) %>

which seems to mean that if the user has permissions for both actions, then they should be able to pass the check.

Unfortunately, this works only for users that can :manage the resource.

Let's take a look at the cancancan codebase, where the rule check happens:

    def relevant?(action, subject)
      subject = subject.values.first if subject.class == Hash
      @match_all || (matches_action?(action) && matches_subject?(subject))
    end

    private

    def matches_action?(action)
      @expanded_actions.include?(:manage) || @expanded_actions.include?(action)
    end

we're mostly interested in the matches_action? method, where the first part is verified if the user has :manage permissions, while the second is verified when the one dimensional array @expanded_actions includes action, which is an array in our case... and this will never happen.

I don't know if the array match was dropped at some point in the past or if the feature was never there (the documentation doesn't mention that feature), anyway I think we should change our codebase in order to support the current behavior.

Also, checking :admin seems to be a bit redundant, as it's going to be checked at the controller level when checking permissions for the edit action.

[jacobherrington]

Glancing at the docs it seems like can is able to receive an array, but can? cannot :smile:

# valid
can [:admin, :manage], Spree::Order, number: 'R987654321'

It's not too much of a leap to:

# invalid
can?([:create, :update], Spree::ProductProperty)