Commit 39d07385 authored by David Lucadou's avatar David Lucadou

Tests for TOTP 2FA and fixes for 2FA TOS bypass

parent 282ab619
......@@ -22,7 +22,7 @@ All notable changes to this project will be documented in this file.
- Easy way to promote to admin or demote to user
- Easy way to disable PNG QR code generation via config file
- Error message when user executes blank basic search
- Gitlab CI for tests
- GitLab CI for tests
- Instructions for deploying with Nginx and auto-starting Puma on boot
- Logging all searches
- Logging report and search browsing
......
......@@ -103,12 +103,12 @@ class RegistrationsController < Devise::RegistrationsController
user.eula_accepted_at = DateTime.now
user.save!
flash[:notice] = I18n.t('.activerecord.errors.models.user.attributes.tos.option_accept')
if is_2fa_required(user)
redirect_to default_2fa_path(user)
else
#if is_2fa_required(user)
# redirect_to default_2fa_path(user)
#else
sign_in user
redirect_to root_path
end
#end
when "disagree-temp" # Disagree, but do not delete account
flash[:notice] = I18n.t('.activerecord.errors.models.user.attributes.tos.option_decline_keep')
user.eula_accepted_at = nil
......@@ -418,10 +418,24 @@ class RegistrationsController < Devise::RegistrationsController
end
def account_recovery_codes
user = current_user
@AccountRecoveryCodes = user.generate_otp_backup_codes!
user.otp_backup_codes_enabled_at = DateTime.now()
user.save!
if current_user.two_factor_enabled?
user = current_user
@AccountRecoveryCodes = user.generate_otp_backup_codes!
user.otp_backup_codes_enabled_at = DateTime.now()
user.save!
else
if current_user.otp_backup_codes
# Only clears recovery codes if no other 2FA methods are enabled
# and the user has recovery codes
current_user.otp_backup_codes = nil
current_user.otp_backup_codes_enabled_at = nil
current_user.save!
flash[:notice] = I18n.t('.activerecord.errors.models.user.attributes.totp.disabled_recovery_codes')
else
flash[:notice] = I18n.t('.activerecord.errors.models.user.attributes.totp.recovery_codes_ungeneratable')
end
redirect_to account_security_path
end
end
def account_security_settings
......
......@@ -90,7 +90,8 @@ en:
code_invalid_disable: "Invalid code; TOTP is still enabled. If you used the same code recently, please wait for the next code to appear on your device."
code_valid_enable: "TOTP enabled. Make sure to generate recovery codes so you don't get locked out of your account!"
disabled: "TOTP disabled."
disabled_recovery_codes: "Because you have no 2FA methods enabled, your account recovery codes have been disabled. To generate and use recovery codes, please enable a 2FA method."
disabled_recovery_codes: "Because you have no 2FA methods enabled, your account recovery codes have been disabled. To generate and use recovery codes, please enable a 2FA method."
recovery_codes_ungeneratable: "Because you have no 2FA methods enabled, you cannot generate account recovery codes. To generate and use recovery codes, please enable a 2FA method."
seed_reset_succeeded: "TOTP seed has been reset"
seed_timeout_reset: "Invalid code. Because you did not enter a correct code in time, the TOTP seed has been regenerated. Please enter a correct value for this new seed."
seed_timeout_setup: "Because you did not confirm the TOTP seed in time, it has expired and been regenerated. Please confirm the new TOTP seed."
......
This diff is collapsed.
module Account2FAHelpers
def enable_2fa(code: nil, navigate_to_page: true, expecting_before: nil, expecting_after: nil, submit_form: true)
if navigate_to_page
visit account_security_2fa_totp_path
end
expect(page).to have_content("Enable TOTP")
expect(page).to have_content("TOTP seed")
if expecting_before
expecting_before.each do |item|
expect(page).to have_content(item)
end
end
totp = nil
unless code
# I have to manually determine the TOTP code since user.current_otp
# is unavailable in tests
code = page.find('input#totp-seed')[:placeholder]
totp = ROTP::TOTP.new(code)
code = totp.now
end
fill_in "user_otp_attempt", with: code
if submit_form
click_on "Enable TOTP"
end
if expecting_after
expecting_after.each do |item|
expect(page).to have_content(item)
end
end
return totp
end
def disable_2fa(code: nil, navigate_to_page: true, expecting_before: nil, expecting_after: nil, submit_form: true)
if navigate_to_page
visit account_security_2fa_totp_path
end
expect(page).to have_content("Disable TOTP")
expect(page).to have_content("TOTP or Recovery Code")
if expecting_before
expecting_before.each do |item|
expect(page).to have_content(item)
end
end
if code.is_a? ROTP::TOTP
code = code.now
else
code = code.to_s
end
fill_in "user_otp_attempt", with: code
if submit_form
click_on "Disable TOTP"
end
if expecting_after
expecting_after.each do |item|
expect(page).to have_content(item)
end
end
end
def get_recovery_codes(navigate_to_page: true, expecting_before: nil, expecting_after: nil)
if navigate_to_page
visit account_security_2fa_recovery_codes_path
end
expect(page).to have_content("Account Recovery Codes")
expect(page).to have_content("Your account recovery codes:")
expect(page).to have_content("Generate new Codes")
if expecting_before
expecting_before.each do |item|
expect(page).to have_content(item)
end
end
recovery_codes = []
(0..4).each do |n|
recovery_codes << page.find("li#recovery-code-#{n}").text
end
if expecting_after
expecting_after.each do |item|
expect(page).to have_content(item)
end
end
return recovery_codes
end
end
......@@ -13,7 +13,7 @@ module AuthenticationHelpers
end
end
def sign_in_with(email, password, navigate_to_page: true, accept_tos: true, expecting_success: true, expecting_before: nil, expecting_after: nil)
def sign_in_with(email, password, totp: nil, navigate_to_page: true, accept_tos: true, expecting_success: true, expecting_before: nil, expecting_after: nil)
if navigate_to_page
visit new_user_session_path
end
......@@ -30,6 +30,23 @@ module AuthenticationHelpers
within(".actions") do
click_button "Sign in"
end
if totp # Supply TOTP code
if page.has_content?("Two-Factor Authentication") && page.has_content?("Authenticate via One-Time Password (TOTP) or Recovery Code")
if totp.is_a? ROTP::TOTP
fill_in "user_otp_attempt", with: totp.now
else
fill_in "user_otp_attempt", with: totp.to_s
end
click_on "Submit"
if expecting_success
expect(page).to have_content("Signed in with One-Time Password. If you used a recovery code, make sure to generate new codes so you don't run out!")
else
expect(page).to have_content("Invalid 2FA or recovery code, please try again")
end
end
end
if accept_tos # Accept the TOS if it is not already accepted
if page.has_content?("You must accept the Terms of Service before continuing.")
......@@ -37,7 +54,9 @@ module AuthenticationHelpers
expect(page).to have_content("Terms of Service")
click_on "Continue"
expect(page).to have_content("Thank you for accepting the terms of service!")
elsif expecting_success
elsif totp.nil? && expecting_success
expect(page).to have_content("Signed in with One-Time Password. If you used a recovery code, make sure to generate new codes so you don't run out!")
elsif totp.nil? && expecting_success
expect(page).to have_content("Signed in successfully")
end
end
......@@ -49,7 +68,7 @@ module AuthenticationHelpers
end
end
def switch_to_user(email, password, accept_tos: true, expecting_success: true, expecting_before: nil, expecting_after: nil)
def switch_to_user(email, password, totp: nil, accept_tos: true, expecting_success: true, expecting_before: nil, expecting_after: nil)
if expecting_before
expecting_before.each do |item|
expect(page).to have_content(item)
......@@ -57,7 +76,7 @@ module AuthenticationHelpers
end
sign_out_with
sign_in_with(email, password, accept_tos: accept_tos, expecting_success: expecting_success)
sign_in_with(email, password, totp: totp, accept_tos: accept_tos, expecting_success: expecting_success)
if expecting_after
expecting_after.each do |item|
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment