Avoid a brakeman warning in related contents
Although it wasn't a real security concern because we were only calling a `find_by` method based on the user input, it's a good practice to avoid using constants based on user parameters. Since we don't use the `find_by` method anymore but we still need to check the associated record exists, we're changing the validations in the `RelatedContent` model to do exactly that.
This commit is contained in:
@@ -4,10 +4,10 @@ class RelatedContentsController < ApplicationController
|
|||||||
respond_to :html, :js
|
respond_to :html, :js
|
||||||
|
|
||||||
def create
|
def create
|
||||||
related_content = RelatedContent.new(
|
related_content = current_user.related_contents.new(
|
||||||
parent_relationable: relationable_object,
|
parent_relationable_id: params[:relationable_id],
|
||||||
child_relationable: related_object,
|
parent_relationable_type: params[:relationable_klass],
|
||||||
author: current_user
|
child_relationable: related_object
|
||||||
)
|
)
|
||||||
|
|
||||||
if related_content.save
|
if related_content.save
|
||||||
@@ -17,7 +17,7 @@ class RelatedContentsController < ApplicationController
|
|||||||
else
|
else
|
||||||
flash[:error] = t("related_content.error", url: Setting["url"])
|
flash[:error] = t("related_content.error", url: Setting["url"])
|
||||||
end
|
end
|
||||||
redirect_to polymorphic_path(relationable_object)
|
redirect_to polymorphic_path(related_content.parent_relationable)
|
||||||
end
|
end
|
||||||
|
|
||||||
def score_positive
|
def score_positive
|
||||||
@@ -41,10 +41,6 @@ class RelatedContentsController < ApplicationController
|
|||||||
params[:url].start_with?(Setting["url"])
|
params[:url].start_with?(Setting["url"])
|
||||||
end
|
end
|
||||||
|
|
||||||
def relationable_object
|
|
||||||
@relationable ||= params[:relationable_klass].singularize.camelize.constantize.find_by(id: params[:relationable_id])
|
|
||||||
end
|
|
||||||
|
|
||||||
def related_object
|
def related_object
|
||||||
if valid_url?
|
if valid_url?
|
||||||
url = params[:url]
|
url = params[:url]
|
||||||
|
|||||||
@@ -6,15 +6,11 @@ class RelatedContent < ApplicationRecord
|
|||||||
include ActsAsParanoidAliases
|
include ActsAsParanoidAliases
|
||||||
|
|
||||||
belongs_to :author, class_name: "User"
|
belongs_to :author, class_name: "User"
|
||||||
belongs_to :parent_relationable, polymorphic: true, touch: true
|
belongs_to :parent_relationable, polymorphic: true, optional: false, touch: true
|
||||||
belongs_to :child_relationable, polymorphic: true, touch: true
|
belongs_to :child_relationable, polymorphic: true, optional: false, touch: true
|
||||||
has_one :opposite_related_content, class_name: self.name, foreign_key: :related_content_id
|
has_one :opposite_related_content, class_name: self.name, foreign_key: :related_content_id
|
||||||
has_many :related_content_scores
|
has_many :related_content_scores
|
||||||
|
|
||||||
validates :parent_relationable_id, presence: true
|
|
||||||
validates :parent_relationable_type, presence: true
|
|
||||||
validates :child_relationable_id, presence: true
|
|
||||||
validates :child_relationable_type, presence: true
|
|
||||||
validates :parent_relationable_id, uniqueness: { scope: [:parent_relationable_type, :child_relationable_id, :child_relationable_type] }
|
validates :parent_relationable_id, uniqueness: { scope: [:parent_relationable_type, :child_relationable_id, :child_relationable_type] }
|
||||||
validate :different_parent_and_child
|
validate :different_parent_and_child
|
||||||
|
|
||||||
|
|||||||
@@ -74,6 +74,7 @@ class User < ApplicationRecord
|
|||||||
class_name: "Poll::Recount",
|
class_name: "Poll::Recount",
|
||||||
foreign_key: :author_id,
|
foreign_key: :author_id,
|
||||||
inverse_of: :author
|
inverse_of: :author
|
||||||
|
has_many :related_contents, foreign_key: :author_id, inverse_of: :author, dependent: nil
|
||||||
has_many :topics, foreign_key: :author_id, inverse_of: :author
|
has_many :topics, foreign_key: :author_id, inverse_of: :author
|
||||||
belongs_to :geozone
|
belongs_to :geozone
|
||||||
|
|
||||||
|
|||||||
@@ -3,9 +3,10 @@ require "rails_helper"
|
|||||||
describe RelatedContent do
|
describe RelatedContent do
|
||||||
let(:parent_relationable) { create([:proposal, :debate].sample) }
|
let(:parent_relationable) { create([:proposal, :debate].sample) }
|
||||||
let(:child_relationable) { create([:proposal, :debate].sample) }
|
let(:child_relationable) { create([:proposal, :debate].sample) }
|
||||||
|
let(:related_content) { build(:related_content) }
|
||||||
|
|
||||||
it "is valid" do
|
it "is valid" do
|
||||||
expect(build(:related_content)).to be_valid
|
expect(related_content).to be_valid
|
||||||
end
|
end
|
||||||
|
|
||||||
it "allows relationables from various classes" do
|
it "allows relationables from various classes" do
|
||||||
@@ -33,6 +34,28 @@ describe RelatedContent do
|
|||||||
expect(related_content).not_to be_valid
|
expect(related_content).not_to be_valid
|
||||||
end
|
end
|
||||||
|
|
||||||
|
it "is not valid with an invalid parent relationable type" do
|
||||||
|
related_content.parent_relationable_type = "NotARealModel"
|
||||||
|
|
||||||
|
expect { related_content.valid? }.to raise_exception "uninitialized constant NotARealModel"
|
||||||
|
end
|
||||||
|
|
||||||
|
it "is not valid with parent relationable ID of a non-existent record" do
|
||||||
|
related_content.parent_relationable_id = related_content.parent_relationable.class.last.id + 1
|
||||||
|
|
||||||
|
expect(related_content).not_to be_valid
|
||||||
|
end
|
||||||
|
|
||||||
|
it "is not valid with an invalid child relationable type" do
|
||||||
|
related_content.child_relationable_type = "NotARealModel"
|
||||||
|
|
||||||
|
expect { related_content.valid? }.to raise_exception "uninitialized constant NotARealModel"
|
||||||
|
end
|
||||||
|
|
||||||
|
it "is not valid with child relationable ID of a non-existent record" do
|
||||||
|
related_content.child_relationable_id = related_content.child_relationable.class.last.id + 1
|
||||||
|
end
|
||||||
|
|
||||||
describe "create_opposite_related_content" do
|
describe "create_opposite_related_content" do
|
||||||
let(:parent_relationable) { create(:proposal) }
|
let(:parent_relationable) { create(:proposal) }
|
||||||
let(:child_relationable) { create(:debate) }
|
let(:child_relationable) { create(:debate) }
|
||||||
|
|||||||
Reference in New Issue
Block a user