summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRobert Speicher <rspeicher@gmail.com>2016-07-13 13:57:30 -0500
committerRobert Speicher <rspeicher@gmail.com>2016-07-13 13:57:30 -0500
commit530f5158e297f3cde27f3566cfe13bad74ba3b50 (patch)
tree270507d396bacdae8a4aa3489edfff0f47b718ca
parent9b0ef1559f78e58b398e428763ff97dfdc53e186 (diff)
downloadgitlab-ce-530f5158e297f3cde27f3566cfe13bad74ba3b50.tar.gz
Revert "Merge branch '18193-developers-can-merge' into 'master' "
This reverts commit 9ca633eb4c62231e4ddff5466c723cf8e2bdb25d, reversing changes made to fb229bbf7970ba908962b837b270adf56f14098f.
-rw-r--r--CHANGELOG1
-rw-r--r--app/assets/javascripts/protected_branches.js.coffee8
-rw-r--r--app/controllers/projects/protected_branches_controller.rb2
-rw-r--r--app/helpers/branches_helper.rb2
-rw-r--r--app/models/merge_request.rb8
-rw-r--r--app/models/project.rb4
-rw-r--r--app/models/repository.rb14
-rw-r--r--app/services/commits/change_service.rb4
-rw-r--r--app/services/files/base_service.rb2
-rw-r--r--app/services/git_push_service.rb3
-rw-r--r--app/services/merge_requests/merge_service.rb4
-rw-r--r--app/services/merge_requests/refresh_service.rb2
-rw-r--r--app/views/projects/merge_requests/widget/open/_conflicts.html.haml2
-rw-r--r--app/views/projects/protected_branches/_branches_list.html.haml2
-rw-r--r--app/views/projects/protected_branches/_protected_branch.html.haml2
-rw-r--r--app/views/projects/protected_branches/index.html.haml8
-rw-r--r--db/migrate/20160519203051_add_developers_can_merge_to_protected_branches.rb9
-rw-r--r--db/migrate/20160629025435_add_column_in_progress_merge_commit_sha_to_merge_requests.rb8
-rw-r--r--db/schema.rb8
-rw-r--r--lib/api/helpers.rb2
-rw-r--r--lib/gitlab/access.rb8
-rw-r--r--lib/gitlab/checks/change_access.rb96
-rw-r--r--lib/gitlab/checks/force_push.rb17
-rw-r--r--lib/gitlab/checks/matching_merge_request.rb18
-rw-r--r--lib/gitlab/force_push_check.rb15
-rw-r--r--lib/gitlab/git_access.rb143
-rw-r--r--lib/gitlab/git_access_wiki.rb2
-rw-r--r--lib/gitlab/user_access.rb48
-rw-r--r--spec/lib/gitlab/diff/position_tracer_spec.rb3
-rw-r--r--spec/lib/gitlab/git_access_spec.rb247
-rw-r--r--spec/lib/gitlab/user_access_spec.rb88
-rw-r--r--spec/models/repository_spec.rb9
-rw-r--r--spec/requests/api/api_helpers_spec.rb4
-rw-r--r--spec/services/git_push_service_spec.rb14
-rw-r--r--spec/services/merge_requests/refresh_service_spec.rb3
35 files changed, 303 insertions, 507 deletions
diff --git a/CHANGELOG b/CHANGELOG
index a42f8c7b916..71987a4a695 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -44,7 +44,6 @@ v 8.10.0 (unreleased)
- Add "Enabled Git access protocols" to Application Settings
- Diffs will create button/diff form on demand no on server side
- Reduce size of HTML used by diff comment forms
- - Protected branches have a "Developers can Merge" setting. !4892 (original implementation by Mathias Vestergaard)
- Fix user creation with stronger minimum password requirements !4054 (nathan-pmt)
- Only show New Snippet button to users that can create snippets.
- PipelinesFinder uses git cache data
diff --git a/app/assets/javascripts/protected_branches.js.coffee b/app/assets/javascripts/protected_branches.js.coffee
index 14afef2e2ee..79c2306e4d2 100644
--- a/app/assets/javascripts/protected_branches.js.coffee
+++ b/app/assets/javascripts/protected_branches.js.coffee
@@ -1,18 +1,18 @@
$ ->
$(".protected-branches-list :checkbox").change (e) ->
name = $(this).attr("name")
- if name == "developers_can_push" || name == "developers_can_merge"
+ if name == "developers_can_push"
id = $(this).val()
- can_push = $(this).is(":checked")
+ checked = $(this).is(":checked")
url = $(this).data("url")
$.ajax
- type: "PATCH"
+ type: "PUT"
url: url
dataType: "json"
data:
id: id
protected_branch:
- "#{name}": can_push
+ developers_can_push: checked
success: ->
row = $(e.target)
diff --git a/app/controllers/projects/protected_branches_controller.rb b/app/controllers/projects/protected_branches_controller.rb
index 10dca47fded..80dad758afa 100644
--- a/app/controllers/projects/protected_branches_controller.rb
+++ b/app/controllers/projects/protected_branches_controller.rb
@@ -50,6 +50,6 @@ class Projects::ProtectedBranchesController < Projects::ApplicationController
end
def protected_branch_params
- params.require(:protected_branch).permit(:name, :developers_can_push, :developers_can_merge)
+ params.require(:protected_branch).permit(:name, :developers_can_push)
end
end
diff --git a/app/helpers/branches_helper.rb b/app/helpers/branches_helper.rb
index bfd23aa4e04..601df5c18df 100644
--- a/app/helpers/branches_helper.rb
+++ b/app/helpers/branches_helper.rb
@@ -12,7 +12,7 @@ module BranchesHelper
def can_push_branch?(project, branch_name)
return false unless project.repository.branch_exists?(branch_name)
- ::Gitlab::UserAccess.new(current_user, project: project).can_push_to_branch?(branch_name)
+ ::Gitlab::GitAccess.new(current_user, project, 'web').can_push_to_branch?(branch_name)
end
def project_branches
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 471e32f3b60..157901378d3 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -552,13 +552,7 @@ class MergeRequest < ActiveRecord::Base
end
def can_be_merged_by?(user)
- access = ::Gitlab::UserAccess.new(user, project: project)
- access.can_push_to_branch?(target_branch) || access.can_merge_to_branch?(target_branch)
- end
-
- def can_be_merged_via_command_line_by?(user)
- access = ::Gitlab::UserAccess.new(user, project: project)
- access.can_push_to_branch?(target_branch)
+ ::Gitlab::GitAccess.new(user, project, 'web').can_push_to_branch?(target_branch)
end
def mergeable_ci_state?
diff --git a/app/models/project.rb b/app/models/project.rb
index 2bd97fc48f1..a66b750cd48 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -832,10 +832,6 @@ class Project < ActiveRecord::Base
protected_branches.matching(branch_name).any?(&:developers_can_push)
end
- def developers_can_merge_to_protected_branch?(branch_name)
- protected_branches.matching(branch_name).any?(&:developers_can_merge)
- end
-
def forked?
!(forked_project_link.nil? || forked_project_link.forked_from_project.nil?)
end
diff --git a/app/models/repository.rb b/app/models/repository.rb
index 09487b62f98..5b670cb4b8f 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -769,9 +769,9 @@ class Repository
end
end
- def merge(user, merge_request, options = {})
- our_commit = rugged.branches[merge_request.target_branch].target
- their_commit = rugged.lookup(merge_request.diff_head_sha)
+ def merge(user, source_sha, target_branch, options = {})
+ our_commit = rugged.branches[target_branch].target
+ their_commit = rugged.lookup(source_sha)
raise "Invalid merge target" if our_commit.nil?
raise "Invalid merge source" if their_commit.nil?
@@ -779,16 +779,14 @@ class Repository
merge_index = rugged.merge_commits(our_commit, their_commit)
return false if merge_index.conflicts?
- commit_with_hooks(user, merge_request.target_branch) do |tmp_ref|
+ commit_with_hooks(user, target_branch) do |ref|
actual_options = options.merge(
parents: [our_commit, their_commit],
tree: merge_index.write_tree(rugged),
- update_ref: tmp_ref
+ update_ref: ref
)
- commit_id = Rugged::Commit.create(rugged, actual_options)
- merge_request.update(in_progress_merge_commit_sha: commit_id)
- commit_id
+ Rugged::Commit.create(rugged, actual_options)
end
end
diff --git a/app/services/commits/change_service.rb b/app/services/commits/change_service.rb
index ed73d8cb8c2..c578097376a 100644
--- a/app/services/commits/change_service.rb
+++ b/app/services/commits/change_service.rb
@@ -23,7 +23,7 @@ module Commits
private
def check_push_permissions
- allowed = ::Gitlab::UserAccess.new(current_user, project: project).can_push_to_branch?(@target_branch)
+ allowed = ::Gitlab::GitAccess.new(current_user, project, 'web').can_push_to_branch?(@target_branch)
unless allowed
raise ValidationError.new('You are not allowed to push into this branch')
@@ -31,7 +31,7 @@ module Commits
true
end
-
+
def create_target_branch(new_branch)
# Temporary branch exists and contains the change commit
return success if repository.find_branch(new_branch)
diff --git a/app/services/files/base_service.rb b/app/services/files/base_service.rb
index 55da949f56a..37c5e321b39 100644
--- a/app/services/files/base_service.rb
+++ b/app/services/files/base_service.rb
@@ -42,7 +42,7 @@ module Files
end
def validate
- allowed = ::Gitlab::UserAccess.new(current_user, project: project).can_push_to_branch?(@target_branch)
+ allowed = ::Gitlab::GitAccess.new(current_user, project, 'web').can_push_to_branch?(@target_branch)
unless allowed
raise_error("You are not allowed to push into this branch")
diff --git a/app/services/git_push_service.rb b/app/services/git_push_service.rb
index e02b50ff9a2..a886f35981f 100644
--- a/app/services/git_push_service.rb
+++ b/app/services/git_push_service.rb
@@ -89,8 +89,7 @@ class GitPushService < BaseService
# Set protection on the default branch if configured
if current_application_settings.default_branch_protection != PROTECTION_NONE
developers_can_push = current_application_settings.default_branch_protection == PROTECTION_DEV_CAN_PUSH ? true : false
- developers_can_merge = current_application_settings.default_branch_protection == PROTECTION_DEV_CAN_MERGE ? true : false
- @project.protected_branches.create({ name: @project.default_branch, developers_can_push: developers_can_push, developers_can_merge: developers_can_merge })
+ @project.protected_branches.create({ name: @project.default_branch, developers_can_push: developers_can_push })
end
end
diff --git a/app/services/merge_requests/merge_service.rb b/app/services/merge_requests/merge_service.rb
index 0dac0614141..f1b1d90c457 100644
--- a/app/services/merge_requests/merge_service.rb
+++ b/app/services/merge_requests/merge_service.rb
@@ -34,7 +34,7 @@ module MergeRequests
committer: committer
}
- commit_id = repository.merge(current_user, merge_request, options)
+ commit_id = repository.merge(current_user, merge_request.diff_head_sha, merge_request.target_branch, options)
merge_request.update(merge_commit_sha: commit_id)
rescue GitHooksService::PreReceiveError => e
merge_request.update(merge_error: e.message)
@@ -43,8 +43,6 @@ module MergeRequests
merge_request.update(merge_error: "Something went wrong during merge")
Rails.logger.error(e.message)
false
- ensure
- merge_request.update(in_progress_merge_commit_sha: nil)
end
def after_merge
diff --git a/app/services/merge_requests/refresh_service.rb b/app/services/merge_requests/refresh_service.rb
index 1daf6bbf553..b11ecd97a57 100644
--- a/app/services/merge_requests/refresh_service.rb
+++ b/app/services/merge_requests/refresh_service.rb
@@ -48,7 +48,7 @@ module MergeRequests
end
def force_push?
- Gitlab::Checks::ForcePush.force_push?(@project, @oldrev, @newrev)
+ Gitlab::ForcePushCheck.force_push?(@project, @oldrev, @newrev)
end
# Refresh merge request diff if we push to source or target branch of merge request
diff --git a/app/views/projects/merge_requests/widget/open/_conflicts.html.haml b/app/views/projects/merge_requests/widget/open/_conflicts.html.haml
index f000cc38a65..06ab0a3fa00 100644
--- a/app/views/projects/merge_requests/widget/open/_conflicts.html.haml
+++ b/app/views/projects/merge_requests/widget/open/_conflicts.html.haml
@@ -4,7 +4,7 @@
%p
Please resolve these conflicts or
- - if @merge_request.can_be_merged_via_command_line_by?(current_user)
+ - if @merge_request.can_be_merged_by?(current_user)
#{link_to "merge this request manually", "#modal_merge_info", class: "how_to_merge_link vlink", "data-toggle" => "modal"}.
- else
ask someone with write access to this repository to merge this request manually.
diff --git a/app/views/projects/protected_branches/_branches_list.html.haml b/app/views/projects/protected_branches/_branches_list.html.haml
index 720d67dff7c..97cb1a9052b 100644
--- a/app/views/projects/protected_branches/_branches_list.html.haml
+++ b/app/views/projects/protected_branches/_branches_list.html.haml
@@ -8,7 +8,6 @@
.table-responsive
%table.table.protected-branches-list
%colgroup
- %col{ width: "20%" }
%col{ width: "30%" }
%col{ width: "25%" }
%col{ width: "25%" }
@@ -19,7 +18,6 @@
%th Protected Branch
%th Commit
%th Developers Can Push
- %th Developers Can Merge
- if can_admin_project
%th
%tbody
diff --git a/app/views/projects/protected_branches/_protected_branch.html.haml b/app/views/projects/protected_branches/_protected_branch.html.haml
index 7fda7f96047..474aec3a97c 100644
--- a/app/views/projects/protected_branches/_protected_branch.html.haml
+++ b/app/views/projects/protected_branches/_protected_branch.html.haml
@@ -16,8 +16,6 @@
(branch was removed from repository)
%td
= check_box_tag("developers_can_push", protected_branch.id, protected_branch.developers_can_push, data: { url: url })
- %td
- = check_box_tag("developers_can_merge", protected_branch.id, protected_branch.developers_can_merge, data: { url: url })
- if can_admin_project
%td
= link_to 'Unprotect', [@project.namespace.becomes(Namespace), @project, protected_branch], data: { confirm: 'Branch will be writable for developers. Are you sure?' }, method: :delete, class: "btn btn-warning btn-sm pull-right"
diff --git a/app/views/projects/protected_branches/index.html.haml b/app/views/projects/protected_branches/index.html.haml
index 101b3f3b452..3fab95751e0 100644
--- a/app/views/projects/protected_branches/index.html.haml
+++ b/app/views/projects/protected_branches/index.html.haml
@@ -36,14 +36,6 @@
= f.label :developers_can_push, "Developers can push", class: "label-light append-bottom-0"
%p.light.append-bottom-0
Allow developers to push to this branch
-
- .form-group
- = f.check_box :developers_can_merge, class: "pull-left"
- .prepend-left-20
- = f.label :developers_can_merge, "Developers can merge", class: "label-light append-bottom-0"
- %p.light.append-bottom-0
- Allow developers to accept merge requests to this branch
= f.submit "Protect", class: "btn-create btn protect-branch-btn", disabled: true
-
%hr
= render "branches_list"
diff --git a/db/migrate/20160519203051_add_developers_can_merge_to_protected_branches.rb b/db/migrate/20160519203051_add_developers_can_merge_to_protected_branches.rb
deleted file mode 100644
index 15ad8e8bcbb..00000000000
--- a/db/migrate/20160519203051_add_developers_can_merge_to_protected_branches.rb
+++ /dev/null
@@ -1,9 +0,0 @@
-class AddDevelopersCanMergeToProtectedBranches < ActiveRecord::Migration
- include Gitlab::Database::MigrationHelpers
-
- disable_ddl_transaction!
-
- def change
- add_column_with_default :protected_branches, :developers_can_merge, :boolean, default: false, allow_null: false
- end
-end
diff --git a/db/migrate/20160629025435_add_column_in_progress_merge_commit_sha_to_merge_requests.rb b/db/migrate/20160629025435_add_column_in_progress_merge_commit_sha_to_merge_requests.rb
deleted file mode 100644
index 7c5f76572ef..00000000000
--- a/db/migrate/20160629025435_add_column_in_progress_merge_commit_sha_to_merge_requests.rb
+++ /dev/null
@@ -1,8 +0,0 @@
-# See http://doc.gitlab.com/ce/development/migration_style_guide.html
-# for more information on how to write migrations for GitLab.
-
-class AddColumnInProgressMergeCommitShaToMergeRequests < ActiveRecord::Migration
- def change
- add_column :merge_requests, :in_progress_merge_commit_sha, :string
- end
-end
diff --git a/db/schema.rb b/db/schema.rb
index 64019cf79bb..f24e47b85b2 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -626,7 +626,6 @@ ActiveRecord::Schema.define(version: 20160712171823) do
t.string "merge_commit_sha"
t.datetime "deleted_at"
t.integer "lock_version", default: 0, null: false
- t.string "in_progress_merge_commit_sha"
end
add_index "merge_requests", ["assignee_id"], name: "index_merge_requests_on_assignee_id", using: :btree
@@ -861,12 +860,11 @@ ActiveRecord::Schema.define(version: 20160712171823) do
add_index "projects", ["visibility_level"], name: "index_projects_on_visibility_level", using: :btree
create_table "protected_branches", force: :cascade do |t|
- t.integer "project_id", null: false
- t.string "name", null: false
+ t.integer "project_id", null: false
+ t.string "name", null: false
t.datetime "created_at"
t.datetime "updated_at"
- t.boolean "developers_can_push", default: false, null: false
- t.boolean "developers_can_merge", default: false, null: false
+ t.boolean "developers_can_push", default: false, null: false
end
add_index "protected_branches", ["project_id"], name: "index_protected_branches_on_project_id", using: :btree
diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb
index 73557cf7db6..77e407b54c5 100644
--- a/lib/api/helpers.rb
+++ b/lib/api/helpers.rb
@@ -17,7 +17,7 @@ module API
def current_user
@current_user ||= (find_user_by_private_token || doorkeeper_guard)
- unless @current_user && Gitlab::UserAccess.new(@current_user).allowed?
+ unless @current_user && Gitlab::UserAccess.allowed?(@current_user)
return nil
end
diff --git a/lib/gitlab/access.rb b/lib/gitlab/access.rb
index de41ea415a6..831f1e635ba 100644
--- a/lib/gitlab/access.rb
+++ b/lib/gitlab/access.rb
@@ -14,10 +14,9 @@ module Gitlab
OWNER = 50
# Branch protection settings
- PROTECTION_NONE = 0
- PROTECTION_DEV_CAN_PUSH = 1
- PROTECTION_FULL = 2
- PROTECTION_DEV_CAN_MERGE = 3
+ PROTECTION_NONE = 0
+ PROTECTION_DEV_CAN_PUSH = 1
+ PROTECTION_FULL = 2
class << self
def values
@@ -55,7 +54,6 @@ module Gitlab
def protection_options
{
"Not protected: Both developers and masters can push new commits, force push, or delete the branch." => PROTECTION_NONE,
- "Protected against pushes: Developers cannot push new commits, but are allowed to accept merge requests to the branch." => PROTECTION_DEV_CAN_MERGE,
"Partially protected: Developers can push new commits, but cannot force push or delete the branch. Masters can do all of those." => PROTECTION_DEV_CAN_PUSH,
"Fully protected: Developers cannot push new commits, force push, or delete the branch. Only masters can do any of those." => PROTECTION_FULL,
}
diff --git a/lib/gitlab/checks/change_access.rb b/lib/gitlab/checks/change_access.rb
deleted file mode 100644
index 5551fac4b8b..00000000000
--- a/lib/gitlab/checks/change_access.rb
+++ /dev/null
@@ -1,96 +0,0 @@
-module Gitlab
- module Checks
- class ChangeAccess
- attr_reader :user_access, :project
-
- def initialize(change, user_access:, project:)
- @oldrev, @newrev, @ref = change.split(' ')
- @branch_name = branch_name(@ref)
- @user_access = user_access
- @project = project
- end
-
- def exec
- error = protected_branch_checks || tag_checks || push_checks
-
- if error
- GitAccessStatus.new(false, error)
- else
- GitAccessStatus.new(true)
- end
- end
-
- protected
-
- def protected_branch_checks
- return unless project.protected_branch?(@branch_name)
-
- if forced_push? && user_access.cannot_do_action?(:force_push_code_to_protected_branches)
- return "You are not allowed to force push code to a protected branch on this project."
- elsif Gitlab::Git.blank_ref?(@newrev) && user_access.cannot_do_action?(:remove_protected_branches)
- return "You are not allowed to delete protected branches from this project."
- end
-
- if matching_merge_request?
- if user_access.can_merge_to_branch?(@branch_name) || user_access.can_push_to_branch?(@branch_name)
- return
- else
- "You are not allowed to merge code into protected branches on this project."
- end
- else
- if user_access.can_push_to_branch?(@branch_name)
- return
- else
- "You are not allowed to push code to protected branches on this project."
- end
- end
- end
-
- def tag_checks
- tag_ref = tag_name(@ref)
-
- if tag_ref && protected_tag?(tag_ref) && user_access.cannot_do_action?(:admin_project)
- "You are not allowed to change existing tags on this project."
- end
- end
-
- def push_checks
- if user_access.cannot_do_action?(:push_code)
- "You are not allowed to push code to this project."
- end
- end
-
- private
-
- def protected_tag?(tag_name)
- project.repository.tag_exists?(tag_name)
- end
-
- def forced_push?
- Gitlab::Checks::ForcePush.force_push?(@project, @oldrev, @newrev)
- end
-
- def matching_merge_request?
- Checks::MatchingMergeRequest.new(@newrev, @branch_name, @project).match?
- end
-
- def branch_name(ref)
- ref = @ref.to_s
- if Gitlab::Git.branch_ref?(ref)
- Gitlab::Git.ref_name(ref)
- else
- nil
- end
- end
-
- def tag_name(ref)
- ref = @ref.to_s
- if Gitlab::Git.tag_ref?(ref)
- Gitlab::Git.ref_name(ref)
- else
- nil
- end
- end
- end
- end
-end
diff --git a/lib/gitlab/checks/force_push.rb b/lib/gitlab/checks/force_push.rb
deleted file mode 100644
index dfa83a0eab3..00000000000
--- a/lib/gitlab/checks/force_push.rb
+++ /dev/null
@@ -1,17 +0,0 @@
-module Gitlab
- module Checks
- class ForcePush
- def self.force_push?(project, oldrev, newrev)
- return false if project.empty_repo?
-
- # Created or deleted branch
- if Gitlab::Git.blank_ref?(oldrev) || Gitlab::Git.blank_ref?(newrev)
- false
- else
- missed_refs, _ = Gitlab::Popen.popen(%W(#{Gitlab.config.git.bin_path} --git-dir=#{project.repository.path_to_repo} rev-list #{oldrev} ^#{newrev}))
- missed_refs.split("\n").size > 0
- end
- end
- end
- end
-end
diff --git a/lib/gitlab/checks/matching_merge_request.rb b/lib/gitlab/checks/matching_merge_request.rb
deleted file mode 100644
index 849848515da..00000000000
--- a/lib/gitlab/checks/matching_merge_request.rb
+++ /dev/null
@@ -1,18 +0,0 @@
-module Gitlab
- module Checks
- class MatchingMergeRequest
- def initialize(newrev, branch_name, project)
- @newrev = newrev
- @branch_name = branch_name
- @project = project
- end
-
- def match?
- @project.merge_requests
- .with_state(:locked)
- .where(in_progress_merge_commit_sha: @newrev, target_branch: @branch_name)
- .exists?
- end
- end
- end
-end
diff --git a/lib/gitlab/force_push_check.rb b/lib/gitlab/force_push_check.rb
new file mode 100644
index 00000000000..93c6a5bb7f5
--- /dev/null
+++ b/lib/gitlab/force_push_check.rb
@@ -0,0 +1,15 @@
+module Gitlab
+ class ForcePushCheck
+ def self.force_push?(project, oldrev, newrev)
+ return false if project.empty_repo?
+
+ # Created or deleted branch
+ if Gitlab::Git.blank_ref?(oldrev) || Gitlab::Git.blank_ref?(newrev)
+ false
+ else
+ missed_refs, _ = Gitlab::Popen.popen(%W(#{Gitlab.config.git.bin_path} --git-dir=#{project.repository.path_to_repo} rev-list #{oldrev} ^#{newrev}))
+ missed_refs.split("\n").size > 0
+ end
+ end
+ end
+end
diff --git a/lib/gitlab/git_access.rb b/lib/gitlab/git_access.rb
index 308f23bc9bc..7679c7e4bb8 100644
--- a/lib/gitlab/git_access.rb
+++ b/lib/gitlab/git_access.rb
@@ -1,17 +1,52 @@
-# Check a user's access to perform a git action. All public methods in this
-# class return an instance of `GitlabAccessStatus`
module Gitlab
class GitAccess
DOWNLOAD_COMMANDS = %w{ git-upload-pack git-upload-archive }
PUSH_COMMANDS = %w{ git-receive-pack }
- attr_reader :actor, :project, :protocol, :user_access
+ attr_reader :actor, :project, :protocol
def initialize(actor, project, protocol)
@actor = actor
@project = project
@protocol = protocol
- @user_access = UserAccess.new(user, project: project)
+ end
+
+ def user
+ return @user if defined?(@user)
+
+ @user =
+ case actor
+ when User
+ actor
+ when DeployKey
+ nil
+ when Key
+ actor.user
+ end
+ end
+
+ def deploy_key
+ actor if actor.is_a?(DeployKey)
+ end
+
+ def can_push_to_branch?(ref)
+ return false unless user
+
+ if project.protected_branch?(ref) && !project.developers_can_push_to_protected_branch?(ref)
+ user.can?(:push_code_to_protected_branches, project)
+ else
+ user.can?(:push_code, project)
+ end
+ end
+
+ def can_read_project?
+ if user
+ user.can?(:read_project, project)
+ elsif deploy_key
+ deploy_key.projects.include?(project)
+ else
+ false
+ end
end
def check(cmd, changes = nil)
@@ -21,11 +56,11 @@ module Gitlab
return build_status_object(false, "No user or key was provided.")
end
- if user && !user_access.allowed?
+ if user && !user_allowed?
return build_status_object(false, "Your account has been blocked.")
end
- unless project && (user_access.can_read_project? || deploy_key_can_read_project?)
+ unless project && can_read_project?
return build_status_object(false, 'The project you were looking for could not be found.')
end
@@ -60,7 +95,7 @@ module Gitlab
end
def user_download_access_check
- unless user_access.can_do_action?(:download_code)
+ unless user.can?(:download_code, project)
return build_status_object(false, "You are not allowed to download code from this project.")
end
@@ -90,8 +125,46 @@ module Gitlab
build_status_object(true)
end
+ def can_user_do_action?(action)
+ @permission_cache ||= {}
+ @permission_cache[action] ||= user.can?(action, project)
+ end
+
def change_access_check(change)
- Checks::ChangeAccess.new(change, user_access: user_access, project: project).exec
+ oldrev, newrev, ref = change.split(' ')
+
+ action =
+ if project.protected_branch?(branch_name(ref))
+ protected_branch_action(oldrev, newrev, branch_name(ref))
+ elsif (tag_ref = tag_name(ref)) && protected_tag?(tag_ref)
+ # Prevent any changes to existing git tag unless user has permissions
+ :admin_project
+ else
+ :push_code
+ end
+
+ unless can_user_do_action?(action)
+ status =
+ case action
+ when :force_push_code_to_protected_branches
+ build_status_object(false, "You are not allowed to force push code to a protected branch on this project.")
+ when :remove_protected_branches
+ build_status_object(false, "You are not allowed to deleted protected branches from this project.")
+ when :push_code_to_protected_branches
+ build_status_object(false, "You are not allowed to push code to protected branches on this project.")
+ when :admin_project
+ build_status_object(false, "You are not allowed to change existing tags on this project.")
+ else # :push_code
+ build_status_object(false, "You are not allowed to push code to this project.")
+ end
+ return status
+ end
+
+ build_status_object(true)
+ end
+
+ def forced_push?(oldrev, newrev)
+ Gitlab::ForcePushCheck.force_push?(project, oldrev, newrev)
end
def protocol_allowed?
@@ -100,38 +173,48 @@ module Gitlab
private
- def matching_merge_request?(newrev, branch_name)
- Checks::MatchingMergeRequest.new(newrev, branch_name, project).match?
+ def protected_branch_action(oldrev, newrev, branch_name)
+ # we dont allow force push to protected branch
+ if forced_push?(oldrev, newrev)
+ :force_push_code_to_protected_branches
+ elsif Gitlab::Git.blank_ref?(newrev)
+ # and we dont allow remove of protected branch
+ :remove_protected_branches
+ elsif project.developers_can_push_to_protected_branch?(branch_name)
+ :push_code
+ else
+ :push_code_to_protected_branches
+ end
end
- def deploy_key
- actor if actor.is_a?(DeployKey)
+ def protected_tag?(tag_name)
+ project.repository.tag_exists?(tag_name)
end
- def deploy_key_can_read_project?
- if deploy_key
- deploy_key.projects.include?(project)
+ def user_allowed?
+ Gitlab::UserAccess.allowed?(user)
+ end
+
+ def branch_name(ref)
+ ref = ref.to_s
+ if Gitlab::Git.branch_ref?(ref)
+ Gitlab::Git.ref_name(ref)
else
- false
+ nil
end
end
- protected
-
- def user
- return @user if defined?(@user)
-
- @user =
- case actor
- when User
- actor
- when DeployKey
- nil
- when Key
- actor.user
- end
+ def tag_name(ref)
+ ref = ref.to_s
+ if Gitlab::Git.tag_ref?(ref)
+ Gitlab::Git.ref_name(ref)
+ else
+ nil
+ end
end
+ protected
+
def build_status_object(status, message = '')
GitAccessStatus.new(status, message)
end
diff --git a/lib/gitlab/git_access_wiki.rb b/lib/gitlab/git_access_wiki.rb
index f71d3575909..8672cbc0ec4 100644
--- a/lib/gitlab/git_access_wiki.rb
+++ b/lib/gitlab/git_access_wiki.rb
@@ -1,7 +1,7 @@
module Gitlab
class GitAccessWiki < GitAccess
def change_access_check(change)
- if user_access.can_do_action?(:create_wiki)
+ if user.can?(:create_wiki, project)
build_status_object(true)
else
build_status_object(false, "You are not allowed to write to this project's wiki.")
diff --git a/lib/gitlab/user_access.rb b/lib/gitlab/user_access.rb
index c0f85e9b3a8..d1b42c1f9b9 100644
--- a/lib/gitlab/user_access.rb
+++ b/lib/gitlab/user_access.rb
@@ -1,23 +1,7 @@
module Gitlab
- class UserAccess
- attr_reader :user, :project
-
- def initialize(user, project: nil)
- @user = user
- @project = project
- end
-
- def can_do_action?(action)
- @permission_cache ||= {}
- @permission_cache[action] ||= user.can?(action, project)
- end
-
- def cannot_do_action?(action)
- !can_do_action?(action)
- end
-
- def allowed?
- return false if user.blank? || user.blocked?
+ module UserAccess
+ def self.allowed?(user)
+ return false if user.blocked?
if user.requires_ldap_check? && user.try_obtain_ldap_lease
return false unless Gitlab::LDAP::Access.allowed?(user)
@@ -25,31 +9,5 @@ module Gitlab
true
end
-
- def can_push_to_branch?(ref)
- return false unless user
-
- if project.protected_branch?(ref) && !project.developers_can_push_to_protected_branch?(ref)
- user.can?(:push_code_to_protected_branches, project)
- else
- user.can?(:push_code, project)
- end
- end
-
- def can_merge_to_branch?(ref)
- return false unless user
-
- if project.protected_branch?(ref) && !project.developers_can_merge_to_protected_branch?(ref)
- user.can?(:push_code_to_protected_branches, project)
- else
- user.can?(:push_code, project)
- end
- end
-
- def can_read_project?
- return false unless user
-
- user.can?(:read_project, project)
- end
end
end
diff --git a/spec/lib/gitlab/diff/position_tracer_spec.rb b/spec/lib/gitlab/diff/position_tracer_spec.rb
index c268f84c759..08312e60f4a 100644
--- a/spec/lib/gitlab/diff/position_tracer_spec.rb
+++ b/spec/lib/gitlab/diff/position_tracer_spec.rb
@@ -1639,8 +1639,7 @@ describe Gitlab::Diff::PositionTracer, lib: true do
committer: committer
}
- merge_request = create(:merge_request, source_branch: second_create_file_commit.sha, target_branch: branch_name, source_project: project)
- repository.merge(current_user, merge_request, options)
+ repository.merge(current_user, second_create_file_commit.sha, branch_name, options)
project.commit(branch_name)
end
diff --git a/spec/lib/gitlab/git_access_spec.rb b/spec/lib/gitlab/git_access_spec.rb
index db33c7a22bb..c79ba11f782 100644
--- a/spec/lib/gitlab/git_access_spec.rb
+++ b/spec/lib/gitlab/git_access_spec.rb
@@ -6,6 +6,67 @@ describe Gitlab::GitAccess, lib: true do
let(:user) { create(:user) }
let(:actor) { user }
+ describe 'can_push_to_branch?' do
+ describe 'push to none protected branch' do
+ it "returns true if user is a master" do
+ project.team << [user, :master]
+ expect(access.can_push_to_branch?("random_branch")).to be_truthy
+ end
+
+ it "returns true if user is a developer" do
+ project.team << [user, :developer]
+ expect(access.can_push_to_branch?("random_branch")).to be_truthy
+ end
+
+ it "returns false if user is a reporter" do
+ project.team << [user, :reporter]
+ expect(access.can_push_to_branch?("random_branch")).to be_falsey
+ end
+ end
+
+ describe 'push to protected branch' do
+ before do
+ @branch = create :protected_branch, project: project
+ end
+
+ it "returns true if user is a master" do
+ project.team << [user, :master]
+ expect(access.can_push_to_branch?(@branch.name)).to be_truthy
+ end
+
+ it "returns false if user is a developer" do
+ project.team << [user, :developer]
+ expect(access.can_push_to_branch?(@branch.name)).to be_falsey
+ end
+
+ it "returns false if user is a reporter" do
+ project.team << [user, :reporter]
+ expect(access.can_push_to_branch?(@branch.name)).to be_falsey
+ end
+ end
+
+ describe 'push to protected branch if allowed for developers' do
+ before do
+ @branch = create :protected_branch, project: project, developers_can_push: true
+ end
+
+ it "returns true if user is a master" do
+ project.team << [user, :master]
+ expect(access.can_push_to_branch?(@branch.name)).to be_truthy
+ end
+
+ it "returns true if user is a developer" do
+ project.team << [user, :developer]
+ expect(access.can_push_to_branch?(@branch.name)).to be_truthy
+ end
+
+ it "returns false if user is a reporter" do
+ project.team << [user, :reporter]
+ expect(access.can_push_to_branch?(@branch.name)).to be_falsey
+ end
+ end
+ end
+
describe '#check with single protocols allowed' do
def disable_protocol(protocol)
settings = ::ApplicationSetting.create_from_defaults
@@ -99,145 +160,103 @@ describe Gitlab::GitAccess, lib: true do
end
describe 'push_access_check' do
- before { merge_into_protected_branch }
- let(:unprotected_branch) { FFaker::Internet.user_name }
+ def protect_feature_branch
+ create(:protected_branch, name: 'feature', project: project)
+ end
- let(:changes) do
- { push_new_branch: "#{Gitlab::Git::BLANK_SHA} 570e7b2ab refs/heads/wow",
+ def changes
+ {
+ push_new_branch: "#{Gitlab::Git::BLANK_SHA} 570e7b2ab refs/heads/wow",
push_master: '6f6d7e7ed 570e7b2ab refs/heads/master',
push_protected_branch: '6f6d7e7ed 570e7b2ab refs/heads/feature',
push_remove_protected_branch: "570e7b2ab #{Gitlab::Git::BLANK_SHA} "\
'refs/heads/feature',
push_tag: '6f6d7e7ed 570e7b2ab refs/tags/v1.0.0',
push_new_tag: "#{Gitlab::Git::BLANK_SHA} 570e7b2ab refs/tags/v7.8.9",
- push_all: ['6f6d7e7ed 570e7b2ab refs/heads/master', '6f6d7e7ed 570e7b2ab refs/heads/feature'],
- merge_into_protected_branch: "0b4bc9a #{merge_into_protected_branch} refs/heads/feature" }
+ push_all: ['6f6d7e7ed 570e7b2ab refs/heads/master', '6f6d7e7ed 570e7b2ab refs/heads/feature']
+ }
end
- def stub_git_hooks
- # Running the `pre-receive` hook is expensive, and not necessary for this test.
- allow_any_instance_of(GitHooksService).to receive(:execute).and_yield
+ def self.permissions_matrix
+ {
+ master: {
+ push_new_branch: true,
+ push_master: true,
+ push_protected_branch: true,
+ push_remove_protected_branch: false,
+ push_tag: true,
+ push_new_tag: true,
+ push_all: true,
+ },
+
+ developer: {
+ push_new_branch: true,
+ push_master: true,
+ push_protected_branch: false,
+ push_remove_protected_branch: false,
+ push_tag: false,
+ push_new_tag: true,
+ push_all: false,
+ },
+
+ reporter: {
+ push_new_branch: false,
+ push_master: false,
+ push_protected_branch: false,
+ push_remove_protected_branch: false,
+ push_tag: false,
+ push_new_tag: false,
+ push_all: false,
+ },
+
+ guest: {
+ push_new_branch: false,
+ push_master: false,
+ push_protected_branch: false,
+ push_remove_protected_branch: false,
+ push_tag: false,
+ push_new_tag: false,
+ push_all: false,
+ }
+ }
end
- def merge_into_protected_branch
- @protected_branch_merge_commit ||= begin
- stub_git_hooks
- project.repository.add_branch(user, unprotected_branch, 'feature')
- target_branch = project.repository.lookup('feature')
- source_branch = project.repository.commit_file(user, FFaker::InternetSE.login_user_name, FFaker::HipsterIpsum.paragraph, FFaker::HipsterIpsum.sentence, unprotected_branch, false)
- rugged = project.repository.rugged
- author = { email: "email@example.com", time: Time.now, name: "Example Git User" }
-
- merge_index = rugged.merge_commits(target_branch, source_branch)
- Rugged::Commit.create(rugged, author: author, committer: author, message: "commit message", parents: [target_branch, source_branch], tree: merge_index.write_tree(rugged))
- end
+ def self.updated_permissions_matrix
+ updated_permissions_matrix = permissions_matrix.dup
+ updated_permissions_matrix[:developer][:push_protected_branch] = true
+ updated_permissions_matrix[:developer][:push_all] = true
+ updated_permissions_matrix
end
- def self.run_permission_checks(permissions_matrix)
- permissions_matrix.keys.each do |role|
- describe "#{role} access" do
- before { project.team << [user, role] }
+ permissions_matrix.keys.each do |role|
+ describe "#{role} access" do
+ before { protect_feature_branch }
+ before { project.team << [user, role] }
- permissions_matrix[role].each do |action, allowed|
- context action do
- subject { access.push_access_check(changes[action]) }
+ permissions_matrix[role].each do |action, allowed|
+ context action do
+ subject { access.push_access_check(changes[action]) }
- it { expect(subject.allowed?).to allowed ? be_truthy : be_falsey }
- end
+ it { expect(subject.allowed?).to allowed ? be_truthy : be_falsey }
end
end
end
end
- permissions_matrix = {
- master: {
- push_new_branch: true,
- push_master: true,
- push_protected_branch: true,
- push_remove_protected_branch: false,
- push_tag: true,
- push_new_tag: true,
- push_all: true,
- merge_into_protected_branch: true
- },
-
- developer: {
- push_new_branch: true,
- push_master: true,
- push_protected_branch: false,
- push_remove_protected_branch: false,
- push_tag: false,
- push_new_tag: true,
- push_all: false,
- merge_into_protected_branch: false
- },
-
- reporter: {
- push_new_branch: false,
- push_master: false,
- push_protected_branch: false,
- push_remove_protected_branch: false,
- push_tag: false,
- push_new_tag: false,
- push_all: false,
- merge_into_protected_branch: false
- },
-
- guest: {
- push_new_branch: false,
- push_master: false,
- push_protected_branch: false,
- push_remove_protected_branch: false,
- push_tag: false,
- push_new_tag: false,
- push_all: false,
- merge_into_protected_branch: false
- }
- }
-
- [['feature', 'exact'], ['feat*', 'wildcard']].each do |protected_branch_name, protected_branch_type|
- context do
- before { create(:protected_branch, name: protected_branch_name, project: project) }
-
- run_permission_checks(permissions_matrix)
- end
-
- context "when 'developers can push' is turned on for the #{protected_branch_type} protected branch" do
- before { create(:protected_branch, name: protected_branch_name, developers_can_push: true, project: project) }
-
- run_permission_checks(permissions_matrix.deep_merge(developer: { push_protected_branch: true, push_all: true, merge_into_protected_branch: true }))
- end
-
- context "when 'developers can merge' is turned on for the #{protected_branch_type} protected branch" do
- before { create(:protected_branch, name: protected_branch_name, developers_can_merge: true, project: project) }
-
- context "when a merge request exists for the given source/target branch" do
- context "when the merge request is in progress" do
- before do
- create(:merge_request, source_project: project, source_branch: unprotected_branch, target_branch: 'feature', state: 'locked', in_progress_merge_commit_sha: merge_into_protected_branch)
- end
+ context "with enabled developers push to protected branches " do
+ updated_permissions_matrix.keys.each do |role|
+ describe "#{role} access" do
+ before { create(:protected_branch, name: 'feature', developers_can_push: true, project: project) }
+ before { project.team << [user, role] }
- run_permission_checks(permissions_matrix.deep_merge(developer: { merge_into_protected_branch: true }))
- end
+ updated_permissions_matrix[role].each do |action, allowed|
+ context action do
+ subject { access.push_access_check(changes[action]) }
- context "when the merge request is not in progress" do
- before do
- create(:merge_request, source_project: project, source_branch: unprotected_branch, target_branch: 'feature', in_progress_merge_commit_sha: nil)
+ it { expect(subject.allowed?).to allowed ? be_truthy : be_falsey }
end
-
- run_permission_checks(permissions_matrix.deep_merge(developer: { merge_into_protected_branch: false }))
end
end
-
- context "when a merge request does not exist for the given source/target branch" do
- run_permission_checks(permissions_matrix.deep_merge(developer: { merge_into_protected_branch: false }))
- end
- end
-
- context "when 'developers can merge' and 'developers can push' are turned on for the #{protected_branch_type} protected branch" do
- before { create(:protected_branch, name: protected_branch_name, developers_can_merge: true, developers_can_push: true, project: project) }
-
- run_permission_checks(permissions_matrix.deep_merge(developer: { push_protected_branch: true, push_all: true, merge_into_protected_branch: true }))
end
end
end
diff --git a/spec/lib/gitlab/user_access_spec.rb b/spec/lib/gitlab/user_access_spec.rb
deleted file mode 100644
index aa9ec243498..00000000000
--- a/spec/lib/gitlab/user_access_spec.rb
+++ /dev/null
@@ -1,88 +0,0 @@
-require 'spec_helper'
-
-describe Gitlab::UserAccess, lib: true do
- let(:access) { Gitlab::UserAccess.new(user, project: project) }
- let(:project) { create(:project) }
- let(:user) { create(:user) }
-
- describe 'can_push_to_branch?' do
- describe 'push to none protected branch' do
- it 'returns true if user is a master' do
- project.team << [user, :master]
- expect(access.can_push_to_branch?('random_branch')).to be_truthy
- end
-
- it 'returns true if user is a developer' do
- project.team << [user, :developer]
- expect(access.can_push_to_branch?('random_branch')).to be_truthy
- end
-
- it 'returns false if user is a reporter' do
- project.team << [user, :reporter]
- expect(access.can_push_to_branch?('random_branch')).to be_falsey
- end
- end
-
- describe 'push to protected branch' do
- let(:branch) { create :protected_branch, project: project }
-
- it 'returns true if user is a master' do
- project.team << [user, :master]
- expect(access.can_push_to_branch?(branch.name)).to be_truthy
- end
-
- it 'returns false if user is a developer' do
- project.team << [user, :developer]
- expect(access.can_push_to_branch?(branch.name)).to be_falsey
- end
-
- it 'returns false if user is a reporter' do
- project.team << [user, :reporter]
- expect(access.can_push_to_branch?(branch.name)).to be_falsey
- end
- end
-
- describe 'push to protected branch if allowed for developers' do
- before do
- @branch = create :protected_branch, project: project, developers_can_push: true
- end
-
- it 'returns true if user is a master' do
- project.team << [user, :master]
- expect(access.can_push_to_branch?(@branch.name)).to be_truthy
- end
-
- it 'returns true if user is a developer' do
- project.team << [user, :developer]
- expect(access.can_push_to_branch?(@branch.name)).to be_truthy
- end
-
- it 'returns false if user is a reporter' do
- project.team << [user, :reporter]
- expect(access.can_push_to_branch?(@branch.name)).to be_falsey
- end
- end
-
- describe 'merge to protected branch if allowed for developers' do
- before do
- @branch = create :protected_branch, project: project, developers_can_merge: true
- end
-
- it 'returns true if user is a master' do
- project.team << [user, :master]
- expect(access.can_merge_to_branch?(@branch.name)).to be_truthy
- end
-
- it 'returns true if user is a developer' do
- project.team << [user, :developer]
- expect(access.can_merge_to_branch?(@branch.name)).to be_truthy
- end
-
- it 'returns false if user is a reporter' do
- project.team << [user, :reporter]
- expect(access.can_merge_to_branch?(@branch.name)).to be_falsey
- end
- end
-
- end
-end
diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb
index e14cec589fe..b39b958450c 100644
--- a/spec/models/repository_spec.rb
+++ b/spec/models/repository_spec.rb
@@ -4,17 +4,16 @@ describe Repository, models: true do
include RepoHelpers
TestBlob = Struct.new(:name)
- let(:project) { create(:project) }
- let(:repository) { project.repository }
+ let(:repository) { create(:project).repository }
let(:user) { create(:user) }
let(:commit_options) do
author = repository.user_to_committer(user)
{ message: 'Test message', committer: author, author: author }
end
let(:merge_commit) do
- merge_request = create(:merge_request, source_branch: 'feature', target_branch: 'master', source_project: project)
- merge_commit_id = repository.merge(user, merge_request, commit_options)
- repository.commit(merge_commit_id)
+ source_sha = repository.find_branch('feature').target
+ merge_commit_sha = repository.merge(user, source_sha, 'master', commit_options)
+ repository.commit(merge_commit_sha)
end
describe '#branch_names_contains' do
diff --git a/spec/requests/api/api_helpers_spec.rb b/spec/requests/api/api_helpers_spec.rb
index 3d5c19aeff3..83025953889 100644
--- a/spec/requests/api/api_helpers_spec.rb
+++ b/spec/requests/api/api_helpers_spec.rb
@@ -49,7 +49,7 @@ describe API::Helpers, api: true do
it "should return nil for a user without access" do
env[API::Helpers::PRIVATE_TOKEN_HEADER] = user.private_token
- allow_any_instance_of(Gitlab::UserAccess).to receive(:allowed?).and_return(false)
+ allow(Gitlab::UserAccess).to receive(:allowed?).and_return(false)
expect(current_user).to be_nil
end
@@ -73,7 +73,7 @@ describe API::Helpers, api: true do
it "should return nil for a user without access" do
env[API::Helpers::PRIVATE_TOKEN_HEADER] = personal_access_token.token
- allow_any_instance_of(Gitlab::UserAccess).to receive(:allowed?).and_return(false)
+ allow(Gitlab::UserAccess).to receive(:allowed?).and_return(false)
expect(current_user).to be_nil
end
diff --git a/spec/services/git_push_service_spec.rb b/spec/services/git_push_service_spec.rb
index 47c0580e0f0..afabeed4a80 100644
--- a/spec/services/git_push_service_spec.rb
+++ b/spec/services/git_push_service_spec.rb
@@ -224,7 +224,7 @@ describe GitPushService, services: true do
it "when pushing a branch for the first time" do
expect(project).to receive(:execute_hooks)
expect(project.default_branch).to eq("master")
- expect(project.protected_branches).to receive(:create).with({ name: "master", developers_can_push: false, developers_can_merge: false })
+ expect(project.protected_branches).to receive(:create).with({ name: "master", developers_can_push: false })
execute_service(project, user, @blankrev, 'newrev', 'refs/heads/master' )
end
@@ -242,17 +242,7 @@ describe GitPushService, services: true do
expect(project).to receive(:execute_hooks)
expect(project.default_branch).to eq("master")
- expect(project.protected_branches).to receive(:create).with({ name: "master", developers_can_push: true, developers_can_merge: false })
-
- execute_service(project, user, @blankrev, 'newrev', 'refs/heads/master')
- end
-
- it "when pushing a branch for the first time with default branch protection set to 'developers can merge'" do
- stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_DEV_CAN_MERGE)
-
- expect(project).to receive(:execute_hooks)
- expect(project.default_branch).to eq("master")
- expect(project.protected_branches).to receive(:create).with({ name: "master", developers_can_push: false, developers_can_merge: true })
+ expect(project.protected_branches).to receive(:create).with({ name: "master", developers_can_push: true })
execute_service(project, user, @blankrev, 'newrev', 'refs/heads/master' )
end
diff --git a/spec/services/merge_requests/refresh_service_spec.rb b/spec/services/merge_requests/refresh_service_spec.rb
index ce643b3f860..06f56d85aa8 100644
--- a/spec/services/merge_requests/refresh_service_spec.rb
+++ b/spec/services/merge_requests/refresh_service_spec.rb
@@ -88,7 +88,8 @@ describe MergeRequests::RefreshService, services: true do
# Merge master -> feature branch
author = { email: 'test@gitlab.com', time: Time.now, name: "Me" }
commit_options = { message: 'Test message', committer: author, author: author }
- @project.repository.merge(@user, @merge_request, commit_options)
+ master_commit = @project.repository.commit('master')
+ @project.repository.merge(@user, master_commit.id, 'feature', commit_options)
commit = @project.repository.commit('feature')
service.new(@project, @user).execute(@oldrev, commit.id, 'refs/heads/feature')
reload_mrs