Commit 259f52dd authored by Nick Thomas's avatar Nick Thomas

Merge branch '325595-prevent-cloud-licenses-from-being-removed' into 'master'

Protect cloud licenses from being removed

See merge request gitlab-org/gitlab!59576
parents 2fffb34c c36fca26
...@@ -47,7 +47,7 @@ class Admin::LicensesController < Admin::ApplicationController ...@@ -47,7 +47,7 @@ class Admin::LicensesController < Admin::ApplicationController
end end
def destroy def destroy
license.destroy Licenses::DestroyService.new(license, current_user).execute
if License.current if License.current
flash[:notice] = _('The license was removed. GitLab has fallen back on the previous license.') flash[:notice] = _('The license was removed. GitLab has fallen back on the previous license.')
...@@ -55,6 +55,10 @@ class Admin::LicensesController < Admin::ApplicationController ...@@ -55,6 +55,10 @@ class Admin::LicensesController < Admin::ApplicationController
flash[:alert] = _('The license was removed. GitLab now no longer has a valid license.') flash[:alert] = _('The license was removed. GitLab now no longer has a valid license.')
end end
redirect_to admin_license_path, status: :found
rescue Licenses::DestroyService::DestroyCloudLicenseError => e
flash[:error] = e.message
redirect_to admin_license_path, status: :found redirect_to admin_license_path, status: :found
end end
......
...@@ -4,10 +4,13 @@ module Licenses ...@@ -4,10 +4,13 @@ module Licenses
class DestroyService < ::Licenses::BaseService class DestroyService < ::Licenses::BaseService
extend ::Gitlab::Utils::Override extend ::Gitlab::Utils::Override
DestroyCloudLicenseError = Class.new(StandardError)
override :execute override :execute
def execute def execute
raise ActiveRecord::RecordNotFound unless license raise ActiveRecord::RecordNotFound unless license
raise Gitlab::Access::AccessDeniedError unless can?(user, :destroy_licenses) raise Gitlab::Access::AccessDeniedError unless can?(user, :destroy_licenses)
raise DestroyCloudLicenseError.new(_('Cloud licenses can not be removed.')) if license.cloud?
license.destroy license.destroy
end end
......
---
title: Protect cloud licenses from being removed
merge_request: 59576
author:
type: changed
...@@ -6,6 +6,10 @@ module API ...@@ -6,6 +6,10 @@ module API
feature_category :license feature_category :license
rescue_from Licenses::DestroyService::DestroyCloudLicenseError do |e|
render_api_error!(e.message, 422)
end
resource :license do resource :license do
desc 'Get information on the currently active license' do desc 'Get information on the currently active license' do
success EE::API::Entities::GitlabLicenseWithActiveUsers success EE::API::Entities::GitlabLicenseWithActiveUsers
......
...@@ -134,4 +134,36 @@ RSpec.describe Admin::LicensesController do ...@@ -134,4 +134,36 @@ RSpec.describe Admin::LicensesController do
end end
end end
end end
describe 'DELETE destroy' do
let(:cloud_licenses) { License.where(cloud: true) }
before do
allow(License).to receive(:current).and_return(create(:license, cloud: is_cloud_license))
end
context 'with a cloud license' do
let(:is_cloud_license) { true }
it 'is can not be removed' do
delete :destroy
expect(response).to redirect_to(admin_license_path)
expect(flash[:error]).to match('Cloud licenses can not be removed.')
expect(cloud_licenses).to be_present
end
end
context 'with a legacy license' do
let(:is_cloud_license) { false }
it 'is can be removed' do
delete :destroy
expect(response).to redirect_to(admin_license_path)
expect(flash[:notice]).to match('The license was removed. GitLab has fallen back on the previous license.')
expect(cloud_licenses).to be_empty
end
end
end
end end
...@@ -71,7 +71,8 @@ RSpec.describe API::License, api: true do ...@@ -71,7 +71,8 @@ RSpec.describe API::License, api: true do
end end
describe 'DELETE /license/:id' do describe 'DELETE /license/:id' do
let(:license) { create(:license, created_at: Time.now, data: build(:gitlab_license, starts_at: Date.today, expires_at: Date.today, restrictions: { add_ons: { 'GitLab_DeployBoard' => 1 }, active_user_count: 2 }).export) } let(:license_type) { nil }
let(:license) { create(:license, created_at: Time.now, data: build(:gitlab_license, type: license_type, starts_at: Date.today, expires_at: Date.today, restrictions: { add_ons: { 'GitLab_DeployBoard' => 1 }, active_user_count: 2 }).export) }
let(:endpoint) { "/license/#{license.id}" } let(:endpoint) { "/license/#{license.id}" }
it 'destroys a license and returns 204' do it 'destroys a license and returns 204' do
...@@ -95,6 +96,17 @@ RSpec.describe API::License, api: true do ...@@ -95,6 +96,17 @@ RSpec.describe API::License, api: true do
expect(response).to have_gitlab_http_status(:forbidden) expect(response).to have_gitlab_http_status(:forbidden)
expect(json_response['message']).to eq('403 Forbidden') expect(json_response['message']).to eq('403 Forbidden')
end end
context 'with a cloud license' do
let(:license_type) { License::CLOUD_LICENSE_TYPE }
it 'returns 422 and does not delete the license' do
delete api(endpoint, admin)
expect(response).to have_gitlab_http_status(:unprocessable_entity)
expect(License.where(id: license.id)).to exist
end
end
end end
describe 'GET /licenses' do describe 'GET /licenses' do
......
...@@ -6680,6 +6680,9 @@ msgstr "" ...@@ -6680,6 +6680,9 @@ msgstr ""
msgid "Closes this %{quick_action_target}." msgid "Closes this %{quick_action_target}."
msgstr "" msgstr ""
msgid "Cloud licenses can not be removed."
msgstr ""
msgid "CloudLicense|Activate" msgid "CloudLicense|Activate"
msgstr "" msgstr ""
......
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