From d48587721a5307b5bc124d42c4ba83b4e21014ae Mon Sep 17 00:00:00 2001 From: Daniel Uber Date: Fri, 5 Nov 2021 13:25:35 -0400 Subject: [PATCH] Only permit valid class names as sponsorable type (#14387) * Only permit valid class names as sponsorable type https://github.com/forem/forem/issues/14386 identified an error could arise if input was provide to the sponsorable_type field in the admin creation form, but it was not a valid constant (since we include the related model in the index when loading @sponsorships, this permits creating a sponsorship that can't easily be managed or deleted). Add a validation to ensure when the sponsorable type is present, it's a class (really, we probably want to also ensure it's available as an associated model type, since we'll be looking it up with find(:sponsorable_id), but this is an initial attempt at adding some guard rails to this form). A more realistic solution would be to add a `Sponsorship::SPONSORABLE_TYPES` constant to the class, and validate the supplied sponsorable type is in `SPONSORABLE_TYPES.map(&:name)`. That requires more domain knowledge about what kind of things can be sponsored than I have, it would certainly include at least ActsAsTaggableOn::Tag but may include other classes (or why would it be polymorphic). * Explicitly list the allowed types for sponsorship I see we can sponsor tags, and suspect that's all we can sponsor meaningfully (the view only shows details when it is a tag). * Use inclusion validation with options to replicate custom method And remove the code, we don't need it any more. * Update app/models/sponsorship.rb Co-authored-by: Michael Kohl Co-authored-by: Michael Kohl --- app/models/sponsorship.rb | 6 ++++++ spec/models/sponsorship_spec.rb | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/app/models/sponsorship.rb b/app/models/sponsorship.rb index c8c5e92a5..337f47e9e 100644 --- a/app/models/sponsorship.rb +++ b/app/models/sponsorship.rb @@ -2,6 +2,7 @@ class Sponsorship < ApplicationRecord LEVELS = %w[gold silver bronze tag media devrel].freeze METAL_LEVELS = %w[gold silver bronze].freeze STATUSES = %w[none pending live].freeze + SPONSORABLE_TYPES = %w[Tag ActsAsTaggableOn::Tag].freeze # media has no fixed amount of credits CREDITS = { gold: 6_000, @@ -19,6 +20,11 @@ class Sponsorship < ApplicationRecord validates :status, presence: true, inclusion: { in: STATUSES } validates :url, url: { allow_blank: true, no_local: true, schemes: %w[http https] } validates :user, :organization, :featured_number, presence: true + validates :sponsorable_type, inclusion: { + in: SPONSORABLE_TYPES, + allow_blank: true, + message: "is not a sponsorable type" + } validate :validate_tag_uniqueness, if: proc { level.to_s == "tag" } validate :validate_level_uniqueness, if: proc { METAL_LEVELS.include?(level) } diff --git a/spec/models/sponsorship_spec.rb b/spec/models/sponsorship_spec.rb index d72c20f7f..71eae0a39 100644 --- a/spec/models/sponsorship_spec.rb +++ b/spec/models/sponsorship_spec.rb @@ -111,6 +111,12 @@ RSpec.describe Sponsorship, type: :model do expect(tag_sponsorship).not_to be_valid expect(tag_sponsorship.errors[:level]).to be_present end + + it "ensures sponsorable_type is loadable when present" do + sponsorship = build(:sponsorship, user: user, organization: org, sponsorable_type: "Unsponsorable") + expect(sponsorship).not_to be_valid + expect(sponsorship.errors[:sponsorable_type]).to include("is not a sponsorable type") + end end end end