Proper way to build "OR" expression in policy involving actor()

I’m getting an unexpected policy access failure.

Here’s the policy:

policies
  policy action :read do
    authorize_if expr(is_public == true or ^actor(:active) == true)
  end
end

When I read the resource like this:

{:ok, _event} = Ash.get(GF.Events.Event2, event.id)

It fails with this policy breakdown:

15:13:00.150 [warning] GF.Events.Event2.read

Policy Breakdown
unknown actor

  Policy | 🔎:

    condition: action == :read

    authorize if: (is_public == true) or (nil == true) | ✘ | 🔎

SAT Solver statement: 

 "action == :read" and
  (("action == :read" and "(is_public == true) or ({:_actor, :active} == true)") or
     not "action == :read")

But when I remove the part of the policy expression after the or, it passes:

authorize_if expr(is_public == true)

I would expect that if expr(is_public == true) passes, then, expr(is_public == true or any_valid_expression) should also pass.

I also tried these variations:

  1. Passing an actor that has the attribute active: true. This passes, as expected.
  2. Passing an actor that has the attribute active: false. This passes, as expected.

It seems that the ^actor(:active) is being evaluated, affecting the boolean expression, even though the first part of the expression should have short-circuited the second.

I saw the “unknown actor” in the policy breakdown, but I’m not sure what to do about it.

How do I solve this?

The way that checks that reference the actor work is that they always are false if there is no actor, regardless of the statement.

This is to prevent you from having a check like is_nil(^actor(:disabled_at)) pass for a non-logged in user and accidentally having a security hole.

The nil == true is, in this case, misleading in the way that it is displayed. When describing filter checks we do it by putting the filter into a string, but what would be more accurate is to put in something like “there is an actor and …”.

If you could open an issue in ash showing the DX issue w/ how we’re displaying this in breakdowns, that would be great. What we can do in reality is replace that whole expression with something like "false (actor required for filter)" or something, because we know already that it won’t pass.

For solving the issue, the following syntax is equivalent to your policy and does not have the same problem:

policy action(:read) do
  authorize_if expr(is_public == true)
  authorize_if expr(^actor(:active) == true)
end

Nice, that works!

I get confused about whether multiple authorize_if statements combine to create a logical OR, or a logical AND expression. So then it’s OR, correct? And if you need an AND, then you need an expr(expr1 and expr2)?

Yep! Policy checks apply top-to-bottom as if it were a script (although it is not executed as such under the hood).

The first check that produces a result of :authorized or :forbidden determines the result of the overall policy.

i.e authorize_if always() produces :authorized and authorize_if never() produces :unknown, meaning we continue to the next check.

To do and you can use an expression with and, and you can also do things like:

policy action(:read) do
  forbid_unless expr(not(something))
  authorize_if expr(something_else)
end

Don’t use the forbid_unless version here, just highlighting tools that may be useful further down the line.

1 Like