mirror of
https://github.com/discourse/discourse.git
synced 2026-08-09 21:45:25 +08:00
Backport of #561 to release/2026.1. --- **Description** Adds validation to ensure the `sso_destination_url` cookie value matches configured SSO provider domains before allowing external redirects. Previously, an attacker who could set cookies could redirect users to arbitrary external domains, enabling phishing attacks. This PR fixes that by validating the destination URL against the enabled URL domains in the `enable_discourse_connect_provider` SiteSetting. --- **Security Advisory:** https://github.com/discourse/discourse/security/advisories/GHSA-378j-ccw4-4fwh
533 lines
17 KiB
Ruby
Vendored
533 lines
17 KiB
Ruby
Vendored
# frozen_string_literal: true
|
|
|
|
RSpec.describe StaticController do
|
|
fab!(:upload)
|
|
|
|
describe "#favicon" do
|
|
let(:filename) { "smallest.png" }
|
|
let(:file) { file_from_fixtures(filename) }
|
|
|
|
let(:upload) { UploadCreator.new(file, filename).create_for(Discourse.system_user.id) }
|
|
|
|
after { Discourse.redis.scan_each(match: "memoize_*").each { |key| Discourse.redis.del(key) } }
|
|
|
|
context "with local store" do
|
|
it "returns the default favicon if favicon has not been configured" do
|
|
get "/favicon/proxied"
|
|
|
|
expect(response.status).to eq(200)
|
|
expect(response.media_type).to eq("image/png")
|
|
expect(response.body.bytesize).to eq(SiteIconManager.favicon.filesize)
|
|
end
|
|
|
|
it "returns the configured favicon" do
|
|
SiteSetting.favicon = upload
|
|
|
|
get "/favicon/proxied"
|
|
|
|
expect(response.status).to eq(200)
|
|
expect(response.media_type).to eq("image/png")
|
|
expect(response.body.bytesize).to eq(upload.filesize)
|
|
end
|
|
end
|
|
|
|
context "with external store" do
|
|
let(:upload) do
|
|
Upload.create!(
|
|
url: "//s3-upload-bucket.s3-us-east-1.amazonaws.com/somewhere/a.png",
|
|
original_filename: filename,
|
|
filesize: file.size,
|
|
user_id: Discourse.system_user.id,
|
|
)
|
|
end
|
|
|
|
before { setup_s3 }
|
|
|
|
it "can proxy a favicon correctly" do
|
|
SiteSetting.favicon = upload
|
|
|
|
stub_request(:get, "https:/#{upload.url}").to_return(status: 200, body: file)
|
|
|
|
get "/favicon/proxied"
|
|
|
|
expect(response.status).to eq(200)
|
|
expect(response.media_type).to eq("image/png")
|
|
expect(response.body.bytesize).to eq(upload.filesize)
|
|
end
|
|
|
|
context "when favicon fails to load" do
|
|
before { FileHelper.stubs(:download).raises(SocketError) }
|
|
|
|
it "creates an admin notice" do
|
|
expect { get "/favicon/proxied" }.to change { AdminNotice.problem.count }.by(1)
|
|
end
|
|
end
|
|
end
|
|
end
|
|
|
|
describe "#cdn_asset" do
|
|
let(:site) { RailsMultisite::ConnectionManagement.current_db }
|
|
|
|
it "can serve assets" do
|
|
begin
|
|
assets_path = Rails.public_path.join("assets")
|
|
|
|
FileUtils.mkdir_p(assets_path)
|
|
|
|
file_path = assets_path.join("test.js.br")
|
|
File.write(file_path, "fake brotli file")
|
|
|
|
get "/cdn_asset/#{site}/test.js.br"
|
|
|
|
expect(response.status).to eq(200)
|
|
expect(response.headers["Cache-Control"]).to match(/public/)
|
|
ensure
|
|
File.delete(file_path)
|
|
end
|
|
end
|
|
end
|
|
|
|
describe "#show" do
|
|
before do
|
|
post = create_post
|
|
SiteSetting.tos_topic_id = post.topic.id
|
|
SiteSetting.guidelines_topic_id = post.topic.id
|
|
SiteSetting.privacy_topic_id = post.topic.id
|
|
end
|
|
|
|
context "with a static file that's present" do
|
|
it "should return the right response for /faq" do
|
|
get "/faq"
|
|
|
|
expect(response.status).to eq(200)
|
|
expect(response.body).to include(I18n.t("js.faq"))
|
|
expect(response.body).to include("<title>FAQ - Discourse</title>")
|
|
end
|
|
end
|
|
|
|
[
|
|
["tos", :tos_url, I18n.t("js.tos")],
|
|
["privacy", :privacy_policy_url, I18n.t("js.privacy")],
|
|
].each do |id, setting_name, text|
|
|
context "with #{id}" do
|
|
context "when #{setting_name} site setting is NOT set" do
|
|
it "renders the #{id} page" do
|
|
get "/#{id}"
|
|
|
|
expect(response.status).to eq(200)
|
|
expect(response.body).to include(text)
|
|
end
|
|
end
|
|
|
|
context "when #{setting_name} site setting is set" do
|
|
before { SiteSetting.set(setting_name, "http://example.com/page") }
|
|
|
|
it "redirects to the #{setting_name}" do
|
|
get "/#{id}"
|
|
|
|
expect(response).to redirect_to("http://example.com/page")
|
|
end
|
|
end
|
|
end
|
|
end
|
|
|
|
context "with a missing file" do
|
|
it "should respond 404" do
|
|
get "/static/does-not-exist"
|
|
expect(response.status).to eq(404)
|
|
end
|
|
|
|
context "with modal pages" do
|
|
it "should return the right response for /signup" do
|
|
get "/signup"
|
|
expect(response.status).to eq(200)
|
|
end
|
|
|
|
it "should return the right response for /password-reset" do
|
|
get "/password-reset"
|
|
expect(response.status).to eq(200)
|
|
end
|
|
end
|
|
end
|
|
|
|
it "should redirect to / when logged in and path is /login without redirect" do
|
|
sign_in(Fabricate(:user))
|
|
get "/login"
|
|
expect(response).to redirect_to("/")
|
|
end
|
|
|
|
it "should display the login template when login is required" do
|
|
SiteSetting.login_required = true
|
|
|
|
get "/login"
|
|
|
|
expect(response.status).to eq(200)
|
|
|
|
expect(response.body).to include(
|
|
PrettyText.cook(I18n.t("login_required.welcome_message", title: SiteSetting.title)),
|
|
)
|
|
end
|
|
|
|
context "when login_required is enabled" do
|
|
before { SiteSetting.login_required = true }
|
|
|
|
%w[faq guidelines rules conduct].each do |page_name|
|
|
it "#{page_name} page redirects to login page for anon" do
|
|
get "/#{page_name}"
|
|
expect(response).to redirect_to "/login"
|
|
end
|
|
|
|
it "#{page_name} page loads for logged in user" do
|
|
sign_in(Fabricate(:user))
|
|
|
|
get "/#{page_name}"
|
|
|
|
expect(response.status).to eq(200)
|
|
expect(response.body).to include(I18n.t("js.guidelines"))
|
|
end
|
|
end
|
|
end
|
|
|
|
context "with crawler view" do
|
|
it "should include correct title" do
|
|
get "/faq", headers: { "HTTP_USER_AGENT" => "Googlebot" }
|
|
expect(response.status).to eq(200)
|
|
expect(response.body).to include("<title>FAQ - Discourse</title>")
|
|
end
|
|
end
|
|
|
|
context "with plugin api extensions" do
|
|
after do
|
|
Rails.application.reload_routes!
|
|
StaticController::CUSTOM_PAGES.clear
|
|
end
|
|
|
|
it "adds new topic-backed pages" do
|
|
routes = Proc.new { get "contact" => "static#show", :id => "contact" }
|
|
Discourse::Application.routes.send(:eval_block, routes)
|
|
|
|
topic_id = Fabricate(:post, cooked: "contact info").topic_id
|
|
SiteSetting.test_some_topic_id = topic_id
|
|
|
|
Plugin::Instance.new.add_topic_static_page("contact", topic_id: "test_some_topic_id")
|
|
|
|
get "/contact"
|
|
|
|
expect(response.status).to eq(200)
|
|
expect(response.body).to include("contact info")
|
|
end
|
|
|
|
it "replaces existing topic-backed pages" do
|
|
topic_id = Fabricate(:post, cooked: "Regular FAQ").topic_id
|
|
SiteSetting.test_some_topic_id = topic_id
|
|
|
|
polish_topic_id = Fabricate(:post, cooked: "Polish FAQ").topic_id
|
|
SiteSetting.test_some_other_topic_id = polish_topic_id
|
|
|
|
Plugin::Instance
|
|
.new
|
|
.add_topic_static_page("faq") do
|
|
current_user&.locale == "pl" ? "test_some_other_topic_id" : "test_some_topic_id"
|
|
end
|
|
|
|
get "/faq"
|
|
|
|
expect(response.status).to eq(200)
|
|
expect(response.body).to include("Regular FAQ")
|
|
|
|
sign_in(Fabricate(:user, locale: "pl"))
|
|
get "/faq"
|
|
|
|
expect(response.status).to eq(200)
|
|
expect(response.body).to include("Polish FAQ")
|
|
end
|
|
end
|
|
|
|
it "does not pollute SiteSetting.title (regression)" do
|
|
SiteSetting.title = "test"
|
|
SiteSetting.short_site_description = "something"
|
|
|
|
expect do
|
|
get "/login"
|
|
get "/login"
|
|
end.to_not change { SiteSetting.title }
|
|
end
|
|
|
|
context "without a subfolder" do
|
|
it "redirects as requested when logged in and path is /login with valid redirect param" do
|
|
sign_in(Fabricate(:user))
|
|
get "/login", params: { redirect: "/foo" }
|
|
expect(response).to redirect_to("/foo")
|
|
end
|
|
|
|
it "redirects to / when logged in and path is /login with invalid redirect param" do
|
|
sign_in(Fabricate(:user))
|
|
get "/login", params: { redirect: "//foo" }
|
|
expect(response).to redirect_to("/")
|
|
get "/login", params: { redirect: "foo" }
|
|
expect(response).to redirect_to("/")
|
|
get "/login", params: { redirect: "http://foo" }
|
|
expect(response).to redirect_to("/")
|
|
get "/login", params: { redirect: "www.foo.bar" }
|
|
expect(response).to redirect_to("/")
|
|
end
|
|
|
|
context "when setting the destination_url cookie" do
|
|
it "respects the redirect parameter in the cookie" do
|
|
get "/login", params: { redirect: "/foo" }
|
|
expect(response.cookies["destination_url"]).to eq("/foo")
|
|
end
|
|
|
|
it "respects the redirect parameter including the query params" do
|
|
get "/login", params: { redirect: "/foo?filter=test" }
|
|
expect(response.cookies["destination_url"]).to eq("/foo?filter=test")
|
|
end
|
|
end
|
|
end
|
|
|
|
context "with a subfolder" do
|
|
before { set_subfolder "/sub_test" }
|
|
|
|
it "redirects as requested when logged in and path is /login with valid redirect param" do
|
|
sign_in(Fabricate(:user))
|
|
get "/login", params: { redirect: "/foo" }
|
|
expect(response).to redirect_to("/sub_test/foo")
|
|
end
|
|
|
|
it "redirects to / when logged in and path is /login with invalid redirect param" do
|
|
sign_in(Fabricate(:user))
|
|
get "/login", params: { redirect: "//foo" }
|
|
expect(response).to redirect_to("/sub_test/")
|
|
get "/login", params: { redirect: "foo" }
|
|
expect(response).to redirect_to("/sub_test/")
|
|
get "/login", params: { redirect: "http://foo" }
|
|
expect(response).to redirect_to("/sub_test/")
|
|
get "/login", params: { redirect: "www.foo.bar" }
|
|
expect(response).to redirect_to("/sub_test/")
|
|
end
|
|
|
|
context "when sets the destination_url cookie" do
|
|
it "respects the redirect parameter in the cookie" do
|
|
get "/login", params: { redirect: "/foo" }
|
|
expect(response.cookies["destination_url"]).to eq("/sub_test/foo")
|
|
end
|
|
|
|
it "respects the redirect parameter including the query params" do
|
|
get "/login", params: { redirect: "/foo?filter=test" }
|
|
expect(response.cookies["destination_url"]).to eq("/sub_test/foo?filter=test")
|
|
end
|
|
end
|
|
end
|
|
end
|
|
|
|
describe "#enter" do
|
|
context "without a redirect path" do
|
|
it "redirects to the root url" do
|
|
post "/login.json"
|
|
expect(response).to redirect_to("/")
|
|
end
|
|
end
|
|
|
|
context "with a redirect path" do
|
|
it "redirects to the redirect path" do
|
|
post "/login.json", params: { redirect: "/foo" }
|
|
expect(response).to redirect_to("/foo")
|
|
end
|
|
end
|
|
|
|
context "with a full url" do
|
|
it "redirects to the correct path" do
|
|
post "/login.json", params: { redirect: "#{Discourse.base_url}/foo" }
|
|
expect(response).to redirect_to("/foo")
|
|
end
|
|
end
|
|
|
|
context "with a redirect path with query params" do
|
|
it "redirects to the redirect path and preserves query params" do
|
|
post "/login.json", params: { redirect: "/foo?bar=1" }
|
|
expect(response).to redirect_to("/foo?bar=1")
|
|
end
|
|
end
|
|
|
|
context "with a period to force a new host" do
|
|
it "redirects to the root path" do
|
|
post "/login.json", params: { redirect: ".org/foo" }
|
|
expect(response).to redirect_to("/")
|
|
end
|
|
end
|
|
|
|
context "with a full url to an external host" do
|
|
it "redirects to the root path" do
|
|
post "/login.json", params: { redirect: "http://eviltrout.com/foo" }
|
|
expect(response).to redirect_to("/")
|
|
end
|
|
end
|
|
|
|
context "with an invalid URL" do
|
|
it "redirects to the root" do
|
|
post "/login.json", params: { redirect: "javascript:alert('trout')" }
|
|
expect(response).to redirect_to("/")
|
|
end
|
|
end
|
|
|
|
context "with an array" do
|
|
it "redirects to the root" do
|
|
post "/login.json", params: { redirect: ["/foo"] }
|
|
expect(response).to redirect_to("/")
|
|
end
|
|
end
|
|
|
|
context "when the redirect path is the login page" do
|
|
it "redirects to the root url" do
|
|
post "/login.json", params: { redirect: login_path }
|
|
expect(response).to redirect_to("/")
|
|
end
|
|
end
|
|
|
|
context "when the redirect path contains the '/login' string" do
|
|
it "redirects to the requested path" do
|
|
post "/login.json", params: { redirect: "/page/login/1" }
|
|
expect(response).to redirect_to("/page/login/1")
|
|
end
|
|
end
|
|
context "when the redirect path is invalid" do
|
|
it "redirects to the root URL" do
|
|
post "/login.json", params: { redirect: "test" }
|
|
expect(response).to redirect_to("/")
|
|
end
|
|
end
|
|
|
|
context "with a subfolder" do
|
|
before { set_subfolder "/sub_test" }
|
|
|
|
context "without a redirect path" do
|
|
it "redirects to the subfolder root" do
|
|
post "/login.json"
|
|
expect(response).to redirect_to("/sub_test/")
|
|
end
|
|
end
|
|
|
|
context "when the redirect path is the login page" do
|
|
it "redirects to the subfolder root" do
|
|
post "/login.json", params: { redirect: "#{Discourse.base_path}/login" }
|
|
expect(response).to redirect_to("/sub_test/")
|
|
end
|
|
end
|
|
|
|
context "when the redirect path is invalid" do
|
|
it "redirects to the subfolder root" do
|
|
post "/login.json", params: { redirect: "test" }
|
|
expect(response).to redirect_to("/sub_test/")
|
|
end
|
|
end
|
|
end
|
|
|
|
context "with sso_destination_url cookie" do
|
|
before { SiteSetting.enable_discourse_connect_provider = true }
|
|
|
|
it "redirects to valid SSO destination URL when provider is configured" do
|
|
SiteSetting.discourse_connect_provider_secrets = "allowed-site.com|secret123"
|
|
cookies[:sso_destination_url] = "https://allowed-site.com/sso?token=abc"
|
|
|
|
post "/login.json"
|
|
|
|
expect(response).to redirect_to("https://allowed-site.com/sso?token=abc")
|
|
expect(response.cookies["sso_destination_url"]).to be_nil
|
|
end
|
|
|
|
it "redirects to valid SSO destination URL with wildcard domain" do
|
|
SiteSetting.discourse_connect_provider_secrets = "*.allowed-domain.com|secret123"
|
|
cookies[:sso_destination_url] = "https://sub.allowed-domain.com/sso?token=abc"
|
|
|
|
post "/login.json"
|
|
|
|
expect(response).to redirect_to("https://sub.allowed-domain.com/sso?token=abc")
|
|
end
|
|
|
|
it "ignores SSO destination URL when domain is not in provider secrets" do
|
|
SiteSetting.discourse_connect_provider_secrets = "allowed-site.com|secret123"
|
|
cookies[:sso_destination_url] = "https://evil-site.com/phishing"
|
|
|
|
post "/login.json"
|
|
|
|
expect(response).to redirect_to("/")
|
|
expect(response.cookies["sso_destination_url"]).to be_nil
|
|
end
|
|
|
|
it "ignores SSO destination URL when provider secrets is empty" do
|
|
SiteSetting.discourse_connect_provider_secrets = ""
|
|
cookies[:sso_destination_url] = "https://some-site.com/sso"
|
|
|
|
post "/login.json"
|
|
|
|
expect(response).to redirect_to("/")
|
|
end
|
|
|
|
it "ignores malformed SSO destination URL" do
|
|
SiteSetting.discourse_connect_provider_secrets = "allowed-site.com|secret123"
|
|
cookies[:sso_destination_url] = "not a valid url"
|
|
|
|
post "/login.json"
|
|
|
|
expect(response).to redirect_to("/")
|
|
end
|
|
|
|
it "ignores SSO destination URL when discourse_connect_provider is disabled" do
|
|
SiteSetting.enable_discourse_connect_provider = false
|
|
SiteSetting.discourse_connect_provider_secrets = "allowed-site.com|secret123"
|
|
cookies[:sso_destination_url] = "https://allowed-site.com/sso"
|
|
|
|
post "/login.json"
|
|
|
|
expect(response).to redirect_to("/")
|
|
end
|
|
|
|
it "deletes sso_destination_url cookie regardless of validity" do
|
|
SiteSetting.discourse_connect_provider_secrets = "allowed-site.com|secret123"
|
|
cookies[:sso_destination_url] = "https://evil-site.com/phishing"
|
|
|
|
post "/login.json"
|
|
|
|
expect(response.cookies["sso_destination_url"]).to be_nil
|
|
end
|
|
end
|
|
end
|
|
|
|
describe "#service_worker_asset" do
|
|
it "works" do
|
|
get "/service-worker.js"
|
|
expect(response.status).to eq(200)
|
|
expect(response.content_type).to start_with("text/javascript")
|
|
expect(response.body).to include("addEventListener")
|
|
end
|
|
end
|
|
|
|
describe "#llms_txt" do
|
|
it "returns 404 when no upload is set" do
|
|
get "/llms.txt"
|
|
expect(response.status).to eq(404)
|
|
end
|
|
|
|
context "with local store" do
|
|
it "returns content as plain text" do
|
|
SiteSetting.authorized_extensions = "txt"
|
|
|
|
file = Tempfile.new(%w[llms .txt])
|
|
file.write("# Test LLMs Content")
|
|
file.rewind
|
|
|
|
upload = UploadCreator.new(file, "llms.txt").create_for(Discourse.system_user.id)
|
|
SiteSetting.llms_txt = upload
|
|
|
|
get "/llms.txt"
|
|
|
|
expect(response.status).to eq(200)
|
|
expect(response.content_type).to start_with("text/plain")
|
|
expect(response.body).to eq("# Test LLMs Content")
|
|
ensure
|
|
file.close
|
|
file.unlink
|
|
end
|
|
end
|
|
end
|
|
end
|