Error in action when policy is added

I have this action on the DashboardGroup resource:

read :get_by_id do
  get? true
  argument :id, :uuid, allow_nil?: false
  argument :organization_id, :uuid, allow_nil?: false
  filter expr(id == ^arg(:id))
end

read :get_by_id do
  get? true
  argument :id, :uuid, allow_nil?: false
  argument :organization_id, :uuid, allow_nil?: false
  filter expr(id == ^arg(:id))
end

Attributes:

attributes do
  uuid_primary_key :id

  attribute :name, :string
  attribute :student_filter, :map
  attribute :educator_id, :uuid
  attribute :organization_id, :uuid

  create_timestamp :inserted_at
  update_timestamp :updated_at
end

attributes do
  uuid_primary_key :id

  attribute :name, :string
  attribute :student_filter, :map
  attribute :educator_id, :uuid
  attribute :organization_id, :uuid

  create_timestamp :inserted_at
  update_timestamp :updated_at
end

Relationships:

belongs_to :educator, User do
  source_attribute :educator_id
  destination_attribute :id
end

belongs_to :educator, User do
  source_attribute :educator_id
  destination_attribute :id
end

Policy:

policy action(:get_by_id) do
  authorize_if relates_to_actor_via(:educator)
end

policy action(:get_by_id) do
  authorize_if relates_to_actor_via(:educator)
end

Struggling with tests. I have this:

test "get_by_id/3", ctx do
  group = generate(dashboard_group(organization_id: ctx.parent_org.id, educator_id: ctx.user.id))

  {:ok, group} = DashboardGroup.get_by_id(group.id, ctx.parent_org.id, actor: ctx.user)
  assert is_nil(group) == false

  user2 = generate(user(type: :educator, first_name: "Bob", last_name: "Marley"))
  generate(organization_member(user_id: user2.id, organization_id: ctx.parent_org.id, role: :base))

  group2 = generate(dashboard_group(organization_id: ctx.parent_org.id, educator_id: user2.id))

  {:ok, group2} = DashboardGroup.get_by_id(group2.id, ctx.parent_org.id, actor: ctx.user)
  assert is_nil(group2) == true
end

test "get_by_id/3", ctx do
  group = generate(dashboard_group(organization_id: ctx.parent_org.id, educator_id: ctx.user.id))

  {:ok, group} = DashboardGroup.get_by_id(group.id, ctx.parent_org.id, actor: ctx.user)
  assert is_nil(group) == false

  user2 = generate(user(type: :educator, first_name: "Bob", last_name: "Marley"))
  generate(organization_member(user_id: user2.id, organization_id: ctx.parent_org.id, role: :base))

  group2 = generate(dashboard_group(organization_id: ctx.parent_org.id, educator_id: user2.id))

  {:ok, group2} = DashboardGroup.get_by_id(group2.id, ctx.parent_org.id, actor: ctx.user)
  assert is_nil(group2) == true
end

The second to last line is giving an error:

** (MatchError) no match of right hand side value: {:error, %Ash.Error.Invalid{errors: [%Ash.Error.Query.NotFound{primary_key: nil, resource: MyApp.Ash.Dashboards.DashboardGroup, splode: Ash.Error, bread_crumbs: [], vars: [], path: [], stacktrace: #Splode.Stacktrace<>, class: :invalid}]}}

** (MatchError) no match of right hand side value: {:error, %Ash.Error.Invalid{errors: [%Ash.Error.Query.NotFound{primary_key: nil, resource: MyApp.Ash.Dashboards.DashboardGroup, splode: Ash.Error, bread_crumbs: [], vars: [], path: [], stacktrace: #Splode.Stacktrace<>, class: :invalid}]}}

But inside LV modules, trying to get a dashboard group by id when the actor does not have permission results in {:ok, nil}

14 Replies

ZachDaniel•7d ago

That is working as intended. Authorization policies for read actions are applied as filters. This is designed to prevent a whole class of security related bugs. If you look at the SQL being run, you'll see a WHERE educator_id = <user.id> in the query Its not clear enough in the policies guide, I'm adding something now. Adding the following to the policy docs now Read Actions and Filtering Behavior An important characteristic of read actions is that, by default, they are filtered by policies rather than returning authorization errors. This means: - When a user is not allowed to see certain records, those records are simply filtered out of the results - Instead of receiving a Forbidden error, users typically get a NotFound error (for single record queries) or an empty/reduced result set (for multi-record queries) - This filtering behavior applies to all read actions, including get, read, and any custom read actions you define For example, if a policy restricts users to only see their own posts, a query for all posts will automatically filter to only return the current user's posts, rather than raising an authorization error. Similarly, attempting to fetch a specific post that belongs to another user will result in a NotFound error rather than Forbidden. This design is a security feature that prevents enumeration attacks and information disclosure. By not distinguishing between "record doesn't exist" and "record exists but you can't access it", the system prevents attackers from probing to discover the existence of protected data, mapping out the system's data structure, or conducting reconnaissance attacks through systematic querying. #### Bypassing this behavior You can bypass this behavior on a case-by-case basis with the authorize_with option, for data layers that support error expressions (all of the core ones except AshSqlite). For example, given a post that the user cannot see:

Ash.get!(Post, 123)
# * Invalid:
# not found

Ash.get!(Post, 123, authorize_with: :error)
# * Forbidden

Ash.get!(Post, 123)
# * Invalid:
# not found

Ash.get!(Post, 123, authorize_with: :error)
# * Forbidden

As for {:ok, nil} vs a not-found error, it could be the difference between read_one and get? get is "get the thing or its an error"

EgeOP•7d ago

The LV code is this:

case DashboardGroup.get_by_id(id, socket.assigns.current_organization.id, actor: socket.assigns.current_user) do
  {:ok, nil} ->
    # Failing permission policy will result in `{:ok, nil}`
    socket
    |> redirect(to: ~p"/dashboard2/groups")
    |> LiveToast.put_toast(:error, "We couldn't find that group.")

  {:ok, group} ->
    assign(socket, :group, group)

  {:error, error} ->
    Logger.error("Error getting dashboard group: #{inspect(error)}")

    socket
    |> redirect(to: ~p"/dashboard2/groups")
    |> LiveToast.put_toast(:error, "We couldn't find that group.")
end

case DashboardGroup.get_by_id(id, socket.assigns.current_organization.id, actor: socket.assigns.current_user) do
  {:ok, nil} ->
    # Failing permission policy will result in `{:ok, nil}`
    socket
    |> redirect(to: ~p"/dashboard2/groups")
    |> LiveToast.put_toast(:error, "We couldn't find that group.")

  {:ok, group} ->
    assign(socket, :group, group)

  {:error, error} ->
    Logger.error("Error getting dashboard group: #{inspect(error)}")

    socket
    |> redirect(to: ~p"/dashboard2/groups")
    |> LiveToast.put_toast(:error, "We couldn't find that group.")
end

When the actor doesn't have permission, the code goes into the first case block. But in tests, when the actor doesn't have permission, I get that error. I'm asking about this discrepancy. It's the same code so shouldn't the behavior be the same?

ZachDaniel•7d ago

Sorry, misunderstood the original question. That is quite strange 🤔 I can't think of a reason that behavior would somehow change in a LV vs in a test...

EgeOP•7d ago

If I comment out the policy, the error goes away

ZachDaniel•7d ago

And your LV user and test user have the same properties? So the only discernible difference is that one is in a LV and one is in a test?

If I comment out the policy, the error goes away

When you say the error goes away, what exactly do you mean? So if you put a made-up id in there, do you get {:ok, nil}?

EgeOP•7d ago

The full test is like this:

setup do
  user = generate(user(type: :educator, first_name: "Ed", last_name: "Teacher"))

  grandparent_org = generate(organization())
  parent_org = generate(organization(parent_id: grandparent_org.id, ancestor_ids: [grandparent_org.id]))
  child_org = generate(organization(parent_id: parent_org.id, ancestor_ids: [parent_org.id, grandparent_org.id]))

  %{user: user, parent_org: parent_org, child_org: child_org, grandparent_org: grandparent_org}
end

test "get_by_id/3", ctx do
  group = generate(dashboard_group(organization_id: ctx.parent_org.id, educator_id: ctx.user.id))

  {:ok, group} = DashboardGroup.get_by_id(group.id, ctx.parent_org.id, actor: ctx.user)
  assert is_nil(group) == false

  user2 = generate(user(type: :educator, first_name: "Bob", last_name: "Marley"))
  generate(organization_member(user_id: user2.id, organization_id: ctx.parent_org.id, role: :base))

  group2 = generate(dashboard_group(organization_id: ctx.parent_org.id, educator_id: user2.id))

  {:ok, group2} = DashboardGroup.get_by_id(group2.id, ctx.parent_org.id, actor: ctx.user)
  assert is_nil(group2) == true
end

setup do
  user = generate(user(type: :educator, first_name: "Ed", last_name: "Teacher"))

  grandparent_org = generate(organization())
  parent_org = generate(organization(parent_id: grandparent_org.id, ancestor_ids: [grandparent_org.id]))
  child_org = generate(organization(parent_id: parent_org.id, ancestor_ids: [parent_org.id, grandparent_org.id]))

  %{user: user, parent_org: parent_org, child_org: child_org, grandparent_org: grandparent_org}
end

test "get_by_id/3", ctx do
  group = generate(dashboard_group(organization_id: ctx.parent_org.id, educator_id: ctx.user.id))

  {:ok, group} = DashboardGroup.get_by_id(group.id, ctx.parent_org.id, actor: ctx.user)
  assert is_nil(group) == false

  user2 = generate(user(type: :educator, first_name: "Bob", last_name: "Marley"))
  generate(organization_member(user_id: user2.id, organization_id: ctx.parent_org.id, role: :base))

  group2 = generate(dashboard_group(organization_id: ctx.parent_org.id, educator_id: user2.id))

  {:ok, group2} = DashboardGroup.get_by_id(group2.id, ctx.parent_org.id, actor: ctx.user)
  assert is_nil(group2) == true
end

With the policy, the {:ok, group2} = DashboardGroup.get_by_id(group2.id, ctx.parent_org.id, actor: ctx.user) line gives the error. If I comment out the policy, then the groups that actually exist are found and returned as one would expect. Only non-existent groups return a not-found error.

** (MatchError) no match of right hand side value: {:error, %Ash.Error.Invalid{errors: [%Ash.Error.Query.InvalidArgument{field: :id, message: "is invalid", value: "test-one-two-three", splode: Ash.Error, bread_crumbs: [], vars: [], path: [], stacktrace: #Splode.Stacktrace<>, class: :invalid}]}}

** (MatchError) no match of right hand side value: {:error, %Ash.Error.Invalid{errors: [%Ash.Error.Query.InvalidArgument{field: :id, message: "is invalid", value: "test-one-two-three", splode: Ash.Error, bread_crumbs: [], vars: [], path: [], stacktrace: #Splode.Stacktrace<>, class: :invalid}]}}

ZachDaniel•7d ago

Interesting. I should be able to reproduce that pretty easily, one sec.

EgeOP•7d ago

So the question is why this policy:

policy action(:get_by_id) do
  authorize_if relates_to_actor_via(:educator)
end

policy action(:get_by_id) do
  authorize_if relates_to_actor_via(:educator)
end

Would result in this code:

{:ok, group2} = DashboardGroup.get_by_id(group2.id, ctx.parent_org.id, actor: ctx.user)

{:ok, group2} = DashboardGroup.get_by_id(group2.id, ctx.parent_org.id, actor: ctx.user)

To return an :error tuple. When it should be returning {:ok, nil} based on how it behaves in LV.

ZachDaniel•7d ago

The LV one is the one that is wrong. get_by_id should return a not found error if the thing does not exist or if policies filtered it it shouldn't even be able to tell which one of those things is true, which is why this is confusing You're sure your LV case returns {:ok, nil}?

EgeOP•7d ago

Pretty sure. I'll play with it some more. Need to step away now for a meeting Will post here later What happens if the resource is being loaded during a read on an associated resource, and the permission check fails? Does the resource get loaded as nil by default?

ZachDaniel•7d ago

Yes

EgeOP•7d ago

OK, maybe that's why I came to believe that it would also return {:ok, nil} when fetching the resource itself

ZachDaniel•7d ago

Yeah, that makes sense. There are options to change how loading related fields that you can't see should be treated

has_one ... do
  allow_forbidden_field? true
end

has_one ... do
  allow_forbidden_field? true
end

and

authorize_read_with :error

authorize_read_with :error

With that combination, loading a thing you can't see would give you %Resource{..., relationship: %Ash.ForbiddenField{}}

EgeOP•7d ago

Okay

Gaming

Programming

Error in action when policy is added

Did you find this page helpful?