palkan / action_policy

Authorization framework for Ruby/Rails applications
https://actionpolicy.evilmartians.io
MIT License
1.4k stars 90 forks source link

authorize! suddenly failing for 'required keyword' on Ruby 3.0.1 #158

Closed mepatterson closed 3 years ago

mepatterson commented 3 years ago

Tell us about your environment

Ruby Version: 3.0.1

Framework Version (Rails, whatever): Rails 6.1

Action Policy Version: 0.5.7

What did you do?

# inside a class that is NOT a controller, but includes ActionPolicy::Behaviour
authorize! MyKlass, to: :index?

What did you expect to happen?

It checks the authorization policy and continues along.

What actually happened?

ArgumentError: wrong number of arguments (given 2, expected 0..1; required keyword: to)
from .../ruby/3.0.1/lib/ruby/gems/3.0.0/gems/action_policy-0.5.7/lib/action_policy/behaviour.rb:37:in `authorize!'
mepatterson commented 3 years ago

NOTE: this was working just fine on Ruby 2.7 and I upgraded my app to 3.0.1 and this blew up

palkan commented 3 years ago

Interesting. Our tests pass, and we have a similar example: https://github.com/palkan/action_policy/blob/73daca0232b2eb7b632b811f83bcdfd307927b79/test/action_policy/behaviour_test.rb#L21

Does your class include only ActionPolicy::Behaviour or some other behaviors as well?

mepatterson commented 3 years ago

It's a StimulusReflex class, so it does other fancy things for sure, but I couldn't find any reason why it would be throwing an arguments error on the "authorize!" call only in Ruby 3

palkan commented 3 years ago

Could you please try to provide a reproduction script? (We have a template)

mepatterson commented 3 years ago

Ok, so I solved this. And, of course, it was my fault.

But... maybe worth noting here in case someone else does the same thing I did.

For StimulusReflex, I was integrating ActionPolicy into Reflex classes. I did this:

    def authorize!(*)
      @authorization_performed = true
      super
    end

    # Call this method if you don't need to authorize the reflex
    def skip_authorization!
      @authorization_performed = true
    end

    after_reflex do
      raise "Unauthorized Reflex! Make sure you're calling `authorize!` in the reflex method" unless @authorization_performed
    end

Of course, def authorize!(*) works in Ruby 2.7 but DOES NOT work in Ruby 3.

So I switched it to def authorize!(record = :__undef__, to:, **options) matching the ActionPolicy code and it works great.

My question: Is there a better way to do this that I'm not thinking about? Some kind of before_authorize?

palkan commented 3 years ago

So I switched it to def authorize!(record = :undef, to:, **options) matching the ActionPolicy code and it works great.

That's a good case for def authorize!(...) (I should probably switch to it in the library).

My question: Is there a better way to do this that I'm not thinking about? Some kind of before_authorize?

authorize! could be called multiple times in theory, so, we can use class-level callbacks here. Maybe, we can add some alias to distinguish main authorization from others.

mepatterson commented 3 years ago

Getting ActionPolicy to work with StimulusReflex classes is a huge win, imo. So much of my code now involves Reflexes morphing some piece of the dom surgically, so being able to authorize that reflex method to say "nope, I don't care how you initiated this reflex, you don't have authority to do that" is really cool.

palkan commented 3 years ago

Getting ActionPolicy to work with StimulusReflex classes is a huge win, imo

Glad to hear that! Would you like to propose a PR to SR docs to describe the usage of Action Policy for authorization (here https://docs.stimulusreflex.com/rtfm/authentication#authorization)?