Commit 520c120f authored by Rémy Coutable's avatar Rémy Coutable

Merge branch '58405-basic-limiting-complexity-of-graphql-queries' into 'master'

Basic limiting complexity of GraphQL queries

Closes #58405

See merge request gitlab-org/gitlab-ce!26629
parents 94e6cc52 f458c561
# frozen_string_literal: true # frozen_string_literal: true
class GitlabSchema < GraphQL::Schema 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 BatchLoader::GraphQL
use Gitlab::Graphql::Authorize use Gitlab::Graphql::Authorize
use Gitlab::Graphql::Present use Gitlab::Graphql::Present
use Gitlab::Graphql::Connections use Gitlab::Graphql::Connections
use Gitlab::Graphql::Tracing use Gitlab::Graphql::Tracing
query_analyzer Gitlab::Graphql::QueryAnalyzers::LogQueryComplexity.analyzer
query(Types::QueryType) query(Types::QueryType)
default_max_page_size 100 default_max_page_size 100
max_complexity DEFAULT_MAX_COMPLEXITY
mutation(Types::MutationType) 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 end
...@@ -3,5 +3,14 @@ ...@@ -3,5 +3,14 @@
module Types module Types
class BaseField < GraphQL::Schema::Field class BaseField < GraphQL::Schema::Field
prepend Gitlab::Graphql::Authorize 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
end end
---
title: Add initial complexity limits to GraphQL queries
merge_request: 26629
author:
type: performance
# 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
# frozen_string_literal: true
require 'spec_helper' require 'spec_helper'
describe GitlabSchema do describe GitlabSchema do
...@@ -31,6 +33,36 @@ describe GitlabSchema do ...@@ -31,6 +33,36 @@ describe GitlabSchema do
expect(connection).to eq(Gitlab::Graphql::Connections::KeysetConnection) expect(connection).to eq(Gitlab::Graphql::Connections::KeysetConnection)
end 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 def field_instrumenters
described_class.instrumenters[:field] described_class.instrumenters[:field]
end end
......
# 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
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 ...@@ -93,6 +93,8 @@ module GraphqlHelpers
end end
def all_graphql_fields_for(class_name, parent_types = Set.new) def all_graphql_fields_for(class_name, parent_types = Set.new)
allow_unlimited_graphql_complexity
type = GitlabSchema.types[class_name.to_s] type = GitlabSchema.types[class_name.to_s]
return "" unless type return "" unless type
...@@ -170,4 +172,10 @@ module GraphqlHelpers ...@@ -170,4 +172,10 @@ module GraphqlHelpers
field_type field_type
end 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 end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment