Skip to content

Commit 3c2fe80

Browse files
authored
Add 2FA/TOTP option to User accounts (#4286)
* Add 2FA/TOTP option to User accounts * Add CLI to disable 2FA for an account * Copilot suggested fixes * More secure sign_in_after_reset_password * more totp tests * Fix bad en-GB strings * Test updates & improvements
1 parent a70f3d4 commit 3c2fe80

File tree

26 files changed

+668
-24
lines changed

26 files changed

+668
-24
lines changed

Gemfile

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,7 @@ end
113113
gem "rollbar"
114114
gem "version", git: "https://github.com/pglombardo/version.git", branch: "master"
115115
gem "madmin"
116+
gem "rotp", "~> 6.2"
116117
gem "rqrcode", "~> 3.2"
117118
gem "turnout2024", require: "turnout"
118119
gem "mission_control-jobs", "~> 1.1.0"

Gemfile.lock

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -583,6 +583,7 @@ GEM
583583
retriable (3.4.1)
584584
rexml (3.4.4)
585585
rollbar (3.7.0)
586+
rotp (6.3.0)
586587
rqrcode (3.2.0)
587588
chunky_png (~> 1.0)
588589
rqrcode_core (~> 2.0)
@@ -830,6 +831,7 @@ DEPENDENCIES
830831
rails (~> 8.1.1)
831832
rails-i18n (~> 8.1.0)
832833
rollbar
834+
rotp (~> 6.2)
833835
rqrcode (~> 3.2)
834836
rubocop-rails-omakase
835837
ruby-debug-ide

app/controllers/concerns/pwpush/first_run.rb

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ def ensure_user_exists
1414

1515
if FirstRunBootCode.needed?
1616
unless Rails.env.test?
17-
if request.path.start_with?(first_run_path)
17+
if controller_path == "users/first_runs"
1818
boot_code = FirstRunBootCode.code
1919
puts <<~MESSAGE
2020
=======================================================================================
@@ -31,7 +31,9 @@ def ensure_user_exists
3131
MESSAGE
3232
end
3333
end
34-
return if request.path.start_with?(first_run_path)
34+
# Use controller_path, not request.path vs first_run_path: SetLocale default_url_options
35+
# (e.g. ?locale=en) can change generated paths and break start_with?, causing a redirect loop.
36+
return if controller_path == "users/first_runs"
3537

3638
redirect_to first_run_url
3739
end

app/controllers/users/passwords_controller.rb

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,4 +33,12 @@ class Users::PasswordsController < Devise::PasswordsController
3333
# def after_sending_reset_password_instructions_path_for(resource_name)
3434
# super(resource_name)
3535
# end
36+
37+
protected
38+
39+
def sign_in_after_reset_password?
40+
return false unless Devise.sign_in_after_reset_password
41+
42+
!resource.otp_required_for_login?
43+
end
3644
end

app/controllers/users/sessions_controller.rb

Lines changed: 53 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,15 @@
11
# frozen_string_literal: true
22

33
class Users::SessionsController < Devise::SessionsController
4+
include Devise::Controllers::Rememberable
5+
46
layout "login"
57

6-
before_action :reject_when_logins_disabled, only: [:new, :create]
8+
# Prepend so this runs before Devise::SessionsController#create (warden.authenticate! would
9+
# otherwise sign in with password only and bypass the OTP step).
10+
# Register reject last so it runs first (prepended callbacks run in reverse order).
11+
prepend_before_action :authenticate_with_two_factor, only: [:create]
12+
prepend_before_action :reject_when_logins_disabled, only: [:new, :create]
713

814
# before_action :configure_sign_in_params, only: [:create]
915

@@ -22,6 +28,52 @@ class Users::SessionsController < Devise::SessionsController
2228
# super
2329
# end
2430

31+
def authenticate_with_two_factor
32+
if sign_in_params[:email].present?
33+
self.resource = resource_class.find_for_database_authentication(
34+
email: sign_in_params[:email]
35+
)
36+
clear_otp_user_from_session
37+
start_two_factor_if_required if resource&.otp_required_for_login?
38+
elsif session[:otp_user_id].present?
39+
complete_two_factor_sign_in
40+
end
41+
end
42+
43+
def start_two_factor_if_required
44+
return unless resource.valid_password?(sign_in_params[:password])
45+
46+
session[:remember_me] = Devise::TRUE_VALUES.include?(sign_in_params[:remember_me])
47+
session[:otp_user_id] = resource.id
48+
render :otp, status: :unprocessable_content
49+
end
50+
51+
def complete_two_factor_sign_in
52+
self.resource = resource_class.find_by(id: session[:otp_user_id])
53+
unless resource
54+
clear_otp_user_from_session
55+
redirect_to new_user_session_path, alert: _("Session expired. Please sign in again.")
56+
return
57+
end
58+
59+
if resource.verify_and_consume_otp!(params[:otp_attempt])
60+
want_remember_me = session.delete(:remember_me)
61+
clear_otp_user_from_session
62+
remember_me(resource) if want_remember_me
63+
set_flash_message!(:notice, :signed_in)
64+
sign_in(resource, event: :authentication)
65+
respond_with resource, location: after_sign_in_path_for(resource)
66+
else
67+
flash.now[:alert] = _("Incorrect verification code.")
68+
render :otp, status: :unprocessable_content
69+
end
70+
end
71+
72+
def clear_otp_user_from_session
73+
session.delete(:otp_user_id)
74+
session.delete(:remember_me)
75+
end
76+
2577
# after_sign_out_path_for
2678
#
2779
# This method is called after the user has signed out.
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
# frozen_string_literal: true
2+
3+
class Users::TwoFactorController < ApplicationController
4+
before_action :authenticate_user!
5+
# show only redirects; do not generate secrets/codes on that hit.
6+
before_action :ensure_otp_secret, except: %i[destroy show]
7+
before_action :ensure_backup_codes, except: %i[destroy show]
8+
9+
def show
10+
redirect_to edit_user_registration_path
11+
end
12+
13+
def backup_codes
14+
end
15+
16+
def verify
17+
end
18+
19+
def create
20+
if current_user.verify_and_consume_otp!(params[:code])
21+
current_user.enable_totp!
22+
session.delete(:otp_backup_codes_plaintext)
23+
redirect_to edit_user_registration_path, notice: _("Two-factor authentication is now enabled.")
24+
else
25+
flash.now[:alert] = _("Incorrect verification code.")
26+
render :verify, status: :unprocessable_content
27+
end
28+
end
29+
30+
def destroy
31+
current_user.disable_totp!
32+
session.delete(:otp_backup_codes_plaintext)
33+
redirect_to edit_user_registration_path, status: :see_other, notice: _("Two-factor authentication has been disabled.")
34+
end
35+
36+
private
37+
38+
def ensure_otp_secret
39+
current_user.ensure_otp_secret!
40+
end
41+
42+
def ensure_backup_codes
43+
if Array(current_user.otp_backup_code_digests).empty?
44+
plaintexts = current_user.generate_otp_backup_codes!
45+
session[:otp_backup_codes_plaintext] = plaintexts
46+
end
47+
@otp_backup_codes_display = session[:otp_backup_codes_plaintext]
48+
end
49+
end

app/helpers/application_helper.rb

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,12 +51,14 @@ def secret_url(push, with_retrieval_step: true, locale: nil)
5151
# @param [String] url - The URL to generate the QR code for
5252
# @return [String] - The SVG QR code
5353
def qr_code(url)
54-
RQRCode::QRCode.new(url).as_svg(
54+
svg = RQRCode::QRCode.new(url).as_svg(
5555
offset: 0,
5656
color: :currentColor,
5757
shape_rendering: "crispEdges",
5858
module_size: 6,
5959
standalone: true
60-
).html_safe
60+
)
61+
# Strip XML declaration so the SVG is valid when embedded in HTML.
62+
svg.sub(/\A<\?xml[^>]*\?>\s*/, "").html_safe
6163
end
6264
end

app/madmin/resources/user_resource.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ class UserResource < Madmin::Resource
2323
# attribute :unconfirmed_email, index: false
2424
attribute :authentication_token, index: false, form: false
2525
attribute :preferred_language, index: false
26+
attribute :otp_required_for_login, index: true, form: false
2627

2728
# Associations
2829
attribute :pushes
Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
# frozen_string_literal: true
2+
3+
# Time-based one-time passwords (TOTP) for web sign-in, with backup recovery codes.
4+
# `last_otp_timestep` stores the Unix timestamp returned by ROTP on the last successful
5+
# TOTP verification (used as ROTP's `after:` to prevent token reuse).
6+
# `otp_secret` is encrypted at rest (Lockbox). Backup codes are stored as HMAC digests only;
7+
# plaintext codes are shown once via session (see Users::TwoFactorController).
8+
module User::TotpAuthentication
9+
extend ActiveSupport::Concern
10+
11+
included do
12+
has_encrypted :otp_secret
13+
14+
serialize :otp_backup_code_digests, coder: ActiveRecord::Coders::JSON.new, type: Array
15+
end
16+
17+
class_methods do
18+
# Used by tests and to build digests; same algorithm as instance digesting.
19+
def digest_otp_backup_code(user_id, code)
20+
OpenSSL::HMAC.hexdigest(
21+
"SHA256",
22+
Rails.application.secret_key_base,
23+
"#{user_id}:#{code.to_s.strip.downcase}"
24+
)
25+
end
26+
end
27+
28+
def totp_issuer
29+
Settings.brand&.title.presence || "Password Pusher"
30+
end
31+
32+
def ensure_otp_secret!
33+
return if otp_secret.present?
34+
35+
update!(otp_secret: ROTP::Base32.random)
36+
end
37+
38+
def enable_totp!
39+
update!(otp_required_for_login: true)
40+
end
41+
42+
def disable_totp!
43+
update!(
44+
otp_required_for_login: false,
45+
otp_secret: nil,
46+
otp_backup_code_digests: [],
47+
last_otp_timestep: nil
48+
)
49+
end
50+
51+
def totp
52+
raise ArgumentError, "otp_secret is blank" if otp_secret.blank?
53+
54+
ROTP::TOTP.new(otp_secret, issuer: totp_issuer)
55+
end
56+
57+
def totp_provisioning_uri
58+
totp.provisioning_uri(email)
59+
end
60+
61+
def totp_manual_entry_secret
62+
otp_secret
63+
end
64+
65+
# Verifies a 6-digit TOTP or a backup code, then consumes it (single-use).
66+
def verify_and_consume_otp!(code)
67+
return false if code.blank?
68+
69+
normalized = code.to_s.strip
70+
71+
if otp_secret.present?
72+
with_lock do
73+
reload # fresh last_otp_timestep; lock prevents concurrent TOTP replays
74+
token_time = totp.verify(normalized, after: last_otp_timestep, drift_behind: 15)
75+
if token_time
76+
update!(last_otp_timestep: token_time.to_i)
77+
return true
78+
end
79+
end
80+
end
81+
82+
consume_backup_code!(normalized)
83+
end
84+
85+
# Returns plaintext codes once; persists only digests. Caller should stash plaintext in session for display.
86+
def generate_otp_backup_codes!
87+
plaintexts = 16.times.map { SecureRandom.hex(5) }
88+
digests = plaintexts.map { |p| self.class.digest_otp_backup_code(id, p) }
89+
update!(otp_backup_code_digests: digests)
90+
plaintexts
91+
end
92+
93+
private
94+
95+
def consume_backup_code!(code)
96+
normalized = code.to_s.strip.downcase
97+
candidate = self.class.digest_otp_backup_code(id, normalized)
98+
digests = Array(otp_backup_code_digests)
99+
match_index = nil
100+
digests.each_with_index do |stored, i|
101+
next unless stored.bytesize == candidate.bytesize
102+
103+
if ActiveSupport::SecurityUtils.secure_compare(stored, candidate)
104+
match_index = i
105+
break
106+
end
107+
end
108+
return false unless match_index
109+
110+
digests.delete_at(match_index)
111+
update!(otp_backup_code_digests: digests)
112+
true
113+
end
114+
end

app/models/user.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
class User < ApplicationRecord
44
include Pwpush::TokenAuthentication
5+
include User::TotpAuthentication
56

67
# Include default devise modules. Others available are:
78
# :timeoutable and :omniauthable

0 commit comments

Comments
 (0)