diff --git a/db/migrate/20251217053122_add_status_to_ci_runner_controller_tokens.rb b/db/migrate/20251217053122_add_status_to_ci_runner_controller_tokens.rb new file mode 100644 index 0000000000000000000000000000000000000000..f63eb34a4b2d6026652eb3161dcff67866ab6184 --- /dev/null +++ b/db/migrate/20251217053122_add_status_to_ci_runner_controller_tokens.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +class AddStatusToCiRunnerControllerTokens < Gitlab::Database::Migration[2.3] + disable_ddl_transaction! + + milestone '18.8' + + INDEX_NAME = 'index_ci_runner_controller_tokens_on_rc_id_and_status' + OLD_INDEX_NAME = 'index_ci_rac_tokens_on_rac_id' + + def up + add_column :ci_runner_controller_tokens, :status, :integer, limit: 2, default: 0, null: false + + add_concurrent_index :ci_runner_controller_tokens, [:runner_controller_id, :status], name: INDEX_NAME + remove_concurrent_index_by_name :ci_runner_controller_tokens, OLD_INDEX_NAME + end + + def down + add_concurrent_index :ci_runner_controller_tokens, :runner_controller_id, name: OLD_INDEX_NAME + + remove_column :ci_runner_controller_tokens, :status + end +end diff --git a/db/schema_migrations/20251217053122 b/db/schema_migrations/20251217053122 new file mode 100644 index 0000000000000000000000000000000000000000..a1d385731336b524918dfda899f78ba033d97b70 --- /dev/null +++ b/db/schema_migrations/20251217053122 @@ -0,0 +1 @@ +15797b51433fc53b79cf313292896e3c308b77ca2881d925103bd4eb305ae07d \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 99384eb26ec0eb1e35bdd2216a5ebd2ac2fff682..e5e65b7e5dbfeefb3cdad1d8b429a94de07b09d8 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -15084,6 +15084,7 @@ CREATE TABLE ci_runner_controller_tokens ( runner_controller_id bigint NOT NULL, created_at timestamp with time zone NOT NULL, updated_at timestamp with time zone NOT NULL, + status smallint DEFAULT 0 NOT NULL, CONSTRAINT check_84d7d76c86 CHECK ((char_length(description) <= 1024)), CONSTRAINT check_ec7c3fc764 CHECK ((char_length(token_digest) <= 255)) ); @@ -41234,8 +41235,6 @@ CREATE UNIQUE INDEX index_ci_project_mirrors_on_project_id ON ci_project_mirrors CREATE UNIQUE INDEX index_ci_project_monthly_usages_on_project_id_and_date ON ci_project_monthly_usages USING btree (project_id, date); -CREATE INDEX index_ci_rac_tokens_on_rac_id ON ci_runner_controller_tokens USING btree (runner_controller_id); - CREATE UNIQUE INDEX index_ci_refs_on_project_id_and_ref_path ON ci_refs USING btree (project_id, ref_path); CREATE UNIQUE INDEX index_ci_resource_groups_on_project_id_and_key ON ci_resource_groups USING btree (project_id, key); @@ -41246,6 +41245,8 @@ CREATE INDEX index_ci_resources_on_project_id ON ci_resources USING btree (proje CREATE UNIQUE INDEX index_ci_resources_on_resource_group_id_and_build_id ON ci_resources USING btree (resource_group_id, build_id); +CREATE INDEX index_ci_runner_controller_tokens_on_rc_id_and_status ON ci_runner_controller_tokens USING btree (runner_controller_id, status); + CREATE UNIQUE INDEX index_ci_runner_controller_tokens_on_token_digest ON ci_runner_controller_tokens USING btree (token_digest); CREATE INDEX index_ci_runner_machines_on_executor_type ON ONLY ci_runner_machines USING btree (executor_type); diff --git a/ee/app/models/ci/runner_controller_token.rb b/ee/app/models/ci/runner_controller_token.rb index f19351201cf854786b095457d5698929a6bf9e43..43145f81421627aea2e0bfb4f0926529340227df 100644 --- a/ee/app/models/ci/runner_controller_token.rb +++ b/ee/app/models/ci/runner_controller_token.rb @@ -18,12 +18,21 @@ class RunnerControllerToken < Ci::ApplicationRecord before_create :ensure_token - private + enum :status, { + active: 0, + revoked: 1 + } def self.token_prefix ::Authn::TokenField::PrefixHelper.prepend_instance_prefix(TOKEN_PREFIX) end + def revoke! + update(status: :revoked) + end + + private + def token_prefix self.class.token_prefix end diff --git a/ee/app/services/ci/runner_controllers/revoke_token_service.rb b/ee/app/services/ci/runner_controllers/revoke_token_service.rb new file mode 100644 index 0000000000000000000000000000000000000000..8b0de0fbc0dbea94dbdf5a96acd9ba3b0b9099f5 --- /dev/null +++ b/ee/app/services/ci/runner_controllers/revoke_token_service.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +module Ci + module RunnerControllers + class RevokeTokenService + attr_reader :token, :current_user + + def initialize(token:, current_user:) + @token = token + @current_user = current_user + end + + def execute + return error_no_permissions unless current_user.can_admin_all_resources? + + if token.revoke! + ServiceResponse.success + else + ServiceResponse.error(message: token.errors.full_messages) + end + end + + private + + def error_no_permissions + ServiceResponse.error(message: 'Administrator permission is required to revoke this token') + end + end + end +end diff --git a/ee/lib/api/ci/runner_controller_tokens.rb b/ee/lib/api/ci/runner_controller_tokens.rb index 523edd7d2d858ebe0aad0c08693a0fc98a841647..d98037625fa74df1360fc2449209d1899deb8604 100644 --- a/ee/lib/api/ci/runner_controller_tokens.rb +++ b/ee/lib/api/ci/runner_controller_tokens.rb @@ -32,7 +32,7 @@ class RunnerControllerTokens < ::API::Base controller = ::Ci::RunnerController.find_by_id(params[:runner_controller_id]) not_found! unless controller - tokens = controller.tokens + tokens = controller.tokens.active present paginate(tokens), with: Entities::Ci::RunnerControllerToken end @@ -53,7 +53,7 @@ class RunnerControllerTokens < ::API::Base controller = ::Ci::RunnerController.find_by_id(params[:runner_controller_id]) not_found! unless controller - token = controller.tokens.find_by_id(params[:id]) + token = controller.tokens.active.find_by_id(params[:id]) if token present token, with: Entities::Ci::RunnerControllerToken else @@ -88,6 +88,36 @@ class RunnerControllerTokens < ::API::Base bad_request!(token.errors.full_messages.to_sentence) end end + + desc 'Revoke a runner controller token' do + detail 'Revoke a token for a specific runner controller.' + success code: 204 + failure [ + { code: 403, message: 'Forbidden' }, + { code: 400, message: 'Bad Request' }, + { code: 404, message: 'Not found' } + ] + tags %w[runner_controller_tokens] + end + params do + requires :runner_controller_id, type: Integer, desc: 'ID of the runner controller' + requires :id, type: Integer, desc: 'ID of the runner controller token' + end + delete ':runner_controller_id/tokens/:id' do + controller = ::Ci::RunnerController.find_by_id(params[:runner_controller_id]) + not_found! unless controller + + token = controller.tokens.active.find_by_id(params[:id]) + not_found! unless token + + result = ::Ci::RunnerControllers::RevokeTokenService.new(token:, current_user:).execute + + if result.success? + no_content! + else + bad_request!(result.message.to_sentence) + end + end end end end diff --git a/spec/factories/ci/runner_controller_tokens.rb b/ee/spec/factories/ci/runner_controller_tokens.rb similarity index 81% rename from spec/factories/ci/runner_controller_tokens.rb rename to ee/spec/factories/ci/runner_controller_tokens.rb index 6d33db13556215eb00d3a8254600721e7f9e4e8d..2684591fdbb9f9b40c1d96c79c18f5c6789399b7 100644 --- a/spec/factories/ci/runner_controller_tokens.rb +++ b/ee/spec/factories/ci/runner_controller_tokens.rb @@ -5,5 +5,9 @@ association :runner_controller, factory: :ci_runner_controller description { "Token for runner controller" } + + trait :revoked do + status { :revoked } + end end end diff --git a/ee/spec/models/ci/runner_controller_token_spec.rb b/ee/spec/models/ci/runner_controller_token_spec.rb index 54341a755874d0a67de57a84bfc24ddda091339f..0a136ac67e4ba4108425a43d51c4ea10a14c5d6b 100644 --- a/ee/spec/models/ci/runner_controller_token_spec.rb +++ b/ee/spec/models/ci/runner_controller_token_spec.rb @@ -35,4 +35,29 @@ token.save! end end + + describe 'enums' do + it { is_expected.to define_enum_for(:status).with_values(active: 0, revoked: 1) } + end + + describe 'scopes' do + describe '.active' do + it 'returns only active tokens' do + active_token = create(:ci_runner_controller_token, status: :active) + revoked_token = create(:ci_runner_controller_token, status: :revoked) + + expect(described_class.active).to include(active_token) + expect(described_class.active).not_to include(revoked_token) + end + end + end + + describe '#revoke' do + it 'sets the status to revoked' do + token = create(:ci_runner_controller_token, status: :active) + token.revoke! + + expect(token.status).to eq('revoked') + end + end end diff --git a/ee/spec/requests/api/ci/runner_controller_tokens_spec.rb b/ee/spec/requests/api/ci/runner_controller_tokens_spec.rb index db55d787af75ca8ce7b752275d448f242f05af8e..81507fe0ef5172e19b21b827816b43f7609ed933 100644 --- a/ee/spec/requests/api/ci/runner_controller_tokens_spec.rb +++ b/ee/spec/requests/api/ci/runner_controller_tokens_spec.rb @@ -31,13 +31,17 @@ describe 'GET /runner_controllers/:runner_controller_id/tokens' do context 'when user is admin' do - it 'returns a list of runner controller tokens' do + it 'returns a list of active runner controller tokens' do create_list(:ci_runner_controller_token, 2, runner_controller: runner_controller) + revoked_token = create(:ci_runner_controller_token, :revoked, runner_controller: runner_controller) get api(path, admin, admin_mode: true) expect(response).to have_gitlab_http_status(:ok) expect(json_response.size).to eq(3) + json_response.each do |token_response| + expect(token_response['id']).not_to eq(revoked_token.id) + end end end @@ -81,6 +85,14 @@ it_behaves_like 'returns status 404 (not found)' end + + context 'when the token is revoked' do + let(:revoked_token) { create(:ci_runner_controller_token, :revoked, runner_controller: runner_controller) } + + subject(:call_endpoint) { get api("#{path}/#{revoked_token.id}", admin, admin_mode: true) } + + it_behaves_like 'returns status 404 (not found)' + end end context 'when user is not admin' do @@ -167,4 +179,62 @@ it_behaves_like 'returns status 404 (not found)' end end + + describe 'DELETE /runner_controllers/:runner_controller_id/tokens/:id' do + context 'when user is admin' do + it 'revokes the runner controller token' do + delete api("#{path}/#{token.id}", admin, admin_mode: true) + + expect(response).to have_gitlab_http_status(:no_content) + expect(token.reload.revoked?).to be true + end + + context 'when token revoke fails' do + before do + allow(Ci::RunnerControllerToken).to receive(:find_by_id).and_return(token) + allow(token).to receive(:revoke!).and_return(false) + allow(token).to receive_message_chain(:errors, :full_messages).and_return(['Revoke failed']) + end + + it 'returns 400 with error message' do + delete api("#{path}/#{token.id}", admin, admin_mode: true) + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']).to eq('400 Bad request - Revoke failed') + end + end + + context 'when runner controller does not exist' do + subject(:call_endpoint) do + delete api("/runner_controllers/#{non_existing_record_id}/tokens/#{token.id}", admin, admin_mode: true) + end + + it_behaves_like 'returns status 404 (not found)' + end + + context 'when runner controller token does not exist' do + subject(:call_endpoint) do + delete api("#{path}/#{non_existing_record_id}", admin, admin_mode: true) + end + + it_behaves_like 'returns status 404 (not found)' + end + + context 'when the token is already revoked' do + let(:revoked_token) { create(:ci_runner_controller_token, :revoked, runner_controller: runner_controller) } + + subject(:call_endpoint) { delete api("#{path}/#{revoked_token.id}", admin, admin_mode: true) } + + it_behaves_like 'returns status 404 (not found)' + end + end + + context 'when user is not admin' do + it 'returns status 403 (forbidden)' do + delete api("#{path}/#{token.id}", non_admin_user) + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + end end diff --git a/ee/spec/services/ci/runner_controllers/revoke_token_service_spec.rb b/ee/spec/services/ci/runner_controllers/revoke_token_service_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..99a75cb0f6b620272d4c6dd77941269056dede73 --- /dev/null +++ b/ee/spec/services/ci/runner_controllers/revoke_token_service_spec.rb @@ -0,0 +1,67 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ci::RunnerControllers::RevokeTokenService, feature_category: :continuous_integration do + include AdminModeHelper + + let(:runner_controller) { create(:ci_runner_controller) } + let(:runner_controller_token) { create(:ci_runner_controller_token, runner_controller: runner_controller) } + let(:admin_user) { create(:admin) } + let(:non_admin_user) { create(:user) } + + describe '#execute' do + subject(:execute) { described_class.new(token: runner_controller_token, current_user: current_user).execute } + + context 'when the user is an admin' do + let(:current_user) { admin_user } + + before do + enable_admin_mode!(current_user) + end + + it 'successfully revokes the token' do + execute + + expect(runner_controller_token.reload.revoked?).to be true + end + + it 'returns a success response' do + response = execute + + expect(response).to be_success + end + + context 'when it fails' do + before do + allow(runner_controller_token).to receive(:revoke!).and_return(false) + allow(runner_controller_token).to receive_message_chain(:errors, :full_messages).and_return(['Some error']) + end + + it 'returns an error response' do + response = execute + + expect(response).to be_error + expect(response.message).to eq(['Some error']) + end + end + end + + context 'when the user is not an admin' do + let(:current_user) { non_admin_user } + + it 'returns an error response indicating insufficient permissions' do + response = execute + + expect(response).to be_error + expect(response.message).to eq('Administrator permission is required to revoke this token') + end + + it 'does not revoke the token' do + execute + + expect(runner_controller_token.reload.active?).to be true + end + end + end +end