diff options
author | Rémy Coutable <remy@gitlab.com> | 2016-04-25 09:26:58 +0000 |
---|---|---|
committer | Rémy Coutable <remy@rymai.me> | 2016-04-26 11:04:15 +0200 |
commit | 20cb5a7b3ecffac346498bda13184005103c1285 (patch) | |
tree | beadbdcc19d1742ee8a3523c2f6b6b4842dca930 | |
parent | 88e60bbbcb676274fd4a84ca4bc7f70497a09671 (diff) | |
download | gitlab-ce-20cb5a7b3ecffac346498bda13184005103c1285.tar.gz |
Merge branch 'fix-project-hook-delete-permissions' into 'master'
Prevent users from deleting Webhooks via API they do not own
Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/15576
See merge request !1959
Signed-off-by: Rémy Coutable <remy@rymai.me>
-rw-r--r-- | CHANGELOG | 5 | ||||
-rw-r--r-- | lib/api/project_hooks.rb | 4 | ||||
-rw-r--r-- | spec/requests/api/project_hooks_spec.rb | 14 |
3 files changed, 19 insertions, 4 deletions
diff --git a/CHANGELOG b/CHANGELOG index c4465c63a06..85c9a1476b4 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,5 +1,10 @@ Please view this file on the master branch, on stable branches it's out of date. +v 8.2.5 + - Fix a window.opener bug that could lead to XSS and open redirects + - Prevent privilege escalation via "impersonate" feature + - Prevent users from deleting Webhooks via API they do not own + v 8.2.4 - Bump Git version requirement to 2.7.4 diff --git a/lib/api/project_hooks.rb b/lib/api/project_hooks.rb index 882d1a083ad..4b7e4a6b1e3 100644 --- a/lib/api/project_hooks.rb +++ b/lib/api/project_hooks.rb @@ -101,10 +101,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 606b226ad77..7abd9ea81f5 100644 --- a/spec/requests/api/project_hooks_spec.rb +++ b/spec/requests/api/project_hooks_spec.rb @@ -140,14 +140,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 |