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

Bulk export doesn't work with Pundit #3404

Open
goldaanton opened this issue Oct 15, 2021 · 4 comments
Open

Bulk export doesn't work with Pundit #3404

goldaanton opened this issue Oct 15, 2021 · 4 comments

Comments

@goldaanton
Copy link

goldaanton commented Oct 15, 2021

Hey, I'm using Pundit to authorize actions in Rails Admin. And recently I discovered, that I can't use bulk export for users, as Pundit tries to check unexpected policies.

Here are my steps to understand what happens:

  1. Choose users and press Export selected Users.

image

  1. And I get this error for reset_user_password action, which is really strange as I didn't use that action.

image

Thanks for any help!

Here is Full Trace:
app/policies/admins/user_policy.rb:46:in 'reset_user_password?' rails_admin (2.2.1) lib/rails_admin/extensions/pundit/authorization_adapter.rb:37:in 'authorized?' rails_admin (2.2.1) lib/rails_admin/config/actions/base.rb:57:in 'block in <class:Base>' rails_admin (2.2.1) lib/rails_admin/config/configurable.rb:75:in 'instance_eval' rails_admin (2.2.1) lib/rails_admin/config/configurable.rb:75:in 'block in register_instance_option' rails_admin (2.2.1) lib/rails_admin/config/configurable.rb:59:in 'block in register_instance_option' rails_admin (2.2.1) lib/rails_admin/config/actions/base.rb:44:in 'block in <class:Base>' rails_admin (2.2.1) lib/rails_admin/config/configurable.rb:75:in 'instance_eval' rails_admin (2.2.1) lib/rails_admin/config/configurable.rb:75:in 'block in register_instance_option' rails_admin (2.2.1) lib/rails_admin/config/configurable.rb:59:in 'block in register_instance_option' rails_admin (2.2.1) lib/rails_admin/config/proxyable/proxy.rb:25:in 'method_missing' rails_admin (2.2.1) lib/rails_admin/config/actions.rb:27:in 'select' rails_admin (2.2.1) lib/rails_admin/config/actions.rb:27:in 'all' rails_admin (2.2.1) app/controllers/rails_admin/main_controller.rb:28:in 'bulk_action' actionpack (5.2.2.1) lib/action_controller/metal/basic_implicit_render.rb:6:in 'send_action' actionpack (5.2.2.1) lib/abstract_controller/base.rb:194:in 'process_action' actionpack (5.2.2.1) lib/action_controller/metal/rendering.rb:30:in 'process_action' actionpack (5.2.2.1) lib/abstract_controller/callbacks.rb:42:in 'block in process_action' activesupport (5.2.2.1) lib/active_support/callbacks.rb:132:in 'run_callbacks' actionpack (5.2.2.1) lib/abstract_controller/callbacks.rb:41:in 'process_action' actionpack (5.2.2.1) lib/action_controller/metal/rescue.rb:22:in 'process_action' actionpack (5.2.2.1) lib/action_controller/metal/instrumentation.rb:34:in 'block in process_action' activesupport (5.2.2.1) lib/active_support/notifications.rb:168:in 'block in instrument' activesupport (5.2.2.1) lib/active_support/notifications/instrumenter.rb:23:in 'instrument' activesupport (5.2.2.1) lib/active_support/notifications.rb:168:in 'instrument' actionpack (5.2.2.1) lib/action_controller/metal/instrumentation.rb:32:in 'process_action' actionpack (5.2.2.1) lib/action_controller/metal/params_wrapper.rb:256:in 'process_action' mongoid (7.2.1) lib/mongoid/railties/controller_runtime.rb:22:in 'process_action' actionpack (5.2.2.1) lib/abstract_controller/base.rb:134:in 'process' actionview (5.2.2.1) lib/action_view/rendering.rb:32:in 'process' actionpack (5.2.2.1) lib/action_controller/metal.rb:191:in 'dispatch' actionpack (5.2.2.1) lib/action_controller/metal.rb:252:in 'dispatch' actionpack (5.2.2.1) lib/action_dispatch/routing/route_set.rb:52:in 'dispatch' actionpack (5.2.2.1) lib/action_dispatch/routing/route_set.rb:34:in 'serve' actionpack (5.2.2.1) lib/action_dispatch/journey/router.rb:52:in 'block in serve' actionpack (5.2.2.1) lib/action_dispatch/journey/router.rb:35:in 'each' actionpack (5.2.2.1) lib/action_dispatch/journey/router.rb:35:in 'serve' actionpack (5.2.2.1) lib/action_dispatch/routing/route_set.rb:840:in 'call' railties (5.2.2.1) lib/rails/engine.rb:524:in 'call' railties (5.2.2.1) lib/rails/railtie.rb:190:in 'public_send' railties (5.2.2.1) lib/rails/railtie.rb:190:in 'method_missing' actionpack (5.2.2.1) lib/action_dispatch/routing/mapper.rb:19:in 'block in <class:Constraints>' actionpack (5.2.2.1) lib/action_dispatch/routing/mapper.rb:48:in 'serve' actionpack (5.2.2.1) lib/action_dispatch/journey/router.rb:52:in 'block in serve' actionpack (5.2.2.1) lib/action_dispatch/journey/router.rb:35:in 'each' actionpack (5.2.2.1) lib/action_dispatch/journey/router.rb:35:in 'serve' actionpack (5.2.2.1) lib/action_dispatch/routing/route_set.rb:840:in 'call' rack-pjax (1.1.0) lib/rack/pjax.rb:12:in 'call' rack-attack (6.5.0) lib/rack/attack.rb:113:in 'call' rack (2.2.3) lib/rack/method_override.rb:24:in 'call' rack (2.2.3) lib/rack/session/abstract/id.rb:266:in 'context' rack (2.2.3) lib/rack/session/abstract/id.rb:260:in 'call' actionpack (5.2.2.1) lib/action_dispatch/middleware/cookies.rb:670:in 'call' rack (2.2.3) lib/rack/deflater.rb:44:in 'call' remotipart (1.4.4) lib/remotipart/middleware.rb:32:in 'call' rack (2.2.3) lib/rack/etag.rb:27:in 'call' rack (2.2.3) lib/rack/conditional_get.rb:40:in 'call' rack (2.2.3) lib/rack/head.rb:12:in 'call' actionpack (5.2.2.1) lib/action_dispatch/middleware/callbacks.rb:28:in 'block in call' activesupport (5.2.2.1) lib/active_support/callbacks.rb:98:in 'run_callbacks' actionpack (5.2.2.1) lib/action_dispatch/middleware/callbacks.rb:26:in 'call' actionpack (5.2.2.1) lib/action_dispatch/middleware/executor.rb:14:in 'call' rollbar (3.2.0) lib/rollbar/middleware/rails/rollbar.rb:25:in 'block in call' rollbar (3.2.0) lib/rollbar.rb:145:in 'scoped' rollbar (3.2.0) lib/rollbar/middleware/rails/rollbar.rb:22:in 'call' actionpack (5.2.2.1) lib/action_dispatch/middleware/debug_exceptions.rb:61:in 'call' rollbar (3.2.0) lib/rollbar/middleware/rails/show_exceptions.rb:22:in 'call_with_rollbar' actionpack (5.2.2.1) lib/action_dispatch/middleware/show_exceptions.rb:33:in 'call' railties (5.2.2.1) lib/rails/rack/logger.rb:38:in 'call_app' railties (5.2.2.1) lib/rails/rack/logger.rb:28:in 'call' actionpack (5.2.2.1) lib/action_dispatch/middleware/remote_ip.rb:81:in 'call' request_store (1.5.0) lib/request_store/middleware.rb:19:in 'call' actionpack (5.2.2.1) lib/action_dispatch/middleware/request_id.rb:27:in 'call' rack (2.2.3) lib/rack/runtime.rb:22:in 'call' activesupport (5.2.2.1) lib/active_support/cache/strategy/local_cache_middleware.rb:29:in 'call' actionpack (5.2.2.1) lib/action_dispatch/middleware/executor.rb:14:in 'call' actionpack (5.2.2.1) lib/action_dispatch/middleware/static.rb:127:in 'call' rack (2.2.3) lib/rack/sendfile.rb:110:in 'call' rack-cors (1.1.1) lib/rack/cors.rb:100:in 'call' railties (5.2.2.1) lib/rails/engine.rb:524:in 'call' puma (4.3.8) lib/puma/configuration.rb:228:in 'call' puma (4.3.8) lib/puma/server.rb:718:in 'handle_request' puma (4.3.8) lib/puma/server.rb:472:in 'process_client' puma (4.3.8) lib/puma/server.rb:328:in 'block in run' puma (4.3.8) lib/puma/thread_pool.rb:134:in 'block in spawn_thread'

@Art3606
Copy link
Contributor

Art3606 commented Oct 26, 2021

The problem was in MainController#bulk_action. For checking if action exists, was used :all scope instead of :bulkable. Here is the pull request with a fix: #3407

@codealchemy
Copy link
Contributor

This should be resolved by #3407, released as a part of v3.0.0.rc

@mshibuya
Copy link
Member

Sorry I wasn't clear, this is not resolved 🙇‍♂️
There may be a case that #3407 happens to work, but this issue is deeper one. Which will be...

Should Pundit policy classes accept ActiveRecord model classes for the record argument? Currently we pass them, but I have a sense that it may not be the way Pundit was designed.

So this issue will be about making the decision on it and (if we decide to go that way) fixing the implementation.

@mshibuya mshibuya reopened this Feb 19, 2022
@codealchemy
Copy link
Contributor

Should Pundit policy classes accept ActiveRecord model classes for the record argument? Currently we pass them, but I have a sense that it may not be the way Pundit was designed.

I might be missing something - but from what I can tell, there doesn't seem to be a strong opinion against it. The README uses an ActiveRecord object, but is clear that the implementation is fairly flexible (though the second argument is required, some cases - like headless - don't necessarily use it).

The second argument is some kind of model object, whose authorization you want to check. This does not need to be an ActiveRecord or even an ActiveModel object, it can be anything really.

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

4 participants