diff --git a/documentation/topics/security/policies.md b/documentation/topics/security/policies.md index e6c8d69c..34c8c94f 100644 --- a/documentation/topics/security/policies.md +++ b/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 diff --git a/lib/ash/error/forbidden/policy.ex b/lib/ash/error/forbidden/policy.ex index 18372057..cfe4fb24 100644 --- a/lib/ash/error/forbidden/policy.ex +++ b/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 diff --git a/lib/ash/policy/authorizer/authorizer.ex b/lib/ash/policy/authorizer/authorizer.ex index 3376dab3..c063778c 100644 --- a/lib/ash/policy/authorizer/authorizer.ex +++ b/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], diff --git a/lib/ash/policy/info.ex b/lib/ash/policy/info.ex index bda4fa26..2bc8ba24 100644 --- a/lib/ash/policy/info.ex +++ b/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" diff --git a/test/actions/bulk/bulk_create_test.exs b/test/actions/bulk/bulk_create_test.exs index 6671fcdf..f0d042c0 100644 --- a/test/actions/bulk/bulk_create_test.exs +++ b/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"), diff --git a/test/policy/simple_test.exs b/test/policy/simple_test.exs index 6d0a5285..6f821769 100644 --- a/test/policy/simple_test.exs +++ b/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)