58404 - setup max depth for graphql

58404 - add change log


58404 - add spec


58404 - add more spec to test depth 2


58404 - fix spec


58404 - fix rubocop


58404 - refactor the code by Bob's advice


58404 - revert changes of all_graphql_fields_for


58404 - change text only


58404 - fix rspec according to gitlab's standard


58404 - revert previous spec


58404 - fix rubocop

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,13 +26,17 @@ class GitlabSchema < GraphQL::Schema
 
   mutation(Types::MutationType)
 
-  def self.execute(query_str = nil, **kwargs)
+  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
 
-  def self.max_query_complexity(ctx)
+    private
+
+    def max_query_complexity(ctx)
       current_user = ctx&.fetch(:current_user, nil)
 
       if current_user&.admin
@@ -40,4 +47,15 @@ class GitlabSchema < GraphQL::Schema
         DEFAULT_MAX_COMPLEXITY
       end
     end
+
+    def max_query_depth(ctx)
+      current_user = ctx&.fetch(:current_user, nil)
+
+      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,16 +35,20 @@ describe GitlabSchema do
     expect(connection).to eq(Gitlab::Graphql::Connections::KeysetConnection)
   end
 
+  describe '.execute' do
     context 'for different types of users' do
-    it 'returns DEFAULT_MAX_COMPLEXITY for no context' 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
+      end
 
-    it 'returns DEFAULT_MAX_COMPLEXITY for no user' do
+      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))
@@ -50,29 +56,57 @@ describe GitlabSchema do
           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))
 
+          described_class.execute('query', context: {})
+        end
+      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))
 
           described_class.execute('query', context: { current_user: user })
         end
 
-    it 'returns ADMIN_COMPLEXITY for an admin user' do
+        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
+      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', { max_complexity: 1234 })
+          expect(GraphQL::Schema).to receive(:execute).with('query', hash_including(max_complexity: 1234))
 
           described_class.execute('query', max_complexity: 1234)
         end
       end
 
+      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_depth: 1234)
+        end
+      end
+    end
+  end
+
   def field_instrumenters
     described_class.instrumenters[:field] + described_class.instrumenters[:field_after_built_ins]
   end

spec/requests/api/graphql/gitlab_schema_spec.rb

@@ -3,16 +3,44 @@ 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) }
 
+  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)
 
         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
     it 'is not too complex' 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