Commit 7a459b7c authored by Bob Van Landuyt's avatar Bob Van Landuyt

Merge branch...

Merge branch '324245-authenticated-admin-graphql-complexity-limits-are-not-being-applied' into 'master'

Resolve "Authenticated / Admin GraphQL complexity limits are not being applied"

See merge request gitlab-org/gitlab!56424
parents dac97b08 75c3a2fd
......@@ -31,9 +31,10 @@ class GitlabSchema < GraphQL::Schema
class << self
def multiplex(queries, **kwargs)
kwargs[:max_complexity] ||= max_query_complexity(kwargs[:context])
kwargs[:max_complexity] ||= max_query_complexity(kwargs[:context]) unless kwargs.key?(:max_complexity)
queries.each do |query|
query[:max_complexity] ||= max_query_complexity(kwargs[:context]) unless query.key?(:max_complexity)
query[:max_depth] = max_query_depth(kwargs[:context])
end
......
......@@ -125,9 +125,9 @@ RSpec.describe 'GitlabSchema configurations' do
subject do
queries = [
{ query: graphql_query_for('project', { 'fullPath' => '$fullPath' }, %w(id name description)) },
{ query: graphql_query_for('echo', { 'text' => "$test" }, []), variables: { "test" => "Hello world" } },
{ query: graphql_query_for('project', { 'fullPath' => project.full_path }, "userPermissions { createIssue }") }
{ query: graphql_query_for('project', { 'fullPath' => '$fullPath' }, %w(id name description)) }, # Complexity 4
{ query: graphql_query_for('echo', { 'text' => "$test" }, []), variables: { "test" => "Hello world" } }, # Complexity 1
{ query: graphql_query_for('project', { 'fullPath' => project.full_path }, "userPermissions { createIssue }") } # Complexity 3
]
post_multiplex(queries, current_user: current_user)
......@@ -139,10 +139,9 @@ RSpec.describe 'GitlabSchema configurations' do
expect(json_response.last['data']['project']).to be_nil
end
it_behaves_like 'imposing query limits' do
it 'fails all queries when only one of the queries is too complex' do
# The `project` query above has a complexity of 5
allow(GitlabSchema).to receive(:max_query_complexity).and_return 4
shared_examples 'query is too complex' do |description, max_complexity|
it description, :aggregate_failures do
allow(GitlabSchema).to receive(:max_query_complexity).and_return max_complexity
subject
......@@ -155,9 +154,15 @@ RSpec.describe 'GitlabSchema configurations' do
# Expect errors for each query
expect(graphql_errors.size).to eq(3)
graphql_errors.each do |single_query_errors|
expect_graphql_errors_to_include(/which exceeds max complexity of 4/)
expect_graphql_errors_to_include(/Query has complexity of 8, which exceeds max complexity of #{max_complexity}/)
end
end
end
it_behaves_like 'imposing query limits' do
# The total complexity of the multiplex query above is 8
it_behaves_like 'query is too complex', 'fails all queries when only one of the queries is too complex', 4
it_behaves_like 'query is too complex', 'fails when all queries combined are too complex', 7
end
context 'authentication' do
......
......@@ -208,6 +208,49 @@ RSpec.describe 'GraphQL' do
end
end
describe 'complexity limits' do
let_it_be(:project) { create(:project, :public) }
let!(:user) { create(:user) }
let(:query_fields) do
<<~QUERY
id
QUERY
end
let(:query) do
graphql_query_for(
'project',
{ 'fullPath' => project.full_path },
query_fields
)
end
before do
stub_const('GitlabSchema::DEFAULT_MAX_COMPLEXITY', 1)
end
context 'unauthenticated user' do
subject { post_graphql(query) }
it 'raises a complexity error' do
subject
expect_graphql_errors_to_include(/which exceeds max complexity/)
end
end
context 'authenticated user' do
subject { post_graphql(query, current_user: user) }
it 'does not raise an error as it uses the `AUTHENTICATED_COMPLEXITY`' do
subject
expect(graphql_errors).to be_nil
end
end
end
describe 'keyset pagination' do
let_it_be(:project) { create(:project, :public) }
let_it_be(:issues) { create_list(:issue, 10, project: project, created_at: Time.now.change(usec: 200)) }
......
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