Don't re-authorize objects that were part of a scoped list

guides/authorization/scoping.md

@@ -8,7 +8,7 @@ index: 4
 ---
 
 
-_Scoping_ is a complementary consideration to authorization. Rather than checking "can this user see this thing?", scoping takes a list of items filters it to the subset which is appropriate for the current viewer and context. The resulting subset is authorized as normal, and, assuming that it was properly scoped, each item should pass authorization checks.
+_Scoping_ is a complementary consideration to authorization. Rather than checking "can this user see this thing?", scoping takes a list of items filters it to the subset which is appropriate for the current viewer and context.
 
 For similar features, see [Pundit scopes](https://github.com/varvet/pundit#scopes) and [Cancan's `.accessible_by`](https://github.com/cancancommunity/cancancan/wiki/Fetching-Records).
 
@@ -43,3 +43,25 @@ end
 ```
 
 The method should return a new list with only the appropriate items for the current `context`.
+
+## Bypassing object-level authorization
+
+If you know that any items returned from `.scope_items` should be visible to the current client, you can skip the normal `.authorized?(obj, ctx)` checks by configuring `reauthorize_scoped_objects(false)` in your type definition. For example:
+
+```ruby
+class Types::Product < Types::BaseObject
+  # Check that singly-loaded objects are visible to the current viewer
+  def self.authorized?(object, context)
+    object.visible_to?(context[:viewer])
+  end
+
+  # Filter any list to only include objects that are visible to the current viewer
+  def self.scope_items(items, context)
+    items.visible_for(context[:viewer])
+  end
+
+  # If an object of this type was returned from `.scope_items`,
+  # don't call `.authorized?` with it.
+  reauthorize_scoped_objects(false)
+end
+```

lib/graphql/execution/interpreter/runtime.rb

@@ -15,10 +15,11 @@ def initialize
             @current_arguments = nil
             @current_result_name = nil
             @current_result = nil
+            @was_authorized_by_scope_items = nil
           end
 
           attr_accessor :current_result, :current_result_name,
-            :current_arguments, :current_field, :current_object
+            :current_arguments, :current_field, :current_object, :was_authorized_by_scope_items
         end
 
         module GraphQLResult
@@ -244,6 +245,7 @@ def run_eager
           root_operation = query.selected_operation
           root_op_type = root_operation.operation_type || "query"
           root_type = schema.root_type_for_operation(root_op_type)
+
           st = get_current_runtime_state
           st.current_object = query.root_value
           st.current_result = @response
@@ -426,12 +428,6 @@ def evaluate_selection(result_name, field_ast_nodes_or_ast_node, owner_object, o
             end
           end
 
-          return_type = field_defn.type
-
-          # This seems janky, but we need to know
-          # the field's return type at this path in order
-          # to propagate `null`
-          return_type_non_null = return_type.non_null?
           # Set this before calling `run_with_directives`, so that the directive can have the latest path
           st = get_current_runtime_state
           st.current_field = field_defn
@@ -441,26 +437,27 @@ def evaluate_selection(result_name, field_ast_nodes_or_ast_node, owner_object, o
           if is_introspection
             owner_object = field_defn.owner.wrap(owner_object, context)
           end
-
+          return_type = field_defn.type
           total_args_count = field_defn.arguments(context).size
           if total_args_count == 0
             resolved_arguments = GraphQL::Execution::Interpreter::Arguments::EMPTY
             if field_defn.extras.size == 0
               evaluate_selection_with_resolved_keyword_args(
-                NO_ARGS, resolved_arguments, field_defn, ast_node, field_ast_nodes, owner_type, owner_object, is_eager_field, result_name, selections_result, parent_object, return_type, return_type_non_null
+                NO_ARGS, resolved_arguments, field_defn, ast_node, field_ast_nodes, owner_type, owner_object, is_eager_field, result_name, selections_result, parent_object, return_type, return_type.non_null?
               )
             else
-              evaluate_selection_with_args(resolved_arguments, field_defn, ast_node, field_ast_nodes, owner_type, owner_object, is_eager_field, result_name, selections_result, parent_object, return_type, return_type_non_null)
+              evaluate_selection_with_args(resolved_arguments, field_defn, ast_node, field_ast_nodes, owner_type, owner_object, is_eager_field, result_name, selections_result, parent_object, return_type)
             end
           else
             @query.arguments_cache.dataload_for(ast_node, field_defn, owner_object) do |resolved_arguments|
-              evaluate_selection_with_args(resolved_arguments, field_defn, ast_node, field_ast_nodes, owner_type, owner_object, is_eager_field, result_name, selections_result, parent_object, return_type, return_type_non_null)
+              evaluate_selection_with_args(resolved_arguments, field_defn, ast_node, field_ast_nodes, owner_type, owner_object, is_eager_field, result_name, selections_result, parent_object, return_type)
             end
           end
         end
 
-        def evaluate_selection_with_args(arguments, field_defn, ast_node, field_ast_nodes, owner_type, object, is_eager_field, result_name, selection_result, parent_object, return_type, return_type_non_null)  # rubocop:disable Metrics/ParameterLists
+        def evaluate_selection_with_args(arguments, field_defn, ast_node, field_ast_nodes, owner_type, object, is_eager_field, result_name, selection_result, parent_object, return_type)  # rubocop:disable Metrics/ParameterLists
           after_lazy(arguments, field: field_defn, ast_node: ast_node, owner_object: object, arguments: arguments, result_name: result_name, result: selection_result) do |resolved_arguments|
+            return_type_non_null = return_type.non_null?
             if resolved_arguments.is_a?(GraphQL::ExecutionError) || resolved_arguments.is_a?(GraphQL::UnauthorizedError)
               continue_value(resolved_arguments, owner_type, field_defn, return_type_non_null, ast_node, result_name, selection_result)
               next
@@ -553,7 +550,10 @@ def evaluate_selection_with_resolved_keyword_args(kwarg_arguments, resolved_argu
             after_lazy(app_result, field: field_defn, ast_node: ast_node, owner_object: object, arguments: resolved_arguments, result_name: result_name, result: selection_result) do |inner_result|
               continue_value = continue_value(inner_result, owner_type, field_defn, return_type_non_null, ast_node, result_name, selection_result)
               if HALT != continue_value
-                continue_field(continue_value, owner_type, field_defn, return_type, ast_node, next_selections, false, object, resolved_arguments, result_name, selection_result)
+                st = get_current_runtime_state
+                was_scoped = st.was_authorized_by_scope_items
+                st.was_authorized_by_scope_items = nil
+                continue_field(continue_value, owner_type, field_defn, return_type, ast_node, next_selections, false, object, resolved_arguments, result_name, selection_result, was_scoped)
               end
             end
           end
@@ -733,7 +733,7 @@ def continue_value(value, parent_type, field, is_non_null, ast_node, result_name
         # Location information from `path` and `ast_node`.
         #
         # @return [Lazy, Array, Hash, Object] Lazy, Array, and Hash are all traversed to resolve lazy values later
-        def continue_field(value, owner_type, field, current_type, ast_node, next_selections, is_non_null, owner_object, arguments, result_name, selection_result) # rubocop:disable Metrics/ParameterLists
+        def continue_field(value, owner_type, field, current_type, ast_node, next_selections, is_non_null, owner_object, arguments, result_name, selection_result, was_scoped) # rubocop:disable Metrics/ParameterLists
           if current_type.non_null?
             current_type = current_type.of_type
             is_non_null = true
@@ -767,12 +767,12 @@ def continue_field(value, owner_type, field, current_type, ast_node, next_select
                 set_result(selection_result, result_name, nil, false, is_non_null)
                 nil
               else
-                continue_field(resolved_value, owner_type, field, resolved_type, ast_node, next_selections, is_non_null, owner_object, arguments, result_name, selection_result)
+                continue_field(resolved_value, owner_type, field, resolved_type, ast_node, next_selections, is_non_null, owner_object, arguments, result_name, selection_result, was_scoped)
               end
             end
           when "OBJECT"
             object_proxy = begin
-              current_type.wrap(value, context)
+              was_scoped ? current_type.wrap_scoped(value, context) : current_type.wrap(value, context)
             rescue GraphQL::ExecutionError => err
               err
             end
@@ -838,10 +838,10 @@ def continue_field(value, owner_type, field, current_type, ast_node, next_select
                 idx += 1
                 if use_dataloader_job
                   @dataloader.append_job do
-                    resolve_list_item(inner_value, inner_type, inner_type_non_null, ast_node, field, owner_object, arguments, this_idx, response_list, next_selections, owner_type)
+                    resolve_list_item(inner_value, inner_type, inner_type_non_null, ast_node, field, owner_object, arguments, this_idx, response_list, next_selections, owner_type, was_scoped)
                   end
                 else
-                  resolve_list_item(inner_value, inner_type, inner_type_non_null, ast_node, field, owner_object, arguments, this_idx, response_list, next_selections, owner_type)
+                  resolve_list_item(inner_value, inner_type, inner_type_non_null, ast_node, field, owner_object, arguments, this_idx, response_list, next_selections, owner_type, was_scoped)
                 end
               end
 
@@ -872,7 +872,7 @@ def continue_field(value, owner_type, field, current_type, ast_node, next_select
           end
         end
 
-        def resolve_list_item(inner_value, inner_type, inner_type_non_null, ast_node, field, owner_object, arguments, this_idx, response_list, next_selections, owner_type) # rubocop:disable Metrics/ParameterLists
+        def resolve_list_item(inner_value, inner_type, inner_type_non_null, ast_node, field, owner_object, arguments, this_idx, response_list, next_selections, owner_type, was_scoped) # rubocop:disable Metrics/ParameterLists
           st = get_current_runtime_state
           st.current_result_name = this_idx
           st.current_result = response_list
@@ -881,7 +881,7 @@ def resolve_list_item(inner_value, inner_type, inner_type_non_null, ast_node, fi
             after_lazy(inner_value, ast_node: ast_node, field: field, owner_object: owner_object, arguments: arguments, result_name: this_idx, result: response_list) do |inner_inner_value|
               continue_value = continue_value(inner_inner_value, owner_type, field, inner_type_non_null, ast_node, this_idx, response_list)
               if HALT != continue_value
-                continue_field(continue_value, owner_type, field, inner_type, ast_node, next_selections, false, owner_object, arguments, this_idx, response_list)
+                continue_field(continue_value, owner_type, field, inner_type, ast_node, next_selections, false, owner_object, arguments, this_idx, response_list, was_scoped)
               end
             end
           end
@@ -961,13 +961,16 @@ def minimal_after_lazy(value, &block)
         def after_lazy(lazy_obj, field:, owner_object:, arguments:, ast_node:, result:, result_name:, eager: false, trace: true, &block)
           if lazy?(lazy_obj)
             orig_result = result
+            st = get_current_runtime_state
+            was_authorized_by_scope_items = st.was_authorized_by_scope_items
             lazy = GraphQL::Execution::Lazy.new(field: field) do
               st = get_current_runtime_state
               st.current_object = owner_object
               st.current_field = field
               st.current_arguments = arguments
               st.current_result_name = result_name
               st.current_result = orig_result
+              st.was_authorized_by_scope_items = was_authorized_by_scope_items
               # Wrap the execution of _this_ method with tracing,
               # but don't wrap the continuation below
               inner_obj = begin

lib/graphql/pagination/connection.rb

@@ -19,7 +19,14 @@ class PaginationImplementationMissingError < GraphQL::Error
       attr_reader :items
 
       # @return [GraphQL::Query::Context]
-      attr_accessor :context
+      attr_reader :context
+
+      def context=(new_ctx)
+        current_runtime_state = Thread.current[:__graphql_runtime_info]
+        query_runtime_state = current_runtime_state[new_ctx.query]
+        @was_authorized_by_scope_items = query_runtime_state.was_authorized_by_scope_items
+        @context = new_ctx
+      end
 
       # @return [Object] the object this collection belongs to
       attr_accessor :parent
@@ -83,6 +90,17 @@ def initialize(items, parent: nil, field: nil, context: nil, first: nil, after:
         else
           default_page_size
         end
+        @was_authorized_by_scope_items = if @context
+          current_runtime_state = Thread.current[:__graphql_runtime_info]
+          query_runtime_state = current_runtime_state[@context.query]
+          query_runtime_state.was_authorized_by_scope_items
+        else
+          nil
+        end
+      end
+
+      def was_authorized_by_scope_items?
+        @was_authorized_by_scope_items
       end
 
       def max_page_size=(new_value)
@@ -247,6 +265,10 @@ def parent
         def cursor
           @cursor ||= @connection.cursor_for(@node)
         end
+
+        def was_authorized_by_scope_items?
+          @connection.was_authorized_by_scope_items?
+        end
       end
     end
   end

lib/graphql/schema/field/scope_extension.rb

@@ -10,7 +10,13 @@ def after_resolve(object:, arguments:, context:, value:, memo:)
           else
             ret_type = @field.type.unwrap
             if ret_type.respond_to?(:scope_items)
-              ret_type.scope_items(value, context)
+              scoped_items = ret_type.scope_items(value, context)
+              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
+              scoped_items
             else
               value
             end

lib/graphql/schema/member/scoped.rb

@@ -15,6 +15,25 @@ module Scoped
         def scope_items(items, context)
           items
         end
+
+        def reauthorize_scoped_objects(new_value = nil)
+          if new_value.nil?
+            if @reauthorize_scoped_objects != nil
+              @reauthorize_scoped_objects
+            else
+              find_inherited_value(:reauthorize_scoped_objects, nil)
+            end
+          else
+            @reauthorize_scoped_objects = new_value
+          end
+        end
+
+        def inherited(subclass)
+          super
+          subclass.class_eval do
+            @reauthorize_scoped_objects = nil
+          end
+        end
       end
     end
   end

lib/graphql/schema/object.rb

@@ -30,6 +30,10 @@ class << self
         # @see authorized_new to make instances
         protected :new
 
+        def wrap_scoped(object, context)
+          scoped_new(object, context)
+        end
+
         # This is called by the runtime to return an object to call methods on.
         def wrap(object, context)
           authorized_new(object, context)
@@ -91,6 +95,10 @@ def authorized_new(object, context)
             end
           end
         end
+
+        def scoped_new(object, context)
+          self.new(object, context)
+        end
       end
 
       def initialize(object, context)

lib/graphql/types/relay/connection_behaviors.rb

@@ -67,9 +67,8 @@ def edge_type(edge_type_class, edge_class: GraphQL::Pagination::Connection::Edge
               type: [edge_type_class, null: edge_nullable],
               null: edges_nullable,
               description: "A list of edges.",
+              scope: false, # Assume that the connection was already scoped.
               connection: false,
-              # Assume that the connection was scoped before this step:
-              scope: false,
             }
 
             if field_options
@@ -170,6 +169,24 @@ def add_page_info_field(obj_type)
             obj_type.field :page_info, GraphQL::Types::Relay::PageInfo, null: false, description: "Information to aid in pagination."
           end
         end
+
+        def edges
+          # Assume that whatever authorization needed to happen
+          # already happened at the connection level.
+          current_runtime_state = Thread.current[:__graphql_runtime_info]
+          query_runtime_state = current_runtime_state[context.query]
+          query_runtime_state.was_authorized_by_scope_items = @object.was_authorized_by_scope_items?
+          @object.edges
+        end
+
+        def nodes
+          # Assume that whatever authorization needed to happen
+          # already happened at the connection level.
+          current_runtime_state = Thread.current[:__graphql_runtime_info]
+          query_runtime_state = current_runtime_state[context.query]
+          query_runtime_state.was_authorized_by_scope_items = @object.was_authorized_by_scope_items?
+          @object.nodes
+        end
       end
     end
   end

lib/graphql/types/relay/edge_behaviors.rb

@@ -12,6 +12,13 @@ def self.included(child_class)
           child_class.node_nullable(true)
         end
 
+        def node
+          current_runtime_state = Thread.current[:__graphql_runtime_info]
+          query_runtime_state = current_runtime_state[context.query]
+          query_runtime_state.was_authorized_by_scope_items = @object.was_authorized_by_scope_items?
+          @object.node
+        end
+
         module ClassMethods
           def inherited(child_class)
             super