Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix WebAuthn relying party generation in auth controller #15039

Merged
merged 2 commits into from
Mar 19, 2024
Merged

Fix WebAuthn relying party generation in auth controller #15039

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3cb6c4d
Extract relying party construction into a concern
klaustopher Mar 19, 2024
40501bd
Allowed devices should be an array
klaustopher Mar 19, 2024
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
19 changes: 19 additions & 0 deletions ...thentication/app/controllers/concerns/two_factor_authentication/webauthn_relying_party.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
module ::TwoFactorAuthentication
module WebauthnRelyingParty
extend ActiveSupport::Concern

protected

def webauthn_relying_party
@webauthn_relying_party ||= begin
origin = "#{Setting.protocol}://#{Setting.host_name}"

WebAuthn::RelyingParty.new(
origin:,
id: URI(origin).host,
name: Setting.app_title
)
end
end
end
end
19 changes: 7 additions & 12 deletions ...tor_authentication/app/controllers/two_factor_authentication/authentication_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@ class AuthenticationController < ApplicationController
include ::TwoFactorAuthentication::RememberToken
# Backup tokens functionality
include ::TwoFactorAuthentication::BackupCodes
# Webauthn relying party based on domain
include ::TwoFactorAuthentication::WebauthnRelyingParty
# Include global layout helper
layout 'no_menu'
layout "no_menu"

# User is not yet logged in, so skip login required check
skip_before_action :check_if_login_required
Expand All @@ -29,7 +31,7 @@ def request_otp
session[:authenticated_user_force_2fa] = service.needs_registration?

if service.needs_registration?
flash[:info] = I18n.t('two_factor_authentication.forced_registration.required_to_add_device')
flash[:info] = I18n.t("two_factor_authentication.forced_registration.required_to_add_device")
redirect_to new_forced_2fa_device_path
elsif !service.requires_token?
complete_stage_redirect
Expand Down Expand Up @@ -142,9 +144,9 @@ def render_login_otp(service)
@active_devices = @user.otp_devices.get_active

if params["back_url"]
render action: 'request_otp', back_url: params["back_url"]
render action: "request_otp", back_url: params["back_url"]
else
render action: 'request_otp'
render action: "request_otp"
end
end

Expand Down Expand Up @@ -206,7 +208,7 @@ def manager
# In case of mis-configuration, block all logins
def ensure_valid_configuration
if manager.invalid_configuration?
render_500 message: I18n.t('two_factor_authentication.error_is_enforced_not_active')
render_500 message: I18n.t("two_factor_authentication.error_is_enforced_not_active")
false
end
end
Expand All @@ -221,12 +223,5 @@ def complete_stage_redirect
def failure_stage_redirect
redirect_to authentication_stage_failure_path :two_factor_authentication
end

def webauthn_relying_party
@webauthn_relying_party ||= WebAuthn::RelyingParty.new(
origin: "#{Setting.protocol}://#{Setting.host_name}",
name: Setting.app_title
)
end
end
end
42 changes: 16 additions & 26 deletions ...es/two_factor_authentication/app/controllers/two_factor_authentication/base_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
module ::TwoFactorAuthentication
class BaseController < ApplicationController
include ::TwoFactorAuthentication::WebauthnRelyingParty

# Ensure 2FA authentication is enabled
before_action :ensure_enabled_2fa

Expand All @@ -10,17 +12,17 @@ class BaseController < ApplicationController

helper_method :optional_webauthn_challenge_url

layout 'no_menu'
layout "no_menu"

def new
if params[:type]
@device_type = params[:type].to_sym
@device = new_device_type! @device_type

render 'two_factor_authentication/two_factor_devices/new'
render "two_factor_authentication/two_factor_devices/new"
else
@available_devices = available_devices
render 'two_factor_authentication/two_factor_devices/new_type'
render "two_factor_authentication/two_factor_devices/new_type"
end
end

Expand All @@ -32,7 +34,7 @@ def make_default
if @device.make_default!
flash[:notice] = t(:notice_successful_update)
else
flash[:error] = t('two_factor_authentication.devices.make_default_failed')
flash[:error] = t("two_factor_authentication.devices.make_default_failed")
end

redirect_to index_path
Expand All @@ -42,14 +44,14 @@ def make_default
# Destroy the given device if its not the default
def destroy
if @device.default && strategy_manager.enforced?
render_400 message: t('two_factor_authentication.devices.is_default_cannot_delete')
render_400 message: t("two_factor_authentication.devices.is_default_cannot_delete")
return
end

if @device.destroy
flash[:notice] = t(:notice_successful_delete)
else
flash[:error] = t('two_factor_authentication.devices.failed_to_delete')
flash[:error] = t("two_factor_authentication.devices.failed_to_delete")
Rails.logger.error "Failed to delete #{@device.id} of user#{target_user.id}. Errors: #{@device.errors.full_messages.join(' ')}"
end

Expand Down Expand Up @@ -89,8 +91,8 @@ def request_device_confirmation_token
request_token_for_device(
@device,
confirm_path: url_for(action: :confirm, device_id: @device.id),
title: I18n.t('two_factor_authentication.devices.confirm_device'),
message: I18n.t('two_factor_authentication.devices.text_confirm_to_complete_html', identifier: @device.identifier)
title: I18n.t("two_factor_authentication.devices.confirm_device"),
message: I18n.t("two_factor_authentication.devices.text_confirm_to_complete_html", identifier: @device.identifier)
)
end

Expand All @@ -112,15 +114,15 @@ def validate_device_token
# rubocop:disable Metrics/AbcSize
def confirm_and_save(result)
if result.success? && @device.confirm_registration_and_save
flash[:notice] = t('two_factor_authentication.devices.registration_complete')
flash[:notice] = t("two_factor_authentication.devices.registration_complete")
true
elsif !result.success?
flash[:notice] = nil
flash[:error] = t('two_factor_authentication.devices.registration_failed_token_invalid')
flash[:error] = t("two_factor_authentication.devices.registration_failed_token_invalid")
false
else
flash[:notice] = nil
flash[:error] = t('two_factor_authentication.devices.registration_failed_update')
flash[:error] = t("two_factor_authentication.devices.registration_failed_update")
false
end
end
Expand All @@ -133,10 +135,10 @@ def request_token_for_device(device, locals)
flash[:notice] = transmit.result if transmit.result.present?

# Request confirmation from user as in the regular login flow
render 'two_factor_authentication/two_factor_devices/confirm', layout: 'base', locals:
render "two_factor_authentication/two_factor_devices/confirm", layout: "base", locals:
else
error = transmit.errors.full_messages.join(". ")
default_message = t('two_factor_authentication.devices.confirm_send_failed')
default_message = t("two_factor_authentication.devices.confirm_send_failed")
flash[:error] = "#{default_message} #{error}"

redirect_to registration_failure_path
Expand Down Expand Up @@ -195,18 +197,6 @@ def verify_webauthn_credential
false
end

def webauthn_relying_party
@webauthn_relying_party ||= begin
origin = "#{Setting.protocol}://#{Setting.host_name}"

WebAuthn::RelyingParty.new(
origin:,
id: URI(origin).host,
name: Setting.app_title
)
end
end

def logout_other_sessions
if current_user == target_user
Rails.logger.info { "First 2FA device registered for #{target_user}, terminating other logged in sessions." }
Expand Down Expand Up @@ -257,7 +247,7 @@ def show_local_breadcrumb
end

def default_breadcrumb
t('two_factor_authentication.label_devices')
t("two_factor_authentication.label_devices")
end

def available_devices
Expand Down
6 changes: 3 additions & 3 deletions modules/two_factor_authentication/app/models/two_factor_authentication/device/webauthn.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,14 @@ def options_for_create(relying_party)
@options_for_create ||= relying_party.options_for_registration(
user: { id: user.webauthn_id, name: user.name },
exclude: TwoFactorAuthentication::Device::Webauthn.where(user:).pluck(:webauthn_external_id),
authenticator_selection: { user_verification: 'discouraged' }
authenticator_selection: { user_verification: "discouraged" }
)
end

def options_for_get(relying_party)
@options_for_get ||= relying_party.options_for_authentication(
user_verification: 'discouraged', # we do not require user verification
allow: webauthn_external_id # TODO: Maybe also allow all other tokens? Let's see
user_verification: "discouraged", # we do not require user verification
allow: [webauthn_external_id] # TODO: Maybe also allow all other tokens? Let's see
)
end

Expand Down
Loading