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

Admin user controller doesn't respect some abilities #3668

Closed
halilim opened this issue Jun 15, 2020 · 1 comment
Closed

Admin user controller doesn't respect some abilities #3668

halilim opened this issue Jun 15, 2020 · 1 comment

Comments

@halilim
Copy link

halilim commented Jun 15, 2020

  1. #collection does not check the current admin's abilities and returns all users. It can probably be fixed by using super.ransack... which makes use of #accessible_by.

    @search = Spree.user_class.ransack(params[:q])

  2. Similar issue for #load_roles, #load_stock_locations & #set_stock_locations, though super wouldn't cut it in these cases.

    def load_roles
    @roles = Spree::Role.all

    def load_stock_locations
    @stock_locations = Spree::StockLocation.all

    #load_stock_locations also doesn't respect user_params and uses params directly instead.
    def set_stock_locations
    @user.stock_locations = Spree::StockLocation.where(id: (params[:user][:stock_location_ids] || []))

  3. #set_roles checks the general ability but not the roles given in params. I.e. a role that can't be managed could be given to a user by manually passing its id in params.

    if user_params[:spree_role_ids] && can?(:manage, Spree::Role)
    @user.spree_roles = Spree::Role.where(id: user_params[:spree_role_ids])

Solidus Version:
2.10.1

To Reproduce
In Admin > Users, with an admin whose permissions defined as:

cannot :manage, ::Spree::User, ['some SQL'], &:foo?
cannot :manage, ::Spree::Role, ['some SQL'], &:bar?

Current behavior

  1. Users with foo? => true are listed
  2. Roles with bar? => true are included in Search > Roles, New User > Roles etc.
  3. Roles with bar? => true can be assigned to a user

Expected behavior

  1. Users with foo? => true are not listed
  2. Roles with bar? => true are excluded from Search > Roles, New User > Roles etc.
  3. Roles with bar? => true cannot be assigned to a user
@kennyadsl
Copy link
Member

We can close this one. Thanks @igorbp and @halilim for reporting!

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

2 participants