slack-ruby/slack-ruby-client

`Slack::Events::Request#verify!` not working with Rack 3

stevenou opened this issue · 11 comments

stevenou commented

The body used in Slack::Events::Request#verify! is now an empty string, which is causing the signature verification to fail.

I am not 100% sure, but I think this started happening when I updated rack. I went from 2.2.8 to 3.0.8. I also upgraded a ton of other gems at the same time, so it could be something else, but rack feels like the most relevant to this issue.

I do still get the payload in the Rails params, so it's not like the body/payload actually disappeared. So while I'm not sure if there is a bug upstream or not, it seems like either way this can be worked around by getting the body/payload some other way?

stevenou commented

FYI I am currently monkeypatching it like so:

@body ||= begin
  "payload=#{CGI.escape(http_request.params[:payload])}"
end
@body

Seems to get the job done for now...

dblock commented

A simple repro or an integration test would be nice. The body is implemented here, so not sure what has changed in newer versions of Rack.

stevenou commented

Digging around a bit more, it seems like the test case for this is based on the URL verification code (https://api.slack.com/apis/connections/events-api#handshake) which uses application/json.

I am using Slack::Events::Request with block actions (https://api.slack.com/interactivity/handling#payloads) which seems to POST with application/x-www-form-urlencoded instead, with the body containing a payload parameter. So my monkey patch is very specific to using it with block actions and does not work when using it with the Events API.

As for repro/integration test. I'm not sure how to do this outside of Rails, so I created a new Rails app and wrote a test for a TestController that uses Slack::Events::Request. https://github.com/InvisibleCommerce/slack-ruby-client-test . Test at https://github.com/InvisibleCommerce/slack-ruby-client-test/blob/main/test/controllers/test_controller_spec.rb

It is written for the block actions application/x-www-form-urlencoded request rather than the Events API application/json request. It may be that the Events API version is not broken at all. I can confirm that if you downgrade rack to v2, the test passes. If you use rack 3.0.8, the test breaks. You can see if you inspect the body, that the body is empty when using rack 3.0.8.

Hope this helps

dblock commented

First, good job narrowing it down to Rack 2 vs. 3. I am not clear on whether the change is related to the type of request being made. Basically, does the events API still work?

But I think we can fix this in all cases. Rename https://github.com/slack-ruby/slack-ruby-client/blob/master/spec/slack/events/request_spec.rb into events_api_request_spec.rb and add interactions_request_spec.rb that uses input as described in https://api.slack.com/interactivity/handling#payloads. Then we need some if's to avoid the monkey patch, proably checking whether request params has payload first.

dblock commented

I can reproduce this on one of my bots, https://github.com/dblock/slack-sup2.

[slack-sup2] [2024-01-31 15:38:02] E, [2024-01-31T15:38:02.961353 #1] ERROR -- : Slack::Events::Request::InvalidSignature: Slack::Events::Request::InvalidSignature
[slack-sup2] [2024-01-31 15:38:02]   /layers/heroku_ruby/gems/vendor/bundle/ruby/2.7.0/gems/slack-ruby-client-0.14.6/lib/slack/events/request.rb:60:in `verify!'
[slack-sup2] [2024-01-31 15:38:02]   /workspace/lib/api/endpoints/slack_endpoint.rb:12:in `block (2 levels) in <class:SlackEndpoint>'
[slack-sup2] [2024-01-31 15:38:02]   /layers/heroku_ruby/gems/vendor/bundle/ruby/2.7.0/gems/grape-1.8.0/lib/grape/endpoint.rb:368:in `instance_eval'
[slack-sup2] [2024-01-31 15:38:02]   /layers/heroku_ruby/gems/vendor/bundle/ruby/2.7.0/gems/grape-1.8.0/lib/grape/endpoint.rb:368:in `block (2 levels) in run_filters'
[slack-sup2] [2024-01-31 15:38:02]   /layers/heroku_ruby/gems/vendor/bundle/ruby/2.7.0/gems/grape-1.8.0/lib/grape/endpoint.rb:368:in `each'
[slack-sup2] [2024-01-31 15:38:02]   /layers/heroku_ruby/gems/vendor/bundle/ruby/2.7.0/gems/grape-1.8.0/lib/grape/endpoint.rb:368:in `block in run_filters'
[slack-sup2] [2024-01-31 15:38:02]   /layers/heroku_ruby/gems/vendor/bundle/ruby/2.7.0/gems/activesupport-6.1.7.2/lib/active_support/notifications.rb:205:in `instrument'
[slack-sup2] [2024-01-31 15:38:02]   /layers/heroku_ruby/gems/vendor/bundle/ruby/2.7.0/gems/grape-1.8.0/lib/grape/endpoint.rb:367:in `run_filters'
[slack-sup2] [2024-01-31 15:38:02]   /layers/heroku_ruby/gems/vendor/bundle/ruby/2.7.0/gems/grape-1.8.0/lib/grape/endpoint.rb:248:in `block in run'
[slack-sup2] [2024-01-31 15:38:02]   /layers/heroku_ruby/gems/vendor/bundle/ruby/2.7.0/gems/activesupport-6.1.7.2/lib/active_support/notifications.rb:205:in `instrument'
[slack-sup2] [2024-01-31 15:38:02]   /layers/heroku_ruby/gems/vendor/bundle/ruby/2.7.0/gems/grape-1.8.0/lib/grape/endpoint.rb:240:in `run'
[slack-sup2] [2024-01-31 15:38:02]   /layers/heroku_ruby/gems/vendor/bundle/ruby/2.7.0/gems/grape-1.8.0/lib/grape/endpoint.rb:316:in `block in build_stack'
[slack-sup2] [2024-01-31 15:38:02]   /layers/heroku_ruby/gems/vendor/bundle/ruby/2.7.0/gems/newrelic_rpm-6.10.0.364/lib/new_relic/agent/instrumentation/middleware_tracing.rb:99:in `call'
[slack-sup2] [2024-01-31 15:38:02]   /layers/heroku_ruby/gems/vendor/bundle/ruby/2.7.0/gems/grape-1.8.0/lib/grape/middleware/base.rb:36:in `call!'
[slack-sup2] [2024-01-31 15:38:02]   /layers/heroku_ruby/gems/vendor/bundle/ruby/2.7.0/gems/grape-1.8.0/lib/grape/middleware/base.rb:29:in `call'
[slack-sup2] [2024-01-31 15:38:02]   /layers/heroku_ruby/gems/vendor/bundle/ruby/2.7.0/gems/newrelic_rpm-6.10.0.364/lib/new_relic/agent/instrumentation/middleware_tracing.rb:99:in `call'
[slack-sup2] [2024-01-31 15:38:02]   /layers/heroku_ruby/gems/vendor/bundle/ruby/2.7.0/gems/grape-1.8.0/lib/grape/middleware/error.rb:39:in `block in call!'
[slack-sup2] [2024-01-31 15:38:02]   /layers/heroku_ruby/gems/vendor/bundle/ruby/2.7.0/gems/grape-1.8.0/lib/grape/middleware/error.rb:38:in `catch'
[slack-sup2] [2024-01-31 15:38:02]   /layers/heroku_ruby/gems/vendor/bundle/ruby/2.7.0/gems/grape-1.8.
dblock commented

In both Rack 2.x and 3.x body is defined as a shortcut to get_header(RACK_INPUT), but in 3.x it looks like it doesn't call rewind. So the correct fix is either in Rack (rack/rack#2152) or we have to first call .rewind in slack-ruby-client instead of trying to re-create a body from payload (could be missing other content POSTed).

stevenou commented

In both Rack 2.x and 3.x body is defined as a shortcut to get_header(RACK_INPUT), but in 3.x it looks like it doesn't call rewind. So the correct fix is either in Rack (rack/rack#2152) or we have to first call .rewind in slack-ruby-client instead of trying to re-create a body from payload (could be missing other content POSTed).

Good find! Agreed, recreating the body from params is not a good long term fix. Thanks for looking into this.

dblock commented

@stevenou I merged the fix, will you please try HEAD? I'll do a release soon.

stevenou commented

Can confirm the integration test is passing.

dblock commented

I released 2.3.0.

dblock commented

I released 2.3.0.

  • 🎉1