From ba18d010f81639617888719c833ec20f3e2226a8 Mon Sep 17 00:00:00 2001
From: Aaron Huntsman <ahuntsman@gitlab.com>
Date: Wed, 28 Aug 2024 22:45:26 -0500
Subject: [PATCH 01/10] Create audit event for container_repository_created

New Container Repository records are created as needed when authenticating
to the container registry. This update audits when a new repository is
created this way.

Changelog: added
EE: true
---
 app/models/container_repository.rb            |  6 ++++--
 ...ntainer_registry_authentication_service.rb |  5 +++++
 doc/user/compliance/audit_event_types.md      |  1 +
 ...ntainer_registry_authentication_service.rb | 19 +++++++++++++++++++
 .../types/container_repository_created.yml    | 10 ++++++++++
 5 files changed, 39 insertions(+), 2 deletions(-)
 create mode 100644 ee/config/audit_events/types/container_repository_created.yml

diff --git a/app/models/container_repository.rb b/app/models/container_repository.rb
index 87f0b27a932096..55958055c966fe 100644
--- a/app/models/container_repository.rb
+++ b/app/models/container_repository.rb
@@ -272,13 +272,15 @@ def self.build_from_path(path)
     self.new(project: path.repository_project, name: path.repository_name)
   end
 
-  def self.find_or_create_from_path!(path)
+  def self.find_or_create_from_path!(path, check_created: false)
+    created = check_created && !ContainerRepository.find_by_path(path)
+
     ContainerRepository.upsert({
       project_id: path.repository_project.id,
       name: path.repository_name
     }, unique_by: %i[project_id name])
 
-    find_by_path!(path)
+    check_created ? [find_by_path!(path), created] : find_by_path!(path)
   end
 
   def self.build_root_repository(project)
diff --git a/app/services/auth/container_registry_authentication_service.rb b/app/services/auth/container_registry_authentication_service.rb
index 829c2f614d5bd1..c5b806bb0406ea 100644
--- a/app/services/auth/container_registry_authentication_service.rb
+++ b/app/services/auth/container_registry_authentication_service.rb
@@ -226,6 +226,11 @@ def ensure_container_repository!(path, actions)
       return if path.has_repository?
       return unless actions.include?('push')
 
+      find_or_create_repository(path)
+    end
+
+    # Overridden in EE
+    def find_or_create_repository(path)
       ContainerRepository.find_or_create_from_path!(path)
     end
 
diff --git a/doc/user/compliance/audit_event_types.md b/doc/user/compliance/audit_event_types.md
index 6f13626848a6c1..4b31ae770590ca 100644
--- a/doc/user/compliance/audit_event_types.md
+++ b/doc/user/compliance/audit_event_types.md
@@ -193,6 +193,7 @@ Audit event types belong to the following product categories.
 
 | Name | Description | Saved to database | Streamed | Introduced in | Scope |
 |:------------|:------------|:------------------|:---------|:--------------|:--------------|
+| [`container_repository_created`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/156037) | Triggered when a container repository is first pushed to the registry | **{check-circle}** Yes | **{check-circle}** Yes | GitLab [17.4](https://gitlab.com/gitlab-org/gitlab/-/issues/362290) | Project |
 | [`container_repository_deleted`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/152967) | Triggered when a project's container registry is deleted | **{check-circle}** Yes | **{check-circle}** Yes | GitLab [17.2](https://gitlab.com/gitlab-org/gitlab/-/issues/362290) | Project |
 | [`container_repository_deletion_marked`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/152967) | Triggered when a project's container repository is marked for deletion | **{check-circle}** Yes | **{check-circle}** Yes | GitLab [17.2](https://gitlab.com/gitlab-org/gitlab/-/issues/362290) | Project |
 | [`container_repository_tags_deleted`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/156066) | Triggered when a project's container repository tag is deleted | **{check-circle}** Yes | **{check-circle}** Yes | GitLab [17.2](https://gitlab.com/gitlab-org/gitlab/-/issues/362290) | Project |
diff --git a/ee/app/services/ee/auth/container_registry_authentication_service.rb b/ee/app/services/ee/auth/container_registry_authentication_service.rb
index 1de0dc9d870d4d..1297f9bb230a0f 100644
--- a/ee/app/services/ee/auth/container_registry_authentication_service.rb
+++ b/ee/app/services/ee/auth/container_registry_authentication_service.rb
@@ -47,6 +47,25 @@ def extra_info
         })
       end
 
+      override :find_or_create_repository
+      def find_or_create_repository(path)
+        repository, created = ContainerRepository.find_or_create_from_path!(path, include_inserted: true)
+
+        if created
+          audit_context = {
+            name: "container_repository_created",
+            author: ::Gitlab::Audit::UnauthenticatedAuthor.new(name: '(System)'),
+            scope: repository.project,
+            target: repository,
+            message: "Container repository #{repository.id} created"
+          }
+
+          ::Gitlab::Audit::Auditor.audit(audit_context)
+        end
+
+        repository
+      end
+
       def access_denied_in_maintenance_mode?
         @access_denied_in_maintenance_mode
       end
diff --git a/ee/config/audit_events/types/container_repository_created.yml b/ee/config/audit_events/types/container_repository_created.yml
new file mode 100644
index 00000000000000..0a22443eb000bb
--- /dev/null
+++ b/ee/config/audit_events/types/container_repository_created.yml
@@ -0,0 +1,10 @@
+---
+name: container_repository_created
+description: Triggered when a container repository is first pushed to the registry
+introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/362290
+introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/156037
+feature_category: container_registry
+milestone: '17.4'
+saved_to_database: true
+streamed: true
+scope: [Project]
\ No newline at end of file
-- 
GitLab


From 20cc8ce3513aad4d1330ec1b42f7171c73a9bebd Mon Sep 17 00:00:00 2001
From: Aaron Huntsman <ahuntsman@gitlab.com>
Date: Thu, 29 Aug 2024 10:54:13 -0500
Subject: [PATCH 02/10] Add specs for updated
 ContainerRepository.find_or_create_by_path

---
 ...ntainer_registry_authentication_service.rb |  2 +-
 spec/models/container_repository_spec.rb      | 29 +++++++++++++++++++
 2 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/ee/app/services/ee/auth/container_registry_authentication_service.rb b/ee/app/services/ee/auth/container_registry_authentication_service.rb
index 1297f9bb230a0f..06bd2e599d94d6 100644
--- a/ee/app/services/ee/auth/container_registry_authentication_service.rb
+++ b/ee/app/services/ee/auth/container_registry_authentication_service.rb
@@ -49,7 +49,7 @@ def extra_info
 
       override :find_or_create_repository
       def find_or_create_repository(path)
-        repository, created = ContainerRepository.find_or_create_from_path!(path, include_inserted: true)
+        repository, created = ::ContainerRepository.find_or_create_from_path!(path, check_created: true)
 
         if created
           audit_context = {
diff --git a/spec/models/container_repository_spec.rb b/spec/models/container_repository_spec.rb
index be22b71342ad22..4b46ed9295eaad 100644
--- a/spec/models/container_repository_spec.rb
+++ b/spec/models/container_repository_spec.rb
@@ -854,6 +854,18 @@ def expected_tags_from(client_tags)
 
     let(:repository_path) { ContainerRegistry::Path.new(path) }
 
+    shared_examples 'new path with check_created set' do
+      let(:repository_with_created) do
+        described_class.find_or_create_from_path!(ContainerRegistry::Path.new(path), check_created: true)
+      end
+
+      it 'returns true created value for a new path' do
+        _, created = repository_with_created
+
+        expect(created).to eq(true)
+      end
+    end
+
     context 'when received multi-level repository path' do
       let(:path) { project.full_path + '/some/image' }
 
@@ -864,6 +876,8 @@ def expected_tags_from(client_tags)
       it 'fabricates repository with a correct name' do
         expect(repository.name).to eq 'some/image'
       end
+
+      it_behaves_like 'new path with check_created set'
     end
 
     context 'when path is too long' do
@@ -892,6 +906,8 @@ def expected_tags_from(client_tags)
       it 'has path including a nested group' do
         expect(repository.path).to include 'nested/test/some/image'
       end
+
+      it_behaves_like 'new path with check_created set'
     end
 
     context 'when received root repository path' do
@@ -904,6 +920,8 @@ def expected_tags_from(client_tags)
       it 'fabricates repository with an empty name' do
         expect(repository.name).to be_empty
       end
+
+      it_behaves_like 'new path with check_created set'
     end
 
     context 'when repository already exists' do
@@ -914,6 +932,17 @@ def expected_tags_from(client_tags)
 
         expect(repository.id).to eq(container_repository.id)
       end
+
+      context 'with check_created set' do
+        it 'returns the existing repository with false created value' do
+          container_repository = create(:container_repository, project: project, name: 'some/image')
+
+          repository, created = described_class.find_or_create_from_path!(ContainerRegistry::Path.new(path), check_created: true)
+
+          expect(repository.id).to eq(container_repository.id)
+          expect(created).to eq(false)
+        end
+      end
     end
 
     context 'when many of the same repository are created at the same time' do
-- 
GitLab


From 03ae469ee97f6e0e905593766cf1f9169c407714 Mon Sep 17 00:00:00 2001
From: Aaron Huntsman <ahuntsman@gitlab.com>
Date: Tue, 3 Sep 2024 23:39:37 -0500
Subject: [PATCH 03/10] Add specs for container_repository_created audit in
 registry auth service

---
 ...ntainer_registry_authentication_service.rb |  2 +-
 ...er_registry_authentication_service_spec.rb | 43 +++++++++++++++++++
 2 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/ee/app/services/ee/auth/container_registry_authentication_service.rb b/ee/app/services/ee/auth/container_registry_authentication_service.rb
index 06bd2e599d94d6..e2571a6b049d70 100644
--- a/ee/app/services/ee/auth/container_registry_authentication_service.rb
+++ b/ee/app/services/ee/auth/container_registry_authentication_service.rb
@@ -54,7 +54,7 @@ def find_or_create_repository(path)
         if created
           audit_context = {
             name: "container_repository_created",
-            author: ::Gitlab::Audit::UnauthenticatedAuthor.new(name: '(System)'),
+            author: current_user || ::Gitlab::Audit::UnauthenticatedAuthor.new(name: '(System)'),
             scope: repository.project,
             target: repository,
             message: "Container repository #{repository.id} created"
diff --git a/ee/spec/services/ee/auth/container_registry_authentication_service_spec.rb b/ee/spec/services/ee/auth/container_registry_authentication_service_spec.rb
index de79414774dd4a..5e8847aa173e4b 100644
--- a/ee/spec/services/ee/auth/container_registry_authentication_service_spec.rb
+++ b/ee/spec/services/ee/auth/container_registry_authentication_service_spec.rb
@@ -94,6 +94,49 @@
 
   context 'when not in maintenance mode' do
     it_behaves_like 'a container registry auth service'
+
+    describe 'container repository factory auditing' do
+      include_context 'container registry auth service context'
+
+      let_it_be(:current_user) { create(:user) }
+      let_it_be(:project) { create(:project) }
+
+      let(:current_params) do
+        { scopes: ["repository:#{project.full_path}:push"] }
+      end
+
+      before_all do
+        project.add_developer(current_user)
+      end
+
+      include_examples 'audit event logging' do
+        let(:operation) { subject }
+        let(:event_type) { 'container_repository_created' }
+        let(:repository) { project.container_repositories.last }
+        let(:fail_condition!) do
+          allow(::ContainerRepository).to receive(:find_or_create_from_path!).and_return(false)
+        end
+
+        let(:author) { current_user }
+
+        let(:attributes) do
+          {
+            author_id: author.id,
+            entity_id: project.id,
+            entity_type: 'Project',
+            details: {
+              event_name: "container_repository_created",
+              author_class: author.class.to_s,
+              author_name: author.name,
+              custom_message: "Container repository #{repository.id} created",
+              target_details: repository.name,
+              target_id: repository.id,
+              target_type: repository.class.to_s
+            }
+          }
+        end
+      end
+    end
   end
 
   context 'when over storage limit' do
-- 
GitLab


From 378588f4a489eaa1c179fb6a91b2a84afb200f11 Mon Sep 17 00:00:00 2001
From: Aaron Huntsman <ahuntsman@gitlab.com>
Date: Thu, 5 Sep 2024 14:04:54 -0500
Subject: [PATCH 04/10] Audit new repo path instead of id

---
 .../ee/auth/container_registry_authentication_service.rb        | 2 +-
 .../ee/auth/container_registry_authentication_service_spec.rb   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/ee/app/services/ee/auth/container_registry_authentication_service.rb b/ee/app/services/ee/auth/container_registry_authentication_service.rb
index e2571a6b049d70..ea179fef37a7fa 100644
--- a/ee/app/services/ee/auth/container_registry_authentication_service.rb
+++ b/ee/app/services/ee/auth/container_registry_authentication_service.rb
@@ -57,7 +57,7 @@ def find_or_create_repository(path)
             author: current_user || ::Gitlab::Audit::UnauthenticatedAuthor.new(name: '(System)'),
             scope: repository.project,
             target: repository,
-            message: "Container repository #{repository.id} created"
+            message: "Container repository #{repository.path} created"
           }
 
           ::Gitlab::Audit::Auditor.audit(audit_context)
diff --git a/ee/spec/services/ee/auth/container_registry_authentication_service_spec.rb b/ee/spec/services/ee/auth/container_registry_authentication_service_spec.rb
index 5e8847aa173e4b..c8e8c733ed681f 100644
--- a/ee/spec/services/ee/auth/container_registry_authentication_service_spec.rb
+++ b/ee/spec/services/ee/auth/container_registry_authentication_service_spec.rb
@@ -128,7 +128,7 @@
               event_name: "container_repository_created",
               author_class: author.class.to_s,
               author_name: author.name,
-              custom_message: "Container repository #{repository.id} created",
+              custom_message: "Container repository #{repository.path} created",
               target_details: repository.name,
               target_id: repository.id,
               target_type: repository.class.to_s
-- 
GitLab


From 0acee383d211a94b9fac6da64224ed6e226bd610 Mon Sep 17 00:00:00 2001
From: Aaron Huntsman <ahuntsman@gitlab.com>
Date: Wed, 11 Sep 2024 01:28:37 -0500
Subject: [PATCH 05/10] Simplify new repo detection, move to EE only

---
 app/models/container_repository.rb            |  6 ++--
 ...ntainer_registry_authentication_service.rb | 10 +++---
 ee/app/models/ee/container_repository.rb      |  9 ++++++
 ...ntainer_registry_authentication_service.rb | 30 ++++++++---------
 ee/spec/models/container_repository_spec.rb   | 32 +++++++++++++++++++
 spec/models/container_repository_spec.rb      | 29 -----------------
 6 files changed, 62 insertions(+), 54 deletions(-)

diff --git a/app/models/container_repository.rb b/app/models/container_repository.rb
index 55958055c966fe..87f0b27a932096 100644
--- a/app/models/container_repository.rb
+++ b/app/models/container_repository.rb
@@ -272,15 +272,13 @@ def self.build_from_path(path)
     self.new(project: path.repository_project, name: path.repository_name)
   end
 
-  def self.find_or_create_from_path!(path, check_created: false)
-    created = check_created && !ContainerRepository.find_by_path(path)
-
+  def self.find_or_create_from_path!(path)
     ContainerRepository.upsert({
       project_id: path.repository_project.id,
       name: path.repository_name
     }, unique_by: %i[project_id name])
 
-    check_created ? [find_by_path!(path), created] : find_by_path!(path)
+    find_by_path!(path)
   end
 
   def self.build_root_repository(project)
diff --git a/app/services/auth/container_registry_authentication_service.rb b/app/services/auth/container_registry_authentication_service.rb
index c5b806bb0406ea..faa4900fe5015f 100644
--- a/app/services/auth/container_registry_authentication_service.rb
+++ b/app/services/auth/container_registry_authentication_service.rb
@@ -226,12 +226,14 @@ def ensure_container_repository!(path, actions)
       return if path.has_repository?
       return unless actions.include?('push')
 
-      find_or_create_repository(path)
+      repo = ContainerRepository.find_or_create_from_path!(path)
+      audit_repository_created(repo)
+
+      repo
     end
 
-    # Overridden in EE
-    def find_or_create_repository(path)
-      ContainerRepository.find_or_create_from_path!(path)
+    def audit_repository_created(repo)
+      # Overridden in EE
     end
 
     # Overridden in EE
diff --git a/ee/app/models/ee/container_repository.rb b/ee/app/models/ee/container_repository.rb
index 24852bd431e15a..7e21184c4fdd08 100644
--- a/ee/app/models/ee/container_repository.rb
+++ b/ee/app/models/ee/container_repository.rb
@@ -59,6 +59,15 @@ def replicables_for_current_secondary(primary_key_in)
       def verification_state_table_class
         ::Geo::ContainerRepositoryState
       end
+
+      override :find_or_create_from_path!
+      def find_or_create_from_path!(path)
+        new_record = !find_by_path(path)
+
+        repo = super(path)
+        repo.define_singleton_method(:_new_record) { true } if new_record
+        repo
+      end
     end
 
     def container_repository_state
diff --git a/ee/app/services/ee/auth/container_registry_authentication_service.rb b/ee/app/services/ee/auth/container_registry_authentication_service.rb
index ea179fef37a7fa..7a556440af80c0 100644
--- a/ee/app/services/ee/auth/container_registry_authentication_service.rb
+++ b/ee/app/services/ee/auth/container_registry_authentication_service.rb
@@ -47,23 +47,19 @@ def extra_info
         })
       end
 
-      override :find_or_create_repository
-      def find_or_create_repository(path)
-        repository, created = ::ContainerRepository.find_or_create_from_path!(path, check_created: true)
-
-        if created
-          audit_context = {
-            name: "container_repository_created",
-            author: current_user || ::Gitlab::Audit::UnauthenticatedAuthor.new(name: '(System)'),
-            scope: repository.project,
-            target: repository,
-            message: "Container repository #{repository.path} created"
-          }
-
-          ::Gitlab::Audit::Auditor.audit(audit_context)
-        end
-
-        repository
+      override :audit_repository_created
+      def audit_repository_created(repository)
+        return unless repository.respond_to?(:_new_record)
+
+        audit_context = {
+          name: "container_repository_created",
+          author: current_user || ::Gitlab::Audit::UnauthenticatedAuthor.new(name: '(System)'),
+          scope: repository.project,
+          target: repository,
+          message: "Container repository #{repository.path} created"
+        }
+
+        ::Gitlab::Audit::Auditor.audit(audit_context)
       end
 
       def access_denied_in_maintenance_mode?
diff --git a/ee/spec/models/container_repository_spec.rb b/ee/spec/models/container_repository_spec.rb
index eb5fd221fa0966..912cccb6e29269 100644
--- a/ee/spec/models/container_repository_spec.rb
+++ b/ee/spec/models/container_repository_spec.rb
@@ -63,4 +63,36 @@
       end
     end
   end
+
+  describe '.find_or_create_from_path!' do
+    let_it_be(:group) { create(:group, name: 'group') }
+    let_it_be(:project) { create(:project, path: 'test', group: group) }
+    let_it_be(:repository) do
+      create(:container_repository, name: 'my_image', project: project)
+    end
+
+    context 'when container repository does not already exist' do
+      it 'does not define `_new_record` on returned object' do
+        path_name = "#{project.full_path}/some/image"
+        repository = described_class.find_or_create_from_path!(ContainerRegistry::Path.new(path_name))
+
+        expect(repository).to respond_to(:_new_record)
+        expect(repository._new_record).to eq(true)
+      end
+    end
+
+    context 'when container repository already exists' do
+      it 'does not define `_new_record` on returned object' do
+        image_name = 'some/image'
+
+        existing_container_repository = create(:container_repository, project: project, name: image_name)
+
+        path_name = "#{project.full_path}/#{image_name}"
+        repository = described_class.find_or_create_from_path!(ContainerRegistry::Path.new(path_name))
+
+        expect(repository.id).to eq(existing_container_repository.id)
+        expect(repository).not_to respond_to(:_new_record)
+      end
+    end
+  end
 end
diff --git a/spec/models/container_repository_spec.rb b/spec/models/container_repository_spec.rb
index 4b46ed9295eaad..be22b71342ad22 100644
--- a/spec/models/container_repository_spec.rb
+++ b/spec/models/container_repository_spec.rb
@@ -854,18 +854,6 @@ def expected_tags_from(client_tags)
 
     let(:repository_path) { ContainerRegistry::Path.new(path) }
 
-    shared_examples 'new path with check_created set' do
-      let(:repository_with_created) do
-        described_class.find_or_create_from_path!(ContainerRegistry::Path.new(path), check_created: true)
-      end
-
-      it 'returns true created value for a new path' do
-        _, created = repository_with_created
-
-        expect(created).to eq(true)
-      end
-    end
-
     context 'when received multi-level repository path' do
       let(:path) { project.full_path + '/some/image' }
 
@@ -876,8 +864,6 @@ def expected_tags_from(client_tags)
       it 'fabricates repository with a correct name' do
         expect(repository.name).to eq 'some/image'
       end
-
-      it_behaves_like 'new path with check_created set'
     end
 
     context 'when path is too long' do
@@ -906,8 +892,6 @@ def expected_tags_from(client_tags)
       it 'has path including a nested group' do
         expect(repository.path).to include 'nested/test/some/image'
       end
-
-      it_behaves_like 'new path with check_created set'
     end
 
     context 'when received root repository path' do
@@ -920,8 +904,6 @@ def expected_tags_from(client_tags)
       it 'fabricates repository with an empty name' do
         expect(repository.name).to be_empty
       end
-
-      it_behaves_like 'new path with check_created set'
     end
 
     context 'when repository already exists' do
@@ -932,17 +914,6 @@ def expected_tags_from(client_tags)
 
         expect(repository.id).to eq(container_repository.id)
       end
-
-      context 'with check_created set' do
-        it 'returns the existing repository with false created value' do
-          container_repository = create(:container_repository, project: project, name: 'some/image')
-
-          repository, created = described_class.find_or_create_from_path!(ContainerRegistry::Path.new(path), check_created: true)
-
-          expect(repository.id).to eq(container_repository.id)
-          expect(created).to eq(false)
-        end
-      end
     end
 
     context 'when many of the same repository are created at the same time' do
-- 
GitLab


From ff7ae8449f296e8af0837f973482d1e60ebca4e0 Mon Sep 17 00:00:00 2001
From: Aaron Huntsman <ahuntsman@gitlab.com>
Date: Wed, 11 Sep 2024 03:10:44 -0500
Subject: [PATCH 06/10] Fix audit spec fail condition

---
 .../ee/auth/container_registry_authentication_service_spec.rb   | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ee/spec/services/ee/auth/container_registry_authentication_service_spec.rb b/ee/spec/services/ee/auth/container_registry_authentication_service_spec.rb
index c8e8c733ed681f..b9f76ac2a6073c 100644
--- a/ee/spec/services/ee/auth/container_registry_authentication_service_spec.rb
+++ b/ee/spec/services/ee/auth/container_registry_authentication_service_spec.rb
@@ -114,7 +114,7 @@
         let(:event_type) { 'container_repository_created' }
         let(:repository) { project.container_repositories.last }
         let(:fail_condition!) do
-          allow(::ContainerRepository).to receive(:find_or_create_from_path!).and_return(false)
+          create(:container_repository, project: project, name: '')
         end
 
         let(:author) { current_user }
-- 
GitLab


From 99ceb8719f0f69d55032b281110f7d289a33c78b Mon Sep 17 00:00:00 2001
From: Aaron Huntsman <ahuntsman@gitlab.com>
Date: Tue, 17 Sep 2024 10:59:58 -0500
Subject: [PATCH 07/10] suggested fixes

---
 ...ntainer_registry_authentication_service.rb |  8 +++---
 doc/user/compliance/audit_event_types.md      |  2 +-
 ee/app/models/ee/container_repository.rb      |  6 ++---
 ...ntainer_registry_authentication_service.rb |  3 ++-
 .../types/container_repository_created.yml    |  4 +--
 ee/spec/models/container_repository_spec.rb   | 25 ++++++++-----------
 ...er_registry_authentication_service_spec.rb |  4 ++-
 7 files changed, 25 insertions(+), 27 deletions(-)

diff --git a/app/services/auth/container_registry_authentication_service.rb b/app/services/auth/container_registry_authentication_service.rb
index faa4900fe5015f..574e4ead1a34f7 100644
--- a/app/services/auth/container_registry_authentication_service.rb
+++ b/app/services/auth/container_registry_authentication_service.rb
@@ -226,13 +226,13 @@ def ensure_container_repository!(path, actions)
       return if path.has_repository?
       return unless actions.include?('push')
 
-      repo = ContainerRepository.find_or_create_from_path!(path)
-      audit_repository_created(repo)
+      repository = ContainerRepository.find_or_create_from_path!(path)
+      audit_repository_created(repository)
 
-      repo
+      repository
     end
 
-    def audit_repository_created(repo)
+    def audit_repository_created(repository)
       # Overridden in EE
     end
 
diff --git a/doc/user/compliance/audit_event_types.md b/doc/user/compliance/audit_event_types.md
index 4b31ae770590ca..d0f2ab212bb307 100644
--- a/doc/user/compliance/audit_event_types.md
+++ b/doc/user/compliance/audit_event_types.md
@@ -193,7 +193,7 @@ Audit event types belong to the following product categories.
 
 | Name | Description | Saved to database | Streamed | Introduced in | Scope |
 |:------------|:------------|:------------------|:---------|:--------------|:--------------|
-| [`container_repository_created`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/156037) | Triggered when a container repository is first pushed to the registry | **{check-circle}** Yes | **{check-circle}** Yes | GitLab [17.4](https://gitlab.com/gitlab-org/gitlab/-/issues/362290) | Project |
+| [`container_repository_created`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/156037) | Triggered when a container repository is first pushed to the registry | **{check-circle}** Yes | **{check-circle}** Yes | GitLab [17.5](https://gitlab.com/gitlab-org/gitlab/-/issues/362290) | Project |
 | [`container_repository_deleted`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/152967) | Triggered when a project's container registry is deleted | **{check-circle}** Yes | **{check-circle}** Yes | GitLab [17.2](https://gitlab.com/gitlab-org/gitlab/-/issues/362290) | Project |
 | [`container_repository_deletion_marked`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/152967) | Triggered when a project's container repository is marked for deletion | **{check-circle}** Yes | **{check-circle}** Yes | GitLab [17.2](https://gitlab.com/gitlab-org/gitlab/-/issues/362290) | Project |
 | [`container_repository_tags_deleted`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/156066) | Triggered when a project's container repository tag is deleted | **{check-circle}** Yes | **{check-circle}** Yes | GitLab [17.2](https://gitlab.com/gitlab-org/gitlab/-/issues/362290) | Project |
diff --git a/ee/app/models/ee/container_repository.rb b/ee/app/models/ee/container_repository.rb
index 7e21184c4fdd08..aec93120de16f7 100644
--- a/ee/app/models/ee/container_repository.rb
+++ b/ee/app/models/ee/container_repository.rb
@@ -64,9 +64,9 @@ def verification_state_table_class
       def find_or_create_from_path!(path)
         new_record = !find_by_path(path)
 
-        repo = super(path)
-        repo.define_singleton_method(:_new_record) { true } if new_record
-        repo
+        repository = super(path)
+        repository.define_singleton_method(:_new_record) { true } if new_record
+        repository
       end
     end
 
diff --git a/ee/app/services/ee/auth/container_registry_authentication_service.rb b/ee/app/services/ee/auth/container_registry_authentication_service.rb
index 7a556440af80c0..6f7968958e7293 100644
--- a/ee/app/services/ee/auth/container_registry_authentication_service.rb
+++ b/ee/app/services/ee/auth/container_registry_authentication_service.rb
@@ -53,9 +53,10 @@ def audit_repository_created(repository)
 
         audit_context = {
           name: "container_repository_created",
-          author: current_user || ::Gitlab::Audit::UnauthenticatedAuthor.new(name: '(System)'),
+          author: current_user || deploy_token&.user || ::Gitlab::Audit::UnauthenticatedAuthor.new(name: '(System)'),
           scope: repository.project,
           target: repository,
+          target_details: repository.path,
           message: "Container repository #{repository.path} created"
         }
 
diff --git a/ee/config/audit_events/types/container_repository_created.yml b/ee/config/audit_events/types/container_repository_created.yml
index 0a22443eb000bb..b998d4072cacb4 100644
--- a/ee/config/audit_events/types/container_repository_created.yml
+++ b/ee/config/audit_events/types/container_repository_created.yml
@@ -4,7 +4,7 @@ description: Triggered when a container repository is first pushed to the regist
 introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/362290
 introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/156037
 feature_category: container_registry
-milestone: '17.4'
+milestone: '17.5'
 saved_to_database: true
 streamed: true
-scope: [Project]
\ No newline at end of file
+scope: [Project]
diff --git a/ee/spec/models/container_repository_spec.rb b/ee/spec/models/container_repository_spec.rb
index 912cccb6e29269..cb8f57b924fddd 100644
--- a/ee/spec/models/container_repository_spec.rb
+++ b/ee/spec/models/container_repository_spec.rb
@@ -65,31 +65,26 @@
   end
 
   describe '.find_or_create_from_path!' do
-    let_it_be(:group) { create(:group, name: 'group') }
-    let_it_be(:project) { create(:project, path: 'test', group: group) }
-    let_it_be(:repository) do
-      create(:container_repository, name: 'my_image', project: project)
+    let_it_be(:project) { create(:project, path: 'test') }
+    let(:image_name) { 'some/image' }
+
+    subject(:repository) do
+      described_class.find_or_create_from_path!(ContainerRegistry::Path.new("#{project.full_path}/#{image_name}"))
     end
 
     context 'when container repository does not already exist' do
-      it 'does not define `_new_record` on returned object' do
-        path_name = "#{project.full_path}/some/image"
-        repository = described_class.find_or_create_from_path!(ContainerRegistry::Path.new(path_name))
-
+      it 'defines `_new_record` on returned object' do
         expect(repository).to respond_to(:_new_record)
         expect(repository._new_record).to eq(true)
       end
     end
 
     context 'when container repository already exists' do
-      it 'does not define `_new_record` on returned object' do
-        image_name = 'some/image'
-
-        existing_container_repository = create(:container_repository, project: project, name: image_name)
-
-        path_name = "#{project.full_path}/#{image_name}"
-        repository = described_class.find_or_create_from_path!(ContainerRegistry::Path.new(path_name))
+      let!(:existing_container_repository) do
+        create(:container_repository, project: project, name: image_name)
+      end
 
+      it 'does not define `_new_record` on returned object' do
         expect(repository.id).to eq(existing_container_repository.id)
         expect(repository).not_to respond_to(:_new_record)
       end
diff --git a/ee/spec/services/ee/auth/container_registry_authentication_service_spec.rb b/ee/spec/services/ee/auth/container_registry_authentication_service_spec.rb
index b9f76ac2a6073c..0b9200fda15aec 100644
--- a/ee/spec/services/ee/auth/container_registry_authentication_service_spec.rb
+++ b/ee/spec/services/ee/auth/container_registry_authentication_service_spec.rb
@@ -101,6 +101,8 @@
       let_it_be(:current_user) { create(:user) }
       let_it_be(:project) { create(:project) }
 
+      let(:deploy_token) { create(:deploy_token, projects: [project], read_registry: true, write_registry: true) }
+
       let(:current_params) do
         { scopes: ["repository:#{project.full_path}:push"] }
       end
@@ -125,7 +127,7 @@
             entity_id: project.id,
             entity_type: 'Project',
             details: {
-              event_name: "container_repository_created",
+              event_name: event_type,
               author_class: author.class.to_s,
               author_name: author.name,
               custom_message: "Container repository #{repository.path} created",
-- 
GitLab


From a7403747295b53c762d4b1bb5c990767c7457626 Mon Sep 17 00:00:00 2001
From: Aaron Huntsman <ahuntsman@gitlab.com>
Date: Thu, 19 Sep 2024 12:25:33 -0500
Subject: [PATCH 08/10] Update specs for auditing registry auth

---
 ...er_registry_authentication_service_spec.rb | 86 +++++++++++++------
 1 file changed, 58 insertions(+), 28 deletions(-)

diff --git a/ee/spec/services/ee/auth/container_registry_authentication_service_spec.rb b/ee/spec/services/ee/auth/container_registry_authentication_service_spec.rb
index 0b9200fda15aec..6ce7dcf21dfd37 100644
--- a/ee/spec/services/ee/auth/container_registry_authentication_service_spec.rb
+++ b/ee/spec/services/ee/auth/container_registry_authentication_service_spec.rb
@@ -98,44 +98,74 @@
     describe 'container repository factory auditing' do
       include_context 'container registry auth service context'
 
-      let_it_be(:current_user) { create(:user) }
       let_it_be(:project) { create(:project) }
 
-      let(:deploy_token) { create(:deploy_token, projects: [project], read_registry: true, write_registry: true) }
-
       let(:current_params) do
         { scopes: ["repository:#{project.full_path}:push"] }
       end
 
-      before_all do
-        project.add_developer(current_user)
+      let(:operation) { subject }
+      let(:event_type) { 'container_repository_created' }
+      let(:repository) { project.container_repositories.last }
+      let(:fail_condition!) do
+        create(:container_repository, project: project, name: '')
+      end
+
+      let(:author) { current_user }
+
+      let(:attributes) do
+        {
+          author_id: author.id,
+          entity_id: project.id,
+          entity_type: 'Project',
+          details: {
+            event_name: event_type,
+            author_class: author.class.to_s,
+            author_name: author.name,
+            custom_message: "Container repository #{repository.path} created",
+            target_details: repository.path,
+            target_id: repository.id,
+            target_type: repository.class.to_s
+          }
+        }
       end
 
-      include_examples 'audit event logging' do
-        let(:operation) { subject }
-        let(:event_type) { 'container_repository_created' }
-        let(:repository) { project.container_repositories.last }
-        let(:fail_condition!) do
-          create(:container_repository, project: project, name: '')
+      context 'with current user' do
+        let_it_be(:current_user) { create(:user) }
+
+        before_all do
+          project.add_developer(current_user)
         end
 
-        let(:author) { current_user }
-
-        let(:attributes) do
-          {
-            author_id: author.id,
-            entity_id: project.id,
-            entity_type: 'Project',
-            details: {
-              event_name: event_type,
-              author_class: author.class.to_s,
-              author_name: author.name,
-              custom_message: "Container repository #{repository.path} created",
-              target_details: repository.name,
-              target_id: repository.id,
-              target_type: repository.class.to_s
-            }
-          }
+        include_examples 'audit event logging' do
+          let(:author) { current_user }
+        end
+      end
+
+      context 'with deploy token' do
+        let_it_be(:user) { create(:user) }
+        let(:deploy_token) do
+          create(:deploy_token, projects: [project], read_registry: true, write_registry: true, user: user)
+        end
+
+        let(:current_params) do
+          { scopes: ["repository:#{project.full_path}:push"], deploy_token: deploy_token }
+        end
+
+        include_examples 'audit event logging' do
+          let(:author) { deploy_token.user }
+        end
+      end
+
+      context 'with anonymous deploy token' do
+        let(:deploy_token) { create(:deploy_token, projects: [project], read_registry: true, write_registry: true) }
+
+        let(:current_params) do
+          { scopes: ["repository:#{project.full_path}:push"], deploy_token: deploy_token }
+        end
+
+        include_examples 'audit event logging' do
+          let(:author) { ::Gitlab::Audit::UnauthenticatedAuthor.new(name: '(System)') }
         end
       end
     end
-- 
GitLab


From 8ab4c7f0109b5e7f72eb6d8d1a74d9bfea99d135 Mon Sep 17 00:00:00 2001
From: Aaron Huntsman <ahuntsman@gitlab.com>
Date: Mon, 30 Sep 2024 23:20:47 -0500
Subject: [PATCH 09/10] Move new record check into service

---
 ...ntainer_registry_authentication_service.rb | 10 +++----
 ee/app/models/ee/container_repository.rb      |  9 -------
 ...ntainer_registry_authentication_service.rb | 15 ++++++++---
 ee/spec/models/container_repository_spec.rb   | 27 -------------------
 ...er_registry_authentication_service_spec.rb | 12 ++++++---
 5 files changed, 24 insertions(+), 49 deletions(-)

diff --git a/app/services/auth/container_registry_authentication_service.rb b/app/services/auth/container_registry_authentication_service.rb
index 574e4ead1a34f7..86405ed16660e5 100644
--- a/app/services/auth/container_registry_authentication_service.rb
+++ b/app/services/auth/container_registry_authentication_service.rb
@@ -226,14 +226,12 @@ def ensure_container_repository!(path, actions)
       return if path.has_repository?
       return unless actions.include?('push')
 
-      repository = ContainerRepository.find_or_create_from_path!(path)
-      audit_repository_created(repository)
-
-      repository
+      find_or_create_repository_from_path(path)
     end
 
-    def audit_repository_created(repository)
-      # Overridden in EE
+    # Overridden in EE
+    def find_or_create_repository_from_path(path)
+      ContainerRepository.find_or_create_from_path!(path)
     end
 
     # Overridden in EE
diff --git a/ee/app/models/ee/container_repository.rb b/ee/app/models/ee/container_repository.rb
index aec93120de16f7..24852bd431e15a 100644
--- a/ee/app/models/ee/container_repository.rb
+++ b/ee/app/models/ee/container_repository.rb
@@ -59,15 +59,6 @@ def replicables_for_current_secondary(primary_key_in)
       def verification_state_table_class
         ::Geo::ContainerRepositoryState
       end
-
-      override :find_or_create_from_path!
-      def find_or_create_from_path!(path)
-        new_record = !find_by_path(path)
-
-        repository = super(path)
-        repository.define_singleton_method(:_new_record) { true } if new_record
-        repository
-      end
     end
 
     def container_repository_state
diff --git a/ee/app/services/ee/auth/container_registry_authentication_service.rb b/ee/app/services/ee/auth/container_registry_authentication_service.rb
index 6f7968958e7293..2ebf0bb12d6cbe 100644
--- a/ee/app/services/ee/auth/container_registry_authentication_service.rb
+++ b/ee/app/services/ee/auth/container_registry_authentication_service.rb
@@ -47,13 +47,20 @@ def extra_info
         })
       end
 
-      override :audit_repository_created
-      def audit_repository_created(repository)
-        return unless repository.respond_to?(:_new_record)
+      override :find_or_create_repository_from_path
+      def find_or_create_repository_from_path(path)
+        new_record = !::ContainerRepository.find_by_path(path)
+
+        repository = super(path)
+        audit_repository_created(repository) if new_record
 
+        repository
+      end
+
+      def audit_repository_created(repository)
         audit_context = {
           name: "container_repository_created",
-          author: current_user || deploy_token&.user || ::Gitlab::Audit::UnauthenticatedAuthor.new(name: '(System)'),
+          author: current_user || deploy_token,
           scope: repository.project,
           target: repository,
           target_details: repository.path,
diff --git a/ee/spec/models/container_repository_spec.rb b/ee/spec/models/container_repository_spec.rb
index cb8f57b924fddd..eb5fd221fa0966 100644
--- a/ee/spec/models/container_repository_spec.rb
+++ b/ee/spec/models/container_repository_spec.rb
@@ -63,31 +63,4 @@
       end
     end
   end
-
-  describe '.find_or_create_from_path!' do
-    let_it_be(:project) { create(:project, path: 'test') }
-    let(:image_name) { 'some/image' }
-
-    subject(:repository) do
-      described_class.find_or_create_from_path!(ContainerRegistry::Path.new("#{project.full_path}/#{image_name}"))
-    end
-
-    context 'when container repository does not already exist' do
-      it 'defines `_new_record` on returned object' do
-        expect(repository).to respond_to(:_new_record)
-        expect(repository._new_record).to eq(true)
-      end
-    end
-
-    context 'when container repository already exists' do
-      let!(:existing_container_repository) do
-        create(:container_repository, project: project, name: image_name)
-      end
-
-      it 'does not define `_new_record` on returned object' do
-        expect(repository.id).to eq(existing_container_repository.id)
-        expect(repository).not_to respond_to(:_new_record)
-      end
-    end
-  end
 end
diff --git a/ee/spec/services/ee/auth/container_registry_authentication_service_spec.rb b/ee/spec/services/ee/auth/container_registry_authentication_service_spec.rb
index 6ce7dcf21dfd37..f177679ea6301d 100644
--- a/ee/spec/services/ee/auth/container_registry_authentication_service_spec.rb
+++ b/ee/spec/services/ee/auth/container_registry_authentication_service_spec.rb
@@ -2,9 +2,15 @@
 
 require 'spec_helper'
 
-RSpec.describe Auth::ContainerRegistryAuthenticationService, feature_category: :container_registry do
+RSpec.describe Auth::ContainerRegistryAuthenticationService, :request_store, feature_category: :container_registry do
   include AdminModeHelper
 
+  let(:ip_address) { '192.168.8.8' }
+
+  before do
+    allow(Gitlab::RequestContext.instance).to receive(:client_ip).and_return(ip_address)
+  end
+
   describe 'with deploy keys' do
     include_context 'container registry auth service context'
 
@@ -153,7 +159,7 @@
         end
 
         include_examples 'audit event logging' do
-          let(:author) { deploy_token.user }
+          let(:author) { deploy_token }
         end
       end
 
@@ -165,7 +171,7 @@
         end
 
         include_examples 'audit event logging' do
-          let(:author) { ::Gitlab::Audit::UnauthenticatedAuthor.new(name: '(System)') }
+          let(:author) { deploy_token }
         end
       end
     end
-- 
GitLab


From ba222cfdb309d2389326e0eb06bce0218a40c206 Mon Sep 17 00:00:00 2001
From: Aaron Huntsman <ahuntsman@gitlab.com>
Date: Tue, 1 Oct 2024 09:44:24 -0500
Subject: [PATCH 10/10] Use deploy_token&.user as audit author pending refactor

---
 .../container_registry_authentication_service.rb     |  2 +-
 ...container_registry_authentication_service_spec.rb | 12 +++---------
 lib/gitlab/audit/deploy_token_author.rb              |  4 ++++
 3 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/ee/app/services/ee/auth/container_registry_authentication_service.rb b/ee/app/services/ee/auth/container_registry_authentication_service.rb
index 2ebf0bb12d6cbe..8c4806d36fdb7b 100644
--- a/ee/app/services/ee/auth/container_registry_authentication_service.rb
+++ b/ee/app/services/ee/auth/container_registry_authentication_service.rb
@@ -60,7 +60,7 @@ def find_or_create_repository_from_path(path)
       def audit_repository_created(repository)
         audit_context = {
           name: "container_repository_created",
-          author: current_user || deploy_token,
+          author: current_user || deploy_token&.user || ::Gitlab::Audit::DeployTokenAuthor.new,
           scope: repository.project,
           target: repository,
           target_details: repository.path,
diff --git a/ee/spec/services/ee/auth/container_registry_authentication_service_spec.rb b/ee/spec/services/ee/auth/container_registry_authentication_service_spec.rb
index f177679ea6301d..930667a9a3abef 100644
--- a/ee/spec/services/ee/auth/container_registry_authentication_service_spec.rb
+++ b/ee/spec/services/ee/auth/container_registry_authentication_service_spec.rb
@@ -2,15 +2,9 @@
 
 require 'spec_helper'
 
-RSpec.describe Auth::ContainerRegistryAuthenticationService, :request_store, feature_category: :container_registry do
+RSpec.describe Auth::ContainerRegistryAuthenticationService, feature_category: :container_registry do
   include AdminModeHelper
 
-  let(:ip_address) { '192.168.8.8' }
-
-  before do
-    allow(Gitlab::RequestContext.instance).to receive(:client_ip).and_return(ip_address)
-  end
-
   describe 'with deploy keys' do
     include_context 'container registry auth service context'
 
@@ -159,7 +153,7 @@
         end
 
         include_examples 'audit event logging' do
-          let(:author) { deploy_token }
+          let(:author) { deploy_token.user }
         end
       end
 
@@ -171,7 +165,7 @@
         end
 
         include_examples 'audit event logging' do
-          let(:author) { deploy_token }
+          let(:author) { ::Gitlab::Audit::DeployTokenAuthor.new }
         end
       end
     end
diff --git a/lib/gitlab/audit/deploy_token_author.rb b/lib/gitlab/audit/deploy_token_author.rb
index 69b42034826ef3..fe6ae6b4993940 100644
--- a/lib/gitlab/audit/deploy_token_author.rb
+++ b/lib/gitlab/audit/deploy_token_author.rb
@@ -12,6 +12,10 @@ def initialize(name: nil)
       def name
         @name || _('Deploy Token')
       end
+
+      def impersonated?
+        false
+      end
     end
   end
 end
-- 
GitLab