Avoid a brakeman security warning

Although it wasn't a real security concern because we were only calling
a `find` 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` method anymore but we still need to check
the associated record exists, we're changing the `followable` validation
in the `Follow` model to do exactly that.
This commit is contained in:
Javi Martín
2020-10-28 01:39:21 +01:00
parent 3215060e94
commit 8a47fe3505
3 changed files with 16 additions and 5 deletions

View File

@@ -3,7 +3,7 @@ class FollowsController < ApplicationController
load_and_authorize_resource
def create
@follow = Follow.create!(user: current_user, followable: find_followable)
@follow = current_user.follows.create!(follow_params)
flash.now[:notice] = t("shared.followable.#{followable_translation_key(@follow.followable)}.create.notice")
render :refresh_follow_button
end
@@ -17,8 +17,8 @@ class FollowsController < ApplicationController
private
def find_followable
params[:followable_type].constantize.find(params[:followable_id])
def follow_params
params.permit(:followable_type, :followable_id)
end
def followable_translation_key(followable)

View File

@@ -3,6 +3,5 @@ class Follow < ApplicationRecord
belongs_to :followable, polymorphic: true
validates :user_id, presence: true
validates :followable_id, presence: true
validates :followable_type, presence: true
validates :followable, presence: true
end

View File

@@ -21,4 +21,16 @@ describe Follow do
follow.followable_type = nil
expect(follow).not_to be_valid
end
it "is not valid with an invalid followable_type" do
follow.followable_type = "NotARealModel"
expect { follow.valid? }.to raise_exception "uninitialized constant NotARealModel"
end
it "is not valid with the ID of a non-existent record" do
follow.followable_id = Proposal.last.id + 1
expect(follow).not_to be_valid
end
end