diff options
author | Rémy Coutable <remy@rymai.me> | 2016-04-25 11:57:38 +0200 |
---|---|---|
committer | Rémy Coutable <remy@rymai.me> | 2016-04-25 11:57:38 +0200 |
commit | abf15a9646db3f01da2566b101753ed9e28d9ebd (patch) | |
tree | 2e74d22be613af6679380f2ada317c1dafd9dd7e | |
parent | fffc8a869128c5830d078e1f9286e7d2d6da420c (diff) | |
parent | 41aa7a89fbe2f35d4a3b66bb55a98f224adc837c (diff) | |
download | gitlab-ce-abf15a9646db3f01da2566b101753ed9e28d9ebd.tar.gz |
Merge branch 'master' of https://dev.gitlab.org/gitlab/gitlabhq
-rw-r--r-- | CHANGELOG | 3 | ||||
-rw-r--r-- | lib/api/project_hooks.rb | 4 | ||||
-rw-r--r-- | spec/requests/api/project_hooks_spec.rb | 14 |
3 files changed, 17 insertions, 4 deletions
diff --git a/CHANGELOG b/CHANGELOG index 253ea4dc96a..7f5432326aa 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -7,6 +7,9 @@ v 8.7.1 (unreleased) - Fix .gitlab-ci.yml parsing issue when hidde job is a template without script definition. !3849 - Fix license detection to detect all license files, not only known licenses. !3878 - Use the `can?` helper instead of `current_user.can?`. !3882 + - Prevent users from deleting Webhooks via API they do not own + - Use the `can?` helper instead of `current_user.can?` + - Fix .gitlab-ci.yml parsing issue when hidde job is a template without script definition v 8.7.0 - Gitlab::GitAccess and Gitlab::GitAccessWiki are now instrumented diff --git a/lib/api/project_hooks.rb b/lib/api/project_hooks.rb index cf9938d25a7..ccca65cbe1c 100644 --- a/lib/api/project_hooks.rb +++ b/lib/api/project_hooks.rb @@ -103,10 +103,10 @@ module API required_attributes! [:hook_id] begin - @hook = ProjectHook.find(params[:hook_id]) - @hook.destroy + @hook = user_project.hooks.destroy(params[:hook_id]) rescue # ProjectHook can raise Error if hook_id not found + not_found!("Error deleting hook #{params[:hook_id]}") end end end diff --git a/spec/requests/api/project_hooks_spec.rb b/spec/requests/api/project_hooks_spec.rb index 142b637d291..ffb93bbb120 100644 --- a/spec/requests/api/project_hooks_spec.rb +++ b/spec/requests/api/project_hooks_spec.rb @@ -148,14 +148,24 @@ describe API::API, 'ProjectHooks', api: true do expect(response.status).to eq(200) end - it "should return success when deleting non existent hook" do + it "should return a 404 error when deleting non existent hook" do delete api("/projects/#{project.id}/hooks/42", user) - expect(response.status).to eq(200) + expect(response.status).to eq(404) end it "should return a 405 error if hook id not given" do delete api("/projects/#{project.id}/hooks", user) expect(response.status).to eq(405) end + + it "shold return a 404 if a user attempts to delete project hooks he/she does not own" do + test_user = create(:user) + other_project = create(:project) + other_project.team << [test_user, :master] + + delete api("/projects/#{other_project.id}/hooks/#{hook.id}", test_user) + expect(response.status).to eq(404) + expect(WebHook.exists?(hook.id)).to be_truthy + end end end |