Fixes various errors when adding deploy keys caused by not exiting the control flow.
When adding a deploy key that already exists in the project the existing key would not be returned, resulting in an attempt to create a new one, which in turn caused a 500 error due to an ActiveRecord exception. When adding a deploy key that exists within another project the key would be joined to the project, but would also attempt to create a new one, which resulted in a 400 error due to the key already existing. Fixes #22741 Fixes #21754 Signed-off-by: Rémy Coutable <remy@rymai.me>
This commit is contained in:
parent
70074733bb
commit
ce4760bbd5
|
@ -12,6 +12,7 @@ Please view this file on the master branch, on stable branches it's out of date.
|
||||||
- Fixed link typo on /help/ui to Alerts section. !6915 (Sam Rose)
|
- Fixed link typo on /help/ui to Alerts section. !6915 (Sam Rose)
|
||||||
- Simpler arguments passed to named_route on toggle_award_url helper method
|
- Simpler arguments passed to named_route on toggle_award_url helper method
|
||||||
- Fix: Backup restore doesn't clear cache
|
- Fix: Backup restore doesn't clear cache
|
||||||
|
- API: Fix project deploy keys 400 and 500 errors when adding an existing key. !6784 (Joshua Welsh)
|
||||||
- Replace jquery.cookie plugin with js.cookie !7085
|
- Replace jquery.cookie plugin with js.cookie !7085
|
||||||
- Use MergeRequestsClosingIssues cache data on Issue#closed_by_merge_requests method
|
- Use MergeRequestsClosingIssues cache data on Issue#closed_by_merge_requests method
|
||||||
- Fix Sign in page 'Forgot your password?' link overlaps on medium-large screens
|
- Fix Sign in page 'Forgot your password?' link overlaps on medium-large screens
|
||||||
|
|
|
@ -49,18 +49,23 @@ module API
|
||||||
attrs = attributes_for_keys [:title, :key]
|
attrs = attributes_for_keys [:title, :key]
|
||||||
attrs[:key].strip! if attrs[:key]
|
attrs[:key].strip! if attrs[:key]
|
||||||
|
|
||||||
|
# Check for an existing key joined to this project
|
||||||
key = user_project.deploy_keys.find_by(key: attrs[:key])
|
key = user_project.deploy_keys.find_by(key: attrs[:key])
|
||||||
present key, with: Entities::SSHKey if key
|
if key
|
||||||
|
present key, with: Entities::SSHKey
|
||||||
|
break
|
||||||
|
end
|
||||||
|
|
||||||
# Check for available deploy keys in other projects
|
# Check for available deploy keys in other projects
|
||||||
key = current_user.accessible_deploy_keys.find_by(key: attrs[:key])
|
key = current_user.accessible_deploy_keys.find_by(key: attrs[:key])
|
||||||
if key
|
if key
|
||||||
user_project.deploy_keys << key
|
user_project.deploy_keys << key
|
||||||
present key, with: Entities::SSHKey
|
present key, with: Entities::SSHKey
|
||||||
|
break
|
||||||
end
|
end
|
||||||
|
|
||||||
|
# Create a new deploy key
|
||||||
key = DeployKey.new attrs
|
key = DeployKey.new attrs
|
||||||
|
|
||||||
if key.valid? && user_project.deploy_keys << key
|
if key.valid? && user_project.deploy_keys << key
|
||||||
present key, with: Entities::SSHKey
|
present key, with: Entities::SSHKey
|
||||||
else
|
else
|
||||||
|
|
|
@ -6,6 +6,7 @@ describe API::API, api: true do
|
||||||
let(:user) { create(:user) }
|
let(:user) { create(:user) }
|
||||||
let(:admin) { create(:admin) }
|
let(:admin) { create(:admin) }
|
||||||
let(:project) { create(:project, creator_id: user.id) }
|
let(:project) { create(:project, creator_id: user.id) }
|
||||||
|
let(:project2) { create(:project, creator_id: user.id) }
|
||||||
let(:deploy_key) { create(:deploy_key, public: true) }
|
let(:deploy_key) { create(:deploy_key, public: true) }
|
||||||
|
|
||||||
let!(:deploy_keys_project) do
|
let!(:deploy_keys_project) do
|
||||||
|
@ -96,6 +97,22 @@ describe API::API, api: true do
|
||||||
post api("/projects/#{project.id}/deploy_keys", admin), key_attrs
|
post api("/projects/#{project.id}/deploy_keys", admin), key_attrs
|
||||||
end.to change{ project.deploy_keys.count }.by(1)
|
end.to change{ project.deploy_keys.count }.by(1)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
it 'returns an existing ssh key when attempting to add a duplicate' do
|
||||||
|
expect do
|
||||||
|
post api("/projects/#{project.id}/deploy_keys", admin), { key: deploy_key.key, title: deploy_key.title }
|
||||||
|
end.not_to change { project.deploy_keys.count }
|
||||||
|
|
||||||
|
expect(response).to have_http_status(201)
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'joins an existing ssh key to a new project' do
|
||||||
|
expect do
|
||||||
|
post api("/projects/#{project2.id}/deploy_keys", admin), { key: deploy_key.key, title: deploy_key.title }
|
||||||
|
end.to change { project2.deploy_keys.count }.by(1)
|
||||||
|
|
||||||
|
expect(response).to have_http_status(201)
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
describe 'DELETE /projects/:id/deploy_keys/:key_id' do
|
describe 'DELETE /projects/:id/deploy_keys/:key_id' do
|
||||||
|
|
Loading…
Reference in New Issue