Commit b41fe9bd authored by Mayra Cabrera's avatar Mayra Cabrera

Merge branch 'security-dm-delete-deploy-key' into 'master'

[master] Fix API to remove deploy key from project instead of deleting it entirely

See merge request gitlab/gitlabhq!2379
parents 33f4f161 739029bb
---
title: Fix API to remove deploy key from project instead of deleting it entirely
merge_request:
author:
type: security
...@@ -148,10 +148,10 @@ module API ...@@ -148,10 +148,10 @@ module API
requires :key_id, type: Integer, desc: 'The ID of the deploy key' requires :key_id, type: Integer, desc: 'The ID of the deploy key'
end end
delete ":id/deploy_keys/:key_id" do delete ":id/deploy_keys/:key_id" do
key = user_project.deploy_keys.find(params[:key_id]) deploy_key_project = user_project.deploy_keys_projects.find_by(deploy_key_id: params[:key_id])
not_found!('Deploy Key') unless key not_found!('Deploy Key') unless deploy_key_project
destroy_conditionally!(key) destroy_conditionally!(deploy_key_project)
end end
end end
end end
......
...@@ -171,7 +171,7 @@ describe API::DeployKeys do ...@@ -171,7 +171,7 @@ describe API::DeployKeys do
deploy_key deploy_key
end end
it 'deletes existing key' do it 'removes existing key from project' do
expect do expect do
delete api("/projects/#{project.id}/deploy_keys/#{deploy_key.id}", admin) delete api("/projects/#{project.id}/deploy_keys/#{deploy_key.id}", admin)
...@@ -179,6 +179,44 @@ describe API::DeployKeys do ...@@ -179,6 +179,44 @@ describe API::DeployKeys do
end.to change { project.deploy_keys.count }.by(-1) end.to change { project.deploy_keys.count }.by(-1)
end end
context 'when the deploy key is public' do
it 'does not delete the deploy key' do
expect do
delete api("/projects/#{project.id}/deploy_keys/#{deploy_key.id}", admin)
expect(response).to have_gitlab_http_status(204)
end.not_to change { DeployKey.count }
end
end
context 'when the deploy key is not public' do
let!(:deploy_key) { create(:deploy_key, public: false) }
context 'when the deploy key is only used by this project' do
it 'deletes the deploy key' do
expect do
delete api("/projects/#{project.id}/deploy_keys/#{deploy_key.id}", admin)
expect(response).to have_gitlab_http_status(204)
end.to change { DeployKey.count }.by(-1)
end
end
context 'when the deploy key is used by other projects' do
before do
create(:deploy_keys_project, project: project2, deploy_key: deploy_key)
end
it 'does not delete the deploy key' do
expect do
delete api("/projects/#{project.id}/deploy_keys/#{deploy_key.id}", admin)
expect(response).to have_gitlab_http_status(204)
end.not_to change { DeployKey.count }
end
end
end
it 'returns 404 Not Found with invalid ID' do it 'returns 404 Not Found with invalid ID' do
delete api("/projects/#{project.id}/deploy_keys/404", admin) delete api("/projects/#{project.id}/deploy_keys/404", admin)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment