Skip to content

Commit

Permalink
Fix permissions to see admin menu items
Browse files Browse the repository at this point in the history
Right now a user without roles like logged users or guests can
see the admin menu partially.

This is because it actually has permission to show zones and
shipping methods and 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: https://github.com/solidusio/solidus/blob/3c8ffcc34f9248b286a9d4ca94d1f9a3197ac7b2/backend/app/controllers/spree/admin/base_controller.rb#L29

Test setup lines have been removed since they are useless now.
  • Loading branch information
kennyadsl committed Nov 12, 2020
1 parent 3c8ffcc commit a8952f6
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 18 deletions.
10 changes: 5 additions & 5 deletions backend/app/views/spree/admin/shared/_settings_sub_menu.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -3,24 +3,24 @@
<%= tab :stores, label: :stores, url: spree.admin_stores_path %>
<% end %>

<% if can?(:show, Spree::PaymentMethod) %>
<% if can?(:admin, Spree::PaymentMethod) %>
<%= tab :payments, url: spree.admin_payment_methods_path %>
<% end %>

<% if can?(:show, Spree::TaxCategory) || can?(:show, Spree::TaxRate) %>
<% if can?(:admin, Spree::TaxCategory) || can?(:admin, Spree::TaxRate) %>
<%= tab :taxes, url: spree.admin_tax_categories_path, match_path: %r(tax_categories|tax_rates) %>
<% end %>

<% if can?(:show, Spree::RefundReason) || can?(:show, Spree::ReimbursementType) ||
<% if can?(:admin, Spree::RefundReason) || can?(:admin, Spree::ReimbursementType) ||
can?(:show, Spree::ReturnReason) || can?(:show, Spree::AdjustmentReason) %>
<%= tab :checkout, url: spree.admin_refund_reasons_path, match_path: %r(refund_reasons|reimbursement_types|return_reasons|adjustment_reasons|store_credit_reasons) %>
<% end %>

<% if can?(:show, Spree::ShippingMethod) || can?(:show, Spree::ShippingCategory) || can?(:show, Spree::StockLocation) %>
<% if can?(:admin, Spree::ShippingMethod) || can?(:admin, Spree::ShippingCategory) || can?(:admin, Spree::StockLocation) %>
<%= tab :shipping, url: spree.admin_shipping_methods_path, match_path: %r(shipping_methods|shipping_categories|stock_locations) %>
<% end %>

<% if can?(:show, Spree::Zone) %>
<% if can?(:admin, Spree::Zone) %>
<%= tab :zones, url: spree.admin_zones_path %>
<% end %>
</ul>
22 changes: 11 additions & 11 deletions backend/lib/spree/backend_configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -133,17 +133,17 @@ def menu_items
'wrench',
condition: -> {
can?(:admin, Spree::Store) ||
can?(:show, Spree::AdjustmentReason) ||
can?(:show, Spree::PaymentMethod) ||
can?(:show, Spree::RefundReason) ||
can?(:show, Spree::ReimbursementType) ||
can?(:show, Spree::ShippingCategory) ||
can?(:show, Spree::ShippingMethod) ||
can?(:show, Spree::StockLocation) ||
can?(:show, Spree::TaxCategory) ||
can?(:show, Spree::TaxRate) ||
can?(:show, Spree::ReturnReason) ||
can?(:show, Spree::Zone)
can?(:admin, Spree::AdjustmentReason) ||
can?(:admin, Spree::PaymentMethod) ||
can?(:admin, Spree::RefundReason) ||
can?(:admin, Spree::ReimbursementType) ||
can?(:admin, Spree::ShippingCategory) ||
can?(:admin, Spree::ShippingMethod) ||
can?(:admin, Spree::StockLocation) ||
can?(:admin, Spree::TaxCategory) ||
can?(:admin, Spree::TaxRate) ||
can?(:admin, Spree::ReturnReason) ||
can?(:admin, Spree::Zone)
},
label: :settings,
partial: 'spree/admin/shared/settings_sub_menu',
Expand Down
2 changes: 0 additions & 2 deletions backend/spec/features/admin/homepage_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,6 @@
custom_authorization! do |_user|
can [:admin, :home], :dashboards
can [:admin, :edit, :index, :show], Spree::Order
cannot [:show], Spree::StockLocation
cannot [:show], Spree::Zone
end

it 'should only display tabs fakedispatch has access to' do
Expand Down

0 comments on commit a8952f6

Please sign in to comment.