From ce2481a81b8b781e06561c5a89ba2a3f178b246a Mon Sep 17 00:00:00 2001 From: David Roetzel Date: Tue, 1 Oct 2024 11:38:42 +0200 Subject: [PATCH] Move OTP secret length to configuration (#32125) --- .../otp_authentication_controller.rb | 2 +- app/models/user.rb | 3 ++- spec/controllers/auth/sessions_controller_spec.rb | 6 +++--- spec/requests/auth/sessions/security_key_options_spec.rb | 2 +- spec/system/oauth_spec.rb | 2 +- 5 files changed, 8 insertions(+), 7 deletions(-) diff --git a/app/controllers/settings/two_factor_authentication/otp_authentication_controller.rb b/app/controllers/settings/two_factor_authentication/otp_authentication_controller.rb index 0bff01ec2..ca8d46afe 100644 --- a/app/controllers/settings/two_factor_authentication/otp_authentication_controller.rb +++ b/app/controllers/settings/two_factor_authentication/otp_authentication_controller.rb @@ -15,7 +15,7 @@ module Settings end def create - session[:new_otp_secret] = User.generate_otp_secret(32) + session[:new_otp_secret] = User.generate_otp_secret redirect_to new_settings_two_factor_authentication_confirmation_path end diff --git a/app/models/user.rb b/app/models/user.rb index fcb0eced7..c32a575ed 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -71,7 +71,8 @@ class User < ApplicationRecord ACTIVE_DURATION = ENV.fetch('USER_ACTIVE_DAYS', 7).to_i.days.freeze devise :two_factor_authenticatable, - otp_secret_encryption_key: Rails.configuration.x.otp_secret + otp_secret_encryption_key: Rails.configuration.x.otp_secret, + otp_secret_length: 32 include LegacyOtpSecret # Must be after the above `devise` line in order to override the legacy method diff --git a/spec/controllers/auth/sessions_controller_spec.rb b/spec/controllers/auth/sessions_controller_spec.rb index 713ea3ff1..4a6956cb0 100644 --- a/spec/controllers/auth/sessions_controller_spec.rb +++ b/spec/controllers/auth/sessions_controller_spec.rb @@ -208,7 +208,7 @@ RSpec.describe Auth::SessionsController do context 'when using two-factor authentication' do context 'with OTP enabled as second factor' do let!(:user) do - Fabricate(:user, email: 'x@y.com', password: 'abcdefgh', otp_required_for_login: true, otp_secret: User.generate_otp_secret(32)) + Fabricate(:user, email: 'x@y.com', password: 'abcdefgh', otp_required_for_login: true, otp_secret: User.generate_otp_secret) end let!(:recovery_codes) do @@ -230,7 +230,7 @@ RSpec.describe Auth::SessionsController do context 'when using email and password after an unfinished log-in attempt to a 2FA-protected account' do let!(:other_user) do - Fabricate(:user, email: 'z@y.com', password: 'abcdefgh', otp_required_for_login: true, otp_secret: User.generate_otp_secret(32)) + Fabricate(:user, email: 'z@y.com', password: 'abcdefgh', otp_required_for_login: true, otp_secret: User.generate_otp_secret) end before do @@ -342,7 +342,7 @@ RSpec.describe Auth::SessionsController do context 'with WebAuthn and OTP enabled as second factor' do let!(:user) do - Fabricate(:user, email: 'x@y.com', password: 'abcdefgh', otp_required_for_login: true, otp_secret: User.generate_otp_secret(32)) + Fabricate(:user, email: 'x@y.com', password: 'abcdefgh', otp_required_for_login: true, otp_secret: User.generate_otp_secret) end let!(:webauthn_credential) do diff --git a/spec/requests/auth/sessions/security_key_options_spec.rb b/spec/requests/auth/sessions/security_key_options_spec.rb index 6246e4beb..e53b9802b 100644 --- a/spec/requests/auth/sessions/security_key_options_spec.rb +++ b/spec/requests/auth/sessions/security_key_options_spec.rb @@ -6,7 +6,7 @@ require 'webauthn/fake_client' RSpec.describe 'Security Key Options' do describe 'GET /auth/sessions/security_key_options' do let!(:user) do - Fabricate(:user, email: 'x@y.com', password: 'abcdefgh', otp_required_for_login: true, otp_secret: User.generate_otp_secret(32)) + Fabricate(:user, email: 'x@y.com', password: 'abcdefgh', otp_required_for_login: true, otp_secret: User.generate_otp_secret) end context 'with WebAuthn and OTP enabled as second factor' do diff --git a/spec/system/oauth_spec.rb b/spec/system/oauth_spec.rb index 0f96a5967..64ac75879 100644 --- a/spec/system/oauth_spec.rb +++ b/spec/system/oauth_spec.rb @@ -179,7 +179,7 @@ RSpec.describe 'Using OAuth from an external app' do end context 'when the user has set up TOTP' do - let(:user) { Fabricate(:user, email: email, password: password, otp_required_for_login: true, otp_secret: User.generate_otp_secret(32)) } + let(:user) { Fabricate(:user, email: email, password: password, otp_required_for_login: true, otp_secret: User.generate_otp_secret) } it 'when accepting the authorization request' do params = { client_id: client_app.uid, response_type: 'code', redirect_uri: client_app.redirect_uri, scope: 'read' }