Improve message when confirming totp device with potential clock skew

https://community.openproject.org/wp/45586

modules/two_factor_authentication/app/controllers/two_factor_authentication/my/two_factor_devices_controller.rb

@@ -79,7 +79,8 @@ module ::TwoFactorAuthentication
           flash[:notice] = t('two_factor_authentication.devices.registration_complete')
           return redirect_to action: :index
         elsif !result.success?
-          flash[:error] = t('two_factor_authentication.devices.registration_failed_token_invalid')
+          message = result.message || t('two_factor_authentication.devices.registration_failed_token_invalid')
+          flash[:error] = t('two_factor_authentication.devices.registration_failed_token', message:)
         else
           flash[:error] = t('two_factor_authentication.devices.registration_failed_update')
         end

modules/two_factor_authentication/config/locales/en.yml

@@ -100,7 +100,8 @@ en:
       make_default_failed: 'Failed to update the default 2FA device.'
       deletion_are_you_sure: 'Are you sure you want to delete this 2FA device?'
       registration_complete: '2FA device registration complete!'
-      registration_failed_token_invalid: '2FA device registration failed, the token was invalid.'
+      registration_failed_token_invalid: 'The token was invalid.'
+      registration_failed_token: '2FA device registration failed: %{message}'
       registration_failed_update: '2FA device registration failed, the token was valid but the device could not be updated.'
       confirm_send_failed: 'Confirmation of your 2FA device failed.'
       button_complete_registration: 'Complete 2FA registration'
@@ -117,6 +118,7 @@ en:
         secret_key: 'Secret key'
         time_based: 'Time based'
         account: 'Account name / Issuer'
+        otp_invalid_drift_notice: "Invalid one-time password. If this happens repeatedly, please make sure your device clock is in sync. Current server time in your zone is %{time}"
         setup: |
           For setting up two-factor authentication with Google Authenticator, download the application from the Apple App store or Google Play Store.
           After opening the app, you can scan the following QR code to register the device.

modules/two_factor_authentication/lib/open_project/two_factor_authentication/token_strategy/totp.rb

@@ -7,7 +7,7 @@ module OpenProject::TwoFactorAuthentication
         result = device.verify_token input_token
 
         # Token did not match value or surrounding drift
-        raise I18n.t(:notice_account_otp_invalid) unless result == true
+        raise verification_failed_message unless result == true
 
         result
       end
@@ -34,6 +34,15 @@ module OpenProject::TwoFactorAuthentication
 
       private
 
+      def verification_failed_message
+        if device.active?
+          I18n.t(:notice_account_otp_invalid)
+        else
+          I18n.t('two_factor_authentication.devices.totp.otp_invalid_drift_notice',
+                 time: Time.zone.now.in_time_zone(User.current.time_zone).strftime('%T'))
+        end
+      end
+
       def send_totp
         Rails.logger.info { "[2FA] ROTP in progress for #{user.login}" }
         # Nothing to do here

modules/two_factor_authentication/spec/controllers/two_factor_authentication/my/two_factor_devices_controller_spec.rb

@@ -171,7 +171,8 @@ describe TwoFactorAuthentication::My::TwoFactorDevicesController do
           it 'redirects to the confirmation on faulty entry' do
             post :confirm, params: { device_id: device.id, otp: '1234' }
             expect(response).to redirect_to action: :confirm, device_id: device.id
-            expect(flash[:error]).to include I18n.t('two_factor_authentication.devices.registration_failed_token_invalid')
+            expect(flash[:error]).to include '2FA device registration failed'
+            expect(flash[:error]).to include ' If this happens repeatedly, please make sure your device clock is in sync'
 
             device.reload
 

modules/two_factor_authentication/spec/features/my_two_factor_devices_spec.rb

@@ -57,8 +57,7 @@ describe 'My Account 2FA configuration',
 
     expect(page).to have_selector('h2', text: I18n.t('two_factor_authentication.devices.confirm_device'))
     expect(page).to have_selector('input#otp')
-    expect(page).to have_selector('.flash.error',
-                                  text: I18n.t('two_factor_authentication.devices.registration_failed_token_invalid'))
+    expect(page).to have_selector('.flash.error', text: '2FA device registration failed: Invalid one-time password.')
 
     # Fill in correct token
     fill_in 'otp', with: sms_token

modules/two_factor_authentication/spec/lib/token_strategies/totp_spec.rb

@@ -0,0 +1,34 @@
+require_relative '../../spec_helper'
+
+describe OpenProject::TwoFactorAuthentication::TokenStrategy::Totp do
+  shared_let(:user) { create(:user) }
+  let(:channel) { :totp }
+  let(:strategy) { described_class.new user:, device:, channel: }
+
+  before do
+    allow(device).to receive(:verify_token).and_return(verify_result)
+  end
+
+  describe 'verify failure' do
+    let(:verify_result) { false }
+
+    subject { strategy.verify 'input' }
+
+    context 'with active device' do
+      let(:device) { build(:two_factor_authentication_device_sms, user:, channel:, active: true) }
+
+      it 'raises a standard error on failure' do
+        expect { subject }.to raise_error('Invalid one-time password.')
+      end
+    end
+
+    context 'with inactive device' do
+      let(:device) { build(:two_factor_authentication_device_sms, user:, channel:, active: false) }
+
+      it 'raises an extended error on failure' do
+        expected = /If this happens repeatedly, please make sure your device clock is in sync/
+        expect { subject }.to raise_error(expected)
+      end
+    end
+  end
+end