diff options
| -rw-r--r-- | app/controllers/repositories_controller.rb | 16 | ||||
| -rw-r--r-- | test/functional/repositories_controller_test.rb | 43 |
2 files changed, 55 insertions, 4 deletions
diff --git a/app/controllers/repositories_controller.rb b/app/controllers/repositories_controller.rb index 04a50e8..6798aa4 100644 --- a/app/controllers/repositories_controller.rb +++ b/app/controllers/repositories_controller.rb @@ -172,13 +172,21 @@ class RepositoriesController < ApplicationController def writable_by @repository = @owner.repositories.find_by_name_in_project!(params[:id], @containing_project) user = User.find_by_login(params[:username]) - if user && user.can_write_to?(@repository) - render :text => "true" - else - render :text => "false" + + if user && result = /^refs\/merge-requests\/(\d+)$/.match(params[:git_path].to_s) # git_path could be a merge request + begin + if merge_request = MergeRequest.find(result[1]) and merge_request.user == user + render :text => "true" and return + end + rescue ActiveRecord::RecordNotFound # No such merge request + end + elsif user && user.can_write_to?(@repository) + render :text => "true" and return end + render :text => 'false' and return end + def real_path @repository = @owner.repositories.find_by_name_in_project!(params[:id], @containing_project) render :text => "#{@repository.real_gitdir}" diff --git a/test/functional/repositories_controller_test.rb b/test/functional/repositories_controller_test.rb index fa2b2bc..4b9cd9f 100644 --- a/test/functional/repositories_controller_test.rb +++ b/test/functional/repositories_controller_test.rb @@ -723,6 +723,49 @@ class RepositoriesControllerTest < ActionController::TestCase :id => project.repositories.mainlines.first.to_param assert_response :success end + + should 'not identify a non-merge request git path as a merge request' do + do_writable_by_get({ + :git_path => "refs/heads/master"}) + assert_response :success + assert_equal 'true', @response.body + end + + should 'identify that a merge request is being pushed to' do + @merge_request = merge_requests(:mikes_to_johans) + assert !@merge_request.user.can_write_to?(@merge_request.target_repository) + do_writable_by_get({ + :username => @merge_request.user.to_param, + :project_id => @merge_request.target_repository.project.to_param, + :id => @merge_request.target_repository.to_param, + :git_path => "refs/merge-requests/#{@merge_request.id}"}) + assert_response :success + assert_equal 'true', @response.body + end + + should 'not allow other users than the owner of a merge request push to a merge request' do + @merge_request = merge_requests(:mikes_to_johans) + do_writable_by_get({ + :username => 'johan', + :project_id => @merge_request.target_repository.project.to_param, + :id => @merge_request.target_repository.to_param, + :git_path => "refs/merge-requests/#{@merge_request.id}"}) + assert_response :success + assert_equal 'false', @response.body + end + + should 'not allow pushes to non-existing merge requests' do + @merge_request = merge_requests(:mikes_to_johans) + do_writable_by_get({ + :username => 'johan', + :project_id => @merge_request.target_repository.project.to_param, + :id => @merge_request.target_repository.to_param, + :git_path => "refs/merge-requests/42"}) + assert_response :success + assert_equal 'false', @response.body + end + + end def do_real_path_get(options={}) |