Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions app/controllers/concerns/two_factor_authenticatable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ module TwoFactorAuthenticatable
NEED_AUTHENTICATION = 'need_two_factor_authentication'.freeze
OTP_LENGTH = 6
DIRECT_OTP_LENGTH = 6
PROOFING_DIRECT_OTP_LENGTH = 6
ALLOWED_OTP_DRIFT_SECONDS = 30
DIRECT_OTP_VALID_FOR_MINUTES = IdentityConfig.store.otp_valid_for
DIRECT_OTP_VALID_FOR_SECONDS = DIRECT_OTP_VALID_FOR_MINUTES * 60
Expand Down
4 changes: 2 additions & 2 deletions app/decorators/user_decorator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,9 @@ def password_reset_profile

def qrcode(otp_secret_key)
options = {
issuer: 'Login.gov',
issuer: APP_NAME,
otp_secret_key: otp_secret_key,
digits: TwoFactorAuthenticatable::DIRECT_OTP_LENGTH,
digits: TwoFactorAuthenticatable::OTP_LENGTH,
interval: IdentityConfig.store.totp_code_interval,
}
url = ROTP::TOTP.new(otp_secret_key, options).provisioning_uri(email)
Expand Down
6 changes: 5 additions & 1 deletion app/forms/otp_verification_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,11 @@ def valid_direct_otp_code?
end

def pattern_matching_otp_code_format
/\A[a-z0-9]{#{otp_code_length}}\z/i
if IdentityConfig.store.enable_numeric_authentication_otp
/\A[0-9]{#{otp_code_length}}\z/i
else
/\A[a-z0-9]{#{otp_code_length}}\z/i
end
end

def otp_code_length
Expand Down
9 changes: 8 additions & 1 deletion app/models/concerns/user_otp_methods.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,15 @@ def max_login_attempts?
end

def create_direct_otp
otp =
if IdentityConfig.store.enable_numeric_authentication_otp
OtpCodeGenerator.generate_digits(TwoFactorAuthenticatable::DIRECT_OTP_LENGTH)
else
OtpCodeGenerator.generate_alphanumeric_digits(TwoFactorAuthenticatable::DIRECT_OTP_LENGTH)
end

update(
direct_otp: OtpCodeGenerator.generate_digits(TwoFactorAuthenticatable::DIRECT_OTP_LENGTH),
direct_otp: otp,
direct_otp_sent_at: Time.zone.now,
)
end
Expand Down
2 changes: 1 addition & 1 deletion app/services/db/auth_app_configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ def self.authenticate(user, code)
def self.confirm(secret, code)
totp = ROTP::TOTP.new(
secret,
digits: TwoFactorAuthenticatable::DIRECT_OTP_LENGTH,
digits: TwoFactorAuthenticatable::OTP_LENGTH,
interval: IdentityConfig.store.totp_code_interval,
)
totp.verify(
Expand Down
4 changes: 3 additions & 1 deletion app/services/phone_confirmation/code_generator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@
module PhoneConfirmation
class CodeGenerator
def self.call
OtpCodeGenerator.generate_digits(TwoFactorAuthenticatable::DIRECT_OTP_LENGTH)
OtpCodeGenerator.generate_alphanumeric_digits(
TwoFactorAuthenticatable::PROOFING_DIRECT_OTP_LENGTH,
)
end
end
end
1 change: 1 addition & 0 deletions app/views/idv/otp_verification/show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
autofocus: true,
numeric: false,
class: 'width-full',
maxlength: TwoFactorAuthenticatable::PROOFING_DIRECT_OTP_LENGTH,
) %>
</div>
<%= submit_tag t('forms.buttons.submit.default'),
Expand Down
6 changes: 4 additions & 2 deletions app/views/shared/_one_time_code_input.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,15 @@ locals:
* transport: WebOTP transport method. Defaults to 'sms'.
* value: Field value. Defaults to `''`.
* class: CSS classes to add (optional)
* numeric: if the field should only accept digits. Defualts to true
* numeric: if the field should only accept digits. Defaults to true
* maxlength: Sets maxlength for the field. Defaults to TwoFactorAuthenticatable::DIRECT_OTP_LENGTH
%>
<% form = local_assigns.delete(:form)
name = local_assigns.delete(:name) { :code }
required = local_assigns.delete(:required) { false }
numeric = local_assigns.delete(:numeric) { true }
transport = local_assigns.delete(:transport) { 'sms' }
maxlength = local_assigns.delete(:maxlength) { TwoFactorAuthenticatable::DIRECT_OTP_LENGTH }
classes = ['field font-family-mono usa-input one-time-code-input']
classes << local_assigns.delete(:class) if local_assigns[:class] %>

Expand All @@ -27,7 +29,7 @@ locals:
input_html: {
'data-transport': transport,
pattern: numeric ? '[0-9]*' : '[a-zA-Z0-9]*',
maxlength: TwoFactorAuthenticatable::DIRECT_OTP_LENGTH,
maxlength: maxlength,
autocomplete: 'one-time-code',
inputmode: numeric ? 'numeric' : 'text',
**local_assigns,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@
value: @presenter.code_value,
required: true,
autofocus: true,
numeric: false,
numeric: IdentityConfig.store.enable_numeric_authentication_otp_input,
maxlength: TwoFactorAuthenticatable::DIRECT_OTP_LENGTH,
) %>
</div>
<%= f.input(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
autofocus: true,
size: 16,
class: 'width-auto',
maxlength: TwoFactorAuthenticatable::OTP_LENGTH,
) %>
<%= f.input(
:remember_device,
Expand Down
1 change: 1 addition & 0 deletions app/views/users/totp_setup/new.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
aria: { label: t('forms.totp_setup.totp_step_4') },
size: 18,
class: 'width-auto',
maxlength: TwoFactorAuthenticatable::OTP_LENGTH,
) %>
<% end %>
<% end %>
Expand Down
4 changes: 4 additions & 0 deletions config/application.yml.default
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ doc_capture_request_valid_for_minutes: 15
email_from: no-reply@login.gov
email_from_display_name: Login.gov
enable_load_testing_mode: false
enable_numeric_authentication_otp: true
enable_numeric_authentication_otp_input: true
enable_partner_api: false
enable_rate_limiting: true
enable_test_routes: true
Expand Down Expand Up @@ -367,6 +369,8 @@ production:
doc_auth_vendor_randomize_percent: 0
doc_auth_vendor_randomize_alternate_vendor: ''
domain_name: login.gov
enable_numeric_authentication_otp: false
enable_numeric_authentication_otp_input: false
enable_test_routes: false
enable_usps_verification: false
hmac_fingerprinter_key:
Expand Down
2 changes: 2 additions & 0 deletions lib/identity_config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,8 @@ def self.build_store(config_map)
config.add(:email_from, type: :string)
config.add(:email_from_display_name, type: :string)
config.add(:enable_load_testing_mode, type: :boolean)
config.add(:enable_numeric_authentication_otp, type: :boolean)
config.add(:enable_numeric_authentication_otp_input, type: :boolean)
config.add(:enable_partner_api, type: :boolean)
config.add(:enable_rate_limiting, type: :boolean)
config.add(:enable_test_routes, type: :boolean)
Expand Down
4 changes: 4 additions & 0 deletions lib/otp_code_generator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

class OtpCodeGenerator
def self.generate_digits(digits)
SecureRandom.random_number(10 ** digits).to_s.rjust(digits, '0')
end

def self.generate_alphanumeric_digits(digits)
ProfanityDetector.without_profanity do
# 5 bits per character means we must multiply what we want by 5
# :length adds zero padding in case it's a smaller number
Expand Down
28 changes: 27 additions & 1 deletion spec/forms/otp_verification_form_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,9 @@
end
end

context 'when the format of the code is not exactly 6 characters' do
context 'when alphanumeric is enabled and the code is not exactly 6 characters' do
it 'returns FormResponse with success: false' do
allow(IdentityConfig.store).to receive(:enable_numeric_authentication_otp).and_return(false)
user = build_stubbed(:user)
invalid_codes = [
'123abcd',
Expand All @@ -56,5 +57,30 @@
end
end
end

context 'when numeric is enabled and the code is not exactly 6 digits' do
it 'returns FormResponse with success: false' do
allow(IdentityConfig.store).to receive(:enable_numeric_authentication_otp).and_return(true)
user = build_stubbed(:user)
invalid_codes = [
'abcdef',
'12345a',
"aaaaa\n123456\naaaaaaaaa",
]

invalid_codes.each do |code|
form = OtpVerificationForm.new(user, code)
allow(user).to receive(:authenticate_direct_otp).with(code).and_return(true)

result = FormResponse.new(
success: false,
errors: {},
extra: { multi_factor_auth_method: 'otp_code' },
)

expect(form.submit).to eq(result), "expected #{code.inspect} to not pass"
end
end
end
end
end
18 changes: 15 additions & 3 deletions spec/lib/otp_code_generator_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,20 @@
OtpCodeGenerator.generate_digits(TwoFactorAuthenticatable::DIRECT_OTP_LENGTH)
end

it 'pads short strings with zeroes' do
expect(SecureRandom).to receive(:random_number).and_return(0)

expect(generate_digits).to eq('0' * TwoFactorAuthenticatable::DIRECT_OTP_LENGTH)
end
end

describe '.generate_alphanumeric_digits' do
subject(:generate_alphanumeric_digits) do
OtpCodeGenerator.generate_alphanumeric_digits(TwoFactorAuthenticatable::DIRECT_OTP_LENGTH)
end

it 'generates crockford-32 encoded words' do
expect(generate_digits).
expect(generate_alphanumeric_digits).
to match(/\A[a-z0-9]{#{TwoFactorAuthenticatable::DIRECT_OTP_LENGTH}}\Z/io)
end

Expand All @@ -19,13 +31,13 @@
Base32::Crockford.decode('ABCDE'),
)

expect(generate_digits).to eq('0ABCDE')
expect(generate_alphanumeric_digits).to eq('0ABCDE')
end

it 'pads short strings with zeroes' do
expect(SecureRandom).to receive(:random_number).and_return(0)

expect(generate_digits).to eq('0' * TwoFactorAuthenticatable::DIRECT_OTP_LENGTH)
expect(generate_alphanumeric_digits).to eq('0' * TwoFactorAuthenticatable::DIRECT_OTP_LENGTH)
end
end
end
4 changes: 3 additions & 1 deletion spec/services/phone_confirmation/confirmaton_session_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,9 @@
end

describe '#matches_code?' do
let(:code) { OtpCodeGenerator.generate_digits(TwoFactorAuthenticatable::DIRECT_OTP_LENGTH) }
let(:code) do
OtpCodeGenerator.generate_alphanumeric_digits(TwoFactorAuthenticatable::DIRECT_OTP_LENGTH)
end
subject do
described_class.new(
code: code,
Expand Down
21 changes: 21 additions & 0 deletions spec/views/shared/_one_time_code_input.html.erb_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -98,4 +98,25 @@
expect(rendered).to have_selector('[data-transport][data-foo="bar"]')
end
end

describe 'maxlength' do
context 'no maxlength given' do
it 'renders input maxlength DIRECT_OTP_LENGTH' do
puts rendered
expect(rendered).to have_selector(
"[maxlength=\"#{TwoFactorAuthenticatable::DIRECT_OTP_LENGTH}\"]",
)
end
end

context 'maxlength given' do
let(:params) { { maxlength: 10 } }

it 'renders input given maxlength' do
expect(rendered).to have_selector(
'[maxlength="10"]',
)
end
end
end
end