diff --git a/lib/ash/policy/authorizer/authorizer.ex b/lib/ash/policy/authorizer/authorizer.ex index 7f328252..5e2407b8 100644 --- a/lib/ash/policy/authorizer/authorizer.ex +++ b/lib/ash/policy/authorizer/authorizer.ex @@ -772,41 +772,46 @@ defmodule Ash.Policy.Authorizer do %{acc | authorizers: Map.put(acc.authorizers, {resource, action}, authorizer)}} end - policies = Ash.Policy.Info.field_policies_for_field(resource, field) - {expr, authorizer} = - case strict_check_result( - %{ - authorizer - | policies: policies - }, - for_fields: [field], - context_description: "accessing field in filter" - ) do - {:authorized, authorizer} -> - {true, authorizer} + if field in Ash.Resource.Info.primary_key(resource) do + # primary keys are always accessible + {true, authorizer} + else + policies = Ash.Policy.Info.field_policies_for_field(resource, field) - {:error, _} -> - {false, authorizer} + case strict_check_result( + %{ + authorizer + | policies: policies + }, + for_fields: [field], + context_description: "accessing field in filter" + ) do + {:authorized, authorizer} -> + {true, authorizer} - {:filter, authorizer, filter} -> - {filter, authorizer} + {:error, _} -> + {false, authorizer} - {:filter_and_continue, filter, _authorizer} -> - raise """ - Was given a partial filter for a field policy for field #{inspect(field)}. + {:filter, authorizer, filter} -> + {filter, authorizer} - Filter: #{inspect(filter)} + {:filter_and_continue, filter, _authorizer} -> + raise """ + Was given a partial filter for a field policy for field #{inspect(field)}. - Field policies must currently use only filter checks or simple checks. - """ + Filter: #{inspect(filter)} - {:continue, _} -> - raise """ - Detected necessity for a runtime check for a field policy for field #{inspect(field)}. + Field policies must currently use only filter checks or simple checks. + """ - Field policies must currently use only filter checks or simple checks. - """ + {:continue, _} -> + raise """ + Detected necessity for a runtime check for a field policy for field #{inspect(field)}. + + Field policies must currently use only filter checks or simple checks. + """ + end end new_acc = %{acc | authorizers: Map.put(acc.authorizers, {resource, action}, authorizer)} @@ -821,92 +826,100 @@ defmodule Ash.Policy.Authorizer do @impl true def add_calculations(query_or_changeset, authorizer, _context) do - accessing_fields = - case query_or_changeset do - %Ash.Query{} = query -> - Ash.Query.accessing(query, [:attributes, :calculations, :aggregates]) - - %Ash.Changeset{} = changeset -> - Ash.Changeset.accessing(changeset, [:attributes, :calculations, :aggregates]) - end - - accessing_fields - |> Enum.group_by(fn field -> - Ash.Policy.Info.field_policies_for_field(query_or_changeset.resource, field) - end) - # primary key doesn't have policies on it, and so is nil here - |> Map.drop([nil, []]) - |> Enum.reduce( - {query_or_changeset, authorizer}, - fn {policies, fields}, {query_or_changeset, authorizer} -> - {expr, authorizer} = - case strict_check_result( - %{ - authorizer - | policies: policies - } - |> add_query_or_changeset(query_or_changeset), - for_fields: fields, - context_description: "selecting or loading fields" - ) do - {:authorized, authorizer} -> - {true, authorizer} - - {:error, _} -> - {false, authorizer} - - {:filter, authorizer, filter} -> - {filter, authorizer} - - {:filter_and_continue, filter, _authorizer} -> - raise """ - Was given a partial filter for a field policy for fields #{inspect(fields)}. - - Filter: #{inspect(filter)} - - Field policies must currently use only filter checks or simple checks. - """ - - {:continue, _} -> - raise """ - Detected necessity for a runtime check for a field policy for fields #{inspect(fields)}. - - Field policies must currently use only filter checks or simple checks. - """ - end - - # This is a hack that we need to clean up. - # Creating this kind of expression should be its own thing that we do - # with something in the `Expr` module - - %{expression: expr} = Ash.Filter.parse!(query_or_changeset.resource, expr) - - {:ok, calculation} = - Ash.Query.Calculation.new( - {:__ash_fields_are_visible__, fields}, - Ash.Resource.Calculation.Expression, - [expr: expr], - :boolean - ) - + if Ash.Policy.Info.field_policies(query_or_changeset.resource) == [] do + # If there are no field policies, access is allowed by default + # and we don't need to add any calculations + {:ok, query_or_changeset, authorizer} + else + accessing_fields = case query_or_changeset do %Ash.Query{} = query -> - {Ash.Query.load( - query, - calculation - ), authorizer} + Ash.Query.accessing(query, [:attributes, :calculations, :aggregates]) - %Ash.Changeset{} = query -> - {Ash.Changeset.load( - query, - calculation - ), authorizer} + %Ash.Changeset{} = changeset -> + Ash.Changeset.accessing(changeset, [:attributes, :calculations, :aggregates]) end - end - ) - |> then(fn {result, authorizer} -> - {:ok, result, authorizer} - end) + + pkey = Ash.Resource.Info.primary_key(query_or_changeset.resource) + + accessing_fields + # primary keys are always accessible + |> Enum.reject(&(&1 in pkey)) + |> Enum.group_by(fn field -> + Ash.Policy.Info.field_policies_for_field(query_or_changeset.resource, field) + end) + |> Enum.reduce( + {query_or_changeset, authorizer}, + fn {policies, fields}, {query_or_changeset, authorizer} -> + {expr, authorizer} = + case strict_check_result( + %{ + authorizer + | policies: policies + } + |> add_query_or_changeset(query_or_changeset), + for_fields: fields, + context_description: "selecting or loading fields" + ) do + {:authorized, authorizer} -> + {true, authorizer} + + {:error, _} -> + {false, authorizer} + + {:filter, authorizer, filter} -> + {filter, authorizer} + + {:filter_and_continue, filter, _authorizer} -> + raise """ + Was given a partial filter for a field policy for fields #{inspect(fields)}. + + Filter: #{inspect(filter)} + + Field policies must currently use only filter checks or simple checks. + """ + + {:continue, _} -> + raise """ + Detected necessity for a runtime check for a field policy for fields #{inspect(fields)}. + + Field policies must currently use only filter checks or simple checks. + """ + end + + # This is a hack that we need to clean up. + # Creating this kind of expression should be its own thing that we do + # with something in the `Expr` module + + %{expression: expr} = Ash.Filter.parse!(query_or_changeset.resource, expr) + + {:ok, calculation} = + Ash.Query.Calculation.new( + {:__ash_fields_are_visible__, fields}, + Ash.Resource.Calculation.Expression, + [expr: expr], + :boolean + ) + + case query_or_changeset do + %Ash.Query{} = query -> + {Ash.Query.load( + query, + calculation + ), authorizer} + + %Ash.Changeset{} = query -> + {Ash.Changeset.load( + query, + calculation + ), authorizer} + end + end + ) + |> then(fn {result, authorizer} -> + {:ok, result, authorizer} + end) + end end defp add_query_or_changeset(