From 00aa4323fd06b33a130c3d953f9a28c7cd48e0cb Mon Sep 17 00:00:00 2001
From: Quinn James <35753203+qcjames53@users.noreply.github.com>
Date: Fri, 19 Jul 2024 13:47:44 -0400
Subject: [PATCH] Fixes #37634 - Prevent users from editing container push
 repositories (#11071)

---
 .../registry/registry_proxies_controller.rb   |  1 +
 app/models/katello/repository.rb              | 12 ++++-
 app/models/katello/root_repository.rb         | 19 +++++++
 .../api/v2/repositories/show.json.rabl        |  2 +
 .../details/views/repository-info.html        | 53 ++++++++++---------
 test/actions/katello/environment_test.rb      |  2 +-
 6 files changed, 61 insertions(+), 28 deletions(-)

diff --git a/app/controllers/katello/api/registry/registry_proxies_controller.rb b/app/controllers/katello/api/registry/registry_proxies_controller.rb
index 34b4bb50e9e..fa21f42727c 100644
--- a/app/controllers/katello/api/registry/registry_proxies_controller.rb
+++ b/app/controllers/katello/api/registry/registry_proxies_controller.rb
@@ -383,6 +383,7 @@ def save_pulp_push_repository_href
           :not_found
         )
       end
+
       instance_repo.update!(version_href: latest_version_href)
       # The Pulp repository should not change after first creation
       if root_repository.repository_references.empty?
diff --git a/app/models/katello/repository.rb b/app/models/katello/repository.rb
index c1a91518f54..e713452454d 100644
--- a/app/models/katello/repository.rb
+++ b/app/models/katello/repository.rb
@@ -32,6 +32,8 @@ class Repository < Katello::Model
 
     EXPORTABLE_TYPES = [YUM_TYPE, FILE_TYPE, ANSIBLE_COLLECTION_TYPE, DOCKER_TYPE, DEB_TYPE].freeze
 
+    ALLOWED_UPDATE_FIELDS = ['version_href', 'last_indexed'].freeze
+
     define_model_callbacks :sync, :only => :after
 
     belongs_to :root, :inverse_of => :repositories, :class_name => "Katello::RootRepository"
@@ -119,6 +121,7 @@ class Repository < Katello::Model
 
     before_validation :set_pulp_id
     before_validation :set_container_repository_name, :unless => :skip_container_name?
+    before_update :prevent_updates, :unless => :allow_updates?
 
     scope :has_url, -> { joins(:root).where.not("#{RootRepository.table_name}.url" => nil) }
     scope :not_uln, -> { joins(:root).where("#{RootRepository.table_name}.url NOT LIKE 'uln%'") }
@@ -180,7 +183,7 @@ class Repository < Katello::Model
              :deb_components, :deb_architectures, :ssl_ca_cert_id, :ssl_ca_cert, :ssl_client_cert, :ssl_client_cert_id,
              :ssl_client_key_id, :os_versions, :ssl_client_key, :ignorable_content, :description, :include_tags, :exclude_tags,
              :ansible_collection_requirements, :ansible_collection_auth_url, :ansible_collection_auth_token,
-             :http_proxy_policy, :http_proxy_id, :to => :root
+             :http_proxy_policy, :http_proxy_id, :prevent_updates, :to => :root
 
     delegate :content_id, to: :root, allow_nil: true
     delegate :repository_type, to: :root
@@ -1011,6 +1014,7 @@ def index_content(options = {})
         repository_type.index_additional_data_proc&.call(self)
       end
       self.update!(last_indexed: DateTime.now)
+
       true
     end
 
@@ -1052,6 +1056,12 @@ def remove_docker_content(manifests)
       DockerMetaTag.cleanup_tags
     end
 
+    def allow_updates?
+      # allow the update if this repo is not in the default view
+      return true unless in_default_view?
+      root.allow_updates?(::Katello::Repository::ALLOWED_UPDATE_FIELDS)
+    end
+
     apipie :class, desc: "A class representing #{model_name.human} object" do
       name 'Repository'
       refs 'Repository'
diff --git a/app/models/katello/root_repository.rb b/app/models/katello/root_repository.rb
index 3304e775abb..ae96e73e224 100644
--- a/app/models/katello/root_repository.rb
+++ b/app/models/katello/root_repository.rb
@@ -46,6 +46,8 @@ class RootRepository < Katello::Model
     MIRRORING_POLICY_COMPLETE = 'mirror_complete'.freeze
     MIRRORING_POLICIES = [MIRRORING_POLICY_ADDITIVE, MIRRORING_POLICY_COMPLETE, MIRRORING_POLICY_CONTENT].freeze
 
+    ALLOWED_UPDATE_FIELDS = ['updated_at', 'content_id'].freeze
+
     belongs_to :product, :inverse_of => :root_repositories, :class_name => "Katello::Product"
     has_one :provider, :through => :product
 
@@ -115,6 +117,7 @@ class RootRepository < Katello::Model
       }
 
     validates :container_push_name_format, inclusion: { in: ['label', 'id'].freeze, allow_nil: true}
+    before_update :prevent_updates, :unless => :allow_updates?
 
     scope :subscribable, -> { where(content_type: RootRepository::SUBSCRIBABLE_TYPES) }
     scope :skipable_metadata_check, -> { where(content_type: RootRepository::SKIPABLE_METADATA_TYPES) }
@@ -455,6 +458,22 @@ def format_arches
       end
     end
 
+    def allow_updates?(additional_allowed_fields = [])
+      # allow updates for non-container-push repos, repos not in default view, and
+      # repos with a library instance
+      return true unless is_container_push && library_instance.present?
+
+      # let updates that contain ONLY allowed strings through
+      allowed_fields = ::Katello::RootRepository::ALLOWED_UPDATE_FIELDS + additional_allowed_fields
+      return true if (changed - allowed_fields).empty?
+
+      false
+    end
+
+    def prevent_updates
+      fail _("Cannot update properties of a container push repository")
+    end
+
     apipie :class, desc: 'A class representing Repository object' do
       name 'Repository'
       refs 'Repository'
diff --git a/app/views/katello/api/v2/repositories/show.json.rabl b/app/views/katello/api/v2/repositories/show.json.rabl
index a6713b13f8d..4d7ac98e0d4 100644
--- a/app/views/katello/api/v2/repositories/show.json.rabl
+++ b/app/views/katello/api/v2/repositories/show.json.rabl
@@ -28,6 +28,8 @@ glue(@resource.root) do
   attributes :http_proxy_name
   attributes :retain_package_versions_count
   attributes :metadata_expire
+  attributes :allow_updates? => :allow_updates
+  attributes :is_container_push? => :is_container_push
 
   node :http_proxy do
     attributes :id => @resource.root&.http_proxy&.id, :name => @resource.root&.http_proxy&.name, :policy => @resource.root&.http_proxy_policy
diff --git a/engines/bastion_katello/app/assets/javascripts/bastion_katello/products/details/repositories/details/views/repository-info.html b/engines/bastion_katello/app/assets/javascripts/bastion_katello/products/details/repositories/details/views/repository-info.html
index a0e1fbe2e2b..397a11d0fd8 100644
--- a/engines/bastion_katello/app/assets/javascripts/bastion_katello/products/details/repositories/details/views/repository-info.html
+++ b/engines/bastion_katello/app/assets/javascripts/bastion_katello/products/details/repositories/details/views/repository-info.html
@@ -8,7 +8,7 @@ <h4 translate>Basic Information</h4>
       <dt translate>Name</dt>
       <dd bst-edit-text="repository.name"
           on-save="save(repository)"
-          readonly="product.redhat || denied('edit_products', product)">
+          readonly="product.redhat || denied('edit_products', product) || !repository.allow_updates || repository.is_container_push">
       </dd>
 
       <dt translate>Label</dt>
@@ -17,7 +17,7 @@ <h4 translate>Basic Information</h4>
       <dt translate>Description</dt>
       <dd bst-edit-textarea="repository.description"
 	  on-save="save(repository)"
-	  readonly="product.redhat || denied('edit_products', product)">
+	  readonly="product.redhat || denied('edit_products', product) || !repository.allow_updates || repository.is_container_push">
       </dd>
 
       <dt translate>Backend Identifier</dt>
@@ -54,7 +54,8 @@ <h4 translate>Publishing Settings</h4>
           </dd>
 
           <dt ng-if="product.redhat != true" translate>Metadata Expiration (Seconds)</dt>
-          <dd bst-edit-number="repository.metadata_expire" on-save="save(repository)" readonly="product.redhat || denied('edit_products', product)"
+          <dd bst-edit-number="repository.metadata_expire" on-save="save(repository)" 
+              readonly="product.redhat || denied('edit_products', product) || !repository.allow_updates || repository.is_container_push"
               deletable="repository.metadata_expire !== null"
               on-delete="clearMetadataExpire()"
               min="1"
@@ -72,7 +73,7 @@ <h4 translate>Sync Settings</h4>
       <dt ng-show="repository.content_type === 'docker'" translate>Registry URL</dt>
       <dd bst-edit-text="repository.url"
           on-save="save(repository)"
-          readonly="product.redhat || denied('edit_products', product)">
+          readonly="product.redhat || denied('edit_products', product) || !repository.allow_updates || repository.is_container_push">
       </dd>
 
       <span ng-show="repository.generic_remote_options !== []">
@@ -93,21 +94,21 @@ <h4 translate>Sync Settings</h4>
         <dt translate>Releases/Distributions</dt>
         <dd bst-edit-text="repository.deb_releases"
             on-save="save(repository)"
-            readonly="denied('edit_products', product)">
+            readonly="denied('edit_products', product) || !repository.allow_updates || repository.is_container_push">
         </dd>
       </span>
       <span ng-if="repository.content_type == 'deb'">
         <dt translate>Components</dt>
         <dd bst-edit-text="repository.deb_components"
             on-save="save(repository)"
-            readonly="denied('edit_products', product)">
+            readonly="denied('edit_products', product) || !repository.allow_updates || repository.is_container_push">
         </dd>
       </span>
       <span ng-if="repository.content_type == 'deb'">
         <dt translate>Architectures</dt>
         <dd bst-edit-text="repository.deb_architectures"
             on-save="save(repository)"
-            readonly="denied('edit_products', product)">
+            readonly="denied('edit_products', product) || !repository.allow_updates || repository.is_container_push">
         </dd>
       </span>
 
@@ -115,7 +116,7 @@ <h4 translate>Sync Settings</h4>
         <dt translate>Upstream Repository Name</dt>
         <dd bst-edit-text="repository.docker_upstream_name"
             on-save="save(repository)"
-            readonly="product.redhat || denied('edit_products', product)">
+            readonly="product.redhat || denied('edit_products', product) || !repository.allow_updates || repository.is_container_push">
         </dd>
       </span>
 
@@ -135,13 +136,13 @@ <h4 translate>Sync Settings</h4>
         <dt translate>Requirements</dt>
         <dd class="overflow-span" bst-edit-textarea="repository.ansible_collection_requirements"
             on-save="save(repository)"
-            readonly="denied('edit_products', product)"
+            readonly="denied('edit_products', product) || !repository.allow_updates || repository.is_container_push"
             edit-trigger="uploadedFile">
         </dd>
 
         <dt translate>Ansible Collection Authorization</dt>
         <dd bst-edit-custom="repository.ansible_collection_auth_exists"
-            readonly="denied('edit_products', product)"
+            readonly="denied('edit_products', product) || !repository.allow_updates || repository.is_container_push"
             on-save="save(repository)"
             formatter="ansibleAuthFilter"
             formatter-options="repository"
@@ -166,13 +167,13 @@ <h4 translate>Sync Settings</h4>
       <dd bst-edit-checkbox="repository.verify_ssl_on_sync"
           formatter="booleanToYesNo"
           on-save="save(repository)"
-          readonly="denied('edit_products', product)">
+          readonly="denied('edit_products', product) || !repository.allow_updates || repository.is_container_push">
       </dd>
 
       <span ng-if="!product.redhat">
         <dt translate>Upstream Authorization</dt>
         <dd bst-edit-custom="repository.upstream_auth_exists"
-            readonly="denied('edit_products', product)"
+            readonly="denied('edit_products', product) || !repository.allow_updates || repository.is_container_push"
             on-save="save(repository)"
             formatter="upstreamPasswordFilter"
             formatter-options="repository"
@@ -203,7 +204,7 @@ <h4 translate>Sync Settings</h4>
       <span ng-show="repository.content_type === 'yum'">
         <dt translate>Yum Metadata Checksum</dt>
         <dd bst-edit-select="checksumTypeDisplay(repository.checksum_type)"
-            readonly="product.redhat || denied('edit_products', product)"
+            readonly="product.redhat || denied('edit_products', product) || !repository.allow_updates || repository.is_container_push"
             selector="repository.checksum_type"
             options="checksums"
             on-save="save(repository)">
@@ -217,14 +218,14 @@ <h4 translate>Sync Settings</h4>
         <dt translate>Retain package versions</dt>
         <dd bst-edit-number="repository.retain_package_versions_count"
             on-save="save(repository)"
-            readonly="denied('edit_products', product)">
+            readonly="denied('edit_products', product) || !repository.allow_updates || repository.is_container_push">
         </dd>
       </span>
 
       <span>
         <dt translate>HTTP Proxy</dt>
         <dd bst-edit-custom="repository.http_proxy_policy"
-            readonly="denied('edit_products', product)"
+            readonly="denied('edit_products', product) || !repository.allow_updates || repository.is_container_push"
             on-save="save(repository)"
             formatter="httpProxyDetailsFilter"
             formatter-options="repository"
@@ -264,7 +265,7 @@ <h4 translate>Sync Settings</h4>
         <dd bst-edit-checkbox="repository.ignore_srpms"
             formatter="booleanToYesNo"
             on-save="save(repository)"
-            readonly="denied('edit_products', product)">
+            readonly="denied('edit_products', product) || !repository.allow_updates || repository.is_container_push">
         </dd>
       </span>
 
@@ -273,7 +274,7 @@ <h4 translate>Sync Settings</h4>
         <dd bst-edit-checkbox="repository.ignore_treeinfo"
             formatter="booleanToYesNo"
             on-save="save(repository)"
-            readonly="denied('edit_products', product)">
+            readonly="denied('edit_products', product) || !repository.allow_updates || repository.is_container_push">
         </dd>
       </span>
 
@@ -282,7 +283,7 @@ <h4 translate>Sync Settings</h4>
         <dd bst-edit-checkbox="repository.unprotected"
             formatter="booleanToYesNo"
             on-save="save(repository)"
-            readonly="product.redhat || denied('edit_products', product)">
+            readonly="product.redhat || denied('edit_products', product) || !repository.allow_updates || repository.is_container_push">
         </dd>
       </span>
 
@@ -301,7 +302,7 @@ <h4 translate>Sync Settings</h4>
       <span ng-if="(repository.content_type === 'yum' && !product.redhat) || repository.content_type === 'deb'">
         <dt translate>GPG Key</dt>
         <dd bst-edit-select="repository.gpg_key.name"
-            readonly="product.redhat ||denied('edit_products', product)"
+            readonly="product.redhat ||denied('edit_products', product) || !repository.allow_updates || repository.is_container_push"
             selector="repository.gpg_key_id"
             options="gpgKeys()"
             on-save="save(repository)">
@@ -309,21 +310,21 @@ <h4 translate>Sync Settings</h4>
       </span>
       <dt translate>SSL CA Cert</dt>
       <dd bst-edit-select="repository.ssl_ca_cert.name"
-          readonly="product.redhat ||denied('edit_products', product)"
+          readonly="product.redhat ||denied('edit_products', product) || !repository.allow_updates || repository.is_container_push"
           selector="repository.ssl_ca_cert_id"
           options="certs()"
           on-save="save(repository)">
       </dd>
       <dt translate>SSL Client Cert</dt>
       <dd bst-edit-select="repository.ssl_client_cert.name"
-          readonly="product.redhat ||denied('edit_products', product)"
+          readonly="product.redhat ||denied('edit_products', product) || !repository.allow_updates || repository.is_container_push"
           selector="repository.ssl_client_cert_id"
           options="certs()"
           on-save="save(repository)">
       </dd>
       <dt translate>SSL Client Key</dt>
       <dd bst-edit-select="repository.ssl_client_key.name"
-          readonly="product.redhat ||denied('edit_products', product)"
+          readonly="product.redhat ||denied('edit_products', product) || !repository.allow_updates || repository.is_container_push"
           selector="repository.ssl_client_key_id"
           options="certs()"
           on-save="save(repository)">
@@ -331,7 +332,7 @@ <h4 translate>Sync Settings</h4>
       <span ng-if="repository.content_type == 'yum' || repository.content_type == 'deb' || repository.content_type == 'docker'">
         <dt translate>Download Policy</dt>
         <dd bst-edit-select="downloadPolicyDisplay(repository.download_policy)"
-            readonly="denied('edit_products', product)"
+            readonly="denied('edit_products', product) || !repository.allow_updates || repository.is_container_push"
             selector="repository.download_policy"
             options="downloadPolicies"
             options-format="id as name for (id, name) in options"
@@ -349,7 +350,7 @@ <h4 translate>Sync Settings</h4>
         <dt translate>Mirroring Policy</dt>
 
         <dd bst-edit-select="mirroringPolicyDisplay(repository.mirroring_policy, repository.content_type)"
-            readonly="denied('edit_products', product)"
+            readonly="denied('edit_products', product) || !repository.allow_updates || repository.is_container_push"
             selector="repository.mirroring_policy"
             options="mirroringPolicies(repository.content_type)"
             options-format="id as name for (id, name) in options"
@@ -360,12 +361,12 @@ <h4 translate>Sync Settings</h4>
         <dt translate>Include Tags</dt>
         <dd bst-edit-text="repository.commaIncludeTags"
             on-save="save(repository)"
-            readonly="denied('edit_products', product)">
+            readonly="denied('edit_products', product) || !repository.allow_updates || repository.is_container_push">
         </dd>
         <dt translate>Exclude Tags</dt>
         <dd bst-edit-text="repository.commaExcludeTags"
             on-save="save(repository)"
-            readonly="denied('edit_products', product)">
+            readonly="denied('edit_products', product) || !repository.allow_updates || repository.is_container_push">
         </dd>
       </span>
     </dl>
diff --git a/test/actions/katello/environment_test.rb b/test/actions/katello/environment_test.rb
index cef497bb964..2806e17030b 100644
--- a/test/actions/katello/environment_test.rb
+++ b/test/actions/katello/environment_test.rb
@@ -22,7 +22,7 @@ class PublishContainerRepositoriesTest < TestBase
 
     it 'does not plan for container push library repos' do
       container_push_repo = ::Katello::RootRepository.find_by(name: 'busybox').library_instance
-      container_push_repo.root.update(is_container_push: true)
+      container_push_repo.root.update_column(:is_container_push, true)
       environment.stubs(:repositories).returns(::Katello::Repository.where(id: container_push_repo.id))
       container_push_repo.expects(:set_container_repository_name).never
       container_push_repo.expects(:clear_smart_proxy_sync_histories).never