diff --git a/lib/ash/policy/info.ex b/lib/ash/policy/info.ex index 63a84c6a..bda4fa26 100644 --- a/lib/ash/policy/info.ex +++ b/lib/ash/policy/info.ex @@ -197,7 +197,6 @@ defmodule Ash.Policy.Info do %Ash.Policy.Policy{ policies: policies, condition: conditions, - checks: checks, access_type: access_type } = policy, default @@ -206,7 +205,6 @@ defmodule Ash.Policy.Info do policy | policies: set_access_type(policies, access_type || default), condition: set_access_type(conditions, access_type || default), - checks: set_access_type(checks, default), access_type: access_type || default } end diff --git a/lib/ash/policy/policy.ex b/lib/ash/policy/policy.ex index 376833f2..fc5b9567 100644 --- a/lib/ash/policy/policy.ex +++ b/lib/ash/policy/policy.ex @@ -7,7 +7,6 @@ defmodule Ash.Policy.Policy do :condition, :policies, :bypass?, - :checks, :description, :access_type ] @@ -52,14 +51,28 @@ defmodule Ash.Policy.Policy do @doc false def transform(policy) do - if Enum.empty?(policy.policies) do - {:error, "Policies must have at least one check."} - else - if policy.condition in [nil, []] do + cond do + Enum.empty?(policy.policies) -> + {:error, "Policies must have at least one check."} + + policy.bypass? && + Enum.all?(List.wrap(policy.policies), &(&1.type in [:forbid_if, :forbid_unless])) -> + {:error, + """ + Bypass policies that can only ever forbid have no effect. + + When a bypass is authorized, it skips all remaining policies (including other bypasses) + and authorizes the request. If it fails, it is ignored and the remaining policies are checked. + + This policy only contains `forbid_if` or `forbid_unless` check types therefore, it can + never have an effect. + """} + + policy.condition in [nil, []] -> {:ok, %{policy | condition: [{Ash.Policy.Check.Static, result: true}]}} - else + + true -> {:ok, policy} - end end end diff --git a/test/resource/policies_test.exs b/test/resource/policies_test.exs new file mode 100644 index 00000000..3a4be4b4 --- /dev/null +++ b/test/resource/policies_test.exs @@ -0,0 +1,27 @@ +defmodule Ash.Test.Resource.PoliciesTest do + @moduledoc false + use ExUnit.Case, async: true + + test "records can belong to other resources" do + assert_raise Spark.Error.DslError, + ~r/Bypass policies that can only ever forbid have no effect/, + fn -> + defmodule HasBadBypassPolicy do + use Ash.Resource, + domain: Ash.Test.Domain, + authorizers: [Ash.Policy.Authorizer] + + attributes do + uuid_primary_key :id + end + + policies do + bypass always() do + forbid_if always() + forbid_unless always() + end + end + end + end + end +end