fix: multiple filter-checks in policy conditions were not composing p…

…roperly

lib/ash/policy/policy.ex

@@ -314,7 +314,7 @@ defmodule Ash.Policy.Policy do
     {condition_expression, authorizer} = condition_expression(condition, authorizer)
 
     case condition_expression do
-      true ->
+      empty when empty in [nil, true] ->
         if bypass? do
           case compile_policy_expression(policies, authorizer) do
             {true, authorizer} ->
@@ -360,57 +360,6 @@ defmodule Ash.Policy.Policy do
       false ->
         compile_policy_expression(rest, authorizer)
 
-      nil ->
-        if bypass? do
-          case compile_policy_expression(policies, authorizer) do
-            {true, authorizer} ->
-              {true, authorizer}
-
-            {false, authorizer} ->
-              {at_least_one_policy_expression, authorizer} =
-                at_least_one_policy_expression(Enum.take_while(rest, &(!&1.bypass?)), authorizer)
-
-              rest = compile_policy_expression(rest, authorizer)
-              {:and, rest, at_least_one_policy_expression}
-
-            {policy_expression, authorizer} ->
-              {at_least_one_policy_expression, authorizer} =
-                at_least_one_policy_expression(Enum.take_while(rest, &(!&1.bypass?)), authorizer)
-
-              case compile_policy_expression(rest, authorizer) do
-                {false, authorizer} ->
-                  {policy_expression, authorizer}
-
-                {true, authorizer} ->
-                  {{:or, policy_expression, at_least_one_policy_expression}, authorizer}
-
-                {rest, authorizer} ->
-                  {{:or, policy_expression, {:and, rest, at_least_one_policy_expression}},
-                   authorizer}
-              end
-          end
-        else
-          case compile_policy_expression(policies, authorizer) do
-            {true, authorizer} ->
-              compile_policy_expression(rest, authorizer)
-
-            {false, authorizer} ->
-              {false, authorizer}
-
-            {policy_expression, authorizer} ->
-              case compile_policy_expression(rest, authorizer) do
-                {true, authorizer} ->
-                  {policy_expression, authorizer}
-
-                {false, authorizer} ->
-                  {false, authorizer}
-
-                {rest, authorizer} ->
-                  {{:and, policy_expression, rest}, authorizer}
-              end
-          end
-        end
-
       condition_expression ->
         if bypass? do
           {at_least_one_policy_expression, authorizer} =
@@ -458,36 +407,21 @@ defmodule Ash.Policy.Policy do
               end
           end
         else
-          {condition_and_policy_expression, authorizer} =
-            case compile_policy_expression(policies, authorizer) do
-              {true, authorizer} ->
-                {condition_expression, authorizer}
-
-              {false, authorizer} ->
-                {false, authorizer}
-
-              {policy_expression, authorizer} ->
-                {{:and, condition_expression, policy_expression}, authorizer}
-            end
-
-          case condition_and_policy_expression do
-            false ->
+          case compile_policy_expression(policies, authorizer) do
+            {true, authorizer} ->
               compile_policy_expression(rest, authorizer)
 
-            true ->
-              {true, authorizer}
+            {false, authorizer} ->
+              {rest_expr, authorizer} = compile_policy_expression(rest, authorizer)
 
-            condition_and_policy_expression ->
-              case compile_policy_expression(rest, authorizer) do
-                {true, authorizer} ->
-                  {condition_and_policy_expression, authorizer}
+              {{:and, {:not, condition_expression}, rest_expr}, authorizer}
 
-                {false, authorizer} ->
-                  {false, authorizer}
+            {policy_expression, authorizer} ->
+              {rest_expr, authorizer} = compile_policy_expression(rest, authorizer)
 
-                {rest, authorizer} ->
-                  {{:and, condition_and_policy_expression, rest}, authorizer}
-              end
+              {{:and,
+                {:or, {:not, condition_expression},
+                 {:and, condition_expression, policy_expression}}, rest_expr}, authorizer}
           end
         end
     end

test/policy/simple_test.exs

@@ -185,4 +185,31 @@ defmodule Ash.Test.Policy.SimpleTest do
       |> Ash.read!(authorize?: true, actor: user)
     end
   end
+
+  test "two filter condition checks combine properly" do
+    user1 = Ash.create!(Ash.Changeset.for_create(User, :create), authorize?: false)
+    user2 = Ash.create!(Ash.Changeset.for_create(User, :create), authorize?: false)
+
+    user1_thing =
+      Ash.Test.Support.PolicySimple.TwoFilters
+      |> Ash.Changeset.for_create(:create, %{user_id: user1.id})
+      |> Ash.create!(authorize?: false)
+
+    user2_thing =
+      Ash.Test.Support.PolicySimple.TwoFilters
+      |> Ash.Changeset.for_create(:create, %{user_id: user2.id})
+      |> Ash.create!(authorize?: false)
+
+    assert [user1_got_thing] =
+             Ash.Test.Support.PolicySimple.TwoFilters
+             |> Ash.read!(authorize?: true, actor: user1)
+
+    assert user1_got_thing.id == user1_thing.id
+
+    assert [user2_got_thing] =
+             Ash.Test.Support.PolicySimple.TwoFilters
+             |> Ash.read!(authorize?: true, actor: user2)
+
+    assert user2_got_thing.id == user2_thing.id
+  end
 end

test/support/policy_simple/domain.ex

@@ -12,5 +12,6 @@ defmodule Ash.Test.Support.PolicySimple.Domain do
     resource(Ash.Test.Support.PolicySimple.Tweet)
     resource(Ash.Test.Support.PolicySimple.Foo)
     resource(Ash.Test.Support.PolicySimple.Always)
+    resource(Ash.Test.Support.PolicySimple.TwoFilters)
   end
 end

test/support/policy_simple/resources/always.ex

@@ -20,6 +20,7 @@ defmodule Ash.Test.Support.PolicySimple.Always do
 
   relationships do
     belongs_to :user, Ash.Test.Support.PolicySimple.User
+    belongs_to :user2, Ash.Test.Support.PolicySimple.User
   end
 
   policies do

test/support/policy_simple/resources/two_filters.ex

@@ -0,0 +1,35 @@
+defmodule Ash.Test.Support.PolicySimple.TwoFilters do
+  @moduledoc false
+
+  use Ash.Resource,
+    domain: Ash.Test.Support.PolicySimple.Domain,
+    data_layer: Ash.DataLayer.Ets,
+    authorizers: [Ash.Policy.Authorizer]
+
+  ets do
+    private?(true)
+  end
+
+  actions do
+    defaults [:read, create: [:user_id]]
+  end
+
+  attributes do
+    uuid_primary_key :id
+  end
+
+  relationships do
+    belongs_to :user, Ash.Test.Support.PolicySimple.User
+    belongs_to :user2, Ash.Test.Support.PolicySimple.User
+  end
+
+  policies do
+    policy relates_to_actor_via(:user) do
+      authorize_if always()
+    end
+
+    policy relates_to_actor_via(:user2) do
+      authorize_if always()
+    end
+  end
+end