mirror of
https://gh.wpcy.net/https://github.com/discourse/discourse.git
synced 2026-05-26 23:09:13 +08:00
This PR combines two security fixes for the discourse-subscriptions plugin. ## Commits ### 1. [`8f0a8e98e9`](8f0a8e98e9) — SECURITY: Fix unauthorized group access via subscription finalize The `/s/finalize` endpoint accepted client-supplied `plan` and `transaction` parameters that determined which group a user would be added to after completing a Stripe payment. During the 3D Secure authentication flow, an authenticated user could complete a cheap subscription in `SubscribeController#create` but supply a premium plan ID when calling `#finalize`, granting themselves access to a higher-tier group they never paid for. This commit fixes the vulnerability by linking the two endpoints server-side using `server_session`. When `#create` produces a transaction requiring 3D Secure authentication (`incomplete` or `open` status), it stores the transaction ID and plan ID in the server session. `#finalize` then reads exclusively from the session instead of accepting client parameters, and clears the entry after successful finalization. On the frontend, `Transaction.finalize()` no longer sends any parameters to the server. (from #434) --- ### 2. [`dcf80dda61`](dcf80dda61) — SECURITY: Use per-request Stripe API key instead of global state Replace `set_api_key` (which mutated global `::Stripe.api_key`) with `set_stripe_api_key` (which stores the key in an instance variable). All Stripe API calls now receive `{ api_key: @stripe_api_key }` as the per-request opts parameter, following the stripe gem's documented per-request configuration pattern. This prevents API key leakage across concurrent requests in multi-threaded environments. (from #590) --- **Security Advisory:** https://github.com/discourse/discourse/security/advisories/GHSA-f866-8fcp-fgvv --- **Security Advisory:** https://github.com/discourse/discourse/security/advisories/GHSA-9vg5-mp49-xghh
149 lines
4.8 KiB
Ruby
Vendored
149 lines
4.8 KiB
Ruby
Vendored
# frozen_string_literal: true
|
|
|
|
RSpec.describe DiscourseSubscriptions::User::PaymentsController do
|
|
before do
|
|
SiteSetting.discourse_subscriptions_enabled = true
|
|
SiteSetting.discourse_subscriptions_secret_key = "secret-key"
|
|
end
|
|
|
|
it "is a subclass of ApplicationController" do
|
|
expect(DiscourseSubscriptions::User::PaymentsController < ::ApplicationController).to eq(true)
|
|
end
|
|
|
|
context "when not authenticated" do
|
|
it "does not get the payment intents" do
|
|
::Stripe::PaymentIntent.expects(:list).never
|
|
get "/s/user/payments.json"
|
|
expect(response.status).to eq(403)
|
|
end
|
|
end
|
|
|
|
context "when authenticated" do
|
|
let(:user) { Fabricate(:user, email: "zasch@example.com") }
|
|
|
|
before do
|
|
sign_in(user)
|
|
Fabricate(:customer, customer_id: "c_345678", user_id: user.id)
|
|
Fabricate(:product, external_id: "prod_8675309")
|
|
Fabricate(:product, external_id: "prod_8675310")
|
|
end
|
|
|
|
it "gets payment intents" do
|
|
created_time = Time.now
|
|
::Stripe::Invoice
|
|
.expects(:list)
|
|
.with({ customer: "c_345678" }, DiscourseSubscriptions::Stripe.request_opts)
|
|
.returns(
|
|
data: [
|
|
{ id: "inv_900007", lines: { data: [plan: { product: "prod_8675309" }] } },
|
|
{ id: "inv_900008", lines: { data: [plan: { product: "prod_8675310" }] } },
|
|
{ id: "inv_900008", lines: { data: [plan: { product: "prod_8675310" }] } },
|
|
],
|
|
)
|
|
|
|
::Stripe::PaymentIntent
|
|
.expects(:list)
|
|
.with({ customer: "c_345678" }, DiscourseSubscriptions::Stripe.request_opts)
|
|
.returns(
|
|
data: [
|
|
{ id: "pi_900008", invoice: "inv_900008", created: created_time },
|
|
{ id: "pi_900008", invoice: "inv_900008", created: created_time },
|
|
{ id: "pi_900007", invoice: "inv_900007", created: Time.now },
|
|
{ id: "pi_007", invoice: "inv_007", created: Time.now },
|
|
],
|
|
)
|
|
|
|
get "/s/user/payments.json"
|
|
|
|
parsed_body = response.parsed_body
|
|
invoice = parsed_body[0]["invoice"]
|
|
|
|
expect(invoice).to eq("inv_900007")
|
|
expect(parsed_body.count).to eq(2)
|
|
end
|
|
|
|
it "gets pricing table one-off purchases" do
|
|
::Stripe::Invoice
|
|
.expects(:list)
|
|
.with({ customer: "c_345678" }, DiscourseSubscriptions::Stripe.request_opts)
|
|
.returns(data: [])
|
|
|
|
::Stripe::PaymentIntent
|
|
.expects(:list)
|
|
.with({ customer: "c_345678" }, DiscourseSubscriptions::Stripe.request_opts)
|
|
.returns(data: [{ id: "pi_900010", invoice: nil, created: Time.now }])
|
|
|
|
get "/s/user/payments.json"
|
|
|
|
parsed_body = response.parsed_body
|
|
|
|
expect(parsed_body.count).to eq(1)
|
|
end
|
|
|
|
it "gets pricing table one-off purchases that show up as related guest payments" do
|
|
SiteSetting.discourse_subscriptions_pricing_table_enabled = true
|
|
::Stripe::Invoice
|
|
.expects(:list)
|
|
.with({ customer: "c_345678" }, DiscourseSubscriptions::Stripe.request_opts)
|
|
.returns(data: [])
|
|
|
|
::Stripe::PaymentIntent
|
|
.expects(:list)
|
|
.with({ customer: "c_345678" }, DiscourseSubscriptions::Stripe.request_opts)
|
|
.returns(data: [])
|
|
|
|
::Stripe::Charge
|
|
.expects(:list)
|
|
.with(
|
|
{ limit: 100, starting_after: nil, expand: ["data.payment_intent"] },
|
|
DiscourseSubscriptions::Stripe.request_opts,
|
|
)
|
|
.returns(
|
|
data: [
|
|
{
|
|
id: "ch_1HtGz2GHcn71qeAp4YjA2oB4",
|
|
amount: 2000,
|
|
currency: "usd",
|
|
billing_details: {
|
|
email: user.email,
|
|
},
|
|
customer: nil, # guest payment
|
|
payment_intent: "pi_1HtGz1GHcn71qeApT9N2Cjln",
|
|
created: Time.now.to_i,
|
|
},
|
|
{
|
|
id: "ch_2HtGz2GHcn71qeAp4YjA2oB4",
|
|
amount: 2000,
|
|
currency: "usd",
|
|
billing_details: {
|
|
email: "zxcv@example.com",
|
|
},
|
|
customer: nil, # different guest
|
|
payment_intent: "pi_2HtGz1GHcn71qeApT9N2Cjln",
|
|
created: Time.now.to_i,
|
|
},
|
|
{
|
|
id: "ch_1HtGz3GHcn71qeAp5YjA2oC5",
|
|
amount: 3000,
|
|
currency: "usd",
|
|
billing_details: {
|
|
email: "fdsa@example.com",
|
|
},
|
|
customer: "cus_1234", # This is not a guest payment
|
|
payment_intent: "pi_3HtGz2GHcn71qeApT9N2Cjln",
|
|
created: Time.now.to_i,
|
|
},
|
|
],
|
|
)
|
|
|
|
get "/s/user/payments.json"
|
|
|
|
parsed_body = response.parsed_body
|
|
|
|
# Validate that only guest payments with the specified email are returned
|
|
expect(parsed_body.count).to eq(1)
|
|
expect(parsed_body.first["id"]).to eq("ch_1HtGz2GHcn71qeAp4YjA2oB4")
|
|
expect(parsed_body.first["customer"]).to be_nil
|
|
end
|
|
end
|
|
end
|