Merge commit from fork

The Policy Refactoring (#2365) introduced a bug where bypass policies
were contributing only their condition to one_condition_matches instead
of their complete expression (condition AND policies).

This caused bypass policies to incorrectly satisfy the "at least one
policy applies" requirement when their condition evaluates to true
but their authorization checks fail.

For example, with these policies:

  bypass always(), do: authorize_if(actor_attribute_equals(:is_admin, true))
  policy action_type(:read), do: authorize_if(always())

The final authorization decision is: one_condition_matches AND all_policies_match

When a bypass condition is true but bypass policies fail, and subsequent
policies have non-matching conditions:

1. one_condition_matches = cond_expr (bypass condition) = true
   (bug - should check if bypass actually authorizes)
2. all_policies_match = (complete_expr OR NOT cond_expr) for each policy
   - For non-matching policies: (false OR NOT false) = true (policies don't apply)
3. Final: true AND true = true (incorrectly authorized)

The bypass condition alone satisfies "at least one policy applies" even
though the bypass fails to authorize.

The fix ensures bypass policies contribute their complete expression
(condition AND policies) to one_condition_matches.

Author: jechol

lib/ash/policy/policy.ex

@@ -49,26 +49,25 @@ defmodule Ash.Policy.Policy do
       {policy, cond_expr, complete_expr}
     end)
     |> List.foldr({false, true}, fn
-      {%{bypass?: true}, cond_expr, complete_expr}, {one_condition_matches, true} ->
+      {%{bypass?: true}, _cond_expr, complete_expr}, {one_condition_matches, true} ->
         {
-          b(cond_expr or one_condition_matches),
-          # Bypass can't relay to the next bypass if there is none
+          b(complete_expr or one_condition_matches),
           complete_expr
         }
 
       {%FieldPolicy{bypass?: true}, true, complete_expr},
       {one_condition_matches, all_policies_match} ->
         {
-          # FieldPolicy Conditions are set to true by default and therefore
-          # have to be ignore to not change the meaning of the
-          # one_condition_matches condition
           one_condition_matches,
           b(complete_expr or all_policies_match)
         }
 
-      {%{bypass?: true}, cond_expr, complete_expr}, {one_condition_matches, all_policies_match} ->
+      {%{bypass?: true}, _cond_expr, complete_expr},
+      {one_condition_matches, all_policies_match} ->
         {
-          b(cond_expr or one_condition_matches),
+          # Bypass should only contribute to "at least one policy applies" if it actually authorizes.
+          # Use complete_expr (condition AND policies) not just condition.
+          b(complete_expr or one_condition_matches),
           b(complete_expr or all_policies_match)
         }
 

test/policy/policy_test.exs

@@ -912,4 +912,62 @@ defmodule Ash.Test.Policy.Policy do
              """
     end
   end
+
+  describe "bypass policy with always-true condition" do
+    test "denies when bypass condition is true but bypass policies fail" do
+      # Scenario: non-admin user performs create action with these policies:
+      #
+      #   bypass always() do
+      #     authorize_if actor_attribute_equals(:is_admin, true)
+      #   end
+      #
+      #   policy action_type(:read) do
+      #     authorize_if always()
+      #   end
+      #
+      # Before fix: Used bypass condition (always) for one_condition_matches
+      #   → one_condition_matches = true (bypass condition matched)
+      #   → all_policies_match = true (no applicable policy)
+      #   → Final: true AND true = true (WRONG! - incorrectly authorized)
+      #
+      # After fix: Use bypass complete_expr (always AND is_admin) for one_condition_matches
+      #   → one_condition_matches = false (bypass failed to authorize)
+      #   → all_policies_match = true (no applicable policy)
+      #   → Final: false AND true = false (CORRECT! - properly denied)
+
+      policies = [
+        # bypass always(), do: authorize_if(actor_attribute_equals(:is_admin, true))
+        %Ash.Policy.Policy{
+          bypass?: true,
+          condition: [{Ash.Policy.Check.Static, result: true}],
+          policies: [
+            %Ash.Policy.Check{
+              type: :authorize_if,
+              check: {Ash.Policy.Check.Static, result: false},
+              check_module: Ash.Policy.Check.Static,
+              check_opts: [result: false]
+            }
+          ]
+        },
+        # policy action_type(:read) do: authorize_if(always())
+        %Ash.Policy.Policy{
+          bypass?: false,
+          condition: [{Ash.Policy.Check.Static, result: false}],
+          policies: [
+            %Ash.Policy.Check{
+              type: :authorize_if,
+              check: {Ash.Policy.Check.Static, result: true},
+              check_module: Ash.Policy.Check.Static,
+              check_opts: [result: true]
+            }
+          ]
+        }
+      ]
+
+      expression = Ash.Policy.Policy.expression(policies, %{})
+
+      # Should deny: bypass failed and no policy condition matched
+      assert expression == false
+    end
+  end
 end