diff options
94 files changed, 1163 insertions, 189 deletions
diff --git a/GITLAB_PAGES_VERSION b/GITLAB_PAGES_VERSION index ee6cdce3c29..faef31a4357 100644 --- a/GITLAB_PAGES_VERSION +++ b/GITLAB_PAGES_VERSION @@ -1 +1 @@ -0.6.1 +0.7.0 diff --git a/GITLAB_WORKHORSE_VERSION b/GITLAB_WORKHORSE_VERSION index 19811903a7f..fcdb2e109f6 100644 --- a/GITLAB_WORKHORSE_VERSION +++ b/GITLAB_WORKHORSE_VERSION @@ -1 +1 @@ -3.8.0 +4.0.0 @@ -413,9 +413,7 @@ end # Gitaly GRPC client gem 'gitaly-proto', '~> 0.88.0', require: 'gitaly' -# Explicitly lock grpc as we know 1.9 is bad -# 1.10 is still being tested. See gitlab-org/gitaly#1059 -gem 'grpc', '~> 1.8.3' +gem 'grpc', '~> 1.10.0' # Locked until https://github.com/google/protobuf/issues/4210 is closed gem 'google-protobuf', '= 3.5.1' diff --git a/Gemfile.lock b/Gemfile.lock index bbdc48f964d..fa99ec3febe 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -345,9 +345,9 @@ GEM google-protobuf (3.5.1) googleapis-common-protos-types (1.0.1) google-protobuf (~> 3.0) - googleauth (0.5.3) + googleauth (0.6.2) faraday (~> 0.12) - jwt (~> 1.4) + jwt (>= 1.4, < 3.0) logging (~> 2.0) memoist (~> 0.12) multi_json (~> 1.11) @@ -371,7 +371,7 @@ GEM rake grape_logging (1.7.0) grape - grpc (1.8.3) + grpc (1.10.0) google-protobuf (~> 3.1) googleapis-common-protos-types (~> 1.0.0) googleauth (>= 0.5.1, < 0.7) @@ -505,7 +505,7 @@ GEM mini_portile2 (2.3.0) minitest (5.7.0) mousetrap-rails (1.4.6) - multi_json (1.12.2) + multi_json (1.13.1) multi_xml (0.6.0) multipart-post (2.0.0) mustermann (1.0.0) @@ -648,7 +648,7 @@ GEM pry (~> 0.10) pry-rails (0.3.5) pry (>= 0.9.10) - public_suffix (3.0.0) + public_suffix (3.0.2) pyu-ruby-sasl (0.0.3.3) rack (1.6.8) rack-accept (0.4.5) @@ -862,10 +862,10 @@ GEM sidekiq (>= 4.2.1) sidekiq-limit_fetch (3.4.0) sidekiq (>= 4) - signet (0.7.3) + signet (0.8.1) addressable (~> 2.3) faraday (~> 0.9) - jwt (~> 1.5) + jwt (>= 1.5, < 3.0) multi_json (~> 1.10) simple_po_parser (1.1.2) simplecov (0.14.1) @@ -1078,7 +1078,7 @@ DEPENDENCIES grape-entity (~> 0.6.0) grape-route-helpers (~> 2.1.0) grape_logging (~> 1.7) - grpc (~> 1.8.3) + grpc (~> 1.10.0) haml_lint (~> 0.26.0) hamlit (~> 2.6.1) hashie-forbidden_attributes diff --git a/PROCESS.md b/PROCESS.md index c24210341e0..5ae191840ff 100644 --- a/PROCESS.md +++ b/PROCESS.md @@ -71,7 +71,7 @@ star, smile, etc.). Some good tips about code reviews can be found in our ## Feature freeze on the 7th for the release on the 22nd -After 7th at 23:59 (Pacific Standard Time Zone) of each month, RC1 of the upcoming release (to be shipped on the 22nd) is created and deployed to GitLab.com and the stable branch for this release is frozen, which means master is no longer merged into it. +After 7th at 23:59 (Pacific Time Zone) of each month, RC1 of the upcoming release (to be shipped on the 22nd) is created and deployed to GitLab.com and the stable branch for this release is frozen, which means master is no longer merged into it. Merge requests may still be merged into master during this period, but they will go into the _next_ release, unless they are manually cherry-picked into the stable branch. diff --git a/app/assets/javascripts/sidebar/components/assignees/sidebar_assignees.js b/app/assets/javascripts/sidebar/components/assignees/sidebar_assignees.vue index 8269fe1281d..b58e04b5e60 100644 --- a/app/assets/javascripts/sidebar/components/assignees/sidebar_assignees.js +++ b/app/assets/javascripts/sidebar/components/assignees/sidebar_assignees.vue @@ -1,3 +1,4 @@ +<script> import Flash from '../../../flash'; import AssigneeTitle from './assignee_title'; import Assignees from './assignees.vue'; @@ -6,11 +7,9 @@ import eventHub from '../../event_hub'; export default { name: 'SidebarAssignees', - data() { - return { - store: new Store(), - loading: false, - }; + components: { + AssigneeTitle, + Assignees, }, props: { mediator: { @@ -27,9 +26,28 @@ export default { default: false, }, }, - components: { - AssigneeTitle, - Assignees, + data() { + return { + store: new Store(), + loading: false, + }; + }, + created() { + this.removeAssignee = this.store.removeAssignee.bind(this.store); + this.addAssignee = this.store.addAssignee.bind(this.store); + this.removeAllAssignees = this.store.removeAllAssignees.bind(this.store); + + // Get events from glDropdown + eventHub.$on('sidebar.removeAssignee', this.removeAssignee); + eventHub.$on('sidebar.addAssignee', this.addAssignee); + eventHub.$on('sidebar.removeAllAssignees', this.removeAllAssignees); + eventHub.$on('sidebar.saveAssignees', this.saveAssignees); + }, + beforeDestroy() { + eventHub.$off('sidebar.removeAssignee', this.removeAssignee); + eventHub.$off('sidebar.addAssignee', this.addAssignee); + eventHub.$off('sidebar.removeAllAssignees', this.removeAllAssignees); + eventHub.$off('sidebar.saveAssignees', this.saveAssignees); }, methods: { assignSelf() { @@ -54,39 +72,24 @@ export default { }); }, }, - created() { - this.removeAssignee = this.store.removeAssignee.bind(this.store); - this.addAssignee = this.store.addAssignee.bind(this.store); - this.removeAllAssignees = this.store.removeAllAssignees.bind(this.store); - - // Get events from glDropdown - eventHub.$on('sidebar.removeAssignee', this.removeAssignee); - eventHub.$on('sidebar.addAssignee', this.addAssignee); - eventHub.$on('sidebar.removeAllAssignees', this.removeAllAssignees); - eventHub.$on('sidebar.saveAssignees', this.saveAssignees); - }, - beforeDestroy() { - eventHub.$off('sidebar.removeAssignee', this.removeAssignee); - eventHub.$off('sidebar.addAssignee', this.addAssignee); - eventHub.$off('sidebar.removeAllAssignees', this.removeAllAssignees); - eventHub.$off('sidebar.saveAssignees', this.saveAssignees); - }, - template: ` - <div> - <assignee-title - :number-of-assignees="store.assignees.length" - :loading="loading || store.isFetching.assignees" - :editable="store.editable" - :show-toggle="!signedIn" - /> - <assignees - v-if="!store.isFetching.assignees" - class="value" - :root-path="store.rootPath" - :users="store.assignees" - :editable="store.editable" - @assign-self="assignSelf" - /> - </div> - `, }; +</script> + +<template> + <div> + <assignee-title + :number-of-assignees="store.assignees.length" + :loading="loading || store.isFetching.assignees" + :editable="store.editable" + :show-toggle="!signedIn" + /> + <assignees + v-if="!store.isFetching.assignees" + class="value" + :root-path="store.rootPath" + :users="store.assignees" + :editable="store.editable" + @assign-self="assignSelf" + /> + </div> +</template> diff --git a/app/assets/javascripts/sidebar/mount_sidebar.js b/app/assets/javascripts/sidebar/mount_sidebar.js index 56cc78ca0ca..ef748f18301 100644 --- a/app/assets/javascripts/sidebar/mount_sidebar.js +++ b/app/assets/javascripts/sidebar/mount_sidebar.js @@ -1,6 +1,6 @@ import Vue from 'vue'; import SidebarTimeTracking from './components/time_tracking/sidebar_time_tracking'; -import SidebarAssignees from './components/assignees/sidebar_assignees'; +import SidebarAssignees from './components/assignees/sidebar_assignees.vue'; import ConfidentialIssueSidebar from './components/confidential/confidential_issue_sidebar.vue'; import SidebarMoveIssue from './lib/sidebar_move_issue'; import LockIssueSidebar from './components/lock/lock_issue_sidebar.vue'; diff --git a/app/assets/javascripts/terminal/terminal.js b/app/assets/javascripts/terminal/terminal.js index 6b9422b1816..904b0093f7b 100644 --- a/app/assets/javascripts/terminal/terminal.js +++ b/app/assets/javascripts/terminal/terminal.js @@ -6,8 +6,14 @@ constructor(options) { this.options = options || {}; - this.options.cursorBlink = options.cursorBlink || true; - this.options.screenKeys = options.screenKeys || true; + if (!Object.prototype.hasOwnProperty.call(this.options, 'cursorBlink')) { + this.options.cursorBlink = true; + } + + if (!Object.prototype.hasOwnProperty.call(this.options, 'screenKeys')) { + this.options.screenKeys = true; + } + this.container = document.querySelector(options.selector); this.setSocketUrl(); diff --git a/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_maintainer_edit.vue b/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_maintainer_edit.vue new file mode 100644 index 00000000000..7a69cce695e --- /dev/null +++ b/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_maintainer_edit.vue @@ -0,0 +1,19 @@ +<script> + export default { + name: 'MRWidgetMaintainerEdit', + props: { + maintainerEditAllowed: { + type: Boolean, + default: false, + required: false, + }, + }, + }; +</script> +<template> + <section class="mr-info-list mr-maintainer-edit"> + <p v-if="maintainerEditAllowed"> + {{ s__("mrWidget|Allows edits from maintainers") }} + </p> + </section> +</template> diff --git a/app/assets/javascripts/vue_merge_request_widget/dependencies.js b/app/assets/javascripts/vue_merge_request_widget/dependencies.js index edb3baa39e4..a1bc28873df 100644 --- a/app/assets/javascripts/vue_merge_request_widget/dependencies.js +++ b/app/assets/javascripts/vue_merge_request_widget/dependencies.js @@ -15,6 +15,7 @@ export { default as WidgetHeader } from './components/mr_widget_header.vue'; export { default as WidgetMergeHelp } from './components/mr_widget_merge_help.vue'; export { default as WidgetPipeline } from './components/mr_widget_pipeline.vue'; export { default as WidgetDeployment } from './components/mr_widget_deployment'; +export { default as WidgetMaintainerEdit } from './components/mr_widget_maintainer_edit.vue'; export { default as WidgetRelatedLinks } from './components/mr_widget_related_links.vue'; export { default as MergedState } from './components/states/mr_widget_merged.vue'; export { default as FailedToMerge } from './components/states/mr_widget_failed_to_merge.vue'; diff --git a/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.js b/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.js index 797f0f6ec0f..df3eb86f35c 100644 --- a/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.js +++ b/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.js @@ -6,6 +6,7 @@ import { WidgetMergeHelp, WidgetPipeline, WidgetDeployment, + WidgetMaintainerEdit, WidgetRelatedLinks, MergedState, ClosedState, @@ -211,6 +212,7 @@ export default { 'mr-widget-merge-help': WidgetMergeHelp, 'mr-widget-pipeline': WidgetPipeline, 'mr-widget-deployment': WidgetDeployment, + 'mr-widget-maintainer-edit': WidgetMaintainerEdit, 'mr-widget-related-links': WidgetRelatedLinks, 'mr-widget-merged': MergedState, 'mr-widget-closed': ClosedState, @@ -251,11 +253,12 @@ export default { :is="componentName" :mr="mr" :service="service" /> + <mr-widget-maintainer-edit + :maintainerEditAllowed="mr.maintainerEditAllowed" /> <mr-widget-related-links v-if="shouldRenderRelatedLinks" :state="mr.state" - :related-links="mr.relatedLinks" - /> + :related-links="mr.relatedLinks" /> </div> <div class="mr-widget-footer" diff --git a/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js b/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js index 9a750ce42bd..5d07bcf1bb9 100644 --- a/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js +++ b/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js @@ -76,6 +76,7 @@ export default class MergeRequestStore { this.canBeMerged = data.can_be_merged || false; this.isMergeAllowed = data.mergeable || false; this.mergeOngoing = data.merge_ongoing; + this.maintainerEditAllowed = data.allow_maintainer_to_push; // Cherry-pick and Revert actions related this.canCherryPickInCurrentMR = currentUser.can_cherry_pick_on_current_merge_request || false; diff --git a/app/assets/stylesheets/framework/dropdowns.scss b/app/assets/stylesheets/framework/dropdowns.scss index 1d7b0b602cc..127583626cf 100644 --- a/app/assets/stylesheets/framework/dropdowns.scss +++ b/app/assets/stylesheets/framework/dropdowns.scss @@ -272,7 +272,7 @@ .divider { height: 1px; - margin: 6px 0; + margin: #{$grid-size / 2} 0; padding: 0; background-color: $dropdown-divider-color; diff --git a/app/assets/stylesheets/pages/merge_requests.scss b/app/assets/stylesheets/pages/merge_requests.scss index f887a11004f..f8708a2c0a3 100644 --- a/app/assets/stylesheets/pages/merge_requests.scss +++ b/app/assets/stylesheets/pages/merge_requests.scss @@ -453,7 +453,8 @@ } } -.mr-links { +.mr-links, +.mr-maintainer-edit { padding-left: $status-icon-size + $status-icon-margin; } diff --git a/app/controllers/concerns/creates_commit.rb b/app/controllers/concerns/creates_commit.rb index 6f4fdcdaa4f..b26a76d2b62 100644 --- a/app/controllers/concerns/creates_commit.rb +++ b/app/controllers/concerns/creates_commit.rb @@ -4,7 +4,7 @@ module CreatesCommit # rubocop:disable Gitlab/ModuleWithInstanceVariables def create_commit(service, success_path:, failure_path:, failure_view: nil, success_notice: nil) - if can?(current_user, :push_code, @project) + if user_access(@project).can_push_to_branch?(branch_name_or_ref) @project_to_commit_into = @project @branch_name ||= @ref else @@ -50,7 +50,7 @@ module CreatesCommit # rubocop:enable Gitlab/ModuleWithInstanceVariables def authorize_edit_tree! - return if can_collaborate_with_project? + return if can_collaborate_with_project?(project, ref: branch_name_or_ref) access_denied! end @@ -123,4 +123,8 @@ module CreatesCommit params[:create_merge_request].present? && (different_project? || @start_branch != @branch_name) # rubocop:disable Gitlab/ModuleWithInstanceVariables end + + def branch_name_or_ref + @branch_name || @ref # rubocop:disable Gitlab/ModuleWithInstanceVariables + end end diff --git a/app/controllers/projects/application_controller.rb b/app/controllers/projects/application_controller.rb index 6025a40348b..6d9b42a2c04 100644 --- a/app/controllers/projects/application_controller.rb +++ b/app/controllers/projects/application_controller.rb @@ -6,7 +6,7 @@ class Projects::ApplicationController < ApplicationController before_action :repository layout 'project' - helper_method :repository, :can_collaborate_with_project? + helper_method :repository, :can_collaborate_with_project?, :user_access private @@ -31,11 +31,12 @@ class Projects::ApplicationController < ApplicationController @repository ||= project.repository end - def can_collaborate_with_project?(project = nil) + def can_collaborate_with_project?(project = nil, ref: nil) project ||= @project can?(current_user, :push_code, project) || - (current_user && current_user.already_forked?(project)) + (current_user && current_user.already_forked?(project)) || + user_access(project).can_push_to_branch?(ref) end def authorize_action!(action) @@ -90,4 +91,9 @@ class Projects::ApplicationController < ApplicationController def check_issues_available! return render_404 unless @project.feature_available?(:issues, current_user) end + + def user_access(project) + @user_access ||= {} + @user_access[project] ||= Gitlab::UserAccess.new(current_user, project: project) + end end diff --git a/app/controllers/projects/blob_controller.rb b/app/controllers/projects/blob_controller.rb index 405726c017c..0c1c286a0a4 100644 --- a/app/controllers/projects/blob_controller.rb +++ b/app/controllers/projects/blob_controller.rb @@ -9,8 +9,12 @@ class Projects::BlobController < Projects::ApplicationController before_action :require_non_empty_project, except: [:new, :create] before_action :authorize_download_code! - before_action :authorize_edit_tree!, only: [:new, :create, :update, :destroy] + + # We need to assign the blob vars before `authorize_edit_tree!` so we can + # validate access to a specific ref. before_action :assign_blob_vars + before_action :authorize_edit_tree!, only: [:new, :create, :update, :destroy] + before_action :commit, except: [:new, :create] before_action :blob, except: [:new, :create] before_action :require_branch_head, only: [:edit, :update] @@ -46,7 +50,7 @@ class Projects::BlobController < Projects::ApplicationController end def edit - if can_collaborate_with_project? + if can_collaborate_with_project?(project, ref: @ref) blob.load_all_data! else redirect_to action: 'show' diff --git a/app/controllers/projects/merge_requests/application_controller.rb b/app/controllers/projects/merge_requests/application_controller.rb index 793ae03fb88..67d4ea2ca8f 100644 --- a/app/controllers/projects/merge_requests/application_controller.rb +++ b/app/controllers/projects/merge_requests/application_controller.rb @@ -15,6 +15,7 @@ class Projects::MergeRequests::ApplicationController < Projects::ApplicationCont def merge_request_params_attributes [ + :allow_maintainer_to_push, :assignee_id, :description, :force_remove_source_branch, diff --git a/app/helpers/merge_requests_helper.rb b/app/helpers/merge_requests_helper.rb index ce57422f45d..fb4fe1c40b7 100644 --- a/app/helpers/merge_requests_helper.rb +++ b/app/helpers/merge_requests_helper.rb @@ -125,6 +125,19 @@ module MergeRequestsHelper link_to(url[merge_request.project, merge_request], data: data_attrs, &block) end + def allow_maintainer_push_unavailable_reason(merge_request) + return if merge_request.can_allow_maintainer_to_push?(current_user) + + minimum_visibility = [merge_request.target_project.visibility_level, + merge_request.source_project.visibility_level].min + + if minimum_visibility < Gitlab::VisibilityLevel::INTERNAL + _('Not available for private projects') + elsif ProtectedBranch.protected?(merge_request.source_project, merge_request.source_branch) + _('Not available for protected branches') + end + end + def merge_params_ee(merge_request) {} end diff --git a/app/helpers/tree_helper.rb b/app/helpers/tree_helper.rb index f6a6d9bebde..b64be89c181 100644 --- a/app/helpers/tree_helper.rb +++ b/app/helpers/tree_helper.rb @@ -49,15 +49,13 @@ module TreeHelper return false unless on_top_of_branch?(project, ref) - can_collaborate_with_project?(project) + can_collaborate_with_project?(project, ref: ref) end def tree_edit_branch(project = @project, ref = @ref) return unless can_edit_tree?(project, ref) - project = project.present(current_user: current_user) - - if project.can_current_user_push_to_branch?(ref) + if user_access(project).can_push_to_branch?(ref) ref else project = tree_edit_project(project) @@ -88,7 +86,16 @@ module TreeHelper end def commit_in_fork_help - "A new branch will be created in your fork and a new merge request will be started." + _("A new branch will be created in your fork and a new merge request will be started.") + end + + def commit_in_single_accessible_branch + branch_name = html_escape(selected_branch) + + message = _("Your changes can be committed to %{branch_name} because a merge "\ + "request is open.") % { branch_name: "<strong>#{branch_name}</strong>" } + + message.html_safe end def path_breadcrumbs(max_links = 6) @@ -125,4 +132,8 @@ module TreeHelper return tree.name end end + + def selected_branch + @branch_name || tree_edit_branch + end end diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 9a7e66a9cbb..c2bae379a94 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -865,7 +865,7 @@ class MergeRequest < ActiveRecord::Base 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) + access.can_update_branch?(target_branch) end def can_be_merged_via_command_line_by?(user) @@ -1087,4 +1087,22 @@ class MergeRequest < ActiveRecord::Base project.merge_requests.merged.where(author_id: author_id).empty? end + + def allow_maintainer_to_push + maintainer_push_possible? && super + end + + alias_method :allow_maintainer_to_push?, :allow_maintainer_to_push + + def maintainer_push_possible? + source_project.present? && for_fork? && + target_project.visibility_level > Gitlab::VisibilityLevel::PRIVATE && + source_project.visibility_level > Gitlab::VisibilityLevel::PRIVATE && + !ProtectedBranch.protected?(source_project, source_branch) + end + + def can_allow_maintainer_to_push?(user) + maintainer_push_possible? && + Ability.allowed?(user, :push_code, source_project) + end end diff --git a/app/models/project.rb b/app/models/project.rb index 5cd1da43645..5f9d9785d64 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -150,6 +150,7 @@ class Project < ActiveRecord::Base # Merge Requests for target project should be removed with it has_many :merge_requests, foreign_key: 'target_project_id' + has_many :source_of_merge_requests, foreign_key: 'source_project_id', class_name: 'MergeRequest' has_many :issues has_many :labels, class_name: 'ProjectLabel' has_many :services @@ -1799,6 +1800,33 @@ class Project < ActiveRecord::Base Badge.where("id IN (#{union.to_sql})") # rubocop:disable GitlabSecurity/SqlInjection end + def merge_requests_allowing_push_to_user(user) + return MergeRequest.none unless user + + developer_access_exists = user.project_authorizations + .where('access_level >= ? ', Gitlab::Access::DEVELOPER) + .where('project_authorizations.project_id = merge_requests.target_project_id') + .limit(1) + .select(1) + source_of_merge_requests.opened + .where(allow_maintainer_to_push: true) + .where('EXISTS (?)', developer_access_exists) + end + + def branch_allows_maintainer_push?(user, branch_name) + return false unless user + + cache_key = "user:#{user.id}:#{branch_name}:branch_allows_push" + + memoized_results = strong_memoize(:branch_allows_maintainer_push) do + Hash.new do |result, cache_key| + result[cache_key] = fetch_branch_allows_maintainer_push?(user, branch_name) + end + end + + memoized_results[cache_key] + end + private def storage @@ -1921,4 +1949,22 @@ class Project < ActiveRecord::Base raise ex end + + def fetch_branch_allows_maintainer_push?(user, branch_name) + check_access = -> do + merge_request = source_of_merge_requests.opened + .where(allow_maintainer_to_push: true) + .find_by(source_branch: branch_name) + + merge_request&.can_be_merged_by?(user) + end + + if RequestStore.active? + RequestStore.fetch("project-#{id}:branch-#{branch_name}:user-#{user.id}:branch_allows_maintainer_push") do + check_access.call + end + else + check_access.call + end + end end diff --git a/app/models/repository.rb b/app/models/repository.rb index e6b88320110..461dfbec2bc 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -651,14 +651,15 @@ class Repository end def last_commit_for_path(sha, path) - commit_by(oid: last_commit_id_for_path(sha, path)) + commit = raw_repository.last_commit_for_path(sha, path) + ::Commit.new(commit, @project) if commit end def last_commit_id_for_path(sha, path) key = path.blank? ? "last_commit_id_for_path:#{sha}" : "last_commit_id_for_path:#{sha}:#{Digest::SHA1.hexdigest(path)}" cache.fetch(key) do - raw_repository.last_commit_id_for_path(sha, path) + last_commit_for_path(sha, path)&.id end end diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb index 3b0550b4dd6..57ab0c23dcd 100644 --- a/app/policies/project_policy.rb +++ b/app/policies/project_policy.rb @@ -61,6 +61,11 @@ class ProjectPolicy < BasePolicy desc "Project has request access enabled" condition(:request_access_enabled, scope: :subject) { project.request_access_enabled } + desc "Has merge requests allowing pushes to user" + condition(:has_merge_requests_allowing_pushes, scope: :subject) do + project.merge_requests_allowing_push_to_user(user).any? + end + features = %w[ merge_requests issues @@ -291,6 +296,15 @@ class ProjectPolicy < BasePolicy prevent :read_issue end + # These rules are included to allow maintainers of projects to push to certain + # to run pipelines for the branches they have access to. + rule { can?(:public_access) & has_merge_requests_allowing_pushes }.policy do + enable :create_build + enable :update_build + enable :create_pipeline + enable :update_pipeline + end + private def team_member? diff --git a/app/presenters/merge_request_presenter.rb b/app/presenters/merge_request_presenter.rb index 08ae49562c7..9f3f2637183 100644 --- a/app/presenters/merge_request_presenter.rb +++ b/app/presenters/merge_request_presenter.rb @@ -78,7 +78,7 @@ class MergeRequestPresenter < Gitlab::View::Presenter::Delegated end def rebase_path - if !rebase_in_progress? && should_be_rebased? && user_can_push_to_source_branch? + if !rebase_in_progress? && should_be_rebased? && can_push_to_source_branch? rebase_project_merge_request_path(project, merge_request) end end @@ -160,7 +160,11 @@ class MergeRequestPresenter < Gitlab::View::Presenter::Delegated end def can_push_to_source_branch? - source_branch_exists? && user_can_push_to_source_branch? + return false unless source_branch_exists? + + !!::Gitlab::UserAccess + .new(current_user, project: source_project) + .can_push_to_branch?(source_branch) end private @@ -191,17 +195,10 @@ class MergeRequestPresenter < Gitlab::View::Presenter::Delegated end.sort.to_sentence end - def user_can_push_to_source_branch? - return false unless source_branch_exists? - - ::Gitlab::UserAccess - .new(current_user, project: source_project) - .can_push_to_branch?(source_branch) - end - def user_can_collaborate_with_project? can?(current_user, :push_code, project) || - (current_user && current_user.already_forked?(project)) + (current_user && current_user.already_forked?(project)) || + can_push_to_source_branch? end def user_can_fork_project? diff --git a/app/serializers/merge_request_widget_entity.rb b/app/serializers/merge_request_widget_entity.rb index a3ebec0efc6..4a812e39ee1 100644 --- a/app/serializers/merge_request_widget_entity.rb +++ b/app/serializers/merge_request_widget_entity.rb @@ -11,6 +11,7 @@ class MergeRequestWidgetEntity < IssuableEntity expose :source_project_id expose :target_branch expose :target_project_id + expose :allow_maintainer_to_push expose :should_be_rebased?, as: :should_be_rebased expose :ff_only_enabled do |merge_request| @@ -29,6 +30,7 @@ class MergeRequestWidgetEntity < IssuableEntity expose :can_push_to_source_branch do |merge_request| presenter(merge_request).can_push_to_source_branch? end + expose :rebase_path do |merge_request| presenter(merge_request).rebase_path end @@ -136,8 +138,8 @@ class MergeRequestWidgetEntity < IssuableEntity end expose :new_blob_path do |merge_request| - if can?(current_user, :push_code, merge_request.project) - project_new_blob_path(merge_request.project, merge_request.source_branch) + if presenter(merge_request).can_push_to_source_branch? + project_new_blob_path(merge_request.source_project, merge_request.source_branch) end end diff --git a/app/services/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb index c8b112132b3..3b3d9239086 100644 --- a/app/services/ci/create_pipeline_service.rb +++ b/app/services/ci/create_pipeline_service.rb @@ -81,7 +81,7 @@ module Ci end def related_merge_requests - MergeRequest.opened.where(source_project: pipeline.project, source_branch: pipeline.ref) + pipeline.project.source_of_merge_requests.opened.where(source_branch: pipeline.ref) end end end diff --git a/app/services/merge_requests/base_service.rb b/app/services/merge_requests/base_service.rb index 23262b62615..231ab76fde4 100644 --- a/app/services/merge_requests/base_service.rb +++ b/app/services/merge_requests/base_service.rb @@ -35,6 +35,14 @@ module MergeRequests end end + def filter_params(merge_request) + super + + unless merge_request.can_allow_maintainer_to_push?(current_user) + params.delete(:allow_maintainer_to_push) + end + end + def merge_request_metrics_service(merge_request) MergeRequestMetricsService.new(merge_request.metrics) end diff --git a/app/services/merge_requests/build_service.rb b/app/services/merge_requests/build_service.rb index 4b186d93772..a98bbdf74dd 100644 --- a/app/services/merge_requests/build_service.rb +++ b/app/services/merge_requests/build_service.rb @@ -6,6 +6,7 @@ module MergeRequests @params_issue_iid = params.delete(:issue_iid) self.merge_request = MergeRequest.new(params) + merge_request.author = current_user merge_request.compare_commits = [] merge_request.source_project = find_source_project merge_request.target_project = find_target_project diff --git a/app/views/projects/_commit_button.html.haml b/app/views/projects/_commit_button.html.haml index b55dc3dce5c..b387e38c1a6 100644 --- a/app/views/projects/_commit_button.html.haml +++ b/app/views/projects/_commit_button.html.haml @@ -3,6 +3,4 @@ = link_to 'Cancel', cancel_path, class: 'btn btn-cancel', data: {confirm: leave_edit_message} - - unless can?(current_user, :push_code, @project) - .inline.prepend-left-10 - = commit_in_fork_help + = render 'shared/projects/edit_information' diff --git a/app/views/projects/blob/_new_dir.html.haml b/app/views/projects/blob/_new_dir.html.haml index 5d48a35dc4c..48ff66900be 100644 --- a/app/views/projects/blob/_new_dir.html.haml +++ b/app/views/projects/blob/_new_dir.html.haml @@ -17,6 +17,4 @@ = submit_tag _("Create directory"), class: 'btn btn-create' = link_to "Cancel", '#', class: "btn btn-cancel", "data-dismiss" => "modal" - - unless can?(current_user, :push_code, @project) - .inline.prepend-left-10 - = commit_in_fork_help + = render 'shared/projects/edit_information' diff --git a/app/views/projects/blob/_upload.html.haml b/app/views/projects/blob/_upload.html.haml index f1324c61500..182d02376bf 100644 --- a/app/views/projects/blob/_upload.html.haml +++ b/app/views/projects/blob/_upload.html.haml @@ -24,6 +24,4 @@ = button_title = link_to _("Cancel"), '#', class: "btn btn-cancel", "data-dismiss" => "modal" - - unless can?(current_user, :push_code, @project) - .inline.prepend-left-10 - = commit_in_fork_help + = render 'shared/projects/edit_information' diff --git a/app/views/projects/clusters/_integration_form.html.haml b/app/views/projects/clusters/_integration_form.html.haml index d4c0cd82ce3..db97203a2aa 100644 --- a/app/views/projects/clusters/_integration_form.html.haml +++ b/app/views/projects/clusters/_integration_form.html.haml @@ -21,6 +21,12 @@ = sprite_icon('status_failed_borderless', size: 16, css_class: 'toggle-icon-svg toggle-status-unchecked') .form-group + %h5= s_('ClusterIntegration|Security') + %p + = s_("ClusterIntegration|The default cluster configuration grants access to a wide set of functionalities needed to successfully build and deploy a containerised application.") + = link_to s_("ClusterIntegration|Learn more about security configuration"), help_page_path('user/project/clusters/index.md', anchor: 'security-implications') + + .form-group %h5= s_('ClusterIntegration|Environment scope') %p = s_("ClusterIntegration|Choose which of your project's environments will use this Kubernetes cluster.") diff --git a/app/views/projects/commit/_change.html.haml b/app/views/projects/commit/_change.html.haml index 93407956f56..21e4664d4e4 100644 --- a/app/views/projects/commit/_change.html.haml +++ b/app/views/projects/commit/_change.html.haml @@ -35,6 +35,4 @@ = submit_tag label, class: 'btn btn-create' = link_to _("Cancel"), '#', class: "btn btn-cancel", "data-dismiss" => "modal" - - unless can?(current_user, :push_code, @project) - .inline.prepend-left-10 - = commit_in_fork_help + = render 'shared/projects/edit_information' diff --git a/app/views/shared/_new_commit_form.html.haml b/app/views/shared/_new_commit_form.html.haml index 0a4a24ae807..9221fd1e025 100644 --- a/app/views/shared/_new_commit_form.html.haml +++ b/app/views/shared/_new_commit_form.html.haml @@ -1,3 +1,6 @@ +- project = @project.present(current_user: current_user) +- branch_name = selected_branch + = render 'shared/commit_message_container', placeholder: placeholder - if @project.empty_repo? @@ -7,12 +10,14 @@ .form-group.branch = label_tag 'branch_name', _('Target Branch'), class: 'control-label' .col-sm-10 - = text_field_tag 'branch_name', @branch_name || tree_edit_branch, required: true, class: "form-control js-branch-name ref-name" + = text_field_tag 'branch_name', branch_name, required: true, class: "form-control js-branch-name ref-name" .js-create-merge-request-container = render 'shared/new_merge_request_checkbox' + - elsif project.can_current_user_push_to_branch?(branch_name) + = hidden_field_tag 'branch_name', branch_name - else - = hidden_field_tag 'branch_name', @branch_name || tree_edit_branch + = hidden_field_tag 'branch_name', branch_name = hidden_field_tag 'create_merge_request', 1 = hidden_field_tag 'original_branch', @ref, class: 'js-original-branch' diff --git a/app/views/shared/issuable/_form.html.haml b/app/views/shared/issuable/_form.html.haml index 6dfabd7ba4c..4c8f03f1498 100644 --- a/app/views/shared/issuable/_form.html.haml +++ b/app/views/shared/issuable/_form.html.haml @@ -33,6 +33,8 @@ = render 'shared/issuable/form/merge_params', issuable: issuable += render 'shared/issuable/form/contribution', issuable: issuable, form: form + - if @merge_request_to_resolve_discussions_of .form-group .col-sm-10.col-sm-offset-2 diff --git a/app/views/shared/issuable/form/_contribution.html.haml b/app/views/shared/issuable/form/_contribution.html.haml new file mode 100644 index 00000000000..0f2d313a5cc --- /dev/null +++ b/app/views/shared/issuable/form/_contribution.html.haml @@ -0,0 +1,20 @@ +- issuable = local_assigns.fetch(:issuable) +- form = local_assigns.fetch(:form) + +- return unless issuable.is_a?(MergeRequest) +- return unless issuable.for_fork? +- return unless can?(current_user, :push_code, issuable.source_project) + +%hr + +.form-group + .control-label + = _('Contribution') + .col-sm-10 + .checkbox + = form.label :allow_maintainer_to_push do + = form.check_box :allow_maintainer_to_push, disabled: !issuable.can_allow_maintainer_to_push?(current_user) + = _('Allow edits from maintainers') + = link_to 'About this feature', help_page_path('user/project/merge_requests/maintainer_access') + .help-block + = allow_maintainer_push_unavailable_reason(issuable) diff --git a/app/views/shared/projects/_edit_information.html.haml b/app/views/shared/projects/_edit_information.html.haml new file mode 100644 index 00000000000..ec9dc8f62c2 --- /dev/null +++ b/app/views/shared/projects/_edit_information.html.haml @@ -0,0 +1,6 @@ +- unless can?(current_user, :push_code, @project) + .inline.prepend-left-10 + - if @project.branch_allows_maintainer_push?(current_user, selected_branch) + = commit_in_single_accessible_branch + - else + = commit_in_fork_help diff --git a/changelogs/unreleased/39444-make-margin-around-dropdown-dividers-4px.yml b/changelogs/unreleased/39444-make-margin-around-dropdown-dividers-4px.yml new file mode 100644 index 00000000000..da65cfff799 --- /dev/null +++ b/changelogs/unreleased/39444-make-margin-around-dropdown-dividers-4px.yml @@ -0,0 +1,5 @@ +--- +title: Set margins around dropdown dividers to 4px +merge_request: 17517 +author: +type: fixed diff --git a/changelogs/unreleased/43780-add-a-paragraph-about-clusters-security-implications.yml b/changelogs/unreleased/43780-add-a-paragraph-about-clusters-security-implications.yml new file mode 100644 index 00000000000..0fa21a2013c --- /dev/null +++ b/changelogs/unreleased/43780-add-a-paragraph-about-clusters-security-implications.yml @@ -0,0 +1,5 @@ +--- +title: Add a paragraph about security implications on Cluster's page +merge_request: 17486 +author: +type: added diff --git a/changelogs/unreleased/bvl-allow-maintainer-to-push.yml b/changelogs/unreleased/bvl-allow-maintainer-to-push.yml new file mode 100644 index 00000000000..a3fefc2889a --- /dev/null +++ b/changelogs/unreleased/bvl-allow-maintainer-to-push.yml @@ -0,0 +1,5 @@ +--- +title: Allow maintainers to push to forks of their projects when a merge request is open +merge_request: 17395 +author: +type: added diff --git a/changelogs/unreleased/refactor-move-sidebar-assignee-vue-component.yml b/changelogs/unreleased/refactor-move-sidebar-assignee-vue-component.yml new file mode 100644 index 00000000000..e77b651363e --- /dev/null +++ b/changelogs/unreleased/refactor-move-sidebar-assignee-vue-component.yml @@ -0,0 +1,5 @@ +--- +title: Move SidebarAssignees vue component +merge_request: 17398 +author: George Tsiolis +type: performance diff --git a/changelogs/unreleased/upgrade-workhorse-4-0-0.yml b/changelogs/unreleased/upgrade-workhorse-4-0-0.yml new file mode 100644 index 00000000000..f9dbdc7fc56 --- /dev/null +++ b/changelogs/unreleased/upgrade-workhorse-4-0-0.yml @@ -0,0 +1,5 @@ +--- +title: Upgrade GitLab Workhorse to 4.0.0 +merge_request: +author: +type: added diff --git a/db/migrate/20180221151752_add_allow_maintainer_to_push_to_merge_requests.rb b/db/migrate/20180221151752_add_allow_maintainer_to_push_to_merge_requests.rb new file mode 100644 index 00000000000..81acfbc3655 --- /dev/null +++ b/db/migrate/20180221151752_add_allow_maintainer_to_push_to_merge_requests.rb @@ -0,0 +1,18 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddAllowMaintainerToPushToMergeRequests < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_column :merge_requests, :allow_maintainer_to_push, :boolean + end + + def down + remove_column :merge_requests, :allow_maintainer_to_push + end +end diff --git a/db/schema.rb b/db/schema.rb index 387b15f8f30..b93e6fa594b 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -1145,6 +1145,7 @@ ActiveRecord::Schema.define(version: 20180307012445) do t.boolean "discussion_locked" t.integer "latest_merge_request_diff_id" t.string "rebase_commit_sha" + t.boolean "allow_maintainer_to_push" end add_index "merge_requests", ["assignee_id"], name: "index_merge_requests_on_assignee_id", using: :btree diff --git a/doc/api/merge_requests.md b/doc/api/merge_requests.md index 25b0807eb18..b9a4f661777 100644 --- a/doc/api/merge_requests.md +++ b/doc/api/merge_requests.md @@ -529,18 +529,19 @@ Creates a new merge request. POST /projects/:id/merge_requests ``` -| Attribute | Type | Required | Description | -| --------- | ---- | -------- | ----------- | -| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user | -| `source_branch` | string | yes | The source branch | -| `target_branch` | string | yes | The target branch | -| `title` | string | yes | Title of MR | -| `assignee_id` | integer | no | Assignee user ID | -| `description` | string | no | Description of MR | -| `target_project_id` | integer | no | The target project (numeric id) | -| `labels` | string | no | Labels for MR as a comma-separated list | -| `milestone_id` | integer | no | The ID of a milestone | -| `remove_source_branch` | boolean | no | Flag indicating if a merge request should remove the source branch when merging | +| Attribute | Type | Required | Description | +| --------- | ---- | -------- | ----------- | +| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user | +| `source_branch` | string | yes | The source branch | +| `target_branch` | string | yes | The target branch | +| `title` | string | yes | Title of MR | +| `assignee_id` | integer | no | Assignee user ID | +| `description` | string | no | Description of MR | +| `target_project_id` | integer | no | The target project (numeric id) | +| `labels` | string | no | Labels for MR as a comma-separated list | +| `milestone_id` | integer | no | The ID of a milestone | +| `remove_source_branch` | boolean | no | Flag indicating if a merge request should remove the source branch when merging | +| `allow_maintainer_to_push` | boolean | no | Whether or not a maintainer of the target project can push to the source branch | ```json { @@ -548,7 +549,7 @@ POST /projects/:id/merge_requests "iid": 1, "target_branch": "master", "source_branch": "test1", - "project_id": 3, + "project_id": 4, "title": "test1", "state": "opened", "upvotes": 0, @@ -569,7 +570,7 @@ POST /projects/:id/merge_requests "state": "active", "created_at": "2012-04-29T08:46:00Z" }, - "source_project_id": 4, + "source_project_id": 3, "target_project_id": 4, "labels": [ ], "description": "fixed login page css paddings", @@ -596,6 +597,7 @@ POST /projects/:id/merge_requests "force_remove_source_branch": false, "web_url": "http://example.com/example/example/merge_requests/1", "discussion_locked": false, + "allow_maintainer_to_push": false, "time_stats": { "time_estimate": 0, "total_time_spent": 0, @@ -613,19 +615,20 @@ Updates an existing merge request. You can change the target branch, title, or e PUT /projects/:id/merge_requests/:merge_request_iid ``` -| Attribute | Type | Required | Description | -| --------- | ---- | -------- | ----------- | -| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user | -| `merge_request_iid` | integer | yes | The ID of a merge request | -| `target_branch` | string | no | The target branch | -| `title` | string | no | Title of MR | -| `assignee_id` | integer | no | The ID of the user to assign the merge request to. Set to `0` or provide an empty value to unassign all assignees. | -| `milestone_id` | integer | no | The ID of a milestone to assign the merge request to. Set to `0` or provide an empty value to unassign a milestone.| -| `labels` | string | no | Comma-separated label names for a merge request. Set to an empty string to unassign all labels. | -| `description` | string | no | Description of MR | -| `state_event` | string | no | New state (close/reopen) | -| `remove_source_branch` | boolean | no | Flag indicating if a merge request should remove the source branch when merging | -| `discussion_locked` | boolean | no | Flag indicating if the merge request's discussion is locked. If the discussion is locked only project members can add, edit or resolve comments. | +| Attribute | Type | Required | Description | +| --------- | ---- | -------- | ----------- | +| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user | +| `merge_request_iid` | integer | yes | The ID of a merge request | +| `target_branch` | string | no | The target branch | +| `title` | string | no | Title of MR | +| `assignee_id` | integer | no | The ID of the user to assign the merge request to. Set to `0` or provide an empty value to unassign all assignees. | +| `milestone_id` | integer | no | The ID of a milestone to assign the merge request to. Set to `0` or provide an empty value to unassign a milestone.| +| `labels` | string | no | Comma-separated label names for a merge request. Set to an empty string to unassign all labels. | +| `description` | string | no | Description of MR | +| `state_event` | string | no | New state (close/reopen) | +| `remove_source_branch` | boolean | no | Flag indicating if a merge request should remove the source branch when merging | +| `discussion_locked` | boolean | no | Flag indicating if the merge request's discussion is locked. If the discussion is locked only project members can add, edit or resolve comments. | +| `allow_maintainer_to_push` | boolean | no | Whether or not a maintainer of the target project can push to the source branch | Must include at least one non-required attribute from above. @@ -634,7 +637,7 @@ Must include at least one non-required attribute from above. "id": 1, "iid": 1, "target_branch": "master", - "project_id": 3, + "project_id": 4, "title": "test1", "state": "opened", "upvotes": 0, @@ -655,7 +658,7 @@ Must include at least one non-required attribute from above. "state": "active", "created_at": "2012-04-29T08:46:00Z" }, - "source_project_id": 4, + "source_project_id": 3, "target_project_id": 4, "labels": [ ], "description": "description1", @@ -682,6 +685,7 @@ Must include at least one non-required attribute from above. "force_remove_source_branch": false, "web_url": "http://example.com/example/example/merge_requests/1", "discussion_locked": false, + "allow_maintainer_to_push": false, "time_stats": { "time_estimate": 0, "total_time_spent": 0, diff --git a/doc/development/new_fe_guide/dependencies.md b/doc/development/new_fe_guide/dependencies.md new file mode 100644 index 00000000000..3417d77a06d --- /dev/null +++ b/doc/development/new_fe_guide/dependencies.md @@ -0,0 +1,3 @@ +# Dependencies + +> TODO: Add Dependencies
\ No newline at end of file diff --git a/doc/development/new_fe_guide/development/accessibility.md b/doc/development/new_fe_guide/development/accessibility.md new file mode 100644 index 00000000000..ed35f08432f --- /dev/null +++ b/doc/development/new_fe_guide/development/accessibility.md @@ -0,0 +1,3 @@ +# Accessibility + +> TODO: Add content diff --git a/doc/development/new_fe_guide/development/components.md b/doc/development/new_fe_guide/development/components.md new file mode 100644 index 00000000000..637099d1e83 --- /dev/null +++ b/doc/development/new_fe_guide/development/components.md @@ -0,0 +1,3 @@ +# Components + +> TODO: Add content diff --git a/doc/development/new_fe_guide/development/design_patterns.md b/doc/development/new_fe_guide/development/design_patterns.md new file mode 100644 index 00000000000..ee06566ed30 --- /dev/null +++ b/doc/development/new_fe_guide/development/design_patterns.md @@ -0,0 +1,3 @@ +# Design patterns + +> TODO: Add content diff --git a/doc/development/new_fe_guide/development/index.md b/doc/development/new_fe_guide/development/index.md new file mode 100644 index 00000000000..f8d64d9ff45 --- /dev/null +++ b/doc/development/new_fe_guide/development/index.md @@ -0,0 +1,29 @@ +# Development + +## [Design patterns](design_patterns.md) + +Examples of proven design patterns used in our codebase. + +## [Components](components.md) + +Documentation on existing components and how to best create a new component. + +## [Accessiblity](accessibility.md) + +Learn how to implement an accessible frontend. + +## [Network requests](network_requests.md) + +Learn how to handle network requests in our codebase. + +## [Security](security.md) + +Learn how to ensure that our frontend is secure. + +## [Performance](performance.md) + +Learn how to keep our frontend performant. + +## [Testing](testing.md) + +Learn how to keep our frontend tested. diff --git a/doc/development/new_fe_guide/development/network_requests.md b/doc/development/new_fe_guide/development/network_requests.md new file mode 100644 index 00000000000..047c00313bc --- /dev/null +++ b/doc/development/new_fe_guide/development/network_requests.md @@ -0,0 +1,3 @@ +# Network requests + +> TODO: Add content diff --git a/doc/development/new_fe_guide/development/performance.md b/doc/development/new_fe_guide/development/performance.md new file mode 100644 index 00000000000..26b07874f0f --- /dev/null +++ b/doc/development/new_fe_guide/development/performance.md @@ -0,0 +1,3 @@ +# Performance + +> TODO: Add content diff --git a/doc/development/new_fe_guide/development/security.md b/doc/development/new_fe_guide/development/security.md new file mode 100644 index 00000000000..debda7de0c6 --- /dev/null +++ b/doc/development/new_fe_guide/development/security.md @@ -0,0 +1,3 @@ +# Security + +> TODO: Add content diff --git a/doc/development/new_fe_guide/development/testing.md b/doc/development/new_fe_guide/development/testing.md new file mode 100644 index 00000000000..c359bd83ed1 --- /dev/null +++ b/doc/development/new_fe_guide/development/testing.md @@ -0,0 +1,3 @@ +# Testing + +> TODO: Add content diff --git a/doc/development/new_fe_guide/index.md b/doc/development/new_fe_guide/index.md new file mode 100644 index 00000000000..08c6a266e7f --- /dev/null +++ b/doc/development/new_fe_guide/index.md @@ -0,0 +1,28 @@ +# Frontend Development Guidelines + +This guide contains all the information to successfully contribute to GitLab's frontend. +This is a living document, and we welcome contributions, feedback and suggestions. + +## [Principles](principles.md) + +Ensure that your frontend contribution starts off in the right direction. + +## [Initiatives](initiatives.md) + +High level overview of where we are going from a frontend perspective. + +## [Development](development/index.md) + +Guidance on topics related to development. + +## [Dependencies](dependencies.md) + +Learn about all the dependencies that make up our frontend, including some of our own custom built libraries. + +## [Style](style/index.md) + +Style guides to keep our code consistent. + +## [Tips](tips.md) + +Tips from our frontend team to develop more efficiently and effectively. diff --git a/doc/development/new_fe_guide/initiatives.md b/doc/development/new_fe_guide/initiatives.md new file mode 100644 index 00000000000..c81ed3579f0 --- /dev/null +++ b/doc/development/new_fe_guide/initiatives.md @@ -0,0 +1,3 @@ +# Initiatives + +> TODO: Add Initiatives diff --git a/doc/development/new_fe_guide/principles.md b/doc/development/new_fe_guide/principles.md new file mode 100644 index 00000000000..2126d202a7e --- /dev/null +++ b/doc/development/new_fe_guide/principles.md @@ -0,0 +1,3 @@ +# Principles + +> TODO: Add principles diff --git a/doc/development/new_fe_guide/style/html.md b/doc/development/new_fe_guide/style/html.md new file mode 100644 index 00000000000..5489def5d6e --- /dev/null +++ b/doc/development/new_fe_guide/style/html.md @@ -0,0 +1,3 @@ +# HTML style guide + +> TODO: Add content diff --git a/doc/development/new_fe_guide/style/index.md b/doc/development/new_fe_guide/style/index.md new file mode 100644 index 00000000000..d2d576b3b46 --- /dev/null +++ b/doc/development/new_fe_guide/style/index.md @@ -0,0 +1,9 @@ +# Style + +## [HTML style guide](html.md) + +## [SCSS style guide](scss.md) + +## [JavaScript style guide](javascript.md) + +## [Vue style guide](vue.md) diff --git a/doc/development/new_fe_guide/style/javascript.md b/doc/development/new_fe_guide/style/javascript.md new file mode 100644 index 00000000000..480d50a211f --- /dev/null +++ b/doc/development/new_fe_guide/style/javascript.md @@ -0,0 +1,3 @@ +# JavaScript style guide + +> TODO: Add content diff --git a/doc/development/new_fe_guide/style/scss.md b/doc/development/new_fe_guide/style/scss.md new file mode 100644 index 00000000000..6f5e818d7db --- /dev/null +++ b/doc/development/new_fe_guide/style/scss.md @@ -0,0 +1,3 @@ +# SCSS style guide + +> TODO: Add content diff --git a/doc/development/new_fe_guide/style/vue.md b/doc/development/new_fe_guide/style/vue.md new file mode 100644 index 00000000000..fd9353e0d3f --- /dev/null +++ b/doc/development/new_fe_guide/style/vue.md @@ -0,0 +1,3 @@ +# Vue style guide + +> TODO: Add content diff --git a/doc/development/new_fe_guide/tips.md b/doc/development/new_fe_guide/tips.md new file mode 100644 index 00000000000..f0cdf52d618 --- /dev/null +++ b/doc/development/new_fe_guide/tips.md @@ -0,0 +1,3 @@ +# Tips + +> TODO: Add tips diff --git a/doc/user/project/clusters/index.md b/doc/user/project/clusters/index.md index 4ac54f96aa2..661697aaeb7 100644 --- a/doc/user/project/clusters/index.md +++ b/doc/user/project/clusters/index.md @@ -109,6 +109,41 @@ you will be notified. You can now proceed to install some pre-defined applications and then enable the Kubernetes cluster integration. +## Security implications + +CAUTION: **Important:** +The whole cluster security is based on a model where [developers](../../permissions.md) +are trusted, so **only trusted users should be allowed to control your clusters**. + +The default cluster configuration grants access to a wide set of +functionalities needed to successfully build and deploy a containerized +application. Bare in mind that the same credentials are used for all the +applications running on the cluster. + +When GitLab creates the cluster, it enables and uses the legacy +[Attribute-based access control (ABAC)](https://kubernetes.io/docs/admin/authorization/abac/). +The newer [RBAC](https://kubernetes.io/docs/admin/authorization/rbac/) +authorization will be supported in a +[future release](https://gitlab.com/gitlab-org/gitlab-ce/issues/29398). + +### Security of GitLab Runners + +GitLab Runners have the [privileged mode](https://docs.gitlab.com/runner/executors/docker.html#the-privileged-mode) +enabled by default, which allows them to execute special commands and running +Docker in Docker. This functionality is needed to run some of the [Auto DevOps] +jobs. This implies the containers are running in privileged mode and you should, +therefore, be aware of some important details. + +The privileged flag gives all capabilities to the running container, which in +turn can do almost everything that the host can do. Be aware of the +inherent security risk associated with performing `docker run` operations on +arbitrary images as they effectively have root access. + +If you don't want to use GitLab Runner in privileged mode, first make sure that +you don't have it installed via the applications, and then use the +[Runner's Helm chart](../../../install/kubernetes/gitlab_runner_chart.md) to +install it manually. + ## Installing applications GitLab provides a one-click install for various applications which will be @@ -118,16 +153,16 @@ added directly to your configured cluster. Those applications are needed for | Application | GitLab version | Description | | ----------- | :------------: | ----------- | | [Helm Tiller](https://docs.helm.sh/) | 10.2+ | Helm is a package manager for Kubernetes and is required to install all the other applications. It will be automatically installed as a dependency when you try to install a different app. It is installed in its own pod inside the cluster which can run the `helm` CLI in a safe environment. | -| [Ingress](https://kubernetes.io/docs/concepts/services-networking/ingress/) | 10.2+ | Ingress can provide load balancing, SSL termination, and name-based virtual hosting. It acts as a web proxy for your applications and is useful if you want to use [Auto DevOps](../../../topics/autodevops/index.md) or deploy your own web apps. | +| [Ingress](https://kubernetes.io/docs/concepts/services-networking/ingress/) | 10.2+ | Ingress can provide load balancing, SSL termination, and name-based virtual hosting. It acts as a web proxy for your applications and is useful if you want to use [Auto DevOps] or deploy your own web apps. | | [Prometheus](https://prometheus.io/docs/introduction/overview/) | 10.4+ | Prometheus is an open-source monitoring and alerting system useful to supervise your deployed applications | -| [GitLab Runner](https://docs.gitlab.com/runner/) | 10.6+ | GitLab Runner is the open source project that is used to run your jobs and send the results back to GitLab. It is used in conjunction with [GitLab CI](https://about.gitlab.com/features/gitlab-ci-cd/), the open-source continuous integration service included with GitLab that coordinates the jobs. | +| [GitLab Runner](https://docs.gitlab.com/runner/) | 10.6+ | GitLab Runner is the open source project that is used to run your jobs and send the results back to GitLab. It is used in conjunction with [GitLab CI/CD](https://about.gitlab.com/features/gitlab-ci-cd/), the open-source continuous integration service included with GitLab that coordinates the jobs. When installing the GitLab Runner via the applications, it will run in **privileged mode** by default. Make sure you read the [security implications](#security-implications) before doing so. | ## Getting the external IP address NOTE: **Note:** You need a load balancer installed in your cluster in order to obtain the external IP address with the following procedure. It can be deployed using the -[**Ingress** application](#installing-appplications). +[**Ingress** application](#installing-applications). In order to publish your web application, you first need to find the external IP address associated to your load balancer. @@ -329,3 +364,4 @@ the deployment variables above, ensuring any pods you create are labelled with [permissions]: ../../permissions.md [ee]: https://about.gitlab.com/products/ +[Auto DevOps]: ../../../topics/autodevops/index.md diff --git a/doc/user/project/merge_requests/img/allow_maintainer_push.png b/doc/user/project/merge_requests/img/allow_maintainer_push.png Binary files differnew file mode 100644 index 00000000000..e73cc1ab5a5 --- /dev/null +++ b/doc/user/project/merge_requests/img/allow_maintainer_push.png diff --git a/doc/user/project/merge_requests/index.md b/doc/user/project/merge_requests/index.md index d3220598933..10d67729734 100644 --- a/doc/user/project/merge_requests/index.md +++ b/doc/user/project/merge_requests/index.md @@ -28,6 +28,7 @@ With GitLab merge requests, you can: - Enable [fast-forward merge requests](#fast-forward-merge-requests) - Enable [semi-linear history merge requests](#semi-linear-history-merge-requests) as another security layer to guarantee the pipeline is passing in the target branch - [Create new merge requests by email](#create-new-merge-requests-by-email) +- Allow maintainers of the target project to push directly to the fork by [allowing edits from maintainers](maintainer_access.md) With **[GitLab Enterprise Edition][ee]**, you can also: diff --git a/doc/user/project/merge_requests/maintainer_access.md b/doc/user/project/merge_requests/maintainer_access.md new file mode 100644 index 00000000000..1ade5db1587 --- /dev/null +++ b/doc/user/project/merge_requests/maintainer_access.md @@ -0,0 +1,13 @@ +# Allow maintainer pushes for merge requests accross forks + +This feature is available for merge requests across forked projects that are +publicly accessible. It makes it easier for maintainers of projects to collaborate +on merge requests across forks. + +When enabling this feature for a merge request, you give can give members with push access to the target project rights to edit files on the source branch of the merge request. + +The feature can only be enabled by users who already have push access to the source project. And only lasts while the merge request is open. + +Enable this functionality while creating a merge request: + + diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 4555184095c..16147ee90c9 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -547,6 +547,7 @@ module API expose :discussion_locked expose :should_remove_source_branch?, as: :should_remove_source_branch expose :force_remove_source_branch?, as: :force_remove_source_branch + expose :allow_maintainer_to_push, if: -> (merge_request, _) { merge_request.for_fork? } expose :web_url do |merge_request, options| Gitlab::UrlBuilder.build(merge_request) diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb index ead1bb7957b..3264a26f7d2 100644 --- a/lib/api/merge_requests.rb +++ b/lib/api/merge_requests.rb @@ -144,6 +144,7 @@ module API optional :milestone_id, type: Integer, desc: 'The ID of a milestone to assign the merge request' optional :labels, type: String, desc: 'Comma-separated list of label names' optional :remove_source_branch, type: Boolean, desc: 'Remove source branch when merging' + optional :allow_maintainer_to_push, type: Boolean, desc: 'Whether a maintainer of the target project can push to the source project' use :optional_params_ee end diff --git a/lib/gitlab/checks/change_access.rb b/lib/gitlab/checks/change_access.rb index 3ce5f807989..51ba09aa129 100644 --- a/lib/gitlab/checks/change_access.rb +++ b/lib/gitlab/checks/change_access.rb @@ -47,7 +47,7 @@ module Gitlab protected def push_checks - if user_access.cannot_do_action?(:push_code) + unless can_push? raise GitAccess::UnauthorizedError, ERROR_MESSAGES[:push_code] end end @@ -183,6 +183,11 @@ module Gitlab def commits @commits ||= project.repository.new_commits(newrev) end + + def can_push? + user_access.can_do_action?(:push_code) || + user_access.can_push_to_branch?(branch_name) + end end end end diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 1445b7924dc..d4f6b543daf 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -1443,12 +1443,12 @@ module Gitlab end end - def last_commit_id_for_path(sha, path) + def last_commit_for_path(sha, path) gitaly_migrate(:last_commit_for_path) do |is_enabled| if is_enabled - last_commit_for_path_by_gitaly(sha, path).id + last_commit_for_path_by_gitaly(sha, path) else - last_commit_id_for_path_by_shelling_out(sha, path) + last_commit_for_path_by_rugged(sha, path) end end end @@ -1896,7 +1896,7 @@ module Gitlab end def last_commit_for_path_by_rugged(sha, path) - sha = last_commit_id_for_path(sha, path) + sha = last_commit_id_for_path_by_shelling_out(sha, path) commit(sha) end diff --git a/lib/gitlab/user_access.rb b/lib/gitlab/user_access.rb index 91b8bb2a83f..fd68b9f2b48 100644 --- a/lib/gitlab/user_access.rb +++ b/lib/gitlab/user_access.rb @@ -68,8 +68,10 @@ module Gitlab return true if project.user_can_push_to_empty_repo?(user) protected_branch_accessible_to?(ref, action: :push) + elsif user.can?(:push_code, project) + true else - user.can?(:push_code, project) + project.branch_allows_maintainer_push?(user, ref) end end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 8a2176a4d72..bb7aaa2fb2f 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -8,8 +8,8 @@ msgid "" msgstr "" "Project-Id-Version: gitlab 1.0.0\n" "Report-Msgid-Bugs-To: \n" -"POT-Creation-Date: 2018-03-05 13:02-0600\n" -"PO-Revision-Date: 2018-03-05 13:02-0600\n" +"POT-Creation-Date: 2018-03-06 17:36+0100\n" +"PO-Revision-Date: 2018-03-06 17:36+0100\n" "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n" "Language-Team: LANGUAGE <LL@li.org>\n" "Language: \n" @@ -102,6 +102,9 @@ msgstr "" msgid "A collection of graphs regarding Continuous Integration" msgstr "" +msgid "A new branch will be created in your fork and a new merge request will be started." +msgstr "" + msgid "A project is where you house your files (repository), plan your work (issues), and publish your documentation (wiki), %{among_other_things_link}." msgstr "" @@ -207,6 +210,9 @@ msgstr "" msgid "All features are enabled for blank projects, from templates, or when importing, but you can disable them afterward in the project settings." msgstr "" +msgid "Allow edits from maintainers" +msgstr "" + msgid "Allows you to add and manage Kubernetes clusters." msgstr "" @@ -288,9 +294,6 @@ msgstr "" msgid "Are you sure you want to delete this pipeline schedule?" msgstr "" -msgid "Are you sure you want to discard your changes?" -msgstr "" - msgid "Are you sure you want to reset registration token?" msgstr "" @@ -392,9 +395,6 @@ msgstr[1] "" msgid "Branch <strong>%{branch_name}</strong> was created. To set up auto deploy, choose a GitLab CI Yaml template and commit your changes. %{link_to_autodeploy_doc}" msgstr "" -msgid "Branch has changed" -msgstr "" - msgid "Branch is already taken" msgstr "" @@ -410,6 +410,15 @@ msgstr "" msgid "Branches" msgstr "" +msgid "Branches|Active" +msgstr "" + +msgid "Branches|Active branches" +msgstr "" + +msgid "Branches|All" +msgstr "" + msgid "Branches|Cant find HEAD commit for this branch" msgstr "" @@ -455,12 +464,39 @@ msgstr "" msgid "Branches|Only a project master or owner can delete a protected branch" msgstr "" -msgid "Branches|Protected branches can be managed in %{project_settings_link}" +msgid "Branches|Overview" +msgstr "" + +msgid "Branches|Protected branches can be managed in %{project_settings_link}." +msgstr "" + +msgid "Branches|Show active branches" +msgstr "" + +msgid "Branches|Show all branches" +msgstr "" + +msgid "Branches|Show more active branches" +msgstr "" + +msgid "Branches|Show more stale branches" +msgstr "" + +msgid "Branches|Show overview of the branches" +msgstr "" + +msgid "Branches|Show stale branches" msgstr "" msgid "Branches|Sort by" msgstr "" +msgid "Branches|Stale" +msgstr "" + +msgid "Branches|Stale branches" +msgstr "" + msgid "Branches|The default branch cannot be deleted" msgstr "" @@ -512,9 +548,6 @@ msgstr "" msgid "Cancel" msgstr "" -msgid "Cancel edit" -msgstr "" - msgid "Cannot modify managed Kubernetes cluster" msgstr "" @@ -662,6 +695,9 @@ msgstr "" msgid "Clone repository" msgstr "" +msgid "Close" +msgstr "" + msgid "ClusterIntegration|%{appList} was successfully installed on your Kubernetes cluster" msgstr "" @@ -1077,6 +1113,9 @@ msgstr "" msgid "ContainerRegistry|With the Docker Container Registry integrated into GitLab, every project can have its own space to store its Docker images." msgstr "" +msgid "Contribution" +msgstr "" + msgid "Contribution guide" msgstr "" @@ -1128,9 +1167,6 @@ msgstr "" msgid "Create empty bare repository" msgstr "" -msgid "Create file" -msgstr "" - msgid "Create lists from labels. Issues with that label appear in that list." msgstr "" @@ -1140,15 +1176,6 @@ msgstr "" msgid "Create merge request and branch" msgstr "" -msgid "Create new branch" -msgstr "" - -msgid "Create new directory" -msgstr "" - -msgid "Create new file" -msgstr "" - msgid "Create new label" msgstr "" @@ -1238,9 +1265,6 @@ msgstr "" msgid "Directory name" msgstr "" -msgid "Discard changes" -msgstr "" - msgid "Dismiss Cycle Analytics introduction box" msgstr "" @@ -1421,9 +1445,6 @@ msgstr "" msgid "Fields on this page are now uneditable, you can configure" msgstr "" -msgid "File name" -msgstr "" - msgid "Files" msgstr "" @@ -1439,6 +1460,9 @@ msgstr "" msgid "Find file" msgstr "" +msgid "Finished" +msgstr "" + msgid "FirstPushedBy|First" msgstr "" @@ -1495,6 +1519,9 @@ msgstr "" msgid "Gitaly Servers" msgstr "" +msgid "Go back" +msgstr "" + msgid "Go to your fork" msgstr "" @@ -1701,6 +1728,15 @@ msgstr "" msgid "LFSStatus|Enabled" msgstr "" +msgid "Label" +msgstr "" + +msgid "LabelSelect|%{firstLabelName} +%{remainingLabelCount} more" +msgstr "" + +msgid "LabelSelect|%{labelsString}, and %{remainingLabelCount} more" +msgstr "" + msgid "Labels" msgstr "" @@ -1760,9 +1796,6 @@ msgstr "" msgid "Leave project" msgstr "" -msgid "Loading the GitLab IDE..." -msgstr "" - msgid "Lock" msgstr "" @@ -1945,6 +1978,12 @@ msgstr "" msgid "Not available" msgstr "" +msgid "Not available for private projects" +msgstr "" + +msgid "Not available for protected branches" +msgstr "" + msgid "Not confidential" msgstr "" @@ -2071,6 +2110,9 @@ msgstr "" msgid "Password" msgstr "" +msgid "Pending" +msgstr "" + msgid "Pipeline" msgstr "" @@ -2149,9 +2191,30 @@ msgstr "" msgid "Pipelines|Build with confidence" msgstr "" +msgid "Pipelines|CI Lint" +msgstr "" + +msgid "Pipelines|Clear Runner Caches" +msgstr "" + msgid "Pipelines|Get started with Pipelines" msgstr "" +msgid "Pipelines|Loading Pipelines" +msgstr "" + +msgid "Pipelines|Run Pipeline" +msgstr "" + +msgid "Pipelines|There are currently no %{scope} pipelines." +msgstr "" + +msgid "Pipelines|There are currently no pipelines." +msgstr "" + +msgid "Pipelines|This project is not currently set up to run pipelines." +msgstr "" + msgid "Pipeline|Retry pipeline" msgstr "" @@ -2404,6 +2467,9 @@ msgstr "" msgid "Public - The project can be accessed without any authentication." msgstr "" +msgid "Push access to this project is necessary in order to enable this option" +msgstr "" + msgid "Push events" msgstr "" @@ -2490,6 +2556,9 @@ msgstr "" msgid "Revert this merge request" msgstr "" +msgid "Running" +msgstr "" + msgid "SSH Keys" msgstr "" @@ -2511,6 +2580,9 @@ msgstr "" msgid "Scheduling Pipelines" msgstr "" +msgid "Search" +msgstr "" + msgid "Search branches and tags" msgstr "" @@ -3278,9 +3350,6 @@ msgstr "" msgid "We want to be sure it is you, please confirm you are not a robot." msgstr "" -msgid "Web IDE" -msgstr "" - msgid "Wiki" msgstr "" @@ -3395,13 +3464,13 @@ msgstr "" msgid "You are going to remove %{group_name}. Removed groups CANNOT be restored! Are you ABSOLUTELY sure?" msgstr "" -msgid "You are going to remove %{project_name_with_namespace}. Removed project CANNOT be restored! Are you ABSOLUTELY sure?" +msgid "You are going to remove %{project_full_name}. Removed project CANNOT be restored! Are you ABSOLUTELY sure?" msgstr "" msgid "You are going to remove the fork relationship to source project %{forked_from_project}. Are you ABSOLUTELY sure?" msgstr "" -msgid "You are going to transfer %{project_name_with_namespace} to another owner. Are you ABSOLUTELY sure?" +msgid "You are going to transfer %{project_full_name} to another owner. Are you ABSOLUTELY sure?" msgstr "" msgid "You are on a read-only GitLab instance." @@ -3467,6 +3536,9 @@ msgstr "" msgid "Your Kubernetes cluster information on this page is still editable, but you are advised to disable and reconfigure" msgstr "" +msgid "Your changes can be committed to %{branch_name} because a merge request is open." +msgstr "" + msgid "Your comment will not be visible to the public." msgstr "" @@ -3513,6 +3585,9 @@ msgstr[1] "" msgid "mrWidget| Please restore it or use a different %{missingBranchName} branch" msgstr "" +msgid "mrWidget|Allows edits from maintainers" +msgstr "" + msgid "mrWidget|Cancel automatic merge" msgstr "" diff --git a/spec/features/merge_request/maintainer_edits_fork_spec.rb b/spec/features/merge_request/maintainer_edits_fork_spec.rb new file mode 100644 index 00000000000..a3323da1b1f --- /dev/null +++ b/spec/features/merge_request/maintainer_edits_fork_spec.rb @@ -0,0 +1,44 @@ +require 'spec_helper' + +describe 'a maintainer edits files on a source-branch of an MR from a fork', :js do + include ProjectForksHelper + let(:user) { create(:user, username: 'the-maintainer') } + let(:target_project) { create(:project, :public, :repository) } + let(:author) { create(:user, username: 'mr-authoring-machine') } + let(:source_project) { fork_project(target_project, author, repository: true) } + + let(:merge_request) do + create(:merge_request, + source_project: source_project, + target_project: target_project, + source_branch: 'fix', + target_branch: 'master', + author: author, + allow_maintainer_to_push: true) + end + + before do + target_project.add_master(user) + sign_in(user) + + visit project_merge_request_path(target_project, merge_request) + click_link 'Changes' + wait_for_requests + first('.js-file-title').click_link 'Edit' + wait_for_requests + end + + it 'mentions commits will go to the source branch' do + expect(page).to have_content('Your changes can be committed to fix because a merge request is open.') + end + + it 'allows committing to the source branch' do + find('.ace_text-input', visible: false).send_keys('Updated the readme') + + click_button 'Commit changes' + wait_for_requests + + expect(page).to have_content('Your changes have been successfully committed') + expect(page).to have_content('Updated the readme') + end +end diff --git a/spec/features/merge_request/user_allows_a_maintainer_to_push_spec.rb b/spec/features/merge_request/user_allows_a_maintainer_to_push_spec.rb new file mode 100644 index 00000000000..eb41d7de8ed --- /dev/null +++ b/spec/features/merge_request/user_allows_a_maintainer_to_push_spec.rb @@ -0,0 +1,85 @@ +require 'spec_helper' + +describe 'create a merge request that allows maintainers to push', :js do + include ProjectForksHelper + let(:user) { create(:user) } + let(:target_project) { create(:project, :public, :repository) } + let(:source_project) { fork_project(target_project, user, repository: true, namespace: user.namespace) } + + def visit_new_merge_request + visit project_new_merge_request_path( + source_project, + merge_request: { + source_project_id: source_project.id, + target_project_id: target_project.id, + source_branch: 'fix', + target_branch: 'master' + }) + end + + before do + sign_in(user) + end + + it 'allows setting maintainer push possible' do + visit_new_merge_request + + check 'Allow edits from maintainers' + + click_button 'Submit merge request' + + wait_for_requests + + expect(page).to have_content('Allows edits from maintainers') + end + + it 'shows a message when one of the projects is private' do + source_project.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE) + + visit_new_merge_request + + expect(page).to have_content('Not available for private projects') + end + + it 'shows a message when the source branch is protected' do + create(:protected_branch, project: source_project, name: 'fix') + + visit_new_merge_request + + expect(page).to have_content('Not available for protected branches') + end + + context 'when the merge request is being created within the same project' do + let(:source_project) { target_project } + + it 'hides the checkbox if the merge request is being created within the same project' do + target_project.add_developer(user) + + visit_new_merge_request + + expect(page).not_to have_content('Allows edits from maintainers') + end + end + + context 'when a maintainer tries to edit the option' do + let(:maintainer) { create(:user) } + let(:merge_request) do + create(:merge_request, + source_project: source_project, + target_project: target_project, + source_branch: 'fixes') + end + + before do + target_project.add_master(maintainer) + + sign_in(maintainer) + end + + it 'it hides the option from maintainers' do + visit edit_project_merge_request_path(target_project, merge_request) + + expect(page).not_to have_content('Allows edits from maintainers') + end + end +end diff --git a/spec/features/projects/user_creates_files_spec.rb b/spec/features/projects/user_creates_files_spec.rb index 7a935dd2477..8993533676b 100644 --- a/spec/features/projects/user_creates_files_spec.rb +++ b/spec/features/projects/user_creates_files_spec.rb @@ -133,13 +133,20 @@ describe 'User creates files' do before do project2.add_reporter(user) visit(project2_tree_path_root_ref) - end - it 'creates and commit new file in forked project', :js do find('.add-to-tree').click click_link('New file') + end + + it 'shows a message saying the file will be committed in a fork' do + message = "A new branch will be created in your fork and a new merge request will be started." + expect(page).to have_content(message) + end + + it 'creates and commit new file in forked project', :js do expect(page).to have_selector('.file-editor') + expect(page).to have_content find('#editor') execute_script("ace.edit('editor').setValue('*.rbca')") diff --git a/spec/fixtures/api/schemas/entities/merge_request_basic.json b/spec/fixtures/api/schemas/entities/merge_request_basic.json index f1199468d53..46031961cca 100644 --- a/spec/fixtures/api/schemas/entities/merge_request_basic.json +++ b/spec/fixtures/api/schemas/entities/merge_request_basic.json @@ -12,7 +12,8 @@ "rebase_in_progress": { "type": "boolean" }, "assignee_id": { "type": ["integer", "null"] }, "subscribed": { "type": ["boolean", "null"] }, - "participants": { "type": "array" } + "participants": { "type": "array" }, + "allow_maintainer_to_push": { "type": "boolean"} }, "additionalProperties": false } diff --git a/spec/fixtures/api/schemas/entities/merge_request_widget.json b/spec/fixtures/api/schemas/entities/merge_request_widget.json index cfbeec58a45..a622bf88b13 100644 --- a/spec/fixtures/api/schemas/entities/merge_request_widget.json +++ b/spec/fixtures/api/schemas/entities/merge_request_widget.json @@ -30,6 +30,7 @@ "source_project_id": { "type": "integer" }, "target_branch": { "type": "string" }, "target_project_id": { "type": "integer" }, + "allow_maintainer_to_push": { "type": "boolean"}, "metrics": { "oneOf": [ { "type": "null" }, diff --git a/spec/fixtures/api/schemas/public_api/v4/merge_requests.json b/spec/fixtures/api/schemas/public_api/v4/merge_requests.json index e86176e5316..0dc2eabec5d 100644 --- a/spec/fixtures/api/schemas/public_api/v4/merge_requests.json +++ b/spec/fixtures/api/schemas/public_api/v4/merge_requests.json @@ -80,7 +80,8 @@ "total_time_spent": { "type": "integer" }, "human_time_estimate": { "type": ["string", "null"] }, "human_total_time_spent": { "type": ["string", "null"] } - } + }, + "allow_maintainer_to_push": { "type": ["boolean", "null"] } }, "required": [ "id", "iid", "project_id", "title", "description", diff --git a/spec/helpers/tree_helper_spec.rb b/spec/helpers/tree_helper_spec.rb index d3b1be599dd..ccac6e29447 100644 --- a/spec/helpers/tree_helper_spec.rb +++ b/spec/helpers/tree_helper_spec.rb @@ -62,4 +62,13 @@ describe TreeHelper do end end end + + describe '#commit_in_single_accessible_branch' do + it 'escapes HTML from the branch name' do + helper.instance_variable_set(:@branch_name, "<script>alert('escape me!');</script>") + escaped_branch_name = '<script>alert('escape me!');</script>' + + expect(helper.commit_in_single_accessible_branch).to include(escaped_branch_name) + end + end end diff --git a/spec/javascripts/sidebar/sidebar_assignees_spec.js b/spec/javascripts/sidebar/sidebar_assignees_spec.js index 2fbb7268e0b..ebaaa6e806b 100644 --- a/spec/javascripts/sidebar/sidebar_assignees_spec.js +++ b/spec/javascripts/sidebar/sidebar_assignees_spec.js @@ -1,6 +1,6 @@ import _ from 'underscore'; import Vue from 'vue'; -import SidebarAssignees from '~/sidebar/components/assignees/sidebar_assignees'; +import SidebarAssignees from '~/sidebar/components/assignees/sidebar_assignees.vue'; import SidebarMediator from '~/sidebar/sidebar_mediator'; import SidebarService from '~/sidebar/services/sidebar_service'; import SidebarStore from '~/sidebar/stores/sidebar_store'; diff --git a/spec/javascripts/vue_mr_widget/components/mr_widget_maintainer_edit_spec.js b/spec/javascripts/vue_mr_widget/components/mr_widget_maintainer_edit_spec.js new file mode 100644 index 00000000000..65b3f721281 --- /dev/null +++ b/spec/javascripts/vue_mr_widget/components/mr_widget_maintainer_edit_spec.js @@ -0,0 +1,23 @@ +import Vue from 'vue'; +import maintainerEditComponent from '~/vue_merge_request_widget/components/mr_widget_maintainer_edit.vue'; +import mountComponent from 'spec/helpers/vue_mount_component_helper'; + +describe('MRWidgetAuthor', () => { + let vm; + + beforeEach(() => { + const Component = Vue.extend(maintainerEditComponent); + + vm = mountComponent(Component, { + maintainerEditAllowed: true, + }); + }); + + afterEach(() => { + vm.$destroy(); + }); + + it('renders the message when maintainers are allowed to edit', () => { + expect(vm.$el.textContent.trim()).toEqual('Allows edits from maintainers'); + }); +}); diff --git a/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js b/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js index 18ba34b55a5..ebe151ac3b1 100644 --- a/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js +++ b/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js @@ -349,6 +349,7 @@ describe('mrWidgetOptions', () => { expect(comps['mr-widget-pipeline-blocked']).toBeDefined(); expect(comps['mr-widget-pipeline-failed']).toBeDefined(); expect(comps['mr-widget-merge-when-pipeline-succeeds']).toBeDefined(); + expect(comps['mr-widget-maintainer-edit']).toBeDefined(); }); }); diff --git a/spec/lib/gitlab/checks/change_access_spec.rb b/spec/lib/gitlab/checks/change_access_spec.rb index b49ddbfc780..48e9902027c 100644 --- a/spec/lib/gitlab/checks/change_access_spec.rb +++ b/spec/lib/gitlab/checks/change_access_spec.rb @@ -30,9 +30,10 @@ describe Gitlab::Checks::ChangeAccess do end end - context 'when the user is not allowed to push code' do + context 'when the user is not allowed to push to the repo' do it 'raises an error' do expect(user_access).to receive(:can_do_action?).with(:push_code).and_return(false) + expect(user_access).to receive(:can_push_to_branch?).with('master').and_return(false) expect { subject.exec }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You are not allowed to push code to this project.') end diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index b20cc34dd5c..bece82e531a 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -278,6 +278,7 @@ project: - custom_attributes - lfs_file_locks - project_badges +- source_of_merge_requests award_emoji: - awardable - user diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml index ddcbb7a0033..0b938892da5 100644 --- a/spec/lib/gitlab/import_export/safe_model_attributes.yml +++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml @@ -168,6 +168,7 @@ MergeRequest: - last_edited_by_id - head_pipeline_id - discussion_locked +- allow_maintainer_to_push MergeRequestDiff: - id - state diff --git a/spec/lib/gitlab/user_access_spec.rb b/spec/lib/gitlab/user_access_spec.rb index 7280acb6c82..40c8286b1b9 100644 --- a/spec/lib/gitlab/user_access_spec.rb +++ b/spec/lib/gitlab/user_access_spec.rb @@ -1,6 +1,8 @@ require 'spec_helper' describe Gitlab::UserAccess do + include ProjectForksHelper + let(:access) { described_class.new(user, project: project) } let(:project) { create(:project, :repository) } let(:user) { create(:user) } @@ -118,6 +120,39 @@ describe Gitlab::UserAccess do end end + describe 'allowing pushes to maintainers of forked projects' do + let(:canonical_project) { create(:project, :public, :repository) } + let(:project) { fork_project(canonical_project, create(:user), repository: true) } + + before do + create( + :merge_request, + target_project: canonical_project, + source_project: project, + source_branch: 'awesome-feature', + allow_maintainer_to_push: true + ) + end + + it 'allows users that have push access to the canonical project to push to the MR branch' do + canonical_project.add_developer(user) + + expect(access.can_push_to_branch?('awesome-feature')).to be_truthy + end + + it 'does not allow the user to push to other branches' do + canonical_project.add_developer(user) + + expect(access.can_push_to_branch?('master')).to be_falsey + end + + it 'does not allow the user to push if he does not have push access to the canonical project' do + canonical_project.add_guest(user) + + expect(access.can_push_to_branch?('awesome-feature')).to be_falsey + end + end + describe 'merge to protected branch if allowed for developers' do before do @branch = create :protected_branch, :developers_can_merge, project: project diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 243eeddc7a8..7986aa31e16 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -2084,4 +2084,82 @@ describe MergeRequest do it_behaves_like 'checking whether a rebase is in progress' end end + + describe '#allow_maintainer_to_push' do + let(:merge_request) do + build(:merge_request, source_branch: 'fixes', allow_maintainer_to_push: true) + end + + it 'is false when pushing by a maintainer is not possible' do + expect(merge_request).to receive(:maintainer_push_possible?) { false } + + expect(merge_request.allow_maintainer_to_push).to be_falsy + end + + it 'is true when pushing by a maintainer is possible' do + expect(merge_request).to receive(:maintainer_push_possible?) { true } + + expect(merge_request.allow_maintainer_to_push).to be_truthy + end + end + + describe '#maintainer_push_possible?' do + let(:merge_request) do + build(:merge_request, source_branch: 'fixes') + end + + before do + allow(ProtectedBranch).to receive(:protected?) { false } + end + + it 'does not allow maintainer to push if the source project is the same as the target' do + merge_request.target_project = merge_request.source_project = create(:project, :public) + + expect(merge_request.maintainer_push_possible?).to be_falsy + end + + it 'allows maintainer to push when both source and target are public' do + merge_request.target_project = build(:project, :public) + merge_request.source_project = build(:project, :public) + + expect(merge_request.maintainer_push_possible?).to be_truthy + end + + it 'is not available for protected branches' do + merge_request.target_project = build(:project, :public) + merge_request.source_project = build(:project, :public) + + expect(ProtectedBranch).to receive(:protected?) + .with(merge_request.source_project, 'fixes') + .and_return(true) + + expect(merge_request.maintainer_push_possible?).to be_falsy + end + end + + describe '#can_allow_maintainer_to_push?' do + let(:target_project) { create(:project, :public) } + let(:source_project) { fork_project(target_project) } + let(:merge_request) do + create(:merge_request, + source_project: source_project, + source_branch: 'fixes', + target_project: target_project) + end + let(:user) { create(:user) } + + before do + allow(merge_request).to receive(:maintainer_push_possible?) { true } + end + + it 'is false if the user does not have push access to the source project' do + expect(merge_request.can_allow_maintainer_to_push?(user)).to be_falsy + end + + it 'is true when the user has push access to the source project' do + source_project.add_developer(user) + + expect(merge_request.can_allow_maintainer_to_push?(user)).to be_truthy + end + end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index b1c9e6754b9..e970cd7dfdb 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -1,6 +1,8 @@ require 'spec_helper' describe Project do + include ProjectForksHelper + describe 'associations' do it { is_expected.to belong_to(:group) } it { is_expected.to belong_to(:namespace) } @@ -3378,4 +3380,103 @@ describe Project do end end end + + context 'with cross project merge requests' do + let(:user) { create(:user) } + let(:target_project) { create(:project, :repository) } + let(:project) { fork_project(target_project, nil, repository: true) } + let!(:merge_request) do + create( + :merge_request, + target_project: target_project, + target_branch: 'target-branch', + source_project: project, + source_branch: 'awesome-feature-1', + allow_maintainer_to_push: true + ) + end + + before do + target_project.add_developer(user) + end + + describe '#merge_requests_allowing_push_to_user' do + it 'returns open merge requests for which the user has developer access to the target project' do + expect(project.merge_requests_allowing_push_to_user(user)).to include(merge_request) + end + + it 'does not include closed merge requests' do + merge_request.close + + expect(project.merge_requests_allowing_push_to_user(user)).to be_empty + end + + it 'does not include merge requests for guest users' do + guest = create(:user) + target_project.add_guest(guest) + + expect(project.merge_requests_allowing_push_to_user(guest)).to be_empty + end + + it 'does not include the merge request for other users' do + other_user = create(:user) + + expect(project.merge_requests_allowing_push_to_user(other_user)).to be_empty + end + + it 'is empty when no user is passed' do + expect(project.merge_requests_allowing_push_to_user(nil)).to be_empty + end + end + + describe '#branch_allows_maintainer_push?' do + it 'allows access if the user can merge the merge request' do + expect(project.branch_allows_maintainer_push?(user, 'awesome-feature-1')) + .to be_truthy + end + + it 'does not allow guest users access' do + guest = create(:user) + target_project.add_guest(guest) + + expect(project.branch_allows_maintainer_push?(guest, 'awesome-feature-1')) + .to be_falsy + end + + it 'does not allow access to branches for which the merge request was closed' do + create(:merge_request, :closed, + target_project: target_project, + target_branch: 'target-branch', + source_project: project, + source_branch: 'rejected-feature-1', + allow_maintainer_to_push: true) + + expect(project.branch_allows_maintainer_push?(user, 'rejected-feature-1')) + .to be_falsy + end + + it 'does not allow access if the user cannot merge the merge request' do + create(:protected_branch, :masters_can_push, project: target_project, name: 'target-branch') + + expect(project.branch_allows_maintainer_push?(user, 'awesome-feature-1')) + .to be_falsy + end + + it 'caches the result' do + control = ActiveRecord::QueryRecorder.new { project.branch_allows_maintainer_push?(user, 'awesome-feature-1') } + + expect { 3.times { project.branch_allows_maintainer_push?(user, 'awesome-feature-1') } } + .not_to exceed_query_limit(control) + end + + context 'when the requeststore is active', :request_store do + it 'only queries per project across instances' do + control = ActiveRecord::QueryRecorder.new { project.branch_allows_maintainer_push?(user, 'awesome-feature-1') } + + expect { 2.times { described_class.find(project.id).branch_allows_maintainer_push?(user, 'awesome-feature-1') } } + .not_to exceed_query_limit(control).with_threshold(2) + end + end + end + end end diff --git a/spec/policies/project_policy_spec.rb b/spec/policies/project_policy_spec.rb index 129344f105f..ea76e604153 100644 --- a/spec/policies/project_policy_spec.rb +++ b/spec/policies/project_policy_spec.rb @@ -308,4 +308,41 @@ describe ProjectPolicy do it_behaves_like 'project policies as master' it_behaves_like 'project policies as owner' it_behaves_like 'project policies as admin' + + context 'when a public project has merge requests allowing access' do + include ProjectForksHelper + let(:user) { create(:user) } + let(:target_project) { create(:project, :public) } + let(:project) { fork_project(target_project) } + let!(:merge_request) do + create( + :merge_request, + target_project: target_project, + source_project: project, + allow_maintainer_to_push: true + ) + end + let(:maintainer_abilities) do + %w(create_build update_build create_pipeline update_pipeline) + end + + subject { described_class.new(user, project) } + + it 'does not allow pushing code' do + expect_disallowed(*maintainer_abilities) + end + + it 'allows pushing if the user is a member with push access to the target project' do + target_project.add_developer(user) + + expect_allowed(*maintainer_abilities) + end + + it 'dissallows abilities to a maintainer if the merge request was closed' do + target_project.add_developer(user) + merge_request.close! + + expect_disallowed(*maintainer_abilities) + end + end end diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index 484322752c0..3764aec0c71 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -616,6 +616,25 @@ describe API::MergeRequests do expect(json_response['changes_count']).to eq('5+') end end + + context 'for forked projects' do + let(:user2) { create(:user) } + let(:project) { create(:project, :public, :repository) } + let(:forked_project) { fork_project(project, user2, repository: true) } + let(:merge_request) do + create(:merge_request, + source_project: forked_project, + target_project: project, + source_branch: 'fixes', + allow_maintainer_to_push: true) + end + + it 'includes the `allow_maintainer_to_push` field' do + get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user) + + expect(json_response['allow_maintainer_to_push']).to be_truthy + end + end end describe 'GET /projects/:id/merge_requests/:merge_request_iid/participants' do @@ -815,6 +834,7 @@ describe API::MergeRequests do context 'forked projects' do let!(:user2) { create(:user) } + let(:project) { create(:project, :public, :repository) } let!(:forked_project) { fork_project(project, user2, repository: true) } let!(:unrelated_project) { create(:project, namespace: create(:user).namespace, creator_id: user2.id) } @@ -872,6 +892,14 @@ describe API::MergeRequests do expect(response).to have_gitlab_http_status(400) end + it 'allows setting `allow_maintainer_to_push`' do + post api("/projects/#{forked_project.id}/merge_requests", user2), + title: 'Test merge_request', source_branch: "feature_conflict", target_branch: "master", + author: user2, target_project_id: project.id, allow_maintainer_to_push: true + expect(response).to have_gitlab_http_status(201) + expect(json_response['allow_maintainer_to_push']).to be_truthy + end + context 'when target_branch and target_project_id is specified' do let(:params) do { title: 'Test merge_request', diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb index c31259239ee..5279ea6164e 100644 --- a/spec/services/merge_requests/update_service_spec.rb +++ b/spec/services/merge_requests/update_service_spec.rb @@ -1,6 +1,8 @@ require 'spec_helper' describe MergeRequests::UpdateService, :mailer do + include ProjectForksHelper + let(:project) { create(:project, :repository) } let(:user) { create(:user) } let(:user2) { create(:user) } @@ -538,5 +540,40 @@ describe MergeRequests::UpdateService, :mailer do let(:open_issuable) { merge_request } let(:closed_issuable) { create(:closed_merge_request, source_project: project) } end + + context 'setting `allow_maintainer_to_push`' do + let(:target_project) { create(:project, :public) } + let(:source_project) { fork_project(target_project) } + let(:user) { create(:user) } + let(:merge_request) do + create(:merge_request, + source_project: source_project, + source_branch: 'fixes', + target_project: target_project) + end + + before do + allow(ProtectedBranch).to receive(:protected?).with(source_project, 'fixes') { false } + end + + it 'does not allow a maintainer of the target project to set `allow_maintainer_to_push`' do + target_project.add_developer(user) + + update_merge_request(allow_maintainer_to_push: true, title: 'Updated title') + + expect(merge_request.title).to eq('Updated title') + expect(merge_request.allow_maintainer_to_push).to be_falsy + end + + it 'is allowed by a user that can push to the source and can update the merge request' do + merge_request.update!(assignee: user) + source_project.add_developer(user) + + update_merge_request(allow_maintainer_to_push: true, title: 'Updated title') + + expect(merge_request.title).to eq('Updated title') + expect(merge_request.allow_maintainer_to_push).to be_truthy + end + end end end diff --git a/spec/views/projects/tree/show.html.haml_spec.rb b/spec/views/projects/tree/show.html.haml_spec.rb index 44b32df0395..3b098320ad7 100644 --- a/spec/views/projects/tree/show.html.haml_spec.rb +++ b/spec/views/projects/tree/show.html.haml_spec.rb @@ -13,6 +13,7 @@ describe 'projects/tree/show' do allow(view).to receive(:can?).and_return(true) allow(view).to receive(:can_collaborate_with_project?).and_return(true) + allow(view).to receive_message_chain('user_access.can_push_to_branch?').and_return(true) allow(view).to receive(:current_application_settings).and_return(Gitlab::CurrentSettings.current_application_settings) end |
