chaps-io / access-granted

Multi-role and whitelist based authorization gem for Rails (and not only Rails!)
MIT License
774 stars 41 forks source link

permission always uses conditions hash when passed a Class as subject #38

Closed agferrier closed 7 years ago

agferrier commented 7 years ago

What is the thinking behind ignoring a block and always attempting to evaluate the conditions hash when passed a Class as a subject.

    def matches_conditions?(subject)
      if @block && !subject.is_a?(Class)
        @block.call(subject, @user)
      else
        matches_hash_conditions?(subject)
      end
    end

I would always like to evaluate a block if it is present (I've got some user related logic I need to execute to determine whether to allow access) Would it be acceptable to change the logic so that blocks are always evaluated if present?

pokonski commented 7 years ago

Can you show your example?

agferrier commented 7 years ago

Below is an extract from my access_policy.rb file

      can :index, Receipt do |receipt, current_user|
        current_user.org.receipt_service_allowed
      end

This rule doesn't work because the block is ignored since the index action passes the Receipt class and not an instance.

The permission should granted based on whether the current user belongs to an org that has been enabled for the receipts service.

Other actions for Receipt are dependent on both the user and the receipt for which the action is requested and these work as requested. e.g.

      can :show, Receipt do |receipt, current_user|
        current_user.org.receipt_service_allowed &&
        receipt.user_id == current_user.id
      end

Rules such as this work really well.

pokonski commented 7 years ago

That does sound like a valid use case, thanks for sharing that 👍 Do you mind preparing a PR with a change for this and a spec to test the new behaviour?

agferrier commented 7 years ago

Happy to do that - I've got a branch ready to go. Can you add me as a collaborator to the repo - I can't push my branch and create the PR

pokonski commented 7 years ago

Sorry, we don't add anyone outside Chaps as collaborators. You can submit a PR from your repository after you fork it here on Github.

agferrier commented 7 years ago

PR now raised https://github.com/chaps-io/access-granted/pull/39

pokonski commented 7 years ago

Merged :+1: