mirror of
https://github.com/discourse/discourse.git
synced 2025-02-25 18:55:32 -06:00
FIX: Query UploadReference in UploadSecurity for existing uploads (#19917)
This fixes a longstanding issue for sites with the secure_uploads setting enabled. What would happen is a scenario like this, since we did not check all places an upload could be linked to whenever we used UploadSecurity to check whether an upload should be secure: * Upload is created and used for site setting, set to secure: false since site setting uploads should not be secure. Let's say favicon * Favicon for the site is used inside a post in a private category, e.g. via a Onebox * We changed the secure status for the upload to true, since it's been used in a private category and we don't check if it's originator was a public place * The site favicon breaks :'( This was a source of constant consternation. Now, when an upload is _not_ being created, and we are checking if an existing upload should be secure, we now check to see what the first record in the UploadReference table is for that upload. If it's something public like a site setting, then we will never change the upload to `secure`.
This commit is contained in:
parent
df4a9f96ae
commit
4d2a95ffe6
@ -537,6 +537,10 @@ task "uploads:sync_s3_acls" => :environment do
|
|||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
#
|
||||||
|
# TODO (martin) Update this rake task to use the _first_ UploadReference
|
||||||
|
# record for each upload to determine security, and do not mark things
|
||||||
|
# as secure if the first record is something public e.g. a site setting.
|
||||||
task "uploads:disable_secure_uploads" => :environment do
|
task "uploads:disable_secure_uploads" => :environment do
|
||||||
RailsMultisite::ConnectionManagement.each_connection do |db|
|
RailsMultisite::ConnectionManagement.each_connection do |db|
|
||||||
unless Discourse.store.external?
|
unless Discourse.store.external?
|
||||||
@ -584,6 +588,10 @@ end
|
|||||||
# the upload secure flag and S3 upload ACLs. Any uploads that
|
# the upload secure flag and S3 upload ACLs. Any uploads that
|
||||||
# have their secure status changed will have all associated posts
|
# have their secure status changed will have all associated posts
|
||||||
# rebaked.
|
# rebaked.
|
||||||
|
#
|
||||||
|
# TODO (martin) Update this rake task to use the _first_ UploadReference
|
||||||
|
# record for each upload to determine security, and do not mark things
|
||||||
|
# as secure if the first record is something public e.g. a site setting.
|
||||||
task "uploads:secure_upload_analyse_and_update" => :environment do
|
task "uploads:secure_upload_analyse_and_update" => :environment do
|
||||||
RailsMultisite::ConnectionManagement.each_connection do |db|
|
RailsMultisite::ConnectionManagement.each_connection do |db|
|
||||||
unless Discourse.store.external?
|
unless Discourse.store.external?
|
||||||
|
@ -13,8 +13,12 @@
|
|||||||
# original post the upload is linked to has far more bearing on its security context
|
# original post the upload is linked to has far more bearing on its security context
|
||||||
# post-upload. If the access_control_post_id does not exist then we just rely
|
# post-upload. If the access_control_post_id does not exist then we just rely
|
||||||
# on the current secure? status, otherwise there would be a lot of additional
|
# on the current secure? status, otherwise there would be a lot of additional
|
||||||
# complex queries and joins to perform. Over time more of these specific
|
# complex queries and joins to perform.
|
||||||
# queries will be implemented.
|
#
|
||||||
|
# These queries will be performed only if the @creating option is false. So if
|
||||||
|
# an upload is included in a post, and it's an upload from a different source
|
||||||
|
# (e.g. a category logo, site setting upload) then we will determine secure
|
||||||
|
# state _based on the first place the upload was referenced_.
|
||||||
#
|
#
|
||||||
# NOTE: When updating this to add more cases where uploads will be marked
|
# NOTE: When updating this to add more cases where uploads will be marked
|
||||||
# secure, consider uploads:secure_upload_analyse_and_update as well, which
|
# secure, consider uploads:secure_upload_analyse_and_update as well, which
|
||||||
@ -35,6 +39,18 @@ class UploadSecurity
|
|||||||
badge_image
|
badge_image
|
||||||
]
|
]
|
||||||
|
|
||||||
|
PUBLIC_UPLOAD_REFERENCE_TYPES = %w[
|
||||||
|
Badge
|
||||||
|
Category
|
||||||
|
CustomEmoji
|
||||||
|
Group
|
||||||
|
SiteSetting
|
||||||
|
ThemeField
|
||||||
|
User
|
||||||
|
UserAvatar
|
||||||
|
UserProfile
|
||||||
|
]
|
||||||
|
|
||||||
def self.register_custom_public_type(type)
|
def self.register_custom_public_type(type)
|
||||||
@@custom_public_types << type if !@@custom_public_types.include?(type)
|
@@custom_public_types << type if !@@custom_public_types.include?(type)
|
||||||
end
|
end
|
||||||
@ -65,6 +81,46 @@ class UploadSecurity
|
|||||||
[false, "no checks satisfied"]
|
[false, "no checks satisfied"]
|
||||||
end
|
end
|
||||||
|
|
||||||
|
private
|
||||||
|
|
||||||
|
def access_control_post
|
||||||
|
@access_control_post ||=
|
||||||
|
@upload.access_control_post_id.present? ? @upload.access_control_post : nil
|
||||||
|
end
|
||||||
|
|
||||||
|
def insecure_context_checks
|
||||||
|
{
|
||||||
|
secure_uploads_disabled: "secure uploads is disabled",
|
||||||
|
insecure_creation_for_modifiers: "one or more creation for_modifiers was satisfied",
|
||||||
|
public_type: "upload is public type",
|
||||||
|
regular_emoji: "upload is used for regular emoji",
|
||||||
|
publicly_referenced_first: "upload was publicly referenced when it was first created",
|
||||||
|
}
|
||||||
|
end
|
||||||
|
|
||||||
|
def secure_context_checks
|
||||||
|
{
|
||||||
|
login_required: "login is required",
|
||||||
|
access_control_post_has_secure_uploads: "access control post dictates security",
|
||||||
|
secure_creation_for_modifiers: "one or more creation for_modifiers was satisfied",
|
||||||
|
uploading_in_composer: "uploading via the composer",
|
||||||
|
already_secure: "upload is already secure",
|
||||||
|
}
|
||||||
|
end
|
||||||
|
|
||||||
|
# The access control check is important because that is the truest indicator
|
||||||
|
# of whether an upload should be secure or not, and thus should be returned
|
||||||
|
# immediately if there is an access control post.
|
||||||
|
def priority_check?(check)
|
||||||
|
check == :access_control_post_has_secure_uploads && access_control_post
|
||||||
|
end
|
||||||
|
|
||||||
|
def perform_check(check)
|
||||||
|
send("#{check}_check")
|
||||||
|
end
|
||||||
|
|
||||||
|
#### START PUBLIC CHECKS ####
|
||||||
|
|
||||||
def secure_uploads_disabled_check
|
def secure_uploads_disabled_check
|
||||||
!SiteSetting.secure_uploads?
|
!SiteSetting.secure_uploads?
|
||||||
end
|
end
|
||||||
@ -78,8 +134,11 @@ class UploadSecurity
|
|||||||
PUBLIC_TYPES.include?(@upload_type) || @@custom_public_types.include?(@upload_type)
|
PUBLIC_TYPES.include?(@upload_type) || @@custom_public_types.include?(@upload_type)
|
||||||
end
|
end
|
||||||
|
|
||||||
def custom_emoji_check
|
def publicly_referenced_first_check
|
||||||
@upload.id.present? && CustomEmoji.exists?(upload_id: @upload.id)
|
return false if @creating
|
||||||
|
first_reference = @upload.upload_references.order(created_at: :asc).first
|
||||||
|
return false if first_reference.blank?
|
||||||
|
PUBLIC_UPLOAD_REFERENCE_TYPES.include?(first_reference.target_type)
|
||||||
end
|
end
|
||||||
|
|
||||||
def regular_emoji_check
|
def regular_emoji_check
|
||||||
@ -89,18 +148,26 @@ class UploadSecurity
|
|||||||
uri.path.include?("images/emoji")
|
uri.path.include?("images/emoji")
|
||||||
end
|
end
|
||||||
|
|
||||||
|
#### END PUBLIC CHECKS ####
|
||||||
|
|
||||||
|
#--------------------------#
|
||||||
|
|
||||||
|
#### START PRIVATE CHECKS ####
|
||||||
|
|
||||||
def login_required_check
|
def login_required_check
|
||||||
SiteSetting.login_required?
|
SiteSetting.login_required?
|
||||||
end
|
end
|
||||||
|
|
||||||
# whether the upload should remain secure or not after posting depends on its context,
|
# Whether the upload should remain secure or not after posting depends on its context,
|
||||||
# which is based on the post it is linked to via access_control_post_id.
|
# which is based on the post it is linked to via access_control_post_id.
|
||||||
# if that post is with_secure_uploads? then the upload should also be secure.
|
|
||||||
# this may change to false if the upload was set to secure on upload e.g. in
|
|
||||||
# a post composer then it turned out that the post itself was not in a secure context
|
|
||||||
#
|
#
|
||||||
# a post is with secure uploads if it is a private message or in a read restricted
|
# If that post is with_secure_uploads? then the upload should also be secure.
|
||||||
# category
|
#
|
||||||
|
# This may change to false if the upload was set to secure on upload e.g. in
|
||||||
|
# a post composer then it turned out that the post itself was not in a secure context.
|
||||||
|
#
|
||||||
|
# A post is with secure uploads if it is a private message or in a read restricted
|
||||||
|
# category. See `Post#with_secure_uploads?` for the full definition.
|
||||||
def access_control_post_has_secure_uploads_check
|
def access_control_post_has_secure_uploads_check
|
||||||
access_control_post&.with_secure_uploads?
|
access_control_post&.with_secure_uploads?
|
||||||
end
|
end
|
||||||
@ -118,41 +185,5 @@ class UploadSecurity
|
|||||||
@upload.secure?
|
@upload.secure?
|
||||||
end
|
end
|
||||||
|
|
||||||
private
|
#### END PRIVATE CHECKS ####
|
||||||
|
|
||||||
def access_control_post
|
|
||||||
@access_control_post ||=
|
|
||||||
@upload.access_control_post_id.present? ? @upload.access_control_post : nil
|
|
||||||
end
|
|
||||||
|
|
||||||
def insecure_context_checks
|
|
||||||
{
|
|
||||||
secure_uploads_disabled: "secure uploads is disabled",
|
|
||||||
insecure_creation_for_modifiers: "one or more creation for_modifiers was satisfied",
|
|
||||||
public_type: "upload is public type",
|
|
||||||
custom_emoji: "upload is used for custom emoji",
|
|
||||||
regular_emoji: "upload is used for regular emoji",
|
|
||||||
}
|
|
||||||
end
|
|
||||||
|
|
||||||
def secure_context_checks
|
|
||||||
{
|
|
||||||
login_required: "login is required",
|
|
||||||
access_control_post_has_secure_uploads: "access control post dictates security",
|
|
||||||
secure_creation_for_modifiers: "one or more creation for_modifiers was satisfied",
|
|
||||||
uploading_in_composer: "uploading via the composer",
|
|
||||||
already_secure: "upload is already secure",
|
|
||||||
}
|
|
||||||
end
|
|
||||||
|
|
||||||
# the access control check is important because that is the truest indicator
|
|
||||||
# of whether an upload should be secure or not, and thus should be returned
|
|
||||||
# immediately if there is an access control post
|
|
||||||
def priority_check?(check)
|
|
||||||
check == :access_control_post_has_secure_uploads && access_control_post
|
|
||||||
end
|
|
||||||
|
|
||||||
def perform_check(check)
|
|
||||||
send("#{check}_check")
|
|
||||||
end
|
|
||||||
end
|
end
|
||||||
|
124
spec/integration/secure_uploads_spec.rb
Normal file
124
spec/integration/secure_uploads_spec.rb
Normal file
@ -0,0 +1,124 @@
|
|||||||
|
# frozen_string_literal: true
|
||||||
|
|
||||||
|
describe "Secure uploads" do
|
||||||
|
fab!(:user) { Fabricate(:user) }
|
||||||
|
fab!(:group) { Fabricate(:group) }
|
||||||
|
fab!(:secure_category) { Fabricate(:private_category, group: group) }
|
||||||
|
|
||||||
|
before do
|
||||||
|
Jobs.run_immediately!
|
||||||
|
|
||||||
|
# this is done so the after_save callbacks for site settings to make
|
||||||
|
# UploadReference records works
|
||||||
|
@original_provider = SiteSetting.provider
|
||||||
|
SiteSetting.provider = SiteSettings::DbProvider.new(SiteSetting)
|
||||||
|
setup_s3
|
||||||
|
stub_s3_store
|
||||||
|
SiteSetting.secure_uploads = true
|
||||||
|
group.add(user)
|
||||||
|
user.reload
|
||||||
|
end
|
||||||
|
|
||||||
|
after { SiteSetting.provider = @original_provider }
|
||||||
|
|
||||||
|
def create_upload
|
||||||
|
filename = "logo.png"
|
||||||
|
file = file_from_fixtures(filename)
|
||||||
|
UploadCreator.new(file, filename).create_for(user.id)
|
||||||
|
end
|
||||||
|
|
||||||
|
def stub_presign_upload_get(upload)
|
||||||
|
# this is necessary because by default any upload inside a secure post is considered "secure"
|
||||||
|
# for the purposes of fetching hotlinked images until proven otherwise, and this is easier
|
||||||
|
# than trying to stub the presigned URL for s3 in a different way
|
||||||
|
stub_request(:get, "https:#{upload.url}").to_return(
|
||||||
|
status: 200,
|
||||||
|
body: file_from_fixtures("logo.png"),
|
||||||
|
)
|
||||||
|
Upload.stubs(:signed_url_from_secure_uploads_url).returns("https:#{upload.url}")
|
||||||
|
end
|
||||||
|
|
||||||
|
it "does not convert an upload to secure when it was first used in a site setting then in a post" do
|
||||||
|
upload = create_upload
|
||||||
|
SiteSetting.favicon = upload
|
||||||
|
expect(upload.reload.upload_references.count).to eq(1)
|
||||||
|
create_post(
|
||||||
|
title: "Secure upload post",
|
||||||
|
raw: "This is a new post <img src=\"#{upload.url}\" />",
|
||||||
|
category: secure_category,
|
||||||
|
user: user,
|
||||||
|
)
|
||||||
|
upload.reload
|
||||||
|
expect(upload.upload_references.count).to eq(2)
|
||||||
|
expect(upload.secure).to eq(false)
|
||||||
|
end
|
||||||
|
|
||||||
|
it "does not convert an upload to insecure when it was first used in a secure post then a site setting" do
|
||||||
|
upload = create_upload
|
||||||
|
create_post(
|
||||||
|
title: "Secure upload post",
|
||||||
|
raw: "This is a new post <img src=\"#{upload.url}\" />",
|
||||||
|
category: secure_category,
|
||||||
|
user: user,
|
||||||
|
)
|
||||||
|
expect(upload.reload.upload_references.count).to eq(1)
|
||||||
|
SiteSetting.favicon = upload
|
||||||
|
upload.reload
|
||||||
|
expect(upload.upload_references.count).to eq(2)
|
||||||
|
expect(upload.secure).to eq(true)
|
||||||
|
end
|
||||||
|
|
||||||
|
it "does not convert an upload to secure when it was first used in a public post then in a secure post" do
|
||||||
|
upload = create_upload
|
||||||
|
|
||||||
|
post =
|
||||||
|
create_post(
|
||||||
|
title: "Public upload post",
|
||||||
|
raw: "This is a new post <img src=\"#{upload.url}\" />",
|
||||||
|
user: user,
|
||||||
|
)
|
||||||
|
upload.reload
|
||||||
|
expect(upload.upload_references.count).to eq(1)
|
||||||
|
expect(upload.secure).to eq(false)
|
||||||
|
expect(upload.access_control_post).to eq(post)
|
||||||
|
|
||||||
|
stub_presign_upload_get(upload)
|
||||||
|
create_post(
|
||||||
|
title: "Secure upload post",
|
||||||
|
raw: "This is a new post <img src=\"#{upload.url}\" />",
|
||||||
|
category: secure_category,
|
||||||
|
user: user,
|
||||||
|
)
|
||||||
|
upload.reload
|
||||||
|
expect(upload.upload_references.count).to eq(2)
|
||||||
|
expect(upload.secure).to eq(false)
|
||||||
|
expect(upload.access_control_post).to eq(post)
|
||||||
|
end
|
||||||
|
|
||||||
|
it "does not convert an upload to insecure when it was first used in a secure post then in a public post" do
|
||||||
|
upload = create_upload
|
||||||
|
|
||||||
|
stub_presign_upload_get(upload)
|
||||||
|
post =
|
||||||
|
create_post(
|
||||||
|
title: "Secure upload post",
|
||||||
|
raw: "This is a new post <img src=\"#{upload.url}\" />",
|
||||||
|
category: secure_category,
|
||||||
|
user: user,
|
||||||
|
)
|
||||||
|
upload.reload
|
||||||
|
expect(upload.upload_references.count).to eq(1)
|
||||||
|
expect(upload.secure).to eq(true)
|
||||||
|
expect(upload.access_control_post).to eq(post)
|
||||||
|
|
||||||
|
create_post(
|
||||||
|
title: "Public upload post",
|
||||||
|
raw: "This is a new post <img src=\"#{upload.url}\" />",
|
||||||
|
user: user,
|
||||||
|
)
|
||||||
|
upload.reload
|
||||||
|
expect(upload.upload_references.count).to eq(2)
|
||||||
|
expect(upload.secure).to eq(true)
|
||||||
|
expect(upload.access_control_post).to eq(post)
|
||||||
|
end
|
||||||
|
end
|
@ -1,13 +1,14 @@
|
|||||||
# frozen_string_literal: true
|
# frozen_string_literal: true
|
||||||
|
|
||||||
RSpec.describe UploadSecurity do
|
RSpec.describe UploadSecurity do
|
||||||
let(:private_category) { Fabricate(:private_category, group: Fabricate(:group)) }
|
fab!(:private_category) { Fabricate(:private_category, group: Fabricate(:group)) }
|
||||||
let(:post_in_secure_context) do
|
fab!(:post_in_secure_context) do
|
||||||
Fabricate(:post, topic: Fabricate(:topic, category: private_category))
|
Fabricate(:post, topic: Fabricate(:topic, category: private_category))
|
||||||
end
|
end
|
||||||
fab!(:upload) { Fabricate(:upload) }
|
fab!(:upload) { Fabricate(:upload) }
|
||||||
let(:type) { nil }
|
let(:type) { nil }
|
||||||
let(:opts) { { type: type, creating: true } }
|
let(:opts) { { type: type, creating: creating } }
|
||||||
|
|
||||||
subject { described_class.new(upload, opts) }
|
subject { described_class.new(upload, opts) }
|
||||||
|
|
||||||
context "when secure uploads is enabled" do
|
context "when secure uploads is enabled" do
|
||||||
@ -16,8 +17,12 @@ RSpec.describe UploadSecurity do
|
|||||||
SiteSetting.secure_uploads = true
|
SiteSetting.secure_uploads = true
|
||||||
end
|
end
|
||||||
|
|
||||||
|
context "when creating the upload" do
|
||||||
|
let(:creating) { true }
|
||||||
|
|
||||||
context "when login_required (everything should be secure except public context items)" do
|
context "when login_required (everything should be secure except public context items)" do
|
||||||
before { SiteSetting.login_required = true }
|
before { SiteSetting.login_required = true }
|
||||||
|
|
||||||
it "returns true" do
|
it "returns true" do
|
||||||
expect(subject.should_be_secure?).to eq(true)
|
expect(subject.should_be_secure?).to eq(true)
|
||||||
end
|
end
|
||||||
@ -29,48 +34,56 @@ RSpec.describe UploadSecurity do
|
|||||||
expect(subject.should_be_secure?).to eq(false)
|
expect(subject.should_be_secure?).to eq(false)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
describe "for a public type group_flair" do
|
describe "for a public type group_flair" do
|
||||||
let(:type) { "group_flair" }
|
let(:type) { "group_flair" }
|
||||||
it "returns false" do
|
it "returns false" do
|
||||||
expect(subject.should_be_secure?).to eq(false)
|
expect(subject.should_be_secure?).to eq(false)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
describe "for a public type avatar" do
|
describe "for a public type avatar" do
|
||||||
let(:type) { "avatar" }
|
let(:type) { "avatar" }
|
||||||
it "returns false" do
|
it "returns false" do
|
||||||
expect(subject.should_be_secure?).to eq(false)
|
expect(subject.should_be_secure?).to eq(false)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
describe "for a public type custom_emoji" do
|
describe "for a public type custom_emoji" do
|
||||||
let(:type) { "custom_emoji" }
|
let(:type) { "custom_emoji" }
|
||||||
it "returns false" do
|
it "returns false" do
|
||||||
expect(subject.should_be_secure?).to eq(false)
|
expect(subject.should_be_secure?).to eq(false)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
describe "for a public type profile_background" do
|
describe "for a public type profile_background" do
|
||||||
let(:type) { "profile_background" }
|
let(:type) { "profile_background" }
|
||||||
it "returns false" do
|
it "returns false" do
|
||||||
expect(subject.should_be_secure?).to eq(false)
|
expect(subject.should_be_secure?).to eq(false)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
describe "for a public type avatar" do
|
describe "for a public type avatar" do
|
||||||
let(:type) { "avatar" }
|
let(:type) { "avatar" }
|
||||||
it "returns false" do
|
it "returns false" do
|
||||||
expect(subject.should_be_secure?).to eq(false)
|
expect(subject.should_be_secure?).to eq(false)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
describe "for a public type category_logo" do
|
describe "for a public type category_logo" do
|
||||||
let(:type) { "category_logo" }
|
let(:type) { "category_logo" }
|
||||||
it "returns false" do
|
it "returns false" do
|
||||||
expect(subject.should_be_secure?).to eq(false)
|
expect(subject.should_be_secure?).to eq(false)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
describe "for a public type category_background" do
|
describe "for a public type category_background" do
|
||||||
let(:type) { "category_background" }
|
let(:type) { "category_background" }
|
||||||
it "returns false" do
|
it "returns false" do
|
||||||
expect(subject.should_be_secure?).to eq(false)
|
expect(subject.should_be_secure?).to eq(false)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
describe "for a custom public type" do
|
describe "for a custom public type" do
|
||||||
let(:type) { "my_custom_type" }
|
let(:type) { "my_custom_type" }
|
||||||
|
|
||||||
@ -84,28 +97,27 @@ RSpec.describe UploadSecurity do
|
|||||||
UploadSecurity.reset_custom_public_types
|
UploadSecurity.reset_custom_public_types
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
describe "for_theme" do
|
describe "for_theme" do
|
||||||
before { upload.stubs(:for_theme).returns(true) }
|
before { upload.stubs(:for_theme).returns(true) }
|
||||||
it "returns false" do
|
|
||||||
expect(subject.should_be_secure?).to eq(false)
|
|
||||||
end
|
|
||||||
end
|
|
||||||
describe "for_site_setting" do
|
|
||||||
before { upload.stubs(:for_site_setting).returns(true) }
|
|
||||||
it "returns false" do
|
|
||||||
expect(subject.should_be_secure?).to eq(false)
|
|
||||||
end
|
|
||||||
end
|
|
||||||
describe "for_gravatar" do
|
|
||||||
before { upload.stubs(:for_gravatar).returns(true) }
|
|
||||||
it "returns false" do
|
it "returns false" do
|
||||||
expect(subject.should_be_secure?).to eq(false)
|
expect(subject.should_be_secure?).to eq(false)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
describe "when the upload is used for a custom emoji" do
|
describe "for_site_setting" do
|
||||||
|
before { upload.stubs(:for_site_setting).returns(true) }
|
||||||
|
|
||||||
|
it "returns false" do
|
||||||
|
expect(subject.should_be_secure?).to eq(false)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
describe "for_gravatar" do
|
||||||
|
before { upload.stubs(:for_gravatar).returns(true) }
|
||||||
|
|
||||||
it "returns false" do
|
it "returns false" do
|
||||||
CustomEmoji.create(name: "meme", upload: upload)
|
|
||||||
expect(subject.should_be_secure?).to eq(false)
|
expect(subject.should_be_secure?).to eq(false)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
@ -125,12 +137,14 @@ RSpec.describe UploadSecurity do
|
|||||||
|
|
||||||
context "when the access control post has_secure_uploads?" do
|
context "when the access control post has_secure_uploads?" do
|
||||||
before { upload.update(access_control_post_id: post_in_secure_context.id) }
|
before { upload.update(access_control_post_id: post_in_secure_context.id) }
|
||||||
|
|
||||||
it "returns true" do
|
it "returns true" do
|
||||||
expect(subject.should_be_secure?).to eq(true)
|
expect(subject.should_be_secure?).to eq(true)
|
||||||
end
|
end
|
||||||
|
|
||||||
context "when the post is deleted" do
|
context "when the post is deleted" do
|
||||||
before { post_in_secure_context.trash! }
|
before { post_in_secure_context.trash! }
|
||||||
|
|
||||||
it "still determines whether the post has secure uploads; returns true" do
|
it "still determines whether the post has secure uploads; returns true" do
|
||||||
expect(subject.should_be_secure?).to eq(true)
|
expect(subject.should_be_secure?).to eq(true)
|
||||||
end
|
end
|
||||||
@ -143,18 +157,21 @@ RSpec.describe UploadSecurity do
|
|||||||
expect(subject.should_be_secure?).to eq(true)
|
expect(subject.should_be_secure?).to eq(true)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
context "when uploading for a group message" do
|
context "when uploading for a group message" do
|
||||||
before { upload.stubs(:for_group_message).returns(true) }
|
before { upload.stubs(:for_group_message).returns(true) }
|
||||||
it "returns true" do
|
it "returns true" do
|
||||||
expect(subject.should_be_secure?).to eq(true)
|
expect(subject.should_be_secure?).to eq(true)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
context "when uploading for a PM" do
|
context "when uploading for a PM" do
|
||||||
before { upload.stubs(:for_private_message).returns(true) }
|
before { upload.stubs(:for_private_message).returns(true) }
|
||||||
it "returns true" do
|
it "returns true" do
|
||||||
expect(subject.should_be_secure?).to eq(true)
|
expect(subject.should_be_secure?).to eq(true)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
context "when upload is already secure" do
|
context "when upload is already secure" do
|
||||||
before { upload.update(secure: true) }
|
before { upload.update(secure: true) }
|
||||||
it "returns true" do
|
it "returns true" do
|
||||||
@ -174,14 +191,119 @@ RSpec.describe UploadSecurity do
|
|||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
context "when checking an existing upload" do
|
||||||
|
let(:creating) { false }
|
||||||
|
|
||||||
|
before do
|
||||||
|
# this is done so the after_save callbacks for site settings to make
|
||||||
|
# UploadReference records works
|
||||||
|
@original_provider = SiteSetting.provider
|
||||||
|
SiteSetting.provider = SiteSettings::DbProvider.new(SiteSetting)
|
||||||
|
setup_s3
|
||||||
|
SiteSetting.secure_uploads = true
|
||||||
|
end
|
||||||
|
|
||||||
|
after { SiteSetting.provider = @original_provider }
|
||||||
|
|
||||||
|
def create_secure_post_reference
|
||||||
|
UploadReference.ensure_exist!(upload_ids: [upload.id], target: post_in_secure_context)
|
||||||
|
upload.update!(access_control_post: post_in_secure_context)
|
||||||
|
end
|
||||||
|
|
||||||
|
describe "when the upload is first used for a post in a secure context" do
|
||||||
|
it "returns true" do
|
||||||
|
create_secure_post_reference
|
||||||
|
expect(subject.should_be_secure?).to eq(true)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
describe "when the upload is first used for a site setting" do
|
||||||
|
it "returns false" do
|
||||||
|
SiteSetting.favicon = upload
|
||||||
|
create_secure_post_reference
|
||||||
|
expect(subject.should_be_secure?).to eq(false)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
describe "when the upload is first used for a theme field" do
|
||||||
|
it "returns false" do
|
||||||
|
Fabricate(:theme_field, type_id: ThemeField.types[:theme_upload_var], upload: upload)
|
||||||
|
create_secure_post_reference
|
||||||
|
expect(subject.should_be_secure?).to eq(false)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
describe "when the upload is first used for a group flair image" do
|
||||||
|
it "returns false" do
|
||||||
|
Fabricate(:group, flair_upload: upload)
|
||||||
|
create_secure_post_reference
|
||||||
|
expect(subject.should_be_secure?).to eq(false)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
describe "when the upload is first used for a custom emoji" do
|
||||||
|
it "returns false" do
|
||||||
|
CustomEmoji.create(name: "meme", upload: upload)
|
||||||
|
create_secure_post_reference
|
||||||
|
expect(subject.should_be_secure?).to eq(false)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
describe "when the upload is first used for a badge" do
|
||||||
|
it "returns false" do
|
||||||
|
Fabricate(:badge, image_upload: upload)
|
||||||
|
create_secure_post_reference
|
||||||
|
expect(subject.should_be_secure?).to eq(false)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
describe "when the upload is first used for a category image (logo, dark logo, background)" do
|
||||||
|
it "returns false" do
|
||||||
|
Fabricate(:category, uploaded_logo: upload)
|
||||||
|
create_secure_post_reference
|
||||||
|
expect(subject.should_be_secure?).to eq(false)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
describe "when the upload is first used for a user profile (profile background, card background)" do
|
||||||
|
it "returns false" do
|
||||||
|
user = Fabricate(:user)
|
||||||
|
user.user_profile.update!(card_background_upload: upload)
|
||||||
|
create_secure_post_reference
|
||||||
|
expect(subject.should_be_secure?).to eq(false)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
describe "when the upload is first used for a user uploaded avatar" do
|
||||||
|
it "returns false" do
|
||||||
|
Fabricate(:user, uploaded_avatar: upload)
|
||||||
|
create_secure_post_reference
|
||||||
|
expect(subject.should_be_secure?).to eq(false)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
describe "when the upload is first used for a UserAvatar" do
|
||||||
|
it "returns false" do
|
||||||
|
Fabricate(:user_avatar, custom_upload: upload)
|
||||||
|
create_secure_post_reference
|
||||||
|
expect(subject.should_be_secure?).to eq(false)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
context "when secure uploads is disabled" do
|
context "when secure uploads is disabled" do
|
||||||
|
let(:creating) { true }
|
||||||
|
|
||||||
before { SiteSetting.secure_uploads = false }
|
before { SiteSetting.secure_uploads = false }
|
||||||
|
|
||||||
it "returns false" do
|
it "returns false" do
|
||||||
expect(subject.should_be_secure?).to eq(false)
|
expect(subject.should_be_secure?).to eq(false)
|
||||||
end
|
end
|
||||||
|
|
||||||
context "for attachments" do
|
context "for attachments" do
|
||||||
before { upload.update(original_filename: "test.pdf") }
|
before { upload.update(original_filename: "test.pdf") }
|
||||||
|
|
||||||
it "returns false" do
|
it "returns false" do
|
||||||
expect(subject.should_be_secure?).to eq(false)
|
expect(subject.should_be_secure?).to eq(false)
|
||||||
end
|
end
|
||||||
|
Loading…
Reference in New Issue
Block a user