Check for valid segments before returning recipients
We were getting a warning by CodeQL regarding a possible code injection in the `send(segment)` code. In practice, this wasn't a big deal because the `self.recipients` method is only called in the admin section, meaning only admin users could try to take advantage of the code injection, and because this code is rarely called with an invalid segment due to several conditions in the code checking that the user segment is valid, with the only exception being the `generate_csv` action in the `Admin::EmailsDownloadController`. In any case, now we're checking that the segment is valid before calling the `send` method. Since now we're making sure that the segment is valid in the `recipients` method itself, we can remove this check from methods calling it.
This commit is contained in:
@@ -64,7 +64,7 @@ class UserSegments
|
|||||||
def self.recipients(segment)
|
def self.recipients(segment)
|
||||||
if geozones[segment.to_s]
|
if geozones[segment.to_s]
|
||||||
all_users.where(geozone: geozones[segment.to_s])
|
all_users.where(geozone: geozones[segment.to_s])
|
||||||
else
|
elsif valid_segment?(segment)
|
||||||
send(segment)
|
send(segment)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|||||||
@@ -13,7 +13,7 @@ class AdminNotification < ApplicationRecord
|
|||||||
before_validation :complete_link_url
|
before_validation :complete_link_url
|
||||||
|
|
||||||
def list_of_recipients
|
def list_of_recipients
|
||||||
UserSegments.recipients(segment_recipient) if valid_segment_recipient?
|
UserSegments.recipients(segment_recipient)
|
||||||
end
|
end
|
||||||
|
|
||||||
def valid_segment_recipient?
|
def valid_segment_recipient?
|
||||||
|
|||||||
@@ -301,6 +301,24 @@ describe UserSegments do
|
|||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
describe ".recipients" do
|
||||||
|
it "does not return any recipients when given an invalid segment" do
|
||||||
|
create(:user, email: "first@email.com")
|
||||||
|
|
||||||
|
recipients = UserSegments.recipients(:invalid_segment)
|
||||||
|
|
||||||
|
expect(recipients).to be nil
|
||||||
|
end
|
||||||
|
|
||||||
|
it "does not return any recipients when given a method name as a segment" do
|
||||||
|
create(:user, email: "first@email.com")
|
||||||
|
|
||||||
|
recipients = UserSegments.recipients(:ancestors)
|
||||||
|
|
||||||
|
expect(recipients).to be nil
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
describe ".user_segment_emails" do
|
describe ".user_segment_emails" do
|
||||||
it "returns list of emails sorted by user creation date" do
|
it "returns list of emails sorted by user creation date" do
|
||||||
create(:user, email: "first@email.com", created_at: 1.day.ago)
|
create(:user, email: "first@email.com", created_at: 1.day.ago)
|
||||||
|
|||||||
Reference in New Issue
Block a user