From 756362f89e4276b08c8be9bf5813a51a900bb0fc Mon Sep 17 00:00:00 2001 From: evazion Date: Tue, 29 Nov 2022 18:09:25 -0600 Subject: [PATCH] Fix #4990: Allow admins to delete uploads. Allow admins to delete media asset files. This only deletes the image file itself, not the upload or media asset record. The upload will still be in the user's upload list, but the image will be gone. The media asset page will still exist, but it will only show the file's metadata, not the image itself. We don't delete the metadata so we have a record of what the file's MD5 was and who uploaded it, to prevent the file from being uploaded again and to take action against the user if necessary. --- app/controllers/media_assets_controller.rb | 9 ++++++- .../src/styles/base/040_colors.scss | 10 +++++-- .../src/styles/common/utilities.scss | 9 +++++-- app/models/media_asset.rb | 18 ++++++++----- app/models/mod_action.rb | 2 ++ app/models/post.rb | 2 +- app/policies/media_asset_policy.rb | 4 +++ app/views/media_assets/destroy.js.erb | 1 + app/views/media_assets/show.html.erb | 12 +++++++++ config/routes.rb | 2 +- .../media_assets_controller_test.rb | 27 +++++++++++++++++++ 11 files changed, 83 insertions(+), 13 deletions(-) create mode 100644 app/views/media_assets/destroy.js.erb diff --git a/app/controllers/media_assets_controller.rb b/app/controllers/media_assets_controller.rb index 6beb3a5b0..91c98f305 100644 --- a/app/controllers/media_assets_controller.rb +++ b/app/controllers/media_assets_controller.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class MediaAssetsController < ApplicationController - respond_to :html, :json, :xml + respond_to :html, :json, :xml, :js rate_limit :image, rate: 5.0/1.seconds, burst: 50 @@ -36,6 +36,13 @@ class MediaAssetsController < ApplicationController end end + def destroy + @media_asset = authorize MediaAsset.find(params[:id]) + @media_asset.trash!(CurrentUser.user) + flash[:notice] = "File deleted" + respond_with(@media_asset) + end + def image media_asset = authorize MediaAsset.find(params[:media_asset_id]) variant = media_asset.variant(params[:variant]) diff --git a/app/javascript/src/styles/base/040_colors.scss b/app/javascript/src/styles/base/040_colors.scss index e6b649e98..9243c83ae 100644 --- a/app/javascript/src/styles/base/040_colors.scss +++ b/app/javascript/src/styles/base/040_colors.scss @@ -119,6 +119,9 @@ html, body[data-current-user-theme="light"] { --success-color: var(--green-5); --error-color: var(--red-5); + --success-hover-color: var(--green-4); + --error-hover-color: var(--red-4); + --error-background-color: var(--red-1); --success-background-color: var(--green-0); --target-background: var(--yellow-0); @@ -342,8 +345,11 @@ html, body[data-current-user-theme="light"] { --link-color: var(--azure-4); --link-hover-color: var(--azure-3); - --success-color: var(--green-3); - --error-color: var(--red-3); + --success-color: var(--green-4); + --error-color: var(--red-4); + + --success-hover-color: var(--green-3); + --error-hover-color: var(--red-3); --default-border-color: var(--grey-7); diff --git a/app/javascript/src/styles/common/utilities.scss b/app/javascript/src/styles/common/utilities.scss index 49fa498e6..1553c3864 100644 --- a/app/javascript/src/styles/common/utilities.scss +++ b/app/javascript/src/styles/common/utilities.scss @@ -252,8 +252,13 @@ $spacer: 0.25rem; /* 4px */ .link-color { color: var(--link-color); } -.text-success { color: var(--success-color); } -.text-error { color: var(--error-color); } +.text-success { color: var(--success-color) !important; } +.text-error { color: var(--error-color) !important; } +.text-danger { color: var(--error-color) !important; } + +a.text-success:hover { color: var(--success-hover-color) !important; } +a.text-error:hover { color: var(--error-hover-color) !important; } +a.text-danger:hover { color: var(--error-hover-color) !important; } .transition-opacity { transition: opacity 0.15s ease; diff --git a/app/models/media_asset.rb b/app/models/media_asset.rb index a5795e799..37ca6b79e 100644 --- a/app/models/media_asset.rb +++ b/app/models/media_asset.rb @@ -392,17 +392,23 @@ class MediaAsset < ApplicationRecord end end - def expunge! - delete_files! - update!(status: :expunged) + def expunge!(current_user, log: true) + with_lock do + delete_files! + update!(status: :expunged) + ModAction.log("expunged media asset ##{id} (md5=#{md5})", :media_asset_expunge, subject: self, user: current_user) if log + end rescue update!(status: :failed) raise end - def trash! - variants.each(&:trash_file!) - update!(status: :deleted) + def trash!(current_user, log: true) + with_lock do + variants.each(&:trash_file!) + update!(status: :deleted) + ModAction.log("deleted media asset ##{id} (md5=#{md5})", :media_asset_delete, subject: self, user: current_user) if log + end rescue update!(status: :failed) raise diff --git a/app/models/mod_action.rb b/app/models/mod_action.rb index f895be5d7..465301eb0 100644 --- a/app/models/mod_action.rb +++ b/app/models/mod_action.rb @@ -42,6 +42,8 @@ class ModAction < ApplicationRecord post_vote_undelete: 233, pool_delete: 62, pool_undelete: 63, + media_asset_delete: 72, + media_asset_expunge: 76, artist_ban: 184, artist_unban: 185, comment_update: 81, diff --git a/app/models/post.rb b/app/models/post.rb index aa3c4bafb..b24913b97 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -780,7 +780,7 @@ class Post < ApplicationRecord decrement_tag_post_counts remove_from_all_pools remove_from_fav_groups - media_asset.trash! + media_asset.trash!(current_user, log: false) destroy update_parent_on_destroy end diff --git a/app/policies/media_asset_policy.rb b/app/policies/media_asset_policy.rb index 6b704ea37..c28e09a9d 100644 --- a/app/policies/media_asset_policy.rb +++ b/app/policies/media_asset_policy.rb @@ -5,6 +5,10 @@ class MediaAssetPolicy < ApplicationPolicy true end + def destroy? + user.is_admin? + end + def image? can_see_image? end diff --git a/app/views/media_assets/destroy.js.erb b/app/views/media_assets/destroy.js.erb new file mode 100644 index 000000000..345366b9b --- /dev/null +++ b/app/views/media_assets/destroy.js.erb @@ -0,0 +1 @@ +location.reload(); diff --git a/app/views/media_assets/show.html.erb b/app/views/media_assets/show.html.erb index 4d093283c..6ce901d33 100644 --- a/app/views/media_assets/show.html.erb +++ b/app/views/media_assets/show.html.erb @@ -73,6 +73,18 @@ <%= search_icon %> <%= Danbooru.config.app_name %> <% end %> <% end %> + + <% if policy(@media_asset).destroy? && @media_asset.post.nil? && !@media_asset.deleted? && !@media_asset.expunged? %> + <% menu.item do %> +
+ <% end %> + + <% menu.item do %> + <%= link_to media_asset_path(@media_asset), class: "text-danger", remote: true, xhr: true, method: :delete, "data-confirm": "This will permanently delete this file. Are you sure?" do %> + <%= delete_icon %> Delete file + <% end %> + <% end %> + <% end %> <% end %> diff --git a/config/routes.rb b/config/routes.rb index 293e9fb7f..260ae31e2 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -141,7 +141,7 @@ Rails.application.routes.draw do get :check, to: redirect {|path_params, req| "/iqdb_queries?#{req.query_string}"} end end - resources :media_assets, only: [:index, :show] do + resources :media_assets, only: [:index, :show, :destroy] do get "/:variant", to: "media_assets#image", as: :image end resources :media_metadata, only: [:index] diff --git a/test/functional/media_assets_controller_test.rb b/test/functional/media_assets_controller_test.rb index 58115735a..60d745bc5 100644 --- a/test/functional/media_assets_controller_test.rb +++ b/test/functional/media_assets_controller_test.rb @@ -44,5 +44,32 @@ class MediaAssetsControllerTest < ActionDispatch::IntegrationTest assert_response :success end end + + context "destroy action" do + should "delete the asset's files" do + @admin = create(:admin_user) + @media_asset = MediaAsset.upload!("test/files/test.jpg") + delete_auth media_asset_path(@media_asset), @admin + + assert_redirected_to @media_asset + + assert_equal("deleted", @media_asset.reload.status) + @media_asset.variants.each do |variant| + assert_nil(variant.open_file) + end + + assert_equal(1, ModAction.count) + assert_equal("media_asset_delete", ModAction.last.category) + assert_equal(@media_asset, ModAction.last.subject) + assert_equal(@admin, ModAction.last.creator) + end + + should "fail for non-admins" do + @media_asset = create(:media_asset) + delete_auth media_asset_path(@media_asset), create(:user) + + assert_response 403 + end + end end end