mirror of
https://gh.wpcy.net/https://github.com/discourse/discourse.git
synced 2026-05-21 00:18:33 +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
202 lines
6.2 KiB
Ruby
Vendored
202 lines
6.2 KiB
Ruby
Vendored
# frozen_string_literal: true
|
|
|
|
RSpec.describe DiscourseSubscriptions::Admin::PlansController do
|
|
before { SiteSetting.discourse_subscriptions_enabled = true }
|
|
|
|
it "is a subclass of AdminController" do
|
|
expect(DiscourseSubscriptions::Admin::PlansController < ::Admin::AdminController).to eq(true)
|
|
end
|
|
|
|
context "when not authenticated" do
|
|
describe "index" do
|
|
it "does not get the plans" do
|
|
::Stripe::Price.expects(:list).never
|
|
get "/s/admin/plans.json"
|
|
end
|
|
|
|
it "not ok" do
|
|
get "/s/admin/plans.json"
|
|
expect(response.status).to eq 404
|
|
end
|
|
end
|
|
|
|
describe "create" do
|
|
it "does not create a plan" do
|
|
::Stripe::Price.expects(:create).never
|
|
post "/s/admin/plans.json", params: { name: "Rick Astley", amount: 1, interval: "week" }
|
|
end
|
|
|
|
it "is not ok" do
|
|
post "/s/admin/plans.json", params: { name: "Rick Astley", amount: 1, interval: "week" }
|
|
expect(response.status).to eq 404
|
|
end
|
|
end
|
|
|
|
describe "show" do
|
|
it "does not show the plan" do
|
|
::Stripe::Price.expects(:retrieve).never
|
|
get "/s/admin/plans/plan_12345.json"
|
|
end
|
|
|
|
it "is not ok" do
|
|
get "/s/admin/plans/plan_12345.json"
|
|
expect(response.status).to eq 404
|
|
end
|
|
end
|
|
|
|
describe "update" do
|
|
it "does not update a plan" do
|
|
::Stripe::Price.expects(:update).never
|
|
delete "/s/admin/plans/plan_12345.json"
|
|
end
|
|
end
|
|
end
|
|
|
|
context "when authenticated" do
|
|
let(:admin) { Fabricate(:admin) }
|
|
|
|
before do
|
|
SiteSetting.discourse_subscriptions_secret_key = "secret-key"
|
|
sign_in(admin)
|
|
end
|
|
|
|
describe "index" do
|
|
it "lists the plans" do
|
|
::Stripe::Price.expects(:list).with(nil, DiscourseSubscriptions::Stripe.request_opts)
|
|
get "/s/admin/plans.json"
|
|
end
|
|
|
|
it "lists the plans for the product" do
|
|
::Stripe::Price.expects(:list).with(
|
|
{ product: "prod_id123" },
|
|
DiscourseSubscriptions::Stripe.request_opts,
|
|
)
|
|
get "/s/admin/plans.json", params: { product_id: "prod_id123" }
|
|
end
|
|
end
|
|
|
|
describe "show" do
|
|
it "shows a plan" do
|
|
::Stripe::Price
|
|
.expects(:retrieve)
|
|
.with("plan_12345", DiscourseSubscriptions::Stripe.request_opts)
|
|
.returns(currency: "aud")
|
|
get "/s/admin/plans/plan_12345.json"
|
|
expect(response.status).to eq 200
|
|
end
|
|
|
|
it "upcases the currency" do
|
|
::Stripe::Price
|
|
.expects(:retrieve)
|
|
.with("plan_12345", DiscourseSubscriptions::Stripe.request_opts)
|
|
.returns(currency: "aud", recurring: { interval: "year" })
|
|
get "/s/admin/plans/plan_12345.json"
|
|
|
|
plan = response.parsed_body
|
|
expect(plan["currency"]).to eq "AUD"
|
|
expect(plan["interval"]).to eq "year"
|
|
end
|
|
end
|
|
|
|
describe "create" do
|
|
it "creates a plan with a nickname" do
|
|
::Stripe::Price.expects(:create).with(
|
|
has_entry(:nickname, "Veg"),
|
|
DiscourseSubscriptions::Stripe.request_opts,
|
|
)
|
|
post "/s/admin/plans.json", params: { nickname: "Veg", metadata: { group_name: "" } }
|
|
end
|
|
|
|
it "creates a plan with a currency" do
|
|
::Stripe::Price.expects(:create).with(
|
|
has_entry(:currency, "AUD"),
|
|
DiscourseSubscriptions::Stripe.request_opts,
|
|
)
|
|
post "/s/admin/plans.json", params: { currency: "AUD", metadata: { group_name: "" } }
|
|
end
|
|
|
|
it "creates a plan with an interval" do
|
|
::Stripe::Price.expects(:create).with(
|
|
has_entry(recurring: { interval: "week" }),
|
|
DiscourseSubscriptions::Stripe.request_opts,
|
|
)
|
|
post "/s/admin/plans.json",
|
|
params: {
|
|
type: "recurring",
|
|
interval: "week",
|
|
metadata: {
|
|
group_name: "",
|
|
},
|
|
}
|
|
end
|
|
|
|
it "creates a plan as a one-time purchase" do
|
|
::Stripe::Price.expects(:create).with(
|
|
Not(has_key(:recurring)),
|
|
DiscourseSubscriptions::Stripe.request_opts,
|
|
)
|
|
post "/s/admin/plans.json", params: { metadata: { group_name: "" } }
|
|
end
|
|
|
|
it "creates a plan with an amount" do
|
|
::Stripe::Price.expects(:create).with(
|
|
has_entry(:unit_amount, "102"),
|
|
DiscourseSubscriptions::Stripe.request_opts,
|
|
)
|
|
post "/s/admin/plans.json", params: { amount: "102", metadata: { group_name: "" } }
|
|
end
|
|
|
|
it "creates a plan with a product" do
|
|
::Stripe::Price.expects(:create).with(
|
|
has_entry(product: "prod_walterwhite"),
|
|
DiscourseSubscriptions::Stripe.request_opts,
|
|
)
|
|
post "/s/admin/plans.json",
|
|
params: {
|
|
product: "prod_walterwhite",
|
|
metadata: {
|
|
group_name: "",
|
|
},
|
|
}
|
|
end
|
|
|
|
it "creates a plan with an active status" do
|
|
::Stripe::Price.expects(:create).with(
|
|
has_entry(:active, "false"),
|
|
DiscourseSubscriptions::Stripe.request_opts,
|
|
)
|
|
post "/s/admin/plans.json", params: { active: "false", metadata: { group_name: "" } }
|
|
end
|
|
|
|
# TODO: Need to fix the metadata tests
|
|
# I think mocha has issues with the metadata fields here.
|
|
|
|
#it 'has metadata' do
|
|
# ::Stripe::Price.expects(:create).with(has_entry(:group_name, "discourse-user-group-name"))
|
|
# post "/s/admin/plans.json", params: { amount: "100", metadata: { group_name: 'discourse-user-group-name' } }
|
|
#end
|
|
|
|
#it "creates a plan with a trial period" do
|
|
# ::Stripe::Price.expects(:create).with(has_entry(trial_period_days: '14'))
|
|
# post "/s/admin/plans.json", params: { trial_period_days: '14' }
|
|
#end
|
|
end
|
|
|
|
describe "update" do
|
|
it "updates a plan" do
|
|
::Stripe::Price.expects(:update).with(
|
|
"plan_12345",
|
|
anything,
|
|
DiscourseSubscriptions::Stripe.request_opts,
|
|
)
|
|
patch "/s/admin/plans/plan_12345.json",
|
|
params: {
|
|
trial_period_days: "14",
|
|
metadata: {
|
|
group_name: "discourse-user-group-name",
|
|
},
|
|
}
|
|
end
|
|
end
|
|
end
|
|
end
|