improvement: show an explanation when no policies apply

documentation/topics/security/policies.md

@@ -432,7 +432,13 @@ config :ash, :policies, private_fields: :include
 
 Policy breakdowns can be fetched on demand for a given forbidden error (either an `Ash.Error.Forbidden` that contains one ore more `Ash.Error.Forbidden.Policy` errors, or an `Ash.Error.Forbidden.Policy` error itself), via `Ash.Error.Forbidden.Policy.report/2`.
 
-Here is an example policy breakdown from tests:
+Additionally, you can request that they be provided in the error message for all raised forbidden errors (without the help text), by setting
+
+```elixir
+config :ash, :policies, show_policy_breakdowns?: true
+```
+
+Here is an example policy breakdown from tests.
 
 ```text
 Policy Breakdown

lib/ash/error/forbidden/policy.ex

@@ -18,6 +18,7 @@ defmodule Ash.Error.Forbidden.Policy do
       context_description: nil,
       policies: [],
       resource: nil,
+      domain: nil,
       action: nil,
       changeset_doesnt_match_filter: false
     ],
@@ -90,6 +91,8 @@ defmodule Ash.Error.Forbidden.Policy do
                 filter,
                 policies,
                 Keyword.merge(opts,
+                  domain: error.domain,
+                  resource: error.resource,
                   must_pass_strict_check?: must_pass_strict_check?,
                   context_description: error.context_description,
                   for_fields: error.for_fields
@@ -156,16 +159,48 @@ defmodule Ash.Error.Forbidden.Policy do
       if Keyword.get(opts, :help_text?, true) do
         ["Policy Breakdown#{policy_context_description}", @help_text]
       else
-        "Policy Breakdown#{policy_context_description}"
+        ["Policy Breakdown#{policy_context_description}"]
       end
 
     policy_explanation =
       policies
       |> Kernel.||([])
       |> Enum.filter(&relevant?(&1, facts))
-      |> Enum.map(&explain_policy(&1, facts, opts[:success?] || false))
-      |> Enum.intersperse("\n")
-      |> title(policy_breakdown_title, false)
+      |> case do
+        [] ->
+          # If no policies are relevant, then we treat them all as relevant
+          title =
+            case policies do
+              [] ->
+                if opts[:domain] && opts[:resource] do
+                  policy_breakdown_title ++
+                    [
+                      "No policies defined on `#{inspect(opts[:domain])}` or `#{inspect(opts[:resource])}`.\nFor safety, at least one policy must apply to all requests.\n"
+                    ]
+                else
+                  policy_breakdown_title ++
+                    [
+                      "No policies defined.\n"
+                    ]
+                end
+
+              _ ->
+                [
+                  "No policy conditions applied to this request.\nFor safety, at least one policy must apply to all requests.\n"
+                ]
+            end
+
+          {policies, title}
+
+        relevant ->
+          {relevant, policy_breakdown_title}
+      end
+      |> then(fn {policies, title} ->
+        policies
+        |> Enum.map(&explain_policy(&1, facts, opts[:success?] || false))
+        |> Enum.intersperse("\n")
+        |> title(title, false)
+      end)
 
     filter =
       if filter do
@@ -217,7 +252,6 @@ defmodule Ash.Error.Forbidden.Policy do
   end
 
   defp title(other, title, semicolon \\ true)
-  defp title([], _, _), do: []
   defp title(other, title, true), do: [title, ":\n", other]
   defp title(other, title, false), do: [title, "\n", other]
 
@@ -275,59 +309,52 @@ defmodule Ash.Error.Forbidden.Policy do
   defp describe_conditions(condition, facts) do
     condition
     |> List.wrap()
-    |> case do
-      [{Ash.Policy.Check.Static, opts}] ->
-        {[], opts[:result]}
+    |> Enum.reduce({[], true}, fn condition, {conditions, status} ->
+      {mod, opts} =
+        case condition do
+          %{module: module, opts: opts} ->
+            {module, opts}
 
-      conditions ->
-        conditions
-        |> Enum.reduce({[], true}, fn condition, {conditions, status} ->
-          {mod, opts} =
-            case condition do
-              %{module: module, opts: opts} ->
-                {module, opts}
+          {module, opts} ->
+            {module, opts}
+        end
 
-              {module, opts} ->
-                {module, opts}
-            end
+      new_status =
+        if status in [false, :unknown] do
+          false
+        else
+          case Policy.fetch_fact(facts, {mod, opts}) do
+            {:ok, true} ->
+              true
 
-          new_status =
-            if status in [false, :unknown] do
+            {:ok, false} ->
               false
-            else
-              case Policy.fetch_fact(facts, {mod, opts}) do
-                {:ok, true} ->
-                  true
 
-                {:ok, false} ->
-                  false
+            _ ->
+              :unknown
+          end
+        end
 
-                _ ->
-                  :unknown
-              end
-            end
+      {[["condition: ", mod.describe(opts)] | conditions], new_status}
+    end)
+    |> then(fn {conditions, status} ->
+      conditions =
+        conditions
+        |> Enum.reverse()
+        |> case do
+          [] ->
+            []
 
-          {[["condition: ", mod.describe(opts)] | conditions], new_status}
-        end)
-        |> then(fn {conditions, status} ->
-          conditions =
-            conditions
-            |> Enum.reverse()
-            |> case do
-              [] ->
-                []
+          conditions ->
+            [
+              conditions
+              |> Enum.intersperse("\n"),
+              "\n"
+            ]
+        end
 
-              conditions ->
-                [
-                  conditions
-                  |> Enum.intersperse("\n"),
-                  "\n"
-                ]
-            end
-
-          {conditions, status}
-        end)
-    end
+      {conditions, status}
+    end)
   end
 
   defp describe_checks(checks, facts, success?) do

lib/ash/policy/authorizer/authorizer.ex

@@ -486,6 +486,7 @@ defmodule Ash.Policy.Authorizer do
       resource: Map.get(state, :resource),
       action: Map.get(state, :action),
       actor: Map.get(state, :actor),
+      domain: Map.get(state, :domain),
       changeset_doesnt_match_filter: true,
       filter: filter
     )
@@ -495,6 +496,7 @@ defmodule Ash.Policy.Authorizer do
     Ash.Error.Forbidden.Policy.exception(
       scenarios: Map.get(state, :scenarios),
       facts: Map.get(state, :facts),
+      domain: Map.get(state, :domain),
       policies: Map.get(state, :policies),
       resource: Map.get(state, :resource),
       action: Map.get(state, :action),
@@ -506,6 +508,7 @@ defmodule Ash.Policy.Authorizer do
   def exception(_, state) do
     Ash.Error.Forbidden.Policy.exception(
       scenarios: Map.get(state, :scenarios),
+      domain: Map.get(state, :domain),
       facts: Map.get(state, :facts),
       policies: Map.get(state, :policies),
       resource: Map.get(state, :resource),
@@ -1507,6 +1510,7 @@ defmodule Ash.Policy.Authorizer do
         {:error,
          Ash.Error.Forbidden.Policy.exception(
            facts: authorizer.facts,
+           domain: Map.get(authorizer, :domain),
            policies: authorizer.policies,
            context_description: opts[:context_description],
            for_fields: opts[:for_fields],
@@ -1529,6 +1533,7 @@ defmodule Ash.Policy.Authorizer do
         {:error,
          Ash.Error.Forbidden.Policy.exception(
            facts: authorizer.facts,
+           domain: Map.get(authorizer, :domain),
            policies: authorizer.policies,
            context_description: opts[:context_description],
            for_fields: opts[:for_fields],

lib/ash/policy/info.ex

@@ -9,7 +9,7 @@ defmodule Ash.Policy.Info do
 
   @doc "Whether or not Ash policy authorizer is configured to show policy breakdowns in error messages"
   def show_policy_breakdowns? do
-    Application.get_env(:ash, :policies)[:show_policy_breakdowns?] || false
+    Keyword.get(Application.get_env(:ash, :policies, []), :show_policy_breakdowns?, true)
   end
 
   @doc "Whether or not Ash policy authorizer is configured to log policy breakdowns"

test/actions/bulk/bulk_create_test.exs

@@ -4,6 +4,7 @@ defmodule Ash.Test.Actions.BulkCreateTest do
 
   import Ash.Expr, only: [expr: 1]
   import ExUnit.CaptureLog
+  require Ash.Expr
 
   alias Ash.Test.Domain, as: Domain
 
@@ -633,6 +634,7 @@ defmodule Ash.Test.Actions.BulkCreateTest do
                tenant: org.id,
                return_records?: true,
                sorted?: true,
+               return_errors?: true,
                authorize?: false
              )
 
@@ -653,6 +655,7 @@ defmodule Ash.Test.Actions.BulkCreateTest do
                return_records?: true,
                tenant: org.id,
                upsert?: true,
+               return_errors?: true,
                upsert_identity: :unique_title,
                upsert_fields: [:title2],
                upsert_condition: expr(upsert_conflict(:title) != "title3"),

test/policy/simple_test.exs

@@ -15,7 +15,47 @@ defmodule Ash.Test.Policy.SimpleTest do
     User
   }
 
+  defmodule ResourceWithNoPolicies do
+    use Ash.Resource,
+      domain: Ash.Test.Domain,
+      authorizers: [Ash.Policy.Authorizer]
+
+    attributes do
+      uuid_primary_key :id
+    end
+
+    actions do
+      defaults [:create, :read]
+    end
+  end
+
+  defmodule ResourceWithAPolicyThatDoesntApply do
+    use Ash.Resource,
+      domain: Ash.Test.Domain,
+      authorizers: [Ash.Policy.Authorizer]
+
+    attributes do
+      uuid_primary_key :id
+    end
+
+    actions do
+      defaults [:create, :read]
+    end
+
+    policies do
+      policy never() do
+        authorize_if always()
+      end
+    end
+  end
+
   setup do
+    Application.put_env(:ash, :policies, show_policy_breakdowns?: true)
+
+    on_exit(fn ->
+      Application.delete_env(:ash, :policies)
+    end)
+
     [
       user: Ash.create!(Ash.Changeset.for_create(User, :create), authorize?: false),
       admin:
@@ -23,6 +63,24 @@ defmodule Ash.Test.Policy.SimpleTest do
     ]
   end
 
+  test "breakdowns for resources with no policies explain the error" do
+    assert_raise Ash.Error.Forbidden,
+                 ~r/No policies defined on `Ash.Test.Domain` or `Ash.Test.Policy.SimpleTest.ResourceWithNoPolicies`/,
+                 fn ->
+                   ResourceWithNoPolicies
+                   |> Ash.read!()
+                 end
+  end
+
+  test "breakdowns for action where no policies that apply explain the error" do
+    assert_raise Ash.Error.Forbidden,
+                 ~r/No policy conditions applied to this request/,
+                 fn ->
+                   ResourceWithAPolicyThatDoesntApply
+                   |> Ash.read!()
+                 end
+  end
+
   test "bypass with condition does not apply subsequent filters", %{admin: admin, user: user} do
     Ash.create!(Ash.Changeset.for_create(Tweet, :create), authorize?: false)