Skip to content

Commit

Permalink
Unify initialization of MFA (#4757)
Browse files Browse the repository at this point in the history
* initialize_mfa
* prompt_mfa renders multifactor_auths/prompt
* require_mfa abstracts initialize_mfa + prompt_mfa
  • Loading branch information
martinemde authored Jun 4, 2024
1 parent 0ff5a07 commit 4ea30bc
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 45 deletions.
21 changes: 21 additions & 0 deletions app/controllers/concerns/require_mfa.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,27 @@
module RequireMfa
extend ActiveSupport::Concern

def require_mfa(user = @user)
return unless user&.mfa_enabled?
initialize_mfa(user)
prompt_mfa
end

# Call initialize_mfa once at the start of the MFA flow for a user (after login, after reset token verified).
def initialize_mfa(user = @user)
delete_mfa_session
create_new_mfa_expiry
session[:mfa_login_started_at] = Time.now.utc.to_s
session[:mfa_user] = user.id
end

def prompt_mfa(alert: nil, status: :ok)
@otp_verification_url = otp_verification_url
setup_webauthn_authentication form_url: webauthn_verification_url
flash.now.alert = alert if alert
render template: "multifactor_auths/prompt", status:
end

def otp_param
params.permit(:otp).fetch(:otp, "")
end
Expand Down
12 changes: 2 additions & 10 deletions app/controllers/email_confirmations_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ class EmailConfirmationsController < ApplicationController
before_action :redirect_to_new_mfa, if: :mfa_required_not_yet_enabled?, only: :unconfirmed
before_action :redirect_to_settings_strong_mfa_required, if: :mfa_required_weak_level_enabled?, only: :unconfirmed
before_action :validate_confirmation_token, only: %i[update otp_update webauthn_update]
before_action :require_mfa, only: %i[update]
before_action :validate_otp, only: :otp_update
before_action :validate_webauthn, only: :webauthn_update
after_action :delete_mfa_expiry_session, only: %i[otp_update webauthn_update]
Expand All @@ -27,16 +28,7 @@ def create
end

def update
if @user.mfa_enabled?
@otp_verification_url = otp_verification_url
setup_webauthn_authentication(form_url: webauthn_verification_url)

create_new_mfa_expiry

render template: "multifactor_auths/prompt"
else
confirm_email
end
confirm_email
end

def otp_update
Expand Down
12 changes: 3 additions & 9 deletions app/controllers/multifactor_auths_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ class MultifactorAuthsController < ApplicationController
before_action :require_mfa_enabled, only: %i[update otp_update]
before_action :require_totp_enabled, only: :destroy
before_action :seed_and_expire, only: :create
before_action :find_mfa_user, only: %i[otp_update webauthn_update]
before_action :find_mfa_user, only: %i[update otp_update webauthn_update]
before_action :validate_otp, only: %i[otp_update]
before_action :require_webauthn_enabled, only: %i[webauthn_update]
before_action :validate_webauthn, only: %i[webauthn_update]
Expand Down Expand Up @@ -44,15 +44,9 @@ def create
end

def update
initialize_mfa(@user)
session[:level] = level_param
@user = current_user

@otp_verification_url = otp_verification_url
setup_webauthn_authentication(form_url: webauthn_verification_url)

create_new_mfa_expiry

render template: "multifactor_auths/prompt"
prompt_mfa
end

def otp_update
Expand Down
16 changes: 4 additions & 12 deletions app/controllers/passwords_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ class PasswordsController < ApplicationController
before_action :ensure_email_present, only: %i[create]

before_action :validate_confirmation_token, only: %i[edit otp_edit webauthn_edit]
before_action :require_mfa, only: %i[edit]
before_action :validate_otp, only: %i[otp_edit]
before_action :validate_webauthn, only: %i[webauthn_edit]
after_action :delete_mfa_expiry_session, only: %i[otp_edit webauthn_edit]
Expand All @@ -17,18 +18,9 @@ def new
end

def edit
if @user.mfa_enabled?
@otp_verification_url = otp_verification_url
setup_webauthn_authentication(form_url: webauthn_verification_url)

create_new_mfa_expiry

render template: "multifactor_auths/prompt"
else
# When user doesn't have mfa, a valid token is a full "magic link" sign in.
verified_sign_in
render :edit
end
# When user doesn't have mfa, a valid token is a full "magic link" sign in.
verified_sign_in
render :edit
end

def create
Expand Down
18 changes: 4 additions & 14 deletions app/controllers/sessions_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,26 +10,16 @@ class SessionsController < Clearance::SessionsController
before_action :webauthn_new_setup, only: :new

before_action :ensure_not_blocked, only: %i[create]
before_action :find_user, only: %i[create]
before_action :require_mfa, only: %i[create]
before_action :find_mfa_user, only: %i[webauthn_create otp_create]
before_action :validate_otp, only: %i[otp_create]
before_action :validate_webauthn, only: %i[webauthn_create]
after_action :delete_mfa_session, only: %i[webauthn_create webauthn_full_create otp_create]
after_action :delete_session_verification, only: :destroy

def create
@user = find_user

if @user&.mfa_enabled?
@otp_verification_url = otp_verification_url
setup_webauthn_authentication(form_url: webauthn_verification_url)
session[:mfa_user] = @user.id
session[:mfa_login_started_at] = Time.now.utc.to_s
create_new_mfa_expiry

render "multifactor_auths/prompt"
else
do_login(two_factor_label: nil, two_factor_method: nil, authentication_method: "password")
end
do_login(two_factor_label: nil, two_factor_method: nil, authentication_method: "password")
end

def webauthn_create
Expand Down Expand Up @@ -125,7 +115,7 @@ def mfa_failure(message)

def find_user
password = params.permit(session: :password).require(:session).fetch(:password, nil)
User.authenticate(who, password) if password.is_a?(String) && who
@user = User.authenticate(who, password) if password.is_a?(String) && who
end

def find_mfa_user
Expand Down

0 comments on commit 4ea30bc

Please sign in to comment.