From 90adc1a2b8b832fc63e4e905332230d912ea65f3 Mon Sep 17 00:00:00 2001
From: Timo Furrer <tfurrer@gitlab.com>
Date: Wed, 15 Feb 2023 14:57:18 +0100
Subject: [PATCH 1/6] Add audit even for agent token creation

Changelog: added
Issue: https://gitlab.com/gitlab-org/gitlab/-/issues/382133
MR: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/112036
EE: true
---
 .../mutations/clusters/agent_tokens/create.rb |  1 +
 .../clusters/agent_tokens/create_service.rb   |  9 +++
 .../clusters/agent_tokens/create_service.rb   | 37 ++++++++++
 .../types/cluster_agent_token_created.yml     |  9 +++
 .../create_service_audit_log_spec.rb          | 67 +++++++++++++++++++
 lib/api/clusters/agent_tokens.rb              |  5 +-
 6 files changed, 127 insertions(+), 1 deletion(-)
 create mode 100644 ee/app/services/ee/clusters/agent_tokens/create_service.rb
 create mode 100644 ee/config/audit_events/types/cluster_agent_token_created.yml
 create mode 100644 ee/spec/services/clusters/agent_tokens/create_service_audit_log_spec.rb

diff --git a/app/graphql/mutations/clusters/agent_tokens/create.rb b/app/graphql/mutations/clusters/agent_tokens/create.rb
index c10e16333507f7..bb359ec134ff3f 100644
--- a/app/graphql/mutations/clusters/agent_tokens/create.rb
+++ b/app/graphql/mutations/clusters/agent_tokens/create.rb
@@ -42,6 +42,7 @@ def resolve(args)
             .new(
               container: cluster_agent.project,
               current_user: current_user,
+              agent: cluster_agent,
               params: args.merge(agent_id: cluster_agent.id)
             )
             .execute
diff --git a/app/services/clusters/agent_tokens/create_service.rb b/app/services/clusters/agent_tokens/create_service.rb
index 2539ffdc5ba392..3c7b79021748a1 100644
--- a/app/services/clusters/agent_tokens/create_service.rb
+++ b/app/services/clusters/agent_tokens/create_service.rb
@@ -5,6 +5,13 @@ module AgentTokens
     class CreateService < ::BaseContainerService
       ALLOWED_PARAMS = %i[agent_id description name].freeze
 
+      attr_reader :agent
+
+      def initialize(container:, current_user: nil, agent: nil, params: {})
+        super(container: container, current_user: current_user, params: params)
+        @agent = agent
+      end
+
       def execute
         return error_no_permissions unless current_user.can?(:create_cluster, container)
 
@@ -42,3 +49,5 @@ def log_activity_event!(token)
     end
   end
 end
+
+Clusters::AgentTokens::CreateService.prepend_mod
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 00000000000000..ef2ad4150528fa
--- /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: container,
+            target: 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 00000000000000..4f7270e30636be
--- /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/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 00000000000000..09f1bbaa8da91c
--- /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(container: project, current_user: user, agent: agent, 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(container: project, current_user: unauthorized_user, agent: agent, params: params).execute
+        end
+      end
+    end
+  end
+
+  def expect_to_audit(current_user, current_project, target, message)
+    audit_context = {
+      name: 'cluster_agent_token_created',
+      author: current_user,
+      scope: current_project,
+      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 68eef21903dfe8..2a5819ce782e95 100644
--- a/lib/api/clusters/agent_tokens.rb
+++ b/lib/api/clusters/agent_tokens.rb
@@ -65,7 +65,10 @@ 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)
+                container: agent.project,
+                current_user: current_user,
+                agent: agent,
+                params: token_params.merge(agent_id: agent.id)
               ).execute
 
               bad_request!(result[:message]) if result[:status] == :error
-- 
GitLab


From c542a55c5dcdecc1bf3aa7c9d034f4c693975655 Mon Sep 17 00:00:00 2001
From: Timo Furrer <tfurrer@gitlab.com>
Date: Wed, 15 Feb 2023 16:03:02 +0100
Subject: [PATCH 2/6] Add audit even for agent token revocation

Changelog: added
Issue: https://gitlab.com/gitlab-org/gitlab/-/issues/382133
MR: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/112036
EE: true
---
 .../mutations/clusters/agent_tokens/revoke.rb |  4 +-
 .../clusters/agent_tokens/revoke_service.rb   | 32 +++++++++
 .../clusters/agent_tokens/revoke_service.rb   | 38 ++++++++++
 .../types/cluster_agent_token_revoked.yml     |  9 +++
 .../revoke_service_audit_log_spec.rb          | 71 +++++++++++++++++++
 lib/api/clusters/agent_tokens.rb              |  3 +-
 locale/gitlab.pot                             |  3 +
 .../agent_tokens/revoke_service_spec.rb       | 46 ++++++++++++
 8 files changed, 204 insertions(+), 2 deletions(-)
 create mode 100644 app/services/clusters/agent_tokens/revoke_service.rb
 create mode 100644 ee/app/services/ee/clusters/agent_tokens/revoke_service.rb
 create mode 100644 ee/config/audit_events/types/cluster_agent_token_revoked.yml
 create mode 100644 ee/spec/services/clusters/agent_tokens/revoke_service_audit_log_spec.rb
 create mode 100644 spec/services/clusters/agent_tokens/revoke_service_spec.rb

diff --git a/app/graphql/mutations/clusters/agent_tokens/revoke.rb b/app/graphql/mutations/clusters/agent_tokens/revoke.rb
index 974db976f1da34..8fea6179144548 100644
--- a/app/graphql/mutations/clusters/agent_tokens/revoke.rb
+++ b/app/graphql/mutations/clusters/agent_tokens/revoke.rb
@@ -16,7 +16,9 @@ class Revoke < BaseMutation
 
         def resolve(id:)
           token = authorized_find!(id: id)
-          token.update(status: token.class.statuses[:revoked])
+
+          ::Clusters::AgentTokens::RevokeService.new(current_project: token.agent.project, current_user: current_user,
+                                                     token: token).execute
 
           { errors: errors_on_object(token) }
         end
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 00000000000000..9eeccf08859a61
--- /dev/null
+++ b/app/services/clusters/agent_tokens/revoke_service.rb
@@ -0,0 +1,32 @@
+# frozen_string_literal: true
+
+module Clusters
+  module AgentTokens
+    class RevokeService
+      attr_reader :current_project, :current_user, :token
+
+      def initialize(current_project:, current_user:, token:)
+        @current_project = current_project
+        @current_user = current_user
+        @token = token
+      end
+
+      def execute
+        return error_no_permissions unless current_user.can?(:create_cluster, current_project)
+
+        token.revoked!
+
+        ServiceResponse.success
+      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/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 00000000000000..52261b24043c60
--- /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: current_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_revoked.yml b/ee/config/audit_events/types/cluster_agent_token_revoked.yml
new file mode 100644
index 00000000000000..7f556d5b677864
--- /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/revoke_service_audit_log_spec.rb b/ee/spec/services/clusters/agent_tokens/revoke_service_audit_log_spec.rb
new file mode 100644
index 00000000000000..9d8ad340bac42c
--- /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(current_project: project, current_user: user, token: agent_token).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(current_project: project, current_user: unauthorized_user, token: agent_token).execute
+        end
+      end
+    end
+  end
+
+  def expect_to_audit(current_user, current_project, target, message)
+    audit_context = {
+      name: 'cluster_agent_token_revoked',
+      author: current_user,
+      scope: current_project,
+      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 2a5819ce782e95..48463ee17eae65 100644
--- a/lib/api/clusters/agent_tokens.rb
+++ b/lib/api/clusters/agent_tokens.rb
@@ -90,7 +90,8 @@ class AgentTokens < ::API::Base
               token = ::Clusters::AgentTokensFinder.new(agent, current_user).find(params[:token_id])
 
               # Skipping explicit error handling and relying on exceptions
-              token.revoked!
+              ::Clusters::AgentTokens::RevokeService.new(current_project: 
+                            agent.project, current_user: current_user, token: token).execute
 
               status :no_content
             end
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index db87d0353cdd6f..b23bc2e787174d 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/revoke_service_spec.rb b/spec/services/clusters/agent_tokens/revoke_service_spec.rb
new file mode 100644
index 00000000000000..3d16dd93a52f90
--- /dev/null
+++ b/spec/services/clusters/agent_tokens/revoke_service_spec.rb
@@ -0,0 +1,46 @@
+# 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(current_project: project, current_user: user, token: agent_token).execute
+
+          expect(agent_token.revoked?).to be true
+        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(current_project: project, current_user: unauthorized_user, token: agent_token).execute
+
+          expect(agent_token.revoked?).to be false
+        end
+      end
+    end
+  end
+end
-- 
GitLab


From dfad651c1b40f99cd5d862ec4c3c00cdd2e63690 Mon Sep 17 00:00:00 2001
From: Timo Furrer <tfurrer@gitlab.com>
Date: Wed, 15 Feb 2023 18:43:24 +0100
Subject: [PATCH 3/6] Add cluster agent token audit event docs

---
 doc/administration/audit_events.md | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/doc/administration/audit_events.md b/doc/administration/audit_events.md
index c51b5fbb431d71..71cba614b80fe7 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:
-- 
GitLab


From 0ef588cb9dc9cf685b9b5f2abef1d57c21748658 Mon Sep 17 00:00:00 2001
From: Timo Furrer <tfurrer@gitlab.com>
Date: Mon, 20 Feb 2023 09:20:41 +0100
Subject: [PATCH 4/6] Refactor AgentTokens create service to deduplicate agent
 info

---
 .../mutations/clusters/agent_tokens/create.rb       |  5 ++---
 .../clusters/agent_tokens/create_service.rb         | 13 +++++++------
 .../ee/clusters/agent_tokens/create_service.rb      |  2 +-
 .../agent_tokens/create_service_audit_log_spec.rb   |  8 ++++----
 lib/api/clusters/agent_tokens.rb                    |  7 +++----
 .../clusters/agent_tokens/create_service_spec.rb    |  8 ++++----
 6 files changed, 21 insertions(+), 22 deletions(-)

diff --git a/app/graphql/mutations/clusters/agent_tokens/create.rb b/app/graphql/mutations/clusters/agent_tokens/create.rb
index bb359ec134ff3f..1b104652bd23ae 100644
--- a/app/graphql/mutations/clusters/agent_tokens/create.rb
+++ b/app/graphql/mutations/clusters/agent_tokens/create.rb
@@ -40,10 +40,9 @@ def resolve(args)
 
           result = ::Clusters::AgentTokens::CreateService
             .new(
-              container: cluster_agent.project,
-              current_user: current_user,
               agent: cluster_agent,
-              params: args.merge(agent_id: cluster_agent.id)
+              current_user: current_user,
+              params: args
             )
             .execute
 
diff --git a/app/services/clusters/agent_tokens/create_service.rb b/app/services/clusters/agent_tokens/create_service.rb
index 3c7b79021748a1..e0c0b613adc132 100644
--- a/app/services/clusters/agent_tokens/create_service.rb
+++ b/app/services/clusters/agent_tokens/create_service.rb
@@ -2,20 +2,21 @@
 
 module Clusters
   module AgentTokens
-    class CreateService < ::BaseContainerService
+    class CreateService
       ALLOWED_PARAMS = %i[agent_id description name].freeze
 
-      attr_reader :agent
+      attr_reader :agent, :current_user, :params
 
-      def initialize(container:, current_user: nil, agent: nil, params: {})
-        super(container: container, current_user: current_user, params: 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)
diff --git a/ee/app/services/ee/clusters/agent_tokens/create_service.rb b/ee/app/services/ee/clusters/agent_tokens/create_service.rb
index ef2ad4150528fa..9c0f54473f57da 100644
--- a/ee/app/services/ee/clusters/agent_tokens/create_service.rb
+++ b/ee/app/services/ee/clusters/agent_tokens/create_service.rb
@@ -24,7 +24,7 @@ def send_audit_event(response)
           audit_context = {
             name: 'cluster_agent_token_created',
             author: current_user,
-            scope: container,
+            scope: agent.project,
             target: agent,
             message: message
           }
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
index 09f1bbaa8da91c..b6e9e141189c20 100644
--- 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
@@ -24,7 +24,7 @@
             /Created cluster agent token 'some-token' with id \d+/
           )
 
-          described_class.new(container: project, current_user: user, agent: agent, params: params).execute
+          described_class.new(agent: agent, current_user: user, params: params).execute
         end
       end
     end
@@ -46,17 +46,17 @@
             'User has insufficient permissions to create a token for this project'
           )
 
-          described_class.new(container: project, current_user: unauthorized_user, agent: agent, params: params).execute
+          described_class.new(agent: agent, current_user: unauthorized_user, params: params).execute
         end
       end
     end
   end
 
-  def expect_to_audit(current_user, current_project, target, message)
+  def expect_to_audit(current_user, scope, target, message)
     audit_context = {
       name: 'cluster_agent_token_created',
       author: current_user,
-      scope: current_project,
+      scope: scope,
       target: target,
       message: message
     }
diff --git a/lib/api/clusters/agent_tokens.rb b/lib/api/clusters/agent_tokens.rb
index 48463ee17eae65..58d4da5f4b9a92 100644
--- a/lib/api/clusters/agent_tokens.rb
+++ b/lib/api/clusters/agent_tokens.rb
@@ -65,10 +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,
                 agent: agent,
-                params: token_params.merge(agent_id: agent.id)
+                current_user: current_user,
+                params: token_params
               ).execute
 
               bad_request!(result[:message]) if result[:status] == :error
@@ -90,7 +89,7 @@ class AgentTokens < ::API::Base
               token = ::Clusters::AgentTokensFinder.new(agent, current_user).find(params[:token_id])
 
               # Skipping explicit error handling and relying on exceptions
-              ::Clusters::AgentTokens::RevokeService.new(current_project: 
+              ::Clusters::AgentTokens::RevokeService.new(current_project:
                             agent.project, current_user: current_user, token: token).execute
 
               status :no_content
diff --git a/spec/services/clusters/agent_tokens/create_service_spec.rb b/spec/services/clusters/agent_tokens/create_service_spec.rb
index dc7abd1504b3c3..519a3ba7ce5327 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
-- 
GitLab


From b178acd15e78aa8a5b5291709e8b312fd4f7716d Mon Sep 17 00:00:00 2001
From: Timo Furrer <tfurrer@gitlab.com>
Date: Mon, 20 Feb 2023 09:25:20 +0100
Subject: [PATCH 5/6] Refactor AgentTokens revoke service to deduplicate agent
 info

---
 app/graphql/mutations/clusters/agent_tokens/revoke.rb     | 3 +--
 app/services/clusters/agent_tokens/revoke_service.rb      | 7 +++----
 .../services/ee/clusters/agent_tokens/revoke_service.rb   | 2 +-
 .../agent_tokens/revoke_service_audit_log_spec.rb         | 8 ++++----
 lib/api/clusters/agent_tokens.rb                          | 3 +--
 .../services/clusters/agent_tokens/revoke_service_spec.rb | 4 ++--
 6 files changed, 12 insertions(+), 15 deletions(-)

diff --git a/app/graphql/mutations/clusters/agent_tokens/revoke.rb b/app/graphql/mutations/clusters/agent_tokens/revoke.rb
index 8fea6179144548..6e9887999213e7 100644
--- a/app/graphql/mutations/clusters/agent_tokens/revoke.rb
+++ b/app/graphql/mutations/clusters/agent_tokens/revoke.rb
@@ -17,8 +17,7 @@ class Revoke < BaseMutation
         def resolve(id:)
           token = authorized_find!(id: id)
 
-          ::Clusters::AgentTokens::RevokeService.new(current_project: token.agent.project, current_user: current_user,
-                                                     token: token).execute
+          ::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/revoke_service.rb b/app/services/clusters/agent_tokens/revoke_service.rb
index 9eeccf08859a61..50efeba49c1e2c 100644
--- a/app/services/clusters/agent_tokens/revoke_service.rb
+++ b/app/services/clusters/agent_tokens/revoke_service.rb
@@ -5,14 +5,13 @@ module AgentTokens
     class RevokeService
       attr_reader :current_project, :current_user, :token
 
-      def initialize(current_project:, current_user:, token:)
-        @current_project = current_project
-        @current_user = current_user
+      def initialize(token:, current_user:)
         @token = token
+        @current_user = current_user
       end
 
       def execute
-        return error_no_permissions unless current_user.can?(:create_cluster, current_project)
+        return error_no_permissions unless current_user.can?(:create_cluster, token.agent.project)
 
         token.revoked!
 
diff --git a/ee/app/services/ee/clusters/agent_tokens/revoke_service.rb b/ee/app/services/ee/clusters/agent_tokens/revoke_service.rb
index 52261b24043c60..e689028214c0eb 100644
--- a/ee/app/services/ee/clusters/agent_tokens/revoke_service.rb
+++ b/ee/app/services/ee/clusters/agent_tokens/revoke_service.rb
@@ -25,7 +25,7 @@ def send_audit_event(token, response)
           audit_context = {
             name: 'cluster_agent_token_revoked',
             author: current_user,
-            scope: current_project,
+            scope: token.agent.project,
             target: token.agent,
             message: message
           }
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
index 9d8ad340bac42c..a5ddbb27419f2e 100644
--- 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
@@ -27,7 +27,7 @@
             "Revoked cluster agent token '#{agent_token.name}' with id #{agent_token.id}"
           )
 
-          described_class.new(current_project: project, current_user: user, token: agent_token).execute
+          described_class.new(token: agent_token, current_user: user).execute
         end
       end
     end
@@ -50,17 +50,17 @@
             "User has insufficient permissions to revoke the token for this project"
           )
 
-          described_class.new(current_project: project, current_user: unauthorized_user, token: agent_token).execute
+          described_class.new(token: agent_token, current_user: unauthorized_user).execute
         end
       end
     end
   end
 
-  def expect_to_audit(current_user, current_project, target, message)
+  def expect_to_audit(current_user, scope, target, message)
     audit_context = {
       name: 'cluster_agent_token_revoked',
       author: current_user,
-      scope: current_project,
+      scope: scope,
       target: target,
       message: message
     }
diff --git a/lib/api/clusters/agent_tokens.rb b/lib/api/clusters/agent_tokens.rb
index 58d4da5f4b9a92..f52f9a2ed9cafe 100644
--- a/lib/api/clusters/agent_tokens.rb
+++ b/lib/api/clusters/agent_tokens.rb
@@ -89,8 +89,7 @@ class AgentTokens < ::API::Base
               token = ::Clusters::AgentTokensFinder.new(agent, current_user).find(params[:token_id])
 
               # Skipping explicit error handling and relying on exceptions
-              ::Clusters::AgentTokens::RevokeService.new(current_project:
-                            agent.project, current_user: current_user, token: token).execute
+              ::Clusters::AgentTokens::RevokeService.new(token: token, current_user: current_user).execute
 
               status :no_content
             end
diff --git a/spec/services/clusters/agent_tokens/revoke_service_spec.rb b/spec/services/clusters/agent_tokens/revoke_service_spec.rb
index 3d16dd93a52f90..283488b28cbf18 100644
--- a/spec/services/clusters/agent_tokens/revoke_service_spec.rb
+++ b/spec/services/clusters/agent_tokens/revoke_service_spec.rb
@@ -20,7 +20,7 @@
 
       context 'when user revokes agent token' do
         it 'succeeds' do
-          described_class.new(current_project: project, current_user: user, token: agent_token).execute
+          described_class.new(token: agent_token, current_user: user).execute
 
           expect(agent_token.revoked?).to be true
         end
@@ -36,7 +36,7 @@
 
       context 'when user attempts to revoke agent token' do
         it 'fails' do
-          described_class.new(current_project: project, current_user: unauthorized_user, token: agent_token).execute
+          described_class.new(token: agent_token, current_user: unauthorized_user).execute
 
           expect(agent_token.revoked?).to be false
         end
-- 
GitLab


From 07348380dfc2a927644e4617b11fca02950c7866 Mon Sep 17 00:00:00 2001
From: Timo Furrer <tfurrer@gitlab.com>
Date: Tue, 21 Feb 2023 09:22:00 +0100
Subject: [PATCH 6/6] Refactor AgentTokens revoke service to return service
 response error on failure

---
 .../clusters/agent_tokens/revoke_service.rb         |  8 +++++---
 lib/api/clusters/agent_tokens.rb                    |  5 +++--
 .../clusters/agent_tokens/revoke_service_spec.rb    | 13 +++++++++++++
 3 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/app/services/clusters/agent_tokens/revoke_service.rb b/app/services/clusters/agent_tokens/revoke_service.rb
index 50efeba49c1e2c..247cedb8e3811b 100644
--- a/app/services/clusters/agent_tokens/revoke_service.rb
+++ b/app/services/clusters/agent_tokens/revoke_service.rb
@@ -13,9 +13,11 @@ def initialize(token:, current_user:)
       def execute
         return error_no_permissions unless current_user.can?(:create_cluster, token.agent.project)
 
-        token.revoked!
-
-        ServiceResponse.success
+        if token.update(status: token.class.statuses[:revoked])
+          ServiceResponse.success
+        else
+          ServiceResponse.error(message: token.errors.full_messages)
+        end
       end
 
       private
diff --git a/lib/api/clusters/agent_tokens.rb b/lib/api/clusters/agent_tokens.rb
index f52f9a2ed9cafe..f0eb7ce2cd6821 100644
--- a/lib/api/clusters/agent_tokens.rb
+++ b/lib/api/clusters/agent_tokens.rb
@@ -88,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
-              ::Clusters::AgentTokens::RevokeService.new(token: token, current_user: current_user).execute
+              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/spec/services/clusters/agent_tokens/revoke_service_spec.rb b/spec/services/clusters/agent_tokens/revoke_service_spec.rb
index 283488b28cbf18..24485a5f66db8a 100644
--- a/spec/services/clusters/agent_tokens/revoke_service_spec.rb
+++ b/spec/services/clusters/agent_tokens/revoke_service_spec.rb
@@ -25,6 +25,19 @@
           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
-- 
GitLab