From 6d841eaadcbccfa4527bd892bf86fc8dbba19455 Mon Sep 17 00:00:00 2001
From: Timothy Andrew <mail@timothyandrew.net>
Date: Wed, 27 Jul 2016 10:14:38 +0530
Subject: Authorize user before creating/updating a protected branch.

1. This is a third line of defence (first in the view, second in the
   controller).

2. Duplicate the `API::Helpers.to_boolean` method in `BaseService`. The
   other alternative is to `include API::Helpers`, but this brings with it
   a number of other methods that might cause conflicts.

3. Return a 403 if authorization fails.
---
 app/services/protected_branches/base_service.rb   | 13 ++++++++++---
 app/services/protected_branches/create_service.rb |  2 ++
 app/services/protected_branches/update_service.rb |  5 +++--
 3 files changed, 15 insertions(+), 5 deletions(-)

(limited to 'app')

diff --git a/app/services/protected_branches/base_service.rb b/app/services/protected_branches/base_service.rb
index a5896587ded..bdd175e8552 100644
--- a/app/services/protected_branches/base_service.rb
+++ b/app/services/protected_branches/base_service.rb
@@ -1,7 +1,5 @@
 module ProtectedBranches
   class BaseService < ::BaseService
-    include API::Helpers
-
     def initialize(project, current_user, params = {})
       super(project, current_user, params)
       @allowed_to_push = params[:allowed_to_push]
@@ -14,7 +12,7 @@ module ProtectedBranches
       set_push_access_levels!
     end
 
-    protected
+    private
 
     def set_merge_access_levels!
       case @allowed_to_merge
@@ -56,5 +54,14 @@ module ProtectedBranches
           'masters'
         end
     end
+
+    protected
+
+    def to_boolean(value)
+      return true if value =~ /^(true|t|yes|y|1|on)$/i
+      return false if value =~ /^(false|f|no|n|0|off)$/i
+
+      nil
+    end
   end
 end
diff --git a/app/services/protected_branches/create_service.rb b/app/services/protected_branches/create_service.rb
index 212c2134638..36019906416 100644
--- a/app/services/protected_branches/create_service.rb
+++ b/app/services/protected_branches/create_service.rb
@@ -3,6 +3,8 @@ module ProtectedBranches
     attr_reader :protected_branch
 
     def execute
+      raise Gitlab::Access::AccessDeniedError unless current_user.can?(:admin_project, project)
+
       ProtectedBranch.transaction do
         @protected_branch = project.protected_branches.new(name: params[:name])
         @protected_branch.save!
diff --git a/app/services/protected_branches/update_service.rb b/app/services/protected_branches/update_service.rb
index 4a2b1be9c93..58f2f774bae 100644
--- a/app/services/protected_branches/update_service.rb
+++ b/app/services/protected_branches/update_service.rb
@@ -4,12 +4,13 @@ module ProtectedBranches
 
     def initialize(project, current_user, id, params = {})
       super(project, current_user, params)
-      @id = id
+      @protected_branch = ProtectedBranch.find(id)
     end
 
     def execute
+      raise Gitlab::Access::AccessDeniedError unless current_user.can?(:admin_project, project)
+
       ProtectedBranch.transaction do
-        @protected_branch = ProtectedBranch.find(@id)
         set_access_levels!
       end
 
-- 
cgit v1.2.1