Closed MikeRogers0 closed 6 years ago
This is causing a deprecation warning on a 5.0 rails app:
DEPRECATION WARNING: Method deep_symbolize_keys is deprecated and will be removed in Rails 5.1, as
ActionController::Parameters
no longer inherits from hash. Using this deprecated behavior exposes potential security problems. If you continue to use this method you may be creating a security vulnerability in your app that can be exploited. Instead, consider using one of these documented methods which are not deprecated: http://api.rubyonrails.org/v5.0.4/classes/ActionController/Parameters.html
I guess a 5.1 app would error out entirely. We'd appreciate a resolution of this problem so we can start looking into upgrading to 5.1.
Thanks!
Ref #84
I'm running into this same problem. As a workaround I've modified my event_retriever
block to convert param
to a hash for the "account.application.deauthorized"
event:
StripeEvent.configure do |events|
events.event_retriever = lambda do |params|
if params[:type] == "account.application.deauthorized"
event = Stripe::Event.construct_from(params.permit!.to_hash.deep_symbolize_keys)
else
Stripe::Event.retrieve(params[:id])
end
end
end
This obviously isn't ideal because it duplicates some of the functionality StripeEvents
attempts to abstract.
I attempted to fork / fix but also ran into the same issues that @MikeRogers0 did trying to run the test suite and finally gave up. I think there may be more general issues related to Rails 5.
@rmm5t just curious, why was this issue closed? It seems to be something that should be fixed π I'm happy to help, but would want to know a fix (with tests) would be accepted before I attempt to get my development environment working.
just curious, why was this issue closed?
I'm not sure either, this issue is still relevant for Rails 5.1
@rmm5t just curious, why was this issue closed?
I don't know; I don't have the context of this problem in my head at the moment, and I didn't close it. I merely referenced another related issue to help with triage.
Re-opening, because it sounds like it's warranted, but I'm a little swamped at the moment to investigate any of this right now. PRs welcome.
Perhaps the best solution is to only support signed webhooks, because then the event can be safely constructed from the POST data.
This eliminates the need for an additional API call to Stripe to fetch the same event data. Not only is that extra API call wasteful, but it becomes complex when handling both normal Stripe and Stripe Connect events.
This should work for all event types (including Connect events), and will verify the signature:
Stripe::Webhook.construct_event(request.body.read, request.headers['Stripe-Signature'], StripeEvent.signing_secret)
StripeEvent could safely construct the event, which makes the event_retriever
unnecessary. Or, it could be renamed to event_preprocessor
and passed the constructed Stripe::Event
. I think this would greatly simplify things, and would require developers to secure their endpoints β which is never a bad thing to encourage.
(BTW β I tried forking this repo in the past, but ran into a ton of issues trying to get the development environment setup & specs running, and eventually gave up π I think that might be a good place for someone to start!)
I totally like your suggestions @kylefox! Do you mind opening a new issue about that? I actually created a short-term fix for this in #94, which also ensures that tests are passing under newest Rails versions, so it should be easier to work on any further improvements
tried forking this repo in the past, but ran into a ton of issues trying to get the development environment setup & specs running, and eventually gave up π I think that might be a good place for someone to start!
That's likely a problem with the appraisal tasks (I'm not a fan of that library, but it's currently assigned as the default rake task, and it doesn't play nicely with all development environments). It's sometimes easier to just run bundle exec rake spec
(and avoid the appraisal gem and associated tasks).
I didn't have any problem using appraisal today though. The bigger issues were outdated RSpec and broken Rails 3.2 build
I didn't have any problem using appraisal today though.
That might be because it appears that you use gemsets (a somewhat messy/outdated means of controlling a gem cache -- bundler has better ways of handling this per project).
Nonetheless, nearly the entirety of appraisal
can be replaced with 5 lines of shell scripting and without conflating the bundler and ruby runtimes. Here's a script I use for that exact purpose across projects:
https://github.com/rmm5t/tilda-bin/blob/master/gemfile-exec
@krasnoukhov Opened #95: Only support signed webhooks.
I'm using stripe_event on a new Rails 5 project, while listening to the webhook "account.application.deauthorized" in my tests & in my event application logs I was noticing the error :
This was being thrown on lib/stripe_event.rb around line 20.
I wrote a fix ( https://github.com/MikeRogers0/stripe_event/tree/bugfix/undefined_method_deep_symbolize_keys - I did open a PR, but the CONTRIBUTING.md suggests you'd prefer an Issue before I open the PR ) which converts the ActionController::Parameters into a hash .
However, I was unable to get the stripe_events specs running my local machine (When I ran
bundle exec rake spec
I kept running into "cannot load such file" errors). I've annotated in the specs where I believe they need to be corrected.I'm happy to finish writing the specs if someone can't point me in the right direction on how to set them up.
When running
bundle && appraisal && bundle exec rake
, I was receiving the output: