From 50c7d2c586601e35b9dfcf971c9fedac1ee84ed5 Mon Sep 17 00:00:00 2001 From: Zach Margolis Date: Tue, 17 Oct 2017 09:36:19 -0400 Subject: [PATCH 1/3] Revert "Revert to serving TwiML from Twimlets" This reverts commit 741e48e72683720e5664ddefdf415d290e4fde2c. **Why**: We want to use more advanced TwiML features like selecting language --- .slim-lint.yml | 3 + app/controllers/voice/otp_controller.rb | 46 ++++++++++++++ app/jobs/voice_otp_sender_job.rb | 39 +----------- app/services/basic_auth_url.rb | 10 +++ app/views/voice/otp/show.xml.slim | 7 +++ config/locales/voice/en.yml | 7 +++ config/locales/voice/es.yml | 7 +++ config/routes.rb | 4 ++ spec/controllers/voice/otp_controller_spec.rb | 62 +++++++++++++++++++ spec/jobs/voice_otp_sender_job_spec.rb | 13 +--- spec/services/basic_auth_url_spec.rb | 31 ++++++++++ 11 files changed, 181 insertions(+), 48 deletions(-) create mode 100644 app/controllers/voice/otp_controller.rb create mode 100644 app/services/basic_auth_url.rb create mode 100644 app/views/voice/otp/show.xml.slim create mode 100644 config/locales/voice/en.yml create mode 100644 config/locales/voice/es.yml create mode 100644 spec/controllers/voice/otp_controller_spec.rb create mode 100644 spec/services/basic_auth_url_spec.rb diff --git a/.slim-lint.yml b/.slim-lint.yml index 81636c752f4..611aae3da75 100644 --- a/.slim-lint.yml +++ b/.slim-lint.yml @@ -7,5 +7,8 @@ linters: exclude: - 'app/views/pages/help.html.slim' - 'app/views/pages/privacy_policy.html.slim' + TagCase: + exclude: + - 'app/views/voice/otp/show.xml.slim' RuboCop: enabled: true diff --git a/app/controllers/voice/otp_controller.rb b/app/controllers/voice/otp_controller.rb new file mode 100644 index 00000000000..63acecbf88e --- /dev/null +++ b/app/controllers/voice/otp_controller.rb @@ -0,0 +1,46 @@ +module Voice + class OtpController < ApplicationController + NUMBER_OF_TIMES_USER_CAN_REPEAT_CODE = 5 + + skip_before_action :verify_authenticity_token + + def show + if code.blank? + render nothing: true, status: :bad_request + return + end + + @message = message + @action_url = action_url + end + + protected + + def code + params[:code].to_s + end + + def message + t('voice.otp.message', code: code_with_pauses) + end + + def code_with_pauses + code.scan(/\d/).join(', ') + end + + def repeat_count + (params[:repeat_count] || NUMBER_OF_TIMES_USER_CAN_REPEAT_CODE).to_i + end + + def action_url + return if repeat_count <= 1 + + BasicAuthUrl.build( + voice_otp_url( + code: code, + repeat_count: repeat_count - 1 + ) + ) + end + end +end diff --git a/app/jobs/voice_otp_sender_job.rb b/app/jobs/voice_otp_sender_job.rb index d16399d6c32..bdd55f48872 100644 --- a/app/jobs/voice_otp_sender_job.rb +++ b/app/jobs/voice_otp_sender_job.rb @@ -1,4 +1,6 @@ class VoiceOtpSenderJob < ApplicationJob + include Rails.application.routes.url_helpers + queue_as :voice def perform(code:, phone:, otp_created_at:) @@ -13,45 +15,10 @@ def otp_valid?(otp_created_at) end def send_otp(twilio_service, code, phone) - code_with_pauses = code.scan(/\d/).join(', ') twilio_service.place_call( to: phone, - url: twimlet_url(code_with_pauses), + url: BasicAuthUrl.build(voice_otp_url(code: code)), record: Figaro.env.twilio_record_voice == 'true' ) end - - def twimlet_url(code) # rubocop:disable Metrics/MethodLength - repeat = message_repeat(code) - - twimlet_menu( - repeat, - 1 => twimlet_menu( - repeat, - 1 => twimlet_menu( - repeat, - 1 => twimlet_menu(repeat, 1 => twimlet_message(message_final(code))) - ) - ) - ) - end - - def message_repeat(code) - I18n.t('jobs.voice_otp_sender_job.message_repeat', code: code) - end - - def message_final(code) - I18n.t('jobs.voice_otp_sender_job.message_final', code: code) - end - - def twimlet_message(message) - 'https://twimlets.com/message?' + { Message: { 0 => message } }.to_query - end - - def twimlet_menu(message, options) - 'https://twimlets.com/menu?' + { - Message: message, - Options: options.to_h, - }.to_query - end end diff --git a/app/services/basic_auth_url.rb b/app/services/basic_auth_url.rb new file mode 100644 index 00000000000..698dbd4cfc0 --- /dev/null +++ b/app/services/basic_auth_url.rb @@ -0,0 +1,10 @@ +module BasicAuthUrl + module_function + + def build(url, user: ENV['SP_NAME'], password: ENV['SP_PASS']) + URI.parse(url).tap do |uri| + uri.user = user + uri.password = password + end.to_s + end +end diff --git a/app/views/voice/otp/show.xml.slim b/app/views/voice/otp/show.xml.slim new file mode 100644 index 00000000000..131b28421ed --- /dev/null +++ b/app/views/voice/otp/show.xml.slim @@ -0,0 +1,7 @@ +doctype xml +Response + Say = @message + - if @action_url + Gather numDigits="1" action=@action_url + Say = t('voice.otp.repeat_instructions') + Hangup / diff --git a/config/locales/voice/en.yml b/config/locales/voice/en.yml new file mode 100644 index 00000000000..3cfdd11d456 --- /dev/null +++ b/config/locales/voice/en.yml @@ -0,0 +1,7 @@ +en: + voice: + otp: + message: > + Hello! Your login.gov one time passcode is, %{code}, + again, your passcode is, %{code}. + repeat_instructions: Press 1 to repeat this message. diff --git a/config/locales/voice/es.yml b/config/locales/voice/es.yml new file mode 100644 index 00000000000..84f30a14e01 --- /dev/null +++ b/config/locales/voice/es.yml @@ -0,0 +1,7 @@ +es: + voice: + otp: + message: > + ¡Hola! Su código de acceso de login.gov es, %{code}, + nuevamente, su código de acceso es %{code}. + repeat_instructions: Presione 1 para repetir este mensaje. diff --git a/config/routes.rb b/config/routes.rb index 8db34d24ba9..cb3293d8e0e 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -17,6 +17,10 @@ match '/api/saml/auth' => 'saml_idp#auth', via: %i[get post] post '/api/service_provider' => 'service_provider#update' + match '/api/voice/otp/:code' => 'voice/otp#show', + via: [:get, :post], + as: :voice_otp, + defaults: { format: :xml } get '/openid_connect/authorize' => 'openid_connect/authorization#index' get '/openid_connect/logout' => 'openid_connect/logout#index' diff --git a/spec/controllers/voice/otp_controller_spec.rb b/spec/controllers/voice/otp_controller_spec.rb new file mode 100644 index 00000000000..318331ff1bd --- /dev/null +++ b/spec/controllers/voice/otp_controller_spec.rb @@ -0,0 +1,62 @@ +require 'rails_helper' + +RSpec.describe Voice::OtpController do + describe '#show' do + subject(:action) { get :show, code: code, repeat_count: repeat_count, format: :xml } + let(:code) { nil } + let(:repeat_count) { nil } + + context 'without a code in the URL' do + let(:code) { nil } + + it 'cannot route to the controller' do + expect { action }.to raise_error(ActionController::UrlGenerationError) + end + end + + context 'with a blank code in the URL' do + let(:code) { '' } + + it 'renders a blank 400' do + action + + expect(response).to be_bad_request + expect(response.body).to be_empty + end + end + + context 'with a code in the URL' do + render_views + + let(:code) { 1234 } + + it 'tells Twilio to the code with pauses in between' do + action + + doc = Nokogiri::XML(response.body) + say = doc.css('Say').first + expect(say.text).to include('1, 2, 3, 4,') + end + + it 'has a with instructions to repeat with a repeat_count' do + action + + doc = Nokogiri::XML(response.body) + gather = doc.css('Gather').first + + expect(gather['action']).to include('repeat_count=4') + end + + context 'when repeat_count counts down to 1' do + let(:repeat_count) { 1 } + + it 'does not have a in the response' do + action + + doc = Nokogiri::XML(response.body) + expect(doc.css('Gather')).to be_empty + end + end + end + end +end diff --git a/spec/jobs/voice_otp_sender_job_spec.rb b/spec/jobs/voice_otp_sender_job_spec.rb index caeb9e2b6d2..4ae5313ff7b 100644 --- a/spec/jobs/voice_otp_sender_job_spec.rb +++ b/spec/jobs/voice_otp_sender_job_spec.rb @@ -21,18 +21,7 @@ call = calls.first expect(call.to).to eq('555-5555') expect(call.from).to match(/(\+19999999999|\+12222222222)/) - - code = '1234'.scan(/\d/).join(', ') - query = URIService.params(call.url) - expect(query['Message']).to eq(t('jobs.voice_otp_sender_job.message_repeat', code: code)) - - nested_query = query - while nested_query['Options'] - nested_url = URI(nested_query['Options']['1']) - nested_query = URIService.params(nested_url) - end - expect(nested_query['Message']['0']). - to eq(t('jobs.voice_otp_sender_job.message_final', code: code)) + expect(call.url).to include('1234') end context 'recording calls' do diff --git a/spec/services/basic_auth_url_spec.rb b/spec/services/basic_auth_url_spec.rb new file mode 100644 index 00000000000..f1d33ac5f63 --- /dev/null +++ b/spec/services/basic_auth_url_spec.rb @@ -0,0 +1,31 @@ +require 'rails_helper' + +RSpec.describe BasicAuthUrl do + describe '.build' do + it 'is the URL as-is with no username or password' do + url = 'https://foo.example.com/bar' + external_url = BasicAuthUrl.build(url, user: nil, password: nil) + + expect(external_url).to eq(url) + end + + context 'with SP_NAME and SP_PASS set in the ENV' do + before do + ENV['SP_NAME'] = 'user' + ENV['SP_PASS'] = 'secret' + end + + after do + ENV.delete('SP_NAME') + ENV.delete('SP_PASS') + end + + it 'uses the values in the ENV' do + url = 'https://foo.example.com/bar' + external_url = BasicAuthUrl.build(url) + + expect(external_url).to eq('https://user:secret@foo.example.com/bar') + end + end + end +end From 4583bd17445b06f2703f2064c6ff3449528a691b Mon Sep 17 00:00:00 2001 From: Zach Margolis Date: Tue, 17 Oct 2017 10:50:27 -0400 Subject: [PATCH 2/3] Update voice OTP TwiML **Why**: Updated code layout - Change basic auth to be a config value, not an ENV var - Set locale in voice OTP URLs - Set "language" attribute inside for better pronunciation --- .reek | 1 + app/controllers/voice/otp_controller.rb | 2 +- app/jobs/voice_otp_sender_job.rb | 3 +- app/services/basic_auth_url.rb | 2 +- app/views/voice/otp/show.xml.slim | 3 +- config/application.yml.example | 6 ++ config/locales/jobs/en.yml | 5 -- config/locales/jobs/es.yml | 6 -- config/locales/jobs/fr.yml | 6 -- config/locales/voice/en.yml | 6 +- config/locales/voice/es.yml | 6 +- config/locales/voice/fr.yml | 7 ++ spec/controllers/voice/otp_controller_spec.rb | 64 ++++++++++++++++++- spec/jobs/voice_otp_sender_job_spec.rb | 15 +++-- spec/services/basic_auth_url_spec.rb | 14 +--- 15 files changed, 100 insertions(+), 46 deletions(-) create mode 100644 config/locales/voice/fr.yml diff --git a/.reek b/.reek index bc5ef4b27cc..de94e952505 100644 --- a/.reek +++ b/.reek @@ -12,6 +12,7 @@ DuplicateMethodCall: - WorkerHealthChecker#status - FileEncryptor#encrypt - UserFlowExporter#self.massage_assets + - BasicAuthUrl#build FeatureEnvy: exclude: - ActiveJob::Logging::LogSubscriber#json_for diff --git a/app/controllers/voice/otp_controller.rb b/app/controllers/voice/otp_controller.rb index 63acecbf88e..bae48d543ef 100644 --- a/app/controllers/voice/otp_controller.rb +++ b/app/controllers/voice/otp_controller.rb @@ -29,7 +29,7 @@ def code_with_pauses end def repeat_count - (params[:repeat_count] || NUMBER_OF_TIMES_USER_CAN_REPEAT_CODE).to_i + (params[:repeat_count].presence || NUMBER_OF_TIMES_USER_CAN_REPEAT_CODE).to_i end def action_url diff --git a/app/jobs/voice_otp_sender_job.rb b/app/jobs/voice_otp_sender_job.rb index bdd55f48872..5e2bec9cb15 100644 --- a/app/jobs/voice_otp_sender_job.rb +++ b/app/jobs/voice_otp_sender_job.rb @@ -1,5 +1,6 @@ class VoiceOtpSenderJob < ApplicationJob include Rails.application.routes.url_helpers + include LocaleHelper queue_as :voice @@ -17,7 +18,7 @@ def otp_valid?(otp_created_at) def send_otp(twilio_service, code, phone) twilio_service.place_call( to: phone, - url: BasicAuthUrl.build(voice_otp_url(code: code)), + url: BasicAuthUrl.build(voice_otp_url(code: code, locale: locale_url_param)), record: Figaro.env.twilio_record_voice == 'true' ) end diff --git a/app/services/basic_auth_url.rb b/app/services/basic_auth_url.rb index 698dbd4cfc0..d43426afee3 100644 --- a/app/services/basic_auth_url.rb +++ b/app/services/basic_auth_url.rb @@ -1,7 +1,7 @@ module BasicAuthUrl module_function - def build(url, user: ENV['SP_NAME'], password: ENV['SP_PASS']) + def build(url, user: Figaro.env.basic_auth_user_name, password: Figaro.env.basic_auth_password) URI.parse(url).tap do |uri| uri.user = user uri.password = password diff --git a/app/views/voice/otp/show.xml.slim b/app/views/voice/otp/show.xml.slim index 131b28421ed..c008ec7b453 100644 --- a/app/views/voice/otp/show.xml.slim +++ b/app/views/voice/otp/show.xml.slim @@ -1,6 +1,7 @@ doctype xml Response - Say = @message + Say language="#{I18n.locale}" + = @message - if @action_url Gather numDigits="1" action=@action_url Say = t('voice.otp.repeat_instructions') diff --git a/config/application.yml.example b/config/application.yml.example index c8189a90ccd..d3759abe9d9 100644 --- a/config/application.yml.example +++ b/config/application.yml.example @@ -60,6 +60,8 @@ development: available_locales: 'en es fr' aws_kms_key_id: 'alias/login-dot-gov-development-keymaker' aws_region: 'us-east-1' + basic_auth_user_name: 'user' + basic_auth_password: 'secret' dashboard_api_token: 'test_token' dashboard_url: 'http://localhost:3001/api/service_providers' database_host: '' @@ -123,6 +125,8 @@ production: available_locales: 'en es fr' aws_kms_key_id: aws_region: + basic_auth_user_name: + basic_auth_password: disable_email_sending: 'false' dashboard_api_token: domain_name: 'login.gov' @@ -184,6 +188,8 @@ test: available_locales: 'en es fr' aws_kms_key_id: 'alias/login-dot-gov-test-keymaker' aws_region: 'us-east-1' + basic_auth_user_name: 'user' + basic_auth_password: 'secret' domain_name: 'www.example.com' database_host: '' database_name: '' diff --git a/config/locales/jobs/en.yml b/config/locales/jobs/en.yml index fc312f77076..9938a11af65 100644 --- a/config/locales/jobs/en.yml +++ b/config/locales/jobs/en.yml @@ -3,8 +3,3 @@ en: jobs: sms_otp_sender_job: message: "%{code} is your %{app} one-time security code." - voice_otp_sender_job: - message_final: Hello! Your login.gov one time security code is, %{code}, again, - your security code is, %{code}, goodbye! - message_repeat: Hello! Your login.gov one time security code is, %{code}, again, - your security code is, %{code}. Press 1 to repeat your code. diff --git a/config/locales/jobs/es.yml b/config/locales/jobs/es.yml index 2f321118986..a6329d59ae1 100644 --- a/config/locales/jobs/es.yml +++ b/config/locales/jobs/es.yml @@ -3,9 +3,3 @@ es: jobs: sms_otp_sender_job: message: "%{code} es su %{app} código de seguridad de sólo un uso." - voice_otp_sender_job: - message_final: "¡Hola! Su código de seguridad de login.gov para uso único es, - %{code}, nuevamente, su código de seguridad es, % {code}, ¡adiós!" - message_repeat: "¡Hola! Su código de seguridad de login.gov para uso único es - %{code}, nuevamente, su código de seguridad es % {code}. Presione 1 para repetir - su código." diff --git a/config/locales/jobs/fr.yml b/config/locales/jobs/fr.yml index bb76630ae82..e3842d4808e 100644 --- a/config/locales/jobs/fr.yml +++ b/config/locales/jobs/fr.yml @@ -3,9 +3,3 @@ fr: jobs: sms_otp_sender_job: message: "%{code} est votre %{app} code de sécurité à utilisation unique." - voice_otp_sender_job: - message_final: Bonjour! Votre code de sécurité à utilisation unique de login.gov - est, %{code}, de nouveau, votre code de sécurité est, %{code}, au revoir! - message_repeat: Bonjour! Votre code de sécurité à utilisation unique de login.gov - est, %{code}, de nouveau, votre code de sécurité est, %{code}. Appuyez sur 1 - pour répéter votre code. diff --git a/config/locales/voice/en.yml b/config/locales/voice/en.yml index 3cfdd11d456..1672f893747 100644 --- a/config/locales/voice/en.yml +++ b/config/locales/voice/en.yml @@ -1,7 +1,7 @@ +--- en: voice: otp: - message: > - Hello! Your login.gov one time passcode is, %{code}, - again, your passcode is, %{code}. + message: Hello! Your login.gov one time passcode is, %{code}, again, your passcode + is, %{code}. repeat_instructions: Press 1 to repeat this message. diff --git a/config/locales/voice/es.yml b/config/locales/voice/es.yml index 84f30a14e01..5a251461957 100644 --- a/config/locales/voice/es.yml +++ b/config/locales/voice/es.yml @@ -1,7 +1,7 @@ +--- es: voice: otp: - message: > - ¡Hola! Su código de acceso de login.gov es, %{code}, - nuevamente, su código de acceso es %{code}. + message: "¡Hola! Su código de acceso de login.gov es, %{code}, nuevamente, su + código de acceso es %{code}." repeat_instructions: Presione 1 para repetir este mensaje. diff --git a/config/locales/voice/fr.yml b/config/locales/voice/fr.yml new file mode 100644 index 00000000000..91322bc05bd --- /dev/null +++ b/config/locales/voice/fr.yml @@ -0,0 +1,7 @@ +--- +fr: + voice: + otp: + message: Bonjour! Votre code de sécurité à utilisation unique de login.gov est, + %{code}, de nouveau, votre code de sécurité est, %{code}, au revoir! + repeat_instructions: Appuyez sur 1 pour répéter votre code. diff --git a/spec/controllers/voice/otp_controller_spec.rb b/spec/controllers/voice/otp_controller_spec.rb index 318331ff1bd..4f8baf5020f 100644 --- a/spec/controllers/voice/otp_controller_spec.rb +++ b/spec/controllers/voice/otp_controller_spec.rb @@ -2,8 +2,13 @@ RSpec.describe Voice::OtpController do describe '#show' do - subject(:action) { get :show, code: code, repeat_count: repeat_count, format: :xml } + subject(:action) do + get :show, + params: { code: code, repeat_count: repeat_count, locale: locale }, + format: :xml + end let(:code) { nil } + let(:locale) { nil } let(:repeat_count) { nil } context 'without a code in the URL' do @@ -38,13 +43,68 @@ expect(say.text).to include('1, 2, 3, 4,') end + it 'sets the lang attribute to english' do + action + + doc = Nokogiri::XML(response.body) + say = doc.css('Say').first + + expect(say[:language]).to eq('en') + end + + context 'when the locale is in spanish' do + let(:locale) { :es } + + it 'sets the lang attribute to english' do + action + + doc = Nokogiri::XML(response.body) + say = doc.css('Say').first + + expect(say[:language]).to eq('es') + end + + it 'passes locale into the action URL' do + action + + doc = Nokogiri::XML(response.body) + gather = doc.css('Gather').first + + params = URIService.params(gather[:action]) + expect(params[:locale]).to eq('es') + end + end + + context 'when the locale is in french' do + let(:locale) { :fr } + + it 'sets the lang attribute to english' do + action + + doc = Nokogiri::XML(response.body) + say = doc.css('Say').first + + expect(say[:language]).to eq('fr') + end + + it 'passes locale into the action URL' do + action + + doc = Nokogiri::XML(response.body) + gather = doc.css('Gather').first + + params = URIService.params(gather[:action]) + expect(params[:locale]).to eq('fr') + end + end + it 'has a with instructions to repeat with a repeat_count' do action doc = Nokogiri::XML(response.body) gather = doc.css('Gather').first - expect(gather['action']).to include('repeat_count=4') + expect(gather[:action]).to include('repeat_count=4') end context 'when repeat_count counts down to 1' do diff --git a/spec/jobs/voice_otp_sender_job_spec.rb b/spec/jobs/voice_otp_sender_job_spec.rb index 4ae5313ff7b..af586490990 100644 --- a/spec/jobs/voice_otp_sender_job_spec.rb +++ b/spec/jobs/voice_otp_sender_job_spec.rb @@ -9,11 +9,13 @@ end it 'initiates the phone call to deliver the OTP', twilio: true do - VoiceOtpSenderJob.perform_now( - code: '1234', - phone: '555-5555', - otp_created_at: Time.zone.now.to_s - ) + I18n.with_locale(:fr) do + VoiceOtpSenderJob.perform_now( + code: '1234', + phone: '555-5555', + otp_created_at: Time.zone.now.to_s + ) + end calls = FakeVoiceCall.calls @@ -21,7 +23,10 @@ call = calls.first expect(call.to).to eq('555-5555') expect(call.from).to match(/(\+19999999999|\+12222222222)/) + expect(call.url).to include('1234') + params = URIService.params(call.url) + expect(params[:locale]).to eq('fr') end context 'recording calls' do diff --git a/spec/services/basic_auth_url_spec.rb b/spec/services/basic_auth_url_spec.rb index f1d33ac5f63..a93255e0a34 100644 --- a/spec/services/basic_auth_url_spec.rb +++ b/spec/services/basic_auth_url_spec.rb @@ -9,18 +9,8 @@ expect(external_url).to eq(url) end - context 'with SP_NAME and SP_PASS set in the ENV' do - before do - ENV['SP_NAME'] = 'user' - ENV['SP_PASS'] = 'secret' - end - - after do - ENV.delete('SP_NAME') - ENV.delete('SP_PASS') - end - - it 'uses the values in the ENV' do + context 'with basic auth username and pass set in the config' do + it 'uses the values in from application.yml/Figaro' do url = 'https://foo.example.com/bar' external_url = BasicAuthUrl.build(url) From 395f69f16f82833a18c157f070fc38707c5ff2e0 Mon Sep 17 00:00:00 2001 From: Zach Margolis Date: Tue, 17 Oct 2017 13:59:28 -0400 Subject: [PATCH 3/3] Encrypt voice OTP code **Why**: URL params and GET params are in our logs, so this keeps plaintext codes out of our logs --- app/controllers/voice/otp_controller.rb | 14 +++++++-- app/jobs/voice_otp_sender_job.rb | 11 ++++++- config/routes.rb | 2 +- spec/controllers/voice/otp_controller_spec.rb | 31 ++++++++++--------- spec/jobs/voice_otp_sender_job_spec.rb | 4 ++- 5 files changed, 43 insertions(+), 19 deletions(-) diff --git a/app/controllers/voice/otp_controller.rb b/app/controllers/voice/otp_controller.rb index bae48d543ef..65a5d9b9436 100644 --- a/app/controllers/voice/otp_controller.rb +++ b/app/controllers/voice/otp_controller.rb @@ -16,8 +16,14 @@ def show protected + def encrypted_code + params[:encrypted_code].to_s + end + def code - params[:code].to_s + return unless encrypted_code.present? + + cipher.decrypt(encrypted_code) end def message @@ -37,10 +43,14 @@ def action_url BasicAuthUrl.build( voice_otp_url( - code: code, + encrypted_code: encrypted_code, repeat_count: repeat_count - 1 ) ) end + + def cipher + Gibberish::AES.new(Figaro.env.attribute_encryption_key) + end end end diff --git a/app/jobs/voice_otp_sender_job.rb b/app/jobs/voice_otp_sender_job.rb index 5e2bec9cb15..eb451232802 100644 --- a/app/jobs/voice_otp_sender_job.rb +++ b/app/jobs/voice_otp_sender_job.rb @@ -18,8 +18,17 @@ def otp_valid?(otp_created_at) def send_otp(twilio_service, code, phone) twilio_service.place_call( to: phone, - url: BasicAuthUrl.build(voice_otp_url(code: code, locale: locale_url_param)), + url: BasicAuthUrl.build( + voice_otp_url( + encrypted_code: cipher.encrypt(code), + locale: locale_url_param + ) + ), record: Figaro.env.twilio_record_voice == 'true' ) end + + def cipher + Gibberish::AES.new(Figaro.env.attribute_encryption_key) + end end diff --git a/config/routes.rb b/config/routes.rb index cb3293d8e0e..44b8b4464c8 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -17,7 +17,7 @@ match '/api/saml/auth' => 'saml_idp#auth', via: %i[get post] post '/api/service_provider' => 'service_provider#update' - match '/api/voice/otp/:code' => 'voice/otp#show', + match '/api/voice/otp' => 'voice/otp#show', via: [:get, :post], as: :voice_otp, defaults: { format: :xml } diff --git a/spec/controllers/voice/otp_controller_spec.rb b/spec/controllers/voice/otp_controller_spec.rb index 4f8baf5020f..419f7c78c50 100644 --- a/spec/controllers/voice/otp_controller_spec.rb +++ b/spec/controllers/voice/otp_controller_spec.rb @@ -4,23 +4,15 @@ describe '#show' do subject(:action) do get :show, - params: { code: code, repeat_count: repeat_count, locale: locale }, + params: { encrypted_code: encrypted_code, repeat_count: repeat_count, locale: locale }, format: :xml end - let(:code) { nil } let(:locale) { nil } let(:repeat_count) { nil } + let(:cipher) { Gibberish::AES.new(Figaro.env.attribute_encryption_key) } - context 'without a code in the URL' do - let(:code) { nil } - - it 'cannot route to the controller' do - expect { action }.to raise_error(ActionController::UrlGenerationError) - end - end - - context 'with a blank code in the URL' do - let(:code) { '' } + context 'with a blank encrypted_code in the URL' do + let(:encrypted_code) { '' } it 'renders a blank 400' do action @@ -30,10 +22,11 @@ end end - context 'with a code in the URL' do + context 'with an encrypted_code in the URL' do render_views - let(:code) { 1234 } + let(:code) { '1234' } + let(:encrypted_code) { cipher.encrypt(code) } it 'tells Twilio to the code with pauses in between' do action @@ -107,6 +100,16 @@ expect(gather[:action]).to include('repeat_count=4') end + it 'puts the encrypted code in the action' do + action + + doc = Nokogiri::XML(response.body) + gather = doc.css('Gather').first + params = URIService.params(gather[:action]) + + expect(cipher.decrypt(params[:encrypted_code])).to eq(code) + end + context 'when repeat_count counts down to 1' do let(:repeat_count) { 1 } diff --git a/spec/jobs/voice_otp_sender_job_spec.rb b/spec/jobs/voice_otp_sender_job_spec.rb index af586490990..294288723e7 100644 --- a/spec/jobs/voice_otp_sender_job_spec.rb +++ b/spec/jobs/voice_otp_sender_job_spec.rb @@ -8,6 +8,8 @@ FakeVoiceCall.calls = [] end + let(:cipher) { Gibberish::AES.new(Figaro.env.attribute_encryption_key) } + it 'initiates the phone call to deliver the OTP', twilio: true do I18n.with_locale(:fr) do VoiceOtpSenderJob.perform_now( @@ -24,8 +26,8 @@ expect(call.to).to eq('555-5555') expect(call.from).to match(/(\+19999999999|\+12222222222)/) - expect(call.url).to include('1234') params = URIService.params(call.url) + expect(cipher.decrypt(params[:encrypted_code])).to eq('1234') expect(params[:locale]).to eq('fr') end