diff --git a/app/policies/gitlab/git/tag_policy.rb b/app/policies/gitlab/git/tag_policy.rb
new file mode 100644
index 0000000000000000000000000000000000000000..ee406f4d1a017ef5c1c35f0866abcc2182550a8c
--- /dev/null
+++ b/app/policies/gitlab/git/tag_policy.rb
@@ -0,0 +1,19 @@
+# frozen_string_literal: true
+
+module Gitlab
+ module Git
+ class TagPolicy < BasePolicy
+ delegate { project }
+
+ condition(:protected_tag, scope: :subject) do
+ ProtectedTag.protected?(project, @subject.name)
+ end
+
+ rule { can?(:admin_tag) & (~protected_tag | can?(:maintainer_access)) }.enable :delete_tag
+
+ def project
+ @subject.repository.container
+ end
+ end
+ end
+end
diff --git a/app/services/tags/destroy_service.rb b/app/services/tags/destroy_service.rb
index 4d1f4043b01ab3b48c6e7fb3edee90979fcc78d4..ae3f989194378cc660ff782724a0b380e9174e5d 100644
--- a/app/services/tags/destroy_service.rb
+++ b/app/services/tags/destroy_service.rb
@@ -2,21 +2,17 @@
module Tags
class DestroyService < BaseService
- def execute(tag_name)
+ def execute(tag_name, skip_find: false)
repository = project.repository
- tag = repository.find_tag(tag_name)
- unless tag
- return error('No such tag', 404)
- end
+ # If we've found the tag upstream we don't need to refind it so we can
+ # pass skip_find: true
+ return error('No such tag', 404) unless skip_find || tag_exists?(tag_name)
if repository.rm_tag(current_user, tag_name)
- ##
- # When a tag in a repository is destroyed,
- # release assets will be destroyed too.
- Releases::DestroyService
- .new(project, current_user, tag: tag_name)
- .execute
+ # When a tag in a repository is destroyed, release assets will be
+ # destroyed too.
+ destroy_releases(tag_name)
unlock_artifacts(tag_name)
@@ -38,6 +34,14 @@ def success(message)
private
+ def tag_exists?(tag_name)
+ repository.find_tag(tag_name)
+ end
+
+ def destroy_releases(tag_name)
+ Releases::DestroyService.new(project, current_user, tag: tag_name).execute
+ end
+
def unlock_artifacts(tag_name)
Ci::RefDeleteUnlockArtifactsWorker.perform_async(project.id, current_user.id, "#{::Gitlab::Git::TAG_REF_PREFIX}#{tag_name}")
end
diff --git a/lib/api/tags.rb b/lib/api/tags.rb
index 3720032f984755d8e18cd1e6abdcd64c636f4a8c..2df9c62dcb7e62694738b0c5f96eb20c35a60b89 100644
--- a/lib/api/tags.rb
+++ b/lib/api/tags.rb
@@ -119,6 +119,7 @@ def find_releases(tags)
desc 'Delete a repository tag' do
success code: 204
failure [
+ { code: 400, message: 'Bad request' },
{ code: 403, message: 'Unauthenticated' },
{ code: 404, message: 'Not found' },
{ code: 412, message: 'Precondition failed' }
@@ -129,16 +130,14 @@ def find_releases(tags)
requires :tag_name, type: String, desc: 'The name of the tag'
end
delete ':id/repository/tags/:tag_name', requirements: TAG_ENDPOINT_REQUIREMENTS, feature_category: :source_code_management do
- authorize_admin_tag
-
tag = user_project.repository.find_tag(params[:tag_name])
not_found!('Tag') unless tag
+ authorize!(:delete_tag, tag)
commit = user_project.repository.commit(tag.dereferenced_target)
destroy_conditionally!(commit, last_updated: commit.authored_date) do
- result = ::Tags::DestroyService.new(user_project, current_user)
- .execute(params[:tag_name])
+ result = ::Tags::DestroyService.new(user_project, current_user).execute(params[:tag_name], skip_find: true)
if result[:status] != :success
render_api_error!(result[:message], result[:return_code])
diff --git a/spec/policies/gitlab/git/tag_policy_spec.rb b/spec/policies/gitlab/git/tag_policy_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..c66488dc18dbbba715a4f3b355681062facd01d4
--- /dev/null
+++ b/spec/policies/gitlab/git/tag_policy_spec.rb
@@ -0,0 +1,56 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::Git::TagPolicy, feature_category: :source_code_management do
+ let_it_be(:project) { create(:project, :public, :repository) }
+ let_it_be(:guest) { create(:user, guest_of: project) }
+ let_it_be(:developer) { create(:user, developer_of: project) }
+ let_it_be(:maintainer) { create(:user, maintainer_of: project) }
+ let_it_be(:owner) { create(:user, owner_of: project) }
+ let_it_be(:tag) { project.repository.tags.first }
+
+ subject { described_class.new(user, tag) }
+
+ context 'when user is a project guest' do
+ let(:user) { guest }
+
+ it { is_expected.to be_disallowed(:delete_tag) }
+ end
+
+ context 'when user is a project developer' do
+ let(:user) { developer }
+
+ it { is_expected.to be_allowed(:delete_tag) }
+
+ context 'when the tag is protected' do
+ let_it_be(:protected_tag) { create(:protected_tag, project: project, name: tag.name) }
+
+ it { is_expected.to be_disallowed(:delete_tag) }
+ end
+ end
+
+ context 'when user is a project maintainer' do
+ let(:user) { maintainer }
+
+ it { is_expected.to be_allowed(:delete_tag) }
+
+ context 'when the tag is protected' do
+ let_it_be(:protected_tag) { create(:protected_tag, project: project, name: tag.name) }
+
+ it { is_expected.to be_allowed(:delete_tag) }
+ end
+ end
+
+ context 'when user is a project owner' do
+ let(:user) { owner }
+
+ it { is_expected.to be_allowed(:delete_tag) }
+
+ context 'when the tag is protected' do
+ let_it_be(:protected_tag) { create(:protected_tag, project: project, name: tag.name) }
+
+ it { is_expected.to be_allowed(:delete_tag) }
+ end
+ end
+end
diff --git a/spec/requests/api/tags_spec.rb b/spec/requests/api/tags_spec.rb
index 6794e05161fd271d29255730fd77737c1d0de0db..8556fd046bd1dc68fcdbc104f45c0a10c46b6ea2 100644
--- a/spec/requests/api/tags_spec.rb
+++ b/spec/requests/api/tags_spec.rb
@@ -537,8 +537,16 @@
end
end
- context 'when authenticated', 'as a maintainer' do
- let(:current_user) { user }
+ context 'when authenticated as a guest' do
+ let(:current_user) { create(:user, guest_of: project) }
+
+ it_behaves_like '403 response' do
+ let(:request) { delete api(route, current_user) }
+ end
+ end
+
+ context 'when authenticated as a developer' do
+ let(:current_user) { create(:user, developer_of: project) }
it_behaves_like 'repository delete tag'
@@ -547,6 +555,42 @@
it_behaves_like 'repository delete tag'
end
+
+ context 'when the tag is protected' do
+ before do
+ create(:protected_tag, project: project, name: tag_name)
+ end
+
+ it_behaves_like '403 response' do
+ let(:request) { delete api(route, current_user) }
+ end
+ end
+ end
+
+ context 'when authenticated as a maintainer' do
+ let(:current_user) { create(:user, maintainer_of: project) }
+
+ it_behaves_like 'repository delete tag'
+
+ context 'when the tag is protected' do
+ before do
+ create(:protected_tag, project: project, name: tag_name)
+ end
+
+ it_behaves_like 'repository delete tag'
+ end
+ end
+
+ context 'when authenticated as an owner' do
+ let(:current_user) { create(:user, owner_of: project) }
+
+ context 'when the tag is protected' do
+ before do
+ create(:protected_tag, project: project, name: tag_name)
+ end
+
+ it_behaves_like 'repository delete tag'
+ end
end
end
diff --git a/spec/services/tags/destroy_service_spec.rb b/spec/services/tags/destroy_service_spec.rb
index 343a87785ad8badb6fc1ffbbb6f900ca560c1ca7..6d4b2f42bfc2cb9bbc35e6d5b3d83656d291965d 100644
--- a/spec/services/tags/destroy_service_spec.rb
+++ b/spec/services/tags/destroy_service_spec.rb
@@ -7,25 +7,46 @@
let(:repository) { project.repository }
let(:user) { create(:user) }
let(:service) { described_class.new(project, user) }
+ let(:skip_find) { false }
- describe '#execute' do
- subject { service.execute(tag_name) }
+ describe '#execute(tag_name, skip_find: false)' do
+ subject(:execute) { service.execute(tag_name, skip_find: skip_find) }
before do
allow(Ci::RefDeleteUnlockArtifactsWorker).to receive(:perform_async)
end
- it 'removes the tag' do
- expect(repository).to receive(:before_remove_tag)
- expect(service).to receive(:success)
+ context 'with tag named v1.1.0' do
+ let(:tag_name) { 'v1.1.0' }
- service.execute('v1.1.0')
- end
+ it 'removes the tag' do
+ allow(repository).to receive(:before_remove_tag)
+ allow(service).to receive(:success)
+
+ execute
+
+ expect(repository).to have_received(:before_remove_tag)
+ expect(service).to have_received(:success)
+ end
+
+ context 'when skip_find is true' do
+ let(:skip_find) { true }
- it 'calls the RefDeleteUnlockArtifactsWorker' do
- expect(Ci::RefDeleteUnlockArtifactsWorker).to receive(:perform_async).with(project.id, user.id, 'refs/tags/v1.1.0')
+ before do
+ allow(repository).to receive(:find_tag)
+ execute
+ end
- service.execute('v1.1.0')
+ it 'does not verify the tag exists in the repository' do
+ expect(repository).not_to have_received(:find_tag)
+ end
+ end
+
+ it 'calls the RefDeleteUnlockArtifactsWorker' do
+ expect(Ci::RefDeleteUnlockArtifactsWorker).to receive(:perform_async).with(project.id, user.id, "refs/tags/#{tag_name}")
+
+ execute
+ end
end
context 'when there is an associated release on the tag' do