[Pro/Enterprise] Pundit integration behaves differently than normal authorization when resolving connections/lists

[ChristianLee-Jobber]

Describe the bug
We have a type that uses pundits as part of the authorization flow. When being resolved as a connection, in graphql version 2.0.24, the authorization check occurs. In graphql-ruby version 2.1.0 this check no longer occurs.

Versions

graphql version: 2.1.0
rails (or other framework): 7.0.8

other applicable versions (graphql-batch, etc):

• graphql-pro 1.24.6, and even 1.29.4 was tested
• graphql-enterprise 1.3.0

GraphQL schema

Include relevant types and fields (in Ruby is best, in GraphQL IDL is ok). Any custom extensions, etc?

module GraphqlSchema
  module Anchor
    module Types
      class PromotionType < GraphqlSchema::Common::Types::AuthenticatedObject
        pundit_policy_class Administration::PromotionPolicy
        pundit_role :read

        description "Promotion information"

        field(:id, GraphqlSchema::Common::Types::EncodedId, null: false, description: "The ID of the promotion")
  # …
end

class Administration::PromotionPolicy < Administration::AdministrationPolicy
  # ...
  class Scope < Administration::AdministrationPolicy::Scope
    FILTERABLE_ROLES = %i(sales_manager account_manager account_executive partnerships_manager payments_solutions_specialist).freeze

    def resolve
      return scope.all if super_user.promo_campaign_manager? || super_user.sales_leadership?

      FILTERABLE_ROLES.each do |role|
        return scope.active.visible_to(role) if super_user.respond_to?("#{role}?") && super_user.send("#{role}?")
      end

      []
    end
  # ...
  end
end

# Field on the query type:
field :promotions, resolver: Resolvers::PromotionsResolver

# The PromotionsResolver
module GraphqlSchema
  module Anchor
    module Resolvers
      class PromotionsResolver < GraphqlSchema::Common::Resolvers::AuthenticatedResolver
        description "All promotions"

        argument :filter, Types::PromotionListFilterInput, required: false, description: "Filters used with the Promotions"

        type Types::PromotionType.connection_type, null: false

        def resolve(filter: nil)
          Promotion.filter_results(filter.to_h)
        end
      end
    end
  end
end

GraphQL query

Example GraphQL query and response (if query execution is involved)

query Promotion {
  promotions {
    nodes {
      trackingTag
      id
      name
      status
      modalDetails {
        modalButtonText
        modalInfo
        modalSubtext
        modalTitle
        modalWelcomeText
      }
    }
  }
}

Steps to reproduce

• In the object being queried, add a def self.authorized? method definition to intercept the authorized call
• In graphql-ruby 2.0.24, query a connection_type from a resolver that uses a pundit
  • self.authorized? method will be hit
    -In graphql-ruby 2.1.0, query a connection_type from a resolver that uses a pundit
  • self.authorized? will not be hit

Expected behavior
The authorized checks should be identical between the 2 cases

Actual behavior
The authorized checks did not trigger

Additional context
I did some digging into this, especially since graphql-ruby 2.1.0 changes how connections get authorized, so I thought this was a problem on us. However, I found a file lib/graphql/schema/field/scope_extension.rb, in which I noticed some diverging behaviour between resolving a normal connection vs one whose objects use pundit_role + pundit_policy_class:

              # With a pundit_role and pundit_policy_class defined, the next lines goes through
              #  a slightly different flow
              scoped_items = ret_type.scope_items(value, context)

              # The next line of code behaves differently after going through pundit.
              # With pundit, we have a case where `scoped_items.equal?(value) == false`,
              # however `scoped_items == value` returns true (they are the same records, but
              # they are different objects in ruby)
              #
              # Without a pundit_role defined, after resolving the `scoped_items` above, we have
              # `scoped_items.equal?(value) == true`
              # 
              # Also, the `ret_type` in this case is the connection, not the object itself, so setting
              # `reauthorize_scoped_objects(true) on the `PromotionType` does not help us get
              # past this check.
              if !scoped_items.equal?(value) && !ret_type.reauthorize_scoped_objects
                current_runtime_state = Thread.current[:__graphql_runtime_info]
                query_runtime_state = current_runtime_state[context.query]
                query_runtime_state.was_authorized_by_scope_items = true
              end

[rmosolgo]

Hey! Sorry for the trouble and thanks for the detailed report.

The only thing that came to mind was #4726, but that was released in graphql-pro v1.26.0, so it wouldn't have affected v1.24.6. I'll take a closer look and follow up here with what I find!

[rmosolgo]

Ok, I read your issue more closely and I think I've spotted the issue. In GraphQL-Ruby v2.1.0, I made the default setting reauthorize_scoped_objects(false), so that, by default, if a list was modified by def self.scope_items, then the items in that list would not be passed to def self.authorized?.

However, that default setting was a bad choice (and maybe a mistake? #3994 (comment)), so in v2.1.7, I reverted the default to reauthorize_scoped_objects(true) -- the same behavior as GraphQL-Ruby versions <2.1.0 (#4720).

If this is the problem that's causing the issue you found, you have two options:

• Continue using GraphQL-Ruby v2.1.0, but add reauthorize_scoped_objects(true) to your base object class. This will bring back the pre-2.1.0 changes.
• Or, update to v2.1.7 where the default was changed (and compatibility was maintained)

Want to give one of those a try? Let me know how it goes!

[ChristianLee-Jobber]

Thank you, using v.2.1.7 fixes this! If only I read the bug fixes in the changelog, I could have saved us both time!

[rmosolgo]

I'm glad to hear it worked! Yeah, well, that feature release was a bit bumpy 😖