diff --git a/app/controllers/sms_controller.rb b/app/controllers/sms_controller.rb index 2d11844de64..142e904bb1e 100644 --- a/app/controllers/sms_controller.rb +++ b/app/controllers/sms_controller.rb @@ -5,29 +5,21 @@ class SmsController < ApplicationController # Twilio supports HTTP Basic Auth for request URL # https://www.twilio.com/docs/usage/security before_action :authenticate + before_action :set_message, only: [:receive] # Disable CSRF check skip_before_action :verify_authenticity_token, only: [:receive] def receive - signature = request.headers[TwilioService::Sms::Request::SIGNATURE_HEADER] - message = TwilioService::Sms::Request.new(request.url, params, signature) + result = SmsForm.new(@message).submit - handle_result(message, SmsForm.new(message).submit) + result.success? ? process_success(result) : process_failure(result) end private - def handle_result(message, result) - if result.success? - process_success(message, result) - else - process_failure(result) - end - end - - def process_success(message, result) - response = TwilioService::Sms::Response.new(message) + def process_success(result) + response = TwilioService::Sms::Response.new(@message) SmsReplySenderJob.perform_later(response.reply) analytics.track_event( @@ -44,7 +36,11 @@ def process_failure(result) result.to_h ) - head :forbidden + if !@message.signature_valid? + head :forbidden + else + head :ok + end end # `http_basic_authenticate_with name` had issues related to testing, so using @@ -71,4 +67,10 @@ def auth_configured?(env) env.twilio_http_basic_auth_username.present? && env.twilio_http_basic_auth_password.present? end + + def set_message + signature = request.headers[TwilioService::Sms::Request::SIGNATURE_HEADER] + + @message = TwilioService::Sms::Request.new(request.url, params, signature) + end end diff --git a/app/services/twilio_service/sms/request.rb b/app/services/twilio_service/sms/request.rb index a02e2046c95..174d77ff205 100644 --- a/app/services/twilio_service/sms/request.rb +++ b/app/services/twilio_service/sms/request.rb @@ -32,10 +32,6 @@ def extra_analytics_attributes } end - private - - attr_reader :url, :params, :signature - # First, validate the message signature using Twilio's library: # https://github.com/twilio/twilio-ruby/wiki/Request-Validator def signature_valid? @@ -51,6 +47,10 @@ def message_valid? # before processing the submission; however this is on-hold during the # initial phase. end + + private + + attr_reader :url, :params, :signature end end end diff --git a/spec/requests/sms_spec.rb b/spec/requests/sms_spec.rb index 06cd7f1a063..83dac5d50df 100644 --- a/spec/requests/sms_spec.rb +++ b/spec/requests/sms_spec.rb @@ -7,6 +7,8 @@ let(:password) { 'auth_password' } let(:access_denied) { 'HTTP Basic: Access denied' } let(:credentials) { "Basic #{Base64.encode64("#{username}:#{password}")}" } + let(:help_message) { 'help' } + let(:invalid_message) { 'blargh' } describe 'HTTP Basic Authentication' do context 'without required credentials' do @@ -30,7 +32,7 @@ receive(:validate).and_return(true) ) - post_message + post_message(help_message) expect(response).to have_http_status(:accepted) end @@ -48,17 +50,29 @@ end context 'when failing' do - it 'does not send a reply' do + it 'does not send a reply and 403s when signature is invalid' do allow_any_instance_of(Twilio::Security::RequestValidator).to( receive(:validate).and_return(false) ) expect(SmsReplySenderJob).to_not receive(:perform_later) - post_message + post_message(help_message) expect(response).to have_http_status(:forbidden) end + + it 'responds with a 200 status when signature is valid' do + allow_any_instance_of(Twilio::Security::RequestValidator).to( + receive(:validate).and_return(true) + ) + + expect(SmsReplySenderJob).to_not receive(:perform_later) + + post_message(invalid_message) + + expect(response).to have_http_status(:ok) + end end context 'when successful' do @@ -69,7 +83,7 @@ expect(SmsReplySenderJob).to receive(:perform_later) - post_message + post_message(help_message) expect(response).to have_http_status(:accepted) end @@ -78,10 +92,10 @@ private - def post_message + def post_message(body) post( api_sms_receive_path, - params: { Body: 'help' }, + params: { Body: body }, headers: { 'HTTP_AUTHORIZATION': credentials } ) end