mirror of
https://gh.wpcy.net/https://github.com/discourse/discourse.git
synced 2026-05-01 23:40:53 +08:00
0b4e6ff170
`Oneboxer.local_topic` skips the `can_see_topic?` check on the target topic when the request's `category_id` matchs the target's category. A user could bypass topic-level access controls (e.g. shared draft) by sending the matching category ID in the inline onebox request. This PR fixes this by always running `can_see_topic?` on the target. Note also shared drafts can be in public topics, but will still be hidden to the user on a topic level. --- **Security Advisory:** https://github.com/discourse/discourse/security/advisories/GHSA-v93g-8f4f-4rgm
163 lines
5 KiB
Ruby
163 lines
5 KiB
Ruby
# frozen_string_literal: true
|
|
|
|
RSpec.describe InlineOneboxController do
|
|
it "requires the user to be logged in" do
|
|
get "/inline-onebox.json", params: { urls: [] }
|
|
expect(response.status).to eq(403)
|
|
end
|
|
|
|
context "when logged in" do
|
|
fab!(:user)
|
|
before { sign_in(user) }
|
|
|
|
it "returns empty JSON for empty input" do
|
|
get "/inline-onebox.json", params: { urls: [] }
|
|
expect(response.status).to eq(200)
|
|
json = response.parsed_body
|
|
expect(json["inline-oneboxes"]).to eq([])
|
|
end
|
|
|
|
it "returns a 413 error if more than 10 urls are sent" do
|
|
get "/inline-onebox.json", params: { urls: ("a".."k").to_a }
|
|
expect(response.status).to eq(413)
|
|
json = response.parsed_body
|
|
expect(json["errors"]).to include(I18n.t("inline_oneboxer.too_many_urls"))
|
|
end
|
|
|
|
it "returns a 429 error for concurrent requests from the same user" do
|
|
blocked = true
|
|
reached = false
|
|
|
|
stub_request(:get, "http://example.com/url-1").to_return do |request|
|
|
reached = true
|
|
sleep 0.001 while blocked
|
|
{ status: 200, body: <<~HTML }
|
|
<html>
|
|
<head>
|
|
<title>
|
|
Concurrent inline-oneboxing test
|
|
</title>
|
|
</head>
|
|
<body></body>
|
|
</html>
|
|
HTML
|
|
end
|
|
|
|
t1 = Thread.new { get "/inline-onebox.json", params: { urls: ["http://example.com/url-1"] } }
|
|
|
|
sleep 0.001 while !reached
|
|
|
|
get "/inline-onebox.json", params: { urls: ["http://example.com/url-2"] }
|
|
expect(response.status).to eq(429)
|
|
expect(response.parsed_body["errors"]).to include(
|
|
I18n.t("inline_oneboxer.concurrency_not_allowed"),
|
|
)
|
|
|
|
blocked = false
|
|
t1.join
|
|
expect(response.status).to eq(200)
|
|
json = response.parsed_body
|
|
expect(json["inline-oneboxes"].size).to eq(1)
|
|
expect(json["inline-oneboxes"][0]["title"]).to eq("Concurrent inline-oneboxing test")
|
|
end
|
|
|
|
it "allows concurrent requests from different users" do
|
|
another_user = Fabricate(:user)
|
|
|
|
blocked = true
|
|
reached = false
|
|
|
|
stub_request(:get, "http://example.com/url-1").to_return do |request|
|
|
reached = true
|
|
sleep 0.001 while blocked
|
|
{ status: 200, body: <<~HTML }
|
|
<html>
|
|
<head>
|
|
<title>
|
|
Concurrent inline-oneboxing test
|
|
</title>
|
|
</head>
|
|
<body></body>
|
|
</html>
|
|
HTML
|
|
end
|
|
|
|
stub_request(:get, "http://example.com/url-2").to_return do |request|
|
|
{ status: 200, body: <<~HTML }
|
|
<html>
|
|
<head>
|
|
<title>
|
|
Concurrent inline-oneboxing test 2
|
|
</title>
|
|
</head>
|
|
<body></body>
|
|
</html>
|
|
HTML
|
|
end
|
|
|
|
t1 =
|
|
Thread.new do
|
|
sign_in(user)
|
|
get "/inline-onebox.json", params: { urls: ["http://example.com/url-1"] }
|
|
end
|
|
|
|
sleep 0.001 while !reached
|
|
|
|
sign_in(another_user)
|
|
get "/inline-onebox.json", params: { urls: ["http://example.com/url-2"] }
|
|
expect(response.status).to eq(200)
|
|
json = response.parsed_body
|
|
expect(json["inline-oneboxes"].size).to eq(1)
|
|
expect(json["inline-oneboxes"][0]["title"]).to eq("Concurrent inline-oneboxing test 2")
|
|
|
|
blocked = false
|
|
t1.join
|
|
expect(response.status).to eq(200)
|
|
json = response.parsed_body
|
|
expect(json["inline-oneboxes"].size).to eq(1)
|
|
expect(json["inline-oneboxes"][0]["title"]).to eq("Concurrent inline-oneboxing test")
|
|
end
|
|
|
|
context "with topic link" do
|
|
fab!(:topic)
|
|
|
|
it "returns information for a valid link" do
|
|
get "/inline-onebox.json", params: { urls: [topic.url] }
|
|
expect(response.status).to eq(200)
|
|
json = response.parsed_body
|
|
onebox = json["inline-oneboxes"][0]
|
|
|
|
expect(onebox).to be_present
|
|
expect(onebox["url"]).to eq(topic.url)
|
|
expect(onebox["title"]).to eq(topic.title)
|
|
end
|
|
end
|
|
|
|
context "with shared draft topic" do
|
|
fab!(:shared_drafts_category, :category)
|
|
fab!(:destination_category, :category)
|
|
fab!(:shared_draft_topic) { Fabricate(:topic, category: shared_drafts_category) }
|
|
fab!(:shared_draft_post) { Fabricate(:post, topic: shared_draft_topic) }
|
|
fab!(:shared_draft) do
|
|
Fabricate(:shared_draft, topic: shared_draft_topic, category: destination_category)
|
|
end
|
|
|
|
before do
|
|
SiteSetting.shared_drafts_category = shared_drafts_category.id
|
|
SiteSetting.shared_drafts_allowed_groups = Group::AUTO_GROUPS[:staff]
|
|
end
|
|
|
|
it "does not leak shared draft topic titles via user-controlled category_id" do
|
|
get "/inline-onebox.json",
|
|
params: {
|
|
urls: [shared_draft_topic.url],
|
|
category_id: shared_drafts_category.id,
|
|
}
|
|
|
|
expect(response.status).to eq(200)
|
|
json = response.parsed_body
|
|
expect(json["inline-oneboxes"][0]).to be_blank
|
|
end
|
|
end
|
|
end
|
|
end
|
