summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--app/graphql/gitlab_schema.rb10
-rw-r--r--app/models/discussion.rb1
-rw-r--r--app/models/member.rb1
-rw-r--r--app/models/milestone.rb4
-rw-r--r--app/models/note.rb4
-rw-r--r--app/models/project.rb4
-rw-r--r--app/policies/commit_policy.rb1
-rw-r--r--app/policies/note_policy.rb2
-rw-r--r--app/services/notification_service.rb2
-rw-r--r--app/services/projects/participants_service.rb57
-rw-r--r--changelogs/unreleased/security-64519-nested-graphql-query-can-cause-denial-of-service.yml5
-rw-r--r--changelogs/unreleased/security-65756-ex-admin-attacker-can-comment-in-internal.yml5
-rw-r--r--changelogs/unreleased/security-hide-private-members-in-project-member-autocomplete.yml3
-rw-r--r--doc/user/project/members/share_project_with_groups.md4
-rw-r--r--lib/gitlab/graphql/query_analyzers/recursion_analyzer.rb62
-rw-r--r--spec/controllers/projects/autocomplete_sources_controller_spec.rb35
-rw-r--r--spec/fixtures/api/graphql/recursive-introspection.graphql57
-rw-r--r--spec/fixtures/api/graphql/recursive-query.graphql47
-rw-r--r--spec/fixtures/api/graphql/small-recursive-introspection.graphql15
-rw-r--r--spec/models/milestone_spec.rb8
-rw-r--r--spec/models/note_spec.rb20
-rw-r--r--spec/models/project_spec.rb8
-rw-r--r--spec/policies/commit_policy_spec.rb48
-rw-r--r--spec/requests/api/graphql/gitlab_schema_spec.rb48
-rw-r--r--spec/services/projects/participants_service_spec.rb104
-rw-r--r--spec/support/helpers/graphql_helpers.rb22
26 files changed, 526 insertions, 51 deletions
diff --git a/app/graphql/gitlab_schema.rb b/app/graphql/gitlab_schema.rb
index 4c8612c8f2e..1899278ff3c 100644
--- a/app/graphql/gitlab_schema.rb
+++ b/app/graphql/gitlab_schema.rb
@@ -18,15 +18,15 @@ class GitlabSchema < GraphQL::Schema
use Gitlab::Graphql::GenericTracing
query_analyzer Gitlab::Graphql::QueryAnalyzers::LoggerAnalyzer.new
-
- query(Types::QueryType)
-
- default_max_page_size 100
+ query_analyzer Gitlab::Graphql::QueryAnalyzers::RecursionAnalyzer.new
max_complexity DEFAULT_MAX_COMPLEXITY
max_depth DEFAULT_MAX_DEPTH
- mutation(Types::MutationType)
+ query Types::QueryType
+ mutation Types::MutationType
+
+ default_max_page_size 100
class << self
def multiplex(queries, **kwargs)
diff --git a/app/models/discussion.rb b/app/models/discussion.rb
index 0d066d0d99f..b8525f7b135 100644
--- a/app/models/discussion.rb
+++ b/app/models/discussion.rb
@@ -16,6 +16,7 @@ class Discussion
:commit_id,
:for_commit?,
:for_merge_request?,
+ :noteable_ability_name,
:to_ability_name,
:editable?,
:visible_for?,
diff --git a/app/models/member.rb b/app/models/member.rb
index e2d26773d45..2654453cf3f 100644
--- a/app/models/member.rb
+++ b/app/models/member.rb
@@ -8,6 +8,7 @@ class Member < ApplicationRecord
include Gitlab::Access
include Presentable
include Gitlab::Utils::StrongMemoize
+ include FromUnion
attr_accessor :raw_invite_token
diff --git a/app/models/milestone.rb b/app/models/milestone.rb
index 2fa0cfc9b93..a9f4cdec901 100644
--- a/app/models/milestone.rb
+++ b/app/models/milestone.rb
@@ -261,6 +261,10 @@ class Milestone < ApplicationRecord
group || project
end
+ def to_ability_name
+ model_name.singular
+ end
+
def group_milestone?
group_id.present?
end
diff --git a/app/models/note.rb b/app/models/note.rb
index 43f349c6fa2..ce60413b8a0 100644
--- a/app/models/note.rb
+++ b/app/models/note.rb
@@ -361,6 +361,10 @@ class Note < ApplicationRecord
end
def to_ability_name
+ model_name.singular
+ end
+
+ def noteable_ability_name
for_snippet? ? noteable.class.name.underscore : noteable_type.demodulize.underscore
end
diff --git a/app/models/project.rb b/app/models/project.rb
index 3525f37f8d5..4d9234e482f 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -1260,6 +1260,10 @@ class Project < ApplicationRecord
end
end
+ def to_ability_name
+ model_name.singular
+ end
+
# rubocop: disable CodeReuse/ServiceClass
def execute_hooks(data, hooks_scope = :push_hooks)
run_after_commit_or_now do
diff --git a/app/policies/commit_policy.rb b/app/policies/commit_policy.rb
index 4d4f0ba9267..4b358c45ec2 100644
--- a/app/policies/commit_policy.rb
+++ b/app/policies/commit_policy.rb
@@ -4,4 +4,5 @@ class CommitPolicy < BasePolicy
delegate { @subject.project }
rule { can?(:download_code) }.enable :read_commit
+ rule { ~can?(:read_commit) }.prevent :create_note
end
diff --git a/app/policies/note_policy.rb b/app/policies/note_policy.rb
index b2af6c874c7..dcde8cefa0d 100644
--- a/app/policies/note_policy.rb
+++ b/app/policies/note_policy.rb
@@ -9,7 +9,7 @@ class NotePolicy < BasePolicy
condition(:editable, scope: :subject) { @subject.editable? }
- condition(:can_read_noteable) { can?(:"read_#{@subject.to_ability_name}") }
+ condition(:can_read_noteable) { can?(:"read_#{@subject.noteable_ability_name}") }
condition(:is_visible) { @subject.visible_for?(@user) }
diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb
index b56b2cf14e3..1709474a6c7 100644
--- a/app/services/notification_service.rb
+++ b/app/services/notification_service.rb
@@ -281,7 +281,7 @@ class NotificationService
end
def send_new_note_notifications(note)
- notify_method = "note_#{note.to_ability_name}_email".to_sym
+ notify_method = "note_#{note.noteable_ability_name}_email".to_sym
recipients = NotificationRecipientService.build_new_note_recipients(note)
recipients.each do |recipient|
diff --git a/app/services/projects/participants_service.rb b/app/services/projects/participants_service.rb
index 7080f388e53..1cd81fe37c7 100644
--- a/app/services/projects/participants_service.rb
+++ b/app/services/projects/participants_service.rb
@@ -7,16 +7,69 @@ module Projects
def execute(noteable)
@noteable = noteable
- participants = noteable_owner + participants_in_noteable + all_members + groups + project_members
+ participants =
+ noteable_owner +
+ participants_in_noteable +
+ all_members +
+ groups +
+ project_members
+
participants.uniq
end
def project_members
- @project_members ||= sorted(project.team.members)
+ @project_members ||= sorted(get_project_members)
+ end
+
+ def get_project_members
+ members = Member.from_union([project_members_through_ancestral_groups,
+ project_members_through_invited_groups,
+ individual_project_members])
+
+ User.id_in(members.select(:user_id))
end
def all_members
[{ username: "all", name: "All Project and Group Members", count: project_members.count }]
end
+
+ private
+
+ def project_members_through_invited_groups
+ groups_with_ancestors_ids = Gitlab::ObjectHierarchy
+ .new(visible_groups)
+ .base_and_ancestors
+ .pluck_primary_key
+
+ GroupMember
+ .active_without_invites_and_requests
+ .with_source_id(groups_with_ancestors_ids)
+ end
+
+ def visible_groups
+ visible_groups = project.invited_groups
+
+ unless project_owner?
+ visible_groups = visible_groups.public_or_visible_to_user(current_user)
+ end
+
+ visible_groups
+ end
+
+ def project_members_through_ancestral_groups
+ project.group.present? ? project.group.members_with_parents : Member.none
+ end
+
+ def individual_project_members
+ project.project_members
+ end
+
+ def project_owner?
+ if project.group.present?
+ project.group.owners.include?(current_user)
+ else
+ project.namespace.owner == current_user
+ end
+ end
end
end
diff --git a/changelogs/unreleased/security-64519-nested-graphql-query-can-cause-denial-of-service.yml b/changelogs/unreleased/security-64519-nested-graphql-query-can-cause-denial-of-service.yml
new file mode 100644
index 00000000000..5ce37b0d032
--- /dev/null
+++ b/changelogs/unreleased/security-64519-nested-graphql-query-can-cause-denial-of-service.yml
@@ -0,0 +1,5 @@
+---
+title: Analyze incoming GraphQL queries and check for recursion
+merge_request:
+author:
+type: security
diff --git a/changelogs/unreleased/security-65756-ex-admin-attacker-can-comment-in-internal.yml b/changelogs/unreleased/security-65756-ex-admin-attacker-can-comment-in-internal.yml
new file mode 100644
index 00000000000..3d9f480ba11
--- /dev/null
+++ b/changelogs/unreleased/security-65756-ex-admin-attacker-can-comment-in-internal.yml
@@ -0,0 +1,5 @@
+---
+title: Disallow unprivileged users from commenting on private repository commits
+merge_request:
+author:
+type: security
diff --git a/changelogs/unreleased/security-hide-private-members-in-project-member-autocomplete.yml b/changelogs/unreleased/security-hide-private-members-in-project-member-autocomplete.yml
new file mode 100644
index 00000000000..5992e93bda2
--- /dev/null
+++ b/changelogs/unreleased/security-hide-private-members-in-project-member-autocomplete.yml
@@ -0,0 +1,3 @@
+---
+title: "Don't leak private members in project member autocomplete suggestions"
+type: security
diff --git a/doc/user/project/members/share_project_with_groups.md b/doc/user/project/members/share_project_with_groups.md
index 9340d239677..79fb2ea50a0 100644
--- a/doc/user/project/members/share_project_with_groups.md
+++ b/doc/user/project/members/share_project_with_groups.md
@@ -44,6 +44,10 @@ Admins are able to share projects with any group in the system.
In the example above, the maximum access level of 'Developer' for members from 'Engineering' means that users with higher access levels in 'Engineering' ('Maintainer' or 'Owner') will only have 'Developer' access to 'Project Acme'.
+## Sharing public project with private group
+
+When sharing a public project with a private group, owners and maintainers of the project will see the name of the group in the `members` page. Owners will also have the possibility to see members of the private group they don't have access to when mentioning them in the issue or merge request.
+
## Share project with group lock
It is possible to prevent projects in a group from [sharing
diff --git a/lib/gitlab/graphql/query_analyzers/recursion_analyzer.rb b/lib/gitlab/graphql/query_analyzers/recursion_analyzer.rb
new file mode 100644
index 00000000000..ccf9e597307
--- /dev/null
+++ b/lib/gitlab/graphql/query_analyzers/recursion_analyzer.rb
@@ -0,0 +1,62 @@
+# frozen_string_literal: true
+
+# Recursive queries, with relatively low effort, can quickly spiral out of control exponentially
+# and may not be picked up by depth and complexity alone.
+module Gitlab
+ module Graphql
+ module QueryAnalyzers
+ class RecursionAnalyzer
+ IGNORED_FIELDS = %w(node edges ofType).freeze
+ RECURSION_THRESHOLD = 2
+
+ def initial_value(query)
+ {
+ recurring_fields: {}
+ }
+ end
+
+ def call(memo, visit_type, irep_node)
+ return memo if skip_node?(irep_node)
+
+ node_name = irep_node.ast_node.name
+ times_encountered = memo[node_name] || 0
+
+ if visit_type == :enter
+ times_encountered += 1
+ memo[:recurring_fields][node_name] = times_encountered if recursion_too_deep?(node_name, times_encountered)
+ else
+ times_encountered -= 1
+ end
+
+ memo[node_name] = times_encountered
+ memo
+ end
+
+ def final_value(memo)
+ recurring_fields = memo[:recurring_fields]
+ recurring_fields = recurring_fields.select { |k, v| recursion_too_deep?(k, v) }
+ if recurring_fields.any?
+ GraphQL::AnalysisError.new("Recursive query - too many of fields '#{recurring_fields}' detected in single branch of the query")
+ end
+ end
+
+ private
+
+ def recursion_too_deep?(node_name, times_encountered)
+ return if IGNORED_FIELDS.include?(node_name)
+
+ times_encountered > recursion_threshold
+ end
+
+ def skip_node?(irep_node)
+ ast_node = irep_node.ast_node
+ !ast_node.is_a?(GraphQL::Language::Nodes::Field) || ast_node.selections.empty?
+ end
+
+ def recursion_threshold
+ RECURSION_THRESHOLD
+ end
+ end
+ end
+ end
+end
diff --git a/spec/controllers/projects/autocomplete_sources_controller_spec.rb b/spec/controllers/projects/autocomplete_sources_controller_spec.rb
index 34765ae3951..fc8fe1ac4f6 100644
--- a/spec/controllers/projects/autocomplete_sources_controller_spec.rb
+++ b/spec/controllers/projects/autocomplete_sources_controller_spec.rb
@@ -8,6 +8,10 @@ describe Projects::AutocompleteSourcesController do
let_it_be(:issue) { create(:issue, project: project) }
let_it_be(:user) { create(:user) }
+ def members_by_username(username)
+ json_response.find { |member| member['username'] == username }
+ end
+
describe 'GET members' do
before do
group.add_owner(user)
@@ -17,22 +21,21 @@ describe Projects::AutocompleteSourcesController do
it 'returns an array of member object' do
get :members, format: :json, params: { namespace_id: group.path, project_id: project.path, type: issue.class.name, type_id: issue.id }
- all = json_response.find {|member| member["username"] == 'all'}
- the_group = json_response.find {|member| member["username"] == group.full_path}
- the_user = json_response.find {|member| member["username"] == user.username}
-
- expect(all.symbolize_keys).to include(username: 'all',
- name: 'All Project and Group Members',
- count: 1)
-
- expect(the_group.symbolize_keys).to include(type: group.class.name,
- name: group.full_name,
- avatar_url: group.avatar_url,
- count: 1)
-
- expect(the_user.symbolize_keys).to include(type: user.class.name,
- name: user.name,
- avatar_url: user.avatar_url)
+ expect(members_by_username('all').symbolize_keys).to include(
+ username: 'all',
+ name: 'All Project and Group Members',
+ count: 1)
+
+ expect(members_by_username(group.full_path).symbolize_keys).to include(
+ type: group.class.name,
+ name: group.full_name,
+ avatar_url: group.avatar_url,
+ count: 1)
+
+ expect(members_by_username(user.username).symbolize_keys).to include(
+ type: user.class.name,
+ name: user.name,
+ avatar_url: user.avatar_url)
end
end
diff --git a/spec/fixtures/api/graphql/recursive-introspection.graphql b/spec/fixtures/api/graphql/recursive-introspection.graphql
new file mode 100644
index 00000000000..db970bb14b6
--- /dev/null
+++ b/spec/fixtures/api/graphql/recursive-introspection.graphql
@@ -0,0 +1,57 @@
+query allSchemaTypes {
+ __schema {
+ types {
+ fields {
+ type{
+ fields {
+ type {
+ fields {
+ type {
+ fields {
+ type {
+ fields {
+ type {
+ fields {
+ type {
+ fields {
+ type {
+ fields {
+ type {
+ fields {
+ type {
+ fields {
+ type {
+ fields {
+ type {
+ fields {
+ type {
+ fields {
+ name
+ }
+ }
+ }
+ }
+ }
+ }
+ }
+ }
+ }
+ }
+ }
+ }
+ }
+ }
+ }
+ }
+ }
+ }
+ }
+ }
+ }
+ }
+ }
+ }
+ }
+ }
+ }
+} \ No newline at end of file
diff --git a/spec/fixtures/api/graphql/recursive-query.graphql b/spec/fixtures/api/graphql/recursive-query.graphql
new file mode 100644
index 00000000000..d1616c4de6e
--- /dev/null
+++ b/spec/fixtures/api/graphql/recursive-query.graphql
@@ -0,0 +1,47 @@
+{
+ project(fullPath: "gitlab-org/gitlab-ce") {
+ group {
+ projects {
+ edges {
+ node {
+ group {
+ projects {
+ edges {
+ node {
+ group {
+ projects {
+ edges {
+ node {
+ group {
+ projects {
+ edges {
+ node {
+ group {
+ projects {
+ edges {
+ node {
+ group {
+ description
+ }
+ }
+ }
+ }
+ }
+ }
+ }
+ }
+ }
+ }
+ }
+ }
+ }
+ }
+ }
+ }
+ }
+ }
+ }
+ }
+ }
+ }
+}
diff --git a/spec/fixtures/api/graphql/small-recursive-introspection.graphql b/spec/fixtures/api/graphql/small-recursive-introspection.graphql
new file mode 100644
index 00000000000..1025043b474
--- /dev/null
+++ b/spec/fixtures/api/graphql/small-recursive-introspection.graphql
@@ -0,0 +1,15 @@
+query allSchemaTypes {
+ __schema {
+ types {
+ fields {
+ type {
+ fields {
+ type {
+ name
+ }
+ }
+ }
+ }
+ }
+ }
+}
diff --git a/spec/models/milestone_spec.rb b/spec/models/milestone_spec.rb
index 2ecbe548520..120ba67f328 100644
--- a/spec/models/milestone_spec.rb
+++ b/spec/models/milestone_spec.rb
@@ -227,6 +227,14 @@ describe Milestone do
end
end
+ describe '#to_ability_name' do
+ it 'returns milestone' do
+ milestone = build(:milestone)
+
+ expect(milestone.to_ability_name).to eq('milestone')
+ end
+ end
+
describe '.search' do
let(:milestone) { create(:milestone, title: 'foo', description: 'bar') }
diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb
index e838154ecb6..3ab88b52568 100644
--- a/spec/models/note_spec.rb
+++ b/spec/models/note_spec.rb
@@ -635,24 +635,30 @@ describe Note do
end
describe '#to_ability_name' do
- it 'returns snippet for a project snippet note' do
- expect(build(:note_on_project_snippet).to_ability_name).to eq('project_snippet')
+ it 'returns note' do
+ expect(build(:note).to_ability_name).to eq('note')
+ end
+ end
+
+ describe '#noteable_ability_name' do
+ it 'returns project_snippet for a project snippet note' do
+ expect(build(:note_on_project_snippet).noteable_ability_name).to eq('project_snippet')
end
it 'returns personal_snippet for a personal snippet note' do
- expect(build(:note_on_personal_snippet).to_ability_name).to eq('personal_snippet')
+ expect(build(:note_on_personal_snippet).noteable_ability_name).to eq('personal_snippet')
end
it 'returns merge_request for an MR note' do
- expect(build(:note_on_merge_request).to_ability_name).to eq('merge_request')
+ expect(build(:note_on_merge_request).noteable_ability_name).to eq('merge_request')
end
it 'returns issue for an issue note' do
- expect(build(:note_on_issue).to_ability_name).to eq('issue')
+ expect(build(:note_on_issue).noteable_ability_name).to eq('issue')
end
- it 'returns issue for a commit note' do
- expect(build(:note_on_commit).to_ability_name).to eq('commit')
+ it 'returns commit for a commit note' do
+ expect(build(:note_on_commit).noteable_ability_name).to eq('commit')
end
end
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index 9f3313e67b5..be2ea2e63ee 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -4441,6 +4441,14 @@ describe Project do
end
end
+ describe '#to_ability_name' do
+ it 'returns project' do
+ project = build(:project_empty_repo)
+
+ expect(project.to_ability_name).to eq('project')
+ end
+ end
+
describe '#execute_hooks' do
let(:data) { { ref: 'refs/heads/master', data: 'data' } }
it 'executes active projects hooks with the specified scope' do
diff --git a/spec/policies/commit_policy_spec.rb b/spec/policies/commit_policy_spec.rb
index 41f6fb08426..40183f51e9e 100644
--- a/spec/policies/commit_policy_spec.rb
+++ b/spec/policies/commit_policy_spec.rb
@@ -8,28 +8,42 @@ describe CommitPolicy do
let(:commit) { project.repository.head_commit }
let(:policy) { described_class.new(user, commit) }
+ shared_examples 'can read commit and create a note' do
+ it 'can read commit' do
+ expect(policy).to be_allowed(:read_commit)
+ end
+
+ it 'can create a note' do
+ expect(policy).to be_allowed(:create_note)
+ end
+ end
+
+ shared_examples 'cannot read commit nor create a note' do
+ it 'can not read commit' do
+ expect(policy).to be_disallowed(:read_commit)
+ end
+
+ it 'can not create a note' do
+ expect(policy).to be_disallowed(:create_note)
+ end
+ end
+
context 'when project is public' do
let(:project) { create(:project, :public, :repository) }
- it 'can read commit and create a note' do
- expect(policy).to be_allowed(:read_commit)
- end
+ it_behaves_like 'can read commit and create a note'
context 'when repository access level is private' do
let(:project) { create(:project, :public, :repository, :repository_private) }
- it 'can not read commit and create a note' do
- expect(policy).to be_disallowed(:read_commit)
- end
+ it_behaves_like 'cannot read commit nor create a note'
context 'when the user is a project member' do
before do
project.add_developer(user)
end
- it 'can read commit and create a note' do
- expect(policy).to be_allowed(:read_commit)
- end
+ it_behaves_like 'can read commit and create a note'
end
end
end
@@ -37,9 +51,7 @@ describe CommitPolicy do
context 'when project is private' do
let(:project) { create(:project, :private, :repository) }
- it 'can not read commit and create a note' do
- expect(policy).to be_disallowed(:read_commit)
- end
+ it_behaves_like 'cannot read commit nor create a note'
context 'when the user is a project member' do
before do
@@ -50,6 +62,18 @@ describe CommitPolicy do
expect(policy).to be_allowed(:read_commit)
end
end
+
+ context 'when the user is a guest' do
+ before do
+ project.add_guest(user)
+ end
+
+ it_behaves_like 'cannot read commit nor create a note'
+
+ it 'cannot download code' do
+ expect(policy).to be_disallowed(:download_code)
+ end
+ 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 e1eb7c7f738..1e799a0a42a 100644
--- a/spec/requests/api/graphql/gitlab_schema_spec.rb
+++ b/spec/requests/api/graphql/gitlab_schema_spec.rb
@@ -13,7 +13,7 @@ describe 'GitlabSchema configurations' do
subject
- expect(graphql_errors.flatten.first['message']).to include('which exceeds max complexity of 1')
+ expect_graphql_errors_to_include /which exceeds max complexity of 1/
end
end
end
@@ -21,12 +21,11 @@ describe 'GitlabSchema configurations' do
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
subject
- expect(graphql_errors.flatten).to include(errors)
+ expect_graphql_errors_to_include /exceeds max depth/
end
end
@@ -36,7 +35,42 @@ describe 'GitlabSchema configurations' do
subject
- expect(Array.wrap(graphql_errors).compact).to be_empty
+ expect_graphql_errors_to_be_empty
+ end
+ end
+ end
+ end
+
+ context 'depth, complexity and recursion checking' do
+ context 'unauthenticated recursive queries' do
+ context 'a not-quite-recursive-enough introspective query' do
+ it 'succeeds' do
+ query = File.read(Rails.root.join('spec/fixtures/api/graphql/small-recursive-introspection.graphql'))
+
+ post_graphql(query, current_user: nil)
+
+ expect_graphql_errors_to_be_empty
+ end
+ end
+
+ context 'a deep but simple recursive introspective query' do
+ it 'fails due to recursion' do
+ query = File.read(Rails.root.join('spec/fixtures/api/graphql/recursive-introspection.graphql'))
+
+ post_graphql(query, current_user: nil)
+
+ expect_graphql_errors_to_include [/Recursive query/]
+ end
+ end
+
+ context 'a deep recursive non-introspective query' do
+ it 'fails due to recursion, complexity and depth' do
+ allow(GitlabSchema).to receive(:max_query_complexity).and_return 1
+ query = File.read(Rails.root.join('spec/fixtures/api/graphql/recursive-query.graphql'))
+
+ post_graphql(query, current_user: nil)
+
+ expect_graphql_errors_to_include [/Recursive query/, /exceeds max complexity/, /exceeds max depth/]
end
end
end
@@ -86,7 +120,7 @@ describe 'GitlabSchema configurations' do
# Expect errors for each query
expect(graphql_errors.size).to eq(3)
graphql_errors.each do |single_query_errors|
- expect(single_query_errors.first['message']).to include('which exceeds max complexity of 4')
+ expect_graphql_errors_to_include(/which exceeds max complexity of 4/)
end
end
end
@@ -103,12 +137,12 @@ describe 'GitlabSchema configurations' do
end
context 'when IntrospectionQuery' do
- it 'is not too complex' do
+ it 'is not too complex nor recursive' do
query = File.read(Rails.root.join('spec/fixtures/api/graphql/introspection.graphql'))
post_graphql(query, current_user: nil)
- expect(graphql_errors).to be_nil
+ expect_graphql_errors_to_be_empty
end
end
diff --git a/spec/services/projects/participants_service_spec.rb b/spec/services/projects/participants_service_spec.rb
index 4def83513a4..239d28557ee 100644
--- a/spec/services/projects/participants_service_spec.rb
+++ b/spec/services/projects/participants_service_spec.rb
@@ -57,4 +57,108 @@ describe Projects::ParticipantsService do
end
end
end
+
+ describe '#project_members' do
+ subject(:usernames) { service.project_members.map { |member| member[:username] } }
+
+ context 'when there is a project in group namespace' do
+ set(:public_group) { create(:group, :public) }
+ set(:public_project) { create(:project, :public, namespace: public_group)}
+
+ set(:public_group_owner) { create(:user) }
+
+ let(:service) { described_class.new(public_project, create(:user)) }
+
+ before do
+ public_group.add_owner(public_group_owner)
+ end
+
+ it 'returns members of a group' do
+ expect(usernames).to include(public_group_owner.username)
+ end
+ end
+
+ context 'when there is a private group and a public project' do
+ set(:public_group) { create(:group, :public) }
+ set(:private_group) { create(:group, :private, :nested) }
+ set(:public_project) { create(:project, :public, namespace: public_group)}
+
+ set(:project_issue) { create(:issue, project: public_project)}
+
+ set(:public_group_owner) { create(:user) }
+ set(:private_group_member) { create(:user) }
+ set(:public_project_maintainer) { create(:user) }
+ set(:private_group_owner) { create(:user) }
+
+ set(:group_ancestor_owner) { create(:user) }
+
+ before(:context) do
+ public_group.add_owner public_group_owner
+ private_group.add_developer private_group_member
+ public_project.add_maintainer public_project_maintainer
+
+ private_group.add_owner private_group_owner
+ private_group.parent.add_owner group_ancestor_owner
+ end
+
+ context 'when the private group is invited to the public project' do
+ before(:context) do
+ create(:project_group_link, group: private_group, project: public_project)
+ end
+
+ context 'when a user who is outside the public project and the private group is signed in' do
+ let(:service) { described_class.new(public_project, create(:user)) }
+
+ it 'does not return the private group' do
+ expect(usernames).not_to include(private_group.name)
+ end
+
+ it 'does not return private group members' do
+ expect(usernames).not_to include(private_group_member.username)
+ end
+
+ it 'returns the project maintainer' do
+ expect(usernames).to include(public_project_maintainer.username)
+ end
+
+ it 'returns project members from an invited public group' do
+ invited_public_group = create(:group, :public)
+ invited_public_group.add_owner create(:user)
+
+ create(:project_group_link, group: invited_public_group, project: public_project)
+
+ expect(usernames).to include(invited_public_group.users.first.username)
+ end
+
+ it 'does not return ancestors of the private group' do
+ expect(usernames).not_to include(group_ancestor_owner.username)
+ end
+ end
+
+ context 'when private group owner is signed in' do
+ let(:service) { described_class.new(public_project, private_group_owner) }
+
+ it 'returns private group members' do
+ expect(usernames).to include(private_group_member.username)
+ end
+
+ it 'returns ancestors of the the private group' do
+ expect(usernames).to include(group_ancestor_owner.username)
+ end
+ end
+
+ context 'when the namespace owner of the public project is signed in' do
+ let(:service) { described_class.new(public_project, public_group_owner) }
+
+ it 'returns private group members' do
+ expect(usernames).to include(private_group_member.username)
+ end
+
+ it 'does not return members of the ancestral groups of the private group' do
+ expect(usernames).to include(group_ancestor_owner.username)
+ end
+ end
+ end
+ end
+ end
end
diff --git a/spec/support/helpers/graphql_helpers.rb b/spec/support/helpers/graphql_helpers.rb
index 4d2ad165fd6..6fb1d279456 100644
--- a/spec/support/helpers/graphql_helpers.rb
+++ b/spec/support/helpers/graphql_helpers.rb
@@ -129,6 +129,7 @@ module GraphqlHelpers
allow_unlimited_graphql_complexity
allow_unlimited_graphql_depth
+ allow_high_graphql_recursion
type = GitlabSchema.types[class_name.to_s]
return "" unless type
@@ -213,6 +214,23 @@ module GraphqlHelpers
end
end
+ def expect_graphql_errors_to_include(regexes_to_match)
+ raise "No errors. Was expecting to match #{regexes_to_match}" if graphql_errors.nil? || graphql_errors.empty?
+
+ error_messages = flattened_errors.collect { |error_hash| error_hash["message"] }
+ Array.wrap(regexes_to_match).flatten.each do |regex|
+ expect(error_messages).to include a_string_matching regex
+ end
+ end
+
+ def expect_graphql_errors_to_be_empty
+ expect(flattened_errors).to be_empty
+ end
+
+ def flattened_errors
+ Array.wrap(graphql_errors).flatten.compact
+ end
+
# Raises an error if no response is found
def graphql_mutation_response(mutation_name)
graphql_data.fetch(GraphqlHelpers.fieldnamerize(mutation_name))
@@ -260,6 +278,10 @@ module GraphqlHelpers
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
+
+ def allow_high_graphql_recursion
+ allow_any_instance_of(Gitlab::Graphql::QueryAnalyzers::RecursionAnalyzer).to receive(:recursion_threshold).and_return 1000
+ end
end
# This warms our schema, doing this as part of loading the helpers to avoid