From 726fa6c76afc9162fe046439f7f11b729190aaa6 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Sun, 29 Sep 2013 16:04:57 +0300 Subject: [PATCH] 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 | 4 ++++ lib/api/repositories.rb | 30 ++++++++++---------------- spec/requests/api/repositories_spec.rb | 13 ++++++++++- 3 files changed, 27 insertions(+), 20 deletions(-) diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 4f189f35196..fdde6d5de6c 100644 --- a/lib/api/helpers.rb +++ b/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 diff --git a/lib/api/repositories.rb b/lib/api/repositories.rb index fef32d3a2fe..c2b229b0172 100644 --- a/lib/api/repositories.rb +++ b/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] diff --git a/spec/requests/api/repositories_spec.rb b/spec/requests/api/repositories_spec.rb index f15abdd3581..2e509ea2933 100644 --- a/spec/requests/api/repositories_spec.rb +++ b/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)