Encrypt totp secret and add expiry to signed secret

Author: Taucher2003

app/models/user.rb

@@ -24,6 +24,9 @@ class User < ApplicationRecord
 
   validates :firstname, length: { maximum: 50 }
   validates :lastname, length: { maximum: 50 }
+
+  encrypts :totp_secret
+
   validates :totp_secret, length: { maximum: 32 }
 
   has_many :backup_codes, inverse_of: :user

app/services/users/mfa/totp/generate_secret_service.rb

@@ -25,7 +25,7 @@ def execute
                                   payload: {
                                     signed_secret: Rails.application
                                                         .message_verifier(:totp_secret)
-                                                        .generate(totp_secret),
+                                                        .generate(totp_secret, expires_in: 30.minutes),
                                     secret: totp_secret,
                                   })
         end

db/migrate/20260726000000_encrypt_totp_secret_on_users.rb

@@ -0,0 +1,12 @@
+# frozen_string_literal: true
+
+class EncryptTotpSecretOnUsers < Code0::ZeroTrack::Database::Migration[1.0]
+  def up
+    remove_index :users, :totp_secret, name: 'index_users_on_totp_secret'
+  end
+
+  def down
+    add_index :users, :totp_secret, unique: true, where: 'totp_secret IS NOT NULL',
+                                    name: 'index_users_on_totp_secret'
+  end
+end

db/schema_migrations/20260726000000

@@ -0,0 +1 @@
+3152386744f06e994087067a6e0367481df2ba73f4cff9a67401074b691f3833

db/structure.sql

@@ -1703,8 +1703,6 @@ CREATE UNIQUE INDEX "index_users_on_LOWER_email" ON users USING btree (lower(ema
 
 CREATE UNIQUE INDEX "index_users_on_LOWER_username" ON users USING btree (lower(username));
 
-CREATE UNIQUE INDEX index_users_on_totp_secret ON users USING btree (totp_secret) WHERE (totp_secret IS NOT NULL);
-
 ALTER TABLE ONLY node_parameters
     ADD CONSTRAINT fk_rails_0d79310cfa FOREIGN KEY (node_function_id) REFERENCES node_functions(id) ON DELETE CASCADE;
 

spec/requests/graphql/mutation/users/mfa/totp/generate_secret_mutation_spec.rb

@@ -31,7 +31,7 @@
 
       expect(secret).to be_present
 
-      signed_totp = Rails.application.message_verifier(:totp_secret).verify(signed_secret)
+      signed_totp = Rails.application.message_verifier(:totp_secret).verified(signed_secret)
       expect(signed_totp).to eq(secret)
     end
   end

spec/requests/graphql/mutation/users/mfa/totp/validate_secret_mutation_spec.rb

@@ -33,7 +33,7 @@
   context 'when user is valid' do
     let(:current_user) { create(:user) }
     let(:secret) { ROTP::Base32.random }
-    let(:signed_secret) { Rails.application.message_verifier(:totp_secret).generate(secret) }
+    let(:signed_secret) { Rails.application.message_verifier(:totp_secret).generate(secret, expires_in: 30.minutes) }
     let(:current_totp) { ROTP::TOTP.new(secret).now }
 
     it 'validates secret and enables totp' do

spec/services/users/mfa/totp/generate_secret_service_spec.rb

@@ -21,18 +21,31 @@
     end
 
     context 'when totp secret is not set' do
+      include ActiveSupport::Testing::TimeHelpers
+
       let(:current_user) { create(:user) }
 
       it { is_expected.to be_success }
 
-      it 'is valid totp secret' do
-        totp = ROTP::TOTP.new(service_response.payload[:signed_secret].split('--').first)
-        expect(totp.secret.length).to eq(48)
+      it 'returns a valid totp secret' do
+        secret = service_response.payload[:secret]
+        totp = ROTP::TOTP.new(secret)
+        expect(totp.now).to be_present
       end
 
       it 'returns a matching secret' do
-        signed_totp = Rails.application.message_verifier(:totp_secret).verify(service_response.payload[:signed_secret])
-        expect(signed_totp).to eq(service_response.payload[:secret])
+        signed_secret = Rails.application.message_verifier(:totp_secret).verified(
+          service_response.payload[:signed_secret]
+        )
+        expect(signed_secret).to eq(service_response.payload[:secret])
+      end
+
+      it 'returns a signed secret that expires' do
+        signed_secret = service_response.payload[:signed_secret]
+
+        travel_to 31.minutes.from_now do
+          expect(Rails.application.message_verifier(:totp_secret).verified(signed_secret)).to be_nil
+        end
       end
     end
   end

spec/services/users/mfa/totp/validate_secret_service_spec.rb

@@ -28,17 +28,37 @@
   context 'when user and secret is valid but totp is not' do
     let(:current_user) { create(:user) }
     let(:secret) { ROTP::Base32.random }
-    let(:signed_secret) { Rails.application.message_verifier(:totp_secret).generate(secret) }
+    let(:signed_secret) { Rails.application.message_verifier(:totp_secret).generate(secret, expires_in: 30.minutes) }
     let(:current_totp) { '00000' }
 
     it { is_expected.not_to be_success }
     it { is_expected.not_to create_audit_event }
   end
 
+  context 'when signed secret has expired' do
+    include ActiveSupport::Testing::TimeHelpers
+
+    # rubocop:disable RSpec/LetSetup -- referenced in the subject
+    let!(:current_user) { create(:user) }
+    let!(:secret) { ROTP::Base32.random }
+    let!(:signed_secret) do
+      Rails.application.message_verifier(:totp_secret).generate(secret, expires_in: 30.minutes)
+    end
+    let!(:current_totp) { ROTP::TOTP.new(secret).now }
+    # rubocop:enable RSpec/LetSetup
+
+    it 'returns invalid_totp_secret error' do
+      travel_to 31.minutes.from_now do
+        expect(service_response).not_to be_success
+        expect(service_response.payload[:error_code]).to eq(:invalid_totp_secret)
+      end
+    end
+  end
+
   context 'when user is valid and secret is valid and totp is valid' do
     let(:current_user) { create(:user) }
     let(:secret) { ROTP::Base32.random }
-    let(:signed_secret) { Rails.application.message_verifier(:totp_secret).generate(secret) }
+    let(:signed_secret) { Rails.application.message_verifier(:totp_secret).generate(secret, expires_in: 30.minutes) }
     let(:current_totp) { ROTP::TOTP.new(secret).now }
 
     it { is_expected.to be_success }