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

Scoping ActionController::Parameters doesn't work when passing a custom policy with with: #70

Closed
louim opened this issue May 30, 2019 · 2 comments

Comments

@louim
Copy link
Contributor

louim commented May 30, 2019

Hey!

I came across a potential problem when scoping params with authorized_scope, when trying to use a specific policy, for example:

class CourseContentsController
  def create
    ...
    authorized_scope(params.require(:course_content), with: CoursePolicy)
    ...
  end
end

Will raise a ActionController::UnfilteredParameters: unable to convert unpermitted parameters to hash error.

Also related, but maybe not a bug, but the default implicit_authorization_target injected here:

def implicit_authorization_target
controller_name.classify.safe_constantize
end

will output Couldn't find policy class for nil when unable to safe_constantize the controller. Maybe that is not possible, but I think it would be helpful for the message to read Couldn't find policy class for CourseContent or maybe a different message since it is not able to find a matching class?

I created a PR with failing tests to better demonstrate the issue.

palkan added a commit that referenced this issue May 30, 2019
When a Hash-like object passed as a record, it is recognized as the optional kwarg.

It could lead to unexpected behaviour (#70).

Temporary solution is to remove it (we don't use it); we should migrate to only-kwargs in the future (i.e. record = nil -> record: nil)
palkan added a commit that referenced this issue May 30, 2019
When a Hash-like object passed as a record, it is recognized as the optional kwarg.

It could lead to unexpected behaviour (#70).

Temporary solution is to replace it with optional arg; we should migrate to only-kwargs in the future (i.e. record = nil -> record: nil)

Thanks to @louim for providing a reproduction test!

Co-authored-by: Louis-Michel Couture <[email protected]>
@palkan
Copy link
Owner

palkan commented May 30, 2019

Fixed and released in 0.3.1.

@palkan palkan closed this as completed May 30, 2019
@louim
Copy link
Contributor Author

louim commented May 30, 2019

Thank you! 🎉

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