Don't allow non-administrators to remove images

Fixes #949

Signed-off-by: Miquel Sabaté Solà <[email protected]>

app/controllers/repositories_controller.rb

@@ -27,6 +27,8 @@ def toggle_star
 
   # Removes all the tags that belong to this repository, and removes it.
   def destroy
+    authorize @repository
+
     # First of all we mark the repo and the tags, so we don't have concurrency
     # problems when "delete" events come in.
     @repository.tags.update_all(marked: true)

app/controllers/tags_controller.rb

@@ -6,6 +6,7 @@ class TagsController < ApplicationController
   # the tags.
   def destroy
     tag = Tag.find(params[:id])
+    authorize tag
 
     # And now remove the tag by the digest. If the repository containing said
     # tags becomes empty after that, remove it too.

app/helpers/repositories_helper.rb

@@ -12,6 +12,12 @@ def render_delete_activity(activity)
     render_repo_activity(activity, "deleted")
   end
 
+  # Returns true if the user can remove the given repository.
+  def can_destroy?(repository)
+    APP_CONFIG.enabled?("delete") &&
+      RepositoryPolicy.new(current_user, repository).destroy?
+  end
+
   protected
 
   # General method for rendering an activity regarding repositories.

app/policies/repository_policy.rb

@@ -13,6 +13,12 @@ def show?
       @repository.namespace.team.users.exists?(user.id)
   end
 
+  # Returns true if the repository can be destroyed.
+  def destroy?
+    raise Pundit::NotAuthorizedError, "must be logged in" unless @user
+    @user.admin? || @repository.namespace.team.owners.exists?(user.id)
+  end
+
   class Scope
     attr_reader :user, :scope
 

app/policies/tag_policy.rb

@@ -0,0 +1,15 @@
+# Defines the policy for the tag object.
+class TagPolicy
+  attr_reader :user, :tag
+
+  def initialize(user, tag)
+    @user = user
+    @tag = tag
+  end
+
+  # Returns true if the tag can be destroyed.
+  def destroy?
+    raise Pundit::NotAuthorizedError, "must be logged in" unless @user
+    @user.admin? || @tag.repository.namespace.team.owners.exists?(user.id)
+  end
+end

app/views/repositories/show.html.slim

@@ -11,7 +11,7 @@
           i.fa.fa-star-o
         span#star-counter.badge
           = @repository.stars.count
-  - if APP_CONFIG.enabled?("delete")
+  - if can_destroy?(@repository)
     #remove-repo.pull-right
       button.remove-repo.btn-x.btn-edit-role.btn.btn-link.btn-xs[
         data-placement="left"
@@ -89,7 +89,7 @@
         col.col-20
         col.col-15
         col.col-20
-        - if APP_CONFIG.enabled?("delete")
+        - if can_destroy?(@repository)
           col.col-5
         thead
           tr
@@ -130,7 +130,7 @@
                       = tag.first.image_id[0, 12]
                 td= tag.first.created_at
 
-                - if APP_CONFIG.enabled?("delete")
+                - if can_destroy?(@repository)
                   td
                     button.remove-tag.fa.fa-lg.fa-trash-o.btn-x.btn-edit-role.btn.btn-link[
                       data-placement="left"

spec/controllers/auth/sessions_controller_spec.rb

@@ -8,7 +8,7 @@
       APP_CONFIG["ldap"] = { "enabled" => true }
     end
 
-    it "sets the session hash on LDAP when it's the first time", focus: true do
+    it "sets the session hash on LDAP when it's the first time" do
       create(:registry)
 
       # Warden magic

spec/features/repositories_spec.rb

@@ -29,6 +29,17 @@
       expect(page).to have_content("Pull Viewer")
     end
 
+    scenario "The delete feature is available only for allowed users", js: true do
+      APP_CONFIG["delete"] = { "enabled" => true }
+
+      visit repository_path(repository)
+      expect(page).to have_content("Delete image")
+
+      login_as user2, scope: :user
+      visit repository_path(repository)
+      expect(page).to_not have_content("Delete image")
+    end
+
     scenario "A user can star a repository", js: true do
       visit repository_path(repository)
       expect(find("#toggle_star")).to be_visible

spec/policies/repository_policy_spec.rb

@@ -1,7 +1,6 @@
 require "rails_helper"
 
 describe RepositoryPolicy do
-
   subject { described_class }
 
   let(:registry)    { create(:registry) }
@@ -57,6 +56,55 @@
     end
   end
 
+  permissions :destroy? do
+    before :each do
+      public_namespace = create(
+        :namespace,
+        team:       team2,
+        visibility: Namespace.visibilities[:visibility_public],
+        registry:   registry)
+      @public_repository = create(:repository, namespace: public_namespace)
+
+      protected_namespace = create(
+        :namespace,
+        team:       team2,
+        visibility: Namespace.visibilities[:visibility_protected],
+        registry:   registry)
+      @protected_repository = create(:repository, namespace: protected_namespace)
+
+      private_namespace = create(:namespace, team: team2, registry: registry)
+      @private_repository = create(:repository, namespace: private_namespace)
+    end
+
+    it "grants access if the user is an admin" do
+      admin = create(:admin)
+      testing_repositories = [@public_repository, @private_repository]
+      testing_repositories.each do |repository|
+        expect(subject).to permit(admin, repository)
+      end
+    end
+
+    it "denies access if the namespace is public" do
+      expect(subject).to_not permit(user, @public_repository)
+    end
+
+    it "denies access if the namespace is protected" do
+      expect(subject).to_not permit(user, @protected_repository)
+    end
+
+    it "grants access if the repository belongs to a namespace of a team member" do
+      user3 = create(:user)
+      TeamUser.create(team: team2, user: user3, role: TeamUser.roles["viewer"])
+      expect(subject).to_not permit(user3, @private_repository)
+      TeamUser.find_by(team: team2, user: user3).update_attributes(role: TeamUser.roles["owner"])
+      expect(subject).to permit(user3, @private_repository)
+    end
+
+    it "denies access if repository is private and the user is no team member or an admin" do
+      expect(subject).to_not permit(user, @private_repository)
+    end
+  end
+
   describe "scope" do
     before :each do
       public_namespace = create(