add test and rebase

2 files changed, 97 insertions, 2 deletions

diff --git a/app/services/auth/container_registry_authentication_service.rb b/app/services/auth/container_registry_authentication_service.rb
index 99a3d9c2cf9..335cfdbc231 100644
--- a/app/services/auth/container_registry_authentication_service.rb
+++ b/app/services/auth/container_registry_authentication_service.rb
@@ -104,7 +104,7 @@ module Auth
       when 'push'
         build_can_push?(requested_project) || user_can_push?(requested_project)
       when '*'
-        requested_project == project || can?(current_user, :admin_container_image, requested_project)
+        user_can_delete?(requested_project)
       else
         false
       end
@@ -122,6 +122,11 @@ module Auth
         (requested_project == project || can?(current_user, :build_read_container_image, requested_project))
     end
 
+    def user_can_delete(requested_project)
+      has_authentication_ability?(:admin_container_image) &&
+        can?(current_user, :admin_container_image, requested_project)
+    end
+
     def user_can_pull?(requested_project)
       has_authentication_ability?(:read_container_image) &&
         can?(current_user, :read_container_image, requested_project)
diff --git a/spec/services/auth/container_registry_authentication_service_spec.rb b/spec/services/auth/container_registry_authentication_service_spec.rb
index e273dfe1552..e8d222a2a7f 100644
--- a/spec/services/auth/container_registry_authentication_service_spec.rb
+++ b/spec/services/auth/container_registry_authentication_service_spec.rb
@@ -57,6 +57,12 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do
     it { expect(payload).to include('access' => []) }
   end
 
+  shared_examples 'a deletable' do
+    it_behaves_like 'a accessible' do
+      let(:actions) { ['*'] }
+    end
+  end
+
   shared_examples 'a pullable' do
     it_behaves_like 'an accessible' do
       let(:actions) { ['pull'] }
@@ -127,6 +133,16 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do
         it_behaves_like 'container repository factory'
       end
 
+      context 'disallow developer to delete images' do
+        before { project.team << [current_user, :developer] }
+
+        let(:current_params) do
+          { scope: "repository:#{project.path_with_namespace}:*" }
+        end
+
+        it_behaves_like 'an inaccessible'
+      end
+
       context 'allow reporter to pull images' do
         before { project.team << [current_user, :reporter] }
 
@@ -140,6 +156,16 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do
         end
       end
 
+      context 'disallow reporter to delete images' do
+        before { project.team << [current_user, :reporter] }
+
+        let(:current_params) do
+          { scope: "repository:#{project.path_with_namespace}:*" }
+        end
+
+        it_behaves_like 'an inaccessible'
+      end
+
       context 'return a least of privileges' do
         before { project.team << [current_user, :reporter] }
 
@@ -161,6 +187,16 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do
         it_behaves_like 'an inaccessible'
         it_behaves_like 'not a container repository factory'
       end
+
+      context 'disallow guest to delete images' do
+        before { project.team << [current_user, :guest] }
+
+        let(:current_params) do
+          { scope: "repository:#{project.path_with_namespace}:*" }
+        end
+
+        it_behaves_like 'an inaccessible'
+      end
     end
 
     context 'for public project' do
@@ -192,6 +228,14 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do
         it_behaves_like 'an inaccessible'
         it_behaves_like 'not a container repository factory'
       end
+
+      context 'disallow anyone to delete images' do
+        let(:current_params) do
+          { scope: "repository:#{project.path_with_namespace}:*" }
+        end
+
+        it_behaves_like 'an inaccessible'
+      end
     end
 
     context 'for internal project' do
@@ -215,17 +259,53 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do
           it_behaves_like 'an inaccessible'
           it_behaves_like 'not a container repository factory'
         end
+
+        context 'disallow anyone to delete images' do
+          let(:current_params) do
+            { scope: "repository:#{project.path_with_namespace}:*" }
+          end
+
+          it_behaves_like 'an inaccessible'
+        end
       end
 
       context 'for external user' do
         let(:current_user) { create(:user, external: true) }
         let(:current_params) do
-          { scope: "repository:#{project.path_with_namespace}:pull,push" }
+          { scope: "repository:#{project.path_with_namespace}:pull,push,*" }
         end
 
         it_behaves_like 'an inaccessible'
         it_behaves_like 'not a container repository factory'
       end
+
+    end
+  end
+
+  context 'delete authorized as admin' do
+    let(:current_project) { create(:empty_project) }
+    let(:current_user) { create(:user) }
+    let(:authentication_abilities) do
+      [
+        :build_read_container_image,
+        :build_create_container_image
+      ]
+    end
+
+    before do
+      current_project.team << [current_project, :admin]
+    end
+
+    it_behaves_like 'a valid token'
+
+    context 'allow to delete images' do
+      let(:current_params) do
+        { scope: "repository:#{current_project.path_with_namespace}:*" }
+      end
+
+      it_behaves_like 'a deletable' do
+        let(:project) { current_project }
+      end
     end
   end
 
@@ -257,6 +337,16 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do
       end
     end
 
+    context 'disallow to delete images' do
+      let(:current_params) do
+        { scope: "repository:#{current_project.path_with_namespace}:*" }
+      end
+
+      it_behaves_like 'an inaccessible' do
+        let(:project) { current_project }
+      end
+    end
+
     context 'for other projects' do
       context 'when pulling' do
         let(:current_params) do