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

Running dangerous :show permission when the resource name is passed and mismatch the controller name #791

Open
enmanuelm19 opened this issue Jun 21, 2022 · 3 comments

Comments

@enmanuelm19
Copy link

Steps to reproduce

When creating a custom controller and delegating permissions on another resource, the resource is treated as a parent and defaulting the action to :show instead of the controller's action.

# ability.rb
def initialize(user)
  can :read, Book
  can :update, Book do
    user.is_admin
  end
end
class BookServices < ApplicationController
  load_and_authorize_resource :book 

  def update
    head :no_content
  end
end

A non admin user can perform the :update action when it shouldn't.

Expected behavior

It should delegate the permissions on the resource set in the helper.

  • A permission set for the :book should be checked against when setting the name of the resource in the helper

Actual behavior

It applies the :show action permission because is hard coded unless you set the parent_action: option

System configuration

Rails version: 5.2.2

Ruby version: 2.7.0

CanCanCan version: 3.3.0

@enmanuelm19
Copy link
Author

I have a PR up to fix this.

If run authorization against the :show action is actually intended, at least a section in the documentation should be present to warn people that setting the resource on the load_and_authorize_resource helper without setting the parent_action: option and in controller named differently that the resource will cause authorization always check to the :show action which is pretty permissive in general.

@coorasse
Copy link
Member

coorasse commented Jul 7, 2022

I agree completely with the point, but I disagree with the fix. I believe the load_and_authorize_resource :book should recognise that :book is actually not a parent.

@enmanuelm19
Copy link
Author

I agree completely with the point, but I disagree with the fix. I believe the load_and_authorize_resource :book should recognise that :book is actually not a parent.

Ok, let me close the PR with this comment, I'll come back later to have a better fix for this.

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