[46618] Changing SiS statsd callback so that it uses actual user ial …

…instead of requested acr value in statsd and logged values (#10615) Co-authored-by: Trevor Bosaw <trevor.bosaw@oddball.io>
department-of-veterans-affairs · Sep 6, 2022 · c0b8cc7 · c0b8cc7
1 parent 64588ef
commit c0b8cc7
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 16 deletions.
diff --git a/app/controllers/v0/sign_in_controller.rb b/app/controllers/v0/sign_in_controller.rb
@@ -28,15 +28,16 @@ def authorize # rubocop:disable Metrics/MethodLength
       context = { type: type, client_id: client_id, acr: acr }
 
       sign_in_logger.info('authorize', context)
-      statsd_increment_acr(SignIn::Constants::Statsd::STATSD_SIS_AUTHORIZE_SUCCESS, context)
+      StatsD.increment(SignIn::Constants::Statsd::STATSD_SIS_AUTHORIZE_SUCCESS,
+                       tags: ["type:#{context[:type]}", "client_id:#{context[:client_id]}", "acr:#{context[:acr]}"])
 
       render body: auth_service(type).render_auth(state: state, acr: acr_for_type), content_type: 'text/html'
     rescue SignIn::Errors::StandardError => e
       sign_in_logger.info('authorize error', { errors: e.message, client_id: client_id, type: type, acr: acr })
       StatsD.increment(SignIn::Constants::Statsd::STATSD_SIS_AUTHORIZE_FAILURE)
       handle_pre_login_error(e, client_id)
     rescue => e
-      log_message_to_sentry(error.message, :error)
+      log_message_to_sentry(e.message, :error)
       StatsD.increment(SignIn::Constants::Statsd::STATSD_SIS_AUTHORIZE_FAILURE)
       handle_pre_login_error(e, client_id)
     end
@@ -73,7 +74,7 @@ def callback # rubocop:disable Metrics/MethodLength
       StatsD.increment(SignIn::Constants::Statsd::STATSD_SIS_CALLBACK_FAILURE)
       handle_pre_login_error(e, state_payload&.client_id)
     rescue => e
-      log_message_to_sentry(error.message, :error)
+      log_message_to_sentry(e.message, :error)
       StatsD.increment(SignIn::Constants::Statsd::STATSD_SIS_CALLBACK_FAILURE)
       handle_pre_login_error(e, state_payload&.client_id)
     end
@@ -260,11 +261,6 @@ def failed_auth_url(params)
       uri.to_s
     end
 
-    def statsd_increment_acr(statsd_code, context)
-      StatsD.increment(statsd_code,
-                       tags: ["type:#{context[:type]}", "client_id:#{context[:client_id]}", "acr:#{context[:acr]}"])
-    end
-
     def render_uplevel_credential(state_payload, state)
       acr_for_type = SignIn::AcrTranslator.new(acr: state_payload.acr, type: state_payload.type, uplevel: true).perform
       render body: auth_service(state_payload.type).render_auth(state: state, acr: acr_for_type),
@@ -277,9 +273,10 @@ def create_login_code(state_payload, user_info, credential_level, service_token_
       SignIn::CredentialInfoCreator.new(csp_user_attributes: user_attributes,
                                         csp_token_response: service_token_response).perform
       user_code_map = SignIn::UserCreator.new(user_attributes: user_attributes, state_payload: state_payload).perform
-      context = { type: state_payload.type, client_id: state_payload.client_id, acr: state_payload.acr }
+      context = { type: state_payload.type, client_id: state_payload.client_id, ial: credential_level.current_ial }
       sign_in_logger.info('callback', context)
-      statsd_increment_acr(SignIn::Constants::Statsd::STATSD_SIS_CALLBACK_SUCCESS, context)
+      StatsD.increment(SignIn::Constants::Statsd::STATSD_SIS_CALLBACK_SUCCESS,
+                       tags: ["type:#{context[:type]}", "client_id:#{context[:client_id]}", "ial:#{context[:ial]}"])
 
       redirect_to SignIn::LoginRedirectUrlGenerator.new(user_code_map: user_code_map).perform
     end

diff --git a/config/initializers/statsd.rb b/config/initializers/statsd.rb
@@ -85,8 +85,10 @@
       SignIn::Constants::Auth::ACR_VALUES.each do |acr|
         StatsD.increment(SignIn::Constants::Statsd::STATSD_SIS_AUTHORIZE_SUCCESS, 0,
                          tags: ["type:#{type}", "client_id:#{client_id}", "acr:#{acr}"])
+      end
+      [IAL::ONE, IAL::TWO].each do |ial|
         StatsD.increment(SignIn::Constants::Statsd::STATSD_SIS_CALLBACK_SUCCESS, 0,
-                         tags: ["type:#{type}", "client_id:#{client_id}", "acr:#{acr}"])
+                         tags: ["type:#{type}", "client_id:#{client_id}", "ial:#{ial}"])
       end
     end
   end

diff --git a/spec/controllers/v0/sign_in_controller_spec.rb b/spec/controllers/v0/sign_in_controller_spec.rb
@@ -423,7 +423,7 @@
     let(:state_value) { 'some-state' }
     let(:code_value) { 'some-code' }
     let(:error_value) { 'some-error' }
-    let(:statsd_tags) { ["type:#{type}", "client_id:#{client_id}", "acr:#{acr}"] }
+    let(:statsd_tags) { ["type:#{type}", "client_id:#{client_id}", "ial:#{ial}"] }
     let(:type) {}
     let(:acr) { nil }
     let(:client_id) { nil }
@@ -629,6 +629,7 @@
 
             context 'and credential should not be uplevelled' do
               let(:acr) { 'ial2' }
+              let(:ial) { 2 }
               let(:client_code) { 'some-client-code' }
               let(:expected_url) do
                 "#{Settings.sign_in.client_redirect_uris.mobile}?code=#{client_code}&state=#{client_state}&type=#{type}"
@@ -639,7 +640,7 @@
                 {
                   type: type,
                   client_id: client_id,
-                  acr: acr
+                  ial: ial
                 }
               end
               let(:expected_user_attributes) do
@@ -767,6 +768,7 @@
 
             context 'and credential should not be uplevelled' do
               let(:acr) { 'loa3' }
+              let(:ial) { 2 }
               let(:credential_ial) { LOA::IDME_CLASSIC_LOA3 }
               let(:client_code) { 'some-client-code' }
               let(:expected_url) do
@@ -778,7 +780,7 @@
                 {
                   type: type,
                   client_id: client_id,
-                  acr: acr
+                  ial: ial
                 }
               end
 
@@ -869,6 +871,7 @@
             let(:response) { OpenStruct.new(access_token: token) }
             let(:level_of_assurance) { LOA::THREE }
             let(:acr) { 'loa3' }
+            let(:ial) { 2 }
             let(:credential_ial) { LOA::IDME_CLASSIC_LOA3 }
             let(:client_code) { 'some-client-code' }
             let(:expected_url) do
@@ -880,7 +883,7 @@
               {
                 type: type,
                 client_id: client_id,
-                acr: acr
+                ial: ial
               }
             end
 
@@ -914,6 +917,7 @@
 
             context 'and dslogon account is not premium' do
               let(:dslogon_assurance) { 'some-dslogon-assurance' }
+              let(:ial) { 1 }
               let(:expected_user_attributes) do
                 {
                   ssn: nil,
@@ -930,6 +934,7 @@
 
             context 'and dslogon account is premium' do
               let(:dslogon_assurance) { LOA::DSLOGON_ASSURANCE_THREE }
+              let(:ial) { 2 }
               let(:expected_user_attributes) do
                 {
                   ssn: user_info.dslogon_idvalue,
@@ -988,6 +993,7 @@
             let(:response) { OpenStruct.new(access_token: token) }
             let(:level_of_assurance) { LOA::THREE }
             let(:acr) { 'loa3' }
+            let(:ial) { 2 }
             let(:credential_ial) { LOA::IDME_CLASSIC_LOA3 }
             let(:client_code) { 'some-client-code' }
             let(:expected_url) do
@@ -999,7 +1005,7 @@
               {
                 type: type,
                 client_id: client_id,
-                acr: acr
+                ial: ial
               }
             end
 
@@ -1033,6 +1039,7 @@
 
             context 'and mhv account is not premium' do
               let(:mhv_assurance) { 'some-mhv-assurance' }
+              let(:ial) { 1 }
               let(:expected_user_attributes) do
                 {
                   mhv_correlation_id: nil,
@@ -1045,6 +1052,7 @@
 
             context 'and mhv account is premium' do
               let(:mhv_assurance) { 'Premium' }
+              let(:ial) { 2 }
               let(:expected_user_attributes) do
                 {
                   mhv_correlation_id: user_info.mhv_uuid,