FIX: prevents custom fields to override raw_invitees

This commit also refactors specs to better test this behavior and enforces the validation of raw invitees.
This commit is contained in:
jjaffeux 2020-08-14 10:17:17 +02:00
parent 6fd4397f44
commit 04abec77ca
6 changed files with 146 additions and 323 deletions

View File

@ -50,27 +50,21 @@ module DiscoursePostEvent
render json: success_json
end
# update is only used for custom fields
# everything else is managed by cooking the post
def update
DistributedMutex.synchronize("discourse-post-event[event-update]") do
event_params[:custom_fields] = (event_params[:custom_fields] || {}).reject { |_, value| value.blank? }
event = Event.find(params[:id])
guardian.ensure_can_edit!(event.post)
guardian.ensure_can_act_on_discourse_post_event!(event)
event.update_with_params!(event_params)
event.update!(event_params)
serializer = EventSerializer.new(event, scope: guardian)
render_json_dump(serializer)
end
end
def create
event = Event.new(id: event_params[:id])
guardian.ensure_can_edit!(event.post)
guardian.ensure_can_create_discourse_post_event!
event.update_with_params!(event_params)
event.publish_update!
serializer = EventSerializer.new(event, scope: guardian)
render_json_dump(serializer)
end
def csv_bulk_invite
require 'csv'
@ -136,17 +130,7 @@ module DiscoursePostEvent
params
.require(:event)
.permit(
:id,
:name,
:starts_at,
:ends_at,
:status,
:url,
:recurrence,
custom_fields: allowed_custom_fields,
raw_invitees: [],
)
.permit(custom_fields: allowed_custom_fields)
end
def ics_request?

View File

@ -101,6 +101,13 @@ module DiscoursePostEvent
end
end
validate :raw_invitees_are_groups
def raw_invitees_are_groups
if self.raw_invitees && User.select(:id).where(username: self.raw_invitees).limit(1).count > 0
errors.add(:base, I18n.t('discourse_post_event.errors.models.event.raw_invitees.only_group'))
end
end
validate :ends_before_start
def ends_before_start
if self.starts_at && self.ends_at && self.starts_at >= self.ends_at
@ -284,17 +291,14 @@ module DiscoursePostEvent
end
def update_with_params!(params)
params[:custom_fields] =
(params[:custom_fields] || {}).reject { |_, value| value.blank? }
case params[:status] ? params[:status].to_i : self.status
when Event.statuses[:private]
raw_invitees =
Set.new(
Array(self.raw_invitees) + Array(params[:raw_invitees]) -
[PUBLIC_GROUP]
).to_a
self.update!(params.merge(raw_invitees: raw_invitees))
if params.key?(:raw_invitees)
params = params.merge(raw_invitees: Array(params[:raw_invitees]) - [PUBLIC_GROUP])
else
params = params.merge(raw_invitees: Array(self.raw_invitees) - [PUBLIC_GROUP])
end
self.update!(params)
self.enforce_private_invitees!
when Event.statuses[:public]
self.update!(params.merge(raw_invitees: [PUBLIC_GROUP]))

View File

@ -49,6 +49,7 @@ en:
models:
event:
only_one_event: A post can only have one event.
only_group: An event accepts only group names.
must_be_in_first_post: An event can only be in the first post of a topic.
raw_invitees_length: "An event is limited to %{count} users/groups."
ends_at_before_starts_at: "An event can't end before it starts."

View File

@ -54,7 +54,7 @@ module DiscoursePostEvent
end
end
if extracted_event[:name].present? && extracted_event[:name]
if extracted_event[:name].present?
if !(Event::MIN_NAME_LENGTH..Event::MAX_NAME_LENGTH).cover?(extracted_event[:name].length)
@post.errors.add(:base, I18n.t('discourse_post_event.errors.models.event.name.length', minimum: Event::MIN_NAME_LENGTH, maximum: Event::MAX_NAME_LENGTH))
return false

View File

@ -19,7 +19,7 @@ describe Post do
context 'public event' do
let(:post_1) { Fabricate(:post) }
let(:event_1) { Fabricate(:event, post: post_1) }
let(:event_1) { Fabricate(:event, post: post_1, raw_invitees: ['trust_level_0']) }
context 'when a post is updated' do
context 'when the post has a valid event' do
@ -161,6 +161,33 @@ describe Post do
end
end
end
context 'updating raw_invitees' do
let(:lurker_1) { Fabricate(:user) }
let(:group_1) { Fabricate(:group) }
it 'doesnt accept usernames' do
event_1.update_with_params!(raw_invitees: [lurker_1.username])
expect(event_1.raw_invitees).to eq(['trust_level_0'])
end
it 'doesnt accept another group than trust_level_0' do
event_1.update_with_params!(raw_invitees: [group_1.name])
expect(event_1.raw_invitees).to eq(['trust_level_0'])
end
end
context 'updating status to private' do
it 'it changes the status and force invitees' do
expect(event_1.raw_invitees).to eq(['trust_level_0'])
expect(event_1.status).to eq(Event.statuses[:public])
event_1.update_with_params!(status: Event.statuses[:private])
expect(event_1.raw_invitees).to eq([])
expect(event_1.status).to eq(Event.statuses[:private])
end
end
end
end
end
@ -451,13 +478,24 @@ describe Post do
post: post_1,
status: Event.statuses[:private],
raw_invitees: [group_1.name],
recurrence: 'FREQ=WEEKLY;BYDAY=MO',
starts_at: 3.hours.ago,
ends_at: nil
)
}
context 'an event with recurrence' do
let(:event_1) {
Fabricate(
:event,
post: post_1,
status: Event.statuses[:private],
raw_invitees: [group_1.name],
recurrence: 'FREQ=WEEKLY;BYDAY=MO',
starts_at: 3.hours.ago,
ends_at: nil
)
}
before do
Invitee.create_attendance!(invitee_1.id, event_1.id, :going)
@ -466,15 +504,43 @@ describe Post do
Jobs.run_later!
end
context 'when the event ends' do
context 'updating the end' do
it 'resends event creation notification to invitees and possible invitees' do
expect(event_1.invitees.count).to eq(1)
expect { event_1.update_with_params!(ends_at: Time.now) }.to change {
expect { event_1.update_with_params!(ends_at: 2.hours.ago) }.to change {
invitee_1.notifications.count + invitee_2.notifications.count
}.by(2)
end
end
end
context 'updating raw_invitees' do
let(:lurker_1) { Fabricate(:user) }
let(:group_2) { Fabricate(:group) }
it 'doesnt accept usernames' do
expect {
event_1.update_with_params!(raw_invitees: [lurker_1.username])
}.to raise_error(ActiveRecord::RecordInvalid)
end
it 'accepts another group than trust_level_0' do
event_1.update_with_params!(raw_invitees: [group_2.name])
expect(event_1.raw_invitees).to eq([group_2.name])
end
end
context 'updating status to public' do
it 'it changes the status and force invitees' do
expect(event_1.raw_invitees).to eq([group_1.name])
expect(event_1.status).to eq(Event.statuses[:private])
event_1.update_with_params!(status: Event.statuses[:public])
expect(event_1.raw_invitees).to eq(['trust_level_0'])
expect(event_1.status).to eq(Event.statuses[:public])
end
end
end
end

View File

@ -18,281 +18,49 @@ module DiscoursePostEvent
let(:invitee1) { Fabricate(:user) }
let(:invitee2) { Fabricate(:user) }
context 'when a post exists' do
let(:invitee3) { Fabricate(:user) }
let(:invitee4) { Fabricate(:user) }
let(:invitee5) { Fabricate(:user) }
let(:group) {
Fabricate(:group).tap do |g|
g.add(invitee2)
g.add(invitee3)
g.save!
end
}
let(:large_group) {
Fabricate(:group).tap do |g|
g.add(invitee2)
g.add(invitee3)
g.add(invitee4)
g.add(invitee5)
g.save!
end
}
context 'Existing post' do
context 'Existing event' do
let(:event_1) { Fabricate(:event, post: post1) }
before do
sign_in(user)
end
it 'creates an event' do
post '/discourse-post-event/events.json', params: {
context 'when updating' do
context 'a custom field' do
context 'allowed custom field' do
before do
SiteSetting.discourse_post_event_allowed_custom_fields = 'foo'
end
it 'works' do
expect(event_1.custom_fields['foo']).to eq(nil)
expect(event_1.raw_invitees).to eq(nil)
put "/discourse-post-event/events/#{event_1.id}.json", params: {
event: {
id: post1.id,
starts_at: 2.days.from_now,
custom_fields: { foo: 1 },
raw_invitees: ["bar"]
}
}
expect(response.parsed_body['event']['custom_fields']['foo']).to eq('1')
# doesn't update other fields
expect(response.parsed_body['event']['raw_invitees']).to eq(nil)
end
end
context 'not allowed custom field' do
it 'doesnt update' do
put "/discourse-post-event/events/#{event_1.id}.json", params: {
event: {
custom_fields: { bar: 1 }
}
}
expect(response.status).to eq(200)
expect(response.parsed_body['event']['id']).to eq(post1.id)
expect(Event).to exist(id: post1.id)
expect(response.parsed_body['custom_fields']).to eq(nil)
end
it 'accepts group as invitees' do
invitees = [group.name]
post '/discourse-post-event/events.json', params: {
event: {
id: post1.id,
raw_invitees: invitees,
starts_at: 2.days.from_now,
status: Event.statuses[:private]
}
}
expect(response.status).to eq(200)
sample_invitees = response.parsed_body['event']['sample_invitees']
expect(sample_invitees.map { |i| i['user']['id'] }).to match_array(group.group_users.map { |gu| gu.user.id })
raw_invitees = response.parsed_body['event']['raw_invitees']
expect(raw_invitees).to match_array(invitees)
end
it 'doesnt accept one user invitee' do
post '/discourse-post-event/events.json', params: {
event: {
id: post1.id,
status: Event.statuses[:private],
raw_invitees: [invitee1.username],
starts_at: 2.days.from_now,
}
}
expect(response.status).to eq(200)
sample_invitees = response.parsed_body['event']['sample_invitees']
expect(sample_invitees.length).to eq(0)
end
it 'accepts one group invitee' do
post '/discourse-post-event/events.json', params: {
event: {
id: post1.id,
status: Event.statuses[:private],
raw_invitees: [group.name],
starts_at: 2.days.from_now,
}
}
expect(response.status).to eq(200)
sample_invitees = response.parsed_body['event']['sample_invitees']
expect(sample_invitees.map { |i| i['user']['username'] }).to match_array(group.group_users.map(&:user).map(&:username))
end
it 'accepts no invitee' do
post '/discourse-post-event/events.json', params: {
event: {
id: post1.id,
raw_invitees: [],
status: Event.statuses[:private],
starts_at: 2.days.from_now,
}
}
expect(response.status).to eq(200)
sample_invitees = response.parsed_body['event']['sample_invitees']
expect(sample_invitees.count).to eq(0)
end
it 'limits displayed invitees' do
post '/discourse-post-event/events.json', params: {
event: {
id: post1.id,
status: Event.statuses[:private],
raw_invitees: [large_group.name],
starts_at: 2.days.from_now,
}
}
expect(response.status).to eq(200)
sample_invitees = response.parsed_body['event']['sample_invitees']
expect(large_group.group_users.length).to eq(4)
expect(sample_invitees.length).to eq(3)
end
context 'when an event exists' do
let(:event) { Fabricate(:event, post: post1) }
context 'when we update the event' do
context 'when an url is defined' do
it 'changes the url' do
url = 'https://www.google.fr'
put "/discourse-post-event/events/#{event.id}.json", params: {
event: { url: url }
}
event.reload
expect(event.url).to eq(url)
end
end
context 'when status changes from standalone to private' do
it 'changes the status, raw_invitees and invitees' do
event.update_with_params!(status: Event.statuses[:standalone])
put "/discourse-post-event/events/#{event.id}.json", params: {
event: {
status: Event.statuses[:private].to_s,
raw_invitees: [group.name]
}
}
event.reload
expect(event.status).to eq(Event.statuses[:private])
expect(event.raw_invitees).to eq([group.name])
expect(event.invitees).to eq([])
end
end
context 'when status changes from standalone to public' do
it 'changes the status' do
event.update_with_params!(status: Event.statuses[:standalone])
put "/discourse-post-event/events/#{event.id}.json", params: {
event: {
status: Event.statuses[:public].to_s
}
}
event.reload
expect(event.status).to eq(Event.statuses[:public])
expect(event.raw_invitees).to eq(['trust_level_0'])
expect(event.invitees).to eq([])
end
end
context 'when status changes from private to standalone' do
it 'changes the status' do
event.update_with_params!(
status: Event.statuses[:private],
raw_invitees: [group.name]
)
event.reload
expect(event.invitees).to eq([])
expect(event.raw_invitees).to eq([group.name])
put "/discourse-post-event/events/#{event.id}.json", params: {
event: {
status: Event.statuses[:standalone].to_s
}
}
event.reload
expect(event.status).to eq(Event.statuses[:standalone])
expect(event.raw_invitees).to eq([])
expect(event.invitees).to eq([])
end
end
context 'when status changes from private to public' do
it 'changes the status, removes raw_invitees and drops invitees' do
event.update_with_params!(
status: Event.statuses[:private],
raw_invitees: [group.name]
)
event.reload
expect(event.invitees).to eq([])
expect(event.raw_invitees).to eq([group.name])
put "/discourse-post-event/events/#{event.id}.json", params: {
event: {
status: Event.statuses[:public].to_s
}
}
event.reload
expect(event.status).to eq(Event.statuses[:public])
expect(event.raw_invitees).to eq(['trust_level_0'])
expect(event.invitees).to eq([])
end
end
context 'when status changes from public to private' do
it 'changes the status, removes raw_invitees and drops invitees not part of new raw_invitees' do
event.update_with_params!(status: Event.statuses[:public])
event.create_invitees([
{ user_id: invitee1.id },
{ user_id: invitee2.id },
])
event.reload
expect(event.invitees.pluck(:user_id)).to match_array([invitee1.id, invitee2.id])
expect(event.raw_invitees).to eq(['trust_level_0'])
put "/discourse-post-event/events/#{event.id}.json", params: {
event: {
status: Event.statuses[:private].to_s,
raw_invitees: [group.name]
}
}
event.reload
expect(event.status).to eq(Event.statuses[:private])
expect(event.raw_invitees).to eq([group.name])
expect(event.invitees.pluck(:user_id)).to eq([invitee2.id])
end
end
context 'when status changes from public to standalone' do
it 'changes the status, removes invitees' do
event.update_with_params!(
status: Event.statuses[:public]
)
event.create_invitees([ { user_id: invitee1.id } ])
event.reload
expect(event.invitees.pluck(:user_id)).to eq([invitee1.id])
expect(event.raw_invitees).to eq(['trust_level_0'])
put "/discourse-post-event/events/#{event.id}.json", params: {
event: {
status: Event.statuses[:standalone].to_s
}
}
event.reload
expect(event.status).to eq(Event.statuses[:standalone])
expect(event.raw_invitees).to eq([])
expect(event.invitees).to eq([])
end
end
@ -315,14 +83,14 @@ module DiscoursePostEvent
context 'current user can manage the event' do
context 'no file is given' do
it 'returns an error' do
post "/discourse-post-event/events/#{event.id}/csv-bulk-invite.json"
post "/discourse-post-event/events/#{event_1.id}/csv-bulk-invite.json"
expect(response.parsed_body['error_type']).to eq('invalid_parameters')
end
end
context 'empty file is given' do
it 'returns an error' do
post "/discourse-post-event/events/#{event.id}/csv-bulk-invite.json", { params: { file: fixture_file_upload(empty_file) } }
post "/discourse-post-event/events/#{event_1.id}/csv-bulk-invite.json", { params: { file: fixture_file_upload(empty_file) } }
expect(response.status).to eq(422)
end
end
@ -334,7 +102,7 @@ module DiscoursePostEvent
it 'enqueues the job and returns 200' do
expect_enqueued_with(job: :discourse_post_event_bulk_invite, args: {
"event_id" => event.id,
"event_id" => event_1.id,
"invitees" => [
{ 'identifier' => 'bob', 'attendance' => 'going' },
{ 'identifier' => 'sam', 'attendance' => 'interested' },
@ -342,7 +110,7 @@ module DiscoursePostEvent
],
"current_user_id" => user.id
}) do
post "/discourse-post-event/events/#{event.id}/csv-bulk-invite.json", { params: { file: fixture_file_upload(valid_file) } }
post "/discourse-post-event/events/#{event_1.id}/csv-bulk-invite.json", { params: { file: fixture_file_upload(valid_file) } }
end
expect(response.status).to eq(200)
@ -358,7 +126,7 @@ module DiscoursePostEvent
end
it 'returns an error' do
post "/discourse-post-event/events/#{event.id}/csv-bulk-invite.json"
post "/discourse-post-event/events/#{event_1.id}/csv-bulk-invite.json"
expect(response.status).to eq(403)
end
end
@ -368,14 +136,14 @@ module DiscoursePostEvent
context 'current user can manage the event' do
context 'no invitees is given' do
it 'returns an error' do
post "/discourse-post-event/events/#{event.id}/bulk-invite.json"
post "/discourse-post-event/events/#{event_1.id}/bulk-invite.json"
expect(response.parsed_body['error_type']).to eq('invalid_parameters')
end
end
context 'empty invitees are given' do
it 'returns an error' do
post "/discourse-post-event/events/#{event.id}/bulk-invite.json", { params: { invitees: [] } }
post "/discourse-post-event/events/#{event_1.id}/bulk-invite.json", { params: { invitees: [] } }
expect(response.status).to eq(400)
end
end
@ -387,7 +155,7 @@ module DiscoursePostEvent
it 'enqueues the job and returns 200' do
expect_enqueued_with(job: :discourse_post_event_bulk_invite, args: {
"event_id" => event.id,
"event_id" => event_1.id,
"invitees" => [
{ 'identifier' => 'bob', 'attendance' => 'going' },
{ 'identifier' => 'sam', 'attendance' => 'interested' },
@ -395,7 +163,7 @@ module DiscoursePostEvent
],
"current_user_id" => user.id
}) do
post "/discourse-post-event/events/#{event.id}/bulk-invite.json", { params: { invitees: [
post "/discourse-post-event/events/#{event_1.id}/bulk-invite.json", { params: { invitees: [
{ 'identifier' => 'bob', 'attendance' => 'going' },
{ 'identifier' => 'sam', 'attendance' => 'interested' },
{ 'identifier' => 'the_foo_bar_group', 'attendance' => 'not_going' }
@ -415,7 +183,7 @@ module DiscoursePostEvent
end
it 'returns an error' do
post "/discourse-post-event/events/#{event.id}/bulk-invite.json"
post "/discourse-post-event/events/#{event_1.id}/bulk-invite.json"
expect(response.status).to eq(403)
end
end
@ -424,17 +192,17 @@ module DiscoursePostEvent
context 'acting user has created the event' do
it 'destroys a event' do
expect(event.persisted?).to be(true)
expect(event_1.persisted?).to be(true)
messages = MessageBus.track_publish do
delete "/discourse-post-event/events/#{event.id}.json"
delete "/discourse-post-event/events/#{event_1.id}.json"
end
expect(messages.count).to eq(1)
message = messages.first
expect(message.channel).to eq("/discourse-post-event/#{event.post.topic_id}")
expect(message.data[:id]).to eq(event.id)
expect(message.channel).to eq("/discourse-post-event/#{event_1.post.topic_id}")
expect(message.data[:id]).to eq(event_1.id)
expect(response.status).to eq(200)
expect(Event).to_not exist(id: event.id)
expect(Event).to_not exist(id: event_1.id)
end
end
@ -446,14 +214,14 @@ module DiscoursePostEvent
end
it 'doesnt destroy the event' do
expect(event.persisted?).to be(true)
delete "/discourse-post-event/events/#{event.id}.json"
expect(event_1.persisted?).to be(true)
delete "/discourse-post-event/events/#{event_1.id}.json"
expect(response.status).to eq(403)
expect(Event).to exist(id: event.id)
expect(Event).to exist(id: event_1.id)
end
it 'doesnt update the event' do
put "/discourse-post-event/events/#{event.id}.json", params: {
put "/discourse-post-event/events/#{event_1.id}.json", params: {
event: {
status: Event.statuses[:public],
}
@ -470,7 +238,7 @@ module DiscoursePostEvent
context 'when topic is public' do
it 'can see the event' do
get "/discourse-post-event/events/#{event.id}.json"
get "/discourse-post-event/events/#{event_1.id}.json"
expect(response.status).to eq(200)
end
@ -478,11 +246,11 @@ module DiscoursePostEvent
context 'when topic is not public' do
before do
event.post.topic.convert_to_private_message(Discourse.system_user)
event_1.post.topic.convert_to_private_message(Discourse.system_user)
end
it 'cant see the event' do
get "/discourse-post-event/events/#{event.id}.json"
get "/discourse-post-event/events/#{event_1.id}.json"
expect(response.status).to eq(404)
end