summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--app/graphql/gitlab_schema.rb42
-rw-r--r--changelogs/unreleased/58404-set-default-max-depth-for-GraphQL.yml5
-rw-r--r--spec/graphql/gitlab_schema_spec.rb86
-rw-r--r--spec/requests/api/graphql/gitlab_schema_spec.rb40
-rw-r--r--spec/support/helpers/graphql_helpers.rb6
5 files changed, 135 insertions, 44 deletions
diff --git a/app/graphql/gitlab_schema.rb b/app/graphql/gitlab_schema.rb
index a12568d5d31..897e12c1b56 100644
--- a/app/graphql/gitlab_schema.rb
+++ b/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
diff --git a/changelogs/unreleased/58404-set-default-max-depth-for-GraphQL.yml b/changelogs/unreleased/58404-set-default-max-depth-for-GraphQL.yml
new file mode 100644
index 00000000000..7e95158a0e0
--- /dev/null
+++ b/changelogs/unreleased/58404-set-default-max-depth-for-GraphQL.yml
@@ -0,0 +1,5 @@
+---
+title: 58404 - setup max depth for GraphQL
+merge_request: 25737
+author: Ken Ding
+type: added
diff --git a/spec/graphql/gitlab_schema_spec.rb b/spec/graphql/gitlab_schema_spec.rb
index 05f10fb40f0..c138c87c4ac 100644
--- a/spec/graphql/gitlab_schema_spec.rb
+++ b/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
diff --git a/spec/requests/api/graphql/gitlab_schema_spec.rb b/spec/requests/api/graphql/gitlab_schema_spec.rb
index b63b4fb34df..dd518274f82 100644
--- a/spec/requests/api/graphql/gitlab_schema_spec.rb
+++ b/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
diff --git a/spec/support/helpers/graphql_helpers.rb b/spec/support/helpers/graphql_helpers.rb
index b49d743fb9a..f15944652fd 100644
--- a/spec/support/helpers/graphql_helpers.rb
+++ b/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
View Lorry Controller Status