chore: add more authorization tests

chore: improve authorization test helper

improvement: support `{:filter, _}` authorization results for changesets

lib/ash/engine/request.ex

@@ -507,31 +507,15 @@ defmodule Ash.Engine.Request do
             {:ok, set_authorizer_state(request, authorizer, :authorized)}
 
           {:filter, filter} ->
-            request
-            |> Map.update!(:query, &Ash.Query.filter(&1, ^filter))
-            |> Map.update(
-              :authorization_filter,
-              filter,
-              &add_to_or_parse(&1, filter, request.resource)
-            )
-            |> set_authorizer_state(authorizer, :authorized)
-            |> try_resolve([request.path ++ [:query]], false)
+            apply_filter(request, authorizer, filter, true)
 
           {:filter_and_continue, _, _} when strict_check_only? ->
             {:error, MustPassStrictCheck.exception(resource: request.resource)}
 
           {:filter_and_continue, filter, new_authorizer_state} ->
-            new_request =
-              request
-              |> Map.update!(:query, &Ash.Query.filter(&1, ^filter))
-              |> Map.update(
-                :authorization_filter,
-                filter,
-                &add_to_or_parse(&1, filter, request.resource)
-              )
-              |> set_authorizer_state(authorizer, new_authorizer_state)
-
-            {:ok, new_request}
+            request
+            |> set_authorizer_state(authorizer, new_authorizer_state)
+            |> apply_filter(authorizer, filter)
 
           {:continue, _} when strict_check_only? ->
             {:error, MustPassStrictCheck.exception(resource: request.resource)}
@@ -562,6 +546,42 @@ defmodule Ash.Engine.Request do
     end
   end
 
+  defp apply_filter(request, authorizer, filter, resolve_data? \\ false)
+
+  defp apply_filter(%{action: %{type: :read}} = request, authorizer, filter, resolve_data?) do
+    request =
+      request
+      |> Map.update!(:query, &Ash.Query.filter(&1, ^filter))
+      |> Map.update(
+        :authorization_filter,
+        filter,
+        &add_to_or_parse(&1, filter, request.resource)
+      )
+      |> set_authorizer_state(authorizer, :authorized)
+
+    if resolve_data? do
+      try_resolve(request, [request.path ++ [:query]], false)
+    else
+      {:ok, request}
+    end
+  end
+
+  defp apply_filter(request, authorizer, filter, resolve_data?) do
+    case do_runtime_filter(request, filter) do
+      {:ok, request} ->
+        request = set_authorizer_state(request, authorizer, :authorized)
+
+        if resolve_data? do
+          try_resolve(request, [request.path ++ [:query]], false)
+        else
+          {:ok, request}
+        end
+
+      {:error, error} ->
+        {:error, error}
+    end
+  end
+
   defp add_to_or_parse(existing_authorization_filter, filter, resource) do
     if existing_authorization_filter do
       Ash.Filter.add_to_filter(existing_authorization_filter, filter)
@@ -652,10 +672,11 @@ defmodule Ash.Engine.Request do
     end
   end
 
-  defp do_runtime_filter(%{data: empty} = request, _filter) when empty in [nil, []],
-    do: {:ok, request}
+  defp do_runtime_filter(%{action: %{type: :read}, data: empty} = request, _filter)
+       when empty in [nil, []],
+       do: {:ok, request}
 
-  defp do_runtime_filter(request, filter) do
+  defp do_runtime_filter(%{action: %{type: :read}} = request, filter) do
     pkey = Ash.Resource.primary_key(request.resource)
 
     pkeys =
@@ -692,6 +713,34 @@ defmodule Ash.Engine.Request do
     end
   end
 
+  defp do_runtime_filter(request, filter) do
+    pkey = Ash.Resource.primary_key(request.resource)
+
+    pkey =
+      request.changeset.data
+      |> Map.take(pkey)
+      |> Map.to_list()
+
+    new_query =
+      request.resource
+      |> Ash.Query.filter(^pkey)
+      |> Ash.Query.filter(^filter)
+      |> Ash.Query.limit(1)
+
+    new_query
+    |> Ash.Actions.Read.unpaginated_read()
+    |> case do
+      {:ok, []} ->
+        {:error, Ash.Error.Forbidden.exception([])}
+
+      {:ok, [_]} ->
+        {:ok, request}
+
+      {:error, error} ->
+        {:error, error}
+    end
+  end
+
   defp try_resolve(request, deps, internal?) do
     Enum.reduce_while(deps, {:ok, request, [], []}, fn dep,
                                                        {:ok, request, notifications, skipped} ->
@@ -918,6 +967,7 @@ defmodule Ash.Engine.Request do
   defp missing_strict_check_dependencies?(authorizer, request) do
     authorizer
     |> Authorizer.strict_check_context(authorizer_state(request, authorizer))
+    |> List.wrap()
     |> Enum.filter(fn dependency ->
       match?(%UnresolvedField{}, Map.get(request, dependency))
     end)

lib/ash/filter/runtime.ex

@@ -155,6 +155,9 @@ defmodule Ash.Filter.Runtime do
           {:error, error} ->
             {:error, error}
 
+          :unknown ->
+            :unknown
+
           _ ->
             {:ok, false}
         end
@@ -167,12 +170,18 @@ defmodule Ash.Filter.Runtime do
           {:error, error} ->
             {:error, error}
 
+          :unknown ->
+            :unknown
+
           _ ->
             {:ok, false}
         end
 
       %Not{expression: expression} ->
         case do_match(record, expression) do
+          :unknown ->
+            :unknown
+
           {:ok, match?} ->
             {:ok, !match?}
 
@@ -191,9 +200,11 @@ defmodule Ash.Filter.Runtime do
       case resolve_expr(expr, record) do
         {:ok, resolved} -> {:cont, {:ok, [resolved | exprs]}}
         {:error, error} -> {:halt, {:error, error}}
+        :unknown -> {:halt, :unknown}
       end
     end)
     |> case do
+      :unknown -> :unknown
       {:ok, resolved} -> {:ok, Enum.reverse(resolved)}
       {:error, error} -> {:error, error}
     end
@@ -225,6 +236,9 @@ defmodule Ash.Filter.Runtime do
       {:error, error} ->
         {:error, error}
 
+      :unknown ->
+        :unknown
+
       _ ->
         {:ok, false}
     end
@@ -238,6 +252,9 @@ defmodule Ash.Filter.Runtime do
       {:error, error} ->
         {:error, error}
 
+      :unknown ->
+        :unknown
+
       _ ->
         {:ok, false}
     end
@@ -281,6 +298,9 @@ defmodule Ash.Filter.Runtime do
 
       {:ok, false} ->
         {:ok, false}
+
+      :unknown ->
+        :unknown
     end
   end
 
@@ -291,6 +311,9 @@ defmodule Ash.Filter.Runtime do
 
       {:ok, false} ->
         do_match(record, right)
+
+      :unknown ->
+        :unknown
     end
   end
 

lib/ash/options_helpers.ex

@@ -24,7 +24,7 @@ defmodule Ash.OptionsHelpers do
     |> sanitize_schema()
     |> Enum.map(fn {key, opts} ->
       if opts[:doc] do
-        {key, Keyword.update!(opts, :doc, &String.replace(&1, "\n", "\n  "))}
+        {key, Keyword.update!(opts, :doc, &String.replace(&1, "\n\n", "  \n"))}
       else
         {key, opts}
       end

mix.exs

@@ -104,6 +104,9 @@ defmodule Ash.MixProject do
           Ash.Query.Calculation,
           Ash.Calculation
         ],
+        values: [
+          Ash.CiString
+        ],
         type: ~r/Ash.Type/,
         data_layer: ~r/Ash.DataLayer/,
         authorizer: ~r/Ash.Authorizer/,
@@ -123,7 +126,7 @@ defmodule Ash.MixProject do
         "filter operators": ~r/Ash.Query.Operator/,
         "filter functions": ~r/Ash.Query.Function/,
         "query expressions": [
-          Ash.Query.BooleanBooleanExpression,
+          Ash.Query.BooleanExpression,
           Ash.Query.Not,
           Ash.Query.Ref,
           Ash.Query.Call
@@ -137,8 +140,10 @@ defmodule Ash.MixProject do
         miscellaneous: [
           Ash.NotLoaded,
           Ash.Error.Stacktrace,
-          Ash.Query.Aggregate
-        ]
+          Ash.Query.Aggregate,
+          Ash.Query.Type
+        ],
+        comparable: ~r/Comparable/
       ]
     ]
   end

test/actions/create_test.exs

@@ -565,6 +565,8 @@ defmodule Ash.Test.Actions.CreateTest do
 
   describe "unauthorized create" do
     test "it does not create the record" do
+      start_supervised({Ash.Test.Authorizer, check: :forbidden, strict_check: :continue})
+
       assert_raise(Ash.Error.Forbidden, fn ->
         Authorized
         |> new()

test/actions/destroy_test.exs

@@ -120,6 +120,8 @@ defmodule Ash.Test.Actions.DestroyTest do
         |> new(%{name: "foobar"})
         |> Api.create!()
 
+      start_supervised({Ash.Test.Authorizer, strict_check: :continue, check: :forbidden})
+
       assert_raise(Ash.Error.Forbidden, fn ->
         Api.destroy!(author, authorize?: true)
       end)

test/actions/side_load_test.exs

@@ -1,6 +1,6 @@
 defmodule Ash.Test.Actions.SideLoadTest do
   @moduledoc false
-  use ExUnit.Case, async: true
+  use ExUnit.Case, async: false
 
   require Ash.Query
 
@@ -120,8 +120,12 @@ defmodule Ash.Test.Actions.SideLoadTest do
   end
 
   setup do
-    Process.put(:authorize?, true)
-    Process.put(:strict_check_context, [:query])
+    start_supervised(
+      {Ash.Test.Authorizer,
+       strict_check: :authorized,
+       check: {:error, Ash.Error.Forbidden.exception([])},
+       strict_check_context: [:query]}
+    )
 
     :ok
   end

test/actions/update_test.exs

@@ -569,6 +569,8 @@ defmodule Ash.Test.Actions.UpdateTest do
         |> new(%{name: "bar"})
         |> Api.create!()
 
+      start_supervised({Ash.Test.Authorizer, check: :forbidden, strict_check: :continue})
+
       assert_raise(Ash.Error.Forbidden, fn ->
         record
         |> new(%{name: "foo"})

test/authorizer/authorizer_test.exs

@@ -0,0 +1,86 @@
+defmodule Ash.Test.Changeset.AuthorizerTest do
+  @moduledoc false
+  use ExUnit.Case, async: false
+
+  require Ash.Query
+
+  defmodule Post do
+    use Ash.Resource,
+      data_layer: Ash.DataLayer.Ets,
+      authorizers: [
+        Ash.Test.Authorizer
+      ]
+
+    ets do
+      private? true
+    end
+
+    attributes do
+      uuid_primary_key :id
+
+      attribute :title, :string, allow_nil?: false
+    end
+  end
+
+  defmodule Api do
+    use Ash.Api
+
+    resources do
+      resource Post
+    end
+  end
+
+  describe "strict check can filter results" do
+    test "a simple filter is applied" do
+      start_supervised(
+        {Ash.Test.Authorizer,
+         strict_check: {:filter, [title: "foo"]}, strict_check_context: [:query]}
+      )
+
+      Post
+      |> Ash.Changeset.for_create(:create, %{title: "test"})
+      |> Api.create!()
+
+      Post
+      |> Ash.Changeset.for_create(:create, %{title: "foo"})
+      |> Api.create!()
+
+      assert [%Post{title: "foo"}] = Api.read!(Post, authorize?: true)
+    end
+
+    test "a simple filter can also be applied to changesets" do
+      start_supervised(
+        {Ash.Test.Authorizer,
+         strict_check: {:filter, [title: "foo"]}, strict_check_context: [:query, :changeset]}
+      )
+
+      # Filter always fails on creates
+      assert_raise Ash.Error.Forbidden, fn ->
+        Post
+        |> Ash.Changeset.for_create(:create, %{title: "test"})
+        |> Api.create!(authorize?: true)
+      end
+
+      good_post =
+        Post
+        |> Ash.Changeset.for_create(:create, %{title: "foo"})
+        |> Api.create!()
+
+      bad_post =
+        Post
+        |> Ash.Changeset.for_create(:create, %{title: "test"})
+        |> Api.create!()
+
+      # Filters apply to the base data
+      assert_raise Ash.Error.Forbidden, fn ->
+        bad_post
+        |> Ash.Changeset.for_update(:update, %{title: "next"})
+        |> Api.update!(authorize?: true)
+      end
+
+      good_post
+      |> Ash.Changeset.for_update(:update, %{title: "next"})
+      |> Api.update!(authorize?: true)
+    end
+  end
+end

test/support/authorizer.ex

@@ -5,26 +5,41 @@ defmodule Ash.Test.Authorizer do
   """
   @behaviour Ash.Authorizer
 
-  alias Ash.Error.Forbidden
+  use Agent
+
+  def start_link(opts) do
+    Agent.start_link(
+      fn ->
+        %{
+          strict_check_result: maybe_forbidden(opts[:strict_check]),
+          check_result: maybe_forbidden(opts[:check]),
+          strict_check_context: opts[:strict_check_context]
+        }
+      end,
+      name: __MODULE__
+    )
+  end
+
+  defp maybe_forbidden(:forbidden), do: {:error, Ash.Error.Forbidden.exception([])}
+  defp maybe_forbidden(other), do: other
 
   def initial_state(_, _, _, _), do: %{}
-  def strict_check_context(_), do: Process.get(:strict_check_context, [])
+  def strict_check_context(_), do: get(:strict_check_context, [])
 
-  def strict_check(_, _) do
-    if Process.get(:authorize?, false) do
-      :authorized
-    else
-      {:error, Forbidden.exception([])}
-    end
-  end
+  def strict_check(state, _),
+    do: get(:strict_check_result, :authorized) |> continue(state)
 
   def check_context(_), do: []
 
-  def check(_, _) do
-    if Process.get(:authorize_check?, false) do
-      :authorized
-    else
-      {:error, :forbidden}
-    end
+  def check(state, _), do: get(:check_result, :authorized) |> continue(state)
+
+  defp continue(:continue, state), do: {:continue, state}
+  defp continue(other, _), do: other
+
+  defp get(key, default) do
+    Agent.get(__MODULE__, &Map.get(&1, key)) || default
+  catch
+    :exit, _ ->
+      default
   end
 end