rename to allowlist

This commit is contained in:
ulferts
2025-06-20 17:25:15 +02:00
parent 31573972b0
commit 0ecd09b06b
16 changed files with 68 additions and 53 deletions
+12 -12
View File
@@ -68,29 +68,29 @@ module Attachments
end
##
# Validates the content type, if a whitelist is set
# Validates the content type, if a allowlist is set
def validate_content_type
# If the whitelist is empty, assume all files are allowed
# If the allowlist is empty, assume all files are allowed
# as before
unless matches_whitelist?(attachment_whitelist)
Rails.logger.info { "Uploaded file #{model.filename} with type #{model.content_type} does not match whitelist" }
errors.add :content_type, :not_whitelisted, value: model.content_type
unless matches_allowlist?(attachment_allowlist)
Rails.logger.info { "Uploaded file #{model.filename} with type #{model.content_type} does not match allowlist" }
errors.add :content_type, :not_allowlisted, value: model.content_type
end
end
##
# Get the user-defined whitelist or a custom whitelist
# Get the user-defined allowlist or a custom allowlist
# defined for this invocation
def attachment_whitelist
Array(options.fetch(:whitelist, Setting.attachment_whitelist))
def attachment_allowlist
Array(options.fetch(:allowlist, Setting.attachment_whitelist))
end
##
# Returns whether the attachment matches the whitelist
def matches_whitelist?(whitelist)
return true if whitelist.empty?
# Returns whether the attachment matches the allowlist
def matches_allowlist?(allowlist)
return true if allowlist.empty?
whitelist.include?(model.content_type) || whitelist.include?("*#{model.extension}")
allowlist.include?(model.content_type) || allowlist.include?("*#{model.extension}")
end
end
end
+2 -2
View File
@@ -36,8 +36,8 @@ module Attachments
# @param whitelist A custom whitelist to validate with, or empty to disable validation
#
# Warning: When passing an empty whitelist, this results in no validations on the content type taking place.
def self.bypass_whitelist(user:, whitelist: [])
new(user:, contract_options: { whitelist: whitelist.map(&:to_s) })
def self.bypass_allowlist(user:, allowlist: [])
new(user:, contract_options: { allowlist: allowlist.map(&:to_s) })
end
end
end
+1 -1
View File
@@ -134,7 +134,7 @@ class BackupJob < ApplicationJob
def store_backup(file_name, backup:, user:)
File.open(file_name) do |file|
call = Attachments::CreateService
.bypass_whitelist(user:)
.bypass_allowlist(user:)
.call(container: backup, filename: file_name, file:, description: "OpenProject backup")
call.on_success do
+1 -1
View File
@@ -117,7 +117,7 @@ module Exports
filename = clean_filename(export_result)
call = Attachments::CreateService
.bypass_whitelist(user: User.current)
.bypass_allowlist(user: User.current)
.call(container:, file:, filename:, description: "")
call.on_success do
+1 -1
View File
@@ -1286,7 +1286,7 @@ en:
attributes:
content_type:
blank: "The content type of the file cannot be blank."
not_whitelisted: "The file was rejected by an automatic filter. '%{value}' is not whitelisted for upload."
not_allowlisted: "The file was rejected by an automatic filter. '%{value}' is not allowed for upload."
format: "%{message}"
capability:
context:
@@ -215,7 +215,7 @@ module Bim
def create_attachment
filename = params[:bcf_file].original_filename
call = Attachments::CreateService
.bypass_whitelist(user: current_user, whitelist: %w[application/zip])
.bypass_allowlist(user: current_user, allowlist: %w[application/zip])
.call(file: params[:bcf_file],
filename:,
description: filename)
@@ -49,6 +49,10 @@ module Bim
.includes(:project, :uploader)
end
def show
frontend_redirect params[:id].to_i
end
def new
@ifc_model = @project.ifc_models.build
prepare_form(@ifc_model)
@@ -58,10 +62,6 @@ module Bim
prepare_form(@ifc_model)
end
def show
frontend_redirect params[:id].to_i
end
def defaults
frontend_redirect @ifc_models.defaults.pluck(:id).uniq
end
@@ -116,7 +116,7 @@ module Bim
if service_result.success?
::Attachments::FinishDirectUploadJob.perform_later attachment.id,
whitelist: false
allowlist: false
flash[:notice] = if new_model
t("ifc_models.flash_messages.upload_successful")
@@ -183,7 +183,7 @@ module Bim
return unless OpenProject::Configuration.direct_uploads?
call = ::Attachments::PrepareUploadService
.bypass_whitelist(user: current_user)
.bypass_allowlist(user: current_user)
.call(filename: "model.ifc", filesize: 0)
call.on_failure { flash[:error] = call.message }
+1 -1
View File
@@ -73,7 +73,7 @@ module Bim::Bcf
def build_snapshot(file, user: User.current)
::Attachments::BuildService
.bypass_whitelist(user:)
.bypass_allowlist(user:)
.call(file:, container: self, filename: file.original_filename, description: "snapshot")
.result
end
@@ -41,7 +41,7 @@ module Bim
delete_attachment name
filename = file.respond_to?(:original_filename) ? file.original_filename : File.basename(file.path)
call = ::Attachments::CreateService
.bypass_whitelist(user: User.current)
.bypass_allowlist(user: User.current)
.call(file:, container: self, filename:, description: name)
call.on_failure { Rails.logger.error "Failed to add #{name} attachment: #{call.message}" }
@@ -78,7 +78,7 @@ module Bim
def build_ifc_attachment(ifc_attachment)
::Attachments::BuildService
.bypass_whitelist(user:)
.bypass_allowlist(user:)
.call(file: ifc_attachment, container: model, filename: ifc_attachment.original_filename, description: "ifc")
.on_failure do |build_attachment_result|
build_attachment_result.errors.each do |error|
@@ -102,7 +102,7 @@ RSpec.shared_examples_for "ifc model contract" do
let(:ifc_file) { FileHelpers.mock_uploaded_file name: "model.ifc", content_type: "application/binary", binary: true }
let(:ifc_attachment) do
Attachments::BuildService
.bypass_whitelist(user: current_user)
.bypass_allowlist(user: current_user)
.call(file: ifc_file, filename: "model.ifc")
.result
end
@@ -118,7 +118,7 @@ RSpec.shared_examples_for "ifc model contract" do
end
let(:ifc_attachment) do
Attachments::BuildService
.bypass_whitelist(user: current_user)
.bypass_allowlist(user: current_user)
.call(file: ifc_file, filename: "model.ifc")
.result
end
@@ -66,15 +66,14 @@ RSpec.describe WorkPackages::ExportJob do
service = double("attachments create service")
expect(Attachments::CreateService)
.to receive(:bypass_whitelist)
.with(user:)
allow(Attachments::CreateService)
.to receive(:bypass_allowlist)
.and_return(service)
expect(Exports::CleanupOutdatedJob)
allow(Exports::CleanupOutdatedJob)
.to receive(:perform_after_grace)
expect(service)
allow(service)
.to(receive(:call))
.and_return(ServiceResult.success(result: attachment))
@@ -85,6 +84,16 @@ RSpec.describe WorkPackages::ExportJob do
expect(subject.job_status).to be_present
expect(subject.job_status[:status]).to eq "success"
expect(subject.job_status[:payload]["download"]).to eq "/api/v3/attachments/1234/content"
expect(Attachments::CreateService)
.to have_received(:bypass_allowlist)
.with(user:)
expect(Exports::CleanupOutdatedJob)
.to have_received(:perform_after_grace)
expect(service)
.to have_received(:call)
end
end
end
@@ -104,28 +104,28 @@ RSpec.describe Attachments::CreateContract do
end
end
context "with an empty whitelist",
context "with an empty allowlist",
with_settings: { attachment_whitelist: %w[] } do
it_behaves_like "contract is valid"
end
context "with a matching mime whitelist",
context "with a matching mime allowlist",
with_settings: { attachment_whitelist: %w[image/png] } do
it_behaves_like "contract is valid"
end
context "with a matching extension whitelist",
context "with a matching extension allowlist",
with_settings: { attachment_whitelist: %w[*.png] } do
it_behaves_like "contract is valid"
end
context "with a non-matching whitelist",
context "with a non-matching allowlist",
with_settings: { attachment_whitelist: %w[*.jpg image/jpeg] } do
it_behaves_like "contract is invalid", content_type: :not_whitelisted
it_behaves_like "contract is invalid", content_type: :not_allowlisted
context "when disabling the whitelist check" do
let(:contract_options) do
{ whitelist: [] }
{ allowlist: [] }
end
it_behaves_like "contract is valid"
@@ -133,7 +133,7 @@ RSpec.describe Attachments::CreateContract do
context "when overriding the whitelist" do
let(:contract_options) do
{ whitelist: %w[*.png] }
{ allowlist: %w[*.png] }
end
it_behaves_like "contract is valid"
@@ -157,7 +157,7 @@ RSpec.shared_examples "it supports direct uploads" do
end
end
context "with an attachment whitelist", with_settings: { attachment_whitelist: ["text/csv"] } do
context "with an attachment allowlist", with_settings: { attachment_whitelist: ["text/csv"] } do
context "with an allowed content type" do
let(:metadata) { { fileName: "cats.csv", fileSize: file.size, contentType: "text/csv" } }
@@ -171,14 +171,14 @@ RSpec.shared_examples "it supports direct uploads" do
it "fails" do
expect(subject.status).to eq 422
expect(subject.body).to include "not whitelisted"
expect(subject.body).to include "'text/plain' is not allowed for upload."
end
end
context "with a non-specific content type not on the whitelist" do
context "with a non-specific content type not on the allowlist" do
let(:metadata) { { fileName: "cats.bin", fileSize: file.size, contentType: "application/binary" } }
# the actual whitelist check will be performed in the FinishDirectUpload job in this case
# the actual allowlist check will be performed in the FinishDirectUpload job in this case
it "still succeeds" do
expect(subject.status).to eq 201
end
@@ -136,7 +136,7 @@ RSpec.describe Attachments::FinishDirectUploadJob, "integration", type: :job do
it_behaves_like "adding a journal to the attachment in the name of the attachment's author"
end
context "with an incompatible attachment whitelist",
context "with an incompatible attachment allowlist",
with_settings: { attachment_whitelist: %w[image/png] } do
let!(:container) { create(:work_package) }
let!(:container_timestamp) { container.updated_at }
@@ -156,9 +156,9 @@ RSpec.describe Attachments::FinishDirectUploadJob, "integration", type: :job do
expect { pending_attachment.reload }.to raise_error(ActiveRecord::RecordNotFound)
end
context "when the job is getting a whitelist override" do
context "when the job is getting a allowlist override" do
it "Does save the attachment" do
job.perform(pending_attachment.id, whitelist: false)
job.perform(pending_attachment.id, allowlist: false)
container.reload
@@ -68,15 +68,14 @@ RSpec.describe WorkPackages::ExportJob do
let(:exporter_instance) { instance_double(exporter) }
it "exports" do
expect(Attachments::CreateService)
.to receive(:bypass_whitelist)
.with(user:)
allow(Attachments::CreateService)
.to receive(:bypass_allowlist)
.and_return(service)
expect(Exports::CleanupOutdatedJob)
allow(Exports::CleanupOutdatedJob)
.to receive(:perform_after_grace)
expect(service)
allow(service)
.to receive(:call) do |file:, **_args|
expect(File.basename(file))
.to start_with "some_title"
@@ -96,6 +95,13 @@ RSpec.describe WorkPackages::ExportJob do
expect(subject.job_status.reference).to eq export
expect(subject.job_status[:status]).to eq "success"
expect(subject.job_status[:payload]["download"]).to eq "/api/v3/attachments/1234/content"
expect(Attachments::CreateService)
.to have_received(:bypass_allowlist)
.with(user:)
expect(Exports::CleanupOutdatedJob)
.to have_received(:perform_after_grace)
end
end