Ash.Policy.FilterCheck

Are we using this wrong? We have the following policy module:

def describe(_opts), do: "Organization is one of the users orgs that they are an admin of"

def filter(actor, authorizer, _opts) do
  organization_id = authorizer.subject.arguments.organization_id
  hierarchy = Ash.calculate!(actor, :organization_hierarchy, args: %{id: actor.id, format: :tree})
  organization_ids = get_ancestry_path(hierarchy, organization_id)

  case filter_where_admin_of(organization_ids, actor.id) do
    [] ->
      false

    _ ->
      true
  end
end

def filter_where_admin_of(organization_ids, user_id) do
  organization_members = Ash.Query.filter(OrganizationMember, user_id == ^user_id and role == :admin)

  organization_ids
  |> Organization.get_by_ids!(load: [organization_members: organization_members])
  |> Enum.flat_map(& &1.organization_members)
  |> Enum.map(& &1.organization_id)
end

def describe(_opts), do: "Organization is one of the users orgs that they are an admin of"

def filter(actor, authorizer, _opts) do
  organization_id = authorizer.subject.arguments.organization_id
  hierarchy = Ash.calculate!(actor, :organization_hierarchy, args: %{id: actor.id, format: :tree})
  organization_ids = get_ancestry_path(hierarchy, organization_id)

  case filter_where_admin_of(organization_ids, actor.id) do
    [] ->
      false

    _ ->
      true
  end
end

def filter_where_admin_of(organization_ids, user_id) do
  organization_members = Ash.Query.filter(OrganizationMember, user_id == ^user_id and role == :admin)

  organization_ids
  |> Organization.get_by_ids!(load: [organization_members: organization_members])
  |> Enum.flat_map(& &1.organization_members)
  |> Enum.map(& &1.organization_id)
end

This is tied to an action:

policy action(:get_groups_for_organization) do
  description "Can only see dashboard groups where the actor is the owner of the group, or is an admin of that group's organization or of any of its parents"
  authorize_if AdminOfOrganizationHigherInHierarchy
end

policy action(:get_groups_for_organization) do
  description "Can only see dashboard groups where the actor is the owner of the group, or is an admin of that group's organization or of any of its parents"
  authorize_if AdminOfOrganizationHigherInHierarchy
end

However, even when the filter function inside policy module returns false (from the [] clause), the policy still authorizes the action. Running DashboardGroup.can_get_groups_for_organization?(current_user, current_organization.id, %{}) returns true. Should we be using SimpleCheck instead?

11 Replies

ZachDaniel•5mo ago

If you're returning statically a value like that then you're essentially writing a simple check anyway. The idea with filter checks is that you can return an expression that will apply some logic to the filter. like expr(exists(...)), this helps you write optimal policies With that said, the reason its returning true is because thats how policies work for read actions they filter the query, as opposed to rejecting the request, for security If the question is "can they read this specific thing", for example, you can supply that thing as the data option

DashboardGroup.can_get_groups_for_organization?(current_user, current_organization.id, %{}, [data: the_thing_you_want_to_know_if_they_can_see])

DashboardGroup.can_get_groups_for_organization?(current_user, current_organization.id, %{}, [data: the_thing_you_want_to_know_if_they_can_see])

If you write it as a simple check instead of a filter check, then you can do this:

policy ... do
  access_type :strict

  authorize_if YourCheckModule
end

policy ... do
  access_type :strict

  authorize_if YourCheckModule
end

strict policies on read actions will produce a forbidden result, as opposed to filtering I was trying to explain this before but I don't think I succeeded, but typically a filter check would do something like this:

def describe(_opts), do: "Organization is one of the users orgs that they are an admin of"

def filter(actor, authorizer, _opts) do
  hierarchy = Ash.calculate!(actor, :organization_hierarchy, args: %{id: actor.id, format: :tree})
  organization_ids = get_ancestry_path(hierarchy, organization_id)

  case filter_where_admin_of(organization_ids, actor.id) do
    [] ->
      false

    admin_of_orgs ->
      expr(organization_id in ^admin_of_orgs)
  end
end

def filter_where_admin_of(organization_ids, user_id) do
  organization_members = Ash.Query.filter(OrganizationMember, user_id == ^user_id and role == :admin)

  organization_ids
  |> Organization.get_by_ids!(load: [organization_members: organization_members])
  |> Enum.flat_map(& &1.organization_members)
  |> Enum.map(& &1.organization_id)
end

def describe(_opts), do: "Organization is one of the users orgs that they are an admin of"

def filter(actor, authorizer, _opts) do
  hierarchy = Ash.calculate!(actor, :organization_hierarchy, args: %{id: actor.id, format: :tree})
  organization_ids = get_ancestry_path(hierarchy, organization_id)

  case filter_where_admin_of(organization_ids, actor.id) do
    [] ->
      false

    admin_of_orgs ->
      expr(organization_id in ^admin_of_orgs)
  end
end

def filter_where_admin_of(organization_ids, user_id) do
  organization_members = Ash.Query.filter(OrganizationMember, user_id == ^user_id and role == :admin)

  organization_ids
  |> Organization.get_by_ids!(load: [organization_members: organization_members])
  |> Enum.flat_map(& &1.organization_members)
  |> Enum.map(& &1.organization_id)
end

If you have everything you need to know without filtering, then you'd do a simple check like this:

def match?(...) do
  {:ok, true/false}
end

def match?(...) do
  {:ok, true/false}
end

Then you can do access_type :strict Access type is explained here in the docs: https://hexdocs.pm/ash/policies.html#access-type

EgeOP•5mo ago

Lot to process. Starting from the top: the question in this case is "should the user be able to fetch groups for the given organization" which is predicated on "are they an admin of that org or any of the parent orgs" When I changed use Ash.Policy.FilterCheck to use Ash.Policy.SimpleCheck, the organization_members = Ash.Query.filter(OrganizationMember, user_id == ^user_id and role == :admin) line started giving compilation errors

ZachDaniel•5mo ago

Thats just a quirk I didn't consider. FilterCheck has require Ash.Query implicitly

EgeOP•5mo ago

Ah, I see And I guess I would change filter to match?

ZachDaniel•5mo ago

Yes SimpleCheck -> "true or false given query/actor" FilterCheck -> "filter expression given query/actor" They have the same end result, with one main exception, which is that SimpleCheck can be used with access_type :strict on the policy.

EgeOP•5mo ago

The can? function is still returning true Ah strict let me add that

ZachDaniel•5mo ago

With all that said, where you are asking can_... you can also do something like data: %Organization{id: id} to say "do these policy filters return this record" it owuld be instances of whatever the resource in question is

EgeOP•5mo ago

It worked when I added strict

ZachDaniel•5mo ago

Right, so now if that action is called w/ an incorrect organization id, it will be forbidden instead of filtered, just keep that in mind

EgeOP•5mo ago

OK. We have two tabs on the page: "My Groups" tab, which runs the groups_for_educator action, which is tied to a policy that simply checks authorize_if relates_to_actor_via(:educator). We also have an "All Groups" tab, which runs the groups_for_organization action, which is tied to this policy above, i.e. they need to be an admin of the org to be able to fetch all groups in that org I think the way I've set it up is correct now - I don't want a filter, because that would filter the results. So simple/strict make sense

ZachDaniel•5mo ago

Makes sense 👍 We default to filtering as it has the least potential to expose internal information, but there are plenty of cases where you'd want :strict

Gaming

Programming

Ash.Policy.FilterCheck

Did you find this page helpful?