diff --git a/lib/rack/twilio_webhook_authentication.rb b/lib/rack/twilio_webhook_authentication.rb index ba522e500..2ca6c53d6 100644 --- a/lib/rack/twilio_webhook_authentication.rb +++ b/lib/rack/twilio_webhook_authentication.rb @@ -28,19 +28,14 @@ class TwilioWebhookAuthentication def initialize(app, auth_token, *paths, &auth_token_lookup) @app = app @auth_token = auth_token - define_singleton_method(:get_auth_token, auth_token_lookup) if block_given? + define_singleton_method(:get_auth_token_for_sid, auth_token_lookup) if block_given? @path_regex = Regexp.union(paths) end def call(env) return @app.call(env) unless env['PATH_INFO'].match(@path_regex) - request = Rack::Request.new(env) - original_url = request.url - params = extract_params!(request) - auth_token = @auth_token || get_auth_token(params['AccountSid']) - validator = Twilio::Security::RequestValidator.new(auth_token) - signature = env['HTTP_X_TWILIO_SIGNATURE'] || '' - if validator.validate(original_url, params, signature) + + if valid_request?(env) @app.call(env) else [ @@ -51,6 +46,31 @@ def call(env) end end + def valid_request?(env) + request = Rack::Request.new(env) + original_url = request.url + params = extract_params!(request) + signature = env['HTTP_X_TWILIO_SIGNATURE'] || '' + + validators = build_validators(params['AccountSid']) + + validators.any? { |validator| validator.validate(original_url, params, signature) } + end + + private + + def build_validators(account_sid) + get_auth_tokens(account_sid).map do |auth_token| + Twilio::Security::RequestValidator.new(auth_token) + end + end + + def get_auth_tokens(account_sid) + tokens = @auth_token || get_auth_token_for_sid(account_sid) + + [tokens].flatten + end + # Extract the params from the the request that we can use to determine the # signature. This _may_ modify the passed in request since it may read/rewind # the body. @@ -66,7 +86,5 @@ def extract_params!(request) body end end - - private :extract_params! end end diff --git a/lib/twilio-ruby/security/request_validator.rb b/lib/twilio-ruby/security/request_validator.rb index f28a6b93d..b8c6ad1c5 100644 --- a/lib/twilio-ruby/security/request_validator.rb +++ b/lib/twilio-ruby/security/request_validator.rb @@ -38,6 +38,8 @@ def validate(url, params, signature) params_hash = {} end + return false unless valid_body + # Check signature of the url with and without port numbers # since signature generation on the back end is inconsistent valid_signature_with_port = secure_compare(build_signature_for(url_with_port, params_hash), signature) diff --git a/spec/rack/twilio_webhook_authentication_spec.rb b/spec/rack/twilio_webhook_authentication_spec.rb index b71a41116..0e531a88c 100644 --- a/spec/rack/twilio_webhook_authentication_spec.rb +++ b/spec/rack/twilio_webhook_authentication_spec.rb @@ -24,19 +24,26 @@ Rack::TwilioWebhookAuthentication.new(@app, nil, /\/voice/, /\/sms/) end.not_to raise_error end + + it 'should initialize with an app, auth token(s) and a path' do + expect do + Rack::TwilioWebhookAuthentication.new(@app, ['ABC', 'DEF'], /\/voice/, /\/sms/) + end.not_to raise_error + end end describe 'calling against one path with dynamic auth token' do + before do + allow_any_instance_of(Rack::Request).to receive(:post?).and_return(true) + allow_any_instance_of(Rack::Request).to receive(:media_type).and_return(Rack::MediaType.type('application/x-www-form-urlencoded')) + allow_any_instance_of(Rack::Request).to receive(:POST).and_return({ 'AccountSid' => 12_345 }) + + @middleware = Rack::TwilioWebhookAuthentication.new(@app, nil, /\/voice/) { |asid| 'qwerty' } + end + it 'should allow a request through if it validates' do - auth_token = 'qwerty' - account_sid = 12_345 - expect_any_instance_of(Rack::Request).to receive(:post?).and_return(true) - expect_any_instance_of(Rack::Request).to receive(:media_type).and_return(Rack::MediaType.type('application/x-www-form-urlencoded')) - expect_any_instance_of(Rack::Request).to receive(:POST).and_return({ 'AccountSid' => account_sid }) - @middleware = Rack::TwilioWebhookAuthentication.new(@app, nil, /\/voice/) { |asid| auth_token } - request_validator = double('RequestValidator') - expect(Twilio::Security::RequestValidator).to receive(:new).with(auth_token).and_return(request_validator) - expect(request_validator).to receive(:validate).and_return(true) + allow_any_instance_of(Twilio::Security::RequestValidator).to receive(:validate).and_return(true) + request = Rack::MockRequest.env_for('/voice') status, headers, body = @middleware.call(request) expect(status).to be(200) @@ -72,6 +79,35 @@ status, headers, body = @middleware.call(request) expect(status).to be(403) end + + context 'with secondary auth_token' do + let(:auth_token) { ['ABC', 'DEF'] } + + it 'should not intercept when the path doesn\'t match' do + expect(Twilio::Security::RequestValidator).to_not receive(:validate) + request = Rack::MockRequest.env_for('/sms') + status, headers, body = @middleware.call(request) + expect(status).to be(200) + end + + it 'should allow a request through if it validates' do + expect_any_instance_of(Twilio::Security::RequestValidator).to( + receive(:validate).and_return(true) + ) + request = Rack::MockRequest.env_for('/voice') + status, headers, body = @middleware.call(request) + expect(status).to be(200) + end + + it 'should short circuit a request to 403 if it does not validate' do + expect_any_instance_of(Twilio::Security::RequestValidator).to( + receive(:validate).and_return(false) + ) + request = Rack::MockRequest.env_for('/voice') + status, headers, body = @middleware.call(request) + expect(status).to be(403) + end + end end describe 'calling against many paths' do @@ -140,6 +176,59 @@ expect(status).to be(200) end + it 'should validate if the body signature is correct for secondary token' do + middleware = Rack::TwilioWebhookAuthentication.new(@app, ['invalid', 'qwerty'], /\/test/) + input = StringIO.new('{"message": "a post body"}') + + request = Rack::MockRequest.env_for( + 'https://example.com/test?bodySHA256=8d90d640c6ba47d595ac56203d7f5c6b511be80fdf44a2055acca75a119b9fd2', + method: 'POST', + input: input + ) + request['HTTP_X_TWILIO_SIGNATURE'] = 'zR5Oq4f6cijN5oz5bisiVuxYnTU=' + request['CONTENT_TYPE'] = 'application/json' + + status, headers, body = middleware.call(request) + + expect(status).to be(200) + end + + it 'should fail if the body does not validate for any token' do + middleware = Rack::TwilioWebhookAuthentication.new(@app, ['invalid', 'invalid2', 'invalid3'], /\/test/) + input = StringIO.new('{"message": "a post body that does not match the bodySHA256"}') + + request = Rack::MockRequest.env_for( + 'https://example.com/test?bodySHA256=79bfb0acaf0045fd30f13d48d4fe296b393d85a3bfbee881a0172b2bd574b11e', + method: 'POST', + input: input + ) + request['HTTP_X_TWILIO_SIGNATURE'] = '+LYlbGr/VmN84YPJQCuWs+9UA7E=' + request['CONTENT_TYPE'] = 'application/json' + + status, headers, body = middleware.call(request) + + expect(status).not_to be(200) + end + + it 'should validate if the body signature is correct for dynamic secondary token' do + middleware = Rack::TwilioWebhookAuthentication.new(@app, nil, /\/test/) do |account_sid| + return ['invalid', 'qwerty'] + end + input = StringIO.new('{"message": "a post body"}') + + request = Rack::MockRequest.env_for( + 'https://example.com/test?bodySHA256=8d90d640c6ba47d595ac56203d7f5c6b511be80fdf44a2055acca75a119b9fd2', + method: 'POST', + input: input + ) + request['HTTP_X_TWILIO_SIGNATURE'] = 'zR5Oq4f6cijN5oz5bisiVuxYnTU=' + request['CONTENT_TYPE'] = 'application/json' + + status, headers, body = middleware.call(request) + + expect(status).to be(200) + end + it 'should validate even if a previous middleware read the body first' do middleware = Rack::TwilioWebhookAuthentication.new(@app, 'qwerty', /\/test/) input = StringIO.new('{"message": "a post body"}')