From bb96354470d66b442a24d8f4256663595846d177 Mon Sep 17 00:00:00 2001 From: Zach Daniel Date: Fri, 23 Jun 2023 10:10:26 -0400 Subject: [PATCH] improvement: support ForbiddenField --- lib/graphql/resolver.ex | 79 +++++++++++++++++++++---------- lib/resource/resource.ex | 19 ++++---- mix.lock | 2 +- test/read_test.exs | 65 +++++++++++++++++++++++++ test/support/resources/comment.ex | 1 + test/support/resources/user.ex | 11 +++++ 6 files changed, 143 insertions(+), 34 deletions(-) diff --git a/lib/graphql/resolver.ex b/lib/graphql/resolver.ex index 822b783..8581bd6 100644 --- a/lib/graphql/resolver.ex +++ b/lib/graphql/resolver.ex @@ -2128,25 +2128,35 @@ defmodule AshGraphql.Graphql.Resolver do Map.get(parent, calculation.name) end - {type, constraints} = unwrap_type_and_constraints(calculation.type, calculation.constraints) - - result = - if Ash.Type.NewType.new_type?(type) && - Ash.Type.NewType.subtype_of(type) == Ash.Type.Union && - function_exported?(type, :graphql_unnested_unions, 1) do - unnested_types = type.graphql_unnested_unions(calculation.constraints) - - calculation = %{calculation | type: type, constraints: constraints} - - resolve_union_result( - result, - {calculation.name, calculation.type, calculation, resource, unnested_types} + case result do + %struct{} when struct == Ash.ForbiddenField -> + Absinthe.Resolution.put_result( + resolution, + to_resolution({:error, Ash.Error.Forbidden.exception([])}, context, api) ) - else - result - end - Absinthe.Resolution.put_result(resolution, to_resolution({:ok, result}, context, api)) + result -> + {type, constraints} = + unwrap_type_and_constraints(calculation.type, calculation.constraints) + + result = + if Ash.Type.NewType.new_type?(type) && + Ash.Type.NewType.subtype_of(type) == Ash.Type.Union && + function_exported?(type, :graphql_unnested_unions, 1) do + unnested_types = type.graphql_unnested_unions(calculation.constraints) + + calculation = %{calculation | type: type, constraints: constraints} + + resolve_union_result( + result, + {calculation.name, calculation.type, calculation, resource, unnested_types, api} + ) + else + result + end + + Absinthe.Resolution.put_result(resolution, to_resolution({:ok, result}, context, api)) + end end defp unwrap_type_and_constraints({:array, type}, constraints), @@ -2185,8 +2195,8 @@ defmodule AshGraphql.Graphql.Resolver do do: resolution def resolve_union( - %{source: parent} = resolution, - {name, _field_type, _field, _resource, _unnested_types} = data + %{source: parent, context: context} = resolution, + {name, _field_type, _field, _resource, _unnested_types, api} = data ) do value = if resolution.definition.alias do @@ -2195,12 +2205,24 @@ defmodule AshGraphql.Graphql.Resolver do Map.get(parent, name) end - result = resolve_union_result(value, data) + case value do + %struct{} when struct == Ash.ForbiddenField -> + Absinthe.Resolution.put_result( + resolution, + to_resolution({:error, Ash.Error.Forbidden.exception([])}, context, api) + ) - Absinthe.Resolution.put_result(resolution, {:ok, result}) + value -> + result = resolve_union_result(value, data) + + Absinthe.Resolution.put_result(resolution, {:ok, result}) + end end - def resolve_attribute(%{source: parent} = resolution, {name, type, constraints}) do + def resolve_attribute( + %{source: parent, context: context} = resolution, + {name, type, constraints, api} + ) do value = if resolution.definition.alias && Ash.Type.can_load?(type, constraints) do Map.get(parent.calculations, {:__ash_graphql_attribute__, resolution.definition.alias}) @@ -2208,7 +2230,16 @@ defmodule AshGraphql.Graphql.Resolver do Map.get(parent, name) end - Absinthe.Resolution.put_result(resolution, {:ok, value}) + case value do + %struct{} when struct == Ash.ForbiddenField -> + Absinthe.Resolution.put_result( + resolution, + to_resolution({:error, Ash.Error.Forbidden.exception([])}, context, api) + ) + + value -> + Absinthe.Resolution.put_result(resolution, {:ok, value}) + end end defp resolve_union_result(value, data) when is_list(value) do @@ -2217,7 +2248,7 @@ defmodule AshGraphql.Graphql.Resolver do defp resolve_union_result( value, - {_name, field_type, field, resource, unnested_types} + {_name, field_type, field, resource, unnested_types, _api} ) do case value do %Ash.Union{type: type, value: value} = union -> diff --git a/lib/resource/resource.ex b/lib/resource/resource.ex index 98bedb5..dfa60fe 100644 --- a/lib/resource/resource.ex +++ b/lib/resource/resource.ex @@ -3154,10 +3154,10 @@ defmodule AshGraphql.Resource do end defp fields(resource, api, schema, query \\ nil) do - attributes(resource, schema) ++ + attributes(resource, api, schema) ++ metadata(query, resource, schema) ++ relationships(resource, api, schema) ++ - aggregates(resource, schema) ++ + aggregates(resource, api, schema) ++ calculations(resource, api, schema) ++ keyset(resource, schema) end @@ -3217,7 +3217,7 @@ defmodule AshGraphql.Resource do end end - defp attributes(resource, schema) do + defp attributes(resource, api, schema) do attribute_names = AshGraphql.Resource.Info.field_names(resource) attributes = @@ -3250,7 +3250,8 @@ defmodule AshGraphql.Resource do attribute, attribute.name, attribute.type, - attribute.constraints + attribute.constraints, + api ), name: to_string(name), type: field_type, @@ -3429,7 +3430,7 @@ defmodule AshGraphql.Resource do end) end - defp aggregates(resource, schema) do + defp aggregates(resource, api, schema) do field_names = AshGraphql.Resource.Info.field_names(resource) resource @@ -3464,7 +3465,7 @@ defmodule AshGraphql.Resource do identifier: aggregate.name, module: schema, middleware: - middleware_for_field(resource, aggregate, aggregate.name, agg_type, constraints), + middleware_for_field(resource, aggregate, aggregate.name, agg_type, constraints, api), name: to_string(name), description: aggregate.description, type: type, @@ -3473,7 +3474,7 @@ defmodule AshGraphql.Resource do end) end - defp middleware_for_field(resource, field, name, type, constraints) do + defp middleware_for_field(resource, field, name, type, constraints, api) do if Ash.Type.NewType.new_type?(type) && Ash.Type.NewType.subtype_of(type) == Ash.Type.Union && function_exported?(type, :graphql_unnested_unions, 1) do @@ -3481,11 +3482,11 @@ defmodule AshGraphql.Resource do [ {{AshGraphql.Graphql.Resolver, :resolve_union}, - {name, type, field, resource, unnested_types}} + {name, type, field, resource, unnested_types, api}} ] else [ - {{AshGraphql.Graphql.Resolver, :resolve_attribute}, {name, type, constraints}} + {{AshGraphql.Graphql.Resolver, :resolve_attribute}, {name, type, constraints, api}} ] end end diff --git a/mix.lock b/mix.lock index 78f2fcb..24a5c01 100644 --- a/mix.lock +++ b/mix.lock @@ -38,7 +38,7 @@ "plug_crypto": {:hex, :plug_crypto, "1.2.5", "918772575e48e81e455818229bf719d4ab4181fcbf7f85b68a35620f78d89ced", [:mix], [], "hexpm", "26549a1d6345e2172eb1c233866756ae44a9609bd33ee6f99147ab3fd87fd842"}, "sobelow": {:hex, :sobelow, "0.12.2", "45f4d500e09f95fdb5a7b94c2838d6b26625828751d9f1127174055a78542cf5", [:mix], [{:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}], "hexpm", "2f0b617dce551db651145662b84c8da4f158e7abe049a76daaaae2282df01c5d"}, "sourceror": {:hex, :sourceror, "0.12.3", "a2ad3a1a4554b486d8a113ae7adad5646f938cad99bf8bfcef26dc0c88e8fade", [:mix], [], "hexpm", "4d4e78010ca046524e8194ffc4683422f34a96f6b82901abbb45acc79ace0316"}, - "spark": {:hex, :spark, "1.1.15", "c0db345f030c928d2c9cf8dbf7574c635664d54b3afaf64ec9c1481d20c48b66", [:mix], [{:nimble_options, "~> 0.5 or ~> 1.0", [hex: :nimble_options, repo: "hexpm", optional: false]}, {:sourceror, "~> 0.1", [hex: :sourceror, repo: "hexpm", optional: false]}], "hexpm", "bd7da17b8af5acd39e49b9dbdc98a21132cade2ff70e6283e09f37a4657362b8"}, + "spark": {:hex, :spark, "1.1.18", "349ad7ec69b389294fd3f17a4e49e772cafbbb71d3571add652a80f7b3c44990", [:mix], [{:nimble_options, "~> 0.5 or ~> 1.0", [hex: :nimble_options, repo: "hexpm", optional: false]}, {:sourceror, "~> 0.1", [hex: :sourceror, repo: "hexpm", optional: false]}], "hexpm", "964b46866e01b39810a82f9ee538f7f25d450cb3223af58b0f4717ce69d6f167"}, "ssl_verify_fun": {:hex, :ssl_verify_fun, "1.1.6", "cf344f5692c82d2cd7554f5ec8fd961548d4fd09e7d22f5b62482e5aeaebd4b0", [:make, :mix, :rebar3], [], "hexpm", "bdb0d2471f453c88ff3908e7686f86f9be327d065cc1ec16fa4540197ea04680"}, "stream_data": {:hex, :stream_data, "0.5.0", "b27641e58941685c75b353577dc602c9d2c12292dd84babf506c2033cd97893e", [:mix], [], "hexpm", "012bd2eec069ada4db3411f9115ccafa38540a3c78c4c0349f151fc761b9e271"}, "telemetry": {:hex, :telemetry, "1.2.1", "68fdfe8d8f05a8428483a97d7aab2f268aaff24b49e0f599faa091f1d4e7f61c", [:rebar3], [], "hexpm", "dad9ce9d8effc621708f99eac538ef1cbe05d6a874dd741de2e689c47feafed5"}, diff --git a/test/read_test.exs b/test/read_test.exs index ceb01ce..f853e16 100644 --- a/test/read_test.exs +++ b/test/read_test.exs @@ -101,6 +101,29 @@ defmodule AshGraphql.ReadTest do assert %{data: %{"currentUserWithMetadata" => %{"bar" => "bar"}}} = result end + test "forbidden fields show errors" do + AshGraphql.Test.User + |> Ash.Changeset.for_create(:create, + name: "My Name" + ) + |> AshGraphql.Test.Api.create!() + + resp = + """ + query CurrentUserWithMetadata { + currentUserWithMetadata { + bar + secret + } + } + """ + |> Absinthe.run(AshGraphql.Test.Schema) + + assert {:ok, _result} = resp + + assert %{errors: [], data: %{"currentUserWithMetadata" => nil}} + end + test "a read with arguments works" do AshGraphql.Test.Post |> Ash.Changeset.for_create(:create, text: "foo", published: true) @@ -188,6 +211,48 @@ defmodule AshGraphql.ReadTest do result end + test "reading relationships works with fragments, without selecting the id field" do + post = + AshGraphql.Test.Post + |> Ash.Changeset.for_create(:create, text: "foo", published: true) + |> AshGraphql.Test.Api.create!() + + AshGraphql.Test.Comment + |> Ash.Changeset.for_create(:create, %{text: "stuff"}) + |> Ash.Changeset.force_change_attribute(:post_id, post.id) + |> AshGraphql.Test.Api.create!() + + resp = + """ + query PostLibrary($published: Boolean) { + postLibrary(published: $published) { + text + id + ...PostFragment + } + } + + fragment PostFragment on Post { + comments{ + id + text + } + } + """ + |> Absinthe.run(AshGraphql.Test.Schema, + variables: %{ + "published" => true + } + ) + + assert {:ok, result} = resp + + refute Map.has_key?(result, :errors) + + assert %{data: %{"postLibrary" => [%{"text" => "foo", "comments" => [%{"text" => "stuff"}]}]}} = + result + end + test "the same relationship can be fetched with different parameters" do post = AshGraphql.Test.Post diff --git a/test/support/resources/comment.ex b/test/support/resources/comment.ex index 6fae94d..f3352cd 100644 --- a/test/support/resources/comment.ex +++ b/test/support/resources/comment.ex @@ -10,6 +10,7 @@ defmodule AshGraphql.Test.Comment do queries do get :get_comment, :read + list :list_comments, :read end mutations do diff --git a/test/support/resources/user.ex b/test/support/resources/user.ex index 454c29a..c3634d4 100644 --- a/test/support/resources/user.ex +++ b/test/support/resources/user.ex @@ -51,6 +51,7 @@ defmodule AshGraphql.Test.User do uuid_primary_key(:id) attribute(:name, :string) + attribute(:secret, :string) end relationships do @@ -70,4 +71,14 @@ defmodule AshGraphql.Test.User do authorize_if(always()) end end + + field_policies do + field_policy :secret do + forbid_if(always()) + end + + field_policy :* do + authorize_if(always()) + end + end end