Skip to content

add Stripe to Teleport CSP#25814

Merged
michellescripts merged 2 commits intomasterfrom
michelle/stripe-csp
May 9, 2023
Merged

add Stripe to Teleport CSP#25814
michellescripts merged 2 commits intomasterfrom
michelle/stripe-csp

Conversation

@michellescripts
Copy link
Copy Markdown
Contributor

@michellescripts michellescripts commented May 8, 2023

Essentially reverts #15891

The Cloud Team plan will leverage Stripe for billing pages/managing billing information.

image

supports https://github.com/gravitational/cloud/issues/3536

@michellescripts michellescripts marked this pull request as ready for review May 8, 2023 16:42
@michellescripts michellescripts requested a review from jentfoo May 8, 2023 16:42
// additional default restrictions
"object-src 'none'",
// auto-pay plans in Cloud use stripe.com to manage billing information
"script-src 'self' https://js.stripe.com",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this need to be applied globally to the DefaultContentSecurityPolicy? Instead could we just append to this policy when a page specifically needs to allow stripe code?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(With a few exceptions) the web UI is a single page app - the "pages" that use stripe are the same as any other part of the web UI, right?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If Stripe is being included as part of the single page app I understand this may make sense. But it sounded like this is new development, and if so, instead we should isolate the Stripe interaction on a different page. We would like to avoid providing stripe this level of trust across our entire application.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We also display an upgrade banner on all pages via Main, and the banner has a CTA with the stripe payment element.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given the above context it sounds like it does need to be included in the default CSP. But similar to this issue we should try to see if we can improve this. We want to be mindful when introducing these integrations to make sure their scope is as limited as possible.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@michellescripts @mcbattirola since it's a golang server side file, can we do:

if plan == "team" {
   headers["script-src 'self'] = "https://js.stripe.com"
}

This change alone will massively reduce the blast radius from any OSS or Enterprise customer

@michellescripts
Copy link
Copy Markdown
Contributor Author

cc @jimbishopp @klizhentas

// additional default restrictions
"object-src 'none'",
// auto-pay plans in Cloud use stripe.com to manage billing information
"script-src 'self' https://js.stripe.com",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given the above context it sounds like it does need to be included in the default CSP. But similar to this issue we should try to see if we can improve this. We want to be mindful when introducing these integrations to make sure their scope is as limited as possible.

@michellescripts michellescripts enabled auto-merge May 9, 2023 15:26
@michellescripts michellescripts added this pull request to the merge queue May 9, 2023
Merged via the queue into master with commit dd8280f May 9, 2023
@michellescripts michellescripts deleted the michelle/stripe-csp branch May 9, 2023 16:31
michellescripts added a commit that referenced this pull request May 9, 2023
michellescripts added a commit that referenced this pull request May 9, 2023
* add billing RBAC to editor role (#25713)

- for new team billing pages

* add Stripe to Teleport CSP (#25814)
michellescripts added a commit that referenced this pull request May 9, 2023
michellescripts added a commit that referenced this pull request May 9, 2023
* add billing RBAC to editor role (#25713)

- for new team billing pages

* add Stripe to Teleport CSP (#25814)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants