Skip to content

Commit 093baf4

Browse files
dblockCopilot
andcommitted
Support non-rewindable Rack inputs for Events signature verification
Follow up on #515 with signature and WEBrick handler integration coverage for Rack 2 and Rack 3, plus middleware ordering documentation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 565c386 commit 093baf4

6 files changed

Lines changed: 127 additions & 2 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
### 3.3.0 (Next)
22

3+
* [#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).
34
* [#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).
45
* [#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).
56
* [#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‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ group :test do
1616
gem 'json-schema'
1717
gem 'mutex_m'
1818
gem 'racc'
19+
gem 'rackup', '~> 2.1'
1920
gem 'rake', '~> 13'
2021
gem 'rspec'
2122
# Lock below 1.1.0, which started writing float timestamps to
@@ -26,6 +27,7 @@ group :test do
2627
gem 'timecop'
2728
gem 'vcr'
2829
gem 'webmock'
30+
gem 'webrick', '~> 1.8'
2931
end
3032

3133
if Gem::Version.new(RUBY_VERSION) >= Gem::Version.new('3.2')

‎README.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -461,6 +461,10 @@ Slack::Events::Request.new(
461461

462462
The `verify!` call may raise `Slack::Events::Request::MissingSigningSecret`, `Slack::Events::Request::InvalidSignature` or `Slack::Events::Request::TimestampExpired` errors.
463463

464+
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.
465+
466+
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.
467+
464468
### Message Handling
465469

466470
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‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,9 @@ def version
4040
def body
4141
@body ||= begin
4242
input = http_request.body
43-
input.rewind
43+
input.rewind if input.respond_to?(:rewind)
4444
body = input.read
45-
input.rewind
45+
input.rewind if input.respond_to?(:rewind)
4646
body
4747
end
4848
end
Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
# frozen_string_literal: true
2+
require 'spec_helper'
3+
require 'rack'
4+
require 'rack/lint'
5+
require 'rack/rewindable_input'
6+
require 'rackup/handler/webrick'
7+
8+
RSpec.describe Slack::Events::Request do
9+
let(:signing_secret) { 'test-signing-secret' }
10+
let(:timestamp) { Time.now.to_i.to_s }
11+
let(:body) { '{"type":"url_verification","challenge":"hello"}' }
12+
let(:signature) do
13+
"v0=#{OpenSSL::HMAC.hexdigest('SHA256', signing_secret, "v0:#{timestamp}:#{body}")}"
14+
end
15+
let(:consume_body) { false }
16+
let(:buffer_body) { false }
17+
let(:observed_inputs) { [] }
18+
let(:app) do
19+
Rack::Lint.new(lambda do |env|
20+
observed_inputs << env.fetch('rack.input')
21+
env['rack.input'] = Rack::RewindableInput.new(env['rack.input']) if buffer_body
22+
env['rack.input'].read if consume_body
23+
slack_request = described_class.new(Rack::Request.new(env), signing_secret: signing_secret)
24+
begin
25+
slack_request.verify!
26+
[200, { 'content-type' => 'application/json' }, [slack_request.body, slack_request.body]]
27+
rescue Slack::Events::Request::InvalidSignature
28+
[401, { 'content-type' => 'text/plain' }, ['invalid signature']]
29+
ensure
30+
env['rack.input'].close if buffer_body
31+
end
32+
end)
33+
end
34+
let(:server) { WEBrick::HTTPServer.new(DoNotListen: true, Logger: WEBrick::Log.new(File::NULL), AccessLog: []) }
35+
36+
def http_request
37+
raw_request = [
38+
'POST /events HTTP/1.1',
39+
'Host: localhost',
40+
'Content-Type: application/json',
41+
"Content-Length: #{body.bytesize}",
42+
"X-Slack-Request-Timestamp: #{timestamp}",
43+
"X-Slack-Signature: #{signature}",
44+
'', body
45+
].join("\r\n")
46+
WEBrick::HTTPRequest.new(server.config).tap { |request| request.parse(StringIO.new(raw_request)) }
47+
end
48+
49+
after do
50+
server.shutdown
51+
end
52+
53+
def dispatch
54+
response = WEBrick::HTTPResponse.new(server.config)
55+
Rackup::Handler::WEBrick.new(server, app).service(http_request, response)
56+
response
57+
end
58+
59+
it 'verifies a signed WEBrick request and caches its complete body' do
60+
http_response = dispatch
61+
expect(http_response.status).to eq 200
62+
expect(http_response.body).to eq body * 2
63+
expect(observed_inputs.first).not_to respond_to(:rewind)
64+
end
65+
66+
context 'with an invalid signature' do
67+
let(:signature) { 'v0=invalid' }
68+
69+
it 'rejects the request' do
70+
http_response = dispatch
71+
expect(http_response.status).to eq 401
72+
end
73+
end
74+
75+
context 'with input already read by another consumer' do
76+
let(:consume_body) { true }
77+
78+
it 'rejects the request because the full body cannot be recovered' do
79+
http_response = dispatch
80+
expect(http_response.status).to eq 401
81+
end
82+
83+
context 'with input buffering before the first consumer' do
84+
let(:buffer_body) { true }
85+
86+
it 'verifies the request and preserves the full body' do
87+
http_response = dispatch
88+
expect(http_response.status).to eq 200
89+
expect(http_response.body).to eq body * 2
90+
end
91+
end
92+
end
93+
end

‎spec/slack/events/request_spec.rb‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,31 @@
7070
end
7171
end
7272

73+
context 'with a non-rewindable body' do
74+
let(:input) { double(read: body) }
75+
76+
before do
77+
allow(http_request).to receive(:body).and_return(input)
78+
end
79+
80+
it 'reads and caches the body without rewinding' do
81+
expect(input).to receive(:read).once.and_return(body)
82+
2.times { expect(request.body).to eq body }
83+
end
84+
85+
it 'validates the signature' do
86+
expect(request).to be_valid
87+
end
88+
89+
context 'with an already consumed body' do
90+
let(:input) { double(read: '') }
91+
92+
it 'rejects the signature rather than accepting an incomplete body' do
93+
expect(request).not_to be_valid
94+
end
95+
end
96+
end
97+
7398
context 'time' do
7499
after do
75100
Timecop.return

0 commit comments

Comments
 (0)