diff --git a/CHANGELOG.md b/CHANGELOG.md
index 9411180abff64eb1a25e4241a196b4b1fc4b69bb..16a36724b4ff230eca266a4dd142b3f3e6ea78f9 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,24 @@
documentation](doc/development/changelog.md) for instructions on adding your own
entry.
+## 12.4.1
+
+### Security (12 changes)
+
+- Standardize error response when route is missing.
+- Do not display project labels that are not visible for user accessing group labels.
+- Show cross-referenced label and milestones in issues' activities only to authorized users.
+- Analyze incoming GraphQL queries and check for recursion.
+- Disallow unprivileged users from commenting on private repository commits.
+- Don't allow maintainers of a target project to delete the source branch of a merge request from a fork.
+- Require Maintainer permission on group where project is transferred to.
+- Don't leak private members in project member autocomplete suggestions.
+- Return 404 on LFS request if project doesn't exist.
+- Mask sentry auth token in Error Tracking dashboard.
+- Fixes a Open Redirect issue in `InternalRedirect`.
+- Sanitize all wiki markup formats with GitLab sanitization pipelines.
+
+
## 12.4.0
### Security (14 changes)
diff --git a/VERSION b/VERSION
index f8c17e78090925106e870ad36842bfd5c7223643..c3b55577163158af6233cebc96f0dadbb3bb7a43 100644
--- a/VERSION
+++ b/VERSION
@@ -1 +1 @@
-12.4.0
+12.4.1
diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 1443a71f6b1611fffa2c8a0328f5b2da5c2590cc..27e88ae569e9feeeb3014f009a285a86c5d704ef 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -16,7 +16,7 @@ class ApplicationController < ActionController::Base
include Gitlab::Tracking::ControllerConcern
include Gitlab::Experimentation::ControllerConcern
- before_action :authenticate_user!
+ before_action :authenticate_user!, except: [:route_not_found]
before_action :enforce_terms!, if: :should_enforce_terms?
before_action :validate_user_service_ticket!
before_action :check_password_expiration
@@ -97,7 +97,9 @@ class ApplicationController < ActionController::Base
if current_user
not_found
else
- authenticate_user!
+ store_location_for(:user, request.fullpath) unless request.xhr?
+
+ redirect_to new_user_session_path, alert: I18n.t('devise.failure.unauthenticated')
end
end
diff --git a/app/controllers/concerns/internal_redirect.rb b/app/controllers/concerns/internal_redirect.rb
index 99bbfd56516e2d81b6c959ffa05923857e3ec46b..a35bc19aa37b9c37a79092686400b181eacb5de1 100644
--- a/app/controllers/concerns/internal_redirect.rb
+++ b/app/controllers/concerns/internal_redirect.rb
@@ -6,7 +6,7 @@ module InternalRedirect
def safe_redirect_path(path)
return unless path
# Verify that the string starts with a `/` and a known route character.
- return unless path =~ %r{^/[-\w].*$}
+ return unless path =~ %r{\A/[-\w].*\z}
uri = URI(path)
# Ignore anything path of the redirect except for the path, querystring and,
diff --git a/app/controllers/concerns/lfs_request.rb b/app/controllers/concerns/lfs_request.rb
index 733265f4099a92ed375e33197ed669b3909589d5..417bb169f3935397d780e64ad8896d8776f9bc02 100644
--- a/app/controllers/concerns/lfs_request.rb
+++ b/app/controllers/concerns/lfs_request.rb
@@ -34,6 +34,7 @@ module LfsRequest
end
def lfs_check_access!
+ return render_lfs_not_found unless project
return if download_request? && lfs_download_access?
return if upload_request? && lfs_upload_access?
diff --git a/app/finders/labels_finder.rb b/app/finders/labels_finder.rb
index e523942ea4cebc867d863c04a0ff9848633e03bc..027cdc4fc7896d951879e82018649b2f28987b7e 100644
--- a/app/finders/labels_finder.rb
+++ b/app/finders/labels_finder.rb
@@ -51,7 +51,7 @@ class LabelsFinder < UnionFinder
end
label_ids << Label.where(group_id: projects.group_ids)
- label_ids << Label.where(project_id: projects.select(:id)) unless only_group_labels?
+ label_ids << Label.where(project_id: ids_user_can_read_labels(projects)) unless only_group_labels?
end
label_ids
@@ -188,4 +188,10 @@ class LabelsFinder < UnionFinder
groups.select { |group| authorized_to_read_labels?(group) }
end
end
+
+ # rubocop: disable CodeReuse/ActiveRecord
+ def ids_user_can_read_labels(projects)
+ Project.where(id: projects.select(:id)).ids_with_issuables_available_for(current_user)
+ end
+ # rubocop: enable CodeReuse/ActiveRecord
end
diff --git a/app/graphql/gitlab_schema.rb b/app/graphql/gitlab_schema.rb
index 4c8612c8f2e7982e6a2159e3f860f0bb5c6eb951..1899278ff3c93768be3deac9dfdabbfff9f985cf 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/helpers/markup_helper.rb b/app/helpers/markup_helper.rb
index d76a0f3a3b8db2eef384b37b51469544160f23e4..e2524938e10f161d3023ba653b3cf65c81c7d018 100644
--- a/app/helpers/markup_helper.rb
+++ b/app/helpers/markup_helper.rb
@@ -133,15 +133,7 @@ module MarkupHelper
issuable_state_filter_enabled: true
)
- html =
- case wiki_page.format
- when :markdown
- markdown_unsafe(text, context)
- when :asciidoc
- asciidoc_unsafe(text)
- else
- wiki_page.formatted_content.html_safe
- end
+ html = markup_unsafe(wiki_page.path, text, context)
prepare_for_rendering(html, context)
end
diff --git a/app/models/concerns/mentionable/reference_regexes.rb b/app/models/concerns/mentionable/reference_regexes.rb
index fec31cd262b41dfc2f99ad083d6b6d2622b5fa21..f44a674b3c97911a0d8409ae1388e360f2ceb46a 100644
--- a/app/models/concerns/mentionable/reference_regexes.rb
+++ b/app/models/concerns/mentionable/reference_regexes.rb
@@ -13,7 +13,9 @@ module Mentionable
def self.other_patterns
[
Commit.reference_pattern,
- MergeRequest.reference_pattern
+ MergeRequest.reference_pattern,
+ Label.reference_pattern,
+ Milestone.reference_pattern
]
end
diff --git a/app/models/discussion.rb b/app/models/discussion.rb
index 0d066d0d99f652b3f1e946c3498352c16c4d3dc4..b8525f7b1350f33097ddaead38f52ff0c1368588 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 e2d26773d450e8d6757255ab6877675b6c20cccf..2654453cf3fdb240234c6c2f00265bd823747e45 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/merge_request.rb b/app/models/merge_request.rb
index 7cdaa3e3ca70a20f96699c6be7565d0ac85a9d7d..32741046f396a6c23e656eebf27318db74a65e67 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -69,6 +69,14 @@ class MergeRequest < ApplicationRecord
has_many :merge_request_assignees
has_many :assignees, class_name: "User", through: :merge_request_assignees
+ KNOWN_MERGE_PARAMS = [
+ :auto_merge_strategy,
+ :should_remove_source_branch,
+ :force_remove_source_branch,
+ :commit_message,
+ :squash_commit_message,
+ :sha
+ ].freeze
serialize :merge_params, Hash # rubocop:disable Cop/ActiveRecordSerialize
after_create :ensure_merge_request_diff
diff --git a/app/models/milestone.rb b/app/models/milestone.rb
index 2fa0cfc9b93765b974398d83fc304087e6685102..a9f4cdec9018dfd4034d9b361c59ec726209a333 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 43f349c6fa2bc08b1753d2a4a5546d7a1009f1b8..ce60413b8a04ad943d85c6df37200ad5342bbcd8 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 3525f37f8d5dff8ca714b69b9a2e36a0c19440a2..74da042d5a550100a9f23e8255e6aa62fe81b20e 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -609,11 +609,11 @@ class Project < ApplicationRecord
joins(:namespace).where(namespaces: { type: 'Group' }).select(:namespace_id)
end
- # Returns ids of projects with milestones available for given user
+ # Returns ids of projects with issuables available for given user
#
- # Used on queries to find milestones which user can see
- # For example: Milestone.where(project_id: ids_with_milestone_available_for(user))
- def ids_with_milestone_available_for(user)
+ # Used on queries to find milestones or labels which user can see
+ # For example: Milestone.where(project_id: ids_with_issuables_available_for(user))
+ def ids_with_issuables_available_for(user)
with_issues_enabled = with_issues_available_for_user(user).select(:id)
with_merge_requests_enabled = with_merge_requests_available_for_user(user).select(:id)
@@ -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/models/system_note_metadata.rb b/app/models/system_note_metadata.rb
index 11cbeb60bba6758a2a79a74e22032175d992fe01..5a44ee7211b7f4961c2f58dd1120aec1d1efc4d8 100644
--- a/app/models/system_note_metadata.rb
+++ b/app/models/system_note_metadata.rb
@@ -10,6 +10,7 @@ class SystemNoteMetadata < ApplicationRecord
commit cross_reference
close duplicate
moved merge
+ label milestone
].freeze
ICON_TYPES = %w[
diff --git a/app/models/wiki_page.rb b/app/models/wiki_page.rb
index 1fa29e5b933e41d3ab555cbf82e436cc8a760893..68241d2bd95f384d57574efcf09cff0396c5f6d3 100644
--- a/app/models/wiki_page.rb
+++ b/app/models/wiki_page.rb
@@ -134,6 +134,12 @@ class WikiPage
@version ||= @page.version
end
+ def path
+ return unless persisted?
+
+ @path ||= @page.path
+ end
+
def versions(options = {})
return [] unless persisted?
diff --git a/app/policies/commit_policy.rb b/app/policies/commit_policy.rb
index 4d4f0ba92674f4e2d24d398605772d2abd3fb15e..4b358c45ec29c4c66bc13c4182bef07f7f40b60c 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/group_policy.rb b/app/policies/group_policy.rb
index 9e8ee3acf00e63da72c83555197834ce9dac0f1e..13e5b4ae41a8ccd8ac8301dc040edc083ddf1ff5 100644
--- a/app/policies/group_policy.rb
+++ b/app/policies/group_policy.rb
@@ -131,6 +131,8 @@ class GroupPolicy < BasePolicy
rule { owner | admin }.enable :read_statistics
+ rule { maintainer & can?(:create_projects) }.enable :transfer_projects
+
def access_level
return GroupMember::NO_ACCESS if @user.nil?
diff --git a/app/policies/namespace_policy.rb b/app/policies/namespace_policy.rb
index fd3bdddded6a84c49d3e5f0df37c70e52742020a..350dd2084996ee0406d789439d081beaff8e721a 100644
--- a/app/policies/namespace_policy.rb
+++ b/app/policies/namespace_policy.rb
@@ -15,6 +15,8 @@ class NamespacePolicy < BasePolicy
end
rule { personal_project & ~can_create_personal_project }.prevent :create_projects
+
+ rule { (owner | admin) & can?(:create_projects) }.enable :transfer_projects
end
NamespacePolicy.prepend_if_ee('EE::NamespacePolicy')
diff --git a/app/policies/note_policy.rb b/app/policies/note_policy.rb
index b2af6c874c7981a90dc8604db680fd9c04539470..dcde8cefa0dfbdd9a7737334da3504c798c4adfd 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/auto_merge/base_service.rb b/app/services/auto_merge/base_service.rb
index e06659a39cdb1c934a6498fd0ed53fa2fac66512..e08b4ac22605a66e49d6a1bf6ddc1bc896eeb8d2 100644
--- a/app/services/auto_merge/base_service.rb
+++ b/app/services/auto_merge/base_service.rb
@@ -3,12 +3,13 @@
module AutoMerge
class BaseService < ::BaseService
include Gitlab::Utils::StrongMemoize
+ include MergeRequests::AssignsMergeParams
def execute(merge_request)
- merge_request.merge_params.merge!(params)
+ assign_allowed_merge_params(merge_request, params.merge(auto_merge_strategy: strategy))
+
merge_request.auto_merge_enabled = true
merge_request.merge_user = current_user
- merge_request.auto_merge_strategy = strategy
return :failed unless merge_request.save
@@ -21,7 +22,7 @@ module AutoMerge
end
def update(merge_request)
- merge_request.merge_params.merge!(params)
+ assign_allowed_merge_params(merge_request, params.merge(auto_merge_strategy: strategy))
return :failed unless merge_request.save
diff --git a/app/services/concerns/merge_requests/assigns_merge_params.rb b/app/services/concerns/merge_requests/assigns_merge_params.rb
new file mode 100644
index 0000000000000000000000000000000000000000..bd870d9a1e78cf08a32a8d8543e517a352b6d0ea
--- /dev/null
+++ b/app/services/concerns/merge_requests/assigns_merge_params.rb
@@ -0,0 +1,24 @@
+# frozen_string_literal: true
+
+module MergeRequests
+ module AssignsMergeParams
+ def self.included(klass)
+ raise "#{self} can not be included in #{klass} without implementing #current_user" unless klass.method_defined?(:current_user)
+ end
+
+ def assign_allowed_merge_params(merge_request, merge_params)
+ known_merge_params = merge_params.to_h.with_indifferent_access.slice(*MergeRequest::KNOWN_MERGE_PARAMS)
+
+ # Not checking `MergeRequest#can_remove_source_branch` as that includes
+ # other checks that aren't needed here.
+ known_merge_params.delete(:force_remove_source_branch) unless current_user.can?(:push_code, merge_request.source_project)
+
+ merge_request.merge_params.merge!(known_merge_params)
+
+ # Delete the known params now that they're assigned, so we don't try to
+ # assign them through an `#assign_attributes` later.
+ # They could be coming in as strings or symbols
+ merge_params.to_h.with_indifferent_access.except!(*MergeRequest::KNOWN_MERGE_PARAMS)
+ end
+ end
+end
diff --git a/app/services/error_tracking/list_projects_service.rb b/app/services/error_tracking/list_projects_service.rb
index 8d08f0cda9434a89adf33160e1ebab96d0f2984f..92d4ef85ecf67c9c9331e365e524c1f8228792df 100644
--- a/app/services/error_tracking/list_projects_service.rb
+++ b/app/services/error_tracking/list_projects_service.rb
@@ -32,7 +32,7 @@ module ErrorTracking
project_slug: 'proj'
)
- setting.token = params[:token]
+ setting.token = token(setting)
setting.enabled = true
end
end
@@ -40,5 +40,12 @@ module ErrorTracking
def can_read?
can?(current_user, :read_sentry_issue, project)
end
+
+ def token(setting)
+ # Use param token if not masked, otherwise use database token
+ return params[:token] unless /\A\*+\z/.match?(params[:token])
+
+ setting.token
+ end
end
end
diff --git a/app/services/merge_requests/base_service.rb b/app/services/merge_requests/base_service.rb
index 7d4227e4a4170ecbc6835d8df6de8a47afa17230..aacc3d6831e6e5555a392264ad922be4ef725d5b 100644
--- a/app/services/merge_requests/base_service.rb
+++ b/app/services/merge_requests/base_service.rb
@@ -2,6 +2,8 @@
module MergeRequests
class BaseService < ::IssuableBaseService
+ include MergeRequests::AssignsMergeParams
+
def create_note(merge_request, state = merge_request.state)
SystemNoteService.change_status(merge_request, merge_request.target_project, current_user, state, nil)
end
@@ -29,6 +31,18 @@ module MergeRequests
private
+ def create(merge_request)
+ self.params = assign_allowed_merge_params(merge_request, params)
+
+ super
+ end
+
+ def update(merge_request)
+ self.params = assign_allowed_merge_params(merge_request, params)
+
+ super
+ end
+
def handle_wip_event(merge_request)
if wip_event = params.delete(:wip_event)
# We update the title that is provided in the params or we use the mr title
diff --git a/app/services/merge_requests/build_service.rb b/app/services/merge_requests/build_service.rb
index 214f145d09b1775ea146a0ad6bfba532c443ebe2..bf4da01723b660bb04eaa072290575c73e74eba0 100644
--- a/app/services/merge_requests/build_service.rb
+++ b/app/services/merge_requests/build_service.rb
@@ -10,13 +10,14 @@ module MergeRequests
# TODO: this should handle all quick actions that don't have side effects
# https://gitlab.com/gitlab-org/gitlab-foss/issues/53658
merge_quick_actions_into_params!(merge_request, only: [:target_branch])
- merge_request.merge_params['force_remove_source_branch'] = params.delete(:force_remove_source_branch) if params.has_key?(:force_remove_source_branch)
# Assign the projects first so we can use policies for `filter_params`
merge_request.author = current_user
merge_request.source_project = find_source_project
merge_request.target_project = find_target_project
+ self.params = assign_allowed_merge_params(merge_request, params)
+
filter_params(merge_request)
# merge_request.assign_attributes(...) below is a Rails
diff --git a/app/services/merge_requests/create_service.rb b/app/services/merge_requests/create_service.rb
index 1c730232abbf1d54803da21e7902f7c977b3daab..9a37a0330fc19d2f2abd82f0afdf3f192eeb7b65 100644
--- a/app/services/merge_requests/create_service.rb
+++ b/app/services/merge_requests/create_service.rb
@@ -9,7 +9,6 @@ module MergeRequests
merge_request.target_project = @project
merge_request.source_project = @source_project
merge_request.source_branch = params[:source_branch]
- merge_request.merge_params['force_remove_source_branch'] = params.delete(:force_remove_source_branch)
create(merge_request)
end
diff --git a/app/services/merge_requests/update_service.rb b/app/services/merge_requests/update_service.rb
index ae678d4c036996241e51ef4c0434eb7e064542ff..7c9abb12b6e41db3a4ac9c1e18799ce14223e572 100644
--- a/app/services/merge_requests/update_service.rb
+++ b/app/services/merge_requests/update_service.rb
@@ -16,10 +16,6 @@ module MergeRequests
params.delete(:force_remove_source_branch)
end
- if params.has_key?(:force_remove_source_branch)
- merge_request.merge_params['force_remove_source_branch'] = params.delete(:force_remove_source_branch)
- end
-
handle_wip_event(merge_request)
update_task_event(merge_request) || update(merge_request)
end
diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb
index b56b2cf14e361a204660c822716fea115fe4e083..1709474a6c7b5302316622bb7a240116bffee354 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/operations/update_service.rb b/app/services/projects/operations/update_service.rb
index 64519501ff475caf2523b38e6a1d05c0f83da729..0ca89664304cce76dc82a07ead8c402a39c0d80c 100644
--- a/app/services/projects/operations/update_service.rb
+++ b/app/services/projects/operations/update_service.rb
@@ -36,15 +36,17 @@ module Projects
organization_slug: settings.dig(:project, :organization_slug)
)
- {
+ params = {
error_tracking_setting_attributes: {
api_url: api_url,
- token: settings[:token],
enabled: settings[:enabled],
project_name: settings.dig(:project, :name),
organization_name: settings.dig(:project, :organization_name)
}
}
+ params[:error_tracking_setting_attributes][:token] = settings[:token] unless /\A\*+\z/.match?(settings[:token]) # Don't update token if we receive masked value
+
+ params
end
def grafana_integration_params
diff --git a/app/services/projects/participants_service.rb b/app/services/projects/participants_service.rb
index 7080f388e53e9352d018698066969bf18ad39f04..1cd81fe37c7b6859596d3efd2e49141f034fa564 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/app/services/projects/transfer_service.rb b/app/services/projects/transfer_service.rb
index 4b3aca58dd7acd5ceb817d097fb645c9eccfa64e..525fc18b312c6ca93b7a64b1d9200fa86ab82a25 100644
--- a/app/services/projects/transfer_service.rb
+++ b/app/services/projects/transfer_service.rb
@@ -98,7 +98,7 @@ module Projects
@new_namespace &&
can?(current_user, :change_namespace, project) &&
@new_namespace.id != project.namespace_id &&
- current_user.can?(:create_projects, @new_namespace)
+ current_user.can?(:transfer_projects, @new_namespace)
end
def update_namespace_and_visibility(to_namespace)
diff --git a/app/views/projects/settings/operations/_error_tracking.html.haml b/app/views/projects/settings/operations/_error_tracking.html.haml
index 583fc08f3751009e6538bdb768ffa7bdee5ed6c0..589d3037eba4218e97037081ba6346a0d7139c38 100644
--- a/app/views/projects/settings/operations/_error_tracking.html.haml
+++ b/app/views/projects/settings/operations/_error_tracking.html.haml
@@ -17,4 +17,4 @@
project: error_tracking_setting_project_json,
api_host: setting.api_host,
enabled: setting.enabled.to_json,
- token: setting.token } }
+ token: setting.token.present? ? '*' * 12 : nil } }
diff --git a/doc/user/project/members/share_project_with_groups.md b/doc/user/project/members/share_project_with_groups.md
index 9340d23967794d9caaf1d6c128ba799788cea184..79fb2ea50a043bce803d52c911b6f73be2824e35 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 0000000000000000000000000000000000000000..ccf9e5973072b2985ae621adf41a550089c3ad62
--- /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/lib/gitlab/other_markup.rb b/lib/gitlab/other_markup.rb
index bc467486eee09072517a66bc95a0021b34c4e631..0dd6b8a809c76796be86801ed619ffb44ada24d7 100644
--- a/lib/gitlab/other_markup.rb
+++ b/lib/gitlab/other_markup.rb
@@ -10,7 +10,7 @@ module Gitlab
def self.render(file_name, input, context)
html = GitHub::Markup.render(file_name, input)
.force_encoding(input.encoding)
- context[:pipeline] = :markup
+ context[:pipeline] ||= :markup
html = Banzai.render(html, context)
diff --git a/lib/gitlab/search_results.rb b/lib/gitlab/search_results.rb
index 782ac534a7bd70caa0ee6ed44a4a31db702350eb..d74e64116cadfa3d3e94847a58cba7eeb1a461d2 100644
--- a/lib/gitlab/search_results.rb
+++ b/lib/gitlab/search_results.rb
@@ -163,7 +163,7 @@ module Gitlab
return Milestone.none if project_ids.nil?
authorized_project_ids_relation =
- Project.where(id: project_ids).ids_with_milestone_available_for(current_user)
+ Project.where(id: project_ids).ids_with_issuables_available_for(current_user)
milestones.where(project_id: authorized_project_ids_relation)
end
diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb
index ed91b5973b883aa65444541ecb997dc136939ffd..993e4020a75b81cad7d8b3d0d6e305f5f86266db 100644
--- a/spec/controllers/application_controller_spec.rb
+++ b/spec/controllers/application_controller_spec.rb
@@ -202,7 +202,7 @@ describe ApplicationController do
expect(response).to have_gitlab_http_status(404)
end
- it 'redirects to login page via authenticate_user! if not authenticated' do
+ it 'redirects to login page if not authenticated' do
get :index
expect(response).to redirect_to new_user_session_path
diff --git a/spec/controllers/concerns/internal_redirect_spec.rb b/spec/controllers/concerns/internal_redirect_spec.rb
index da68c8c869740866f197dfd66382eb441f97cbab..e5e50cfd55eebe928b91d98f53617813dc8ad0da 100644
--- a/spec/controllers/concerns/internal_redirect_spec.rb
+++ b/spec/controllers/concerns/internal_redirect_spec.rb
@@ -19,7 +19,8 @@ describe InternalRedirect do
[
'Hello world',
'//example.com/hello/world',
- 'https://example.com/hello/world'
+ 'https://example.com/hello/world',
+ "not-starting-with-a-slash\n/starting/with/slash"
]
end
diff --git a/spec/controllers/concerns/lfs_request_spec.rb b/spec/controllers/concerns/lfs_request_spec.rb
index cb8c0b8f71c376b87e91c08e19d06061ffd9a7a6..823b9a504341f57f8cf0b538c97fc5af746b3ba4 100644
--- a/spec/controllers/concerns/lfs_request_spec.rb
+++ b/spec/controllers/concerns/lfs_request_spec.rb
@@ -16,13 +16,17 @@ describe LfsRequest do
end
def project
- @project ||= Project.find(params[:id])
+ @project ||= Project.find_by(id: params[:id])
end
def download_request?
true
end
+ def upload_request?
+ false
+ end
+
def ci?
false
end
@@ -49,4 +53,41 @@ describe LfsRequest do
expect(assigns(:storage_project)).to eq(project)
end
end
+
+ context 'user is authenticated without access to lfs' do
+ before do
+ allow(controller).to receive(:authenticate_user)
+ allow(controller).to receive(:authentication_result) do
+ Gitlab::Auth::Result.new
+ end
+ end
+
+ context 'with access to the project' do
+ it 'returns 403' do
+ get :show, params: { id: project.id }
+
+ expect(response.status).to eq(403)
+ end
+ end
+
+ context 'without access to the project' do
+ context 'project does not exist' do
+ it 'returns 404' do
+ get :show, params: { id: 'does not exist' }
+
+ expect(response.status).to eq(404)
+ end
+ end
+
+ context 'project is private' do
+ let(:project) { create(:project, :private) }
+
+ it 'returns 404' do
+ get :show, params: { id: project.id }
+
+ expect(response.status).to eq(404)
+ 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 34765ae395164625803d40728ef02b97cf79ebea..fc8fe1ac4f603b8dd16959d0988c144bd4144ea2 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/controllers/projects/commits_controller_spec.rb b/spec/controllers/projects/commits_controller_spec.rb
index 9c4d6fdcb2abeae4288e92d9a8a6b017b47510dd..1977e92e42bc293faef869799c94ff0b866db339 100644
--- a/spec/controllers/projects/commits_controller_spec.rb
+++ b/spec/controllers/projects/commits_controller_spec.rb
@@ -142,7 +142,7 @@ describe Projects::CommitsController do
context 'token authentication' do
context 'public project' do
- it_behaves_like 'authenticates sessionless user', :show, :atom, public: true do
+ it_behaves_like 'authenticates sessionless user', :show, :atom, { public: true, ignore_incrementing: true } do
before do
public_project = create(:project, :repository, :public)
@@ -152,7 +152,7 @@ describe Projects::CommitsController do
end
context 'private project' do
- it_behaves_like 'authenticates sessionless user', :show, :atom, public: false do
+ it_behaves_like 'authenticates sessionless user', :show, :atom, { public: false, ignore_incrementing: true } do
before do
private_project = create(:project, :repository, :private)
private_project.add_maintainer(user)
diff --git a/spec/controllers/projects/error_tracking_controller_spec.rb b/spec/controllers/projects/error_tracking_controller_spec.rb
index 4c224e960a636687adcc480357879839cea5fd26..31868f5f7173167cb2b633b0044f6da2682089ea 100644
--- a/spec/controllers/projects/error_tracking_controller_spec.rb
+++ b/spec/controllers/projects/error_tracking_controller_spec.rb
@@ -146,7 +146,7 @@ describe Projects::ErrorTrackingController do
it 'redirects to sign-in page' do
post :list_projects, params: list_projects_params
- expect(response).to have_gitlab_http_status(:unauthorized)
+ expect(response).to have_gitlab_http_status(:redirect)
end
end
diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb
index c9558abab333b8828e77971218ec60bad1325688..d36336a9f67c4dc606240803daaec96d851219d6 100644
--- a/spec/controllers/projects/issues_controller_spec.rb
+++ b/spec/controllers/projects/issues_controller_spec.rb
@@ -1441,7 +1441,7 @@ describe Projects::IssuesController do
context 'private project with token authentication' do
let(:private_project) { create(:project, :private) }
- it_behaves_like 'authenticates sessionless user', :index, :atom do
+ it_behaves_like 'authenticates sessionless user', :index, :atom, ignore_incrementing: true do
before do
default_params.merge!(project_id: private_project, namespace_id: private_project.namespace)
@@ -1449,7 +1449,7 @@ describe Projects::IssuesController do
end
end
- it_behaves_like 'authenticates sessionless user', :calendar, :ics do
+ it_behaves_like 'authenticates sessionless user', :calendar, :ics, ignore_incrementing: true do
before do
default_params.merge!(project_id: private_project, namespace_id: private_project.namespace)
diff --git a/spec/controllers/projects/tags_controller_spec.rb b/spec/controllers/projects/tags_controller_spec.rb
index b99b5d611fc99796f20616cf904d3bcf8d393fdd..f077b4c99fc6d6fb6b71ca7efa87b0658d247e2e 100644
--- a/spec/controllers/projects/tags_controller_spec.rb
+++ b/spec/controllers/projects/tags_controller_spec.rb
@@ -41,7 +41,7 @@ describe Projects::TagsController do
context 'private project with token authentication' do
let(:private_project) { create(:project, :repository, :private) }
- it_behaves_like 'authenticates sessionless user', :index, :atom do
+ it_behaves_like 'authenticates sessionless user', :index, :atom, ignore_incrementing: true do
before do
default_params.merge!(project_id: private_project, namespace_id: private_project.namespace)
diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb
index ea7dd78329a8a3b65b5560b5045b330c2dfe2878..e0df9556eb8bfe645212d0b76c6511a02d715a9e 100644
--- a/spec/controllers/projects_controller_spec.rb
+++ b/spec/controllers/projects_controller_spec.rb
@@ -1149,7 +1149,7 @@ describe ProjectsController do
context 'private project with token authentication' do
let(:private_project) { create(:project, :private) }
- it_behaves_like 'authenticates sessionless user', :show, :atom do
+ it_behaves_like 'authenticates sessionless user', :show, :atom, ignore_incrementing: true do
before do
default_params.merge!(id: private_project, namespace_id: private_project.namespace)
diff --git a/spec/features/projects/pipelines/pipelines_spec.rb b/spec/features/projects/pipelines/pipelines_spec.rb
index 4fb72eb87379ddb8f37fedb3dc26b9a7f3b2be5d..76d8ad1638bac165d3b021ca971e20ba8685c2dc 100644
--- a/spec/features/projects/pipelines/pipelines_spec.rb
+++ b/spec/features/projects/pipelines/pipelines_spec.rb
@@ -827,7 +827,10 @@ describe 'Pipelines', :js do
context 'when project is private' do
let(:project) { create(:project, :private, :repository) }
- it { expect(page).to have_content 'You need to sign in' }
+ it 'redirects the user to sign_in and displays the flash alert' do
+ expect(page).to have_content 'You need to sign in'
+ expect(page.current_path).to eq("/users/sign_in")
+ end
end
end
diff --git a/spec/features/projects/tags/user_views_tags_spec.rb b/spec/features/projects/tags/user_views_tags_spec.rb
index f344b6827159ffeac1d6279ae2af292a916d6c07..bc570f502bf1b37e08d89c54d85415070144cae4 100644
--- a/spec/features/projects/tags/user_views_tags_spec.rb
+++ b/spec/features/projects/tags/user_views_tags_spec.rb
@@ -15,7 +15,7 @@ describe 'User views tags', :feature do
it do
visit project_tags_path(project, format: :atom)
- expect(page).to have_gitlab_http_status(401)
+ expect(page.current_path).to eq("/users/sign_in")
end
end
diff --git a/spec/finders/labels_finder_spec.rb b/spec/finders/labels_finder_spec.rb
index 2681f098fec2cce7dbe79a6090d14744844d2d47..7590c399cf9cf07643821f5de901010bdbd12616 100644
--- a/spec/finders/labels_finder_spec.rb
+++ b/spec/finders/labels_finder_spec.rb
@@ -128,6 +128,89 @@ describe LabelsFinder do
expect(finder.execute).to eq [private_subgroup_label_1]
end
end
+
+ context 'when including labels from group projects with limited visibility' do
+ let(:finder) { described_class.new(user, group_id: group_4.id) }
+ let(:group_4) { create(:group) }
+ let(:limited_visibility_project) { create(:project, :public, group: group_4) }
+ let(:visible_project) { create(:project, :public, group: group_4) }
+ let!(:group_label_1) { create(:group_label, group: group_4) }
+ let!(:limited_visibility_label) { create(:label, project: limited_visibility_project) }
+ let!(:visible_label) { create(:label, project: visible_project) }
+
+ shared_examples 'with full visibility' do
+ it 'returns all projects labels' do
+ expect(finder.execute).to eq [group_label_1, limited_visibility_label, visible_label]
+ end
+ end
+
+ shared_examples 'with limited visibility' do
+ it 'returns only authorized projects labels' do
+ expect(finder.execute).to eq [group_label_1, visible_label]
+ end
+ end
+
+ context 'when merge requests and issues are not visible for non members' do
+ before do
+ limited_visibility_project.project_feature.update!(
+ merge_requests_access_level: ProjectFeature::PRIVATE,
+ issues_access_level: ProjectFeature::PRIVATE
+ )
+ end
+
+ context 'when user is not a group member' do
+ it_behaves_like 'with limited visibility'
+ end
+
+ context 'when user is a group member' do
+ before do
+ group_4.add_developer(user)
+ end
+
+ it_behaves_like 'with full visibility'
+ end
+ end
+
+ context 'when merge requests are not visible for non members' do
+ before do
+ limited_visibility_project.project_feature.update!(
+ merge_requests_access_level: ProjectFeature::PRIVATE
+ )
+ end
+
+ context 'when user is not a group member' do
+ it_behaves_like 'with full visibility'
+ end
+
+ context 'when user is a group member' do
+ before do
+ group_4.add_developer(user)
+ end
+
+ it_behaves_like 'with full visibility'
+ end
+ end
+
+ context 'when issues are not visible for non members' do
+ before do
+ limited_visibility_project.project_feature.update!(
+ issues_access_level: ProjectFeature::PRIVATE
+ )
+ end
+
+ context 'when user is not a group member' do
+ it_behaves_like 'with full visibility'
+ end
+
+ context 'when user is a group member' do
+ before do
+ group_4.add_developer(user)
+ end
+
+ it_behaves_like 'with full visibility'
+ end
+ end
+ end
end
context 'filtering by project_id' do
diff --git a/spec/fixtures/api/graphql/recursive-introspection.graphql b/spec/fixtures/api/graphql/recursive-introspection.graphql
new file mode 100644
index 0000000000000000000000000000000000000000..db970bb14b6a92bd27d02de39a2b2cd661ea97e2
--- /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 0000000000000000000000000000000000000000..d1616c4de6eb1968d6b50bae4135349576f22c4e
--- /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 0000000000000000000000000000000000000000..1025043b4744e9fb25a1a4d09e0cf7aa826b8de4
--- /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/helpers/markup_helper_spec.rb b/spec/helpers/markup_helper_spec.rb
index 364f215420abbf081751c83659ae3a7507717d2c..32851249b2e25e488f0278e9825d8daece6e98e2 100644
--- a/spec/helpers/markup_helper_spec.rb
+++ b/spec/helpers/markup_helper_spec.rb
@@ -3,18 +3,18 @@
require 'spec_helper'
describe MarkupHelper do
- let!(:project) { create(:project, :repository) }
-
- let(:user) { create(:user, username: 'gfm') }
- let(:commit) { project.commit }
- let(:issue) { create(:issue, project: project) }
- let(:merge_request) { create(:merge_request, source_project: project, target_project: project) }
- let(:snippet) { create(:project_snippet, project: project) }
-
- before do
- # Ensure the generated reference links aren't redacted
+ set(:project) { create(:project, :repository) }
+ set(:user) do
+ user = create(:user, username: 'gfm')
project.add_maintainer(user)
+ user
+ end
+ set(:issue) { create(:issue, project: project) }
+ set(:merge_request) { create(:merge_request, source_project: project, target_project: project) }
+ set(:snippet) { create(:project_snippet, project: project) }
+ let(:commit) { project.commit }
+ before do
# Helper expects a @project instance variable
helper.instance_variable_set(:@project, project)
@@ -44,8 +44,8 @@ describe MarkupHelper do
describe "override default project" do
let(:actual) { issue.to_reference }
- let(:second_project) { create(:project, :public) }
- let(:second_issue) { create(:issue, project: second_project) }
+ set(:second_project) { create(:project, :public) }
+ set(:second_issue) { create(:issue, project: second_project) }
it 'links to the issue' do
expected = urls.project_issue_path(second_project, second_issue)
@@ -55,7 +55,7 @@ describe MarkupHelper do
describe 'uploads' do
let(:text) { "" }
- let(:group) { create(:group) }
+ set(:group) { create(:group) }
subject { helper.markdown(text) }
@@ -77,7 +77,7 @@ describe MarkupHelper do
end
describe "with a group in the context" do
- let(:project_in_group) { create(:project, group: group) }
+ set(:project_in_group) { create(:project, group: group) }
before do
helper.instance_variable_set(:@group, group)
@@ -235,38 +235,48 @@ describe MarkupHelper do
end
describe '#render_wiki_content' do
+ let(:wiki) { double('WikiPage', path: "file.#{extension}") }
+ let(:context) do
+ {
+ pipeline: :wiki, project: project, project_wiki: wiki,
+ page_slug: 'nested/page', issuable_state_filter_enabled: true
+ }
+ end
+
before do
- @wiki = double('WikiPage')
- allow(@wiki).to receive(:content).and_return('wiki content')
- allow(@wiki).to receive(:slug).and_return('nested/page')
- helper.instance_variable_set(:@project_wiki, @wiki)
+ expect(wiki).to receive(:content).and_return('wiki content')
+ expect(wiki).to receive(:slug).and_return('nested/page')
+ helper.instance_variable_set(:@project_wiki, wiki)
end
- it "uses Wiki pipeline for markdown files" do
- allow(@wiki).to receive(:format).and_return(:markdown)
+ context 'when file is Markdown' do
+ let(:extension) { 'md' }
- expect(helper).to receive(:markdown_unsafe).with('wiki content',
- pipeline: :wiki, project: project, project_wiki: @wiki, page_slug: "nested/page",
- issuable_state_filter_enabled: true)
+ it 'renders using #markdown_unsafe helper method' do
+ expect(helper).to receive(:markdown_unsafe).with('wiki content', context)
- helper.render_wiki_content(@wiki)
+ helper.render_wiki_content(wiki)
+ end
end
- it "uses Asciidoctor for asciidoc files" do
- allow(@wiki).to receive(:format).and_return(:asciidoc)
+ context 'when file is Asciidoc' do
+ let(:extension) { 'adoc' }
- expect(helper).to receive(:asciidoc_unsafe).with('wiki content')
+ it 'renders using Gitlab::Asciidoc' do
+ expect(Gitlab::Asciidoc).to receive(:render)
- helper.render_wiki_content(@wiki)
+ helper.render_wiki_content(wiki)
+ end
end
- it "uses the Gollum renderer for all other file types" do
- allow(@wiki).to receive(:format).and_return(:rdoc)
- formatted_content_stub = double('formatted_content')
- expect(formatted_content_stub).to receive(:html_safe)
- allow(@wiki).to receive(:formatted_content).and_return(formatted_content_stub)
+ context 'any other format' do
+ let(:extension) { 'foo' }
- helper.render_wiki_content(@wiki)
+ it 'renders all other formats using Gitlab::OtherMarkup' do
+ expect(Gitlab::OtherMarkup).to receive(:render)
+
+ helper.render_wiki_content(wiki)
+ end
end
end
diff --git a/spec/models/milestone_spec.rb b/spec/models/milestone_spec.rb
index 2ecbe548520d45eb0512f3d575c83a51557df03a..120ba67f3284c704402a45a543fcce365f6fdde7 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 4c320b4b145c6d7dcbdc7ec93f99432c37b77c58..3ab88b52568b717cf927953307eae41326ac735e 100644
--- a/spec/models/note_spec.rb
+++ b/spec/models/note_spec.rb
@@ -379,6 +379,63 @@ describe Note do
expect(label_note.cross_reference?).to be_falsy
end
end
+
+ context 'when system note metadata is not present' do
+ let(:note) { build(:note, :system) }
+
+ before do
+ allow(note).to receive(:system_note_metadata).and_return(nil)
+ end
+
+ it 'delegates to the system note service' do
+ expect(SystemNotes::IssuablesService).to receive(:cross_reference?).with(note.note)
+
+ note.cross_reference?
+ end
+ end
+
+ context 'with a system note' do
+ let(:issue) { create(:issue, project: create(:project, :repository)) }
+ let(:note) { create(:system_note, note: "test", noteable: issue, project: issue.project) }
+
+ shared_examples 'system_note_metadata includes note action' do
+ it 'delegates to the cross-reference regex' do
+ expect(note).to receive(:matches_cross_reference_regex?)
+
+ note.cross_reference?
+ end
+ end
+
+ context 'with :label action' do
+ let!(:metadata) {create(:system_note_metadata, note: note, action: :label)}
+
+ it_behaves_like 'system_note_metadata includes note action'
+
+ it { expect(note.cross_reference?).to be_falsy }
+
+ context 'with cross reference label note' do
+ let(:label) { create(:label, project: issue.project)}
+ let(:note) { create(:system_note, note: "added #{label.to_reference} label", noteable: issue, project: issue.project) }
+
+ it { expect(note.cross_reference?).to be_truthy }
+ end
+ end
+
+ context 'with :milestone action' do
+ let!(:metadata) {create(:system_note_metadata, note: note, action: :milestone)}
+
+ it_behaves_like 'system_note_metadata includes note action'
+
+ it { expect(note.cross_reference?).to be_falsy }
+
+ context 'with cross reference milestone note' do
+ let(:milestone) { create(:milestone, project: issue.project)}
+ let(:note) { create(:system_note, note: "added #{milestone.to_reference} milestone", noteable: issue, project: issue.project) }
+
+ it { expect(note.cross_reference?).to be_truthy }
+ end
+ end
+ end
end
describe 'clear_blank_line_code!' do
@@ -578,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 9f3313e67b5e802e44091792998f24a31fcfa836..1bda3094e7513ce2e1dd9053e1d10f332aadcfcf 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -3416,7 +3416,7 @@ describe Project do
end
end
- describe '.ids_with_milestone_available_for' do
+ describe '.ids_with_issuables_available_for' do
let!(:user) { create(:user) }
it 'returns project ids with milestones available for user' do
@@ -3426,7 +3426,7 @@ describe Project do
project_4 = create(:project, :public)
project_4.project_feature.update(issues_access_level: ProjectFeature::PRIVATE, merge_requests_access_level: ProjectFeature::PRIVATE )
- project_ids = described_class.ids_with_milestone_available_for(user).pluck(:id)
+ project_ids = described_class.ids_with_issuables_available_for(user).pluck(:id)
expect(project_ids).to include(project_2.id, project_3.id)
expect(project_ids).not_to include(project_1.id, project_4.id)
@@ -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/models/wiki_page_spec.rb b/spec/models/wiki_page_spec.rb
index 18c62c917dc742c3f4a3cab7a9dc74dc21d2734e..9014276dcf8c8b43ab6e131280295bcbc2a54153 100644
--- a/spec/models/wiki_page_spec.rb
+++ b/spec/models/wiki_page_spec.rb
@@ -439,6 +439,23 @@ describe WikiPage do
end
end
+ describe '#path' do
+ let(:path) { 'mypath.md' }
+ let(:wiki_page) { instance_double('Gitlab::Git::WikiPage', path: path).as_null_object }
+
+ it 'returns the path when persisted' do
+ page = described_class.new(wiki, wiki_page, true)
+
+ expect(page.path).to eq(path)
+ end
+
+ it 'returns nil when not persisted' do
+ page = described_class.new(wiki, wiki_page, false)
+
+ expect(page.path).to be_nil
+ end
+ end
+
describe '#directory' do
context 'when the page is at the root directory' do
it 'returns an empty string' do
diff --git a/spec/policies/commit_policy_spec.rb b/spec/policies/commit_policy_spec.rb
index 41f6fb0842620190e8bcf7e003f5e946a9ad7639..40183f51e9e9f726aad2e4c53cd611d9de648f1e 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/policies/group_policy_spec.rb b/spec/policies/group_policy_spec.rb
index 603e7e874c9c81f366e46bedf822f7cf39ecdb81..aeb09c1dc3ac02829c7ca68b36f25f9e11bdbe01 100644
--- a/spec/policies/group_policy_spec.rb
+++ b/spec/policies/group_policy_spec.rb
@@ -354,6 +354,88 @@ describe GroupPolicy do
end
end
+ context 'transfer_projects' do
+ shared_examples_for 'allowed to transfer projects' do
+ before do
+ group.update(project_creation_level: project_creation_level)
+ end
+
+ it { is_expected.to be_allowed(:transfer_projects) }
+ end
+
+ shared_examples_for 'not allowed to transfer projects' do
+ before do
+ group.update(project_creation_level: project_creation_level)
+ end
+
+ it { is_expected.to be_disallowed(:transfer_projects) }
+ end
+
+ context 'reporter' do
+ let(:current_user) { reporter }
+
+ it_behaves_like 'not allowed to transfer projects' do
+ let(:project_creation_level) { ::Gitlab::Access::NO_ONE_PROJECT_ACCESS }
+ end
+
+ it_behaves_like 'not allowed to transfer projects' do
+ let(:project_creation_level) { ::Gitlab::Access::MAINTAINER_PROJECT_ACCESS }
+ end
+
+ it_behaves_like 'not allowed to transfer projects' do
+ let(:project_creation_level) { ::Gitlab::Access::DEVELOPER_MAINTAINER_PROJECT_ACCESS }
+ end
+ end
+
+ context 'developer' do
+ let(:current_user) { developer }
+
+ it_behaves_like 'not allowed to transfer projects' do
+ let(:project_creation_level) { ::Gitlab::Access::NO_ONE_PROJECT_ACCESS }
+ end
+
+ it_behaves_like 'not allowed to transfer projects' do
+ let(:project_creation_level) { ::Gitlab::Access::MAINTAINER_PROJECT_ACCESS }
+ end
+
+ it_behaves_like 'not allowed to transfer projects' do
+ let(:project_creation_level) { ::Gitlab::Access::DEVELOPER_MAINTAINER_PROJECT_ACCESS }
+ end
+ end
+
+ context 'maintainer' do
+ let(:current_user) { maintainer }
+
+ it_behaves_like 'not allowed to transfer projects' do
+ let(:project_creation_level) { ::Gitlab::Access::NO_ONE_PROJECT_ACCESS }
+ end
+
+ it_behaves_like 'allowed to transfer projects' do
+ let(:project_creation_level) { ::Gitlab::Access::MAINTAINER_PROJECT_ACCESS }
+ end
+
+ it_behaves_like 'allowed to transfer projects' do
+ let(:project_creation_level) { ::Gitlab::Access::DEVELOPER_MAINTAINER_PROJECT_ACCESS }
+ end
+ end
+
+ context 'owner' do
+ let(:current_user) { owner }
+
+ it_behaves_like 'not allowed to transfer projects' do
+ let(:project_creation_level) { ::Gitlab::Access::NO_ONE_PROJECT_ACCESS }
+ end
+
+ it_behaves_like 'allowed to transfer projects' do
+ let(:project_creation_level) { ::Gitlab::Access::MAINTAINER_PROJECT_ACCESS }
+ end
+
+ it_behaves_like 'allowed to transfer projects' do
+ let(:project_creation_level) { ::Gitlab::Access::DEVELOPER_MAINTAINER_PROJECT_ACCESS }
+ end
+ end
+ end
+
context "create_projects" do
context 'when group has no project creation level set' do
before_all do
diff --git a/spec/policies/namespace_policy_spec.rb b/spec/policies/namespace_policy_spec.rb
index 216aaae70ee261592b08f5165fede4fbab25f03e..909c17fe8b52059297e90e99d99fb739466e9112 100644
--- a/spec/policies/namespace_policy_spec.rb
+++ b/spec/policies/namespace_policy_spec.rb
@@ -6,7 +6,7 @@ describe NamespacePolicy do
let(:admin) { create(:admin) }
let(:namespace) { create(:namespace, owner: owner) }
- let(:owner_permissions) { [:create_projects, :admin_namespace, :read_namespace, :read_statistics] }
+ let(:owner_permissions) { [:create_projects, :admin_namespace, :read_namespace, :read_statistics, :transfer_projects] }
subject { described_class.new(current_user, namespace) }
@@ -31,6 +31,7 @@ describe NamespacePolicy do
let(:owner) { create(:user, projects_limit: 0) }
it { is_expected.to be_disallowed(:create_projects) }
+ it { is_expected.to be_disallowed(:transfer_projects) }
end
end
diff --git a/spec/requests/api/graphql/gitlab_schema_spec.rb b/spec/requests/api/graphql/gitlab_schema_spec.rb
index e1eb7c7f7389a4b0b09d069c68db4fbed8d0aef8..1e799a0a42a88ffa95b967520b67d1f497983fc7 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/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb
index 8179da2f97c00bf3ad83d80adc5c0d48f3ebc96e..05160a33e616511ab17e58dc9af4e2db8b77ba1f 100644
--- a/spec/requests/api/merge_requests_spec.rb
+++ b/spec/requests/api/merge_requests_spec.rb
@@ -1737,6 +1737,38 @@ describe API::MergeRequests do
expect(json_response['state']).to eq('closed')
expect(json_response['force_remove_source_branch']).to be_truthy
end
+
+ context 'with a merge request across forks' do
+ let(:fork_owner) { create(:user) }
+ let(:source_project) { fork_project(project, fork_owner) }
+ let(:target_project) { project }
+
+ let(:merge_request) do
+ create(:merge_request,
+ source_project: source_project,
+ target_project: target_project,
+ source_branch: 'fixes',
+ merge_params: { 'force_remove_source_branch' => false })
+ end
+
+ it 'is true for an authorized user' do
+ put api("/projects/#{target_project.id}/merge_requests/#{merge_request.iid}", fork_owner), params: { state_event: 'close', remove_source_branch: true }
+
+ expect(response).to have_gitlab_http_status(200)
+ expect(json_response['state']).to eq('closed')
+ expect(json_response['force_remove_source_branch']).to be true
+ end
+
+ it 'is false for an unauthorized user' do
+ expect do
+ put api("/projects/#{target_project.id}/merge_requests/#{merge_request.iid}", target_project.owner), params: { state_event: 'close', remove_source_branch: true }
+ end.not_to change { merge_request.reload.merge_params }
+
+ expect(response).to have_gitlab_http_status(200)
+ expect(json_response['state']).to eq('closed')
+ expect(json_response['force_remove_source_branch']).to be false
+ end
+ end
end
context "to close a MR" do
diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb
index 2d8ef9c06dca8ef7e668e195aa8a7a167062d2ca..99d2a68ef53c2f5c08728d57f9e714b30ab715e8 100644
--- a/spec/requests/api/projects_spec.rb
+++ b/spec/requests/api/projects_spec.rb
@@ -2648,6 +2648,22 @@ describe API::Projects do
expect(response).to have_gitlab_http_status(400)
end
end
+
+ context 'when authenticated as developer' do
+ before do
+ group.add_developer(user)
+ end
+
+ context 'target namespace allows developers to create projects' do
+ let(:group) { create(:group, project_creation_level: ::Gitlab::Access::DEVELOPER_MAINTAINER_PROJECT_ACCESS) }
+
+ it 'fails transferring the project to the target namespace' do
+ put api("/projects/#{project.id}/transfer", user), params: { namespace: group.id }
+
+ expect(response).to have_gitlab_http_status(400)
+ end
+ end
+ end
end
it_behaves_like 'custom attributes endpoints', 'projects' do
diff --git a/spec/services/auto_merge/base_service_spec.rb b/spec/services/auto_merge/base_service_spec.rb
index a409f21a7e45c7922201af6f728686f7cedf834f..0a6bcb1badccc44d598b26160b55290148b9d32b 100644
--- a/spec/services/auto_merge/base_service_spec.rb
+++ b/spec/services/auto_merge/base_service_spec.rb
@@ -51,7 +51,7 @@ describe AutoMerge::BaseService do
expect(merge_request.merge_params['commit_message']).to eq("Merge branch 'patch-12' into 'master'")
expect(merge_request.merge_params['sha']).to eq('200fcc9c260f7219eaf0daba87d818f0922c5b18')
expect(merge_request.merge_params['should_remove_source_branch']).to eq(false)
- expect(merge_request.merge_params['squash']).to eq(false)
+ expect(merge_request.squash).to eq(false)
expect(merge_request.merge_params['squash_commit_message']).to eq('Update README.md')
end
end
@@ -108,7 +108,6 @@ describe AutoMerge::BaseService do
'commit_message' => "Merge branch 'patch-12' into 'master'",
'sha' => "200fcc9c260f7219eaf0daba87d818f0922c5b18",
'should_remove_source_branch' => false,
- 'squash' => false,
'squash_commit_message' => "Update README.md"
}
end
diff --git a/spec/services/auto_merge/merge_when_pipeline_succeeds_service_spec.rb b/spec/services/auto_merge/merge_when_pipeline_succeeds_service_spec.rb
index 931b52470c40c2effc664809ba57f58a1d04ff8c..ccbb4e7c30d7eb2a817c73530790f27add2d1260 100644
--- a/spec/services/auto_merge/merge_when_pipeline_succeeds_service_spec.rb
+++ b/spec/services/auto_merge/merge_when_pipeline_succeeds_service_spec.rb
@@ -59,8 +59,9 @@ describe AutoMerge::MergeWhenPipelineSucceedsService do
it 'sets the params, merge_user, and flag' do
expect(merge_request).to be_valid
expect(merge_request.merge_when_pipeline_succeeds).to be_truthy
- expect(merge_request.merge_params).to include commit_message: 'Awesome message'
+ expect(merge_request.merge_params).to include 'commit_message' => 'Awesome message'
expect(merge_request.merge_user).to be user
+ expect(merge_request.auto_merge_strategy).to eq AutoMergeService::STRATEGY_MERGE_WHEN_PIPELINE_SUCCEEDS
end
it 'creates a system note' do
@@ -73,7 +74,7 @@ describe AutoMerge::MergeWhenPipelineSucceedsService do
end
context 'already approved' do
- let(:service) { described_class.new(project, user, new_key: true) }
+ let(:service) { described_class.new(project, user, should_remove_source_branch: true) }
let(:build) { create(:ci_build, ref: mr_merge_if_green_enabled.source_branch) }
before do
@@ -90,7 +91,7 @@ describe AutoMerge::MergeWhenPipelineSucceedsService do
expect(SystemNoteService).not_to receive(:merge_when_pipeline_succeeds)
service.execute(mr_merge_if_green_enabled)
- expect(mr_merge_if_green_enabled.merge_params).to have_key(:new_key)
+ expect(mr_merge_if_green_enabled.merge_params).to have_key('should_remove_source_branch')
end
end
end
diff --git a/spec/services/concerns/merge_requests/assigns_merge_params_spec.rb b/spec/services/concerns/merge_requests/assigns_merge_params_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..5b653aa331c53e1f703e50bda7af66c716491b19
--- /dev/null
+++ b/spec/services/concerns/merge_requests/assigns_merge_params_spec.rb
@@ -0,0 +1,70 @@
+require 'spec_helper'
+
+describe MergeRequests::AssignsMergeParams do
+ it 'raises an error when used from an instance that does not respond to #current_user' do
+ define_class = -> { Class.new { include MergeRequests::AssignsMergeParams }.new }
+
+ expect { define_class.call }.to raise_error %r{can not be included in (.*) without implementing #current_user}
+ end
+
+ describe '#assign_allowed_merge_params' do
+ let(:merge_request) { build(:merge_request) }
+
+ let(:params) do
+ { commit_message: 'Commit Message',
+ 'should_remove_source_branch' => true,
+ unknown_symbol: 'Unknown symbol',
+ 'unknown_string' => 'Unknown String' }
+ end
+
+ subject(:merge_request_service) do
+ Class.new do
+ attr_accessor :current_user
+
+ include MergeRequests::AssignsMergeParams
+
+ def initialize(user)
+ @current_user = user
+ end
+ end
+ end
+
+ it 'only assigns known parameters to the merge request' do
+ service = merge_request_service.new(merge_request.author)
+
+ service.assign_allowed_merge_params(merge_request, params)
+
+ expect(merge_request.merge_params).to eq('commit_message' => 'Commit Message', 'should_remove_source_branch' => true)
+ end
+
+ it 'returns a hash without the known merge params' do
+ service = merge_request_service.new(merge_request.author)
+
+ result = service.assign_allowed_merge_params(merge_request, params)
+
+ expect(result).to eq({ 'unknown_symbol' => 'Unknown symbol', 'unknown_string' => 'Unknown String' })
+ end
+
+ context 'the force_remove_source_branch param' do
+ let(:params) { { force_remove_source_branch: true } }
+
+ it 'assigns the param if the user is allowed to do that' do
+ service = merge_request_service.new(merge_request.author)
+
+ result = service.assign_allowed_merge_params(merge_request, params)
+
+ expect(merge_request.force_remove_source_branch?).to be true
+ expect(result).to be_empty
+ end
+
+ it 'only removes the param if the user is not allowed to do that' do
+ service = merge_request_service.new(build(:user))
+
+ result = service.assign_allowed_merge_params(merge_request, params)
+
+ expect(merge_request.force_remove_source_branch?).to be_falsy
+ expect(result).to be_empty
+ end
+ end
+ end
+end
diff --git a/spec/services/error_tracking/list_projects_service_spec.rb b/spec/services/error_tracking/list_projects_service_spec.rb
index 730fccc599e16a150883a86e5725033415cfb36d..a272a604184e14de998f6814cdb3be2753c54dfe 100644
--- a/spec/services/error_tracking/list_projects_service_spec.rb
+++ b/spec/services/error_tracking/list_projects_service_spec.rb
@@ -50,6 +50,19 @@ describe ErrorTracking::ListProjectsService do
end
end
+ context 'masked param token' do
+ let(:params) { ActionController::Parameters.new(token: "*********", api_host: new_api_host) }
+
+ before do
+ expect(error_tracking_setting).to receive(:list_sentry_projects)
+ .and_return({ projects: [] })
+ end
+
+ it 'uses database token' do
+ expect { subject.execute }.not_to change { error_tracking_setting.token }
+ end
+ end
+
context 'sentry client raises exception' do
context 'Sentry::Client::Error' do
before do
diff --git a/spec/services/merge_requests/build_service_spec.rb b/spec/services/merge_requests/build_service_spec.rb
index d546a09268064855c804dff5c8e39e816c755e41..68e53553043ee4b45f06d0d0f89b1647b446fcd9 100644
--- a/spec/services/merge_requests/build_service_spec.rb
+++ b/spec/services/merge_requests/build_service_spec.rb
@@ -83,8 +83,9 @@ describe MergeRequests::BuildService do
expect(merge_request.force_remove_source_branch?).to be_falsey
end
- context 'with force_remove_source_branch parameter' do
+ context 'with force_remove_source_branch parameter when the user is authorized' do
let(:mr_params) { params.merge(force_remove_source_branch: '1') }
+ let(:source_project) { fork_project(project, user) }
let(:merge_request) { described_class.new(project, user, mr_params).execute }
it 'assigns force_remove_source_branch' do
diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb
index d3c4c436901d0b578c711d54d397319faa74472c..d31f5dc0176b8085a371c3f62e389ddfbdf42818 100644
--- a/spec/services/merge_requests/update_service_spec.rb
+++ b/spec/services/merge_requests/update_service_spec.rb
@@ -646,5 +646,29 @@ describe MergeRequests::UpdateService, :mailer do
expect(merge_request.allow_collaboration).to be_truthy
end
end
+
+ context 'updating `force_remove_source_branch`' do
+ let(:target_project) { create(:project, :repository, :public) }
+ let(:source_project) { fork_project(target_project, nil, repository: true) }
+ let(:user) { target_project.owner }
+ let(:merge_request) do
+ create(:merge_request,
+ source_project: source_project,
+ source_branch: 'fixes',
+ target_project: target_project)
+ end
+
+ it "cannot be done by members of the target project when they don't have access" do
+ expect { update_merge_request(force_remove_source_branch: true) }
+ .not_to change { merge_request.reload.force_remove_source_branch? }.from(nil)
+ end
+
+ it 'can be done by members of the target project if they can push to the source project' do
+ source_project.add_developer(user)
+
+ expect { update_merge_request(force_remove_source_branch: true) }
+ .to change { merge_request.reload.force_remove_source_branch? }.from(nil).to(true)
+ end
+ end
end
end
diff --git a/spec/services/projects/operations/update_service_spec.rb b/spec/services/projects/operations/update_service_spec.rb
index b2f9fd6df79790206cdb5852f8d444be2799898a..81d59a98b9b9e12c47ecff4851775c4b83d14270 100644
--- a/spec/services/projects/operations/update_service_spec.rb
+++ b/spec/services/projects/operations/update_service_spec.rb
@@ -145,6 +145,27 @@ describe Projects::Operations::UpdateService do
end
end
+ context 'with masked param token' do
+ let(:params) do
+ {
+ error_tracking_setting_attributes: {
+ enabled: false,
+ token: '*' * 8
+ }
+ }
+ end
+
+ before do
+ create(:project_error_tracking_setting, project: project, token: 'token')
+ end
+
+ it 'does not update token' do
+ expect(result[:status]).to eq(:success)
+
+ expect(project.error_tracking_setting.token).to eq('token')
+ end
+ end
+
context 'with invalid parameters' do
let(:params) { {} }
diff --git a/spec/services/projects/participants_service_spec.rb b/spec/services/projects/participants_service_spec.rb
index 4def83513a478ae7bce78107c0ff60f9da5ed0e0..239d28557eeff33577a6de2b0ca7fed584afbd99 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/services/projects/transfer_service_spec.rb b/spec/services/projects/transfer_service_spec.rb
index 26d8ac9b479b7d074533bd6e39981300c8f0662f..298867f483b3fa06b5117fab345e1b24d0c8cd7e 100644
--- a/spec/services/projects/transfer_service_spec.rb
+++ b/spec/services/projects/transfer_service_spec.rb
@@ -222,6 +222,24 @@ describe Projects::TransferService do
it { expect(project.errors[:new_namespace]).to include('Project with same name or path in target namespace already exists') }
end
+ context 'target namespace allows developers to create projects' do
+ let(:group) { create(:group, project_creation_level: ::Gitlab::Access::DEVELOPER_MAINTAINER_PROJECT_ACCESS) }
+
+ context 'the user is a member of the target namespace with developer permissions' do
+ subject(:transfer_project_result) { transfer_project(project, user, group) }
+
+ before do
+ group.add_developer(user)
+ end
+
+ it 'does not allow project transfer to the target namespace' do
+ expect(transfer_project_result).to eq false
+ expect(project.namespace).to eq(user.namespace)
+ expect(project.errors[:new_namespace]).to include('Transfer failed, please contact an admin.')
+ end
+ end
+ end
+
def transfer_project(project, user, new_namespace)
service = Projects::TransferService.new(project, user)
diff --git a/spec/support/controllers/sessionless_auth_controller_shared_examples.rb b/spec/support/controllers/sessionless_auth_controller_shared_examples.rb
index b5149a0fcb153970006b063ea9710a3b352e27f6..bc95fcd6b884d6bd29c7a0c70a0327cc6dfb1c91 100644
--- a/spec/support/controllers/sessionless_auth_controller_shared_examples.rb
+++ b/spec/support/controllers/sessionless_auth_controller_shared_examples.rb
@@ -34,8 +34,15 @@ shared_examples 'authenticates sessionless user' do |path, format, params|
context 'when the personal access token has no api scope', unless: params[:public] do
it 'does not log the user in' do
- expect(authentication_metrics)
- .to increment(:user_unauthenticated_counter)
+ # Several instances of where these specs are shared route the request
+ # through ApplicationController#route_not_found which does not involve
+ # the usual auth code from Devise, so does not increment the
+ # :user_unauthenticated_counter
+ #
+ unless params[:ignore_incrementing]
+ expect(authentication_metrics)
+ .to increment(:user_unauthenticated_counter)
+ end
personal_access_token.update(scopes: [:read_user])
@@ -84,8 +91,15 @@ shared_examples 'authenticates sessionless user' do |path, format, params|
end
it "doesn't log the user in otherwise", unless: params[:public] do
- expect(authentication_metrics)
- .to increment(:user_unauthenticated_counter)
+ # Several instances of where these specs are shared route the request
+ # through ApplicationController#route_not_found which does not involve
+ # the usual auth code from Devise, so does not increment the
+ # :user_unauthenticated_counter
+ #
+ unless params[:ignore_incrementing]
+ expect(authentication_metrics)
+ .to increment(:user_unauthenticated_counter)
+ end
get path, params: default_params.merge(private_token: 'token')
diff --git a/spec/support/helpers/graphql_helpers.rb b/spec/support/helpers/graphql_helpers.rb
index 4d2ad165fd685f8f61e344ee1576d13c790a8a05..6fb1d279456cd7ad95387a87cb42d9e88f8ea8dc 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
diff --git a/spec/support/shared_examples/controllers/todos_shared_examples.rb b/spec/support/shared_examples/controllers/todos_shared_examples.rb
index f3f9abb7da2527ed0770067766b486ebb2298228..914bf506320930e10cedd3b2c82863b30b155b3b 100644
--- a/spec/support/shared_examples/controllers/todos_shared_examples.rb
+++ b/spec/support/shared_examples/controllers/todos_shared_examples.rb
@@ -39,7 +39,7 @@ shared_examples 'todos actions' do
post_create
end.to change { user.todos.count }.by(0)
- expect(response).to have_gitlab_http_status(parent.is_a?(Group) ? 401 : 302)
+ expect(response).to have_gitlab_http_status(302)
end
end
end