feat: Allow field policies to hide private fields (#1289)

* Allow field policies to hide private fields

* Create option for how to handle private fields

* Improve docs

.formatter.exs

@@ -163,6 +163,7 @@ spark_locals_without_parens = [
   primary?: 1,
   primary_key?: 1,
   private?: 1,
+  private_fields: 1,
   public?: 1,
   publish: 2,
   publish: 3,

documentation/topics/security/policies.md

@@ -359,6 +359,30 @@ In results, forbidden fields will be replaced with a special value: `%Ash.Forbid
 
 When these fields are referred to in filters, they will be replaced with an expression that evaluates to `nil`. To support this behavior, only simple and filter checks are allowed in field policies.
 
+### Handeling private fields in internal functions
+
+When calling internal functions like `Ash.read!/1`, private fields will by default always be shown.
+Even if field policies applies to the resource. You can change the default behaviour by setting the
+`private_fields` option on field policies.
+
+```elixir
+field_policies do
+  private_fields :include
+end
+```
+
+The different options are:
+- `:show` will always show private fields
+- `:hide` will always hide private fields
+- `:include` will let you to write field policies for private fields and private fields
+  will be shown or hidden depending on the outcome of the policy
+
+If you want to overwrite the default option that is `:show`, you can do that by setting a global flag:
+
+```elixir
+config :ash, :policies, private_fields: :include
+```
+
 ## Debugging and Logging
 
 ### Policy Breakdowns

lib/ash/changeset/changeset.ex

@@ -538,12 +538,16 @@ defmodule Ash.Changeset do
 
   Provide a list of field types to narrow down the returned results.
   """
-  def accessing(changeset, types \\ [:attributes, :relationships, :calculations, :attributes]) do
+  def accessing(
+        changeset,
+        types \\ [:attributes, :relationships, :calculations, :attributes],
+        only_public? \\ true
+      ) do
     changeset.resource
     |> Ash.Query.new()
     |> Ash.Query.load(changeset.load)
     |> Map.put(:select, changeset.select)
-    |> Ash.Query.accessing(types)
+    |> Ash.Query.accessing(types, only_public?)
   end
 
   @spec fully_atomic_changeset(

lib/ash/policy/authorizer/authorizer.ex

@@ -356,6 +356,15 @@ defmodule Ash.Policy.Authorizer do
     entities: [
       @field_policy_bypass,
       @field_policy
+    ],
+    schema: [
+      private_fields: [
+        type: {:one_of, [:show, :hide, :include]},
+        default: Application.compile_env(:ash, :policies)[:private_fields] || :show,
+        doc: """
+        How private fields should be handeled by field policies in internal functions. See the [Policies guide](documentation/topics/security/policies.md#field-policies) for more.
+        """
+      ]
     ]
   }
 
@@ -895,13 +904,24 @@ defmodule Ash.Policy.Authorizer do
       # and we don't need to add any calculations
       {:ok, query_or_changeset, authorizer}
     else
+      only_public? =
+        case Ash.Policy.Info.private_fields_policy(query_or_changeset.resource) do
+          :include -> false
+          :show -> true
+          :hide -> false
+        end
+
       accessing_fields =
         case query_or_changeset do
           %Ash.Query{} = query ->
-            Ash.Query.accessing(query, [:attributes, :calculations, :aggregates])
+            Ash.Query.accessing(query, [:attributes, :calculations, :aggregates], only_public?)
 
           %Ash.Changeset{} = changeset ->
-            Ash.Changeset.accessing(changeset, [:attributes, :calculations, :aggregates])
+            Ash.Changeset.accessing(
+              changeset,
+              [:attributes, :calculations, :aggregates],
+              only_public?
+            )
         end
 
       pkey = Ash.Resource.Info.primary_key(query_or_changeset.resource)

lib/ash/policy/authorizer/transformers/add_missing_field_policies.ex

@@ -9,24 +9,31 @@ defmodule Ash.Policy.Authorizer.Transformers.AddMissingFieldPolicies do
   def after?(_), do: true
 
   def transform(dsl) do
-    non_pkey_fields =
-      dsl
-      |> Ash.Resource.Info.fields([:aggregates, :calculations, :attributes])
-      |> Enum.reject(fn
-        %{public?: false} ->
-          true
-
-        %{primary_key?: true} ->
-          true
-
-        _ ->
-          false
-      end)
-      |> Enum.map(& &1.name)
-
     if Enum.empty?(Ash.Policy.Info.field_policies(dsl)) do
       {:ok, dsl}
     else
+      exclude_private_fields? =
+        case Ash.Policy.Info.private_fields_policy(dsl) do
+          :include -> false
+          :show -> true
+          :hide -> true
+        end
+
+      non_pkey_fields =
+        dsl
+        |> Ash.Resource.Info.fields([:aggregates, :calculations, :attributes])
+        |> Enum.reject(fn
+          %{public?: false} when exclude_private_fields? ->
+            true
+
+          %{primary_key?: true} ->
+            true
+
+          _ ->
+            false
+        end)
+        |> Enum.map(& &1.name)
+
       dsl
       |> replace_asterisk(non_pkey_fields)
       |> ensure_field_coverage(non_pkey_fields)

lib/ash/policy/info.ex

@@ -143,6 +143,10 @@ defmodule Ash.Policy.Info do
     |> set_access_type(default_access_type(resource))
   end
 
+  def private_fields_policy(resource) do
+    Extension.get_opt(resource, [:field_policies], :private_fields)
+  end
+
   def policies(domain, resource) do
     if domain do
       do_policies(domain) ++ do_policies(resource)

test/policy/field_policy_test.exs

@@ -4,7 +4,7 @@ defmodule Ash.Test.Policy.FieldPolicyTest do
 
   require Ash.Query
 
-  alias Ash.Test.Support.PolicyField.{Ticket, User}
+  alias Ash.Test.Support.PolicyField.{Post, Ticket, User}
 
   setup do
     rep =
@@ -44,6 +44,13 @@ defmodule Ash.Test.Policy.FieldPolicyTest do
             representative_id: rep.id,
             reporter_id: other_user.id
           })
+        ),
+      post:
+        Ash.create!(
+          Ash.Changeset.for_create(Post, :create, %{
+            representative_id: rep.id,
+            reporter_id: user.id
+          })
         )
     ]
   end
@@ -230,6 +237,63 @@ defmodule Ash.Test.Policy.FieldPolicyTest do
     end
   end
 
+  describe "private_fields" do
+    test "when reading a private value and private_fields_policy is :hide, its value is not displayed",
+         %{
+           user: user,
+           ticket: ticket
+         } do
+      assert %Ash.ForbiddenField{field: :top_secret, type: :attribute} ==
+               Ticket
+               |> Ash.Query.select(:top_secret)
+               |> Ash.Query.for_read(:read, %{}, authorize?: true, actor: user)
+               |> Ash.Query.filter(id == ^ticket.id)
+               |> Ash.read_one!()
+               |> Map.get(:top_secret)
+    end
+
+    test "when reading a private value and private_fields_policy is :show, its value is displayed",
+         %{
+           user: user
+         } do
+      assert nil ==
+               User
+               |> Ash.Query.select(:top_secret)
+               |> Ash.Query.for_read(:read, %{}, authorize?: true, actor: user)
+               |> Ash.Query.filter(id == ^user.id)
+               |> Ash.read_one!()
+               |> Map.get(:top_secret)
+    end
+
+    test "when reading a private value, covered by field policy the user is not supposed to see,
+      it's value is not displayed",
+         %{
+           post: post
+         } do
+      assert %Ash.ForbiddenField{field: :internal_status, type: :attribute} ==
+               Post
+               |> Ash.Query.select(:internal_status)
+               |> Ash.Query.for_read(:read, %{}, authorize?: true)
+               |> Ash.Query.filter(id == ^post.id)
+               |> Ash.read_one!()
+               |> Map.get(:internal_status)
+    end
+
+    test "when reading a private value, covered by field policy the user is can see, its value is displayed",
+         %{
+           user: user,
+           post: post
+         } do
+      assert nil ==
+               Post
+               |> Ash.Query.select(:internal_status)
+               |> Ash.Query.for_read(:read, %{}, authorize?: true, actor: user)
+               |> Ash.Query.filter(id == ^post.id)
+               |> Ash.read_one!()
+               |> Map.get(:internal_status)
+    end
+  end
+
   describe "filters" do
     test "filters are replaced with the appropriate field policies", %{
       representative: representative,

test/support/policy_field/resources/post.ex

@@ -0,0 +1,63 @@
+defmodule Ash.Test.Support.PolicyField.Post do
+  @moduledoc false
+  use Ash.Resource,
+    domain: Ash.Test.Support.PolicyField.Domain,
+    data_layer: Ash.DataLayer.Ets,
+    authorizers: [Ash.Policy.Authorizer]
+
+  ets do
+    private? true
+  end
+
+  actions do
+    default_accept :*
+    defaults [:read, :destroy, create: :*, update: :*]
+  end
+
+  attributes do
+    uuid_primary_key :id
+
+    attribute :internal_status, :string do
+      public?(false)
+    end
+
+    attribute :title, :string do
+      public?(true)
+    end
+
+    attribute :description, :string do
+      public?(true)
+    end
+  end
+
+  relationships do
+    belongs_to :representative, Ash.Test.Support.PolicyField.User do
+      public?(true)
+      allow_nil? false
+    end
+
+    belongs_to :reporter, Ash.Test.Support.PolicyField.User do
+      public?(true)
+      allow_nil? false
+    end
+  end
+
+  policies do
+    policy always() do
+      authorize_if always()
+    end
+  end
+
+  field_policies do
+    private_fields :include
+
+    field_policy :internal_status do
+      authorize_if relates_to_actor_via(:representative)
+      authorize_if relates_to_actor_via(:reporter)
+    end
+
+    field_policy :* do
+      authorize_if always()
+    end
+  end
+end

test/support/policy_field/resources/ticket.ex

@@ -17,6 +17,10 @@ defmodule Ash.Test.Support.PolicyField.Ticket do
   attributes do
     uuid_primary_key :id
 
+    attribute :top_secret, :string do
+      public?(false)
+    end
+
     attribute :internal_status, :string do
       public?(true)
     end
@@ -49,6 +53,8 @@ defmodule Ash.Test.Support.PolicyField.Ticket do
   end
 
   field_policies do
+    private_fields :hide
+
     field_policy :status do
       authorize_if relates_to_actor_via(:representative)
       authorize_if relates_to_actor_via(:reporter)

test/support/policy_field/resources/user.ex

@@ -26,6 +26,10 @@ defmodule Ash.Test.Support.PolicyField.User do
       public?(true)
       # only you can see your own points
     end
+
+    attribute :top_secret, :string do
+      public?(false)
+    end
   end
 
   relationships do
@@ -49,6 +53,8 @@ defmodule Ash.Test.Support.PolicyField.User do
   end
 
   field_policies do
+    private_fields :show
+
     field_policy_bypass :* do
       authorize_if actor_attribute_equals(:role, :admin)
     end