Allow maintainers to edit directly in a fork

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

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

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'

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

app/policies/project_policy.rb

@@ -307,6 +307,12 @@ class ProjectPolicy < BasePolicy
     enable :update_pipeline
   end
+  # A wrapper around `push_code` and `push_single_branch` to avoid several
+  # `push_code`: User can push everything to the repo
+  # `push_single_brach`: User can push to a single branch in the repo
+  # `push_to_repo`: User can push something to this repo.
+  rule { can?(:push_code) | can?(:push_single_branch) }.enable :push_to_repo
+
   private
   def team_member?

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?

app/serializers/merge_request_widget_entity.rb

@@ -30,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
@@ -137,7 +138,7 @@ class MergeRequestWidgetEntity < IssuableEntity
   end
   expose :new_blob_path do |merge_request|
-    if can?(current_user, :push_code, merge_request.project)
+    if presenter(merge_request).can_push_to_source_branch?
       project_new_blob_path(merge_request.project, merge_request.source_branch)
     end
   end

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'

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'

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'

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'

app/views/shared/_new_commit_form.html.haml

+- 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'

app/views/shared/projects/_edit_information.html.haml

+- 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

lib/gitlab/checks/change_access.rb

@@ -47,7 +47,7 @@ module Gitlab
       protected
       def push_checks
-        if user_access.cannot_do_action?(:push_code)
+        if user_access.cannot_do_action?(:push_to_repo)
           raise GitAccess::UnauthorizedError, ERROR_MESSAGES[:push_code]
         end
       end

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 ""

spec/features/merge_request/maintainer_edits_fork_spec.rb

+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) }
+  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_developer(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

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')")

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 = '&lt;script&gt;alert(&#39;escape me!&#39;);&lt;/script&gt;'
+
+      expect(helper.commit_in_single_accessible_branch).to include(escaped_branch_name)
+    end
+  end
 end

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