diff --git a/app/graphql/mutations/clusters/agent_tokens/create.rb b/app/graphql/mutations/clusters/agent_tokens/create.rb index c10e16333507f786a223150f9e5f5af32e229c6e..1b104652bd23ae48f85be60a5e3835eba34448c8 100644 --- a/app/graphql/mutations/clusters/agent_tokens/create.rb +++ b/app/graphql/mutations/clusters/agent_tokens/create.rb @@ -40,9 +40,9 @@ def resolve(args) result = ::Clusters::AgentTokens::CreateService .new( - container: cluster_agent.project, + agent: cluster_agent, current_user: current_user, - params: args.merge(agent_id: cluster_agent.id) + params: args ) .execute diff --git a/app/graphql/mutations/clusters/agent_tokens/revoke.rb b/app/graphql/mutations/clusters/agent_tokens/revoke.rb index 974db976f1da34ef529cd2955d34880a547219e3..6e9887999213e76a24938fa5003b5437bc743349 100644 --- a/app/graphql/mutations/clusters/agent_tokens/revoke.rb +++ b/app/graphql/mutations/clusters/agent_tokens/revoke.rb @@ -16,7 +16,8 @@ class Revoke < BaseMutation def resolve(id:) token = authorized_find!(id: id) - token.update(status: token.class.statuses[:revoked]) + + ::Clusters::AgentTokens::RevokeService.new(token: token, current_user: current_user).execute { errors: errors_on_object(token) } end diff --git a/app/services/clusters/agent_tokens/create_service.rb b/app/services/clusters/agent_tokens/create_service.rb index 2539ffdc5ba3925dc13279a8b48020527116eee0..e0c0b613adc13201fc48ed8db39dc8bdedf6430b 100644 --- a/app/services/clusters/agent_tokens/create_service.rb +++ b/app/services/clusters/agent_tokens/create_service.rb @@ -2,13 +2,21 @@ module Clusters module AgentTokens - class CreateService < ::BaseContainerService + class CreateService ALLOWED_PARAMS = %i[agent_id description name].freeze + attr_reader :agent, :current_user, :params + + def initialize(agent:, current_user:, params:) + @agent = agent + @current_user = current_user + @params = params + end + def execute - return error_no_permissions unless current_user.can?(:create_cluster, container) + return error_no_permissions unless current_user.can?(:create_cluster, agent.project) - token = ::Clusters::AgentToken.new(filtered_params.merge(created_by_user: current_user)) + token = ::Clusters::AgentToken.new(filtered_params.merge(agent_id: agent.id, created_by_user: current_user)) if token.save log_activity_event!(token) @@ -42,3 +50,5 @@ def log_activity_event!(token) end end end + +Clusters::AgentTokens::CreateService.prepend_mod diff --git a/app/services/clusters/agent_tokens/revoke_service.rb b/app/services/clusters/agent_tokens/revoke_service.rb new file mode 100644 index 0000000000000000000000000000000000000000..247cedb8e3811bb58c71ca1731cd23e6cf43d39c --- /dev/null +++ b/app/services/clusters/agent_tokens/revoke_service.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +module Clusters + module AgentTokens + class RevokeService + attr_reader :current_project, :current_user, :token + + def initialize(token:, current_user:) + @token = token + @current_user = current_user + end + + def execute + return error_no_permissions unless current_user.can?(:create_cluster, token.agent.project) + + if token.update(status: token.class.statuses[:revoked]) + ServiceResponse.success + else + ServiceResponse.error(message: token.errors.full_messages) + end + end + + private + + def error_no_permissions + ServiceResponse.error( + message: s_('ClusterAgent|User has insufficient permissions to revoke the token for this project')) + end + end + end +end + +Clusters::AgentTokens::RevokeService.prepend_mod diff --git a/doc/administration/audit_events.md b/doc/administration/audit_events.md index c51b5fbb431d7166f2e597fd3b906f4e85a771bc..71cba614b80fe71454d458cf92885f666cd572d2 100644 --- a/doc/administration/audit_events.md +++ b/doc/administration/audit_events.md @@ -314,6 +314,15 @@ The following actions on projects generate project audit events: - An environment is protected or unprotected. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/216164) in GitLab 15.8. +### GitLab agent for Kubernetes events + +The following actions on projects generate agent audit events: + +- A cluster agent token is created. + Introduced in GitLab 15.9 +- A cluster agent token is revoked. + Introduced in GitLab 15.9 + ### Instance events **(PREMIUM SELF)** The following user actions on a GitLab instance generate instance audit events: diff --git a/ee/app/services/ee/clusters/agent_tokens/create_service.rb b/ee/app/services/ee/clusters/agent_tokens/create_service.rb new file mode 100644 index 0000000000000000000000000000000000000000..9c0f54473f57dae9bf2228c70b2357fd11c1819b --- /dev/null +++ b/ee/app/services/ee/clusters/agent_tokens/create_service.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +module EE + module Clusters + module AgentTokens + module CreateService + def execute + super.tap do |response| + send_audit_event(response) + end + end + + private + + def send_audit_event(response) + message = if response.success? + "Created cluster agent token '#{response.payload[:token].name}' " \ + "with id #{response.payload[:token].id}" + else + "Attempted to create cluster agent token '#{params[:name]}' but " \ + "failed with message: #{response.message}" + end + + audit_context = { + name: 'cluster_agent_token_created', + author: current_user, + scope: agent.project, + target: agent, + message: message + } + + ::Gitlab::Audit::Auditor.audit(audit_context) + end + end + end + end +end diff --git a/ee/app/services/ee/clusters/agent_tokens/revoke_service.rb b/ee/app/services/ee/clusters/agent_tokens/revoke_service.rb new file mode 100644 index 0000000000000000000000000000000000000000..e689028214c0eb93f9a302125a4b97d92571ea05 --- /dev/null +++ b/ee/app/services/ee/clusters/agent_tokens/revoke_service.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +module EE + module Clusters + module AgentTokens + module RevokeService + def execute + super.tap do |response| + send_audit_event(token, response) + end + end + + private + + def send_audit_event(token, response) + return unless token + + message = if response.success? + "Revoked cluster agent token '#{token.name}' with id #{token.id}" + else + "Attempted to revoke cluster agent token '#{token.name}' with " \ + "id #{token.id} but failed with message: #{response.message}" + end + + audit_context = { + name: 'cluster_agent_token_revoked', + author: current_user, + scope: token.agent.project, + target: token.agent, + message: message + } + + ::Gitlab::Audit::Auditor.audit(audit_context) + end + end + end + end +end diff --git a/ee/config/audit_events/types/cluster_agent_token_created.yml b/ee/config/audit_events/types/cluster_agent_token_created.yml new file mode 100644 index 0000000000000000000000000000000000000000..4f7270e30636be908563bd6548988c9623afba5b --- /dev/null +++ b/ee/config/audit_events/types/cluster_agent_token_created.yml @@ -0,0 +1,9 @@ +--- +name: cluster_agent_token_created +description: Event triggered when a user creates a cluster agent token +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/382133 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/112036 +feature_category: kubernetes_management +milestone: '15.10' +saved_to_database: true +streamed: true diff --git a/ee/config/audit_events/types/cluster_agent_token_revoked.yml b/ee/config/audit_events/types/cluster_agent_token_revoked.yml new file mode 100644 index 0000000000000000000000000000000000000000..7f556d5b6778646fb48f4c404942fcf282366b23 --- /dev/null +++ b/ee/config/audit_events/types/cluster_agent_token_revoked.yml @@ -0,0 +1,9 @@ +--- +name: cluster_agent_token_revoked +description: Event triggered when a user revokes a cluster agent token +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/382133 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/112036 +feature_category: kubernetes_management +milestone: '15.10' +saved_to_database: true +streamed: true diff --git a/ee/spec/services/clusters/agent_tokens/create_service_audit_log_spec.rb b/ee/spec/services/clusters/agent_tokens/create_service_audit_log_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..b6e9e141189c20681c1c9648553980cd0cfc0a6d --- /dev/null +++ b/ee/spec/services/clusters/agent_tokens/create_service_audit_log_spec.rb @@ -0,0 +1,67 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Clusters::AgentTokens::CreateService, feature_category: :kubernetes_management do + describe '#execute' do + let_it_be(:agent) { create(:cluster_agent) } + + let(:user) { agent.created_by_user } + let(:project) { agent.project } + let(:params) { { name: 'some-token', description: 'Test Agent Token', agent_id: agent.id } } + + context 'when user is authorized' do + before do + project.add_maintainer(user) + end + + context 'when user creates agent token' do + it 'creates AuditEvent with success message' do + expect_to_audit( + user, + project, + agent, + /Created cluster agent token 'some-token' with id \d+/ + ) + + described_class.new(agent: agent, current_user: user, params: params).execute + end + end + end + + context 'when user is not authorized' do + let(:unauthorized_user) { create(:user) } + + before do + project.add_guest(unauthorized_user) + end + + context 'when user attempts to create agent token' do + it 'creates audit logs with failure message' do + expect_to_audit( + unauthorized_user, + project, + agent, + "Attempted to create cluster agent token 'some-token' but failed with message: " \ + 'User has insufficient permissions to create a token for this project' + ) + + described_class.new(agent: agent, current_user: unauthorized_user, params: params).execute + end + end + end + end + + def expect_to_audit(current_user, scope, target, message) + audit_context = { + name: 'cluster_agent_token_created', + author: current_user, + scope: scope, + target: target, + message: message + } + + expect(::Gitlab::Audit::Auditor).to receive(:audit).with(audit_context) + .and_call_original + end +end diff --git a/ee/spec/services/clusters/agent_tokens/revoke_service_audit_log_spec.rb b/ee/spec/services/clusters/agent_tokens/revoke_service_audit_log_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..a5ddbb27419f2e5a7bbd617ec1acf0f6bd279152 --- /dev/null +++ b/ee/spec/services/clusters/agent_tokens/revoke_service_audit_log_spec.rb @@ -0,0 +1,71 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Clusters::AgentTokens::RevokeService, feature_category: :kubernetes_management do + describe '#execute' do + let(:agent) { create(:cluster_agent) } + let(:agent_token) { create(:cluster_agent_token, agent: agent) } + let(:project) { agent.project } + let(:user) { agent.created_by_user } + + before do + project.add_maintainer(user) + end + + context 'when user is authorized' do + before do + project.add_maintainer(user) + end + + context 'when user revokes agent token' do + it 'creates AuditEvent with success message' do + expect_to_audit( + user, + project, + agent, + "Revoked cluster agent token '#{agent_token.name}' with id #{agent_token.id}" + ) + + described_class.new(token: agent_token, current_user: user).execute + end + end + end + + context 'when user is not authorized' do + let(:unauthorized_user) { create(:user) } + + before do + project.add_guest(unauthorized_user) + end + + context 'when user attempts to revoke agent token' do + it 'creates audit logs with failure message' do + expect_to_audit( + unauthorized_user, + project, + agent, + "Attempted to revoke cluster agent token '#{agent_token.name}' with " \ + "id #{agent_token.id} but failed with message: " \ + "User has insufficient permissions to revoke the token for this project" + ) + + described_class.new(token: agent_token, current_user: unauthorized_user).execute + end + end + end + end + + def expect_to_audit(current_user, scope, target, message) + audit_context = { + name: 'cluster_agent_token_revoked', + author: current_user, + scope: scope, + target: target, + message: message + } + + expect(::Gitlab::Audit::Auditor).to receive(:audit).with(audit_context) + .and_call_original + end +end diff --git a/lib/api/clusters/agent_tokens.rb b/lib/api/clusters/agent_tokens.rb index 68eef21903dfe8e10cfdf2b35f969a89f670f8d5..f0eb7ce2cd6821d5e1178168f6fb29ac6670d1b1 100644 --- a/lib/api/clusters/agent_tokens.rb +++ b/lib/api/clusters/agent_tokens.rb @@ -65,7 +65,9 @@ class AgentTokens < ::API::Base agent = ::Clusters::AgentsFinder.new(user_project, current_user).find(params[:agent_id]) result = ::Clusters::AgentTokens::CreateService.new( - container: agent.project, current_user: current_user, params: token_params.merge(agent_id: agent.id) + agent: agent, + current_user: current_user, + params: token_params ).execute bad_request!(result[:message]) if result[:status] == :error @@ -86,8 +88,9 @@ class AgentTokens < ::API::Base agent = ::Clusters::AgentsFinder.new(user_project, current_user).find(params[:agent_id]) token = ::Clusters::AgentTokensFinder.new(agent, current_user).find(params[:token_id]) - # Skipping explicit error handling and relying on exceptions - token.revoked! + result = ::Clusters::AgentTokens::RevokeService.new(token: token, current_user: current_user).execute + + bad_request!(result[:message]) if result[:status] == :error status :no_content end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index db87d0353cdd6fdc87d5f68ecb3df579574c7dcc..b23bc2e787174dcb16c7b5f7d5d90bc3d5eee1f9 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -9641,6 +9641,9 @@ msgstr "" msgid "ClusterAgent|User has insufficient permissions to create a token for this project" msgstr "" +msgid "ClusterAgent|User has insufficient permissions to revoke the token for this project" +msgstr "" + msgid "ClusterAgent|You have insufficient permissions to create a cluster agent for this project" msgstr "" diff --git a/spec/services/clusters/agent_tokens/create_service_spec.rb b/spec/services/clusters/agent_tokens/create_service_spec.rb index dc7abd1504b3c3795a2acc068ec9e38964a935e6..519a3ba7ce5327f4f81eca3c8e47548e2a01293d 100644 --- a/spec/services/clusters/agent_tokens/create_service_spec.rb +++ b/spec/services/clusters/agent_tokens/create_service_spec.rb @@ -2,14 +2,14 @@ require 'spec_helper' -RSpec.describe Clusters::AgentTokens::CreateService do - subject(:service) { described_class.new(container: project, current_user: user, params: params) } +RSpec.describe Clusters::AgentTokens::CreateService, feature_category: :kubernetes_management do + subject(:service) { described_class.new(agent: cluster_agent, current_user: user, params: params) } let_it_be(:user) { create(:user) } let(:cluster_agent) { create(:cluster_agent) } let(:project) { cluster_agent.project } - let(:params) { { agent_id: cluster_agent.id, description: 'token description', name: 'token name' } } + let(:params) { { description: 'token description', name: 'token name' } } describe '#execute' do subject { service.execute } @@ -75,7 +75,7 @@ it 'returns validation errors', :aggregate_failures do expect(subject.status).to eq(:error) - expect(subject.message).to eq(["Agent must exist", "Name can't be blank"]) + expect(subject.message).to eq(["Name can't be blank"]) end end end diff --git a/spec/services/clusters/agent_tokens/revoke_service_spec.rb b/spec/services/clusters/agent_tokens/revoke_service_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..24485a5f66db8ab4a465defa07d71abf84ed02ca --- /dev/null +++ b/spec/services/clusters/agent_tokens/revoke_service_spec.rb @@ -0,0 +1,59 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Clusters::AgentTokens::RevokeService, feature_category: :kubernetes_management do + describe '#execute' do + let(:agent) { create(:cluster_agent) } + let(:agent_token) { create(:cluster_agent_token, agent: agent) } + let(:project) { agent.project } + let(:user) { agent.created_by_user } + + before do + project.add_maintainer(user) + end + + context 'when user is authorized' do + before do + project.add_maintainer(user) + end + + context 'when user revokes agent token' do + it 'succeeds' do + described_class.new(token: agent_token, current_user: user).execute + + expect(agent_token.revoked?).to be true + end + end + + context 'when there is a validation failure' do + before do + agent_token.name = '' # make the record invalid, as we require a name to be present + end + + it 'fails without raising an error', :aggregate_failures do + result = described_class.new(token: agent_token, current_user: user).execute + + expect(result[:status]).to eq(:error) + expect(result[:message]).to eq(["Name can't be blank"]) + end + end + end + + context 'when user is not authorized' do + let(:unauthorized_user) { create(:user) } + + before do + project.add_guest(unauthorized_user) + end + + context 'when user attempts to revoke agent token' do + it 'fails' do + described_class.new(token: agent_token, current_user: unauthorized_user).execute + + expect(agent_token.revoked?).to be false + end + end + end + end +end