mirror of
https://github.com/discourse/discourse.git
synced 2024-11-25 10:20:58 -06:00
bfc3132bb2
What is the problem here? In multiple controllers, we are accepting a `limit` params but do not impose any upper bound on the values being accepted. Without an upper bound, we may be allowing arbituary users from generating DB queries which may end up exhausing the resources on the server. What is the fix here? A new `fetch_limit_from_params` helper method is introduced in `ApplicationController` that can be used by controller actions to safely get the limit from the params as a default limit and maximum limit has to be set. When an invalid limit params is encountered, the server will respond with the 400 response code.
321 lines
8.9 KiB
Ruby
321 lines
8.9 KiB
Ruby
# frozen_string_literal: true
|
|
|
|
RSpec.describe DraftsController do
|
|
fab!(:user) { Fabricate(:user) }
|
|
|
|
describe "#index" do
|
|
it "requires you to be logged in" do
|
|
get "/drafts.json"
|
|
expect(response.status).to eq(403)
|
|
end
|
|
|
|
describe "when limit params is invalid" do
|
|
before { sign_in(user) }
|
|
|
|
include_examples "invalid limit params", "/drafts.json", described_class::INDEX_LIMIT
|
|
end
|
|
|
|
it "returns correct stream length after adding a draft" do
|
|
sign_in(user)
|
|
Draft.set(user, "xxx", 0, "{}")
|
|
get "/drafts.json"
|
|
expect(response.status).to eq(200)
|
|
parsed = response.parsed_body
|
|
expect(response.parsed_body["drafts"].length).to eq(1)
|
|
end
|
|
|
|
it "has empty stream after deleting last draft" do
|
|
sign_in(user)
|
|
Draft.set(user, "xxx", 0, "{}")
|
|
Draft.clear(user, "xxx", 0)
|
|
get "/drafts.json"
|
|
expect(response.status).to eq(200)
|
|
expect(response.parsed_body["drafts"].length).to eq(0)
|
|
end
|
|
|
|
it "does not include topic details when user cannot see topic" do
|
|
topic = Fabricate(:private_message_topic)
|
|
topic_user = topic.user
|
|
other_user = Fabricate(:user)
|
|
Draft.set(topic_user, "topic_#{topic.id}", 0, "{}")
|
|
Draft.set(other_user, "topic_#{topic.id}", 0, "{}")
|
|
|
|
sign_in(topic_user)
|
|
get "/drafts.json"
|
|
expect(response.status).to eq(200)
|
|
expect(response.parsed_body["drafts"].first["title"]).to eq(topic.title)
|
|
|
|
sign_in(other_user)
|
|
get "/drafts.json"
|
|
expect(response.status).to eq(200)
|
|
expect(response.parsed_body["drafts"].first["title"]).to eq(nil)
|
|
end
|
|
end
|
|
|
|
describe "#show" do
|
|
it "returns a draft if requested" do
|
|
sign_in(user)
|
|
Draft.set(user, "hello", 0, "test")
|
|
|
|
get "/drafts/hello.json"
|
|
expect(response.status).to eq(200)
|
|
expect(response.parsed_body["draft"]).to eq("test")
|
|
end
|
|
end
|
|
|
|
describe "#create" do
|
|
it "requires you to be logged in" do
|
|
post "/drafts.json"
|
|
expect(response.status).to eq(403)
|
|
end
|
|
|
|
it "saves a draft" do
|
|
sign_in(user)
|
|
|
|
post "/drafts.json", params: { draft_key: "xyz", data: { my: "data" }.to_json, sequence: 0 }
|
|
|
|
expect(response.status).to eq(200)
|
|
expect(Draft.get(user, "xyz", 0)).to eq(%q({"my":"data"}))
|
|
end
|
|
|
|
it "returns 404 when the key is missing" do
|
|
sign_in(Fabricate(:user))
|
|
post "/drafts.json", params: { data: { my: "data" }.to_json, sequence: 0 }
|
|
expect(response.status).to eq(404)
|
|
end
|
|
|
|
it "checks for an conflict on update" do
|
|
sign_in(user)
|
|
post = Fabricate(:post, user: user)
|
|
|
|
post "/drafts.json",
|
|
params: {
|
|
draft_key: "topic",
|
|
sequence: 0,
|
|
data: { postId: post.id, originalText: post.raw, action: "edit" }.to_json,
|
|
}
|
|
|
|
expect(response.status).to eq(200)
|
|
expect(response.parsed_body["conflict_user"]).to eq(nil)
|
|
|
|
post "/drafts.json",
|
|
params: {
|
|
draft_key: "topic",
|
|
sequence: 0,
|
|
data: { postId: post.id, originalText: "something else", action: "edit" }.to_json,
|
|
}
|
|
|
|
expect(response.status).to eq(200)
|
|
expect(response.parsed_body["conflict_user"]["id"]).to eq(post.last_editor.id)
|
|
expect(response.parsed_body["conflict_user"]).to include("avatar_template")
|
|
end
|
|
|
|
it "cant trivially resolve conflicts without interaction" do
|
|
sign_in(user)
|
|
|
|
DraftSequence.next!(user, "abc")
|
|
|
|
post "/drafts.json",
|
|
params: {
|
|
draft_key: "abc",
|
|
sequence: 0,
|
|
data: { a: "test" }.to_json,
|
|
owner: "abcdefg",
|
|
}
|
|
|
|
expect(response.status).to eq(200)
|
|
expect(response.parsed_body["draft_sequence"]).to eq(1)
|
|
end
|
|
|
|
it "has a clean protocol for ownership handover" do
|
|
sign_in(user)
|
|
|
|
post "/drafts.json",
|
|
params: {
|
|
draft_key: "abc",
|
|
sequence: 0,
|
|
data: { a: "test" }.to_json,
|
|
owner: "abcdefg",
|
|
}
|
|
|
|
expect(response.status).to eq(200)
|
|
expect(response.parsed_body["draft_sequence"]).to eq(0)
|
|
|
|
post "/drafts.json",
|
|
params: {
|
|
draft_key: "abc",
|
|
sequence: 0,
|
|
data: { b: "test" }.to_json,
|
|
owner: "hijklmnop",
|
|
}
|
|
|
|
expect(response.status).to eq(200)
|
|
expect(response.parsed_body["draft_sequence"]).to eq(1)
|
|
|
|
expect(DraftSequence.current(user, "abc")).to eq(1)
|
|
|
|
post "/drafts.json",
|
|
params: {
|
|
draft_key: "abc",
|
|
sequence: 1,
|
|
data: { c: "test" }.to_json,
|
|
owner: "hijklmnop",
|
|
}
|
|
|
|
expect(response.status).to eq(200)
|
|
expect(response.parsed_body["draft_sequence"]).to eq(2)
|
|
|
|
post "/drafts.json",
|
|
params: {
|
|
draft_key: "abc",
|
|
sequence: 2,
|
|
data: { c: "test" }.to_json,
|
|
owner: "abc",
|
|
}
|
|
|
|
expect(response.status).to eq(200)
|
|
expect(response.parsed_body["draft_sequence"]).to eq(3)
|
|
end
|
|
|
|
it "raises an error for out-of-sequence draft setting" do
|
|
sign_in(user)
|
|
seq = DraftSequence.next!(user, "abc")
|
|
Draft.set(user, "abc", seq, { b: "test" }.to_json)
|
|
|
|
post "/drafts.json",
|
|
params: {
|
|
draft_key: "abc",
|
|
sequence: seq - 1,
|
|
data: { a: "test" }.to_json,
|
|
}
|
|
|
|
expect(response.status).to eq(409)
|
|
|
|
post "/drafts.json",
|
|
params: {
|
|
draft_key: "abc",
|
|
sequence: seq + 1,
|
|
data: { a: "test" }.to_json,
|
|
}
|
|
|
|
expect(response.status).to eq(409)
|
|
end
|
|
|
|
context "when data is too big" do
|
|
let(:user) { Fabricate(:user) }
|
|
let(:data) { "a" * (SiteSetting.max_draft_length + 1) }
|
|
|
|
before do
|
|
SiteSetting.max_draft_length = 500
|
|
sign_in(user)
|
|
end
|
|
|
|
it "returns an error" do
|
|
post "/drafts.json",
|
|
params: {
|
|
draft_key: "xyz",
|
|
data: { reply: data }.to_json,
|
|
sequence: 0,
|
|
}
|
|
expect(response).to have_http_status :bad_request
|
|
end
|
|
end
|
|
|
|
context "when data is not too big" do
|
|
context "when data is not proper JSON" do
|
|
let(:user) { Fabricate(:user) }
|
|
let(:data) { "not-proper-json" }
|
|
|
|
before { sign_in(user) }
|
|
|
|
it "returns an error" do
|
|
post "/drafts.json", params: { draft_key: "xyz", data: data, sequence: 0 }
|
|
expect(response).to have_http_status :bad_request
|
|
end
|
|
end
|
|
end
|
|
end
|
|
|
|
describe "#destroy" do
|
|
it "destroys drafts when required" do
|
|
sign_in(user)
|
|
Draft.set(user, "xxx", 0, "hi")
|
|
delete "/drafts/xxx.json", params: { sequence: 0 }
|
|
|
|
expect(response.status).to eq(200)
|
|
expect(Draft.get(user, "xxx", 0)).to eq(nil)
|
|
end
|
|
|
|
it "denies attempts to destroy unowned draft" do
|
|
sign_in(Fabricate(:admin))
|
|
user = Fabricate(:user)
|
|
Draft.set(user, "xxx", 0, "hi")
|
|
delete "/drafts/xxx.json", params: { sequence: 0, username: user.username }
|
|
|
|
# Draft is not deleted because request is not via API
|
|
expect(Draft.get(user, "xxx", 0)).to be_present
|
|
end
|
|
|
|
shared_examples "for a passed user" do
|
|
it "deletes draft" do
|
|
api_key = Fabricate(:api_key).key
|
|
Draft.set(recipient, "xxx", 0, "hi")
|
|
|
|
delete "/drafts/xxx.json",
|
|
params: {
|
|
sequence: 0,
|
|
username: recipient.username,
|
|
},
|
|
headers: {
|
|
HTTP_API_USERNAME: caller.username,
|
|
HTTP_API_KEY: api_key,
|
|
}
|
|
|
|
expect(response.status).to eq(response_code)
|
|
|
|
if draft_deleted
|
|
expect(Draft.get(recipient, "xxx", 0)).to eq(nil)
|
|
else
|
|
expect(Draft.get(recipient, "xxx", 0)).to be_present
|
|
end
|
|
end
|
|
end
|
|
|
|
describe "api called by admin" do
|
|
include_examples "for a passed user" do
|
|
let(:caller) { Fabricate(:admin) }
|
|
let(:recipient) { Fabricate(:user) }
|
|
let(:response_code) { 200 }
|
|
let(:draft_deleted) { true }
|
|
end
|
|
end
|
|
|
|
describe "api called by tl4 user" do
|
|
include_examples "for a passed user" do
|
|
let(:caller) { Fabricate(:trust_level_4) }
|
|
let(:recipient) { Fabricate(:user) }
|
|
let(:response_code) { 403 }
|
|
let(:draft_deleted) { false }
|
|
end
|
|
end
|
|
|
|
describe "api called by regular user" do
|
|
include_examples "for a passed user" do
|
|
let(:caller) { Fabricate(:user) }
|
|
let(:recipient) { Fabricate(:user) }
|
|
let(:response_code) { 403 }
|
|
let(:draft_deleted) { false }
|
|
end
|
|
end
|
|
|
|
describe "api called by admin for another admin" do
|
|
include_examples "for a passed user" do
|
|
let(:caller) { Fabricate(:admin) }
|
|
let(:recipient) { Fabricate(:admin) }
|
|
let(:response_code) { 200 }
|
|
let(:draft_deleted) { true }
|
|
end
|
|
end
|
|
end
|
|
end
|