From 5d7682c7ddbf01dc0f6a4d244d215c2904b8e11b Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Wed, 25 Mar 2026 06:35:09 -0400 Subject: [PATCH] Extract `security_key_options` endpoint to standalone controller (#38367) --- .../security_key_options_controller.rb | 24 +++++++++ app/controllers/auth/sessions_controller.rb | 17 ------ config/routes.rb | 4 +- .../sessions/security_key_options_spec.rb | 54 +++++++++++++------ 4 files changed, 66 insertions(+), 33 deletions(-) create mode 100644 app/controllers/auth/sessions/security_key_options_controller.rb diff --git a/app/controllers/auth/sessions/security_key_options_controller.rb b/app/controllers/auth/sessions/security_key_options_controller.rb new file mode 100644 index 0000000000..1bd2b4043c --- /dev/null +++ b/app/controllers/auth/sessions/security_key_options_controller.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +class Auth::Sessions::SecurityKeyOptionsController < ApplicationController + skip_before_action :check_self_destruct! + skip_before_action :require_functional! + skip_before_action :update_user_sign_in + + def show + user = User.find_by(id: session[:attempt_user_id]) + + if user&.webauthn_enabled? + options_for_get = WebAuthn::Credential.options_for_get( + allow: user.webauthn_credentials.pluck(:external_id), + user_verification: 'discouraged' + ) + + session[:webauthn_challenge] = options_for_get.challenge + + render json: options_for_get, status: 200 + else + render json: { error: t('webauthn_credentials.not_enabled') }, status: 401 + end + end +end diff --git a/app/controllers/auth/sessions_controller.rb b/app/controllers/auth/sessions_controller.rb index 077f4d9db5..67bee2344e 100644 --- a/app/controllers/auth/sessions_controller.rb +++ b/app/controllers/auth/sessions_controller.rb @@ -38,23 +38,6 @@ class Auth::SessionsController < Devise::SessionsController flash.delete(:notice) end - def webauthn_options - user = User.find_by(id: session[:attempt_user_id]) - - if user&.webauthn_enabled? - options_for_get = WebAuthn::Credential.options_for_get( - allow: user.webauthn_credentials.pluck(:external_id), - user_verification: 'discouraged' - ) - - session[:webauthn_challenge] = options_for_get.challenge - - render json: options_for_get, status: 200 - else - render json: { error: t('webauthn_credentials.not_enabled') }, status: 401 - end - end - protected def find_user diff --git a/config/routes.rb b/config/routes.rb index bff3f9d034..1bc32a2861 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -76,8 +76,10 @@ Rails.application.routes.draw do namespace :auth do resource :setup, only: [:show, :update], controller: :setup resource :challenge, only: [:create] - get 'sessions/security_key_options', to: 'sessions#webauthn_options' post 'captcha_confirmation', to: 'confirmations#confirm_captcha', as: :captcha_confirmation + namespace :sessions do + resource :security_key_options, only: :show + end end end diff --git a/spec/requests/auth/sessions/security_key_options_spec.rb b/spec/requests/auth/sessions/security_key_options_spec.rb index e53b9802b4..17ec364a9b 100644 --- a/spec/requests/auth/sessions/security_key_options_spec.rb +++ b/spec/requests/auth/sessions/security_key_options_spec.rb @@ -5,9 +5,9 @@ require 'webauthn/fake_client' RSpec.describe 'Security Key Options' do describe 'GET /auth/sessions/security_key_options' do - let!(:user) do - Fabricate(:user, email: 'x@y.com', password: 'abcdefgh', otp_required_for_login: true, otp_secret: User.generate_otp_secret) - end + subject { get '/auth/sessions/security_key_options' } + + let!(:user) { Fabricate(:user, email: 'x@y.com', password: 'abcdefgh', otp_required_for_login: true, otp_secret: User.generate_otp_secret) } context 'with WebAuthn and OTP enabled as second factor' do let(:domain) { "#{Rails.configuration.x.use_https ? 'https' : 'http'}://#{Rails.configuration.x.web_domain}" } @@ -19,31 +19,55 @@ RSpec.describe 'Security Key Options' do user.update(webauthn_id: WebAuthn.generate_user_id) Fabricate( :webauthn_credential, - user_id: user.id, external_id: public_key_credential.id, - public_key: public_key_credential.public_key + public_key: public_key_credential.public_key, + user_id: user.id ) - post '/auth/sign_in', params: { user: { email: user.email, password: user.password } } end - it 'returns http success' do - get '/auth/sessions/security_key_options' + context 'when signed in' do + before { post '/auth/sign_in', params: { user: { email: user.email, password: user.password } } } - expect(response) - .to have_http_status 200 - expect(response.content_type) - .to start_with('application/json') + it 'returns http success' do + subject + + expect(response) + .to have_http_status 200 + expect(response.media_type) + .to eq('application/json') + expect(response.parsed_body) + .to include( + challenge: be_present, + userVerification: eq('discouraged'), + allowCredentials: contain_exactly(include(type: 'public-key', id: be_present)) + ) + end + end + + context 'when not signed in' do + it 'returns http unauthorized' do + subject + + expect(response) + .to have_http_status 401 + expect(response.media_type) + .to eq('application/json') + expect(response.parsed_body) + .to include(error: I18n.t('webauthn_credentials.not_enabled')) + end end end context 'when WebAuthn not enabled' do it 'returns http unauthorized' do - get '/auth/sessions/security_key_options' + subject expect(response) .to have_http_status 401 - expect(response.content_type) - .to start_with('application/json') + expect(response.media_type) + .to eq('application/json') + expect(response.parsed_body) + .to include(error: I18n.t('webauthn_credentials.not_enabled')) end end end