discourse/plugins/discourse-subscriptions/spec/requests/admin/plans_controller_spec.rb
Alan Guo Xiang Tan c34f2aac8d SECURITY: Fixes for discourse-subscriptions
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
2026-03-31 15:12:45 +01:00

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