Proposal: Allow creation of policy check groups

Ash Forum Ash Chat
1

Consider a resource that has a bunch of actions in it and I’m creating policies for them.

Some actions have some policies checks that are common between them and some are not.

Right now, I would write the policies something like this:

policies do
  policy action([:a, :b, :c, ...]) do
    # all actions common checks
  end

  policy action :a do
    # specific a checks
  end

  policy action :b do
    # specific b checks
  end

  policy action [:a, :b] do
    # common checks between :a and :b
  end

  ...
end

The issue that I see with this is that as I keep adding more and more policies, it gets harder and harder to read them since I need to find all places some action can match a policy to get the full picture of what policies an action has (and from my experience, that incentivizes the developers to ignore the policies or write wrong/incomplete ones).

A way that I though to mitigate this it to have a explicit policy block per action, that way I just need to check that action policy block and I get the full picture of what its policies checks are.

The obvious problem with this approach is that now I have a bunch of code duplication:

policies do
  policy action(:a) do
    # common checks
    # common checks between :a and :b
    # specific :a checks
  end

  policy action(:b) do
    # common checks
    # common checks between :a and :b
    # specific :b checks
  end

  policy action(:c) do
    ...
  end 

  ...
end

So, the proposal is a way to help this scenario by allowing the creation of check groups, basically a check group contains a list of checks the same way a policy code block has and a name, then, that check block can be referenced inside any policy block and their checks would be applied to that policy block.

It would work kinda similar to phoenix route pipelines.

So, the above solution with check groups would be something like this:

policies do
  check_group :common do
    # check 1
    # check 2
    ...
  end

  check_group :common_a_and_b do
    # check 1
    # check 2
    ...
  end

  policy action(:a) do
    check_group :common
    check_group :common_a_and_b
    # specific :a checks
  end

  policy action(:b) do
    check_group :common
    check_group :common_a_and_b
    # specific :a checks
  end

  policy action(:c) do
    ...
  end 

  ...

Of course, if I just have 2~3 actions, then this is more verbose and probably will use more lines, but the idea is that this would help with bigger resources that have a bunch of actions (5~20).

Do you think something like this would be feasible @zachdaniel ?

2

:a: thinking: its an interesting proposal. One thing that can be complex is the way that these compose. For example:

policy action(:a) do
  check_group :common
  authorize_if :a
  authorize_if :b
end

and

policy action(:a) do
  authorize_if :a
  check_group :common
  authorize_if :b
end

compose differently, its not a simple as a && common && b, and modeling the workflow this way could be pretty confusing.

With that said, you can actually experiment with this pretty easily using macros.

defmacrop common_a_and_b() do
  quote do
     ...your checks
  end
end

...


policy action(:a) do
  common_a_and_b()
end
3

The way I was thinking about this is that it just “injects” the group in there, so, for example, if the group is define as follows:

check_policy :common  do
  authorize_if :check_1
  forbid_unless :check_2
end

Then, I would read the first example as:

policy action(:a) do
  authorize_if :check_1
  forbid_unless :check_2
  authorize_if :a
  authorize_if :b
end

And the second one as:

policy action(:a) do
  authorize_if :a
  authorize_if :check_1
  forbid_unless :check_2
  authorize_if :b
end

And not like if the group is a check by itself.

4

yeah i get what you’re saying but i think it may ultimately add more complexity than it removes.

5

Technically, what you want does not belong to Ash by any means. You might simply define your own macro to embed the code and call it from anywhere. Somewhat alongside the following lines would do.

defmodule MyMacros.Code do
  @moduledoc """
  The module exporting some goodnesses working with code.
  """

  defmacro define_code_group(name, args \\ [], do: block) do
    module = __CALLER__.module

    if not Module.has_attribute?(module, name) do
      Module.register_attribute(module, :code_groups, persist: true, accumulate: true)
    end

    Module.put_attribute(
      module,
      :code_groups,
      {name, [args: args, block: block]}
    )
  end

  defmacro embed_code_groups(names, args \\ []) do
    blocks =
      __CALLER__.module
      |> Module.get_attribute(:code_groups)
      |> Kernel.||([])
      |> Keyword.take(List.wrap(names))
      |> Enum.reverse()
      |> Enum.map(&elem(&1, 1))
      |> Enum.flat_map(&expand_args(&1, args, __CALLER__.context))

    {:__block__, [file: __CALLER__.file, line: __CALLER__.line], blocks}
  end

  defp expand_args([args: args, block: block], args_values, context) do
    args_values
    |> Keyword.take(args)
    |> Enum.map(fn {k, v} -> {:=, [], [{k, [], context}, v]} end)
    |> Kernel.++(List.wrap(block))
  end
end

and then use it as

defmodule MyMacros.Test do
  @moduledoc false
  import MyMacros.Code

  define_code_group :foo, [:var1, :var2] do
    IO.inspect(var1)
    IO.inspect(var2)
  end

  define_code_group :foo do
    IO.inspect(3)
  end

  embed_code_groups(:foo, var1: 42, var2: :ok)

  def call do
    embed_code_groups(:foo, var1: 42, var2: :ok)
  end
end

MyMacros.Test.call()
6

Hmm, I’m not sure how that code would help since I’m talking about check groups, not groups of code with args etc.

But your code did help me to come up with a solution, here is the code:

defmodule Ui.Ash.Policy.Macros do
  defmacro check_group(name, do: block) do
    module = __CALLER__.module

    if not Module.has_attribute?(module, name) do
      Module.register_attribute(module, :check_groups, persist: true, accumulate: true)
    end

    if module |> Module.get_attribute(:check_groups) |> Keyword.get(name) do
      raise CompileError,
        description: "check_group \"#{name}\" for module #{module} already defined"
    end

    Module.put_attribute(module, :check_groups, {name, [block: block]})
  end

  defmacro check_group(name) do
    module = __CALLER__.module

    module
    |> Module.get_attribute(:check_groups)
    |> Kernel.||([])
    |> Keyword.fetch(name)
    |> case do
      {:ok, block} ->
        block

      :error ->
        raise CompileError,
          description: "Unknown check_group named \"#{name}\" for module #{module}"
    end
  end
end

defmodule Ui.Resources.Resource1.Policies do
  @moduledoc false

  use Spark.Dsl.Fragment,
    of: Ash.Resource,
    authorizers: [Ash.Policy.Authorizer]

  import Ui.Ash.Policy.Macros

  policies do
    check_group :common do
      forbid_unless actor_attribute_equals(:role, :admin)
      forbid_unless actor_attribute_equals(:active?, true)
    end

    policy action(:read) do
      check_group :common
      authorize_if always()
    end
  end
end

I didn’t test it too much, but seem to work fine so far

7

My code does exactly the same, but it can be reused for any code to be embedded.

Basically, mine was a general solution, and yours is tied to this particular usecase.

Besides that, my groups were composable and accepted additional arguments just in case.