Respect authorization in Repository API

* dont allow protect/unprotect branches for users without master permissions
* dont allow access to Repository api for guests

lib/api/helpers.rb

@@ -64,6 +64,10 @@ module API
       end
     end
 
+    def authorize_admin_project
+      authorize! :admin_project, user_project
+    end
+
     def can?(object, action, subject)
       abilities.allowed?(object, action, subject)
     end

lib/api/repositories.rb

@@ -2,6 +2,7 @@ module API
   # Projects API
   class Repositories < Grape::API
     before { authenticate! }
+    before { authorize! :download_code, user_project }
 
     resource :projects do
       helpers do
@@ -44,13 +45,12 @@ module API
       # Example Request:
       #   PUT /projects/:id/repository/branches/:branch/protect
       put ":id/repository/branches/:branch/protect" do
-        @branch = user_project.repo.heads.find { |item| item.name == params[:branch] }
-        not_found! unless @branch
-        protected = user_project.protected_branches.find_by_name(@branch.name)
+        authorize_admin_project
 
-        unless protected
-          user_project.protected_branches.create(name: @branch.name)
-        end
+        @branch = user_project.repository.find_branch(params[:branch])
+        not_found! unless @branch
+        protected_branch = user_project.protected_branches.find_by_name(@branch.name)
+        user_project.protected_branches.create(name: @branch.name) unless protected_branch
 
         present @branch, with: Entities::RepoObject, project: user_project
       end
@@ -63,13 +63,12 @@ module API
       # Example Request:
       #   PUT /projects/:id/repository/branches/:branch/unprotect
       put ":id/repository/branches/:branch/unprotect" do
-        @branch = user_project.repo.heads.find { |item| item.name == params[:branch] }
-        not_found! unless @branch
-        protected = user_project.protected_branches.find_by_name(@branch.name)
+        authorize_admin_project
 
-        if protected
-          protected.destroy
-        end
+        @branch = user_project.repository.find_branch(params[:branch])
+        not_found! unless @branch
+        protected_branch = user_project.protected_branches.find_by_name(@branch.name)
+        protected_branch.destroy if protected_branch
 
         present @branch, with: Entities::RepoObject, project: user_project
       end
@@ -92,8 +91,6 @@ module API
       # Example Request:
       #   GET /projects/:id/repository/commits
       get ":id/repository/commits" do
-        authorize! :download_code, user_project
-
         page = (params[:page] || 0).to_i
         per_page = (params[:per_page] || 20).to_i
         ref = params[:ref_name] || user_project.try(:default_branch) || 'master'
@@ -110,7 +107,6 @@ module API
       # Example Request:
       #   GET /projects/:id/repository/commits/:sha
       get ":id/repository/commits/:sha" do
-        authorize! :download_code, user_project
         sha = params[:sha]
         commit = user_project.repository.commit(sha)
         not_found! "Commit" unless commit
@@ -125,7 +121,6 @@ module API
       # Example Request:
       #   GET /projects/:id/repository/commits/:sha/diff
       get ":id/repository/commits/:sha/diff" do
-        authorize! :download_code, user_project
         sha = params[:sha]
         result = CommitLoadContext.new(user_project, current_user, {id: sha}).execute
         not_found! "Commit" unless result[:commit]
@@ -140,8 +135,6 @@ module API
       # Example Request:
       #   GET /projects/:id/repository/tree
       get ":id/repository/tree" do
-        authorize! :download_code, user_project
-
         ref = params[:ref_name] || user_project.try(:default_branch) || 'master'
         path = params[:path] || nil
 
@@ -166,7 +159,6 @@ module API
       # Example Request:
       #   GET /projects/:id/repository/blobs/:sha
       get [ ":id/repository/blobs/:sha", ":id/repository/commits/:sha/blob" ] do
-        authorize! :download_code, user_project
         required_attributes! [:filepath]
 
         ref = params[:sha]

spec/requests/api/repositories_spec.rb

@@ -8,7 +8,8 @@ describe API::API do
   let(:user) { create(:user) }
   let(:user2) { create(:user) }
   let!(:project) { create(:project_with_code, creator_id: user.id) }
-  let!(:users_project) { create(:users_project, user: user, project: project, project_access: UsersProject::MASTER) }
+  let!(:master) { create(:users_project, user: user, project: project, project_access: UsersProject::MASTER) }
+  let!(:guest) { create(:users_project, user: user2, project: project, project_access: UsersProject::GUEST) }
 
   before { project.team << [user, :reporter] }
 
@@ -32,6 +33,11 @@ describe API::API do
       json_response['protected'].should == false
     end
 
+    it "should return a 403 error if guest" do
+      get api("/projects/#{project.id}/repository/branches", user2)
+      response.status.should == 403
+    end
+
     it "should return a 404 error if branch is not available" do
       get api("/projects/#{project.id}/repository/branches/unknown", user)
       response.status.should == 404
@@ -53,6 +59,11 @@ describe API::API do
       response.status.should == 404
     end
 
+    it "should return a 403 error if guest" do
+      put api("/projects/#{project.id}/repository/branches/new_design/protect", user2)
+      response.status.should == 403
+    end
+
     it "should return success when protect branch again" do
       put api("/projects/#{project.id}/repository/branches/new_design/protect", user)
       put api("/projects/#{project.id}/repository/branches/new_design/protect", user)