pay-rails / pay

Payments for Ruby on Rails apps
https://github.com/pay-rails/pay
MIT License
1.98k stars 316 forks source link

Enhance Strip checkout button using javascript_tag instead of script element #1046

Closed Mth0158 closed 3 months ago

Mth0158 commented 3 months ago

Pull Request

Summary: Using the setup mentioned in the GoRails video (here), @excid3 mentioned to use the pay/stripe/checkout_button partial.

This partial uses an inline script which will cause issues when your Rails app uses a Content-Security-Policy (config/initializers/content_security_policy.rb) that forbids inline_script, which should always be the case.

Using Rails helper javascript_tag, along nonce: true instead of a raw <script> will smoothly integrate for those using the Rails CSP feature.

Obviously, I could generate the views using bin/rails generate pay:views and do this change if this PR is not accepted.

Related Issue: N/A

Description: I am using a CSP on all my apps to prevent several security breaches, notably XSS attacks.

Testing: I have used the <%= javascript_tag nonce: true do %> ... <% end %> syntax with and without a CSP. It works in both cases, not having a CSP will not raise because of the nonce: true that will be ignored.

Screenshots (if applicable): Using the actual pay/stripe/checkout_button partial with a CSP activated:

image

CSP example (Rails default):

Rails.application.configure do
  config.content_security_policy do |policy|
    policy.default_src :self, :https
    policy.font_src    :self, :https, :data
    policy.img_src     :self, :https, :data
    policy.object_src  :none
    policy.script_src  :self, :https
    policy.style_src   :self, :https
    # Specify URI for violation reports
    # policy.report_uri "/csp-violation-report-endpoint"
  end

  # Generate session nonces for permitted importmap, inline scripts, and inline styles.
  config.content_security_policy_nonce_generator = ->(request) { request.session.id.to_s }
  config.content_security_policy_nonce_directives = %w[script-src style-src]

  # Report violations without enforcing the policy.
  # config.content_security_policy_report_only = true
end

Checklist:

Additional Notes:

  1. Other inline scripts occurences @excid3 I searched for <script occurrences in the repo, I think this change could be also applied to the pay/payments/show view. But I do not know in which scenario this partial is used (or if it's used at all?).

  2. Documentation This is not related to this PR, but maybe mentioning in the Stripe documentation what you showed in the GoRails video (ie the astonishing simplicity of setting up a Stripe checkout) could be a nice addition?

excid3 commented 3 months ago

There's really no need for this partial anymore. You can create a checkout session and link directly to it's URL which is easier.

<%= link_to "Checkout", @checkout_session.url %>

Let's just drop this feature in the next release instead.

excid3 commented 3 months ago

Note to self, record an updated video for Pay v8.

Mth0158 commented 3 months ago

@excid3 even better! I will update my code with your answer.

excid3 commented 3 months ago

I think I removed it previously from here: https://github.com/pay-rails/pay/blob/main/docs/stripe/8_stripe_checkout.md

Docs could use a full-rewrite soon also. 😬