search

stripe-samples / subscription-use-cases

Create subscriptions with fixed prices or usage based billing.
https://stripe.com/docs/billing
MIT License
797 stars 400 forks source link

clientSecret should not be passed as a queryPararm? #179

Closed MosesMendoza closed 2 years ago

MosesMendoza commented 2 years ago

Bug report

Describe the bug

Hi there. Working on an integration when I came across the code in the frontend vanillajs example that passes the client secret between pages: https://github.com/stripe-samples/subscription-use-cases/blob/master/fixed-price-subscriptions/client/vanillajs/prices.js#L53-L55.

I know this repo is just for example and not expected to be a highly secure configuration. But in the tutorials online the handling of client secret was fairly opaque to me. For example in https://stripe.com/docs/billing/subscriptions/build-subscription, I inferred I should be accept the POST response from the Subscription.create call as JSON and then do some in-page DOM modifications to show the new payment element created w/the client secret, but I wasn't sure. Then when I found this github repo I was like "Voila! They just make a GET request and pass the clientSecret as a query param! Then the new subscribe.html page has its own js script lifecycle, and sets up the payment element - so easy! Client secret must not be that secret after all!"

Until I came across the API docs for payment element, https://stripe.com/docs/payments/payment-intents#passing-to-client, which explicitly state

Don’t log it, embed it in URLs, or expose it to anyone other than the customer.

Like I said, I understand the example repo says its not supposed to be secure, but shouldn't it follow the major expectations set by the API docs? This seems like a fairly substantial code flow difference. Passing the client secret as a query param is embedding it in the URL, to get logged, cached, etc. Or am I confused here (admittedly very likely)?

Anyway, I figured out how I'm going to proceed personally, but thought some attention might be given to this issue. Thanks!

hideokamoto-stripe commented 2 years ago

Thanks for your feedback! How do you think that using the window.sessionStorage instead of the URL querystring?

Thanks!

hideokamoto-stripe commented 2 years ago

We've removed the queryParam from the example. PR: #220

Thanks for the feedback :)