diff --git a/app/models/auth_app_configuration.rb b/app/models/auth_app_configuration.rb index 35c51ea70d6..5ebe6a7db68 100644 --- a/app/models/auth_app_configuration.rb +++ b/app/models/auth_app_configuration.rb @@ -2,12 +2,13 @@ class AuthAppConfiguration < ApplicationRecord include EncryptableAttribute + include UserSuppliedNameAttributes encrypted_attribute(name: :otp_secret_key) belongs_to :user - validates :name, presence: true + validates :name, presence: true, length: { maximum: UserSuppliedNameAttributes::MAX_NAME_LENGTH } def mfa_enabled? otp_secret_key.present? diff --git a/app/models/concerns/user_supplied_name_attributes.rb b/app/models/concerns/user_supplied_name_attributes.rb new file mode 100644 index 00000000000..eda881d6457 --- /dev/null +++ b/app/models/concerns/user_supplied_name_attributes.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +module UserSuppliedNameAttributes + MAX_NAME_LENGTH = 20 + + # In cases where the webauthn method is face or touch unlock, we override the name field + # with device and browser information that may be longer than other user supplied names. + WEBAUTHN_MAX_NAME_LENGTH = 80 +end diff --git a/app/models/piv_cac_configuration.rb b/app/models/piv_cac_configuration.rb index 89e5b5b5715..a22172636c5 100644 --- a/app/models/piv_cac_configuration.rb +++ b/app/models/piv_cac_configuration.rb @@ -1,9 +1,10 @@ # frozen_string_literal: true class PivCacConfiguration < ApplicationRecord - belongs_to :user + include UserSuppliedNameAttributes - validates :name, presence: true + belongs_to :user + validates :name, presence: true, length: { maximum: UserSuppliedNameAttributes::MAX_NAME_LENGTH } def mfa_enabled? x509_dn_uuid.present? diff --git a/app/models/webauthn_configuration.rb b/app/models/webauthn_configuration.rb index 7d0acdb2a3e..76056e44afe 100644 --- a/app/models/webauthn_configuration.rb +++ b/app/models/webauthn_configuration.rb @@ -1,8 +1,13 @@ # frozen_string_literal: true class WebauthnConfiguration < ApplicationRecord + include UserSuppliedNameAttributes + belongs_to :user - validates :name, presence: true + validates :name, presence: true, + length: { + maximum: UserSuppliedNameAttributes::WEBAUTHN_MAX_NAME_LENGTH, + } validates :credential_id, presence: true validates :credential_public_key, presence: true validate :valid_transports diff --git a/spec/controllers/users/totp_setup_controller_spec.rb b/spec/controllers/users/totp_setup_controller_spec.rb index 895406cd13f..f9ace18c0c3 100644 --- a/spec/controllers/users/totp_setup_controller_spec.rb +++ b/spec/controllers/users/totp_setup_controller_spec.rb @@ -82,7 +82,7 @@ end describe '#confirm' do - let(:name) { SecureRandom.hex } + let(:name) { SecureRandom.hex[0, 15] } let(:success) { false } before do diff --git a/spec/factories/auth_app_configurations.rb b/spec/factories/auth_app_configurations.rb index 8413405d69d..9cdaf83d6a7 100644 --- a/spec/factories/auth_app_configurations.rb +++ b/spec/factories/auth_app_configurations.rb @@ -2,7 +2,7 @@ Faker::Config.locale = :en factory :auth_app_configuration do - name { Faker::Lorem.unique.words.join(' ') } + name { Faker::Lorem.unique.words.join(' ')[0, UserSuppliedNameAttributes::MAX_NAME_LENGTH] } otp_secret_key { SecureRandom.hex(16) } user end diff --git a/spec/factories/piv_cac_configurations.rb b/spec/factories/piv_cac_configurations.rb index 2a7cea7ddb0..d5823625a72 100644 --- a/spec/factories/piv_cac_configurations.rb +++ b/spec/factories/piv_cac_configurations.rb @@ -2,7 +2,7 @@ Faker::Config.locale = :en factory :piv_cac_configuration do - name { Faker::Lorem.unique.words.join(' ') } + name { Faker::Lorem.unique.words.join(' ')[0, UserSuppliedNameAttributes::MAX_NAME_LENGTH] } x509_dn_uuid { Random.uuid } user end diff --git a/spec/forms/totp_setup_form_spec.rb b/spec/forms/totp_setup_form_spec.rb index 9a68f393bff..43c920a3e09 100644 --- a/spec/forms/totp_setup_form_spec.rb +++ b/spec/forms/totp_setup_form_spec.rb @@ -4,7 +4,7 @@ let(:user) { create(:user) } let(:secret) { user.generate_totp_secret } let(:code) { generate_totp_code(secret) } - let(:name) { SecureRandom.hex } + let(:name) { SecureRandom.hex[0, 19] } describe '#submit' do context 'when TOTP code is valid' do diff --git a/spec/models/auth_app_configuration_spec.rb b/spec/models/auth_app_configuration_spec.rb new file mode 100644 index 00000000000..c45333c1428 --- /dev/null +++ b/spec/models/auth_app_configuration_spec.rb @@ -0,0 +1,25 @@ +require 'rails_helper' + +RSpec.describe AuthAppConfiguration do + describe 'Associations' do + it { is_expected.to belong_to(:user) } + end + + describe 'name validations' do + context 'when a user supplies a name longer than max allowable characters' do + before do + subject.name = Faker::Lorem.characters( + number: UserSuppliedNameAttributes::MAX_NAME_LENGTH + 1, + ) + end + it { is_expected.not_to be_valid } + end + + context 'when a user supples a name with the max allowable character length' do + before do + subject.name = Faker::Lorem.characters(number: UserSuppliedNameAttributes::MAX_NAME_LENGTH) + end + it { is_expected.to be_valid } + end + end +end diff --git a/spec/models/piv_cac_configuration_spec.rb b/spec/models/piv_cac_configuration_spec.rb new file mode 100644 index 00000000000..50ee843f616 --- /dev/null +++ b/spec/models/piv_cac_configuration_spec.rb @@ -0,0 +1,25 @@ +require 'rails_helper' + +RSpec.describe PivCacConfiguration do + describe 'Associations' do + it { is_expected.to belong_to(:user) } + end + + describe 'name validations' do + context 'when a user supplies a name longer than max allowable characters' do + before do + subject.name = Faker::Lorem.characters( + number: UserSuppliedNameAttributes::MAX_NAME_LENGTH + 1, + ) + end + it { is_expected.not_to be_valid } + end + + context 'when a user supples a name with the max allowable character length' do + before do + subject.name = Faker::Lorem.characters(number: UserSuppliedNameAttributes::MAX_NAME_LENGTH) + end + it { is_expected.to be_valid } + end + end +end diff --git a/spec/models/webauthn_configuration_spec.rb b/spec/models/webauthn_configuration_spec.rb index 33d8cfcc037..a652f1e23d9 100644 --- a/spec/models/webauthn_configuration_spec.rb +++ b/spec/models/webauthn_configuration_spec.rb @@ -106,4 +106,28 @@ it { expect(subject).not_to be_valid } end end + + describe 'name validations' do + context 'when a user supplies a name longer than max allowable characters' do + before do + subject.name = Faker::Lorem.characters( + number: UserSuppliedNameAttributes::WEBAUTHN_MAX_NAME_LENGTH + 1, + ) + subject.credential_id = '111' + subject.credential_public_key = '222' + end + it { is_expected.not_to be_valid } + end + + context 'when a user supples a name with the max allowable character length' do + before do + subject.name = Faker::Lorem.characters( + number: UserSuppliedNameAttributes::WEBAUTHN_MAX_NAME_LENGTH, + ) + subject.credential_id = '111' + subject.credential_public_key = '222' + end + it { is_expected.to be_valid } + end + end end