Add API endpoint for updating protected branch

Included in this commit are 3 squashed commits:

- Support strong param "chaining"

Rather than duplicate the strong param declarations in an EE-override,
let's rework this method so we can call `super(:new, :params, :to,
:include)`

- EE override to allow :code_owner_approval_required

- Force COAR false if feature is disabled

8 files changed, 602 insertions, 9 deletions

diff --git a/app/controllers/projects/protected_branches_controller.rb b/app/controllers/projects/protected_branches_controller.rb
index c5454883060..d4f7d0bc521 100644
--- a/app/controllers/projects/protected_branches_controller.rb
+++ b/app/controllers/projects/protected_branches_controller.rb
@@ -19,9 +19,13 @@ class Projects::ProtectedBranchesController < Projects::ProtectedRefsController
     [:merge_access_levels, :push_access_levels]
   end
 
-  def protected_ref_params
-    params.require(:protected_branch).permit(:name,
-                                             merge_access_levels_attributes: access_level_attributes,
-                                             push_access_levels_attributes: access_level_attributes)
+  def protected_ref_params(*attrs)
+    attrs = ([:name,
+              merge_access_levels_attributes: access_level_attributes,
+              push_access_levels_attributes: access_level_attributes] + attrs).uniq
+
+    params.require(:protected_branch).permit(attrs)
   end
 end
+
+Projects::ProtectedBranchesController.prepend_if_ee('EE::Projects::ProtectedBranchesController')
diff --git a/app/views/projects/protected_branches/shared/_branches_list.html.haml b/app/views/projects/protected_branches/shared/_branches_list.html.haml
index 1913d06a6f8..7615d60ba27 100644
--- a/app/views/projects/protected_branches/shared/_branches_list.html.haml
+++ b/app/views/projects/protected_branches/shared/_branches_list.html.haml
@@ -15,10 +15,17 @@
           %col
       %thead
         %tr
-          %th Protected branch (#{@protected_branches_count})
-          %th Last commit
-          %th Allowed to merge
-          %th Allowed to push
+          %th
+            = (s_("ProtectedBranch|Protected branch (%{protected_branches_count})") % { protected_branches_count: @protected_branches_count }).html_safe
+          %th
+            = s_("ProtectedBranch|Last commit")
+          %th
+            = s_("ProtectedBranch|Allowed to merge")
+          %th
+            = s_("ProtectedBranch|Allowed to push")
+          - if @project.merge_requests_require_code_owner_approval
+            %th
+              = s_("ProtectedBranch|Code owner approval")
           - if can_admin_project
             %th
       %tbody
diff --git a/ee/app/controllers/ee/projects/protected_branches_controller.rb b/ee/app/controllers/ee/projects/protected_branches_controller.rb
new file mode 100644
index 00000000000..f1964210371
--- /dev/null
+++ b/ee/app/controllers/ee/projects/protected_branches_controller.rb
@@ -0,0 +1,20 @@
+# frozen_string_literal: true
+
+module EE
+  module Projects
+    module ProtectedBranchesController
+      extend ::Gitlab::Utils::Override
+
+      protected
+
+      override :protected_ref_params
+      def protected_ref_params(*attrs)
+        params_hash = super(:code_owner_approval_required)
+
+        params_hash[:code_owner_approval_required] = false unless project.code_owner_approval_required_available?
+
+        params_hash
+      end
+    end
+  end
+end
diff --git a/ee/lib/ee/api/protected_branches.rb b/ee/lib/ee/api/protected_branches.rb
new file mode 100644
index 00000000000..736899a94ef
--- /dev/null
+++ b/ee/lib/ee/api/protected_branches.rb
@@ -0,0 +1,38 @@
+# frozen_string_literal: true
+
+module EE
+  module API
+    module ProtectedBranches
+      extend ActiveSupport::Concern
+
+      BRANCH_ENDPOINT_REQUIREMENTS = ::API::API::NAMESPACE_OR_PROJECT_REQUIREMENTS.merge(name: ::API::API::NO_SLASH_URL_PART_REGEX)
+
+      prepended do
+        params do
+          requires :id, type: String, desc: 'The ID of a project'
+        end
+        resource :projects, requirements: ::API::API::NAMESPACE_OR_PROJECT_REQUIREMENTS do
+          desc 'Update the code_owner_approval_required state of an existing protected branch' do
+            success ::API::Entities::ProtectedBranch
+          end
+          params do
+            requires :name, type: String, desc: 'The name of the branch'
+
+            use :optional_params_ee
+          end
+          # rubocop: disable CodeReuse/ActiveRecord
+          patch ':id/protected_branches/:name', requirements: BRANCH_ENDPOINT_REQUIREMENTS do
+            render_api_error!(protected_branch.errors.full_messages, 404) unless user_project.feature_available?(:code_owners)
+
+            protected_branch = user_project.protected_branches.find_by!(name: params[:name])
+
+            protected_branch.update_attribute(:code_owner_approval_required, declared_params[:code_owner_approval_required])
+
+            present protected_branch, with: ::API::Entities::ProtectedBranch, project: user_project
+          end
+          # rubocop: enable CodeReuse/ActiveRecord
+        end
+      end
+    end
+  end
+end
diff --git a/ee/spec/controllers/ee/projects/protected_branches_controller_spec.rb b/ee/spec/controllers/ee/projects/protected_branches_controller_spec.rb
new file mode 100644
index 00000000000..828b3d075a5
--- /dev/null
+++ b/ee/spec/controllers/ee/projects/protected_branches_controller_spec.rb
@@ -0,0 +1,68 @@
+# frozen_string_literal: true
+require "spec_helper"
+
+describe Projects::ProtectedBranchesController do
+  let(:project) { create(:project, :repository) }
+  let(:protected_branch) { create(:protected_branch, project: project) }
+  let(:project_params) { { namespace_id: project.namespace.to_param, project_id: project } }
+  let(:user) { create(:user) }
+
+  before do
+    project.add_maintainer(user)
+  end
+
+  shared_examples "protected branch with code owner approvals feature" do |boolean|
+    it "sets code owner approvals to #{boolean} when protecting the branch" do
+      expect do
+        post(:create, params: project_params.merge(protected_branch: create_params))
+      end.to change(ProtectedBranch, :count).by(1)
+
+      expect(ProtectedBranch.last.attributes["code_owner_approval_required"]).to eq(boolean)
+    end
+  end
+
+  describe "POST #create" do
+    let(:maintainer_access_level) { [{ access_level: Gitlab::Access::MAINTAINER }] }
+    let(:access_level_params) do
+      { merge_access_levels_attributes: maintainer_access_level,
+        push_access_levels_attributes: maintainer_access_level }
+    end
+    let(:create_params) do
+      attributes_for(:protected_branch).merge(access_level_params)
+    end
+
+    before do
+      sign_in(user)
+    end
+
+    context "when code_owner_approval_required is 'false'" do
+      before do
+        create_params[:code_owner_approval_required] = false
+      end
+
+      it_behaves_like "protected branch with code owner approvals feature", false
+    end
+
+    context "when code_owner_approval_required is 'true'" do
+      before do
+        create_params[:code_owner_approval_required] = true
+      end
+
+      context "when the feature is enabled" do
+        before do
+          stub_licensed_features(code_owner_approval_required: true)
+        end
+
+        it_behaves_like "protected branch with code owner approvals feature", true
+      end
+
+      context "when the feature is not enabled" do
+        before do
+          stub_licensed_features(code_owner_approval_required: false)
+        end
+
+        it_behaves_like "protected branch with code owner approvals feature", false
+      end
+    end
+  end
+end
diff --git a/ee/spec/features/protected_branches_spec.rb b/ee/spec/features/protected_branches_spec.rb
new file mode 100644
index 00000000000..7b55a18885e
--- /dev/null
+++ b/ee/spec/features/protected_branches_spec.rb
@@ -0,0 +1,146 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe 'Protected Branches', :js do
+  include ProtectedBranchHelpers
+
+  let(:user) { create(:user, :admin) }
+  let(:project) { create(:project, :repository) }
+
+  before do
+    sign_in(user)
+  end
+
+  describe 'code owner approval' do
+    describe 'when project requires code owner approval' do
+      before do
+        stub_licensed_features(protected_refs_for_users: true, code_owner_approval_required: true)
+        project.update!(merge_requests_require_code_owner_approval: true)
+      end
+
+      describe 'protect a branch form' do
+        let!(:protected_branch) { create(:protected_branch, project: project) }
+
+        it 'has code owner toggle' do
+          visit project_settings_repository_path(project)
+
+          expect(page).to have_content("Require approval from code owners")
+        end
+      end
+
+      describe 'protect branch table' do
+        context 'has a protected branch with code owner approval toggled on' do
+          let!(:protected_branch) { create(:protected_branch, project: project, code_owner_approval_required: true) }
+
+          it 'shows code owner approval toggle' do
+            visit project_settings_repository_path(project)
+
+            expect(page).to have_content("Code owner approval")
+          end
+
+          it 'displays toggle on' do
+            visit project_settings_repository_path(project)
+
+            expect(page).to have_css('.js-project-feature-toggle.is-checked')
+          end
+        end
+
+        context 'has a protected branch with code owner approval toggled off ' do
+          let!(:protected_branch) { create(:protected_branch, project: project, code_owner_approval_required: false) }
+
+          it 'displays toggle off' do
+            visit project_settings_repository_path(project)
+
+            page.within '.qa-protected-branches-list' do
+              expect(page).not_to have_css('.js-project-feature-toggle.is-checked')
+            end
+          end
+        end
+      end
+    end
+
+    describe 'when project does not require code owner approval' do
+      before do
+        stub_licensed_features(protected_refs_for_users: true, code_owner_approval_required: false)
+      end
+
+      it 'does not have code owner approval in the form' do
+        visit project_settings_repository_path(project)
+
+        expect(page).not_to have_content("Require approval from code owners")
+      end
+
+      it 'does not have code owner approval in the table' do
+        visit project_settings_repository_path(project)
+
+        expect(page).not_to have_content("Code owner approval")
+      end
+    end
+  end
+
+  describe 'access control' do
+    describe 'with ref permissions for users enabled' do
+      before do
+        stub_licensed_features(protected_refs_for_users: true)
+      end
+
+      include_examples 'protected branches > access control > EE'
+    end
+
+    describe 'with ref permissions for users disabled' do
+      before do
+        stub_licensed_features(protected_refs_for_users: false)
+      end
+
+      include_examples 'protected branches > access control > CE'
+
+      context 'with existing access levels' do
+        let(:protected_branch) { create(:protected_branch, project: project) }
+
+        it 'shows users that can push to the branch' do
+          protected_branch.push_access_levels.new(user: create(:user, name: 'Jane'))
+            .save!(validate: false)
+
+          visit project_settings_repository_path(project)
+
+          expect(page).to have_content("The following user can also push to this branch: "\
+                                       "Jane")
+        end
+
+        it 'shows groups that can push to the branch' do
+          protected_branch.push_access_levels.new(group: create(:group, name: 'Team Awesome'))
+            .save!(validate: false)
+
+          visit project_settings_repository_path(project)
+
+          expect(page).to have_content("Members of this group can also push to "\
+                                       "this branch: Team Awesome")
+        end
+
+        it 'shows users that can merge into the branch' do
+          protected_branch.merge_access_levels.new(user: create(:user, name: 'Jane'))
+            .save!(validate: false)
+
+          visit project_settings_repository_path(project)
+
+          expect(page).to have_content("The following user can also merge into "\
+                                       "this branch: Jane")
+        end
+
+        it 'shows groups that have can push to the branch' do
+          protected_branch.merge_access_levels.new(group: create(:group, name: 'Team Awesome'))
+            .save!(validate: false)
+          protected_branch.merge_access_levels.new(group: create(:group, name: 'Team B'))
+            .save!(validate: false)
+
+          visit project_settings_repository_path(project)
+
+          expect(page).to have_content("Members of these groups can also merge into "\
+                                       "this branch:")
+          expect(page).to have_content(/(Team Awesome|Team B) and (Team Awesome|Team B)/)
+        end
+      end
+    end
+  end
+end
diff --git a/ee/spec/requests/api/protected_branches_spec.rb b/ee/spec/requests/api/protected_branches_spec.rb
new file mode 100644
index 00000000000..379ec4e62c6
--- /dev/null
+++ b/ee/spec/requests/api/protected_branches_spec.rb
@@ -0,0 +1,308 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe API::ProtectedBranches do
+  let(:user) { create(:user) }
+  let!(:project) { create(:project, :repository) }
+  let(:protected_name) { 'feature' }
+  let(:branch_name) { protected_name }
+  let!(:protected_branch) do
+    create(:protected_branch, project: project, name: protected_name)
+  end
+
+  describe "GET /projects/:id/protected_branches/:branch" do
+    let(:route) { "/projects/#{project.id}/protected_branches/#{branch_name}" }
+
+    shared_examples_for 'protected branch' do
+      it 'returns the protected branch' do
+        get api(route, user)
+
+        expect(response).to have_gitlab_http_status(200)
+        expect(json_response['unprotect_access_levels']).to eq([])
+      end
+
+      context 'with per user/group access levels' do
+        let(:push_user) { create(:user) }
+        let(:merge_group) { create(:group) }
+        let(:unprotect_group) { create(:group) }
+
+        before do
+          project.add_developer(push_user)
+          project.project_group_links.create(group: merge_group)
+          project.project_group_links.create(group: unprotect_group)
+          protected_branch.push_access_levels.create!(user: push_user)
+          protected_branch.merge_access_levels.create!(group: merge_group)
+          protected_branch.unprotect_access_levels.create!(group: unprotect_group)
+        end
+
+        it 'returns access level details' do
+          get api(route, user)
+
+          push_user_ids = json_response['push_access_levels'].map {|level| level['user_id']}
+          merge_group_ids = json_response['merge_access_levels'].map {|level| level['group_id']}
+          unprotect_group_ids = json_response['unprotect_access_levels'].map {|level| level['group_id']}
+
+          expect(response).to have_gitlab_http_status(200)
+          expect(push_user_ids).to include(push_user.id)
+          expect(merge_group_ids).to include(merge_group.id)
+          expect(unprotect_group_ids).to include(unprotect_group.id)
+        end
+      end
+    end
+
+    context 'when authenticated as a maintainer' do
+      before do
+        project.add_maintainer(user)
+      end
+
+      it_behaves_like 'protected branch'
+
+      context 'when protected branch contains a wildcard' do
+        let(:protected_name) { 'feature*' }
+
+        it_behaves_like 'protected branch'
+      end
+
+      context 'when protected branch contains a period' do
+        let(:protected_name) { 'my.feature' }
+
+        it_behaves_like 'protected branch'
+      end
+    end
+
+    context 'when authenticated as a guest' do
+      before do
+        project.add_guest(user)
+      end
+
+      it_behaves_like '403 response' do
+        let(:request) { get api(route, user) }
+      end
+    end
+  end
+
+  describe "PATCH /projects/:id/protected_branches/:branch" do
+    let(:route) { "/projects/#{project.id}/protected_branches/#{branch_name}" }
+
+    context 'when authenticated as a maintainer' do
+      before do
+        project.add_maintainer(user)
+      end
+
+      context "when the feature is enabled" do
+        before do
+          stub_licensed_features(code_owner_approval_required: true)
+        end
+
+        it "updates the protected branch" do
+          expect do
+            patch api(route, user), params: { code_owner_approval_required: true }
+          end.to change { protected_branch.reload.code_owner_approval_required }
+
+          expect(response).to have_gitlab_http_status(200)
+          expect(json_response['code_owner_approval_required']).to eq(true)
+        end
+      end
+
+      context "when the feature is disabled" do
+        it "does not change the protected branch" do
+          expect do
+            patch api(route, user), params: { code_owner_approval_required: true }
+          end.not_to change { protected_branch.reload.code_owner_approval_required }
+
+          expect(response).to have_gitlab_http_status(200)
+          expect(json_response['code_owner_approval_required']).to eq(false)
+        end
+      end
+    end
+
+    context 'when authenticated as a guest' do
+      before do
+        project.add_guest(user)
+      end
+
+      it "returns a 403 response" do
+        patch api(route, user)
+
+        expect(response).to have_gitlab_http_status(403)
+      end
+    end
+  end
+
+  describe 'POST /projects/:id/protected_branches' do
+    let(:branch_name) { 'new_branch' }
+    let(:post_endpoint) { api("/projects/#{project.id}/protected_branches", user) }
+
+    def expect_protection_to_be_successful
+      expect(response).to have_gitlab_http_status(201)
+      expect(json_response['name']).to eq(branch_name)
+    end
+
+    context 'when authenticated as a maintainer' do
+      before do
+        project.add_maintainer(user)
+      end
+
+      it 'protects a single branch' do
+        post post_endpoint, params: { name: branch_name }
+
+        expect(response).to have_gitlab_http_status(201)
+        expect(json_response['unprotect_access_levels'][0]['access_level']).to eq(Gitlab::Access::MAINTAINER)
+      end
+
+      it 'protects a single branch and only admins can unprotect' do
+        post post_endpoint, params: { name: branch_name, unprotect_access_level: Gitlab::Access::ADMIN }
+
+        expect(response).to have_gitlab_http_status(201)
+        expect(json_response['name']).to eq(branch_name)
+        expect(json_response['push_access_levels'][0]['access_level']).to eq(Gitlab::Access::MAINTAINER)
+        expect(json_response['merge_access_levels'][0]['access_level']).to eq(Gitlab::Access::MAINTAINER)
+        expect(json_response['unprotect_access_levels'][0]['access_level']).to eq(Gitlab::Access::ADMIN)
+      end
+
+      context "code_owner_approval_required" do
+        context "when feature is enabled" do
+          before do
+            stub_licensed_features(code_owner_approval_required: true)
+          end
+
+          it "sets :code_owner_approval_required to true when the param is true" do
+            expect(project.protected_branches.find_by_name(branch_name)).to be_nil
+
+            post post_endpoint, params: { name: branch_name, code_owner_approval_required: true }
+
+            expect(response).to have_gitlab_http_status(201)
+            expect(json_response["code_owner_approval_required"]).to eq(true)
+
+            new_branch = project.protected_branches.find_by_name(branch_name)
+            expect(new_branch.code_owner_approval_required).to be_truthy
+            expect(new_branch[:code_owner_approval_required]).to be_truthy
+          end
+
+          it "sets :code_owner_approval_required to false when the param is false" do
+            expect(project.protected_branches.find_by_name(branch_name)).to be_nil
+
+            post post_endpoint, params: { name: branch_name, code_owner_approval_required: false }
+
+            expect(response).to have_gitlab_http_status(201)
+            expect(json_response["code_owner_approval_required"]).to eq(false)
+
+            new_branch = project.protected_branches.find_by_name(branch_name)
+            expect(new_branch.code_owner_approval_required).to be_falsy
+            expect(new_branch[:code_owner_approval_required]).to be_falsy
+          end
+        end
+
+        context "when feature is not enabled" do
+          it "sets :code_owner_approval_required to false when the param is false" do
+            expect(project.protected_branches.find_by_name(branch_name)).to be_nil
+
+            post post_endpoint, params: { name: branch_name, code_owner_approval_required: true }
+
+            expect(response).to have_gitlab_http_status(201)
+            expect(json_response["code_owner_approval_required"]).to eq(false)
+
+            new_branch = project.protected_branches.find_by_name(branch_name)
+            expect(new_branch.code_owner_approval_required).to be_falsy
+            expect(new_branch[:code_owner_approval_required]).to be_falsy
+          end
+        end
+      end
+
+      context 'with granular access' do
+        let(:invited_group) do
+          create(:project_group_link, project: project).group
+        end
+
+        let(:project_member) do
+          create(:project_member, project: project).user
+        end
+
+        it 'can protect a branch while allowing an individual user to push' do
+          push_user = project_member
+
+          post post_endpoint, params: { name: branch_name, allowed_to_push: [{ user_id: push_user.id }] }
+
+          expect_protection_to_be_successful
+          expect(json_response['push_access_levels'][0]['user_id']).to eq(push_user.id)
+        end
+
+        it 'can protect a branch while allowing an individual user to merge' do
+          merge_user = project_member
+
+          post post_endpoint, params: { name: branch_name, allowed_to_merge: [{ user_id: merge_user.id }] }
+
+          expect_protection_to_be_successful
+          expect(json_response['merge_access_levels'][0]['user_id']).to eq(merge_user.id)
+        end
+
+        it 'can protect a branch while allowing an individual user to unprotect' do
+          unprotect_user = project_member
+
+          post post_endpoint, params: { name: branch_name, allowed_to_unprotect: [{ user_id: unprotect_user.id }] }
+
+          expect_protection_to_be_successful
+          expect(json_response['unprotect_access_levels'][0]['user_id']).to eq(unprotect_user.id)
+        end
+
+        it 'can protect a branch while allowing a group to push' do
+          push_group = invited_group
+
+          post post_endpoint, params: { name: branch_name, allowed_to_push: [{ group_id: push_group.id }] }
+
+          expect_protection_to_be_successful
+          expect(json_response['push_access_levels'][0]['group_id']).to eq(push_group.id)
+        end
+
+        it 'can protect a branch while allowing a group to merge' do
+          merge_group = invited_group
+
+          post post_endpoint, params: { name: branch_name, allowed_to_merge: [{ group_id: merge_group.id }] }
+
+          expect_protection_to_be_successful
+          expect(json_response['merge_access_levels'][0]['group_id']).to eq(merge_group.id)
+        end
+
+        it 'can protect a branch while allowing a group to unprotect' do
+          unprotect_group = invited_group
+
+          post post_endpoint, params: { name: branch_name, allowed_to_unprotect: [{ group_id: unprotect_group.id }] }
+
+          expect_protection_to_be_successful
+          expect(json_response['unprotect_access_levels'][0]['group_id']).to eq(unprotect_group.id)
+        end
+
+        it "fails if users don't all have access to the project" do
+          push_user = create(:user)
+
+          post post_endpoint, params: { name: branch_name, allowed_to_merge: [{ user_id: push_user.id }] }
+
+          expect(response).to have_gitlab_http_status(422)
+          expect(json_response['message'][0]).to match(/is not a member of the project/)
+        end
+
+        it "fails if groups aren't all invited to the project" do
+          merge_group = create(:group)
+
+          post post_endpoint, params: { name: branch_name, allowed_to_merge: [{ group_id: merge_group.id }] }
+
+          expect(response).to have_gitlab_http_status(422)
+          expect(json_response['message'][0]).to match(/does not have access to the project/)
+        end
+
+        it 'avoids creating default access levels unless necessary' do
+          push_user = project_member
+
+          post post_endpoint, params: { name: branch_name, allowed_to_push: [{ user_id: push_user.id }] }
+
+          expect(response).to have_gitlab_http_status(201)
+          expect(json_response['push_access_levels'].count).to eq(1)
+          expect(json_response['merge_access_levels'].count).to eq(1)
+          expect(json_response['push_access_levels'][0]['user_id']).to eq(push_user.id)
+          expect(json_response['push_access_levels'][0]['access_level']).to eq(Gitlab::Access::MAINTAINER)
+        end
+      end
+    end
+  end
+end
diff --git a/lib/api/protected_branches.rb b/lib/api/protected_branches.rb
index ca75ee906ce..c7665c20234 100644
--- a/lib/api/protected_branches.rb
+++ b/lib/api/protected_branches.rb
@@ -42,7 +42,7 @@ module API
       end
       # rubocop: enable CodeReuse/ActiveRecord
 
-      desc 'Protect a single branch or wildcard' do
+      desc 'Protect a single branch' do
         success Entities::ProtectedBranch
       end
       params do
@@ -93,3 +93,5 @@ module API
     end
   end
 end
+
+API::ProtectedBranches.prepend_if_ee('EE::API::ProtectedBranches')