summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorBrett Walker <bwalker@gitlab.com>2019-03-27 15:02:25 -0500
committerBrett Walker <bwalker@gitlab.com>2019-04-04 08:39:30 -0500
commitf458c561070d754cd546b07caf60dfa7ffb06293 (patch)
treeef4c65fb5b6767030c0c8b88223f415eabfe88be
parent815901e322b60d28983f52a7ce5e98555285bef8 (diff)
downloadgitlab-ce-58405-basic-limiting-complexity-of-graphql-queries.tar.gz
Initial field and query complexity limits58405-basic-limiting-complexity-of-graphql-queries
It makes all Types::BaseField default to a complexity of 1. Queries themselves now have limited complexity, scaled to the type of user: no user, authenticated user, or an admin user.
-rw-r--r--app/graphql/gitlab_schema.rb30
-rw-r--r--app/graphql/types/base_field.rb9
-rw-r--r--changelogs/unreleased/58405-basic-limiting-complexity-of-graphql-queries.yml5
-rw-r--r--lib/gitlab/graphql/query_analyzers/log_query_complexity.rb18
-rw-r--r--spec/graphql/gitlab_schema_spec.rb32
-rw-r--r--spec/graphql/types/base_field_spec.rb19
-rw-r--r--spec/requests/api/graphql/gitlab_schema_spec.rb16
-rw-r--r--spec/support/helpers/graphql_helpers.rb8
8 files changed, 137 insertions, 0 deletions
diff --git a/app/graphql/gitlab_schema.rb b/app/graphql/gitlab_schema.rb
index 06d26309b5b..ff4d0611da9 100644
--- a/app/graphql/gitlab_schema.rb
+++ b/app/graphql/gitlab_schema.rb
@@ -1,13 +1,43 @@
# frozen_string_literal: true
class GitlabSchema < GraphQL::Schema
+ # Took our current most complicated query in use, issues.graphql,
+ # with a complexity of 19, and added a 20 point buffer to it.
+ # These values will evolve over time.
+ DEFAULT_MAX_COMPLEXITY = 40
+ AUTHENTICATED_COMPLEXITY = 50
+ ADMIN_COMPLEXITY = 60
+
use BatchLoader::GraphQL
use Gitlab::Graphql::Authorize
use Gitlab::Graphql::Present
use Gitlab::Graphql::Connections
+ query_analyzer Gitlab::Graphql::QueryAnalyzers::LogQueryComplexity.analyzer
+
query(Types::QueryType)
default_max_page_size 100
+
+ max_complexity DEFAULT_MAX_COMPLEXITY
+
mutation(Types::MutationType)
+
+ def self.execute(query_str = nil, **kwargs)
+ kwargs[:max_complexity] ||= max_query_complexity(kwargs[:context])
+
+ super(query_str, **kwargs)
+ end
+
+ def self.max_query_complexity(ctx)
+ current_user = ctx&.fetch(:current_user)
+
+ if current_user&.admin
+ ADMIN_COMPLEXITY
+ elsif current_user
+ AUTHENTICATED_COMPLEXITY
+ else
+ DEFAULT_MAX_COMPLEXITY
+ end
+ end
end
diff --git a/app/graphql/types/base_field.rb b/app/graphql/types/base_field.rb
index 2b2ea64c00b..8c8b8a82d3e 100644
--- a/app/graphql/types/base_field.rb
+++ b/app/graphql/types/base_field.rb
@@ -3,5 +3,14 @@
module Types
class BaseField < GraphQL::Schema::Field
prepend Gitlab::Graphql::Authorize
+
+ DEFAULT_COMPLEXITY = 1
+
+ def initialize(*args, **kwargs, &block)
+ # complexity is already defaulted to 1, but let's make it explicit
+ kwargs[:complexity] ||= DEFAULT_COMPLEXITY
+
+ super(*args, **kwargs, &block)
+ end
end
end
diff --git a/changelogs/unreleased/58405-basic-limiting-complexity-of-graphql-queries.yml b/changelogs/unreleased/58405-basic-limiting-complexity-of-graphql-queries.yml
new file mode 100644
index 00000000000..058a120b500
--- /dev/null
+++ b/changelogs/unreleased/58405-basic-limiting-complexity-of-graphql-queries.yml
@@ -0,0 +1,5 @@
+---
+title: Add initial complexity limits to GraphQL queries
+merge_request: 26629
+author:
+type: performance
diff --git a/lib/gitlab/graphql/query_analyzers/log_query_complexity.rb b/lib/gitlab/graphql/query_analyzers/log_query_complexity.rb
new file mode 100644
index 00000000000..95db130dbfc
--- /dev/null
+++ b/lib/gitlab/graphql/query_analyzers/log_query_complexity.rb
@@ -0,0 +1,18 @@
+# frozen_string_literal: true
+
+module Gitlab
+ module Graphql
+ module QueryAnalyzers
+ class LogQueryComplexity
+ class << self
+ def analyzer
+ GraphQL::Analysis::QueryComplexity.new do |query, complexity|
+ # temporary until https://gitlab.com/gitlab-org/gitlab-ce/issues/59587
+ Rails.logger.info("[GraphQL Query Complexity] #{complexity} | admin? #{query.context[:current_user]&.admin?}")
+ end
+ end
+ end
+ end
+ end
+ end
+end
diff --git a/spec/graphql/gitlab_schema_spec.rb b/spec/graphql/gitlab_schema_spec.rb
index b9ddb427e85..a535d9cdc7e 100644
--- a/spec/graphql/gitlab_schema_spec.rb
+++ b/spec/graphql/gitlab_schema_spec.rb
@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
require 'spec_helper'
describe GitlabSchema do
@@ -31,6 +33,36 @@ 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 user' do
+ expect(GraphQL::Schema).to receive(:execute).with('query', hash_including(max_complexity: GitlabSchema::DEFAULT_MAX_COMPLEXITY))
+
+ described_class.execute('query')
+ end
+
+ it 'returns AUTHENTICATED_COMPLEXITY for a logged in user' do
+ user = build :user
+
+ 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
+ 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
+
+ it 'returns what was passed on the query' do
+ expect(GraphQL::Schema).to receive(:execute).with('query', { max_complexity: 1234 })
+
+ described_class.execute('query', max_complexity: 1234)
+ end
+ end
+
def field_instrumenters
described_class.instrumenters[:field]
end
diff --git a/spec/graphql/types/base_field_spec.rb b/spec/graphql/types/base_field_spec.rb
new file mode 100644
index 00000000000..b5697ee5245
--- /dev/null
+++ b/spec/graphql/types/base_field_spec.rb
@@ -0,0 +1,19 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Types::BaseField do
+ context 'when considering complexity' do
+ it 'defaults to 1' do
+ field = described_class.new(name: 'test', type: GraphQL::STRING_TYPE, null: true)
+
+ expect(field.to_graphql.complexity).to eq 1
+ end
+
+ it 'has specified value' do
+ field = described_class.new(name: 'test', type: GraphQL::STRING_TYPE, null: true, complexity: 12)
+
+ expect(field.to_graphql.complexity).to eq 12
+ end
+ end
+end
diff --git a/spec/requests/api/graphql/gitlab_schema_spec.rb b/spec/requests/api/graphql/gitlab_schema_spec.rb
new file mode 100644
index 00000000000..708a000532b
--- /dev/null
+++ b/spec/requests/api/graphql/gitlab_schema_spec.rb
@@ -0,0 +1,16 @@
+require 'spec_helper'
+
+describe 'GitlabSchema configurations' do
+ include GraphqlHelpers
+
+ let(:project) { create(:project, :repository) }
+ let!(:query) { graphql_query_for('project', 'fullPath' => project.full_path) }
+
+ it 'shows an error if complexity it too high' 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
diff --git a/spec/support/helpers/graphql_helpers.rb b/spec/support/helpers/graphql_helpers.rb
index ca28325eab9..f59f42ee902 100644
--- a/spec/support/helpers/graphql_helpers.rb
+++ b/spec/support/helpers/graphql_helpers.rb
@@ -93,6 +93,8 @@ module GraphqlHelpers
end
def all_graphql_fields_for(class_name, parent_types = Set.new)
+ allow_unlimited_graphql_complexity
+
type = GitlabSchema.types[class_name.to_s]
return "" unless type
@@ -170,4 +172,10 @@ module GraphqlHelpers
field_type
end
+
+ # for most tests, we want to allow unlimited complexity
+ def allow_unlimited_graphql_complexity
+ 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
end