From 3e8d28f5002ca1fa6191294025baf05e289f9a63 Mon Sep 17 00:00:00 2001
From: Matt Hinz <matt.hinz@gsa.gov>
Date: Fri, 22 Mar 2024 11:21:47 -0700
Subject: [PATCH 1/2] Don't use stale document capture user ids for
 effective_user

If the user deletes their account, it is possible that the user id stored in the session for document capture will not be valid. This results in analytics events being improperly attributed to `anonymous-uuid`.

changelog: Bug Fixes, Identity verification, Fix log attribute issue associated with hybrid handoff.

Co-authored-by: Doug Price <douglas.price@gsa.gov>
---
 app/controllers/concerns/effective_user.rb    | 10 +++-
 .../concerns/effective_user_spec.rb           | 58 ++++++++++++++++---
 2 files changed, 59 insertions(+), 9 deletions(-)

diff --git a/app/controllers/concerns/effective_user.rb b/app/controllers/concerns/effective_user.rb
index f3edc90644c..28dd9d51059 100644
--- a/app/controllers/concerns/effective_user.rb
+++ b/app/controllers/concerns/effective_user.rb
@@ -1,7 +1,15 @@
 module EffectiveUser
   def effective_user
     return current_user if effective_user_id == current_user&.id
-    return User.find_by(id: effective_user_id) if effective_user_id
+
+    user = User.find_by(id: effective_user_id) if effective_user_id
+
+    if user.nil?
+      session.delete(:doc_capture_user_id)
+      return current_user
+    end
+
+    user
   end
 
   private
diff --git a/spec/controllers/concerns/effective_user_spec.rb b/spec/controllers/concerns/effective_user_spec.rb
index e2edd372999..b7c6dcf1004 100644
--- a/spec/controllers/concerns/effective_user_spec.rb
+++ b/spec/controllers/concerns/effective_user_spec.rb
@@ -14,6 +14,31 @@
       it 'returns nil' do
         expect(subject).to be_nil
       end
+
+      context 'with valid doc capture session user id' do
+        before do
+          session[:doc_capture_user_id] = user.id
+        end
+
+        it 'returns session user id' do
+          expect(subject).to eq user
+        end
+      end
+
+      context 'with invalid doc capture session user id' do
+        before do
+          session[:doc_capture_user_id] = -1
+        end
+
+        it 'returns session user id' do
+          expect(subject).to be_nil
+        end
+
+        it 'deletes the session key' do
+          subject
+          expect(session).not_to include(:doc_capture_user_id)
+        end
+      end
     end
 
     context 'non-existent user' do
@@ -22,23 +47,40 @@
       end
     end
 
-    context 'logged out with doc capture session user id' do
+    context 'logged in' do
       before do
-        session[:doc_capture_user_id] = user.id
+        stub_sign_in user
       end
 
       it 'returns session user id' do
         expect(subject).to eq user
       end
-    end
 
-    context 'logged in' do
-      before do
-        stub_sign_in user
+      context 'with valid doc capture session user id that is not the logged-in user' do
+        let(:doc_capture_user) { create(:user) }
+
+        before do
+          session[:doc_capture_user_id] = doc_capture_user.id
+        end
+
+        it 'returns doc capture user id' do
+          expect(subject).to eq(doc_capture_user)
+        end
       end
 
-      it 'returns session user id' do
-        expect(subject).to eq user
+      context 'with invalid doc capture session user id' do
+        before do
+          session[:doc_capture_user_id] = -1
+        end
+
+        it 'returns logged in user' do
+          expect(subject).to eql(user)
+        end
+
+        it 'deletes the session key' do
+          subject
+          expect(session).not_to include(:doc_capture_user_id)
+        end
       end
     end
   end

From f7ffa1924f63d578119e2fcbfa815befdf496fdb Mon Sep 17 00:00:00 2001
From: Matt Hinz <matt.hinz@gsa.gov>
Date: Fri, 22 Mar 2024 14:10:45 -0700
Subject: [PATCH 2/2] Update spec/controllers/concerns/effective_user_spec.rb

Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com>
---
 spec/controllers/concerns/effective_user_spec.rb | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/spec/controllers/concerns/effective_user_spec.rb b/spec/controllers/concerns/effective_user_spec.rb
index b7c6dcf1004..04e09132c24 100644
--- a/spec/controllers/concerns/effective_user_spec.rb
+++ b/spec/controllers/concerns/effective_user_spec.rb
@@ -78,8 +78,7 @@
         end
 
         it 'deletes the session key' do
-          subject
-          expect(session).not_to include(:doc_capture_user_id)
+          expect { subject }.to(change { session[:doc_capture_user_id] }.to(nil))
         end
       end
     end