SECURITY: Limit number of drafts per user and length of `draft_key`
The hidden site setting max_drafts_per_user defaults to 10_000 drafts per user. The longest key should be "topic_<MAX_BIG_INT>" which is 25 characters.
This commit is contained in:
parent
fed34a330b
commit
2232e15020
|
@ -40,6 +40,19 @@ class DraftsController < ApplicationController
|
|||
raise Discourse::InvalidParameters.new(:data)
|
||||
end
|
||||
|
||||
if reached_max_drafts_per_user?(params)
|
||||
render_json_error I18n.t("draft.too_many_drafts.title"),
|
||||
status: 403,
|
||||
extras: {
|
||||
description:
|
||||
I18n.t(
|
||||
"draft.too_many_drafts.description",
|
||||
base_url: Discourse.base_url,
|
||||
),
|
||||
}
|
||||
return
|
||||
end
|
||||
|
||||
sequence =
|
||||
begin
|
||||
Draft.set(
|
||||
|
@ -115,4 +128,13 @@ class DraftsController < ApplicationController
|
|||
|
||||
render json: success_json
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def reached_max_drafts_per_user?(params)
|
||||
user_id = current_user.id
|
||||
|
||||
Draft.where(user_id: user_id).count >= SiteSetting.max_drafts_per_user &&
|
||||
!Draft.exists?(user_id: user_id, draft_key: params[:draft_key])
|
||||
end
|
||||
end
|
||||
|
|
|
@ -9,6 +9,8 @@ class Draft < ActiveRecord::Base
|
|||
|
||||
has_many :upload_references, as: :target, dependent: :delete_all
|
||||
|
||||
validates :draft_key, length: { maximum: 25 }
|
||||
|
||||
after_commit :update_draft_count, on: %i[create destroy]
|
||||
|
||||
class OutOfSequence < StandardError
|
||||
|
|
|
@ -978,6 +978,9 @@ en:
|
|||
sequence_conflict_error:
|
||||
title: "draft error"
|
||||
description: "Draft is being edited in another window. Please reload this page."
|
||||
too_many_drafts:
|
||||
title: "Too many drafts"
|
||||
description: "You have reached the maximum number of allowed drafts. Please delete some of [your drafts](%{base_url}/my/activity/drafts) and try again."
|
||||
draft_backup:
|
||||
pm_title: "Backup Drafts from ongoing topics"
|
||||
pm_body: "Topic containing backup drafts"
|
||||
|
|
|
@ -1118,6 +1118,9 @@ posting:
|
|||
max_draft_length:
|
||||
default: 400_000
|
||||
hidden: true
|
||||
max_drafts_per_user:
|
||||
default: 10_000
|
||||
hidden: true
|
||||
max_form_template_title_length:
|
||||
default: 100
|
||||
min: 5
|
||||
|
|
|
@ -298,4 +298,6 @@ RSpec.describe Draft do
|
|||
expect(drafts[0].post.id).to eq(post.id)
|
||||
end
|
||||
end
|
||||
|
||||
it { is_expected.to validate_length_of(:draft_key).is_at_most(25) }
|
||||
end
|
||||
|
|
|
@ -234,6 +234,45 @@ RSpec.describe DraftsController do
|
|||
end
|
||||
end
|
||||
end
|
||||
|
||||
it "returns 403 when the maximum amount of drafts per users is reached" do
|
||||
SiteSetting.max_drafts_per_user = 2
|
||||
|
||||
user1 = Fabricate(:user)
|
||||
sign_in(user1)
|
||||
|
||||
data = { my: "data" }.to_json
|
||||
|
||||
# creating the first draft should work
|
||||
post "/drafts.json", params: { draft_key: "TOPIC_1", data: data }
|
||||
expect(response.status).to eq(200)
|
||||
|
||||
# same draft key, so shouldn't count against the limit
|
||||
post "/drafts.json", params: { draft_key: "TOPIC_1", data: data, sequence: 0 }
|
||||
expect(response.status).to eq(200)
|
||||
|
||||
# different draft key, so should count against the limit
|
||||
post "/drafts.json", params: { draft_key: "TOPIC_2", data: data }
|
||||
expect(response.status).to eq(200)
|
||||
|
||||
# limit should be reached now
|
||||
post "/drafts.json", params: { draft_key: "TOPIC_3", data: data }
|
||||
expect(response.status).to eq(403)
|
||||
|
||||
# updating existing draft should still work
|
||||
post "/drafts.json", params: { draft_key: "TOPIC_1", data: data, sequence: 1 }
|
||||
expect(response.status).to eq(200)
|
||||
|
||||
# creating a new draft as a different user should still work
|
||||
user2 = Fabricate(:user)
|
||||
sign_in(user2)
|
||||
post "/drafts.json", params: { draft_key: "TOPIC_3", data: data }
|
||||
expect(response.status).to eq(200)
|
||||
|
||||
# check the draft counts just to be safe
|
||||
expect(Draft.where(user_id: user1.id).count).to eq(2)
|
||||
expect(Draft.where(user_id: user2.id).count).to eq(1)
|
||||
end
|
||||
end
|
||||
|
||||
describe "#destroy" do
|
||||
|
|
Loading…
Reference in New Issue