Merge branch 'ericy_ts/protected_branches_api' into 'master'

Extending API for protected branches

See merge request !12756

app/models/concerns/protected_branch_access.rb

 module ProtectedBranchAccess
   extend ActiveSupport::Concern
 
+  ALLOWED_ACCESS_LEVELS ||= [
+    Gitlab::Access::MASTER,
+    Gitlab::Access::DEVELOPER,
+    Gitlab::Access::NO_ACCESS
+  ].freeze
+
   included do
     include ProtectedRefAccess
 
@@ -9,11 +15,7 @@ module ProtectedBranchAccess
     delegate :project, to: :protected_branch
 
     validates :access_level, presence: true, inclusion: {
-      in: [
-        Gitlab::Access::MASTER,
-        Gitlab::Access::DEVELOPER,
-        Gitlab::Access::NO_ACCESS
-      ]
+      in: ALLOWED_ACCESS_LEVELS
     }
 
     def self.human_access_levels

changelogs/unreleased/ericy_ts-protected_branches_api.yml

+---
+title: Add API for protected branches to allow for wildcard matching and no access
+  restrictions
+merge_request: 12756
+author: Eric Yu

doc/api/README.md

@@ -43,6 +43,7 @@ following locations:
 - [Project Access Requests](access_requests.md)
 - [Project Members](members.md)
 - [Project Snippets](project_snippets.md)
+- [Protected Branches](protected_branches.md)
 - [Repositories](repositories.md)
 - [Repository Files](repository_files.md)
 - [Runners](runners.md)

doc/api/branches.md

@@ -95,6 +95,8 @@ Example response:
 
 ## Protect repository branch
 
+>**Note:** This API endpoint is deprecated in favor of `POST /projects/:id/protected_branches`.
+
 Protects a single project repository branch. This is an idempotent function,
 protecting an already protected repository branch still returns a `200 OK`
 status code.
@@ -143,6 +145,8 @@ Example response:
 
 ## Unprotect repository branch
 
+>**Note:** This API endpoint is deprecated in favor of `DELETE /projects/:id/protected_branches/:name`
+
 Unprotects a single project repository branch. This is an idempotent function,
 unprotecting an already unprotected repository branch still returns a `200 OK`
 status code.

doc/api/protected_branches.md

+# Protected branches API
+
+>**Note:** This feature was introduced in GitLab 9.5
+
+**Valid access levels**
+
+The access levels are defined in the `ProtectedBranchAccess::ALLOWED_ACCESS_LEVELS` constant. Currently, these levels are recognized:
+```
+0  => No access
+30 => Developer access
+40 => Master access
+```
+
+## List protected branches
+
+Gets a list of protected branches from a project.
+
+```
+GET /projects/:id/protected_branches
+```
+
+| 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 |
+
+```bash
+curl --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" 'https://gitlab.example.com/api/v4/projects/5/protected_branches'
+```
+
+Example response:
+
+```json
+[
+  {
+    "name": "master",
+    "push_access_levels": [
+      {
+        "access_level": 40,
+        "access_level_description": "Masters"
+      }
+    ],
+    "merge_access_levels": [
+      {
+        "access_level": 40,
+        "access_level_description": "Masters"
+      }
+    ]
+  },
+  ...
+]
+```
+
+## Get a single protected branch or wildcard protected branch
+
+Gets a single protected branch or wildcard protected branch.
+
+```
+GET /projects/:id/protected_branches/:name
+```
+
+| 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 |
+| `name` | string | yes | The name of the branch or wildcard |
+
+```bash
+curl --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" 'https://gitlab.example.com/api/v4/projects/5/protected_branches/master'
+```
+
+Example response:
+
+```json
+{
+  "name": "master",
+  "push_access_levels": [
+    {
+      "access_level": 40,
+      "access_level_description": "Masters"
+    }
+  ],
+  "merge_access_levels": [
+    {
+      "access_level": 40,
+      "access_level_description": "Masters"
+    }
+  ]
+}
+```
+
+## Protect repository branches
+
+Protects a single repository branch or several project repository
+branches using a wildcard protected branch.
+
+```
+POST /projects/:id/protected_branches
+```
+
+```bash
+curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" 'https://gitlab.example.com/api/v4/projects/5/protected_branches?name=*-stable&push_access_level=30&merge_access_level=30'
+```
+
+| 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 |
+| `name` | string | yes | The name of the branch or wildcard |
+| `push_access_level` | string | no | Access levels allowed to push (defaults: `40`, master access level) |
+| `merge_access_level` | string | no | Access levels allowed to merge (defaults: `40`, master access level) |
+
+Example response:
+
+```json
+{
+  "name": "*-stable",
+  "push_access_levels": [
+    {
+      "access_level": 30,
+      "access_level_description": "Developers + Masters"
+    }
+  ],
+  "merge_access_levels": [
+    {
+      "access_level": 30,
+      "access_level_description": "Developers + Masters"
+    }
+  ]
+}
+```
+
+## Unprotect repository branches
+
+Unprotects the given protected branch or wildcard protected branch.
+
+```
+DELETE /projects/:id/protected_branches/:name
+```
+
+```bash
+curl --request PUT --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" 'https://gitlab.example.com/api/v4/projects/5/protected_branches/*-stable'
+```
+
+| 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 |
+| `name` | string | yes | The name of the branch |

lib/api/api.rb

@@ -123,6 +123,7 @@ module API
     mount ::API::ProjectHooks
     mount ::API::Projects
     mount ::API::ProjectSnippets
+    mount ::API::ProtectedBranches
     mount ::API::Repositories
     mount ::API::Runner
     mount ::API::Runners

lib/api/branches.rb

@@ -37,6 +37,7 @@ module API
         present branch, with: Entities::RepoBranch, project: user_project
       end
 
+      # Note: This API will be deprecated in favor of the protected branches API.
       # Note: The internal data model moved from `developers_can_{merge,push}` to `allowed_to_{merge,push}`
       # in `gitlab-org/gitlab-ce!5081`. The API interface has not been changed (to maintain compatibility),
       # but it works with the changed data model to infer `developers_can_merge` and `developers_can_push`.
@@ -65,9 +66,9 @@ module API
         service_args = [user_project, current_user, protected_branch_params]
 
         protected_branch = if protected_branch
-                             ProtectedBranches::ApiUpdateService.new(*service_args).execute(protected_branch)
+                             ::ProtectedBranches::ApiUpdateService.new(*service_args).execute(protected_branch)
                            else
-                             ProtectedBranches::ApiCreateService.new(*service_args).execute
+                             ::ProtectedBranches::ApiCreateService.new(*service_args).execute
                            end
 
         if protected_branch.valid?
@@ -77,6 +78,7 @@ module API
         end
       end
 
+      # Note: This API will be deprecated in favor of the protected branches API.
       desc 'Unprotect a single branch' do
         success Entities::RepoBranch
       end

lib/api/entities.rb

@@ -240,7 +240,7 @@ module API
       end
 
       expose :protected do |repo_branch, options|
-        ProtectedBranch.protected?(options[:project], repo_branch.name)
+        ::ProtectedBranch.protected?(options[:project], repo_branch.name)
       end
 
       expose :developers_can_push do |repo_branch, options|
@@ -299,6 +299,19 @@ module API
       expose :deleted_file?, as: :deleted_file
     end
 
+    class ProtectedRefAccess < Grape::Entity
+      expose :access_level
+      expose :access_level_description do |protected_ref_access|
+        protected_ref_access.humanize
+      end
+    end
+
+    class ProtectedBranch < Grape::Entity
+      expose :name
+      expose :push_access_levels, using: Entities::ProtectedRefAccess
+      expose :merge_access_levels, using: Entities::ProtectedRefAccess
+    end
+
     class Milestone < Grape::Entity
       expose :id, :iid
       expose :project_id, if: -> (entity, options) { entity&.project_id }

lib/api/protected_branches.rb

+module API
+  class ProtectedBranches < Grape::API
+    include PaginationParams
+
+    BRANCH_ENDPOINT_REQUIREMENTS = API::PROJECT_ENDPOINT_REQUIREMENTS.merge(branch: API::NO_SLASH_URL_PART_REGEX)
+
+    before { authorize_admin_project }
+
+    params do
+      requires :id, type: String, desc: 'The ID of a project'
+    end
+    resource :projects, requirements: API::PROJECT_ENDPOINT_REQUIREMENTS do
+      desc "Get a project's protected branches" do
+        success Entities::ProtectedBranch
+      end
+      params do
+        use :pagination
+      end
+      get ':id/protected_branches' do
+        protected_branches = user_project.protected_branches.preload(:push_access_levels, :merge_access_levels)
+
+        present paginate(protected_branches), with: Entities::ProtectedBranch, project: user_project
+      end
+
+      desc 'Get a single protected branch' do
+        success Entities::ProtectedBranch
+      end
+      params do
+        requires :name, type: String, desc: 'The name of the branch or wildcard'
+      end
+      get ':id/protected_branches/:name', requirements: BRANCH_ENDPOINT_REQUIREMENTS do
+        protected_branch = user_project.protected_branches.find_by!(name: params[:name])
+
+        present protected_branch, with: Entities::ProtectedBranch, project: user_project
+      end
+
+      desc 'Protect a single branch or wildcard' do
+        success Entities::ProtectedBranch
+      end
+      params do
+        requires :name, type: String, desc: 'The name of the protected branch'
+        optional :push_access_level, type: Integer, default: Gitlab::Access::MASTER,
+                                     values: ProtectedBranchAccess::ALLOWED_ACCESS_LEVELS,
+                                     desc: 'Access levels allowed to push (defaults: `40`, master access level)'
+        optional :merge_access_level, type: Integer, default: Gitlab::Access::MASTER,
+                                      values: ProtectedBranchAccess::ALLOWED_ACCESS_LEVELS,
+                                      desc: 'Access levels allowed to merge (defaults: `40`, master access level)'
+      end
+      post ':id/protected_branches' do
+        protected_branch = user_project.protected_branches.find_by(name: params[:name])
+        if protected_branch
+          conflict!("Protected branch '#{params[:name]}' already exists")
+        end
+
+        protected_branch_params = {
+          name: params[:name],
+          push_access_levels_attributes: [{ access_level: params[:push_access_level] }],
+          merge_access_levels_attributes: [{ access_level: params[:merge_access_level] }]
+        }
+
+        service_args = [user_project, current_user, protected_branch_params]
+
+        protected_branch = ::ProtectedBranches::CreateService.new(*service_args).execute
+        
+        if protected_branch.persisted?
+          present protected_branch, with: Entities::ProtectedBranch, project: user_project
+        else
+          render_api_error!(protected_branch.errors.full_messages, 422)
+        end
+      end
+
+      desc 'Unprotect a single branch'
+      params do
+        requires :name, type: String, desc: 'The name of the protected branch'
+      end
+      delete ':id/protected_branches/:name', requirements: BRANCH_ENDPOINT_REQUIREMENTS do
+        protected_branch = user_project.protected_branches.find_by!(name: params[:name])
+
+        protected_branch.destroy
+
+        status 204
+      end
+    end
+  end
+end

spec/factories/protected_branches.rb

@@ -57,5 +57,11 @@ FactoryGirl.define do
         protected_branch.merge_access_levels.new(access_level: Gitlab::Access::MASTER)
       end
     end
+
+    trait :no_one_can_merge do
+      after(:create) do |protected_branch|
+        protected_branch.merge_access_levels.first.update!(access_level: Gitlab::Access::NO_ACCESS)
+      end
+    end
   end
 end

spec/requests/api/protected_branches_spec.rb

+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" do
+    let(:route) { "/projects/#{project.id}/protected_branches" }
+
+    shared_examples_for 'protected branches' do
+      it 'returns the protected branches' do
+        get api(route, user), per_page: 100
+
+        expect(response).to have_gitlab_http_status(200)
+        expect(response).to include_pagination_headers
+        expect(json_response).to be_an Array
+
+        protected_branch_names = json_response.map { |x| x['name'] }
+        expected_branch_names = project.protected_branches.map { |x| x['name'] }
+        expect(protected_branch_names).to match_array(expected_branch_names)
+      end
+    end
+
+    context 'when authenticated as a master' do
+      before do
+        project.add_master(user)
+      end
+
+      it_behaves_like 'protected branches'
+    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 "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['name']).to eq(branch_name)
+        expect(json_response['push_access_levels'][0]['access_level']).to eq(::Gitlab::Access::MASTER)
+        expect(json_response['merge_access_levels'][0]['access_level']).to eq(::Gitlab::Access::MASTER)
+      end
+
+      context 'when protected branch does not exist' do
+        let(:branch_name) { 'unknown' }
+
+        it_behaves_like '404 response' do
+          let(:request) { get api(route, user) }
+          let(:message) { '404 Not found' }
+        end
+      end
+    end
+
+    context 'when authenticated as a master' do
+      before do
+        project.add_master(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
+    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 'POST /projects/:id/protected_branches' do
+    let(:branch_name) { 'new_branch' }
+
+    context 'when authenticated as a master' do 
+      before do
+        project.add_master(user)
+      end
+
+      it 'protects a single branch' do
+        post api("/projects/#{project.id}/protected_branches", user), name: branch_name
+
+        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::MASTER)
+        expect(json_response['merge_access_levels'][0]['access_level']).to eq(Gitlab::Access::MASTER)
+      end
+
+      it 'protects a single branch and developers can push' do
+        post api("/projects/#{project.id}/protected_branches", user),
+            name: branch_name, push_access_level: 30
+
+        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::DEVELOPER)
+        expect(json_response['merge_access_levels'][0]['access_level']).to eq(Gitlab::Access::MASTER)
+      end
+
+      it 'protects a single branch and developers can merge' do
+        post api("/projects/#{project.id}/protected_branches", user),
+            name: branch_name, merge_access_level: 30
+
+        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::MASTER)
+        expect(json_response['merge_access_levels'][0]['access_level']).to eq(Gitlab::Access::DEVELOPER)
+      end
+
+      it 'protects a single branch and developers can push and merge' do
+        post api("/projects/#{project.id}/protected_branches", user),
+            name: branch_name, push_access_level: 30, merge_access_level: 30
+
+        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::DEVELOPER)
+        expect(json_response['merge_access_levels'][0]['access_level']).to eq(Gitlab::Access::DEVELOPER)
+      end
+
+      it 'protects a single branch and no one can push' do
+        post api("/projects/#{project.id}/protected_branches", user),
+            name: branch_name, push_access_level: 0
+
+        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::NO_ACCESS)
+        expect(json_response['merge_access_levels'][0]['access_level']).to eq(Gitlab::Access::MASTER)
+      end
+
+      it 'protects a single branch and no one can merge' do
+        post api("/projects/#{project.id}/protected_branches", user),
+            name: branch_name, merge_access_level: 0
+
+        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::MASTER)
+        expect(json_response['merge_access_levels'][0]['access_level']).to eq(Gitlab::Access::NO_ACCESS)
+      end
+
+      it 'protects a single branch and no one can push or merge' do
+        post api("/projects/#{project.id}/protected_branches", user),
+            name: branch_name, push_access_level: 0, merge_access_level: 0
+
+        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::NO_ACCESS)
+        expect(json_response['merge_access_levels'][0]['access_level']).to eq(Gitlab::Access::NO_ACCESS)
+      end
+
+      it 'returns a 409 error if the same branch is protected twice' do
+        post api("/projects/#{project.id}/protected_branches", user), name: protected_name
+        expect(response).to have_gitlab_http_status(409)
+      end
+
+      context 'when branch has a wildcard in its name' do
+        let(:branch_name) { 'feature/*' }
+
+        it "protects multiple branches with a wildcard in the name" do
+          post api("/projects/#{project.id}/protected_branches", user), name: branch_name
+
+          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::MASTER)
+          expect(json_response['merge_access_levels'][0]['access_level']).to eq(Gitlab::Access::MASTER)
+        end
+      end
+    end
+
+    context 'when authenticated as a guest' do
+      before do
+        project.add_guest(user)
+      end
+
+      it "returns a 403 error if guest" do
+        post api("/projects/#{project.id}/protected_branches/", user), name: branch_name
+
+        expect(response).to have_gitlab_http_status(403)
+      end
+    end
+  end
+
+  describe "DELETE /projects/:id/protected_branches/unprotect/:branch" do
+    before do
+      project.add_master(user)
+    end
+
+    it "unprotects a single branch" do
+      delete api("/projects/#{project.id}/protected_branches/#{branch_name}", user)
+
+      expect(response).to have_gitlab_http_status(204)
+    end
+
+    it "returns 404 if branch does not exist" do
+      delete api("/projects/#{project.id}/protected_branches/barfoo", user)
+
+      expect(response).to have_gitlab_http_status(404)
+    end
+
+    context 'when branch has a wildcard in its name' do
+      let(:protected_name) { 'feature*' }
+      
+      it "unprotects a wildcard branch" do
+        delete api("/projects/#{project.id}/protected_branches/#{branch_name}", user)
+
+        expect(response).to have_gitlab_http_status(204)
+      end
+    end
+  end
+end