fix: fully expand calculation and aggregate references for applying authorization

lib/ash/actions/read/read.ex

@@ -177,6 +177,15 @@ defmodule Ash.Actions.Read do
     {calculations_in_query, calculations_at_runtime, query} =
       Ash.Actions.Read.Calculations.split_and_load_calculations(query.api, query, missing_pkeys?)
 
+    query =
+      add_calc_context_to_query(
+        query,
+        opts[:actor],
+        opts[:authorize?],
+        query.tenant,
+        opts[:tracer]
+      )
+
     query =
       if opts[:initial_data] do
         select = source_fields(query) ++ (query.select || [])
@@ -433,20 +442,9 @@ defmodule Ash.Actions.Read do
   defp agg_refs(query, calculations_in_query) do
     calculations_in_query
     |> Enum.flat_map(fn {_, expr} ->
-      Ash.Filter.used_aggregates(expr, :all, true)
+      Ash.Filter.used_aggregates(expr, :all)
     end)
-    |> Enum.concat(
+    |> Enum.concat(Map.values(query.aggregates))
-      Enum.map(query.aggregates, fn {_, aggregate} ->
-        %Ash.Query.Ref{
-          attribute: aggregate,
-          relationship_path: [],
-          resource: query.resource,
-          input?: false
-        }
-      end)
-    )
-    |> Enum.uniq_by(&{&1.relationship_path, &1.attribute, !!&1.input?})
-    |> Enum.map(&{&1.relationship_path, &1.attribute})
   end
 
   defp source_fields(query) do
@@ -1063,6 +1061,10 @@ defmodule Ash.Actions.Read do
         raise Ash.Error.Framework.AssumptionFailed,
           message: "unhandled calculation in filter statement #{inspect(ref)}"
 
+      %Ash.Query.Ref{attribute: %Ash.Resource.Aggregate{}} = ref ->
+        raise Ash.Error.Framework.AssumptionFailed,
+          message: "unhandled calculation in filter statement #{inspect(ref)}"
+
       %Ash.Query.Ref{
         attribute: %Ash.Query.Calculation{} = calc,
         relationship_path: relationship_path
@@ -1454,7 +1456,7 @@ defmodule Ash.Actions.Read do
   @doc false
   def update_aggregate_filters(
         filter,
-        resource,
+        _resource,
         authorize?,
         relationship_path_filters,
         actor,
@@ -1462,44 +1464,16 @@ defmodule Ash.Actions.Read do
         tracer
       ) do
     if authorize? do
-      Filter.update_aggregates(filter, fn aggregate, ref ->
+      Filter.update_aggregates(filter, fn aggregate, _ref ->
         if aggregate.authorize? do
-          case Map.fetch(
+          authorize_aggregate(
-                 relationship_path_filters,
+            aggregate,
-                 {ref.relationship_path ++ aggregate.relationship_path,
+            relationship_path_filters,
-                  aggregate.query.action.name}
+            actor,
-               ) do
+            authorize?,
-            {:ok, authorization_filter} ->
+            tenant,
-              %{
+            tracer
-                aggregate
+          )
-                | query: Ash.Query.do_filter(aggregate.query, authorization_filter),
-                  join_filters:
-                    add_join_filters(
-                      aggregate.join_filters,
-                      aggregate.relationship_path,
-                      ref.resource ||
-                        Ash.Resource.Info.related(resource, ref.relationship_path),
-                      relationship_path_filters,
-                      ref.relationship_path
-                    )
-              }
-              |> add_calc_context(actor, true, tenant, tracer)
-
-            _ ->
-              %{
-                aggregate
-                | join_filters:
-                    add_join_filters(
-                      aggregate.join_filters,
-                      aggregate.relationship_path,
-                      ref.resource ||
-                        Ash.Resource.Info.related(resource, ref.relationship_path),
-                      relationship_path_filters,
-                      ref.relationship_path
-                    )
-              }
-              |> add_calc_context(actor, false, tenant, tracer)
-          end
         else
           aggregate
         end
@@ -1526,7 +1500,9 @@ defmodule Ash.Actions.Read do
         |> Ash.Resource.Info.primary_action!(:read)
         |> Map.get(:name)
 
-      case Map.fetch(path_filters, {prefix ++ path, action}) do
+      last_relationship = last_relationship(resource, prefix ++ path)
+
+      case Map.fetch(path_filters, {last_relationship.source, last_relationship.name, action}) do
         {:ok, filter} ->
           Map.update(current_join_filters, path, filter, fn current_filter ->
             Ash.Query.BooleanExpression.new(:and, current_filter, filter)
@@ -2111,32 +2087,7 @@ defmodule Ash.Actions.Read do
 
       aggregate =
         if authorize? && aggregate.authorize? do
-          case Map.fetch(path_filters, {aggregate.relationship_path, aggregate.query.action.name}) do
+          authorize_aggregate(aggregate, path_filters, actor, authorize?, tenant, tracer)
-            {:ok, filter} ->
-              %{
-                aggregate
-                | query: Ash.Query.do_filter(aggregate.query, filter),
-                  join_filters:
-                    add_join_filters(
-                      aggregate.join_filters,
-                      aggregate.relationship_path,
-                      query.resource,
-                      path_filters
-                    )
-              }
-
-            :error ->
-              %{
-                aggregate
-                | join_filters:
-                    add_join_filters(
-                      aggregate.join_filters,
-                      aggregate.relationship_path,
-                      query.resource,
-                      path_filters
-                    )
-              }
-          end
         else
           aggregate
         end
@@ -2145,6 +2096,243 @@ defmodule Ash.Actions.Read do
     end)
   end
 
+  defp authorize_aggregate(aggregate, path_filters, actor, authorize?, tenant, tracer) do
+    aggregate = add_calc_context(aggregate, actor, authorize?, tenant, tracer)
+    last_relationship = last_relationship(aggregate.resource, aggregate.relationship_path)
+
+    additional_filter =
+      case Map.fetch(
+             path_filters,
+             {last_relationship.source, last_relationship.name, aggregate.query.action.name}
+           ) do
+        :error ->
+          true
+
+        {:ok, filter} ->
+          filter
+      end
+
+    with {:ok, filter} <-
+           filter_with_related(aggregate.query, authorize?, path_filters),
+         filter =
+           update_aggregate_filters(
+             filter,
+             aggregate.query.resource,
+             authorize?,
+             path_filters,
+             actor,
+             tenant,
+             tracer
+           ),
+         {:ok, field} <-
+           aggregate_field_with_related_filters(
+             aggregate,
+             path_filters,
+             actor,
+             authorize?,
+             tenant,
+             tracer
+           ) do
+      %{
+        aggregate
+        | query: Ash.Query.filter(%{aggregate.query | filter: filter}, ^additional_filter),
+          field: field,
+          join_filters:
+            add_join_filters(
+              aggregate.join_filters,
+              aggregate.relationship_path,
+              aggregate.resource,
+              path_filters
+            )
+      }
+    else
+      {:error, error} ->
+        raise "Error processing aggregate authorization filter for #{inspect(aggregate)}: #{inspect(error)}"
+    end
+  end
+
+  defp aggregate_field_with_related_filters(
+         %{field: nil},
+         _path_filters,
+         _actor,
+         _authorize?,
+         _tenant,
+         _tracer
+       ),
+       do: {:ok, nil}
+
+  defp aggregate_field_with_related_filters(
+         %{field: %Ash.Query.Calculation{} = field} = agg,
+         path_filters,
+         actor,
+         authorize?,
+         tenant,
+         tracer
+       ) do
+    calc = add_calc_context(field, actor, authorize?, tenant, tracer)
+
+    related_resource = Ash.Resource.Info.related(agg.resource, agg.relationship_path)
+
+    if calc.module.has_expression?() do
+      expr =
+        case calc.module.expression(calc.opts, calc.context) do
+          %Ash.Query.Function.Type{} = expr ->
+            expr
+
+          expr ->
+            {:ok, expr} = Ash.Query.Function.Type.new([expr, calc.type, calc.constraints])
+            expr
+        end
+
+      {:ok, expr} =
+        Ash.Filter.hydrate_refs(
+          expr,
+          %{
+            resource: related_resource,
+            public?: false
+          }
+        )
+
+      expr =
+        add_calc_context_to_filter(
+          expr,
+          actor,
+          authorize?,
+          tenant,
+          tracer
+        )
+
+      case do_filter_with_related(related_resource, expr, path_filters, []) do
+        {:ok, expr} ->
+          {:ok, %{field | module: Ash.Resource.Calculation.Expression, opts: [expr: expr]}}
+
+        {:error, error} ->
+          {:error, error}
+      end
+    else
+      {:ok, calc}
+    end
+  end
+
+  defp aggregate_field_with_related_filters(
+         %{field: %Ash.Query.Aggregate{} = field} = agg,
+         path_filters,
+         actor,
+         authorize?,
+         tenant,
+         tracer
+       ) do
+    field = add_calc_context(field, actor, authorize?, tenant, tracer)
+
+    if authorize? && field.authorize? do
+      authorize_aggregate(field, path_filters, actor, authorize?, tenant, tracer)
+    else
+      {:ok, agg}
+    end
+  end
+
+  defp aggregate_field_with_related_filters(
+         aggregate,
+         path_filters,
+         actor,
+         authorize?,
+         tenant,
+         tracer
+       )
+       when is_atom(aggregate.field) do
+    related_resource = Ash.Resource.Info.related(aggregate.resource, aggregate.relationship_path)
+
+    case Ash.Resource.Info.field(related_resource, aggregate.field) do
+      %Ash.Resource.Calculation{} = resource_calculation ->
+        {module, opts} = resource_calculation.calculation
+
+        case Ash.Query.Calculation.new(
+               resource_calculation.name,
+               module,
+               opts,
+               {resource_calculation.type, resource_calculation.constraints},
+               %{},
+               resource_calculation.filterable?,
+               resource_calculation.load
+             ) do
+          {:ok, calculation} ->
+            aggregate_field_with_related_filters(
+              %{aggregate | field: calculation},
+              path_filters,
+              actor,
+              authorize?,
+              tenant,
+              tracer
+            )
+
+          {:error, error} ->
+            {:error, error}
+        end
+
+      %Ash.Resource.Aggregate{} = resource_aggregate ->
+        read_action =
+          resource_aggregate.read_action ||
+            Ash.Resource.Info.primary_action!(related_resource, :read).name
+
+        with %{valid?: true} = aggregate_query <-
+               Ash.Query.for_read(related_resource, read_action),
+             %{valid?: true} = aggregate_query <-
+               Ash.Query.Aggregate.build_query(aggregate_query,
+                 filter: aggregate.filter,
+                 sort: aggregate.sort
+               ),
+             {:ok, query_aggregate} <-
+               Ash.Query.Aggregate.new(
+                 related_resource,
+                 resource_aggregate.name,
+                 resource_aggregate.kind,
+                 path: resource_aggregate.relationship_path,
+                 query: aggregate_query,
+                 field: resource_aggregate.field,
+                 default: resource_aggregate.default,
+                 filterable?: resource_aggregate.filterable?,
+                 type: resource_aggregate.type,
+                 constraints: resource_aggregate.constraints,
+                 implementation: resource_aggregate.implementation,
+                 uniq?: resource_aggregate.uniq?,
+                 read_action: read_action,
+                 authorize?: resource_aggregate.authorize?,
+                 join_filters:
+                   Map.new(resource_aggregate.join_filters, &{&1.relationship_path, &1.filter})
+               ) do
+          aggregate_field_with_related_filters(
+            %{aggregate | field: query_aggregate},
+            path_filters,
+            actor,
+            authorize?,
+            tenant,
+            tracer
+          )
+        else
+          {:error, error} ->
+            {:error, error}
+
+          %{errors: errors} ->
+            {:error, errors}
+        end
+
+      _ ->
+        {:ok, aggregate.field}
+    end
+  end
+
+  defp aggregate_field_with_related_filters(
+         aggregate,
+         _path_filters,
+         _actor,
+         _authorize?,
+         _tenant,
+         _tracer
+       )
+       when is_atom(aggregate.field) do
+    {:ok, aggregate.field}
+  end
+
   defp filter_with_related(
          query,
          authorize?,
@@ -2214,7 +2402,10 @@ defmodule Ash.Actions.Read do
           last_relationship.read_action ||
             Ash.Resource.Info.primary_action!(last_relationship.destination, :read).name
 
-        case Map.get(path_filters, {path, read_action}) do
+        case Map.get(
+               path_filters,
+               {last_relationship.source, last_relationship.name, read_action}
+             ) do
           nil ->
             {:cont, {:ok, filter}}
 
@@ -2235,13 +2426,6 @@ defmodule Ash.Actions.Read do
          filter
          |> Ash.Filter.map(fn
            %Ash.Query.Exists{at_path: at_path, path: exists_path, expr: exists_expr} = exists ->
-             path_filters =
-               path_filters
-               |> Enum.filter(fn {{path, _}, _} ->
-                 List.starts_with?(path, prefix ++ at_path ++ exists_path)
-               end)
-               |> Map.new()
-
              {:ok, new_expr} =
                do_filter_with_related(
                  resource,
@@ -2261,6 +2445,13 @@ defmodule Ash.Actions.Read do
     end
   end
 
+  defp last_relationship(resource, list) do
+    path = :lists.droplast(list)
+    last = List.last(list)
+
+    Ash.Resource.Info.relationship(Ash.Resource.Info.related(resource, path), last)
+  end
+
   defp set_phase(query, phase \\ :preparing)
        when phase in ~w[preparing before_action after_action executing around_transaction]a,
        do: %{query | phase: phase}

lib/ash/api/api.ex

@@ -1263,24 +1263,30 @@ defmodule Ash.Api do
   end
 
   defp apply_filter(query, resource, api, filter, authorizer, authorizer_state, opts) do
-    case opts[:filter_with] || :filter do
+    case Ash.Filter.hydrate_refs(filter, %{resource: resource, public?: false}) do
-      :filter ->
+      {:ok, filter} ->
-        Ash.Query.filter(or_query(query, resource, api), ^filter)
+        case opts[:filter_with] || :filter do
+          :filter ->
+            Ash.Query.filter(or_query(query, resource, api), ^filter)
 
-      :error ->
+          :error ->
-        Ash.Query.filter(
+            Ash.Query.filter(
-          or_query(query, resource, api),
+              or_query(query, resource, api),
-          if ^filter do
+              if ^filter do
-            true
+                true
-          else
+              else
-            error(Ash.Error.Forbidden.Placeholder, %{
+                error(Ash.Error.Forbidden.Placeholder, %{
-              authorizer: ^inspect(authorizer)
+                  authorizer: ^inspect(authorizer)
+                })
+              end
+            )
+            |> Ash.Query.set_context(%{
+              private: %{authorizer_state: %{authorizer => authorizer_state}}
             })
-          end
+        end
-        )
+
-        |> Ash.Query.set_context(%{
+      {:error, error} ->
-          private: %{authorizer_state: %{authorizer => authorizer_state}}
+        raise "Error building authorization filter: #{inspect(filter)}: #{inspect(error)}"
-        })
     end
   end
 

lib/ash/data_layer/ets/ets.ex

@@ -612,6 +612,7 @@ defmodule Ash.DataLayer.Ets do
 
   def do_add_aggregates(records, api, _resource, aggregates) do
     # TODO support crossing apis by getting the destination api, and set destination query context.
+
     Enum.reduce_while(records, {:ok, []}, fn record, {:ok, records} ->
       aggregates
       |> Enum.reduce_while(
@@ -648,7 +649,8 @@ defmodule Ash.DataLayer.Ets do
                      [record],
                      api
                    ),
-                 {:ok, filtered} <- filter_matches(related, query.filter, api),
+                 {:ok, filtered} <-
+                   filter_matches(related, query.filter, api),
                  sorted <- Sort.runtime_sort(filtered, query.sort, api: api) do
               field = field || Enum.at(Ash.Resource.Info.primary_key(query.resource), 0)
 
@@ -697,12 +699,12 @@ defmodule Ash.DataLayer.Ets do
       :count ->
         if uniq? do
           records
-          |> Stream.map(&Map.get(&1, field))
+          |> Stream.map(&field_value(&1, field))
           |> Stream.uniq()
           |> Stream.reject(&is_nil/1)
           |> Enum.count()
         else
-          Enum.count(records, &(not is_nil(Map.get(&1, field))))
+          Enum.count(records, &(not is_nil(field_value(&1, field))))
         end
 
       :exists ->
@@ -720,13 +722,13 @@ defmodule Ash.DataLayer.Ets do
             default
 
           [record | _rest] ->
-            Map.get(record, field)
+            field_value(record, field)
         end
 
       :list ->
         records
         |> Enum.map(fn record ->
-          Map.get(record, field)
+          field_value(record, field)
         end)
         |> then(fn values ->
           if uniq? do
@@ -741,11 +743,11 @@ defmodule Ash.DataLayer.Ets do
         |> then(fn records ->
           if uniq? do
             records
-            |> Stream.map(&Map.get(&1, field))
+            |> Stream.map(&field_value(&1, field))
             |> Stream.uniq()
           else
             records
-            |> Stream.map(&Map.get(&1, field))
+            |> Stream.map(&field_value(&1, field))
           end
         end)
         |> Enum.reduce({nil, 0}, fn value, {sum, count} ->
@@ -782,7 +784,7 @@ defmodule Ash.DataLayer.Ets do
 
       kind when kind in [:sum, :max, :min] ->
         records
-        |> Enum.map(&Map.get(&1, field))
+        |> Enum.map(&field_value(&1, field))
         |> case do
           [] ->
             nil
@@ -823,6 +825,31 @@ defmodule Ash.DataLayer.Ets do
     end
   end
 
+  defp field_value(nil, _), do: nil
+
+  defp field_value(record, field) when is_atom(field) do
+    Map.get(record, field)
+  end
+
+  defp field_value(record, %struct{load: load, name: name})
+       when struct in [Ash.Query.Aggregate, Ash.Query.Calculation] do
+    if load do
+      Map.get(record, load)
+    else
+      case struct do
+        Ash.Query.Aggregate ->
+          Map.get(record.aggregates, name)
+
+        Ash.Query.Calculation ->
+          Map.get(record.calculations, name)
+      end
+    end
+  end
+
+  defp field_value(record, %{name: name}) do
+    Map.get(record, name)
+  end
+
   defp get_records(resource, tenant) do
     with {:ok, table} <- wrap_or_create_table(resource, tenant),
          {:ok, record_tuples} <- ETS.Set.to_list(table),

lib/ash/filter/filter.ex

@@ -1016,6 +1016,7 @@ defmodule Ash.Filter do
         _ref ->
           false
       end)
+      |> expand_aggregates()
 
     if return_refs? do
       refs
@@ -1025,6 +1026,17 @@ defmodule Ash.Filter do
     |> Enum.uniq()
   end
 
+  defp expand_aggregates(aggregates) do
+    aggregates
+    |> Enum.flat_map(fn
+      %{field: %Ash.Query.Aggregate{} = inner_aggregate} = aggregate ->
+        [aggregate, inner_aggregate | expand_aggregates(aggregate)]
+
+      other ->
+        [other]
+    end)
+  end
+
   def put_at_path(value, []), do: value
   def put_at_path(value, [key | rest]), do: [{key, put_at_path(value, rest)}]
 
@@ -1111,14 +1123,21 @@ defmodule Ash.Filter do
   end
 
   @doc false
-  def relationship_filters(api, query, actor, tenant, aggregates, authorize?) do
+  def relationship_filters(
+        api,
+        query,
+        actor,
+        tenant,
+        aggregates,
+        authorize?,
+        filters \\ %{}
+      ) do
     if authorize? do
       paths_with_refs =
         query.filter
-        |> relationship_paths(true, true)
+        |> relationship_paths(true, true, true)
         |> Enum.map(fn {path, refs} ->
-          refs = Enum.filter(refs, &(&1 && &1.input?))
+          refs = Enum.filter(refs, & &1.input?)
-
           {path, refs}
         end)
         |> Enum.reject(fn {path, refs} -> path == [] || refs == [] end)
@@ -1128,10 +1147,20 @@ defmodule Ash.Filter do
 
       paths_with_refs
       |> Enum.map(&elem(&1, 0))
-      |> Enum.reduce_while({:ok, %{}}, fn path, {:ok, filters} ->
+      |> Enum.reduce_while({:ok, filters}, fn path, {:ok, filters} ->
-        add_authorization_path_filter(filters, path, api, query, actor, tenant, refs)
+        last_relationship = last_relationship(query.resource, path)
+        add_authorization_path_filter(filters, last_relationship, api, query, actor, tenant, refs)
       end)
-      |> add_aggregate_path_authorization(api, refs, aggregates, query, actor, tenant, refs)
+      |> add_aggregate_path_authorization(
+        api,
+        refs,
+        aggregates,
+        query,
+        actor,
+        tenant,
+        refs,
+        authorize?
+      )
     else
       {:ok, %{}}
     end
@@ -1139,27 +1168,18 @@ defmodule Ash.Filter do
 
   defp add_authorization_path_filter(
          filters,
-         path,
+         last_relationship,
          api,
-         query,
+         _query,
          actor,
          tenant,
-         refs,
+         _refs,
          base_related_query \\ nil,
-         aggregate? \\ false
+         _aggregate? \\ false
        ) do
-    last_relationship =
+    case relationship_query(last_relationship, actor, tenant, base_related_query) do
-      Enum.reduce(path, nil, fn
-        relationship, nil ->
-          Ash.Resource.Info.relationship(query.resource, relationship)
-
-        relationship, acc ->
-          Ash.Resource.Info.relationship(acc.destination, relationship)
-      end)
-
-    case relationship_query(query.resource, path, actor, tenant, base_related_query) do
       %{errors: []} = related_query ->
-        if filters[{path, related_query.action.name}] do
+        if filters[{last_relationship.source, last_relationship.name, related_query.action.name}] do
           {:cont, {:ok, filters}}
         else
           related_query
@@ -1169,10 +1189,6 @@ defmodule Ash.Filter do
               name: last_relationship.name
             }
           })
-          |> Ash.Query.set_context(%{
-            filter_only?: !aggregate?,
-            filter_references: refs[path] || []
-          })
           |> Ash.Query.select([])
           |> api.can(actor,
             run_queries?: false,
@@ -1187,12 +1203,18 @@ defmodule Ash.Filter do
                {:ok,
                 Map.put(
                   filters,
-                  {path, related_query.action.name},
+                  {last_relationship.source, last_relationship.name, related_query.action.name},
                   authorized_related_query.filter
                 )}}
 
             {:ok, false, _error} ->
-              {:halt, {:ok, Map.put(filters, {path, related_query.action.name}, false)}}
+              {:halt,
+               {:ok,
+                Map.put(
+                  filters,
+                  {last_relationship.source, last_relationship.name, related_query.action.name},
+                  false
+                )}}
 
             {:error, error} ->
               {:halt, {:error, error}}
@@ -1212,10 +1234,11 @@ defmodule Ash.Filter do
          query,
          actor,
          tenant,
-         refs
+         refs,
+         authorize?
        ) do
     refs
-    |> Enum.flat_map(fn {path, refs} ->
+    |> Enum.flat_map(fn {_path, refs} ->
       refs
       |> Enum.filter(
         &match?(
@@ -1223,43 +1246,60 @@ defmodule Ash.Filter do
           &1
         )
       )
-      |> Enum.map(fn ref ->
+      |> Enum.map(& &1.attribute)
-        {path, ref.attribute}
-      end)
     end)
     |> Enum.concat(aggregates)
-    |> Enum.reduce_while({:ok, path_filters}, fn {path, aggregate}, {:ok, filters} ->
+    |> Enum.reduce_while({:ok, path_filters}, fn aggregate, {:ok, filters} ->
       aggregate.relationship_path
       |> :lists.droplast()
       |> Ash.Query.Aggregate.subpaths()
       |> Enum.reduce_while({:ok, filters}, fn subpath, {:ok, filters} ->
-        related = Ash.Resource.Info.related(query.resource, subpath)
+        last_relationship = last_relationship(query.resource, subpath)
 
         add_authorization_path_filter(
           filters,
-          path ++ subpath,
+          last_relationship,
           api,
           query,
           actor,
           tenant,
           refs,
-          Ash.Query.for_read(related, Ash.Resource.Info.primary_action(related, :read).name),
+          Ash.Query.for_read(
+            last_relationship.destination,
+            Ash.Resource.Info.primary_action(last_relationship.destination, :read).name
+          ),
           true
         )
       end)
       |> case do
         {:ok, filters} ->
-          add_authorization_path_filter(
+          last_relationship = last_relationship(aggregate.resource, aggregate.relationship_path)
-            filters,
+
-            path ++ aggregate.relationship_path,
+          case relationship_filters(
-            api,
+                 api,
-            query,
+                 aggregate.query,
-            actor,
+                 actor,
-            tenant,
+                 tenant,
-            refs,
+                 [],
-            aggregate.query,
+                 authorize?,
-            true
+                 filters
-          )
+               ) do
+            {:ok, filters} ->
+              add_authorization_path_filter(
+                filters,
+                last_relationship,
+                api,
+                query,
+                actor,
+                tenant,
+                refs,
+                aggregate.query,
+                true
+              )
+
+            {:error, error} ->
+              {:error, error}
+          end
 
         {:error, error} ->
           {:error, error}
@@ -1267,8 +1307,7 @@ defmodule Ash.Filter do
     end)
   end
 
-  defp relationship_query(resource, [last], actor, tenant, base) do
+  defp relationship_query(relationship, actor, tenant, base) do
-    relationship = Ash.Resource.Info.relationship(resource, last)
     base_query = base || Ash.Query.new(relationship.destination)
 
     action =
@@ -1292,12 +1331,6 @@ defmodule Ash.Filter do
     end
   end
 
-  defp relationship_query(resource, [next | rest], actor, tenant, base) do
-    resource
-    |> Ash.Resource.Info.related(next)
-    |> relationship_query(rest, actor, tenant, base)
-  end
-
   defp group_refs_by_all_paths(paths_with_refs) do
     all_paths_with_refs =
       paths_with_refs
@@ -1917,21 +1950,32 @@ defmodule Ash.Filter do
     end
   end
 
-  def relationship_paths(filter_or_expression, include_exists? \\ false, with_reference? \\ false)
+  def relationship_paths(
-  def relationship_paths(nil, _, _), do: []
+        filter_or_expression,
-  def relationship_paths(%{expression: nil}, _, _), do: []
+        include_exists? \\ false,
+        with_refs? \\ false,
+        expand_aggregates? \\ false
+      )
 
-  def relationship_paths(%__MODULE__{expression: expression}, include_exists?, with_reference?),
+  def relationship_paths(nil, _, _, _), do: []
-    do: relationship_paths(expression, include_exists?, with_reference?)
+  def relationship_paths(%__MODULE__{expression: nil}, _, _, _), do: []
 
-  def relationship_paths(expression, include_exists?, with_reference?) do
+  def relationship_paths(
+        %__MODULE__{expression: expression},
+        include_exists?,
+        with_refs?,
+        expand_aggregates?
+      ),
+      do: relationship_paths(expression, include_exists?, with_refs?, expand_aggregates?)
+
+  def relationship_paths(expression, include_exists?, with_refs?, expand_aggregates?) do
     paths =
       expression
-      |> do_relationship_paths(include_exists?, with_reference?)
+      |> do_relationship_paths(include_exists?, with_refs?, expand_aggregates?)
       |> List.wrap()
       |> List.flatten()
 
-    if with_reference? do
+    if with_refs? do
       paths
       |> Enum.group_by(&elem(&1, 0), &elem(&1, 1))
       |> Map.new(fn {key, values} ->
@@ -1944,6 +1988,29 @@ defmodule Ash.Filter do
     end
   end
 
+  defp do_relationship_paths(
+         %Ref{
+           relationship_path: path,
+           resource: resource,
+           attribute: %Ash.Query.Aggregate{field: field, relationship_path: agg_path}
+         },
+         include_exists?,
+         with_references?,
+         true
+       ) do
+    case field do
+      nil ->
+        []
+
+      field when is_atom(field) ->
+        []
+
+      field ->
+        %Ref{relationship_path: path ++ agg_path, resource: resource, attribute: field}
+        |> do_relationship_paths(include_exists?, with_references?, true)
+    end
+  end
+
   defp do_relationship_paths(
          %Ref{
            relationship_path: path,
@@ -1951,7 +2018,8 @@ defmodule Ash.Filter do
            attribute: %Ash.Query.Calculation{module: module, opts: opts, context: context}
          } = ref,
          include_exists?,
-         with_references?
+         with_references?,
+         expand_aggregates?
        ) do
     if module.has_expression?() do
       expression = module.expression(opts, context)
@@ -1971,7 +2039,7 @@ defmodule Ash.Filter do
 
           nested =
             expression
-            |> do_relationship_paths(include_exists?, with_references?)
+            |> do_relationship_paths(include_exists?, with_references?, expand_aggregates?)
             |> List.wrap()
             |> List.flatten()
 
@@ -2030,29 +2098,56 @@ defmodule Ash.Filter do
     end
   end
 
-  defp do_relationship_paths(%Ref{relationship_path: path} = ref, _, true) do
+  defp do_relationship_paths(
+         %Ref{relationship_path: path, attribute: %Ash.Query.Aggregate{} = aggregate} = ref,
+         include_exists?,
+         with_refs?,
+         true
+       ) do
+    this_agg_ref =
+      if with_refs? do
+        {path, ref}
+      else
+        {path}
+      end
+
+    [this_agg_ref | aggregate_refs(path, aggregate, include_exists?, with_refs?)]
+  end
+
+  defp do_relationship_paths(%Ref{relationship_path: path} = ref, _, true, _) do
     [{path, ref}]
   end
 
-  defp do_relationship_paths(%Ref{relationship_path: path}, _, false) do
+  defp do_relationship_paths(%Ref{relationship_path: path}, _, false, _) do
     [{path}]
   end
 
   defp do_relationship_paths(
          %BooleanExpression{left: left, right: right},
          include_exists?,
-         with_reference?
+         with_refs?,
+         expand_aggregates?
        ) do
-    do_relationship_paths(left, include_exists?, with_reference?) ++
+    do_relationship_paths(left, include_exists?, with_refs?, expand_aggregates?) ++
-      do_relationship_paths(right, include_exists?, with_reference?)
+      do_relationship_paths(right, include_exists?, with_refs?, expand_aggregates?)
   end
 
-  defp do_relationship_paths(%Not{expression: expression}, include_exists?, with_reference?) do
+  defp do_relationship_paths(
-    do_relationship_paths(expression, include_exists?, with_reference?)
+         %Not{expression: expression},
+         include_exists?,
+         with_refs?,
+         expand_aggregates?
+       ) do
+    do_relationship_paths(expression, include_exists?, with_refs?, expand_aggregates?)
   end
 
-  defp do_relationship_paths(%Ash.Query.Exists{at_path: at_path}, false, with_reference?) do
+  defp do_relationship_paths(
-    if with_reference? do
+         %Ash.Query.Exists{at_path: at_path},
+         false,
+         with_refs?,
+         _expand_aggregates?
+       ) do
+    if with_refs? do
       [{at_path, nil}]
     else
       [{at_path}]
@@ -2062,71 +2157,214 @@ defmodule Ash.Filter do
   defp do_relationship_paths(
          %Ash.Query.Exists{path: path, expr: expression, at_path: at_path},
          include_exists?,
-         false
+         false,
+         expand_aggregates?
        ) do
     expression
-    |> do_relationship_paths(include_exists?, false)
+    |> do_relationship_paths(include_exists?, false, expand_aggregates?)
     |> List.flatten()
     |> Enum.flat_map(fn {rel_path} ->
       [{at_path}, {at_path ++ path ++ rel_path}]
     end)
-    |> Kernel.++(parent_relationship_paths(expression, at_path, include_exists?, false))
+    |> Kernel.++(
+      parent_relationship_paths(expression, at_path, include_exists?, false, expand_aggregates?)
+    )
   end
 
   defp do_relationship_paths(
          %Ash.Query.Exists{path: path, expr: expression, at_path: at_path},
          include_exists?,
-         true
+         true,
+         expand_aggregates?
        ) do
     expression
-    |> do_relationship_paths(include_exists?, true)
+    |> do_relationship_paths(include_exists?, true, expand_aggregates?)
     |> List.flatten()
     |> Enum.flat_map(fn {rel_path, ref} ->
       [{at_path, nil}, {at_path ++ path ++ rel_path, ref}]
     end)
-    |> Kernel.++(parent_relationship_paths(expression, at_path, include_exists?, true))
+    |> Kernel.++(
+      parent_relationship_paths(expression, at_path, include_exists?, true, expand_aggregates?)
+    )
   end
 
   defp do_relationship_paths(
          %{__operator__?: true, left: left, right: right},
          include_exists?,
-         with_reference?
+         with_refs?,
+         expand_aggregates?
        ) do
-    Enum.flat_map([left, right], &do_relationship_paths(&1, include_exists?, with_reference?))
+    Enum.flat_map(
+      [left, right],
+      &do_relationship_paths(&1, include_exists?, with_refs?, expand_aggregates?)
+    )
   end
 
-  defp do_relationship_paths({key, value}, include_exists?, with_reference?) when is_atom(key) do
+  defp do_relationship_paths({key, value}, include_exists?, with_refs?, expand_aggregates?)
-    do_relationship_paths(value, include_exists?, with_reference?)
+       when is_atom(key) do
+    do_relationship_paths(value, include_exists?, with_refs?, expand_aggregates?)
   end
 
   defp do_relationship_paths(
          %{__function__?: true, arguments: arguments},
          include_exists?,
-         with_reference?
+         with_refs?,
+         expand_aggregates?
        ) do
-    Enum.flat_map(arguments, &do_relationship_paths(&1, include_exists?, with_reference?))
+    Enum.flat_map(
+      arguments,
+      &do_relationship_paths(&1, include_exists?, with_refs?, expand_aggregates?)
+    )
   end
 
-  defp do_relationship_paths(value, include_exists?, with_reference?) when is_list(value) do
+  defp do_relationship_paths(value, include_exists?, with_refs?, expand_aggregates?)
-    Enum.flat_map(value, &do_relationship_paths(&1, include_exists?, with_reference?))
+       when is_list(value) do
+    Enum.flat_map(
+      value,
+      &do_relationship_paths(&1, include_exists?, with_refs?, expand_aggregates?)
+    )
   end
 
-  defp do_relationship_paths(value, include_exists?, with_references?)
+  defp do_relationship_paths(value, include_exists?, with_references?, expand_aggregates?)
        when is_map(value) and not is_struct(value) do
     Enum.flat_map(value, fn {key, value} ->
-      do_relationship_paths(key, include_exists?, with_references?) ++
+      do_relationship_paths(key, include_exists?, with_references?, expand_aggregates?) ++
-        do_relationship_paths(value, include_exists?, with_references?)
+        do_relationship_paths(value, include_exists?, with_references?, expand_aggregates?)
     end)
   end
 
-  defp do_relationship_paths(_, _, _), do: []
+  defp do_relationship_paths(_, _, _, _), do: []
 
-  defp parent_relationship_paths(expression, at_path, include_exists?, with_reference?) do
+  defp aggregate_refs(path, aggregate, include_exists?, with_refs?) do
+    query_rel_paths =
+      if aggregate.query && aggregate.query.filter do
+        aggregate.query.filter
+        |> relationship_paths(include_exists?, with_refs?, true)
+      else
+        []
+      end
+
+    if aggregate.field do
+      related = Ash.Resource.Info.related(aggregate.resource, aggregate.relationship_path)
+
+      field_ref =
+        case aggregate.field do
+          field when is_atom(field) ->
+            Ash.Resource.Info.field(related, aggregate.field)
+
+          field ->
+            field
+        end
+
+      field_ref = field_to_ref(aggregate.resource, field_ref)
+
+      query_rel_paths ++ do_relationship_paths(field_ref, include_exists?, with_refs?, true)
+    else
+      query_rel_paths
+    end
+    |> Enum.map(fn
+      {agg_path} ->
+        {path ++ aggregate.relationship_path ++ agg_path}
+
+      {agg_path, ref} ->
+        {path ++ aggregate.relationship_path ++ agg_path,
+         %{
+           ref
+           | relationship_path: path ++ aggregate.relationship_path ++ ref.relationship_path,
+             input?: true
+         }}
+    end)
+  end
+
+  defp field_to_ref(resource, %Ash.Resource.Attribute{} = attr) do
+    %Ref{
+      resource: resource,
+      attribute: attr,
+      relationship_path: []
+    }
+  end
+
+  defp field_to_ref(resource, %Ash.Resource.Aggregate{} = aggregate) do
+    related = Ash.Resource.Info.related(resource, aggregate.relationship_path)
+
+    read_action =
+      aggregate.read_action || Ash.Resource.Info.primary_action!(related, :read).name
+
+    with %{valid?: true} = aggregate_query <- Ash.Query.for_read(related, read_action),
+         %{valid?: true} = aggregate_query <-
+           Ash.Query.Aggregate.build_query(aggregate_query,
+             filter: aggregate.filter,
+             sort: aggregate.sort
+           ) do
+      case Aggregate.new(
+             resource,
+             aggregate.name,
+             aggregate.kind,
+             path: aggregate.relationship_path,
+             query: aggregate_query,
+             field: aggregate.field,
+             default: aggregate.default,
+             filterable?: aggregate.filterable?,
+             type: aggregate.type,
+             constraints: aggregate.constraints,
+             implementation: aggregate.implementation,
+             uniq?: aggregate.uniq?,
+             read_action: read_action,
+             authorize?: aggregate.authorize?,
+             join_filters: Map.new(aggregate.join_filters, &{&1.relationship_path, &1.filter})
+           ) do
+        {:ok, query_aggregate} ->
+          field_to_ref(resource, query_aggregate)
+
+        {:error, error} ->
+          raise "Could not construct aggregate #{inspect(aggregate)}: #{inspect(error)}"
+      end
+    else
+      %{errors: errors} ->
+        raise "Could not construct aggregate #{inspect(aggregate)}: #{inspect(errors)}"
+    end
+  end
+
+  defp field_to_ref(resource, %Ash.Resource.Calculation{} = calc) do
+    {module, opts} = calc.calculation
+
+    case Calculation.new(
+           calc.name,
+           module,
+           opts,
+           {calc.type, calc.constraints},
+           %{},
+           calc.filterable?,
+           calc.load
+         ) do
+      {:ok, calc} ->
+        field_to_ref(resource, calc)
+
+      {:error, error} ->
+        raise "Could not construct calculation #{inspect(calc)}: #{inspect(error)}"
+    end
+  end
+
+  defp field_to_ref(resource, field) do
+    %Ref{
+      resource: resource,
+      attribute: field,
+      relationship_path: []
+    }
+  end
+
+  defp parent_relationship_paths(
+         expression,
+         at_path,
+         include_exists?,
+         with_refs?,
+         expand_aggregates?
+       ) do
     expression
     |> flat_map(fn
       %Ash.Query.Parent{expr: expr} ->
         expr
-        |> do_relationship_paths(include_exists?, with_reference?)
+        |> do_relationship_paths(include_exists?, with_refs?, expand_aggregates?)
         |> Enum.flat_map(fn
           {rel_path, ref} ->
             [{at_path ++ rel_path, ref}]
@@ -2682,12 +2920,7 @@ defmodule Ash.Filter do
                  constraints: aggregate.constraints,
                  implementation: aggregate.implementation,
                  uniq?: aggregate.uniq?,
-                 read_action:
+                 read_action: read_action,
-                   aggregate.read_action ||
-                     Ash.Resource.Info.primary_action!(
-                       Ash.Resource.Info.related(context.resource, aggregate.relationship_path),
-                       :read
-                     ).name,
                  authorize?: aggregate.authorize?,
                  join_filters: Map.new(aggregate.join_filters, &{&1.relationship_path, &1.filter})
                ) do
@@ -3907,4 +4140,11 @@ defmodule Ash.Filter do
       false
     end
   end
+
+  defp last_relationship(resource, list) do
+    path = :lists.droplast(list)
+    last = List.last(list)
+
+    Ash.Resource.Info.relationship(Ash.Resource.Info.related(resource, path), last)
+  end
 end

lib/ash/policy/check/built_in_checks.ex

@@ -96,6 +96,15 @@ defmodule Ash.Policy.Check.Builtins do
   ```
   """
   @spec filtering_on(atom | list(atom), atom) :: Ash.Policy.Check.ref()
+  @deprecated """
+  `filtering_on/2` check is deprecated. Instead, add arguments and add policies that said arguments are set.
+
+  For complex queries, policies on what is being filtered on require multiple authorization passes of
+  the same resource, leading to a large amount of typically unnecessary complexity.
+
+  Additionally, they could yield false negatives in some scenarios, and more work would be needed
+  to ensure that they don't.
+  """
   def filtering_on(path \\ [], field) do
     {Ash.Policy.Check.FilteringOn, path: List.wrap(path), field: field}
   end

lib/ash/policy/check/filtering_on.ex

@@ -11,23 +11,6 @@ defmodule Ash.Policy.Check.FilteringOn do
   def requires_original_data?(_, _), do: false
 
   @impl true
-  def match?(
-        _actor,
-        %{
-          query: %Ash.Query{context: %{filter_only?: true, filter_references: references}}
-        },
-        opts
-      ) do
-    path = opts[:path] || []
-    field = opts[:field] || raise "Must provide field to #{inspect(__MODULE__)}"
-
-    references
-    |> Enum.filter(&(&1.relationship_path == path))
-    |> Enum.any?(fn ref ->
-      Ash.Query.Ref.name(ref) == field
-    end)
-  end
-
   def match?(_actor, %{query: %Ash.Query{} = query}, opts) do
     path = opts[:path] || []
     field = opts[:field] || raise "Must provide field to #{inspect(__MODULE__)}"

lib/ash/query/aggregate.ex

@@ -470,7 +470,20 @@ defmodule Ash.Query.Aggregate do
     def inspect(%{query: query} = aggregate, opts) do
       field =
         if aggregate.field do
-          [aggregate.field]
+          if is_atom(aggregate.field) do
+            [to_string(aggregate.field)]
+          else
+            case aggregate.field do
+              %{agg_name: agg_name} ->
+                [to_string(agg_name)]
+
+              %{calc_name: calc_name} ->
+                [to_string(calc_name)]
+
+              _ ->
+                [inspect(aggregate.field)]
+            end
+          end
         else
           []
         end

test/policy/complex_test.exs

@@ -129,6 +129,8 @@ defmodule Ash.Test.Policy.ComplexTest do
       |> Api.read_one!()
       |> Map.get(:count_of_commenters)
 
+    assert count_of_commenters_without_authorization == 3
+
     count_of_commenters_with_authorization =
       Post
       |> Ash.Query.load(:count_of_commenters)
@@ -137,7 +139,6 @@ defmodule Ash.Test.Policy.ComplexTest do
       |> Map.get(:count_of_commenters)
 
     assert count_of_commenters_with_authorization == 2
-    assert count_of_commenters_without_authorization == 3
   end
 
   test "aggregates in calculations are authorized", %{me: me} do