diff options
50 files changed, 724 insertions, 447 deletions
diff --git a/CHANGELOG b/CHANGELOG index c25b594f3c5..7bc020c8d68 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,6 +1,7 @@ Please view this file on the master branch, on stable branches it's out of date. v 8.10.0 (unreleased) + - Fix profile activity heatmap to show correct day name (eanplatter) - Expose {should,force}_remove_source_branch (Ben Boeckel) - Disable PostgreSQL statement timeout during migrations - Fix projects dropdown loading performance with a simplified api cal. !5113 (tiagonbotelho) @@ -16,6 +17,7 @@ v 8.10.0 (unreleased) - Align flash messages with left side of page content !4959 (winniehell) - Display tooltip for "Copy to Clipboard" button !5164 (winniehell) - Use default cursor for table header of project files !5165 (winniehell) + - Store when and yaml variables in builds table - Display last commit of deleted branch in push events !4699 (winniehell) - Escape file extension when parsing search results !5141 (winniehell) - Apply the trusted_proxies config to the rack request object for use with rack_attack @@ -54,6 +56,7 @@ 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 @@ -64,6 +67,7 @@ v 8.10.0 (unreleased) - Collapse large diffs by default (!4990) - Fix mentioned users list on diff notes - Fix creation of deployment on build that is retried, redeployed or rollback + - Don't parse Rinku returned value to DocFragment when it didn't change the original html string. - Check for conflicts with existing Project's wiki path when creating a new project. - Show last push widget in upstream after push to fork - Fix stage status shown for pipelines diff --git a/app/assets/javascripts/lib/utils/datetime_utility.js.coffee b/app/assets/javascripts/lib/utils/datetime_utility.js.coffee index 178963fe0aa..2371e913844 100644 --- a/app/assets/javascripts/lib/utils/datetime_utility.js.coffee +++ b/app/assets/javascripts/lib/utils/datetime_utility.js.coffee @@ -2,7 +2,7 @@ w.gl ?= {} w.gl.utils ?= {} - w.gl.utils.days = ['Monday', 'Tuesday', 'Wednesday', 'Thursday', 'Friday', 'Saturday', 'Sunday'] + w.gl.utils.days = ['Sunday', 'Monday', 'Tuesday', 'Wednesday', 'Thursday', 'Friday', 'Saturday'] w.gl.utils.formatDate = (datetime) -> dateFormat(datetime, 'mmm d, yyyy h:MMtt Z') diff --git a/app/assets/javascripts/protected_branches.js.coffee b/app/assets/javascripts/protected_branches.js.coffee index 79c2306e4d2..14afef2e2ee 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" + if name == "developers_can_push" || name == "developers_can_merge" id = $(this).val() - checked = $(this).is(":checked") + can_push = $(this).is(":checked") url = $(this).data("url") $.ajax - type: "PUT" + type: "PATCH" url: url dataType: "json" data: id: id protected_branch: - developers_can_push: checked + "#{name}": can_push success: -> row = $(e.target) diff --git a/app/assets/stylesheets/framework/files.scss b/app/assets/stylesheets/framework/files.scss index 71e4b50f2af..407f1873431 100644 --- a/app/assets/stylesheets/framework/files.scss +++ b/app/assets/stylesheets/framework/files.scss @@ -70,7 +70,7 @@ } &.wiki { - padding: $gl-padding; + padding: 30px $gl-padding; .highlight { margin-bottom: 9px; diff --git a/app/assets/stylesheets/framework/typography.scss b/app/assets/stylesheets/framework/typography.scss index 3575984b229..8659604cb8b 100644 --- a/app/assets/stylesheets/framework/typography.scss +++ b/app/assets/stylesheets/framework/typography.scss @@ -37,39 +37,41 @@ } h1 { - font-size: 1.3em; + font-size: 2em; font-weight: 600; - margin: 24px 0 12px; - padding: 0 0 10px; - border-bottom: 1px solid #e7e9ed; + margin: 1em 0 10px; + padding: 0 0 0.3em; + border-bottom: 1px solid $btn-default-border; color: $gl-gray-dark; } h2 { - font-size: 1.2em; + font-size: 1.6em; font-weight: 600; - margin: 24px 0 12px; + margin: 1em 0 10px; + padding-bottom: 0.3em; + border-bottom: 1px solid $btn-default-border; color: $gl-gray-dark; } h3 { - margin: 24px 0 12px; - font-size: 1.1em; + margin: 1em 0 10px; + font-size: 1.4em; } h4 { - margin: 24px 0 12px; - font-size: 0.98em; + margin: 1em 0 10px; + font-size: 1.25em; } h5 { - margin: 24px 0 12px; - font-size: 0.95em; + margin: 1em 0 10px; + font-size: 1em; } h6 { - margin: 24px 0 12px; - font-size: 0.90em; + margin: 1em 0 10px; + font-size: 0.95em; } blockquote { @@ -115,7 +117,7 @@ ul, ol { padding: 0; - margin: 6px 0 6px 28px !important; + margin: 3px 0 3px 28px !important; } li { diff --git a/app/controllers/projects/protected_branches_controller.rb b/app/controllers/projects/protected_branches_controller.rb index 80dad758afa..10dca47fded 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) + params.require(:protected_branch).permit(:name, :developers_can_push, :developers_can_merge) end end diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 1803aa8eab4..4e5bcff9cf8 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -5,7 +5,7 @@ class ProjectsController < Projects::ApplicationController before_action :project, except: [:new, :create] before_action :repository, except: [:new, :create] before_action :assign_ref_vars, only: [:show], if: :repo_exists? - before_action :tree, only: [:show], if: :project_view_files? + before_action :tree, only: [:show], if: [:repo_exists?, :project_view_files?] # Authorize before_action :authorize_admin_project!, only: [:edit, :update, :housekeeping, :download_export, :export, :remove_export, :generate_new_export] diff --git a/app/helpers/branches_helper.rb b/app/helpers/branches_helper.rb index 601df5c18df..bfd23aa4e04 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::GitAccess.new(current_user, project, 'web').can_push_to_branch?(branch_name) + ::Gitlab::UserAccess.new(current_user, project: project).can_push_to_branch?(branch_name) end def project_branches diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index b24527247e0..ffac3a22efc 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -5,6 +5,7 @@ module Ci belongs_to :erased_by, class_name: 'User' serialize :options + serialize :yaml_variables validates :coverage, numericality: true, allow_blank: true validates_presence_of :ref @@ -52,6 +53,8 @@ module Ci new_build.stage = build.stage new_build.stage_idx = build.stage_idx new_build.trigger_request = build.trigger_request + new_build.yaml_variables = build.yaml_variables + new_build.when = build.when new_build.user = user new_build.environment = build.environment new_build.save @@ -118,7 +121,12 @@ module Ci end def variables - predefined_variables + yaml_variables + project_variables + trigger_variables + variables = [] + variables += predefined_variables + variables += yaml_variables if yaml_variables + variables += project_variables + variables += trigger_variables + variables end def merge_request @@ -395,30 +403,6 @@ module Ci self.update(erased_by: user, erased_at: Time.now, artifacts_expire_at: nil) end - def yaml_variables - global_yaml_variables + job_yaml_variables - end - - def global_yaml_variables - if pipeline.config_processor - pipeline.config_processor.global_variables.map do |key, value| - { key: key, value: value, public: true } - end - else - [] - end - end - - def job_yaml_variables - if pipeline.config_processor - pipeline.config_processor.job_variables(name).map do |key, value| - { key: key, value: value, public: true } - end - else - [] - end - end - def project_variables project.variables.map do |variable| { key: variable.key, value: variable.value, public: false } diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 157901378d3..471e32f3b60 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -552,7 +552,13 @@ class MergeRequest < ActiveRecord::Base end def can_be_merged_by?(user) - ::Gitlab::GitAccess.new(user, project, 'web').can_push_to_branch?(target_branch) + 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) end def mergeable_ci_state? diff --git a/app/models/project.rb b/app/models/project.rb index 1cdf22f6a51..a805f5d97bc 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -838,6 +838,10 @@ 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 5b670cb4b8f..09487b62f98 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -769,9 +769,9 @@ class Repository end end - def merge(user, source_sha, target_branch, options = {}) - our_commit = rugged.branches[target_branch].target - their_commit = rugged.lookup(source_sha) + def merge(user, merge_request, options = {}) + our_commit = rugged.branches[merge_request.target_branch].target + their_commit = rugged.lookup(merge_request.diff_head_sha) raise "Invalid merge target" if our_commit.nil? raise "Invalid merge source" if their_commit.nil? @@ -779,14 +779,16 @@ class Repository merge_index = rugged.merge_commits(our_commit, their_commit) return false if merge_index.conflicts? - commit_with_hooks(user, target_branch) do |ref| + commit_with_hooks(user, merge_request.target_branch) do |tmp_ref| actual_options = options.merge( parents: [our_commit, their_commit], tree: merge_index.write_tree(rugged), - update_ref: ref + update_ref: tmp_ref ) - Rugged::Commit.create(rugged, actual_options) + commit_id = Rugged::Commit.create(rugged, actual_options) + merge_request.update(in_progress_merge_commit_sha: commit_id) + commit_id end end diff --git a/app/services/ci/create_builds_service.rb b/app/services/ci/create_builds_service.rb index 2dcb052d274..3b21f0acb96 100644 --- a/app/services/ci/create_builds_service.rb +++ b/app/services/ci/create_builds_service.rb @@ -36,7 +36,9 @@ module Ci :allow_failure, :stage, :stage_idx, - :environment) + :environment, + :when, + :yaml_variables) build_attrs.merge!(pipeline: @pipeline, ref: @pipeline.ref, diff --git a/app/services/commits/change_service.rb b/app/services/commits/change_service.rb index c578097376a..ed73d8cb8c2 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::GitAccess.new(current_user, project, 'web').can_push_to_branch?(@target_branch) + allowed = ::Gitlab::UserAccess.new(current_user, project: project).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 37c5e321b39..55da949f56a 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::GitAccess.new(current_user, project, 'web').can_push_to_branch?(@target_branch) + allowed = ::Gitlab::UserAccess.new(current_user, project: project).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 a886f35981f..e02b50ff9a2 100644 --- a/app/services/git_push_service.rb +++ b/app/services/git_push_service.rb @@ -89,7 +89,8 @@ 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 - @project.protected_branches.create({ name: @project.default_branch, developers_can_push: developers_can_push }) + 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 }) end end diff --git a/app/services/merge_requests/merge_service.rb b/app/services/merge_requests/merge_service.rb index f1b1d90c457..0dac0614141 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.diff_head_sha, merge_request.target_branch, options) + commit_id = repository.merge(current_user, merge_request, options) merge_request.update(merge_commit_sha: commit_id) rescue GitHooksService::PreReceiveError => e merge_request.update(merge_error: e.message) @@ -43,6 +43,8 @@ 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 b11ecd97a57..1daf6bbf553 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::ForcePushCheck.force_push?(@project, @oldrev, @newrev) + Gitlab::Checks::ForcePush.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 06ab0a3fa00..f000cc38a65 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_by?(current_user) + - if @merge_request.can_be_merged_via_command_line_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 97cb1a9052b..720d67dff7c 100644 --- a/app/views/projects/protected_branches/_branches_list.html.haml +++ b/app/views/projects/protected_branches/_branches_list.html.haml @@ -8,6 +8,7 @@ .table-responsive %table.table.protected-branches-list %colgroup + %col{ width: "20%" } %col{ width: "30%" } %col{ width: "25%" } %col{ width: "25%" } @@ -18,6 +19,7 @@ %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 474aec3a97c..7fda7f96047 100644 --- a/app/views/projects/protected_branches/_protected_branch.html.haml +++ b/app/views/projects/protected_branches/_protected_branch.html.haml @@ -16,6 +16,8 @@ (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 883d3e3af1e..151e1d64851 100644 --- a/app/views/projects/protected_branches/index.html.haml +++ b/app/views/projects/protected_branches/index.html.haml @@ -36,6 +36,14 @@ = 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 new file mode 100644 index 00000000000..15ad8e8bcbb --- /dev/null +++ b/db/migrate/20160519203051_add_developers_can_merge_to_protected_branches.rb @@ -0,0 +1,9 @@ +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 new file mode 100644 index 00000000000..7c5f76572ef --- /dev/null +++ b/db/migrate/20160629025435_add_column_in_progress_merge_commit_sha_to_merge_requests.rb @@ -0,0 +1,8 @@ +# 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/migrate/20160715134306_add_index_for_pipeline_user_id.rb b/db/migrate/20160715134306_add_index_for_pipeline_user_id.rb index e09caa0e6d7..7c991c6d998 100644 --- a/db/migrate/20160715134306_add_index_for_pipeline_user_id.rb +++ b/db/migrate/20160715134306_add_index_for_pipeline_user_id.rb @@ -1,6 +1,8 @@ class AddIndexForPipelineUserId < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers + disable_ddl_transaction! + def change add_concurrent_index :ci_commits, :user_id end diff --git a/db/migrate/20160716115710_add_when_and_yaml_variables_to_ci_builds.rb b/db/migrate/20160716115710_add_when_and_yaml_variables_to_ci_builds.rb new file mode 100644 index 00000000000..3e084023a65 --- /dev/null +++ b/db/migrate/20160716115710_add_when_and_yaml_variables_to_ci_builds.rb @@ -0,0 +1,8 @@ +class AddWhenAndYamlVariablesToCiBuilds < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + def change + add_column :ci_builds, :when, :string + add_column :ci_builds, :yaml_variables, :text + end +end diff --git a/db/schema.rb b/db/schema.rb index 48ecdc6ba1b..8882377f9f4 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20160715134306) do +ActiveRecord::Schema.define(version: 20160716115710) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -168,6 +168,8 @@ ActiveRecord::Schema.define(version: 20160715134306) do t.string "environment" t.datetime "artifacts_expire_at" t.integer "artifacts_size" + t.string "when" + t.text "yaml_variables" end add_index "ci_builds", ["commit_id", "stage_idx", "created_at"], name: "index_ci_builds_on_commit_id_and_stage_idx_and_created_at", using: :btree @@ -626,8 +628,8 @@ ActiveRecord::Schema.define(version: 20160715134306) do t.integer "merge_user_id" t.string "merge_commit_sha" t.datetime "deleted_at" + t.string "in_progress_merge_commit_sha" end - add_index "merge_requests", ["assignee_id"], name: "index_merge_requests_on_assignee_id", using: :btree add_index "merge_requests", ["author_id"], name: "index_merge_requests_on_author_id", using: :btree add_index "merge_requests", ["created_at", "id"], name: "index_merge_requests_on_created_at_and_id", using: :btree @@ -860,11 +862,12 @@ ActiveRecord::Schema.define(version: 20160715134306) 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_push", default: false, null: false + t.boolean "developers_can_merge", 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 77e407b54c5..73557cf7db6 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.allowed?(@current_user) + unless @current_user && Gitlab::UserAccess.new(@current_user).allowed? return nil end diff --git a/lib/banzai/filter/autolink_filter.rb b/lib/banzai/filter/autolink_filter.rb index fac7dad3243..9ed45707515 100644 --- a/lib/banzai/filter/autolink_filter.rb +++ b/lib/banzai/filter/autolink_filter.rb @@ -56,6 +56,8 @@ module Banzai # period (e.g., http://localhost:3000/) rinku = Rinku.auto_link(html, :urls, options, IGNORE_PARENTS.to_a, 1) + return if rinku == html + # Rinku returns a String, so parse it back to a Nokogiri::XML::Document # for further processing. @doc = parse_html(rinku) diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb index 01ef13df57a..a48dc542b14 100644 --- a/lib/ci/gitlab_ci_yaml_processor.rb +++ b/lib/ci/gitlab_ci_yaml_processor.rb @@ -31,28 +31,34 @@ module Ci raise ValidationError, e.message end - def builds_for_stage_and_ref(stage, ref, tag = false, trigger_request = nil) - builds.select do |build| - build[:stage] == stage && - process?(build[:only], build[:except], ref, tag, trigger_request) + def jobs_for_ref(ref, tag = false, trigger_request = nil) + @jobs.select do |_, job| + process?(job[:only], job[:except], ref, tag, trigger_request) end end - def builds - @jobs.map do |name, job| - build_job(name, job) + def jobs_for_stage_and_ref(stage, ref, tag = false, trigger_request = nil) + jobs_for_ref(ref, tag, trigger_request).select do |_, job| + job[:stage] == stage end end - def global_variables - @variables + def builds_for_ref(ref, tag = false, trigger_request = nil) + jobs_for_ref(ref, tag, trigger_request).map do |name, job| + build_job(name, job) + end end - def job_variables(name) - job = @jobs[name.to_sym] - return [] unless job + def builds_for_stage_and_ref(stage, ref, tag = false, trigger_request = nil) + jobs_for_stage_and_ref(stage, ref, tag, trigger_request).map do |name, job| + build_job(name, job) + end + end - job[:variables] || [] + def builds + @jobs.map do |name, job| + build_job(name, job) + end end private @@ -95,11 +101,10 @@ module Ci commands: [job[:before_script] || @before_script, job[:script]].flatten.compact.join("\n"), tag_list: job[:tags] || [], name: name, - only: job[:only], - except: job[:except], allow_failure: job[:allow_failure] || false, when: job[:when] || 'on_success', environment: job[:environment], + yaml_variables: yaml_variables(name), options: { image: job[:image] || @image, services: job[:services] || @services, @@ -111,6 +116,24 @@ module Ci } end + def yaml_variables(name) + variables = global_variables.merge(job_variables(name)) + variables.map do |key, value| + { key: key, value: value, public: true } + end + end + + def global_variables + @variables || {} + end + + def job_variables(name) + job = @jobs[name.to_sym] + return {} unless job + + job[:variables] || {} + end + def validate! @jobs.each do |name, job| validate_job!(name, job) diff --git a/lib/gitlab/access.rb b/lib/gitlab/access.rb index 831f1e635ba..de41ea415a6 100644 --- a/lib/gitlab/access.rb +++ b/lib/gitlab/access.rb @@ -14,9 +14,10 @@ module Gitlab OWNER = 50 # Branch protection settings - PROTECTION_NONE = 0 - PROTECTION_DEV_CAN_PUSH = 1 - PROTECTION_FULL = 2 + PROTECTION_NONE = 0 + PROTECTION_DEV_CAN_PUSH = 1 + PROTECTION_FULL = 2 + PROTECTION_DEV_CAN_MERGE = 3 class << self def values @@ -54,6 +55,7 @@ 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 new file mode 100644 index 00000000000..5551fac4b8b --- /dev/null +++ b/lib/gitlab/checks/change_access.rb @@ -0,0 +1,96 @@ +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 new file mode 100644 index 00000000000..dfa83a0eab3 --- /dev/null +++ b/lib/gitlab/checks/force_push.rb @@ -0,0 +1,17 @@ +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 new file mode 100644 index 00000000000..849848515da --- /dev/null +++ b/lib/gitlab/checks/matching_merge_request.rb @@ -0,0 +1,18 @@ +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 deleted file mode 100644 index 93c6a5bb7f5..00000000000 --- a/lib/gitlab/force_push_check.rb +++ /dev/null @@ -1,15 +0,0 @@ -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 7679c7e4bb8..308f23bc9bc 100644 --- a/lib/gitlab/git_access.rb +++ b/lib/gitlab/git_access.rb @@ -1,52 +1,17 @@ +# 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 + attr_reader :actor, :project, :protocol, :user_access def initialize(actor, project, protocol) @actor = actor @project = project @protocol = protocol - 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 + @user_access = UserAccess.new(user, project: project) end def check(cmd, changes = nil) @@ -56,11 +21,11 @@ module Gitlab return build_status_object(false, "No user or key was provided.") end - if user && !user_allowed? + if user && !user_access.allowed? return build_status_object(false, "Your account has been blocked.") end - unless project && can_read_project? + unless project && (user_access.can_read_project? || deploy_key_can_read_project?) return build_status_object(false, 'The project you were looking for could not be found.') end @@ -95,7 +60,7 @@ module Gitlab end def user_download_access_check - unless user.can?(:download_code, project) + unless user_access.can_do_action?(:download_code) return build_status_object(false, "You are not allowed to download code from this project.") end @@ -125,46 +90,8 @@ 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) - 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) + Checks::ChangeAccess.new(change, user_access: user_access, project: project).exec end def protocol_allowed? @@ -173,48 +100,38 @@ module Gitlab private - 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 + def matching_merge_request?(newrev, branch_name) + Checks::MatchingMergeRequest.new(newrev, branch_name, project).match? end - def protected_tag?(tag_name) - project.repository.tag_exists?(tag_name) - end - - 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 - nil - end + def deploy_key + actor if actor.is_a?(DeployKey) end - def tag_name(ref) - ref = ref.to_s - if Gitlab::Git.tag_ref?(ref) - Gitlab::Git.ref_name(ref) + def deploy_key_can_read_project? + if deploy_key + deploy_key.projects.include?(project) else - nil + false end end protected + def user + return @user if defined?(@user) + + @user = + case actor + when User + actor + when DeployKey + nil + when Key + actor.user + end + end + 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 8672cbc0ec4..f71d3575909 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.can?(:create_wiki, project) + if user_access.can_do_action?(:create_wiki) 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 d1b42c1f9b9..c0f85e9b3a8 100644 --- a/lib/gitlab/user_access.rb +++ b/lib/gitlab/user_access.rb @@ -1,7 +1,23 @@ module Gitlab - module UserAccess - def self.allowed?(user) - return false if user.blocked? + 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? if user.requires_ldap_check? && user.try_obtain_ldap_lease return false unless Gitlab::LDAP::Access.allowed?(user) @@ -9,5 +25,31 @@ 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/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index 1b1b1bdf52d..3edce4d339c 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/spec/controllers/projects_controller_spec.rb @@ -43,6 +43,26 @@ describe ProjectsController do end end + context "project with empty repo" do + let(:empty_project) { create(:project_empty_repo, :public) } + + before { sign_in(user) } + + User.project_views.keys.each do |project_view| + context "with #{project_view} view set" do + before do + user.update_attributes(project_view: project_view) + + get :show, namespace_id: empty_project.namespace.path, id: empty_project.path + end + + it "renders the empty project view" do + expect(response).to render_template('empty') + end + end + end + end + context "rendering default project view" do render_views diff --git a/spec/factories/ci/builds.rb b/spec/factories/ci/builds.rb index fe05a0cfc00..5fb671df570 100644 --- a/spec/factories/ci/builds.rb +++ b/spec/factories/ci/builds.rb @@ -15,6 +15,11 @@ FactoryGirl.define do services: ["postgres"] } end + yaml_variables do + [ + { key: :DB_NAME, value: 'postgres', public: true } + ] + end pipeline factory: :ci_pipeline diff --git a/spec/lib/banzai/filter/autolink_filter_spec.rb b/spec/lib/banzai/filter/autolink_filter_spec.rb index 84c2ddf444e..dca7f997570 100644 --- a/spec/lib/banzai/filter/autolink_filter_spec.rb +++ b/spec/lib/banzai/filter/autolink_filter_spec.rb @@ -15,6 +15,16 @@ describe Banzai::Filter::AutolinkFilter, lib: true do expect(filter(act).to_html).to eq exp end + context 'when the input contains no links' do + it 'does not parse_html back the rinku returned value' do + act = HTML::Pipeline.parse('<p>This text contains no links to autolink</p>') + + expect_any_instance_of(described_class).not_to receive(:parse_html) + + filter(act).to_html + end + end + context 'Rinku schemes' do it 'autolinks http' do doc = filter("See #{link}") @@ -58,6 +68,16 @@ describe Banzai::Filter::AutolinkFilter, lib: true do expect(filter(act).to_html).to eq exp end end + + context 'when the input contains link' do + it 'does parse_html back the rinku returned value' do + act = HTML::Pipeline.parse("<p>See #{link}</p>") + + expect_any_instance_of(described_class).to receive(:parse_html).at_least(:once).and_call_original + + filter(act).to_html + end + end end context 'other schemes' do diff --git a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb index bcbf409c8b0..ad6587b4c25 100644 --- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb +++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb @@ -19,15 +19,14 @@ module Ci expect(config_processor.builds_for_stage_and_ref(type, "master").first).to eq({ stage: "test", stage_idx: 1, - except: nil, name: :rspec, - only: nil, commands: "pwd\nrspec", tag_list: [], options: {}, allow_failure: false, when: "on_success", environment: nil, + yaml_variables: [] }) end @@ -432,11 +431,9 @@ module Ci expect(config_processor.builds_for_stage_and_ref("test", "master").size).to eq(1) expect(config_processor.builds_for_stage_and_ref("test", "master").first).to eq({ - except: nil, stage: "test", stage_idx: 1, name: :rspec, - only: nil, commands: "pwd\nrspec", tag_list: [], options: { @@ -446,6 +443,7 @@ module Ci allow_failure: false, when: "on_success", environment: nil, + yaml_variables: [] }) end @@ -461,11 +459,9 @@ module Ci expect(config_processor.builds_for_stage_and_ref("test", "master").size).to eq(1) expect(config_processor.builds_for_stage_and_ref("test", "master").first).to eq({ - except: nil, stage: "test", stage_idx: 1, name: :rspec, - only: nil, commands: "pwd\nrspec", tag_list: [], options: { @@ -475,101 +471,126 @@ module Ci allow_failure: false, when: "on_success", environment: nil, + yaml_variables: [] }) end end describe 'Variables' do - context 'when global variables are defined' do - it 'returns global variables' do - variables = { - VAR1: 'value1', - VAR2: 'value2', - } + let(:config_processor) { GitlabCiYamlProcessor.new(YAML.dump(config), path) } - config = YAML.dump({ + subject { config_processor.builds.first[:yaml_variables] } + + context 'when global variables are defined' do + let(:variables) do + { VAR1: 'value1', VAR2: 'value2' } + end + let(:config) do + { variables: variables, before_script: ['pwd'], rspec: { script: 'rspec' } - }) + } + end - config_processor = GitlabCiYamlProcessor.new(config, path) + it 'returns global variables' do + expect(subject).to contain_exactly( + { key: :VAR1, value: 'value1', public: true }, + { key: :VAR2, value: 'value2', public: true } + ) + end + end + + context 'when job and global variables are defined' do + let(:global_variables) do + { VAR1: 'global1', VAR3: 'global3' } + end + let(:job_variables) do + { VAR1: 'value1', VAR2: 'value2' } + end + let(:config) do + { + before_script: ['pwd'], + variables: global_variables, + rspec: { script: 'rspec', variables: job_variables } + } + end - expect(config_processor.global_variables).to eq(variables) + it 'returns all unique variables' do + expect(subject).to contain_exactly( + { key: :VAR3, value: 'global3', public: true }, + { key: :VAR1, value: 'value1', public: true }, + { key: :VAR2, value: 'value2', public: true } + ) end end context 'when job variables are defined' do - context 'when syntax is correct' do - it 'returns job variables' do - variables = { - KEY1: 'value1', - SOME_KEY_2: 'value2' - } + let(:config) do + { + before_script: ['pwd'], + rspec: { script: 'rspec', variables: variables } + } + end + + context 'when also global variables are defined' do - config = YAML.dump( - { before_script: ['pwd'], - rspec: { - variables: variables, - script: 'rspec' } - }) + end - config_processor = GitlabCiYamlProcessor.new(config, path) + context 'when syntax is correct' do + let(:variables) do + { VAR1: 'value1', VAR2: 'value2' } + end - expect(config_processor.job_variables(:rspec)).to eq variables + it 'returns job variables' do + expect(subject).to contain_exactly( + { key: :VAR1, value: 'value1', public: true }, + { key: :VAR2, value: 'value2', public: true } + ) end end context 'when syntax is incorrect' do context 'when variables defined but invalid' do - it 'raises error' do - variables = [:KEY1, 'value1', :KEY2, 'value2'] - - config = YAML.dump( - { before_script: ['pwd'], - rspec: { - variables: variables, - script: 'rspec' } - }) + let(:variables) do + [ :VAR1, 'value1', :VAR2, 'value2' ] + end - expect { GitlabCiYamlProcessor.new(config, path) } + it 'raises error' do + expect { subject } .to raise_error(GitlabCiYamlProcessor::ValidationError, - /job: variables should be a map/) + /job: variables should be a map/) end end context 'when variables key defined but value not specified' do - it 'returns empty array' do - config = YAML.dump( - { before_script: ['pwd'], - rspec: { - variables: nil, - script: 'rspec' } - }) - - config_processor = GitlabCiYamlProcessor.new(config, path) + let(:variables) do + nil + end + it 'returns empty array' do ## # When variables config is empty, we assume this is a valid # configuration, see issue #18775 # - expect(config_processor.job_variables(:rspec)) - .to be_an_instance_of(Array).and be_empty + expect(subject).to be_an_instance_of(Array) + expect(subject).to be_empty end end end end context 'when job variables are not defined' do - it 'returns empty array' do - config = YAML.dump({ + let(:config) do + { before_script: ['pwd'], rspec: { script: 'rspec' } - }) - - config_processor = GitlabCiYamlProcessor.new(config, path) + } + end - expect(config_processor.job_variables(:rspec)).to eq [] + it 'returns empty array' do + expect(subject).to be_an_instance_of(Array) + expect(subject).to be_empty end end end @@ -681,11 +702,9 @@ module Ci expect(config_processor.builds_for_stage_and_ref("test", "master").size).to eq(1) expect(config_processor.builds_for_stage_and_ref("test", "master").first).to eq({ - except: nil, stage: "test", stage_idx: 1, name: :rspec, - only: nil, commands: "pwd\nrspec", tag_list: [], options: { @@ -701,6 +720,7 @@ module Ci when: "on_success", allow_failure: false, environment: nil, + yaml_variables: [] }) end @@ -819,17 +839,16 @@ module Ci it "doesn't create jobs that start with dot" do expect(subject.size).to eq(1) expect(subject.first).to eq({ - except: nil, stage: "test", stage_idx: 1, name: :normal_job, - only: nil, commands: "test", tag_list: [], options: {}, when: "on_success", allow_failure: false, environment: nil, + yaml_variables: [] }) end end @@ -865,30 +884,28 @@ module Ci it "is correctly supported for jobs" do expect(subject.size).to eq(2) expect(subject.first).to eq({ - except: nil, stage: "build", stage_idx: 0, name: :job1, - only: nil, commands: "execute-script-for-job", tag_list: [], options: {}, when: "on_success", allow_failure: false, environment: nil, + yaml_variables: [] }) expect(subject.second).to eq({ - except: nil, stage: "build", stage_idx: 0, name: :job2, - only: nil, commands: "execute-script-for-job", tag_list: [], options: {}, when: "on_success", allow_failure: false, environment: nil, + yaml_variables: [] }) end end diff --git a/spec/lib/gitlab/diff/position_tracer_spec.rb b/spec/lib/gitlab/diff/position_tracer_spec.rb index 08312e60f4a..c268f84c759 100644 --- a/spec/lib/gitlab/diff/position_tracer_spec.rb +++ b/spec/lib/gitlab/diff/position_tracer_spec.rb @@ -1639,7 +1639,8 @@ describe Gitlab::Diff::PositionTracer, lib: true do committer: committer } - repository.merge(current_user, second_create_file_commit.sha, branch_name, options) + 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) project.commit(branch_name) end diff --git a/spec/lib/gitlab/git_access_spec.rb b/spec/lib/gitlab/git_access_spec.rb index c79ba11f782..db33c7a22bb 100644 --- a/spec/lib/gitlab/git_access_spec.rb +++ b/spec/lib/gitlab/git_access_spec.rb @@ -6,67 +6,6 @@ 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 @@ -160,96 +99,46 @@ describe Gitlab::GitAccess, lib: true do end describe 'push_access_check' do - def protect_feature_branch - create(:protected_branch, name: 'feature', project: project) - end + before { merge_into_protected_branch } + let(:unprotected_branch) { FFaker::Internet.user_name } - def changes - { - push_new_branch: "#{Gitlab::Git::BLANK_SHA} 570e7b2ab refs/heads/wow", + let(:changes) do + { 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'] - } + push_all: ['6f6d7e7ed 570e7b2ab refs/heads/master', '6f6d7e7ed 570e7b2ab refs/heads/feature'], + merge_into_protected_branch: "0b4bc9a #{merge_into_protected_branch} refs/heads/feature" } end - 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 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 + 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 end - 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]) } + 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" } - it { expect(subject.allowed?).to allowed ? be_truthy : be_falsey } - end - end + 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 end - context "with enabled developers push to protected branches " do - updated_permissions_matrix.keys.each do |role| + def self.run_permission_checks(permissions_matrix) + 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] } - updated_permissions_matrix[role].each do |action, allowed| + permissions_matrix[role].each do |action, allowed| context action do subject { access.push_access_check(changes[action]) } @@ -259,5 +148,97 @@ describe Gitlab::GitAccess, lib: true do 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 + + run_permission_checks(permissions_matrix.deep_merge(developer: { merge_into_protected_branch: true })) + end + + 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) + 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 end diff --git a/spec/lib/gitlab/user_access_spec.rb b/spec/lib/gitlab/user_access_spec.rb new file mode 100644 index 00000000000..aa9ec243498 --- /dev/null +++ b/spec/lib/gitlab/user_access_spec.rb @@ -0,0 +1,88 @@ +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/build_spec.rb b/spec/models/build_spec.rb index e8171788872..481416319dd 100644 --- a/spec/models/build_spec.rb +++ b/spec/models/build_spec.rb @@ -208,7 +208,7 @@ describe Ci::Build, models: true do end before do - build.update_attributes(stage: 'stage') + build.update_attributes(stage: 'stage', yaml_variables: yaml_variables) end it { is_expected.to eq(predefined_variables + yaml_variables) } @@ -260,22 +260,6 @@ describe Ci::Build, models: true do it { is_expected.to eq(predefined_variables + predefined_trigger_variable + yaml_variables + secure_variables + trigger_variables) } end - - context 'when job variables are defined' do - ## - # Job-level variables are defined in gitlab_ci.yml fixture - # - context 'when job variables are unique' do - let(:build) { create(:ci_build, name: 'staging') } - - it 'includes job variables' do - expect(subject).to include( - { key: :KEY1, value: 'value1', public: true }, - { key: :KEY2, value: 'value2', public: true } - ) - end - end - end end end end diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index b39b958450c..e14cec589fe 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -4,16 +4,17 @@ describe Repository, models: true do include RepoHelpers TestBlob = Struct.new(:name) - let(:repository) { create(:project).repository } + let(:project) { create(:project) } + let(:repository) { 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 - source_sha = repository.find_branch('feature').target - merge_commit_sha = repository.merge(user, source_sha, 'master', commit_options) - repository.commit(merge_commit_sha) + 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) 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 83025953889..3d5c19aeff3 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(Gitlab::UserAccess).to receive(:allowed?).and_return(false) + allow_any_instance_of(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(Gitlab::UserAccess).to receive(:allowed?).and_return(false) + allow_any_instance_of(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 afabeed4a80..47c0580e0f0 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 }) + expect(project.protected_branches).to receive(:create).with({ name: "master", developers_can_push: false, developers_can_merge: false }) execute_service(project, user, @blankrev, 'newrev', 'refs/heads/master' ) end @@ -242,7 +242,17 @@ 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 }) + 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 }) 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 06f56d85aa8..ce643b3f860 100644 --- a/spec/services/merge_requests/refresh_service_spec.rb +++ b/spec/services/merge_requests/refresh_service_spec.rb @@ -88,8 +88,7 @@ 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 } - master_commit = @project.repository.commit('master') - @project.repository.merge(@user, master_commit.id, 'feature', commit_options) + @project.repository.merge(@user, @merge_request, commit_options) commit = @project.repository.commit('feature') service.new(@project, @user).execute(@oldrev, commit.id, 'refs/heads/feature') reload_mrs |