FIX: Do not allow setting admin and staff for TrustLevelSetting (#25107)

This fixes an issue where any string for an enum site setting
(such as TrustLevelSetting) would be converted to an integer
if the default value for the enum was an integer. This is an
issue because things like "admin" and "staff" would get silently
converted to 0 which is "valid" because it's TrustLevel[0],
but it's unexpected behaviour. It's best to just let the site
setting validator catch this broken value.
This commit is contained in:
Martin Brennan 2024-01-03 16:55:28 +10:00 committed by GitHub
parent 4e6d4193ea
commit e8deed874b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 37 additions and 1 deletions

View File

@ -231,7 +231,14 @@ class SiteSettings::TypeSupervisor
elsif type == self.class.types[:null] && val != "" elsif type == self.class.types[:null] && val != ""
type = get_data_type(name, val) type = get_data_type(name, val)
elsif type == self.class.types[:enum] elsif type == self.class.types[:enum]
val = @defaults_provider[name].is_a?(Integer) ? val.to_i : val.to_s val =
(
if @defaults_provider[name].is_a?(Integer) && Integer(val, exception: false)
val.to_i
else
val.to_s
end
)
elsif type == self.class.types[:uploaded_image_list] && val.present? elsif type == self.class.types[:uploaded_image_list] && val.present?
val = val.is_a?(String) ? val : val.map(&:id).join("|") val = val.is_a?(String) ? val : val.map(&:id).join("|")
elsif type == self.class.types[:upload] && val.present? elsif type == self.class.types[:upload] && val.present?

View File

@ -14,4 +14,33 @@ RSpec.describe TrustLevelSetting do
expect(value[:value]).to eq(0) expect(value[:value]).to eq(0)
end end
end end
describe ".valid_value?" do
let(:deprecated_test) { "#{Rails.root}/spec/fixtures/site_settings/deprecated_test.yml" }
before { SiteSetting.load_settings(deprecated_test) }
it "allows all trust levels as valid values" do
expect(TrustLevelSetting.valid_value?(TrustLevel[0])).to eq(true)
expect(TrustLevelSetting.valid_value?(TrustLevel[1])).to eq(true)
expect(TrustLevelSetting.valid_value?(TrustLevel[2])).to eq(true)
expect(TrustLevelSetting.valid_value?(TrustLevel[3])).to eq(true)
expect(TrustLevelSetting.valid_value?(TrustLevel[4])).to eq(true)
expect(TrustLevelSetting.valid_value?(20)).to eq(false)
end
it "does not allow 'admin' or 'staff' as valid values" do
expect(TrustLevelSetting.valid_value?("admin")).to eq(false)
expect(TrustLevelSetting.valid_value?("staff")).to eq(false)
end
it "does not allow setting 'admin' or 'staff' as valid values" do
expect { SiteSetting.min_trust_level_to_allow_invite = "admin" }.to raise_error(
Discourse::InvalidParameters,
)
expect { SiteSetting.min_trust_level_to_allow_invite = "staff" }.to raise_error(
Discourse::InvalidParameters,
)
end
end
end end