Merge branch '58404-set-default-max-depth-for-GraphQL' into 'master'

58404 - setup max depth for graphql

Closes #58404

See merge request gitlab-org/gitlab-ce!25737

app/graphql/gitlab_schema.rb

@@ -7,6 +7,9 @@ class GitlabSchema < GraphQL::Schema
   AUTHENTICATED_COMPLEXITY = 250
   ADMIN_COMPLEXITY         = 300
 
+  ANONYMOUS_MAX_DEPTH = 10
+  AUTHENTICATED_MAX_DEPTH = 15
+
   use BatchLoader::GraphQL
   use Gitlab::Graphql::Authorize
   use Gitlab::Graphql::Present
@@ -23,21 +26,36 @@ class GitlabSchema < GraphQL::Schema
 
   mutation(Types::MutationType)
 
-  def self.execute(query_str = nil, **kwargs)
-    kwargs[:max_complexity] ||= max_query_complexity(kwargs[:context])
+  class << self
+    def execute(query_str = nil, **kwargs)
+      kwargs[:max_complexity] ||= max_query_complexity(kwargs[:context])
+      kwargs[:max_depth] ||= max_query_depth(kwargs[:context])
 
-    super(query_str, **kwargs)
-  end
+      super(query_str, **kwargs)
+    end
+
+    private
+
+    def max_query_complexity(ctx)
+      current_user = ctx&.fetch(:current_user, nil)
+
+      if current_user&.admin
+        ADMIN_COMPLEXITY
+      elsif current_user
+        AUTHENTICATED_COMPLEXITY
+      else
+        DEFAULT_MAX_COMPLEXITY
+      end
+    end
 
-  def self.max_query_complexity(ctx)
-    current_user = ctx&.fetch(:current_user, nil)
+    def max_query_depth(ctx)
+      current_user = ctx&.fetch(:current_user, nil)
 
-    if current_user&.admin
-      ADMIN_COMPLEXITY
-    elsif current_user
-      AUTHENTICATED_COMPLEXITY
-    else
-      DEFAULT_MAX_COMPLEXITY
+      if current_user
+        AUTHENTICATED_MAX_DEPTH
+      else
+        ANONYMOUS_MAX_DEPTH
+      end
     end
   end
 end

changelogs/unreleased/58404-set-default-max-depth-for-GraphQL.yml

+---
+title: 58404 - setup max depth for GraphQL
+merge_request: 25737
+author: Ken Ding
+type: added

spec/graphql/gitlab_schema_spec.rb

@@ -3,6 +3,8 @@
 require 'spec_helper'
 
 describe GitlabSchema do
+  let(:user) { build :user }
+
   it 'uses batch loading' do
     expect(field_instrumenters).to include(BatchLoader::GraphQL)
   end
@@ -33,43 +35,75 @@ describe GitlabSchema do
     expect(connection).to eq(Gitlab::Graphql::Connections::KeysetConnection)
   end
 
-  context 'for different types of users' do
-    it 'returns DEFAULT_MAX_COMPLEXITY for no context' do
-      expect(GraphQL::Schema)
-        .to receive(:execute)
-        .with('query', hash_including(max_complexity: GitlabSchema::DEFAULT_MAX_COMPLEXITY))
+  describe '.execute' do
+    context 'for different types of users' do
+      context 'when no context' do
+        it 'returns DEFAULT_MAX_COMPLEXITY' do
+          expect(GraphQL::Schema)
+            .to receive(:execute)
+            .with('query', hash_including(max_complexity: GitlabSchema::DEFAULT_MAX_COMPLEXITY))
 
-      described_class.execute('query')
-    end
+          described_class.execute('query')
+        end
+      end
 
-    it 'returns DEFAULT_MAX_COMPLEXITY for no user' do
-      expect(GraphQL::Schema)
-        .to receive(:execute)
-        .with('query', hash_including(max_complexity: GitlabSchema::DEFAULT_MAX_COMPLEXITY))
+      context 'when no user' do
+        it 'returns DEFAULT_MAX_COMPLEXITY' do
+          expect(GraphQL::Schema)
+            .to receive(:execute)
+            .with('query', hash_including(max_complexity: GitlabSchema::DEFAULT_MAX_COMPLEXITY))
 
-      described_class.execute('query', context: {})
-    end
+          described_class.execute('query', context: {})
+        end
 
-    it 'returns AUTHENTICATED_COMPLEXITY for a logged in user' do
-      user = build :user
+        it 'returns ANONYMOUS_MAX_DEPTH' do
+          expect(GraphQL::Schema)
+            .to receive(:execute)
+            .with('query', hash_including(max_depth: GitlabSchema::ANONYMOUS_MAX_DEPTH))
 
-      expect(GraphQL::Schema).to receive(:execute).with('query', hash_including(max_complexity: GitlabSchema::AUTHENTICATED_COMPLEXITY))
+          described_class.execute('query', context: {})
+        end
+      end
 
-      described_class.execute('query', context: { current_user: user })
-    end
+      context 'when a logged in user' do
+        it 'returns AUTHENTICATED_COMPLEXITY' do
+          expect(GraphQL::Schema).to receive(:execute).with('query', hash_including(max_complexity: GitlabSchema::AUTHENTICATED_COMPLEXITY))
 
-    it 'returns ADMIN_COMPLEXITY for an admin user' do
-      user = build :user, :admin
+          described_class.execute('query', context: { current_user: user })
+        end
 
-      expect(GraphQL::Schema).to receive(:execute).with('query', hash_including(max_complexity: GitlabSchema::ADMIN_COMPLEXITY))
+        it 'returns AUTHENTICATED_MAX_DEPTH' do
+          expect(GraphQL::Schema).to receive(:execute).with('query', hash_including(max_depth: GitlabSchema::AUTHENTICATED_MAX_DEPTH))
 
-      described_class.execute('query', context: { current_user: user })
-    end
+          described_class.execute('query', context: { current_user: user })
+        end
+      end
+
+      context 'when an admin user' do
+        it 'returns ADMIN_COMPLEXITY' do
+          user = build :user, :admin
+
+          expect(GraphQL::Schema).to receive(:execute).with('query', hash_including(max_complexity: GitlabSchema::ADMIN_COMPLEXITY))
+
+          described_class.execute('query', context: { current_user: user })
+        end
+      end
+
+      context 'when max_complexity passed on the query' do
+        it 'returns what was passed on the query' do
+          expect(GraphQL::Schema).to receive(:execute).with('query', hash_including(max_complexity: 1234))
+
+          described_class.execute('query', max_complexity: 1234)
+        end
+      end
 
-    it 'returns what was passed on the query' do
-      expect(GraphQL::Schema).to receive(:execute).with('query', { max_complexity: 1234 })
+      context 'when max_depth passed on the query' do
+        it 'returns what was passed on the query' do
+          expect(GraphQL::Schema).to receive(:execute).with('query', hash_including(max_depth: 1234))
 
-      described_class.execute('query', max_complexity: 1234)
+          described_class.execute('query', max_depth: 1234)
+        end
+      end
     end
   end
 

spec/requests/api/graphql/gitlab_schema_spec.rb

@@ -3,15 +3,43 @@ require 'spec_helper'
 describe 'GitlabSchema configurations' do
   include GraphqlHelpers
 
-  it 'shows an error if complexity is too high' do
-    project = create(:project, :repository)
-    query   = graphql_query_for('project', { 'fullPath' => project.full_path }, %w(id name description))
+  let(:project) { create(:project, :repository) }
+  let(:query) { graphql_query_for('project', { 'fullPath' => project.full_path }, %w(id name description)) }
+  let(:current_user) { create(:user) }
 
-    allow(GitlabSchema).to receive(:max_query_complexity).and_return 1
+  describe '#max_complexity' do
+    context 'when complexity is too high' do
+      it 'shows an error' do
+        allow(GitlabSchema).to receive(:max_query_complexity).and_return 1
 
-    post_graphql(query, current_user: nil)
+        post_graphql(query, current_user: nil)
 
-    expect(graphql_errors.first['message']).to include('which exceeds max complexity of 1')
+        expect(graphql_errors.first['message']).to include('which exceeds max complexity of 1')
+      end
+    end
+  end
+
+  describe '#max_depth' do
+    context 'when query depth is too high' do
+      it 'shows error' do
+        errors = [{ "message" => "Query has depth of 2, which exceeds max depth of 1" }]
+        allow(GitlabSchema).to receive(:max_query_depth).and_return 1
+
+        post_graphql(query)
+
+        expect(graphql_errors).to eq(errors)
+      end
+    end
+
+    context 'when query depth is within range' do
+      it 'has no error' do
+        allow(GitlabSchema).to receive(:max_query_depth).and_return 5
+
+        post_graphql(query)
+
+        expect(graphql_errors).to be_nil
+      end
+    end
   end
 
   context 'when IntrospectionQuery' do

spec/support/helpers/graphql_helpers.rb

@@ -102,6 +102,7 @@ module GraphqlHelpers
 
   def all_graphql_fields_for(class_name, parent_types = Set.new)
     allow_unlimited_graphql_complexity
+    allow_unlimited_graphql_depth
 
     type = GitlabSchema.types[class_name.to_s]
     return "" unless type
@@ -190,4 +191,9 @@ module GraphqlHelpers
     allow_any_instance_of(GitlabSchema).to receive(:max_complexity).and_return nil
     allow(GitlabSchema).to receive(:max_query_complexity).with(any_args).and_return nil
   end
+
+  def allow_unlimited_graphql_depth
+    allow_any_instance_of(GitlabSchema).to receive(:max_depth).and_return nil
+    allow(GitlabSchema).to receive(:max_query_depth).with(any_args).and_return nil
+  end
 end