Commit 4c7c9a2b authored by Sean McGivern's avatar Sean McGivern

Extract EE-specific lines from MRs controller

Move access checks to their own method so they can be overridden, and
port an EE-only method to exist in CE too, with an EE-specific override.
parent 1a7a861f
...@@ -11,7 +11,6 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo ...@@ -11,7 +11,6 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
prepend ::EE::Projects::MergeRequestsController prepend ::EE::Projects::MergeRequestsController
skip_before_action :merge_request, only: [:index, :bulk_update] skip_before_action :merge_request, only: [:index, :bulk_update]
before_action :whitelist_query_limiting_ee, only: [:merge, :show]
before_action :whitelist_query_limiting, only: [:assign_related_issues, :update] before_action :whitelist_query_limiting, only: [:assign_related_issues, :update]
before_action :authorize_update_issuable!, only: [:close, :edit, :update, :remove_wip, :sort] before_action :authorize_update_issuable!, only: [:close, :edit, :update, :remove_wip, :sort]
before_action :set_issuables_index, only: [:index] before_action :set_issuables_index, only: [:index]
...@@ -171,8 +170,9 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo ...@@ -171,8 +170,9 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
end end
def merge def merge
return access_denied! unless @merge_request.can_be_merged_by?(current_user) access_check_result = merge_access_check
return render_404 unless @merge_request.approved?
return access_check_result if access_check_result
status = merge! status = merge!
...@@ -260,9 +260,9 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo ...@@ -260,9 +260,9 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
return :failed return :failed
end end
merge_request_service = ::MergeRequests::MergeService.new(@project, current_user, merge_params) merge_service = ::MergeRequests::MergeService.new(@project, current_user, merge_params)
unless merge_request_service.hooks_validation_pass?(@merge_request) unless merge_service.hooks_validation_pass?(@merge_request)
return :hook_validation_error return :hook_validation_error
end end
...@@ -328,13 +328,12 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo ...@@ -328,13 +328,12 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
access_denied! unless access_check access_denied! unless access_check
end end
def merge_access_check
access_denied! unless @merge_request.can_be_merged_by?(current_user)
end
def whitelist_query_limiting def whitelist_query_limiting
# Also see https://gitlab.com/gitlab-org/gitlab-ce/issues/42441 # Also see https://gitlab.com/gitlab-org/gitlab-ce/issues/42441
Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-ce/issues/42438') Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-ce/issues/42438')
end end
def whitelist_query_limiting_ee
# Also see https://gitlab.com/gitlab-org/gitlab-ee/issues/4793
Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-ee/issues/4792')
end
end end
...@@ -51,6 +51,11 @@ module MergeRequests ...@@ -51,6 +51,11 @@ module MergeRequests
end end
end end
# Overridden in EE.
def hooks_validation_pass?(_merge_request)
true
end
private private
def error_check! def error_check!
......
...@@ -5,6 +5,11 @@ module EE ...@@ -5,6 +5,11 @@ module EE
APPROVAL_RENDERING_ACTIONS = [:approve, :approvals, :unapprove].freeze APPROVAL_RENDERING_ACTIONS = [:approve, :approvals, :unapprove].freeze
prepended do
before_action :whitelist_query_limiting_ee_merge, only: [:merge]
before_action :whitelist_query_limiting_ee_show, only: [:show]
end
def approve def approve
unless merge_request.can_approve?(current_user) unless merge_request.can_approve?(current_user)
return render_404 return render_404
...@@ -59,6 +64,23 @@ module EE ...@@ -59,6 +64,23 @@ module EE
end end
end end
end end
private
def merge_access_check
super_result = super
return super_result if super_result
return render_404 unless @merge_request.approved?
end
def whitelist_query_limiting_ee_merge
::Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-ee/issues/4792')
end
def whitelist_query_limiting_ee_show
::Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-ee/issues/4793')
end
end end
end end
end end
...@@ -10,6 +10,7 @@ module EE ...@@ -10,6 +10,7 @@ module EE
super super
end end
override :hooks_validation_pass?
def hooks_validation_pass?(merge_request) def hooks_validation_pass?(merge_request)
# handle_merge_error needs this. We should move that to a separate # handle_merge_error needs this. We should move that to a separate
# object instead of relying on the order of method calls. # object instead of relying on the order of method calls.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment