Using an orphaned "union type" doesn't properly restrict when loading
thijsnado opened this issue · 1 comments
Describe the bug
If you try using a union type to restrict what something loads, it doesn't work if that union type isn't also exposed as a field somewhere.:
# Union type
class Types::MyUnion
possible_types Types::TypeA, Types::TypeB
end
# Mutation type
class Mutations::DestroyMyUnion
argument :object_id, ID, required: true, loads: Types::MyUnion
field :success, Boolean, null: false
# un-commenting the line below makes things work as expected
# field :object, Types::MyUnion, null: false
def resolve(object:)
object.destroy
end
end
If you are using a globally unique ID to describe objects, than any globably unique ID will be accepted in the above scenario. If the union type is exposed as a field anywhere or added to orphaned types (only works in older versions, extra types doesn't work) things work as expected in that it will filter out unwanted ones.
This bug may also exist for interfaces but I haven't gotten around to testing that yet. I'll comment on this issue when I do.
Versions
graphql
version: 2.3.14
rails
(or other framework): 7.0.8.4
GraphQL schema
Here is a test schema with rspec tests inlined to illustrate the issue. Note how the field for the union is commented out.
# frozen_string_literal: true
require "spec_helper"
module Types
module TestInterfaceType
include Types::BaseInterface
field :id, String, null: false
field :hello, String, null: false
end
class TestConcreteType1 < Types::BaseObject
graphql_name "TestConcreteType1"
implements TestInterfaceType
skip_authorization!
global_id_field :id
def hello
"hello from concrete type 1"
end
end
class TestConcreteType2 < Types::BaseObject
graphql_name "TestConcreteType2"
skip_authorization!
global_id_field :id
end
class TestConcreteType3 < Types::BaseObject
graphql_name "TestConcreteType3"
implements TestInterfaceType
skip_authorization!
global_id_field :id
def hello
"hello from concrete type 3"
end
end
class TestUnionType < BaseUnion
possible_types TestConcreteType1, TestConcreteType2
end
class TestQueryType < Types::BaseObject
skip_authorization!
field :concrete_type_1, Types::TestConcreteType1, null: false do
argument :concrete_type_1_id, ID, required: true, loads: Types::TestConcreteType1
end
# field :union_type, Types::TestUnionType, null: false do
# argument :union_type_id, ID, required: true, loads: Types::TestUnionType
# end
field :interface_type, Types::TestInterfaceType, null: false do
argument :interface_type_id, ID, required: true, loads: Types::TestInterfaceType
end
def concrete_type_1(concrete_type_1:)
concrete_type_1
end
def union_type(union_type:)
union_type
end
def interface_type(interface_type:)
interface_type
end
end
end
module Mutations
class MutateUnionType < Mutations::BaseMutation
argument :union_type_id, ID, required: true, loads: Types::TestUnionType
field :success, Boolean, null: false
def authorized?(union_type:)
true
end
def resolve(union_type:)
{success: union_type.present?}
end
end
end
module Types
class TestMutationType < Types::BaseReturnObject
field :mutate_union_type, mutation: Mutations::MutateUnionType
end
end
class TestBaseSchema < GraphQL::Schema
query(Types::TestQueryType)
mutation(Types::TestMutationType)
extra_types [
Types::TestUnionType
]
def self.id_from_object(object, type_definition, _query_ctx)
GraphQL::Schema::UniqueWithinType.encode(type_definition.name, object.id)
end
def self.object_from_id(id, _query_ctx)
gql_type_name, item_id = GraphQL::Schema::UniqueWithinType.decode(id)
OpenStruct.new(type: gql_type_name.constantize, id: item_id)
end
def self.resolve_type(type, object, ctx)
object.type
end
end
describe BaseSchema do
def described_class
TestBaseSchema
end
describe "locking down of load to only allowed types" do
it "won't load objects that don't match type" do
concrete_type_1 = OpenStruct.new(type: Types::TestConcreteType1, id: 1)
concrete_type_1_id = described_class.id_from_object(concrete_type_1, Types::TestConcreteType1, {})
query = <<~GQL
query {
concreteType1(concreteType1Id: "#{concrete_type_1_id}") {
id
hello
}
}
GQL
result = described_class.execute(query)
expect(result.dig("data", "concreteType1", "id")).to eq(concrete_type_1_id)
concrete_type_2 = OpenStruct.new(type: Types::TestConcreteType2, id: 2)
concrete_type_2_id = described_class.id_from_object(concrete_type_2, Types::TestConcreteType2, {})
query = <<~GQL
query {
concreteType1(concreteType1Id: "#{concrete_type_2_id}") {
id
hello
}
}
GQL
result = described_class.execute(query)
expect(result.dig("data", "concreteType1", "id")).to eq(nil)
end
end
describe "locking down union" do
it "won't load objects that don't match type" do
concrete_type_1 = OpenStruct.new(type: Types::TestConcreteType1, id: 1)
concrete_type_1_id = described_class.id_from_object(concrete_type_1, Types::TestConcreteType1, {})
query = <<~GQL
query {
unionType(unionTypeId: "#{concrete_type_1_id}") {
__typename
...on TestConcreteType1 {
id
hello
}
}
}
GQL
result = described_class.execute(query)
expect(result.dig("data", "unionType", "id")).to eq(concrete_type_1_id)
concrete_type_3 = OpenStruct.new(type: Types::TestConcreteType3, id: 3)
concrete_type_3_id = described_class.id_from_object(concrete_type_3, Types::TestConcreteType3, {})
query = <<~GQL
query {
unionType(unionTypeId: "#{concrete_type_3_id}") {
__typename
...on TestConcreteType2 {
id
}
}
}
GQL
result = described_class.execute(query)
expect(result.dig("data", "unionType", "id")).to eq(nil)
end
end
describe "locking down interface" do
it "won't load objects that don't match type" do
concrete_type_1 = OpenStruct.new(type: Types::TestConcreteType1, id: 1)
concrete_type_1_id = described_class.id_from_object(concrete_type_1, Types::TestConcreteType1, {})
query = <<~GQL
query {
interfaceType(interfaceTypeId: "#{concrete_type_1_id}") {
id
hello
}
}
GQL
result = described_class.execute(query)
expect(result.dig("data", "interfaceType", "id")).to eq(concrete_type_1_id)
concrete_type_2 = OpenStruct.new(type: Types::TestConcreteType2, id: 2)
concrete_type_2_id = described_class.id_from_object(concrete_type_2, Types::TestConcreteType2, {})
query = <<~GQL
query {
interfaceType(interfaceTypeId: "#{concrete_type_2_id}") {
id
hello
}
}
GQL
result = described_class.execute(query)
expect(result.dig("data", "interfaceType", "id")).to eq(nil)
end
end
describe "locking down union type mutation" do
it "won't load objects that don't match type", focus: true do
concrete_type_1 = OpenStruct.new(type: Types::TestConcreteType1, id: 1)
concrete_type_1_id = described_class.id_from_object(concrete_type_1, Types::TestConcreteType1, {})
query = <<~GQL
mutation {
mutateUnionType(input: {unionTypeId: "#{concrete_type_1_id}"}) {
success
}
}
GQL
result = described_class.execute(query)
expect(result.dig("data", "mutateUnionType", "success")).to eq(true)
concrete_type_3 = OpenStruct.new(type: Types::TestConcreteType3, id: 3)
concrete_type_3_id = described_class.id_from_object(concrete_type_3, Types::TestConcreteType3, {})
query = <<~GQL
mutation {
mutateUnionType(input: {unionTypeId: "#{concrete_type_3_id}"}) {
success
}
}
GQL
result = described_class.execute(query)
expect(result.dig("data", "mutateUnionType", "success")).to eq(nil)
end
end
end
GraphQL query
See test steps for mutation example
Steps to reproduce
Steps to reproduce the behavior
Expected behavior
I would expect a union type to behave the same in loading an object whether it was exposed as a field or not.
Actual behavior
The union type doesn't restrict loading types that aren't part of the union like I would expect unless it is also declared as the return type on a field or declared in orphaned types (on older versions).
Hey, thanks for the detailed write-up. I definitely agree that this should be supported one way or another, I'll just have to take a look at how it can be accomplished. This method is supposed to return true
for types in cases like this, but maybe it's not:
graphql-ruby/lib/graphql/schema/warden.rb
Lines 213 to 216 in e510a44