Using an orphaned "union type" doesn't properly restrict when loading

[thijsnado]

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).

[rmosolgo]

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

	# @return [Boolean] True if this type is used for `loads:` but not in the schema otherwise and not _explicitly_ hidden.
	def loadable?(type, _ctx)
	!reachable_type_set.include?(type) && visible_type?(type)
	end