From 9e318bd99deb90a93130cd4ef79e54f18555d4dc Mon Sep 17 00:00:00 2001
From: Kamil Trzcinski <ayufan@ayufan.eu>
Date: Fri, 13 May 2016 12:20:23 -0500
Subject: [PATCH] Fix container registry permissions

---
 app/models/ability.rb                              |  1 +
 .../container_registry_authentication_service.rb   |  6 +++++-
 app/services/projects/destroy_service.rb           |  2 +-
 ...ntainer_registry_authentication_service_spec.rb | 14 ++++++++++++++
 4 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/app/models/ability.rb b/app/models/ability.rb
index 2465c1f424c..09dea54689e 100644
--- a/app/models/ability.rb
+++ b/app/models/ability.rb
@@ -61,6 +61,7 @@ class Ability
           :read_merge_request,
           :read_note,
           :read_commit_status,
+          :read_container_registry,
           :download_code
         ]
 
diff --git a/app/services/jwt/container_registry_authentication_service.rb b/app/services/jwt/container_registry_authentication_service.rb
index 91bad347278..b60cd3c57e5 100644
--- a/app/services/jwt/container_registry_authentication_service.rb
+++ b/app/services/jwt/container_registry_authentication_service.rb
@@ -3,6 +3,8 @@ module JWT
     AUDIENCE = 'container_registry'
 
     def execute
+      return error('not found', 404) unless registry.enabled
+
       if params[:offline_token]
         return error('forbidden', 403) unless current_user
       end
@@ -65,9 +67,11 @@ module JWT
     end
 
     def can_access?(requested_project, requested_action)
+      return false unless requested_project.container_registry_enabled?
+
       case requested_action
       when 'pull'
-        requested_project.public? || requested_project == project || can?(current_user, :read_container_registry, requested_project)
+        requested_project == project || can?(current_user, :read_container_registry, requested_project)
       when 'push'
         requested_project == project || can?(current_user, :create_container_registry, requested_project)
       else
diff --git a/app/services/projects/destroy_service.rb b/app/services/projects/destroy_service.rb
index 0ff2bc3cb81..d3920ac8baa 100644
--- a/app/services/projects/destroy_service.rb
+++ b/app/services/projects/destroy_service.rb
@@ -64,7 +64,7 @@ module Projects
     end
 
     def remove_registry_tags
-      return unless Gitlab.config.registry.enabled
+      return true unless Gitlab.config.registry.enabled
 
       project.container_registry_repository.delete_tags
     end
diff --git a/spec/services/jwt/container_registry_authentication_service_spec.rb b/spec/services/jwt/container_registry_authentication_service_spec.rb
index 1873ea2639b..672a7579dd3 100644
--- a/spec/services/jwt/container_registry_authentication_service_spec.rb
+++ b/spec/services/jwt/container_registry_authentication_service_spec.rb
@@ -7,6 +7,7 @@ describe JWT::ContainerRegistryAuthenticationService, services: true do
   let(:rsa_key) { OpenSSL::PKey::RSA.generate(512) }
   let(:registry_settings) do
     {
+      enabled: true,
       issuer: 'rspec',
       key: nil
     }
@@ -146,7 +147,20 @@ describe JWT::ContainerRegistryAuthenticationService, services: true do
           it_behaves_like 'a forbidden'
         end
       end
+    end
+
+    context 'for project without container registry' do
+      let(:project) { create(:empty_project, :public, container_registry_enabled: false) }
+
+      before { project.update(container_registry_enabled: false) }
 
+      context 'disallow when pulling' do
+        let(:current_params) do
+          { scope: "repository:#{project.path_with_namespace}:pull" }
+        end
+
+        it_behaves_like 'a forbidden'
+      end
     end
   end
 
-- 
GitLab