integrallis/stripe_event

Undefined method `deep_symbolize_keys`

MikeRogers0 opened this issue ยท 12 comments

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 :

NoMethodError:
       undefined method `deep_symbolize_keys' for #<ActionController::Parameters:0x007fe84b498780>

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:

$ bundle && appraisal && bundle exec rake
Using rake 10.5.0
Using concurrent-ruby 1.0.5
Using i18n 0.8.4
Using minitest 5.10.2
Using thread_safe 0.3.6
Using builder 3.2.3
Using erubi 1.6.0
Using mini_portile2 2.2.0
Using rack 2.0.3
Using nio4r 2.1.0
Using websocket-extensions 0.1.2
Using mime-types-data 3.2016.0521
Using arel 8.0.0
Using public_suffix 2.0.5
Using bundler 1.14.6
Using thor 0.19.4
Using json 2.1.0
Using docile 1.1.5
Using simplecov-html 0.10.1
Using tins 1.14.0
Using safe_yaml 1.0.4
Using diff-lcs 1.3
Using multipart-post 2.0.0
Using hashdiff 0.3.4
Using method_source 0.8.2
Using rspec-core 2.99.2
Using rspec-mocks 2.99.4
Using tzinfo 1.2.3
Using nokogiri 1.8.0
Using rack-test 0.6.3
Using sprockets 3.7.1
Using websocket-driver 0.6.5
Using mime-types 3.1
Using addressable 2.5.1
Using appraisal 2.2.0
Using simplecov 0.14.1
Using term-ansicolor 1.6.0
Using crack 0.4.3
Using rspec-expectations 2.99.2
Using faraday 0.12.1
Using activesupport 5.1.1
Using loofah 2.0.3
Using mail 2.6.6
Using coveralls 0.8.21
Using webmock 1.24.6
Using rspec-collection_matchers 1.1.3
Using stripe 2.12.0
Using rails-dom-testing 2.0.3
Using globalid 0.4.0
Using activemodel 5.1.1
Using rails-html-sanitizer 1.0.3
Using stripe_event 1.6.0 from source at `.`
Using activejob 5.1.1
Using activerecord 5.1.1
Using actionview 5.1.1
Using actionpack 5.1.1
Using actioncable 5.1.1
Using actionmailer 5.1.1
Using railties 5.1.1
Using sprockets-rails 3.2.0
Using rspec-rails 2.99.0
Using rails 5.1.1
Bundle complete! 7 Gemfile dependencies, 62 gems now installed.
Use `bundle show [gemname]` to see where a bundled gem is installed.
WARN: Unresolved specs during Gem::Specification.reset:
      rake (>= 0)
      thor (>= 0.14.0)
WARN: Clearing out unresolved specs.
Please report a bug if this causes problems.
Warning: Your gemfiles directive in .travis.yml is incorrect. Run `appraisal generate --travis` to get the correct configuration.
>> bundle check --gemfile='/Users/Mike/Workspace/Forks/stripe_event/gemfiles/rails3.2.gemfile' || bundle install --gemfile='/Users/Mike/Workspace/Forks/stripe_event/gemfiles/rails3.2.gemfile' --retry 1
The Gemfile's dependencies are satisfied
>> bundle check --gemfile='/Users/Mike/Workspace/Forks/stripe_event/gemfiles/rails4.1.gemfile' || bundle install --gemfile='/Users/Mike/Workspace/Forks/stripe_event/gemfiles/rails4.1.gemfile' --retry 1
The Gemfile's dependencies are satisfied
>> bundle check --gemfile='/Users/Mike/Workspace/Forks/stripe_event/gemfiles/rails4.2.gemfile' || bundle install --gemfile='/Users/Mike/Workspace/Forks/stripe_event/gemfiles/rails4.2.gemfile' --retry 1
The Gemfile's dependencies are satisfied
>> BUNDLE_GEMFILE=/Users/Mike/Workspace/Forks/stripe_event/gemfiles/rails3.2.gemfile bundle exec rake spec
/Users/Mike/.rbenv/versions/2.3.1/bin/ruby -S rspec ./spec/controllers/webhook_controller_spec.rb ./spec/lib/stripe_event_spec.rb
/Users/Mike/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/activesupport-3.2.22.5/lib/active_support/dependencies.rb:251:in `require': cannot load such file -- test/unit/assertions (LoadError)
	from /Users/Mike/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/activesupport-3.2.22.5/lib/active_support/dependencies.rb:251:in `block in require'
	from /Users/Mike/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/activesupport-3.2.22.5/lib/active_support/dependencies.rb:236:in `load_dependency'
	from /Users/Mike/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/activesupport-3.2.22.5/lib/active_support/dependencies.rb:251:in `require'
	from /Users/Mike/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/rspec-rails-2.99.0/lib/rspec/rails/adapters.rb:17:in `rescue in <module:Rails>'
	from /Users/Mike/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/rspec-rails-2.99.0/lib/rspec/rails/adapters.rb:12:in `<module:Rails>'
	from /Users/Mike/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/rspec-rails-2.99.0/lib/rspec/rails/adapters.rb:6:in `<module:RSpec>'
	from /Users/Mike/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/rspec-rails-2.99.0/lib/rspec/rails/adapters.rb:5:in `<top (required)>'
	from /Users/Mike/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/activesupport-3.2.22.5/lib/active_support/dependencies.rb:251:in `require'
	from /Users/Mike/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/activesupport-3.2.22.5/lib/active_support/dependencies.rb:251:in `block in require'
	from /Users/Mike/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/activesupport-3.2.22.5/lib/active_support/dependencies.rb:236:in `load_dependency'
	from /Users/Mike/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/activesupport-3.2.22.5/lib/active_support/dependencies.rb:251:in `require'
	from /Users/Mike/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/rspec-rails-2.99.0/lib/rspec/rails.rb:11:in `<top (required)>'
	from /Users/Mike/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/activesupport-3.2.22.5/lib/active_support/dependencies.rb:251:in `require'
	from /Users/Mike/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/activesupport-3.2.22.5/lib/active_support/dependencies.rb:251:in `block in require'
	from /Users/Mike/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/activesupport-3.2.22.5/lib/active_support/dependencies.rb:236:in `load_dependency'
	from /Users/Mike/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/activesupport-3.2.22.5/lib/active_support/dependencies.rb:251:in `require'
	from /Users/Mike/Workspace/Forks/stripe_event/spec/rails_helper.rb:3:in `<top (required)>'
	from /Users/Mike/Workspace/Forks/stripe_event/spec/controllers/webhook_controller_spec.rb:1:in `require'
	from /Users/Mike/Workspace/Forks/stripe_event/spec/controllers/webhook_controller_spec.rb:1:in `<top (required)>'
	from /Users/Mike/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/rspec-core-2.99.2/lib/rspec/core/configuration.rb:1065:in `load'
	from /Users/Mike/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/rspec-core-2.99.2/lib/rspec/core/configuration.rb:1065:in `block in load_spec_files'
	from /Users/Mike/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/rspec-core-2.99.2/lib/rspec/core/configuration.rb:1065:in `each'
	from /Users/Mike/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/rspec-core-2.99.2/lib/rspec/core/configuration.rb:1065:in `load_spec_files'
	from /Users/Mike/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/rspec-core-2.99.2/lib/rspec/core/command_line.rb:18:in `run'
	from /Users/Mike/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/rspec-core-2.99.2/lib/rspec/core/runner.rb:103:in `run'
	from /Users/Mike/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/rspec-core-2.99.2/lib/rspec/core/runner.rb:17:in `block in autorun'
/Users/Mike/.rbenv/versions/2.3.1/bin/ruby -S rspec ./spec/controllers/webhook_controller_spec.rb ./spec/lib/stripe_event_spec.rb failed

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!

rmm5t commented

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 commented

@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.

rmm5t commented

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

rmm5t commented

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

rmm5t commented

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