Improve: Make deny_filter robust and add regression test

- Change deny_filter from [id: {:in, []}] to expr(false)
- Add regression test to ensure deny-filter matches 0 records
This commit is contained in:
Moritz 2026-01-08 23:29:37 +01:00
parent 42a463f422
commit c95a6fac69
Signed by: moritz
GPG key ID: 1020A035E5DD0824
2 changed files with 48 additions and 3 deletions

View file

@ -179,10 +179,11 @@ defmodule Mv.Authorization.Checks.HasPermission do
# Helper function to return a filter that never matches (deny all records)
# Used when authorization should be denied (fail-closed)
# Returns [id: {:in, []}] which means "id IN []" - never matches (correct deny-all)
# NOTE: [id: {:not, {:in, []}}] would be "NOT (id IN [])" = true for all IDs (allow-all) - WRONG!
#
# Using `expr(false)` avoids depending on the primary key being named `:id`.
# This is more robust than [id: {:in, []}] which assumes the primary key is `:id`.
defp deny_filter do
[id: {:in, []}]
expr(false)
end
# Helper to extract action type from authorizer

View file

@ -0,0 +1,44 @@
defmodule Mv.Authorization.Checks.HasPermissionFailClosedTest do
@moduledoc """
Regression tests to ensure deny-filter behavior is fail-closed (matches no records).
These tests verify that when HasPermission.auto_filter returns a deny-filter
(e.g., when actor is nil or no permission is found), the filter actually
matches zero records in the database.
This prevents regressions like the previous bug where [id: {:not, {:in, []}}]
was used, which logically evaluates to "NOT (id IN [])" = true for all IDs,
effectively allowing all records instead of denying them.
"""
use Mv.DataCase, async: true
alias Mv.Authorization.Checks.HasPermission
import Mv.Fixtures
test "auto_filter deny-filter matches no records (regression for NOT IN [] allow-all bug)" do
# Arrange: create some members in DB
_m1 = member_fixture()
_m2 = member_fixture()
# Build a minimal authorizer with a stable action type (:read)
authorizer = %Ash.Policy.Authorizer{
resource: Mv.Membership.Member,
action: %{type: :read}
}
# Act: missing actor must yield a deny-all filter (fail-closed)
deny_filter = HasPermission.auto_filter(nil, authorizer, [])
# Apply the returned filter to a real DB query (no authorization involved)
query =
Mv.Membership.Member
|> Ash.Query.new()
|> Ash.Query.filter_input(deny_filter)
{:ok, results} = Ash.read(query, domain: Mv.Membership, authorize?: false)
# Assert: deny-filter must match nothing
assert results == []
end
end