LG-11101: Support multiple valid MFA to satisfy authentication request (Part 2 of 2)

app/controllers/application_controller.rb

@@ -7,6 +7,7 @@ class ApplicationController < ActionController::Base
   include VerifySpAttributesConcern
   include EffectiveUser
   include SecondMfaReminderConcern
+  include TwoFactorAuthenticatableMethods
 
   # Prevent CSRF attacks by raising an exception.
   # For APIs, you may want to use :null_session instead.
@@ -380,7 +381,7 @@ def service_provider_mfa_policy
     @service_provider_mfa_policy ||= ServiceProviderMfaPolicy.new(
       user: current_user,
       service_provider: sp_from_sp_session,
-      auth_method: user_session[:auth_method],
+      auth_methods_session:,
       aal_level_requested: sp_session[:aal_level_requested],
       piv_cac_requested: sp_session[:piv_cac_requested],
       phishing_resistant_requested: sp_session[:phishing_resistant_requested],

app/controllers/concerns/reauthentication_required_concern.rb

@@ -1,29 +1,20 @@
 module ReauthenticationRequiredConcern
   include MfaSetupConcern
+  include TwoFactorAuthenticatableMethods
 
   def confirm_recently_authenticated_2fa
-    return unless user_fully_authenticated?
-    non_remembered_device_authentication = user_session[:auth_method].present? &&
-                                           user_session[:auth_method] != 'remember_device'
-    return if recently_authenticated? && non_remembered_device_authentication
+    return if !user_fully_authenticated? || auth_methods_session.recently_authenticated_2fa?
 
     analytics.user_2fa_reauthentication_required(
-      auth_method: user_session[:auth_method],
-      authenticated_at: user_session[:authn_at],
+      auth_method: auth_methods_session.last_auth_event&.[](:auth_method),
+      authenticated_at: auth_methods_session.last_auth_event&.[](:at),
     )
 
     prompt_for_second_factor
   end
 
   private
 
-  def recently_authenticated?
-    return false if user_session.blank?
-    authn_at = user_session[:authn_at]
-    return false if authn_at.blank?
-    authn_at > Time.zone.now - IdentityConfig.store.reauthn_window
-  end
-
   def prompt_for_second_factor
     store_location(request.url)
     user_session[:context] = 'reauthentication'

app/controllers/concerns/remember_device_concern.rb

@@ -53,9 +53,7 @@ def revoke_remember_device(user)
   private
 
   def expired_for_interval?(user, interval)
-    unless user_session[:auth_method] == TwoFactorAuthenticatable::AuthMethod::REMEMBER_DEVICE
-      return false
-    end
+    return false unless has_remember_device_auth_event?
     remember_cookie = remember_device_cookie
     return true if remember_cookie.nil?
 
@@ -65,8 +63,13 @@ def expired_for_interval?(user, interval)
     )
   end
 
+  def has_remember_device_auth_event?
+    auth_methods_session.auth_events.any? do |auth_event|
+      auth_event[:auth_method] == TwoFactorAuthenticatable::AuthMethod::REMEMBER_DEVICE
+    end
+  end
+
   def handle_valid_remember_device_cookie(remember_device_cookie:)
-    user_session[:auth_method] = TwoFactorAuthenticatable::AuthMethod::REMEMBER_DEVICE
     mark_user_session_authenticated(
       auth_method: TwoFactorAuthenticatable::AuthMethod::REMEMBER_DEVICE,
       authentication_type: :device_remembered,

app/controllers/concerns/two_factor_authenticatable_methods.rb

@@ -149,13 +149,11 @@ def update_invalid_user
   end
 
   def handle_valid_verification_for_confirmation_context(auth_method:)
-    user_session[:auth_method] = auth_method
     mark_user_session_authenticated(auth_method:, authentication_type: :valid_2fa_confirmation)
     reset_second_factor_attempts_count
   end
 
   def handle_valid_verification_for_authentication_context(auth_method:)
-    user_session[:auth_method] = auth_method
     mark_user_session_authenticated(auth_method:, authentication_type: :valid_2fa)
     create_user_event(:sign_in_after_2fa)
 
@@ -167,8 +165,6 @@ def reset_second_factor_attempts_count
   end
 
   def mark_user_session_authenticated(auth_method:, authentication_type:)
-    user_session[TwoFactorAuthenticatable::NEED_AUTHENTICATION] = false
-    user_session[:authn_at] = Time.zone.now
     auth_methods_session.authenticate!(auth_method)
     mark_user_session_authenticated_analytics(authentication_type)
   end

app/policies/service_provider_mfa_policy.rb

@@ -1,17 +1,17 @@
 class ServiceProviderMfaPolicy
-  attr_reader :mfa_context, :auth_method, :service_provider
+  attr_reader :mfa_context, :auth_methods_session, :service_provider
 
   def initialize(
     user:,
     service_provider:,
-    auth_method:,
+    auth_methods_session:,
     aal_level_requested:,
     piv_cac_requested:,
     phishing_resistant_requested:
   )
     @user = user
     @mfa_context = MfaContext.new(user)
-    @auth_method = auth_method
+    @auth_methods_session = auth_methods_session
     @service_provider = service_provider
     @aal_level_requested = aal_level_requested
     @piv_cac_requested = piv_cac_requested
@@ -22,13 +22,18 @@ def user_needs_sp_auth_method_verification?
     # If the user needs to setup a new MFA method, return false so they go to
     # setup instead of verification
     return false if user_needs_sp_auth_method_setup?
+    return false if !piv_cac_required? && !phishing_resistant_required?
+    valid_auth_methods_for_sp_auth.blank?
+  end
 
+  def valid_auth_methods_for_sp_auth
+    all_auth_methods = auth_methods_session.auth_events.pluck(:auth_method)
     if piv_cac_required?
-      auth_method.to_s != TwoFactorAuthenticatable::AuthMethod::PIV_CAC
+      all_auth_methods & [TwoFactorAuthenticatable::AuthMethod::PIV_CAC]
     elsif phishing_resistant_required?
-      !TwoFactorAuthenticatable::AuthMethod.phishing_resistant?(auth_method)
+      all_auth_methods & TwoFactorAuthenticatable::AuthMethod::PHISHING_RESISTANT_METHODS.to_a
     else
-      false
+      all_auth_methods
     end
   end
 

app/services/auth_methods_session.rb

@@ -17,4 +17,15 @@ def authenticate!(auth_method)
   def auth_events
     user_session[:auth_events] || []
   end
+
+  def last_auth_event
+    auth_events.last
+  end
+
+  def recently_authenticated_2fa?
+    auth_events.any? do |auth_event|
+      auth_event[:auth_method] != TwoFactorAuthenticatable::AuthMethod::REMEMBER_DEVICE &&
+        auth_event[:at] > Time.zone.now - IdentityConfig.store.reauthn_window
+    end
+  end
 end

spec/controllers/concerns/reauthentication_required_concern_spec.rb

@@ -27,8 +27,15 @@ def index
     end
 
     context 'authenticated outside the authn window' do
+      let(:travel_time) { (IdentityConfig.store.reauthn_window + 1).seconds }
+
       before do
-        controller.user_session[:authn_at] -= IdentityConfig.store.reauthn_window
+        controller.auth_methods_session.authenticate!(TwoFactorAuthenticatable::AuthMethod::TOTP)
+        travel travel_time
+      end
+
+      around do |example|
+        freeze_time { example.run }
       end
 
       it 'redirects to 2FA options' do
@@ -44,12 +51,11 @@ def index
       end
 
       it 'records analytics' do
-        controller.user_session[:auth_method] = TwoFactorAuthenticatable::AuthMethod::TOTP
         stub_analytics
         expect(@analytics).to receive(:track_event).with(
           'User 2FA Reauthentication Required',
-          authenticated_at: controller.user_session[:authn_at],
-          auth_method: 'totp',
+          authenticated_at: travel_time.ago,
+          auth_method: TwoFactorAuthenticatable::AuthMethod::TOTP,
         )
         get :index
       end

spec/controllers/saml_idp_controller_spec.rb

@@ -832,7 +832,9 @@ def name_id_version(format_urn)
 
         it 'returns AAL3 authn_context when AAL3 is requested' do
           allow(controller).to receive(:user_session).and_return(
-            { auth_method: TwoFactorAuthenticatable::AuthMethod::PIV_CAC },
+            auth_events: [
+              { auth_method: TwoFactorAuthenticatable::AuthMethod::PIV_CAC, at: Time.zone.now },
+            ],
           )
           user = create(:user, :with_piv_or_cac)
           auth_settings = saml_settings(
@@ -849,7 +851,9 @@ def name_id_version(format_urn)
 
         it 'returns AAL3-HSPD12 authn_context when AAL3-HSPD12 is requested' do
           allow(controller).to receive(:user_session).and_return(
-            { auth_method: TwoFactorAuthenticatable::AuthMethod::PIV_CAC },
+            auth_events: [
+              { auth_method: TwoFactorAuthenticatable::AuthMethod::PIV_CAC, at: Time.zone.now },
+            ],
           )
           user = create(:user, :with_piv_or_cac)
           auth_settings = saml_settings(
@@ -866,7 +870,9 @@ def name_id_version(format_urn)
 
         it 'returns AAL2-HSPD12 authn_context when AAL2-HSPD12 is requested' do
           allow(controller).to receive(:user_session).and_return(
-            { auth_method: TwoFactorAuthenticatable::AuthMethod::PIV_CAC },
+            auth_events: [
+              { auth_method: TwoFactorAuthenticatable::AuthMethod::PIV_CAC, at: Time.zone.now },
+            ],
           )
           user = create(:user, :with_piv_or_cac)
           auth_settings = saml_settings(
@@ -883,7 +889,9 @@ def name_id_version(format_urn)
 
         it 'returns AAL2-phishing-resistant authn_context when AAL2-phishing-resistant requested' do
           allow(controller).to receive(:user_session).and_return(
-            { auth_method: TwoFactorAuthenticatable::AuthMethod::WEBAUTHN },
+            auth_events: [
+              { auth_method: TwoFactorAuthenticatable::AuthMethod::WEBAUTHN, at: Time.zone.now },
+            ],
           )
           user = create(:user, :with_webauthn)
           auth_settings = saml_settings(

spec/controllers/two_factor_authentication/backup_code_verification_controller_spec.rb

@@ -43,9 +43,6 @@
 
           post :create, params: payload
 
-          expect(subject.user_session[:auth_method]).to eq(
-            TwoFactorAuthenticatable::AuthMethod::BACKUP_CODE,
-          )
           expect(subject.user_session[:auth_events]).to eq(
             [
               auth_method: TwoFactorAuthenticatable::AuthMethod::BACKUP_CODE,
@@ -146,7 +143,6 @@
 
         expect(response).to render_template(:show)
         expect(flash[:error]).to eq t('two_factor_authentication.invalid_backup_code')
-        expect(subject.user_session[:auth_method]).to eq nil
         expect(subject.user_session[:auth_events]).to eq nil
         expect(subject.user_session[TwoFactorAuthenticatable::NEED_AUTHENTICATION]).to eq true
       end

spec/controllers/two_factor_authentication/otp_verification_controller_spec.rb

@@ -173,7 +173,6 @@
       end
 
       it 'does not set auth_method and requires 2FA' do
-        expect(controller.user_session[:auth_method]).to eq nil
         expect(controller.user_session[:auth_events]).to eq nil
         expect(controller.user_session[TwoFactorAuthenticatable::NEED_AUTHENTICATION]).to eq true
       end
@@ -301,7 +300,6 @@
             otp_delivery_preference: 'sms',
           }
 
-          expect(subject.user_session[:auth_method]).to eq TwoFactorAuthenticatable::AuthMethod::SMS
           expect(subject.user_session[:auth_events]).to eq(
             [
               {

spec/controllers/two_factor_authentication/personal_key_verification_controller_spec.rb

@@ -71,9 +71,6 @@
         freeze_time do
           post :create, params: payload
 
-          expect(subject.user_session[:auth_method]).to eq(
-            TwoFactorAuthenticatable::AuthMethod::PERSONAL_KEY,
-          )
           expect(subject.user_session[:auth_events]).to eq(
             [
               auth_method: TwoFactorAuthenticatable::AuthMethod::PERSONAL_KEY,
@@ -138,7 +135,6 @@
 
         post :create, params: payload
 
-        expect(subject.user_session[:auth_method]).to eq nil
         expect(subject.user_session[:auth_events]).to eq nil
         expect(subject.user_session[TwoFactorAuthenticatable::NEED_AUTHENTICATION]).to eq true
       end

spec/controllers/two_factor_authentication/piv_cac_verification_controller_spec.rb

@@ -69,9 +69,6 @@
         freeze_time do
           get :show, params: { token: 'good-token' }
 
-          expect(subject.user_session[:auth_method]).to eq(
-            TwoFactorAuthenticatable::AuthMethod::PIV_CAC,
-          )
           expect(subject.user_session[:auth_events]).to eq(
             [
               auth_method: TwoFactorAuthenticatable::AuthMethod::PIV_CAC,
@@ -163,7 +160,6 @@
       end
 
       it 'does not set auth_method and requires 2FA' do
-        expect(subject.user_session[:auth_method]).to eq nil
         expect(subject.user_session[:auth_events]).to eq nil
         expect(subject.user_session[TwoFactorAuthenticatable::NEED_AUTHENTICATION]).to eq true
       end

spec/controllers/two_factor_authentication/totp_verification_controller_spec.rb

@@ -24,9 +24,6 @@
           post :create, params: { code: generate_totp_code(@secret) }
 
           expect(response).to redirect_to account_path
-          expect(subject.user_session[:auth_method]).to eq(
-            TwoFactorAuthenticatable::AuthMethod::TOTP,
-          )
           expect(subject.user_session[:auth_events]).to eq(
             [
               auth_method: TwoFactorAuthenticatable::AuthMethod::TOTP,
@@ -112,7 +109,6 @@
       end
 
       it 'does not set auth_method and still requires 2FA' do
-        expect(subject.user_session[:auth_method]).to eq nil
         expect(subject.user_session[:auth_events]).to eq nil
         expect(subject.user_session[TwoFactorAuthenticatable::NEED_AUTHENTICATION]).to eq true
       end

spec/controllers/two_factor_authentication/webauthn_verification_controller_spec.rb

@@ -161,9 +161,6 @@
           freeze_time do
             patch :confirm, params: params
 
-            expect(subject.user_session[:auth_method]).to eq(
-              TwoFactorAuthenticatable::AuthMethod::WEBAUTHN,
-            )
             expect(subject.user_session[:auth_events]).to eq(
               [
                 auth_method: TwoFactorAuthenticatable::AuthMethod::WEBAUTHN,
@@ -200,9 +197,6 @@
 
             freeze_time do
               patch :confirm, params: params
-              expect(subject.user_session[:auth_method]).to eq(
-                TwoFactorAuthenticatable::AuthMethod::WEBAUTHN_PLATFORM,
-              )
               expect(subject.user_session[:auth_events]).to eq(
                 [
                   auth_method: TwoFactorAuthenticatable::AuthMethod::WEBAUTHN_PLATFORM,
@@ -267,7 +261,6 @@
         it 'redirects to webauthn show page' do
           patch :confirm, params: params
           expect(response).to redirect_to login_two_factor_webauthn_url(platform: true)
-          expect(subject.user_session[:auth_method]).to eq nil
           expect(subject.user_session[:auth_events]).to eq nil
           expect(subject.user_session[TwoFactorAuthenticatable::NEED_AUTHENTICATION]).to eq true
         end

spec/controllers/users/piv_cac_authentication_setup_controller_spec.rb

@@ -75,8 +75,6 @@
         allow(PivCacService).to receive(:decode_token).with(bad_token) { bad_token_response }
         allow(subject).to receive(:user_session).and_return(piv_cac_nonce: nonce)
         subject.user_session[:piv_cac_nickname] = nickname
-        subject.user_session[:authn_at] = Time.zone.now
-        subject.user_session[:auth_method] = TwoFactorAuthenticatable::AuthMethod::SMS
       end
 
       let(:nonce) { 'nonce' }

spec/controllers/users/piv_cac_login_controller_spec.rb

@@ -125,8 +125,6 @@
             expect(controller.user_session[TwoFactorAuthenticatable::NEED_AUTHENTICATION]).
               to eq false
 
-            expect(controller.user_session[:authn_at]).to_not be nil
-            expect(controller.user_session[:authn_at].class).to eq ActiveSupport::TimeWithZone
             expect(controller.auth_methods_session.auth_events).to match(
               [
                 {