Skip to content

Commit 68350bd

Browse files
authored
fix: Policy Authorizer strict checking forbid_unless (#2678)
forbid_unless checks would not :halt in policy_fails_statically?, causing a subsequent authorize_if to cause the result to be authorized.
1 parent edf88bc commit 68350bd

File tree

2 files changed

+37
-1
lines changed

2 files changed

+37
-1
lines changed

lib/ash/policy/authorizer/authorizer.ex

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1966,7 +1966,7 @@ defmodule Ash.Policy.Authorizer do
19661966
{check.check_module, check.check_opts}
19671967
) do
19681968
{:ok, false, updated_auth} ->
1969-
{:cont, {:forbidden, updated_auth}}
1969+
{:halt, {:forbidden, updated_auth}}
19701970

19711971
{:ok, _, updated_auth} ->
19721972
{:cont, {status, updated_auth}}

test/policy/simple_test.exs

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,28 @@ defmodule Ash.Test.Policy.SimpleTest do
7878
end
7979
end
8080

81+
defmodule ResourceWithForbidUnlessAndAuthorizeIf do
82+
use Ash.Resource,
83+
domain: Ash.Test.Domain,
84+
authorizers: [Ash.Policy.Authorizer]
85+
86+
attributes do
87+
uuid_primary_key :id
88+
end
89+
90+
actions do
91+
defaults [:create, :read]
92+
end
93+
94+
policies do
95+
policy action_type(:read) do
96+
access_type :strict
97+
forbid_unless actor_attribute_equals(:role, :admin)
98+
authorize_if actor_attribute_equals(:has_read_permission, true)
99+
end
100+
end
101+
end
102+
81103
defmodule ResourceWithAnImpossibleCreatePolicy do
82104
use Ash.Resource,
83105
domain: Ash.Test.Domain,
@@ -398,6 +420,20 @@ defmodule Ash.Test.Policy.SimpleTest do
398420
assert message =~ "authorize if: id == \"#{actor_id}\" | ? | 🔎"
399421
end
400422

423+
test "Ash.can? returns false when forbid_unless denies and authorize_if would pass" do
424+
# Actor is NOT admin but DOES have read permission.
425+
# forbid_unless :role == :admin should deny before authorize_if is considered.
426+
actor = %{role: :viewer, has_read_permission: true}
427+
428+
refute Ash.can?({ResourceWithForbidUnlessAndAuthorizeIf, :read}, actor)
429+
end
430+
431+
test "Ash.can? returns true when both forbid_unless and authorize_if pass" do
432+
actor = %{role: :admin, has_read_permission: true}
433+
434+
assert Ash.can?({ResourceWithForbidUnlessAndAuthorizeIf, :read}, actor)
435+
end
436+
401437
test "strict read policies do not result in a filter" do
402438
thing =
403439
ResourceWithAStrictReadPolicy

0 commit comments

Comments
 (0)