Support non-rewindable Rack inputs for Events signature verification

CHANGELOG.md

@@ -1,5 +1,6 @@
 ### 3.3.0 (Next)
 
+* [#607](https://github.com/slack-ruby/slack-ruby-client/pull/607): Support non-rewindable Rack 3 request bodies when verifying Events API signatures, with WEBrick integration coverage; follows [#515](https://github.com/slack-ruby/slack-ruby-client/pull/515) - [@olleolleolle](https://github.com/olleolleolle), [@dblock](https://github.com/dblock).
 * [#606](https://github.com/slack-ruby/slack-ruby-client/pull/606): Add configured client copies with error callbacks for Slack and transport failures - [@dblock](https://github.com/dblock).
 * [#605](https://github.com/slack-ruby/slack-ruby-client/pull/605): Add configured client copies with request and response callbacks, including pagination - [@dblock](https://github.com/dblock).
 * [#603](https://github.com/slack-ruby/slack-ruby-client/pull/603): Enforce consistent exception messages with rubocop-exception_messages and run lint separately from the test matrix - [@dblock](https://github.com/dblock).

Gemfile

@@ -16,6 +16,7 @@ group :test do
   gem 'json-schema'
   gem 'mutex_m'
   gem 'racc'
+  gem 'rackup', '~> 2.1'
   gem 'rake', '~> 13'
   gem 'rspec'
   # Lock below 1.1.0, which started writing float timestamps to
@@ -26,6 +27,7 @@ group :test do
   gem 'timecop'
   gem 'vcr'
   gem 'webmock'
+  gem 'webrick', '~> 1.8'
 end
 
 if Gem::Version.new(RUBY_VERSION) >= Gem::Version.new('3.2')

README.md

@@ -461,6 +461,10 @@ Slack::Events::Request.new(
 
 The `verify!` call may raise `Slack::Events::Request::MissingSigningSecret`, `Slack::Events::Request::InvalidSignature` or `Slack::Events::Request::TimestampExpired` errors.
 
+Both rewindable request bodies and non-rewindable streams permitted by Rack 3 are supported. `Slack::Events::Request#body` caches the raw body. Rewindable inputs are rewound before and after reading, preserving access for other consumers. Non-rewindable inputs are consumed once; use `slack_request.body` for subsequent access to the raw body.
+
+Verify the signature before middleware or application code reads a non-rewindable input. Bytes already consumed from a one-shot stream cannot be recovered, and verification will fail. If multiple consumers need to read `rack.input`, install `Rack::RewindableInput::Middleware` (Rack 3) before any body-reading middleware to buffer the input.
+
 ### Message Handling
 
 All text in Slack uses the same [system of formatting and escaping](https://api.slack.com/docs/formatting): chat messages, direct messages, file comments, etc. [Slack::Messages::Formatting](lib/slack/messages/formatting.rb) provides convenience methods to format and parse messages.

lib/slack/events/request.rb

@@ -40,9 +40,9 @@ def version
       def body
         @body ||= begin
           input = http_request.body
-          input.rewind
+          input.rewind if input.respond_to?(:rewind)
           body = input.read
-          input.rewind
+          input.rewind if input.respond_to?(:rewind)
           body
         end
       end

spec/slack/events/request_rack_spec.rb

@@ -0,0 +1,93 @@
+# frozen_string_literal: true
+require 'spec_helper'
+require 'rack'
+require 'rack/lint'
+require 'rack/rewindable_input'
+require 'rackup/handler/webrick'
+
+RSpec.describe Slack::Events::Request do
+  let(:signing_secret) { 'test-signing-secret' }
+  let(:timestamp) { Time.now.to_i.to_s }
+  let(:body) { '{"type":"url_verification","challenge":"hello"}' }
+  let(:signature) do
+    "v0=#{OpenSSL::HMAC.hexdigest('SHA256', signing_secret, "v0:#{timestamp}:#{body}")}"
+  end
+  let(:consume_body) { false }
+  let(:buffer_body) { false }
+  let(:observed_inputs) { [] }
+  let(:app) do
+    Rack::Lint.new(lambda do |env|
+      observed_inputs << env.fetch('rack.input')
+      env['rack.input'] = Rack::RewindableInput.new(env['rack.input']) if buffer_body
+      env['rack.input'].read if consume_body
+      slack_request = described_class.new(Rack::Request.new(env), signing_secret: signing_secret)
+      begin
+        slack_request.verify!
+        [200, { 'content-type' => 'application/json' }, [slack_request.body, slack_request.body]]
+      rescue Slack::Events::Request::InvalidSignature
+        [401, { 'content-type' => 'text/plain' }, ['invalid signature']]
+      ensure
+        env['rack.input'].close if buffer_body
+      end
+    end)
+  end
+  let(:server) { WEBrick::HTTPServer.new(DoNotListen: true, Logger: WEBrick::Log.new(File::NULL), AccessLog: []) }
+
+  def http_request
+    raw_request = [
+      'POST /events HTTP/1.1',
+      'Host: localhost',
+      'Content-Type: application/json',
+      "Content-Length: #{body.bytesize}",
+      "X-Slack-Request-Timestamp: #{timestamp}",
+      "X-Slack-Signature: #{signature}",
+      '', body
+    ].join("\r\n")
+    WEBrick::HTTPRequest.new(server.config).tap { |request| request.parse(StringIO.new(raw_request)) }
+  end
+
+  after do
+    server.shutdown
+  end
+
+  def dispatch
+    response = WEBrick::HTTPResponse.new(server.config)
+    Rackup::Handler::WEBrick.new(server, app).service(http_request, response)
+    response
+  end
+
+  it 'verifies a signed WEBrick request and caches its complete body' do
+    http_response = dispatch
+    expect(http_response.status).to eq 200
+    expect(http_response.body).to eq body * 2
+    expect(observed_inputs.first).not_to respond_to(:rewind)
+  end
+
+  context 'with an invalid signature' do
+    let(:signature) { 'v0=invalid' }
+
+    it 'rejects the request' do
+      http_response = dispatch
+      expect(http_response.status).to eq 401
+    end
+  end
+
+  context 'with input already read by another consumer' do
+    let(:consume_body) { true }
+
+    it 'rejects the request because the full body cannot be recovered' do
+      http_response = dispatch
+      expect(http_response.status).to eq 401
+    end
+
+    context 'with input buffering before the first consumer' do
+      let(:buffer_body) { true }
+
+      it 'verifies the request and preserves the full body' do
+        http_response = dispatch
+        expect(http_response.status).to eq 200
+        expect(http_response.body).to eq body * 2
+      end
+    end
+  end
+end

spec/slack/events/request_spec.rb

@@ -70,6 +70,31 @@
     end
   end
 
+  context 'with a non-rewindable body' do
+    let(:input) { double(read: body) }
+
+    before do
+      allow(http_request).to receive(:body).and_return(input)
+    end
+
+    it 'reads and caches the body without rewinding' do
+      expect(input).to receive(:read).once.and_return(body)
+      2.times { expect(request.body).to eq body }
+    end
+
+    it 'validates the signature' do
+      expect(request).to be_valid
+    end
+
+    context 'with an already consumed body' do
+      let(:input) { double(read: '') }
+
+      it 'rejects the signature rather than accepting an incomplete body' do
+        expect(request).not_to be_valid
+      end
+    end
+  end
+
   context 'time' do
     after do
       Timecop.return