fix: fix pattern match error on certain policy conditions

improvement: various policy behavior cleanups

lib/ash/policy/authorizer/authorizer.ex

@@ -489,6 +489,10 @@ defmodule Ash.Policy.Authorizer do
         log_successful_policy_breakdown(authorizer, filter)
         {:filter, strict_check_all_facts(authorizer), filter}
 
+      {:filter_and_continue, filter, authorizer} ->
+        log_successful_policy_breakdown(authorizer, filter)
+        {:filter, strict_check_all_facts(authorizer), filter}
+
       {:error, error} ->
         {:error, error}
 
@@ -959,7 +963,7 @@ defmodule Ash.Policy.Authorizer do
             match?(
               {:ok, _},
               Ash.Policy.Policy.fetch_fact(authorizer.facts, {check_module, opts})
-            ) || check_module.type() == :filter
+            )
         end)
       end)
 

lib/ash/policy/checker.ex

@@ -86,8 +86,8 @@ defmodule Ash.Policy.Checker do
 
       {:ok, scenarios, authorizer} ->
         scenarios
-        |> Ash.Policy.SatSolver.simplify_clauses()
         |> remove_scenarios_with_impossible_facts(authorizer)
+        |> Ash.Policy.SatSolver.simplify_clauses()
         |> case do
           [] -> {:ok, false, authorizer}
           scenarios -> {:ok, scenarios, authorizer}
@@ -99,16 +99,18 @@ defmodule Ash.Policy.Checker do
   end
 
   defp remove_scenarios_with_impossible_facts(scenarios, authorizer) do
-    # Remove any scenarios with a fact that must be a certain value, but are not, at strict check time
-    # They aren't true, so that scenario isn't possible
-
     Enum.reject(scenarios, fn scenario ->
       Enum.any?(scenario, fn {{mod, opts}, required_value} ->
-        opts[:access_type] == :strict &&
-          not match?(
-            {:ok, ^required_value},
-            Policy.fetch_fact(authorizer.facts, {mod, opts})
-          )
+        case Policy.fetch_fact(authorizer.facts, {mod, opts}) do
+          {:ok, :unknown} ->
+            opts[:access_type] == :strict
+
+          {:ok, value} ->
+            value != required_value
+
+          :error ->
+            opts[:access_type] == :strict
+        end
       end)
     end)
   end

lib/ash/policy/policy.ex

@@ -443,13 +443,13 @@ defmodule Ash.Policy.Policy do
           {condition_and_policy_expression, authorizer} =
             case compile_policy_expression(policies, authorizer) do
               {true, authorizer} ->
-                {condition, authorizer}
+                {condition_expression, authorizer}
 
               {false, authorizer} ->
                 {false, authorizer}
 
               {policy_expression, authorizer} ->
-                {{:and, condition, policy_expression}, authorizer}
+                {{:and, condition_expression, policy_expression}, authorizer}
             end
 
           case condition_and_policy_expression do

lib/ash/policy/sat_solver.ex

@@ -51,36 +51,33 @@ defmodule Ash.Policy.SatSolver do
   def simplify_clauses([scenario]), do: [scenario]
 
   def simplify_clauses(scenarios) do
-    unnecessary_clauses =
-      scenarios
-      |> Enum.with_index()
-      |> Enum.flat_map(fn {scenario, index} ->
-        scenario
-        |> Enum.flat_map(fn {fact, _value} ->
-          if Enum.find(scenarios, fn other_scenario ->
-               scenario_makes_fact_irrelevant?(other_scenario, scenario, fact)
+    indexed = Enum.with_index(scenarios)
+
+    indexed
+    |> Enum.find_value(fn {scenario, index} ->
+      Enum.find_value(scenario, fn {fact, _value} ->
+        case Enum.find_value(indexed, fn {other_scenario, other_index} ->
+               if scenario != other_scenario &&
+                    scenario_makes_fact_irrelevant?(other_scenario, scenario, fact) do
+                 other_index
+               end
              end) do
-            [fact]
-          else
-            []
+          nil ->
+            nil
+
+          other_index ->
+            {fact, other_index, index}
         end
       end)
-        |> Enum.map(fn fact ->
-          {index, fact}
     end)
-      end)
-      |> Enum.group_by(&elem(&1, 0), &elem(&1, 1))
-
-    case unnecessary_clauses do
-      empty when empty == %{} ->
+    |> case do
+      nil ->
         scenarios
 
-      unnecessary_clauses ->
-        unnecessary_clauses
-        |> Enum.reduce(scenarios, fn {index, facts}, scenarios ->
-          List.update_at(scenarios, index, &Map.drop(&1, facts))
-        end)
-        |> Enum.reject(&(&1 == %{}))
+      {fact, index1, index2} ->
+        scenarios
+        |> List.update_at(index1, &Map.delete(&1, fact))
+        |> List.update_at(index2, &Map.delete(&1, fact))
         |> Enum.uniq()
         |> simplify_clauses()
     end
@@ -91,18 +88,12 @@ defmodule Ash.Policy.SatSolver do
       do: false
 
   def scenario_makes_fact_irrelevant?(potential_irrelevant_maker, scenario, fact) do
-    scenario_is_subset?(Map.delete(potential_irrelevant_maker, fact), scenario) &&
+    Map.delete(potential_irrelevant_maker, fact) == Map.delete(scenario, fact) &&
       Map.has_key?(potential_irrelevant_maker, fact) && Map.has_key?(scenario, fact) &&
       Map.get(potential_irrelevant_maker, fact) !=
         Map.get(scenario, fact)
   end
 
-  defp scenario_is_subset?(left, right) do
-    Enum.all?(left, fn {fact, value} ->
-      Map.get(right, fact) == value
-    end)
-  end
-
   @spec add_negations_and_solve(term, term) :: term | no_return()
   defp add_negations_and_solve(cnf, negations) do
     solve_expression(cnf ++ negations)

test/support/policy_complex/resources/post.ex

@@ -11,13 +11,13 @@ defmodule Ash.Test.Support.PolicyComplex.Post do
       authorize_if always()
     end
 
-    policy action_type(:read) do
+    policy [action_type(:read)] do
       authorize_if relates_to_actor_via(:author)
       authorize_if relates_to_actor_via([:author, :friends])
     end
 
     policy action_type(:create) do
-      authorize_if relating_to_actor(:author)
+      authorize_if always()
     end
   end