Skip to content

Commit

Permalink
Merge pull request #1824 from 18F/mb-add-twilio-analytics
Browse files Browse the repository at this point in the history
Add analytics event for Twilio errors
  • Loading branch information
monfresh authored Nov 30, 2017
2 parents 61c94d4 + f348640 commit 3716fe2
Show file tree
Hide file tree
Showing 11 changed files with 81 additions and 7 deletions.
1 change: 1 addition & 0 deletions app/controllers/accounts_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ class AccountsController < ApplicationController
layout 'card_wide'

def show
analytics.track_event(Analytics::ACCOUNT_VISIT)
cacher = Pii::Cacher.new(current_user, user_session)

@view_model = AccountShow.new(
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/openid_connect/logout_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ def index

result = @logout_form.submit

analytics.track_event(Analytics::OPENID_CONNECT_LOGOUT, result.to_h.except(:redirect_uri))
analytics.track_event(Analytics::LOGOUT_INITIATED, result.to_h.except(:redirect_uri))

if (redirect_uri = result.extra[:redirect_uri])
sign_out
Expand Down
9 changes: 9 additions & 0 deletions app/controllers/saml_idp_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ def metadata
end

def logout
track_logout_event
prepare_saml_logout_response_and_request

return handle_saml_logout_response if slo.successful_saml_response?
Expand Down Expand Up @@ -64,4 +65,12 @@ def render_template_for(message, action_url, type)
layout: false
)
end

def track_logout_event
result = {
sp_initiated: params[:SAMLRequest].present?,
oidc: false,
}
analytics.track_event(Analytics::LOGOUT_INITIATED, result)
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ def send_code
private

def invalid_phone_number(exception)
analytics.track_event(Analytics::TWILIO_PHONE_VALIDATION_FAILED, error: exception.message)
flash[:error] = case exception.code
when TwilioService::SMS_ERROR_CODE
t('errors.messages.invalid_sms_number')
Expand Down
2 changes: 2 additions & 0 deletions app/forms/openid_connect_logout_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ def extra_analytics_attributes
{
client_id: service_provider.issuer,
redirect_uri: redirect_uri,
sp_initiated: true,
oidc: true,
}
end

Expand Down
6 changes: 4 additions & 2 deletions app/services/analytics.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ def uuid
user.uuid
end

ACCOUNT_VISIT = 'Account Page Visited'.freeze
EMAIL_AND_PASSWORD_AUTH = 'Email and Password Authentication'.freeze
EMAIL_CHANGE_REQUEST = 'Email Change Request'.freeze
EMAIL_CONFIRMATION = 'Email Confirmation'.freeze
Expand All @@ -55,16 +56,16 @@ def uuid
IDV_REVIEW_COMPLETE = 'IdV: review complete'.freeze
IDV_REVIEW_VISIT = 'IdV: review info visited'.freeze
INVALID_AUTHENTICITY_TOKEN = 'Invalid Authenticity Token'.freeze
OTP_DELIVERY_SELECTION = 'OTP: Delivery Selection'.freeze
LOGOUT_INITIATED = 'Logout Initiated'.freeze
MULTI_FACTOR_AUTH = 'Multi-Factor Authentication'.freeze
MULTI_FACTOR_AUTH_ENTER_OTP_VISIT = 'Multi-Factor Authentication: enter OTP visited'.freeze
MULTI_FACTOR_AUTH_MAX_ATTEMPTS = 'Multi-Factor Authentication: max attempts reached'.freeze
MULTI_FACTOR_AUTH_PHONE_SETUP = 'Multi-Factor Authentication: phone setup'.freeze
MULTI_FACTOR_AUTH_MAX_SENDS = 'Multi-Factor Authentication: max otp sends reached'.freeze
OPENID_CONNECT_BEARER_TOKEN = 'OpenID Connect: bearer token authentication'.freeze
OPENID_CONNECT_LOGOUT = 'OpenID Connect: logout'.freeze
OPENID_CONNECT_REQUEST_AUTHORIZATION = 'OpenID Connect: authorization request'.freeze
OPENID_CONNECT_TOKEN = 'OpenID Connect: token'.freeze
OTP_DELIVERY_SELECTION = 'OTP: Delivery Selection'.freeze
PASSWORD_CHANGED = 'Password Changed'.freeze
PASSWORD_CREATION = 'Password Creation'.freeze
PASSWORD_MAX_ATTEMPTS = 'Password Max Attempts Reached'.freeze
Expand All @@ -79,6 +80,7 @@ def uuid
SIGN_IN_PAGE_VISIT = 'Sign in page visited'.freeze
TOTP_SETUP = 'TOTP Setup'.freeze
TOTP_USER_DISABLED = 'TOTP: User Disabled TOTP'.freeze
TWILIO_PHONE_VALIDATION_FAILED = 'Twilio Phone Validation Failed'.freeze
USER_REGISTRATION_AGENCY_HANDOFF_PAGE_VISIT = 'User registration: agency handoff visited'.freeze
USER_REGISTRATION_AGENCY_HANDOFF_COMPLETE = 'User registration: agency handoff complete'.freeze
USER_REGISTRATION_EMAIL = 'User Registration: Email Submitted'.freeze
Expand Down
3 changes: 3 additions & 0 deletions spec/controllers/accounts_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
describe '#show' do
context 'when user has an active identity' do
it 'renders the profile and does not redirect out of the app' do
stub_analytics
user = create(:user, :signed_up)
user.identities << Identity.create(
service_provider: 'http://localhost:3000',
Expand All @@ -21,6 +22,8 @@

sign_in user

expect(@analytics).to receive(:track_event).with(Analytics::ACCOUNT_VISIT)

get :show

expect(response).to_not be_redirect
Expand Down
14 changes: 10 additions & 4 deletions spec/controllers/openid_connect/logout_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,12 @@
it 'tracks analytics' do
stub_analytics
expect(@analytics).to receive(:track_event).
with(Analytics::OPENID_CONNECT_LOGOUT,
success: true, client_id: service_provider, errors: {})
with(Analytics::LOGOUT_INITIATED,
success: true,
client_id: service_provider,
errors: {},
sp_initiated: true,
oidc: true)

action
end
Expand All @@ -77,10 +81,12 @@
it 'tracks analytics' do
stub_analytics
expect(@analytics).to receive(:track_event).
with(Analytics::OPENID_CONNECT_LOGOUT,
with(Analytics::LOGOUT_INITIATED,
success: false,
client_id: service_provider,
errors: hash_including(:post_logout_redirect_uri))
errors: hash_including(:post_logout_redirect_uri),
sp_initiated: true,
oidc: true)

action
end
Expand Down
19 changes: 19 additions & 0 deletions spec/controllers/saml_idp_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,25 @@
delete :logout
expect(subject.session[:foo]).to be_nil
end

it 'tracks the event when idp-initiated' do
stub_analytics
result = { sp_initiated: false, oidc: false }

expect(@analytics).to receive(:track_event).with(Analytics::LOGOUT_INITIATED, result)

delete :logout
end

it 'tracks the event when sp-initiated' do
allow(controller).to receive(:saml_request).and_return(FakeSamlRequest.new)
stub_analytics
result = { sp_initiated: true, oidc: false }

expect(@analytics).to receive(:track_event).with(Analytics::LOGOUT_INITIATED, result)

delete :logout, params: { SAMLRequest: 'foo' }
end
end

describe 'POST /api/saml/logout' do
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,29 @@ def index

expect(flash[:error]).to eq(failed_to_send_otp)
end

it 'records an analytics event when Twilio responds with an error' do
stub_analytics
twilio_error = Twilio::REST::RestError.new('error message', '', '400')
allow(SmsOtpSenderJob).to receive(:perform_now).and_raise(twilio_error)
analytics_hash = {
success: true,
errors: {},
otp_delivery_preference: 'sms',
resend: nil,
context: 'confirmation',
country_code: '1',
area_code: '202',
}

expect(@analytics).to receive(:track_event).
with(Analytics::OTP_DELIVERY_SELECTION, analytics_hash)

expect(@analytics).to receive(:track_event).
with(Analytics::TWILIO_PHONE_VALIDATION_FAILED, error: 'error message')

get :send_code, params: { otp_delivery_selection_form: { otp_delivery_preference: 'sms' } }
end
end

context 'when selecting an invalid delivery method' do
Expand Down
8 changes: 8 additions & 0 deletions spec/support/fake_saml_request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,12 @@ def valid?
def name_id_format
'urn:oasis:names:tc:SAML:2.0:nameid-format:persistent'
end

def response_url
'http://localhost:3000'
end

def request_id
'123'
end
end

0 comments on commit 3716fe2

Please sign in to comment.