Fix permissions to see admin menu items

[kennyadsl]

Description

Closes #3834.

Right now a user without roles like logged users or guests user can see the admin menu partially, see #3834.

This behavior has been introduced with 295df5e#diff-757abc24f29994c5e06e233c60ca1f3972c2f5019fba628df718eea681adb61fR68 and it happens because a non-admin user actually has permission to show zones and shipping methods; the current code is checking against the :show ability, so it seems to be legit.

We use :admin for the rest of the menu items checks though, and this is also what we use in the controller to determine if
we can access that page, see:

solidus/backend/app/controllers/spree/admin/base_controller.rb

Line 29 in 3c8ffcc

authorize! :admin, record

Test setup lines have been removed since they are useless now.

Checklist:

• I have followed Pull Request guidelines
• I have added a detailed description into each commit message
• I have updated Guides and README accordingly to this change (if needed)
• I have added tests to cover this change (if needed)
• I have attached screenshots to this PR for visual changes (if needed)

[peterberkenbosch]

👍 nice!

[elia]

Shouldn't these two be changed to:

      cannot [:admin], Spree::StockLocation
      cannot [:admin], Spree::Zone

So we can be sure that the settings tab is still visible even when the user is not able to access some of its sub-resources?

[kennyadsl]

Even with the suggested change, the settings tab is not visible. I could change it this way to make sure the settings tab is displayed if one of the inner tab can be accessed but this won't test that the rest of the settings are excluded:

context 'as fakedispatch user' do
    before do
      allow_any_instance_of(Spree::Admin::BaseController).to receive(:try_spree_current_user).and_return(nil)
    end

    custom_authorization! do |_user|
      can [:admin, :home], :dashboards
      can [:admin, :edit, :index, :show], Spree::Order
      can [:admin], Spree::Zone
    end

    it 'should only display tabs fakedispatch has access to' do
      visit spree.admin_path
      expect(page).to have_link('Orders')
      expect(page).not_to have_link('Products')
      expect(page).not_to have_link('Promotions')
      expect(page).to have_link('Settings')
    end
  end

Let me think about something to test that.

[kennyadsl]

@elia maybe I've got a possible test, let me know your thoughts!

[elia]

Thanks @kennyadsl this look like a nice canary to catch if anything is going wrong with permissions without checking all the possible combinations 👍

[elia]

Thanks @kennyadsl this look like a nice canary to catch if anything is going wrong with permissions without checking all the possible combinations 👍