From ace38cd31f75cb9fec9993dcddfe9d28bb196d71 Mon Sep 17 00:00:00 2001 From: SpaceEEC Date: Mon, 3 Oct 2022 18:16:53 +0200 Subject: [PATCH] improvement: validate accepted and rejected attributes in actions (#395) --- lib/ash/resource/dsl.ex | 3 +- .../resource/transformers/default_accept.ex | 13 ++-- .../resource/transformers/validate_accept.ex | 64 +++++++++++++++++++ test/resource/validate_accept_test.exs | 55 ++++++++++++++++ 4 files changed, 128 insertions(+), 7 deletions(-) create mode 100644 lib/ash/resource/transformers/validate_accept.ex create mode 100644 test/resource/validate_accept_test.exs diff --git a/lib/ash/resource/dsl.ex b/lib/ash/resource/dsl.ex index 425cb5cc..c64e1e42 100644 --- a/lib/ash/resource/dsl.ex +++ b/lib/ash/resource/dsl.ex @@ -1212,7 +1212,8 @@ defmodule Ash.Resource.Dsl do Ash.Resource.Transformers.RequireUniqueFieldNames, Ash.Resource.Transformers.ValidateRelationshipAttributes, Ash.Resource.Transformers.ValidateEagerIdentities, - Ash.Resource.Transformers.ValidateAggregatesSupported + Ash.Resource.Transformers.ValidateAggregatesSupported, + Ash.Resource.Transformers.ValidateAccept ] @moduledoc """ diff --git a/lib/ash/resource/transformers/default_accept.ex b/lib/ash/resource/transformers/default_accept.ex index 583a775b..644b7bf4 100644 --- a/lib/ash/resource/transformers/default_accept.ex +++ b/lib/ash/resource/transformers/default_accept.ex @@ -9,6 +9,7 @@ defmodule Ash.Resource.Transformers.DefaultAccept do public_attribute_names = dsl_state |> Transformer.get_entities([:attributes]) + |> Enum.reject(& &1.private?) |> Enum.map(& &1.name) default_accept = @@ -22,19 +23,19 @@ defmodule Ash.Resource.Transformers.DefaultAccept do |> Transformer.get_entities([:actions]) |> Enum.reject(&(&1.type == :read)) |> Enum.reduce({:ok, dsl_state}, fn action, {:ok, dsl_state} -> - accept = + {accept, reject} = case {action.accept, action.reject} do - {_, :all} -> [] - {nil, reject} -> reject(default_accept, reject) - {:all, reject} -> reject(public_attribute_names, reject) - {accept, reject} -> reject(accept, reject) + {_, :all} -> {[], public_attribute_names} + {nil, reject} -> {reject(default_accept, reject), reject} + {:all, reject} -> {reject(public_attribute_names, reject), reject} + {accept, reject} -> {reject(accept, reject), reject} end new_dsl_state = Transformer.replace_entity( dsl_state, [:actions], - %{action | accept: accept}, + %{action | accept: accept, reject: reject}, &(&1.name == action.name && &1.type == action.type) ) diff --git a/lib/ash/resource/transformers/validate_accept.ex b/lib/ash/resource/transformers/validate_accept.ex new file mode 100644 index 00000000..19c10714 --- /dev/null +++ b/lib/ash/resource/transformers/validate_accept.ex @@ -0,0 +1,64 @@ +defmodule Ash.Resource.Transformers.ValidateAccept do + @moduledoc "Validates that accept and reject lists only contain valid attributes" + use Spark.Dsl.Transformer + + alias Spark.Dsl.Transformer + alias Spark.Error.DslError + + @impl true + def after_compile?, do: true + + @impl true + def transform(dsl_state) do + {private_attributes, public_attributes} = + dsl_state + |> Transformer.get_entities([:attributes]) + |> Enum.split_with(& &1.private?) + + public_attribute_names = MapSet.new(public_attributes, & &1.name) + private_attribute_names = MapSet.new(private_attributes, & &1.name) + + Transformer.get_entities(dsl_state, [:actions]) + |> Enum.each(fn + %{name: action_name, accept: accept, reject: reject} -> + validate_attribute_name = fn attribute_name, type -> + cond do + MapSet.member?(private_attribute_names, attribute_name) -> + raise DslError, + path: [:actions, action_name, type, attribute_name], + message: "#{attribute_name} is a private attribute" + + MapSet.member?(public_attribute_names, attribute_name) -> + :ok + + true -> + raise DslError, + path: [:actions, action_name, type, attribute_name], + message: "#{attribute_name} is not an attribute" + end + end + + Enum.each( + accept, + &validate_attribute_name.( + &1, + :accept + ) + ) + + Enum.each( + reject, + &validate_attribute_name.( + &1, + :reject + ) + ) + + # read types do not have accept / reject fields + %{type: :read} -> + :ok + end) + + :ok + end +end diff --git a/test/resource/validate_accept_test.exs b/test/resource/validate_accept_test.exs new file mode 100644 index 00000000..88976d42 --- /dev/null +++ b/test/resource/validate_accept_test.exs @@ -0,0 +1,55 @@ +defmodule Ash.Test.Resource.ValidateAcceptTest do + @moduledoc false + use ExUnit.Case, async: true + + import Ash.Test.Helpers + alias Spark.Error.DslError + + test "Accepting an attribute that does not exist raises an error" do + assert_raise DslError, ~r/invalid is not an attribute/, fn -> + defposts do + actions do + create :example_action, accept: [:invalid] + end + end + end + end + + test "Accepting an attribute that is private raises an error" do + assert_raise DslError, ~r/secret is a private attribute/, fn -> + defposts do + attributes do + attribute :secret, :string, private?: true + end + + actions do + create :example_action, accept: [:secret] + end + end + end + end + + test "Rejecting an attribute that does not exist raises an error" do + assert_raise DslError, ~r/invalid is not an attribute/, fn -> + defposts do + actions do + create :example_action, reject: [:invalid] + end + end + end + end + + test "Rejecting an attribute that is private raises an error" do + assert_raise DslError, ~r/secret is a private attribute/, fn -> + defposts do + attributes do + attribute :secret, :string, private?: true + end + + actions do + create :example_action, reject: [:secret] + end + end + end + end +end