mirror of
https://github.com/discourse/discourse.git
synced 2026-08-06 13:08:40 +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
229 lines
7.9 KiB
Ruby
Vendored
229 lines
7.9 KiB
Ruby
Vendored
# frozen_string_literal: true
|
|
|
|
describe DiscourseSubscriptions::Campaign do
|
|
describe "campaign data is refreshed" do
|
|
let(:user) { Fabricate(:user) }
|
|
let(:user2) { Fabricate(:user) }
|
|
let(:subscription) do
|
|
{
|
|
id: "sub_1234",
|
|
items: {
|
|
data: [
|
|
{
|
|
price: {
|
|
product: "prodct_23456",
|
|
unit_amount: 1000,
|
|
recurring: {
|
|
interval: "month",
|
|
},
|
|
},
|
|
},
|
|
],
|
|
},
|
|
}
|
|
end
|
|
let(:invoice) do
|
|
{
|
|
id: "in_1234",
|
|
paid: true,
|
|
lines: {
|
|
data: [
|
|
{
|
|
plan: nil,
|
|
price: {
|
|
product: "prodct_65432",
|
|
active: true,
|
|
unit_amount: 1000,
|
|
recurring: nil,
|
|
},
|
|
},
|
|
],
|
|
},
|
|
}
|
|
end
|
|
let(:invoice2) do
|
|
{
|
|
id: "in_1235",
|
|
paid: false,
|
|
lines: {
|
|
data: [
|
|
{
|
|
plan: nil,
|
|
price: {
|
|
product: "prodct_65433",
|
|
active: true,
|
|
unit_amount: 600,
|
|
recurring: nil,
|
|
},
|
|
},
|
|
],
|
|
},
|
|
}
|
|
end
|
|
let(:invoice_with_nil_lines) { { id: "in_1236", paid: true, lines: nil } }
|
|
let(:invoice_with_nil_price) do
|
|
{ id: "in_1237", paid: true, lines: { data: [{ plan: nil, price: nil }] } }
|
|
end
|
|
|
|
before do
|
|
Fabricate(:product, external_id: "prodct_23456")
|
|
Fabricate(:customer, product_id: "prodct_23456", user_id: user.id, customer_id: "x")
|
|
Fabricate(:product, external_id: "prodct_65432")
|
|
Fabricate(:customer, product_id: "prodct_65432", user_id: user2.id, customer_id: "y")
|
|
Fabricate(:product, external_id: "prodct_65433")
|
|
Fabricate(:customer, product_id: "prodct_65433", user_id: user2.id, customer_id: "y")
|
|
SiteSetting.discourse_subscriptions_public_key = "public-key"
|
|
SiteSetting.discourse_subscriptions_secret_key = "secret-key"
|
|
end
|
|
|
|
describe "refresh_data" do
|
|
context "for all subscription purchases" do
|
|
it "refreshes the campaign data properly" do
|
|
::Stripe::Subscription
|
|
.expects(:list)
|
|
.with(anything, DiscourseSubscriptions::Stripe.request_opts)
|
|
.returns(data: [subscription], has_more: false)
|
|
::Stripe::Invoice
|
|
.expects(:list)
|
|
.with(anything, DiscourseSubscriptions::Stripe.request_opts)
|
|
.returns(data: [invoice, invoice2], has_more: false)
|
|
|
|
DiscourseSubscriptions::Campaign.new.refresh_data
|
|
|
|
expect(SiteSetting.discourse_subscriptions_campaign_subscribers).to eq 1
|
|
expect(SiteSetting.discourse_subscriptions_campaign_amount_raised).to eq 20.00
|
|
end
|
|
|
|
it "checks if the goal is completed or not" do
|
|
SiteSetting.discourse_subscriptions_campaign_goal = 5
|
|
::Stripe::Subscription
|
|
.expects(:list)
|
|
.with(anything, DiscourseSubscriptions::Stripe.request_opts)
|
|
.returns(data: [subscription], has_more: false)
|
|
::Stripe::Invoice
|
|
.expects(:list)
|
|
.with(anything, DiscourseSubscriptions::Stripe.request_opts)
|
|
.returns(data: [invoice], has_more: false)
|
|
|
|
DiscourseSubscriptions::Campaign.new.refresh_data
|
|
expect(Discourse.redis.get("subscriptions_goal_met_date")).to be_present
|
|
end
|
|
|
|
it "checks if goal is < 90% met after being met" do
|
|
SiteSetting.discourse_subscriptions_campaign_goal = 25
|
|
Discourse.redis.set("subscriptions_goal_met_date", 10.days.ago)
|
|
::Stripe::Subscription
|
|
.expects(:list)
|
|
.with(anything, DiscourseSubscriptions::Stripe.request_opts)
|
|
.returns(data: [subscription], has_more: false)
|
|
::Stripe::Invoice
|
|
.expects(:list)
|
|
.with(anything, DiscourseSubscriptions::Stripe.request_opts)
|
|
.returns(data: [invoice], has_more: false)
|
|
|
|
DiscourseSubscriptions::Campaign.new.refresh_data
|
|
expect(Discourse.redis.get("subscriptions_goal_met_date")).to be_blank
|
|
end
|
|
|
|
it "handles invoices with nil lines gracefully" do
|
|
::Stripe::Subscription
|
|
.expects(:list)
|
|
.with(anything, DiscourseSubscriptions::Stripe.request_opts)
|
|
.returns(data: [subscription], has_more: false)
|
|
::Stripe::Invoice
|
|
.expects(:list)
|
|
.with(anything, DiscourseSubscriptions::Stripe.request_opts)
|
|
.returns(data: [invoice_with_nil_lines], has_more: false)
|
|
|
|
expect { DiscourseSubscriptions::Campaign.new.refresh_data }.not_to raise_error
|
|
end
|
|
|
|
it "handles invoices with nil price gracefully" do
|
|
::Stripe::Subscription
|
|
.expects(:list)
|
|
.with(anything, DiscourseSubscriptions::Stripe.request_opts)
|
|
.returns(data: [subscription], has_more: false)
|
|
::Stripe::Invoice
|
|
.expects(:list)
|
|
.with(anything, DiscourseSubscriptions::Stripe.request_opts)
|
|
.returns(data: [invoice_with_nil_price], has_more: false)
|
|
|
|
expect { DiscourseSubscriptions::Campaign.new.refresh_data }.not_to raise_error
|
|
end
|
|
end
|
|
|
|
context "with a campaign product set" do
|
|
let(:user2) { Fabricate(:user) }
|
|
let(:campaign_subscription) do
|
|
{
|
|
id: "sub_5678",
|
|
items: {
|
|
data: [
|
|
{
|
|
price: {
|
|
product: "prod_use",
|
|
unit_amount: 10_000,
|
|
recurring: {
|
|
interval: "year",
|
|
},
|
|
},
|
|
},
|
|
],
|
|
},
|
|
}
|
|
end
|
|
|
|
before do
|
|
Fabricate(:product, external_id: "prod_use")
|
|
Fabricate(:customer, product_id: "prod_use", user_id: user2.id, customer_id: "y")
|
|
SiteSetting.discourse_subscriptions_campaign_product = "prod_use"
|
|
end
|
|
|
|
it "refreshes campaign data with only the campaign product/subscriptions" do
|
|
::Stripe::Subscription
|
|
.expects(:list)
|
|
.with(anything, DiscourseSubscriptions::Stripe.request_opts)
|
|
.returns(data: [subscription, campaign_subscription], has_more: false)
|
|
::Stripe::Invoice
|
|
.expects(:list)
|
|
.with(anything, DiscourseSubscriptions::Stripe.request_opts)
|
|
.returns(data: [invoice], has_more: false)
|
|
|
|
DiscourseSubscriptions::Campaign.new.refresh_data
|
|
|
|
expect(SiteSetting.discourse_subscriptions_campaign_subscribers).to eq 1
|
|
expect(SiteSetting.discourse_subscriptions_campaign_amount_raised).to eq 8.33
|
|
end
|
|
end
|
|
end
|
|
end
|
|
|
|
describe "campaign is automatically created" do
|
|
before { SiteSetting.discourse_subscriptions_secret_key = "secret-key" }
|
|
|
|
describe "create_campaign" do
|
|
it "successfully creates the campaign group, product, and prices" do
|
|
::Stripe::Product
|
|
.expects(:create)
|
|
.with(anything, DiscourseSubscriptions::Stripe.request_opts)
|
|
.returns(id: "prod_campaign")
|
|
::Stripe::Price
|
|
.expects(:create)
|
|
.with(anything, DiscourseSubscriptions::Stripe.request_opts)
|
|
.times(6)
|
|
|
|
DiscourseSubscriptions::Campaign.new.create_campaign
|
|
|
|
group = Group.find_by(name: "campaign_supporters")
|
|
|
|
expect(group[:full_name]).to eq "Supporters"
|
|
expect(SiteSetting.discourse_subscriptions_campaign_group.to_i).to eq group.id
|
|
|
|
expect(DiscourseSubscriptions::Product.where(external_id: "prod_campaign").length).to eq 1
|
|
|
|
expect(SiteSetting.discourse_subscriptions_campaign_enabled).to eq true
|
|
expect(SiteSetting.discourse_subscriptions_campaign_product).to eq "prod_campaign"
|
|
end
|
|
end
|
|
end
|
|
end
|