0
0
Fork 0
mirror of https://github.com/discourse/discourse.git synced 2026-08-09 21:45:25 +08:00
discourse/spec/requests/static_controller_spec.rb
Juan David Martinez 8c8cae07c9 SECURITY: Validate sso_destination_url cookie to prevent open redirect [backport 2026.1]
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
2026-03-31 15:12:50 +01:00

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