Merge branch 'ajk-graphql-lazy-auth' into 'master'

Lazy authorization for GraphQL types

See merge request gitlab-org/gitlab!45263

.gitignore

@@ -101,3 +101,5 @@ apollo.config.js
 /tmp/matching_tests.txt
 ee/changelogs/unreleased-ee
 /sitespeed-result
+tags.lock
+tags.temp

app/graphql/gitlab_schema.rb

@@ -30,6 +30,8 @@ class GitlabSchema < GraphQL::Schema
 
   default_max_page_size 100
 
+  lazy_resolve ::Gitlab::Graphql::Lazy, :force
+
   class << self
     def multiplex(queries, **kwargs)
       kwargs[:max_complexity] ||= max_query_complexity(kwargs[:context])

bin/feature-flag

@@ -104,7 +104,7 @@ class FeatureFlagOptionParser
       end
 
       # Name is a first name
-      options.name = argv.first
+      options.name = argv.first.downcase.gsub(/-/, '_')
 
       options
     end

config/feature_flags/development/graphql_lazy_authorization.yml

+---
+name: graphql_lazy_authorization
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/45263
+rollout_issue_url: 
+type: development
+group: group::plan
+default_enabled: false

lib/gitlab/graphql/authorize/authorize_field_service.rb

@@ -46,6 +46,8 @@ module Gitlab
 
         # Returns any authorize metadata from @field
         def field_authorizations
+          return [] if @field.metadata[:authorize] == true
+
           Array.wrap(@field.metadata[:authorize])
         end
 
@@ -54,7 +56,7 @@ module Gitlab
             # The field is a built-in/scalar type, or a list of scalars
             # authorize using the parent's object
             parent_typed_object.object
-          elsif @field.connection? || resolved_type.is_a?(Array)
+          elsif @field.connection? || @field.type.list? || resolved_type.is_a?(Array)
             # The field is a connection or a list of non-built-in types, we'll
             # authorize each element when rendering
             nil
@@ -75,16 +77,25 @@ module Gitlab
             # no need to do anything
           elsif authorizing_object
             # Authorizing fields representing scalars, or a simple field with an object
-            resolved_type if allowed_access?(current_user, authorizing_object)
+            ::Gitlab::Graphql::Lazy.with_value(authorizing_object) do |object|
+              resolved_type if allowed_access?(current_user, object)
+            end
           elsif @field.connection?
-            # A connection with pagination, modify the visible nodes on the
-            # connection type in place
-            resolved_type.object.edge_nodes.to_a.keep_if { |node| allowed_access?(current_user, node) }
-            resolved_type
-          elsif resolved_type.is_a? Array
+            ::Gitlab::Graphql::Lazy.with_value(resolved_type) do |type|
+              # A connection with pagination, modify the visible nodes on the
+              # connection type in place
+              nodes = to_nodes(type)
+              nodes.keep_if { |node| allowed_access?(current_user, node) } if nodes
+              type
+            end
+          elsif @field.type.list? || resolved_type.is_a?(Array)
             # A simple list of rendered types  each object being an object to authorize
-            resolved_type.select do |single_object_type|
-              allowed_access?(current_user, realized(single_object_type).object)
+            ::Gitlab::Graphql::Lazy.with_value(resolved_type) do |items|
+              items.select do |single_object_type|
+                object_type = realized(single_object_type)
+                object = object_type.try(:object) || object_type
+                allowed_access?(current_user, object)
+              end
             end
           else
             raise "Can't authorize #{@field}"
@@ -93,18 +104,23 @@ module Gitlab
 
         # Ensure that we are dealing with realized objects, not delayed promises
         def realized(thing)
-          case thing
-          when BatchLoader::GraphQL
-            thing.sync
-          when GraphQL::Execution::Lazy
-            thing.value # part of the private api, but we need to unwrap it here.
+          ::Gitlab::Graphql::Lazy.force(thing)
+        end
+
+        # Try to get the connection
+        # can be at type.object or at type
+        def to_nodes(type)
+          if type.respond_to?(:nodes)
+            type.nodes
+          elsif type.respond_to?(:object)
+            to_nodes(type.object)
           else
-            thing
+            nil
           end
         end
 
         def allowed_access?(current_user, object)
-          object = object.sync if object.respond_to?(:sync)
+          object = realized(object)
 
           authorizations.all? do |ability|
             Ability.allowed?(current_user, ability, object)

lib/gitlab/graphql/lazy.rb

@@ -3,17 +3,45 @@
 module Gitlab
   module Graphql
     class Lazy
+      include Gitlab::Utils::StrongMemoize
+
+      def initialize(&block)
+        @proc = block
+      end
+
+      def force
+        strong_memoize(:force) { self.class.force(@proc.call) }
+      end
+
+      def then(&block)
+        self.class.new { yield force }
+      end
+
       # Force evaluation of a (possibly) lazy value
       def self.force(value)
         case value
+        when ::Gitlab::Graphql::Lazy
+          value.force
         when ::BatchLoader::GraphQL
           value.sync
+        when ::GraphQL::Execution::Lazy
+          value.value # part of the private api, but we can force this as well
         when ::Concurrent::Promise
-          value.execute.value
+          value.execute if value.state == :unscheduled
+
+          value.value # value.value(10.seconds)
         else
           value
         end
       end
+
+      def self.with_value(unforced, &block)
+        if Feature.enabled?(:graphql_lazy_authorization)
+          self.new { unforced }.then(&block)
+        else
+          block.call(unforced)
+        end
+      end
     end
   end
 end

spec/bin/feature_flag_spec.rb

@@ -13,7 +13,7 @@ RSpec.describe 'bin/feature-flag' do
     let(:options) { FeatureFlagOptionParser.parse(argv) }
     let(:creator) { described_class.new(options) }
     let(:existing_flags) do
-      { 'existing-feature-flag' => File.join('config', 'feature_flags', 'development', 'existing-feature-flag.yml') }
+      { 'existing_feature_flag' => File.join('config', 'feature_flags', 'development', 'existing_feature_flag.yml') }
     end
 
     before do
@@ -32,12 +32,12 @@ RSpec.describe 'bin/feature-flag' do
 
     it 'properly creates a feature flag' do
       expect(File).to receive(:write).with(
-        File.join('config', 'feature_flags', 'development', 'feature-flag-name.yml'),
+        File.join('config', 'feature_flags', 'development', 'feature_flag_name.yml'),
         anything)
 
       expect do
         subject
-      end.to output(/name: feature-flag-name/).to_stdout
+      end.to output(/name: feature_flag_name/).to_stdout
     end
 
     context 'when running on master' do

spec/lib/gitlab/graphql/lazy_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::Graphql::Lazy do
+  def load(key)
+    BatchLoader.for(key).batch do |keys, loader|
+      keys.each { |x| loader.call(x, x * x) }
+    end
+  end
+
+  let(:value) { double(x: 1) }
+
+  describe '#force' do
+    subject { described_class.new { value.x } }
+
+    it 'can extract the value' do
+      expect(subject.force).to be 1
+    end
+
+    it 'can derive new lazy values' do
+      expect(subject.then { |x| x + 2 }.force).to be 3
+    end
+
+    it 'only evaluates once' do
+      expect(value).to receive(:x).once
+
+      expect(subject.force).to eq(subject.force)
+    end
+
+    it 'deals with nested laziness' do
+      expect(described_class.new { load(10) }.force).to eq(100)
+      expect(described_class.new { described_class.new { 5 } }.force).to eq 5
+    end
+  end
+
+  describe '.with_value' do
+    let(:inner) { described_class.new { value.x } }
+
+    subject { described_class.with_value(inner) { |x| x.to_s } }
+
+    it 'defers the application of a block to a value' do
+      expect(value).not_to receive(:x)
+
+      expect(subject).to be_an_instance_of(described_class)
+    end
+
+    it 'evaluates to the application of the block to the value' do
+      expect(value).to receive(:x).once
+
+      expect(subject.force).to eq(inner.force.to_s)
+    end
+  end
+
+  describe '.force' do
+    context 'when given a plain value' do
+      subject { described_class.force(1) }
+
+      it 'unwraps the value' do
+        expect(subject).to be 1
+      end
+    end
+
+    context 'when given a wrapped lazy value' do
+      subject { described_class.force(described_class.new { 2 }) }
+
+      it 'unwraps the value' do
+        expect(subject).to be 2
+      end
+    end
+
+    context 'when the value is from a batchloader' do
+      subject { described_class.force(load(3)) }
+
+      it 'syncs the value' do
+        expect(subject).to be 9
+      end
+    end
+
+    context 'when the value is a GraphQL lazy' do
+      subject { described_class.force(GitlabSchema.after_lazy(load(3)) { |x| x + 1 } ) }
+
+      it 'forces the evaluation' do
+        expect(subject).to be 10
+      end
+    end
+
+    context 'when the value is a promise' do
+      subject { described_class.force(::Concurrent::Promise.new { 4 }) }
+
+      it 'executes the promise and waits for the value' do
+        expect(subject).to be 4
+      end
+    end
+  end
+end

spec/support/helpers/graphql_helpers.rb

@@ -478,6 +478,8 @@ module GraphqlHelpers
       use Gitlab::Graphql::Authorize
       use Gitlab::Graphql::Pagination::Connections
 
+      lazy_resolve ::Gitlab::Graphql::Lazy, :force
+
       query(query_type)
     end
 

spec/support/shared_examples/graphql/label_fields.rb

@@ -106,13 +106,11 @@ RSpec.shared_examples 'querying a GraphQL type with labels' do
     end
 
     it 'batches queries for labels by title' do
-      pending('See: https://gitlab.com/gitlab-org/gitlab/-/issues/217767')
-
       multi_selection = query_for(label_b, label_c)
       single_selection = query_for(label_d)
 
       expect { run_query(multi_selection) }
-        .to issue_same_number_of_queries_as { run_query(single_selection) }
+        .to issue_same_number_of_queries_as { run_query(single_selection) }.ignoring_cached_queries
     end
   end