Skip to content

Commit 02d70e1

Browse files
committed
Remove private uploads endpoint, and moveable attachments as authorized downloads will go through uploads endpoint
1 parent e91efd3 commit 02d70e1

17 files changed

+81
-301
lines changed

app/controllers/private_uploads_controller.rb

Lines changed: 0 additions & 44 deletions
This file was deleted.

app/controllers/uploads_controller.rb

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
class UploadsController < ApplicationController
44
rescue_from ActionController::MissingFile, with: :raise_not_found_exception
5+
rescue_from SecurityError, with: :raise_not_found_exception
6+
rescue_from CanCan::AccessDenied, with: :raise_not_found_exception
57

68
MODELS_OVERRIDES = {
79
"operator_document_file" => "document_file",
@@ -58,22 +60,37 @@ def parse_upload_path
5860
def ensure_valid_db_record
5961
raise_not_found_exception unless allowed_models.include?(@model_name)
6062

61-
model_class = @model_name.classify.constantize
62-
raise_not_found_exception unless model_class.uploaders.keys.map(&:to_s).include?(@uploader_name) # ensure valid uploader
63+
@model_class = @model_name.classify.constantize
64+
raise_not_found_exception unless @model_class.uploaders.keys.map(&:to_s).include?(@uploader_name) # ensure valid uploader
6365

64-
record = model_class.find(@record_id)
65-
uploader = record.public_send(@uploader_name)
66-
db_filenames = [uploader.file.file, *uploader.versions.values.map { |v| v.file.file }].map { |f| File.basename(f) }
66+
find_record
67+
ensure_valid_filename
68+
end
69+
70+
def find_record
71+
@record = if current_user.present? && current_user.admin?
72+
@model_class.with_deleted.find(@record_id)
73+
else
74+
@model_class.find(@record_id)
75+
end
76+
@uploader = @record.public_send(@uploader_name)
77+
rescue ActiveRecord::RecordNotFound, NameError
78+
raise_not_found_exception
79+
end
80+
81+
def ensure_valid_filename
82+
# do not check for admins, could download other files like from papertrail history
83+
return if current_user.present? && current_user.admin?
84+
85+
db_filenames = [@uploader.file.file, *@uploader.versions.values.map { |v| v.file.file }].map { |f| File.basename(f) }
6786

6887
unless db_filenames.include?(File.basename(@sanitized_filepath))
6988
raise_not_found_exception
7089
end
71-
rescue NameError, ActiveRecord::RecordNotFound
72-
raise_not_found_exception
7390
end
7491

7592
def allowed_models
76-
uploads_root = ApplicationUploader.new.public_root.join("uploads")
93+
uploads_root = ApplicationUploader.new.root.join("uploads")
7794
Dir.entries(uploads_root)
7895
.select { |entry| File.directory?(uploads_root.join(entry)) }
7996
.reject { |entry| entry.start_with?(".") || entry == "tmp" }

app/models/ability.rb

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@ def initialize(user = nil)
3333
can [:read], User, id: user.id
3434
end
3535
end
36-
3736
can :read, [Country, Fmu, Category, Subcategory, Law, Species,
3837
OperatorDocument, OperatorDocumentHistory, RequiredOperatorDocument,
3938
RequiredOperatorDocumentGroup, ObservationReport, ObservationDocument,

app/models/concerns/moveable_attachment.rb

Lines changed: 0 additions & 41 deletions
This file was deleted.

app/models/gov_document.rb

Lines changed: 1 addition & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@ class GovDocument < ApplicationRecord
3333
belongs_to :required_gov_document, -> { with_archived }, inverse_of: :gov_documents
3434

3535
mount_base64_uploader :attachment, GovDocumentUploader
36-
include MoveableAttachment
3736

3837
before_validation :set_expire_date, if: proc { |d| d.required_gov_document.valid_period? }
3938
before_validation :clear_wrong_fields
@@ -43,11 +42,7 @@ class GovDocument < ApplicationRecord
4342
validates :start_date, presence: true, if: proc { |d| d.required_gov_document.valid_period? && d.has_data? }
4443
validates :expire_date, presence: true, if: proc { |d| d.required_gov_document.valid_period? && d.has_data? }
4544

46-
after_update :move_previous_attachment_to_private_directory, if: :saved_change_to_attachment?
47-
48-
skip_callback :commit, :after, :remove_attachment!
49-
after_destroy :move_attachment_to_private_directory
50-
after_restore :move_attachment_to_public_directory
45+
skip_callback :commit, :after, :remove_attachment! # skip removal after destroying the record, skip after update done in uploader
5146
after_real_destroy :remove_attachment!
5247

5348
scope :with_archived, -> { unscope(where: :deleted_at) }
@@ -97,27 +92,6 @@ def set_country
9792
self.country_id = required_gov_document.country_id
9893
end
9994

100-
def move_previous_attachment_to_private_directory
101-
previous_attachment_filename = previous_changes[:attachment][0]
102-
return if previous_attachment_filename.blank?
103-
104-
from = File.join(attachment.public_root, attachment.store_dir, previous_attachment_filename)
105-
to = File.join(attachment.private_root, attachment.store_dir, previous_attachment_filename)
106-
FileUtils.makedirs(File.dirname(to))
107-
system "mv #{Shellwords.escape(from)} #{Shellwords.escape(to)}"
108-
end
109-
110-
# we only want to move current attachment back to public directory
111-
def move_attachment_to_public_directory
112-
attachment_attr = self[:attachment]
113-
return if attachment_attr.nil?
114-
115-
from = File.join(attachment.private_root, attachment.store_dir, attachment_attr)
116-
to = File.join(attachment.public_root, attachment.store_dir, attachment_attr)
117-
FileUtils.makedirs(File.dirname(to))
118-
system "mv #{Shellwords.escape(from)} #{Shellwords.escape(to)}"
119-
end
120-
12195
def clear_wrong_fields
12296
case required_gov_document.document_type
12397
when "file"

app/models/observation_document.rb

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ class ObservationDocument < ApplicationRecord
1818
has_paper_trail
1919
acts_as_paranoid
2020
mount_base64_uploader :attachment, ObservationDocumentUploader
21-
include MoveableAttachment
2221

2322
enum :document_type, {
2423
"Government Documents" => 0, "Company Documents" => 1, "Photos" => 2,
@@ -32,7 +31,5 @@ class ObservationDocument < ApplicationRecord
3231
has_and_belongs_to_many :observations, inverse_of: :observation_documents
3332

3433
skip_callback :commit, :after, :remove_attachment!
35-
after_destroy :move_attachment_to_private_directory
36-
after_restore :move_attachment_to_public_directory
3734
after_real_destroy :remove_attachment!
3835
end

app/models/observation_report.rb

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
class ObservationReport < ApplicationRecord
1818
has_paper_trail
1919
mount_base64_uploader :attachment, ObservationReportUploader
20-
include MoveableAttachment
2120

2221
acts_as_paranoid
2322

@@ -35,8 +34,6 @@ class ObservationReport < ApplicationRecord
3534
validates :publication_date, presence: true
3635

3736
skip_callback :commit, :after, :remove_attachment!
38-
after_destroy :move_attachment_to_private_directory
39-
after_restore :move_attachment_to_public_directory
4037
after_real_destroy :remove_attachment!
4138

4239
attr_accessor :skip_observers_sync

app/uploaders/application_uploader.rb

Lines changed: 2 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,9 @@ class ApplicationUploader < CarrierWave::Uploader::Base
44
storage :file
55

66
def root
7-
return private_root if private_upload?
7+
return Rails.root.join("tmp") if Rails.env.test?
88

9-
public_root
9+
Rails.root
1010
end
1111

1212
def cache_dir
@@ -15,12 +15,6 @@ def cache_dir
1515
super
1616
end
1717

18-
def url(*args)
19-
return super&.gsub("/uploads/", "/private/uploads/") if private_upload?
20-
21-
super
22-
end
23-
2418
def store_dir
2519
"uploads/#{model.class.to_s.underscore}/#{mounted_as}/#{model.id}"
2620
end
@@ -29,22 +23,6 @@ def exists?
2923
file.present?
3024
end
3125

32-
def private_upload?
33-
false
34-
end
35-
36-
def public_root
37-
return Rails.root.join("tmp") if Rails.env.test?
38-
39-
Rails.root
40-
end
41-
42-
def private_root
43-
return Rails.root.join("tmp", "private") if Rails.env.test?
44-
45-
Rails.root.join("private")
46-
end
47-
4826
def original_filename
4927
if file.present?
5028
file.filename

app/uploaders/gov_document_uploader.rb

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,6 @@ def filename
1717
sanitize_filename(filename)
1818
end
1919

20-
def private_upload?
21-
model.deleted? || !model.paper_trail.live?
22-
end
23-
2420
protected
2521

2622
def random_token

app/uploaders/observation_document_uploader.rb

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,4 @@ def filename
1919

2020
sanitize_filename(filename + File.extname(super))
2121
end
22-
23-
def private_upload?
24-
model.deleted?
25-
end
2622
end

0 commit comments

Comments
 (0)