Merge branch 'sh-fix-issue-53783-ce' into 'master'
Fix enabling project deploy key for admins See merge request gitlab-org/gitlab-ce!23043
This commit is contained in:
commit
f1cf655021
|
@ -46,7 +46,9 @@ class Projects::DeployKeysController < Projects::ApplicationController
|
|||
end
|
||||
|
||||
def enable
|
||||
Projects::EnableDeployKeyService.new(@project, current_user, params).execute
|
||||
key = Projects::EnableDeployKeyService.new(@project, current_user, params).execute
|
||||
|
||||
return render_404 unless key
|
||||
|
||||
respond_to do |format|
|
||||
format.html { redirect_to_repository_settings(@project, anchor: 'js-deploy-keys-settings') }
|
||||
|
@ -54,19 +56,16 @@ class Projects::DeployKeysController < Projects::ApplicationController
|
|||
end
|
||||
end
|
||||
|
||||
# rubocop: disable CodeReuse/ActiveRecord
|
||||
def disable
|
||||
deploy_key_project = @project.deploy_keys_projects.find_by(deploy_key_id: params[:id])
|
||||
deploy_key_project = Projects::DisableDeployKeyService.new(@project, current_user, params).execute
|
||||
|
||||
return render_404 unless deploy_key_project
|
||||
|
||||
deploy_key_project.destroy!
|
||||
|
||||
respond_to do |format|
|
||||
format.html { redirect_to_repository_settings(@project, anchor: 'js-deploy-keys-settings') }
|
||||
format.json { head :ok }
|
||||
end
|
||||
end
|
||||
# rubocop: enable CodeReuse/ActiveRecord
|
||||
|
||||
protected
|
||||
|
||||
|
|
|
@ -0,0 +1,13 @@
|
|||
# frozen_string_literal: true
|
||||
|
||||
module Projects
|
||||
class DisableDeployKeyService < BaseService
|
||||
def execute
|
||||
# rubocop: disable CodeReuse/ActiveRecord
|
||||
deploy_key_project = project.deploy_keys_projects.find_by(deploy_key_id: params[:id])
|
||||
# rubocop: enable CodeReuse/ActiveRecord
|
||||
|
||||
deploy_key_project&.destroy!
|
||||
end
|
||||
end
|
||||
end
|
|
@ -2,9 +2,10 @@
|
|||
|
||||
module Projects
|
||||
class EnableDeployKeyService < BaseService
|
||||
# rubocop: disable CodeReuse/ActiveRecord
|
||||
def execute
|
||||
key = accessible_keys.find_by(id: params[:key_id] || params[:id])
|
||||
key_id = params[:key_id] || params[:id]
|
||||
key = find_accessible_key(key_id)
|
||||
|
||||
return unless key
|
||||
|
||||
unless project.deploy_keys.include?(key)
|
||||
|
@ -13,12 +14,15 @@ module Projects
|
|||
|
||||
key
|
||||
end
|
||||
# rubocop: enable CodeReuse/ActiveRecord
|
||||
|
||||
private
|
||||
|
||||
def accessible_keys
|
||||
current_user.accessible_deploy_keys
|
||||
def find_accessible_key(key_id)
|
||||
if current_user.admin?
|
||||
DeployKey.find_by_id(key_id)
|
||||
else
|
||||
current_user.accessible_deploy_keys.find_by_id(key_id)
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -0,0 +1,5 @@
|
|||
---
|
||||
title: Fix enabling project deploy key for admins
|
||||
merge_request: 23043
|
||||
author:
|
||||
type: fixed
|
|
@ -27,12 +27,8 @@ describe Projects::DeployKeysController do
|
|||
let(:project2) { create(:project, :internal)}
|
||||
let(:project_private) { create(:project, :private)}
|
||||
|
||||
let(:deploy_key_internal) do
|
||||
create(:deploy_key, key: 'ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAAAgQCdMHEHyhRjbhEZVddFn6lTWdgEy5Q6Bz4nwGB76xWZI5YT/1WJOMEW+sL5zYd31kk7sd3FJ5L9ft8zWMWrr/iWXQikC2cqZK24H1xy+ZUmrRuJD4qGAaIVoyyzBL+avL+lF8J5lg6YSw8gwJY/lX64/vnJHUlWw2n5BF8IFOWhiw== dummy@gitlab.com')
|
||||
end
|
||||
let(:deploy_key_actual) do
|
||||
create(:deploy_key, key: 'ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAAAgQDNd/UJWhPrpb+b/G5oL109y57yKuCxE+WUGJGYaj7WQKsYRJmLYh1mgjrl+KVyfsWpq4ylOxIfFSnN9xBBFN8mlb0Fma5DC7YsSsibJr3MZ19ZNBprwNcdogET7aW9I0In7Wu5f2KqI6e5W/spJHCy4JVxzVMUvk6Myab0LnJ2iQ== dummy@gitlab.com')
|
||||
end
|
||||
let(:deploy_key_internal) { create(:deploy_key) }
|
||||
let(:deploy_key_actual) { create(:deploy_key) }
|
||||
let!(:deploy_key_public) { create(:deploy_key, public: true) }
|
||||
|
||||
let!(:deploy_keys_project_internal) do
|
||||
|
@ -63,4 +59,145 @@ describe Projects::DeployKeysController do
|
|||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe '/enable/:id' do
|
||||
let(:deploy_key) { create(:deploy_key) }
|
||||
let(:project2) { create(:project) }
|
||||
let!(:deploy_keys_project_internal) do
|
||||
create(:deploy_keys_project, project: project2, deploy_key: deploy_key)
|
||||
end
|
||||
|
||||
context 'with anonymous user' do
|
||||
before do
|
||||
sign_out(:user)
|
||||
end
|
||||
|
||||
it 'redirects to login' do
|
||||
expect do
|
||||
put :enable, id: deploy_key.id, namespace_id: project.namespace, project_id: project
|
||||
end.not_to change { DeployKeysProject.count }
|
||||
|
||||
expect(response).to have_http_status(302)
|
||||
expect(response).to redirect_to(new_user_session_path)
|
||||
end
|
||||
end
|
||||
|
||||
context 'with user with no permission' do
|
||||
before do
|
||||
sign_in(create(:user))
|
||||
end
|
||||
|
||||
it 'returns 404' do
|
||||
expect do
|
||||
put :enable, id: deploy_key.id, namespace_id: project.namespace, project_id: project
|
||||
end.not_to change { DeployKeysProject.count }
|
||||
|
||||
expect(response).to have_http_status(404)
|
||||
end
|
||||
end
|
||||
|
||||
context 'with user with permission' do
|
||||
before do
|
||||
project2.add_maintainer(user)
|
||||
end
|
||||
|
||||
it 'returns 302' do
|
||||
expect do
|
||||
put :enable, id: deploy_key.id, namespace_id: project.namespace, project_id: project
|
||||
end.to change { DeployKeysProject.count }.by(1)
|
||||
|
||||
expect(DeployKeysProject.where(project_id: project.id, deploy_key_id: deploy_key.id).count).to eq(1)
|
||||
expect(response).to have_http_status(302)
|
||||
expect(response).to redirect_to(namespace_project_settings_repository_path(anchor: 'js-deploy-keys-settings'))
|
||||
end
|
||||
|
||||
it 'returns 404' do
|
||||
put :enable, id: 0, namespace_id: project.namespace, project_id: project
|
||||
|
||||
expect(response).to have_http_status(404)
|
||||
end
|
||||
end
|
||||
|
||||
context 'with admin' do
|
||||
before do
|
||||
sign_in(create(:admin))
|
||||
end
|
||||
|
||||
it 'returns 302' do
|
||||
expect do
|
||||
put :enable, id: deploy_key.id, namespace_id: project.namespace, project_id: project
|
||||
end.to change { DeployKeysProject.count }.by(1)
|
||||
|
||||
expect(DeployKeysProject.where(project_id: project.id, deploy_key_id: deploy_key.id).count).to eq(1)
|
||||
expect(response).to have_http_status(302)
|
||||
expect(response).to redirect_to(namespace_project_settings_repository_path(anchor: 'js-deploy-keys-settings'))
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe '/disable/:id' do
|
||||
let(:deploy_key) { create(:deploy_key) }
|
||||
let!(:deploy_key_project) { create(:deploy_keys_project, project: project, deploy_key: deploy_key) }
|
||||
|
||||
context 'with anonymous user' do
|
||||
before do
|
||||
sign_out(:user)
|
||||
end
|
||||
|
||||
it 'redirects to login' do
|
||||
put :disable, id: deploy_key.id, namespace_id: project.namespace, project_id: project
|
||||
|
||||
expect(response).to have_http_status(302)
|
||||
expect(response).to redirect_to(new_user_session_path)
|
||||
expect(DeployKey.find(deploy_key.id)).to eq(deploy_key)
|
||||
end
|
||||
end
|
||||
|
||||
context 'with user with no permission' do
|
||||
before do
|
||||
sign_in(create(:user))
|
||||
end
|
||||
|
||||
it 'returns 404' do
|
||||
put :disable, id: deploy_key.id, namespace_id: project.namespace, project_id: project
|
||||
|
||||
expect(response).to have_http_status(404)
|
||||
expect(DeployKey.find(deploy_key.id)).to eq(deploy_key)
|
||||
end
|
||||
end
|
||||
|
||||
context 'with user with permission' do
|
||||
it 'returns 302' do
|
||||
put :disable, id: deploy_key.id, namespace_id: project.namespace, project_id: project
|
||||
|
||||
expect(response).to have_http_status(302)
|
||||
expect(response).to redirect_to(namespace_project_settings_repository_path(anchor: 'js-deploy-keys-settings'))
|
||||
|
||||
expect { DeployKey.find(deploy_key.id) }.to raise_error(ActiveRecord::RecordNotFound)
|
||||
end
|
||||
|
||||
it 'returns 404' do
|
||||
put :disable, id: 0, namespace_id: project.namespace, project_id: project
|
||||
|
||||
expect(response).to have_http_status(404)
|
||||
end
|
||||
end
|
||||
|
||||
context 'with admin' do
|
||||
before do
|
||||
sign_in(create(:admin))
|
||||
end
|
||||
|
||||
it 'returns 302' do
|
||||
expect do
|
||||
put :disable, id: deploy_key.id, namespace_id: project.namespace, project_id: project
|
||||
end.to change { DeployKey.count }.by(-1)
|
||||
|
||||
expect(response).to have_http_status(302)
|
||||
expect(response).to redirect_to(namespace_project_settings_repository_path(anchor: 'js-deploy-keys-settings'))
|
||||
|
||||
expect { DeployKey.find(deploy_key.id) }.to raise_error(ActiveRecord::RecordNotFound)
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
Loading…
Reference in New Issue