Initial field and query complexity limits
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.
This commit is contained in:
parent
815901e322
commit
f458c56107
8 changed files with 137 additions and 0 deletions
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -0,0 +1,5 @@
|
|||
---
|
||||
title: Add initial complexity limits to GraphQL queries
|
||||
merge_request: 26629
|
||||
author:
|
||||
type: performance
|
18
lib/gitlab/graphql/query_analyzers/log_query_complexity.rb
Normal file
18
lib/gitlab/graphql/query_analyzers/log_query_complexity.rb
Normal file
|
@ -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
|
|
@ -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
|
||||
|
|
19
spec/graphql/types/base_field_spec.rb
Normal file
19
spec/graphql/types/base_field_spec.rb
Normal file
|
@ -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
|
16
spec/requests/api/graphql/gitlab_schema_spec.rb
Normal file
16
spec/requests/api/graphql/gitlab_schema_spec.rb
Normal file
|
@ -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
|
|
@ -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
|
||||
|
|
Loading…
Reference in a new issue