action_policy icon indicating copy to clipboard operation
action_policy copied to clipboard

Policy lookup for authorized_scope returns default policy instead of using implicit authorization target

Open rosalynnas opened this issue 1 year ago • 3 comments

Tell us about your environment

Ruby Version: ruby 3.1.4p223 (2023-03-30 revision 957bb7cb81) [arm64-darwin23]

Framework Version (Rails, whatever): rails 6.1.7.6

Action Policy Version: action_policy 0.6.7

What did you do?

We setup two different authorization namespaces, which means we rely on the implicit_authorization_target to find the correct policy when the target is not inferrable. We also created a default policy for when the lookup finds nothing.

We encountered an issue when calling authorized_scope(params.require(:record)) in a controller that relies on an implicit authorization target.

What did you expect to happen?

Since I'm passing a params object which can't be used to infer the correct policy, I would expect authorized_scope to fall back to the Implicit authorization target and return the associated policy.

What actually happened?

The method fails to find the correct policy and returns the default policy instead.

The problem seems to relate to the following lines in the ActionPolicy::Behaviours::Scoping module

def authorized_scope(target, type: nil, as: :default, scope_options: nil, **options)
  [...]

  policy = policy_for(record: target, allow_nil: true, **options) # This line returns the default policy
  policy ||= policy_for(record: implicit_authorization_target!, **options) # This line is not used

  [...]
end

Because there is a default policy defined, the first policy_for returns that default policy class and the second call with implicit_authorization_target! never gets executed (which would return the expected policy for a parameter scope).

I think the first policy lookup could be done with default: nil to ensure it doesn't fallback on the default policy in the first lookup attempt, but only in the second call. But I am not sure this would not break other expected behaviour in other use cases.

rosalynnas avatar Mar 15 '24 20:03 rosalynnas

Thanks for the detailed report!

I think the first policy lookup could be done with default: nil to ensure it doesn't fallback on the default policy in the first lookup attempt, but only in the second call. But I am not sure this would not break other expected behaviour in other use cases.

Yeah, that would change the order of lookup 🤔 But only for scopes (in #authorize! and #allowed_to? we already fallback to the implicit target if no record is specified).

I think, we can safely update the current behaviour and add a configuration option to opt-in to it (and make it the default in v1.0).

palkan avatar Mar 26 '24 00:03 palkan

Hi @palkan. I'm trying to understand the situation and helping. Correct me if I'm thinking wrong. I have 3 tables in DB: users, posts, drafts.

class ApplicationPolicy < ActionPolicy::Base
  scope_for :data do |data|
    return data
  end

  def default_authorization_policy_class
    DefaultPolicy
  end
end
class DefaultPolicy < ApplicationPolicy
  relation_scope do |relation|
    return relation if user.admin?
    relation.where(user: user)
  end
end
class PostPolicy < DefaultPolicy
end
class UserPolicy < ApplicationPolicy
  relation_scope do |relation|
    return relation if user.admin?
    relation.where(id: user.id)
  end
end
policy = PostPolicy.new(user: admin)
policy.authorized_scope(Post.all) # => Post.all
policy.authorized_scope(User.all) # => User.all
policy.authorized_scope(Draft.all) # => Draft.all (working via #default_authorization_policy_class)
policy.authorized_scope("Test", type: :data) # => "Test"

This is expected behavior, right?

killondark avatar Apr 03 '24 14:04 killondark

Yes, in this case, the policy class for Draft.all is coming from the #default_authorization_policy_class and that's an expected behaviour.

P.S. I've set up an online reproduction playground: https://runruby.dev/?gist=2389f6d70c53c308012adec8a5f32053

palkan avatar Apr 19 '24 17:04 palkan