Merge pull request #15039 from opf/fix-relying-party

Fix WebAuthn relying party generation in auth controller

modules/two_factor_authentication/app/controllers/concerns/two_factor_authentication/webauthn_relying_party.rb

@@ -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

modules/two_factor_authentication/app/controllers/two_factor_authentication/authentication_controller.rb

@@ -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
@@ -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
@@ -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
 
@@ -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
@@ -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

modules/two_factor_authentication/app/controllers/two_factor_authentication/base_controller.rb

@@ -1,5 +1,7 @@
 module ::TwoFactorAuthentication
   class BaseController < ApplicationController
+    include ::TwoFactorAuthentication::WebauthnRelyingParty
+
     # Ensure 2FA authentication is enabled
     before_action :ensure_enabled_2fa
 
@@ -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
 
@@ -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
@@ -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
 
@@ -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
 
@@ -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
@@ -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
@@ -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." }
@@ -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

modules/two_factor_authentication/app/models/two_factor_authentication/device/webauthn.rb

@@ -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