From eb13f0fcf864e654c099dd4ad8003ec4a0ab6a82 Mon Sep 17 00:00:00 2001
From: Malick Diarra <malick.diarra@gsa.gov>
Date: Fri, 26 Apr 2024 09:24:53 -0400
Subject: [PATCH 1/9] changelog: Internal, Logging, Adding dn uuid to logging
 dn configuration for easier cross pki logging

---
 app/forms/user_piv_cac_verification_form.rb | 1 +
 1 file changed, 1 insertion(+)

diff --git a/app/forms/user_piv_cac_verification_form.rb b/app/forms/user_piv_cac_verification_form.rb
index d479f2c6e68..90dfdc15057 100644
--- a/app/forms/user_piv_cac_verification_form.rb
+++ b/app/forms/user_piv_cac_verification_form.rb
@@ -57,6 +57,7 @@ def extra_analytics_attributes
     {
       multi_factor_auth_method: 'piv_cac',
       piv_cac_configuration_id: piv_cac_configuration&.id,
+      piv_cac_configuration_dn_uuid: x509_dn_uuid,
       multi_factor_auth_method_created_at: piv_cac_configuration&.created_at&.strftime('%s%L'),
     }
   end

From f71cb282433838dbf11771c0b2d04514655d5e53 Mon Sep 17 00:00:00 2001
From: Malick Diarra <malick.diarra@gsa.gov>
Date: Fri, 26 Apr 2024 09:30:32 -0400
Subject: [PATCH 2/9] include user uuid to query

---
 .../piv_cac_verification_controller.rb                        | 1 +
 app/services/piv_cac_service.rb                               | 4 ++--
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/app/controllers/two_factor_authentication/piv_cac_verification_controller.rb b/app/controllers/two_factor_authentication/piv_cac_verification_controller.rb
index 39b89f881fa..cc61b400d26 100644
--- a/app/controllers/two_factor_authentication/piv_cac_verification_controller.rb
+++ b/app/controllers/two_factor_authentication/piv_cac_verification_controller.rb
@@ -22,6 +22,7 @@ def redirect_to_piv_cac_service
       redirect_to PivCacService.piv_cac_service_link(
         nonce: piv_cac_nonce,
         redirect_uri: login_two_factor_piv_cac_url,
+        uuid: current_user.uuid,
       ), allow_other_host: true
     end
 
diff --git a/app/services/piv_cac_service.rb b/app/services/piv_cac_service.rb
index f85279f4a22..6562d3361be 100644
--- a/app/services/piv_cac_service.rb
+++ b/app/services/piv_cac_service.rb
@@ -14,14 +14,14 @@ def decode_token(token)
         token_decoded(token)
     end
 
-    def piv_cac_service_link(nonce:, redirect_uri:)
+    def piv_cac_service_link(nonce:, redirect_uri:, uuid:)
       uri = if FeatureManagement.development_and_identity_pki_disabled?
               URI(test_piv_cac_entry_url)
             else
               URI(randomize_uri(IdentityConfig.store.piv_cac_service_url))
             end
       # add the nonce and redirect uri
-      uri.query = { nonce: nonce, redirect_uri: redirect_uri }.to_query
+      uri.query = { nonce: nonce, redirect_uri: redirect_uri, uuid: uuid }.to_query
       uri.to_s
     end
 

From c1e32a65ebf66e14a3ca2f9db31e22ebc61efee2 Mon Sep 17 00:00:00 2001
From: Malick Diarra <malick.diarra@gsa.gov>
Date: Mon, 29 Apr 2024 08:58:12 -0400
Subject: [PATCH 3/9] use key id

---
 .../piv_cac_verification_controller.rb                        | 1 -
 app/forms/concerns/piv_cac_form_helpers.rb                    | 3 +--
 app/forms/user_piv_cac_verification_form.rb                   | 3 ++-
 app/services/piv_cac_service.rb                               | 4 ++--
 4 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/app/controllers/two_factor_authentication/piv_cac_verification_controller.rb b/app/controllers/two_factor_authentication/piv_cac_verification_controller.rb
index cc61b400d26..39b89f881fa 100644
--- a/app/controllers/two_factor_authentication/piv_cac_verification_controller.rb
+++ b/app/controllers/two_factor_authentication/piv_cac_verification_controller.rb
@@ -22,7 +22,6 @@ def redirect_to_piv_cac_service
       redirect_to PivCacService.piv_cac_service_link(
         nonce: piv_cac_nonce,
         redirect_uri: login_two_factor_piv_cac_url,
-        uuid: current_user.uuid,
       ), allow_other_host: true
     end
 
diff --git a/app/forms/concerns/piv_cac_form_helpers.rb b/app/forms/concerns/piv_cac_form_helpers.rb
index 348d035cab0..f64b294d8f2 100644
--- a/app/forms/concerns/piv_cac_form_helpers.rb
+++ b/app/forms/concerns/piv_cac_form_helpers.rb
@@ -18,9 +18,9 @@ def token_decoded
 
   def not_error_token
     possible_error = @data['error']
+    self.key_id = @data['key_id']
     if possible_error
       self.error_type = possible_error
-      self.key_id = @data['key_id']
       false
     else
       self.x509_dn_uuid = @data['uuid']
@@ -35,7 +35,6 @@ def token_has_correct_nonce
       true
     else
       self.error_type = 'token.invalid'
-      self.key_id = @data['key_id']
       false
     end
   end
diff --git a/app/forms/user_piv_cac_verification_form.rb b/app/forms/user_piv_cac_verification_form.rb
index 90dfdc15057..3b7f8136e76 100644
--- a/app/forms/user_piv_cac_verification_form.rb
+++ b/app/forms/user_piv_cac_verification_form.rb
@@ -18,7 +18,7 @@ def submit
     FormResponse.new(
       success: success,
       errors: errors,
-      extra: extra_analytics_attributes.merge(error_type ? { key_id: key_id } : {}),
+      extra: extra_analytics_attributes,
     )
   end
 
@@ -58,6 +58,7 @@ def extra_analytics_attributes
       multi_factor_auth_method: 'piv_cac',
       piv_cac_configuration_id: piv_cac_configuration&.id,
       piv_cac_configuration_dn_uuid: x509_dn_uuid,
+      cert_key_id: key_id,
       multi_factor_auth_method_created_at: piv_cac_configuration&.created_at&.strftime('%s%L'),
     }
   end
diff --git a/app/services/piv_cac_service.rb b/app/services/piv_cac_service.rb
index 6562d3361be..f85279f4a22 100644
--- a/app/services/piv_cac_service.rb
+++ b/app/services/piv_cac_service.rb
@@ -14,14 +14,14 @@ def decode_token(token)
         token_decoded(token)
     end
 
-    def piv_cac_service_link(nonce:, redirect_uri:, uuid:)
+    def piv_cac_service_link(nonce:, redirect_uri:)
       uri = if FeatureManagement.development_and_identity_pki_disabled?
               URI(test_piv_cac_entry_url)
             else
               URI(randomize_uri(IdentityConfig.store.piv_cac_service_url))
             end
       # add the nonce and redirect uri
-      uri.query = { nonce: nonce, redirect_uri: redirect_uri, uuid: uuid }.to_query
+      uri.query = { nonce: nonce, redirect_uri: redirect_uri }.to_query
       uri.to_s
     end
 

From 536c54c2866a544582252ed220a066e715fce06d Mon Sep 17 00:00:00 2001
From: Malick Diarra <malick.diarra@gsa.gov>
Date: Mon, 29 Apr 2024 09:29:31 -0400
Subject: [PATCH 4/9] update rspec so it accounts for key id

---
 spec/forms/user_piv_cac_login_form_spec.rb        |  3 ++-
 spec/forms/user_piv_cac_verification_form_spec.rb | 11 +++++++----
 2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/spec/forms/user_piv_cac_login_form_spec.rb b/spec/forms/user_piv_cac_login_form_spec.rb
index 8c05c4e6e5d..22af2b9ac80 100644
--- a/spec/forms/user_piv_cac_login_form_spec.rb
+++ b/spec/forms/user_piv_cac_login_form_spec.rb
@@ -22,6 +22,7 @@
           'uuid' => piv_cac_configuration.x509_dn_uuid,
           'subject' => 'x509-subject',
           'nonce' => nonce,
+          'key_id' => 'foo',
         }
       end
 
@@ -30,7 +31,7 @@
 
         expect(result.success?).to eq true
         expect(result.errors).to eq({})
-        expect(result.extra).to eq({ key_id: nil })
+        expect(result.extra).to eq({ key_id: 'foo' })
       end
     end
 
diff --git a/spec/forms/user_piv_cac_verification_form_spec.rb b/spec/forms/user_piv_cac_verification_form_spec.rb
index 8318e777108..34c276cbded 100644
--- a/spec/forms/user_piv_cac_verification_form_spec.rb
+++ b/spec/forms/user_piv_cac_verification_form_spec.rb
@@ -19,6 +19,7 @@
           'uuid' => x509_dn_uuid,
           'subject' => 'x509-subject',
           'nonce' => nonce,
+          'key_id' => 'foo',
         }
       end
 
@@ -33,7 +34,7 @@
             multi_factor_auth_method: 'piv_cac',
             piv_cac_configuration_id: nil,
             multi_factor_auth_method_created_at: nil,
-            key_id: nil,
+            key_id: 'foo',
           )
 
           expect(form.error_type).to eq 'user.no_piv_cac_associated'
@@ -52,7 +53,7 @@
             multi_factor_auth_method: 'piv_cac',
             multi_factor_auth_method_created_at: nil,
             piv_cac_configuration_id: nil,
-            key_id: nil,
+            key_id: 'foo',
           )
           expect(form.error_type).to eq 'user.piv_cac_mismatch'
         end
@@ -72,6 +73,7 @@
             multi_factor_auth_method: 'piv_cac',
             piv_cac_configuration_id: piv_cac_configuration.id,
             multi_factor_auth_method_created_at: piv_cac_configuration.created_at.strftime('%s%L'),
+            key_id: 'foo',
           )
         end
 
@@ -101,7 +103,7 @@
     context 'when token is invalid' do
       let(:token) { 'bad-token' }
       let(:token_response) do
-        { 'error' => 'token.bad', 'nonce' => nonce }
+        { 'error' => 'token.bad', 'nonce' => nonce, key_id: 'foo' }
       end
 
       it 'returns FormResponse with success: false' do
@@ -114,7 +116,7 @@
           multi_factor_auth_method: 'piv_cac',
           multi_factor_auth_method_created_at: nil,
           piv_cac_configuration_id: nil,
-          key_id: nil,
+          key_id: 'foo',
         )
       end
     end
@@ -132,6 +134,7 @@
           multi_factor_auth_method: 'piv_cac',
           multi_factor_auth_method_created_at: nil,
           piv_cac_configuration_id: nil,
+          key_id: nil,
         )
       end
     end

From 3d9b995deef6ff353c2204d97ec697e912e6b7c5 Mon Sep 17 00:00:00 2001
From: Malick Diarra <malick.diarra@gsa.gov>
Date: Mon, 29 Apr 2024 10:43:13 -0400
Subject: [PATCH 5/9] update rspec and fix tests

---
 app/forms/concerns/piv_cac_form_helpers.rb            |  1 +
 app/forms/user_piv_cac_setup_form.rb                  |  3 ++-
 app/forms/user_piv_cac_verification_form.rb           |  2 +-
 .../piv_cac_verification_controller_spec.rb           | 11 ++++++++++-
 spec/forms/user_piv_cac_verification_form_spec.rb     | 10 ++++++++--
 5 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/app/forms/concerns/piv_cac_form_helpers.rb b/app/forms/concerns/piv_cac_form_helpers.rb
index f64b294d8f2..6c37b4e3907 100644
--- a/app/forms/concerns/piv_cac_form_helpers.rb
+++ b/app/forms/concerns/piv_cac_form_helpers.rb
@@ -35,6 +35,7 @@ def token_has_correct_nonce
       true
     else
       self.error_type = 'token.invalid'
+      self.key_id = @data['key_id']
       false
     end
   end
diff --git a/app/forms/user_piv_cac_setup_form.rb b/app/forms/user_piv_cac_setup_form.rb
index e99b965c497..955996dcd35 100644
--- a/app/forms/user_piv_cac_setup_form.rb
+++ b/app/forms/user_piv_cac_setup_form.rb
@@ -21,7 +21,7 @@ def submit
     FormResponse.new(
       success: success && process_valid_submission,
       errors: errors,
-      extra: extra_analytics_attributes.merge(error_type ? { key_id: key_id } : {}),
+      extra: extra_analytics_attributes,
     )
   end
 
@@ -61,6 +61,7 @@ def piv_cac_not_already_associated
   def extra_analytics_attributes
     {
       multi_factor_auth_method: 'piv_cac',
+      key_id: key_id,
     }
   end
 
diff --git a/app/forms/user_piv_cac_verification_form.rb b/app/forms/user_piv_cac_verification_form.rb
index 3b7f8136e76..269353cb4f6 100644
--- a/app/forms/user_piv_cac_verification_form.rb
+++ b/app/forms/user_piv_cac_verification_form.rb
@@ -58,7 +58,7 @@ def extra_analytics_attributes
       multi_factor_auth_method: 'piv_cac',
       piv_cac_configuration_id: piv_cac_configuration&.id,
       piv_cac_configuration_dn_uuid: x509_dn_uuid,
-      cert_key_id: key_id,
+      key_id: key_id,
       multi_factor_auth_method_created_at: piv_cac_configuration&.created_at&.strftime('%s%L'),
     }
   end
diff --git a/spec/controllers/two_factor_authentication/piv_cac_verification_controller_spec.rb b/spec/controllers/two_factor_authentication/piv_cac_verification_controller_spec.rb
index 6cf9579d712..520d84c26f3 100644
--- a/spec/controllers/two_factor_authentication/piv_cac_verification_controller_spec.rb
+++ b/spec/controllers/two_factor_authentication/piv_cac_verification_controller_spec.rb
@@ -25,24 +25,28 @@
       'subject' => x509_subject,
       'issuer' => x509_issuer,
       'nonce' => nonce,
+      'key_id' => 'foo',
     )
     allow(PivCacService).to receive(:decode_token).with('good-other-token').and_return(
       'uuid' => user.piv_cac_configurations.first.x509_dn_uuid + 'X',
       'subject' => x509_subject + 'X',
       'issuer' => x509_issuer,
       'nonce' => nonce,
+      'key_id' => 'foo',
     )
     allow(PivCacService).to receive(:decode_token).with('bad-token').and_return(
       'uuid' => 'bad-uuid',
       'subject' => bad_dn,
       'issuer' => x509_issuer,
       'nonce' => nonce,
+      'key_id' => 'foo',
     )
     allow(PivCacService).to receive(:decode_token).with('bad-nonce').and_return(
       'uuid' => user.piv_cac_configurations.first.x509_dn_uuid,
       'subject' => x509_subject,
       'issuer' => x509_issuer,
       'nonce' => 'bad-' + nonce,
+      'key_id' => 'foo',
     )
   end
 
@@ -119,6 +123,8 @@
           new_device: nil,
           multi_factor_auth_method_created_at: cfg.created_at.strftime('%s%L'),
           piv_cac_configuration_id: cfg.id,
+          piv_cac_configuration_dn_uuid: cfg.x509_dn_uuid,
+          key_id: 'foo',
         }
         expect(@analytics).to receive(:track_mfa_submit_event).
           with(submit_attributes)
@@ -154,6 +160,8 @@
             new_device: false,
             multi_factor_auth_method_created_at: cfg.created_at.strftime('%s%L'),
             piv_cac_configuration_id: cfg.id,
+            piv_cac_configuration_dn_uuid: cfg.x509_dn_uuid,
+            key_id: 'foo',
           }
           expect(@analytics).to receive(:track_mfa_submit_event).
             with(submit_attributes)
@@ -264,7 +272,8 @@
           multi_factor_auth_method: 'piv_cac',
           multi_factor_auth_method_created_at: nil,
           new_device: nil,
-          key_id: nil,
+          key_id: 'foo',
+          piv_cac_configuration_dn_uuid: 'bad-uuid',
           piv_cac_configuration_id: nil,
         }
         expect(@analytics).to receive(:track_mfa_submit_event).
diff --git a/spec/forms/user_piv_cac_verification_form_spec.rb b/spec/forms/user_piv_cac_verification_form_spec.rb
index 34c276cbded..9a1abf4da36 100644
--- a/spec/forms/user_piv_cac_verification_form_spec.rb
+++ b/spec/forms/user_piv_cac_verification_form_spec.rb
@@ -34,7 +34,8 @@
             multi_factor_auth_method: 'piv_cac',
             piv_cac_configuration_id: nil,
             multi_factor_auth_method_created_at: nil,
-            key_id: 'foo',
+            piv_cac_configuration_dn_uuid: nil,
+            key_id: nil,
           )
 
           expect(form.error_type).to eq 'user.no_piv_cac_associated'
@@ -53,6 +54,7 @@
             multi_factor_auth_method: 'piv_cac',
             multi_factor_auth_method_created_at: nil,
             piv_cac_configuration_id: nil,
+            piv_cac_configuration_dn_uuid: 'some-random-uuid',
             key_id: 'foo',
           )
           expect(form.error_type).to eq 'user.piv_cac_mismatch'
@@ -74,6 +76,7 @@
             piv_cac_configuration_id: piv_cac_configuration.id,
             multi_factor_auth_method_created_at: piv_cac_configuration.created_at.strftime('%s%L'),
             key_id: 'foo',
+            piv_cac_configuration_dn_uuid: x509_dn_uuid,
           )
         end
 
@@ -90,6 +93,7 @@
               multi_factor_auth_method: 'piv_cac',
               piv_cac_configuration_id: nil,
               multi_factor_auth_method_created_at: nil,
+              piv_cac_configuration_dn_uuid: nil,
               key_id: nil,
             )
 
@@ -115,8 +119,9 @@
           errors: { type: 'token.bad' },
           multi_factor_auth_method: 'piv_cac',
           multi_factor_auth_method_created_at: nil,
+          piv_cac_configuration_dn_uuid: nil,
           piv_cac_configuration_id: nil,
-          key_id: 'foo',
+          key_id: nil,
         )
       end
     end
@@ -134,6 +139,7 @@
           multi_factor_auth_method: 'piv_cac',
           multi_factor_auth_method_created_at: nil,
           piv_cac_configuration_id: nil,
+          piv_cac_configuration_dn_uuid: nil,
           key_id: nil,
         )
       end

From 555509a1b5a88356deebfaa4ac47fd66caef81ea Mon Sep 17 00:00:00 2001
From: Malick Diarra <malick.diarra@gsa.gov>
Date: Tue, 30 Apr 2024 10:13:55 -0400
Subject: [PATCH 6/9] fix spec

---
 spec/forms/user_piv_cac_setup_form_spec.rb | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/spec/forms/user_piv_cac_setup_form_spec.rb b/spec/forms/user_piv_cac_setup_form_spec.rb
index 9b2b0871162..41130f6ef03 100644
--- a/spec/forms/user_piv_cac_setup_form_spec.rb
+++ b/spec/forms/user_piv_cac_setup_form_spec.rb
@@ -20,12 +20,13 @@
           'uuid' => x509_dn_uuid,
           'subject' => 'x509-subject',
           'nonce' => nonce,
+          'key_id' => 'foo',
         }
       end
 
       it 'returns FormResponse with success: true' do
         result = instance_double(FormResponse)
-        extra = { multi_factor_auth_method: 'piv_cac' }
+        extra = { multi_factor_auth_method: 'piv_cac', key_id: 'foo' }
 
         expect(FormResponse).to receive(:new).
           with(success: true, errors: {}, extra: extra).and_return(result)
@@ -47,7 +48,7 @@
 
         it 'returns FormResponse with success: true' do
           result = instance_double(FormResponse)
-          extra = { multi_factor_auth_method: 'piv_cac' }
+          extra = { multi_factor_auth_method: 'piv_cac', key_id: 'foo' }
 
           expect(FormResponse).to receive(:new).
             with(success: true, errors: {}, extra: extra).and_return(result)
@@ -62,7 +63,7 @@
 
         it 'returns FormResponse with success: false' do
           result = instance_double(FormResponse)
-          extra = { multi_factor_auth_method: 'piv_cac', key_id: nil }
+          extra = { multi_factor_auth_method: 'piv_cac', key_id: 'foo' }
 
           expect(FormResponse).to receive(:new).
             with(success: false, errors: { type: 'piv_cac.already_associated' },
@@ -115,7 +116,7 @@
 
       it 'returns FormResponse with success: false' do
         result = instance_double(FormResponse)
-        extra = { multi_factor_auth_method: 'piv_cac' }
+        extra = { multi_factor_auth_method: 'piv_cac', key_id: nil }
 
         expect(FormResponse).to receive(:new).
           with(success: false, errors: {}, extra: extra).and_return(result)

From 17b4375935452d1fe75fee2f83294b51830b3dc5 Mon Sep 17 00:00:00 2001
From: Malick Diarra <malick.diarra@gsa.gov>
Date: Thu, 2 May 2024 09:48:43 -0400
Subject: [PATCH 7/9] make sure to still send foo

---
 spec/forms/user_piv_cac_verification_form_spec.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/spec/forms/user_piv_cac_verification_form_spec.rb b/spec/forms/user_piv_cac_verification_form_spec.rb
index 9a1abf4da36..e82748379f1 100644
--- a/spec/forms/user_piv_cac_verification_form_spec.rb
+++ b/spec/forms/user_piv_cac_verification_form_spec.rb
@@ -94,7 +94,7 @@
               piv_cac_configuration_id: nil,
               multi_factor_auth_method_created_at: nil,
               piv_cac_configuration_dn_uuid: nil,
-              key_id: nil,
+              key_id: 'foo',
             )
 
             expect(Event).to_not receive(:create)

From 37f330de8a38c6474a94c34c878093b15680e07b Mon Sep 17 00:00:00 2001
From: Malick Diarra <malick.diarra@gsa.gov>
Date: Fri, 3 May 2024 11:43:59 -0400
Subject: [PATCH 8/9] move setting up key id to when we decode token

---
 app/forms/concerns/piv_cac_form_helpers.rb | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/app/forms/concerns/piv_cac_form_helpers.rb b/app/forms/concerns/piv_cac_form_helpers.rb
index 6c37b4e3907..b9c860d08d1 100644
--- a/app/forms/concerns/piv_cac_form_helpers.rb
+++ b/app/forms/concerns/piv_cac_form_helpers.rb
@@ -13,12 +13,12 @@ def valid_token?
 
   def token_decoded
     @data = PivCacService.decode_token(token)
+    self.key_id = @data['key_id']
     true
   end
 
   def not_error_token
     possible_error = @data['error']
-    self.key_id = @data['key_id']
     if possible_error
       self.error_type = possible_error
       false
@@ -35,7 +35,6 @@ def token_has_correct_nonce
       true
     else
       self.error_type = 'token.invalid'
-      self.key_id = @data['key_id']
       false
     end
   end

From 679246e4c9f1bda50f2dafb7c50067c983f52cf4 Mon Sep 17 00:00:00 2001
From: Malick Diarra <malick.diarra@gsa.gov>
Date: Fri, 3 May 2024 14:52:21 -0400
Subject: [PATCH 9/9] replace self

---
 app/forms/concerns/piv_cac_form_helpers.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/forms/concerns/piv_cac_form_helpers.rb b/app/forms/concerns/piv_cac_form_helpers.rb
index b9c860d08d1..f4b578b1805 100644
--- a/app/forms/concerns/piv_cac_form_helpers.rb
+++ b/app/forms/concerns/piv_cac_form_helpers.rb
@@ -13,7 +13,7 @@ def valid_token?
 
   def token_decoded
     @data = PivCacService.decode_token(token)
-    self.key_id = @data['key_id']
+    @key_id = @data['key_id']
     true
   end