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

feat: allow if and unless options for skipping authorize and load, similar to check_authorization #808

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

ammarghaus
Copy link

This PR adds if and unless options for controller helpers for skipping authorization and loading of resources.
The idea is to allow these helpers to behave the same as check_authorization.

Why: These options allows more control over the behaviour of these helpers with run time conditions.

I'd also like to share the particular use case that prompted this feature.
If a controller action concern skips authorization for one of the actions it defines, it runs the risk of that skip
direction being overwritten by any other skip direction that might be invoked in the controller including the concern.
It can also conflict with any other concern included in the controller.

Take the following concern for example.

module TestConcern
  extend ActiveSupport::Concern
  included do
    skip_authorize_resource only: :concern_action

    def concern_action; end
  end
end

and a controller that includes the concern:

class FooController < ::ApplicationController
  include TestConcern
  skip_authorize_resource only: :controller_action
end

Now authorization will not be skipped for concern_action

Having this feature allows to work around this problem by declaring the skip_authorization in the base_controller
with a controller action to determine if the authorization should be skipped or not.
A list of actions is then maintained by the base controller.
Child controllers and concerns append actions to the shared list for skipping the authorization.
The controller action uses the shared list to determine if authorization should be skipped or not.
This allows multiple controllers and concerns down the tree to skip relevant actions without conflict.

Help needed

In addition to the normal feedback and review, I need a little help with the implementation.
I have violated the rubocop rule Metrics/ClassLength by 7 lines and am not quite sure how to avoid it.

@ammarghaus ammarghaus force-pushed the feature/if-unless-for-skip-helpers branch from ae502b2 to c225c36 Compare December 2, 2022 15:37
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

Successfully merging this pull request may close these issues.

1 participant